Agent Host Copilot CLI: Support async shell completion notifications#318511
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Copilot Agent Host support for async system.notification events so shell/background completions can appear in chat even when they arrive outside the normal request/response flow.
Changes:
- Adds Copilot SDK notification plumbing and translation for supported system notification kinds.
- Threads system-initiated turn metadata through agent-host protocol state and chat session/model history.
- Restores system notifications as chat progress messages and adds tests for notification turn/progress behavior.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts |
Updates fake session event typing for server-initiated request metadata. |
src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts |
Adds tests for system-initiated history and notification progress conversion. |
src/vs/workbench/contrib/chat/common/chatSessionsService.ts |
Extends chat session request/history types with system-initiated metadata. |
src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts |
Preserves system-initiated fields when creating chat model requests. |
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts |
Converts agent-host system notification response parts to chat progress messages. |
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts |
Propagates system-initiated labels through active/restored agent-host sessions. |
src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts |
Adds Copilot system notification translation and lifecycle tests. |
src/vs/platform/agentHost/node/copilot/copilotSystemNotification.ts |
Introduces notification content cleanup and supported-kind label mapping. |
src/vs/platform/agentHost/node/copilot/copilotSessionWrapper.ts |
Exposes the Copilot SDK system.notification event. |
src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts |
Handles notifications by appending to active turns or synthesizing system-initiated turns. |
src/vs/platform/agentHost/common/state/protocol/channels-session/state.ts |
Adds system-initiated labels to turn state. |
src/vs/platform/agentHost/common/state/protocol/channels-session/reducer.ts |
Carries system-initiated labels through active and completed turns. |
src/vs/platform/agentHost/common/state/protocol/channels-session/actions.ts |
Adds system-initiated labels to turn-start actions. |
Copilot's findings
- Files reviewed: 13/13 changed files
- Comments generated: 0
| /** User's message */ | ||
| userMessage: UserMessage; | ||
| /** Display label for a system-initiated turn. */ | ||
| systemInitiatedLabel?: string; |
There was a problem hiding this comment.
Make sure these get upstreamed in the protocol
There was a problem hiding this comment.
Incoming: microsoft/agent-host-protocol#150
Adopting again since we're going with: microsoft/agent-host-protocol#155
|
I'm suggest probably resetting before f65ffe9 and merging |
f65ffe9 to
a7b4d47
Compare
| } | ||
|
|
||
| this._logService.info(`[Copilot:${sessionId}] System notification received: kind=${e.data.kind.type}`); | ||
| if (this._turnId) { |
There was a problem hiding this comment.
I don't understand this, what kind of system notification is this that becomes part of the response?
There was a problem hiding this comment.
These are SDK system.notification events, which are:
a background shell finishing (SystemNotificationShellCompleted / SystemNotificationShellDetachedCompleted) or a background agent finishing (SystemNotificationAgentCompleted). When one arrives mid-turn we attach it as part of the current response.
a7b4d47 to
5a4c6d5
Compare
| */ | ||
| export const SUPPORTED_PROTOCOL_VERSIONS: readonly string[] = Object.freeze([ | ||
| PROTOCOL_VERSION, | ||
| '0.3.0', |
There was a problem hiding this comment.
Why is the constant not referenced here?
There was a problem hiding this comment.
ah, good point. I think it might have to come from upstream first though. Let me go file issue / tackle
Resolves: #318505
Related: #312178
system.notificationevents from Agent Host Copilot sessions when the custom terminal tool is disabled and the SDK native shell tool is in use.shell_completed,shell_detached_completed, andagent_completed.SystemNotificationResponsePartwhen a notification arrives during an active turn, so in-turn notifications do not create duplicate turns.session.idleso later async completions do not attach to a stale turn.Before:

After:

Inspirations from:
system.notification, translate supported kinds, and trigger a system-initiated follow-up turn. SeeCopilotCLISession.CopilotCLISessionService.onDidReceiveSystemNotification.isSystemInitiated/systemInitiatedLabel; this PR threads those fields through the Agent Host session path instead of inventing a new UI concept. SeeChatModel.addRequest.ChatListRendererand its label fallback atsystemInitiatedLabel ?? messageText.SystemNotificationResponsePart.