chore(#7018,#7019): vendor the fullsend-owned Claude-on-Vertex extension and bump pi to 0.85.0 - #7025
Merged
Conversation
Replace twoGiants/pi-anthropic-vertex v0.1.13 with fullsend-ai/pi-anthropic-vertex v0.1.0 in the sandbox image. The fullsend-owned fork wraps pi's own Anthropic transport with a request-rewriting fetch and carries no @anthropic-ai/* dependency, eliminating the sync/compat.json drift risk and the SDK-override check that the old extension required. Its CI runs the suite against pi 0.84.4 and 0.85.0. Unblocks the pi 0.85.0 bump held on the Dependency Dashboard (#2682). Files changed: - Containerfile: version, SHA256, download URL, comment block, prune list (swap .pi/.vscode/sync/PLAN.md for CLAUDE.md/ CONTRIBUTING.md; drop npm ci SDK-override notes) - renovate.json: depNameTemplate and description in both the packageRules entry and the customManagers regex entry - internal/runtime/pi.go: piVertexExtensionPath comment - docs/contributing/runtime-implementation.md: Claude-on-Vertex section and pin-table row - images/README.md: pin-table row Note: pre-commit hooks were not run. pre-commit could not complete (infrastructure failure: sandbox network policy blocked git fetch for hook repos), and hooks were partially verified via direct execution (gofmt, go vet, check-json, trailing-whitespace, end-of-file checks). Closes #7019
Site previewPreview: https://dff11ef9-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 Review · Commit: |
Member
|
/fs-fix-stop |
|
Fix agent disabled for this PR. Remove the |
|
🤖 Finished Review · ✅ Success · Started 8:22 PM UTC · Completed 8:41 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.08 |
|
Risk Assessment: elevated (3/5) DetailsElevated risk driven by two protected paths (Containerfile, images/README.md) and high file churn in Tier 2 (3.91 avg) — nearly all changed files show heavy recent commit activity with significant fix/revert density. Mitigating factors: bot author, well-scoped chore issue, no security-sensitive or CI workflow changes, and a new guard test added. |
ReviewFindingsMedium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
internal/runtime/pi_run.go(file-level): Line 450 · [low] stale-reference
The PR systematically removed 'interim' from the extension's characterization in pi.go, Containerfile, and runtime-implementation.md, but this comment still reads 'The interim Claude-on-Vertex provider.' This is an inconsistency in the PR's own terminology update.
Suggested fix: Change 'The interim Claude-on-Vertex provider' to 'The vendored Claude-on-Vertex extension' or 'The Claude-on-Vertex extension' to match the PR's updated terminology.
The vendor swap updated the five files that named twoGiants directly, but several claims it left behind were made false by the swap itself, and two it rewrote landed narrower than the truth. Credential hygiene. pi.go and the docs called Run's `unset ANTHROPIC_*` "belt-and-braces" because the new extension reads none of those variables. That is true of the extension and false of the process: pi's built-in anthropic provider is registered in the same pi process and resolves its credential from ANTHROPIC_AUTH_TOKEN, then ANTHROPIC_OAUTH_TOKEN / ANTHROPIC_API_KEY, so a stray value in the agent-writable .env would authenticate a direct-to-Anthropic path that never reaches Vertex. The unset is still load-bearing for that reason; wording that invites its removal is the risk. pi_run.go, where the unset actually happens, still carried the old "the bundled Anthropic SDK would send a stray ANTHROPIC_API_KEY to Google as X-Api-Key" rationale, and fullsend-agent.js carried the same claim for childEnv -- both describe an SDK the extension no longer has. Noted while documenting this, and left for a follow-up because it is a behaviour change rather than a stale claim: neither unset list covers ANTHROPIC_OAUTH_TOKEN, so one of the three names pi resolves is unscrubbed today, bounded by the *.googleapis.com egress allowlist. Compat allowlist. The swap concluded there is nothing version-sensitive left to re-check. One surface survives: the extension forwards pi's compat flags through its own VERTEX_COMPAT_KEYS allowlist, so a compat key a new pi release adds is not forwarded. The allowlist is `satisfies readonly (keyof AnthropicMessagesCompat)[]`, which requires the listed keys to be real but never requires the list to be complete, so the extension's own tsc stays green on a new upstream key. Recorded as a PI_VERSION-bump step in the Containerfile, renovate.json and the docs. The re-check guidance also said to confirm the peerDependencies floor; that floor is never installed under `npm ci --omit=peer`, so it is a declaration to read, not a gate -- the extension's CI matrix against the pinned PI_VERSION is the actual compatibility check. Stale contrasts. Three sites described pi-xai-vertex by contrast with the Anthropic extension's mirrored internals, which no longer exist: the Containerfile comment, renovate.json (the last mention of sync/compat.json as a live concern anywhere in the repo), and the Grok docs bullet. Also: rename the docs heading, which still said "interim"; restore the 26-package count and the #6464 origin reference in the images/README.md row; correct pi_run.go's model-id comment, which named pi 0.84.2 and claimed the catalog is registered verbatim -- the extension registers the running pi's catalog, so the alias table can name an id the pinned PI_VERSION lacks (`fable` maps to claude-fable-5-1, which lands only from pi 0.85.0, #6882), and the provider swap also flips pi's supportsToolReferences default off. New guard. TestSandboxImagePinsAreRenovateTracked proves a customManager exists for each pin, but not that it points at the repo the Containerfile downloads from -- a swap that moved the URL and left depNameTemplate behind would keep every test green while Renovate bumped against the abandoned repo. TestSandboxImageExtensionPinsMatchRenovateDepNames ties the two together, driven from renovate.json so a pin whose URL stops matching fails loudly instead of dropping out of the loop. Verified to fail both when depNameTemplate is reverted to the old owner and when the download is moved to a different host and owner. Verified: sandbox image builds; in the built image the extension is v0.1.0 with no @anthropic-ai/* dependency, google-auth-library present, all four pruned paths gone, tree 555 root; `pi -e .../anthropic-vertex` loads under the pinned pi 0.84.4 and emits the documented "[pi-anthropic-vertex] disabled: ..." message with no project set, and reaches the ADC check with one set. go build, go vet, ./internal/sandbox and ./internal/runtime pass; pre-commit clean on every changed file. Assisted-by: Claude (fix), Grok (review), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
force-pushed
the
agent/7019-vendor-fullsend-pi-anthropic-vertex
branch
from
September 4, 2026 20:44
bedf185 to
ee43f06
Compare
waynesun09
added a commit
to fullsend-ai/pi-anthropic-vertex
that referenced
this pull request
Sep 4, 2026
…ecided `satisfies readonly (keyof AnthropicMessagesCompat)[]` checks that every listed key exists but says nothing about keys that are not listed, so a compat key added by a future pi release would be dropped silently and the catalog test (which only asserts catalog ⊆ allowlist) would stay green. Add VERTEX_COMPAT_DROPPED for the keys deliberately not forwarded and a type-level witness that is `never` only when the two lists cover every key of pi's compat type; a new key now fails `npm run lint` and names itself. Verified: removing a dropped key produces "Type 'true' is not assignable to type '"sendSessionAffinityHeaders"'". Also run ci on tag pushes, so a release exercises both matrix legs rather than only the peer the lockfile resolves. Found in review of fullsend-ai/fullsend#7025. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 8:47 PM UTC · Completed 9:29 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
Same facts, shorter: how the extension works, the env contract, why the ANTHROPIC_* unset stays (pi's built-in anthropic provider in the same process; ANTHROPIC_OAUTH_TOKEN gap is #7029), and what to do on a bump (the extension's CI matrix is the compatibility check; compat flags reach Vertex only through its allowlist). Containerfile, Renovate and image README blocks trimmed to match. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
added a commit
that referenced
this pull request
Sep 4, 2026
pi 0.85.0's bundled Anthropic catalog carries claude-fable-5-1, and #7025 bumps PI_VERSION to it, so the fleet default tracks the newer id rather than settling for claude-fable-5. The extension registers the catalog of the pi that is actually installed, so this table follows the pin. Restate the alias comment accordingly: the block is a preference order, most-preferred first, not an upgrade path — read the other way it said to move sonnet from 5 to 4-6. It also records why sonnet and opus stay on the 4-6 generation: neither fleet Vertex project serves opus 5 or 4-8, and sonnet 5 is served in dev only. Depends on #7025 merging first; on pi 0.84.4 this id is not in the bundled catalog. Assisted-by: Claude (fix, review), Grok (review) Assisted-by: Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
This was referenced Sep 4, 2026
Merged
The vendored Claude-on-Vertex extension no longer carries an Anthropic SDK and its CI covers 0.85.0, which was the last thing holding the pi bump on the Dependency Dashboard (#2682). PI_VERSION 0.84.4 -> 0.85.0. The Node floor assertion beside it still holds: npm metadata gives engines.node ">=22.19.0" for both releases. The 0.84.4 comment is kept but reworded as a FLOOR (the https-proxy-agent named-export bug that breaks google-vertex under HTTPS_PROXY was fixed in 0.84.4, so the rationale still applies below the pin, #8610). piGoogleVertexModels gains gemini-3.8-flash. This table is the bundled google-vertex catalog copied verbatim and is what the Agent tool accepts as a Gemini id, so a missed entry makes us reject a model the running pi serves. 0.85.0's data file adds exactly that one id and changes nothing else. Worth recording how this is checked: diff dist/providers/data/google-vertex.json between the pins, not the generated wrapper -- the wrapper is unchanged and reading it says the catalog is unchanged, which is wrong. The docs re-check column now says so. Closes #7018 by moving the example/default Gemini reference from gemini-3.7-flash to gemini-3.8-flash across config, docs and fixtures. gemini-3.7-flash stays in the catalog table: 0.85.0 removes nothing. Provenance comments are annotated, not rewritten, and only where the cited module was actually re-diffed between 0.84.4 and 0.85.0. Byte-identical: modes/json-event.js, modes/print-mode.js, core/sdk.js, core/package-manager.js, core/auth-storage.js, core/pi-manifest.js, and defaultActiveToolNames; jiti stays 2.7.0. cli/args.js differs only in help text (new PI_SERVER_DIR and PI_SERVER_ID env docs, no new option -- and the pluginformat env denylist already wildcards PI_*). core/tools/bash.js differs by 120 lines but the killProcessTree call sites are identical, so stray_processes.go's claim survives. Because json-event.js is byte-identical the --mode json wire contract did not move, so the parsePiStream fixtures stay valid. pi_run.go's alias comment is flipped rather than deleted: it warned that the table can name an id the pinned pi lacks, with "fable" -> claude-fable-5-1 as the example. 0.85.0's catalog adds that id, so the bump closes the gap the comment described (#6882). Verified locally against an image built from this Containerfile with the xai-vertex 0.2.1 pin from #7024 applied on top, so it matches main after that merge: pi --version -> 0.85.0 pi -ne -e .../anthropic-vertex -e .../xai-vertex --list-models -> 14 anthropic-vertex ids incl. claude-fable-5-1 (new in 0.85.0), plus xai-vertex; no stderr pi -ne --list-models | google-vertex -> 14 gemini ids, and that set is byte-identical to piGoogleVertexModels pi --print --mode json --model anthropic-vertex/claude-sonnet-5 -> exit 0, 14 NDJSON lines, session version 3 that captured stream through parsePiStream -> sessionID captured, no error, exactly one ResultEvent, subtype stop go build, go vet, and ./internal/{config,runtime,harness,sandbox,pluginformat} pass; node --test on fullsend-agent.test.mjs passes 32/32. Assisted-by: Claude (fix), Grok (review), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
changed the title
|
🤖 Review · Commit: |
Two review passes over 4cc8e34 found six things the bump missed. Three are stale provenance the bump should have swept; two are claims the bump itself introduced that do not survive checking; one is a missing repeatable check. Claims the bump got wrong. pi_progress.go argued the parsePiStream fixtures still hold "because dist/modes/json-event.js is byte-identical". The conclusion is right but the reasoning is not: json-event.js only projects message_update and passes everything else through, so on its own it proves almost nothing. The shapes come from core/session-manager.js (still emits CURRENT_SESSION_VERSION = 3 with the same header literal) and core/agent-session.js (differs only in compaction/idle lifecycle, with an unchanged set of emitted type literals). The comment now says that, because the wrong version teaches the next bumper a check that would pass while a shape moved. Likewise runtime-implementation.md claimed "the modules cited here are byte-identical at the pinned 0.85.0" over a whole section: false for two of them -- core/extensions/loader.js and the bundled cli.js did change, though the createJiti call and the isBundledNode expression the section relies on did not (isBundledNode just moved to config.js). Narrowed to what was actually verified. Stale provenance the bump should have swept. The one test that runs a REAL pi binary still named 0.84.4 in its install instructions and skip message, in a file this bump edited -- so the reported "32/32 passing" never exercised that gated path against the new pin; both now derive from the Containerfile's PI_VERSION. pi.go's JITI_FS_CACHE claim was left bare while its sibling in pi_run.go was annotated. And docs' unattended-mode parity line said "verified ... empirically on the pinned build", which the bump silently re-pointed at a build those checks never ran against; it now names the then-pinned 0.84.2 explicitly. Two smaller ones: piReservedOptions is the list an option-set change would invalidate, so the "0.85.0 changes only help text" note belongs there and not only on the parser-shape comment next to it; and the --ignore-scripts rationale beside PI_VERSION said "no postinstall needed", which 0.85.0 makes untrue in the tree (@earendil-works/chord -> esbuild declares one) even though nothing on the bin path reaches it. Missing check. The catalog claim rested on a one-time manual diff with nothing to catch drift on the next bump -- which is the exact failure that produced this issue: reading the generated wrapper (unchanged) instead of the data file (gained gemini-3.8-flash) reports "catalog unchanged" and is wrong. internal/runtime/testdata/pi/check-vertex-catalog.sh fetches the pinned PI_VERSION's pi-ai, diffs dist/providers/data/google-vertex.json against piGoogleVertexModels, and names which side each missing id is on. Verified to fail both ways: dropping gemini-3.8-flash from the table, and running the current table against 0.84.4. The docs re-check column now points at it and keeps only the durable rule. Live checks added for two gaps the review named. docs asks for an adaptive and a non-adaptive model on a bump; the bump smoked only claude-sonnet-5 (forceAdaptiveThinking). anthropic-vertex/claude-haiku-4-5 (non-adaptive) now also returns stopReason "stop" on 0.85.0. And --list-models does not prove a model serves: google-vertex/gemini-3.8-flash generates successfully at CLOUD_ML_REGION global. It 404s in us-east5/us-central1, but so does the outgoing gemini-3.7-flash default, so that is project and region enablement rather than anything about the new id. Not resolved, recorded instead: claude-fable-5-1 is newly reachable at 0.85.0 and is the one alias target carrying supportsMidConvoEffort, the single Anthropic compat key 0.85.0 adds and which the pinned extension v0.1.0 deliberately drops. A live request for it fails in this project on a data-sharing opt-in, so it is confirmed by --list-models only. Assisted-by: Claude (fix), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 10:25 PM UTC · Completed 11:07 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.94 |
waynesun09
approved these changes
Sep 4, 2026
waynesun09
left a comment
waynesun09
left a comment
Member
There was a problem hiding this comment.
Reviewed with three model-diverse squad rounds (Claude, Grok, Codex gpt-5.6-sol), local sandbox image build on pi 0.85.0 with the extension, and live Vertex runs on claude-sonnet-5, claude-haiku-4-5 and gemini-3.8-flash; evidence in the PR body.
waynesun09
dismissed
fullsend-ai-review[bot]’s stale review
September 4, 2026 22:35
Both findings are cosmetic comment-wrap notes (one already outdated by later commits); resolved with a pointer to the v0.1.1 repin follow-up. Human-approved after three squad rounds and live verification.
waynesun09
deleted the
agent/7019-vendor-fullsend-pi-anthropic-vertex
branch
September 4, 2026 22:47
|
🤖 Finished Retro · ✅ Success · Started 10:49 PM UTC · Completed 11:02 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.01 |
waynesun09
added a commit
that referenced
this pull request
Sep 4, 2026
Brings in #7025 (pi 0.85.0, the fullsend-owned anthropic-vertex extension, gemini-3.8-flash), which is what makes claude-fable-5-1 resolvable — this branch depends on it. Two conflicts, both resolved in favour of main plus this branch's additions: - internal/runtime/pi_run.go: took main's rewritten catalog header, which already narrates the fable/0.85.0 relationship, and dropped the duplicate sentence from the piModelAliases block so the story is told once. The preference-order table and the served-model rationale stay. - docs/runtimes/pi.md: took main's gemini-3.8-flash example and kept the note that a per-repo override also reaches sub-agent dispatch. The sub-agent alias test now cites gemini-3.8-flash too, so it matches the example the docs carry. Assisted-by: Claude (fix, review), Grok (review) Assisted-by: Codex (review), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
added a commit
that referenced
this pull request
Sep 4, 2026
The three comment blocks around the alias table had grown to explain themselves in prose, and the catalog header picked up a ragged seam in the #7025 merge. Reflow them so the rule is visible before the code: - piAgentModels: the twenty-line in-loop comment wrapping eight lines of code moves to the doc comment as a three-row worked example, one row per branch. One short line stays inline on the non-obvious argument. - piModelAliases: say plainly that the arrows are what to move to as enablement lands, not a history — the old heading read as an instruction to downgrade sonnet from 5 to 4-6. - the catalog header: two paragraphs instead of one, no dangling clause, and the environment-override note next to the forms it qualifies. Comments only; no behaviour change. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Retro: PR #7025 — vendor fullsend-owned Claude-on-Vertex extension + pi 0.85.0 bumpTimeline. Issue #7019 requested replacing Review pipeline. 6 review runs fired: 1 cancelled (superseded), 1 succeeded, 1 failed (double timeout), 2 cancelled (rapid human pushes), 1 still running at merge time. The human disabled the fix agent early via Evidence for existing issues:
Proposals filed
|
|
Review skipped — this PR is already merged. The Posted by fullsend post-review check |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes that had to land together: the sandbox image now vendors the fullsend-owned Claude-on-Vertex pi extension, and pi itself moves to 0.85.0.
The extension swap (#7019).
twoGiants/pi-anthropic-vertexv0.1.13 →fullsend-ai/pi-anthropic-vertexv0.1.0. The old extension mirrored ~220 lines of pi'sstreamSimplehelpers and pinned@anthropic-ai/sdkunderneath a client cast, so every pi bump was a drift risk; itssync/compat.jsonstopped at pi 0.81.1 and from pi 0.84.3 it hard-failedclaude-opus-5with400 fallbacks: Extra inputs are not permitted. The replacement wraps pi's own Anthropic transport with a request-rewritingfetchand carries no@anthropic-ai/*dependency at all.The pi bump (unblocks #2682). That SDK coupling was the last thing holding
@earendil-works/pi-coding-agent0.85.0 on the Dependency Dashboard, soARG PI_VERSIONmoves 0.84.4 → 0.85.0 in the same PR. (Not ticking the dashboard box here — leaving that to whoever processes #2682.)Default Gemini id (#7018). 0.85.0's catalog adds
gemini-3.8-flash, so the example/default Gemini reference moves fromgemini-3.7-flashtogemini-3.8-flash.gemini-3.7-flashstays in the catalog table — 0.85.0 removes nothing.How the pi bump was checked
piGoogleVertexModelsis the bundledgoogle-vertexcatalog copied verbatim and is what the sub-agentAgenttool accepts as a Gemini id, so a missed entry makes fullsend reject a model the running pi serves. The way to check it is to diffdist/providers/data/google-vertex.jsonbetween the pins — not the generated wrapper, which is unchanged and misleadingly suggests the catalog is too. 0.85.0 adds exactlygemini-3.8-flash.Provenance comments (
verified on 0.84.4) were annotated, not rewritten, and only where the cited module was actually re-diffed. Byte-identical between 0.84.4 and 0.85.0:modes/json-event.js,modes/print-mode.js,core/sdk.js,core/package-manager.js,core/auth-storage.js,core/pi-manifest.js,defaultActiveToolNames; jiti stays 2.7.0.cli/args.jsdiffers only in help text (addsPI_SERVER_DIR/PI_SERVER_IDdocs, no new option).core/tools/bash.jsdiffers by 120 lines but itskillProcessTreecall sites are identical, sostray_processes.go's claim survives. Becausejson-event.jsis byte-identical the--mode jsonwire contract did not move, so theparsePiStreamfixtures stay valid.Verification
Run against an image built from this Containerfile with #7024's
xai-vertex0.2.1 pin applied on top, so it matchesmainafter that merge.claude-fable-5-1appearing matters:piModelAliasesmapsfableto it, and 0.84.4's catalog did not carry it, so the bump closes that gap (#6882).The Gemini table was checked against the running pi rather than by eye:
A real run on Vertex through the image, and that stream through the parser:
The extension install step and the built image were also verified for the swap itself: v0.1.0 present with no
@anthropic-ai/*dependency,google-auth-libraryinstalled (26 packages), all four pruned paths gone, tree555 root, andpi -e .../anthropic-vertexemitting the documented[pi-anthropic-vertex] disabled: ...message when no project is set.Notes for reviewers
TestSandboxImageExtensionPinsMatchRenovateDepNames, ties each tarball pin's download URL to its RenovatedepNameTemplate. The existing test only proved a customManager exists for the ARG, so a swap that moved the URL and leftdepNameTemplatebehind would keep every test green while Renovate bumped against the abandoned repo.Run'sunset ANTHROPIC_*is not dead code now that the extension reads none of those variables: pi's built-inanthropicprovider runs in the same process and resolvesANTHROPIC_AUTH_TOKEN/ANTHROPIC_OAUTH_TOKEN/ANTHROPIC_API_KEYfrom the environment. The comments at all three sites say so.ANTHROPIC_OAUTH_TOKENis not yet in the scrub list — tracked separately in pi: the anthropic-vertex credential scrub misses ANTHROPIC_OAUTH_TOKEN, one of the three names pi's built-in anthropic provider resolves #7029.Closes #7019
Closes #7018