Skip to content

Detach rewriter: also wrap sync-mode commands ending in trailing &#313379

Merged
meganrogge merged 1 commit intomainfrom
merogge/detach-trailing-amp
Apr 30, 2026
Merged

Detach rewriter: also wrap sync-mode commands ending in trailing &#313379
meganrogge merged 1 commit intomainfrom
merogge/detach-trailing-amp

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

Fixes microsoft/vscode-internalbacklog#7605

When the chat.tools.terminal.detachBackgroundProcesses setting is enabled, also wrap commands with nohup when the agent passed mode='sync' but the command line ends with a single trailing & (POSIX background operator, distinct from &&).

Previously the rewriter only fired for mode='async' (legacy isBackground: true). When the agent used mode='sync' + &, the & was interpreted by the foreground shell and the process was SIGHUP'd as soon as the tool's terminal was disposed — fine for interactive UX, but it breaks eval harnesses where the eval step connects to the service from a fresh process and gets Connection refused.

Evidence

terminalbench2 run 25136645365 (claude-opus-4.6) had three failures attributable to this:

  • hf-model-inferencepython app.py > /app/server.log 2>&1 &
  • install-windows-3.11qemu-system-i386 ... -vnc :1 -qmp ... -monitor ... (no daemonize)
  • qemu-alpine-sshnohup expect setup_vm.exp > /app/vm_output.log 2>&1 & in sync mode

In all three the agent's same-turn verification (curl, ps aux, ss -tlnp) confirmed the service was alive — it just didn't survive to the eval phase.

Behavior

  • Setting still gates the rewrite, so default users are unaffected.
  • Eval harnesses already set chat.tools.terminal.detachBackgroundProcesses=true (see vscode-copilot-evaluation/src/vsCodeApplication.ts), so they pick this up automatically.
  • PowerShell has no equivalent trailing-& operator, so the Windows path remains async-only.
  • Existing nohup/shell -c wrapping of compound constructs and the de-duplication of an already-trailing & continue to work.

Copilot AI review requested due to automatic review settings April 29, 2026 23:42
@meganrogge meganrogge self-assigned this Apr 29, 2026
@meganrogge meganrogge added the bug Issue identified by VS Code Team member as probable bug label Apr 29, 2026
@meganrogge meganrogge added this to the 1.119.0 milestone Apr 29, 2026
@meganrogge meganrogge removed the bug Issue identified by VS Code Team member as probable bug label Apr 29, 2026
@meganrogge meganrogge enabled auto-merge (squash) April 29, 2026 23:43
@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 Apr 29, 2026
@vs-code-engineering
Copy link
Copy Markdown
Contributor

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

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

Extends the terminal chat tool’s background-detach command-line rewriter so eval harnesses (which enable chat.tools.terminal.detachBackgroundProcesses) also detach POSIX commands invoked in mode='sync' when the command line ends with a single trailing &, preventing services from dying when the tool terminal is disposed.

Changes:

  • Gate rewriter on the detach setting first, then detach when isBackground=true or the command ends with a bare trailing & (POSIX).
  • Keep Windows behavior async-only (PowerShell path only rewrites when isBackground=true).
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineRewriter/commandLineBackgroundDetachRewriter.ts Expand detach criteria to include sync-mode POSIX commands ending in trailing &; preserve Windows async-only behavior.

Copilot's findings

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

@meganrogge meganrogge merged commit fd6f65c into main Apr 30, 2026
30 checks passed
@meganrogge meganrogge deleted the merogge/detach-trailing-amp branch April 30, 2026 00:08
@vs-code-engineering
Copy link
Copy Markdown
Contributor

🚀 Queued eval-assessment publish build for dc1d3cbdcb24b73f2b17611a67ef27975fabee97 (step 2/2).

@vs-code-engineering
Copy link
Copy Markdown
Contributor

🔬 Queued eval-assessment benchmark for a6f887c92a.

Results will be posted back here when the run completes.

@vs-code-engineering
Copy link
Copy Markdown
Contributor

✅ Eval-assessment build published.

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

📊 Eval-assessment benchmark complete.

Analysis Results

Resolution Rate

Benchmark Total Cases Passed Failed Resolved Rate
terminalbench2 89 59 30 66.29%

Token Usage

Metric Value
Total Tokens 59,731,878
Input Tokens 58,729,020
Output Tokens 1,002,858
Cached Tokens 55,911,102

Step Counts

Metric Value
Total Steps 1,654
Mean Steps/Instance 18.58

@vs-code-engineering
Copy link
Copy Markdown
Contributor

📊 Eval-assessment benchmark complete.

Analysis Results

Resolution Rate

Benchmark Total Cases Passed Failed Resolved Rate
terminalbench2 89 49 40 55.06%

Token Usage

Metric Value
Total Tokens 72,069,420
Input Tokens 71,143,963
Output Tokens 925,457
Cached Tokens 59,988,608

Step Counts

Metric Value
Total Steps 1,672
Mean Steps/Instance 18.79

@vs-code-engineering
Copy link
Copy Markdown
Contributor

📊 Eval-assessment benchmark complete.

Analysis Results

Resolution Rate

Benchmark Total Cases Passed Failed Resolved Rate
terminalbench2 89 51 38 57.30%

Token Usage

Metric Value
Total Tokens 68,038,993
Input Tokens 66,592,678
Output Tokens 1,446,315
Cached Tokens 63,330,502

Step Counts

Metric Value
Total Steps 1,955
Mean Steps/Instance 21.97

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants