fix: the fleet-tools test depended on a private SDK field, so it failed on file order

[saucam]

Unblocks #295, whose daemon-check is red. The failure is not #295's fault — it touches no fleet code.

What actually failed

Not an assertion mismatch — a TypeError:

undefined is not an object (evaluating 'Object.keys(server.instance._registeredTools)')
  at src/tests/collaboration.test.ts:2374

_registeredTools is a private field of the MCP SDK's McpServer. Reading it goes wrong two ways, and the second is what bit us:

1. It breaks on any SDK upgrade that renames or restructures internals.
2. src/tests/provider-claude.test.ts:56 installs a process-global mock.module("@anthropic-ai/claude-agent-sdk", ...) whose fake createSdkMcpServer returns { instance: { tools } } — no _registeredTools. Bun's module mocks leak across files within a run, so whether this test saw the real SDK or that fake depended entirely on which file the runner reached first.

Locally collaboration.test.ts is 4th and provider-claude.test.ts is 14th, so it passes. CI's readdir order differs, and it threw.

Why it's not #294 or #295

• feat: repeat-tool guard — break the loop before the budget runs out #295 changes config.ts, daemon/guard/*, session.ts, docs — no fleet code, no dependency files
• feat: the fleet board contract — clients can finally see the conductor (P5.0) #294 (fleet board) touched no fleet.ts
• the test passes on main locally, and both SDKs are lockfile-pinned (claude-agent-sdk@0.3.220, @modelcontextprotocol/sdk@1.29.0), identical local and CI

It's a latent order-dependent bug that surfaces whenever file ordering shifts — which adding any test file can do. #295 adds src/daemon/guard/repeat-tool.test.ts.

I confirmed the leak mechanism directly: two synthetic files in the same run, one registering that mock and one importing the SDK. Alone the second sees _registeredResources,…,_registeredTools,…; after the mock it sees tools — exactly the CI shape.

The fix

Fix the coupling, not the symptom. buildFleetMcpServer now builds through an exported fleetToolDefinitions() and passes that array through verbatim; the test asserts on those definitions instead of reaching into the SDK.

That still proves what the old test set out to prove — that pick() actually filters what reaches the model, not merely that the constant is well-formed — while being immune to both the SDK's internals and the mock leak.

Adds a second test pinning the seam the split introduces: fleetToolDefinitions would be worthless if the builder stopped calling it. That one asserts only the SDK's public config shape, so it holds under a real or mocked SDK.

Verification

• previously-failing order (provider-claude first) — 227 pass, 0 fail
• reverse order — 227 pass, 0 fail
• full suite — 2311 pass, 0 fail; lint + typecheck clean

Note for #295

Merge main into it after this lands and its daemon-check should go green.

Worth knowing separately

provider-claude.test.ts's mock is still process-global and still shape-unfaithful. Nothing else reads _registeredTools today (I checked — this test was the only consumer), so this PR closes the live issue. But the next thing to reach into a mocked SDK object hits the same trap. Making that mock mirror the real shape would be a reasonable follow-up; I left it out to keep this fix to the failing path.

[saucam]

Redundant — #296 landed the same fix (a0bc9a2) about an hour before I started, and my local origin/main was stale when I diagnosed this. That's my error: I should have re-fetched before opening.

#296's approach is also the better one. It derives registeredToolNames from the array handed to createSdkMcpServer (same principle as my fleetToolDefinitions), and keeps the cross-check against the live MCP surface guarded by if (!internals) return — so it degrades to a skip if the SDK's internals move again rather than going red. Mine dropped that cross-check entirely.

Closing. One observation from my diagnosis that may still be worth a follow-up: src/tests/provider-claude.test.ts:56 installs a process-global mock.module("@anthropic-ai/claude-agent-sdk") whose fake createSdkMcpServer returns { instance: { tools } } with no _registeredTools. Bun's module mocks leak across files within a run, so that mock — not only an SDK bundling change — can produce exactly the observed undefined is not an object shape, depending on file order. #296's guard absorbs it either way; making the mock mirror the real shape would remove the trap for whatever reaches into a mocked SDK object next.

[highflame-oracle]

🔮 Oracle Review

🎯 Start Here

src/daemon/fleet.ts (~15 min) — Logic changes in fleet.ts

---

📋 PR Summary

What this PR does: Fixes flaky test failures in collaboration.test.ts by removing dependency on a private SDK field (_registeredTools) that was causing failures when Bun's module mocks leaked between test files in different execution orders.

Key changes:

• Extracted fleetToolDefinitions() as a public API function in fleet.ts
• Refactored buildFleetMcpServer to use the exported tool definitions instead of accessing private SDK internals
• Updated tests to assert on public fleetToolDefinitions() instead of SDK's _registeredTools field
• Added integration test verifying buildFleetMcpServer correctly calls fleetToolDefinitions()

Areas affected: src/daemon/fleet.ts, src/tests/collaboration.test.ts

Testing notes: Verified 227+ tests pass in both problematic and reverse file orderings; full suite 2311 tests pass with lint and typecheck clean.

---

🔍 Code Review

This is an excellent root-cause fix that properly decouples tests from private SDK internals rather than patching the symptom. The author's thorough investigation identified the Bun module mock leakage mechanism and implemented a clean separation that makes tests robust against SDK changes and execution order.

What's good:

• ✨ Strong diagnostic work identifying the coupling to private SDK fields as the root cause
• ✨ Proper architectural fix extracting a stable public API (fleetToolDefinitions) for testing seams
• ✨ Comprehensive verification across multiple test execution orderings to prevent regression
• ✨ Clear documentation of the issue mechanism and why adjacent PRs were not responsible

Review Stats:

---

Generated by Oracle - Highflame's AI Code Reviewer

[highflame-oracle]

Avoid as never type assertion

The refactor introduces an as never cast on the tools argument passed to createSdkMcpServer. While this may be necessary to satisfy the compiler after extracting buildFleetTools, it completely bypasses type checking for the tool definitions. If the tool shape returned by buildFleetTools deviates from what the SDK expects, this error will be hidden until runtime. Consider using satisfies or explicit generic typing if possible to maintain type safety.

Suggested fix:

Suggested change

	const pick = <T extends { name: string }>(tools: T[]): T[] =>
	If strict typing is impossible, consider a comment explaining *why* `as never` is required, or check if the SDK's `createSdkMcpServer` can be typed to accept the inferred return type of `buildFleetTools`.