Skip to content

Commit b7490aa

Browse files
cynarlabclaude
andcommitted
fix(javascript): catch the first call of a first-declared function breakpoint and explain a late entry stop (#858)
js-debug's stopOnEntry breakpoint sits at line 0 col 0 and V8 resolves it to the first breakable position in source order — the body of a function declared above the first statement — so the "entry" stop is that function's first call and recurs at every later call (microsoft/vscode-js-debug#2430). The forced entry stop that binds a pre-launch function breakpoint was that first call, auto-continued and lost. The CDP bridge now reports an entry stop whose paused frame is the function it just armed (matched by [[FunctionLocation]]) as the function breakpoint, and annotates an entry stop that landed inside a function in the stopped event's text; ChildSessionManager routes entry stops to the bridge with nothing armed. The adapter's inert --inspect-brk=9229 append and its --inspect promotion are removed (js-debug strips --inspect-brk into stopOnEntry itself). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 4351d5b commit b7490aa

11 files changed

Lines changed: 179 additions & 38 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
**A JavaScript function breakpoint on the first-declared function no longer misses its first call, and a late entry stop says where it landed** — js-debug implements `stopOnEntry` with a breakpoint at line 1, column 1 that V8 resolves to the first breakable position in source order: for a file whose first declaration is a function the program calls later, that is the function's body, so the "entry" stop fires at the function's first call (after the top-level code before it ran) and again at every later call. The forced entry stop that binds a pre-launch function breakpoint was that very first call, and auto-continuing it lost the call for good with the breakpoint listed `verified: true`. The CDP bridge now recognises an entry stop whose paused frame is the function it just armed — matched by `[[FunctionLocation]]`, never by name — and reports it as the function breakpoint. A `stopOnEntry` launch that lands inside a function carries the explanation in the stop's `text` (`start_debugging`, `wait_for_stop`, `list_debug_sessions`): where it stopped, why, that the code before it has run, that later calls stop again, and what to do instead. The JavaScript adapter also stops appending `--inspect-brk=9229` to `runtimeArgs` for `stopOnEntry` and promoting a user's `--inspect` to `--inspect-brk` — js-debug strips `--inspect-brk` into `stopOnEntry` itself, so the first was inert and the second forced an entry stop nobody asked for. Reported upstream as microsoft/vscode-js-debug#2430 (#858)

‎docs/javascript/README.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,19 @@ Both `dapLaunchArgs` and `adapterLaunchConfig` accept launch settings. When the
158158
in both, `adapterLaunchConfig` wins, including `stopOnEntry`. Setting it to `true` keeps the target
159159
paused at entry; `restart_debugging` replays that intent. An honoured `noDebug` disables the entry pause.
160160

161+
**Where the entry stop lands** (issue #858): js-debug implements `stopOnEntry` with a breakpoint at line 1,
162+
column 1 of the program, and V8 resolves that to the first breakable position in *source* order. For a file
163+
whose first declaration is a function that the program calls later, that position is inside the function —
164+
so the "entry" stop fires at the function's first call, after the top-level code before it has run (output
165+
included), and fires again at every later call. mcp-debugger cannot move the stop, but it says so: the
166+
stop's `lastStop.text` (in the `start_debugging` answer, `wait_for_stop`, `list_debug_sessions`) explains
167+
where it landed and why. To stop at the first statement, put a statement above the function or set a line
168+
breakpoint on it. A function breakpoint on that first-declared function is not affected: the stop that
169+
binds it *is* the function's first call, and it is reported as the function breakpoint. Reported upstream as
170+
[vscode-js-debug#2430](https://github.com/microsoft/vscode-js-debug/issues/2430). Node's `--inspect-brk` is not an alternative: js-debug strips it from `runtimeArgs`
171+
into `stopOnEntry`, so a bare `--inspect` or `--inspect-brk` in `adapterLaunchConfig.runtimeArgs` is
172+
forwarded as given and changes nothing about the entry stop.
173+
161174
Use `envFile` to load dotenv values relative to the effective `cwd`, then override individual values
162175
with `env`. A `null` value removes an inherited or file-defined variable:
163176

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// A file whose first declaration is a function the program calls later. For
2+
// such a file js-debug's stopOnEntry breakpoint (line 1, column 1) is resolved
3+
// by V8 to the first breakable position in SOURCE order — inside work() — so
4+
// the "entry" stop is really work's first call (issue #858). Drives the
5+
// function-breakpoint and stopOnEntry e2e cases for that issue.
6+
function work(base) {
7+
const total = base + 1; // WORK_ENTRY_LINE
8+
return total;
9+
}
10+
console.log('started');
11+
setTimeout(() => console.log('late total', work(41)), 1500);

‎packages/adapter-javascript/src/javascript-debug-adapter.ts‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -737,45 +737,18 @@ export class JavascriptDebugAdapter extends EventEmitter implements IDebugAdapte
737737
}
738738
}
739739

740-
// Append any user-provided args last and normalize/dedupe
741-
let finalArgs = this.normalizeAndDedupeArgs([...computedArgs, ...userRuntimeArgs]);
742-
743-
// Normalize Node inspector flags: ensure explicit port form, and add --inspect-brk when stopOnEntry is true
740+
// Append any user-provided args last and normalize/dedupe. Node inspector
741+
// flags pass through as given: js-debug itself strips an --inspect-brk out
742+
// of runtimeArgs into stopOnEntry (its resolveParams), so the
743+
// --inspect-brk=9229 this transform used to append for stopOnEntry was
744+
// inert, and promoting a user's bare --inspect to --inspect-brk forced an
745+
// entry stop nobody asked for (issue #858).
746+
const finalArgs = this.normalizeAndDedupeArgs([...computedArgs, ...userRuntimeArgs]);
744747

745748
result.runtimeExecutable = runtimeExecutableSync;
746749
if (finalArgs.length > 0) {
747750
result.runtimeArgs = finalArgs;
748751
}
749-
// Normalize Node inspector flags for js-debug.
750-
// If an --inspect/--inspect-brk flag is present, ensure it includes an explicit port.
751-
if (isNodeRuntime) {
752-
const findInspectIndex = () =>
753-
finalArgs.findIndex(
754-
(a) =>
755-
a === '--inspect' ||
756-
a === '--inspect-brk' ||
757-
a.startsWith('--inspect=') ||
758-
a.startsWith('--inspect-brk=')
759-
);
760-
const idx = findInspectIndex();
761-
if (idx !== -1) {
762-
const port = 9229;
763-
const arg = finalArgs[idx];
764-
const m = arg.match(/^--inspect(?:-brk)?=(\d+)$/);
765-
if (m) {
766-
// Port is already explicit in the flag; no rewrite needed
767-
} else {
768-
// Promote to explicit port for consistency and reliable auto-attach
769-
finalArgs[idx] = `--inspect-brk=${port}`;
770-
result.runtimeArgs = finalArgs;
771-
}
772-
} else if (stopOnEntry === true) {
773-
// Ensure a deterministic single-session stop on entry when requested
774-
const port = 9229;
775-
finalArgs = [...finalArgs, `--inspect-brk=${port}`];
776-
result.runtimeArgs = finalArgs;
777-
}
778-
}
779752

780753
// Forward every js-debug key the caller passed that this transform does
781754
// not derive — trace, perScriptSourcemaps, timeouts, sourceMapPathOverrides,

‎packages/adapter-javascript/tests/unit/javascript-debug-adapter.transform.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,21 @@ describe('JavascriptDebugAdapter.transformLaunchConfig', () => {
211211
expect(ra[ra.length - 1]).toBe('--my-flag');
212212
});
213213

214+
it('does not add or rewrite Node inspector flags: stopOnEntry is js-debug\'s, and a user --inspect is forwarded as given (issue #858)', async () => {
215+
// js-debug strips any --inspect-brk from runtimeArgs into stopOnEntry
216+
// (vendored bundle, resolveParams), so the --inspect-brk=9229 the
217+
// transform used to append was inert — and promoting a user's --inspect
218+
// to --inspect-brk forced an entry stop nobody asked for.
219+
const program = path.resolve('/proj/app.js');
220+
const withEntry = await adapter.transformLaunchConfig({ program, stopOnEntry: true } as any);
221+
expect(withEntry.stopOnEntry).toBe(true);
222+
expect(((withEntry.runtimeArgs ?? []) as string[]).some((a) => a.startsWith('--inspect'))).toBe(false);
223+
224+
const withInspect = await adapter.transformLaunchConfig({ program, stopOnEntry: false, runtimeArgs: ['--inspect'] } as any);
225+
expect(withInspect.runtimeArgs).toEqual(['--inspect']);
226+
expect(withInspect.stopOnEntry).toBe(false);
227+
});
228+
214229
it('runtimeExecutable override: "tsx" results in empty hooks', async () => {
215230
const program = path.resolve('/proj/app.ts');
216231
const cfg = await adapter.transformLaunchConfig({

‎skills/debugging/references/javascript.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Shorthand: `create_debug_session { "language": "javascript", "host": "127.0.0.1"
6868
## Quirks
6969

7070
- **Child-session architecture.** js-debug runs a parent session for launch orchestration and spawns a child session for the actual debuggee. This is invisible to you: the proxy routes evaluate/step/stack commands to the active context automatically. Never create a second MCP session for the "other" half.
71-
- **Entry pause auto-continues.** With `stopOnEntry: false` (the default) the debugger automatically continues past entry breakpoints, so execution runs straight to your first breakpoint. A breakpoint reached as the program starts is in the `start_debugging` answer; one reached later — a route handler, a timer — leaves the launch answering `pending: true`, and `wait_for_stop` collects it. Set `dapLaunchArgs: { "stopOnEntry": true }` only when you want control at the first line.
71+
- **Entry pause auto-continues.** With `stopOnEntry: false` (the default) the debugger automatically continues past entry breakpoints, so execution runs straight to your first breakpoint. A breakpoint reached as the program starts is in the `start_debugging` answer; one reached later — a route handler, a timer — leaves the launch answering `pending: true`, and `wait_for_stop` collects it. Set `dapLaunchArgs: { "stopOnEntry": true }` only when you want control at the first line — and read `lastStop.text` when you get it: for a file whose first declaration is a function the program calls later, js-debug's entry breakpoint resolves *inside that function*, so the "entry" stop is its first call (top-level code before it has already run) and every later call stops as "entry" again. The text says so; a line breakpoint on the first statement is the reliable way to stop there.
7272
- **Stack filtering hides Node internals.** `get_stack_trace` returns user frames only by default; pass `includeInternals: true` if you genuinely need Node.js internal frames. If execution initially stops inside internals, just `continue_execution`.
7373
- **TypeScript auto-detection.** Point `scriptPath` at the `.ts` file when `tsx`/`ts-node` is available. If neither is installed you get a warning (not an error) — fall back to the compiled `.js` with source maps.
7474
- **Child processes are not auto-attached.** `autoAttachChildProcesses` defaults to `false`; pass it as `true` in `dapLaunchArgs` to debug `spawn`-ed Node children.
3.85 KB
Binary file not shown.

‎src/proxy/child-session-manager.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,10 @@ export class ChildSessionManager extends EventEmitter {
10811081
await this.bridgeAttachSettled;
10821082
let out = evt;
10831083
if (evt.event === 'stopped') {
1084-
if (bridge.hasArmedOrPending()) {
1084+
// An entry stop always goes through the bridge: with nothing armed
1085+
// it can still be a late entry inside a function (issue #858).
1086+
const reason = (evt.body as { reason?: string } | undefined)?.reason;
1087+
if (bridge.hasArmedOrPending() || reason === 'entry') {
10851088
try {
10861089
out = await bridge.processStoppedEvent(evt);
10871090
} catch (err) {

‎tests/e2e/mcp-server-smoke-js-function-bp.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ const __dirname = path.dirname(__filename);
3333
const ROOT = path.resolve(__dirname, '../..');
3434
const FIXTURE = path.resolve(ROOT, 'examples', 'javascript', 'function_bp_test.js');
3535
const ATTACH_TARGET = path.resolve(ROOT, 'examples', 'javascript', 'function_bp_attach_target.js');
36+
// A function declared above the first statement and called 1.5 s in (issue #858)
37+
const FUNCTION_FIRST = path.resolve(ROOT, 'examples', 'javascript', 'function_first.js');
38+
const WORK_ENTRY_LINE = 7;
3639

3740
const COMPUTE_DECL_LINE = 9;
3841
const COMPUTE_ENTRY_LINE = 10;
@@ -197,6 +200,50 @@ describe('MCP Server JavaScript Function Breakpoints', () => {
197200
expect(contRes.success).toBe(true);
198201
}, 60000);
199202

203+
it('catches the first call of a function declared above the first statement (issue #858)', async () => {
204+
// js-debug's entry breakpoint resolves into work's body, so the forced
205+
// entry stop that binds the function breakpoint IS work's first call —
206+
// before this fix that stop was auto-continued and the only call was lost.
207+
const sid = await createSession('js-fnbp-function-first');
208+
const bpRes = await callToolSafely(mcpClient!, 'set_breakpoint', { sessionId: sid, function: 'work' });
209+
expect(bpRes.success).toBe(true);
210+
211+
const startResponse = parseSdkToolResult(await mcpClient!.callTool({
212+
name: 'start_debugging',
213+
arguments: { sessionId: sid, scriptPath: FUNCTION_FIRST, dapLaunchArgs: { stopOnEntry: false } }
214+
}));
215+
expect(startResponse.state).toBeDefined();
216+
217+
const stack = await waitForPausedState(sid);
218+
expect(stack, 'the only call of work() ran to completion without stopping').not.toBeNull();
219+
const top = stack!.stackFrames![0];
220+
expect(top.name).toContain('work');
221+
expect(top.line).toBe(WORK_ENTRY_LINE);
222+
expect((await getSessionSnapshot(sid))?.lastStop?.reason).toBe('function breakpoint');
223+
const output = await callToolSafely(mcpClient!, 'get_output', { sessionId: sid, since: 0 });
224+
const text = JSON.stringify((output as { entries?: unknown[] }).entries ?? []);
225+
expect(text).toContain('started');
226+
expect(text).not.toContain('late total');
227+
228+
await callToolSafely(mcpClient!, 'continue_execution', { sessionId: sid });
229+
}, 60000);
230+
231+
it('says so when a stopOnEntry launch stops inside a function instead of at the first statement (issue #858)', async () => {
232+
const sid = await createSession('js-late-entry');
233+
const startResponse = parseSdkToolResult(await mcpClient!.callTool({
234+
name: 'start_debugging',
235+
arguments: { sessionId: sid, scriptPath: FUNCTION_FIRST, dapLaunchArgs: { stopOnEntry: true } }
236+
})) as { state?: string; data?: { reason?: string } };
237+
expect(startResponse.state).toBe('paused');
238+
expect(startResponse.data?.reason).toBe('entry');
239+
240+
const snapshot = await getSessionSnapshot(sid) as { lastStop?: { reason?: string; text?: string } } | undefined;
241+
expect(snapshot?.lastStop?.reason).toBe('entry');
242+
expect(snapshot?.lastStop?.text).toMatch(/entry stop landed inside work\(\) at its first call/);
243+
const stack = await waitForPausedState(sid);
244+
expect(stack!.stackFrames![0].name).toContain('work');
245+
}, 60000);
246+
200247
it('defers a lazily-loaded module function until a pause after the require, then binds and stops', async () => {
201248
const sid = await createSession('js-fnbp-deferred');
202249

‎tests/proxy/cdp-function-breakpoint-bridge.test.ts‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,22 @@ class FakeCdpClient extends EventEmitter {
8888
return this.calls.filter((c) => c.method === method);
8989
}
9090

91-
pause(opts: { hitBreakpoints?: string[]; callFrameId?: string; scriptId?: string; url?: string } = {}): void {
91+
pause(opts: {
92+
hitBreakpoints?: string[];
93+
callFrameId?: string;
94+
scriptId?: string;
95+
url?: string;
96+
functionName?: string;
97+
functionLocation?: { scriptId: string; lineNumber: number; columnNumber: number };
98+
} = {}): void {
9299
this.emit('cdp-event', 'Debugger.paused', {
93100
reason: 'other',
94101
hitBreakpoints: opts.hitBreakpoints ?? [],
95102
callFrames: [
96103
{
97104
callFrameId: opts.callFrameId ?? 'frame-1',
98-
functionName: '',
105+
functionName: opts.functionName ?? '',
106+
...(opts.functionLocation ? { functionLocation: opts.functionLocation } : {}),
99107
location: { scriptId: opts.scriptId ?? '159', lineNumber: 0, columnNumber: 0 },
100108
url: opts.url ?? ''
101109
}
@@ -482,6 +490,58 @@ describe('CdpFunctionBreakpointBridge', () => {
482490
});
483491
});
484492

493+
describe('late entry stops (issue #858)', () => {
494+
// js-debug sets its stopOnEntry breakpoint at line 1 col 1 and V8 resolves
495+
// it to the first breakable position in source order — inside a function
496+
// declared above the first statement — so the "entry" stop is really that
497+
// function's first call.
498+
const WORK_LOCATION = { scriptId: '159', lineNumber: 2, columnNumber: 14 }; // the fake's [[FunctionLocation]]
499+
500+
it('relabels an entry stop that lands inside the function just armed as that function breakpoint', async () => {
501+
cdp.frameFunctions.set('work', 'obj-work');
502+
const body = await bridge.sync([fnBp('work')]);
503+
const adapterId = body.breakpoints[0].id!;
504+
await attach();
505+
506+
// js-debug's own entry breakpoint id is foreign to us; the frame is work's body
507+
cdp.pause({ hitBreakpoints: ['js-debug-entry-bp'], functionName: 'work', functionLocation: WORK_LOCATION });
508+
await bridge.waitForResolution();
509+
const out = await bridge.processStoppedEvent(stoppedEvent('entry'));
510+
const stopped = out.body as DebugProtocol.StoppedEvent['body'];
511+
expect(stopped.reason).toBe('function breakpoint');
512+
expect(stopped.hitBreakpointIds).toEqual([adapterId]);
513+
});
514+
515+
it('leaves an entry stop inside some other function to the note path (no false relabel by name)', async () => {
516+
cdp.frameFunctions.set('work', 'obj-work');
517+
await bridge.sync([fnBp('work')]);
518+
await attach();
519+
cdp.pause({ hitBreakpoints: ['js-debug-entry-bp'], functionName: 'work', functionLocation: { scriptId: '159', lineNumber: 40, columnNumber: 9 } });
520+
await bridge.waitForResolution();
521+
const out = await bridge.processStoppedEvent(stoppedEvent('entry'));
522+
const stopped = out.body as DebugProtocol.StoppedEvent['body'];
523+
expect(stopped.reason).toBe('entry');
524+
expect(stopped.text).toMatch(/entry stop landed inside work\(\)/);
525+
});
526+
527+
it('annotates a late entry stop with no function breakpoints at all, and leaves a top-level entry stop alone', async () => {
528+
await attach();
529+
expect(bridge.hasArmedOrPending()).toBe(false);
530+
531+
cdp.pause({ hitBreakpoints: ['js-debug-entry-bp'], functionName: 'helper', functionLocation: { scriptId: '159', lineNumber: 0, columnNumber: 15 } });
532+
const late = await bridge.processStoppedEvent(stoppedEvent('entry'));
533+
const lateBody = late.body as DebugProtocol.StoppedEvent['body'];
534+
expect(lateBody.reason).toBe('entry');
535+
expect(lateBody.text).toMatch(/entry stop landed inside helper\(\) at its first call/);
536+
expect(lateBody.text).toMatch(/every later call of helper\(\) stops as "entry" again/);
537+
538+
cdp.resume();
539+
cdp.pause({ hitBreakpoints: ['js-debug-entry-bp'], functionName: '' });
540+
const top = await bridge.processStoppedEvent(stoppedEvent('entry'));
541+
expect((top.body as DebugProtocol.StoppedEvent['body']).text).toBeUndefined();
542+
});
543+
});
544+
485545
describe('deferred binding', () => {
486546
it('re-resolves pending names at each pause and emits changed on late bind', async () => {
487547
await bridge.sync([fnBp('helper.doWork')]);

0 commit comments

Comments
 (0)