Skip to content

Commit e5b6d5c

Browse files
debugmcpdevcynarlabclaude
authored
fix(#378,#379): skip child-event settle for non-child adapters; surface weak anchor matches (#380)
#378: waitForChildEventFlush paid an unconditional 150ms settle per terminal signal on every adapter — the typeof guard was dead code because MinimalDapClient defines flushChildEvents unconditionally, and terminal slots chain, so a normal teardown paid 300-450ms of pure sleep on python/ruby/go/java/dotnet/rust/cpp/mock. flushChildEvents now returns undefined when no childSessionManager exists and the worker skips the flush-and-settle entirely on an empty first round; the js-debug two-round + settle path is preserved byte-for-byte (#366 stays fixed). #379: the #367 substring/comment-stripped relaxation of expectedContent and statement anchors silently weakened the "cannot land on the wrong line" guarantee — a substring pass was indistinguishable from an exact match, and restart re-resolution (nearLine always supplied) could snap a generic anchor among multiple matches without a trace. Matching behavior is unchanged; the signal is restored: - assertLineContent reports matchQuality (exact/substring/comment-stripped); set_breakpoint warns on non-exact matches, quoting the actual line - resolveStatement reports the candidate list when nearLine picked among multiple matches; set_breakpoint and restart_debugging surface it (anchorResolution.moved[].candidates + warning) - statement+expectedContent compatibility gate aligned with the relaxed predicate (trailing comments ignored, either side may be a substring) - tool schema, skill content, and docs reworded to the true guarantee ("only lands on a line containing your text") and the stale whole-line equality bullet in tool-reference fixed Verified: 3647 unit tests green, e2e smoke 76/76, live python teardown terminal chain ~1ms (was 300ms+), live js-debug run-to-completion output capture intact, live substring warning confirmed. Co-authored-by: JF <john.franklin@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent a0af3bf commit e5b6d5c

15 files changed

Lines changed: 330 additions & 54 deletions

‎docs/tool-reference.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,10 @@ Sets a breakpoint in a source file.
141141
- `sessionId` (string, required): The ID of the debug session.
142142
- `file` (string, required): Path to the source file (absolute or relative to project root).
143143
- `line` (number, required): Line number where to set breakpoint (1-indexed).
144-
- `statement` (string, optional): **Content addressing** — instead of `line`, pass the text of the target line, like an Edit-tool match — a distinctive substring is enough (leading/trailing whitespace and trailing `//`/`#` comments are ignored; an exact whole-line match always wins over substring matches). Cannot land on the wrong line; if the text appears on multiple lines the error lists every match; anchors re-resolve across `restart_debugging` after file edits. Provide `statement` OR `line`, not both. See [Statement anchors](#statement-anchors).
144+
- `statement` (string, optional): **Content addressing** — instead of `line`, pass the text of the target line, like an Edit-tool match — a distinctive substring is enough (leading/trailing whitespace and trailing `//`/`#` comments are ignored; an exact whole-line match always wins over substring matches). Can only land on a line containing your stated text — an inexact or multi-candidate match still sets the breakpoint but adds a `warning` to the response; if the text appears on multiple lines the error lists every match; anchors re-resolve across `restart_debugging` after file edits. Provide `statement` OR `line`, not both. See [Statement anchors](#statement-anchors).
145145
- `function` (string, optional): **Symbol addressing** — break on entry to a function/method by name (DAP function breakpoint). Session-global: no `file` or `line` at all, and names survive edits better than both. Composes with `condition` only. Supported by Python, Go, Rust, C/C++, .NET, Java, and JavaScript (JavaScript names are dotted runtime paths delivered over the CDP bridge — see [Function breakpoints](#function-breakpoints)); Ruby is accepted with a warning and validated at launch.
146146
- `nearLine` (number, optional): With `statement` only — when the statement text appears on multiple lines, bind to the match closest to this line (ties go to the lower line).
147-
- `expectedContent` (string, optional): With `line` only — assert the text of the target line before setting; a distinctive substring is enough (leading/trailing whitespace and trailing `//`/`#` comments are ignored). On a mismatch the breakpoint is **not** set and the error shows the actual content of that line and its neighbors — a fast, self-explanatory failure instead of a breakpoint that silently lands on the wrong line. See [Content assertions and loud snapping](#content-assertions-and-loud-snapping).
147+
- `expectedContent` (string, optional): With `line` only — assert the text of the target line before setting; a distinctive substring is enough (leading/trailing whitespace and trailing `//`/`#` comments are ignored). On a mismatch the breakpoint is **not** set and the error shows the actual content of that line and its neighbors — a fast, self-explanatory failure instead of a breakpoint that silently lands on the wrong line. A relaxed match (substring, or one that only holds after ignoring text past a `//`/`#` marker) still sets the breakpoint but adds a `warning` quoting the actual line. See [Content assertions and loud snapping](#content-assertions-and-loud-snapping).
148148
- `condition` (string, optional): Conditional expression — only break (or log) when it evaluates truthy.
149149
- `logMessage` (string, optional): Create a **logpoint** instead of a pausing breakpoint — see [Logpoints](#logpoints) below.
150150
- `suspendPolicy` (string, optional): Suspend policy when the breakpoint is hit — `"all"` suspends all threads (default), `"thread"` suspends only the event thread. Only supported by the Java/JDI adapter.
@@ -207,12 +207,12 @@ Relatedly, when a debug adapter *accepts* a breakpoint but binds it to a differe
207207
{ "sessionId": "...", "file": "/abs/app.py", "statement": "total = sum(prices)" }
208208
```
209209

210-
- **Matching**: whole-line equality after trimming leading/trailing whitespace. Multi-line input is rejected — anchor on the first line of a multi-line construct.
211-
- **Ambiguity is an error, and the error is the disambiguation UI**: every matching `line: content` pair is listed (capped at 20); add `nearLine` to bind to the closest match.
210+
- **Matching**: whole-line equality after trimming leading/trailing whitespace; when no exact match exists anywhere, a substring match against comment-stripped lines is accepted (exact matches always win — the two populations never mix). Multi-line input is rejected — anchor on the first line of a multi-line construct.
211+
- **Ambiguity is an error, and the error is the disambiguation UI**: every matching `line: content` pair is listed (capped at 20); add `nearLine` to bind to the closest match. A `nearLine` pick among multiple matches adds a `warning` naming every candidate line — proximity is a heuristic, not a guarantee.
212212
- **Blank/comment anchors are rejected** (`#`, `//`, `/*` prefixes) — debuggers cannot break there reliably.
213-
- **The anchor is stored on the breakpoint record** (visible in `list_breakpoints`) and **re-resolves on `restart_debugging`**: after you edit the file — the whole point of a debug session — the relaunch re-finds each anchored statement in the current file (the breakpoint's previous line breaks ties between duplicates). Moves are reported in the restart response's `data.anchorResolution.moved`; anchors that no longer match keep their previous line and warn (`data.anchorResolution.stale`) rather than failing the restart or dropping state.
213+
- **The anchor is stored on the breakpoint record** (visible in `list_breakpoints`) and **re-resolves on `restart_debugging`**: after you edit the file — the whole point of a debug session — the relaunch re-finds each anchored statement in the current file (the breakpoint's previous line breaks ties between duplicates). Moves are reported in the restart response's `data.anchorResolution.moved` — a move picked among multiple matching lines carries a `candidates` list and a restart `warning`; anchors that no longer match keep their previous line and warn (`data.anchorResolution.stale`) rather than failing the restart or dropping state.
214214
- Same readable-file requirement as `expectedContent` (no Java FQCNs, no attach sessions); composes with `condition`, `logMessage`, and `suspendPolicy` unchanged.
215-
- A matching `expectedContent` alongside `statement` is accepted as redundant; a *different* one is an error (contradictory intent).
215+
- A matching `expectedContent` alongside `statement` is accepted as redundant (matching under the same relaxed rules: trailing comments ignored, either value may be a substring of the other); a genuinely *different* one is an error (contradictory intent).
216216

217217
#### Function breakpoints
218218

‎skills/debugging/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ Rules that prevent 90% of failed sessions:
4848

4949
1. State a hypothesis about where reality diverges from expectation *before* setting breakpoints.
5050
2. Set at most two breakpoints: last-known-good and first-known-bad. Run, inspect, halve the interval. Bisection beats stepping line-by-line from the top. Move the window mid-session with `remove_breakpoint` / `clear_breakpoints`; `list_breakpoints` shows what is currently set (with verified state and adapter ids).
51-
- Prefer `statement: "<line text>"` over line numbers: it matches like an Edit-tool `old_string` (whole line or a distinctive substring — whitespace-trimmed, trailing comments ignored, exact matches win), cannot land on the wrong line, lists every occurrence on ambiguity (add `nearLine` to pick one), and re-resolves across `restart_debugging` after you edit the file. When you do address by line, pass `expectedContent: "<line text or distinctive substring>"` (trailing comments ignored) so a stale or off-by-one line number fails immediately with the actual nearby lines. A response saying `requested line N, bound to line M` means the adapter moved the breakpoint — trust the bound line.
51+
- Prefer `statement: "<line text>"` over line numbers: it matches like an Edit-tool `old_string` (whole line or a distinctive substring — whitespace-trimmed, trailing comments ignored, exact matches win), only lands on a line containing your text (inexact or multi-candidate matches are flagged in the response `warning`), lists every occurrence on ambiguity (add `nearLine` to pick one), and re-resolves across `restart_debugging` after you edit the file. When you do address by line, pass `expectedContent: "<line text or distinctive substring>"` (trailing comments ignored) so a stale or off-by-one line number fails immediately with the actual nearby lines. A response saying `requested line N, bound to line M` means the adapter moved the breakpoint — trust the bound line.
5252
- `function: "name"` breaks on entry to a symbol with no file or line at all — names survive edits best. Supported by Python/Go/Rust/.NET/Java/JavaScript (Java accepts bare `method`, `Class.method`, or fully-qualified names and binds every concrete overload; JavaScript names are dotted runtime paths like `obj.method` bound to the current function value — main-module function declarations bind at launch, functions in lazily-loaded modules bind at the next pause).
5353
3. When pausing is too disruptive (hot loops, live or attached processes), use a **logpoint**: `set_breakpoint` with `logMessage: "x={x}"` streams interpolated values into `get_output` without stopping the program (Python/JS/Go/Rust; Java and .NET reject it with a clear error).
5454
4. At each pause, record what you *learned* (variable values, actual control flow), not just where you are.

‎src/proxy/dap-proxy-interfaces.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,10 @@ export interface IDapClient {
176176
/**
177177
* Resolves once all child-session events enqueued so far (output included)
178178
* have been forwarded (issue #366). Optional: only MinimalDapClient
179-
* (js-debug child sessions) implements it; absent means nothing to flush.
179+
* implements it. Absent method or an undefined return both mean the client
180+
* has no child sessions — nothing to flush (issue #378).
180181
*/
181-
flushChildEvents?(): Promise<void>;
182+
flushChildEvents?(): Promise<void> | undefined;
182183
}
183184

184185
/**

‎src/proxy/dap-proxy-worker.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,14 +1607,16 @@ export class DapProxyWorker {
16071607
* wedged chain cannot hang shutdown.
16081608
*/
16091609
private async waitForChildEventFlush(): Promise<void> {
1610-
if (typeof this.dapClient?.flushChildEvents !== 'function') {
1610+
// An undefined return means the client has no child sessions (issue
1611+
// #378): every adapter shares MinimalDapClient, so the method exists
1612+
// everywhere and only its return value tells child-session adapters
1613+
// apart. Skipping here — not inside the rounds — is what avoids the
1614+
// settle for python/ruby/go/etc.
1615+
const firstFlush = this.dapClient?.flushChildEvents?.();
1616+
if (!firstFlush) {
16111617
return;
16121618
}
1613-
const boundedFlush = async (): Promise<void> => {
1614-
const flush = this.dapClient?.flushChildEvents?.();
1615-
if (!flush) {
1616-
return;
1617-
}
1619+
const boundedFlush = async (flush: Promise<void>): Promise<void> => {
16181620
let timer: NodeJS.Timeout | undefined;
16191621
const backstop = new Promise<void>(resolve => {
16201622
timer = setTimeout(resolve, 2000);
@@ -1630,10 +1632,13 @@ export class DapProxyWorker {
16301632
// Two rounds with a short settle: the parent session's terminated can
16311633
// arrive while child output is still UNREAD in the child socket (not on
16321634
// the chain yet). The settle lets those bytes be read and enqueued; the
1633-
// second flush drains them. Only paid by child-session adapters.
1634-
await boundedFlush();
1635+
// second flush drains them.
1636+
await boundedFlush(firstFlush);
16351637
await new Promise(resolve => setTimeout(resolve, 150));
1636-
await boundedFlush();
1638+
const secondFlush = this.dapClient?.flushChildEvents?.();
1639+
if (secondFlush) {
1640+
await boundedFlush(secondFlush);
1641+
}
16371642
}
16381643

16391644
/**

‎src/proxy/minimal-dap.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,13 @@ export class MinimalDapClient extends EventEmitter {
129129

130130
/**
131131
* Resolves once all child events enqueued so far (output included) have
132-
* been forwarded to this client's listeners (issue #366). No-op for
133-
* adapters without child sessions.
132+
* been forwarded to this client's listeners (issue #366). Returns
133+
* undefined for adapters without child sessions so callers can skip the
134+
* flush-and-settle entirely (issue #378) — every adapter shares this
135+
* client class, so the method's mere presence says nothing.
134136
*/
135-
flushChildEvents(): Promise<void> {
136-
return this.childSessionManager?.flushEvents() ?? Promise.resolve();
137+
flushChildEvents(): Promise<void> | undefined {
138+
return this.childSessionManager?.flushEvents();
137139
}
138140

139141
/**

‎src/server.ts‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import {
6262
} from './utils/bp-addressing.js';
6363
import { isRedactionEnabled } from './utils/redaction-mode.js';
6464
import { getVariableAccessMode, requiresExplicitNames } from './utils/variable-access.js';
65-
import { assertLineContent, resolveStatement } from './utils/breakpoint-resolver.js';
65+
import { assertLineContent, resolveStatement, stripTrailingComment } from './utils/breakpoint-resolver.js';
6666

6767
const DEFAULT_LANGUAGES = Object.freeze([DebugLanguage.PYTHON, DebugLanguage.MOCK] as const);
6868

@@ -693,18 +693,29 @@ export class DebugMcpServer {
693693
'Provide line or statement, not both. Use nearLine (with statement) to disambiguate repeated statements.'
694694
);
695695
}
696-
if (
697-
req.statement !== undefined &&
698-
req.expectedContent !== undefined &&
699-
req.statement.trim() !== req.expectedContent.trim()
700-
) {
696+
if (req.statement !== undefined && req.expectedContent !== undefined) {
701697
// A matching expectedContent alongside a statement anchor is redundant
702698
// but harmless — agents combine them constantly (issue #280), so only
703-
// genuinely contradictory intent is an error.
704-
throw new McpError(
705-
McpErrorCode.InvalidParams,
706-
'statement and expectedContent disagree; a statement anchor is already content-addressed. Provide one or the other (a matching expectedContent is accepted as redundant).'
707-
);
699+
// genuinely contradictory intent is an error. "Matching" mirrors the
700+
// relaxed assertLineContent predicate (issue #379): either value may be
701+
// the more complete form of the other, and stale trailing comments on
702+
// either side are ignored.
703+
const stmt = req.statement.trim();
704+
const exp = req.expectedContent.trim();
705+
const strippedStmt = stripTrailingComment(req.statement).trim();
706+
const strippedExp = stripTrailingComment(req.expectedContent).trim();
707+
const compatible =
708+
stmt.includes(exp) ||
709+
exp.includes(stmt) ||
710+
(strippedStmt !== '' &&
711+
strippedExp !== '' &&
712+
(strippedStmt.includes(strippedExp) || strippedExp.includes(strippedStmt)));
713+
if (!compatible) {
714+
throw new McpError(
715+
McpErrorCode.InvalidParams,
716+
'statement and expectedContent disagree; a statement anchor is already content-addressed. Provide one or the other (a matching expectedContent is accepted as redundant).'
717+
);
718+
}
708719
}
709720
if (req.nearLine !== undefined && req.statement === undefined) {
710721
throw new McpError(
@@ -738,13 +749,23 @@ export class DebugMcpServer {
738749

739750
let line: number;
740751
let anchor: { statement: string; nearLine?: number } | undefined;
752+
// Relaxed content matches succeed but must say so (issue #379): a
753+
// substring or comment-stripped pass is indistinguishable from an exact
754+
// one in the stored breakpoint, so the warning is the agent's only
755+
// signal that its stated content only weakly pinned the line.
756+
let addressingWarning: string | undefined;
741757
if (req.statement !== undefined) {
742758
const lines = await readLinesForContentAddressing('statement addressing');
743759
const resolution = resolveStatement(lines, req.statement, resolved.path, req.nearLine);
744760
if (!resolution.ok) {
745761
throw new McpError(McpErrorCode.InvalidParams, resolution.message);
746762
}
747763
line = resolution.line;
764+
if (resolution.candidates !== undefined) {
765+
addressingWarning =
766+
`statement matches ${resolution.candidates.length} lines (${resolution.candidates.join(', ')}); ` +
767+
`nearLine ${req.nearLine} selected line ${line} — verify against the echoed content.`;
768+
}
748769
anchor = {
749770
statement: req.statement.trim(),
750771
...(req.nearLine !== undefined ? { nearLine: req.nearLine } : {})
@@ -759,10 +780,19 @@ export class DebugMcpServer {
759780
if (!check.ok) {
760781
throw new McpError(McpErrorCode.InvalidParams, check.message);
761782
}
783+
if (check.matchQuality === 'substring') {
784+
addressingWarning =
785+
`expectedContent matched line ${line} as a substring of "${check.actual}", ` +
786+
`not the whole line — confirm this is the intended line.`;
787+
} else if (check.matchQuality === 'comment-stripped') {
788+
addressingWarning =
789+
`expectedContent matched line ${line} only after ignoring text past a '//' or '#' comment marker, ` +
790+
`even inside strings (actual: "${check.actual}") — confirm this is the intended line.`;
791+
}
762792
}
763793
}
764794

765-
return this.sessionManager.setBreakpoint(req.sessionId, {
795+
const result = await this.sessionManager.setBreakpoint(req.sessionId, {
766796
file: resolved.path,
767797
line,
768798
condition: req.condition,
@@ -773,6 +803,13 @@ export class DebugMcpServer {
773803
...(supportsLoudSnapping(mode) ? { requestedLine: line } : {}),
774804
...(anchor !== undefined ? { anchor } : {})
775805
});
806+
if (addressingWarning === undefined) {
807+
return result;
808+
}
809+
return {
810+
...result,
811+
warning: result.warning ? `${result.warning}; ${addressingWarning}` : addressingWarning
812+
};
776813
}
777814

778815
// The breakpoint management tools below deliberately skip validateSession's
@@ -1016,7 +1053,7 @@ export class DebugMcpServer {
10161053
if (supportsStatementAnchors(bpMode)) {
10171054
setBreakpointExtraProps.statement = {
10181055
type: 'string',
1019-
description: 'Address by content instead of line number: the text of the target line, like an Edit-tool match — a distinctive substring is enough (leading/trailing whitespace and trailing //- or #-comments are ignored; an exact whole-line match always wins over substring matches). Preferred over line — it cannot land on the wrong line and survives file edits across restart_debugging. If the text appears on multiple lines, the error lists every match; add nearLine to pick one. Provide statement OR line, not both'
1056+
description: 'Address by content instead of line number: the text of the target line, like an Edit-tool match — a distinctive substring is enough (leading/trailing whitespace and trailing //- or #-comments are ignored; an exact whole-line match always wins over substring matches). Preferred over line — it can only land on a line containing your text (an inexact or multi-candidate match adds a warning to the response) and survives file edits across restart_debugging. If the text appears on multiple lines, the error lists every match; add nearLine to pick one. Provide statement OR line, not both'
10201057
};
10211058
setBreakpointExtraProps.nearLine = {
10221059
type: 'number',

0 commit comments

Comments
 (0)