fix(a2ui-renderer): spell COPY sources from the root context the compose stanza declares - #2524
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
github-actions
Bot
added
compose
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a686b0d82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9924ace to
439eb99
Compare
The bottleneck stopped being discovery and became the ability to act. This is the artifact for acting: order and dependency as facts, checked against the tree, with no recommendation about what should land. "Ready" means nothing blocks it mechanically, not that it should merge. Every dependency was verified by tree lookup against origin/main and each PR head. No PR body was treated as evidence. The queue is 26 open PRs, not 13. MUST NOT MERGE AS-IS, at the top rather than buried: #2524 — base is feat/dockerfile-copy-anchor, not main. It deletes two entries from a baseline file that does not exist on main. Needs #2521, then retarget, then merge. #2526 — real failures on merge-decision and python-tests. Its purpose is to make python-tests capable of failing, so this may be the ratchet working; that has to be settled by its author, because merging it red turns a required check into permanent noise. #2517 — check-suit-release-notes fails. Not diagnosed. VERIFIED DEPENDENCIES (4): #2521 -> #2524 _known_copy_gaps.yaml absent on main #2514 -> #2515 JUICEFS_MEDIA_MINIO_REFORMAT_RUNBOOK.md absent on main, cited at jetson-combiner-archon-assignment:56 #2501 -> #2502 juicefs-cache-bounds.sh absent on main, cited twice in B850_BRINGBACK_RUNBOOK (:162, :213) #2519 -> future branch-protection-sync trigger work; #2519 adds the 5 missing `branch =` keys, without which the workflow's branch="main" fallback targets a branch 4 of those repos lack One assumed dependency corrected: #2502 -> #2501 is narrower than stated. juicefs-cross-node-setup.sh AND its make target already exist on main (mk/egress.mk:318); only juicefs-cache-bounds.sh is missing. The anchor ratchet would not have flagged it either way. COLLISION MAP: 7 files. Only one is a hard conflict — pmoves/Makefile, edited by #2521, #2523 and #2526 in the same ~20 lines around the validate-* block. #2523 should land first of the three: it is a 9-line deletion with no dependants, and rebasing two insertions onto a deletion is cheap while the reverse is not. Correction to the brief as requested: among currently open PRs only #2526 touches merge-gate.yml. The expected trim-2511 change is not open yet. Two facts that make red checks unreadable right now, stated up front: - `emit lifecycle trail` now fails on pull_request as well as push. Root cause pinned: the Archon commit our gitlink points at (1e02907ac) has seven nested gitlinks and no .gitmodules at all, so any recursive checkout dies at the first one. Archon's defect surfacing through our gitlink, not ours. Unowned. - merging needs an admin override: required_approving_review_count is 1 and the operator authors every PR, so self-approval is impossible. Six operator decisions listed separately from PR dependencies. Also disclosed: a gap in the COPY gate I shipped in #2521. It resolves COPY sources against compose build: contexts only, and never reads .github/workflows/integrations-ghcr.matrix.json — a second source of (dockerfile, context) pairs. Not theoretical: #2524's Dockerfile fix would have broken the a2ui-renderer GHCR publish, and a follow-up commit (8121e95) on that branch had to correct the matrix. The gate did not and could not catch it. Extending it to read the matrix is the follow-up. Anchor ratchet: 435 findings, 435 baselined, 0 new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bottleneck stopped being discovery and became the ability to act. This is the artifact for acting: order and dependency as facts, checked against the tree, with no recommendation about what should land. "Ready" means nothing blocks it mechanically, not that it should merge. Every dependency was verified by tree lookup against origin/main and each PR head. No PR body was treated as evidence. The queue is 26 open PRs, not 13. MUST NOT MERGE AS-IS, at the top rather than buried: #2524 — base is feat/dockerfile-copy-anchor, not main. It deletes two entries from a baseline file that does not exist on main. Needs #2521, then retarget, then merge. #2526 — real failures on merge-decision and python-tests. Its purpose is to make python-tests capable of failing, so this may be the ratchet working; that has to be settled by its author, because merging it red turns a required check into permanent noise. #2517 — check-suit-release-notes fails. Not diagnosed. VERIFIED DEPENDENCIES (4): #2521 -> #2524 _known_copy_gaps.yaml absent on main #2514 -> #2515 JUICEFS_MEDIA_MINIO_REFORMAT_RUNBOOK.md absent on main, cited at jetson-combiner-archon-assignment:56 #2501 -> #2502 juicefs-cache-bounds.sh absent on main, cited twice in B850_BRINGBACK_RUNBOOK (:162, :213) #2519 -> future branch-protection-sync trigger work; #2519 adds the 5 missing `branch =` keys, without which the workflow's branch="main" fallback targets a branch 4 of those repos lack One assumed dependency corrected: #2502 -> #2501 is narrower than stated. juicefs-cross-node-setup.sh AND its make target already exist on main (mk/egress.mk:318); only juicefs-cache-bounds.sh is missing. The anchor ratchet would not have flagged it either way. COLLISION MAP: 7 files. Only one is a hard conflict — pmoves/Makefile, edited by #2521, #2523 and #2526 in the same ~20 lines around the validate-* block. #2523 should land first of the three: it is a 9-line deletion with no dependants, and rebasing two insertions onto a deletion is cheap while the reverse is not. Correction to the brief as requested: among currently open PRs only #2526 touches merge-gate.yml. The expected trim-2511 change is not open yet. Two facts that make red checks unreadable right now, stated up front: - `emit lifecycle trail` now fails on pull_request as well as push. Root cause pinned: the Archon commit our gitlink points at (1e02907ac) has seven nested gitlinks and no .gitmodules at all, so any recursive checkout dies at the first one. Archon's defect surfacing through our gitlink, not ours. Unowned. - merging needs an admin override: required_approving_review_count is 1 and the operator authors every PR, so self-approval is impossible. Six operator decisions listed separately from PR dependencies. Also disclosed: a gap in the COPY gate I shipped in #2521. It resolves COPY sources against compose build: contexts only, and never reads .github/workflows/integrations-ghcr.matrix.json — a second source of (dockerfile, context) pairs. Not theoretical: #2524's Dockerfile fix would have broken the a2ui-renderer GHCR publish, and a follow-up commit (8121e95) on that branch had to correct the matrix. The gate did not and could not catch it. Extending it to read the matrix is the follow-up. Anchor ratchet: 435 findings, 435 baselined, 0 new. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
8121e95 to
f0c9510
Compare
…ose stanza declares The service has never been able to build. Both compose stanzas (docker-compose.yml:3730, docker-compose.core.yml:1439) declare `context: .` — which is `pmoves/` — while the Dockerfile copied `package.json` as if the context were the service directory. There is no `pmoves/package.json`, so docker died on the first COPY: > [builder 3/6] COPY package.json package-lock.json* ./: ERROR: failed to compute cache key: "/package.json": not found #2228 added that stanza specifically to make the renderer "a fleet capability rather than npm-scripts-only". The capability never worked. Fixed by mirroring evo-controller, the worked example of the same root-context pattern (`COPY services/evo-controller/requirements.txt .`): line 4 COPY services/a2ui-renderer/package.json services/a2ui-renderer/package-lock.json* ./ line 6 COPY services/a2ui-renderer/ . Line 6 was a bare `COPY . .`, which under a root context would have dropped the entire pmoves/ tree into /app — thousands of files, and `tsc --rootDir src` would not have found a src/ to compile even if npm ci had somehow passed. evo-controller solves the same problem the same way with `COPY services/evo-controller/ .`. Also added `**/node_modules` to pmoves/.dockerignore. The builder runs `npm ci` and THEN copies its source tree, so a developer's on-disk node_modules would land on top of the clean install and ship host-arch binaries into the image. Verified subtractive: no Dockerfile in this repo copies node_modules from a build context — the only occurrences are `--from=<stage>`, which does not read the context at all. Baseline dropped from 2 entries to 0. The gate's own stale-detection is what surfaced them, which is the contract working: BEFORE (fix applied, baseline untouched): COPY sources: 0 findings, 0 baselined, 0 new (19 not statically checkable) STALE COPY BASELINE — 2 entries no longer occur: COPY_UNRESOLVED|pmoves/services/a2ui-renderer/Dockerfile:4|pmoves|package-lock.json* COPY_UNRESOLVED|pmoves/services/a2ui-renderer/Dockerfile:4|pmoves|package.json exit=1 AFTER (baseline regenerated): COPY sources: 0 findings, 0 baselined, 0 new (19 not statically checkable) validate-dockerfile-paths: OK (91 dockerfiles, 80 referenced, 26 baseline) exit=0 Verified by building, not by satisfying the gate. `docker build` of the builder stage and of the full image both exit 0; npm ci and `tsc && remotion bundle` both succeed. The image runs: container reaches HEALTHCHECK status `healthy` and /healthz returns {"status":"healthy","service":"a2ui-renderer","version":"2.0.0","port":8107, "remotion":true,"nats":false}. (nats:false is expected for a bare docker run with no NATS_URL; the service treats it as non-fatal.) Stacked on feat/dockerfile-copy-anchor (#2521) because the baseline file this edits does not exist on main yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…file now assumes The COPY-path fix moved this Dockerfile to root-context spelling, which compose already declared (`context: .` = pmoves/). The GHCR matrix did not follow: it still selected `pmoves/services/a2ui-renderer`, and integrations-ghcr.yml passes that straight to Buildx as `context: integration-src/<context>`. Docker then resolves the new sources beneath the service dir — effectively pmoves/services/a2ui-renderer/services/a2ui-renderer/ — so a manual or `all` GHCR publish died on the first COPY while compose builds fine. Verified both directions, by building rather than by reading: $ docker build --target builder -f .../Dockerfile pmoves/services/a2ui-renderer ERROR: failed to compute cache key: "/services/a2ui-renderer": not found exit=1 $ docker build --target builder -f .../Dockerfile pmoves exit=0 `pmoves` is the existing house value for this matrix — the sibling a2ui-nats-bridge entry already uses it — so this aligns the two declared contexts rather than inventing a third. Swept the rest of the matrix while here: a2ui-renderer was the only one of 15 entries whose Dockerfile had a COPY source missing under its declared GHCR context, and it is now 0. Worth noting the ratchet in #2521 reads COMPOSE build contexts only; the GHCR matrix is a second, unchecked declaration of the same thing, which is why this drifted silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
f0c9510 to
7bde1c1
Compare
Docker Hardening ValidationHardening Validation ReportValidated: Wed Aug 12 11:55:49 UTC 2026Services CheckedPMOVES.AI Docker Hardening Validation[INFO] Checking: pmoves/docker-compose.hardened.yml [INFO] Validating: hi-rag-gateway-v2 [INFO] Validating: extract-worker [INFO] Validating: langextract [INFO] Validating: presign [INFO] Validating: render-webhook [INFO] Validating: retrieval-eval [INFO] Validating: pdf-ingest [INFO] Validating: jellyfin-bridge [INFO] Validating: invidious-companion-proxy [INFO] Validating: ffmpeg-whisper [INFO] Validating: media-video [INFO] Validating: media-audio [INFO] Validating: hi-rag-gateway-v2-gpu [INFO] Validating: hi-rag-gateway-gpu [INFO] Validating: deepresearch [INFO] Validating: supaserch [INFO] Validating: publisher-discord [INFO] Validating: mesh-agent [INFO] Validating: nats-echo-req [INFO] Validating: nats-echo-res [INFO] Validating: comfy-watcher [INFO] Validating: grayjay-plugin-host [INFO] Validating: agent-zero [INFO] Validating: p7-room-orchestrator [INFO] Validating: archon [INFO] Validating: channel-monitor [INFO] Validating: pmoves-yt [INFO] Validating: notebook-sync [INFO] Validating: supabase_service_role_key [INFO] Validating: supabase_jwt_secret [INFO] Validating: p7_control_token ====================================== |
1 participant
The service has never been able to build
Both compose stanzas —
docker-compose.yml:3730anddocker-compose.core.yml:1439— declarecontext: ., which ispmoves/. The Dockerfile copiedpackage.jsonas if the context were the service directory. There is nopmoves/package.json.Reproduced against
mainbefore touching anything:#2228 added that compose stanza specifically to make the renderer "a fleet capability rather than npm-scripts-only." The capability never worked.
The fix
Mirrors
evo-controller, the worked example of the same root-context pattern (COPY services/evo-controller/requirements.txt .):Line 6 mattered as much as line 4. A bare
COPY . .under a root context would have dropped the entirepmoves/tree into/app— andtsc --rootDir srcwould then have had nosrc/to compile even ifnpm cihad somehow passed.evo-controllersolves the same problem the same way withCOPY services/evo-controller/ ..Plus one guard
Added
**/node_modulestopmoves/.dockerignore. The builder runsnpm ciand then copies its source tree, so a developer's on-disknode_moduleswould land on top of the clean install and ship host-arch binaries into the image.Verified subtractive before adding it: no Dockerfile in this repo copies
node_modulesfrom a build context. Every occurrence is--from=<stage>, which doesn't read the context at all. So this shrinks the context for the 24 root-context builds and changes what none of them receive.The ratchet did its job
The gate's stale-detection is what surfaced the now-fixed entries — this is the contract from #2521 working on its first real fix, not a manual edit of the baseline.
Before (fix applied, baseline untouched):
After (baseline regenerated):
Baseline count 2 → 0. It only goes down.
Verified by building, not by satisfying the gate
A COPY fix that passes the gate and dies at
npm cihas only moved the failure, so I built it.docker build --target buildernpm ciclean,tsc && remotion bundleclean (⚡️ Cached bundle,+ /app/build)docker build(full, both stages)docker run+ HEALTHCHECKhealthyGET /healthz{"status":"healthy","service":"a2ui-renderer","version":"2.0.0","port":8107,"remotion":true,"nats":false}"nats":falseis expected for a baredocker runwith noNATS_URL; the service logsNATS connection failed (non-fatal)and serves anyway."remotion":trueis the load-bearing one — the Remotion bundle the build produces is present and loads.Built on the 4090 node (Docker 29.6.2, linux/x86_64). Both stages pull public base images and
npm cireaches the registry, so this is not an offline build.Gates
pytest pmoves/tools/tests/test_validate_dockerfile_paths.py— 20 passedpython pmoves/tools/validate_command_anchors.py— 435 findings, 435 baselined, 0 new🤖 Generated with Claude Code