[feat] Build and ship the mobile gh image (4/12) - #5683
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded mobile CI checks, multi-architecture image builds, smoke tests, optional Compose deployment at ChangesMobile CI and deployment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant Buildx
participant GHCR
participant MobileContainer
PullRequest->>GitHubActions: trigger mobile checks
GitHubActions->>GitHubActions: typecheck mobile packages and validate image metadata
GitHubActions->>Buildx: build amd64 and arm64 images
Buildx->>GHCR: publish images and architecture caches when enabled
GitHubActions->>MobileContainer: run mobile route and user smoke tests
GitHubActions->>GHCR: merge architecture images when enabled
sequenceDiagram
participant RunScript as run.sh
participant Compose
participant WebMobile as web-mobile
participant Traefik
RunScript->>Compose: enable with-web-mobile profile
Compose->>WebMobile: start mobile server on port 3000
Traefik->>WebMobile: route /m traffic without path stripping
RunScript->>Compose: include with-web-mobile during shutdown
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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: 6
🧹 Nitpick comments (1)
.github/workflows/17-check-mobile.yml (1)
44-44: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon each checkout.None of the three
actions/checkout@v6steps (lines 44, 73, 119) disable credential persistence. Static analysis (zizmor) flags this asartipacked: the job's git credentials remain on disk and can leak if a later step or a compromised action touches the working tree. Since no step in this workflow needsgit push, disable persistence on all three checkouts.🔒 Proposed fix
- uses: actions/checkout@v6 + with: + persist-credentials: false(apply the same
with:block at all three occurrences)Also applies to: 73-73, 119-119
Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ecd8f93a-d0d9-49cc-a5a3-a16793702491
📒 Files selected for processing (11)
.github/workflows/17-check-mobile.yml.gitignoredocs/design/agenta-mobile/README.mdhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.gh.examplehosting/docker-compose/run.sh
| | [plans/2026-07-26-wp5-device-gate.md](./plans/2026-07-26-wp5-device-gate.md) | WP5 device gate (flag-gated middleware, both directions) — **READY TO EXECUTE**; default-off, T8 banner-retirement deferred to flag-flip | | ||
|
|
||
| Wave-2 plans (WP2 auth + project drawer, WP3b mobile chat skin, WP4 product pages, WP5 device | ||
| gate) are **deliberately unwritten** — they must be planned against the real wave-1 code and the | ||
| finalized sessions surface. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the WP5 planning status consistent.
Line 39 marks the WP5 plan as READY TO EXECUTE. Lines 41-43 state that the WP5 plan is deliberately unwritten. Keep one status.
| ```bash | ||
| cd web && pnpm install | ||
| pnpm --filter @agenta/mobile lint # bans + tokens:check | ||
| pnpm --filter @agenta/mobile types:check | ||
| pnpm build-mobile && test -f web/../web/mobile/.next/standalone/mobile/server.js | ||
| pnpm dev-mobile # → http://localhost:3000/m, check light+dark |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the standalone-file check path.
After cd web on Line 163, Line 166 resolves web/../web/mobile/... to web/web/mobile/.... The check fails after a valid build.
Proposed fix
-pnpm build-mobile && test -f web/../web/mobile/.next/standalone/mobile/server.js
+pnpm build-mobile && test -f mobile/.next/standalone/mobile/server.js📝 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.
| ```bash | |
| cd web && pnpm install | |
| pnpm --filter @agenta/mobile lint # bans + tokens:check | |
| pnpm --filter @agenta/mobile types:check | |
| pnpm build-mobile && test -f web/../web/mobile/.next/standalone/mobile/server.js | |
| pnpm dev-mobile # → http://localhost:3000/m, check light+dark |
| - **Chores pending:** `.gitignore` allowlist entries for the `mobile-*` skills (they're tracked | ||
| via `git add -f`, matching repo precedent — new files inside them would be invisible to | ||
| `git status` until allowlisted); Fern regen so the FE `include_ended` param stops being a | ||
| runtime cast. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the completed .gitignore chore.
These lines mark the mobile skill allowlists as pending, but this change adds those allowlists in .gitignore Lines 103-105 and 130-132. Mark the chore complete or remove it.
2bdbdca to
1976012
Compare
85f8fd4 to
7edd737
Compare
1976012 to
a10ef27
Compare
7edd737 to
578f9fb
Compare
a10ef27 to
af8398c
Compare
578f9fb to
602ba19
Compare
602ba19 to
9629004
Compare
|
Both workflow findings were real and are fixed at the commit that introduced the file. Least privilege. Script injection. The four README findings are docs accuracy. I have applied the one that mattered outside this PR (a machine-specific absolute path in the WP3a plan, which leaked one developer's checkout into the repo); the remaining status/wording reconciliations are tracked but not yet applied. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/17-check-mobile.yml (1)
94-106: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate
TAGbefore publishing it as a job output.
INPUT_TAGis safe in this shell step, butTAGlater re-enters shell source through expressions such as Line 164. A dispatch value containing a quote and shell syntax can execute in thebuild-imagejob, which haspackages: write.Reject invalid Docker tags before writing
image_tagto$GITHUB_OUTPUT. This also rejects newline-based output injection.Proposed fix
else TAG="${INPUT_TAG:-manual-${SHA}}" PUSH="$INPUT_PUSH" + if ! [[ "$TAG" =~ ^[A-Za-z0-9_][A-Za-z0-9_.-]{0,127}$ ]]; then + echo "::error::image_tag is not a valid Docker tag" + exit 1 + fi REF="$(printf "%s" "${GITHUB_REF_NAME}" | tr '[:upper:]' '[:lower:]' | tr -cs 'a-z0-9._-' '-')"Source: Linters/SAST tools
docs/design/agenta-mobile/README.md (3)
39-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse one WP5 planning status.
Line 39 states that the WP5 plan is ready to execute. Lines 41-43 state that WP5 is deliberately unwritten. Update one statement so the runbook has one status.
162-167: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the standalone-file path.
After
cd web, the current path resolves toweb/web/mobile/.next/.... The verification command fails after a valid build.-pnpm build-mobile && test -f web/../web/mobile/.next/standalone/mobile/server.js +pnpm build-mobile && test -f mobile/.next/standalone/mobile/server.js
186-189: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the completed mobile-skill allowlist chore.
This PR adds the
.gitignoreallowlist entries formobile-*skills. Do not list that work as pending.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bb9a0966-ca0f-47ff-87ee-e001112dc9c8
📒 Files selected for processing (11)
.github/workflows/17-check-mobile.yml.gitignoredocs/design/agenta-mobile/README.mdhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.gh.examplehosting/docker-compose/run.sh
🚧 Files skipped from review as they are similar to previous changes (9)
- hosting/docker-compose/ee/env.ee.gh.example
- hosting/docker-compose/oss/docker-compose.gh.yml
- hosting/docker-compose/oss/env.oss.gh.example
- hosting/docker-compose/ee/docker-compose.gh.yml
- hosting/docker-compose/oss/docker-compose.gh.local.yml
- hosting/docker-compose/run.sh
- hosting/docker-compose/oss/docker-compose.gh.ssl.yml
- .gitignore
- hosting/docker-compose/ee/docker-compose.gh.local.yml
af8398c to
abe74aa
Compare
9629004 to
5fb3cb8
Compare
5fb3cb8 to
2f47ef7
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Assert the exact root status — real, fixed. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e8b8da7f-2348-431b-b4e1-f84dfe6d55b8
📒 Files selected for processing (11)
.github/workflows/17-check-mobile.yml.gitignoredocs/design/agenta-mobile/README.mdhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.gh.examplehosting/docker-compose/run.sh
🚧 Files skipped from review as they are similar to previous changes (9)
- hosting/docker-compose/ee/env.ee.gh.example
- hosting/docker-compose/oss/docker-compose.gh.ssl.yml
- hosting/docker-compose/oss/env.oss.gh.example
- hosting/docker-compose/run.sh
- hosting/docker-compose/oss/docker-compose.gh.yml
- hosting/docker-compose/ee/docker-compose.gh.local.yml
- hosting/docker-compose/ee/docker-compose.gh.yml
- hosting/docker-compose/oss/docker-compose.gh.local.yml
- .gitignore
mobile-app-structure, mobile-shadcn-conventions, and mobile-motion-patterns are already tracked via force-add; allowlist them so future files inside (e.g. references/) stay visible to git status.
Also runs types:check for @agenta/chat alongside @agenta/mobile in the typecheck job: verified locally (pnpm turbo run types:check --filter=@agenta/chat exits 0, pulling in @agenta/shared/entities/playground via turbo dependsOn).
Add an opt-in web-mobile service (with-web-mobile profile) to the five prod/gh compose files (oss gh/gh.ssl/gh.local, ee gh/gh.local), plus the AGENTA_WEB_MOBILE_IMAGE_NAME/TAG env-example entries. run.sh gains --with-mobile to activate the profile and reap it on shutdown.
The image smoke test asserts that the bare root 404s, because basePath /m owns the prefix. It did so by treating a successful `curl -sf` as the failure, but `curl -sf` fails on every 4xx and 5xx alike. A root that returned 500 read as a pass. Compare the status code itself, and dump the container log when it is anything other than 404.
…he /m segment Three findings from the same review, all in how this workflow and its compose siblings treat untrusted or ambiguous input. `image_tag` comes from workflow_dispatch and was written to GITHUB_OUTPUT unchecked, then interpolated into later run: blocks as an image name. It is now rejected unless it is a Docker tag. The check is a case statement rather than a grep because grep matches per line: a value containing a newline passes a line-based check and forges a second step output. The smoke test asserted curl's exit code, which cannot tell 404 from 500 and succeeds on a 3xx that never rendered a page. It now asserts each status directly: 200 for /m and /m/__env.js, 404 for the bare root. The five gh/ssl compose files routed the mobile app with PathPrefix(`/m`), which also matches /mobile, /metrics and anything else whose first two characters are /m, taking those paths from the web catch-all. They now use the segment-exact form the dev compose files already had.
6e2e450 to
854acef
Compare
2f47ef7 to
d395582
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bf364e75-36cd-475b-891b-76fcdaf0e343
📒 Files selected for processing (11)
.github/workflows/17-check-mobile.yml.gitignoredocs/design/agenta-mobile/README.mdhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.gh.examplehosting/docker-compose/run.sh
🚧 Files skipped from review as they are similar to previous changes (9)
- hosting/docker-compose/ee/env.ee.gh.example
- .gitignore
- hosting/docker-compose/oss/env.oss.gh.example
- hosting/docker-compose/oss/docker-compose.gh.ssl.yml
- hosting/docker-compose/ee/docker-compose.gh.yml
- hosting/docker-compose/run.sh
- hosting/docker-compose/oss/docker-compose.gh.local.yml
- hosting/docker-compose/oss/docker-compose.gh.yml
- hosting/docker-compose/ee/docker-compose.gh.local.yml
| - name: Smoke-test the image serves /m | ||
| run: | | ||
| IMAGE="ghcr.io/agenta-ai/agenta-web-mobile:${{ needs.prepare.outputs.image_tag }}-${{ matrix.arch }}" | ||
| if [ "${{ needs.prepare.outputs.push }}" = "true" ]; then | ||
| docker pull "$IMAGE" | ||
| fi | ||
| docker run -d --name mobile-smoke -p 3000:3000 "$IMAGE" | ||
| for i in $(seq 1 30); do | ||
| if curl -sf http://127.0.0.1:3000/m >/dev/null; then break; fi | ||
| sleep 1 | ||
| done | ||
| # Assert the status itself rather than curl's exit code: `-sf` succeeds on a 3xx | ||
| # (the page never rendered) and fails identically on 404 and 500. | ||
| expect_status() { | ||
| local path="$1" want="$2" got | ||
| got="$(curl -s -o /dev/null -w '%{http_code}' "http://127.0.0.1:3000${path}")" | ||
| if [ "$got" != "$want" ]; then | ||
| echo "::error::expected ${path} to return ${want}, got ${got}" | ||
| docker logs mobile-smoke | ||
| exit 1 | ||
| fi | ||
| } | ||
| expect_status /m 200 | ||
| expect_status /m/__env.js 200 | ||
| # basePath /m owns the prefix, so the bare root belongs to nothing. | ||
| expect_status / 404 | ||
| docker rm -f mobile-smoke |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound and handle smoke-test HTTP requests.
The curl calls at Line 184 and Line 191 have no connection or total timeout. If the container accepts a connection but does not respond, one request can block the job indefinitely.
At Line 191, a connection failure can terminate the script before docker logs mobile-smoke runs. Convert transport failures to a status value so the existing diagnostic path runs.
Proposed fix
docker run -d --name mobile-smoke -p 3000:3000 "$IMAGE"
for i in $(seq 1 30); do
- if curl -sf http://127.0.0.1:3000/m >/dev/null; then break; fi
+ if curl --connect-timeout 2 --max-time 5 -sf http://127.0.0.1:3000/m >/dev/null; then break; fi
sleep 1
done
# Assert the status itself rather than curl's exit code: `-sf` succeeds on a 3xx
# (the page never rendered) and fails identically on 404 and 500.
expect_status() {
local path="$1" want="$2" got
- got="$(curl -s -o /dev/null -w '%{http_code}' "http://127.0.0.1:3000${path}")"
+ if ! got="$(curl --connect-timeout 2 --max-time 5 -s -o /dev/null -w '%{http_code}' "http://127.0.0.1:3000${path}")"; then
+ got="000"
+ fi
if [ "$got" != "$want" ]; then📝 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: Smoke-test the image serves /m | |
| run: | | |
| IMAGE="ghcr.io/agenta-ai/agenta-web-mobile:${{ needs.prepare.outputs.image_tag }}-${{ matrix.arch }}" | |
| if [ "${{ needs.prepare.outputs.push }}" = "true" ]; then | |
| docker pull "$IMAGE" | |
| fi | |
| docker run -d --name mobile-smoke -p 3000:3000 "$IMAGE" | |
| for i in $(seq 1 30); do | |
| if curl -sf http://127.0.0.1:3000/m >/dev/null; then break; fi | |
| sleep 1 | |
| done | |
| # Assert the status itself rather than curl's exit code: `-sf` succeeds on a 3xx | |
| # (the page never rendered) and fails identically on 404 and 500. | |
| expect_status() { | |
| local path="$1" want="$2" got | |
| got="$(curl -s -o /dev/null -w '%{http_code}' "http://127.0.0.1:3000${path}")" | |
| if [ "$got" != "$want" ]; then | |
| echo "::error::expected ${path} to return ${want}, got ${got}" | |
| docker logs mobile-smoke | |
| exit 1 | |
| fi | |
| } | |
| expect_status /m 200 | |
| expect_status /m/__env.js 200 | |
| # basePath /m owns the prefix, so the bare root belongs to nothing. | |
| expect_status / 404 | |
| docker rm -f mobile-smoke | |
| - name: Smoke-test the image serves /m | |
| run: | | |
| IMAGE="ghcr.io/agenta-ai/agenta-web-mobile:${{ needs.prepare.outputs.image_tag }}-${{ matrix.arch }}" | |
| if [ "${{ needs.prepare.outputs.push }}" = "true" ]; then | |
| docker pull "$IMAGE" | |
| fi | |
| docker run -d --name mobile-smoke -p 3000:3000 "$IMAGE" | |
| for i in $(seq 1 30); do | |
| if curl --connect-timeout 2 --max-time 5 -sf http://127.0.0.1:3000/m >/dev/null; then break; fi | |
| sleep 1 | |
| done | |
| # Assert the status itself rather than curl's exit code: `-sf` succeeds on a 3xx | |
| # (the page never rendered) and fails identically on 404 and 500. | |
| expect_status() { | |
| local path="$1" want="$2" got | |
| if ! got="$(curl --connect-timeout 2 --max-time 5 -s -o /dev/null -w '%{http_code}' "http://127.0.0.1:3000${path}")"; then | |
| got="000" | |
| fi | |
| if [ "$got" != "$want" ]; then | |
| echo "::error::expected ${path} to return ${want}, got ${got}" | |
| docker logs mobile-smoke | |
| exit 1 | |
| fi | |
| } | |
| expect_status /m 200 | |
| expect_status /m/__env.js 200 | |
| # basePath /m owns the prefix, so the bare root belongs to nothing. | |
| expect_status / 404 | |
| docker rm -f mobile-smoke |
🧰 Tools
🪛 zizmor (1.28.0)
[info] 178-178: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 179-179: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
| Branch **`feat/agenta-mobile-wave-1`** (off `feat/sessions-continuity-fixes`, worktree | ||
| `.claude/worktrees/big-agents-sessions`). Not pushed. Executed subagent-driven: every task got a | ||
| fresh implementer + spec-compliance review + code-quality review; fix rounds were re-reviewed. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale local branch state from the committed handoff.
This file is committed in the current pull request, but Lines 47-49 still state that feat/agenta-mobile-wave-1 is “Not pushed” and reference a local worktree. Replace this with the current repository state or mark it as historical.
1 participant
Context
Lane 1 added the mobile app's production Dockerfile but deliberately left it unwired ("build wiring follows"). This lane connects it, so
/mcan actually ship in a gh image.Changes
A CI workflow runs the mobile typecheck and builds the gh image on changes under
web/mobile. Theweb-mobileservice joins the gh compose stacks for both editions. The mobile skills are allowlisted in.gitignore, which otherwise ignores everything under.claude/skills.Tests / notes
docs/design/agenta-mobile/plans/2026-07-25-wp1-infra-tail.md: the image builds before anything routes to it, so a half-deployed stack never serves a 502 at/m.