Skip to content

fix(#512): latch a late init ACK instead of losing it between retry windows - #515

Merged
debugmcpdev merged 2 commits into
mainfrom
fix/512-init-ack-latch
Aug 27, 2026
Merged

fix(#512): latch a late init ACK instead of losing it between retry windows#515
debugmcpdev merged 2 commits into
mainfrom
fix/512-init-ack-latch

Conversation

@debugmcpdev

Copy link
Copy Markdown
Collaborator

Fixes #512.

What

sendInitWithRetry registered its init-received listener per attempt and removed it when that attempt's window expired. An ACK landing during the inter-attempt backoff sleep fired into a void and was lost forever. Attempt 1's window is 500ms, so any worker boot slower than that fell into the gap — and a dry-run worker (which acks, finishes, exits by design) then converted the miss into Proxy process not available on every remaining retry, failing the launch ~16s later even though the worker did everything right.

Root-caused live while validating #501: with a fresh js attach to the server, worker spawn→ACK latency measured 669–842ms vs 338–427ms unattached (0/6 vs 6/6 cycle success — the boundary is exactly the 500ms window). The ACK provably reached the parent's ChildProcess in every failed cycle, captured by an evaluate_expression-injected monkeypatch on ChildProcess.prototype.emit after strace's ptrace overhead made the race vanish. Nothing is inspection-specific — a loaded CI host can add the same ~300ms, so this likely explains a family of flaky Failed to initialize proxy after 6 attempts failures.

Changes

  • One latch listener spans the whole retry sequence; both the attempt window and the backoff sleep end early the moment the ACK arrives (checked again at each loop top).
  • A failed send skips the pointless ACK window — nothing was delivered — preserving the previous retry cadence for the send-failure path.
  • Once the worker has exited without acking (lastExitDetails recorded), the loop fails fast with the existing detailed exit message instead of burning the remaining ~15s of retries: after exit, no ACK can arrive.

Testing

  • proxy-manager.handshake.test.ts: the test that pinned the drop ("retries when acknowledgement arrives after the first timeout") now asserts the latch (resolves on a single send); new tests for later-attempt-only ACK and the fast-fail path (2 sends, detailed message, no 15s burn).
  • proxy-manager.start.test.ts: the exhaustion test now produces its exit details realistically (worker exits mid-retry) and asserts fast failure with the details attached.
  • Suites run ~6× faster as a side effect of the fast-fail (17s → 2.9s for the two files).
  • Live: integrated with js attach to a Node process that forks children strands those children (second startDebugging target ignored, child waits forever) #501, the self-attach stress harness went 0/6 → 6/6, with failing-window launches completing in ~1s (ACK latched during backoff).

🤖 Generated with Claude Code

https://claude.ai/code/session_01NgA5kHzkhFZLqbWNZRTsVG

…indows

sendInitWithRetry registered its init-received listener per attempt and
removed it when that attempt's window expired, so an ACK landing during the
inter-attempt backoff sleep fired into a void and was lost forever. Attempt
1's window is 500ms; any worker boot slower than that (inspected process,
loaded CI host) fell into the gap, and a dry-run worker — which acks,
finishes, and exits — then turned every remaining retry into 'Proxy process
not available', failing the launch ~16s later even though the worker did
everything right.

Root-caused live by attaching mcp-debugger to an mcp-debugger server while
validating #501: with a fresh js attach, worker spawn-to-ACK latency
measured 669-842ms vs 338-427ms unattached, reproducing the failure 6/6;
the ACK provably reached the parent's ChildProcess in every failed run
(instrumented via evaluate_expression monkeypatching, since strace's ptrace
overhead made the race vanish).

Changes:
- One long-lived latch listener spans the whole retry sequence; both the
  attempt window and the backoff sleep end early the moment the ACK arrives.
- A failed send skips the pointless ACK window (nothing was delivered),
  preserving the previous retry cadence.
- Once the worker has exited without acking (lastExitDetails recorded), the
  loop fails fast with the detailed exit message instead of burning the
  remaining ~15s of retries against a dead process.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NgA5kHzkhFZLqbWNZRTsVG
@debugmcpdev
debugmcpdev enabled auto-merge (squash) August 27, 2026 14:15
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/proxy/proxy-manager.ts 93.02% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@debugmcpdev
debugmcpdev merged commit d62bd0e into main Aug 27, 2026
8 checks passed
@debugmcpdev
debugmcpdev deleted the fix/512-init-ack-latch branch August 27, 2026 15:16
debugmcpdev added a commit that referenced this pull request Aug 27, 2026
…st (#521)

Follow-up from the #515 review: the retry loop breaks out early once the
worker has exited (no ack can arrive), but the thrown message was still
built from the constant retry budget — a launch that fast-failed on
attempt 2 reported "after 6 attempts", misreading how long the launch
spent trying.

Track the attempts actually made and say which exit happened:
- exhausted:  "Failed to initialize proxy after 6 attempts."
- fast-fail:  "Failed to initialize proxy after 2 attempts (proxy
  exited; further retries skipped)."

The exit details appended to the message (code/signal/stderr) are
unchanged. The two pinning tests now assert the new wording, and the
exhaustion test pins that the full count keeps its plain form.

Fixes #517


Claude-Session: https://claude.ai/code/session_01XtvNu3aNB1nw1URRL5UTgR

Co-authored-by: CI Bot <ci@example.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
debugmcpdev added a commit that referenced this pull request Aug 27, 2026
…an't stall adoption (#531)

The post-attach 'initialized' event can share a socket chunk with the
attach response. MinimalDapClient dispatches events synchronously while
the response's awaiter is still parked in the microtask queue, so the
waitForEvent listener that handlePostAttachInit registers afterwards
never sees it — the 3s wait times out in full, and the CDP
function-breakpoint bridge attaches only after the forced entry pause
has been auto-continued. Pre-launch function breakpoints on ESM
module-scoped names then silently never arm (they can only resolve at a
pause, and the missed entry pause was the last one).

Count 'initialized' events from wireChildEvents onward and have both
waiters (initializeChild's and handlePostAttachInit's) consume the
latch against a baseline snapshotted before their triggering request is
sent — same shape as the #515 init-ACK latch. The latch check and
waitForEvent's listener registration share one synchronous frame, so no
event can slip between them.

Fixes #529


Claude-Session: https://claude.ai/code/session_01RiTRwc8dshMYgmbp5o5m5t

Co-authored-by: CI Bot <ci@example.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProxyManager loses a late init ACK: any worker boot >500ms fails the whole launch as 'Failed to initialize proxy after 6 attempts'

1 participant