Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- **An adopted thread anchor survives `list_threads` and follow-up inspection calls** — the anchor adopted by `get_stack_trace`'s frameless-thread fallback (or an explicit `get_stack_trace {threadId}`, #465) was silently reverted to the stale stopped-event thread by the functional-core state echo on the very next DAP response or event: `list_threads` clobbered it, and even the documented `threadId` recovery survived exactly one inspection call because that call's own response reverted it — so stack → threads → locals answered about the frameless thread and returned nothing. The echo no longer overwrites the live anchor (the imperative fast path is authoritative), and adoptions are mirrored into the functional-core snapshot so the two stores can't disagree again (#496)
- **js breakpoints tell the truth about binding — attach and launch** — js-debug binds breakpoints in a child session while the parent answers with provisional stubs (`verified:false`, "Unbound breakpoint", parent-space ids), and nothing reconciled the two on the attach path: a breakpoint set after `attach_to_process` raced a fire-and-forget child mirror, and one set before attach stayed "Unbound breakpoint" with no adapterId forever — even after it demonstrably fired — because js-debug answers a no-change re-send with an empty echo and emits no late bind event for it. The proxy now hands the child's authoritative `setBreakpoints` response back through the sync path (marked child-sourced, bounded 3s wait), `attach_to_process` gains the post-launch belt-and-braces re-sync (with a clear+re-set to force a fresh echo for already-registered sets), and every previously-silent mirror failure mode logs. Child-origin `breakpoint` events are tagged at the proxy, so a late parent stub whose integer id collides with the stored child id can no longer downgrade a verified record by id-match, and parent-space/provisional-stub ids never enter the store (the #495 intermittent signature); `syncBreakpointsForFile` also normalizes raw l10n keys before storing, closing #471's last gap (#500, #495)
- **Attach-mode `statement`/`expectedContent` rejection states the real reason** — the error claimed the file "is a class name or remote path" even for a readable local `.py` the server had just echoed contents from, sending agents down a path-problem rabbit hole that doesn't exist; the rejection (which is the documented contract) now says what's actually going on — content addressing is not supported for attach sessions because the debuggee's loaded source, not the host's copy, is the authority — while the class-name wording remains for the case it genuinely describes (Java FQCNs) (#497)
- **Java attach to a `suspend=y` JVM no longer freezes the target permanently** — the #465 post-attach pause called `vm.suspend()` unconditionally, so a JVM launched with JDWP `suspend=y` (already at JDI suspend count 1) ended up at count 2 while `continue`'s single `vm.resume()` only took it back to 1: the first continue "succeeded" but nothing ever ran, breakpoints never fired, and the session was stuck. The bridge's pause is now idempotent — when every thread is already suspended it reports the stopped event without deepening the suspend count (same for the single-thread pause path), so one continue releases the VM; pausing a genuinely running VM is unchanged (regression from #483, #489)
- **A typo'd attach `adapterConfig` key now gets a did-you-mean warning instead of silence** — every attach-capable adapter declares the attach keys its debugger actually consumes (`supportedAttachKeys`), and `attach_to_process` warns about keys outside that set with an edit-distance suggestion (`pathMapping (did you mean pathMappings?)`) while still **forwarding** them to the debug adapter untouched, so upstream debugger options the list doesn't model keep working with no mcp-debugger release; keys an adapter's transform genuinely drops keep warning as ignored (#450's contract). The javascript, ruby, java, and dotnet attach transforms also move from closed allowlists to the python/cpp deny-list pattern, so advanced attach options (js-debug's `localRoot`/`remoteRoot`/`sourceMaps`/`skipFiles`/`continueOnAttach`, rdbg's `localfsMap`, netcoredbg's `sourceFileMap`/`symbolOptions`) now genuinely reach the debugger. Thanks @abhijeetnardele24-hash for the `supportedAttachKeys` + did-you-mean design (#466)
- **Java attach is inspectable on the first try** — the JDI bridge never suspends the VM on attach and java's policy lacked the post-attach pause every other attach adapter has, so the session reported `paused` while the JVM kept running: empty stack, "the debugger may not be paused", and no discoverable recovery. Java now pauses the whole VM after attach (the bridge anchors its stopped event to a thread that can actually report frames), the frameless-thread notes name a real recovery (`threadId` targeting or continue+pause), and `get_local_variables` no longer contradicts the session state (#465)
Expand Down
18 changes: 11 additions & 7 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -667,20 +667,20 @@ export class DebugMcpServer {
sessionId: string,
file: string,
options?: { requireExists?: boolean }
): Promise<{ path: string; contentAddressable: boolean }> {
): Promise<{ path: string; contentAddressable: boolean; nonAddressableReason?: 'non-file-identifier' | 'attach' }> {
// Check if the adapter handles non-file source identifiers (e.g. Java FQCNs)
const policy = this.sessionManager.getSessionPolicy(sessionId);
if (policy.isNonFileSourceIdentifier?.(file)) {
this.logger.info(`[DebugMcpServer.resolveBreakpointFile] Non-file source identifier detected: ${file}`);
return { path: file, contentAddressable: false };
return { path: file, contentAddressable: false, nonAddressableReason: 'non-file-identifier' };
}

// Attach sessions may debug a target on a remote filesystem (container,
// pod, another machine); host-side existence checks don't apply. Pass the
// path through as-is — the debugger knows its own filesystem best.
if (this.sessionManager.getSession(sessionId)?.attachMode) {
this.logger.info(`[DebugMcpServer.resolveBreakpointFile] Attach session: skipping host file check for ${file}`);
return { path: file, contentAddressable: false };
return { path: file, contentAddressable: false, nonAddressableReason: 'attach' };
}

const fileCheck = await this.fileChecker.checkExists(file);
Expand Down Expand Up @@ -750,10 +750,14 @@ export class DebugMcpServer {

const readLinesForContentAddressing = async (feature: string): Promise<string[]> => {
if (!resolved.contentAddressable) {
throw new McpError(
McpErrorCode.InvalidParams,
`${feature} requires a source file readable by the mcp-debugger server; "${req.file}" is a class name or remote path. Use line addressing instead.`
);
// Two distinct causes, two honest reasons (issue #497): an attach
// session's file may be perfectly readable here — the rule is that
// the debuggee's loaded source is the authority, not the host's copy.
const reason =
resolved.nonAddressableReason === 'attach'
? `${feature} is not supported for attach sessions — the debuggee's loaded source may not match the file on the mcp-debugger host. Use line addressing instead.`
: `${feature} requires a source file readable by the mcp-debugger server; "${req.file}" is a class name or remote path. Use line addressing instead.`;
throw new McpError(McpErrorCode.InvalidParams, reason);
}
const lines = await this.lineReader.getFileLines(resolved.path);
if (!lines) {
Expand Down
53 changes: 49 additions & 4 deletions tests/core/unit/server/server-statement-anchor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,61 @@ describe('set_breakpoint statement anchors (#271)', () => {
await expect(callSetBreakpoint({})).rejects.toThrow(/[Mm]issing required/);
});

it('rejects statement for attach sessions', async () => {
it('rejects statement for attach sessions with the attach-specific reason (issue #497)', async () => {
mockSessionManager.getSession.mockReturnValue({
id: 'test-session',
sessionLifecycle: 'active',
attachMode: true
});

await expect(
callSetBreakpoint({ statement: 'return total' })
).rejects.toThrow(/line addressing/i);
const err = await callSetBreakpoint({ statement: 'return total' }).then(
() => { throw new Error('expected rejection'); },
(e: Error) => e
);
// The rejection is correct; the reason must be too — the file is a
// readable local path, not "a class name or remote path" (issue #497).
expect(err.message).toMatch(/not supported for attach sessions/);
expect(err.message).toMatch(/line addressing/i);
expect(err.message).not.toMatch(/class name or remote path/);
});

it('rejects expectedContent for attach sessions with the attach-specific reason (issue #497)', async () => {
mockSessionManager.getSession.mockReturnValue({
id: 'test-session',
sessionLifecycle: 'active',
attachMode: true
});

const err = await callSetBreakpoint({ line: 6, expectedContent: 'return total' }).then(
() => { throw new Error('expected rejection'); },
(e: Error) => e
);
expect(err.message).toMatch(/expectedContent/);
expect(err.message).toMatch(/not supported for attach sessions/);
expect(err.message).not.toMatch(/class name or remote path/);
});

it('keeps the class-name wording for non-file source identifiers', async () => {
mockSessionManager.getSessionPolicy.mockReturnValue({
isNonFileSourceIdentifier: () => true
});

const err = await callToolHandler({
method: 'tools/call',
params: {
name: 'set_breakpoint',
arguments: {
sessionId: 'test-session',
file: 'com.example.MyClass',
statement: 'return total'
}
}
}).then(
() => { throw new Error('expected rejection'); },
(e: Error) => e
);
expect(err.message).toMatch(/class name or remote path/);
expect(err.message).toMatch(/line addressing/i);
});

it('is rejected in assert mode, naming the env value', async () => {
Expand Down
Loading