diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index a6a1b5b2..60ccbe84 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -127,8 +127,12 @@ Search the code and `/lab` first. The kit and its homes: - Native approvals, questions, child turns, cancel and reconnect render through the shared session components for every engine; never a per-engine copy. Engine pickers and engine-specific UI read `GET /api/config` engines and - capability flags, never `engine === "..."`. Gateway approval cards: - `components/chat/gateway-approval-card.tsx` over `components/ai/approval-card.tsx`. + capability flags, never `engine === "..."`. Every human approval (gateway and + provider-native) renders through one card, `components/ai/approval-card.tsx`, + which owns the presentation, focus and Escape / Cmd-Enter shortcuts; the lane + wrappers `components/chat/gateway-approval-card.tsx` and + `components/chat/native-approval-card.tsx` own their decision semantics, never + a second layout. - A thread has one stream: the thread-scoped events endpoint through `use-thread-stream.ts` into the shared store and timeline builder. Never a second reducer, stream controller or poll. diff --git a/frontend/app/lab/component-lab.tsx b/frontend/app/lab/component-lab.tsx index be65177c..c18cb589 100644 --- a/frontend/app/lab/component-lab.tsx +++ b/frontend/app/lab/component-lab.tsx @@ -15,7 +15,6 @@ import { } from "@remixicon/react"; import Link from "next/link"; import * as React from "react"; -import { ApprovalCard } from "@/components/ai/approval-card"; import { ContextCardStack } from "@/components/ai/context-card"; import { DiffTable } from "@/components/ai/diff-table"; import { FilterTable } from "@/components/ai/filter-table"; @@ -49,6 +48,8 @@ import { } from "@/components/base/table/table"; import { Tab, TabList, TabPanel, Tabs } from "@/components/base/tabs/tabs"; import { type OrbState, ThinkingOrb } from "@/components/base/thinking-orb"; +import { GatewayApprovalCard } from "@/components/chat/gateway-approval-card"; +import { NativeApprovalCard } from "@/components/chat/native-approval-card"; import { AsteriskMark } from "@/components/foundations/brand/asterisk-mark"; import * as Modal from "@/components/base/modal/modal"; import { ARTIFACT_CAPABILITY_ROWS } from "./artifact-capability-matrix"; @@ -465,20 +466,44 @@ export function ComponentLab() { -
- {}} - onDeny={() => {}} - /> +
+
+ {}} + /> + + +
diff --git a/frontend/components/ai/approval-card.interaction.test.tsx b/frontend/components/ai/approval-card.interaction.test.tsx new file mode 100644 index 00000000..62ba1531 --- /dev/null +++ b/frontend/components/ai/approval-card.interaction.test.tsx @@ -0,0 +1,173 @@ +// The interaction half of the approval-card tests: register happy-dom's +// globals BEFORE react-dom/client loads, mount the real lane wrappers, press +// keys and click buttons, and read focus and the resulting layout off the live +// DOM. The static-markup tests beside this file lock what renders; these lock +// what the keyboard and the two-step deny actually do. +import { afterEach, describe, expect, test } from "bun:test"; +import { GlobalRegistrator } from "@happy-dom/global-registrator"; + +if (typeof window === "undefined") GlobalRegistrator.register(); + +// Tell React these tests drive it through act(), so state flushes synchronously. +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +// react-aria (the reason Input) touches a couple of browser APIs happy-dom +// does not ship; stub the ones it reaches during mount. +if (typeof HTMLElement !== "undefined" && !("scrollIntoView" in HTMLElement.prototype)) { + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { + value: () => {}, + configurable: true, + writable: true, + }); +} +if (typeof window !== "undefined" && typeof window.matchMedia !== "function") { + window.matchMedia = (media: string) => + ({ + matches: false, + media, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, + }) as unknown as MediaQueryList; +} +if (typeof globalThis.ResizeObserver === "undefined") { + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; +} + +const { act } = await import("react"); +const { createRoot } = await import("react-dom/client"); +const { NativeApprovalCard } = await import("../chat/native-approval-card"); +const { GatewayApprovalCard } = await import("../chat/gateway-approval-card"); + +import type { ReactElement } from "react"; +import type { GatewayApproval } from "@/lib/gateway-approvals"; +import type { PendingApproval } from "../chat/approval-state"; + +let roots: Array<{ root: ReturnType; container: HTMLElement }> = []; + +function mount(ui: ReactElement): HTMLElement { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => root.render(ui)); + roots.push({ root, container }); + return container; +} + +afterEach(() => { + for (const { root, container } of roots) { + act(() => root.unmount()); + container.remove(); + } + roots = []; + (document.activeElement as HTMLElement | null)?.blur?.(); +}); + +function press(el: Element, key: string, mods: { ctrl?: boolean; meta?: boolean } = {}) { + act(() => { + el.dispatchEvent( + new KeyboardEvent("keydown", { + key, + bubbles: true, + cancelable: true, + ctrlKey: !!mods.ctrl, + metaKey: !!mods.meta, + }), + ); + }); +} + +function button(container: HTMLElement, text: string): HTMLButtonElement | undefined { + return [...container.querySelectorAll("button")].find((b) => b.textContent?.trim() === text) as + | HTMLButtonElement + | undefined; +} + +const request: PendingApproval = { + id: "n1", + sessionId: "s", + requestKind: "command", + detail: "git status", +}; + +function approval(overrides: Partial = {}): GatewayApproval { + return { + id: "g1", + runId: "r1", + toolName: "deploy_service", + arguments: { service: "billing" }, + status: "pending", + requestedAt: "2026-01-01T00:00:00Z", + resolvedAt: null, + resolvedBy: null, + ...overrides, + }; +} + +describe("native approval card interaction", () => { + test("takes focus on appear, and the keyboard approves and declines", () => { + const decisions: string[] = []; + const container = mount( + { + decisions.push(d); + }} + />, + ); + const card = container.querySelector('[data-testid="native-approval-card"]'); + if (!card) throw new Error("native card did not render"); + expect(document.activeElement).toBe(card); + + press(card, "Enter", { meta: true }); + expect(decisions).toContain("accept"); + + press(card, "Escape"); + expect(decisions).toContain("decline"); + }); +}); + +describe("gateway approval card interaction", () => { + test("a decided card never takes focus", () => { + mount( + , + ); + expect(document.activeElement).toBe(document.body); + }); + + test("two-step deny: Escape opens the reason box and focuses it; Keep reviewing returns focus", () => { + const container = mount(); + const card = container.querySelector('[data-testid="gateway-approval-card"]') as HTMLElement; + expect(document.activeElement).toBe(card); + // No reason box until the user chooses to deny. + expect(card.querySelector('[aria-label="Reason for denying"]')).toBeNull(); + + // Escape enters the deny flow; focus moves to the reason box. + press(card, "Escape"); + const reason = card.querySelector('[aria-label="Reason for denying"]'); + expect(reason).not.toBeNull(); + expect(document.activeElement).toBe(reason); + expect(button(container, "Keep reviewing")).toBeTruthy(); + + // Backing out removes the reason box and returns focus to the card, so the + // shortcuts keep working. + act(() => button(container, "Keep reviewing")?.click()); + expect(card.querySelector('[aria-label="Reason for denying"]')).toBeNull(); + expect(document.activeElement).toBe(card); + }); +}); diff --git a/frontend/components/ai/approval-card.test.tsx b/frontend/components/ai/approval-card.test.tsx new file mode 100644 index 00000000..ea27f889 --- /dev/null +++ b/frontend/components/ai/approval-card.test.tsx @@ -0,0 +1,86 @@ +import { describe, expect, test } from "bun:test"; +import { renderToStaticMarkup } from "react-dom/server"; +import { type ApprovalAction, ApprovalCard, approvalShortcut } from "./approval-card"; + +// The keyboard contract is pure, so it is locked here without a DOM: every +// shortcut the card binds, and every press that is NOT a shortcut. The +// interaction half (real key presses, focus, the two-step deny) lives in +// approval-card.interaction.test.tsx. +describe("approvalShortcut", () => { + const noMods = { ctrl: false, meta: false }; + + test("Escape rejects", () => { + expect(approvalShortcut("Escape", noMods)).toBe("reject"); + }); + + test("Cmd+Enter and Ctrl+Enter approve", () => { + expect(approvalShortcut("Enter", { ctrl: false, meta: true })).toBe("approve"); + expect(approvalShortcut("Enter", { ctrl: true, meta: false })).toBe("approve"); + }); + + test("a bare Enter is not a shortcut (plain typing stays safe)", () => { + expect(approvalShortcut("Enter", noMods)).toBeNull(); + }); + + test("other keys are not shortcuts", () => { + expect(approvalShortcut("a", noMods)).toBeNull(); + expect(approvalShortcut(" ", { ctrl: true, meta: false })).toBeNull(); + }); +}); + +const ACTIONS: ApprovalAction[] = [ + { id: "deny", label: "Deny", tone: "secondary", onSelect: () => {} }, + { id: "approve", label: "Approve", tone: "primary", onSelect: () => {} }, +]; + +const render = (props: Partial[0]> = {}) => + renderToStaticMarkup( + , + ); + +describe("ApprovalCard", () => { + test("pending renders the title, actions, and a focusable group", () => { + const html = render(); + expect(html).toContain('data-testid="card-under-test"'); + expect(html).toContain('role="group"'); + expect(html).toContain('aria-label="Approval"'); + expect(html).toContain(">Deny<"); + expect(html).toContain(">Approve<"); + }); + + test("busy disables every action (the submitting state)", () => { + const html = render({ busy: true }); + // Both footer buttons carry the native disabled attribute while in flight. + expect(html.match(/disabled/g)?.length ?? 0).toBeGreaterThanOrEqual(2); + }); + + test("error renders the actionable error line (the submission-error state)", () => { + const html = render({ error: "Could not send your decision - try again" }); + expect(html).toContain("Could not send your decision - try again"); + }); + + test("note and chip render when supplied; reason slot renders its content", () => { + const html = render({ + note: "Approved by dana", + chip: { color: "lime", label: "Approved" }, + reason: reason, + }); + expect(html).toContain("Approved by dana"); + expect(html).toContain(">Approved<"); + expect(html).toContain('data-testid="reason-slot"'); + }); + + test("no actions renders no footer buttons (a resolved card)", () => { + const html = render({ actions: [] }); + expect(html).not.toContain(">Deny<"); + expect(html).not.toContain(">Approve<"); + }); +}); diff --git a/frontend/components/ai/approval-card.tsx b/frontend/components/ai/approval-card.tsx index 2b7a73e1..a25a9fc5 100644 --- a/frontend/components/ai/approval-card.tsx +++ b/frontend/components/ai/approval-card.tsx @@ -1,153 +1,180 @@ "use client"; -import { useState } from "react"; -import { RiCloseLine } from "@remixicon/react"; -import { cx } from "@/utils/cx"; +// The approval card (#351): one presentation, focus and keyboard model for +// every human approval decision, so the gateway tool-approval lane +// (components/chat/gateway-approval-card.tsx) and the provider-native lane +// (components/chat/native-approval-card.tsx) look and behave identically +// instead of drawing two different cards for the same choice. The lane +// wrappers own their decision semantics and lifecycle; this card owns the +// shell - header, body, footer button order, the focus-on-appear and the +// Escape (reject) / Cmd-Enter (approve) shortcuts - and renders exactly the +// actions it is handed, in the order given. It replaces the earlier +// radio-select prompt that only one lane ever used. + +import { RiShieldCheckLine } from "@remixicon/react"; +import { type ReactNode, useEffect, useMemo, useRef } from "react"; +import { Chip } from "@/components/base/badges/chip"; import { Button } from "@/components/base/buttons/button"; -export interface ApprovalOption { - label: string; - detail?: string; +export type ApprovalActionTone = "primary" | "secondary" | "ghost"; + +export interface ApprovalAction { + /** Stable id; also the handle Cmd/Ctrl+Enter or Escape can target. */ + readonly id: string; + readonly label: string; + readonly tone: ApprovalActionTone; + readonly onSelect: () => void; } -export interface ApprovalCardProps { - question: string; - options: ApprovalOption[]; - onApprove: (option: ApprovalOption) => void; - onDeny: () => void; - /** Adds a free-text "Type something…" row that overrides the radio choice. */ - allowCustom?: boolean; - className?: string; +export interface ApprovalChip { + readonly color: "yellow" | "lime" | "rose" | "neutral"; + readonly label: string; } /** - * Richer approval prompt: radio-select option rows, an optional custom-answer - * field, and a deny / approve footer. Approve stays disabled until a choice is - * made. Ported from the beautiful-ui ApprovalCard demo onto our tokens + - * the Button. + * The keyboard contract, pure so it is unit-tested without a DOM: Escape + * rejects and Cmd/Ctrl+Enter approves. Returns which role the key press + * invokes, or null when the press is not one of the two shortcuts. */ +export function approvalShortcut( + key: string, + modifiers: { readonly ctrl: boolean; readonly meta: boolean }, +): "approve" | "reject" | null { + if (key === "Escape") return "reject"; + if (key === "Enter" && (modifiers.ctrl || modifiers.meta)) return "approve"; + return null; +} + +export interface ApprovalCardProps { + readonly testId: string; + /** Lead line, e.g. "Tool approval: deploy" or "Approval required to run a command". */ + readonly title: ReactNode; + readonly subtitle?: ReactNode; + /** Status pill (the gateway lane; native approvals carry no status). */ + readonly chip?: ApprovalChip; + /** Structured detail: the gateway argument list or the native detail block. */ + readonly body?: ReactNode; + /** A resolved or in-flight status line (gateway lane). */ + readonly note?: ReactNode; + /** Actionable error, shown only while the card is still actionable. */ + readonly error?: string | null; + /** Reason-capture row, rendered above the footer only while present (the + * gateway deny flow supplies it after the user chooses to deny). */ + readonly reason?: ReactNode; + readonly actions: readonly ApprovalAction[]; + /** The action Cmd/Ctrl+Enter triggers (an approve-role button). */ + readonly approveActionId?: string; + /** The action Escape triggers (a reject-role button). */ + readonly rejectActionId?: string; + /** Hold keyboard focus: true only for the single pending card, so decided + * cards never grab focus (and a reload never scrolls to one). */ + readonly autoFocus?: boolean; + /** Disables the footer and the shortcuts while a decision is in flight. */ + readonly busy?: boolean; + /** Accessible name for the focusable card group. */ + readonly ariaLabel: string; +} + export function ApprovalCard({ - question, - options, - onApprove, - onDeny, - allowCustom = false, - className, + testId, + title, + subtitle, + chip, + body, + note, + error, + reason, + actions, + approveActionId, + rejectActionId, + autoFocus = false, + busy = false, + ariaLabel, }: ApprovalCardProps) { - const [selected, setSelected] = useState(null); - const [custom, setCustom] = useState(""); + const ref = useRef(null); + // Changes whenever the footer swaps (e.g. entering the deny flow), so focus + // can be reclaimed after the previously focused button unmounts. + const actionKey = useMemo(() => actions.map((a) => a.id).join("|"), [actions]); - const customActive = allowCustom && custom.trim().length > 0; - const canApprove = selected !== null || customActive; + // Move focus into the card when it appears and whenever its buttons change - + // but never steal it from wherever the user is already working (a focused + // reason box inside this card, or another card above it). Claiming it only + // when nothing holds it means just the one pending card ever takes focus. + useEffect(() => { + if (!autoFocus) return; + const el = ref.current; + if (!el) return; + const active = document.activeElement; + if (active && active !== document.body) return; + el.focus(); + }, [autoFocus, actionKey]); - function approve() { - if (customActive) { - onApprove({ label: custom.trim() }); - return; - } - if (selected !== null) onApprove(options[selected]); + function handleKeyDown(event: React.KeyboardEvent) { + if (busy) return; + const role = approvalShortcut(event.key, { + ctrl: event.ctrlKey, + meta: event.metaKey, + }); + if (!role) return; + const targetId = role === "approve" ? approveActionId : rejectActionId; + if (!targetId) return; + const action = actions.find((candidate) => candidate.id === targetId); + if (!action) return; + event.preventDefault(); + action.onSelect(); } return ( -
-
-
- {question} - +
+
+ + + +
+

{title}

+ {subtitle &&

{subtitle}

} +
+ {chip && ( + + {chip.label} + + )} +
-
- {options.map((option, index) => { - const active = selected === index && !customActive; - return ( - - ); - })} + {body} - {allowCustom && ( - - )} -
-
+ {note &&

{note}

} + {error &&

{error}

} -
- - -
-
+ {reason} + + {actions.length > 0 && ( +
+ {actions.map((action) => ( + + ))} +
+ )} +
); } diff --git a/frontend/components/chat/gateway-approval-card.test.tsx b/frontend/components/chat/gateway-approval-card.test.tsx index 5e8ab624..a36480b5 100644 --- a/frontend/components/chat/gateway-approval-card.test.tsx +++ b/frontend/components/chat/gateway-approval-card.test.tsx @@ -30,14 +30,14 @@ describe("GatewayApprovalCard", () => { test("pending renders the tool name, argument summary, and Approve/Deny actions", () => { const html = render(approval()); expect(html).toContain('data-testid="gateway-approval-card"'); + expect(html).toContain("Tool approval:"); expect(html).toContain("deploy_service"); - expect(html).toContain("Allow deploy_service?"); // One-level argument summary: key + value (long values truncated). expect(html).toContain("service"); expect(html).toContain("billing"); expect(html).toContain(`${"x".repeat(79)}…`); expect(html).not.toContain("x".repeat(81)); - // The composed vendored approval card provides the actions. + // The shared shell renders the Approve/Deny footer. expect(html).toContain(">Approve<"); expect(html).toContain(">Deny<"); expect(html).toContain(">Pending<"); diff --git a/frontend/components/chat/gateway-approval-card.tsx b/frontend/components/chat/gateway-approval-card.tsx index 8b8990d3..b6bb47c3 100644 --- a/frontend/components/chat/gateway-approval-card.tsx +++ b/frontend/components/chat/gateway-approval-card.tsx @@ -1,15 +1,15 @@ "use client"; -// Gateway approval container (#77): the human side of the approval lane. Wraps -// (never forks) the vendored components/ai/approval-card.tsx with the run -// context it needs - tool name, one-level argument summary, status - and wires -// Approve/Deny to lib/gateway-approvals.ts with an optimistic pending -> -// resolved transition that rolls back on API failure. +// Gateway approval container (#77, unified in #351): the human side of the +// approval lane. Supplies the run context (tool name, one-level argument +// summary, status) to the shared ApprovalCard and wires Approve / Deny to +// lib/gateway-approvals.ts with an optimistic pending -> resolved transition +// that rolls back on API failure. The denial reason is captured only after the +// user chooses to deny, then carried to the agent when the decision continues +// the thread. -import { RiShieldCheckLine } from "@remixicon/react"; -import { useState } from "react"; -import { ApprovalCard } from "@/components/ai/approval-card"; -import { Chip } from "@/components/base/badges/chip"; +import { useEffect, useRef, useState } from "react"; +import { type ApprovalAction, ApprovalCard } from "@/components/ai/approval-card"; import { Input } from "@/components/base/input/input"; import { type GatewayApproval, @@ -57,6 +57,10 @@ function resolvedLine(status: GatewayApprovalStatus, approval: GatewayApproval): return at ? `${verb}${by} · ${at}` : `${verb}${by}`; } +function decisionInFlightLine(decision: GatewayApprovalDecision): string { + return decision === "approve" ? "Approving…" : "Denying…"; +} + export function GatewayApprovalCard({ approval, onResolved, @@ -66,16 +70,25 @@ export function GatewayApprovalCard({ onResolved?: () => void; }) { const [resolution, setResolution] = useState(idleResolution); - // Why a denial: carried to the agent when the decision continues the thread. + // The reason is only entered after the user chooses to deny. + const [denying, setDenying] = useState(false); const [reason, setReason] = useState(""); + const reasonRef = useRef(null); + + // Opening the deny flow unmounts the Deny button that had focus; move focus + // to the reason box so the shortcuts keep working and the user can type. + useEffect(() => { + if (denying) reasonRef.current?.focus(); + }, [denying]); const status = effectiveStatus(approval.status, resolution); const error = resolution.phase === "idle" ? resolution.error : null; const entries = summarizeApprovalArguments(approval.arguments); const badge = STATUS_BADGE[status]; const requestedAt = formatTimestamp(approval.requestedAt); + const submitting = resolution.phase === "submitting"; async function submit(decision: GatewayApprovalDecision) { - if (resolution.phase === "submitting") return; + if (submitting) return; setResolution(beginResolution(resolution, decision)); try { const settledStatus = await resolveGatewayApproval( @@ -86,40 +99,64 @@ export function GatewayApprovalCard({ setResolution(resolutionSucceeded(settledStatus)); onResolved?.(); } catch (cause) { - const httpStatus = - cause instanceof GatewayApprovalRequestError ? cause.httpStatus : null; + const httpStatus = cause instanceof GatewayApprovalRequestError ? cause.httpStatus : null; setResolution(resolutionFailed(httpStatus)); // Lost the race: someone else already resolved it; refetch shows the truth. if (httpStatus === 409) onResolved?.(); } } - return ( -
-
-
-
- - - -
-

- Tool approval:{" "} - {approval.toolName} -

- {requestedAt && ( -

- Requested {requestedAt} -

- )} -
-
- - {badge.label} - -
+ const pending = status === "pending"; + const actions: ApprovalAction[] = !pending + ? [] + : denying + ? [ + { + id: "cancel-deny", + label: "Keep reviewing", + tone: "ghost", + onSelect: () => setDenying(false), + }, + { + id: "confirm-deny", + label: "Deny", + tone: "primary", + onSelect: () => void submit("deny"), + }, + ] + : [ + { + id: "deny", + label: "Deny", + tone: "secondary", + onSelect: () => setDenying(true), + }, + { + id: "approve", + label: "Approve", + tone: "primary", + onSelect: () => void submit("approve"), + }, + ]; - {entries.length > 0 && ( + return ( + + Tool approval: {approval.toolName} + + } + subtitle={requestedAt ? `Requested ${requestedAt}` : undefined} + chip={badge} + autoFocus={pending} + busy={submitting} + approveActionId={denying ? "confirm-deny" : "approve"} + rejectActionId={denying ? "cancel-deny" : "deny"} + actions={actions} + body={ + entries.length > 0 ? (
{entries.map((entry) => (
@@ -128,50 +165,29 @@ export function GatewayApprovalCard({
))}
- )} - - {status !== "pending" && ( -

- {resolution.phase === "submitting" - ? decisionInFlightLine(resolution.decision) - : resolvedLine(status, approval)} -

- )} - {/* Action errors only matter while the card is still actionable - once - the record resolves (e.g. the 409 refetch landed) the resolved line - is the whole truth. */} - {status === "pending" && error && ( -

{error}

- )} -
- - {status === "pending" && ( - <> + ) : undefined + } + note={ + submitting + ? decisionInFlightLine(resolution.decision) + : pending + ? undefined + : resolvedLine(status, approval) + } + error={pending ? error : null} + reason={ + pending && denying ? ( - void submit("approve")} - onDeny={() => void submit("deny")} - /> - - )} -
+ ) : undefined + } + /> ); } - -function decisionInFlightLine(decision: GatewayApprovalDecision): string { - return decision === "approve" ? "Approving…" : "Denying…"; -} diff --git a/frontend/components/chat/native-approval-card.test.tsx b/frontend/components/chat/native-approval-card.test.tsx new file mode 100644 index 00000000..e44a3382 --- /dev/null +++ b/frontend/components/chat/native-approval-card.test.tsx @@ -0,0 +1,93 @@ +import { describe, expect, test } from "bun:test"; +import { renderToStaticMarkup } from "react-dom/server"; +import { selectPendingApproval } from "./approval-state"; +import { NativeApprovalCard } from "./native-approval-card"; +import type { NativeFrame } from "./native-events"; + +function frame(seq: number, eventType: string, payload: unknown): NativeFrame { + return { + schemaVersion: 1, + eventId: `event-${seq}`, + seq, + provider: "t3", + eventType, + native: { + sessionId: "runtime-thread-1", + parentSessionId: null, + messageId: null, + partId: null, + callId: null, + }, + payload, + }; +} + +const requested = frame(1, "approval.requested", { + id: "approval-1", + sessionID: "runtime-thread-1", + requestKind: "command", + detail: "git status", +}); + +const render = (props: { submitting?: boolean; error?: string | null }) => { + const request = selectPendingApproval([requested]); + if (!request) return ""; + return renderToStaticMarkup( + {}} + />, + ); +}; + +describe("NativeApprovalCard", () => { + test("pending renders the request kind, detail, and all four decisions in order", () => { + const html = render({}); + expect(html).toContain('data-testid="native-approval-card"'); + expect(html).toContain("Approval required to run a command"); + expect(html).toContain("git status"); + expect(html).toContain(">Cancel turn<"); + expect(html).toContain(">Decline<"); + expect(html).toContain(">Always allow this session<"); + expect(html).toContain(">Approve once<"); + // Order: cancel, decline, session, approve (primary last). + expect(html.indexOf("Cancel turn")).toBeLessThan(html.indexOf("Decline")); + expect(html.indexOf("Decline")).toBeLessThan(html.indexOf("Always allow this session")); + expect(html.indexOf("Always allow this session")).toBeLessThan(html.indexOf("Approve once")); + }); + + test("submitting disables the actions and shows progress on the primary", () => { + const html = render({ submitting: true }); + expect(html).toContain(">Responding…<"); + expect(html).not.toContain(">Approve once<"); + expect(html.match(/disabled/g)?.length ?? 0).toBeGreaterThanOrEqual(4); + }); + + test("submission error renders the error line", () => { + const html = render({ error: "Could not send your decision - try again" }); + expect(html).toContain("Could not send your decision - try again"); + }); + + test("the card is removed once the approval resolves", () => { + // Removal is driven by the selection: a responded frame clears the pending + // request, so the lane renders nothing. + const resolved = selectPendingApproval([ + requested, + frame(2, "approval.responded", { requestId: "approval-1", decision: "accept" }), + ]); + expect(resolved).toBeNull(); + const html = resolved + ? renderToStaticMarkup( + {}} + />, + ) + : ""; + expect(html).not.toContain('data-testid="native-approval-card"'); + }); +}); diff --git a/frontend/components/chat/native-approval-card.tsx b/frontend/components/chat/native-approval-card.tsx index 13ccd2ce..12f3fd33 100644 --- a/frontend/components/chat/native-approval-card.tsx +++ b/frontend/components/chat/native-approval-card.tsx @@ -1,9 +1,22 @@ "use client"; -import { RiShieldCheckLine } from "@remixicon/react"; -import { Button } from "@/components/base/buttons/button"; +// Provider-native approval container (unified in #351): the human side of the +// engine-native approval lane (OpenCode/Codex/Pi `approval.*` frames). It +// supplies the request context to the shared ApprovalCard and wires the four +// native decisions - Approve once, Always allow this session, Decline, Cancel +// turn - to the session's onRespond. These cards carry no status: the lane +// removes them on resolution (see approval-state.selectPendingApproval). + +import { type ApprovalAction, ApprovalCard } from "@/components/ai/approval-card"; import type { ApprovalDecision, PendingApproval } from "./approval-state"; +const ACTION_PHRASE: Record = { + command: "run a command", + "file-read": "read a file", + "file-change": "change files", + other: "use a tool", +}; + export function NativeApprovalCard({ request, submitting, @@ -15,67 +28,43 @@ export function NativeApprovalCard({ error: string | null; onRespond: (decision: ApprovalDecision) => void | Promise; }) { - const action = - request.requestKind === "command" - ? "run a command" - : request.requestKind === "file-read" - ? "read a file" - : request.requestKind === "file-change" - ? "change files" - : "use a tool"; + const respond = (decision: ApprovalDecision) => () => void onRespond(decision); + + const actions: ApprovalAction[] = [ + { id: "cancel", label: "Cancel turn", tone: "ghost", onSelect: respond("cancel") }, + { id: "decline", label: "Decline", tone: "secondary", onSelect: respond("decline") }, + { + id: "acceptForSession", + label: "Always allow this session", + tone: "secondary", + onSelect: respond("acceptForSession"), + }, + { + id: "accept", + label: submitting ? "Responding…" : "Approve once", + tone: "primary", + onSelect: respond("accept"), + }, + ]; + return ( -
-
- - - -
-

Approval required to {action}

- {request.detail && ( -
-              {request.detail}
-            
- )} -
-
- {error &&

{error}

} -
- - - - -
-
+ + {request.detail} + + ) : undefined + } + /> ); }