fix: stop asserting fleet tool registration through MCP SDK internals - #296
Open
saucam wants to merge 1 commit into
Open
fix: stop asserting fleet tool registration through MCP SDK internals#296saucam wants to merge 1 commit into
saucam wants to merge 1 commit into
Conversation
`ORCHESTRATOR_FLEET_TOOLS > the BUILT server registers exactly those tools`
read `server.instance._registeredTools` — a private field on the MINIFIED
copy of the MCP SDK that @anthropic-ai/claude-agent-sdk bundles. That is not
a stable surface, and it moved: the field now reads back undefined, so the
test dies with
TypeError: undefined is not an object
(evaluating 'Object.keys(server.instance._registeredTools)')
No code changed to cause this. The workflow pins `bun-version: latest`; #294
went green on bun 1.3.13 at 00:35 and the same job re-run at 14:49 on bun
1.3.14 fails on main's own tree. Dependencies are identical either side
(--frozen-lockfile, claude-agent-sdk@0.3.220) — only the runtime moved. Every
open PR is red until this lands.
buildFleetMcpServer now materialises the picked tool array once and returns
its names as `registeredToolNames`, derived from the exact array handed to
`createSdkMcpServer`. That keeps what the test was actually protecting — that
`pick()` is applied on the way in, rather than a constant quietly diverging
from the real surface — while depending on nothing minified.
A second test cross-checks `registeredToolNames` against the live server so
the accessor cannot drift from reality. It reads the private field only as a
corroborating signal and skips when absent, so an SDK move degrades it to a
no-op instead of re-breaking the build.
Pinning bun would also go green today and re-break on the next bump.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
mainis red right now, and this unblocks itEvery open PR currently fails
daemon-checkon one test:No code caused this. The workflow pins
bun-version: latest:That last row is the proof: main's own commit fails the same test today having passed this morning. Dependencies are identical either side —
--frozen-lockfile,claude-agent-sdk@0.3.220both runs. Only the runtime moved.Why the test was fragile
It read
server.instance._registeredTools, a private field on a minified bundled copy of the MCP SDK —@anthropic-ai/claude-agent-sdkinlines it, and the class shows up at runtime asCk. Minified internals are not a surface anything should assert on; a patch-level bump was always going to be able to break it.The fix
buildFleetMcpServernow materialises the picked tool array once and returns its names asregisteredToolNames, derived from the exact array handed tocreateSdkMcpServer.That preserves what the test was actually protecting — its own comment says it best, "Asserting the constant alone would pass while
pick()silently ignored it — the filter is what actually reaches the model" — because the assertion still runs against what went into the SDK, not against a restatement of the constant. It just no longer depends on anything minified.A second test cross-checks
registeredToolNamesagainst the live server's internals so the new accessor can't drift from reality. It treats the private field as a corroborating signal and skips when absent, so the next SDK move degrades it to a no-op instead of re-breaking the build.Why not just pin bun
bun-version: 1.3.13would go green today and re-break on the next bump, with the same debugging cost. Worth doing separately if you want CI reproducibility, but it isn't a fix for this test.Verification
Note
Found while opening #295 (repeat-tool guard). That PR is blocked on this one; it should go green once this lands and it rebases.
🤖 Generated with Claude Code