Don't send background notifications to main agent when subagent is used#308836
Don't send background notifications to main agent when subagent is used#308836meganrogge wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts terminal tool background-notification behavior to avoid sending “background command completed” steering messages to the parent chat session when the terminal was started by a subagent (which lacks the context/tool loop to act on that notification).
Changes:
- Gate background completion notifications on
!invocation.subAgentInvocationIdto suppress steering messages for subagent-initiated background terminals. - Update inline comments to document why subagent background notifications are skipped and how subagents should retrieve output.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts | Suppresses background terminal completion steering notifications for subagent tool invocations. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts:1403
- This introduces new behavior that disables background completion notifications when
invocation.subAgentInvocationIdis set. Given the existing comprehensiveRunInTerminalTooltest suite, it would be good to add coverage asserting that subagent invocations do not register a background notification / steering message (and that the tool’s returned text doesn’t claim a notification will arrive).
// Skip notifications for subagent-initiated terminals: the subagent runs in its
// own tool calling loop and cannot receive steering messages. It can poll with
// get_terminal_output instead.
if (this._configurationService.getValue(TerminalChatAgentToolsSettingId.BackgroundNotifications) && !invocation.subAgentInvocationId) {
this._registerCompletionNotification(toolTerminal.instance, termId, chatSessionResource, command, outputMonitor);
- Files reviewed: 1/1 changed files
- Comments generated: 1
| // background terminal, and continue the output monitor for prompt-for-input detection. | ||
| // Skip notifications for subagent-initiated terminals: the subagent runs in its | ||
| // own tool calling loop and cannot receive steering messages. It can poll with | ||
| // get_terminal_output instead. | ||
| if (this._configurationService.getValue(TerminalChatAgentToolsSettingId.BackgroundNotifications) && !invocation.subAgentInvocationId) { | ||
| this._registerCompletionNotification(toolTerminal.instance, termId, chatSessionResource, command, outputMonitor); |
There was a problem hiding this comment.
With this change, subagent-initiated background terminals will no longer trigger steering notifications, but other parts of this tool still tell the model it will be “automatically notified” when chat.tools.terminal.backgroundNotifications is enabled (e.g. the timeout hint around notificationHint and the tool/model descriptions). That becomes incorrect for subagent invocations and can cause the subagent to wait/assume a notification that never arrives. Consider centralizing a shouldSendBackgroundNotifications = configEnabled && !invocation.subAgentInvocationId flag and using it both here and anywhere we generate those “you will be notified” hints (or wording them so they don’t promise notifications in subagent context).
This issue also appears on line 1399 of the same file.
fix #308048
The subagent only had access to
run_in_terminal— it couldn't check output, send input, or kill terminals. We've added access to those tools for the subagent microsoft/vscode-copilot-chat#5050.Background terminal completion notifications were sent to the parent session, which had no context for them. When it's a subagent, we no longer do this.