From b7db48d5ce7a730e8a80dc2aae8c9c9dc0eff2a1 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Thu, 8 Oct 2026 16:51:43 +0200 Subject: [PATCH] fix: pin browser approvals to their page and withdraw them on stop Co-Authored-By: Claude Opus 5.5 --- browser_extension/src/panel.ts | 57 +++++++++++++------ .../copilot/chat/AIChatManager.svelte.ts | 19 ++++++- .../copilot/chat/global/browserTools.ts | 8 +++ .../components/sessions/SessionWrapper.svelte | 7 ++- .../(logged)/sessions/browser/+page.svelte | 15 +++-- 5 files changed, 84 insertions(+), 22 deletions(-) diff --git a/browser_extension/src/panel.ts b/browser_extension/src/panel.ts index 90b1abef37..b8320b07aa 100644 --- a/browser_extension/src/panel.ts +++ b/browser_extension/src/panel.ts @@ -30,9 +30,12 @@ async function main() { const reply = (m: object) => frame.contentWindow?.postMessage(m, origin) if (msg?.type === 'wm-browser:hello') { reply({ type: 'wm-browser:ready' }) + } else if (msg?.type === 'wm-browser:cancel') { + generation++ + declineOpen?.() } else if (msg?.type === 'wm-browser:call' && typeof msg.id === 'string') { try { - const result = await run({ tool: msg.tool, args: msg.args ?? {} } as Call) + const result = await run({ tool: msg.tool, args: msg.args ?? {} } as Call, generation) reply({ type: 'wm-browser:result', id: msg.id, ok: true, result }) } catch (err) { const error = err instanceof Error ? err.message : String(err) @@ -42,32 +45,32 @@ async function main() { }) } -async function run(call: Call): Promise { +async function run(call: Call, gen: number): Promise { const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }) if (tab?.id === undefined) throw new Error('No active tab') const tabId = tab.id switch (call.tool) { case 'read': - return exec(tabId, readPage, [30_000]) + return (await exec({ tabId }, readPage, [30_000]))[0] case 'screenshot': return chrome.tabs.captureVisibleTab(tab.windowId, { format: 'jpeg', quality: 80 }) case 'click': { - const label = await exec(tabId, pageAction, ['describe', call.args]) - await confirmAction(`Click ${label}`, tab) - return exec(tabId, pageAction, ['click', call.args]) + const [label, documentId] = await exec({ tabId }, pageAction, ['describe', call.args]) + await confirmAction(`Click ${label}`, tab, gen) + return execConfirmed(tabId, documentId, ['click', call.args]) } case 'type': { - const label = await exec(tabId, pageAction, ['describe', call.args]) + const [label, documentId] = await exec({ tabId }, pageAction, ['describe', call.args]) const submit = call.args.submit ? ' and submit' : '' - await confirmAction(`Type "${call.args.text}" into ${label}${submit}`, tab) - return exec(tabId, pageAction, ['type', call.args]) + await confirmAction(`Type "${call.args.text}" into ${label}${submit}`, tab, gen) + return execConfirmed(tabId, documentId, ['type', call.args]) } case 'navigate': { const url = new URL(call.args.url) if (url.protocol !== 'https:' && url.protocol !== 'http:') { throw new Error('Only http(s) URLs can be opened') } - await confirmAction(`Open ${url.href}`, tab) + await confirmAction(`Open ${url.href}`, tab, gen) await chrome.tabs.update(tabId, { url: url.href }) return `Navigating to ${url.href}` } @@ -79,23 +82,43 @@ async function run(call: Call): Promise { // Injected functions run in the page and must be self-contained; they report failure as // `{ error }` because a throw inside them reaches us without its message. async function exec( - tabId: number, + target: chrome.scripting.InjectionTarget, func: (...args: A) => { result?: unknown; error?: string }, args: A -): Promise { - const [injection] = await chrome.scripting.executeScript({ target: { tabId }, func, args }) +): Promise<[any, string]> { + const [injection] = await chrome.scripting.executeScript({ target, func, args }) const out = injection?.result as { result?: unknown; error?: string } | undefined if (!out) throw new Error('The page did not answer') if (out.error) throw new Error(out.error) - return out.result + return [out.result, injection.documentId] +} + +/** Pinned to the document the user confirmed: if the tab navigated meanwhile, this fails. */ +async function execConfirmed( + tabId: number, + documentId: string, + args: Parameters +): Promise { + const target = { tabId, documentIds: [documentId] } + const [result] = await exec(target, pageAction, args).catch((e: Error) => { + throw /No document with id/.test(e.message) + ? new Error('The page changed while waiting for approval. Read it again.') + : e + }) + return result } let confirmQueue: Promise = Promise.resolve() +// Bumped when the chat stops its turn: a confirmation asked before that is never shown, and +// the open one is declined. +let generation = 0 +let declineOpen: (() => void) | undefined /** Shown outside the Windmill frame, so neither the chat nor the page can answer it. */ -function confirmAction(text: string, tab: chrome.tabs.Tab): Promise { +function confirmAction(text: string, tab: chrome.tabs.Tab, gen: number): Promise { const ask = () => new Promise((resolve, reject) => { + if (gen !== generation) return reject(new Error('Stopped by the user')) const box = document.getElementById('confirm')! document.getElementById('confirm-text')!.textContent = text document.getElementById('confirm-host')!.textContent = `On ${tab.url ?? 'the active tab'}` @@ -105,12 +128,14 @@ function confirmAction(text: string, tab: chrome.tabs.Tab): Promise { const done = (ok: boolean) => { box.hidden = true allow.onclick = deny.onclick = null + declineOpen = undefined if (ok) resolve() else reject(new Error('The user declined this action')) } allow.onclick = () => done(true) deny.onclick = () => done(false) - allow.focus() + declineOpen = () => done(false) + // Nothing is focused: a keystroke meant for the chat composer must not approve. }) const next = confirmQueue.then(ask, ask) confirmQueue = next.catch(() => {}) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 00bfece649..47a9836112 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -133,6 +133,7 @@ import { } from '$lib/aiStore' import type { WorkspaceMutationTarget } from './workspaceTools' import { fullSessionAccess, resolveSessionAccess } from './global/sessionAccess' +import { cancelBrowserCalls } from './global/browserTools' import type { SessionAccess } from './sessionCapabilities' import { filterSessionTools } from './global/sessionToolset' import { @@ -736,7 +737,10 @@ export class AIChatManager implements ChatViewHost { (this.autonomyMode === AIAutonomyMode.ACCEPT_EDIT || this.autonomyMode === AIAutonomyMode.YOLO) ) - autoAcceptToolConfirmationsAvailable = $derived(supportsAutoAcceptToolConfirmations(this.mode)) + // Never in the side panel: page content in context could steer an unconfirmed workspace tool. + autoAcceptToolConfirmationsAvailable = $derived( + !this.browserTools && supportsAutoAcceptToolConfirmations(this.mode) + ) autoAcceptToolConfirmationsActive = $derived( this.autonomyMode === AIAutonomyMode.YOLO && this.autoAcceptToolConfirmationsAvailable ) @@ -2702,6 +2706,18 @@ export class AIChatManager implements ChatViewHost { } } + /** For a host that renders no preview pane: forms stay inline and runs open in a new tab, + * as in the docked chat. */ + detachPreview = () => { + this.openRunInPreview = undefined + this.openRunForm = undefined + this.closeRunForm = undefined + this.showRunInPlaceOfForm = undefined + this.isRunFormInPreview = undefined + this.openArtifact = undefined + this.closeArtifact = undefined + } + enableBrowserTools = () => { this.browserTools = true if (this.mode === AIMode.GLOBAL) { @@ -4485,6 +4501,7 @@ export class AIChatManager implements ChatViewHost { }) this.abortController?.abort(cancelReason) this.cancelLoadingTools() + if (this.browserTools) cancelBrowserCalls() } cancelInlineRequest = (reason?: string) => { diff --git a/frontend/src/lib/components/copilot/chat/global/browserTools.ts b/frontend/src/lib/components/copilot/chat/global/browserTools.ts index bef9fc14bc..a3f4000895 100644 --- a/frontend/src/lib/components/copilot/chat/global/browserTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/browserTools.ts @@ -42,6 +42,14 @@ export function connectBrowserBridge(): Promise { return connection } +/** Settles every pending call and withdraws the extension's open confirmations, so a stopped + * turn leaves no approval behind that could still act on the page. */ +export function cancelBrowserCalls() { + for (const p of pending.values()) p.reject(new Error('Stopped by the user')) + pending.clear() + if (parentOrigin) window.parent.postMessage({ type: 'wm-browser:cancel' }, parentOrigin) +} + function callBrowser(tool: BrowserToolName, args: Record): Promise { if (!parentOrigin) return Promise.reject(new Error('The browser extension is not connected.')) const id = randomUUID() diff --git a/frontend/src/lib/components/sessions/SessionWrapper.svelte b/frontend/src/lib/components/sessions/SessionWrapper.svelte index af00454f81..c47de7f340 100644 --- a/frontend/src/lib/components/sessions/SessionWrapper.svelte +++ b/frontend/src/lib/components/sessions/SessionWrapper.svelte @@ -53,11 +53,15 @@ let { sessionId, - headerInPage = false + headerInPage = false, + onNewSession = undefined }: { sessionId: string /** True for the session the page is showing: its header rides the page header bar. */ headerInPage?: boolean + /** Shows the fresh session that replaces an archived or deleted one, for a host that is + * not the sessions page. */ + onNewSession?: (id: string) => void } = $props() // Parent keys by sessionId; this wrapper only mounts when the session exists. @@ -132,6 +136,7 @@ // usable, and routing to a sibling would feel arbitrary. async function resetToNewSession() { const fresh = createSession() + if (onNewSession) return onNewSession(fresh.id) selectSession(fresh.id) // The page derives the visible session from the `session` query, not // currentSessionId: navigate so the URL leaves the deleted/archived session, diff --git a/frontend/src/routes/(root)/(logged)/sessions/browser/+page.svelte b/frontend/src/routes/(root)/(logged)/sessions/browser/+page.svelte index b59654ddf8..ce483f7cd5 100644 --- a/frontend/src/routes/(root)/(logged)/sessions/browser/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/sessions/browser/+page.svelte @@ -7,6 +7,7 @@ import { createSession, findEmptyLandingSession, + isTearingDownOpenSession, selectSession, sessionLastActivityAt, sessionPageHref, @@ -34,13 +35,19 @@ } $effect(() => { - if (!sessionState.hydrated || session) return + // A delete awaiting its fork's removal opens the fresh session itself (onNewSession). + if (!sessionState.hydrated || session || isTearingDownOpenSession()) return untrack(() => open((findEmptyLandingSession() ?? createSession()).id)) }) $effect(() => { - if (!session || !bridgeConnected) return - untrack(() => getOrCreateRuntime(session).manager.enableBrowserTools()) + if (!session) return + const connected = bridgeConnected + untrack(() => { + const manager = getOrCreateRuntime(session).manager + manager.detachPreview() + if (connected) manager.enableBrowserTools() + }) }) const recentSessions = $derived( @@ -87,7 +94,7 @@ {#if sessionId} {#key sessionId} - + {/key} {/if} {/if}