Skip to content

fix(terminal): recover from lost VSCode shell-integration completion signals (#800)#834

Draft
edelauna wants to merge 1 commit into
mainfrom
issue/800
Draft

fix(terminal): recover from lost VSCode shell-integration completion signals (#800)#834
edelauna wants to merge 1 commit into
mainfrom
issue/800

Conversation

@edelauna

@edelauna edelauna commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Related GitHub Issue

Closes: #800

Description

VSCode's shell-integration API can silently fail to signal command completion for certain command shapes: onDidEndTerminalShellExecution never fires and the execution.read() stream never closes on its own, even though the command has genuinely finished (or, in the worst case, VSCode delivers zero stream data at all, not even the start marker). Without a fallback, the task stalls forever on a "Running" command with no way to recover short of reloading the window — this is the root cause of #800 and several related Discord reports.

The fix: TerminalProcess.run() now detects the ]633;D completion marker directly in the accumulated stream data as it arrives, and self-finalizes as soon as it sees it — independent of whether the stream or the event ever confirms completion. It then waits a brief (1s) grace period for the real onDidEndTerminalShellExecution event to still capture an accurate exit code, and proceeds without one if it doesn't arrive.

What this deliberately does NOT do: guess a "gone quiet, must be done" idle-timeout. An earlier version of this fix included a 15s idle-timeout as a fallback for the case where even the D marker never arrives. That was reverted after live testing showed it fires falsely on legitimate long-running, quiet commands — e.g. a cold cd src && npx tsc --noEmit on this repo takes ~24s with zero interim output, which is indistinguishable from the broken-signal case by elapsed time alone. Any fixed threshold either fires falsely on real work or is too long to be useful, so this fix only self-finalizes on positive proof (the marker itself).

Related fix: while investigating, found that a late/stale onDidEndTerminalShellExecution event for an already-superseded execution could be misapplied to whatever command is currently running on a reused terminal — corrupting its exit code and prematurely clearing its process state. TerminalRegistry now tracks the specific execution each TerminalProcess was started with (ownExecution) and ignores end-events that don't match the terminal's current process, regardless of what terminal.activeShellExecution currently points to.

Observability: added permanent diagnostic logging ([Terminal Process] stream chunk #N, D-marker detection, and post-marker wait outcome) so this class of bug is diagnosable going forward without needing to re-instrument.

Known residual gap

Some command shapes reliably cause VSCode to emit zero stream data at all — not just a lost completion signal, but no data whatsoever, confirmed via a controlled e2e repro (100% reproducible in this environment) for:

  • A python3 -c "..." command containing a literal embedded newline (auto-wrapped by prepareCommandForShellIntegration into { ...\n... })
  • A ( ... ) subshell wrapper (a construct we don't generate ourselves, but a model could)

For these, VSCode's own onDidStartTerminalShellExecution event was observed reporting a corrupted command field — the script's stderr output was concatenated directly into what VSCode thinks is the command-line string — direct evidence this is a VSCode-internal shell-integration parser bug, not something addressable by changing our own command-wrapping strategy. Manual live testing after this fix confirmed: normal long-running quiet commands (tsc --noEmit) now work correctly, but this specific narrow class of command shape still hangs, requiring the user to reload the window as before. This is a real, upstream VSCode bug that we could not safely paper over without reintroducing false positives on legitimate commands.

Test Procedure

  • Unit tests: src/integrations/terminal/__tests__/TerminalProcess.spec.ts and TerminalRegistry.spec.ts — new regression tests cover: (1) self-finalizing on the D marker when the event never fires, (2) NOT falsely completing a long-running silent command, (3) a late/stale end-event for a superseded execution not corrupting the next command's state (verified this test fails without the TerminalRegistry fix, confirming it's a real regression guard).
  • E2E: apps/vscode-e2e/src/suite/tools/fast-exit-shell-race.test.ts — drives the real VSCode integrated terminal (not the Execa fallback) with a fast-exiting multi-line command, verified against a live, non-mocked VSCode instance.
  • Manual: rebuilt and reloaded the extension locally; confirmed tsc --noEmit-style commands no longer falsely report "still running," while documenting the residual gap above via direct reproduction.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes.
  • Documentation Impact: No user-facing documentation updates required.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: df3dfbfe-8c15-49fe-ac7e-7a4403b3dc74

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue/800

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@edelauna edelauna changed the title fix(terminal): detect D marker directly to recover from lost VSCode s… fix(terminal): recover from lost VSCode shell-integration completion signals (#800) Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@edelauna edelauna mentioned this pull request Jul 5, 2026
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.

[BUG]

1 participant