Revert claude agent SDK lockfile bump from #333870 - #334094
Conversation
#333870 bumped transitive deps in build/agent-sdk/agents/claude/ while @anthropic-ai/claude-agent-sdk stayed pinned at 0.3.239. Those deps are peers, so npm installs them into node_modules, and node_modules is what package.ts tars. The tarball bytes changed and upload.ts refused to overwrite the existing agent-sdk/claude/0.3.239/<target>.tgz blob: remote: 4d2a122a30085d088c124769fd01febd4814e1ce3022bf8534be3434103c58b6 local: 3f428bf4db0ed63b7ff0587033a30e998eacaf246da933732764a0a8f8d6589f Restores the lockfile to the state that produced the blobs already on the CDN. Leaves the dependencies-check.yml and build/npm/gyp changes from #333870 alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It restores vulnerable dependencies; the patched graph should use a new immutable artifact identity.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (1)
- build/agent-sdk/agents/claude/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The follow-up this PR suggested is up as #334113, taking a different route than the lockfile freeze proposed here. Rather than pinning the peer graph so it stops moving, that PR stops installing peers at all ( A freeze would have kept the peers in the tarball and forbidden them from changing, which is a rule someone has to keep obeying. This makes the collision structurally impossible instead. |
) * build: omit peer deps from agent SDK tarballs, bump claude to 0.3.258 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> * build: verify the staged tree for every agent SDK, not just claude `--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> * build: exercise the real tool() path in the SDK load probe, bound it 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> * build: trim comments in agent-sdk package.ts 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> * build: correct which SDK call the load probe leans on 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> * build: drop SDK-specific logic from the staged-tree check 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> * test: re-record /model stdout for claude 0.3.258 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> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Reverts the
build/agent-sdk/agents/claude/package-lock.jsonportion of #333870. Thedependencies-check.ymlandbuild/npm/gyp/package-lock.jsonchanges from that PR stay.Why
The release build is red on every platform:
#333870 bumped six transitive deps in the claude agent folder while
@anthropic-ai/claude-agent-sdkstayed pinned at0.3.239:@hono/node-serverbody-parserbody-parser/node_modules/content-typefast-urihonoip-addressAll six are
"peer": true, sonpm ciinstalls them intonode_modules, andnode_modulesis exactly whatpackage.tstars. Different tree, different tarball, different sha.The CDN path is
agent-sdk/claude/<version>/<target>.tgz. Nothing in it reflects the transitive graph, so the SDK version can hold still while the content moves underneath it, andupload.tscorrectly refuses to rewrite history. Codex passed in the same run because its lockfile wasn't touched.This restores the lockfile to the state that produced the blobs already on the CDN. It was live on
mainfrom #333256 (Aug 28) until yesterday morning, so it is the known-good state, not a guess.Tradeoff, please read before approving
The revert reintroduces advisories that #333870 had cleared.
npm audit --package-lock-onlyinbuild/agent-sdk/agents/claude:qs)fast-uri,hono, andip-addresson top of the sameqsoneThe high-severity pair is GHSA-mwp4-54f8-5fhr and friends in
ip-address(SSRF / trust-boundary bypass).Worth weighing that these are peers pulled in behind the Claude SDK rather than code we call, but I did not chase down whether the SDK actually loads them, so treat the exposure as unquantified.
The alternative, if we would rather ship the newer deps than unblock cheaply: delete every blob under
agent-sdk/claude/0.3.239/in the Azure Portal, not justwin32-arm64, and re-run. Otherwise0.3.239means one dep graph on some platforms and another elsewhere. Note the blobs go up withmax-age=31536000, immutable, so anyone who already cached the old tarball keeps it.Happy to switch this PR to that approach instead if that is the call.
This is the second time
#333636 made the identical lockfile edit on Aug 31 at 14:35 and was reverted 90 minutes later in
2b82959, with a bareThis reverts commit ...message and no explanation. Same failure, near-certainly. #333870 then re-landed the same bumps two days later.Follow-up
Not in this PR, but the invariant we actually depend on is "the whole lockfile is frozen unless the SDK version changes," and nothing enforces it.
build/agent-sdk/test/versionSync.test.tschecks the SDK's own version in three places and says nothing about the other 109 entries. Options I can see:agents/*/package-lock.jsonin that test, so touching transitive deps forces an explicit acknowledgement.urlTemplateandproduct.agentSdks.I lean toward 1. Filing separately.
Verification
cd build && node --test 'agent-sdk/**/*.test.ts'passes.package.jsonis byte-identical between the two lockfile states, sonpm cistays in sync.4d2a122a….chmodPlatformBinariessets0o755, which is a no-op on Windows, so a macOS-built tarball cannot match a pipeline-built one byte for byte. The release build is the real check.🤖 Generated with Claude Code