fix(attach): generous verify window + paused-stack readiness (load-flaky attach e2e) - #398
Merged
Merged
Conversation
…aky attach e2e) Two attach-mode e2e tests failed under full-suite CPU load (parallel unit project alongside the serial e2e project) while passing in isolation. Both root causes were general contract gaps, fixed in the implementation rather than by conforming the tests: - attachVerifyTimeoutMs default 5s -> 20s. The verify deadline only ever bites when the adapter is alive but the target is slow to report threads (js-debug child adoption on a loaded host, warming JVM) - adapter death still fails fast via the proxyGone latch, and the poll exits as soon as threads appear. A false "attach failed" on a healthy target is strictly worse for an agent than a slower genuine failure. verifyTimeout knob unchanged; the failure-by-design probe in comprehensive-mcp-tools now passes a small verifyTimeout explicitly. - Paused => usable stack on the agent-facing get_stack_trace path (ensureStackReady): an empty-but-successful stackTrace while PAUSED is nearly always a transient race (netcoredbg materializes the managed stack a beat after the post-attach pause stop - the milder sibling of #353's 0x80131302) or a frameless runtime thread tracked as current. Bounded retry (~3s, exits on first frame), fallback scan of the other stopped threads (adopts the frame-bearing one so scopes/evaluate anchor to it), honest empty + explanatory note as last resort. Not-paused and no-known-thread empty results now carry a note too. Internal callers keep the single-shot behavior. The two e2e tests keep their strict semantics (assertion diagnostics only). Follow-ups filed: #396 (threads response clobbers currentThreadId), #397 (tool:response logs success:true for failed attach). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Two attach-mode e2e tests failed in a full-suite run while passing in isolation — the parallel
unitproject runs alongside the seriale2eproject, so e2e tests execute under heavy CPU load:mcp-server-smoke-js-function-bp.test.ts(attach mode) —attach_to_processfailed with "Attach did not become debuggable: no threads reported within 5000ms". For js-debug,threadsis child-routed; until the child session is adopted the parent reports zero threads, and under load adoption exceeds the 5s default verify window. The adapter was healthy — the deadline hard-failed a good attach.mcp-server-smoke-dotnet-attach.test.ts— after the post-attach pause ([BUG] .NET attach: pause_execution doesn't suspend, and breakpoints refuse to bind despite a valid Portable PDB #353 fix),get_stack_traceissued 7ms after thestoppedevent got a successful DAP response with 0 frames from netcoredbg. A passing run shows identical timing with 2 frames, so this is netcoredbg's post-pause sync window (the milder sibling of [BUG] .NET attach: pause_execution doesn't suspend, and breakpoints refuse to bind despite a valid Portable PDB #353's0x80131302), and/or the attach-discoveredthreads[0]being a frameless runtime thread.Both are general contract gaps, so the fixes are implementation hardening — the e2e tests keep their strict semantics as the spec (only assertion diagnostics were added).
Changes
Attach verify window default 5s → 20s (
attachVerifyTimeoutMs)verifyTimeoutknob is unchanged (validated, clamped to 10 min); the failure-by-design attach probe incomprehensive-mcp-tools.test.tsnow passesverifyTimeout: 2000to stay fast. Tool descriptions, docs, and skill references updated.Paused ⇒ usable stack on the agent-facing
get_stack_tracepath (ensureStackReady)stackTracewith success + zero frames: bounded retry (~3s, 250ms interval, exits on first frame, aborts if the session leaves PAUSED), then a fallback scan of the other stopped threads — the first frame-bearing thread is adopted viasetCurrentThreadId(soscopes/evaluateanchor correctly) and the response carries anotenaming the switch.notetelling the agent what to try (list_threads, retry). The not-paused and no-known-thread empty results now carry explanatory notes too.success: falsestill throws (JavaScript attach mode: attach_to_process reports success + 'paused' but js-debug child session never connects to the target #124), missingstackFramesstill throws. Internal callers keep the single-shot behavior — only the MCP tool path (which resolves the thread implicitly) opts in.Verification
npm test(unit + integration + e2e, 260 files / 3934 tests) green — the exact configuration that produced the original failures, with both previously failing tests passing under load.comprehensive-mcp-toolsattach-failure probe confirmed fast (~0.9s).Follow-ups filed
threadsresponse clobbersProxyManager.currentThreadIdwiththreads[0].id, discovered while tracing the dotnet failure.tool:responsetelemetry logssuccess: truefor a failedattach_to_process, which misled triage of this very failure.🤖 Generated with Claude Code