fix: report pending success instead of failing when step/pause outlives grace window#144
Merged
Merged
Conversation
…es grace window
Step over/into/out and pause_execution blocked on the 'stopped' DAP event
with a hardcoded 5s deadline and returned a spurious success:false
("did not complete within 5s") when the debuggee was simply still
executing — e.g. stepping over a long-running call. The eventual stop was
then absorbed silently by the background listener.
These timeouts imposed a fixed requirement on debuggee runtime behavior.
Convert them to grace windows with truthful async semantics, mirroring
continue_execution:
- On expiry, return success:true with state 'running' and data.pending,
plus a message explaining the session becomes 'paused' when the
operation completes (the persistent handleStopped listener already
handles that transition, with no timeout).
- Extract the 5000ms literals into protected stepGraceMs/pauseGraceMs
fields (test-shrinkable), matching the attach* field pattern.
- Replace ErrorMessages.stepTimeout/pauseTimeout with informational
stepStillRunning/pausePending factories.
- Surface pending:true and the truthful message in the step tool
response instead of an unconditional "Stepped over".
- Document the blocking contract in the step/continue/pause tool
descriptions so LLM clients know a pending step is not a failure.
- Add a test pinning the async completion path (late stop -> PAUSED).
The remaining debuggee-dependent timeouts found in the same audit are
tracked separately: evaluate_expression request timeouts (#142) and the
attach thread-verification window (#143).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Codecov flagged the new pending branch in the step_over/into/out handler as uncovered. This is the layer an MCP client actually sees, so pin the contract: a pending DebugResult from the session manager must surface as success:true with state 'running', pending:true, the truthful still-running message (not "Stepped X"), and no location. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 7, 2026
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.
Problem
step_over/step_into/step_outandpause_executionblocked on thestoppedDAP event with a hardcoded 5s deadline and returned a spurioussuccess:false("Step operation did not complete within 5s") when the debuggee was simply still executing — e.g. stepping over a long-running call, or pausing a target blocked in native code. The eventual stop was then absorbed silently by the background listener, so the user was told the step failed and the session later flipped topausedwith no report. This was previously observed in real use (docs/archive/MCP_DEBUGGER_COMPREHENSIVE_TEST_RESULTS.md).These timeouts imposed a fixed requirement on debuggee runtime behavior; they existed to keep the wait finite, not because a slow step is an error.
Fix — truthful async semantics
Mirror how
continue_executionalready works (ack now, stop reported asynchronously):success:true,state:"running",pending:truewith a message explaining the session becomespausedwhen the operation completes — the persistenthandleStoppedlistener (which has no timeout) already handles that transition.5000literals into protectedstepGraceMs/pauseGraceMsfields (test-shrinkable), matching the existingattach*field pattern.ErrorMessages.stepTimeout/pauseTimeoutwith informationalstepStillRunning/pausePendingfactories.pending:trueand the truthful message in the step tool response instead of an unconditional "Stepped over".step_*/continue_execution/pause_executiontool descriptions so LLM clients know a pending step is not a failure.docs/patterns/error-handling.md,event-management.md) to show the grace-window pattern.Testing
stopped→PAUSED).time.sleep(15)in a Python session returns the pending success after ~5s (previously a spurious failure), the session flips topausedat the post-sleep line when the sleep ends, and a trivial step still returns immediately with location/context.Follow-ups filed from the same audit
evaluate_expressioncan spuriously time out on long-running expressions (uniform 35s/30s/30s request timeouts across three layers)🤖 Generated with Claude Code