Skip to content

run_in_terminal: promote sync command to background after idle output#315885

Merged
meganrogge merged 2 commits into
mainfrom
merogge/terminal-idle-silence-promotion
May 12, 2026
Merged

run_in_terminal: promote sync command to background after idle output#315885
meganrogge merged 2 commits into
mainfrom
merogge/terminal-idle-silence-promotion

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

@meganrogge meganrogge commented May 11, 2026

Fixes #315884

If a synchronous run_in_terminal call produces no output for N ms, win the foreground race with a new idleSilence candidate that mirrors the existing timeout handler: promote the execution to background, return the terminal ID + output collected so far, and append a steering hint. The process is never killed. The model can then call get_terminal_output to keep watching, send_to_terminal if it suspects an input prompt, or kill_terminal if it judges the command stuck.

Gated on a new setting chat.tools.terminal.idleSilenceTimeoutMs (default 60000; 0 disables).

Why this is safe

  • Commands that produce output regularly (npm install, cargo build, tsc watch, pytest, …) reset the timer and never trip.
  • The async (waitStrategy === 'idle') path is unchanged — it already has the OutputMonitor idle signal.
  • The listener + RunOnceScheduler are owned by the existing raceCleanup DisposableStore, so they go away when another race candidate wins.

Cases this catches

Silent long-running commands the foreground path has no answer for today: jj snapshotting a Firefox-sized checkout (the real repro that motivated this), stalled SSH handshakes, git fetch against a dead remote, etc. See #315884 for the full repro.

If a synchronous run_in_terminal call produces no output for N ms, win the
foreground race with a new idleSilence candidate that mirrors the existing
timeout handler: promote the execution to background, return the terminal
ID + output collected so far, append a steering hint. The process is never
killed.

Gated on chat.tools.terminal.idleSilenceTimeoutMs (default 60000, 0
disables). Listener and scheduler are owned by the existing raceCleanup
DisposableStore so they go away when another candidate wins. Async
(waitStrategy === 'idle') path is unchanged.

Fixes #315884
Copilot AI review requested due to automatic review settings May 11, 2026 22:05
@meganrogge meganrogge self-assigned this May 11, 2026
@meganrogge meganrogge added this to the 1.121.0 milestone May 11, 2026
@meganrogge meganrogge enabled auto-merge (squash) May 11, 2026 22:06
@meganrogge
Copy link
Copy Markdown
Collaborator Author

/requires-eval-assessment terminalbench2 gpt-5.4,claude-opus-4.6,claude-opus-4.7

@meganrogge meganrogge added the ~requires-eval-assessment Evals will be run and will generate a report upon completion label May 11, 2026
anthonykim1
anthonykim1 previously approved these changes May 11, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the run_in_terminal tool’s synchronous (foreground) execution path to avoid indefinite blocking when a command produces no output for an extended period, by promoting the execution to a background terminal after an “idle output” threshold and returning partial output plus steering guidance.

Changes:

  • Added a new setting chat.tools.terminal.idleSilenceTimeoutMs (default 60000, 0 disables) to control idle-silence promotion behavior.
  • Extended the foreground Promise.race in runInTerminalTool.ts with an idleSilence candidate driven by a RunOnceScheduler that resets on terminal output.
  • Added a new steering note for the idle-silence promotion result so the model can continue via get_terminal_output, provide input, or terminate if needed.
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts Registers the new chat.tools.terminal.idleSilenceTimeoutMs configuration setting and schema metadata.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts Implements the idle-output promotion race candidate and emits an idle-silence steering note when it wins.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

@vs-code-engineering
Copy link
Copy Markdown
Contributor

⏳ Queued vscode build for 62faacea0ff91faae6954a6d05279c15c3f0b1c8 (step 1/2).

@meganrogge meganrogge disabled auto-merge May 11, 2026 22:19
Replace the boolean mentionTimeout parameter on _buildInputNeededSteeringText with a 'none' | 'timeout' | 'idleSilence' discriminator so the idle-silence promotion result no longer reuses the timeout wording. Add focused unit tests covering each mode.
@meganrogge
Copy link
Copy Markdown
Collaborator Author

Addressed both review comments in 6127d25:

Comment 1 (idle-silence wording): Replaced the mentionTimeout: boolean parameter on _buildInputNeededSteeringText with a 'none' | 'timeout' | 'idleSilence' discriminator. The idle-silence branch now emits its own wording ("Producing no output for an extended period does not mean the command failed...") instead of reusing the timeout phrasing. kill_terminal is still advertised because the command may be genuinely hung, but the message no longer claims a timeout occurred.

Comment 2 (test coverage): Added a focused input-needed steering text suite that exercises the helper for each of the three modes and asserts:

  • none mode mentions neither timeout nor kill_terminal,
  • timeout mode mentions timeout and advertises kill_terminal,
  • idleSilence mode advertises kill_terminal but does not say "timeout" and describes the no-output condition.

A full integration test of the race candidate itself would require extending the mock terminal with an onData emitter plus fake timers; happy to follow up with that in a separate change if you'd like deeper coverage of the race wiring.

@meganrogge meganrogge added ~requires-eval-assessment Evals will be run and will generate a report upon completion and removed ~requires-eval-assessment Evals will be run and will generate a report upon completion labels May 11, 2026
@vs-code-engineering
Copy link
Copy Markdown
Contributor

⏳ Queued vscode build for 6127d25b49b46fcdbda2e7e99e7c764d80e197cb (step 1/2).

@vs-code-engineering vs-code-engineering Bot removed the ~requires-eval-assessment Evals will be run and will generate a report upon completion label May 11, 2026
@meganrogge meganrogge added the ~requires-eval-assessment Evals will be run and will generate a report upon completion label May 12, 2026
@vs-code-engineering
Copy link
Copy Markdown
Contributor

⏳ Queued vscode build for 6127d25b49b46fcdbda2e7e99e7c764d80e197cb (step 1/2).

@vs-code-engineering
Copy link
Copy Markdown
Contributor

🔄 First vscode build failed; retried failed stages: https://dev.azure.com/monacotools/Monaco/_build/results?buildId=438396

@vs-code-engineering vs-code-engineering Bot removed the ~requires-eval-assessment Evals will be run and will generate a report upon completion label May 12, 2026
@meganrogge meganrogge merged commit c414bd9 into main May 12, 2026
25 checks passed
@meganrogge meganrogge deleted the merogge/terminal-idle-silence-promotion branch May 12, 2026 20:14
@meganrogge meganrogge added the ~requires-eval-assessment Evals will be run and will generate a report upon completion label May 12, 2026
@vs-code-engineering
Copy link
Copy Markdown
Contributor

❌ Failed to queue vscode build for 6127d25b49b46fcdbda2e7e99e7c764d80e197cb due to an Azure DevOps permission issue — eval-assessment did not start.

The federated identity vscode-bot-managed-identity is missing Queue builds permission on pipeline ⭐️ VS Code (id 111) in project Monaco.

Fix: in Azure DevOps, open the pipeline → ⋮ → Security, and grant vscode-bot-managed-identity the Queue builds permission, then re-apply the ~requires-eval-assessment label.

az pipelines run output
ERROR: Unable to resolve the reference 'refs/pull/315885/merge' to a specific version. Verify the reference exists in the source repository.

@vs-code-engineering vs-code-engineering Bot removed the ~requires-eval-assessment Evals will be run and will generate a report upon completion label May 12, 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.

run_in_terminal: promote sync command to background after N seconds of no output

4 participants