Skip to content

fix(proxy): thread-anchor guard on threads responses + honest tool:response success - #407

Merged
debugmcpdev merged 1 commit into
mainfrom
fix/396-397-thread-anchor-and-tool-response
Aug 22, 2026
Merged

fix(proxy): thread-anchor guard on threads responses + honest tool:response success#407
debugmcpdev merged 1 commit into
mainfrom
fix/396-397-thread-anchor-and-tool-response

Conversation

@debugmcpdev

Copy link
Copy Markdown
Collaborator

Fixes #396
Fixes #397

Two small correctness/telemetry fixes from the attach-hardening follow-ups.

#396 — ProxyManager no longer clobbers currentThreadId with threads[0]

handleDapResponse captured a thread id from every successful threads response, unconditionally overwriting the id anchored by a stopped event. Any list_threads call (or internal threads poll — the attach verify loop, the stack-readiness scan, pause discovery) landing while paused on a non-first thread silently retargeted subsequent stackTrace/scopes/evaluate to threads[0].

The capture now only fires when no thread id is known (currentThreadId == null), preserving the fallback for adapters that omit threadId from stopped events.

  • New test: stopped event anchors thread 12 → a threads response listing [1, 12] no longer retargets to 1.
  • The existing fallback-capture test (no prior anchor → captures threads[0]) still passes unchanged.
  • The dap-core functional mirror only captures on stopped already, so no core divergence.

#397tool:response log line reports the payload's own success

The structured tool:response line hardcoded success: true (meaning "the handler didn't throw"), directly contradicting payloads like a failed attach_to_process that return { success: false } without throwing — actively misleading during triage and hiding failure rates from log-based metrics.

extractPayloadSuccess() now parses the first text content entry and mirrors its boolean success when present; payloads without one (or non-JSON) still log success: true. tool:error (thrown path) is unchanged.

  • New tests assert tool:response carries success: false for a failing attach payload and success: true for a successful one.
  • docs/logging-format-specification.md updated: documents the new success semantics and drops the response field that was never actually emitted.

Verification

  • npx vitest run tests/unit/proxy/ tests/core/unit/server/ — 37 files, 663 tests pass
  • Full npm test green
  • npm run lint clean

🤖 Generated with Claude Code

…sponse success

- ProxyManager only captures threads[0] as currentThreadId when no thread
  is anchored yet, so list_threads and internal thread polls no longer
  retarget stackTrace/scopes/evaluate away from the stopped thread (#396)
- The tool:response log line mirrors the tool payload's own success
  boolean instead of hardcoding true (#397); logging-format spec synced

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/server.ts 88.88% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@debugmcpdev
debugmcpdev merged commit 2014358 into main Aug 22, 2026
8 of 9 checks passed
@debugmcpdev
debugmcpdev deleted the fix/396-397-thread-anchor-and-tool-response branch August 22, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants