Skip to content

Deprecate chat.tools.terminal.backgroundNotifications — always send terminal notifications#309555

Merged
meganrogge merged 2 commits intomainfrom
merogge/bring-setting-depr
Apr 13, 2026
Merged

Deprecate chat.tools.terminal.backgroundNotifications — always send terminal notifications#309555
meganrogge merged 2 commits intomainfrom
merogge/bring-setting-depr

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

@meganrogge meganrogge commented Apr 13, 2026

Addresses #309523
Addresses #308548

Why this needs to be fixed now

This was filed as part of testing the terminal tool improvements (#309095). The chat.tools.terminal.backgroundNotifications setting has an inconsistent off-state: setting it to false suppresses notifications for true async terminals (mode=async) but not for foreground terminals that moved to background (timeout/input-needed). This is because the code intentionally bypasses the setting for fg-to-bg transitions — the tool result already promises the model "you will be notified when it completes," so suppressing the notification would leave the agent unable to learn the command finished.

This means the setting was never truly "off" — it only partially suppressed notifications, making it confusing for users and impossible to test deterministically (#309523).

What changed

  • Always send completion/input-needed notifications for any terminal in background execution (both true async and fg-to-bg), except for subagent-initiated terminals which can't receive steering messages
  • Deprecated the setting with default changed to true and a deprecation message
  • Updated tool description text to always include the notification language, removing the conditional phrasing that was misleading
  • Removed the backgroundNotifications parameter plumbing from all model description helper functions

Rationale

  • Notifications are strictly better for agent quality than requiring polling — less latency, fewer wasted get_terminal_output calls
  • The fg-to-bg path already always notified regardless of the setting, so behavior was already inconsistent
  • The setting was experimental and gated for A/B rollout (experiment: { mode: 'auto' }), not intended as a permanent user preference
  • The only cost of always-on is extra LLM turns for true async terminals, which is the correct behavior anyway

Copilot AI review requested due to automatic review settings April 13, 2026 17:31
@meganrogge meganrogge self-assigned this Apr 13, 2026
@meganrogge meganrogge added this to the 1.117.0 milestone Apr 13, 2026
@meganrogge meganrogge enabled auto-merge (squash) April 13, 2026 17:32
@github-actions
Copy link
Copy Markdown
Contributor

Screenshot Changes

Base: cf2074e6 Current: e7bcee60

Changed (2)

chat/aiCustomizations/aiCustomizationManagementEditor/McpBrowseMode/Light
Before After
before after
editor/inlineCompletions/other/JumpToHint/Dark
Before After
before after

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 deprecates chat.tools.terminal.backgroundNotifications and makes terminal completion/input-needed steering notifications unconditional for background terminal executions (excluding subagent-initiated terminals), to remove confusing/untestable partial “off” behavior.

Changes:

  • Deprecated chat.tools.terminal.backgroundNotifications (default set to true + deprecation message).
  • Removed backgroundNotifications plumbing from runInTerminal tool model-description helpers and always included notification guidance.
  • Updated RunInTerminalTool to always register background completion/input-needed notifications unless invoked by a subagent.
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts Deprecates the background notifications setting and updates default/deprecation messaging.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts Removes config gating + always registers background notifications; updates model-facing tool descriptions accordingly.

Copilot's findings

Comments suppressed due to low confidence (3)

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts:235

  • This model-facing guidance says notifications happen for "async terminal commands" only, but notifications are now sent for any command that continues running after the tool returns (including sync-timeout / fg→bg). Consider rewording to "background terminal commands" or explicitly include the sync-timeout/continue-in-background case so the model’s expectations match actual behavior.
Best Practices:
- Quote variables: "$var" instead of $var to handle spaces
- Use find with -exec or xargs for file operations
- Be specific with commands to avoid excessive output
- Avoid printing credentials unless absolutely required
- NEVER run sleep or similar wait commands in a terminal. You will be automatically notified on your next turn when async terminal commands complete or need input. Use ${TerminalToolId.GetTerminalOutput} to check output before then

Interactive Input Handling:
- When a terminal command is waiting for interactive input, do NOT suggest alternatives or ask the user whether to proceed. Instead, use the vscode_askQuestions tool to collect the needed values from the user, then send them.
- Send exactly one answer per prompt using ${TerminalToolId.SendToTerminal}. Never send multiple answers in a single send.
- After each send, call ${TerminalToolId.GetTerminalOutput} to read the next prompt before sending the next answer.
- Continue one prompt at a time until the command finishes.`);

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts:334

  • The tool’s modelDescription section is titled and worded as "Async terminal notifications" and refers specifically to commands in an async terminal, but the implementation now also notifies for sync executions that time out/continue in background. Please adjust this text to describe notifications for any background/continued execution (mode=async and mode=sync when the command continues running after return).
		displayName: localize('runInTerminalTool.displayName', 'Run in Terminal'),
		modelDescription: `${modelDescription}\n\nExecution mode:\n- mode='sync': wait for completion up to timeout; if still running, return with a terminal ID.\n- mode='async': wait for an initial idle/output signal, then return with terminal output snapshot and ID. Timeout caps how long to wait for the initial idle/output signal.\n- Prefer mode='sync' for commands that will prompt for interactive input (e.g., npm init, interactive installers, configuration wizards).\n\nAsync terminal notifications: When a command finishes in an async terminal, you will be automatically notified on your next turn with the exit code and terminal output. You will also be notified if the terminal needs input. Use ${TerminalToolId.GetTerminalOutput} to check output before then. Do NOT poll or sleep to wait for completion.`,
		userDescription: localize('runInTerminalTool.userDescription', 'Run commands in the terminal'),

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts:1709

  • In the timeout guidance, the tool now promises an automatic notification only "when it completes", but background notifications also include input-needed events. To keep the model instructions accurate (and consistent with the rest of the descriptions), mention that it will be notified if the terminal needs input as well, not just on completion.
			resultText.push(`Note: Command timed out after ${timeoutValue}ms. The command may still be running in terminal ID ${termId}. You will be automatically notified on your next turn when it completes. Use ${TerminalToolId.GetTerminalOutput} to check output before then, ${TerminalToolId.SendToTerminal} to send further input, or ${TerminalToolId.KillTerminal} to stop it. Do NOT use sleep or manual polling to wait. Evaluate the terminal output to determine if the command is waiting for input (e.g. a password prompt, confirmation, or interactive question). A normal shell prompt does NOT count as waiting for input. ${inputAction}\n\n`);
		}
  • Files reviewed: 2/2 changed files
  • Comments generated: 2

@anthonykim1
Copy link
Copy Markdown
Contributor

@meganrogge Seems like test might be acting flaky

@meganrogge meganrogge merged commit a400068 into main Apr 13, 2026
40 of 41 checks passed
@meganrogge meganrogge deleted the merogge/bring-setting-depr branch April 13, 2026 19:11
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.

4 participants