Skip to content

fix(mcp): preserve live graph request cancellation - #4333

Open
ScriptedAlchemy wants to merge 1 commit into
masterfrom
t3/fix-runtime-cancellation-races
Open

ScriptedAlchemy wants to merge 1 commit into
masterfrom
t3/fix-runtime-cancellation-races

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Summary

Stop a cancelled MCP graph request's blocking worker from reading another file after a subsequent ping has acknowledged cancellation.

Motivation

The caller's MCP cancellation signal was relayed asynchronously into a native request token and then into a reconstructed graph signal. A ping could finish before those relays ran, allowing a paused rename-preview worker to resume with an uncancelled replacement signal.

Changes

  • Retain the caller's live signal in the existing daemon request cancellation registry and retrieve it only for the matching native token.
  • Keep the request lease through in-process settlement, preserving daemon cancellation, deadlines, request isolation, and the existing typed invalid-request result for duplicate IDs.
  • Cover immediate signal visibility without polling a relay, daemon cancellation, isolation, and cancellation before registration. The existing strict rename-preview tests and all budgets remain unchanged.

No wire or persisted contracts change. No LSP source changes are included.

Verification

All checks ran through Bazel on this commit:

  • Strict transport-cancellation regression: 20/20 runs passed, each executing exactly one test with the original one-source-read assertion.
  • Strict deadline regression: 1 passed, with the existing anti-vacuity helper.
  • Daemon request-cancellation registry tests: 10 passed.
  • Affected daemon-service and composition-root rustfmt checks: passed.
  • Clippy for daemon-service unit tests and the MCP suite, with the repository's warnings-as-errors configuration: passed.
  • Native LSP refresh regression: 10/10 exact runs passed; full application unit suite: 510 passed.
  • One additional native LSP protocol trace passed: initialize write to Python response 110.270 ms, response to native stdout read 0.151 ms, child termination to reap 1.720 ms. Stdout reached EOF before test exit. The roughly five-second successful duration is the production diagnostics quiet window, distinct from the initialization budget. The historical initialize timeout did not reproduce and its cause remains unproven.

Commands used --remote_download_outputs=all for these invocations after the initial build encountered missing remote CAS artifacts; no repository cache configuration changed. A full workspace build is not claimed. Focused checks cover the changed daemon-service/composition-root boundary and the reported native LSP behavior.

Checklist

  • Original strict assertions and timeouts preserved
  • Relevant behavioral tests, formatting, and Clippy passed
  • No secrets, credentials, or environment files included
  • No wire or persisted contract changes

@changeset-bot

changeset-bot Bot commented Oct 18, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: fb85df8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ScriptedAlchemy
ScriptedAlchemy marked this pull request as ready for review October 18, 2026 21:23
@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) October 18, 2026 21:24

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment thread crates/tracedecay/src/daemon/invocation_executor.rs

This branch has not been deployed

No deployments
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.

1 participant