From 4733c1a265f7f87d925c1b3dbafeeaebd987fb02 Mon Sep 17 00:00:00 2001 From: Ho Date: Tue, 27 Oct 2026 02:28:13 +0800 Subject: [PATCH] feat(desktop): verify preview open and page load end to end Generated-by: OpenAI Codex --- apps/desktop/README.md | 21 ++ apps/desktop/package.json | 1 + .../artifact-preview-verification-smoke.mjs | 292 ++++++++++++++++++ .../__tests__/browser-document-load.test.ts | 98 ++++++ .../src/main/__tests__/browser-tools.test.ts | 284 ++++++++++++++++- apps/desktop/src/main/browser/browser-host.ts | 10 + .../desktop/src/main/browser/browser-tools.ts | 204 ++++++++---- .../src/main/browser/browser-verification.ts | 211 +++++++++++++ apps/desktop/src/main/browser/controller.ts | 44 ++- .../desktop/src/main/browser/document-load.ts | 75 +++++ apps/desktop/src/main/preview-preflight.ts | 114 ++++--- knip.json | 1 + 12 files changed, 1218 insertions(+), 137 deletions(-) create mode 100644 apps/desktop/scripts/artifact-preview-verification-smoke.mjs create mode 100644 apps/desktop/src/main/__tests__/browser-document-load.test.ts create mode 100644 apps/desktop/src/main/browser/browser-verification.ts create mode 100644 apps/desktop/src/main/browser/document-load.ts diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 9c834ae9ef1..0a3ae6a8979 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -99,13 +99,34 @@ the resulting URL belongs to the Desktop machine, not the Host's localhost. launch success also does not prove page load; browser observation is required before reporting that the page loaded or its interaction worked (#5235). +`browser_navigate`, `browser_snapshot` and `browser_click` return structured +evidence through the existing Client Capability result. `status` distinguishes +completed actions, failures, timeouts, unsupported surfaces, unknown observations +and invalidated evidence. `opened`/`loaded` describe the observed browser document; +they require a completed load event for the current document, not just a stopped +spinner or readable DOM. A truncated response fails verification; stopping a +pending load invalidates its evidence. Partial pages remain usable in the UI. +These results do not verify the intended content. Check the snapshot title and requested +marker before reporting a preview verified. A click reports matches with +`interaction: not_verified`: compare the requested change in a subsequent +snapshot with the same `documentId`. Navigation or view replacement invalidates +that comparison. Observed URLs omit credentials, query and fragment, and managed +preview bearer paths are redacted. No observed URL is synthesized from a request. + Focused regression checks (build workspace dependencies first): ```sh npm run build:main --workspace apps/desktop node --test apps/desktop/dist/main/__tests__/managed-artifact-preview.test.js apps/desktop/dist/main/__tests__/runtime-host-artifacts-ipc-main.test.js +npm --workspace @maka/desktop run smoke:preview:run ``` +The preview smoke uses a temporary Electron profile, actual managed previews, +production browser tools and real Chromium JavaScript/clicks. It covers the +title/marker/counter workflow, revoked/expired URLs, an unreachable endpoint, +HTTP errors, truncated responses, stopped loads and an unfinished load. Artifact storage reads are fixture data; +this focused smoke does not replace a model-driven `dev:worktree` check. + ## Worktree development profiles Run `npm run dev:worktree` from the repository root to start desktop HMR with diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 1920c41934d..82c7beb5e63 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -56,6 +56,7 @@ "smoke:browser": "npm run build:workspace-deps && npm run build:main && npm run smoke:browser:run", "smoke:workhub-browser:run": "electron scripts/workhub-browser-presentation-smoke.mjs", "smoke:browser:run": "electron scripts/browser-observe-act-smoke.mjs", + "smoke:preview:run": "electron scripts/artifact-preview-verification-smoke.mjs", "smoke:browser-load-failure:run": "electron scripts/browser-load-failure-smoke.mjs", "build:assistant-web": "vite build --config vite.assistant-web.config.ts", "typecheck:assistant-web": "tsc -p tsconfig.assistant-web.json --noEmit" diff --git a/apps/desktop/scripts/artifact-preview-verification-smoke.mjs b/apps/desktop/scripts/artifact-preview-verification-smoke.mjs new file mode 100644 index 00000000000..7fbe6fe34f6 --- /dev/null +++ b/apps/desktop/scripts/artifact-preview-verification-smoke.mjs @@ -0,0 +1,292 @@ +#!/usr/bin/env electron +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/** + * #5235: the Electron boundary is the real WebContents load/error lifecycle, + * the sealed CDP bridge, sandboxed ArtifactPreview scripts and native click. + * Fake IPage tests cannot establish that Chromium ran the fixture JavaScript. + * Artifact storage reads are fixtures; preview serving and all browser tools + * below are production implementations. This is not a model/Dev E2E claim. + */ +import assert from 'node:assert/strict'; +import { mkdtempSync } from 'node:fs'; +import { rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createServer } from 'node:http'; +import { app, BrowserWindow } from 'electron'; +import { ManagedArtifactPreview } from '../dist/main/managed-artifact-preview.js'; +import { buildManagedArtifactPreviewTools } from '../dist/main/managed-artifact-preview-tools.js'; +import { + buildBrowserNavigateTool, + buildBrowserSnapshotTool, + buildBrowserClickTool, +} from '../dist/main/browser/browser-tools.js'; +import { BrowserViewController } from '../dist/main/browser/controller.js'; +import { BrowserViewManager } from '../dist/main/browser/view-manager.js'; +import { createBrowserViewHost } from '../dist/main/browser/automation-host.js'; +import { provideBrowserViewHost } from '../dist/main/browser/browser-host.js'; +import { withBrowserOriginAdmission } from '../dist/main/browser/browser-origin-admission.js'; +import { releaseBrowserSession } from '../dist/main/browser/session.js'; +import { runPreviewPreflight } from '../dist/main/preview-preflight.js'; +import { createPreviewPreflightAuthority } from '../dist/main/preview-preflight-probes.js'; +import { redactPreviewCapabilities } from '../dist/main/browser/browser-verification.js'; + +// Electron's default loadURL warning contains the bearer path. Preserve the +// warning in this isolated test process, but never print the capability. +process.removeAllListeners('warning'); +process.on('warning', (warning) => console.error(redactPreviewCapabilities(warning.message))); + +const html = `Preview 5235 +

PREVIEW_READY_5235

`; +const bytes = Buffer.from(html); +const client = { + getArtifact: async () => ({ + id: 'a1', + sessionId: 's1', + turnId: 't1', + createdAt: 0, + name: 'preview-5235.html', + kind: 'html', + sizeBytes: bytes.length, + source: 'tool_result', + }), + streamArtifact: async (_session, _artifact, write) => { + await write(bytes); + return bytes.length; + }, +}; +const userData = mkdtempSync(join(tmpdir(), 'maka-preview-5235-')); +app.setPath('userData', userData); +app.on('window-all-closed', () => {}); +const watchdog = setTimeout(() => { + console.error('FAIL preview smoke deadline'); + app.exit(1); +}, 150_000); +const log = (name) => console.log(`PASS ${name}`); +let sequence = 0; +function context(sessionId = 's1') { + return { + sessionId, + turnId: 't1', + toolCallId: `c${++sequence}`, + cwd: userData, + abortSignal: new AbortController().signal, + emitOutput() {}, + }; +} +function browserCall(tool, args, approvedUrl, sessionId = 's1') { + return withBrowserOriginAdmission({ sessionId, url: approvedUrl }, () => + tool.impl(args, context(sessionId)), + ); +} +function assertNotOpened(result, status = 'failed') { + assert.equal(result.status, status); + assert.equal(result.opened, false); + assert.equal(result.loaded, false); + assert.equal(result.url, null); + assert.equal(result.title, null); + assert.equal(result.documentId, undefined); +} + +async function main() { + let code = 1; + await app.whenReady(); + console.log('Running managed preview / Chromium regression checks'); + const win = new BrowserWindow({ show: false, width: 900, height: 650 }); + win.showInactive(); + const manager = new BrowserViewManager({ + create: (id) => new BrowserViewController(win.contentView, id, () => {}), + }); + provideBrowserViewHost(createBrowserViewHost(manager, () => true)); + const previews = new ManagedArtifactPreview(); + const expiring = new ManagedArtifactPreview(100); + const sockets = new Set(); + const slowServer = createServer((req, res) => { + if (req.url === '/partial' || req.url === '/stop') { + res.writeHead(200, { 'content-type': 'text/html', 'content-length': '100000' }); + res.write('Partial document'); + const timer = setTimeout(() => { + if (req.url === '/partial') res.end(); + else manager.get('s1').stop(); + }, 250); + res.on('close', () => clearTimeout(timer)); + return; + } + res.writeHead(200, { 'content-type': 'text/html' }); + res.write('Still loading

Unfinished body'); + }); + slowServer.on('connection', (socket) => { + sockets.add(socket); + socket.on('close', () => sockets.delete(socket)); + }); + try { + const [artifactPreview] = buildManagedArtifactPreviewTools((sessionId, artifactId, signal) => + previews.prepare('smoke', client, sessionId, artifactId, signal), + ); + const endpoint = await artifactPreview.impl({ artifactId: 'a1' }, context()); + assert.equal(endpoint.reachable, true); + assert.equal(endpoint.loaded, false); + const navigate = buildBrowserNavigateTool(); + const snapshot = buildBrowserSnapshotTool(); + const click = buildBrowserClickTool(); + const navigation = await browserCall(navigate, { url: endpoint.url }, endpoint.url); + assert.equal(navigation.status, 'completed', JSON.stringify(navigation)); + assert.equal(navigation.loaded, true); + assert.equal(navigation.title, 'Preview 5235'); + assert.equal(navigation.matchesRequestedUrl, true); + manager.get('s1').setViewport({ x: 0, y: 0, width: 900, height: 650 }); + const before = await browserCall(snapshot, {}, endpoint.url); + assert.equal(before.status, 'completed', JSON.stringify(before)); + assert.equal(before.title, 'Preview 5235'); + assert.match(before.snapshot, /PREVIEW_READY_5235/); + assert.match(before.snapshot, /]*>0<\/button>/); + const ref = before.snapshot + .split('\n') + .find((line) => /]*>0<\/button>/.test(line)) + ?.match(/\[(\d+)\]/)?.[1]; + assert.ok(ref, 'snapshot must expose a real counter ref'); + const action = await browserCall(click, { ref }, endpoint.url); + assert.equal(action.status, 'completed', JSON.stringify(action)); + assert.equal(action.click.interaction, 'not_verified'); + const after = await browserCall(snapshot, {}, endpoint.url); + assert.equal(after.status, 'completed', JSON.stringify(after)); + assert.equal(after.documentId, before.documentId); + assert.equal(before.documentId, navigation.documentId); + assert.match(after.snapshot, /PREVIEW_READY_5235/); + assert.match(after.snapshot, /]*>1<\/button>/); + assert.doesNotMatch( + JSON.stringify([navigation, before, action, after]), + new RegExp(new URL(endpoint.url).pathname), + ); + log( + 'ArtifactPreview → navigate → snapshot → click → snapshot; title, marker, real JavaScript 0 → 1', + ); + + const preflight = await runPreviewPreflight(createPreviewPreflightAuthority(), { + sessionId: 's1', + origin: new URL(endpoint.url).origin, + abortSignal: new AbortController().signal, + }); + assert.equal(preflight.endpoint, 'verified'); // HEAD / returns 404, still only a listener. + assert.equal(preflight.capabilities.find((c) => c.id === 'url_schemes').status, 'unsupported'); + const missing = new URL('missing', endpoint.url).href; + assertNotOpened(await browserCall(navigate, { url: missing }, missing)); + log('listener verified and file unsupported do not turn an HTTP 404 into preview success'); + + await previews.releaseUrl(endpoint.url); + assertNotOpened(await browserCall(navigate, { url: endpoint.url }, endpoint.url)); + log('revoked preview fails without opened/loaded or old document evidence'); + + const expired = await expiring.prepare('expiry', client, 's1', 'a1'); + await new Promise((resolve) => setTimeout(resolve, 150)); + assertNotOpened(await browserCall(navigate, { url: expired.url }, expired.url)); + log('expired preview fails without opened/loaded'); + + // Reserve and release an OS-selected port, then immediately navigate to it. + const reservation = createServer(); + await new Promise((resolve) => reservation.listen(0, '127.0.0.1', resolve)); + const unreachable = `http://127.0.0.1:${reservation.address().port}/unreachable`; + await new Promise((resolve) => reservation.close(resolve)); + assertNotOpened(await browserCall(navigate, { url: unreachable }, unreachable)); + log('unreachable endpoint fails'); + + await new Promise((resolve) => slowServer.listen(0, '127.0.0.1', resolve)); + const failureResults = []; + for (const [path, expectedStatus] of [ + ['partial', 'failed'], + ['stop', 'invalidated'], + ]) { + const url = `http://127.0.0.1:${slowServer.address().port}/${path}`; + const view = manager.get('s1'); + const errors = []; + const wc = win.contentView.children[0].webContents; + const recordFailure = (_event, code, _description, failedUrl, mainFrame) => { + if (mainFrame && failedUrl === url) errors.push(code); + }; + wc.on('did-fail-load', recordFailure); + let result; + try { + result = await browserCall(navigate, { url }, url); + } finally { + wc.off('did-fail-load', recordFailure); + } + assert.ok( + errors.includes(path === 'partial' ? -354 : -3), + 'real main-frame load failure must occur', + ); + // A readable, committed DOM must not substitute for a completed load. + assert.equal(view.state().loadError, null, 'keep the existing partial-page UI usable'); + failureResults.push({ path, expectedStatus, result }); + } + const incorrect = failureResults.filter( + ({ expectedStatus, result }) => + result.status !== expectedStatus || result.loaded || result.opened, + ); + assert.deepEqual( + incorrect.map(({ path, result }) => ({ path, status: result.status, loaded: result.loaded })), + [], + ); + for (const { path, expectedStatus, result } of failureResults) { + assertNotOpened(result, expectedStatus); + log(`${path}: committed interrupted document never reports loaded`); + } + const slowUrl = `http://127.0.0.1:${slowServer.address().port}/slow`; + const started = Date.now(); + assertNotOpened(await browserCall(navigate, { url: slowUrl }, slowUrl), 'timeout'); + assert.ok( + Date.now() - started < 40_000, + 'existing navigation deadline must bound an unfinished document', + ); + log('never-completing real page returns bounded timeout'); + + await releaseBrowserSession('s1'); + const unobserved = await browserCall(snapshot, {}, new URL(slowUrl).origin); + assert.notEqual(unobserved.status, 'completed'); + assert.equal(unobserved.documentId, undefined); + const replacement = await previews.prepare('smoke', client, 's1', 'a1'); + const restored = await browserCall(navigate, { url: replacement.url }, replacement.url); + assert.equal(restored.status, 'completed', JSON.stringify(restored)); + assert.notEqual(restored.documentId, before.documentId); + log('closed/recreated page cannot reuse prior evidence'); + code = 0; + } catch (error) { + // Tool results are already sanitized. Avoid dumping raw CDP URLs or stacks. + console.error( + `FAIL preview verification: ${String(error.message).replace(/\/[a-f0-9]{64}\/index\.html/gi, '/[preview-token]/index.html')}`, + ); + } finally { + clearTimeout(watchdog); + await previews.close(); + await expiring.close(); + for (const socket of sockets) socket.destroy(); + if (slowServer.listening) await new Promise((resolve) => slowServer.close(resolve)); + await releaseBrowserSession('s1'); + await manager.disposeAll(); + provideBrowserViewHost(null); + win.destroy(); + await rm(userData, { recursive: true, force: true, maxRetries: 3 }); + app.exit(code); + } +} +// Do not await app readiness at ESM top level: Electron must finish loading the entry first. +main().catch(() => { + console.error('FAIL preview smoke setup'); + app.exit(1); +}); diff --git a/apps/desktop/src/main/__tests__/browser-document-load.test.ts b/apps/desktop/src/main/__tests__/browser-document-load.test.ts new file mode 100644 index 00000000000..22f6f36371e --- /dev/null +++ b/apps/desktop/src/main/__tests__/browser-document-load.test.ts @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { BrowserDocumentLoad } from '../browser/document-load.js'; + +const url = 'https://example.com/preview'; + +test('completion needs a commit in this navigation, including same-URL reloads', () => { + const load = new BrowserDocumentLoad(); + assert.equal(load.snapshot().state, 'unknown'); + load.start(url); + load.commit(url); + load.finish(url); + const previous = load.snapshot(); + assert.equal(previous.state, 'completed'); + load.start(url); + load.finish(url); // The previous document cannot complete a pending reload. + assert.equal(load.snapshot().state, 'loading'); + assert.notEqual(load.snapshot().navigationId, previous.navigationId); + assert.equal(previous.state, 'completed', 'published snapshots remain immutable'); + load.commit(url); + load.finish(url); + assert.equal(load.snapshot().state, 'completed'); +}); + +for (const code of [-354, -3]) { + test(`a committed failure ${code} survives finish and same-document events`, () => { + const load = new BrowserDocumentLoad(); + load.start(url); + load.commit(url); + load.fail(url, code); + load.inPage(`${url}#hash`); + load.finish(`${url}#hash`); + assert.equal(load.snapshot().state, code === -3 ? 'stopped' : 'failed'); + assert.equal(load.snapshot().errorCode, code); + }); +} + +test('stop invalidates a pending load but does not revoke an already completed load', () => { + const load = new BrowserDocumentLoad(); + load.start(url); + load.commit(url); + load.stop(); + load.finish(url); + assert.equal(load.snapshot().state, 'stopped'); + load.start(url); + load.commit(url); + load.finish(url); + load.stop(); + assert.equal(load.snapshot().state, 'completed'); +}); + +test('superseding navigation and redirects cannot reuse old completion or failures', () => { + const load = new BrowserDocumentLoad(); + load.start(url); + load.commit(url); + load.start(`${url}/next`); + load.fail(url, -3); + load.finish(url); + load.redirect(`${url}/final`); + load.commit(`${url}/final`); + load.finish(`${url}/next`); + assert.equal(load.snapshot().state, 'loading'); + load.finish(`${url}/final`); + assert.equal(load.snapshot().state, 'completed'); + load.fail(url, -354); + assert.equal(load.snapshot().state, 'completed'); +}); + +test('same-document navigation preserves the outcome without creating completion', () => { + const load = new BrowserDocumentLoad(); + load.start(url); + load.commit(url); + load.inPage(`${url}#pending`); + assert.equal(load.snapshot().state, 'loading'); + load.finish(`${url}#pending`); + const complete = load.snapshot(); + load.inPage(`${url}#complete`); + assert.deepEqual(load.snapshot(), complete); +}); diff --git a/apps/desktop/src/main/__tests__/browser-tools.test.ts b/apps/desktop/src/main/__tests__/browser-tools.test.ts index 8af1becfe21..8cf5ea0aab3 100644 --- a/apps/desktop/src/main/__tests__/browser-tools.test.ts +++ b/apps/desktop/src/main/__tests__/browser-tools.test.ts @@ -26,6 +26,7 @@ import { strict as assert } from 'node:assert'; import { afterEach, describe, it } from 'node:test'; import type { IPage } from '@jackwener/opencli/types'; +import type { BrowserVerificationResult } from '../browser/browser-verification.js'; import type { ComputerUseToolSet } from '@maka/runtime/computer-use-tools'; import type { MakaTool, MakaToolContext } from '@maka/runtime/tool-runtime'; import { withBrowserOriginAdmission } from '../browser/browser-origin-admission.js'; @@ -40,7 +41,7 @@ import { readHtmlJs, takeoverNote, } from '../browser/browser-tools.js'; -import { type BridgeLike, resetBrowserSessionsForTest, setBridgeFactoryForTest } from '../browser/session.js'; +import { type BridgeLike, resetBrowserSessionsForTest, revokeHiddenBrowserActions, setBridgeFactoryForTest } from '../browser/session.js'; import { createDesktopNativeCapabilityProvider } from '../runtime-host-native-capabilities.js'; import { type BrowserViewHost, @@ -54,6 +55,14 @@ type FakePageConfig = { afterClickUrl?: string; afterFillUrl?: string; title?: string; + locationError?: string; + loading?: boolean; + alive?: boolean; + loadError?: { code: number }; + httpStatus?: number; + load?: { navigationId: number; state: 'unknown' | 'loading' | 'completed' | 'failed' | 'stopped'; errorCode?: number }; + gotoImpl?: (browser: FakeBrowser, url: string) => Promise; + evaluateImpl?: (browser: FakeBrowser) => void; click?: { matches_n: number; match_level: 'exact' | 'stable' | 'reidentified' }; fill?: { verified: boolean; actual: string; match_level: 'exact' | 'stable' | 'reidentified' }; snapshot?: unknown; @@ -91,9 +100,13 @@ function createFakeBrowser(url: string): FakeBrowser { function makeFakePage(cfg: FakePageConfig, browser: FakeBrowser): IPage { return { getCurrentUrl: async () => browser.url || null, - goto: async (url: string) => browser.navigate(cfg.afterGotoUrl ?? url), + goto: async (url: string) => cfg.gotoImpl ? cfg.gotoImpl(browser, url) : browser.navigate(cfg.afterGotoUrl ?? url), evaluate: async (js: string) => { - if (js.includes('location.href')) return browser.url as never; + if (js.includes('location.href')) { + if (cfg.locationError) throw new Error(cfg.locationError); + cfg.evaluateImpl?.(browser); + return (js.includes('document.title') ? { url: browser.url, title: cfg.title ?? '' } : browser.url) as never; + } if (js.includes('document.title')) return (cfg.title ?? '') as never; if (js.includes('outerHTML')) { cfg.extractImpl?.(browser); @@ -152,7 +165,11 @@ function install(cfg: FakePageConfig): FakeBrowser { openOriginLease: (_sessionId, approvedUrl, kind) => { const lease = originLeases.open(approvedUrl, kind); cfg.onLeaseOpened?.(browser); - return lease; + return { ...lease, snapshot: () => ({ ...lease.snapshot(), document: { + viewId: 'view-1', alive: cfg.alive ?? true, loading: cfg.loading ?? false, + loadError: cfg.loadError ?? null, httpStatus: cfg.httpStatus ?? 200, + load: cfg.load ?? { navigationId: 1, state: 'completed' }, + } }) }; }, canDrive: () => true, resolveEndpoint: async (id) => ({ cdpEndpoint: `ws://127.0.0.1:1/${id}` }), @@ -177,10 +194,15 @@ function ctx(): MakaToolContext { }; } -function run

(tool: MakaTool, args: P): Promise { +async function run

(tool: MakaTool, args: P): Promise { + const result = await runRaw(tool, args); + return typeof result === 'string' ? result : result.summary; +} + +function runRaw(tool: MakaTool, args: P, signal?: AbortSignal): Promise { return withBrowserOriginAdmission( { sessionId: 's1', url: 'https://example.com/approved?secret=grant' }, - () => Promise.resolve(tool.impl(args, ctx())) as Promise, + () => Promise.resolve(tool.impl(args, { ...ctx(), ...(signal ? { abortSignal: signal } : {}) })), ); } @@ -206,6 +228,25 @@ describe('browser tool helpers', () => { }); describe('browser tool execution', () => { + it('does not report Loaded when observing the destination loses the connection', async () => { + install({ locationError: 'CDP connection closed' }); + const output = await run(buildBrowserNavigateTool(), { url: 'https://example.com/preview' }); + assert.doesNotMatch(output, /Loaded/); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/preview' }); + assert.equal(result.status, 'failed'); + assert.equal(result.loaded, false); + assert.equal(result.url, null); + }); + + it('discards a snapshot that spans a same-Origin navigation', async () => { + install({ snapshotImpl: (browser) => { + browser.navigate('https://example.com/replacement'); + return 'stale document evidence'; + } }); + const output = await run(buildBrowserSnapshotTool(), {}); + assert.doesNotMatch(output, /stale document evidence/); + }); + it('navigate rejects a non-web URL before connecting', async () => { install({}); await assert.rejects(run(buildBrowserNavigateTool(), { url: 'file:///etc/passwd' }), /Not a navigable URL/); @@ -292,15 +333,13 @@ describe('browser tool execution', () => { }, ); assert.equal(resolved, 2); - assert.deepEqual(result, { - outcome: 'success', - content: [ - { - type: 'text', - text: 'Navigated to https://other.example. Access to the new site requires approval on the next Browser call.', - }, - ], - }); + assert.equal(result.outcome, 'error'); + assert.deepEqual(result.content, []); + const evidence = result.structuredContent as BrowserVerificationResult; + assert.equal(evidence.status, 'invalidated'); + assert.equal(evidence.loaded, false); + assert.equal(evidence.url, null); + assert.equal(evidence.summary, 'Navigated to https://other.example. Access to the new site requires approval on the next Browser call.'); assert.doesNotMatch(JSON.stringify(result), /private snapshot|token/); }); @@ -400,6 +439,219 @@ describe('browser tool execution', () => { it('a tool fails with a clear message when no host is injected', async () => { // no install(): host stays null - await assert.rejects(run(buildBrowserSnapshotTool(), {}), /only available inside the desktop app/); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/' }); + assert.equal(result.status, 'unsupported'); + assert.equal(result.opened, false); + assert.equal(result.preflight?.surface, 'unsupported'); + }); +}); + +describe('structured browser verification', () => { + for (const failed of [false, true]) { + it(`publishes actual Browser result through Client Capability structuredContent (${failed ? 'failure' : 'success'})`, async () => { + install({ title: 'Preview 5235', ...(failed ? { httpStatus: 404 } : {}) }); + const computerUseTools = [] as unknown as ComputerUseToolSet; + computerUseTools.clearSession = () => undefined; + computerUseTools.sessionEvents = {} as ComputerUseToolSet['sessionEvents']; + const provider = createDesktopNativeCapabilityProvider({ + browserTools: [buildBrowserNavigateTool()], + resolveBrowserUrl: () => 'https://example.com/preview', + releaseBrowserSession() {}, computerUseTools, releaseDesktopInteractionSession() {}, + }); + const result = await provider.call!({ + kind: 'client.capability.call', invocationId: 'i1', registrationId: 'r1', + offerId: 'desktop_browser', serverId: 'desktop_browser', toolName: 'browser_navigate', + arguments: { url: 'https://example.com/preview' }, + sessionId: 's1', turnId: 't1', toolCallId: 'c1', cwd: '/tmp', + }, { + signal: new AbortController().signal, accept: async () => undefined, + requestInteraction: async () => assert.fail('Unexpected interaction'), + }); + assert.equal(result.outcome, failed ? 'error' : 'success'); + assert.deepEqual(result.content, []); + const evidence = result.structuredContent as BrowserVerificationResult; + assert.equal(evidence.kind, 'browser_verification'); + assert.equal(evidence.loaded, !failed); + assert.equal(evidence.title, failed ? null : 'Preview 5235'); + assert.equal(evidence.status, failed ? 'failed' : 'completed'); + }); + } + + it('returns load evidence and a stable document identity without exposing a preview capability', async () => { + const token = 'a'.repeat(64); + const url = `https://example.com/${token}/index.html?secret=private#fragment`; + install({ title: 'Preview 5235', snapshot: `PREVIEW_READY_5235 ${url} [1] button "0"` }); + const navigation = await runRaw(buildBrowserNavigateTool(), { url }); + assert.equal(navigation.status, 'completed'); + assert.equal(navigation.opened, true); + assert.equal(navigation.loaded, true); + assert.equal(navigation.title, 'Preview 5235'); + assert.equal(navigation.matchesRequestedUrl, true); + assert.equal(navigation.preflight?.surface, 'verified'); + assert.equal(navigation.preflight?.capabilities.find((c) => c.id === 'url_schemes')?.status, 'unsupported'); + const before = await runRaw(buildBrowserSnapshotTool(), {}); + const click = await runRaw(buildBrowserClickTool(), { ref: '[1]' }); + const after = await runRaw(buildBrowserSnapshotTool(), {}); + assert.equal(before.documentId, navigation.documentId); + assert.equal(after.documentId, before.documentId); + assert.equal(click.click?.interaction, 'not_verified'); + // No fake counter mutation: only a real browser may prove the HTML script worked. + assert.equal(before.snapshot, after.snapshot); + assert.doesNotMatch(JSON.stringify([navigation, before, click, after]), new RegExp(`${token}|secret=|private|fragment`)); + }); + + it('does not relabel an old page as a newly loaded target', async () => { + install({ gotoImpl: async () => {} }); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/new' }); + assert.equal(result.status, 'unknown'); + assert.equal(result.error?.code, 'no_document_commit'); + assert.equal(result.url, null); + assert.equal(result.opened, false); + }); + + for (const config of [{ loadError: { code: -102 } }, { httpStatus: 404 }, { httpStatus: 500 }]) { + it(`rejects controller load failure ${JSON.stringify(config)}`, async () => { + install(config); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/preview' }); + assert.equal(result.status, 'failed'); + assert.equal(result.error?.code, 'load_failed'); + assert.deepEqual(result.evidence, ['loadError' in config ? 'controller_load_error' : 'controller_http_status']); + assert.equal(result.opened, false); + assert.equal(result.loaded, false); + assert.equal(result.title, null); + assert.equal(buildBrowserNavigateTool().resultOutcome?.(result), 'error'); + }); + } + + it('reports the swallowed goto load-event timeout honestly when the controller is still loading', async () => { + install({ loading: true }); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/slow' }); + assert.equal(result.status, 'timeout'); + assert.equal(result.opened, false); + assert.equal(result.loaded, false); + }); + + for (const [state, status, errorCode] of [ + ['failed', 'failed', -354], ['stopped', 'invalidated', -3], ['unknown', 'unknown', undefined], + ['loading', 'unknown', undefined], + ] as const) { + for (const action of ['navigate', 'snapshot', 'click'] as const) { + it(`${action} rejects a readable non-loading document with ${state} load evidence`, async () => { + const browser = install({ loading: false, load: { navigationId: 2, state, errorCode } }); + const result = action === 'navigate' + ? await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/preview' }) + : action === 'snapshot' ? await runRaw(buildBrowserSnapshotTool(), {}) + : await runRaw(buildBrowserClickTool(), { ref: '1' }); + assert.equal(result.status, status); + assert.equal(result.opened, false); + assert.equal(result.loaded, false); + assert.equal(result.documentId, undefined); + assert.equal(browser.clicks, 0); + }); + } + } + + it('invalidates observation when a new navigation starts before committing', async () => { + const config: FakePageConfig = { evaluateImpl: () => { + config.load = { navigationId: 2, state: 'loading' }; + } }; + install(config); + const result = await runRaw(buildBrowserSnapshotTool(), {}); + assert.equal(result.status, 'invalidated'); + assert.equal(result.loaded, false); + assert.equal(result.snapshot, undefined); + }); + + it('discards a snapshot spanning an entire same-URL reload even before an epoch update', async () => { + const config: FakePageConfig = { snapshotImpl: () => { + config.load = { navigationId: 2, state: 'completed' }; + return 'old document snapshot'; + } }; + install(config); + const result = await runRaw(buildBrowserSnapshotTool(), {}); + assert.notEqual(result.status, 'completed'); + assert.equal(result.loaded, false); + assert.equal(result.snapshot, undefined); + }); + + it('bounds a goto that never settles using the existing session deadline', async (t) => { + t.mock.timers.enable({ apis: ['setTimeout'] }); + let started!: () => void; + const entered = new Promise((resolve) => { started = resolve; }); + install({ gotoImpl: async () => { started(); await new Promise(() => {}); } }); + const pending = runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/slow' }); + await entered; + t.mock.timers.tick(35_000); + const result = await pending; + assert.equal(result.status, 'timeout'); + assert.equal(result.loaded, false); + t.mock.timers.reset(); + }); + + it('preserves cancellation instead of reporting a verification finding', async () => { + let started!: () => void; + const entered = new Promise((resolve) => { started = resolve; }); + install({ gotoImpl: async () => { started(); await new Promise(() => {}); } }); + const controller = new AbortController(); + const pending = runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/slow' }, controller.signal); + await entered; + controller.abort(); + await assert.rejects(pending, /canceled/); + }); + + it('consumes unknown preflight reasons without attempting navigation', async () => { + let navigated = false; + install({ gotoImpl: async () => { navigated = true; } }); + const result = await runRaw(buildBrowserNavigateTool({ + guiSurfaceAvailable: () => true, + browserDrivable: () => false, + probeLoopback: () => assert.fail('Navigation does not introduce a listener probe'), + }), { url: 'https://example.com/' }); + assert.equal(result.status, 'unknown'); + assert.equal(result.preflight?.surface, 'unknown'); + assert.equal(result.opened, false); + assert.equal(navigated, false); + }); + + it('discards snapshot evidence when the page closes during observation', async () => { + const config: FakePageConfig = { snapshotImpl: () => { + config.alive = false; + return 'closed page evidence'; + } }; + install(config); + const result = await runRaw(buildBrowserSnapshotTool(), {}); + assert.equal(result.status, 'invalidated'); + assert.equal(result.snapshot, undefined); + assert.equal(result.documentId, undefined); + assert.equal(result.loaded, false); + }); + + it('invalidates navigation evidence when visibility admission is revoked', async () => { + let started!: () => void; + const entered = new Promise((resolve) => { started = resolve; }); + install({ gotoImpl: async () => { started(); await new Promise(() => {}); } }); + const pending = runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/slow' }); + await entered; + revokeHiddenBrowserActions(() => false); + const result = await pending; + assert.equal(result.status, 'invalidated'); + assert.equal(result.error?.code, 'view_revoked'); + assert.equal(result.documentId, undefined); + assert.equal(result.loaded, false); + }); + + it('marks an actual same-Origin redirect as observed rather than echoing the request', async () => { + install({ afterGotoUrl: 'https://example.com/final', title: 'Final' }); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/start' }); + assert.equal(result.url, 'https://example.com/final'); + assert.equal(result.matchesRequestedUrl, false); + }); + + it('does not export raw CDP errors containing a bearer URL', async () => { + const token = 'b'.repeat(64); + install({ locationError: `Error observing https://example.com/${token}/index.html?secret=private` }); + const result = await runRaw(buildBrowserNavigateTool(), { url: 'https://example.com/preview' }); + assert.equal(result.status, 'failed'); + assert.doesNotMatch(JSON.stringify(result), new RegExp(`${token}|private|secret=`)); }); }); diff --git a/apps/desktop/src/main/browser/browser-host.ts b/apps/desktop/src/main/browser/browser-host.ts index a2e50f13c7e..7de6aab13e4 100644 --- a/apps/desktop/src/main/browser/browser-host.ts +++ b/apps/desktop/src/main/browser/browser-host.ts @@ -31,12 +31,22 @@ * error normalization is needed (the controller throws CdpBridgeError directly). */ +import type { BrowserDocumentLoadSnapshot } from './document-load.js'; import type { BrowserActionKind } from './logic.js'; export interface BrowserOriginLeaseSnapshot { readonly epoch: number; readonly url: string; readonly violatedUrl?: string; + /** Existing controller load facts, scoped to this view (never a second load probe). */ + readonly document?: { + readonly viewId: string; + readonly alive: boolean; + readonly loading: boolean; + readonly load: BrowserDocumentLoadSnapshot; + readonly loadError: { readonly code: number } | null; + readonly httpStatus: number | null; + }; } export interface BrowserOriginLease { diff --git a/apps/desktop/src/main/browser/browser-tools.ts b/apps/desktop/src/main/browser/browser-tools.ts index 1997ba8bdcd..a843d2fe908 100644 --- a/apps/desktop/src/main/browser/browser-tools.ts +++ b/apps/desktop/src/main/browser/browser-tools.ts @@ -27,9 +27,17 @@ import { type BrowserPageRun, type TakeoverMode, browserAutomationAvailable, + BrowserActionBlockedError, + BrowserActionRevokedError, withBrowserPage, } from './session.js'; import { parseNavigable } from './logic.js'; +import { runPreviewPreflight, type PreviewPreflightAuthority } from '../preview-preflight.js'; +import { createPreviewPreflightAuthority } from '../preview-preflight-probes.js'; +import { + BrowserEvidenceError, type BrowserVerificationResult, + observeDocument, redactPreviewCapabilities, verificationFailure, +} from './browser-verification.js'; /** * Generic observe→act browser tools over opencli's numbered-ref model: @@ -45,8 +53,8 @@ import { parseNavigable } from './logic.js'; const BROWSER_TOOL_CATEGORY = 'browser' as const; -// Above opencli's internal 30s CDP guard so a slow load surfaces the CDP -// command timeout (which names the navigation) rather than our generic one. +// Preserve the existing action budget above opencli's 30s guard. Its goto can +// swallow a load-event timeout; controller observation must still reject success. const NAVIGATE_TIMEOUT_MS = 35_000; const MAX_WAIT_SECONDS = 120; /** Per-call markdown budget; long pages page through `start`/`next_start_char`. */ @@ -117,7 +125,10 @@ async function runBrowserAction(input: { assertOriginLease(lease); const value = await input.run(guardBrowserPage(page, lease), info); const finished = assertOriginLease(lease); - if ((input.takeover ?? 'observe') === 'mutate' && finished.epoch !== started.epoch) { + if ((input.takeover ?? 'observe') !== 'navigate' && ( + finished.epoch !== started.epoch || + finished.document?.load?.navigationId !== started.document?.load?.navigationId + )) { return { kind: 'navigated', url: finished.url, requiresApproval: false }; } return { kind: 'completed', value }; @@ -162,110 +173,171 @@ function guardBrowserPage(page: IPage, lease: BrowserOriginLease): IPage { }) as IPage; } -export function buildBrowserNavigateTool(): MakaTool<{ url: string }, string> { +/** Preserve Runtime cancellation; failures are explicit business results, not successful text. */ +async function verifiedAction( + action: BrowserVerificationResult['action'], + signal: AbortSignal, + run: () => Promise, +): Promise { + try { + const result = await run(); + signal.throwIfAborted(); + return result; + } catch (error) { + if (signal.aborted || (error instanceof Error && error.name === 'BrowserActionCanceledError')) throw error; + if (error instanceof BrowserActionBlockedError) { + return verificationFailure(action, new BrowserEvidenceError('unknown', 'view_unavailable', 'This conversation does not currently admit browser actions.')); + } + if (error instanceof BrowserActionRevokedError) { + return verificationFailure(action, new BrowserEvidenceError('invalidated', 'view_revoked', 'The conversation lost browser admission during the action.')); + } + return verificationFailure(action, error); + } +} + +function invalidatedResult( + action: BrowserVerificationResult['action'], + result: { url: string; requiresApproval: boolean }, +): BrowserVerificationResult { + return { + ...verificationFailure(action, new BrowserEvidenceError('invalidated', 'document_changed', 'The page navigated during the action; observe it again before verifying content or interaction.')), + summary: navigationResult(result.url, result.requiresApproval), + }; +} + +function verificationOutcome(result: BrowserVerificationResult) { + return result.status === 'completed' ? 'success' as const : 'error' as const; +} + +export function buildBrowserNavigateTool( + preflight: PreviewPreflightAuthority = createPreviewPreflightAuthority(), +): MakaTool<{ url: string }, BrowserVerificationResult> { return { name: 'browser_navigate', displayName: '浏览器导航', description: - 'Open a URL in the conversation\'s embedded browser. Pass a full http:// or https:// URL; other schemes are rejected. ' + - 'Returns the sanitized URL actually landed on (after redirects). Follow with browser_snapshot to see what is on the page.', + 'Open an http:// or https:// URL in the conversation\'s embedded browser. Returns structured opened/loaded evidence, sanitized observed URL, title and error. ' + + 'loaded confirms browser load, not the intended content. Follow with browser_snapshot and check the requested title/content before reporting a preview verified. Never infer interaction success from navigation or click alone.', parameters: z.object({ url: z.string().min(1).max(4000).describe('Full http:// or https:// URL to open. Other schemes are rejected.'), }), categoryHint: BROWSER_TOOL_CATEGORY, + resultOutcome: verificationOutcome, impl: async ({ url: rawUrl }, { sessionId, abortSignal }) => { - // Validate BEFORE goto: opencli's goto sends Page.navigate directly, - // bypassing the view's will-navigate guard, so the scheme check is here. const url = parseNavigable(rawUrl); - if (!url) { - throw new Error(`Not a navigable URL: ${JSON.stringify(rawUrl)}. Pass a full http:// or https:// URL.`); - } - const result = await runBrowserAction({ - sessionId, - label: 'navigate', - abortSignal, - timeoutMs: NAVIGATE_TIMEOUT_MS, - // goto re-commits the document with the stealth script, so this clears a - // pending takeover without an extra reload. - takeover: 'navigate', - run: async (page, info) => { - await page.goto(url, { waitUntil: 'load' }); - // Read the document's real location: goto caches the REQUESTED url, so - // getCurrentUrl() would just echo it back and a redirect would never - // be visible. - const landed = await page.evaluate('window.location.href').catch(() => url); - return { landed: typeof landed === 'string' && landed ? landed : url, info }; - }, + if (!url) throw new Error('Not a navigable URL. Pass a full http:// or https:// URL.'); + return verifiedAction('navigate', abortSignal, async () => { + // Consume #5234's classification. No extra endpoint probe: a listener is not load evidence. + const capabilities = await runPreviewPreflight(preflight, { sessionId, abortSignal }); + if (capabilities.surface !== 'verified') { + return { + ...verificationFailure('navigate', new BrowserEvidenceError(capabilities.surface, 'preview_surface', capabilities.summary)), + preflight: capabilities, + }; + } + const result = await runBrowserAction({ + sessionId, label: 'navigate', abortSignal, timeoutMs: NAVIGATE_TIMEOUT_MS, + takeover: 'navigate', + run: async (page, info) => { + const lease = info.originLease!; + const before = lease.snapshot(); + await page.goto(url, { waitUntil: 'load' }); + // opencli can resolve goto after a load-event timeout or Page.navigate errorText. + // The controller already owns the actual commit/error/loading facts. + const observation = await observeDocument(page, lease, true); + if (lease.snapshot().epoch === before.epoch) { + throw new BrowserEvidenceError('unknown', 'no_document_commit', 'Navigation did not confirm a new document; the previous page is not target evidence.'); + } + return { observation, info }; + }, + }); + if (result.kind === 'navigated') return invalidatedResult('navigate', result); + const { observation, info } = result.value; + return { + kind: 'browser_verification', action: 'navigate', status: 'completed', + opened: true, loaded: true, url: observation.url, title: observation.title, + documentId: observation.documentId, matchesRequestedUrl: observation.rawUrl === url, + evidence: ['controller_navigation_commit', 'controller_load_complete', 'dom_location_and_title'], + preflight: capabilities, + summary: `Loaded ${observation.url}. Observe the requested content before verifying the preview.` + takeoverNote(info), + }; }); - if (result.kind === 'navigated') return navigationResult(result.url, result.requiresApproval); - return `Loaded ${sanitizedPageUrl(result.value.landed)}` + takeoverNote(result.value.info); }, }; } -export function buildBrowserSnapshotTool(): MakaTool, string> { +export function buildBrowserSnapshotTool(): MakaTool, BrowserVerificationResult> { return { name: 'browser_snapshot', displayName: '浏览器快照', description: - 'Observe the current page as a list of interactive elements (links, buttons, inputs), each tagged with a `[N]` ' + - 'reference you pass to browser_click / browser_type. This is the primary way to see what is on the page before acting.', + 'Observe the current document as structured URL/title/load evidence and a snapshot with interactive `[N]` references for browser_click/browser_type. ' + + 'Compare the requested content and post-action state using observations with the same documentId. A click result alone never verifies an interaction.', parameters: z.object({}), categoryHint: BROWSER_TOOL_CATEGORY, - impl: async (_args, { sessionId, abortSignal }) => { + resultOutcome: verificationOutcome, + impl: async (_args, { sessionId, abortSignal }) => verifiedAction('snapshot', abortSignal, async () => { const result = await runBrowserAction({ - sessionId, - label: 'snapshot', - abortSignal, + sessionId, label: 'snapshot', abortSignal, run: async (page, info) => { + const observation = await observeDocument(page, info.originLease!); const snapshot = await page.snapshot({ interactive: true }); - const url = (await page.getCurrentUrl?.()) ?? ''; - return { snapshot, url, info }; + // Re-check controller facts after snapshot; the outer action fences the epoch too. + const after = await observeDocument(page, info.originLease!); + if (observation.documentId !== after.documentId) throw new BrowserEvidenceError('invalidated', 'document_changed', 'The document changed during snapshot.'); + return { snapshot, observation: after, info }; }, }); - if (result.kind === 'navigated') return navigationResult(result.url, result.requiresApproval); - const text = - typeof result.value.snapshot === 'string' - ? result.value.snapshot - : JSON.stringify(result.value.snapshot, null, 2); - return (result.value.url ? `${result.value.url}\n\n${text}` : text) + takeoverNote(result.value.info); - }, + if (result.kind === 'navigated') return invalidatedResult('snapshot', result); + const { snapshot, observation, info } = result.value; + const text = redactPreviewCapabilities( + (typeof snapshot === 'string' ? snapshot : JSON.stringify(snapshot, null, 2)) + .replaceAll(observation.rawUrl, observation.url), + ); + return { + kind: 'browser_verification', action: 'snapshot', status: 'completed', + opened: true, loaded: true, url: observation.url, title: observation.title, + documentId: observation.documentId, snapshot: text, + evidence: ['controller_load_complete', 'dom_location_and_title', 'browser_snapshot'], + summary: `${observation.url}\n\n${text}` + takeoverNote(info), + }; + }), }; } -export function buildBrowserClickTool(): MakaTool<{ ref: string }, string> { +export function buildBrowserClickTool(): MakaTool<{ ref: string }, BrowserVerificationResult> { return { name: 'browser_click', displayName: '浏览器点击', description: - 'Click an element by its browser_snapshot reference (like "[12]") or a CSS selector. ' + - 'Reports how many elements matched and the match confidence; re-snapshot if multiple matched.', + 'Click an element by its browser_snapshot reference (like "[12]") or a CSS selector. Returns match evidence, not interaction verification. ' + + 'Take another browser_snapshot and compare the requested change on the same documentId to verify the interaction.', parameters: z.object({ ref: z.string().min(1).max(2000).describe('Element reference from browser_snapshot (like "[12]") or a CSS selector.'), }), categoryHint: BROWSER_TOOL_CATEGORY, - impl: async ({ ref }, { sessionId, abortSignal }) => { + resultOutcome: verificationOutcome, + impl: async ({ ref }, { sessionId, abortSignal }) => verifiedAction('click', abortSignal, async () => { const result = await runBrowserAction({ - sessionId, - label: 'click', - abortSignal, - // A mutating action: harden a taken-over page (reload once) before clicking. - takeover: 'mutate', + sessionId, label: 'click', abortSignal, takeover: 'mutate', run: async (page, info) => { + await observeDocument(page, info.originLease!); const outcome = await page.click(normalizeElementRef(ref)); - return { outcome, info }; + const observation = await observeDocument(page, info.originLease!); + return { outcome, observation, info }; }, }); - if (result.kind === 'navigated') return navigationResult(result.url, result.requiresApproval); - const { matches_n, match_level } = result.value.outcome; - return ( - `Clicked ${ref} (matched ${matches_n} element${matches_n === 1 ? '' : 's'}, ${match_level} match).` + - (matches_n > 1 - ? ' Multiple matches — verify the right element reacted, or re-snapshot for a tighter ref.' - : '') + - takeoverNote(result.value.info) - ); - }, + if (result.kind === 'navigated') return invalidatedResult('click', result); + const { outcome, observation, info } = result.value; + return { + kind: 'browser_verification', action: 'click', status: 'completed', + opened: true, loaded: true, url: observation.url, title: observation.title, + documentId: observation.documentId, + click: { matches: outcome.matches_n, confidence: outcome.match_level, interaction: 'not_verified' }, + evidence: ['controller_load_complete', 'dom_location_and_title', 'browser_click_match'], + summary: `Clicked (matched ${outcome.matches_n} element(s), ${outcome.match_level} match). Verify the requested change with browser_snapshot.` + takeoverNote(info), + }; + }), }; } @@ -273,7 +345,7 @@ function sanitizedPageUrl(value: string): string { try { const url = new URL(value); if (url.protocol !== 'http:' && url.protocol !== 'https:') return 'an unapproved page'; - return `${url.origin}${url.pathname}`; + return redactPreviewCapabilities(`${url.origin}${url.pathname}`); } catch { return 'an unapproved page'; } diff --git a/apps/desktop/src/main/browser/browser-verification.ts b/apps/desktop/src/main/browser/browser-verification.ts new file mode 100644 index 00000000000..4246ceac7ce --- /dev/null +++ b/apps/desktop/src/main/browser/browser-verification.ts @@ -0,0 +1,211 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import type { IPage } from '@jackwener/opencli/types'; +import type { PreviewPreflightResult } from '../preview-preflight.js'; +import type { BrowserOriginLease } from './browser-host.js'; + +export interface BrowserVerificationResult { + readonly kind: 'browser_verification'; + readonly action: 'navigate' | 'snapshot' | 'click'; + readonly status: 'completed' | 'failed' | 'timeout' | 'unsupported' | 'unknown' | 'invalidated'; + readonly opened: boolean; + /** Browser load only; target content and requested interactions still need observation. */ + readonly loaded: boolean; + /** Observed URL, sanitized. Never populated from the requested URL. */ + readonly url: string | null; + readonly title: string | null; + readonly documentId?: string; + readonly matchesRequestedUrl?: boolean; + readonly snapshot?: string; + readonly click?: { + readonly matches: number; + readonly confidence: string; + readonly interaction: 'not_verified'; + }; + readonly evidence: readonly string[]; + readonly preflight?: PreviewPreflightResult; + readonly error?: { readonly code: string; readonly message: string }; + readonly summary: string; +} + +export class BrowserEvidenceError extends Error { + constructor( + readonly status: Exclude, + readonly code: string, + message: string, + readonly evidence: readonly string[] = [], + ) { + super(message); + } +} + +/** Managed preview paths are bearer capabilities, including when printed inside a snapshot. */ +export function redactPreviewCapabilities(value: string): string { + return value.replace(/\/[a-f0-9]{64}\/index\.html/gi, '/[preview-token]/index.html'); +} + +export function observedPageUrl(value: string): string { + const url = new URL(value); + return redactPreviewCapabilities(`${url.origin}${url.pathname}`); +} + +/** One DOM read, fenced by the existing page-host lease and controller load facts. */ +export async function observeDocument(page: IPage, lease: BrowserOriginLease, afterGoto = false) { + const before = lease.snapshot(); + const document = before.document; + if (!document) + throw new BrowserEvidenceError( + 'unknown', + 'unobserved', + 'Controller load evidence is unavailable.', + ); + if (!document.alive) + throw new BrowserEvidenceError('invalidated', 'page_closed', 'The browser page was closed.'); + if (document.load?.state === 'stopped') { + throw new BrowserEvidenceError( + 'invalidated', + 'load_stopped', + 'The document load was stopped before completion.', + ['controller_load_stopped'], + ); + } + if (document.load?.state === 'failed') { + throw new BrowserEvidenceError( + 'failed', + 'load_failed', + `The target document failed to load (browser error ${document.load.errorCode}).`, + ['controller_load_failure'], + ); + } + if (document.loadError) { + throw new BrowserEvidenceError( + 'failed', + 'load_failed', + `The target document failed to load (browser error ${document.loadError.code}).`, + ['controller_load_error'], + ); + } + if (document.httpStatus !== null && document.httpStatus >= 400) { + throw new BrowserEvidenceError( + 'failed', + 'load_failed', + `The target document returned HTTP ${document.httpStatus}.`, + ['controller_http_status'], + ); + } + if (document.loading) + throw new BrowserEvidenceError( + afterGoto ? 'timeout' : 'unknown', + 'document_loading', + afterGoto + ? 'The document has not finished loading within the browser action budget.' + : 'The current document is still loading; no completed load was observed.', + ['controller_load_state'], + ); + if (document.load?.state !== 'completed') { + throw new BrowserEvidenceError( + 'unknown', + 'load_unobserved', + 'No completed load was observed for the current document.', + ['controller_load_state'], + ); + } + const observed = await page.evaluate<{ url: string; title: string }>( + '({ url: window.location.href, title: document.title })', + ); + const after = lease.snapshot(); + if ( + before.epoch !== after.epoch || + !after.document?.alive || + before.document?.viewId !== after.document.viewId || + document.load.navigationId !== after.document.load?.navigationId + ) { + throw new BrowserEvidenceError( + 'invalidated', + 'document_changed', + 'The document changed during observation.', + ); + } + if ( + !observed || + typeof observed.url !== 'string' || + typeof observed.title !== 'string' || + observed.url !== after.url || + !/^https?:\/\//.test(observed.url) + ) { + throw new BrowserEvidenceError( + 'unknown', + 'unobserved', + 'Could not confirm the current HTTP document location and title.', + ); + } + if ( + after.document.load?.state !== 'completed' || + after.document.loading || + after.document.loadError || + (after.document.httpStatus !== null && after.document.httpStatus >= 400) + ) { + throw new BrowserEvidenceError( + 'invalidated', + 'document_changed', + 'The document load state changed during observation.', + ); + } + return { + url: observedPageUrl(observed.url), + rawUrl: observed.url, + title: redactPreviewCapabilities(observed.title), + documentId: `${document.viewId}:${before.epoch}`, + }; +} + +export function verificationFailure( + action: BrowserVerificationResult['action'], + error: unknown, +): BrowserVerificationResult { + // Do not copy raw CDP error messages: they can contain the requested bearer URL. + const timeout = error instanceof Error && /timed?\s*out/i.test(error.message); + const status = + error instanceof BrowserEvidenceError ? error.status : timeout ? 'timeout' : 'failed'; + const code = + error instanceof BrowserEvidenceError + ? error.code + : timeout + ? 'timeout' + : 'browser_action_failed'; + const message = + error instanceof BrowserEvidenceError + ? error.message + : timeout + ? 'The browser action timed out.' + : 'The browser action failed; no document or interaction was verified.'; + return { + kind: 'browser_verification', + action, + status, + opened: false, + loaded: false, + url: null, + title: null, + evidence: error instanceof BrowserEvidenceError ? error.evidence : [], + error: { code, message }, + summary: message, + }; +} diff --git a/apps/desktop/src/main/browser/controller.ts b/apps/desktop/src/main/browser/controller.ts index 9330613eedc..a9f7a493dbe 100644 --- a/apps/desktop/src/main/browser/controller.ts +++ b/apps/desktop/src/main/browser/controller.ts @@ -17,9 +17,11 @@ * under the License. */ +import { randomUUID } from 'node:crypto'; import { BrowserWindow, type Session, shell, type View, WebContentsView } from 'electron'; import { CdpBridge, type AutomationEndpoint } from './cdp-bridge.js'; import type { BrowserActionLease, BrowserOriginLease } from './browser-host.js'; +import { BrowserDocumentLoad } from './document-load.js'; import { BrowserOriginLeaseTracker } from './browser-origin-lease.js'; import { browserViewWebPreferences } from './options.js'; import { @@ -62,6 +64,9 @@ export class BrowserViewController { private destroyed = false; private loadError: BrowserState['loadError'] = null; private navigationUrl: string | null = null; + private readonly viewId = randomUUID(); + private httpStatus: number | null = null; + private readonly documentLoad = new BrowserDocumentLoad(); /** True while the view holds real on-screen bounds (last setViewport painted it). */ private shownWithBounds = false; private automation: CdpBridge | null = null; @@ -118,22 +123,38 @@ export class BrowserViewController { wc.on('did-start-navigation', (details) => { if (!details.isMainFrame || details.isSameDocument) return; this.navigationUrl = details.url; + this.documentLoad.start(details.url); + this.httpStatus = null; this.emitState(); }); wc.on('did-redirect-navigation', (details) => { - if (details.isMainFrame) this.navigationUrl = details.url; + if (details.isMainFrame && !details.isSameDocument) { + this.navigationUrl = details.url; + this.documentLoad.redirect(details.url); + } }); wc.on('did-start-loading', () => this.emitState()); wc.on('did-stop-loading', () => this.emitState()); - wc.on('did-navigate', () => { + wc.on('did-navigate', (_event, url, httpResponseCode) => { + this.documentLoad.commit(url); // Stop, downloads and 204 responses never commit a new document. Keep // the previous failed address actionable until navigation really commits. this.loadError = null; + this.httpStatus = httpResponseCode; + this.recordNavigation(); + }); + wc.on('did-navigate-in-page', (_event, url, isMainFrame) => { + if (!isMainFrame) return; + this.documentLoad.inPage(url); this.recordNavigation(); }); - wc.on('did-navigate-in-page', () => this.recordNavigation()); + wc.on('did-finish-load', () => this.documentLoad.finish(wc.getURL())); + wc.on('did-fail-load', (_event, code, _description, url, isMainFrame) => { + if (isMainFrame) this.documentLoad.fail(url, code); + }); wc.on('page-title-updated', () => this.emitState()); wc.on('did-fail-provisional-load', (_event, code, _description, url, isMainFrame) => { + if (isMainFrame) this.documentLoad.fail(url, code); // A body transfer can fail after a usable document commits. Only a // provisional failure should replace the page with the retry UI. // Stop and superseding navigations report ERR_ABORTED (-3). A failed @@ -221,7 +242,21 @@ export class BrowserViewController { } openOriginLease(approvedUrl: string, kind: BrowserActionKind): BrowserOriginLease { - return this.originLeases.open(approvedUrl, kind); + const lease = this.originLeases.open(approvedUrl, kind); + return { + ...lease, + snapshot: () => ({ + ...lease.snapshot(), + document: { + viewId: this.viewId, + alive: !this.destroyed && !this.wc.isDestroyed(), + loading: !this.destroyed && !this.wc.isDestroyed() && this.wc.isLoading(), + loadError: this.loadError ? { code: this.loadError.code } : null, + httpStatus: this.httpStatus, + load: this.documentLoad.snapshot(), + }, + }), + }; } async navigate(input: string): Promise { @@ -249,6 +284,7 @@ export class BrowserViewController { } stop(): void { + this.documentLoad.stop(); this.wc.stop(); } diff --git a/apps/desktop/src/main/browser/document-load.ts b/apps/desktop/src/main/browser/document-load.ts new file mode 100644 index 00000000000..c5efd9436d5 --- /dev/null +++ b/apps/desktop/src/main/browser/document-load.ts @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** Controller-owned facts from Electron events; this never initiates or waits for a load. */ +export interface BrowserDocumentLoadSnapshot { + readonly navigationId: number; + readonly state: 'unknown' | 'loading' | 'completed' | 'failed' | 'stopped'; + readonly errorCode?: number; +} + +export class BrowserDocumentLoad { + private current: BrowserDocumentLoadSnapshot = { navigationId: 0, state: 'unknown' }; + private url = ''; + private committed = false; + + start(url: string): void { + this.current = { navigationId: this.current.navigationId + 1, state: 'loading' }; + this.url = url; + this.committed = false; + } + + redirect(url: string): void { + if (this.current.state === 'loading' && !this.committed) this.url = url; + } + + commit(url: string): void { + if (this.current.state === 'loading' && url === this.url) this.committed = true; + } + + inPage(url: string): void { + // A hash/history update changes the URL, not the document's load outcome. + if (this.committed) this.url = url; + } + + finish(url: string): void { + if (this.current.state === 'loading' && this.committed && url === this.url) { + this.current = { navigationId: this.current.navigationId, state: 'completed' }; + } + } + + fail(url: string, errorCode: number): void { + if (this.current.state !== 'loading' || url !== this.url) return; + this.current = { + navigationId: this.current.navigationId, + state: errorCode === -3 ? 'stopped' : 'failed', + errorCode, + }; + } + + stop(): void { + if (this.current.state === 'loading') { + this.current = { navigationId: this.current.navigationId, state: 'stopped', errorCode: -3 }; + } + } + + snapshot(): BrowserDocumentLoadSnapshot { + return this.current; + } +} diff --git a/apps/desktop/src/main/preview-preflight.ts b/apps/desktop/src/main/preview-preflight.ts index 018bd5182f6..856b35c4ed4 100644 --- a/apps/desktop/src/main/preview-preflight.ts +++ b/apps/desktop/src/main/preview-preflight.ts @@ -383,58 +383,70 @@ export function buildPreviewPreflightTools( // capability, whose descriptor carries neither, and the Runtime Host // assigns both itself. Declaring them here would only mislead a reader. impl: async ({ origin: rawOrigin }, { sessionId, abortSignal }) => { - abortSignal.throwIfAborted(); - // Validate before probing so a malformed origin is an argument error the - // caller can fix, not an `unknown` capability that reads like a finding. - const origin = rawOrigin === undefined ? undefined : assertLoopbackOrigin(rawOrigin); - - const guiSurface = classifyGuiSurface( - await observe(abortSignal, () => authority.guiSurfaceAvailable()), - ); - const urlSchemes = classifyUrlSchemes(probeNavigableSchemes()); - // Skip the drive check when there is demonstrably no view host: asking - // would throw, and an exception dressed as `unknown` would hide the - // settled `unsupported` answer the caller needs. Desktop, the only - // publisher today, registers its host before any offer is published, so - // this branch is unreachable there; it is kept because GUI availability - // is part of the classification #5235 builds on, and that contract must - // answer correctly for a publisher without a view host rather than throw. - const browserView = - guiSurface.status === 'unsupported' - ? BROWSER_VIEW_WITHOUT_HOST - : classifyBrowserView( - await observe(abortSignal, () => - authority.browserDrivable({ sessionId, signal: abortSignal }), - ), - ); - const loopbackEndpoint = - origin === undefined - ? undefined - : classifyLoopbackEndpoint({ - origin, - observed: await observe(abortSignal, () => - authority.probeLoopback({ origin, signal: abortSignal }), - ), - }); - - const capabilities = [ - guiSurface, - urlSchemes, - browserView, - ...(loopbackEndpoint ? [loopbackEndpoint] : []), - ]; - const surface = derivePreviewSurface({ guiSurface, browserView }); - const endpoint: PreviewEndpointStatus = loopbackEndpoint?.status ?? 'not_checked'; - const covered = surface === 'verified' && endpoint === 'verified'; - return { - kind: 'preview_preflight', - surface, - endpoint, - capabilities, - summary: summarizePreviewPreflight({ capabilities, surface, endpoint }), - ...(covered ? {} : { alternative: ARTIFACT_PREVIEW_HANDOFF }), - }; + return runPreviewPreflight(authority, { origin: rawOrigin, sessionId, abortSignal }); }, }; return [preflight as MakaTool]; } + +/** Shared execution: callers consume the same classification and reasons as the tool. */ +export async function runPreviewPreflight( + authority: PreviewPreflightAuthority, + { origin: rawOrigin, sessionId, abortSignal }: { + origin?: string; + sessionId: string; + abortSignal: AbortSignal; + }, +): Promise { + abortSignal.throwIfAborted(); + // Validate before probing so a malformed origin is an argument error the + // caller can fix, not an `unknown` capability that reads like a finding. + const origin = rawOrigin === undefined ? undefined : assertLoopbackOrigin(rawOrigin); + + const guiSurface = classifyGuiSurface( + await observe(abortSignal, () => authority.guiSurfaceAvailable()), + ); + const urlSchemes = classifyUrlSchemes(probeNavigableSchemes()); + // Skip the drive check when there is demonstrably no view host: asking + // would throw, and an exception dressed as `unknown` would hide the + // settled `unsupported` answer the caller needs. Desktop, the only + // publisher today, registers its host before any offer is published, so + // this branch is unreachable there; it is kept because GUI availability + // is part of the classification #5235 builds on, and that contract must + // answer correctly for a publisher without a view host rather than throw. + const browserView = + guiSurface.status === 'unsupported' + ? BROWSER_VIEW_WITHOUT_HOST + : classifyBrowserView( + await observe(abortSignal, () => + authority.browserDrivable({ sessionId, signal: abortSignal }), + ), + ); + const loopbackEndpoint = + origin === undefined + ? undefined + : classifyLoopbackEndpoint({ + origin, + observed: await observe(abortSignal, () => + authority.probeLoopback({ origin, signal: abortSignal }), + ), + }); + + const capabilities = [ + guiSurface, + urlSchemes, + browserView, + ...(loopbackEndpoint ? [loopbackEndpoint] : []), + ]; + const surface = derivePreviewSurface({ guiSurface, browserView }); + const endpoint: PreviewEndpointStatus = loopbackEndpoint?.status ?? 'not_checked'; + const covered = surface === 'verified' && endpoint === 'verified'; + return { + kind: 'preview_preflight', + surface, + endpoint, + capabilities, + summary: summarizePreviewPreflight({ capabilities, surface, endpoint }), + ...(covered ? {} : { alternative: ARTIFACT_PREVIEW_HANDOFF }), + }; +} diff --git a/knip.json b/knip.json index 3bea6940e9a..24680e56d79 100644 --- a/knip.json +++ b/knip.json @@ -21,6 +21,7 @@ "stories/**/*.@(ts|tsx)", "scripts/browser-observe-act-smoke.mjs", "scripts/browser-load-failure-smoke.mjs", + "scripts/artifact-preview-verification-smoke.mjs", "scripts/client-plugin-p1-smoke.mjs", "scripts/workhub-browser-presentation-smoke.mjs", "src/renderer/astryx-theme/maka.d.ts",