[feat] Route phones to /m behind an off-by-default flag (5/12) - #5684
[feat] Route phones to /m behind an off-by-default flag (5/12)#5684ardaerzin wants to merge 13 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR documents the Agenta Mobile architecture and execution plans. It adds a shared mobile-gate decision core, desktop and mobile middleware, opt-in runtime configuration, mobile escape navigation, Compose wiring, and unit and Playwright coverage. ChangesMobile platform and device gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant DesktopMiddleware
participant MobileGate
participant MobileMiddleware
Browser->>DesktopMiddleware: Request a document route
DesktopMiddleware->>MobileGate: Evaluate desktop gate
MobileGate-->>DesktopMiddleware: Return redirect or continue
DesktopMiddleware-->>Browser: Return 307 or next response
Browser->>MobileMiddleware: Request a mobile route
MobileMiddleware->>MobileGate: Evaluate reverse gate
MobileGate-->>MobileMiddleware: Return redirect or continue
MobileMiddleware-->>Browser: Return 307 or mobile response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (4)
docs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.md (2)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the GitButler workflow when workspace mode is active.
This instruction runs raw
git branch --show-currentbefore it detects GitButler workspace mode. Usebutcommands for workspace detection and commits. Add the required lane assignment and post-commit file-scope verification.As per coding guidelines, use the
butCLI instead of rawgit branchorgit commitwhen GitButler workspace mode is active; assign exactly one lane's files and verify each commit withgit show --stat --name-only.Source: Coding guidelines
463-468: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun the required frontend lint fixer before each web commit.
The listed task sequences run tests, builds, or checks, but they omit
pnpm lint-fixfromweb. Add it before each commit and fix all reported errors.
docs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.md#L463-L468: runcd web && pnpm lint-fixbefore committing the shared package changes.docs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.md#L541-L546: runcd web && pnpm lint-fixbefore committing the OSS and EE middleware changes.docs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.md#L819-L824: runcd web && pnpm lint-fixbefore committing the mobile middleware and UI changes.docs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.md#L988-L993: runcd web && pnpm lint-fixbefore committing the Playwright TypeScript changes.As per coding guidelines, run
pnpm lint-fixfromwebbefore committing web changes.Source: Coding guidelines
web/oss/src/middleware.ts (1)
21-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing
Vary: User-Agentheader on gate redirects in both middleware twins.decideDesktopGatedecisions depend on theUser-Agentheader and cookies, but neitherweb/oss/src/middleware.tsnor its documented byte-identical twinweb/ee/src/middleware.tssets aVaryheader on theredirect/set-cookie-redirectresponses. A shared cache or CDN in front of either deployment could serve a cached redirect to the wrong device class.
web/oss/src/middleware.ts#L21-L45: addresponse.headers.set("Vary", "User-Agent, Cookie")on both theredirectandset-cookie-redirectbranches.web/ee/src/middleware.ts#L21-L45: apply the identical fix to keep the twin files byte-identical, as required by the file's own "TWIN NOTE" comment.docs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.md (1)
121-131: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExtend path filters when workspace dependencies are wired.
The trigger currently excludes
web/packages/**. When Task 7 adds@agenta/entities,@agenta/shared, or@agenta/chatimports, changes in those packages can change the mobile image without starting this workflow.Add the specific dependency paths, or a scoped
web/packages/**filter, when the Dockerfile and Turbo dependency wiring lands.Also applies to: 571-574
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b4e177e9-4308-43e8-ac09-b9952611442f
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
docs/design/agenta-mobile/README.mddocs/design/agenta-mobile/chat-headless-contract.mddocs/design/agenta-mobile/design.mddocs/design/agenta-mobile/plans/2026-07-12-wp0-sessions-query-and-stamping.mddocs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.mddocs/design/agenta-mobile/plans/2026-07-12-wp3a-chat-headless-core.mddocs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.mddocs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.mdhosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/oss/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/run.shweb/ee/src/middleware.tsweb/mobile/package.jsonweb/mobile/src/middleware.tsweb/mobile/src/pages/index.tsxweb/mobile/tests/unit/middleware.test.tsweb/mobile/vitest.config.tsweb/oss/src/middleware.tsweb/oss/tests/playwright/acceptance/mobile-gate/gate.spec.tsweb/packages/agenta-shared/package.jsonweb/packages/agenta-shared/src/utils/mobileGate/index.tsweb/packages/agenta-shared/tests/unit/mobileGate.test.tsweb/turbo.json
| useTurnRenderModel(message, ctx): RenderItem[] // lifted from AgentMessage.tsx:329-416 | ||
| useComposerAttachments({sessionId, limits}): {files, rejections, add, remove, clear, atMax, toParts} | ||
| useSessionHydration({sessionId}) | ||
| useApprovalDock({messages, onRespond}): {current, count, respond, approveAll, renderer} | ||
| useClientToolDispatch() | ||
| useAgentChatQueue(...) // exists — moves in | ||
| useAgentModelKeyStatus(...) // exists — moves in | ||
| useConversationScroll(ref, {messages, status}) // DESKTOP-ONLY opt-in |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(chat-headless-contract\.md|.*WP3.*|.*approval.*|.*Approvall.*|.*Approval.*)' || true
echo
echo "== target lines =="
if [ -f docs/design/agenta-mobile/chat-headless-contract.md ]; then
sed -n '1,130p' docs/design/agenta-mobile/chat-headless-contract.md | cat -n
fi
echo
echo "== search useApprovalDock =="
rg -n "useApprovalDock|approval|Approval|approveAll|renderer" -S . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -n 200
echo
echo "== search WP3 contract snippets =="
rg -n "function useApprovalDock|useApprovalDock\\(|approveAll|registry|renderer|render" docs/design/agenta-mobile -S . || trueRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== changed files if available =="
git diff --name-only -- docs/design/agenta-mobile/chat-headless-contract.md || true
echo
echo "== staged/unstaged diff around target =="
git diff -- docs/design/agenta-mobile/chat-headless-contract.md | sed -n '1,220p' || true
echo
echo "== locate markdown by exact filename variants =="
fd -a 'chat-headless-contract\.md$' . || trueRepository: Agenta-AI/agenta
Length of output: 350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target document relevant sections =="
sed -n '80,112p' docs/design/agenta-mobile/chat-headless-contract.md | cat -n
echo
echo "== WP3 plan approval hook sections =="
sed -n '680,705p' docs/design/agenta-mobile/plans/2026-07-12-wp3a-chat-headless-core.md | cat -n
echo
echo "== WP3 plan approval registry section =="
sed -n '705,740p' docs/design/agenta-mobile/plans/2026-07-12-wp3a-chat-headless-core.md | cat -n
echo
echo "== README summary references =="
sed -n '120,135p' docs/design/agenta-mobile/README.md | cat -nRepository: Agenta-AI/agenta
Length of output: 11550
Remove renderer from the useApprovalDock contract.
useApprovalDock is documented to return only current, count, respond, and approveAll; approval body rendering is resolved through the approval registry and approved copy overrides. Return the registry-entry values from resolveApprovalBody, or update the contract and WP3a task contract consistently before implementation.
| **Files** | ||
| - Modify: `api/oss/src/core/sessions/service.py` (`query_sessions` — after fetching streams, | ||
| batch-fetch the latest turn per session via `SessionTurnsDAO` (one query, | ||
| `DISTINCT ON (session_id) ... ORDER BY session_id, turn_index DESC` or the DAO's existing | ||
| latest-turn helper from the turn-index fix `9613e7964e`) and attach `references` (+ | ||
| `trace_id` if cheap) to each row) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Require a batch latest-turn query for R3.
R3 requires one query for all session IDs, but this step permits the existing latest_turn helper. The audit states that helper is per-session and that no batch helper exists. Using it makes /sessions/query an N+1 path. Require latest_turn_per_session(session_ids) with DISTINCT ON, and keep the one-call assertion in the service test.
| - [ ] `cd web && pnpm turbo run lint --filter=@agenta/mobile --dry=json | python3 -c "import json,sys; d=json.load(sys.stdin); print([t['taskId'] for t in d['tasks']])"` → includes `@agenta/mobile#lint` (proves `pnpm run lint` in `11-check-code-styling.yml:96` reaches mobile, tokens:check chained). | ||
| - [ ] `cd web && pnpm -r --filter=!agenta-web-tests --if-present run test:unit --reporter=dot 2>&1 | grep -i chat` → `@agenta/chat` suite executes (the exact Phase-1 command from `run-tests.ts:200`); confirm `web/packages/agenta-chat/test-results/junit.xml` exists afterwards (matches the publish glob `12:133-135`). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve failures from the verification commands.
Lines 515-516 pipe Turbo and unit-test output into Python or grep without pipefail. A failed producer can therefore produce a successful overall command.
Add set -o pipefail before these pipelines, or capture and check the producer exit status separately.
🧰 Tools
🪛 LanguageTool
[locale-violation] ~516-~516: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...nta-chat/test-results/junit.xmlexists afterwards (matches the publish glob12:133-135`)...
(AFTERWARDS_US)
| docker run -d --name mobile-smoke -p 3000:3000 agenta-web-mobile:smoke | ||
| sleep 3 | ||
| curl -sf -o /dev/null -w '%{http_code}\n' http://127.0.0.1:3000/m # expect: 200 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Replace the fixed three-second wait with a readiness loop.
Line 536 uses sleep 3 before checking the server. A slower runner can fail before the entrypoint and Next.js server finish starting.
Reuse the bounded polling loop from the workflow. Add an exit trap to remove mobile-smoke after failures.
| } | ||
| ``` | ||
|
|
||
| - [ ] Run: `cd web && pnpm --filter @agenta/shared test:unit -- mobileGate` — expect **25 passed**. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the expected unit-test count.
web/packages/agenta-shared/tests/unit/mobileGate.test.ts defines 27 tests. This step says to expect 25 passed. Update the expected count.
| Opt-in in dev too: `run.sh --dev --with-mobile` (originally it rode `with-web` and auto-started, | ||
| but a live dev run showed the second Next dev server pushes an 8GB Docker VM into OOM-killing | ||
| the main web app's first big Turbopack compile — dmesg-confirmed `next-server` kills at ~4.5GB | ||
| RSS. Running both dev servers comfortably wants a 12GB+ VM). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the documented Compose entrypoint path.
The command does not state a working directory. Use hosting/docker-compose/run.sh and specify the required OSS or EE edition with the dev and mobile options.
As per coding guidelines, run the local dev stack through hosting/docker-compose/run.sh and use the same edition and image mode as the loaded environment.
Source: Coding guidelines
| ### WP5 device gate — EXECUTED (2026-07-26, 5 commits) | ||
|
|
||
| Ships the mobile device gate (design.md "Gate and routing") behind a runtime flag, | ||
| `AGENTA_MOBILE_GATE`, **default off**: with the flag off, request behavior is byte-identical to | ||
| today. `NoMobilePageWrapper` retirement (T8) is specified but deliberately **not executed** — | ||
| it ships only in the deployment window where the flag is actually flipped on. See | ||
| [plans/2026-07-26-wp5-device-gate.md](./plans/2026-07-26-wp5-device-gate.md) for the full task | ||
| breakdown and grounding facts. | ||
|
|
||
| | Commit | Content | | ||
| |--------|---------| | ||
| | `cf272e1227` | T1: `@agenta/shared/utils/mobileGate` — pure, framework-free decision core (detection, deep-link maps, cookie semantics, documented exceptions); 27 unit tests in the package's vitest harness | | ||
| | `6b3aaf5654` | T2: `web/oss/src/middleware.ts` + `web/ee/src/middleware.ts` — twin desktop forward-gate adapters (byte-identical, both NEW files) wrapping the shared core | | ||
| | `2df5e4d2a1` | T3: `web/mobile/src/middleware.ts` — reverse-gate middleware carrying a declared verbatim copy of the reverse-gate subset (mobile has zero workspace deps until WP2), new minimal mobile vitest harness, and the "View desktop site" `?view=desktop` escape link on the placeholder page | | ||
| | `7be0b8d528` | T4: plumb `AGENTA_MOBILE_GATE` through dev + gh compose files (default `false`), documented in both dev env examples | | ||
| | `9ac651525f` | T6: self-skipping Playwright UA-emulation smoke (`web/oss/tests/playwright/acceptance/mobile-gate/gate.spec.ts`), 6 tests, skips unless the runner asserts `AGENTA_MOBILE_GATE=true` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the WP5 commit record.
The heading says five commits and the table lists five commits. The section also claims T1-T7 executed and later refers to a six-commit review. Add the T7 documentation commit and update the count, or correct the execution claim.
Also applies to: 182-182
85f8fd4 to
7edd737
Compare
45fa66f to
5243f24
Compare
7edd737 to
578f9fb
Compare
5243f24 to
7b97468
Compare
578f9fb to
602ba19
Compare
7b97468 to
2ac567c
Compare
602ba19 to
9629004
Compare
2ac567c to
e9e9e65
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/design/agenta-mobile/README.md (2)
41-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove executed WP5 work from the unwritten wave-2 list.
This file records WP5 T1-T7 as executed at Lines 154-169, but these sections still list WP5 as unwritten work. Keep WP2, WP3b, and WP4 in the wave-2 planning list. State that only T8 remains deferred.
Also applies to: 221-225
231-236: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake sequential shell snippets independent of the current working directory.
Both snippets change directories and then use paths that assume the repository root. Use subshells, reset to the repository root, or use paths relative to the current directory.
docs/design/agenta-mobile/README.md#L231-L236: checkmobile/.next/standalone/mobile/server.jsafter enteringweb.docs/design/agenta-mobile/plans/2026-07-12-wp3a-chat-headless-core.md#L39-L49: wrap each verification command in(cd ... && ...)or change directory once.
♻️ Duplicate comments (3)
docs/design/agenta-mobile/plans/2026-07-12-wp0-sessions-query-and-stamping.md (1)
3-15: 📐 Maintainability & Code Quality | 🟠 MajorReconcile the execution status in both plans.
Each header claims completion while later text records unfinished or unverified work.
docs/design/agenta-mobile/plans/2026-07-12-wp0-sessions-query-and-stamping.md#L3-L15: Keep one authoritative state for the four tasks.docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md#L3-L13: Mark Phase 6 unverified, or verify the image build before claiming all phases are complete.docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md (2)
787-793: 🎯 Functional Correctness | 🟠 MajorUse the executed dark-theme role-map values.
The header says dark
accentmust usep.scales.zinc[2].darkand darkdestructive-foregroundmust usep.componentsDark.Button.primaryColor. The map still usesp.surface.controlItemBgActive.darkandp.surface.white.dark, which generates incorrect dark tokens.
930-935: 🎯 Functional Correctness | 🟡 MinorUse paths relative to the command working directory.
Both verification blocks change directory and then retain repository-relative prefixes.
docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md#L930-L935: Aftercd web/mobile, usesrc/components/uiandpackage.json.docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md#L1095-L1100: Aftercd web, usemobile/.next/..., or run the checks from the repository root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d098db38-1061-46ef-a31b-4c754dd726da
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
docs/design/agenta-mobile/README.mddocs/design/agenta-mobile/chat-headless-contract.mddocs/design/agenta-mobile/design.mddocs/design/agenta-mobile/plans/2026-07-12-wp0-sessions-query-and-stamping.mddocs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.mddocs/design/agenta-mobile/plans/2026-07-12-wp3a-chat-headless-core.mddocs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.mddocs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.mdhosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/oss/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/run.shweb/ee/src/middleware.tsweb/mobile/package.jsonweb/mobile/src/middleware.tsweb/mobile/src/pages/index.tsxweb/mobile/tests/unit/middleware.test.tsweb/mobile/vitest.config.tsweb/oss/src/middleware.tsweb/oss/tests/playwright/acceptance/mobile-gate/gate.spec.tsweb/packages/agenta-shared/package.jsonweb/packages/agenta-shared/src/utils/mobileGate/index.tsweb/packages/agenta-shared/tests/unit/mobileGate.test.tsweb/turbo.json
🚧 Files skipped from review as they are similar to previous changes (20)
- hosting/docker-compose/oss/env.oss.dev.example
- web/packages/agenta-shared/package.json
- hosting/docker-compose/ee/docker-compose.gh.yml
- web/turbo.json
- hosting/docker-compose/oss/docker-compose.gh.yml
- web/mobile/src/pages/index.tsx
- web/oss/tests/playwright/acceptance/mobile-gate/gate.spec.ts
- web/mobile/package.json
- web/mobile/vitest.config.ts
- web/oss/src/middleware.ts
- web/packages/agenta-shared/tests/unit/mobileGate.test.ts
- hosting/docker-compose/run.sh
- hosting/docker-compose/ee/env.ee.dev.example
- web/ee/src/middleware.ts
- hosting/docker-compose/oss/docker-compose.dev.yml
- web/mobile/tests/unit/middleware.test.ts
- web/mobile/src/middleware.ts
- hosting/docker-compose/ee/docker-compose.dev.yml
- web/packages/agenta-shared/src/utils/mobileGate/index.ts
- docs/design/agenta-mobile/plans/2026-07-26-wp5-device-gate.md
| - Escape hatch both ways: a `agenta-mobile-optout` / `agenta-mobile-optin` cookie set by | ||
| "View desktop site" / "Open mobile version" links; middleware honors it. This also fixes the | ||
| current banner's dismissal-not-persisted annoyance. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== design file slice =="
sed -n '90,125p' docs/design/agenta-mobile/design.md 2>/dev/null || true
echo
echo "== relevant mentions =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'agenta-mobile-(optin|optout)|View desktop site|Open mobile version|mobile-gate|mobile gate|query parameter|queryParams|preference|cookie' \
. | head -n 200Repository: Agenta-AI/agenta
Length of output: 26895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== shared mobileGate implementation =="
sed -n '1,190p' web/packages/agenta-shared/src/utils/mobileGate/index.ts 2>/dev/null || true
echo
echo "== OSS middleware route/contract =="
sed -n '1,70p' web/oss/src/middleware.ts 2>/dev/null || true
echo
echo "== design section around Rules with context =="
sed -n '96,120p' docs/design/agenta-mobile/design.md 2>/dev/null || true
echo
echo "== deterministic query precedence probe from source text =="
python3 - <<'PY'
from pathlib import Path
p = Path('web/packages/agenta-shared/src/utils/mobileGate/index.ts')
s = p.read_text()
print('MOBILE_OPTIN_COOKIE=', 'MOBILE_OPTIN_COOKIE' in s)
print('MOBILE_OPTOUT_COOKIE=', 'MOBILE_OPTOUT_COOKIE' in s)
for needle in ['input.cookie(MOBILE_OPTIN_COOKIE)', 'input.cookie(MOBILE_OPTOUT_COOKIE)', '?view=mobile', '?view=desktop']:
print(f'contains {needle!r}:', needle in s)
print('cookie preference checks precede query settings:', s.index('input.cookie(MOBILE_OPTIN_COOKIE)') < s.index('?view=mobile'))
PYRepository: Agenta-AI/agenta
Length of output: 11156
Document the mobile-gate query parameters and precedence.
decideDesktopGate and decideMobileGate accept ?view=desktop / ?view=mobile, strip them after setting the corresponding cookie, and existing cookies are honored before query-based redirect decisions. Update this section to name view and state that a matching cookie overrides the query parameter.
| - Escape hatch both ways: a `agenta-mobile-optout` / `agenta-mobile-optin` cookie set by | ||
| "View desktop site" / "Open mobile version" links; middleware honors it. This also fixes the | ||
| current banner's dismissal-not-persisted annoyance. | ||
| - `NoMobilePageWrapper` is retired once the gate ships. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align wrapper retirement with the flag-flip runbook.
The design says to retire NoMobilePageWrapper when the gate ships. The current rollout keeps T8 unexecuted and retires the wrapper only after AGENTA_MOBILE_GATE=true, WP2, and WP4 are ready. Update this condition to prevent premature removal.
| ### Residual gaps (the revised WP0 scope) | ||
|
|
||
| 1. **`updated_at` ordering/cursor** — the query DAO windows on `id` (uuid7 ≈ creation order); | ||
| the FE sorts client-side per page, which breaks "last-activity" ordering across pages for | ||
| infinite scroll. Add `updated_at` support to `apply_windowing` + switch the sessions query | ||
| to it. | ||
| 2. **Title search** — `SessionQuery` has no free-text filter; `name` is now a real column, so | ||
| this is a plain `ilike`. | ||
| 3. **References echo on list rows** — `/sessions/query` rows do not carry the session's agent | ||
| references, but the mobile list must label each row with its agent and resolve | ||
| continue-session without N per-session turn lookups. Hydrate latest-turn references onto the | ||
| response rows (the service already joins turns for filtering). | ||
| 4. **Runtime-shape zod test for `querySessions`** — the wrapper + schema exist but the drift- | ||
| pinning unit test was never written (the false-green tsc class). | ||
| 5. Optional, deferrable: liveness-flags filter on the root query (mobile can filter client-side | ||
| from the returned `flags`); Fern regen so `include_ended` stops being a runtime cast. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mark the WP0 residuals as completed or historical.
The README states that WP0 is complete and that updated ordering, search, references, and the zod wire test are implemented. This file still presents those items as open gaps and active WP0 scope. Move them to an implemented-history section or update their status.
Also applies to: 367-367
| """apply_windowing must support `updated_at` as the order/cursor attribute. | ||
|
|
||
| The sessions list is ordered by last activity (`updated_at` is heartbeat-fed on | ||
| session_streams). Both ORDER BY and the keyset cursor filters must ride updated_at — | ||
| ordering by updated_at while cursor-filtering on another column paginates incorrectly. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep R1 NULL-safe in both implementation and tests.
The execution note requires coalesce(updated_at, created_at), but these steps still use plain updated_at. Because updated_at is nullable, descending PostgreSQL ordering places null rows first.
The test also rejects created_at in the full SQL statement. select(SessionStreamDBE) includes that column, and the correct ordering intentionally uses it inside coalesce.
Update the implementation and cursor instructions to use the same coalesced expression. Inspect the WHERE and ORDER BY fragments separately, and reject only bare created_at references.
Also applies to: 122-139, 148-149
| - [ ] In `web/turbo.json`, add these three task entries inside `"tasks"` (place after `"@agenta/ee#build"`; the generic `dev` task already covers `pnpm dev-mobile`): | ||
| ```json | ||
| "@agenta/mobile#build": { | ||
| "inputs": [ | ||
| "src/**", | ||
| "public/**", | ||
| "next.config.ts", | ||
| "postcss.config.mjs", | ||
| "tsconfig.json", | ||
| "components.json" | ||
| ], | ||
| "outputs": [".next/**", "!.next/cache/**"], | ||
| "env": ["NODE_ENV", "NEXT_PUBLIC_*"] | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg '(^|/)(package.json|turbo.json|pnpm-lock.yaml)$|2026-07-12-wp1-mobile-foundation\.md$' || true
echo
echo "== target plan excerpt =="
if [ -f docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md ]; then
wc -l docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md
sed -n '330,375p' docs/design/agenta-mobile/plans/2026-07-12-wp1-mobile-foundation.md
fi
echo
echo "== existing web/turbo.json =="
if [ -f web/turbo.json ]; then
wc -l web/turbo.json
sed -n '1,220p' web/turbo.json
fi
echo
echo "== package manifests and lockfile package entry examples =="
for f in package.json web/package.json mobile/package.json apps/mobile/package.json packages/mobile/package.json pnpm-lock.yaml; do
if [ -f "$f" ]; then
echo "-- $f --"
sed -n '1,220p' "$f"
fi
done
echo
echo "== turbo defaults reference in docs/code =="
rg -n '"$TURBO_DEFAULT$"|TURBO_DEFAULT|array\[.*package\.json|`@agenta/mobile`#build|turbo.*inputs' -S .Repository: Agenta-AI/agenta
Length of output: 17760
🌐 Web query:
Turbo $TURBO_DEFAULT$ default package.json inputs documentation
💡 Result:
In Turborepo, the inputs key in your turbo.json file defines the file glob patterns used to calculate a task's hash for caching purposes [1][2]. When you define an inputs array for a task, you effectively opt out of Turborepo's default behavior, which is to include all files in the package that are tracked by Git [3][1]. Because defining inputs disables this default behavior (and causes Turborepo to stop respecting your .gitignore for that specific task), you may need to restore the default behavior while still applying custom modifications [1][2]. The $TURBO_DEFAULT$ microsyntax is used to re-introduce those default inputs into your custom list [3][4]. Common Usage: You can use $TURBO_DEFAULT$ to build upon the default set of inputs, such as when you want to ignore a specific file that doesn't affect the task's output [1][2]. Example: { "tasks": { "build": { "inputs": ["$TURBO_DEFAULT$", "!README.md"] } } } In this example, Turborepo includes all tracked files (the default behavior) but ignores changes to the README.md file, allowing the task to hit the cache even if the README is modified [1][2]. Important Notes: 1. Default Inputs: By default, if no inputs key is provided, all files in the package that are tracked by Git are included [1][2]. 2. Mandatory Inputs: Certain files, such as package.json, turbo.json, and package manager lockfiles, are always considered inputs, even if you attempt to exclude them [3][5][6]. 3. Interaction with global.inputs: If futureFlags.globalConfiguration is enabled, files specified in global.inputs are automatically prepended to every task's inputs [4][7].
Citations:
- 1: https://turborepo.dev/docs/crafting-your-repository/configuring-tasks
- 2: https://turborepo.dev/en/docs/crafting-your-repository/configuring-tasks
- 3: https://turborepo.dev/docs/reference/configuration
- 4: https://github.com/vercel/turbo/blob/main/skills/turborepo/references/configuration/tasks.md
- 5: https://v2-8-6.turborepo.dev/docs/reference/configuration
- 6: https://v2-7-6-canary-0.turborepo.dev/docs/reference/configuration
- 7: https://github.com/vercel/turborepo/blob/main/skills/turborepo/references/configuration/tasks.md
Keep package.json in the mobile build cache key.
Defining inputs overrides Turbo’s default file selection, so @agenta/mobile#build only hashes the fields currently listed. Add $TURBO_DEFAULT$ with a !tests/** ignore, or list web/mobile/package.json and any other manifest that affects the build.
| - name: Determine build metadata | ||
| id: meta | ||
| run: | | ||
| PR="${{ github.event.pull_request.number }}" | ||
| INPUT_TAG="${{ inputs.image_tag }}" | ||
| SHA="$(git rev-parse --short HEAD)" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not interpolate inputs.image_tag into shell source.
Line 197 inserts a workflow-dispatch input directly into INPUT_TAG="...". A value containing shell syntax can execute commands in the prepare job. Pass dynamic values through env: and validate the tag before using it.
Proposed fix
- name: Determine build metadata
id: meta
+ env:
+ INPUT_TAG: ${{ inputs.image_tag }}
+ PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
- PR="${{ github.event.pull_request.number }}"
- INPUT_TAG="${{ inputs.image_tag }}"
+ PR="$PR_NUMBER"
SHA="$(git rev-parse --short HEAD)"
+
+ if [[ -n "$INPUT_TAG" && ! "$INPUT_TAG" =~ ^[A-Za-z0-9._-]+$ ]]; then
+ echo "::error::image_tag contains invalid characters"
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Determine build metadata | |
| id: meta | |
| run: | | |
| PR="${{ github.event.pull_request.number }}" | |
| INPUT_TAG="${{ inputs.image_tag }}" | |
| SHA="$(git rev-parse --short HEAD)" | |
| - name: Determine build metadata | |
| id: meta | |
| env: | |
| INPUT_TAG: ${{ inputs.image_tag }} | |
| PR_NUMBER: ${{ github.event.pull_request.number }} | |
| run: | | |
| PR="$PR_NUMBER" | |
| SHA="$(git rev-parse --short HEAD)" | |
| if [[ -n "$INPUT_TAG" && ! "$INPUT_TAG" =~ ^[A-Za-z0-9._-]+$ ]]; then | |
| echo "::error::image_tag contains invalid characters" | |
| exit 1 | |
| fi |
9629004 to
5fb3cb8
Compare
e9e9e65 to
89fe72f
Compare
5fb3cb8 to
2f47ef7
Compare
89fe72f to
bfad8e0
Compare
…iddleware sandbox
2f47ef7 to
d395582
Compare
bfad8e0 to
29f6858
Compare
1 participant
Context
With a mobile app at
/m, a phone that lands on the desktop app should be able to get to it, and a laptop that lands on/mshould be able to get back. Neither should happen without the deployment asking for it: self-hosters who do not want a mobile surface must be unaffected.Changes
The decision is a pure function in
@agenta/shared(mobileGate), with three thin adapters: OSS middleware, EE middleware, and the mobile app's reverse gate. Keeping the decision separate from the adapters is what makes it testable without a request.It is off by default. The middleware returns immediately unless the deployment sets
AGENTA_MOBILE_GATE=true, read at request time so it can be flipped on a running standalone server. An?view=desktop/?view=mobileescape hatch sets a long-lived cookie either way, and an OAuth callback is never bounced mid-flow (that would drop the one-time code).Tests / notes
/mwith no trailing slash) needed an explicit"/"matcher entry, and the flag needed mirroring into a dev env file for the middleware sandbox.What to QA
AGENTA_MOBILE_GATEunset, open/on a phone. Nothing redirects. This is the default and the one that matters for self-hosters.true, then open/on a phone. You land on/m. Tap "View desktop site" and you stay on desktop across reloads.