fix(#6971): allow claude.exe and pi on scaffold Vertex profile, warn on profile shadowing - #6975
Conversation
|
🤖 Review · Commit: |
|
🤖 Finished Review · ✅ Success · Started 4:53 PM UTC · Completed 5:12 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.28 |
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 is very low (1.4) due to minimal scope, zero protected/security paths, and bot authorship; Tier 2 is elevated (2.5) because core files (run.go, run_test.go) have high churn with many authors and frequent fix commits; Tier 3 is low (1.3) with perfect issue-to-PR alignment, clear acceptance criteria coverage, and safe non-breaking changes (allowlist entries and warning-only message); composite weighted score 2 (moderate) preserved from prior assessment. Previous runRisk Assessment: moderate (2/5) DetailsSmall, well-tested bug fix (97 lines, 50% test ratio) adding allowlist entries and a warning message; Tier 1 is very low (1.1) due to minimal scope and no sensitive paths, but Tier 2 is elevated (3.3) because run.go is a high-churn core file with many recent authors and frequent fix commits; Tier 3 is low (1.5) with clear issue-to-PR scope alignment and all acceptance criteria met. |
ReviewFindingsLow
Previous runReviewFindingsLow
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/fullsend/fullsend/.fullsend/.fullsend-cache/resources/sha256/21644e0238d0b83db763fa138c412322223b0ba4a919a542b839e11b6a3d62e7/scripts/post-fix.sh failed: exit status 1) · Started 5:14 PM UTC · Completed 5:22 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.19 |
701718b to
e170772
Compare
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/fullsend/actions/runs/33783187471 Details: |
Site previewPreview: https://7943f6a0-site.fullsend-ai.workers.dev Commit: |
|
🤖 Review · Commit: |
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>
…lved profile fullsend run imports the harness-resolved profiles first and then imports <fullsend-dir>/profiles/, which deletes and re-imports every id it finds, so a directory copy silently replaces a profile the harness already carries. That is how the functional tests kept failing with policy_denied after the fixed Vertex profile was fetched from fullsend-ai/agents (#6962): the stale scaffold copy won without a trace. Emit a warning per shadowed id naming both copies. The override stays in place (per-repo customization relies on it); a local-path profile that already lives in profiles/ is the same file and is not reported, and duplicate ids in the directory are reported once. Document the precedence next to the provider rule in customizing-agents.md. Closes #6971 Assisted-by: Claude (code, fix, review), Grok (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
The claude.exe gap (#6971) was a class of failure, not a one-off: the gateway's OPA matches a profile's binaries: glob against the kernel-resolved exe of the process that opens the connection, so every runtime has to name the file that actually runs, and a runtime pin bump or a new runtime can silently break that. Cover the other runtimes the same way: - codex: bin/codex.js spawns vendor/<triple>/bin/codex (verified with npm pack --dry-run on the 0.152.1 pin), matched by the **/codex the OpenAI profile already carries. - pi: an npm package executed by node, matched by **/node. - opencode: follows the claude.exe pattern (opencode-ai ships a stub bin/opencode.exe that postinstall replaces with the platform binary, opencode-linux-x64/bin/opencode). The runtime is still a stub and not in the sandbox image, so nothing is added to the profiles; the mapping is pre-declared so the test fails the moment opencode joins config.ValidRuntimes until both inference profiles allow **/opencode and **/opencode.exe. TestScaffoldProfilesAllowRuntimeBinaries walks config.ValidRuntimes and requires each selectable runtime to have a declared mapping whose globs are present in the scaffold profiles it uses; the runtime matrix in docs/contributing/runtime-implementation.md gains an "egress binary identity" row with the same facts and the verification recipe. Refs #6971 Assisted-by: Claude (code, review), Grok (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
5fd4c54 to
e75bc41
Compare
|
Pushed Runtime coverage. A reviewer pass corrected my first model of the gateway rule: OPA matches the Docs. Rationale moved out of the how-to pages: the profile snippet carries one comment, the error text is a Troubleshooting row in both user guides with the |
|
🤖 Finished Review · ✅ Success · Started 5:59 PM UTC · Completed 6:38 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.66 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
ready-for-merge
|
🤖 Finished Retro · ✅ Success · Started 6:55 PM UTC · Completed 7:13 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.46 |
Retro: PR #6975 — scaffold Vertex profile binary allowlist fixTimeline
Total agent spend: ~$17. Wasted: ~$11.3 (67%). What went well
Evidence for existing issues
Proposals filed
|
Rebased onto origin/main, which brings fullsend-ai/fullsend#6975 into the generator's embedded scaffold: the Vertex profile now allows `**/claude.exe` and `**/pi` alongside `**/claude`. On the v0.40.0 sandbox image the Claude binary is `claude.exe`, so before that fix it matched no profile's binaries list, no egress policy applied to it, and every request it made was refused — which is what a local `--runtime claude` run of this example hit. The example's copied profile carries the fix now, and examples/README.md gains the step that was missing for anyone trying to run one: these are deliberately absent from this repository's config.yaml, so `fullsend run` cannot resolve them until you register a copy. The exact error and the `fullsend agent add` command that avoids it are both in the README — found by hitting it. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Rebased onto origin/main, which brings fullsend-ai/fullsend#6975 into the generator's embedded scaffold: the Vertex profile now allows `**/claude.exe` and `**/pi` alongside `**/claude`. On the v0.40.0 sandbox image the Claude binary is `claude.exe`, so before that fix it matched no profile's binaries list, no egress policy applied to it, and every request it made was refused — which is what a local `--runtime claude` run of this example hit. The example's copied profile carries the fix now, and examples/README.md gains the step that was missing for anyone trying to run one: these are deliberately absent from this repository's config.yaml, so `fullsend run` cannot resolve them until you register a copy. The exact error and the `fullsend agent add` command that avoids it are both in the README — found by hitting it. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
2 participants
Summary
Every Claude run on the v0.40.0 sandbox image (Claude Code 2.1.258) that loads the Vertex profile shipped in this repo fails on its first call with
API Error: Error code policy_denied, 0 tokens. The gateway log shows why: OpenShell's OPA denies/usr/lib/node_modules/@anthropic-ai/claude-code/bin/claude.exe -> sts.googleapis.com:443because the scaffold profile only allowlists**/claude. Claude Code 2.1.2xx installs its native binary atbin/claude.exeeven on Linux, and since #6647 that pinned install is the binary that runs in the sandbox. The fleet copy in fullsend-ai/agents was fixed by fullsend-ai/agents#1118; this repo's embedded copy, whichfunctional-testsand local runs load through--fullsend-dir, was not. See #6971 for the full analysis.Three commits:
fix(scaffold): allow claude.exe and pi on the Vertex profile— adds**/claude.exeand**/pitointernal/scaffold/fullsend-repo/profiles/fullsend-vertex-ai.yamlso the list matches the agents copy (**/piis carried for parity; pi itself runs via node), pins the whole list inTestScaffoldVertexProfile_BinaryAllowlist, and updates the bring-your-own-agent guide and the runtime egress diagram that still showed the old list.fix(run): warn when a profiles/ directory copy shadows a harness-resolved profile—fullsend runimports harness-resolved profiles first and then<fullsend-dir>/profiles/, which deletes and re-imports every id it finds, so a stale directory copy silently undid the fixed profile the functional run had just fetched (functional-tests: every triage case fails with API Error policy_denied on main since 2026-09-03 #6962). The runner now warns per shadowed id, naming the directory and the harness copy. Warning only, no precedence change; a local-path profile that already lives inprofiles/and the runner-generatedfullsend-gitlab-forgeprofile are not reported.test(profiles): pin the egress binary each runtime needs on its profile— the same class of gap covered for every runtime. OpenShell's OPA matches abinaries:glob against the connecting process's/proc/<pid>/exeor any ancestor's, which splits runtimes in two: pi and codex are wrapped by node (**/nodeadmits them through the ancestor;**/codexalso names codex's ownvendor/<triple>/bin/codex, spawned bybin/codex.js), while Claude Code is exec'd directly through a symlink tobin/claude.exeand so has no wrapper ancestor to fall back on. That is why sandbox: scaffold Vertex profile lacks **/claude.exe, so every 0.40.0-image Claude run fails with policy_denied #6971 could happen at all, and it is the discriminator for which runtimes a pin bump can break.TestScaffoldProfilesAllowRuntimeBinarieswalksconfig.ValidRuntimes()and fails for any selectable runtime with no declared mapping, or whose profiles lack its globs. OpenCode is exec'd directly too (opencode-aiships a stubbin/opencode.exethatpostinstall.mjsreplaces withopencode-linux-x64/bin/opencode), but it is still a stub, not in the sandbox image, and which of the two paths gets exec'd is a Containerfile decision nobody has made — so no globs are guessed at now: the note lives onOpenCodeRuntime, and the test fails the dayopencodejoinsValidRuntimesuntil a mapping exists. Negative check: removing**/claude.exefrom the scaffold profile fails the test withprofile fullsend-vertex-ai must allowlist **/claude.exe for the claude runtime.Docs, written as walkthroughs rather than rationale dumps: the bring-your-own-agent profile snippet keeps a one-line comment and gains a Troubleshooting row keyed on the literal
API Error: Error code policy_deniedwith thegrep DENIEDcommand;running-agents-locally.mdgets the same symptom with the log line to look for and theprofiles/directory trap;customizing-agents.mdshows the actual warning text instead of describing it; the contributor-facing mechanism moved to an "Egress binary identity per runtime" section plus a checklist step and aCODEX_VERSIONre-check row.Verification
functional-testson this PR (run 33781026441, first head):CASES: 4 (4 OK, 0 FAIL)onAgent: claude-opus-4-6 (v2.1.258), imagefullsend-sandbox@sha256:f8255971…, non-zero cost per case, noDENIED … claude.exelines. This is the first live confirmation that OpenShell'sglob.matchaccepts**/claude.exe.go test ./internal/scaffold/... ./internal/cli/...,go vet,make lint(pre-commit incl. gitlint) pass locally one17077233.Follow-ups filed
allow_uninspected_credentialsonapi.anthropic.com,**/uv/**/uvxon package-registries).Closes #6971
Fixes #6962