From fe514fca0e783f59f252b958f51ad694c23640a4 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 26 Aug 2026 20:43:01 -0400 Subject: [PATCH] fix(#496): keep the adopted thread anchor authoritative across DAP responses/events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The functional-core state echo in handleProxyMessage copied dapState.currentThreadId (written only by the stopped handler) back over the imperative anchor on every DAP response, every event (even output), and status messages. Any anchor adopted via setCurrentThreadId() — the frameless-thread fallback and get_stack_trace {threadId} (#465) — was reverted by the next proxy message: list_threads clobbered it, and each inspection call consumed it via its own response. The imperative currentThreadId is now authoritative (the stopped fast path writes it; the echo is gone), and both adoption paths mirror into the functional-core snapshot so the stores cannot disagree. A genuine stopped event still re-anchors unconditionally. Closes #496 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 + src/proxy/proxy-manager.ts | 22 ++-- tests/unit/proxy/proxy-manager.start.test.ts | 106 +++++++++++++++++++ 3 files changed, 122 insertions(+), 7 deletions(-) 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;