From 531ed44236fb9c6178dbc18c983779c72a612c0d Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 12 Nov 2026 08:03:33 -0500 Subject: [PATCH 1/4] fix(session): a tool call in flight holds its tree against the idle policy (#1127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With MCP_DEBUG_SESSION_IDLE_MS set, a tool call counted as activity only at the moment it was dispatched, and only wait_for_stop held its tree. A call that outlasted the window had its session closed underneath it: a 2 s Java launch under a 1 s window answered "Proxy exited during initialization", and a 6 s Python launch on a loaded host lost its session under a 5 s window. The rule now: the idle clock runs from the END of the last call that named the tree, and a call in flight holds it. - tool-dispatch: holdForTheCall stamps the clock, holds the tree for the call, and on the way out releases and stamps again — under the gate the stamp already had (the tool takes sessionId, the caller may see the session). wait_for_stop's own hold is gone; one mechanism. - sweepIdle marks the tree as closing before its first await, and a call that arrives during that close waits for it and answers closedBy: "idle-policy" with the final outcome, where it used to run against a session being torn down. Nothing is awaited unless a close is in flight, so the check, the stamp and the hold run in one turn. - idleMsOf answers 0 while the tree is held, so list_debug_sessions and /health no longer show a session with a call in flight as about to expire. No test ran the idle policy against a real server; that is how this got through. tests/adapters/python/integration/python-idle-policy.test.ts does: an 800 ms window under a real debugpy launch. It fails on main with "Session not found" for the launch's own session. Co-Authored-By: Claude Fable 5.2 --- CLAUDE.md | 2 +- changelog.d/1025-idle-policy.added.md | 2 +- changelog.d/1127.fixed.md | 1 + docs/architecture/session-tree.md | 7 +- docs/tool-reference.md | 2 +- src/server/handlers/execution-tools.ts | 9 +- src/server/tool-dispatch.ts | 53 +++++-- src/session/session-manager-core.ts | 136 ++++++++++++------ src/skill-content.ts | 2 +- .../integration/python-idle-policy.test.ts | 85 +++++++++++ .../server/handlers/execution-tools.test.ts | 38 ++--- .../unit/server/idle-expiry-answer.test.ts | 122 ++++++++++++++++ tests/core/unit/server/server-test-helpers.ts | 1 + .../server/tool-dispatch-owner-key.test.ts | 45 +++++- .../unit/session/session-idle-policy.test.ts | 79 +++++++++- tests/unit/server-coverage.test.ts | 2 + 16 files changed, 494 insertions(+), 92 deletions(-) create mode 100644 changelog.d/1127.fixed.md create mode 100644 tests/adapters/python/integration/python-idle-policy.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 381177e5..265a3b55 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -222,7 +222,7 @@ The codebase follows a **layered architecture with dependency injection** and ** - Entry point for MCP protocol communication - `src/server.ts` is the composition root and the shared core (`DebugMcpServer`: dependencies, lifecycle, session validation, breakpoint gating, the public facade methods, the `NotificationHub`); it implements `ToolContext` and passes itself to the handlers. It owns no SDK `Server`: `src/server/protocol-facade.ts` (`createProtocolServer(core, { era, deliver })`) builds one cheap SDK `Server` per connection or request over the core, and `src/server/notification-hub.ts` fans the coalesced `resources/updated` pings and `resources/list_changed` out to every facade's sink (issue #1025) - `src/server/` holds the tool schemas (`tool-schemas.ts`), argument coercion (`tool-arguments.ts`), the tools/call dispatch wrapper (`tool-dispatch.ts` over `handlers/index.ts`), argument validation (`tool-validation.ts`), result envelopes and the session-error catch helpers (`tool-result.ts`), one handler module per tool family (`handlers/*.ts`) over their shared helpers (`handlers/shared.ts`), and the resource (`output-resources.ts`) and prompt (`prompts.ts`) handlers - - Supports STDIO and Streamable HTTP transport modes, both in both protocol eras (issue #1025): `stdio` is served through the SDK's `serveStdio(factory, { transport })`, which pins a connection to the era its opening message chose (2026-07-28 for a `server/discover` probe, 2025-11-25 and earlier for `initialize`); `http` routes each request with the SDK's `isLegacyRequest` — a POST carrying the 2026 per-request envelope goes to a strict `createMcpHandler(factory, { legacy: 'reject' })` behind `toNodeHandler` (one facade per request, sessionless), everything 2025-shaped to the sessionful app as before. One core per process in both: on `http` every MCP session is a facade over the same debug sessions (the id is the capability; one process per agent is the isolation), a debug session outlives the MCP session that created it and ends by its own close, its program ending, or the server exiting. The per-debug-session idle policy is the operator's opt-in (`MCP_DEBUG_SESSION_IDLE_MS`, off by default on every transport; sweep `MCP_DEBUG_SESSION_IDLE_SWEEP_MS`): when set, an abandoned session tree is closed unless a client is attending the process (a 2025 session's open SSE stream, or a 2026-07-28 client's open `subscriptions/listen` stream), `list_debug_sessions` rows show `idleMs`/`expiresInMs`, and a call naming a session the policy closed answers `closedBy: "idle-policy"`. A session outside the caller's owner scope is hidden from the call's lookups (`src/session/session-scope.ts`), so every tool answers it as an unknown id. The HTTP reapers (`MCP_HTTP_STALE_SESSION_MS`, `MCP_HTTP_STREAM_LOST_SESSION_MS`) reap abandoned MCP sessions only (issue #1025) + - Supports STDIO and Streamable HTTP transport modes, both in both protocol eras (issue #1025): `stdio` is served through the SDK's `serveStdio(factory, { transport })`, which pins a connection to the era its opening message chose (2026-07-28 for a `server/discover` probe, 2025-11-25 and earlier for `initialize`); `http` routes each request with the SDK's `isLegacyRequest` — a POST carrying the 2026 per-request envelope goes to a strict `createMcpHandler(factory, { legacy: 'reject' })` behind `toNodeHandler` (one facade per request, sessionless), everything 2025-shaped to the sessionful app as before. One core per process in both: on `http` every MCP session is a facade over the same debug sessions (the id is the capability; one process per agent is the isolation), a debug session outlives the MCP session that created it and ends by its own close, its program ending, or the server exiting. The per-debug-session idle policy is the operator's opt-in (`MCP_DEBUG_SESSION_IDLE_MS`, off by default on every transport; sweep `MCP_DEBUG_SESSION_IDLE_SWEEP_MS`): when set, an abandoned session tree is closed unless a client is attending the process (a 2025 session's open SSE stream, or a 2026-07-28 client's open `subscriptions/listen` stream), `list_debug_sessions` rows show `idleMs`/`expiresInMs`, and a call naming a session the policy closed answers `closedBy: "idle-policy"`. A call in flight holds its tree — the dispatch (`src/server/tool-dispatch.ts` `holdForTheCall`) stamps the clock when a call naming a session starts and again when it answers, holds the tree in between, and makes a call that arrives during the sweep's close wait for it (issue #1127). A session outside the caller's owner scope is hidden from the call's lookups (`src/session/session-scope.ts`), so every tool answers it as an unknown id. The HTTP reapers (`MCP_HTTP_STALE_SESSION_MS`, `MCP_HTTP_STREAM_LOST_SESSION_MS`) reap abandoned MCP sessions only (issue #1025) - Dynamically discovers available language adapters 2. **Adapter System** (NEW) diff --git a/changelog.d/1025-idle-policy.added.md b/changelog.d/1025-idle-policy.added.md index 3a3a4a1f..2d390a23 100644 --- a/changelog.d/1025-idle-policy.added.md +++ b/changelog.d/1025-idle-policy.added.md @@ -1 +1 @@ -**An opt-in per-debug-session idle policy; nothing expires by default.** A debug session ends by its own close, its program ending, or the server exiting, on every transport — no timer ends one unless the operator asks. With `MCP_DEBUG_SESSION_IDLE_MS` set, a debug-session tree that no tool call has named for that long is closed through the normal close cascade — a launch root closes its subtree and ends its program, an attach root releases its targets — unless a client is attending the process (an open SSE stream), in which case nothing expires; a tool call naming any session in a tree refreshes the whole tree, an in-flight `wait_for_stop` holds it, and children never expire on a clock of their own. The sweep runs every `MCP_DEBUG_SESSION_IDLE_SWEEP_MS` (default 60 s). The agent can see it: `list_debug_sessions` rows carry `idleMs` (and `expiresInMs` when a policy is on), a tool call naming a session the policy closed answers what happened — when, under which window, and whether its program was ended, released, or was not running by then (`closedBy: "idle-policy"`) — instead of "Session not found", and the server instructions mention the policy when it is on. The HTTP reapers (`MCP_HTTP_STALE_SESSION_MS`, `MCP_HTTP_STREAM_LOST_SESSION_MS`) still reap abandoned MCP sessions, but reaping one no longer closes any debug session: **a crashed client's debug sessions stay until someone closes them, the idle window (if set) runs out, or the server exits.** Set the variable on a shared or long-lived `http` server, and wherever an attached process must not stay paused (the Kubernetes recipe sets 5 minutes). Step 2 of #1025. +**An opt-in per-debug-session idle policy; nothing expires by default.** A debug session ends by its own close, its program ending, or the server exiting, on every transport — no timer ends one unless the operator asks. With `MCP_DEBUG_SESSION_IDLE_MS` set, a debug-session tree that no tool call has named for that long is closed through the normal close cascade — a launch root closes its subtree and ends its program, an attach root releases its targets — unless a client is attending the process (an open SSE stream), in which case nothing expires; a tool call naming any session in a tree refreshes the whole tree and holds it for as long as the call runs (the window starts over when it answers), and children never expire on a clock of their own. The sweep runs every `MCP_DEBUG_SESSION_IDLE_SWEEP_MS` (default 60 s). The agent can see it: `list_debug_sessions` rows carry `idleMs` (and `expiresInMs` when a policy is on), a tool call naming a session the policy closed answers what happened — when, under which window, and whether its program was ended, released, or was not running by then (`closedBy: "idle-policy"`) — instead of "Session not found", and the server instructions mention the policy when it is on. The HTTP reapers (`MCP_HTTP_STALE_SESSION_MS`, `MCP_HTTP_STREAM_LOST_SESSION_MS`) still reap abandoned MCP sessions, but reaping one no longer closes any debug session: **a crashed client's debug sessions stay until someone closes them, the idle window (if set) runs out, or the server exits.** Set the variable on a shared or long-lived `http` server, and wherever an attached process must not stay paused (the Kubernetes recipe sets 5 minutes). Step 2 of #1025. diff --git a/changelog.d/1127.fixed.md b/changelog.d/1127.fixed.md new file mode 100644 index 00000000..c3f2ad9e --- /dev/null +++ b/changelog.d/1127.fixed.md @@ -0,0 +1 @@ +**The idle policy no longer closes a session underneath its own call.** With `MCP_DEBUG_SESSION_IDLE_MS` set, a tool call counted as activity only at the moment it was dispatched, so a call that ran longer than the window — a launch that builds first, a slow launch on a busy host, an `attach_to_process` with a long `verifyTimeout` — had its session closed while it was still in flight and answered a generic launch failure. Every tool call that names a session now holds that session's tree for as long as it runs (the hold was `wait_for_stop`'s alone), and the idle clock starts over when the call answers, so a slow call no longer uses up the window it was made in. `list_debug_sessions` shows `idleMs: 0` for a session with a call in flight, and a call that arrives while the policy is closing a session waits for that close and answers `closedBy: "idle-policy"`. (#1127) diff --git a/docs/architecture/session-tree.md b/docs/architecture/session-tree.md index 5352c01f..b9f0fd46 100644 --- a/docs/architecture/session-tree.md +++ b/docs/architecture/session-tree.md @@ -495,8 +495,11 @@ Two guardrails: Only roots have a clock: a tool call naming any session in a tree refreshes the tree's root, and a child never expires on a clock of its own — the tree goes with its root (decided 2026-11-09, amending the per-descendant clocks this section first proposed: a worker paused under a parent - the agent is driving must not be terminated by a timer). An in-flight `wait_for_stop` holds the - root for the duration of the call. The sweep is skipped entirely while a client is attending the + the agent is driving must not be terminated by a timer). A tool call in flight holds the root for + as long as it runs, whatever the tool, and the clock restarts when it answers (issue #1127: + the hold was `wait_for_stop`'s alone at first, and a launch slower than the window had its + session closed underneath it); a call that arrives while the sweep is closing its tree waits + for that close and is told what happened. The sweep is skipped entirely while a client is attending the process — any open legacy SSE stream or modern `subscriptions/listen` stream — so a connected client never loses a paused session to the clock; idle time still accrues from the last tool call, so a vanished client's already-idle sessions end at the first unattended sweep. Expiry diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 7e8637c2..6e070fd7 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -132,7 +132,7 @@ Lists all active debugging sessions — the whole process's list, whichever MCP Errored sessions include optional `diagnostics` with the current launch attempt's server-host `proxyLogPath` and remote-safe `proxyLogResource`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. -**`idleMs`:** milliseconds since a tool call last named this session or any session in its tree (a child reports its root's) — what tells a forgotten session from a live one. **`expiresInMs`:** present only when the operator turned the idle policy on (`MCP_DEBUG_SESSION_IDLE_MS`): how long the tree has before the policy closes it; any tool call naming the session resets it, a `wait_for_stop` in flight holds it, and nothing expires while a client is attending the process. +**`idleMs`:** milliseconds since the last tool call naming this session, or any session in its tree, answered (a child reports its root's); `0` while such a call is still running — what tells a forgotten session from a live one. **`expiresInMs`:** present only when the operator turned the idle policy on (`MCP_DEBUG_SESSION_IDLE_MS`): how long the tree has before the policy closes it; any tool call naming the session resets it and holds it for as long as the call runs (a slow launch or a long `wait_for_stop` is never closed underneath itself, and the window starts over when it answers), and nothing expires while a client is attending the process. **`parentSessionId`:** present on child rows only. A child session is created by the server, never by `create_debug_session`, when the adapter reports a process the debuggee started: a Python launch or attach — debugpy's sub-process debugging is on by default; `dapLaunchArgs: { subProcess: false }` opts a launch out and an attach target decides for itself with `--configure-subProcess` — where debugpy reports each child, or a JavaScript launch or verified-local attach — js-debug's `autoAttachChildProcesses` is on by default for launch or an inspector target that can read both a temporary server-side probe file and the installed js-debug bootloader files; other attaches default it off with a warning; `dapLaunchArgs: { autoAttachChildProcesses: false }` opts a launch out, `adapterConfig: { autoAttachChildProcesses: false }` an attach — where js-debug reports every Node process the program forks or spawns, `cluster` workers and grandchildren included (see [the session tree](./architecture/session-tree.md)). The child inherits the parent's `language`, its `name` is the adapter's own label for the process (debugpy: `Subprocess `; js-debug: `