Repository navigation
fix: hold a session for every call in flight, and make tree answers speak for the tree (#1127, #1129, #1128, #1137) - #1149
Merged
Conversation
…olicy (#1127) 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 <noreply@anthropic.com>
…a substring (#1129) /session[_-]?id/i matched SESSION_IDLE, so the server's own idle-policy settings (MCP_DEBUG_SESSION_IDLE_MS, MCP_DEBUG_SESSION_IDLE_SWEEP_MS) were displayed as [REDACTED] by dev_server_status and every log line that goes through sanitizeEnvForLogging. The values reached the backend; the operator could not read back which window was in force. The rule now reads the word that follows `session` to its end and masks when it is id, ids, identifier or identifiers: SESSION_ID, sessionIds, SessionIDValue (an acronym ends before the capital that opens the next word), SESSION_IDENTIFIER. A run-on spelling (sessionid, JSESSIONID) has no boundary to say where its words end and is masked whatever follows. The regex the issue suggested, /session[_-]?id(?![a-z])/i, would have unmasked sessionIdValue and SESSION_IDS: under /i the lookahead rejects an upper-case letter too. What changes, measured against the old pattern over 264 spellings: - masked before, shown now: after a boundary, `id` beginning a longer word (SESSION_IDLE, SESSION_IDX, SESSION_IDP, SESSION_IDENTITY, SESSION_IDEA, SESSION_IDEMPOTENT, sessionIdle, session_idx) and the all-caps run-on SESSION_IDVALUE, which cannot be told from SESSION_IDLE; - shown before, masked now: the pair with a separator the old pattern did not know (session.id, SESSION ID). tests/unit/shared/own-settings-stay-readable.test.ts guards the class: every MCP_*, DEBUG_MCP_* and DEV_PROXY_* name in the sources must display its value unless it is listed there as a secret on purpose. /auth/ and /token/ stay broad: no setting of ours trips them today. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…1137) Two answers on a session with children described the launched process alone and misled the caller about the tree. They edit the same sentence and the same per-run record, so they land together. #1128. The documented flow — a breakpoint in the child's code, start the parent, wait — ended with the parent answering "ran to completion (exit code 0) without hitting any breakpoint. Verified but never hit: child.py:12", after a child session had stopped on that line twice. summarizeProgramEnd read the root's own lastStop and breakpoints; a child's stops live on the child's row, which is gone once the child is closed. - A stop the agent can see on a descendant is counted on every ancestor (ManagedSession.descendantStops, per run, reset at launch). Auto-continued entry stops, and js-debug's leaked module-entry pause, are not counted. - The summary gets the rule a single session already has: once the tree stopped, the unhit list is withheld and the sentence counts the stops — "Its child sessions stopped 2 times during this run." All three callers (launch, hot restart, wait_for_stop) go through the one builder. No per-breakpoint attribution: debugpy never sends hitBreakpointIds, so it would have rested on someone having read each stop's location. #1137. A released child left `childrenReleased: 1` and nothing else, while the pending message promised a pause only that child could reach. - adoptReportedChild records why with the count, as a category (adoption-off, cap, unsupported, adoption-failed) and the cap in force — never the free-text reason, which embeds an adapter error. - describeReleasedChildren (tree-settle.ts) builds one sentence from that record, and every launch, restart and wait message that carries the count ends with it: "1 child process was released to run undebugged (child sessions are off: DEBUG_MCP_MAX_CHILD_SESSIONS=0): a breakpoint only it reaches will not hit." The detach answer keeps its own childrenReleased, which counts what the detach itself released. Measured with real debugpy and js-debug before and after: the Python and JavaScript subprocess-tree suites carry the issues' flows, the closed-child one included. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…id rule, scoped waits (#1127, #1129, #1137) An independent review of this branch, run with a shell and a mutation pass, found one important gap and several smaller ones. Each fix here was written against a test that failed first. #1127 was still open on one path. create_debug_session with an attach target attaches inside the call and names no session, so the dispatch had nothing to hold: under an 800 ms window the new session was closed under its own attach ("Created session but attach failed: … Proxy exited during initialization"), reproduced on a real server. - SessionManagerCore.holdForCall is now the one primitive for "a tool call is in flight on this tree": it stamps the clock, holds the tree, and its release stamps again. The dispatch takes it for every tool that names a session; create_debug_session takes it around its inline attach. - The root is captured when the hold is taken, so a call that removes the session it named (closing a child) still restarts the tree's clock. It used to stamp the removed id, a no-op: after a 60 s close the root read 60 s idle. - idleCloseInFlight takes the caller's key. A child's row is removed before its tree's close is over, where the dispatch's scope check can no longer see it, and a stranger naming that id was made to wait — a timing difference from an id that never existed. - The dispatch never waits twice for the same close, so a mark that outlived its close cannot spin it. #1129: the rule fails closed. The word-end rule of the first fix showed SESSION_IDSTR, SESSION_IDHASH, SESSION_IDCOOKIE, sessionIdcookie and SESSION_IDENTIFIERs — names of a session id. The rule now masks whatever follows `session…id`, as the old pattern did, and knows one exception: the word is "idle". The only names shown that were masked before are the idle ones. A second `session…id` later in the same name is read too. #1137 wording: the cap cause is past tense (the answer may come long after the tree was full), "adoption failed" has no pronoun (the count may be plural) and no semicolon (causes are joined with one). Docs: three sentences still named wait_for_stop as the only hold; the "stopped N times" count is every visible stop of a child, not breakpoint hits, and the docs no longer say otherwise. Tests added for the gaps the mutation pass found: a stray sessionId the caller does own, the released cap's reset at relaunch, the error-state answer, a wait that answers for a child while the root has released children, and a real-server case for create+attach. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This was referenced Nov 12, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
debugmcpdev
commented
Nov 13, 2026
debugmcpdev
left a comment
debugmcpdev
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed head 12bd50d against main 785f199, with independent passes over idle holds/close races and owner scoping, session-tree stop/release reporting, and environment redaction. No additional confirmed defects requiring changes were found.
The call hold is acquired before yielding, released in finally, and refreshes the root after a child closes. Inline create-and-attach holds its newly created session. Idle-close markers retain owner scoping after rows disappear. Descendant stop counts survive child removal and reset on relaunch; release explanations use the same record as childrenReleased. The redaction review also exercised 3,024 sensitive-name combinations without unmasking session-ID names.
Verified CI on this exact head: Linux 462 files / 9,171 tests passed (6 skipped); Windows 462 files / 9,172 tests passed (5 skipped). Lint, CodeQL, container, language-host and both MCP conformance jobs succeeded. A fresh local suite run was not completed because the isolated environment could not set up dependencies under its filesystem restrictions.
Codecov's patch check remains in progress; its report states 98.57% patch coverage with two uncovered lines. The known root-only warning and grandchild-release limitations are explicitly tracked in #1150 and are not introduced by this change.
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
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.
Cluster 1 of the bugs gating 0.27.0: four defects in the idle policy (#1025 step 2) and the session tree (#1081/#1082).
Closes #1127, closes #1129, closes #1128, closes #1137.
What was wrong, and what changes
#1127 — the idle policy closed a session underneath its own call
A tool call counted as activity only when it was dispatched, and only
wait_for_stopheld its tree. A call that outlasted the window lost its session mid-flight.The rule now: the idle clock runs from the end of the last call on the tree, and a call in flight holds it.
SessionManagerCore.holdForCallis the one primitive: it stamps the clock, holds the tree, and its release stamps again.tool-dispatch.ts(holdForTheCall) takes it for every tool that names a session, under the gate the stamp already had (the tool takessessionId, the caller may see the session).wait_for_stop's own hold is removed.create_debug_sessionwith an attach target names no session, so it takes the hold itself around its inline attach. The review found this path still open after my first fix.sweepIdlemarks a tree as closing before its firstawait. A call from inside the session's scope that arrives during that close waits for it and answersclosedBy: "idle-policy"with the final outcome. Before, it ran against a session being torn down.idleMsOfanswers 0 while the tree is held, solist_debug_sessions(idleMs,expiresInMs) and/healthno longer show a session with a call in flight as about to expire.#1129 —
MCP_DEBUG_SESSION_IDLE_MSdisplayed as[REDACTED]/session[_-]?id/iis a substring rule andSESSION_IDLEcontainsSESSION_ID.The rule still masks whatever follows
session…id, and now knows one exception: the word that starts atidis "idle". It fails closed, so a name it cannot read stays masked.The regex suggested in the issue,
/session[_-]?id(?![a-z])/i, is not used: under/ithe lookahead also rejects an upper-case letter, sosessionIdValueandSESSION_IDSwould have stopped being masked.Masking changes, measured against the old pattern over 372 spellings:
SESSION_IDLE,SESSION_IDLE_MS,sessionIdle,sessionIdleMs,SESSION_IDLING,sessionidlesession.id,SESSION IDSESSION_IDX,SESSION_IDENTITYandSESSION_IDHASH(still masked)/auth/and/token/stay broad: I scanned the project's own setting names and none trips them. A new guard test (tests/unit/shared/own-settings-stay-readable.test.ts) fails when anyMCP_*,DEBUG_MCP_*orDEV_PROXY_*name in the sources is masked without being listed there as a secret on purpose.#1128 — "without hitting any breakpoint" after a child session stopped on one
summarizeProgramEndread the launched process's own stop record and breakpoints. A child's stops live on the child's row, which is gone once the child is closed.ManagedSession.descendantStops, per run).The program ran to completion (exit code 0). Its child sessions stopped 2 times during this run.#1137 —
childrenReleased: 1with no explanationadoptReportedChildrecords why with the count, as a category (child sessions off, cap reached, proxy cannot host children, adoption failed).wait_for_stopmessage that carries the count ends with one sentence built from that record:1 child process was released to run undebugged (child sessions are off: DEBUG_MCP_MAX_CHILD_SESSIONS=0): a breakpoint only it reaches will not hit.At the cap it reads(the tree was at its child-session cap of 32, which DEBUG_MCP_MAX_CHILD_SESSIONS raises).Behaviour a client can observe
idleMsis0while a call on the session runs, and counts from the call's answer afterwards.closedBy: "idle-policy".unhitBreakpointsis absent from a session's end answer when a child session stopped during the run.childrenReleasedare one sentence longer. No field is removed or reshaped; the tool list is unchanged at 29.Evidence
Each new test was run failing before its fix.
python-idle-policy.test.ts, 800 ms window)start_debuggingansweredSession not foundfor its own sessioncreate_debug_sessionwith an attach target, before its fixPauseTest.java) after the fixidleMs=2 expiresInMs=998; 3 s laterclosedBy: "idle-policy"The program ran to completion (exit code 0). Its child sessions stopped 2 times during this run.DEBUG_MCP_MAX_CHILD_SESSIONS=0typecheck:all,lint,check-docs,changelog:checkThe first push was refused by the hook: one Go case timed out on Delve's first build under the hook's load, the known flake #889. The file passed alone (3 of 3) and the second push passed with no change.
#1129 could not be checked through the already-running dev proxies (they loaded the old sanitizer at start); the unit test at
backend-logger.mjscovers the surface the issue shows, assertingmode: 'shared'.No test ran the idle policy against a real server before this PR; that is how #1127 got through.
tests/adapters/python/integration/python-idle-policy.test.tsis the first.Review
One independent review with a shell: it ran the suites and a 34-mutation pass. No Critical finding. Fixed in the last commit:
create_debug_sessionwith an attach target was still swept under its own attach.SESSION_IDSTR,SESSION_IDHASHand similar names. It now fails closed.wait_for_stopas the only hold; a doc sentence that called every child stop a breakpoint hit.Not fixed here, in #1150: the "never bound" warning beside the new sentences, and a grandchild released under a child that the root's answers do not count.
Calls I made
sweepIdledoes not also consult the in-flight guard: every claim on it is made inside a tool call, so it is always under a hold.hitBreakpointIds, so attribution would have depended on someone having read each stop's location. Cost: after a child stopped anywhere, the parent's end answer no longer lists a breakpoint in the parent's own code that was never hit. The count is of every stop the agent could see on a child, not of breakpoint hits alone.SESSION_IDENTITYandSESSION_IDXmasked. Only "idle" is exempt.Known limits (follow-up issue #1150)
childrenlist shows a child already paused there. Seen live in the Session tree: the parent's exit summary says 'without hitting any breakpoint. Verified but never hit: child.py:12' after a child session stopped there #1128 flow.unhitBreakpoints[].verifiedis the launched process's own flag.subProcess: falseopt-out still ends with "Verified but never hit: child.py:12" (the comment on Session tree: the parent's exit summary says 'without hitting any breakpoint. Verified but never hit: child.py:12' after a child session stopped there #1128).childrenReleasedcounts releases on the answering session's own connection, so a grandchild released under a child is not counted or explained on the root's answers.🤖 Generated with Claude Code