Skip to content

Commit 141f49d

Browse files
debugmcpdevcynarlabclaude
authored
fix(launch): answer a launch that never stops as pending instead of sitting out the 30 s ceiling (#815) (#825)
* fix(launch): answer a launch that never stops as pending instead of sitting out the 30 s ceiling (#815) For every adapter but js-debug, start_debugging on a launch with nothing armed to stop it (no breakpoints, no entry stop, a server or a long loop) waited the full 30 s readiness ceiling and then answered `running` with no word about the wait: those policies count only a pause as ready, and the real proxy reports adapter-configured synchronously inside start(), before the wait can listen, so the wait settled only on a stop, an exit or the ceiling. Measured on main: Ruby long_running.rb ~27 s, Python pause_test.py 32.6 s; a breakpoint in the readiness wait's handleConfigured never fired. - launch-arming.ts: describeLaunchArming(session, stopOnEntry) — armed when pausing line breakpoints (a logpoint counts unless the adapter advertises supportsLogPoints), function breakpoints, stopOnEntry or breakOnExceptions 'all' are set. 'uncaught' does not count: it is the launch default on most policies and a crash is not a stop that comes soon. - launch-readiness.ts: the ceiling is the caller's (ceilingMs) and the wait reports how it settled ('stopped' | 'configured' | 'ended' | 'ceiling' | 'already-terminal'); the "adapter may have failed" warning is logged only when the session is still initializing at the ceiling. - debug-launcher.ts: armed → launchReadyCeilingMs (30 s), unarmed → launchGraceMs (5 s), both SessionManagerOperations tunables; a ceiling with the session RUNNING answers data.pending: true and ErrorMessages.launchStillRunning — what was not reached, or why no stop is coming and what to do. restart_debugging inherits it. The stale "python/go/cpp always request an entry stop" comment is corrected. - debuggee-tools.ts: start_debugging and restart_debugging hoist pending to the top level like the step tools (#598). Not changed: js-debug's ready-on-RUNNING (issue #823), and the two no-isSessionReady fallbacks (mock/default only). After (real rdbg, tests/adapters/ruby/integration): the unarmed launch answers running + pending in ~7 s including spawn; a launch with a breakpoint still answers paused/breakpoint. Closes #815 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(launch): tell the tool description about pending; review follow-ups (#815) Review of #825: - The LLM-facing start_debugging description said only "Start debugging a script"; it now says what the launch answers with (paused with a reason, stopped with the exit code, or running + pending:true after ~5 s unarmed / 30 s armed) the way every other pending-capable tool's description does; restart_debugging's says it answers the same way. Tool-list snapshots re-recorded (24). - docs/architecture/api-reference.md: startDebugging/restartDebugging return shapes mention the readiness window and data.pending. - CLAUDE.md's launch-collaborator line names launch-arming.ts and the ceiling outcome. - The unarmed message no longer hedges on "where that filter is on": C/C++ pauses on an uncaught throw through SIGABRT with no filter, so it says "an uncaught exception the debugger catches by default". - The handler comment cites the step tools' pending contract as #144 and attach's as #598, not #598 for both; the readiness test header no longer claims the module had no tests. - changelog.d/815.changed.md → 815.fixed.md: the repo files fix(...) commits under fixed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(server): pin restart_debugging's top-level pending hoist (#815) The start_debugging hoist had a test; the restart mirror did not. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: JF <john.franklin@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 9f9bd71 commit 141f49d

45 files changed

Lines changed: 644 additions & 153 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ The codebase follows a **layered architecture with dependency injection** and **
238238
- `SessionManager` (`session-manager.ts`): Final composition class that extends SessionManagerOperations. Implements `handleAutoContinue(sessionId)` which auto-continues past entry breakpoints when `stopOnEntry=false`
239239
- `SessionManagerOperations` is itself a facade over per-slice collaborators, wired as protected fields and always reached at call time (`this.breakpoints.…`):
240240
- `launch/proxy-launcher.ts` (`ProxyLauncher.start(session, request)`, the one proxy launch both launch and attach go through): adapter creation under an `AdapterLease`, the configuration transform, executable resolution, `ProxyConfig` assembly; `launch/proxy-failure-diagnostics.ts` (`failProxySetup`/`logProxyFailure`): the shared failure teardown and record
241-
- `launch/debug-launcher.ts` (`DebugLauncher`): `startDebugging` (dry runs included), `restartDebugging`; `launch/launch-readiness.ts` (`waitForLaunchReadiness`): the post-handshake wait for the first stop / configured / termination
241+
- `launch/debug-launcher.ts` (`DebugLauncher`): `startDebugging` (dry runs included), `restartDebugging`; `launch/launch-readiness.ts` (`waitForLaunchReadiness`): the post-handshake wait for the first stop / configured / termination / the caller's ceiling, reported as an outcome; `launch/launch-arming.ts` (`describeLaunchArming`): what the launch has armed that could stop it soon, which picks the ceiling (30 s armed, 5 s unarmed; issue #815) and words the `pending: true` answer
242242
- `attach/attach-controller.ts` (`AttachController`): `attachToProcess`, `detachFromProcess`; `attach/attach-verification.ts` (`verifyAttachThreads`, `pauseAfterAttach`): the thread poll and post-attach pause that make PAUSED real before it is reported
243243
- `breakpoints/breakpoint-controller.ts` (`BreakpointController`): set/remove/clear plus the replace-all DAP re-sends
244244
- `breakpoints/anchor-resolution.ts` (`reresolveAnchors`) and `breakpoints/launch-warnings.ts` (pure warning builders)

‎changelog.d/815.fixed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
**`start_debugging` no longer sits out a 30 s ceiling on a launch that never stops** — for every adapter but js-debug, a launch with nothing armed to stop it (no breakpoints, no entry stop, a server or a long loop) waited the full 30 s readiness ceiling and then answered `state: "running"` with no word about the wait: those policies count only a pause as ready, and the proxy reports its configured state before the wait can listen. The wait now depends on what the launch has armed: with breakpoints (a logpoint counts when the adapter does not run it on), function breakpoints, `stopOnEntry` or `breakOnExceptions: "all"`, the first stop still gets the full ceiling, so a launch that reaches a breakpoint keeps answering `paused` with its reason; with nothing armed (the `"uncaught"` default does not count — a crash is not a stop that comes soon) the wait is a 5 s grace window. Either way, when the ceiling passes with the program still running the response says so — `pending: true` at the top level like the step tools, and a message naming what was not reached ("still running after 30s without reaching 2 breakpoint(s)") or why no stop is coming soon and what to do instead — and `restart_debugging` answers the same way. The "adapter may have failed to start" warning is logged only when the session is still initializing at the ceiling (#815)

‎docs/architecture/adapter-policy-pattern.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ export interface AdapterPolicy {
9292
getLocalScopeName?(): string | string[];
9393

9494
// === Session readiness (optional) ===
95+
// Whether a launch is reportable in this state. Every policy but js-debug answers PAUSED only,
96+
// so the launcher waits for the first stop — for the full ceiling when something is armed to
97+
// stop the program, for a short grace window when nothing is (issue #815; launch-arming.ts).
9598
isSessionReady?(state: SessionState, options: { stopOnEntry?: boolean }): boolean;
9699

97100
// === Non-file source identifiers (optional, e.g. Java FQCNs) ===

‎docs/architecture/api-reference.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,10 @@ startDebugging(
314314

315315
Delegates to `DebugLauncher` (`src/session/launch/debug-launcher.ts`), which owns the launch sequence end to end.
316316

317-
**Returns**: `DebugResult` — `{ success, state, error?, data?, canContinue?, errorType?, errorCode? }`. For a dry run, `data` carries `dryRun`, `command` and `script`. A dry run leaves the session lifecycle-TERMINATED but present; `startDebugging` accepts a present session in any lifecycle state — `DebugLauncher.launch` tears down any leftover proxy and resets the per-launch state (issue #793).
317+
**Returns**: `DebugResult` — `{ success, state, error?, data?, canContinue?, errorType?, errorCode? }`. For a dry run, `data` carries `dryRun`, `command` and `script`. A dry run leaves the session lifecycle-TERMINATED but present; `startDebugging` accepts a present session in any lifecycle state — `DebugLauncher.launch` tears down any leftover proxy and resets the per-launch state (issue #793). The result reflects the session after its first stop, its exit, or the readiness window (`launchReadyCeilingMs` when breakpoints, an entry stop or `breakOnExceptions: 'all'` are armed, `launchGraceMs` otherwise — `describeLaunchArming` in `launch/launch-arming.ts`); when that window passes with the program still running, `state` is `RUNNING` and `data.pending` is `true` with `data.message` saying what was or was not reached (issue #815).
318318

319319
#### `restartDebugging(sessionId: string): Promise<DebugResult>`
320-
Replays the session's last real launch and re-applies breakpoints (content anchors are re-resolved). Launch-mode sessions only. Exposed as the `restart_debugging` MCP tool.
320+
Replays the session's last real launch and re-applies breakpoints (content anchors are re-resolved). Launch-mode sessions only. Exposed as the `restart_debugging` MCP tool. Goes through the same readiness window as `startDebugging`, so a relaunch still running when it passes answers `RUNNING` with `data.pending: true` (issue #815).
321321

322322
#### `setBreakpoint(sessionId: string, bp: { file: string; line: number; condition?: string; logMessage?: string; suspendPolicy?: 'all' | 'thread'; requestedLine?: number; anchor?: { statement: string; nearLine?: number } }): Promise<{ breakpoint: Breakpoint; warning?: string }>`
323323
Sets a breakpoint in a file. Internally sends a DAP `setBreakpoints` request for all breakpoints in the same source file. `requestedLine` records the originally requested line for loud snapping; `anchor` stores the content anchor that `restartDebugging` re-resolves (issue #271); `warning` carries live-sync failures. Neither extra field ever enters the DAP payload. Accepts a session in any lifecycle state; with no live debuggee the breakpoint is stored (`verified: false`, no warning) and applied on the next launch (issue #806).

‎docs/patterns/event-management.md‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -347,14 +347,19 @@ proxyManager.on('exit', onExit);
347347
**Location**: `src/session/launch/launch-readiness.ts` (`waitForLaunchReadiness`)
348348

349349
```typescript
350-
// Wait for the adapter to be configured, the first stop event, or termination.
351-
// Readiness can be satisfied by: stopped, adapter-configured, terminated,
352-
// exited, or exit. The wait never rejects — every outcome resolves, including
353-
// the 30s ceiling, so the caller reports whatever state the session is in
354-
// rather than failing a launch that is merely slow.
350+
// Wait for the adapter to be configured, the first stop event, termination,
351+
// or the ceiling. Readiness can be satisfied by: stopped, adapter-configured,
352+
// terminated, exited, or exit. The wait never rejects — every outcome resolves
353+
// with a word on how it settled ('stopped' | 'configured' | 'ended' |
354+
// 'ceiling' | 'already-terminal'), so the caller reports the session as it is
355+
// and says "still running" for a ceiling rather than failing a launch that is
356+
// merely slow. The ceiling is the caller's (issue #815): the full 30 s when
357+
// something is armed to stop the program, a short grace window when nothing
358+
// is — every policy but js-debug is ready only on a pause, and the real proxy
359+
// reports adapter-configured before this wait can listen.
355360
// `session.proxyManager` is re-read on every access: a terminal event handler
356361
// may null it while the wait is in flight.
357-
return new Promise<void>((resolve) => {
362+
return new Promise<LaunchReadinessOutcome>((resolve) => {
358363
let resolved = false;
359364
let timeoutId: ReturnType<typeof setTimeout> | undefined;
360365

@@ -367,18 +372,23 @@ return new Promise<void>((resolve) => {
367372
session.proxyManager?.removeListener('exit', handleExit);
368373
};
369374

370-
const handleStopped = () => {
371-
if (!resolved) { resolved = true; cleanup(); resolve(); }
375+
const settle = (outcome: LaunchReadinessOutcome, narration: string) => {
376+
if (resolved) return;
377+
resolved = true; cleanup();
378+
ctx.logger.info(`[SessionManager] Session ${sessionId} ${narration}`);
379+
resolve(outcome);
372380
};
381+
382+
const handleStopped = () => settle('stopped', 'stopped on entry');
373383
const handleConfigured = () => {
374384
// The adapter policy decides whether "configured and running" counts as
375385
// ready; without one, it does unless the caller asked to stop on entry.
376386
const readyOnRunning = policy.isSessionReady
377387
? policy.isSessionReady(SessionState.RUNNING, { stopOnEntry: dapLaunchArgs?.stopOnEntry })
378388
: !dapLaunchArgs?.stopOnEntry;
379-
if (!resolved && readyOnRunning) { resolved = true; cleanup(); resolve(); }
389+
if (readyOnRunning) settle('configured', `running (stopOnEntry=${dapLaunchArgs?.stopOnEntry ?? false})`);
380390
};
381-
// handleTerminated / handleExited / handleExit follow the same shape.
391+
// handleTerminated / handleExited / handleExit settle 'ended'.
382392

383393
// Checked BEFORE any listener is registered: the caller decided readiness
384394
// synchronously just before this call and nothing has been awaited since,
@@ -387,7 +397,7 @@ return new Promise<void>((resolve) => {
387397
const currentState = ctx.getSession(sessionId).state;
388398
if (currentState === SessionState.STOPPED || currentState === SessionState.ERROR) {
389399
resolved = true;
390-
resolve();
400+
resolve('already-terminal');
391401
return;
392402
}
393403

@@ -397,17 +407,24 @@ return new Promise<void>((resolve) => {
397407
session.proxyManager?.once('exited', handleExited);
398408
session.proxyManager?.once('exit', handleExit);
399409

400-
// Ceiling after 30 seconds: log and resolve, never reject.
410+
// The ceiling: a program that is running has simply not stopped — the caller
411+
// answers pending: true. A session still initializing here is the adapter not
412+
// answering, which is what the warning has always meant. Resolve, never reject.
401413
timeoutId = setTimeout(() => {
402-
if (!resolved) {
403-
resolved = true; cleanup();
404-
ctx.logger.warn(ErrorMessages.adapterReadyTimeout(30));
405-
resolve();
414+
if (resolved) return;
415+
resolved = true; cleanup();
416+
if (session.state === SessionState.RUNNING || session.state === SessionState.PAUSED) {
417+
ctx.logger.info(`[SessionManager] Session ${sessionId} still running after the ${ceilingMs / 1000}s readiness ceiling; answering with pending`);
418+
} else {
419+
ctx.logger.warn(ErrorMessages.adapterReadyTimeout(ceilingMs / 1000));
406420
}
407-
}, 30000);
421+
resolve('ceiling');
422+
}, ceilingMs);
408423
});
409424
```
410425

426+
The launcher (`src/session/launch/debug-launcher.ts`) picks `ceilingMs` from `describeLaunchArming(session, stopOnEntry)` (`launch-arming.ts`): `launchReadyCeilingMs` (30 s) when line/function breakpoints, a pausing logpoint, `stopOnEntry` or `breakOnExceptions: 'all'` are armed, `launchGraceMs` (5 s) otherwise — both `SessionManagerOperations` tunables. A `'ceiling'` outcome with the session `RUNNING` becomes `data.pending: true` and `ErrorMessages.launchStillRunning(...)` on the launch response, hoisted to the top level by the `start_debugging`/`restart_debugging` handlers.
427+
411428
## Testing Event Patterns
412429

413430
### Testing Event Emissions

‎docs/tool-reference.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,12 @@ Starts debugging a script.
390390
}
391391
```
392392

393+
**When the launch answers:** after the adapter's handshake, `start_debugging` waits for the program's first stop, its exit, or a ceiling, and reports the session as it is at that moment (issue #815):
394+
395+
- **A stop arrives** — `state: "paused"` with `reason` (the example above), which is why a launch with breakpoints set answers "paused at the first breakpoint" for every adapter but js-debug (js-debug counts a configured, running program as ready when no entry stop was requested, and answers `running` right away — see issue #823).
396+
- **The program ends first** — `state: "stopped"` with the run-to-completion summary and `exitCode` below.
397+
- **Neither, within the ceiling** — `state: "running"` with **`pending: true`** (top level and in `data`, like `step_over`) and a `message` that says so. The ceiling depends on what the launch has armed: with line breakpoints (a logpoint counts when the adapter does not run it on), function breakpoints, `stopOnEntry` or `breakOnExceptions: "all"`, the first stop gets 30 s and the message names what was not reached (`The program is still running after 30s without reaching 2 breakpoint(s)…`); with nothing armed — the `"uncaught"` default does not count, a crash is not a stop that comes soon — the wait is a 5 s grace window and the message says why no stop is coming (`…nothing is armed to stop it soon (no breakpoints, no entry stop, no caught-exception filter)…`) and what to do (`list_debug_sessions`, `get_output`, or set breakpoints and `restart_debugging`). A server or a long-running script therefore answers in about 5 s instead of 30. The stop, if one comes later, is reported by `list_debug_sessions` as the state flips to `paused`.
398+
393399
**JavaScript launch configuration:** Both launch option objects accept js-debug settings;
394400
`adapterLaunchConfig` wins when a key appears in both. The effective `stopOnEntry` value controls
395401
the entry pause and is replayed by `restart_debugging`. An honoured `noDebug` disables that pause.
@@ -465,6 +471,7 @@ Restarts the debuggee in one call: terminates the current program (if still runn
465471
- Restart is implemented uniformly as terminate + relaunch (the DAP-spec-blessed emulation; no adapter advertises native restart), so every launch-mode language works identically. Native DAP `restart` is a possible future optimization.
466472
- The launch configuration is replayed verbatim (script, args, `dapLaunchArgs`, `adapterLaunchConfig`, `breakOnExceptions`); there are no per-restart overrides — call `start_debugging` for a different configuration.
467473
- **The output buffer starts fresh**: `outputReset: true` signals that `get_output` cursors from the previous launch are stale — read from `since: 0`.
474+
- The readiness wait and its `pending: true` answer are the same as `start_debugging`'s ("When the launch answers" above): a relaunch with nothing armed answers `running` after the short grace window.
468475
- Not available for **attach sessions** (no launch configuration to replay — detach and re-attach instead) or for sessions that were never launched, including dry-run-only sessions — call `start_debugging` on the same session instead.
469476

470477
---

‎skills/debugging/SKILL.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Rules that prevent 90% of failed sessions:
3939
- **Use real frame IDs.** Take `id` from `get_stack_trace` frames; it is adapter-assigned and is not 0-indexed.
4040
- **Expand variable containers.** If a variable entry carries a `variablesReference`, call `get_variables` again with that reference to see children (Python's "special variables", object fields, array elements).
4141
- **Respect session state.** Stepping, evaluation, and variable reads require `PAUSED`. After `continue_execution` the session is `RUNNING`; after a step or breakpoint hit it returns to `PAUSED` with a persisted stop reason telling you why it stopped (`breakpoint`, `step`, `entry`, `exception`, ...).
42+
- **A `start_debugging` that answers `running` with `pending: true` is not stuck.** The program did not stop within the readiness window (30 s with breakpoints or an entry stop armed, 5 s with nothing armed — a server, a long loop); the message says what was or was not armed. Check `list_debug_sessions` for the state to flip to `paused`, read `get_output`, or set breakpoints and `restart_debugging`.
4243
- **Breakpoints may verify late.** Some adapters (debugpy, JDI) report breakpoints unverified until the module/class loads; that is normal, not an error.
4344
- **`<redacted:...>` placeholders are masking, not program state.** Credential-shaped values and values of sensitive variable names (`password`, `api_key`, ...) are masked by default in variable/evaluate/output results; a `redaction` field reports what was hidden. The real value is intact in the debuggee — don't "fix" it, and don't retry the read. The user can disable masking by restarting the server with `DEBUG_MCP_NO_REDACT=1`.
4445
- **If `get_variables` demands `names`, the server is in least-privilege mode** (`DEBUG_MCP_VARIABLE_ACCESS=explicit`): pass the exact variable names you need (`names: ["user", "total"]`; case-sensitive, misses reported in `notFound`) instead of dumping the scope. `evaluate_expression` still works for targeted reads.

‎src/server/handlers/debuggee-tools.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ export const startDebuggingTool: ToolHandler = async (ctx, args) => {
3737
if (debugResult.data) {
3838
responsePayload.data = debugResult.data;
3939
}
40+
// `pending` is part of the public tool-result contract, as for the step
41+
// tools (issue #144) and attach_to_process (issue #598): a launch answered
42+
// while the program still runs — nothing armed stopped it within the
43+
// readiness window (issue #815) — is visible at the top level.
44+
if (debugResult.data?.pending) {
45+
responsePayload.pending = true;
46+
}
4047
// Top-level warning join (set_breakpoint pattern): intake
4148
// normalization notes (issue #305) plus any session-manager
4249
// warning (unbound function breakpoints, issue #308).
@@ -74,6 +81,11 @@ export const restartDebuggingTool: ToolHandler = async (ctx, args) => {
7481
if (restartWarning) {
7582
responsePayload.warning = restartWarning;
7683
}
84+
// A relaunch that is still running at the readiness ceiling (issue
85+
// #815): same top-level pending as start_debugging.
86+
if (debugResult.data.pending) {
87+
responsePayload.pending = true;
88+
}
7789
}
7890
return jsonResult(responsePayload);
7991
} catch (error) {

0 commit comments

Comments
 (0)