Skip to content

[v0.8 MCP 8] close late probe sessions - #1338

Open
sethkarten wants to merge 2 commits into
v080/mcp-split-m5-project-trustfrom
v080/mcp-split-m8-probe-boundaries
Open

[v0.8 MCP 8] close late probe sessions#1338
sethkarten wants to merge 2 commits into
v080/mcp-split-m5-project-trustfrom
v080/mcp-split-m8-probe-boundaries

Conversation

@sethkarten

@sethkarten sethkarten commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Replacement scope

This PR reconstructs and supersedes the unique implementation delta reviewed in #1266 without rewriting that historical branch. The original PR remains the immutable discussion record: #1266

  • Base: v080/mcp-split-m5-project-trust
  • Replacement branch: v080/mcp-split-m8-probe-boundaries
  • Replacement commit: bba4f89774eb8cca21db081b1314e7c3d6602ba3
  • Propagation/reconciliation merge commits are intentionally excluded.
  • fix(mcp): make cleanup failures observable #1264 is intentionally omitted from the replacement stacks because its declared-base-to-head tree delta is empty.
  • Frozen feat(kernel): dispatch host requests as capabilities #1243 (77b188b92dc91365cb2bc41bdb46a50669d104a8) is the shared foundation. For reconstructed deltas it is a proven tree-compatible base, not an ancestry claim about the historical PR stack.

Validation

  • Biome 2.5.5 on the exact changed paths: pass
  • root tsgo --noEmit: pass
  • Core focused suite on the final Core tip with live daemon/RLM environment removed and single-worker execution: 11 files, 388 tests passed
  • MCP focused suite on the final MCP tip: 9 files, 106 tests passed
  • Independent Terra tree/delta review: pass

No original PR was retargeted, closed, merged, or otherwise mutated.


Note

Medium Risk
Introduces MCP endpoint probing with timeout/cleanup semantics and redacts errors to avoid credential leaks; public mcp test behavior changes to always fail before settings access.

Overview
Adds runMcpDeclarationProbe in mcp-probe.ts: a read-only MCP handshake (initializetools/list) over an injected transport only, with disabled/offline/trust guards, 1–10s timeouts, generic redacted errors (no URL/credential leakage), and always-on session cleanup—including closing sessions whose open resolves after the deadline, with optional onLateCleanupFailure for late close failures.

Public CLI change: mcp test is removed from help/registry and rejected immediately in public-command.ts (before parsing or settings), so probing stays an internal injected-executor path; declaration test still returns the offline preview via existing command execution (tests unchanged for legacy transport injection).

Tests cover handshake shape, guard rails, redaction, late open/close, and fractional timeout floors.

Reviewed by Cursor Bugbot for commit 09faec9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Close late MCP probe sessions and remove mcp test from public CLI

  • Adds runMcpDeclarationProbe in mcp-probe.ts: a bounded, read-only MCP handshake over an injected transport that runs initialize then tools/list, redacts all transport/protocol errors, and guarantees session cleanup even when open resolves after the timeout deadline.
  • Late-opened sessions are closed asynchronously; failures are reported via an optional onLateCleanupFailure callback rather than swallowed silently.
  • Removes mcp test from the CLI: the subcommand spec is deleted from the registry and runMcpDeclarationCommand rejects immediately if the first argument is 'test'.
  • Behavioral Change: callers that previously invoked mcp test now receive an error before any settings access.

Macroscope summarized 09faec9.

Reconstruct the unique net delta from PR #1266, excluding propagation merges.

@jonaowen jonaowen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The late-open cleanup mechanics are mostly sound, but three contract blockers remain.

  1. The probe accepts a caller-supplied trusted: true boolean. The exact stack base already mints and validates opaque ProjectMcpDeclarationAdmission capabilities; a boolean is forgeable and does not prove the declaration passed that boundary. Consume/validate the opaque admission or a provenance-bound trusted snapshot, and test forged, released, and stale admission.
  2. The 2025-03-26 handshake is not protocol-valid. clientInfo lacks required version, the client never sends mandatory notifications/initialized before tools/list, the transport cannot express notifications, and the untyped initialize result/protocol version is ignored. Add notification support, valid client metadata, negotiated-version validation, and exact lifecycle/payload tests.
  3. mcp test remains advertised by malformed-command usage from parseMcpDeclarationCommand() even though registry help removes it and the exact public spelling is rejected. Separate public usage from the retained internal parser (or remove the internal spelling) and test malformed public MCP invocations.

The deadline rejection, late-open handlers, redaction, and timer cleanup do not show an immediate unhandled-rejection defect, but they cannot cure these authority/protocol blockers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants