Skip to content

chat: stop agent host config update loop across windows#320015

Merged
roblourens merged 3 commits into
mainfrom
roblou/agents/config-update-loop-logs-analysis
Jun 5, 2026
Merged

chat: stop agent host config update loop across windows#320015
roblourens merged 3 commits into
mainfrom
roblou/agents/config-update-loop-logs-analysis

Conversation

@roblourens
Copy link
Copy Markdown
Member

@roblourens roblourens commented Jun 4, 2026

Fix #320011

Problem

The local agent host's root config is shared across all VS Code windows. AgentHostTerminalContribution pushes a couple of host-managed values into defaultShell and and listened torootState.onDidChange to re-push them.disableCustomTerminalTool it

The bug: that listener re-pushed on every root-state change, including value changes made by other windows. So two windows with different terminal settings would ping-pong root/configChanged actions forever, each forcing its own value back over the other's.

Captured agent-host protocol logs show ~8000 root/configChanged dispatches in 25s, alternating between two windows:

Window disableCustomTerminalTool defaultShell
79ed98fd false PowerShell 7 (pwsh.exe)
d551eb09 true Windows PowerShell v1.0

Fix

The rootState.onDidChange re-push only ever needed to do one thing: retry the initial push once the host's root-config schema hydrates (the first push from _reconcile() can race an undefined rootState.value). It was never meant to react to value changes.

present**, tracked via a Set<AgentHostConfigKey>. Value-only changes (another window writing a different value) are ignored, which breaks the loop. The existing value-equality guard still prevents redundant self-dispatches.

Refactor

While here, the two managed keys are now a data-driven table (IManagedRootConfigKey[]). The schema gate, value-equality guard, dispatch, and hydration-retry are shared in a single generic _push(entry). Adding a new managed root-config key is now one table no new fields, no copy/pasted push logic.entry

Notes for reviewers

  • One existing test (re-dispatches disableCustomTerminalTool when the enabled setting changes) gained an await flush() because the push pipeline is now uniformly async (computeValue may be async). The dispatch simply lands a microtask behavior is otherwise unchanged.later
  • Added 2 regression tests asserting a value-only root-state change from another window does not trigger a re-dispatch.
  • This stops windows from fighting. The underlying design multiple windows share one singleton root config, so it's last-writer- is unchanged; reconciling divergent per-window settings would be a larger design change.wins reality

Type-check (tsgo), ESLint, and all 15 tests pass.

(Written by Copilot)

The local agent host's root config is shared across all VS Code windows.
AgentHostTerminalContribution re-pushed its own managed values (defaultShell,
disableCustomTerminalTool) on every rootState.onDidChange, including value
changes made by *other* windows. Two windows with different terminal settings
would therefore ping-pong RootConfigChanged actions forever (~8000 dispatches
in 25s in the captured logs).

Fix: only re-push when a managed key's *schema* first appears (host
hydration), not on value-only changes from other windows. Also refactor the
managed keys into a data-driven table so the schema gate, value-equality
guard, dispatch, and hydration-retry are shared - adding a new managed key is
now a single entry with no copy/pasted logic.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 4, 2026 23:00
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 fixes an infinite cross-window update loop where AgentHostTerminalContribution re-pushed agent-host root-config values on every rootState change, causing multiple VS Code windows with differing terminal settings to “fight” via repeated root/configChanged dispatches. The new approach only re-pushes when a managed key’s schema first appears (hydration), and consolidates the managed-key logic into a single data-driven pipeline.

Changes:

  • Refactors managed agent-host root-config pushes into a declarative _managedKeys table and a shared async _push(entry) pipeline.
  • Changes the rootState.onDidChange behavior to re-push only on schema appearance (hydration), ignoring value-only changes to prevent cross-window ping-pong.
  • Updates/extends tests with flush() usage and adds regressions ensuring value-only root-state updates don’t trigger re-dispatch.
Show a summary per file
File Description
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostTerminalContribution.ts Implements schema-seen gating to prevent cross-window config wars; refactors pushes into a shared managed-key pipeline.
src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostTerminalContribution.test.ts Adds regression coverage for value-only root-state changes and adjusts timing with await flush() for async push behavior.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

roblourens and others added 2 commits June 4, 2026 16:13
Flip the agent host root-config key polarity so the custom terminal tool is
disabled by default in a natural way. `enableCustomTerminalTool` defaults to
false, and since `getRootValue` returns undefined when unset and consumers
compare against `true`, "unset -> disabled" now falls out naturally without
touching getRootValue. The workbench-side push also drops its negation and
maps 1:1 to the `chat.agentHost.customTerminalTool.enabled` setting.

Note: this is a host-side schema key (not part of the versioned wire
protocol), so no protocol bump. A stale `disableCustomTerminalTool` value in
a persisted agent-host-config.json is simply ignored; the workbench re-pushes
the new key on connect.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses review feedback: after awaiting computeValue(), re-check
_schemaHasKey() rather than just rootState.config existence, so a host
restart / schema refresh that retracts the key can't defeat the schema
gate for older / 3rd-party hosts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@roblourens roblourens marked this pull request as ready for review June 5, 2026 04:13
@roblourens roblourens enabled auto-merge (squash) June 5, 2026 04:13
@roblourens roblourens merged commit 557cfb0 into main Jun 5, 2026
25 checks passed
@roblourens roblourens deleted the roblou/agents/config-update-loop-logs-analysis branch June 5, 2026 06:27
@vs-code-engineering vs-code-engineering Bot added this to the 1.124.0 milestone Jun 5, 2026
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.

rootConfig update loop

3 participants