From 320d616b79c91c16ca4544db0e2f606c07e93591 Mon Sep 17 00:00:00 2001 From: Matias Palma Date: Mon, 20 Apr 2026 22:45:58 -0400 Subject: [PATCH 1/2] fix: key tool-permission resolvers by call id instead of window.resolveTool The previous implementation stored the in-flight permission resolver on `window.resolveTool`, which meant two overlapping permission prompts (two chat submissions racing, a tool loop retrying over its own pending prompt, etc.) silently overwrote the first resolver. The first Promise then dangled forever and its `await permissionPromise` never returned, leaking the first tool call's state and stalling the outer send loop. It also polluted the global `Window` type with an implementation detail. Changes: - AiChatComponent keeps a ref-backed Map. Each permission prompt registers its own resolver by call id, so the Allow/Deny buttons on the UI look up the resolver for the currently visible `pendingTool.id` rather than a shared global. - If a new prompt arrives while a previous one is still unresolved, the older entry is resolved as denied before the new one takes over the single-slot dialog. That's the safe default: the user never actually saw the older dialog (only the latest renders), so the earlier tool is not approved. - A cleanup effect resolves every remaining entry as denied on unmount so awaiters don't leak past the component's lifetime. - Dropped the global `resolveTool?: (allowed: boolean) => void` declaration from `src/api/trixty.ts`. --- .../builtin.ai-assistant/AiChatComponent.tsx | 44 ++++++++++++++++--- apps/desktop/src/api/trixty.ts | 1 - 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/apps/desktop/src/addons/builtin.ai-assistant/AiChatComponent.tsx b/apps/desktop/src/addons/builtin.ai-assistant/AiChatComponent.tsx index ecb7393a..1bdb3fc9 100644 --- a/apps/desktop/src/addons/builtin.ai-assistant/AiChatComponent.tsx +++ b/apps/desktop/src/addons/builtin.ai-assistant/AiChatComponent.tsx @@ -77,6 +77,21 @@ const AiChatComponent: React.FC = () => { const scrollRef = useRef(null); const abortControllerRef = useRef(null); + // Per-call permission resolvers. Previously a single `window.resolveTool` + // held the in-flight resolver globally, which meant two concurrent + // permission prompts (two chat submissions racing, or a tool-call loop + // overlapping with a retry) would silently overwrite the first resolver + // and leave the first Promise dangling forever. A Map keyed by call id + // keeps each prompt independent; on unmount we resolve everything as + // denied so pending awaiters don't leak past the component's lifetime. + const pendingResolversRef = useRef void>>(new Map()); + useEffect(() => { + const resolvers = pendingResolversRef.current; + return () => { + for (const resolver of resolvers.values()) resolver(false); + resolvers.clear(); + }; + }, []); useEffect(() => { // Load last used model from storage @@ -449,13 +464,18 @@ const AiChatComponent: React.FC = () => { if (aiSettings.alwaysAllowTools) { result = await executeToolInternal(toolName, toolArgs); } else { - // Manual permission needed + // Manual permission needed. The dialog UI has a single slot, so + // any older unresolved prompt is already invisible to the user; + // we deny them here instead of letting their Promises dangle. const permissionPromise = new Promise((resolve) => { + for (const [oldId, oldResolver] of pendingResolversRef.current) { + if (oldId !== callId) { + oldResolver(false); + pendingResolversRef.current.delete(oldId); + } + } + pendingResolversRef.current.set(callId, resolve); setPendingTool({ id: callId, name: toolName, args: toolArgs }); - window.resolveTool = (allowed: boolean) => { - setPendingTool(null); - resolve(allowed); - }; }); const allowed = await permissionPromise; @@ -766,13 +786,23 @@ const AiChatComponent: React.FC = () => {