diff --git a/CHANGELOG.md b/CHANGELOG.md index 51c59b8a..f6b93b09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`packageManager` pinned to pnpm@10.33.0** — a different pnpm major regenerated `pnpm-lock.yaml` incompatibly (one contributor diff silently dropped the entire security `overrides` block); the pin plus a CONTRIBUTING note make the wrong-pnpm case fail fast, and the workflows read the pin instead of duplicating a version (#478) ### 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) - **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) diff --git a/src/proxy/proxy-manager.ts b/src/proxy/proxy-manager.ts index ad3b54f4..a5930b77 100644 --- a/src/proxy/proxy-manager.ts +++ b/src/proxy/proxy-manager.ts @@ -18,7 +18,8 @@ import { DAPSessionState, addPendingRequest, removePendingRequest, - clearPendingRequests + clearPendingRequests, + setCurrentThreadId as setCoreCurrentThreadId } from '../dap-core/index.js'; import type { ProxyStatusMessage, @@ -571,6 +572,12 @@ export class ProxyManager extends EventEmitter implements IProxyManager { setCurrentThreadId(threadId: number): void { this.currentThreadId = threadId; + // Mirror into the functional-core snapshot so an adopted anchor (the + // frameless-thread fallback, get_stack_trace {threadId}) is not stale + // there (issue #496). + if (this.dapState) { + this.dapState = setCoreCurrentThreadId(this.dapState, threadId); + } } private async prepareSpawnContext(config: ProxyConfig): Promise<{ @@ -1030,12 +1037,10 @@ export class ProxyManager extends EventEmitter implements IProxyManager { // Sync local state with functional core state this.isInitialized = result.newState.initialized; this.adapterConfigured = result.newState.adapterConfigured; - // Only update currentThreadId if the core provided a concrete number. - // Avoid overwriting the value we set in the fast-path dapEvent handler with null/undefined. - const coreTid = (result.newState as { currentThreadId?: number | null }).currentThreadId; - if (typeof coreTid === 'number') { - this.currentThreadId = coreTid; - } + // The imperative currentThreadId is authoritative — the fast-path + // stopped handler in handleDapEvent writes it. Restoring the core's + // copy here clobbered anchors adopted via setCurrentThreadId() on + // every subsequent response/event (issue #496). } // Resolve/reject pending DAP request Promises. The functional core above only @@ -1085,6 +1090,9 @@ export class ProxyManager extends EventEmitter implements IProxyManager { const first = threads.length ? threads[0]?.id : undefined; if (typeof first === 'number') { this.currentThreadId = first; + if (this.dapState) { + this.dapState = setCoreCurrentThreadId(this.dapState, first); + } } } } catch { diff --git a/tests/unit/proxy/proxy-manager.start.test.ts b/tests/unit/proxy/proxy-manager.start.test.ts index 593efebe..f9be0598 100644 --- a/tests/unit/proxy/proxy-manager.start.test.ts +++ b/tests/unit/proxy/proxy-manager.start.test.ts @@ -1340,6 +1340,112 @@ describe('ProxyManager.start', () => { expect(proxyManager.getCurrentThreadId()).toBe(12); }); + // Issue #496: an anchor adopted via setCurrentThreadId() (the frameless-thread + // fallback and the get_stack_trace {threadId} path) must survive subsequent + // proxy messages — the functional-core state echo used to restore the stale + // stopped-event thread over it on every response and event. + describe('adopted thread anchor persistence (issue #496)', () => { + const primeStoppedOnThread1 = () => { + (proxyManager as unknown as { proxyProcess: IProxyProcess | null }).proxyProcess = fakeProcess; + (proxyManager as unknown as { isInitialized: boolean }).isInitialized = true; + (proxyManager as unknown as { sessionId: string | null }).sessionId = baseConfig.sessionId; + (proxyManager as unknown as { dapState: ReturnType | null }).dapState = + createInitialState(baseConfig.sessionId); + + // The DAP-stopped thread reports no frames in the #496 scenario; the + // session layer then adopts a frame-bearing thread via setCurrentThreadId. + (proxyManager as unknown as { + handleProxyMessage: (message: object) => void; + }).handleProxyMessage({ + type: 'dapEvent', + sessionId: baseConfig.sessionId, + event: 'stopped', + body: { threadId: 1, reason: 'pause' } + }); + expect(proxyManager.getCurrentThreadId()).toBe(1); + // handleProxyMessage syncs isInitialized from the functional-core state, + // which this test primes as uninitialized — restore the flag. + (proxyManager as unknown as { isInitialized: boolean }).isInitialized = true; + + proxyManager.setCurrentThreadId(2); + expect(proxyManager.getCurrentThreadId()).toBe(2); + }; + + const respondToDapWith = (body: object) => { + fakeProcess.sendCommand.mockImplementation((payload) => { + if (payload.cmd === 'dap') { + (proxyManager as unknown as { + handleProxyMessage: (message: object) => void; + }).handleProxyMessage({ + type: 'dapResponse', + sessionId: baseConfig.sessionId, + requestId: payload.requestId, + success: true, + response: { + type: 'response', + seq: 21, + request_seq: 7, + command: payload.dapCommand, + success: true, + body + } + }); + } + }); + }; + + it('survives a threads response (list_threads)', async () => { + primeStoppedOnThread1(); + respondToDapWith({ threads: [{ id: 1, name: 'main' }, { id: 2, name: 'worker' }] }); + + await proxyManager.sendDapRequest('threads'); + + expect(proxyManager.getCurrentThreadId()).toBe(2); + }); + + it('survives an unrelated output event', () => { + primeStoppedOnThread1(); + + (proxyManager as unknown as { + handleProxyMessage: (message: object) => void; + }).handleProxyMessage({ + type: 'dapEvent', + sessionId: baseConfig.sessionId, + event: 'output', + body: { category: 'stdout', output: 'hello\n' } + }); + + expect(proxyManager.getCurrentThreadId()).toBe(2); + }); + + it('survives a stackTrace response (anchor is not single-use)', async () => { + primeStoppedOnThread1(); + respondToDapWith({ + stackFrames: [{ id: 1000, name: 'main', line: 7, column: 1 }], + totalFrames: 1 + }); + + await proxyManager.sendDapRequest('stackTrace', { threadId: 2 }); + + expect(proxyManager.getCurrentThreadId()).toBe(2); + }); + + it('still re-anchors on a genuine stopped event', () => { + primeStoppedOnThread1(); + + (proxyManager as unknown as { + handleProxyMessage: (message: object) => void; + }).handleProxyMessage({ + type: 'dapEvent', + sessionId: baseConfig.sessionId, + event: 'stopped', + body: { threadId: 7, reason: 'breakpoint' } + }); + + expect(proxyManager.getCurrentThreadId()).toBe(7); + }); + }); + it('rejects DAP requests on proxy error', async () => { (proxyManager as unknown as { proxyProcess: IProxyProcess | null }).proxyProcess = fakeProcess; (proxyManager as unknown as { isInitialized: boolean }).isInitialized = true;