agentHost: adopt notify/sessionSummaryChanged protocol notification#308889
agentHost: adopt notify/sessionSummaryChanged protocol notification#308889connor4312 merged 1 commit intomainfrom
Conversation
- Syncs agent-host-protocol to pick up the new `notify/sessionSummaryChanged` notification type, which lets clients keep cached session lists in sync without subscribing to every session URI individually. - Emits the notification from AgentHostStateManager: after the session reducer runs, an identity check on the summary object detects changes, which are debounced (100ms) and flushed as partial diffs. - Replaces the onDidAction-based subscription-peeking pattern in AgentHostSessionListController with the cleaner notification-based approach, removing the need for StateComponents/isSessionAction imports and the getSubscriptionUnmanaged call. - Adds unit tests for emission, coalescing, no-op suppression, and cleanup on session deletion. Adopts microsoft/agent-host-protocol#51 Fixes #305330 (Commit message generated by Copilot)
There was a problem hiding this comment.
Pull request overview
This PR updates VS Code’s agent host integration to use the new notify/sessionSummaryChanged protocol notification so clients can keep cached session lists in sync without subscribing to every session URI.
Changes:
- Adds and documents the
notify/sessionSummaryChangedprotocol notification and updates the protocol sync marker. - Emits debounced/coalesced
sessionSummaryChangednotifications fromAgentHostStateManagerwhen session summaries change. - Switches
AgentHostSessionListControllerto update from notifications instead of peeking subscriptions/actions, and adds unit tests for notification behavior.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts | Consume sessionSummaryChanged notifications and maintain a cached summary map for partial updates. |
| src/vs/platform/agentHost/node/agentHostStateManager.ts | Track last-notified summaries and emit debounced/coalesced sessionSummaryChanged notifications with partial diffs. |
| src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts | Add tests for emission, debouncing/coalescing, no-op suppression, and deletion cleanup. |
| src/vs/platform/agentHost/common/state/protocol/version/registry.ts | Register the new notification type in the protocol version registry. |
| src/vs/platform/agentHost/common/state/protocol/notifications.ts | Define and document the new ISessionSummaryChangedNotification notification type. |
| src/vs/platform/agentHost/common/state/protocol/.ahp-version | Bump synced agent-host-protocol revision marker. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 2
| * Identity fields (`resource`, `provider`, `createdAt`) never change and | ||
| * MUST be omitted by senders; receivers SHOULD ignore them if present. | ||
| */ | ||
| changes: Partial<ISessionSummary>; |
There was a problem hiding this comment.
ISessionSummaryChangedNotification.changes is typed as Partial<ISessionSummary>, but the doc states identity fields (resource, provider, createdAt) MUST be omitted. Consider tightening the type to Partial<Omit<ISessionSummary, 'resource' | 'provider' | 'createdAt'>> (or a dedicated ISessionSummaryChanges type) so callers can’t accidentally include immutable fields.
| changes: Partial<ISessionSummary>; | |
| changes: Partial<Omit<ISessionSummary, 'resource' | 'provider' | 'createdAt'>>; |
| if (!cached) { | ||
| return; | ||
| } | ||
| const updated = { ...cached, ...n.changes }; |
There was a problem hiding this comment.
When applying notify/sessionSummaryChanged, spreading ...n.changes can overwrite immutable summary fields (e.g. resource, provider, createdAt) if a server bug/misbehaving peer sends them, which can desync _cachedSummaries from _items. Since the protocol explicitly treats those fields as immutable, consider explicitly applying only the allowed mutable keys (or filtering them out) instead of a raw spread merge.
| const updated = { ...cached, ...n.changes }; | |
| const mutableChanges = Object.fromEntries( | |
| Object.entries(n.changes).filter(([key]) => key !== 'resource' && key !== 'provider' && key !== 'createdAt') | |
| ) as Partial<ISessionSummary>; | |
| const updated = { ...cached, ...mutableChanges }; |
notify/sessionSummaryChangednotification type, which lets clientskeep cached session lists in sync without subscribing to every
session URI individually.
reducer runs, an identity check on the summary object detects
changes, which are debounced (100ms) and flushed as partial diffs.
AgentHostSessionListController with the cleaner notification-based
approach, removing the need for StateComponents/isSessionAction
imports and the getSubscriptionUnmanaged call.
cleanup on session deletion.
Adopts https://github.com/microsoft/agent-host-protocol/pull/51
Fixes #305330
(Commit message generated by Copilot)