build: make agent SDK tarballs a function of version and target - #334113
Merged
Conversation
npm 7+ installs peerDependencies automatically, so the claude tarball has been carrying 100 packages the agent host never loads — @modelcontextprotocol/sdk, zod, ajv and their transitive graph. The SDK inlines all of that into sdk.mjs at publish time: sdk.mjs statically imports node builtins and nothing else, and the one external module it resolves at runtime is its own native binary package. Every reference to those packages on the VS Code side is an `import type`, which TypeScript erases. Adding --omit=peer to the packaging install leaves exactly two packages in the tarball, both pinned to the SDK version. That makes the bytes a function of (SDK version, target) and nothing else, so a transitive peer bump can no longer change the content at a CDN path that is already published — the failure that took #333870 and its revert #334094. Unlike --omit=optional, this doesn't touch the native binary package. codex declares no peers, so its tarball is byte-identical either way. That the SDK inlines its peers is an implementation detail Anthropic never promised, so package.ts now runs a load probe before tarring: in a child process it imports sdk.mjs out of the staged tree and builds an MCP server from it, peers absent. If a future version starts importing a peer for real, the build fails there rather than on a user's machine against a tarball that is already immutable on the CDN. Bumping claude in the same change since the CDN path moves regardless. 0.3.258 adds a required Query.updateSettings, hence the three test fakes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The load probe needs a bounded timeout and must exercise the production sdk.tool(...) path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
 |
build/agent-sdk/package.ts — The guard only checks that tool is a function; it never invokes the production sdk.tool(...)… |
|
build/agent-sdk/package.ts — A child process does not prevent a stray top-level handle from wedging this synchronous build:… |
 |
build/agent-sdk/README.md — This statement is inaccurate: `src/vs/platform/agentHost/node/claude/clientTools/claudeJsonSchemaToZ… |
What changed in this PR
Makes Agent SDK tarballs reproducible by excluding peer dependencies, validating Claude SDK loading, and upgrading Claude to 0.3.258.
Changes:
- Adds
--omit=peerand Claude SDK load validation. - Updates Claude SDK pins and lockfiles.
- Updates packaging documentation and test fakes.
| File | Description |
|---|---|
src/vs/platform/agentHost/test/node/claudeSdkPipeline.test.ts |
Updates the query fake. |
src/vs/platform/agentHost/test/node/claudeAgent.test.ts |
Updates the agent query fake. |
src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts |
Updates the integration query fake. |
package.json |
Updates the development SDK pin. |
package-lock.json |
Locks root Claude dependencies. |
build/agent-sdk/README.md |
Documents tarball composition. |
build/agent-sdk/package.ts |
Omits peers and adds load validation. |
build/agent-sdk/agents/claude/package.json |
Updates the build SDK pin. |
build/agent-sdk/agents/claude/package-lock.json |
Locks Claude 0.3.258 packages. |
Files not reviewed (1)
- build/agent-sdk/agents/claude/package-lock.json: Generated file
Suppressed comments (2)
build/agent-sdk/package.ts:190
- This 20-line JSDoc exceeds the repository's 1–2 short sentence limit and duplicates rationale already documented in the README. Condense it to the function contract and keep the detailed packaging rationale in
build/agent-sdk/README.md; the same applies to the narrative inline blocks within this helper.
/**
* Smoke-checks the packaged claude tree before it is tarred: imports the SDK's
* ESM entry point and builds an in-process MCP server out of it, with the
* peerDependencies absent (see `npmCi`).
*
build/agent-sdk/package.ts:250
- This narrative block exceeds the repository's one-line limit for inline method comments and repeats the README. Keep only the non-obvious link between peer omission and the load guard here.
// `--omit=peer` drops the auto-installed peerDependencies. The agent host
// never loads them out of the tarball: the SDK's own entry points inline
// everything they need (MCP, zod, ajv) at bundle time, and the workbench
// passes its own zod / MCP types across the boundary. Omitting them keeps
// the tarball a pure function of (SDK version, target), so a transitive
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Raymond Zhao (rzhao271)
previously approved these changes
Sep 2, 2026
`--omit=peer` applies to every SDK, and `Sdk` is an open string type, so adding one is a single folder under `agents/`. The load probe that justified the flag only ran for claude, which left any other SDK — codex today, anything added later — inheriting the flag with nothing checking it. Replace the `if (sdk === 'claude')` guard with a `verifyStagedTree` dispatcher whose `default` branch fails the build. The per-SDK checks stay different on purpose: claude's tarball is dynamic-imported by the agent host, so the build imports it too; codex's never is, since the host spawns the vendored binary directly, so the binary layout is what's worth asserting. codex gets a structural check — the platform package vendors exactly one rust triple, holding a non-empty executable binary. It deliberately does not copy `codexAgent.ts`'s `sdkTarget → triple` table; a second copy could drift and then validate a path nothing uses. Also fixes a latent bug in `chmodPlatformBinaries`: the claude branch looked for `claude` on every target, so it silently skipped win32's `claude.exe`. Nothing shipped broken — the registry already publishes that binary 0755 and Windows ignores POSIX modes on extract — but the loop's filename assumption was wrong, and the new assertion checks the same path it chmods. Verified by fault injection against a real extracted tree: all seven codex checks and the unknown-SDK branch fire, and an untouched tree passes. Five real builds (claude darwin-arm64/win32-x64/linux-x64-musl, codex darwin-arm64/win32-arm64) succeed; the claude darwin-arm64 sha is byte-identical to one built before these checks existed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TylerLeonhardt
changed the title
…with a timeout Three fixes from PR review. The probe checked that `tool` was a function but never called it. The shipped path is `buildClientToolMcpServer`, which passes a zod raw shape into `sdk.tool()` and the result into `createSdkMcpServer()`. A future SDK that resolved zod lazily inside `tool()` would sail past the old check and break at runtime. The probe now makes that exact call, using VS Code's own zod, which is what the agent host hands across the boundary. Verified by substituting a `tool()` that resolves a peer from disk: exit 1 with ERR_MODULE_NOT_FOUND. `spawnSync` without a timeout blocks forever, so the old comment claiming a child process kept a stray handle from wedging the build was wrong. Added a 2 minute timeout and a `result.signal` check, since a timeout surfaces as SIGTERM with a null status and would otherwise report a confusing exit code. The README claimed every reference to the peers in non-test `src/` was `import type`. That is true of `@modelcontextprotocol/sdk` but not of zod: `claudeJsonSchemaToZod.ts` imports `z` at runtime. The invariant that `--omit=peer` actually needs is narrower, that zod comes from VS Code's own dependency rather than the downloaded tree, so the README says that instead. Tarball sha is unchanged, since the probe file sits outside `node_modules`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback: the comments on the new verification code ran far longer than the code they described. Cut them roughly in half, and point at README.md for the rationale instead of restating it in three places. Also drops `nativeBinaryName` for a one-line `exeName(base, sdkTarget)`. It took an `Sdk` parameter every call site already knew statically, and `sdk === 'claude' ? 'claude' : 'codex'` would have silently returned 'codex' for any SDK added later. The only rule the two share is the `.exe` suffix on win32. No behavior change: claude darwin-arm64 still builds to sha256 1050d42b5e86f1d5b0c3a910e5325894d7b1dcfb684fe08ff4ffbf09dcfe0cda and codex darwin-arm64 to a32d7afd7f088e8e4fb9f237283bfb93f656ac1da5c78fb879d31e48422a241d. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
createSdkMcpServer() is what validates and converts the zod raw shape; tool() is a plain constructor that never touches zod. Verified by passing a non-zod shape: tool() returns fine, createSdkMcpServer() throws "inputSchema must be a Zod schema or raw shape". Comment and README said tool() was the load-bearing call. The sequence was already right, only the explanation was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
verifyStagedTree was a switch with a claude case that imported sdk.mjs and replayed buildClientToolMcpServer's call shape (zod raw shape into tool(), result into createSdkMcpServer()) and a codex case that asserted the vendor/<triple>/bin layout. That put one SDK's API into the packaging step for a small gain: a peer that stops being inlined will come back as a static import, which a plain import of the entry point already catches. Now nothing in the check is conditioned on which SDK is building: - The entry point comes from the installed manifest's `main`, which is the same path claudeAgentSdkService.ts imports at runtime. codex declares no `main`, so it is skipped without a special case. - Every native binary must be present, non-empty and executable. The per-SDK binary layouts move into listPlatformBinaries, which chmodPlatformBinaries now shares, so the chmod and the assertion can no longer disagree about where the binaries are. That is also the new-SDK guard: no layout entry means no binaries found, and the build fails naming the function to edit. Removes the zod dependency from the build script and ~50 lines. Fault-injected, all caught: binary missing / empty / not executable, codex vendor/ removed, sdk.mjs importing an uninstalled peer (inserted after the shebang so it is a real ERR_MODULE_NOT_FOUND), and listPlatformBinaries returning [] for an unknown SDK. Tarball bytes unchanged: claude darwin-arm64 1050d42b…, claude win32-x64 b4e00f75…, codex darwin-arm64 a32d7afd…. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bumped CLI now backticks the model name in its `/model` slash command output, so the recorded request no longer matched the live one and the E2E replay failed on Linux and macOS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TylerLeonhardt
marked this pull request as ready for review
September 2, 2026 22:28
TylerLeonhardt
enabled auto-merge (squash)
September 2, 2026 22:28
Raymond Zhao (rzhao271)
approved these changes
Sep 2, 2026
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
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.
Follow-up to #334094, which reverted a lockfile bump because the resulting tarball collided with an already-published CDN blob.
The problem
CDN paths are content-addressed and immutable.
upload.tsHEADs first and refuses to overwrite a blob whose sha256 differs. So the tarball atagent-sdk/claude/<version>/<target>.tgzhas to be a function of the version and target and nothing else.It wasn't. npm 7+ auto-installs
peerDependencies, so 100 packages (@modelcontextprotocol/sdk,zod,ajvand their graph) landed innode_modulesand got tarred. Bump any of them transitively and the bytes change while the version does not, and the next upload fails against the published blob. That is what #333870 did.The fix
npm ci --ignore-scripts --omit=peer. A claude tarball is now exactly two packages,@anthropic-ai/claude-agent-sdkand itsclaude-agent-sdk-<target>binary package, both pinned to the SDK version. Peer churn can no longer move the bytes.The peers were never loaded from that tarball:
sdk.mjsstatically imports node builtins and nothing else. The one external module it resolves at runtime is its own native binary package. It inlines MCP, zod and ajv at publish time.@modelcontextprotocol/sdkisimport typein all eight places it appears in non-testsrc/.zodis the interesting one.claudeJsonSchemaToZod.tsimportszat runtime, so this is not an unused dependency. But that zod is VS Code's own root dependency, shipped in the product, and the raw shapes it builds are passed into the SDK. Nothing resolves zod out of the downloaded tree. That narrower invariant is what--omit=peeractually needs, and it is what the load probe is built around.--omit=peerinstall drives a full MCPinitialize/tools/list/tools/callround-trip with identical output.Size is not the point. The peers are about 4% of a 90MB tarball.
--omit=optionalwould be a very different flag, since the native binary ships as an optional dependency. That is whatfindMissingNativeOptionalDepexists to catch.Also bumps claude to 0.3.258 from 0.3.239, since the flag changes the bytes and the version has to move anyway. 0.3.258 adds
updateSettingsas a requiredQuerymember, so three test fakes get a stub. Worth flagging: the compile-time_assertBindingsMatchSdkdrift check did not catch that, because it only covers bindings we opt into. The fakes'implements Querydid the wider structural check by accident.Verifying it, for every SDK
That the SDK inlines its peers is an implementation detail Anthropic never promised, and the
peerDependenciesblock says the opposite. So the assumption needs a guard. Since--omit=peerapplies to every SDK, that guard cannot be special-cased to one.Sdkis an open string type, so adding an SDK is one folder underagents/, which would otherwise inherit the flag unchecked.verifyStagedTreeruns against the finished tree just before it is tarred, and every SDK must have a case. Thedefaultbranch fails the build rather than skipping. The checks are deliberately different, because copying one onto the other would assert something we do not depend on.claude. The agent host dynamic-imports
sdk.mjsout of the tarball, so the build imports it too, in a child process and under a timeout. It then repeats the shipped call shape frombuildClientToolMcpServer: a zod raw shape intosdk.tool(), the result intocreateSdkMcpServer(). Checking that the exports merely exist would miss a peer resolved lazily insidetool(). The native binary is then asserted present, non-empty and executable.codex. The agent host never loads JS from that tarball. It spawns
vendor/<rust-triple>/bin/codex[.exe]directly. So the check is structural: exactly one vendored triple, holding a runnable binary.codexAgent.ts'ssdkTarget → tripletable is deliberately not copied here, because a second copy could drift and then validate a path nothing uses. A renamed triple stays the runtime's to catch.Both are cross-target safe. The file checks are pure
stat, and the claude probe only runs platform-independent JS that never reaches for the native binary.Drive-by
chmodPlatformBinaries's claude branch looked forclaudeon every target, silently skipping win32'sclaude.exe. Nothing shipped broken, since the registry already publishes that binary0755and Windows ignores POSIX modes on extract. But the assumption was wrong, and the new assertion checks the same path the loop chmods. Both now derive the name from the target.Testing
Fault injection against a real extracted codex tree. Every check fires, and an untouched tree passes:
The claude probe is non-vacuous in three separate ways, each checked by substituting a broken SDK:
sdk.mjsgives exit 1 withERR_MODULE_NOT_FOUNDtool()that resolves a peer from disk gives the samesignal: 'SIGTERM'Five real builds, all exit 0:
The claude darwin-arm64 sha is byte-identical across builds made before the verification existed and after the probe was extended, so the checks do not perturb the artifact. The codex sha is unchanged from
main, since codex declares no peers and the flag is inert there. The win32-x64 claude build confirms the.exepath resolves.cd build && npm run typecheckandeslintare clean.Supersedes the lockfile-freeze follow-up suggested in #334094.