diff --git a/web/src/structured.test.ts b/web/src/structured.test.ts index 5f6c7e3..fabda8b 100644 --- a/web/src/structured.test.ts +++ b/web/src/structured.test.ts @@ -933,7 +933,7 @@ describe("structured connection: approvals", () => { conn.dispose(); }); - it("removes the buttons once the approval resolves, even with no accompanying transcript delta", () => { + it("hides a standalone resolved approval entirely when no tool group absorbs it", () => { const ws = new FakeSocket(); const conn = mount({ sockets: [ws] }); ws.open(); @@ -949,7 +949,68 @@ describe("structured connection: approvals", () => { // Resolved on another client: state clears with no transcript delta at all. ws.emit({ type: "reduced_state", state: idleState }); expect(conn.el.querySelector(".st-approval-acts")).toBeNull(); - expect(conn.el.querySelector(".st-approval-resolved")).not.toBeNull(); + // Standalone resolved approval is hidden — no card, no compact line. + expect(conn.el.querySelector(".st-row-approval")).toBeNull(); + conn.dispose(); + }); + + it("absorbs a resolved approval into the tool group as a compact line", () => { + const ws = new FakeSocket(); + const conn = mount({ sockets: [ws] }); + ws.open(); + ws.emit({ + type: "transcript_snapshot", + snapshot: { + cursor: 0, + rows: [ + row("user:1", "user", { content: { text: "go" } }), + toolRow("tool:1", "first"), + row("approval:1", "approval", { toolCall: { title: "Write file.ts" }, options: allApprovalOptions }), + toolRow("tool:2", "second"), + ], + }, + }); + ws.emit({ type: "reduced_state", state: idleState }); + // 2+ tools in the turn → tool group exists; resolved approval is inside it. + const group = conn.el.querySelector(".st-tool-group"); + expect(group).not.toBeNull(); + const compact = group!.querySelector(".st-approval-compact"); + expect(compact).not.toBeNull(); + expect(compact!.textContent).toContain("Write file.ts"); + expect(compact!.textContent).toContain("approved"); + // No full-card approval anywhere on the transcript. + expect(conn.el.querySelector(".st-approval")).toBeNull(); + conn.dispose(); + }); + + it("keeps a pending approval standalone even when a tool group exists in the same turn", () => { + const ws = new FakeSocket(); + const conn = mount({ sockets: [ws] }); + ws.open(); + ws.emit({ + type: "transcript_snapshot", + snapshot: { + cursor: 0, + rows: [ + row("user:1", "user", { content: { text: "go" } }), + toolRow("tool:1", "first"), + row("approval:1", "approval", { toolCall: { title: "Write file.ts" }, options: allApprovalOptions }), + toolRow("tool:2", "second"), + ], + }, + }); + ws.emit({ + type: "reduced_state", + state: { ...idleState, pending_approval: { nonce: "n1", request: { sessionId: "s1", toolCall: {}, options: allApprovalOptions } } }, + }); + // Pending approval stays outside the tool group as a full card. + const approvalCard = conn.el.querySelector(".st-approval"); + expect(approvalCard).not.toBeNull(); + expect(conn.el.querySelectorAll(".st-approval-acts button").length).toBeGreaterThan(0); + // It should not be inside the tool group. + const group = conn.el.querySelector(".st-tool-group"); + expect(group).not.toBeNull(); + expect(group!.querySelector(".st-approval")).toBeNull(); conn.dispose(); }); diff --git a/web/src/structured.ts b/web/src/structured.ts index 82db8f2..0589451 100644 --- a/web/src/structured.ts +++ b/web/src/structured.ts @@ -402,13 +402,15 @@ export function createStructuredConnection( return el; } - /** Group tool rows (and intermediate agent text between them) by turn. - * An agent_text/agent_thought row is "intermediate" when a tool row follows - * it in the same turn — it gets absorbed into the group so the collapsed - * block preserves chronological order. The *last* agent text with no - * subsequent tool stays standalone (the prominent final answer). - * Returns groups keyed by `turn:`, each with `toolCount` (for the - * header label) and `rows` (tools + intermediate text, in order). */ + /** Group tool rows (and intermediate agent text / resolved approvals between + * them) by turn. An agent_text/agent_thought row is "intermediate" when a + * tool row follows it in the same turn — it gets absorbed into the group so + * the collapsed block preserves chronological order. Resolved approval rows + * (not the currently pending one) are absorbed the same way. The *last* + * agent text with no subsequent tool stays standalone (the prominent final + * answer). Returns groups keyed by `turn:`, each with `toolCount` + * (for the header label) and `rows` (tools + intermediate text/approvals, + * in order). */ function activityGroupsByTurn(): Map { // First: count tools per turn to know which turns qualify for grouping. const toolCounts = new Map(); @@ -422,33 +424,35 @@ export function createStructuredConnection( // Second: build groups with interleaved text for turns with 2+ tools. const groups = new Map(); turnId = "before-first-message"; - // Pending text rows that might be intermediate (not yet confirmed by a + // Pending rows that might be intermediate (not yet confirmed by a // following tool). If no tool follows, they stay standalone. - let pendingText: TranscriptRow[] = []; + let pending: TranscriptRow[] = []; let currentGid = `turn:${turnId}`; for (const row of rows) { if (row.kind === "user") { turnId = row.id; currentGid = `turn:${turnId}`; - pendingText = []; + pending = []; continue; } const count = toolCounts.get(currentGid) ?? 0; if (count < 2) continue; - if (row.kind === "agent_text" || row.kind === "agent_thought") { - pendingText.push(row); + const isIntermediateText = row.kind === "agent_text" || row.kind === "agent_thought"; + const isResolvedApproval = row.kind === "approval" && row.id !== pendingApprovalRowId; + if (isIntermediateText || isResolvedApproval) { + pending.push(row); continue; } if (row.kind === "tool_call" || row.kind === "tool_diff") { const group = groups.get(currentGid) ?? { toolCount: count, rows: [] }; - // Flush any pending text — a tool follows, so it's intermediate. - group.rows.push(...pendingText); - pendingText = []; + // Flush any pending rows — a tool follows, so they're intermediate. + group.rows.push(...pending); + pending = []; group.rows.push(row); groups.set(currentGid, group); } } - // pendingText at end of a turn = final text, stays outside (not flushed). + // pending at end of a turn = final text/approvals, stays outside (not flushed). return groups; } @@ -500,6 +504,10 @@ export function createStructuredConnection( children.push(groupEl); continue; } + // Resolved approvals not absorbed into a tool group are hidden — the + // tool row itself already identifies the action, so a standalone + // "answered" card adds nothing. Pending approvals always render. + if (row.kind === "approval" && row.id !== pendingApprovalRowId) continue; const rowEl = buildRowElement(row); if (!rowEl) continue; rowEls.set(row.id, rowEl); diff --git a/web/src/structuredRender.test.ts b/web/src/structuredRender.test.ts index fcc3be0..0b6e485 100644 --- a/web/src/structuredRender.test.ts +++ b/web/src/structuredRender.test.ts @@ -368,13 +368,14 @@ describe("buildTranscriptRow: approval", () => { expect(el.querySelector(".st-approval-path")!.textContent).toBe("/etc/x"); }); - it("hides the buttons and shows an answered note once resolved", () => { + it("renders a compact one-liner once resolved", () => { const el = buildTranscriptRow(approvalRow(allThreeOptions), { pending: false, onDecide: () => Promise.resolve(), }); expect(el.querySelector(".st-approval-acts")).toBeNull(); - expect(el.querySelector(".st-approval-resolved")!.textContent).toBe("answered"); + expect(el.classList.contains("st-approval-compact")).toBe(true); + expect(el.textContent).toContain("approved"); }); it("invokes onDecide with the clicked decision and disables the buttons", () => { diff --git a/web/src/structuredRender.ts b/web/src/structuredRender.ts index 41a87ce..31ef0a4 100644 --- a/web/src/structuredRender.ts +++ b/web/src/structuredRender.ts @@ -704,6 +704,16 @@ function offeredDecisions(options: AcpPermissionOption[]): StructuredApprovalDec function buildApprovalRow(row: TranscriptRow, approval: ApprovalRowState): HTMLElement { const request = (row.data ?? {}) as { toolCall?: AcpToolCallLike; options?: AcpPermissionOption[] }; const toolCall = request.toolCall ?? {}; + + // Resolved approvals render as a compact one-liner inside a tool group + // (or are hidden entirely when standalone — see rebuildTranscript). + if (!approval.pending) { + const wrap = el("div", "st-row st-row-approval st-approval-compact"); + const label = toolCall.title ?? toolKindLabel(toolCall.kind); + wrap.textContent = `${label} — approved`; + return wrap; + } + const wrap = el("div", "st-row st-row-approval"); const card = el("div", "st-approval"); wrap.appendChild(card); @@ -723,45 +733,39 @@ function buildApprovalRow(row: TranscriptRow, approval: ApprovalRowState): HTMLE card.appendChild(pathEl); } - if (approval.pending) { - const acts = el("div", "st-approval-acts"); - const mk = (label: string, decision: StructuredApprovalDecision, cls: string) => { - const btn = document.createElement("button"); - btn.type = "button"; - btn.className = cls ? `act ${cls}` : "act"; - btn.textContent = label; - btn.addEventListener("click", () => { - const buttons = [...acts.querySelectorAll("button")]; - buttons.forEach((b) => (b.disabled = true)); - approval.onDecide(decision).catch(() => { - // Rejects only when this approval is still pending (see - // ApprovalRowState) — restore the controls so the operator can - // retry; the error itself is surfaced by the connection. - buttons.forEach((b) => (b.disabled = false)); - }); + const acts = el("div", "st-approval-acts"); + const mk = (label: string, decision: StructuredApprovalDecision, cls: string) => { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = cls ? `act ${cls}` : "act"; + btn.textContent = label; + btn.addEventListener("click", () => { + const buttons = [...acts.querySelectorAll("button")]; + buttons.forEach((b) => (b.disabled = true)); + approval.onDecide(decision).catch(() => { + // Rejects only when this approval is still pending (see + // ApprovalRowState) — restore the controls so the operator can + // retry; the error itself is surfaced by the connection. + buttons.forEach((b) => (b.disabled = false)); }); - return btn; - }; - const offered = offeredDecisions(request.options ?? []); - const specs: [StructuredApprovalDecision, string, string][] = [ - ["allow_once", "Allow once", "primary"], - ["allow_for_session", "Allow for session", ""], - ["deny", "Deny", "deny"], - ]; - for (const [decision, label, cls] of specs) { - if (offered.includes(decision)) acts.appendChild(mk(label, decision, cls)); - } - if (acts.children.length > 0) { - card.appendChild(acts); - } else { - const none = el("div", "st-approval-resolved"); - none.textContent = "no decision offered"; - card.appendChild(none); - } + }); + return btn; + }; + const offered = offeredDecisions(request.options ?? []); + const specs: [StructuredApprovalDecision, string, string][] = [ + ["allow_once", "Allow once", "primary"], + ["allow_for_session", "Allow for session", ""], + ["deny", "Deny", "deny"], + ]; + for (const [decision, label, cls] of specs) { + if (offered.includes(decision)) acts.appendChild(mk(label, decision, cls)); + } + if (acts.children.length > 0) { + card.appendChild(acts); } else { - const resolved = el("div", "st-approval-resolved"); - resolved.textContent = "answered"; - card.appendChild(resolved); + const none = el("div", "st-approval-resolved"); + none.textContent = "no decision offered"; + card.appendChild(none); } return wrap; diff --git a/web/src/styles.css b/web/src/styles.css index 99471d7..1770ea7 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -3040,6 +3040,16 @@ input.leaf-rename:focus { color: var(--faint); } +/* compact resolved approval inside a tool group — same muted treatment + as intermediate agent text so the audit trail is preserved without + drawing attention to itself. */ +.st-approval-compact { + color: var(--muted); + font-size: var(--fs-12); + padding: var(--sp-1) var(--sp-2); + font-style: italic; +} + .act { appearance: none; font: inherit;