From 0901ec04e6f92185be546f0ca2861a376dfb6b32 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Mon, 28 Sep 2026 16:46:53 +0000 Subject: [PATCH 01/12] Add conversation-scoped destructive edit approval and reset Destructive canonical Petrinaut calls from Brunch, every remove tool and deleteItemsByIds, wait for approval inside their in-band browser call turn, so later calls stay queued behind them. Allow runs the call, Deny settles it as not applied so Brunch accepts the result instead of failing the follow-up, and Stop cancels a pending approval. An inline approval lists the requested removals while the call waits. Clear AI chat in ordinary Brunch starts a fresh, persisted conversation and resets conversation-only approvals without replacing the model or deleting old history. Always allow lasts only for the current mounted conversation until leaving or reloading. Keep experiment lifecycle work in the next layer and leave stock approvals unchanged. Refs FE-1793 --- .../brunch-conversation-id.ts | 24 ++ .../brunch-mutation-approval.test.tsx | 326 +++++++++++++++++ .../brunch-mutation-approval.tsx | 328 ++++++++++++++++++ .../in-band-browser-call.ts | 17 + ...anonical-state-change.integration.test.tsx | 8 + .../local-storage-demo-app.test.tsx | 69 +++- .../local-storage-demo-app.tsx | 75 +++- .../@hashintel/petrinaut/docs/ai-assistant.md | 4 +- 8 files changed, 841 insertions(+), 10 deletions(-) create mode 100644 apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx create mode 100644 apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-conversation-id.ts b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-conversation-id.ts index f1832ce1f6e..4219bafccd7 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-conversation-id.ts +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-conversation-id.ts @@ -17,6 +17,30 @@ interface ConversationStorage { setItem(key: string, value: string): void; } +/** Change only the pointer; old conversation history and the model are retained. */ +export const replaceBrunchConversationId = ( + netId: string, + conversationId: string, + storage: ConversationStorage = window.localStorage, +): void => { + let stored: Record = {}; + try { + stored = JSON.parse( + storage.getItem(conversationStorageKey) ?? "{}", + ) as Record; + } catch { + // A broken pointer cache must not prevent starting a new conversation. + } + try { + storage.setItem( + conversationStorageKey, + JSON.stringify({ ...stored, [netId]: conversationId }), + ); + } catch { + // The host retains the new id for this page load. + } +}; + export const getOrCreateBrunchConversationId = ( netId: string, storage: ConversationStorage = window.localStorage, diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx new file mode 100644 index 00000000000..739ffed3838 --- /dev/null +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx @@ -0,0 +1,326 @@ +/** + * @vitest-environment jsdom + */ +import { fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { canonicalContent } from "@hashintel/brunch-agent-plugin-sdcpn"; + +import { + createBrunchMutationAdmission, + createBrunchMutationApprovalCoordinator, + createBrunchMutationApprovalInteractiveTools, + createBrunchMutationApprovalWidget, +} from "./brunch-mutation-approval"; +import { createInBandBrowserCalls } from "./in-band-browser-call"; + +import type { FlueClient } from "@flue/sdk"; + +vi.hoisted(() => { + window.matchMedia = (media) => ({ + media, + matches: false, + onchange: null, + addListener() {}, + removeListener() {}, + addEventListener() {}, + removeEventListener() {}, + dispatchEvent: () => true, + }); +}); + +afterEach(() => vi.unstubAllGlobals()); + +const destructiveInput = { + items: [ + { type: "place", id: "queue" }, + { type: "differentialEquation", id: "decay" }, + ], +}; + +describe("Brunch destructive edit approval", () => { + test("covers every destructive canonical tool and no constructive one", () => { + const toolNames = createBrunchMutationApprovalInteractiveTools( + createBrunchMutationApprovalCoordinator(), + ).map(({ toolName }) => toolName); + expect(toolNames).toContain("deleteItemsByIds"); + expect(toolNames).toContain("removeArc"); + expect(toolNames).not.toContain("addPlace"); + expect(toolNames).not.toContain("updatePlace"); + }); + + test("renders itemized removals and resolves Allow without submitting a tool result", async () => { + const coordinator = createBrunchMutationApprovalCoordinator(); + const decision = coordinator.request({ + toolCallId: "delete-1", + signal: new AbortController().signal, + }); + const ApprovalWidget = createBrunchMutationApprovalWidget( + coordinator, + "deleteItemsByIds", + ); + const submit = vi.fn(); + render( + , + ); + + expect(screen.getByText(/Remove place.*queue/u)).not.toBeNull(); + expect( + screen.getByText(/Remove differential equation.*decay/u), + ).not.toBeNull(); + expect( + screen.getByText(/associated arcs or references may also be removed/iu), + ).not.toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Allow" })); + + await expect(decision).resolves.toEqual({ decision: "allow" }); + expect(submit).not.toHaveBeenCalled(); + }); + + test("Always allow is scoped to one coordinator and abort revokes stale UI authority", async () => { + const coordinator = createBrunchMutationApprovalCoordinator(); + const first = coordinator.request({ + toolCallId: "delete-1", + signal: new AbortController().signal, + }); + coordinator.resolve("delete-1", "always-allow"); + await expect(first).resolves.toEqual({ decision: "allow" }); + await expect( + coordinator.request({ + toolCallId: "delete-2", + signal: new AbortController().signal, + }), + ).resolves.toEqual({ decision: "allow" }); + + const fresh = createBrunchMutationApprovalCoordinator(); + const controller = new AbortController(); + const pending = fresh.request({ + toolCallId: "delete-3", + signal: controller.signal, + }); + controller.abort(); + await expect(pending).resolves.toEqual({ + decision: "deny", + reason: "The destructive edit was stopped before approval.", + }); + expect(fresh.resolve("delete-3", "allow")).toBe(false); + expect(fresh.hasPending("delete-3")).toBe(false); + }); + + test("historical rendering cannot create approval authority", () => { + const coordinator = createBrunchMutationApprovalCoordinator(); + const ApprovalWidget = createBrunchMutationApprovalWidget( + coordinator, + "deleteItemsByIds", + ); + const { container } = render( + , + ); + expect(container.innerHTML).toBe(""); + expect(coordinator.resolve("historical-call", "allow")).toBe(false); + }); +}); + +describe("Brunch destructive edit approval on in-band browser calls", () => { + const binding = { + conversationId: "conversation", + documentId: "document", + incarnationId: "incarnation", + }; + + const issuedCalls = (toolName: string, input: unknown) => { + const posted: unknown[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (_url, init) => { + if (init?.method === "POST") { + posted.push( + typeof init.body === "string" ? JSON.parse(init.body) : init.body, + ); + return new Response(null, { status: 200 }); + } + return Response.json({ + capability: "capability", + binding: canonicalContent(binding), + toolName, + input, + }); + }), + ); + const coordinator = createBrunchMutationApprovalCoordinator(); + const prepareInput = vi.fn(); + const calls = createInBandBrowserCalls({ + client: Promise.resolve({ + url: "http://brunch.local/agents/chat/instance", + } as FlueClient), + principalKey: "principal", + binding, + metadataFor: async () => undefined, + prepareInput, + admit: createBrunchMutationAdmission(coordinator), + }); + return { calls, coordinator, posted, prepareInput }; + }; + + test("a denied removal settles as not applied without running", async () => { + const input = { placeId: "queue" }; + const { calls, coordinator, posted } = issuedCalls("removePlace", input); + const execute = vi.fn(async () => ({ applied: true })); + const run = calls.run( + { + toolCallId: "remove-1", + toolName: "removePlace", + input, + signal: new AbortController().signal, + }, + execute, + ); + await vi.waitFor(() => + expect(coordinator.hasPending("remove-1")).toBe(true), + ); + coordinator.resolve("remove-1", "deny"); + + await expect(run).resolves.toBeUndefined(); + expect(execute).not.toHaveBeenCalled(); + expect(posted).toEqual([ + expect.objectContaining({ + output: { + applied: false, + reason: "The user denied this destructive edit.", + }, + }), + ]); + }); + + test("an allowed removal runs once and reports its own result", async () => { + const input = { placeId: "queue" }; + const { calls, coordinator, posted } = issuedCalls("removePlace", input); + const execute = vi.fn(async () => ({ + applied: true, + title: "Removed place", + })); + const run = calls.run( + { + toolCallId: "remove-1", + toolName: "removePlace", + input, + signal: new AbortController().signal, + }, + execute, + ); + await vi.waitFor(() => + expect(coordinator.hasPending("remove-1")).toBe(true), + ); + coordinator.resolve("remove-1", "allow"); + + await run; + expect(execute).toHaveBeenCalledOnce(); + expect(posted).toEqual([ + expect.objectContaining({ + output: { applied: true, title: "Removed place" }, + }), + ]); + }); + + test("the host records a removal's starting revision only once it is allowed", async () => { + const input = { placeId: "queue" }; + const denied = issuedCalls("removePlace", input); + const deniedRun = denied.calls.run( + { + toolCallId: "remove-1", + toolName: "removePlace", + input, + signal: new AbortController().signal, + }, + vi.fn(async () => ({ applied: true })), + ); + await vi.waitFor(() => + expect(denied.coordinator.hasPending("remove-1")).toBe(true), + ); + expect(denied.prepareInput).not.toHaveBeenCalled(); + denied.coordinator.resolve("remove-1", "deny"); + await deniedRun; + expect(denied.prepareInput).not.toHaveBeenCalled(); + + const allowed = issuedCalls("removePlace", input); + const allowedRun = allowed.calls.run( + { + toolCallId: "remove-2", + toolName: "removePlace", + input, + signal: new AbortController().signal, + }, + vi.fn(async () => ({ applied: true })), + ); + await vi.waitFor(() => + expect(allowed.coordinator.hasPending("remove-2")).toBe(true), + ); + expect(allowed.prepareInput).not.toHaveBeenCalled(); + allowed.coordinator.resolve("remove-2", "allow"); + await allowedRun; + expect(allowed.prepareInput).toHaveBeenCalledOnce(); + }); + + test("a constructive call runs without asking", async () => { + const input = { + id: "place", + name: "Place", + colorId: null, + dynamicsEnabled: false, + differentialEquationId: null, + x: 0, + y: 0, + }; + const { calls, coordinator } = issuedCalls("addPlace", input); + const request = vi.spyOn(coordinator, "request"); + const execute = vi.fn(async () => ({ applied: true })); + + await calls.run( + { + toolCallId: "add-1", + toolName: "addPlace", + input, + signal: new AbortController().signal, + }, + execute, + ); + expect(request).not.toHaveBeenCalled(); + expect(execute).toHaveBeenCalledOnce(); + }); + + test("Stop before approval reports nothing and never runs", async () => { + const input = { placeId: "queue" }; + const { calls, coordinator, posted } = issuedCalls("removePlace", input); + const controller = new AbortController(); + const execute = vi.fn(async () => ({ applied: true })); + const run = calls.run( + { + toolCallId: "remove-1", + toolName: "removePlace", + input, + signal: controller.signal, + }, + execute, + ); + await vi.waitFor(() => + expect(coordinator.hasPending("remove-1")).toBe(true), + ); + controller.abort(); + + await run; + expect(execute).not.toHaveBeenCalled(); + expect(posted).toEqual([]); + }); +}); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx new file mode 100644 index 00000000000..9dddb237a3e --- /dev/null +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -0,0 +1,328 @@ +import { useSyncExternalStore } from "react"; + +import { Button } from "@hashintel/ds-components"; +import { css } from "@hashintel/ds-helpers/css"; +import { + mutationActionInputSchemas, + type PetrinautAiMutationToolName, +} from "@hashintel/petrinaut-core"; +import { + definePetrinautAiInteractiveTool, + type PetrinautAiInteractiveTool, + type PetrinautAiInteractiveToolWidgetProps, +} from "@hashintel/petrinaut/ui"; + +import type { createInBandBrowserCalls } from "./in-band-browser-call"; + +export type BrunchMutationApprovalChoice = "allow" | "always-allow" | "deny"; + +export type BrunchMutationApprovalDecision = + | { readonly decision: "allow" } + | { readonly decision: "deny"; readonly reason: string }; + +type PendingApproval = { + readonly resolve: (decision: BrunchMutationApprovalDecision) => void; + readonly removeAbortListener: () => void; +}; + +export interface BrunchMutationApprovalCoordinator { + request(params: { + readonly toolCallId: string; + readonly signal: AbortSignal; + }): Promise; + resolve(toolCallId: string, choice: BrunchMutationApprovalChoice): boolean; + hasPending(toolCallId: string): boolean; + subscribe: (listener: () => void) => () => void; + dispose(): void; +} + +const stoppedReason = "The destructive edit was stopped before approval."; +const deniedReason = "The user denied this destructive edit."; + +/** One ephemeral approval authority. Create once per mounted conversation. */ +export const createBrunchMutationApprovalCoordinator = + (): BrunchMutationApprovalCoordinator => { + const pending = new Map(); + const listeners = new Set<() => void>(); + let alwaysAllow = false; + let disposed = false; + const notify = () => listeners.forEach((listener) => listener()); + const settle = ( + toolCallId: string, + decision: BrunchMutationApprovalDecision, + ) => { + const approval = pending.get(toolCallId); + if (!approval) return false; + pending.delete(toolCallId); + approval.removeAbortListener(); + approval.resolve(decision); + notify(); + return true; + }; + + return { + request: ({ toolCallId, signal }) => { + if (disposed || signal.aborted) + return Promise.resolve({ decision: "deny", reason: stoppedReason }); + if (alwaysAllow) return Promise.resolve({ decision: "allow" }); + return new Promise((resolve) => { + const onAbort = () => { + settle(toolCallId, { decision: "deny", reason: stoppedReason }); + }; + signal.addEventListener("abort", onAbort, { once: true }); + pending.set(toolCallId, { + resolve, + removeAbortListener: () => + signal.removeEventListener("abort", onAbort), + }); + notify(); + }); + }, + resolve: (toolCallId, choice) => { + if (!pending.has(toolCallId) || disposed) return false; + if (choice === "always-allow") alwaysAllow = true; + return settle( + toolCallId, + choice === "deny" + ? { decision: "deny", reason: deniedReason } + : { decision: "allow" }, + ); + }, + hasPending: (toolCallId) => pending.has(toolCallId), + subscribe: (listener) => { + listeners.add(listener); + return () => listeners.delete(listener); + }, + dispose: () => { + disposed = true; + alwaysAllow = false; + for (const toolCallId of pending.keys()) + settle(toolCallId, { decision: "deny", reason: stoppedReason }); + }, + }; + }; + +const destructiveToolNames = [ + "removePlace", + "removeTransition", + "removeArc", + "removeType", + "removeTypeElement", + "removeDifferentialEquation", + "removeParameter", + "removeScenario", + "removeMetric", + "removeSubnet", + "removeComponentInstance", + "deleteItemsByIds", +] as const satisfies readonly PetrinautAiMutationToolName[]; + +type DestructiveToolName = (typeof destructiveToolNames)[number]; + +export const requiresBrunchMutationApproval = ( + toolName: string, +): toolName is DestructiveToolName => + (destructiveToolNames as readonly string[]).includes(toolName); + +const spacedWords = (camelCase: string) => + camelCase.replace(/[A-Z]/gu, (letter) => ` ${letter.toLowerCase()}`); + +const removalDescriptions = ( + toolName: DestructiveToolName, + input: unknown, +): readonly string[] => { + switch (toolName) { + case "removePlace": + return [ + `Remove place — ${mutationActionInputSchemas.removePlace.parse(input).placeId}`, + ]; + case "removeTransition": + return [ + `Remove transition — ${mutationActionInputSchemas.removeTransition.parse(input).transitionId}`, + ]; + case "removeArc": { + const { arcDirection, endpoint, placeId, transitionId } = + mutationActionInputSchemas.removeArc.parse(input); + const target = + placeId ?? + (endpoint?.kind === "componentPort" + ? `${endpoint.componentInstanceId} / ${endpoint.portPlaceId}` + : endpoint?.placeId); + return [`Remove ${arcDirection} arc — ${transitionId} ↔ ${target}`]; + } + case "removeType": + return [ + `Remove type — ${mutationActionInputSchemas.removeType.parse(input).typeId}`, + ]; + case "removeTypeElement": { + const { elementId, typeId } = + mutationActionInputSchemas.removeTypeElement.parse(input); + return [`Remove type element — ${typeId} / ${elementId}`]; + } + case "removeDifferentialEquation": + return [ + `Remove differential equation — ${mutationActionInputSchemas.removeDifferentialEquation.parse(input).equationId}`, + ]; + case "removeParameter": + return [ + `Remove parameter — ${mutationActionInputSchemas.removeParameter.parse(input).parameterId}`, + ]; + case "removeScenario": + return [ + `Remove scenario — ${mutationActionInputSchemas.removeScenario.parse(input).scenarioId}`, + ]; + case "removeMetric": + return [ + `Remove metric — ${mutationActionInputSchemas.removeMetric.parse(input).metricId}`, + ]; + case "removeSubnet": + return [ + `Remove subnet — ${mutationActionInputSchemas.removeSubnet.parse(input).subnetId}`, + ]; + case "removeComponentInstance": + return [ + `Remove component instance — ${mutationActionInputSchemas.removeComponentInstance.parse(input).instanceId}`, + ]; + case "deleteItemsByIds": + return mutationActionInputSchemas.deleteItemsByIds + .parse(input) + .items.map(({ id, type }) => `Remove ${spacedWords(type)} — ${id}`); + } +}; + +type InBandAdmission = NonNullable< + Parameters[0]["admit"] +>; + +/** + * Destructive canonical calls wait for approval within their own turn, so + * later calls stay queued behind them, and before the host records the + * revision they start from. A denial settles the call as not applied, which + * Brunch accepts like any unchanged-document result. + */ +export const createBrunchMutationAdmission = + (approval: BrunchMutationApprovalCoordinator): InBandAdmission => + async ({ toolCallId, toolName, signal }) => { + if (!requiresBrunchMutationApproval(toolName)) return { admitted: true }; + const decision = await approval.request({ toolCallId, signal }); + return decision.decision === "allow" + ? { admitted: true } + : { + admitted: false, + output: { applied: false, reason: decision.reason }, + }; + }; + +const containerStyle = css({ + display: "flex", + flexDirection: "column", + gap: "2", + padding: "3", + borderWidth: "thin", + borderStyle: "solid", + borderColor: "neutral.a20", + borderRadius: "lg", + backgroundColor: "neutral.s00", +}); +const actionsStyle = css({ display: "flex", gap: "2", flexWrap: "wrap" }); + +type WidgetProps = PetrinautAiInteractiveToolWidgetProps; + +const settledText = (output: unknown): string => { + if (typeof output !== "object" || output === null) + return "Model edits completed."; + if ("reason" in output && typeof output.reason === "string") + return output.reason; + if ("title" in output && typeof output.title === "string") + return output.title; + return "Model edits completed."; +}; + +export const createBrunchMutationApprovalWidget = ( + coordinator: BrunchMutationApprovalCoordinator, + toolName: DestructiveToolName, +) => { + const Widget = ({ + input, + toolCallId, + state, + submittedOutput, + }: WidgetProps) => { + const pending = useSyncExternalStore( + coordinator.subscribe, + () => coordinator.hasPending(toolCallId), + () => false, + ); + if (state === "submitted") return

{settledText(submittedOutput)}

; + if (!pending) return null; + return ( +
+ Allow these destructive edits? +
    + {removalDescriptions(toolName, input).map((description) => ( +
  • {description}
  • + ))} +
+

+ Associated arcs or references may also be removed. Always allow + applies to destructive edits in this conversation only, until you + leave or reload. +

+
+ + + +
+
+ ); + }; + return Widget; +}; + +const passthrough = { parse: (value: unknown) => value }; + +/** Inline approval for each destructive canonical tool, shown only while its call waits. */ +export const createBrunchMutationApprovalInteractiveTools = ( + coordinator: BrunchMutationApprovalCoordinator, +): readonly PetrinautAiInteractiveTool[] => + destructiveToolNames.map((toolName) => + definePetrinautAiInteractiveTool({ + toolName, + inputSchema: { + parse: (value) => mutationActionInputSchemas[toolName].parse(value), + }, + outputSchema: passthrough, + component: createBrunchMutationApprovalWidget(coordinator, toolName), + }), + ); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts b/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts index d415004f8f2..f39ae378bce 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts @@ -37,6 +37,18 @@ export const createInBandBrowserCalls = (input: { toolName: string; input: unknown; }) => void; + /** + * Decides, after the claim and before `prepareInput`, whether the call may + * start. A refused call settles with the given output and never starts. + */ + readonly admit?: (call: { + readonly toolCallId: string; + readonly toolName: string; + readonly signal: AbortSignal; + }) => Promise< + | { readonly admitted: true } + | { readonly admitted: false; readonly output: unknown } + >; }) => { const claim = async (call: { readonly toolCallId: string; @@ -183,6 +195,11 @@ export const createInBandBrowserCalls = (input: { let started = false; try { if (call.signal.aborted) return; + const admission = (await input.admit?.(call)) ?? { admitted: true }; + if (!admission.admitted) { + await issued.submit(admission.output); + return; + } input.prepareInput({ toolCallId: call.toolCallId, toolName: call.toolName, diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.canonical-state-change.integration.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.canonical-state-change.integration.test.tsx index a1f568333fb..b05d551b6cd 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.canonical-state-change.integration.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.canonical-state-change.integration.test.tsx @@ -260,6 +260,14 @@ test("real panel scenario and metric add/update/remove calls produce persisted r target: { value: "Create a baseline scenario and throughput metric." }, }); fireEvent.click(screen.getByRole("button", { name: "Send message" })); + // The first removal asks once; Always allow covers the second in this conversation. + fireEvent.click( + await screen.findByRole( + "button", + { name: "Always allow" }, + { timeout: 15_000 }, + ), + ); expect( await screen.findByText( "All six scenario and metric tools returned.", diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx index 507299c5878..9e5f3b07945 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx @@ -55,6 +55,21 @@ import type { PetrinautAiMessage, } from "@hashintel/petrinaut/ui"; +const destructiveApprovalToolNames = [ + "removePlace", + "removeTransition", + "removeArc", + "removeType", + "removeTypeElement", + "removeDifferentialEquation", + "removeParameter", + "removeScenario", + "removeMetric", + "removeSubnet", + "removeComponentInstance", + "deleteItemsByIds", +]; + const defaultTransportOptions = vi.hoisted(() => ({ current: null as unknown, })); @@ -401,9 +416,14 @@ describe("local storage demo Brunch voice integration", () => { expect(aiAssistant.requestStop).toBeTypeOf("function"); expect(aiAssistant.executeMutation).toBeUndefined(); + // Destructive approval and the experiment draft are available; the + // voice-only brunch_ask widget is never mounted here. expect( aiAssistant.interactiveTools?.map(({ toolName }) => toolName), - ).toEqual([brunchTools.draftPetrinautExperiment]); + ).toEqual([ + ...destructiveApprovalToolNames, + brunchTools.draftPetrinautExperiment, + ]); expect(aiAssistant.resolveToolPresentation).toBeTypeOf("function"); expect(aiAssistant.workingLabel).toBe("Brunch is working"); expect( @@ -1157,6 +1177,48 @@ describe("local storage demo Brunch controls", () => { brunchPreviewConfig.isBrunchConfigured = true; }); + test("clearing ordinary Brunch starts a persisted fresh conversation without replacing the model", async () => { + seedStoredNet("clear-incarnation"); + localStorage.setItem(assistantSelectionStorageKey, "brunch"); + flueClientMock.current = { + history: async () => ({ + conversation: { settlements: [], messages: [] }, + offset: "0", + }), + observe: () => ({ + close: vi.fn(), + getSnapshot: () => ({ phase: "absent" }), + refresh: vi.fn(), + subscribe: () => () => {}, + }), + }; + const view = render( + {}} search={{}} />, + ); + await waitFor(() => expect(editorProps.current?.aiAssistant).toBeDefined()); + const first = editorProps.current?.aiAssistant as PetrinautAiAssistant; + const originalId = first.conversationId; + const handle = editorProps.current?.handle; + expect(first.canClearMessages).toBe(true); + act(() => first.onClearMessages?.()); + const next = editorProps.current?.aiAssistant as PetrinautAiAssistant; + expect(next.conversationId).not.toBe(originalId); + expect(next.conversationId).toContain( + "brunch-construction-v1:clear-incarnation:", + ); + expect(next.automaticTools).not.toBe(first.automaticTools); + expect(editorProps.current?.handle).toBe(handle); + const nextId = next.conversationId; + view.unmount(); + render( {}} search={{}} />); + await waitFor(() => + expect( + (editorProps.current?.aiAssistant as PetrinautAiAssistant | undefined) + ?.conversationId, + ).toBe(nextId), + ); + }); + test.each(["metaKey", "ctrlKey"])( "reserves %s + Shift + K for the assistant and keeps plain K for the palette", (modifier) => { @@ -1244,7 +1306,10 @@ describe("local storage demo Brunch controls", () => { ); expect( aiAssistant.interactiveTools?.map(({ toolName }) => toolName), - ).toEqual([brunchTools.draftPetrinautExperiment]); + ).toEqual([ + ...destructiveApprovalToolNames, + brunchTools.draftPetrinautExperiment, + ]); expect(transportOptions.mapClientToolInput).toEqual(expect.any(Function)); // Every configured Brunch browser tool, the draft included, settles in band. expect(aiAssistant.inBandBrowserTools?.has(createExperimentToolName)).toBe( diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index c7be2851338..f04c0831521 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -73,12 +73,19 @@ import { import { brunchPetrinautClientToolNames } from "./brunch-client-tools"; import { brunchEvaluationConversationIdFrom, + getOrCreateBrunchConversationId, ordinaryConstructionConversationIdFrom, + replaceBrunchConversationId, } from "./brunch-conversation-id"; import { createBrunchDraftExperimentInteractiveTool, resolveDraftAuthorityFromHistory, } from "./brunch-draft-experiment-interactive-tool"; +import { + createBrunchMutationAdmission, + createBrunchMutationApprovalCoordinator, + createBrunchMutationApprovalInteractiveTools, +} from "./brunch-mutation-approval"; import { BrunchPanelConversationTracker, type BrunchPanelAdmissionTarget, @@ -610,10 +617,22 @@ export const LocalStorageDemoApp = ({ documentId: currentDocument.documentId, title, }); - const baseConstructionConversationId = - currentDocument === null || !brunchSelected - ? undefined - : ordinaryConstructionConversationIdFrom(currentDocument.incarnationId); + const [freshConversationIds, setFreshConversationIds] = useState< + Record + >({}); + const incarnationId = currentDocument?.incarnationId; + const baseConstructionConversationId = useMemo(() => { + if (!brunchSelected || incarnationId === undefined) return undefined; + const initialId = ordinaryConstructionConversationIdFrom(incarnationId); + return ( + freshConversationIds[incarnationId] ?? + getOrCreateBrunchConversationId( + initialId, + window.localStorage, + () => initialId, + ) + ); + }, [brunchSelected, freshConversationIds, incarnationId]); const fixtureProcessAgentConfiguration = useMemo< FixtureProcessAgentConfiguration | undefined >( @@ -640,6 +659,22 @@ export const LocalStorageDemoApp = ({ [baseProcessAgentBinding], ); const conversationId = processAgentBinding?.conversationId ?? null; + // The panel aborts browser-call signals on stop, unmount and conversation + // replacement. A new binding gets a new, non-persisted approval authority. + const mutationApproval = useMemo( + () => ({ + binding: processAgentBinding, + coordinator: createBrunchMutationApprovalCoordinator(), + }), + [processAgentBinding], + ); + const mutationApprovalTools = useMemo( + () => + createBrunchMutationApprovalInteractiveTools( + mutationApproval.coordinator, + ), + [mutationApproval], + ); const processAgentSession = useProcessAgentSession({ binding: processAgentBinding, brunchSelected, @@ -830,9 +865,15 @@ export const LocalStorageDemoApp = ({ prepareInput: (call) => { canonicalHostTools?.mapClientToolInput(call); }, + admit: createBrunchMutationAdmission(mutationApproval.coordinator), }) : undefined, - [constructionBrowser, flueClientPromise, canonicalHostTools], + [ + constructionBrowser, + flueClientPromise, + canonicalHostTools, + mutationApproval, + ], ); const draftInteractiveTool = useMemo( @@ -889,13 +930,18 @@ export const LocalStorageDemoApp = ({ } : {}), ...(conversationId === null ? {} : { conversationId }), - canClearMessages: flueClientPromise === null, + // Ordinary Brunch clears by starting a fresh conversation, so its saved + // history is kept; Stock clears its local messages. + canClearMessages: true, // These exact-name tools override the static registry only while a // document binding is attached. Every other canonical capability remains // on Petrinaut's registry. inBandBrowserTools, automaticTools: [...(canonicalHostTools?.tools ?? [])], - interactiveTools: draftInteractiveTool ? [draftInteractiveTool] : [], + interactiveTools: [ + ...(inBandBrowserTools ? mutationApprovalTools : []), + ...(draftInteractiveTool ? [draftInteractiveTool] : []), + ], transport: petrinautAiChatTransport, ...(flueClientPromise === null ? {} @@ -926,6 +972,18 @@ export const LocalStorageDemoApp = ({ })); }, onClearMessages: () => { + if (flueClientPromise !== null && incarnationId !== undefined) { + mutationApproval.coordinator.dispose(); + const initialId = + ordinaryConstructionConversationIdFrom(incarnationId); + const nextId = `${initialId}:${crypto.randomUUID()}`; + replaceBrunchConversationId(initialId, nextId); + setFreshConversationIds((current) => ({ + ...current, + [incarnationId]: nextId, + })); + return; + } if (!currentNetId || flueClientPromise !== null) { return; } @@ -949,6 +1007,9 @@ export const LocalStorageDemoApp = ({ canonicalHostTools, inBandBrowserTools, draftInteractiveTool, + incarnationId, + mutationApproval, + mutationApprovalTools, constructionBrowser, conversationTracker, conversationId, diff --git a/libs/@hashintel/petrinaut/docs/ai-assistant.md b/libs/@hashintel/petrinaut/docs/ai-assistant.md index 85d4c95f366..ff5abc1bae0 100644 --- a/libs/@hashintel/petrinaut/docs/ai-assistant.md +++ b/libs/@hashintel/petrinaut/docs/ai-assistant.md @@ -63,6 +63,8 @@ Timing is shown when supplied or observed during this session; unavailable tool durations show a dash. Disclosure icons are neutral; status dots distinguish pending, completed, and failed tools. +Before Brunch removes model elements, an approval lists the requested removals. Associated arcs or references may also be removed. **Allow** applies that removal; **Deny** skips it and tells Brunch nothing was changed. Brunch's later edits wait until you answer. **Always allow** permits later removals only in the current mounted conversation, until you leave or reload. It does not grant permission for another conversation or browser session. Stop cancels a pending approval. Stock auto-layout approval is unchanged. + When the host supplies them, Voice also shows a collapsed brief directly under your message, an immediate spoken-agent reply before the work, and a wrap-up after the produced cards. The brief says **Preparing for Brunch** while its fields are being prepared, **Sending to Brunch** once the fields are ready but not yet accepted, and **Sent to Brunch** after acceptance. Expand a prepared brief to see **Prepared from what you said** and its right-aligned fields. These optional parts are absent in hosts that do not provide them. In Chat, a small neutral voice-bars icon marks user messages sent using Voice; typed messages have no icon. In Voice, those per-message icons are hidden. Hosts that provide live input captions can show your words while you speak. This partial text is display-only: it does not submit work or start preparing a brief. The finalized transcript replaces it in the same bubble before preparation starts. New spoken words and status labels fade in; reduced-motion preferences disable these effects. @@ -287,7 +289,7 @@ Voice ends when the panel closes. If Realtime Voice is interrupted, allow microphone access or check the connection, then select **Reconnect voice mode**. **Clear AI chat** is unavailable while a Voice session is active. -The delete button appears in the top right of the panel once the conversation contains messages. When no interview is active and the host permits clearing, **Clear AI chat** wipes the local conversation, stops any in-flight stream, and tells the host app to forget the messages if it persists them. Hosts with canonical history may disable this control. The Brunch panel disables it because clearing only the browser view would not delete Flue history and the conversation would return on rehydration. +The delete button appears in the top right once the conversation contains messages. When Voice is inactive, **Clear AI chat** in ordinary Brunch starts a fresh conversation and resets conversation-only approvals. It preserves the model and the old saved history; it is not a history-deletion action. Reopening the page returns to the new conversation. Other hosts can clear local messages or disable this control. An interrupted Voice session shows a gentle red waveform without a visible status label. Recovery controls remain available and screen readers still announce the interruption. Open **Voice issues** for a short title and explanation. **Copy details** becomes **Copied** after success; **Dismiss** clears the displayed issues without ending Voice. From b01dbcae00aa6ae3ded0d9bb9f5524e4151afe73 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Tue, 29 Sep 2026 20:29:15 +0200 Subject: [PATCH 02/12] Trim destructive edit approval to the surface its callers use --- .../brunch-mutation-approval.tsx | 24 +++++------------ .../in-band-browser-call.ts | 26 ++++++++++--------- .../local-storage-demo-app.tsx | 5 +--- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx index 9dddb237a3e..cee2f1368de 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -12,11 +12,11 @@ import { type PetrinautAiInteractiveToolWidgetProps, } from "@hashintel/petrinaut/ui"; -import type { createInBandBrowserCalls } from "./in-band-browser-call"; +import type { InBandBrowserCallAdmission } from "./in-band-browser-call"; -export type BrunchMutationApprovalChoice = "allow" | "always-allow" | "deny"; +type BrunchMutationApprovalChoice = "allow" | "always-allow" | "deny"; -export type BrunchMutationApprovalDecision = +type BrunchMutationApprovalDecision = | { readonly decision: "allow" } | { readonly decision: "deny"; readonly reason: string }; @@ -119,9 +119,7 @@ const destructiveToolNames = [ type DestructiveToolName = (typeof destructiveToolNames)[number]; -export const requiresBrunchMutationApproval = ( - toolName: string, -): toolName is DestructiveToolName => +const requiresBrunchMutationApproval = (toolName: string) => (destructiveToolNames as readonly string[]).includes(toolName); const spacedWords = (camelCase: string) => @@ -190,18 +188,9 @@ const removalDescriptions = ( } }; -type InBandAdmission = NonNullable< - Parameters[0]["admit"] ->; - -/** - * Destructive canonical calls wait for approval within their own turn, so - * later calls stay queued behind them, and before the host records the - * revision they start from. A denial settles the call as not applied, which - * Brunch accepts like any unchanged-document result. - */ +/** Later calls stay queued behind a waiting approval; a denial settles as not applied. */ export const createBrunchMutationAdmission = - (approval: BrunchMutationApprovalCoordinator): InBandAdmission => + (approval: BrunchMutationApprovalCoordinator): InBandBrowserCallAdmission => async ({ toolCallId, toolName, signal }) => { if (!requiresBrunchMutationApproval(toolName)) return { admitted: true }; const decision = await approval.request({ toolCallId, signal }); @@ -312,7 +301,6 @@ export const createBrunchMutationApprovalWidget = ( const passthrough = { parse: (value: unknown) => value }; -/** Inline approval for each destructive canonical tool, shown only while its call waits. */ export const createBrunchMutationApprovalInteractiveTools = ( coordinator: BrunchMutationApprovalCoordinator, ): readonly PetrinautAiInteractiveTool[] => diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts b/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts index f39ae378bce..0c074529a4a 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts @@ -23,6 +23,19 @@ const issuedInputSchemas: Readonly< [brunchTools.draftPetrinautExperiment]: draftPetrinautExperimentInputSchema, }; +/** + * Decides, after the claim and before `prepareInput`, whether a call may + * start. A refused call settles with the given output and never starts. + */ +export type InBandBrowserCallAdmission = (call: { + readonly toolCallId: string; + readonly toolName: string; + readonly signal: AbortSignal; +}) => Promise< + | { readonly admitted: true } + | { readonly admitted: false; readonly output: unknown } +>; + /** The callback crosses the same single-owner HTTP process that is running the Flue tool. */ export const createInBandBrowserCalls = (input: { readonly client: Promise; @@ -37,18 +50,7 @@ export const createInBandBrowserCalls = (input: { toolName: string; input: unknown; }) => void; - /** - * Decides, after the claim and before `prepareInput`, whether the call may - * start. A refused call settles with the given output and never starts. - */ - readonly admit?: (call: { - readonly toolCallId: string; - readonly toolName: string; - readonly signal: AbortSignal; - }) => Promise< - | { readonly admitted: true } - | { readonly admitted: false; readonly output: unknown } - >; + readonly admit?: InBandBrowserCallAdmission; }) => { const claim = async (call: { readonly toolCallId: string; diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index f04c0831521..e89efe80ffa 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -659,8 +659,7 @@ export const LocalStorageDemoApp = ({ [baseProcessAgentBinding], ); const conversationId = processAgentBinding?.conversationId ?? null; - // The panel aborts browser-call signals on stop, unmount and conversation - // replacement. A new binding gets a new, non-persisted approval authority. + // Each binding gets its own non-persisted approval authority. const mutationApproval = useMemo( () => ({ binding: processAgentBinding, @@ -930,8 +929,6 @@ export const LocalStorageDemoApp = ({ } : {}), ...(conversationId === null ? {} : { conversationId }), - // Ordinary Brunch clears by starting a fresh conversation, so its saved - // history is kept; Stock clears its local messages. canClearMessages: true, // These exact-name tools override the static registry only while a // document binding is attached. Every other canonical capability remains From a15dae8859678d03d560ceeab220147a0c4c7892 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Tue, 29 Sep 2026 20:43:53 +0200 Subject: [PATCH 03/12] Settle waiting destructive edit approvals when the conversation binding changes --- .../local-storage-demo-app.test.tsx | 77 +++++++++++++++++++ .../local-storage-demo-app.tsx | 10 ++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx index 9e5f3b07945..cbc5f1c80f1 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx @@ -1219,6 +1219,83 @@ describe("local storage demo Brunch controls", () => { ); }); + test("a destructive edit waiting for approval settles when the conversation is replaced", async () => { + seedStoredNet("pending-incarnation"); + localStorage.setItem(assistantSelectionStorageKey, "brunch"); + flueClientMock.current = { + url: "http://brunch.local/agents/chat/instance", + history: async () => ({ + conversation: { settlements: [], messages: [] }, + offset: "0", + }), + observe: () => ({ + close: vi.fn(), + getSnapshot: () => ({ phase: "absent" }), + refresh: vi.fn(), + subscribe: () => () => {}, + }), + }; + const posted: unknown[] = []; + const claimed = vi.fn(); + vi.stubGlobal( + "fetch", + vi.fn(async (url, init) => { + const target = new URL(url instanceof Request ? url.url : url); + if (!target.pathname.includes("/browser-calls/remove-1")) + return new Response(null, { status: 404 }); + if (init?.method === "POST") { + posted.push( + typeof init.body === "string" ? JSON.parse(init.body) : init.body, + ); + return new Response(null, { status: 200 }); + } + claimed(); + return Response.json({ + capability: "capability", + binding: target.searchParams.get("binding"), + toolName: "removePlace", + input: { placeId: "queue" }, + }); + }), + ); + try { + render( {}} search={{}} />); + await waitFor(() => + expect( + (editorProps.current?.aiAssistant as PetrinautAiAssistant | undefined) + ?.inBandBrowserTools, + ).toBeDefined(), + ); + const assistant = editorProps.current + ?.aiAssistant as PetrinautAiAssistant; + const execute = vi.fn(async () => ({ applied: true })); + const run = assistant.inBandBrowserTools?.run( + { + toolCallId: "remove-1", + toolName: "removePlace", + input: { placeId: "queue" }, + signal: new AbortController().signal, + }, + execute, + ); + await waitFor(() => expect(claimed).toHaveBeenCalled()); + act(() => assistant.onClearMessages?.()); + + await run; + expect(execute).not.toHaveBeenCalled(); + expect(posted).toEqual([ + expect.objectContaining({ + output: { + applied: false, + reason: "The destructive edit was stopped before approval.", + }, + }), + ]); + } finally { + vi.unstubAllGlobals(); + } + }); + test.each(["metaKey", "ctrlKey"])( "reserves %s + Shift + K for the assistant and keeps plain K for the palette", (modifier) => { diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index e89efe80ffa..c0c892614bd 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -667,6 +667,14 @@ export const LocalStorageDemoApp = ({ }), [processAgentBinding], ); + // The panel stays mounted when the binding changes, so a replaced authority + // must settle the approvals still waiting on it. + const liveMutationApprovalRef = useRef(mutationApproval.coordinator); + useEffect(() => { + if (liveMutationApprovalRef.current !== mutationApproval.coordinator) + liveMutationApprovalRef.current.dispose(); + liveMutationApprovalRef.current = mutationApproval.coordinator; + }, [mutationApproval]); const mutationApprovalTools = useMemo( () => createBrunchMutationApprovalInteractiveTools( @@ -970,7 +978,6 @@ export const LocalStorageDemoApp = ({ }, onClearMessages: () => { if (flueClientPromise !== null && incarnationId !== undefined) { - mutationApproval.coordinator.dispose(); const initialId = ordinaryConstructionConversationIdFrom(incarnationId); const nextId = `${initialId}:${crypto.randomUUID()}`; @@ -1005,7 +1012,6 @@ export const LocalStorageDemoApp = ({ inBandBrowserTools, draftInteractiveTool, incarnationId, - mutationApproval, mutationApprovalTools, constructionBrowser, conversationTracker, From 92b1fb41ac65bc1e53d3b3b47baf0b91ca1fa195 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Tue, 29 Sep 2026 22:27:30 +0200 Subject: [PATCH 04/12] Show the destructive edit approval only while a call waits for it Every destructive tool was registered as interactive, so its widget replaced the normal tool row everywhere: rows went blank while a call was claimed or after a decision, and completed deletions lost their usual tool row. The approval is now registered only for tools with a call waiting on the coordinator; every other row keeps its normal presentation. --- .../brunch-mutation-approval.test.tsx | 20 +++++++++++ .../brunch-mutation-approval.tsx | 19 ++++++++-- .../local-storage-demo-app.test.tsx | 35 ++++++------------- .../local-storage-demo-app.tsx | 17 ++++++++- 4 files changed, 62 insertions(+), 29 deletions(-) diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx index 739ffed3838..237b9ecde7d 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx @@ -53,6 +53,7 @@ describe("Brunch destructive edit approval", () => { const coordinator = createBrunchMutationApprovalCoordinator(); const decision = coordinator.request({ toolCallId: "delete-1", + toolName: "deleteItemsByIds", signal: new AbortController().signal, }); const ApprovalWidget = createBrunchMutationApprovalWidget( @@ -87,6 +88,7 @@ describe("Brunch destructive edit approval", () => { const coordinator = createBrunchMutationApprovalCoordinator(); const first = coordinator.request({ toolCallId: "delete-1", + toolName: "deleteItemsByIds", signal: new AbortController().signal, }); coordinator.resolve("delete-1", "always-allow"); @@ -94,6 +96,7 @@ describe("Brunch destructive edit approval", () => { await expect( coordinator.request({ toolCallId: "delete-2", + toolName: "deleteItemsByIds", signal: new AbortController().signal, }), ).resolves.toEqual({ decision: "allow" }); @@ -102,6 +105,7 @@ describe("Brunch destructive edit approval", () => { const controller = new AbortController(); const pending = fresh.request({ toolCallId: "delete-3", + toolName: "deleteItemsByIds", signal: controller.signal, }); controller.abort(); @@ -113,6 +117,22 @@ describe("Brunch destructive edit approval", () => { expect(fresh.hasPending("delete-3")).toBe(false); }); + test("reports which tools wait for a decision, keeping the list stable between changes", () => { + const coordinator = createBrunchMutationApprovalCoordinator(); + const idle = coordinator.pendingToolNames(); + expect(idle).toEqual([]); + void coordinator.request({ + toolCallId: "remove-1", + toolName: "removePlace", + signal: new AbortController().signal, + }); + const waiting = coordinator.pendingToolNames(); + expect(waiting).toEqual(["removePlace"]); + expect(coordinator.pendingToolNames()).toBe(waiting); + coordinator.resolve("remove-1", "deny"); + expect(coordinator.pendingToolNames()).toEqual([]); + }); + test("historical rendering cannot create approval authority", () => { const coordinator = createBrunchMutationApprovalCoordinator(); const ApprovalWidget = createBrunchMutationApprovalWidget( diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx index cee2f1368de..ac0cbcecc30 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -21,6 +21,7 @@ type BrunchMutationApprovalDecision = | { readonly decision: "deny"; readonly reason: string }; type PendingApproval = { + readonly toolName: string; readonly resolve: (decision: BrunchMutationApprovalDecision) => void; readonly removeAbortListener: () => void; }; @@ -28,10 +29,13 @@ type PendingApproval = { export interface BrunchMutationApprovalCoordinator { request(params: { readonly toolCallId: string; + readonly toolName: string; readonly signal: AbortSignal; }): Promise; resolve(toolCallId: string, choice: BrunchMutationApprovalChoice): boolean; hasPending(toolCallId: string): boolean; + /** Stable until the set of waiting tool names changes. */ + pendingToolNames: () => readonly string[]; subscribe: (listener: () => void) => () => void; dispose(): void; } @@ -46,7 +50,14 @@ export const createBrunchMutationApprovalCoordinator = const listeners = new Set<() => void>(); let alwaysAllow = false; let disposed = false; - const notify = () => listeners.forEach((listener) => listener()); + let pendingToolNames: readonly string[] = []; + const notify = () => { + const next = [ + ...new Set([...pending.values()].map((approval) => approval.toolName)), + ]; + if (next.join() !== pendingToolNames.join()) pendingToolNames = next; + listeners.forEach((listener) => listener()); + }; const settle = ( toolCallId: string, decision: BrunchMutationApprovalDecision, @@ -61,7 +72,7 @@ export const createBrunchMutationApprovalCoordinator = }; return { - request: ({ toolCallId, signal }) => { + request: ({ toolCallId, toolName, signal }) => { if (disposed || signal.aborted) return Promise.resolve({ decision: "deny", reason: stoppedReason }); if (alwaysAllow) return Promise.resolve({ decision: "allow" }); @@ -71,6 +82,7 @@ export const createBrunchMutationApprovalCoordinator = }; signal.addEventListener("abort", onAbort, { once: true }); pending.set(toolCallId, { + toolName, resolve, removeAbortListener: () => signal.removeEventListener("abort", onAbort), @@ -89,6 +101,7 @@ export const createBrunchMutationApprovalCoordinator = ); }, hasPending: (toolCallId) => pending.has(toolCallId), + pendingToolNames: () => pendingToolNames, subscribe: (listener) => { listeners.add(listener); return () => listeners.delete(listener); @@ -193,7 +206,7 @@ export const createBrunchMutationAdmission = (approval: BrunchMutationApprovalCoordinator): InBandBrowserCallAdmission => async ({ toolCallId, toolName, signal }) => { if (!requiresBrunchMutationApproval(toolName)) return { admitted: true }; - const decision = await approval.request({ toolCallId, signal }); + const decision = await approval.request({ toolCallId, toolName, signal }); return decision.decision === "allow" ? { admitted: true } : { diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx index cbc5f1c80f1..7603ffa30e0 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx @@ -55,21 +55,6 @@ import type { PetrinautAiMessage, } from "@hashintel/petrinaut/ui"; -const destructiveApprovalToolNames = [ - "removePlace", - "removeTransition", - "removeArc", - "removeType", - "removeTypeElement", - "removeDifferentialEquation", - "removeParameter", - "removeScenario", - "removeMetric", - "removeSubnet", - "removeComponentInstance", - "deleteItemsByIds", -]; - const defaultTransportOptions = vi.hoisted(() => ({ current: null as unknown, })); @@ -416,14 +401,9 @@ describe("local storage demo Brunch voice integration", () => { expect(aiAssistant.requestStop).toBeTypeOf("function"); expect(aiAssistant.executeMutation).toBeUndefined(); - // Destructive approval and the experiment draft are available; the - // voice-only brunch_ask widget is never mounted here. expect( aiAssistant.interactiveTools?.map(({ toolName }) => toolName), - ).toEqual([ - ...destructiveApprovalToolNames, - brunchTools.draftPetrinautExperiment, - ]); + ).toEqual([brunchTools.draftPetrinautExperiment]); expect(aiAssistant.resolveToolPresentation).toBeTypeOf("function"); expect(aiAssistant.workingLabel).toBe("Brunch is working"); expect( @@ -1278,10 +1258,18 @@ describe("local storage demo Brunch controls", () => { }, execute, ); + const interactiveToolNames = () => + ( + editorProps.current?.aiAssistant as PetrinautAiAssistant | undefined + )?.interactiveTools?.map(({ toolName }) => toolName); await waitFor(() => expect(claimed).toHaveBeenCalled()); + await waitFor(() => + expect(interactiveToolNames()).toContain("removePlace"), + ); act(() => assistant.onClearMessages?.()); await run; + expect(interactiveToolNames()).not.toContain("removePlace"); expect(execute).not.toHaveBeenCalled(); expect(posted).toEqual([ expect.objectContaining({ @@ -1383,10 +1371,7 @@ describe("local storage demo Brunch controls", () => { ); expect( aiAssistant.interactiveTools?.map(({ toolName }) => toolName), - ).toEqual([ - ...destructiveApprovalToolNames, - brunchTools.draftPetrinautExperiment, - ]); + ).toEqual([brunchTools.draftPetrinautExperiment]); expect(transportOptions.mapClientToolInput).toEqual(expect.any(Function)); // Every configured Brunch browser tool, the draft included, settles in band. expect(aiAssistant.inBandBrowserTools?.has(createExperimentToolName)).toBe( diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index c0c892614bd..64b56b1ce1c 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -16,6 +16,7 @@ import { useMemo, useRef, useState, + useSyncExternalStore, } from "react"; import { @@ -675,13 +676,27 @@ export const LocalStorageDemoApp = ({ liveMutationApprovalRef.current.dispose(); liveMutationApprovalRef.current = mutationApproval.coordinator; }, [mutationApproval]); - const mutationApprovalTools = useMemo( + const allMutationApprovalTools = useMemo( () => createBrunchMutationApprovalInteractiveTools( mutationApproval.coordinator, ), [mutationApproval], ); + // A registered widget replaces the tool's row, so only calls still waiting + // for a decision render as approvals; others keep the normal tool row. + const pendingApprovalToolNames = useSyncExternalStore( + mutationApproval.coordinator.subscribe, + mutationApproval.coordinator.pendingToolNames, + mutationApproval.coordinator.pendingToolNames, + ); + const mutationApprovalTools = useMemo( + () => + allMutationApprovalTools.filter(({ toolName }) => + pendingApprovalToolNames.includes(toolName), + ), + [allMutationApprovalTools, pendingApprovalToolNames], + ); const processAgentSession = useProcessAgentSession({ binding: processAgentBinding, brunchSelected, From 5be5775e407caacebbe0801ae7f0b0e4c60e9d2b Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Tue, 29 Sep 2026 23:43:52 +0200 Subject: [PATCH 05/12] Show the destructive edit approval only on the waiting call's row Approval widgets were matched by tool name, so while one removal waited, earlier rows of the same tool swapped to the widget until the decision settled. Petrinaut interactive tools can now choose which validated inputs they handle, and the approval matches only the input of the call waiting on the coordinator. --- .changeset/conversation-permissions.md | 5 ++++ .../brunch-mutation-approval.test.tsx | 28 ++++++++++++++++++ .../brunch-mutation-approval.tsx | 29 ++++++++++++++++--- .../in-band-browser-call.ts | 1 + .../src/ui/types/ai-interactive-tool.ts | 7 +++++ .../interactive-tools/registry.test.tsx | 18 ++++++++++++ .../interactive-tools/registry.ts | 2 +- 7 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 .changeset/conversation-permissions.md diff --git a/.changeset/conversation-permissions.md b/.changeset/conversation-permissions.md new file mode 100644 index 00000000000..74534441fc8 --- /dev/null +++ b/.changeset/conversation-permissions.md @@ -0,0 +1,5 @@ +--- +"@hashintel/petrinaut": patch +--- + +Allow hosts to show interactive tool controls only for matching inputs, such as destructive changes requiring approval. diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx index 237b9ecde7d..901263f0a00 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx @@ -54,6 +54,7 @@ describe("Brunch destructive edit approval", () => { const decision = coordinator.request({ toolCallId: "delete-1", toolName: "deleteItemsByIds", + input: destructiveInput, signal: new AbortController().signal, }); const ApprovalWidget = createBrunchMutationApprovalWidget( @@ -89,6 +90,7 @@ describe("Brunch destructive edit approval", () => { const first = coordinator.request({ toolCallId: "delete-1", toolName: "deleteItemsByIds", + input: destructiveInput, signal: new AbortController().signal, }); coordinator.resolve("delete-1", "always-allow"); @@ -97,6 +99,7 @@ describe("Brunch destructive edit approval", () => { coordinator.request({ toolCallId: "delete-2", toolName: "deleteItemsByIds", + input: destructiveInput, signal: new AbortController().signal, }), ).resolves.toEqual({ decision: "allow" }); @@ -106,6 +109,7 @@ describe("Brunch destructive edit approval", () => { const pending = fresh.request({ toolCallId: "delete-3", toolName: "deleteItemsByIds", + input: destructiveInput, signal: controller.signal, }); controller.abort(); @@ -124,6 +128,7 @@ describe("Brunch destructive edit approval", () => { void coordinator.request({ toolCallId: "remove-1", toolName: "removePlace", + input: { placeId: "queue" }, signal: new AbortController().signal, }); const waiting = coordinator.pendingToolNames(); @@ -133,6 +138,29 @@ describe("Brunch destructive edit approval", () => { expect(coordinator.pendingToolNames()).toEqual([]); }); + test("matches approval rows only to the waiting call's input", () => { + const coordinator = createBrunchMutationApprovalCoordinator(); + void coordinator.request({ + toolCallId: "remove-2", + toolName: "removePlace", + input: { placeId: "queue" }, + signal: new AbortController().signal, + }); + expect( + coordinator.isPendingInput("removePlace", { placeId: "queue" }), + ).toBe(true); + expect( + coordinator.isPendingInput("removePlace", { placeId: "server" }), + ).toBe(false); + expect( + coordinator.isPendingInput("removeTransition", { placeId: "queue" }), + ).toBe(false); + coordinator.resolve("remove-2", "deny"); + expect( + coordinator.isPendingInput("removePlace", { placeId: "queue" }), + ).toBe(false); + }); + test("historical rendering cannot create approval authority", () => { const coordinator = createBrunchMutationApprovalCoordinator(); const ApprovalWidget = createBrunchMutationApprovalWidget( diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx index ac0cbcecc30..0ef6df24419 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -1,5 +1,6 @@ import { useSyncExternalStore } from "react"; +import { canonicalContent } from "@hashintel/brunch-agent-plugin-sdcpn"; import { Button } from "@hashintel/ds-components"; import { css } from "@hashintel/ds-helpers/css"; import { @@ -22,6 +23,7 @@ type BrunchMutationApprovalDecision = type PendingApproval = { readonly toolName: string; + readonly input: ReturnType; readonly resolve: (decision: BrunchMutationApprovalDecision) => void; readonly removeAbortListener: () => void; }; @@ -30,10 +32,12 @@ export interface BrunchMutationApprovalCoordinator { request(params: { readonly toolCallId: string; readonly toolName: string; + readonly input: unknown; readonly signal: AbortSignal; }): Promise; resolve(toolCallId: string, choice: BrunchMutationApprovalChoice): boolean; hasPending(toolCallId: string): boolean; + isPendingInput(toolName: string, input: unknown): boolean; /** Stable until the set of waiting tool names changes. */ pendingToolNames: () => readonly string[]; subscribe: (listener: () => void) => () => void; @@ -72,7 +76,7 @@ export const createBrunchMutationApprovalCoordinator = }; return { - request: ({ toolCallId, toolName, signal }) => { + request: ({ toolCallId, toolName, input, signal }) => { if (disposed || signal.aborted) return Promise.resolve({ decision: "deny", reason: stoppedReason }); if (alwaysAllow) return Promise.resolve({ decision: "allow" }); @@ -83,6 +87,7 @@ export const createBrunchMutationApprovalCoordinator = signal.addEventListener("abort", onAbort, { once: true }); pending.set(toolCallId, { toolName, + input: canonicalContent(input), resolve, removeAbortListener: () => signal.removeEventListener("abort", onAbort), @@ -101,6 +106,13 @@ export const createBrunchMutationApprovalCoordinator = ); }, hasPending: (toolCallId) => pending.has(toolCallId), + isPendingInput: (toolName, input) => { + const content = canonicalContent(input); + return [...pending.values()].some( + (approval) => + approval.toolName === toolName && approval.input === content, + ); + }, pendingToolNames: () => pendingToolNames, subscribe: (listener) => { listeners.add(listener); @@ -132,7 +144,9 @@ const destructiveToolNames = [ type DestructiveToolName = (typeof destructiveToolNames)[number]; -const requiresBrunchMutationApproval = (toolName: string) => +const requiresBrunchMutationApproval = ( + toolName: string, +): toolName is DestructiveToolName => (destructiveToolNames as readonly string[]).includes(toolName); const spacedWords = (camelCase: string) => @@ -204,9 +218,14 @@ const removalDescriptions = ( /** Later calls stay queued behind a waiting approval; a denial settles as not applied. */ export const createBrunchMutationAdmission = (approval: BrunchMutationApprovalCoordinator): InBandBrowserCallAdmission => - async ({ toolCallId, toolName, signal }) => { + async ({ toolCallId, toolName, input, signal }) => { if (!requiresBrunchMutationApproval(toolName)) return { admitted: true }; - const decision = await approval.request({ toolCallId, toolName, signal }); + const decision = await approval.request({ + toolCallId, + toolName, + input: mutationActionInputSchemas[toolName].parse(input), + signal, + }); return decision.decision === "allow" ? { admitted: true } : { @@ -324,6 +343,8 @@ export const createBrunchMutationApprovalInteractiveTools = ( parse: (value) => mutationActionInputSchemas[toolName].parse(value), }, outputSchema: passthrough, + // Earlier rows of the same tool keep their normal presentation. + shouldHandle: (input) => coordinator.isPendingInput(toolName, input), component: createBrunchMutationApprovalWidget(coordinator, toolName), }), ); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts b/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts index 0c074529a4a..7a83ad998ce 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/in-band-browser-call.ts @@ -30,6 +30,7 @@ const issuedInputSchemas: Readonly< export type InBandBrowserCallAdmission = (call: { readonly toolCallId: string; readonly toolName: string; + readonly input: unknown; readonly signal: AbortSignal; }) => Promise< | { readonly admitted: true } diff --git a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts index fcdd4de1be8..5ff9aad3332 100644 --- a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts +++ b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts @@ -48,6 +48,8 @@ export type PetrinautAiInteractiveToolDefinition = { inputSchema: PetrinautAiInteractiveToolSchema; /** Runtime contract for the widget's submitted output. */ outputSchema: PetrinautAiInteractiveToolSchema; + /** Render an interaction only for matching validated inputs. Defaults to all. */ + shouldHandle?: (input: Input) => boolean; /** * Optionally map text submitted through the assistant composer to this * tool's output. Petrinaut validates both the pending input and mapped @@ -65,6 +67,7 @@ type ErasedInteractiveToolDefinition = { placement?: "work" | "card"; parseInput: (value: unknown) => unknown; parseOutput: (value: unknown) => unknown; + shouldHandle?: (input: unknown) => boolean; fromComposerText?: (params: { input: unknown; text: string }) => unknown; component: ComponentType< PetrinautAiInteractiveToolWidgetProps @@ -87,6 +90,7 @@ export const definePetrinautAiInteractiveTool = ( definition: PetrinautAiInteractiveToolDefinition, ): PetrinautAiInteractiveTool => { const fromComposerText = definition.fromComposerText; + const shouldHandle = definition.shouldHandle; return { toolName: definition.toolName, @@ -95,6 +99,9 @@ export const definePetrinautAiInteractiveTool = ( placement: definition.placement, parseInput: (value) => definition.inputSchema.parse(value), parseOutput: (value) => definition.outputSchema.parse(value), + shouldHandle: shouldHandle + ? (value) => shouldHandle(definition.inputSchema.parse(value)) + : undefined, fromComposerText: fromComposerText ? ({ input, text }) => definition.outputSchema.parse( diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx index 4204e6a3a34..2b1056304b3 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx @@ -24,6 +24,24 @@ const hostTool = definePetrinautAiInteractiveTool({ }); describe("interactive tool registry", () => { + test("renders host confirmation only for inputs selected by its validated predicate", () => { + const conditional = definePetrinautAiInteractiveTool({ + toolName: "mutate", + inputSchema: { + parse: (input: unknown) => input as { destructive: boolean }, + }, + outputSchema: { parse: (output: unknown) => output }, + shouldHandle: (input) => input.destructive, + component: () => null, + }); + expect( + getInteractiveTool("mutate", { destructive: false }, [conditional]), + ).toBeUndefined(); + expect( + getInteractiveTool("mutate", { destructive: true }, [conditional]), + ).toBeDefined(); + }); + test("resolves and validates a registered dynamic host tool", () => { const definition = resolveDynamicInteractiveTool( "confirmRelease", diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts index 379f34da8e5..0d7b9d2dde6 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts @@ -60,7 +60,7 @@ export const getInteractiveTool = ( ? { toolName: hostDefinition.toolName, placement: hostDefinition.placement, - shouldHandle: () => true, + shouldHandle: hostDefinition.shouldHandle ?? (() => true), parseInput: hostDefinition.parseInput, parseOutput: hostDefinition.parseOutput, fromComposerText: hostDefinition.fromComposerText, From d28c4ff3f6a43a6c19c5dfb301bc834478911be7 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 13:23:48 +0200 Subject: [PATCH 06/12] Match destructive edit approvals to their call and never throw while picking a row Co-authored-by: Cursor --- .changeset/conversation-permissions.md | 2 +- .../brunch-mutation-approval.test.tsx | 36 +++------ .../brunch-mutation-approval.tsx | 26 ++----- .../src/ui/types/ai-interactive-tool.ts | 19 ++++- .../Editor/panels/ai-assistant-panel.tsx | 14 ++-- .../ai-assistant-contents.tsx | 6 +- .../ai-assistant-contents/tool-list.tsx | 14 ++-- .../apply-auto-layout-widget.test.tsx | 10 ++- .../interactive-tools/registry.test.tsx | 76 +++++++++++++++---- .../interactive-tools/registry.ts | 22 +++--- .../interactive-tools/types.ts | 9 ++- 11 files changed, 137 insertions(+), 97 deletions(-) diff --git a/.changeset/conversation-permissions.md b/.changeset/conversation-permissions.md index 74534441fc8..02e7adb0a67 100644 --- a/.changeset/conversation-permissions.md +++ b/.changeset/conversation-permissions.md @@ -2,4 +2,4 @@ "@hashintel/petrinaut": patch --- -Allow hosts to show interactive tool controls only for matching inputs, such as destructive changes requiring approval. +Allow hosts to show interactive tool controls only for matching tool calls, such as a destructive change waiting for approval. diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx index 901263f0a00..f4ca6b5f06a 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx @@ -54,7 +54,6 @@ describe("Brunch destructive edit approval", () => { const decision = coordinator.request({ toolCallId: "delete-1", toolName: "deleteItemsByIds", - input: destructiveInput, signal: new AbortController().signal, }); const ApprovalWidget = createBrunchMutationApprovalWidget( @@ -90,7 +89,6 @@ describe("Brunch destructive edit approval", () => { const first = coordinator.request({ toolCallId: "delete-1", toolName: "deleteItemsByIds", - input: destructiveInput, signal: new AbortController().signal, }); coordinator.resolve("delete-1", "always-allow"); @@ -99,7 +97,6 @@ describe("Brunch destructive edit approval", () => { coordinator.request({ toolCallId: "delete-2", toolName: "deleteItemsByIds", - input: destructiveInput, signal: new AbortController().signal, }), ).resolves.toEqual({ decision: "allow" }); @@ -109,7 +106,6 @@ describe("Brunch destructive edit approval", () => { const pending = fresh.request({ toolCallId: "delete-3", toolName: "deleteItemsByIds", - input: destructiveInput, signal: controller.signal, }); controller.abort(); @@ -128,7 +124,6 @@ describe("Brunch destructive edit approval", () => { void coordinator.request({ toolCallId: "remove-1", toolName: "removePlace", - input: { placeId: "queue" }, signal: new AbortController().signal, }); const waiting = coordinator.pendingToolNames(); @@ -138,27 +133,18 @@ describe("Brunch destructive edit approval", () => { expect(coordinator.pendingToolNames()).toEqual([]); }); - test("matches approval rows only to the waiting call's input", () => { + test("a malformed call is refused before it can wait for an approval that never renders", async () => { const coordinator = createBrunchMutationApprovalCoordinator(); - void coordinator.request({ - toolCallId: "remove-2", - toolName: "removePlace", - input: { placeId: "queue" }, - signal: new AbortController().signal, - }); - expect( - coordinator.isPendingInput("removePlace", { placeId: "queue" }), - ).toBe(true); - expect( - coordinator.isPendingInput("removePlace", { placeId: "server" }), - ).toBe(false); - expect( - coordinator.isPendingInput("removeTransition", { placeId: "queue" }), - ).toBe(false); - coordinator.resolve("remove-2", "deny"); - expect( - coordinator.isPendingInput("removePlace", { placeId: "queue" }), - ).toBe(false); + const request = vi.spyOn(coordinator, "request"); + await expect( + createBrunchMutationAdmission(coordinator)({ + toolCallId: "remove-1", + toolName: "removePlace", + input: {}, + signal: new AbortController().signal, + }), + ).rejects.toThrow(); + expect(request).not.toHaveBeenCalled(); }); test("historical rendering cannot create approval authority", () => { diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx index 0ef6df24419..d66d818d103 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -1,6 +1,5 @@ import { useSyncExternalStore } from "react"; -import { canonicalContent } from "@hashintel/brunch-agent-plugin-sdcpn"; import { Button } from "@hashintel/ds-components"; import { css } from "@hashintel/ds-helpers/css"; import { @@ -23,7 +22,6 @@ type BrunchMutationApprovalDecision = type PendingApproval = { readonly toolName: string; - readonly input: ReturnType; readonly resolve: (decision: BrunchMutationApprovalDecision) => void; readonly removeAbortListener: () => void; }; @@ -32,12 +30,10 @@ export interface BrunchMutationApprovalCoordinator { request(params: { readonly toolCallId: string; readonly toolName: string; - readonly input: unknown; readonly signal: AbortSignal; }): Promise; resolve(toolCallId: string, choice: BrunchMutationApprovalChoice): boolean; hasPending(toolCallId: string): boolean; - isPendingInput(toolName: string, input: unknown): boolean; /** Stable until the set of waiting tool names changes. */ pendingToolNames: () => readonly string[]; subscribe: (listener: () => void) => () => void; @@ -76,7 +72,7 @@ export const createBrunchMutationApprovalCoordinator = }; return { - request: ({ toolCallId, toolName, input, signal }) => { + request: ({ toolCallId, toolName, signal }) => { if (disposed || signal.aborted) return Promise.resolve({ decision: "deny", reason: stoppedReason }); if (alwaysAllow) return Promise.resolve({ decision: "allow" }); @@ -87,7 +83,6 @@ export const createBrunchMutationApprovalCoordinator = signal.addEventListener("abort", onAbort, { once: true }); pending.set(toolCallId, { toolName, - input: canonicalContent(input), resolve, removeAbortListener: () => signal.removeEventListener("abort", onAbort), @@ -106,13 +101,6 @@ export const createBrunchMutationApprovalCoordinator = ); }, hasPending: (toolCallId) => pending.has(toolCallId), - isPendingInput: (toolName, input) => { - const content = canonicalContent(input); - return [...pending.values()].some( - (approval) => - approval.toolName === toolName && approval.input === content, - ); - }, pendingToolNames: () => pendingToolNames, subscribe: (listener) => { listeners.add(listener); @@ -220,12 +208,9 @@ export const createBrunchMutationAdmission = (approval: BrunchMutationApprovalCoordinator): InBandBrowserCallAdmission => async ({ toolCallId, toolName, input, signal }) => { if (!requiresBrunchMutationApproval(toolName)) return { admitted: true }; - const decision = await approval.request({ - toolCallId, - toolName, - input: mutationActionInputSchemas[toolName].parse(input), - signal, - }); + // An approval row renders only for inputs its schema accepts. + mutationActionInputSchemas[toolName].parse(input); + const decision = await approval.request({ toolCallId, toolName, signal }); return decision.decision === "allow" ? { admitted: true } : { @@ -344,7 +329,8 @@ export const createBrunchMutationApprovalInteractiveTools = ( }, outputSchema: passthrough, // Earlier rows of the same tool keep their normal presentation. - shouldHandle: (input) => coordinator.isPendingInput(toolName, input), + shouldHandle: (_input, { toolCallId }) => + coordinator.hasPending(toolCallId), component: createBrunchMutationApprovalWidget(coordinator, toolName), }), ); diff --git a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts index 5ff9aad3332..d7e0c87a655 100644 --- a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts +++ b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts @@ -48,8 +48,11 @@ export type PetrinautAiInteractiveToolDefinition = { inputSchema: PetrinautAiInteractiveToolSchema; /** Runtime contract for the widget's submitted output. */ outputSchema: PetrinautAiInteractiveToolSchema; - /** Render an interaction only for matching validated inputs. Defaults to all. */ - shouldHandle?: (input: Input) => boolean; + /** + * Render an interaction only for matching calls. Defaults to all. Inputs + * the schema rejects are never handled. + */ + shouldHandle?: (input: Input, call: { toolCallId: string }) => boolean; /** * Optionally map text submitted through the assistant composer to this * tool's output. Petrinaut validates both the pending input and mapped @@ -67,7 +70,7 @@ type ErasedInteractiveToolDefinition = { placement?: "work" | "card"; parseInput: (value: unknown) => unknown; parseOutput: (value: unknown) => unknown; - shouldHandle?: (input: unknown) => boolean; + shouldHandle?: (input: unknown, call: { toolCallId: string }) => boolean; fromComposerText?: (params: { input: unknown; text: string }) => unknown; component: ComponentType< PetrinautAiInteractiveToolWidgetProps @@ -100,7 +103,15 @@ export const definePetrinautAiInteractiveTool = ( parseInput: (value) => definition.inputSchema.parse(value), parseOutput: (value) => definition.outputSchema.parse(value), shouldHandle: shouldHandle - ? (value) => shouldHandle(definition.inputSchema.parse(value)) + ? (value, call) => { + let input: Input; + try { + input = definition.inputSchema.parse(value); + } catch { + return false; + } + return shouldHandle(input, call); + } : undefined, fromComposerText: fromComposerText ? ({ input, text }) => diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx index 2dd23c6f5fc..d848155e3dd 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx @@ -982,8 +982,7 @@ const ConversationAiAssistantPanel = ({ } if (!aiAssistant.inBandBrowserTools?.has(toolCall.toolName)) { resolveDynamicInteractiveTool( - toolCall.toolName, - toolCall.input, + toolCall, aiAssistant.interactiveTools ?? [], ); return; @@ -1163,7 +1162,10 @@ const ConversationAiAssistantPanel = ({ toolCall.input, ); if ( - getInteractiveTool(toolName, commandInput, aiAssistant.interactiveTools) + getInteractiveTool( + { toolName, toolCallId: toolCall.toolCallId, input: commandInput }, + aiAssistant.interactiveTools, + ) ) { return; } @@ -1870,11 +1872,7 @@ const ConversationAiAssistantPanel = ({ continue; } - const definition = getInteractiveTool( - part.toolName, - part.input, - interactiveTools, - ); + const definition = getInteractiveTool(part, interactiveTools); if (!definition?.fromComposerText) { continue; } diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents.tsx index 40551588c73..19a88431971 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents.tsx @@ -1038,11 +1038,7 @@ export const AiAssistantContents = ({ message.parts.some((part) => { if (part.type !== "dynamic-tool" || part.state !== "input-available") return false; - const tool = getInteractiveTool( - part.toolName, - part.input, - interactiveTools, - ); + const tool = getInteractiveTool(part, interactiveTools); return tool !== undefined && tool.placement !== "card"; }), ); diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents/tool-list.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents/tool-list.tsx index b03f78fd10e..0be7d52ef56 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents/tool-list.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/ai-assistant-contents/tool-list.tsx @@ -446,8 +446,15 @@ export const toToolRenderItem = ( } : defaultSummary; + const id = + typeof part.toolCallId === "string" + ? part.toolCallId + : `${message.id}-${part.type}`; const interactiveDefinition = hasInteractiveToolInput(state) - ? getInteractiveTool(toolName, part.input, interactiveTools) + ? getInteractiveTool( + { toolName, toolCallId: id, input: part.input }, + interactiveTools, + ) : undefined; const interactive = interactiveDefinition ? { @@ -458,10 +465,7 @@ export const toToolRenderItem = ( : undefined; return { - id: - typeof part.toolCallId === "string" - ? part.toolCallId - : `${message.id}-${part.type}`, + id, state, input: part.input, output: part.output, diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/apply-auto-layout-widget.test.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/apply-auto-layout-widget.test.tsx index c974dea2657..bc4b7efa196 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/apply-auto-layout-widget.test.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/apply-auto-layout-widget.test.tsx @@ -96,12 +96,16 @@ describe("ApplyAutoLayoutWidget", () => { describe("applyAutoLayoutInteractiveTool.shouldHandle", () => { test("returns true only when askUserFirst is explicitly true", () => { + const call = { toolCallId: "layout-1" }; expect( - applyAutoLayoutInteractiveTool.shouldHandle({ askUserFirst: true }), + applyAutoLayoutInteractiveTool.shouldHandle({ askUserFirst: true }, call), ).toBe(true); expect( - applyAutoLayoutInteractiveTool.shouldHandle({ askUserFirst: false }), + applyAutoLayoutInteractiveTool.shouldHandle( + { askUserFirst: false }, + call, + ), ).toBe(false); - expect(applyAutoLayoutInteractiveTool.shouldHandle({})).toBe(false); + expect(applyAutoLayoutInteractiveTool.shouldHandle({}, call)).toBe(false); }); }); diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx index 2b1056304b3..b5bb63c77ab 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx @@ -23,6 +23,12 @@ const hostTool = definePetrinautAiInteractiveTool({ component: () => null, }); +const toolCall = (toolName: string, input: unknown) => ({ + toolName, + toolCallId: `${toolName}-call`, + input, +}); + describe("interactive tool registry", () => { test("renders host confirmation only for inputs selected by its validated predicate", () => { const conditional = definePetrinautAiInteractiveTool({ @@ -35,17 +41,58 @@ describe("interactive tool registry", () => { component: () => null, }); expect( - getInteractiveTool("mutate", { destructive: false }, [conditional]), + getInteractiveTool(toolCall("mutate", { destructive: false }), [ + conditional, + ]), ).toBeUndefined(); expect( - getInteractiveTool("mutate", { destructive: true }, [conditional]), + getInteractiveTool(toolCall("mutate", { destructive: true }), [ + conditional, + ]), + ).toBeDefined(); + }); + + test("lets a host predicate tell apart calls with identical inputs", () => { + const waiting = definePetrinautAiInteractiveTool({ + toolName: "mutate", + inputSchema: { parse: (input: unknown) => input }, + outputSchema: { parse: (output: unknown) => output }, + shouldHandle: (_input, { toolCallId }) => toolCallId === "waiting", + component: () => null, + }); + const input = { placeId: "queue" }; + expect( + getInteractiveTool({ toolName: "mutate", toolCallId: "waiting", input }, [ + waiting, + ]), ).toBeDefined(); + expect( + getInteractiveTool({ toolName: "mutate", toolCallId: "earlier", input }, [ + waiting, + ]), + ).toBeUndefined(); + }); + + test("leaves a call its predicate's schema rejects to the normal tool row", () => { + const conditional = definePetrinautAiInteractiveTool({ + toolName: "confirmRelease", + inputSchema: { + parse: (): { question: string } => { + throw new Error("Expected a question"); + }, + }, + outputSchema: { parse: (output: unknown) => output }, + shouldHandle: () => true, + component: () => null, + }); + expect( + getInteractiveTool(toolCall("confirmRelease", {}), [conditional]), + ).toBeUndefined(); }); test("resolves and validates a registered dynamic host tool", () => { const definition = resolveDynamicInteractiveTool( - "confirmRelease", - { question: "Ship this change?" }, + toolCall("confirmRelease", { question: "Ship this change?" }), [hostTool], ); @@ -91,8 +138,7 @@ describe("interactive tool registry", () => { component: () => null, }); const definition = resolveDynamicInteractiveTool( - "answerQuestion", - { question: "Which environment?" }, + toolCall("answerQuestion", { question: "Which environment?" }), [mappedTool], ); @@ -118,8 +164,7 @@ describe("interactive tool registry", () => { component: () => null, }); const invalidDefinition = resolveDynamicInteractiveTool( - "invalidAnswer", - {}, + toolCall("invalidAnswer", {}), [invalidOutputTool], ); @@ -130,8 +175,7 @@ describe("interactive tool registry", () => { test("does not map composer text when the host omits the mapper", () => { const definition = resolveDynamicInteractiveTool( - "confirmRelease", - { question: "Ship this change?" }, + toolCall("confirmRelease", { question: "Ship this change?" }), [hostTool], ); @@ -140,16 +184,20 @@ describe("interactive tool registry", () => { test("rejects an unregistered dynamic tool by name", () => { expect(() => - resolveDynamicInteractiveTool("missingHostTool", {}, [hostTool]), + resolveDynamicInteractiveTool(toolCall("missingHostTool", {}), [ + hostTool, + ]), ).toThrow("Unknown AI tool: missingHostTool"); }); test("preserves the built-in applyAutoLayout branching behavior", () => { expect( - getInteractiveTool("applyAutoLayout", { askUserFirst: true }, [hostTool]), + getInteractiveTool(toolCall("applyAutoLayout", { askUserFirst: true }), [ + hostTool, + ]), ).toBeDefined(); expect( - getInteractiveTool("applyAutoLayout", { askUserFirst: false }, [ + getInteractiveTool(toolCall("applyAutoLayout", { askUserFirst: false }), [ hostTool, ]), ).toBeUndefined(); @@ -164,7 +212,7 @@ describe("interactive tool registry", () => { }); expect(() => - getInteractiveTool("applyAutoLayout", { askUserFirst: true }, [ + getInteractiveTool(toolCall("applyAutoLayout", { askUserFirst: true }), [ conflictingTool, ]), ).toThrow("conflicts with a built-in tool"); diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts index 0d7b9d2dde6..8d99eecacac 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts @@ -3,7 +3,7 @@ import { applyAutoLayoutInteractiveTool } from "./apply-auto-layout-widget"; import type { PetrinautAiInteractiveTool } from "../../../../../types/ai-interactive-tool"; import type { AiToolOutput } from "../tool-summaries"; -import type { InteractiveToolDefinition } from "./types"; +import type { InteractiveToolCall, InteractiveToolDefinition } from "./types"; /** * Registry of AI tools that require an inline chat widget for user input. @@ -27,8 +27,7 @@ export const interactiveTools: Record< }; export const getInteractiveTool = ( - toolName: string, - input: unknown, + { toolName, toolCallId, input }: InteractiveToolCall, hostTools: readonly PetrinautAiInteractiveTool[] = [], ): InteractiveToolDefinition | undefined => { const builtInDescriptor = interactiveTools[toolName]; @@ -70,24 +69,25 @@ export const getInteractiveTool = ( if (!descriptor) { return undefined; } - return descriptor.shouldHandle(input) ? descriptor : undefined; + return descriptor.shouldHandle(input, { toolCallId }) + ? descriptor + : undefined; }; /** Resolve a dynamic call only when the host explicitly registered its name. */ export const resolveDynamicInteractiveTool = ( - toolName: string, - input: unknown, + call: InteractiveToolCall, hostTools: readonly PetrinautAiInteractiveTool[], ): InteractiveToolDefinition => { - if (!hostTools.some((tool) => tool.toolName === toolName)) { - throw new Error(`Unknown AI tool: ${toolName}`); + if (!hostTools.some((tool) => tool.toolName === call.toolName)) { + throw new Error(`Unknown AI tool: ${call.toolName}`); } - const descriptor = getInteractiveTool(toolName, input, hostTools); + const descriptor = getInteractiveTool(call, hostTools); if (!descriptor) { - throw new Error(`Unknown AI tool: ${toolName}`); + throw new Error(`Unknown AI tool: ${call.toolName}`); } - descriptor.parseInput(input); + descriptor.parseInput(call.input); return descriptor; }; diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/types.ts b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/types.ts index 598ef0cb0f3..5cea8c33d02 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/types.ts +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/types.ts @@ -9,6 +9,13 @@ import type { ComponentType } from "react"; export type InteractiveToolWidgetProps = PetrinautAiInteractiveToolWidgetProps; +/** A tool call as the interactive-tool registry sees it. */ +export type InteractiveToolCall = { + toolName: string; + toolCallId: string; + input: unknown; +}; + /** * Descriptor for an AI tool that requires synchronous user input rendered * inline in the chat. The registry maps tool names to a definition; the panel @@ -25,7 +32,7 @@ export type InteractiveToolDefinition = { * input shape (e.g. `applyAutoLayout` is interactive only when * `askUserFirst: true`). */ - shouldHandle: (input: unknown) => boolean; + shouldHandle: (input: unknown, call: { toolCallId: string }) => boolean; /** Parse the raw input into the widget's typed input. */ parseInput: (raw: unknown) => Input; /** Parse the widget's output before submitting it to the AI SDK. */ From 88d7e522d7937ff7004e1c61bf0ac05f06ca79f5 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 15:31:32 +0200 Subject: [PATCH 07/12] Clarify per-call mutation denial Co-authored-by: Amp --- libs/@hashintel/petrinaut/docs/ai-assistant.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/@hashintel/petrinaut/docs/ai-assistant.md b/libs/@hashintel/petrinaut/docs/ai-assistant.md index ff5abc1bae0..addd7c63ac4 100644 --- a/libs/@hashintel/petrinaut/docs/ai-assistant.md +++ b/libs/@hashintel/petrinaut/docs/ai-assistant.md @@ -63,7 +63,7 @@ Timing is shown when supplied or observed during this session; unavailable tool durations show a dash. Disclosure icons are neutral; status dots distinguish pending, completed, and failed tools. -Before Brunch removes model elements, an approval lists the requested removals. Associated arcs or references may also be removed. **Allow** applies that removal; **Deny** skips it and tells Brunch nothing was changed. Brunch's later edits wait until you answer. **Always allow** permits later removals only in the current mounted conversation, until you leave or reload. It does not grant permission for another conversation or browser session. Stop cancels a pending approval. Stock auto-layout approval is unchanged. +Before Brunch removes model elements, an approval lists the requested removals. Associated arcs or references may also be removed. **Allow** applies that removal; **Deny** withholds that call and tells Brunch nothing was changed. Brunch's later calls in the same response wait until you answer, then continue to run. **Always allow** permits later removals only in the current mounted conversation, until you leave or reload. It does not grant permission for another conversation or browser session. Stop cancels a pending approval. Auto-layout asks separately; see `applyAutoLayout` below. When the host supplies them, Voice also shows a collapsed brief directly under your message, an immediate spoken-agent reply before the work, and a wrap-up after the produced cards. The brief says **Preparing for Brunch** while its fields are being prepared, **Sending to Brunch** once the fields are ready but not yet accepted, and **Sent to Brunch** after acceptance. Expand a prepared brief to see **Prepared from what you said** and its right-aligned fields. These optional parts are absent in hosts that do not provide them. In Chat, a small neutral voice-bars icon marks user messages sent using Voice; typed messages have no icon. In Voice, those per-message icons are hidden. From bf80b006806e43d658e44239f47f6e58d2a19897 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 15:31:42 +0200 Subject: [PATCH 08/12] Dispose mutation approvals on unmount Co-authored-by: Amp --- .../local-storage-demo-app.test.tsx | 20 +++++++++++++++++++ .../local-storage-demo-app.tsx | 1 + 2 files changed, 21 insertions(+) diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx index 7603ffa30e0..33163c94f2c 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx @@ -74,6 +74,9 @@ const flueClientMock = vi.hoisted(() => ({ current: null as unknown })); const flueClientOptions = vi.hoisted(() => ({ current: null as unknown })); const renderedPetrinaut = vi.hoisted(() => ({ aiAssistant: null as unknown })); const renderedAssistants = vi.hoisted(() => [] as PetrinautAiAssistant[]); +const mutationApprovalCoordinators = vi.hoisted( + () => [] as { dispose: () => void }[], +); vi.mock("@flue/sdk", () => ({ createFlueClient: (options: unknown) => { flueClientOptions.current = options; @@ -89,6 +92,20 @@ vi.mock("./brunch-preview-config", () => ({ resolveBrunchPreviewConfig: () => brunchPreviewConfig, })); +vi.mock("./brunch-mutation-approval", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + createBrunchMutationApprovalCoordinator: () => { + const coordinator = actual.createBrunchMutationApprovalCoordinator(); + vi.spyOn(coordinator, "dispose"); + mutationApprovalCoordinators.push(coordinator); + return coordinator; + }, + }; +}); + const editorProps = vi.hoisted(() => ({ current: null as { aiAssistant?: unknown; @@ -430,7 +447,10 @@ describe("local storage demo Brunch voice integration", () => { ], ); + const mutationApprovalCoordinator = mutationApprovalCoordinators.at(-1); + expect(mutationApprovalCoordinator).toBeDefined(); rendered.unmount(); + expect(mutationApprovalCoordinator?.dispose).toHaveBeenCalledOnce(); vi.unstubAllGlobals(); }); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index 64b56b1ce1c..7de506b463a 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -676,6 +676,7 @@ export const LocalStorageDemoApp = ({ liveMutationApprovalRef.current.dispose(); liveMutationApprovalRef.current = mutationApproval.coordinator; }, [mutationApproval]); + useEffect(() => () => liveMutationApprovalRef.current.dispose(), []); const allMutationApprovalTools = useMemo( () => createBrunchMutationApprovalInteractiveTools( From b99f11410b65519d30edd0305338214522efbae8 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 15:31:55 +0200 Subject: [PATCH 09/12] Document interactive tool predicate updates Co-authored-by: Amp --- libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts index d7e0c87a655..e1de735aa7c 100644 --- a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts +++ b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts @@ -50,7 +50,8 @@ export type PetrinautAiInteractiveToolDefinition = { outputSchema: PetrinautAiInteractiveToolSchema; /** * Render an interaction only for matching calls. Defaults to all. Inputs - * the schema rejects are never handled. + * the schema rejects are never handled. This runs during render; when it + * depends on host state, rebuild `interactiveTools` as that state changes. */ shouldHandle?: (input: Input, call: { toolCallId: string }) => boolean; /** From cac4ca649739bf8b76c60cab5744d391c74f0dd7 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 16:23:01 +0200 Subject: [PATCH 10/12] Reopen the approval coordinator when Strict Mode remounts the conversation Co-authored-by: Cursor --- .../brunch-mutation-approval.test.tsx | 29 +++++++++++++++++++ .../brunch-mutation-approval.tsx | 18 ++++++++---- .../local-storage-demo-app.test.tsx | 6 ++-- .../local-storage-demo-app.tsx | 8 ++--- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx index f4ca6b5f06a..3c88aefa702 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx @@ -117,6 +117,35 @@ describe("Brunch destructive edit approval", () => { expect(fresh.hasPending("delete-3")).toBe(false); }); + test("closing stops waiting approvals and reopening accepts new ones", async () => { + const coordinator = createBrunchMutationApprovalCoordinator(); + const waiting = coordinator.request({ + toolCallId: "delete-1", + toolName: "deleteItemsByIds", + signal: new AbortController().signal, + }); + coordinator.close(); + await expect(waiting).resolves.toEqual({ + decision: "deny", + reason: "The destructive edit was stopped before approval.", + }); + await expect( + coordinator.request({ + toolCallId: "delete-2", + toolName: "deleteItemsByIds", + signal: new AbortController().signal, + }), + ).resolves.toMatchObject({ decision: "deny" }); + + coordinator.open(); + void coordinator.request({ + toolCallId: "delete-3", + toolName: "deleteItemsByIds", + signal: new AbortController().signal, + }); + expect(coordinator.hasPending("delete-3")).toBe(true); + }); + test("reports which tools wait for a decision, keeping the list stable between changes", () => { const coordinator = createBrunchMutationApprovalCoordinator(); const idle = coordinator.pendingToolNames(); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx index d66d818d103..22451d007af 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -37,7 +37,10 @@ export interface BrunchMutationApprovalCoordinator { /** Stable until the set of waiting tool names changes. */ pendingToolNames: () => readonly string[]; subscribe: (listener: () => void) => () => void; - dispose(): void; + /** Settles waiting approvals as stopped and refuses new ones until reopened. */ + close(): void; + /** React Strict Mode closes and reopens the coordinator of a mounted conversation. */ + open(): void; } const stoppedReason = "The destructive edit was stopped before approval."; @@ -49,7 +52,7 @@ export const createBrunchMutationApprovalCoordinator = const pending = new Map(); const listeners = new Set<() => void>(); let alwaysAllow = false; - let disposed = false; + let closed = false; let pendingToolNames: readonly string[] = []; const notify = () => { const next = [ @@ -73,7 +76,7 @@ export const createBrunchMutationApprovalCoordinator = return { request: ({ toolCallId, toolName, signal }) => { - if (disposed || signal.aborted) + if (closed || signal.aborted) return Promise.resolve({ decision: "deny", reason: stoppedReason }); if (alwaysAllow) return Promise.resolve({ decision: "allow" }); return new Promise((resolve) => { @@ -91,7 +94,7 @@ export const createBrunchMutationApprovalCoordinator = }); }, resolve: (toolCallId, choice) => { - if (!pending.has(toolCallId) || disposed) return false; + if (!pending.has(toolCallId) || closed) return false; if (choice === "always-allow") alwaysAllow = true; return settle( toolCallId, @@ -106,12 +109,15 @@ export const createBrunchMutationApprovalCoordinator = listeners.add(listener); return () => listeners.delete(listener); }, - dispose: () => { - disposed = true; + close: () => { + closed = true; alwaysAllow = false; for (const toolCallId of pending.keys()) settle(toolCallId, { decision: "deny", reason: stoppedReason }); }, + open: () => { + closed = false; + }, }; }; diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx index 33163c94f2c..12b35bcf115 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx @@ -75,7 +75,7 @@ const flueClientOptions = vi.hoisted(() => ({ current: null as unknown })); const renderedPetrinaut = vi.hoisted(() => ({ aiAssistant: null as unknown })); const renderedAssistants = vi.hoisted(() => [] as PetrinautAiAssistant[]); const mutationApprovalCoordinators = vi.hoisted( - () => [] as { dispose: () => void }[], + () => [] as { close: () => void }[], ); vi.mock("@flue/sdk", () => ({ createFlueClient: (options: unknown) => { @@ -99,7 +99,7 @@ vi.mock("./brunch-mutation-approval", async (importOriginal) => { ...actual, createBrunchMutationApprovalCoordinator: () => { const coordinator = actual.createBrunchMutationApprovalCoordinator(); - vi.spyOn(coordinator, "dispose"); + vi.spyOn(coordinator, "close"); mutationApprovalCoordinators.push(coordinator); return coordinator; }, @@ -450,7 +450,7 @@ describe("local storage demo Brunch voice integration", () => { const mutationApprovalCoordinator = mutationApprovalCoordinators.at(-1); expect(mutationApprovalCoordinator).toBeDefined(); rendered.unmount(); - expect(mutationApprovalCoordinator?.dispose).toHaveBeenCalledOnce(); + expect(mutationApprovalCoordinator?.close).toHaveBeenCalledOnce(); vi.unstubAllGlobals(); }); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index 7de506b463a..22dd3b2b619 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -670,13 +670,11 @@ export const LocalStorageDemoApp = ({ ); // The panel stays mounted when the binding changes, so a replaced authority // must settle the approvals still waiting on it. - const liveMutationApprovalRef = useRef(mutationApproval.coordinator); useEffect(() => { - if (liveMutationApprovalRef.current !== mutationApproval.coordinator) - liveMutationApprovalRef.current.dispose(); - liveMutationApprovalRef.current = mutationApproval.coordinator; + const { coordinator } = mutationApproval; + coordinator.open(); + return () => coordinator.close(); }, [mutationApproval]); - useEffect(() => () => liveMutationApprovalRef.current.dispose(), []); const allMutationApprovalTools = useMemo( () => createBrunchMutationApprovalInteractiveTools( From 7945526ab8d283454100bbdbedefcdea2ee937c0 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 18:13:11 +0200 Subject: [PATCH 11/12] Simplify conversation approval selection and narrow its public predicate Co-authored-by: Amp --- .../brunch-mutation-approval.test.tsx | 34 +++++++++----- .../brunch-mutation-approval.tsx | 46 ++++--------------- .../local-storage-demo-app.test.tsx | 29 ++++++++---- .../local-storage-demo-app.tsx | 21 +++++---- .../src/ui/types/ai-interactive-tool.ts | 20 ++------ .../interactive-tools/registry.test.tsx | 23 ++++++---- .../interactive-tools/registry.ts | 3 +- 7 files changed, 82 insertions(+), 94 deletions(-) diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx index 3c88aefa702..fe5fa665da3 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.test.tsx @@ -1,7 +1,7 @@ /** * @vitest-environment jsdom */ -import { fireEvent, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import { afterEach, describe, expect, test, vi } from "vitest"; import { canonicalContent } from "@hashintel/brunch-agent-plugin-sdcpn"; @@ -29,7 +29,10 @@ vi.hoisted(() => { }); }); -afterEach(() => vi.unstubAllGlobals()); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); const destructiveInput = { items: [ @@ -146,20 +149,28 @@ describe("Brunch destructive edit approval", () => { expect(coordinator.hasPending("delete-3")).toBe(true); }); - test("reports which tools wait for a decision, keeping the list stable between changes", () => { + test("changes its snapshot for every pending call, including calls of the same tool", () => { const coordinator = createBrunchMutationApprovalCoordinator(); - const idle = coordinator.pendingToolNames(); - expect(idle).toEqual([]); + const idle = coordinator.getVersion(); void coordinator.request({ toolCallId: "remove-1", toolName: "removePlace", signal: new AbortController().signal, }); - const waiting = coordinator.pendingToolNames(); - expect(waiting).toEqual(["removePlace"]); - expect(coordinator.pendingToolNames()).toBe(waiting); + const waiting = coordinator.getVersion(); + expect(waiting).not.toBe(idle); + expect(coordinator.getVersion()).toBe(waiting); + void coordinator.request({ + toolCallId: "remove-2", + toolName: "removePlace", + signal: new AbortController().signal, + }); + const bothWaiting = coordinator.getVersion(); + expect(bothWaiting).not.toBe(waiting); coordinator.resolve("remove-1", "deny"); - expect(coordinator.pendingToolNames()).toEqual([]); + expect(coordinator.getVersion()).not.toBe(bothWaiting); + expect(coordinator.hasPending("remove-2")).toBe(true); + coordinator.close(); }); test("a malformed call is refused before it can wait for an approval that never renders", async () => { @@ -182,7 +193,7 @@ describe("Brunch destructive edit approval", () => { coordinator, "deleteItemsByIds", ); - const { container } = render( + render( { toolCallId="historical-call" />, ); - expect(container.innerHTML).toBe(""); + fireEvent.click(screen.getByRole("button", { name: "Allow" })); + expect(coordinator.hasPending("historical-call")).toBe(false); expect(coordinator.resolve("historical-call", "allow")).toBe(false); }); }); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx index 22451d007af..f1c304fbe94 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/brunch-mutation-approval.tsx @@ -1,5 +1,3 @@ -import { useSyncExternalStore } from "react"; - import { Button } from "@hashintel/ds-components"; import { css } from "@hashintel/ds-helpers/css"; import { @@ -21,7 +19,6 @@ type BrunchMutationApprovalDecision = | { readonly decision: "deny"; readonly reason: string }; type PendingApproval = { - readonly toolName: string; readonly resolve: (decision: BrunchMutationApprovalDecision) => void; readonly removeAbortListener: () => void; }; @@ -34,8 +31,8 @@ export interface BrunchMutationApprovalCoordinator { }): Promise; resolve(toolCallId: string, choice: BrunchMutationApprovalChoice): boolean; hasPending(toolCallId: string): boolean; - /** Stable until the set of waiting tool names changes. */ - pendingToolNames: () => readonly string[]; + /** Changes whenever a call enters or leaves the approval gate. */ + getVersion: () => number; subscribe: (listener: () => void) => () => void; /** Settles waiting approvals as stopped and refuses new ones until reopened. */ close(): void; @@ -53,12 +50,9 @@ export const createBrunchMutationApprovalCoordinator = const listeners = new Set<() => void>(); let alwaysAllow = false; let closed = false; - let pendingToolNames: readonly string[] = []; + let version = 0; const notify = () => { - const next = [ - ...new Set([...pending.values()].map((approval) => approval.toolName)), - ]; - if (next.join() !== pendingToolNames.join()) pendingToolNames = next; + version += 1; listeners.forEach((listener) => listener()); }; const settle = ( @@ -75,7 +69,7 @@ export const createBrunchMutationApprovalCoordinator = }; return { - request: ({ toolCallId, toolName, signal }) => { + request: ({ toolCallId, signal }) => { if (closed || signal.aborted) return Promise.resolve({ decision: "deny", reason: stoppedReason }); if (alwaysAllow) return Promise.resolve({ decision: "allow" }); @@ -85,7 +79,6 @@ export const createBrunchMutationApprovalCoordinator = }; signal.addEventListener("abort", onAbort, { once: true }); pending.set(toolCallId, { - toolName, resolve, removeAbortListener: () => signal.removeEventListener("abort", onAbort), @@ -104,7 +97,7 @@ export const createBrunchMutationApprovalCoordinator = ); }, hasPending: (toolCallId) => pending.has(toolCallId), - pendingToolNames: () => pendingToolNames, + getVersion: () => version, subscribe: (listener) => { listeners.add(listener); return () => listeners.delete(listener); @@ -240,33 +233,11 @@ const actionsStyle = css({ display: "flex", gap: "2", flexWrap: "wrap" }); type WidgetProps = PetrinautAiInteractiveToolWidgetProps; -const settledText = (output: unknown): string => { - if (typeof output !== "object" || output === null) - return "Model edits completed."; - if ("reason" in output && typeof output.reason === "string") - return output.reason; - if ("title" in output && typeof output.title === "string") - return output.title; - return "Model edits completed."; -}; - export const createBrunchMutationApprovalWidget = ( coordinator: BrunchMutationApprovalCoordinator, toolName: DestructiveToolName, ) => { - const Widget = ({ - input, - toolCallId, - state, - submittedOutput, - }: WidgetProps) => { - const pending = useSyncExternalStore( - coordinator.subscribe, - () => coordinator.hasPending(toolCallId), - () => false, - ); - if (state === "submitted") return

{settledText(submittedOutput)}

; - if (!pending) return null; + const Widget = ({ input, toolCallId }: WidgetProps) => { return (
- coordinator.hasPending(toolCallId), + shouldHandle: ({ toolCallId }) => coordinator.hasPending(toolCallId), component: createBrunchMutationApprovalWidget(coordinator, toolName), }), ); diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx index 12b35bcf115..c97dc742dac 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.test.tsx @@ -420,7 +420,12 @@ describe("local storage demo Brunch voice integration", () => { expect(aiAssistant.executeMutation).toBeUndefined(); expect( aiAssistant.interactiveTools?.map(({ toolName }) => toolName), - ).toEqual([brunchTools.draftPetrinautExperiment]); + ).toEqual( + expect.arrayContaining([ + brunchTools.draftPetrinautExperiment, + "removePlace", + ]), + ); expect(aiAssistant.resolveToolPresentation).toBeTypeOf("function"); expect(aiAssistant.workingLabel).toBe("Brunch is working"); expect( @@ -1268,6 +1273,7 @@ describe("local storage demo Brunch controls", () => { ); const assistant = editorProps.current ?.aiAssistant as PetrinautAiAssistant; + const initialTools = assistant.interactiveTools; const execute = vi.fn(async () => ({ applied: true })); const run = assistant.inBandBrowserTools?.run( { @@ -1278,18 +1284,16 @@ describe("local storage demo Brunch controls", () => { }, execute, ); - const interactiveToolNames = () => - ( - editorProps.current?.aiAssistant as PetrinautAiAssistant | undefined - )?.interactiveTools?.map(({ toolName }) => toolName); + const interactiveTools = () => + (editorProps.current?.aiAssistant as PetrinautAiAssistant | undefined) + ?.interactiveTools; await waitFor(() => expect(claimed).toHaveBeenCalled()); - await waitFor(() => - expect(interactiveToolNames()).toContain("removePlace"), - ); + await waitFor(() => expect(interactiveTools()).not.toBe(initialTools)); + const waitingTools = interactiveTools(); act(() => assistant.onClearMessages?.()); await run; - expect(interactiveToolNames()).not.toContain("removePlace"); + expect(interactiveTools()).not.toBe(waitingTools); expect(execute).not.toHaveBeenCalled(); expect(posted).toEqual([ expect.objectContaining({ @@ -1391,7 +1395,12 @@ describe("local storage demo Brunch controls", () => { ); expect( aiAssistant.interactiveTools?.map(({ toolName }) => toolName), - ).toEqual([brunchTools.draftPetrinautExperiment]); + ).toEqual( + expect.arrayContaining([ + brunchTools.draftPetrinautExperiment, + "removePlace", + ]), + ); expect(transportOptions.mapClientToolInput).toEqual(expect.any(Function)); // Every configured Brunch browser tool, the draft included, settles in band. expect(aiAssistant.inBandBrowserTools?.has(createExperimentToolName)).toBe( diff --git a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx index 22dd3b2b619..d7666b91685 100644 --- a/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx +++ b/apps/petrinaut-website/src/main/app/local-storage-demo/local-storage-demo-app.tsx @@ -683,19 +683,20 @@ export const LocalStorageDemoApp = ({ [mutationApproval], ); // A registered widget replaces the tool's row, so only calls still waiting - // for a decision render as approvals; others keep the normal tool row. - const pendingApprovalToolNames = useSyncExternalStore( + // for a decision render as approvals. Refresh the registry when call identities + // change; shouldHandle is the single gate, including for same-name calls. + const approvalVersion = useSyncExternalStore( mutationApproval.coordinator.subscribe, - mutationApproval.coordinator.pendingToolNames, - mutationApproval.coordinator.pendingToolNames, + mutationApproval.coordinator.getVersion, + mutationApproval.coordinator.getVersion, ); const mutationApprovalTools = useMemo( - () => - allMutationApprovalTools.filter(({ toolName }) => - pendingApprovalToolNames.includes(toolName), - ), - [allMutationApprovalTools, pendingApprovalToolNames], - ); + () => ({ + version: approvalVersion, + tools: [...allMutationApprovalTools], + }), + [allMutationApprovalTools, approvalVersion], + ).tools; const processAgentSession = useProcessAgentSession({ binding: processAgentBinding, brunchSelected, diff --git a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts index b7d4c116152..04f3a4052dc 100644 --- a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts +++ b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts @@ -50,11 +50,11 @@ export type PetrinautAiInteractiveToolDefinition = { /** Runtime contract for the widget's submitted output. */ outputSchema: PetrinautAiInteractiveToolSchema; /** - * Render an interaction only for matching calls. Defaults to all. Inputs - * the schema rejects are never handled. This runs during render; when it + * Render an interaction only for matching call identities. Defaults to all. + * This runs during render without parsing the input; when it * depends on host state, rebuild `interactiveTools` as that state changes. */ - shouldHandle?: (input: Input, call: { toolCallId: string }) => boolean; + shouldHandle?: (call: { toolCallId: string }) => boolean; /** * Optionally map text submitted through the assistant composer to this * tool's output. Petrinaut validates both the pending input and mapped @@ -72,7 +72,7 @@ type ErasedInteractiveToolDefinition = { placement?: "work" | "card"; parseInput: (value: unknown) => unknown; parseOutput: (value: unknown) => unknown; - shouldHandle?: (input: unknown, call: { toolCallId: string }) => boolean; + shouldHandle?: (call: { toolCallId: string }) => boolean; fromComposerText?: (params: { input: unknown; text: string }) => unknown; component: ComponentType< PetrinautAiInteractiveToolWidgetProps @@ -104,17 +104,7 @@ export const definePetrinautAiInteractiveTool = ( placement: definition.placement, parseInput: (value) => definition.inputSchema.parse(value), parseOutput: (value) => definition.outputSchema.parse(value), - shouldHandle: shouldHandle - ? (value, call) => { - let input: Input; - try { - input = definition.inputSchema.parse(value); - } catch { - return false; - } - return shouldHandle(input, call); - } - : undefined, + shouldHandle, fromComposerText: fromComposerText ? ({ input, text }) => definition.outputSchema.parse( diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx index b5bb63c77ab..2069402ceee 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx @@ -30,26 +30,26 @@ const toolCall = (toolName: string, input: unknown) => ({ }); describe("interactive tool registry", () => { - test("renders host confirmation only for inputs selected by its validated predicate", () => { + test("selects host calls without parsing their input during lookup", () => { + const parse = vi.fn((input: unknown) => input); const conditional = definePetrinautAiInteractiveTool({ toolName: "mutate", - inputSchema: { - parse: (input: unknown) => input as { destructive: boolean }, - }, + inputSchema: { parse }, outputSchema: { parse: (output: unknown) => output }, - shouldHandle: (input) => input.destructive, + shouldHandle: ({ toolCallId }) => toolCallId === "mutate-call", component: () => null, }); expect( getInteractiveTool(toolCall("mutate", { destructive: false }), [ conditional, ]), - ).toBeUndefined(); + ).toBeDefined(); expect( getInteractiveTool(toolCall("mutate", { destructive: true }), [ conditional, ]), ).toBeDefined(); + expect(parse).not.toHaveBeenCalled(); }); test("lets a host predicate tell apart calls with identical inputs", () => { @@ -57,7 +57,7 @@ describe("interactive tool registry", () => { toolName: "mutate", inputSchema: { parse: (input: unknown) => input }, outputSchema: { parse: (output: unknown) => output }, - shouldHandle: (_input, { toolCallId }) => toolCallId === "waiting", + shouldHandle: ({ toolCallId }) => toolCallId === "waiting", component: () => null, }); const input = { placeId: "queue" }; @@ -73,7 +73,7 @@ describe("interactive tool registry", () => { ).toBeUndefined(); }); - test("leaves a call its predicate's schema rejects to the normal tool row", () => { + test("validates selected calls when resolving them, not during lookup", () => { const conditional = definePetrinautAiInteractiveTool({ toolName: "confirmRelease", inputSchema: { @@ -87,7 +87,12 @@ describe("interactive tool registry", () => { }); expect( getInteractiveTool(toolCall("confirmRelease", {}), [conditional]), - ).toBeUndefined(); + ).toBeDefined(); + expect(() => + resolveDynamicInteractiveTool(toolCall("confirmRelease", {}), [ + conditional, + ]), + ).toThrow("Expected a question"); }); test("resolves and validates a registered dynamic host tool", () => { diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts index 8d99eecacac..8f6bc2b8e01 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts @@ -59,7 +59,8 @@ export const getInteractiveTool = ( ? { toolName: hostDefinition.toolName, placement: hostDefinition.placement, - shouldHandle: hostDefinition.shouldHandle ?? (() => true), + shouldHandle: (_input, call) => + hostDefinition.shouldHandle?.(call) ?? true, parseInput: hostDefinition.parseInput, parseOutput: hostDefinition.parseOutput, fromComposerText: hostDefinition.fromComposerText, From ea3e8f3d333e42e6cdcf57be68541176d7ebf803 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Wed, 30 Sep 2026 22:03:15 +0200 Subject: [PATCH 12/12] Report host-declined dynamic tool calls as declined Document that shouldHandle only suppresses the widget for tools the host executes itself, and name the declined call instead of reporting a registered tool as unknown. Refs FE-1793 Co-authored-by: Cursor --- .../src/ui/types/ai-interactive-tool.ts | 4 ++++ .../interactive-tools/registry.test.tsx | 17 +++++++++++++++++ .../interactive-tools/registry.ts | 4 +++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts index 04f3a4052dc..7850f0fab4f 100644 --- a/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts +++ b/libs/@hashintel/petrinaut/src/ui/types/ai-interactive-tool.ts @@ -53,6 +53,10 @@ export type PetrinautAiInteractiveToolDefinition = { * Render an interaction only for matching call identities. Defaults to all. * This runs during render without parsing the input; when it * depends on host state, rebuild `interactiveTools` as that state changes. + * + * Declining only suppresses the widget for tools the host executes itself + * (`inBandBrowserTools`). Any other registered tool is completed solely by + * its widget, so a declined call fails rather than waiting for a result. */ shouldHandle?: (call: { toolCallId: string }) => boolean; /** diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx index 2069402ceee..943c303d1d1 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.test.tsx @@ -195,6 +195,23 @@ describe("interactive tool registry", () => { ).toThrow("Unknown AI tool: missingHostTool"); }); + test("reports a registered host tool that declines a call as declined", () => { + const declining = definePetrinautAiInteractiveTool({ + toolName: "confirmRelease", + inputSchema: { parse: (input: unknown) => input }, + outputSchema: { parse: (output: unknown) => output }, + shouldHandle: () => false, + component: () => null, + }); + expect(() => + resolveDynamicInteractiveTool(toolCall("confirmRelease", {}), [ + declining, + ]), + ).toThrow( + "AI tool confirmRelease was declined by the host for call confirmRelease-call", + ); + }); + test("preserves the built-in applyAutoLayout branching behavior", () => { expect( getInteractiveTool(toolCall("applyAutoLayout", { askUserFirst: true }), [ diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts index 8f6bc2b8e01..b4d188dbdf8 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel/interactive-tools/registry.ts @@ -86,7 +86,9 @@ export const resolveDynamicInteractiveTool = ( const descriptor = getInteractiveTool(call, hostTools); if (!descriptor) { - throw new Error(`Unknown AI tool: ${call.toolName}`); + throw new Error( + `AI tool ${call.toolName} was declined by the host for call ${call.toolCallId}`, + ); } descriptor.parseInput(call.input);