Skip to content

Ask about a page visit an approval rule covers instead of refusing it - #855

Merged
davidmckayv merged 3 commits into
mainfrom
david/approval-ask-on-reads
Oct 28, 2026
Merged

davidmckayv merged 3 commits into
mainfrom
david/approval-ask-on-reads

Conversation

@davidmckayv

Copy link
Copy Markdown
Contributor

What this changes

With an approval rule that asks before a Bot opens a page (for example "Ask before taking action" on computer_navigate), the Bot did nothing and told the person the action "requires an approval flow that isn't available in this chat". The person was never shown a card.

Why it happened. The browser sends a Bot's looks at a page (navigate, read, snapshot, scroll) straight to the computer. Only changes (click, type, key, run command, write file) go through /api/approvals/computer, which carries the conversation the approval card is drawn in. The gate still decides every action, so when a rule asked about a look there was no conversation to ask in: approvalAction threw ApprovalRefusedError, and the computer route answered a bare 500 with the refusal text.

The fix.

  • approvalAction throws ApprovalNeedsConversationError (a subclass of ApprovalRefusedError, so existing handling is unchanged) when it has no continuation.
  • The computer routes answer that with 409 { needsConversation: true, notPerformed: true } instead of a 500.
  • failedComputerCall carries the flag, and the approval wrapper in approval-tools.tsx sends the same call again through /api/approvals/computer. There it waits on the inline card like any change. The inline card now wraps every computer_* tool, not only changes.
  • Looks that no rule asks about still go straight to the computer, so the common path gets no extra round trip and no transcript upload.

Tests (red on main, green here):

  • server/tests/approval-needs-conversation.test.ts: the real gateway and routes, a gate that asks through approvalAction, and a real listener standing in for the computer. Asserts the 409 with the flag and that the computer was never called.
  • app/tests/approval-tools.test.tsx:
    • a flagged look goes again through the gate and waits;
    • an unflagged look never touches the gate;
    • needsConversation is not misread as stale refs.

Checked live on a local stack with a rule asking before computer_navigate: the card appears, Allow once opens the page and the Bot answers from it, and Deny tells the Bot it was declined.

Checks:

  • bun test app/tests: 1537 pass.
  • Server approval, computer, gateway, control and human-input tests with TEST_DATABASE_URL: 585 pass.
  • lint, format:check, typecheck and typecheck:tests are clean.

Where it runs

  • New state that outlives a request? None. The retry is one call in the browser; a decision resumes through the existing durable approval request rows.
  • What happens on the second replica? Nothing new. The retried call takes the existing /api/approvals/computer path, which reads the saved conversation from Intelligence exactly as changes do today.
  • Anything serialised? No.
  • Anything fanned out to a browser? No; it uses the existing approvals inbox invalidation.
  • New listener, port, or schedule? No.

Boundary and audit

  • Every acting call still goes through the gateway. The retried call runs through approvalComputerTools, then the gateway: resolve, decide, audit, act.
  • New refusals and failures each write a row. The first attempt keeps its decision and failure rows as before, and the gated retry opens its own approval request.
  • Nothing new is trusted from the client. The server still checks the tool call against the saved conversation before acting.

Changelog

  • A line in CHANGELOG.md under Unreleased.

The browser sends a Bot's looks at a page (navigate, snapshot, scroll) straight
to the computer; only changes travel through the approval route that carries
the conversation. A rule that asks about a look, such as "ask before opening
pages", left the gate nowhere to ask, so the route answered a bare 500 and the
Bot told the person the action needed an approval flow that wasn't available.

The gate now marks that case (ApprovalNeedsConversationError), the computer
routes answer it as 409 needsConversation with nothing done, and the browser
sends the same call again through the approval route, where it waits on the
inline card like any change. Looks nothing asks about still go straight to
the computer.
Anthropic's SDK 2.3.0, which CI now installs for the AG2 harness, sends the
key as Authorization: Bearer instead of x-api-key. The agent's key still
reaches Anthropic; the test only looked for the old header, so it failed on
every pull request. It now accepts either, and passes with Anthropic's SDK
1.17.0 and 2.3.0 alike.

(cherry picked from commit a1391e7)

@guidovizoso guidovizoso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sound fix, well scoped and well tested. Approving, with two small follow-ups:

  1. The inline approval wrapper now covers more tools than it needs to (app/src/lib/copilot/approval-tools.tsx:59). tool.name.startsWith("computer_") also wraps computer_request_secret, computer_request_sign_in and computer_request_help, which never go through a rule. It works today, since with no pending request InlineApproval just draws the tool's own card. But it puts the inbox query and an extra component under the secret and sign-in cards, so a later change to InlineApproval could break them. Suggest naming the looks explicitly (navigate, read, snapshot, screenshot, scroll, list_files, read_file), or leaving out computer_request_*.
  2. Activity may read oddly. The first direct attempt still writes its refusal and failure rows, and then the retry is allowed. Worth checking that Activity doesn't show "refused" right before "Allowed once" for the same page visit.

Nit: the import reorders in server/src/approvals/types.ts and server/src/computer/routes.ts are unrelated formatter churn.

@davidmckayv
davidmckayv merged commit f58ae7e into main Oct 28, 2026
19 checks passed
@davidmckayv
davidmckayv deleted the david/approval-ask-on-reads branch October 28, 2026 02:11
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.

2 participants