agentHost: drive tool execution from the session input queue - #328989
Merged
Conversation
Subagent tool calls could stall indefinitely. A user reported 16 subagents running overnight that "keep stalling and dying for no apparent reason", needing the main agent to repeatedly repair them. Log analysis found 16 permission requests that were never answered, and 80 subagent chat channels unsubscribed ~12ms after a single provider error. The cause is structural rather than a single bug. Answering a tool call was owned by the per-turn chat observer: it rendered the call AND invoked the tool AND dispatched the outcome. So anything that tore down an observer -- a provider error disposing the parent turn's store, a turn ending, a reconnect, or simply never observing a subagent chat -- left the agent blocked on an obligation nobody was left to answer. Invert the relationship. The protocol already maintains SessionState.inputNeeded: a session-level queue of every outstanding blocker, each entry self-sufficient so a client can answer it without subscribing to the owning chat. It is a derived projection recomputed from tool-call status, so it is a set that can be re-read rather than a stream that can be missed. Make that queue the driver: - A session-level watcher owns all four blocker kinds and is the single caller of invokeTool. Chat observers only render. - One shared ChatToolInvocation per call, created by whichever side arrives first, so the card an observer renders in its subagent group is the same object the watcher executes. - Claimed calls run with chat context so confirmations render inline. Unclaimed non-confirmable calls run headlessly. Unclaimed confirmable calls wait for an observer, then deny rather than surface a modal nobody can see. - Chat input requests and MCP authentication get the same treatment; both could previously stall with no surface at all. This removes the class rather than the instances: an obligation is now answered because the session says it is outstanding, not because some particular observer happened to still be alive. Also stop counting toolClientExecution entries as user-blocking. That entry means a client is running the tool, not that a user was asked, so it must not raise InputNeeded -- otherwise every client tool call flags the session as needing input for its whole duration, and an approved call keeps presenting as blocked. Mirrors microsoft/agent-host-protocol#380. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Moves agent-host tool execution ownership from turn observers to the session-level input queue.
Changes:
- Adds queue-driven tool execution, request timeouts, and shared invocations.
- Publishes auto-approved client executions without marking sessions as input-needed.
- Expands regression coverage for execution, confirmation, authentication, and elicitation flows.
Show a summary per file
| File | Description |
|---|---|
agentHostClientTools.test.ts |
Tests queue-driven execution and timeout behavior. |
agentHostSessionHandler.ts |
Implements session-level request handling and tool execution. |
agentSideEffects.test.ts |
Tests auto-approved execution status. |
agentSideEffects.ts |
Publishes all running client-tool requests. |
channels-session/state.ts |
Documents client execution status semantics. |
channels-session/reducer.ts |
Excludes client execution from user-blocking status. |
Review details
Suppressed comments (6)
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts:2003
- This timer is one-shot: if the request is rendered at the five-second check, it returns permanently. If that observer is disposed later while the input remains outstanding—the teardown scenario this change is intended to recover from—nothing re-arms and the elicitation can stall forever. Observe claim changes and start a grace timer whenever the request becomes unclaimed, until the request is removed.
itemStore.add(disposableTimeout(() => {
if (cancelled || this._renderedRequests.get().has(inputKey)) {
return;
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts:2086
- Like the chat-input and authentication paths, this one-shot check stops monitoring after finding a claim at five seconds. If the observer is disposed later without answering, the confirmation remains outstanding indefinitely. Keep watching claim state and start/restart the denial grace period when the last claim disappears.
itemStore.add(disposableTimeout(() => {
if (!this._renderedRequests.get().has(key)) {
this._logService.warn(`[AgentHost] Denying confirmation for ${initial.toolCall.toolName} (callId=${initial.toolCall.toolCallId}): no session claimed it within ${UNOBSERVED_CLIENT_TOOL_GRACE_MS}ms`);
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts:2067
- A claimed authentication request also exhausts its only timer at five seconds. If the rendering observer is torn down after that point without authenticating, the outstanding request is never revisited and the MCP call remains blocked. Re-arm the grace period when the claim is released rather than checking claim state only once.
itemStore.add(disposableTimeout(() => {
if (!this._renderedRequests.get().has(key)) {
this._logService.warn(`[AgentHost] Cancelling MCP authentication for ${initial.toolCall.toolName} (callId=${initial.toolCall.toolCallId}): no session claimed it within ${UNOBSERVED_CLIENT_TOOL_GRACE_MS}ms`);
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts:2083
- For an own-client
ToolConfirmation, this branch only waits or denies._setupClientToolCallno longer invokes the tool, whileagentSideEffects.ts:510does not enqueueToolClientExecutionuntil the protocol status isRunning, which itself requiresChatToolCallConfirmed. A rendered pending client call therefore remains inStreamingwith no path to show or answer its local confirmation. This request must drive the shared client invocation (or otherwise transition it into its confirmation UI).
// A confirmation that no sub/agent observer claims within the
// grace window is auto-denied so the agent is not left blocked
// on a surface that never renders.
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts:2119
- The retain count cannot bridge the documented confirmation→execution succession. The host removes the confirmation request before adding the execution request (
agentSideEffects.ts:505-518), andautorunPerKeyedItemdisposes removed stores before running setup for additions, so the count reaches zero here and deletes the shared invocation. The execution watcher then creates a second invocation while the observer still renders the first. Key the retained lifecycle by tool call across request-kind transitions, or defer cleanup until the final queue state is known.
this._clientToolRetainCounts.delete(key);
this._forgetResolvedToolCall(key);
this._clientToolInvocations.delete(key);
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts:3126
- This set is not safe when the same turn is rendered by multiple observers—for example, a parent observes a subagent while that subagent chat is also open directly. Both claims add the same key, but disposing either observer deletes it, so the session watcher can deny/cancel a request even though the other observer is still rendering it. Track a reference count per key and remove the rendered state only when the final claim is disposed.
this._renderedRequests.set(new Set(this._renderedRequests.get()).add(key), undefined);
return toDisposable(() => {
const next = new Set(this._renderedRequests.get());
next.delete(key);
this._renderedRequests.set(next, undefined);
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Balanced
Sibling resources (default, peer and subagent chats) can be open against the same backend session at once, and each installed its own session-level watcher over the same inputNeeded queue. Each had independent per-request state, so one client-tool request executed the tool once per open resource; _resolveToolCall only deduplicates the eventual dispatch, long after the tool's side effects have already run N times. Ref-count a single watcher per backend session instead, keeping it alive while any sibling holds a reference. The resource-to-backend mapping is recorded at install time rather than resolved during teardown, when provisional session state may already be gone. The claim registry now records which observer is rendering a request, so a claimed tool executes with that observer's chat context instead of whichever sibling happened to install the watcher. Also reattach the withInputNeededStatus documentation, which described the old "any non-empty queue" rule and had come loose from its function. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TylerLeonhardt
approved these changes
Aug 4, 2026
DonJayamanne
added a commit
that referenced
this pull request
Aug 4, 2026
* origin/main: (31 commits) Improve workspace picker preselection (#328995) agentHost: support Codex custom agents and runtime enablement (#328956) Finalizes customEditorPriority proposal. Closes #292379 (#329002) Add Agents window startup A/A experiment trigger (#328454) sessions: show created session pill in response summary (#328984) Fix onboarding microphone picker visibility (#329011) Explains how to develop the markdown editor (#329009) Conditional agent-window auth for signed-out users (#328990) Fix BYOK enterprise policy handling in agent host Agent Host changes for fix/agent-host-byok-enterprise-policy agentHost: drive tool execution from the session input queue (#328989) Make Integrated Browser smoke tests deterministic across build qualities (#328983) Accept box sizing screenshot changes Avoid large Component Fixtures step outputs Remove component fixture box sizing reset fix: guard stale line numbers in test decorations (fixes #328988) sessions: fix maximized side pane toggle (#328974) Add component fixture rendering controls Reduce floating panel margins for layout consistency (#328963) agentHost: support file completions across workspace roots (#328944) ... # Conflicts: # src/vs/sessions/SESSIONS.md # src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts
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.
The problem
A user reported 16 subagents running overnight that "keep stalling and dying for no apparent reason. I have to ask again and again the main agent to repair."
Log analysis of the reported session found 16 permission requests that were never answered, and — 12ms after a single provider error — 80 subagent chat channels unsubscribed at once.
That is not one bug. It is structural: answering a tool call was owned by the per-turn chat observer, which rendered the call, invoked the tool, and dispatched the outcome. Any event that tore down an observer left the agent blocked on an obligation nobody was left to answer:
Each of those has been fixed individually before. They keep coming back because the ownership is wrong.
The inversion
The protocol already maintains SessionState.inputNeeded — a session-level queue of every outstanding blocker, where each entry is self-sufficient (carrying the chat URI plus every id needed to respond) precisely so a client can answer it without having subscribed to the owning chat.
Critically, it is a derived projection recomputed from tool-call status on every tool-affecting action, not an event stream. It is a set you can re-read, not a sequence you can miss.
So make it the driver:
ChatToolInvocationper call, created by whichever side arrives first, so the card an observer renders in its subagent group is the same object the watcher executes — one point of truth.ChatInputelicitations andToolAuthenticationget the same treatment. Both could previously stall with no surface at all — MCP auth especially, sincegetMcpAuthenticationRequiredServersdeliberately excludes servers that have a tool-call entry, on the assumption the tool card surfaces them.An obligation is now answered because the session says it is outstanding — not because a particular observer happened to still be alive. That removes the class, not the instances.
Status fix
Stops counting
toolClientExecutionentries as user-blocking. That entry means a client is running the tool, not that a user was asked, so it must not raiseInputNeeded.Two live bugs today: every client tool call (
toolSearch, browser tools) flags the session Input Needed for its entire duration; and approving a call does not clear it, because the confirmation entry is replaced by an execution entry under a new id. Mirrors microsoft/agent-host-protocol#380.This is also what unblocks putting auto-approved client tools into
inputNeeded— they were previously excluded to avoid exactly that status flash, which left them with no session-level record and therefore no recovery path.Notes for review
canRequestPreApprovalis a "might", not a "will" — a tool can set it and still auto-approve at runtime. So an unclaimed such tool waits and may be denied even though it would have run fine. Deliberately conservative: denying beats a modal nobody can answer.toolConfirmation→toolClientExecution) with different ids but the same key, so shared state is refcounted and released only when the last one goes.Validation
AgentHostClientTools 39 · AgentHost 1652 · AgentSession 552 · BlockedSessions 31 — all passing. ESLint and hygiene clean. Typecheck is byte-identical to clean
main(verified by stashing and re-measuring).Test suite gained 5 tests covering: single execution of a claimed call, the shared invocation being the same object rendered in a subagent chat, headless execution of an unclaimed non-confirmable tool, denial of an unclaimed confirmable tool, and the new
ChatInput/ToolAuthenticationtimeout paths.One pre-existing failure on
mainis unrelated and untouched:ResponseSelectionSideChatController › follows the selection as the transcript scrolls.