Skip to content

sessions: don't flash input needed for auto-approved tools#325679

Open
benibenj wants to merge 4 commits into
mainfrom
benibenj/agents/when-using-the-browser-tools-in-the-agent
Open

sessions: don't flash input needed for auto-approved tools#325679
benibenj wants to merge 4 commits into
mainfrom
benibenj/agents/when-using-the-browser-tools-in-the-agent

Conversation

@benibenj

Copy link
Copy Markdown
Contributor

Fix

When a session runs with bypass approvals (autoApprove) in the Agents/sessions window, client-contributed tools (e.g. the browser tools) briefly flashed the session as input needed in the sessions list, then cleared on their own without any user action.

Root cause

Auto-approved client tools deliberately transition through PendingConfirmation — the host tags the tool call _meta.autoApproveBySetting = true, and the owning client auto-approves and runs it. During that auto-approval round-trip + client-side execution, the session was reported as InputNeeded through two host-owned paths:

  1. agentSideEffects._syncToolInputNeeded pushed a ToolConfirmation entry (during PendingConfirmation) and a ToolClientExecution entry (during Running) into the session inputNeeded list.
  2. The chat status projected by chatSummaryFromState reported InputNeeded for a PendingConfirmation tool call; the sessions list aggregates this.

The generated protocol reducers (channels-*/reducer.ts) are synced from the sibling agent-host-protocol repo and must not be hand-edited, so the fix lives in the two host-owned touch points.

Changes

  • agentSideEffects.ts: _syncToolInputNeeded skips both the ToolConfirmation and ToolClientExecution session inputNeeded entries for a tool call flagged autoApproveBySetting.
  • sessionState.ts: chatSummaryFromState demotes an InputNeeded status caused solely by auto-approved confirmations back to InProgress (preserving IsRead/IsArchived flags).

Both are scoped to the parameter confirmation gate (PendingConfirmation) only — a genuine post-execution PendingResultConfirmation still surfaces as input needed even when the parameter gate was auto-approved.

Notes for reviewers

  • Auto-approved calls are intentionally excluded from the blocker/stall telemetry driven by _setSessionInputNeeded: by definition they don't block the session on user input, so the 5-minute "blocked past threshold" telemetry doesn't apply (execution time is still captured on completion via toolInvoked).
  • The raw ChatState.status from the generated reducer still momentarily reads InputNeeded for a full-chat subscriber during the sub-second auto-approval window; the sessions list (which drives the reported badge) is fixed here. The reducer-level change belongs in the agent-host-protocol repo.

Tests

  • agentSideEffects.test.ts: auto-approved tool produces no session inputNeeded entry; a genuine result confirmation still surfaces.
  • chatSummaryStatus.test.ts (new): spurious InputNeeded demoted to InProgress; genuine / mixed / result-confirmation cases preserved.

Fixes #325631

When a session runs with bypass approvals, client-contributed tools (e.g. the browser tools) briefly showed the session as `input needed` in the sessions window before clearing on their own. Auto-approved calls transition through PendingConfirmation and Running while the owning client auto-approves and runs them, and both host-owned status paths surfaced that as InputNeeded.

Skip the session `inputNeeded` queue entries and demote the aggregated chat-summary status for tool calls flagged `autoApproveBySetting`, scoped to the parameter-confirmation gate so a genuine PendingResultConfirmation still surfaces.

Fixes #325631

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 13, 2026 18:49
@benibenj benibenj enabled auto-merge (squash) July 13, 2026 18:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents auto-approved client tools from briefly marking Agent sessions as needing input.

Changes:

  • Suppresses session input requests for auto-approved parameter confirmations and execution.
  • Corrects summarized chat status while preserving genuine confirmations.
  • Adds coverage for auto-approved and result-confirmation flows.
Show a summary per file
File Description
agentSideEffects.ts Filters auto-approved tool input requests.
sessionState.ts Projects auto-approved confirmations as in progress.
agentSideEffects.test.ts Tests session input-request behavior.
chatSummaryStatus.test.ts Tests summary status projection.

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 6
  • Review effort level: Medium

Comment thread src/vs/platform/agentHost/common/state/sessionState.ts Outdated
Comment thread src/vs/platform/agentHost/node/agentSideEffects.ts Outdated
Comment thread src/vs/platform/agentHost/common/state/sessionState.ts Outdated
Comment thread src/vs/platform/agentHost/test/node/agentSideEffects.test.ts Outdated
Comment thread src/vs/platform/agentHost/test/node/agentSideEffects.test.ts Outdated
Comment thread src/vs/platform/agentHost/test/common/state/chatSummaryStatus.test.ts Outdated
Only demote InputNeeded to InProgress when an auto-approved pending confirmation is actually present and no genuine blocker exists, so a restored summary with no loaded active turn keeps its InputNeeded status. Condense verbose comments per review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@benibenj

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed in 2e17211:

  • Over-eager demotion (sessionState.ts): chatSummaryStatus now only demotes InputNeededInProgress when an auto-approved PendingConfirmation is positively present and no genuine blocker exists. A restored summary whose activeTurn isn't loaded (e.g. from createChatState) now keeps its InputNeeded status instead of fabricating InProgress. Added a regression test for that case.
  • Verbose comments (both source files + all three tests): condensed to single-line clarifications.

meganrogge
meganrogge previously approved these changes Jul 13, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…using-the-browser-tools-in-the-agent

# Conflicts:
#	src/vs/platform/agentHost/node/agentSideEffects.ts
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.

Session shows as input needed when using browser tools with bypass approvals

4 participants