diff --git a/CHANGELOG.md b/CHANGELOG.md index f6b93b09..2a4f59bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/server.ts b/src/server.ts index 1fbb710c..1f593110 100644 --- a/src/server.ts +++ b/src/server.ts @@ -667,12 +667,12 @@ 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, @@ -680,7 +680,7 @@ export class DebugMcpServer { // 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); @@ -750,10 +750,14 @@ export class DebugMcpServer { const readLinesForContentAddressing = async (feature: string): Promise => { 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) { diff --git a/tests/core/unit/server/server-statement-anchor.test.ts b/tests/core/unit/server/server-statement-anchor.test.ts index 20d7526e..d46708d1 100644 --- a/tests/core/unit/server/server-statement-anchor.test.ts +++ b/tests/core/unit/server/server-statement-anchor.test.ts @@ -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 () => {