agentHost: dedupe no-op root state actions#314597
Merged
Merged
Conversation
Localized to the sessions provider
Skip envelope emission in AgentHostStateManager when a root action's reducer output deeply equals the previous root state. This prevents a feedback loop where a contribution re-publishes a root config value that already matches: the server would still fire a RootStateSubscription change, the contribution would react and re-dispatch, and so on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents infinite update loops in the agent host protocol by skipping action-envelope emission for root actions whose reducer output is deeply equal to the previous root state. This reduces unnecessary rootState change events and avoids feedback cycles when the same root config values are re-published.
Changes:
- Deduplicate no-op root actions in
AgentHostStateManager._applyAndEmitvia deep equality and skip envelope emission/serverSeqbump when unchanged. - Add a unit test verifying
RootConfigChangedno-op dispatches do not emit envelopes, including deep-equal object values. - Refactor agent-provider icon mapping out of
session.tsintoBaseAgentHostSessionsProvider.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/services/sessions/common/session.ts | Removes exported iconForAgentProvider helper (moved to provider implementation). |
| src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts | Adds private icon mapping for agent providers used when syncing session types from root state. |
| src/vs/platform/agentHost/node/agentHostStateManager.ts | Adds deep-equality no-op detection for root actions to avoid emitting envelopes when root state is unchanged. |
| src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts | Adds coverage ensuring no-op root config actions do not emit envelopes, including deep-equal nested values. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 1
Per review feedback, only RootConfigChanged is dispatched as a true no-op (its reducer spreads values even when the patch matches). Other root actions like RootActiveSessionsChanged fire frequently (every turn start/complete) and always carry a real value, so the deep-compare was wasted work for them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Screenshot ChangesBase: Changed (12)
|
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/Default/Light
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithArtifacts/Dark
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithArtifacts/Light
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithFileChanges/Dark
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithTodos/Light
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithTodosAndFileChanges/Dark
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithArtifactsAndFileChanges/Dark
| Before | After |
|---|---|
 |
 |
chat/input/chatInput/WithArtifactsAndFileChanges/Light
| Before | After |
|---|---|
 |
 |
chat/widget/chatWidget/SimpleQA/Light
| Before | After |
|---|---|
 |
 |
chat/widget/chatWidget/PendingToolApproval/Dark
| Before | After |
|---|---|
 |
 |
chat/widget/chatWidget/PendingToolApproval/Light
| Before | After |
|---|---|
 |
 |
Instead of running the reducer and post-comparing output, inspect the RootConfigChanged action's patch against the current config values before calling rootReducer. This avoids allocating a new state object entirely for no-op dispatches, rather than allocating and discarding it. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Merge-mode pre-check computes the merged result and deep-compares against current values, so a patch adding a new key (even with value undefined) is not mistakenly treated as a no-op - Test now asserts serverSeq does not advance on deduped (no-op) actions (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
osortega
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Skip envelope emission in
AgentHostStateManager._applyAndEmitwhen a root action's reducer output deeply equals the previous root state.Why
rootReduceralways allocates a new state object, even for no-opRootConfigChangedactions (it just spreads the existing values). Combined withRootStateSubscription.onDidChangefiring on every applied envelope, that meant any contribution that re-published an unchanged root config value would trigger an event, react to it, and re-publish — an infinite loop. The recent #314391 fix worked around one instance of this inagentHostTerminalContribution.ts, but the underlying server-side behavior was still wrong.What
_applyAndEmit, when the action is a root action, run the reducer, deep-compare with the previous root state viaobjects.equals, and bail out (no envelope, noserverSeqbump) when unchanged.Notes for reviewers
src/vs/platform/agentHost/common/state/protocol/reducers.tsis auto-generated from the protocol repo byscripts/sync-agent-host-protocol.ts, so the dedupe lives at the call site rather than inside the reducer.agentHostTerminalContribution.tsfrom Avoid infinite loop in syncing agent host config #314391 is left in place as defense in depth (it still avoids an unnecessary RPC).(Written by Copilot)