fix(#6612): make the pinned Claude Code the one that runs in the sandbox - #6647
Conversation
Local check of the new step (rootless Podman on Fedora, against the pinned
|
PR Summary by QodoEnsure sandbox runs the pinned Claude Code version
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding |
|
🤖 Finished Review · ✅ Success · Started 2:31 PM UTC · Completed 2:49 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.22 |
Site previewPreview: https://23eeefeb-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsTier 1 signals unchanged from prior assessment. Containerfile remains a high-churn hotspot (24 commits/30d, 5 authors, 6 fix/revert in 90d) and issue carries security+priority/high labels, but these were already factored into the prior score of 2. Good test coverage, no dependency or CI workflow changes, and well-scoped bug fix continue to mitigate risk. Previous runRisk Assessment: moderate (2/5) DetailsSmall, focused bug fix (3 files, 386 lines) with high churn on Containerfile (24 commits/30d, 6 authors, 14 fix/revert commits in 90d), elevated by security+priority/high issue labels, mitigated by good test coverage (new test file, TEST_FILE_RATIO=0.33), no dependency or CI workflow changes, and well-scoped changes matching issue requirements. Previous run (2)Risk Assessment: moderate (2/5) DetailsSmall, focused bug fix (3 files, 71 lines) to ensure the pinned Claude Code version takes precedence over the base image's pre-installed version. Elevated by high churn on Containerfile (24 commits in 30d, 6 authors, 14 fix/revert commits in 90d) and security/priority-high issue labels. Mitigated by good test coverage (new test file verifying version pin), small blast radius, and well-scoped changes matching issue requirements. Overall moderate risk. |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
|
Full-image verification (Fedora 44, rootless Podman 5.8.4, linux/amd64 — run by the pi-worker session; mac-mini repeat pending)Build: In-image, identical as root and as Negative check: Agent run through the new image: not completed — host-environmental, not a PR defect. Model matrix on Vertex (the fleet's Vertex project,
|
afea9b1 to
3beaecb
Compare
|
🤖 Finished Review · ✅ Success · Started 6:17 PM UTC · Completed 6:36 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.56 |
The OpenShell base image bundles its own Claude Code at /usr/local/bin/claude, installed upstream with the unpinned `curl claude.ai/install.sh`, so it is whatever was current when the base was built (2.1.156 on the pinned base). /usr/local/bin precedes npm's global bin (/usr/bin) on the sandbox PATH, so the npm install pinned by CLAUDE_CODE_VERSION was never the `claude` that ran: every Renovate bump of the pin has been a runtime no-op, and model aliases resolved from 2.1.156's table (`sonnet` -> claude-sonnet-4-5@20250929 on Vertex while the pi runtime maps the same alias to claude-sonnet-4-6). Every fleet run's `Agent: ... (v2.1.156)` line shows it. Point /usr/local/bin/claude at the npm install right after it, and assert at build time that `claude --version` on PATH reports CLAUDE_CODE_VERSION, so a base-image change cannot silently bring the shadow back. fullsend-code inherits the fix from this image. Add a Containerfile guard test in the style of the pi image guards, and note in the runtime implementation guide that the pin is what runs only because of this step. Signed-off-by: Wayne Sun <gsun@redhat.com>
The implementer's page carried its facts in a handful of 3-4 KB single-paragraph bullets and a matrix whose three PostToolUse rows repeated identical cells. Same content, rearranged so it can be scanned: - an "On this page" list and an explicit "Adding a runtime" checklist - the security matrix split into host-side controls, sandbox tool hooks (with the shared PostToolUse chain described once) and bootstrap/ artifacts - the sandbox hook contract split into files/wiring, tool-name vocabulary (diagnostics as a table), wire protocol (table), sanitizer scope (per stage), hook fail modes (table), environment (table), suppression reachability and the numbered Claude Code caveats - a new "Pinned runtime binaries in the sandbox image" section holding the Claude Code pin/shadow rule from #6612 next to the pi and extension pins and what to re-check on each bump - pi runtime internals split into posture, unattended operation, process/exit codes, agent definition translation (table), hook adapter contract, the two Vertex extensions (project/region/auth as a table), nested-binary exposure, not-yet-exercised items, other clouds Anchors other pages link to (#sandbox-hook-contract, issue and ADR reference from the previous version is still present. Signed-off-by: Wayne Sun <gsun@redhat.com>
3beaecb to
66d724d
Compare
|
🤖 Finished Retro · ✅ Success · Started 7:58 PM UTC · Completed 8:16 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.41 |
Retro: PR #6647 — make the pinned Claude Code the one that runs in the sandboxPR: #6647 by waynesun09 (human-authored, Workflow timeline
Total agent cost: ~$18.63 ($1.41 code + $0.73 triage + $16.49 review across 3 runs). Code agent failure: staged correct changes but never committedThe code agent run (32898527242) correctly understood the issue, created branch Ironically, the code agent itself was running on A proposal is filed below for enforcing Review agent: good quality, known operational issuesQuality was strong. Findings were accurate and appropriately low-severity: protected-path flag for Known operational issues observed — all well-covered by existing open issues:
Overall assessmentThe PR was well-crafted with thorough author self-testing (local builds, negative tests, Vertex model-alias matrix). Review quality was good. The main workflow gap is the code agent's failure to commit its correct changes — a single missing Proposals filed
|
Investigating the #6647 behaviour-test failures showed both were misreported by the harness-wait timeout path, not caused by the deadline or the artifact lookup: - Attempt 1: the harness runs existed within 2s of the label, but the pool-org installation token was rate limited (403) for the rest of the wait. listHarnessRunsAfter turned every listing error into an empty list, so the timeout said "no recent workflow runs found". Client-side retries also stretched the 12-minute wait to 26 minutes. - Attempt 2: dispatch declined the event (collaborator permission lookup failed for the labelling actor on a freshly recreated pool repo), the harness matrix never expanded, and no artifact was ever uploaded. The timeout showed a "successful" run with no explanation. Record listing errors during the wait instead of swallowing them, bound each poll's API calls to the remaining wait budget, and make the timeout diagnostics state the agent job's state per run (detecting the unexpanded matrix), classify listed artifacts against the trigger time, and report the recorded errors. Add a nowFunc seam so the timeout branch is unit-testable. The 15-minute harnessWait and run-first fallback from the first cut of this branch are dropped: neither addressed an observed failure, and the fallback added an API call per run per poll under rate pressure. Closes #6697 Assisted-by: Claude (fix), Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Investigating the #6647 behaviour-test failures showed both were misreported by the harness-wait timeout path, not caused by the deadline or the artifact lookup: - Attempt 1: the harness runs existed within 2s of the label, but the pool-org installation token was rate limited (403) for the rest of the wait. listHarnessRunsAfter turned every listing error into an empty list, so the timeout said "no recent workflow runs found". Client-side retries also stretched the 12-minute wait to 26 minutes. - Attempt 2: dispatch declined the event (collaborator permission lookup failed for the labelling actor on a freshly recreated pool repo), the harness matrix never expanded, and no artifact was ever uploaded. The timeout showed a "successful" run with no explanation. Record listing errors during the wait instead of swallowing them, bound each poll's API calls to the remaining wait budget, and make the timeout diagnostics state the agent job's state per run (detecting the unexpanded matrix), classify listed artifacts against the trigger time, and report the recorded errors. Add a nowFunc seam so the timeout branch is unit-testable. The 15-minute harnessWait and run-first fallback from the first cut of this branch are dropped: neither addressed an observed failure, and the fallback added an API call per run per poll under rate pressure. Closes #6697 Assisted-by: Claude (fix), Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Investigating the #6647 behaviour-test failures showed both were misreported by the harness-wait timeout path, not caused by the deadline or the artifact lookup: - Attempt 1: the harness runs existed within 2s of the label, but the pool-org installation token was rate limited (403) for the rest of the wait. listHarnessRunsAfter turned every listing error into an empty list, so the timeout said "no recent workflow runs found". Client-side retries also stretched the 12-minute wait to 26 minutes. - Attempt 2: dispatch declined the event (collaborator permission lookup failed for the labelling actor on a freshly recreated pool repo), the harness matrix never expanded, and no artifact was ever uploaded. The timeout showed a "successful" run with no explanation. Record listing errors during the wait instead of swallowing them, bound each poll's API calls to the remaining wait budget, and make the timeout diagnostics state the agent job's state per run (detecting the unexpanded matrix), classify listed artifacts against the trigger time, and report the recorded errors. Add a nowFunc seam so the timeout branch is unit-testable. The 15-minute harnessWait and run-first fallback from the first cut of this branch are dropped: neither addressed an observed failure, and the fallback added an API call per run per poll under rate pressure. Closes #6697 Assisted-by: Claude (fix), Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Investigating the #6647 behaviour-test failures showed both were misreported by the harness-wait timeout path, not caused by the deadline or the artifact lookup: - Attempt 1: the harness runs existed within 2s of the label, but the pool-org installation token was rate limited (403) for the rest of the wait. listHarnessRunsAfter turned every listing error into an empty list, so the timeout said "no recent workflow runs found". Client-side retries also stretched the 12-minute wait to 26 minutes. - Attempt 2: dispatch declined the event (collaborator permission lookup failed for the labelling actor on a freshly recreated pool repo), the harness matrix never expanded, and no artifact was ever uploaded. The timeout showed a "successful" run with no explanation. Record listing errors during the wait instead of swallowing them, bound each poll's API calls to the remaining wait budget, and make the timeout diagnostics state the agent job's state per run (detecting the unexpanded matrix), classify listed artifacts against the trigger time, and report the recorded errors. Add a nowFunc seam so the timeout branch is unit-testable. The 15-minute harnessWait and run-first fallback from the first cut of this branch are dropped: neither addressed an observed failure, and the fallback added an API call per run per poll under rate pressure. Closes #6697 Assisted-by: Claude (fix), Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Investigating the #6647 behaviour-test failures showed both were misreported by the harness-wait timeout path, not caused by the deadline or the artifact lookup: - Attempt 1: the harness runs existed within 2s of the label, but the pool-org installation token was rate limited (403) for the rest of the wait. listHarnessRunsAfter turned every listing error into an empty list, so the timeout said "no recent workflow runs found". Client-side retries also stretched the 12-minute wait to 26 minutes. - Attempt 2: dispatch declined the event (collaborator permission lookup failed for the labelling actor on a freshly recreated pool repo), the harness matrix never expanded, and no artifact was ever uploaded. The timeout showed a "successful" run with no explanation. Record listing errors during the wait instead of swallowing them, bound each poll's API calls to the remaining wait budget, and make the timeout diagnostics state the agent job's state per run (detecting the unexpanded matrix), classify listed artifacts against the trigger time, and report the recorded errors. Add a nowFunc seam so the timeout branch is unit-testable. The 15-minute harnessWait and run-first fallback from the first cut of this branch are dropped: neither addressed an observed failure, and the fallback added an API call per run per poll under rate pressure. Closes #6697 Assisted-by: Claude (fix), Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Investigating the #6647 behaviour-test failures showed both were misreported by the harness-wait timeout path, not caused by the deadline or the artifact lookup: - Attempt 1: the harness runs existed within 2s of the label, but the pool-org installation token was rate limited (403) for the rest of the wait. listHarnessRunsAfter turned every listing error into an empty list, so the timeout said "no recent workflow runs found". Client-side retries also stretched the 12-minute wait to 26 minutes. - Attempt 2: dispatch declined the event (collaborator permission lookup failed for the labelling actor on a freshly recreated pool repo), the harness matrix never expanded, and no artifact was ever uploaded. The timeout showed a "successful" run with no explanation. Record listing errors during the wait instead of swallowing them, bound each poll's API calls to the remaining wait budget, and make the timeout diagnostics state the agent job's state per run (detecting the unexpanded matrix), classify listed artifacts against the trigger time, and report the recorded errors. Add a nowFunc seam so the timeout branch is unit-testable. The 15-minute harnessWait and run-first fallback from the first cut of this branch are dropped: neither addressed an observed failure, and the fallback added an API call per run per poll under rate pressure. Closes #6697 Assisted-by: Claude (fix), Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Since f317cdb pool repos are deleted and recreated on allocation. awaitCreation waits only until GetRepo stops returning 404 — repo visibility. On the #6647 run the suite had already pushed files into the recreated repo with its installation token, yet when it labelled the issue the harness dispatch's collaborator permission lookup for the labelling actor answered 200 with an empty role_name, the role fell to none and the harness matrix came back empty (#6697). The stale state is the collaborator view of the bot on the new repo ID, a consistency domain separate from repo visibility and from the installation's repository list. Add awaitActorAccess as the last ensure step: learn the account the token acts as from the author of the repo's newest commit (API commits made with an installation token are attributed to the app's bot user; GET /user is the fallback for PATs), then poll GetCollaboratorPermission with the existing 1s-doubling backoff until a role resolves (7 attempts, ~63s) and fail allocation with a clear error otherwise. Clients without the lookup, or whose identity cannot be learned, skip the wait with a log line. The GitHub LiveClient gains LatestCommitAuthorLogin. Closes #6701 Assisted-by: Claude (fix), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Since f317cdb pool repos are deleted and recreated on allocation. awaitCreation waits only until GetRepo stops returning 404 — repo visibility. On the #6647 run the suite had already pushed files into the recreated repo with its installation token, yet when it labelled the issue the harness dispatch's collaborator permission lookup for the labelling actor answered 200 with an empty role_name, the role fell to none and the harness matrix came back empty (#6697). The stale state is the collaborator view of the bot on the new repo ID, a consistency domain separate from repo visibility and from the installation's repository list. Add awaitActorAccess as the last ensure step: learn the account the token acts as from the author of the repo's newest commit (API commits made with an installation token are attributed to the app's bot user; GET /user is the fallback for PATs), then poll GetCollaboratorPermission with the existing 1s-doubling backoff until a role resolves (7 attempts, ~63s) and fail allocation with a clear error otherwise. Clients without the lookup, or whose identity cannot be learned, skip the wait with a log line. The GitHub LiveClient gains LatestCommitAuthorLogin. Closes #6701 Assisted-by: Claude (fix), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Claude Code 2.1.2xx ships as a Bun-compiled binary named claude.exe even on Linux, and since #6647 the CLAUDE_CODE_VERSION-pinned install is the binary that runs in the sandbox. The scaffold Vertex profile only allowlisted **/claude, so OpenShell's OPA denied claude.exe the STS call and every Claude run on the 0.40.0 image failed on its first request with "API Error: Error code policy_denied" (0 tokens). The fleet copy in fullsend-ai/agents was fixed by fullsend-ai/agents#1118; this repo's embedded copy, which functional-tests and local runs load through --fullsend-dir, was not. Add **/claude.exe and **/pi so the binaries list matches the agents copy (**/pi is carried for parity with that copy; pi itself runs via node), pin the whole list in a scaffold test so the two copies cannot drift on this again, and update the bring-your-own-agent guide and the runtime egress diagram that still showed the old list. Refs #6971 Assisted-by: Claude (code, fix, review), Grok (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Claude Code 2.1.2xx installs its native binary at bin/claude.exe even on Linux, and since #6647 the CLAUDE_CODE_VERSION-pinned install is the binary that runs in the sandbox. The scaffold Vertex profile only allowlisted **/claude, so OpenShell's OPA denied claude.exe the STS call and every Claude run on the 0.40.0 image failed on its first request with "API Error: Error code policy_denied" (0 tokens). The fleet copy in fullsend-ai/agents was fixed by fullsend-ai/agents#1118; this repo's embedded copy, which functional-tests and local runs load through --fullsend-dir, was not. Add **/claude.exe and **/pi so the binaries list matches the agents copy (**/pi is carried for parity with that copy; pi itself runs via node), pin the whole list in a scaffold test so the two copies cannot drift on this again, and update the bring-your-own-agent guide and the runtime egress diagram that still showed the old list. Refs #6971 Assisted-by: Claude (code, fix, review), Grok (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Claude Code 2.1.2xx installs its native binary at bin/claude.exe even on Linux, and since #6647 the CLAUDE_CODE_VERSION-pinned install is the binary that runs in the sandbox. The scaffold Vertex profile only allowlisted **/claude, so OpenShell's OPA denied claude.exe the STS call and every Claude run on the 0.40.0 image failed on its first request with "API Error: Error code policy_denied" (0 tokens). The fleet copy in fullsend-ai/agents was fixed by fullsend-ai/agents#1118; this repo's embedded copy, which functional-tests and local runs load through --fullsend-dir, was not. Add **/claude.exe and **/pi so the binaries list matches the agents copy (**/pi is carried for parity with that copy; pi itself runs via node), pin the whole list in a scaffold test so the two copies cannot drift on this again, and update the bring-your-own-agent guide and the runtime egress diagram that still showed the old list. Refs #6971 Assisted-by: Claude (code, fix, review), Grok (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
Summary
The OpenShell base image bundles its own Claude Code at
/usr/local/bin/claude(installed upstream with the unpinnedcurl claude.ai/install.sh, so it is whatever was current when the base was built — 2.1.156 on the base we pin), and/usr/local/binprecedes npm's global bin (/usr/bin) on the sandboxPATH. The npm install pinned byCLAUDE_CODE_VERSIONwas therefore never theclaudethat ran: every Renovate bump of the pin has been a runtime no-op, and model aliases resolved from 2.1.156's table (sonnet→claude-sonnet-4-5@20250929on Vertex, while the pi runtime maps the same alias toclaude-sonnet-4-6). Every fleet run's→ Agent: … (v2.1.156)line shows it.This points
/usr/local/bin/claudeat the npm install right after it and asserts at build time thatclaude --versiononPATHreportsCLAUDE_CODE_VERSION, so a base-image change cannot silently bring the shadow back.fullsend-codeinherits the fix from this image.Related Issue
Fixes #6612
Changes
images/sandbox/Containerfile: afternpm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}, replace/usr/local/bin/claudewith a symlink to$(npm prefix -g)/bin/claudeand fail the build ifclaude --version≠CLAUDE_CODE_VERSION.internal/sandbox/sandbox_claude_image_test.go:TestSandboxImageClaudeCodePinWinsguards both halves (symlink after the install, version assertion), in the style of the pi image guards.docs/contributing/runtime-implementation.md: restructured for readability (second commit) — "On this page" list, an explicit "Adding a runtime" checklist, the security matrix split into host-side / sandbox-hook / bootstrap tables with the shared PostToolUse chain described once, the hook contract and pi internals broken into subsections and tables, and a new Pinned runtime binaries in the sandbox image section that records the Claude Code pin/shadow rule from this PR next to the pi and extension pins. Same facts, no anchor changes (#sandbox-hook-contract,#pi-runtime-internals-6464still resolve); every identifier/issue/ADR reference from the old text is still present (checked mechanically).Not in this PR (tracked on #6612): deciding what
sonnet/opus/haikushould mean on the Claude runtime once the real pin runs (explicit ids in harnesses /agents:entries, orANTHROPIC_DEFAULT_*_MODEL), and asking upstream whether bundling agent CLIs in the base image is intentional.Evidence
Live: fullsend-ai/fullsend run 32895730379 (
Agent: claude-opus-4-6 (v2.1.156)), pi-xai-vertex runs 32893364520 / 32894519933 (Model: sonnet → claude-sonnet-4-5@20250929,v2.1.156), konflux-ci/.fullsend triage/code/review/fix runs since Aug 18 — allv2.1.156.Local check of the new step against the pinned
fullsend-sandbox@sha256:46adf184…(matching pin builds,CLAUDE_CODE_VERSION=9.9.9fails the assertion): results in a follow-up comment.Testing
go test ./internal/sandbox/ -run TestSandboxImagepassesmake lintpassessandbox-imagesPR build (CI) — this is the real verification: the assertion runs inside the buildChecklist
Note for reviewers
This moves every Claude-runtime agent from 2.1.156 to 2.1.243 in one step. The hook/tool-name contracts and
--printJSON parsing have only ever been exercised against 2.1.156 in CI, so please let e2e/functional run fully rather than treating this as a trivial Containerfile edit. Agents' harness image pins move separately (fullsend-ai/agents, after the next release — #6607).