From 3ad999a618e698a23386d7bbe0755872d1ef2d63 Mon Sep 17 00:00:00 2001 From: Hayt <41ea58f1e64c243627e8acde7c89be667052ee6e17d8f021c1195be4324ebf04@buzz.block.builderlab.xyz> Date: Mon, 10 Aug 2026 12:13:01 -0400 Subject: [PATCH] fix(desktop): fail-closed PermissionDecisionButtons for allow_always and unknown kinds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit allow_always rendered as an ordinary green Allow button, giving the observer feed a one-click path to a durable grant — the same UX concern Wes flagged in B2. Unknown kinds also rendered as green Allow (Wes: should fail closed). Tighten PermissionDecisionButtons: - allow_once → actionable green Allow button (unchanged path) - reject_* family → actionable red Deny button (unchanged path) - allow_always → non-actionable amber badge: "Permanent grant — use request card". The thread card (D5 disclosure) is the correct surface; the observer feed is advanced/debug and should not offer one-click durable grants. - Unknown kinds → silently omitted (fail closed); no button rendered. Add LifecycleActivity.render.test.mjs with five cases covering allow_once, reject_once, allow_always (badge present, no button), unknown kind (nothing rendered), and a mixed allow_once + allow_always card. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../LifecycleActivity.render.test.mjs | 202 ++++++++++++++++++ .../LifecycleActivity.tsx | 26 ++- 2 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs diff --git a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs new file mode 100644 index 000000000..5e3a44da8 --- /dev/null +++ b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs @@ -0,0 +1,202 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import React from "react"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { LifecycleActivity } from "./LifecycleActivity.tsx"; + +// --------------------------------------------------------------------------- +// Shared fixtures +// --------------------------------------------------------------------------- + +const BASE_PROPS = { + agentAvatarUrl: null, + agentName: "Test Agent", + agentPubkey: "pubkey123", +}; + +const BASE_IDENTITY = { + turnId: "turn-1", + sessionId: "session-1", + channelId: "channel-1", +}; + +/** + * Build a pending permission lifecycle item with the given options array. + * The card is actionable (awaiting a user decision) and has a request nonce. + */ +function pendingPermissionItem(options) { + return { + id: "perm-1", + type: "lifecycle", + renderClass: "permission", + title: "Tool requires approval", + text: "Run shell command", + timestamp: "2026-08-10T00:00:00.000Z", + requestNonce: "nonce-abc", + actionable: true, + options, + ...BASE_IDENTITY, + }; +} + +// --------------------------------------------------------------------------- +// allow_once — renders a green actionable Allow button +// --------------------------------------------------------------------------- + +test("test_allow_once_renders_actionable_allow_button", () => { + const html = renderToStaticMarkup( + React.createElement(LifecycleActivity, { + ...BASE_PROPS, + item: pendingPermissionItem([ + { optionId: "opt-allow", kind: "allow_once", label: "Allow once" }, + ]), + }), + ); + + // The button must be present and labelled correctly. + assert.ok( + html.includes("permission-decision-opt-allow"), + "allow_once option should render a button with its optionId testid", + ); + assert.ok( + html.includes("Allow once"), + "allow_once option should show its label", + ); + + // The persistent-grant badge must NOT appear for a pure allow_once card. + assert.ok( + !html.includes("permission-decision-persistent-grant"), + "allow_once card should not render the persistent-grant badge", + ); +}); + +// --------------------------------------------------------------------------- +// reject_once — renders a red actionable Deny button +// --------------------------------------------------------------------------- + +test("test_reject_once_renders_actionable_deny_button", () => { + const html = renderToStaticMarkup( + React.createElement(LifecycleActivity, { + ...BASE_PROPS, + item: pendingPermissionItem([ + { optionId: "opt-deny", kind: "reject_once" }, + ]), + }), + ); + + assert.ok( + html.includes("permission-decision-opt-deny"), + "reject_once option should render a button with its optionId testid", + ); + // Deny button uses destructive styling; verify at least the testid is there. + assert.ok( + !html.includes("permission-decision-persistent-grant"), + "reject_once card should not render the persistent-grant badge", + ); +}); + +// --------------------------------------------------------------------------- +// allow_always — non-actionable badge, no clickable button +// --------------------------------------------------------------------------- + +test("test_allow_always_renders_non_actionable_persistent_grant_badge", () => { + const html = renderToStaticMarkup( + React.createElement(LifecycleActivity, { + ...BASE_PROPS, + item: pendingPermissionItem([ + { optionId: "opt-always", kind: "allow_always", label: "Always allow" }, + ]), + }), + ); + + // Must show the non-actionable badge. + assert.ok( + html.includes("permission-decision-persistent-grant"), + "allow_always option should render the persistent-grant badge", + ); + assert.ok( + html.includes("Permanent grant"), + "persistent-grant badge should contain differentiating copy", + ); + + // Must NOT render a clickable button for this optionId. + assert.ok( + !html.includes("permission-decision-opt-always"), + "allow_always option must not render an actionable button", + ); + // No ); })} + {/* allow_always: non-actionable badge — the thread card is the correct + surface for persistent grants (D5 disclosure). The observer feed + shows it as an informational note only. */} + {hasPersistentGrant ? ( + + Permanent grant — use request card + + ) : null} ); }