diff --git a/apps/extension/src/tools/__tests__/human-loop.test.ts b/apps/extension/src/tools/__tests__/human-loop.test.ts index 04f6352e..530c2a45 100644 --- a/apps/extension/src/tools/__tests__/human-loop.test.ts +++ b/apps/extension/src/tools/__tests__/human-loop.test.ts @@ -399,6 +399,127 @@ describe("handleRequestHelp", () => { } }); + it("does not keep the RPC pending when cleanup hangs after content finishes", async () => { + vi.useFakeTimers(); + const chromeEvents = installHelpLifecycleChrome(); + const sendToTab = vi.fn(async (_tabId: number, message: { type?: string }) => { + if (message.type === "bsk-help-request") return { type: "bsk-help-ack", ok: true }; + return new Promise(() => {}); + }); + const deps = baseDeps({ autoAttachLifecycle: undefined, sendToTab }); + + try { + const pending = handleRequestHelp( + fakeManager("abcd", 99, 5), + baseParams({ tab_id: 5, timeout_ms: 60_000 }), + deps, + ); + await vi.waitFor(() => + expect(sendToTab).toHaveBeenCalledWith( + 5, + expect.objectContaining({ type: "bsk-help-request" }), + ), + ); + + const request = sendToTab.mock.calls[0]?.[1] as { requestId: string }; + chromeEvents.runtimeOnMessage.emit( + { + type: "bsk-help-finish", + requestId: request.requestId, + outcome: "continued", + }, + { tab: { id: 5 } as chrome.tabs.Tab } as chrome.runtime.MessageSender, + vi.fn(), + ); + await vi.waitFor(() => + expect(sendToTab).toHaveBeenCalledWith( + 5, + expect.objectContaining({ type: "bsk-help-cancel" }), + ), + ); + + await vi.advanceTimersByTimeAsync(1_100); + await expect(pending).resolves.toMatchObject({ outcome: "continued", tab_id: 5 }); + } finally { + vi.useRealTimers(); + } + }); + + it.each([ + "content", + "timeout", + "abort", + ] as const)("cancels all help overlays when notification cleanup hangs (%s)", async (finishPath) => { + vi.useFakeTimers(); + const chromeEvents = installHelpLifecycleChrome(); + const abort = new AbortController(); + const sendToTab = vi.fn(async (_tabId: number, _message: { requestId: string }) => ({ + type: "bsk-help-ack", + ok: true, + })); + const notifications = { + create: vi.fn(async (id: string) => id), + clear: vi.fn(() => new Promise(() => {})), + }; + const deps = baseDeps({ + autoAttachLifecycle: undefined, + sendToTab, + notifications, + signal: abort.signal, + tabsApi: { + get: vi.fn(async (id: number) => ({ id, windowId: 99, active: id === 5 }) as never), + query: vi.fn(async () => [{ id: 5, windowId: 99, active: true }] as never), + }, + }); + + try { + const pending = handleRequestHelp( + fakeManager("abcd", 99, 5), + baseParams({ tab_id: 5, timeout_ms: 60_000 }), + deps, + ); + await vi.waitFor(() => + expect(sendToTab).toHaveBeenCalledWith( + 5, + expect.objectContaining({ type: "bsk-help-request" }), + ), + ); + const { requestId } = sendToTab.mock.calls[0][1]; + chromeEvents.tabsOnCreated.emit({ id: 6, windowId: 99 } as chrome.tabs.Tab); + await vi.advanceTimersByTimeAsync(200); + expect(sendToTab).toHaveBeenCalledWith( + 6, + expect.objectContaining({ type: "bsk-help-request", requestId }), + ); + + if (finishPath === "content") { + chromeEvents.runtimeOnMessage.emit( + { type: "bsk-help-finish", requestId, outcome: "continued" }, + { tab: { id: 5 } as chrome.tabs.Tab } as chrome.runtime.MessageSender, + vi.fn(), + ); + } else if (finishPath === "abort") { + abort.abort(); + } else { + await vi.advanceTimersByTimeAsync(60_000); + } + + await vi.advanceTimersByTimeAsync(1_100); + const expected = { + content: { outcome: "continued" }, + timeout: { outcome: "timed_out" }, + abort: { code: "cancelled" }, + }[finishPath]; + await expect(pending).resolves.toMatchObject(expected); + expect(notifications.clear).toHaveBeenCalledWith(`bsk-help:${requestId}`); + for (const tabId of [5, 6]) { + expect(sendToTab).toHaveBeenCalledWith(tabId, { type: "bsk-help-cancel", requestId }); + } + } finally { + vi.useRealTimers(); + } + }); + it("returns completed when explicit completion criteria match", async () => { const deps = baseDeps({ sendToTab: vi.fn(async () => ({ type: "bsk-help-ack", ok: true })), diff --git a/apps/extension/src/tools/human-loop.ts b/apps/extension/src/tools/human-loop.ts index d53c4e92..92f491b8 100644 --- a/apps/extension/src/tools/human-loop.ts +++ b/apps/extension/src/tools/human-loop.ts @@ -45,6 +45,7 @@ const HELP_SEND_RETRY_DELAY_MS = 350; const HELP_REARM_DEBOUNCE_MS = 150; const HELP_REARM_MAX_ATTEMPTS = 12; const HELP_REARM_RETRY_DELAY_MS = 400; +const HELP_CLEANUP_TIMEOUT_MS = 1_000; const DEFAULT_COMPLETION_STABLE_MS = 1_000; const COMPLETION_POLL_MS = 500; @@ -288,15 +289,29 @@ async function refreshHelpTargets( } async function cleanupHelp(help: ActiveHelpRequest): Promise { - if (help.deps.notifications) { - await help.deps.notifications.clear(help.notificationId).catch(() => {}); - } const tabsToCancel = new Set([help.primaryTabId, ...help.overlayTabIds]); - await Promise.all( - [...tabsToCancel].map((tabId) => + // A stalled notification must not prevent cancellation of the page overlays. + await Promise.all([ + ...[...tabsToCancel].map((tabId) => help.deps.sendToTab(tabId, { type: HELP_CANCEL, requestId: help.requestId }).catch(() => {}), ), - ); + help.deps.notifications?.clear(help.notificationId).catch(() => {}), + ]); +} + +/** Never let best-effort UI cleanup hold the daemon RPC open indefinitely. */ +async function awaitCleanupWithinBudget(cleanup: Promise): Promise { + let timer: ReturnType | undefined; + try { + await Promise.race([ + cleanup, + new Promise((resolve) => { + timer = setTimeout(resolve, HELP_CLEANUP_TIMEOUT_MS); + }), + ]); + } finally { + if (timer !== undefined) clearTimeout(timer); + } } async function finishHelp( @@ -316,7 +331,7 @@ async function finishHelp( for (const tabId of help.overlayTabIds) clearRearmTimer(tabId); if (notifyContent) { const cleanup = cleanupHelp(help); - if (waitForCleanup) await cleanup; + if (waitForCleanup) await awaitCleanupWithinBudget(cleanup); else void cleanup; } help.resolve(value);