fix(mcp): v0.1 CRIT cluster — turnID resolver wired + tool-result double-cap #157

2026-05-12T22:51:10Z

KofTwentyTwo commented

2026-05-12 22:51:10 +00:00

(Migrated from github.com)

Summary

Two critical audit findings from .planning/audit-2026-05-12/mcp-agent-audit.md, scoped to v0.1's "It actually works" milestone. Both about tool-call observability — the Dev Overlay's Tools tab + replay log now reflect production reality.

#20 — `ReplayingToolResultObserver.turnIDResolver` permanently nil (CRIT-1)

The four-seam AGENT-10 contract (SEC-07 dual-write + ME-04 channel + drain Task + ReplayLog writer) was compiled but dead in production. MCPRuntimeWiring.build constructed the observer with the { nil } placeholder before the orchestrator existed, and there was no production write site to flip the resolver afterwards. Every observer.record(...) returned at the guard.

Fix: ReplayingToolResultObserver.turnIDResolver is now a lock-protected slot (OSAllocatedUnfairLock<ResolverBox> — same pattern as InProcessConfirmationCache); attach(turnIDResolver:) replaces it. AgentOrchestrator exposes a currentTurnID() accessor. AppDelegate.installAgent calls observer.attach(...) right after constructing the orchestrator.

#21 — tool-result bytes double-capped at 8KB; doc-comment wrong (CRIT-2)

SanitizeForModel.prepareForBoundary already 8-KB-caps inside the MCP dispatcher; ToolResultPacker.pack then re-caps the already-capped bytes. Packed.fullBytes was never > 8 KB, contradicting the doc-comment that promised "full blob still flows to ReplayLog … observability is not lossy".

Fix: ToolResultPacker.pack becomes a pass-through. The dispatcher's prepareForBoundary (sanitize + 8 KB head-truncate with the …[tool-result-truncated at 8192 bytes] marker) is the sole cap site. Doc-comment rewritten to describe the new contract truthfully.

Test plan

swift test --package-path packages/MCP — green
swift test --package-path packages/AgentCore — green (including new test_OS6b_replayRowMatchesDispatcherBytesNoSecondCap)
swift test --package-path packages/Replay — green
bash scripts/check-app-builds.sh — green (App target compiles cleanly with the new attach(...) wiring + observer lock-slot)
All 17 other boundary gates green (sweep run via for s in scripts/check-*.sh; do bash "$s"; done)
New tests in App/Tests/AppTests/MCPRuntimeWiringTests.swift:
- test_attach_turnIDResolver_flipsObserverFromDeadToLive — pre-attach drops, post-attach emits
- test_attach_replayLogReceivesSECDualWriteCorrelatedToTurnID — end-to-end through the production observer wiring + ReplayLog SQLite assertion
Not run: xcodebuild test — the JarvisAppTests xctest harness is upstream-broken on Xcode 26 ad-hoc Debug bundles (onnxruntime static-link duplication blocks build-for-testing); documented in CLAUDE.md. The new tests compile-check via App target build coverage; runtime verification is gated on the upstream fix.

Commits

fix(mcp): remove redundant inner tool-result cap — closes #21
fix(mcp): wire turnIDResolver post-installAgent — closes #20

🤖 Generated with Claude Code

## Summary Two critical audit findings from `.planning/audit-2026-05-12/mcp-agent-audit.md`, scoped to v0.1's "It actually works" milestone. Both about tool-call observability — the Dev Overlay's Tools tab + replay log now reflect production reality. ### #20 — `ReplayingToolResultObserver.turnIDResolver` permanently nil (CRIT-1) The four-seam AGENT-10 contract (SEC-07 dual-write + ME-04 channel + drain Task + ReplayLog writer) was compiled but **dead in production**. `MCPRuntimeWiring.build` constructed the observer with the `{ nil }` placeholder before the orchestrator existed, and there was no production write site to flip the resolver afterwards. Every `observer.record(...)` returned at the guard. Fix: `ReplayingToolResultObserver.turnIDResolver` is now a lock-protected slot (`OSAllocatedUnfairLock<ResolverBox>` — same pattern as `InProcessConfirmationCache`); `attach(turnIDResolver:)` replaces it. `AgentOrchestrator` exposes a `currentTurnID()` accessor. `AppDelegate.installAgent` calls `observer.attach(...)` right after constructing the orchestrator. ### #21 — tool-result bytes double-capped at 8KB; doc-comment wrong (CRIT-2) `SanitizeForModel.prepareForBoundary` already 8-KB-caps inside the MCP dispatcher; `ToolResultPacker.pack` then re-caps the already-capped bytes. `Packed.fullBytes` was never > 8 KB, contradicting the doc-comment that promised "full blob still flows to ReplayLog … observability is not lossy". Fix: `ToolResultPacker.pack` becomes a pass-through. The dispatcher's `prepareForBoundary` (sanitize + 8 KB head-truncate with the `…[tool-result-truncated at 8192 bytes]` marker) is the sole cap site. Doc-comment rewritten to describe the new contract truthfully. ## Test plan - [x] `swift test --package-path packages/MCP` — green - [x] `swift test --package-path packages/AgentCore` — green (including new `test_OS6b_replayRowMatchesDispatcherBytesNoSecondCap`) - [x] `swift test --package-path packages/Replay` — green - [x] `bash scripts/check-app-builds.sh` — green (App target compiles cleanly with the new `attach(...)` wiring + observer lock-slot) - [x] All 17 other boundary gates green (sweep run via `for s in scripts/check-*.sh; do bash "$s"; done`) - [x] New tests in `App/Tests/AppTests/MCPRuntimeWiringTests.swift`: - `test_attach_turnIDResolver_flipsObserverFromDeadToLive` — pre-attach drops, post-attach emits - `test_attach_replayLogReceivesSECDualWriteCorrelatedToTurnID` — end-to-end through the production observer wiring + ReplayLog SQLite assertion - [ ] **Not run:** `xcodebuild test` — the JarvisAppTests xctest harness is upstream-broken on Xcode 26 ad-hoc Debug bundles (`onnxruntime` static-link duplication blocks `build-for-testing`); documented in `CLAUDE.md`. The new tests compile-check via App target build coverage; runtime verification is gated on the upstream fix. ## Commits - `fix(mcp): remove redundant inner tool-result cap — closes #21` - `fix(mcp): wire turnIDResolver post-installAgent — closes #20` 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/Jarvis#157