diff --git a/.agents/skills/migrate-application-operation/SKILL.md b/.agents/skills/migrate-application-operation/SKILL.md index e7f0039e84b..6caa5ff7c57 100644 --- a/.agents/skills/migrate-application-operation/SKILL.md +++ b/.agents/skills/migrate-application-operation/SKILL.md @@ -78,6 +78,43 @@ Classify each as `migrate`, `defer`, or `non-goal`. Do not migrate adjacent oper Preserve behavior unless the task explicitly changes it. Stop and report a decision when surfaces currently disagree on security or compatibility behavior; do not silently choose one. +## Freeze observable behavior before editing + +Treat the legacy route or tool as an ordered program, not merely a bag of business logic. Before moving code, write a compact baseline for every in-scope entry point and add focused characterization tests for behavior not already pinned down. + +Capture all of these when they apply: + +- Accepted inputs, including trimming, blank omission, duplicate query keys, aliases, defaults, and bounds. +- Authentication and authorization order, minimum roles, resource membership, concealment, and exact error/status mapping. +- Exact success bodies, optional fields, status codes, redirects, cookies, headers, and binary or stream behavior. +- Mutation ordering, transaction boundaries, idempotency, no-ops, and observable state after each possible partial failure. +- Audit, notification, analytics, and billing timing plus exact semantic dimensions and attribution. +- Browser or protocol state ownership, concurrency isolation, expiry, callback ordering, and cleanup behavior. +- Every value newly crossing into HTML, JavaScript, SQL, URLs, logs, provider payloads, or another encoding context. + +Compare the old statement order with the proposed application lifecycle explicitly: + +```text +legacy parse/normalize + -> legacy authorization checks + -> branch-specific canonical lookup + -> mutation(s) + -> per-step side effects + -> response or redirect catch +``` + +Moving those steps under a wrapper may change behavior even when each individual call is reused. In particular: + +- `projectAudit` and `afterSuccess` run only after `execute` returns. They cannot describe earlier committed mutations when a later step throws. Make the compound mutation atomic or define explicit partial-result/failure projection semantics before migrating it. +- Operation metadata is executable policy. Adding a resource role to a workspace-only legacy read is an authorization change, not an architectural cleanup. +- A shared error policy does not automatically preserve route-local concealment, subclass ordering, browser redirects, or branch-specific messages. +- A shared contract does not automatically preserve manual `URLSearchParams` normalization or exact legacy response unions. +- A shared use case may own domain behavior while separate surface presenters still preserve different wire shapes. +- Per-flow identity is insufficient when another part of the flow remains in browser-global state such as one cookie. +- Passing a newly supported parameter through old rendering code creates a new security boundary even when the renderer itself is unchanged. + +Fail fast if the baseline cannot be established from code, tests, or an explicit product decision. Do not infer that behavior is unimportant because it was previously implicit. + ## Keep the layers distinct Use these responsibilities: @@ -270,6 +307,10 @@ Add focused tests for every migrated surface and principal kind allowed by the o - Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. - Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. - Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. +- Compatibility characterization: legacy normalization, exact response/redirect/cookie behavior, concealment, error subclass precedence, and branch-specific output. +- Failure sequencing: inject a failure after each independently committing step and assert persisted state plus audit, analytics, and notification effects. +- Concurrency: overlap stateful browser or provider flows and prove each callback consumes only its own state and return destination. +- Rendering boundaries: exercise hostile values for every newly connected input that reaches HTML, inline JavaScript, URLs, logs, or provider requests. Run at minimum: diff --git a/.claude/commands/migrate-application-operation.md b/.claude/commands/migrate-application-operation.md index bc61333c358..d8048553e78 100644 --- a/.claude/commands/migrate-application-operation.md +++ b/.claude/commands/migrate-application-operation.md @@ -77,6 +77,43 @@ Classify each as `migrate`, `defer`, or `non-goal`. Do not migrate adjacent oper Preserve behavior unless the task explicitly changes it. Stop and report a decision when surfaces currently disagree on security or compatibility behavior; do not silently choose one. +## Freeze observable behavior before editing + +Treat the legacy route or tool as an ordered program, not merely a bag of business logic. Before moving code, write a compact baseline for every in-scope entry point and add focused characterization tests for behavior not already pinned down. + +Capture all of these when they apply: + +- Accepted inputs, including trimming, blank omission, duplicate query keys, aliases, defaults, and bounds. +- Authentication and authorization order, minimum roles, resource membership, concealment, and exact error/status mapping. +- Exact success bodies, optional fields, status codes, redirects, cookies, headers, and binary or stream behavior. +- Mutation ordering, transaction boundaries, idempotency, no-ops, and observable state after each possible partial failure. +- Audit, notification, analytics, and billing timing plus exact semantic dimensions and attribution. +- Browser or protocol state ownership, concurrency isolation, expiry, callback ordering, and cleanup behavior. +- Every value newly crossing into HTML, JavaScript, SQL, URLs, logs, provider payloads, or another encoding context. + +Compare the old statement order with the proposed application lifecycle explicitly: + +```text +legacy parse/normalize + -> legacy authorization checks + -> branch-specific canonical lookup + -> mutation(s) + -> per-step side effects + -> response or redirect catch +``` + +Moving those steps under a wrapper may change behavior even when each individual call is reused. In particular: + +- `projectAudit` and `afterSuccess` run only after `execute` returns. They cannot describe earlier committed mutations when a later step throws. Make the compound mutation atomic or define explicit partial-result/failure projection semantics before migrating it. +- Operation metadata is executable policy. Adding a resource role to a workspace-only legacy read is an authorization change, not an architectural cleanup. +- A shared error policy does not automatically preserve route-local concealment, subclass ordering, browser redirects, or branch-specific messages. +- A shared contract does not automatically preserve manual `URLSearchParams` normalization or exact legacy response unions. +- A shared use case may own domain behavior while separate surface presenters still preserve different wire shapes. +- Per-flow identity is insufficient when another part of the flow remains in browser-global state such as one cookie. +- Passing a newly supported parameter through old rendering code creates a new security boundary even when the renderer itself is unchanged. + +Fail fast if the baseline cannot be established from code, tests, or an explicit product decision. Do not infer that behavior is unimportant because it was previously implicit. + ## Keep the layers distinct Use these responsibilities: @@ -269,6 +306,10 @@ Add focused tests for every migrated surface and principal kind allowed by the o - Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. - Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. - Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. +- Compatibility characterization: legacy normalization, exact response/redirect/cookie behavior, concealment, error subclass precedence, and branch-specific output. +- Failure sequencing: inject a failure after each independently committing step and assert persisted state plus audit, analytics, and notification effects. +- Concurrency: overlap stateful browser or provider flows and prove each callback consumes only its own state and return destination. +- Rendering boundaries: exercise hostile values for every newly connected input that reaches HTML, inline JavaScript, URLs, logs, or provider requests. Run at minimum: diff --git a/.cursor/commands/migrate-application-operation.md b/.cursor/commands/migrate-application-operation.md index 9fac674ca6f..0742f452523 100644 --- a/.cursor/commands/migrate-application-operation.md +++ b/.cursor/commands/migrate-application-operation.md @@ -73,6 +73,43 @@ Classify each as `migrate`, `defer`, or `non-goal`. Do not migrate adjacent oper Preserve behavior unless the task explicitly changes it. Stop and report a decision when surfaces currently disagree on security or compatibility behavior; do not silently choose one. +## Freeze observable behavior before editing + +Treat the legacy route or tool as an ordered program, not merely a bag of business logic. Before moving code, write a compact baseline for every in-scope entry point and add focused characterization tests for behavior not already pinned down. + +Capture all of these when they apply: + +- Accepted inputs, including trimming, blank omission, duplicate query keys, aliases, defaults, and bounds. +- Authentication and authorization order, minimum roles, resource membership, concealment, and exact error/status mapping. +- Exact success bodies, optional fields, status codes, redirects, cookies, headers, and binary or stream behavior. +- Mutation ordering, transaction boundaries, idempotency, no-ops, and observable state after each possible partial failure. +- Audit, notification, analytics, and billing timing plus exact semantic dimensions and attribution. +- Browser or protocol state ownership, concurrency isolation, expiry, callback ordering, and cleanup behavior. +- Every value newly crossing into HTML, JavaScript, SQL, URLs, logs, provider payloads, or another encoding context. + +Compare the old statement order with the proposed application lifecycle explicitly: + +```text +legacy parse/normalize + -> legacy authorization checks + -> branch-specific canonical lookup + -> mutation(s) + -> per-step side effects + -> response or redirect catch +``` + +Moving those steps under a wrapper may change behavior even when each individual call is reused. In particular: + +- `projectAudit` and `afterSuccess` run only after `execute` returns. They cannot describe earlier committed mutations when a later step throws. Make the compound mutation atomic or define explicit partial-result/failure projection semantics before migrating it. +- Operation metadata is executable policy. Adding a resource role to a workspace-only legacy read is an authorization change, not an architectural cleanup. +- A shared error policy does not automatically preserve route-local concealment, subclass ordering, browser redirects, or branch-specific messages. +- A shared contract does not automatically preserve manual `URLSearchParams` normalization or exact legacy response unions. +- A shared use case may own domain behavior while separate surface presenters still preserve different wire shapes. +- Per-flow identity is insufficient when another part of the flow remains in browser-global state such as one cookie. +- Passing a newly supported parameter through old rendering code creates a new security boundary even when the renderer itself is unchanged. + +Fail fast if the baseline cannot be established from code, tests, or an explicit product decision. Do not infer that behavior is unimportant because it was previously implicit. + ## Keep the layers distinct Use these responsibilities: @@ -265,6 +302,10 @@ Add focused tests for every migrated surface and principal kind allowed by the o - Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. - Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. - Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. +- Compatibility characterization: legacy normalization, exact response/redirect/cookie behavior, concealment, error subclass precedence, and branch-specific output. +- Failure sequencing: inject a failure after each independently committing step and assert persisted state plus audit, analytics, and notification effects. +- Concurrency: overlap stateful browser or provider flows and prove each callback consumes only its own state and return destination. +- Rendering boundaries: exercise hostile values for every newly connected input that reaches HTML, inline JavaScript, URLs, logs, or provider requests. Run at minimum: diff --git a/.github/workflows/publish-sim-cli.yml b/.github/workflows/publish-sim-cli.yml new file mode 100644 index 00000000000..f36e514ec69 --- /dev/null +++ b/.github/workflows/publish-sim-cli.yml @@ -0,0 +1,149 @@ +name: Publish Sim API CLI Package + +on: + push: + branches: [main, staging, dev] + paths: + - 'packages/sim-cli/**' + +permissions: + contents: read + +concurrency: + group: publish-sim-cli-${{ github.ref }} + cancel-in-progress: true + +jobs: + publish-npm: + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-4vcpu-ubuntu-2404' || 'ubuntu-latest' }} + timeout-minutes: 15 + steps: + - name: Checkout repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 + with: + bun-version: 1.3.14 + + - name: Setup Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 + with: + node-version: '20' + + - name: Cache Bun dependencies + uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5 + with: + path: | + ~/.bun/install/cache + node_modules + **/node_modules + key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lock') }} + restore-keys: | + ${{ runner.os }}-bun- + + - name: Install dependencies + run: bun install --frozen-lockfile --ignore-scripts + + - name: Verify npm authentication + env: + NPM_CONFIG_TOKEN: ${{ secrets.NPM_TOKEN }} + run: bun pm whoami + + - name: Run tests + working-directory: packages/sim-cli + run: bun run test + + - name: Type-check package + working-directory: packages/sim-cli + run: bun run type-check + + - name: Build package + working-directory: packages/sim-cli + run: bun run build + + - name: Resolve release channel + id: release + working-directory: packages/sim-cli + env: + BRANCH: ${{ github.ref_name }} + run: | + BASE_VERSION="$(bun -p "require('./package.json').version")" + if [[ ! "$BASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "Package version must be a stable X.Y.Z base, got '$BASE_VERSION'." >&2 + exit 1 + fi + + case "$BRANCH" in + dev) + VERSION="${BASE_VERSION}-dev.${GITHUB_RUN_NUMBER}.${GITHUB_RUN_ATTEMPT}" + TAG="dev" + ;; + staging) + VERSION="${BASE_VERSION}-preview.${GITHUB_RUN_NUMBER}.${GITHUB_RUN_ATTEMPT}" + TAG="staging" + ;; + main) + VERSION="$BASE_VERSION" + TAG="latest" + ;; + *) + echo "Unsupported release branch '$BRANCH'." >&2 + exit 1 + ;; + esac + + bun pm pkg set "version=$VERSION" + RESOLVED_VERSION="$(bun -p "require('./package.json').version")" + if [ "$RESOLVED_VERSION" != "$VERSION" ]; then + echo "Version injection mismatch: wanted '$VERSION', got '$RESOLVED_VERSION'." >&2 + exit 1 + fi + + { + echo "version=$VERSION" + echo "tag=$TAG" + } >> "$GITHUB_OUTPUT" + + - name: Smoke-test packed Node bundle + working-directory: packages/sim-cli + run: | + set -euo pipefail + SMOKE_DIR="$(mktemp -d "$RUNNER_TEMP/sim-cli-smoke.XXXXXX")" + PACKAGE_PATH="$SMOKE_DIR/sim-cli.tgz" + bun pm pack --ignore-scripts --filename "$PACKAGE_PATH" --quiet + tar -xzf "$PACKAGE_PATH" -C "$SMOKE_DIR" + "$SMOKE_DIR/package/dist/index.js" --version + + - name: Check if version already exists + id: version_check + working-directory: packages/sim-cli + env: + VERSION: ${{ steps.release.outputs.version }} + run: | + if bun pm view "sim@$VERSION" version > /dev/null 2>&1; then + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "exists=false" >> "$GITHUB_OUTPUT" + fi + + - name: Publish to npm + if: steps.version_check.outputs.exists == 'false' + working-directory: packages/sim-cli + env: + NPM_CONFIG_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_TAG: ${{ steps.release.outputs.tag }} + run: bun publish --access public --tag "$NPM_TAG" --no-save + + - name: Summarize release + if: steps.version_check.outputs.exists == 'false' + env: + VERSION: ${{ steps.release.outputs.version }} + NPM_TAG: ${{ steps.release.outputs.tag }} + run: echo "Published sim@$VERSION with the '$NPM_TAG' tag." + + - name: Summarize skipped release + if: steps.version_check.outputs.exists == 'true' + env: + VERSION: ${{ steps.release.outputs.version }} + run: echo "Skipped sim@$VERSION because that version is already published." diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 09cdf7dbb48..007df411aa5 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -123,6 +123,9 @@ jobs: - name: Repo audits run: bun run check:audits + - name: Verify docs manifest is in sync + run: bun run docs-manifest:check + - name: Migration safety (zero-downtime) audit run: | if [ "${{ github.event_name }}" = "pull_request" ]; then @@ -264,4 +267,4 @@ jobs: AWS_REGION: 'us-west-2' ENCRYPTION_KEY: '7cf672e460e430c1fba707575c2b0e2ad5a99dddf9b7b7e3b5646e630861db1c' # dummy key for CI only TURBO_CACHE_DIR: .turbo - run: bunx turbo run build --filter=sim + run: bunx turbo run build --filter=@sim/app diff --git a/apps/desktop/src/main/browser-agent/cdp.test.ts b/apps/desktop/src/main/browser-agent/cdp.test.ts index 2efa344dd71..2f21c1ea6d9 100644 --- a/apps/desktop/src/main/browser-agent/cdp.test.ts +++ b/apps/desktop/src/main/browser-agent/cdp.test.ts @@ -2,8 +2,9 @@ import { describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -import { WebContentsView, type WebFrameMain } from 'electron' +import { nativeImage, type WebContents, WebContentsView, type WebFrameMain } from 'electron' import { + captureScreenshot, clickAt, ensureInstrumented, evaluateInIsolatedFrame, @@ -482,3 +483,89 @@ describe('browser-agent CDP theme', () => { }) }) }) + +/** + * The browser panel shows a LIVE view, so a capture must not perturb the page. + * Chromium serves `clip` by applying device-emulation params to the widget and + * syncing visual properties, which the user sees as the page rescaling and + * snapping back. Resolution is bounded on the returned image instead. + */ +describe('browser-agent screenshot capture', () => { + function captureFixture(imageSize: { width: number; height: number } | null) { + const contents = new WebContentsView().webContents + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + const resized = { + toJPEG: vi.fn(() => Buffer.from('resized')), + } + // Shared module-level mock: without this, a later fixture reads the + // earlier test's decoded image. + vi.mocked(nativeImage.createFromBuffer).mockReset() + vi.mocked(nativeImage.createFromBuffer).mockReturnValue({ + isEmpty: vi.fn(() => imageSize === null), + getSize: vi.fn(() => imageSize ?? { width: 0, height: 0 }), + resize: vi.fn(() => resized), + toJPEG: vi.fn(() => Buffer.alloc(0)), + } as unknown as ReturnType) + return { contents, resized } + } + + function screenshotParams(contents: WebContents): Record { + const call = vi + .mocked(contents.debugger.sendCommand) + .mock.calls.find(([method]) => method === 'Page.captureScreenshot') + if (!call) throw new Error('no capture was requested') + return call[1] as Record + } + + it('never sends a clip, which would emulate the live page for the capture', async () => { + const { contents } = captureFixture({ width: 4096, height: 2048 }) + + await captureScreenshot(contents) + + expect(screenshotParams(contents)).not.toHaveProperty('clip') + }) + + /** + * A 2048px CSS viewport bounded to 1024px is scale 0.5, and the capture + * arrives at device resolution (4096px on a 2x display). The resize is what + * lands the image on the CSS-relative size the coordinate contract + * (cssX = imageX / scale) assumes. + */ + it('downscales the returned image to the CSS-relative size', async () => { + const { contents, resized } = captureFixture({ width: 4096, height: 2048 }) + + const shot = await captureScreenshot(contents) + + const image = vi.mocked(nativeImage.createFromBuffer).mock.results[0].value + expect(image.resize).toHaveBeenCalledWith({ width: 1024, height: 512, quality: 'good' }) + expect(resized.toJPEG).toHaveBeenCalled() + expect(shot).toEqual({ + dataUrl: `data:image/jpeg;base64,${Buffer.from('resized').toString('base64')}`, + scale: 0.5, + }) + }) + + it('skips the re-encode when the capture already matches the target size', async () => { + const { contents } = captureFixture({ width: 1024, height: 512 }) + + const shot = await captureScreenshot(contents) + + const image = vi.mocked(nativeImage.createFromBuffer).mock.results[0].value + expect(image.resize).not.toHaveBeenCalled() + expect(shot).toEqual({ dataUrl: 'data:image/jpeg;base64,c2lt', scale: 0.5 }) + }) + + it('returns the raw capture when the image cannot be decoded', async () => { + const { contents } = captureFixture(null) + + const shot = await captureScreenshot(contents) + + expect(shot).toEqual({ dataUrl: 'data:image/jpeg;base64,c2lt', scale: 0.5 }) + }) +}) diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index 4a7088e1b24..36dffe4da0d 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -10,7 +10,8 @@ */ import type { BrowserTheme } from '@sim/browser-protocol' import { createLogger } from '@sim/logger' -import type { WebContents, WebFrameMain } from 'electron' +import { sleep } from '@sim/utils/helpers' +import { nativeImage, type WebContents, type WebFrameMain } from 'electron' const logger = createLogger('BrowserAgentCdp') @@ -127,12 +128,29 @@ export async function setColorScheme(contents: WebContents, theme: BrowserTheme) }) } +/** Live drag-interception state while a dragPointer call is in flight. */ +interface DragInterception { + intercepted: boolean + data: Record | null +} +const dragInterceptionsByContents = new WeakMap() + function handleDebuggerEvent( contents: WebContents, method: string, params: Record, parentSessionId?: string ): void { + if (method === 'Input.dragIntercepted') { + const interception = dragInterceptionsByContents.get(contents) + if (interception) { + interception.intercepted = true + const data = params.data + interception.data = + data && typeof data === 'object' ? (data as Record) : null + } + return + } if (method === 'Target.attachedToTarget') { const sessionId = typeof params.sessionId === 'string' ? params.sessionId : '' const targetInfo = params.targetInfo @@ -352,6 +370,14 @@ export async function evaluateInIsolatedFrame( */ const MAX_SCREENSHOT_EDGE = 1024 const SCREENSHOT_QUALITY = 70 +/** + * Quality of the intermediate capture, before the in-process downscale + * re-encodes at {@link SCREENSHOT_QUALITY}. Higher than the final quality so + * the two lossy passes together land near where one pass did — the model reads + * text out of these frames, and compression artifacts on glyphs cost more than + * the transient bytes do. + */ +const SCREENSHOT_CAPTURE_QUALITY = 90 interface CdpViewport { clientWidth: number @@ -361,13 +387,23 @@ interface CdpViewport { /** * Screenshot via CDP (works while the view is hidden), bounded in resolution. * - * `clip.scale` is relative to CSS pixels, so passing the CSS viewport with a - * scale of 1 already sidesteps the device pixel ratio — an unclipped capture - * on a 2x display returns a 2x image. Scaling further down keeps the longest - * edge within {@link MAX_SCREENSHOT_EDGE}. Falls back to an unclipped capture - * when layout metrics are unavailable. + * The capture is deliberately UNCLIPPED. Chromium implements `clip` by applying + * device-emulation parameters (viewport offset and scale) to the widget and + * synchronizing visual properties, then restoring them. On a headless target + * that is invisible; against the live, composited WebContentsView the Sim + * resource panel shows, it is a real visual-properties round-trip, and the page + * visibly rescales and snaps back — the screenshot flash. `panel.ts`'s own + * snapshot capture refuses to scale a visible surface for the same reason. + * + * Bounding resolution therefore happens here instead, on the returned image. + * The output keeps the dimensions the clipped capture produced, so `scale` + * still maps image pixels back to CSS pixels for the coordinate tools + * (cssX = imageX / scale) — including on a 2x display, where an unclipped + * capture arrives at device resolution and this is what brings it back down. */ -export async function captureScreenshot(contents: WebContents): Promise { +export async function captureScreenshot( + contents: WebContents +): Promise<{ dataUrl: string; scale: number }> { const metrics = await send<{ cssLayoutViewport?: CdpViewport layoutViewport?: CdpViewport @@ -376,23 +412,34 @@ export async function captureScreenshot(contents: WebContents): Promise const viewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport const width = viewport?.clientWidth ?? 0 const height = viewport?.clientHeight ?? 0 - const clip = - width > 0 && height > 0 - ? { - x: 0, - y: 0, - width, - height, - scale: Math.min(1, MAX_SCREENSHOT_EDGE / Math.max(width, height)), - } - : undefined + const scale = + width > 0 && height > 0 ? Math.min(1, MAX_SCREENSHOT_EDGE / Math.max(width, height)) : 1 const result = await send<{ data: string }>(contents, 'Page.captureScreenshot', { format: 'jpeg', - quality: SCREENSHOT_QUALITY, - ...(clip ? { clip } : {}), + quality: SCREENSHOT_CAPTURE_QUALITY, }) - return `data:image/jpeg;base64,${result.data}` + const captured = `data:image/jpeg;base64,${result.data}` + + const targetWidth = Math.round(width * scale) + const targetHeight = Math.round(height * scale) + // Without layout metrics there is no CSS frame of reference to resize + // against, so the raw capture is the honest answer — the same fallback the + // clipped path took. + if (targetWidth <= 0 || targetHeight <= 0) return { dataUrl: captured, scale } + + const image = nativeImage.createFromBuffer(Buffer.from(result.data, 'base64')) + const size = image.isEmpty() ? { width: 0, height: 0 } : image.getSize() + if (size.width === 0 || size.height === 0) return { dataUrl: captured, scale } + if (size.width === targetWidth && size.height === targetHeight) { + return { dataUrl: captured, scale } + } + + const resized = image.resize({ width: targetWidth, height: targetHeight, quality: 'good' }) + return { + dataUrl: `data:image/jpeg;base64,${resized.toJPEG(SCREENSHOT_QUALITY).toString('base64')}`, + scale, + } } /** One half of a trusted key press (`Input.dispatchKeyEvent` params). */ @@ -430,7 +477,8 @@ export async function clickAt( contents: WebContents, x: number, y: number, - moveBeforePress = true + moveBeforePress = true, + clickCount = 1 ): Promise { if (moveBeforePress) await moveMouse(contents, x, y) let pressed = false @@ -439,22 +487,26 @@ export async function clickAt( // response (navigation/process swap). In that ambiguous case a release is // safer than leaving Blink's pointer state stuck down. pressed = true - await sendInput(contents, 'Input.dispatchMouseEvent', { - type: 'mousePressed', - x, - y, - button: 'left', - buttons: 1, - clickCount: 1, - }) - await sendInput(contents, 'Input.dispatchMouseEvent', { - type: 'mouseReleased', - x, - y, - button: 'left', - buttons: 0, - clickCount: 1, - }) + // A multi-click is a sequence of press/release pairs with an increasing + // clickCount — Blink synthesizes dblclick from the pair whose count is 2. + for (let count = 1; count <= clickCount; count++) { + await sendInput(contents, 'Input.dispatchMouseEvent', { + type: 'mousePressed', + x, + y, + button: 'left', + buttons: 1, + clickCount: count, + }) + await sendInput(contents, 'Input.dispatchMouseEvent', { + type: 'mouseReleased', + x, + y, + button: 'left', + buttons: 0, + clickCount: count, + }) + } pressed = false } finally { if (pressed && !contents.isDestroyed()) { @@ -473,6 +525,131 @@ export async function clickAt( } } +/** + * Drags the pointer from one viewport point to another through the trusted + * pipeline: press, a threshold-crossing nudge, interpolated moves with the + * button held, a settle hold over the target, then release or drop. + * + * Two drag models are covered by the one call. Pointer-sensor libraries + * (dnd-kit, react-beautiful-dnd, canvas apps) treat the held-button move + * sequence exactly like a human drag. Native HTML5 `draggable="true"` + * sources instead START a Blink drag session on the press+move — with + * `Input.setInterceptDrags` enabled, Chromium reports it as + * `Input.dragIntercepted` and the remaining movement is delivered as trusted + * `Input.dispatchDragEvent` dragEnter/dragOver events ending in a `drop` + * (the technique Playwright uses). Both paths are trusted input. + */ +export async function dragPointer( + contents: WebContents, + from: { x: number; y: number }, + to: { x: number; y: number }, + steps = 12, + stepDelayMs = 20 +): Promise<{ nativeDragIntercepted: boolean }> { + const interception: DragInterception = { intercepted: false, data: null } + dragInterceptionsByContents.set(contents, interception) + let interceptEnabled = false + try { + await send(contents, 'Input.setInterceptDrags', { enabled: true }) + interceptEnabled = true + } catch { + // Chromium without drag interception: the pointer-only path still works + // for pointer-sensor drags; native HTML5 sources will report no effect. + } + await moveMouse(contents, from.x, from.y) + let pressed = false + let dragEnterSent = false + const dragMove = async (x: number, y: number) => { + if (interception.intercepted && interception.data) { + await sendInput(contents, 'Input.dispatchDragEvent', { + type: dragEnterSent ? 'dragOver' : 'dragEnter', + x, + y, + data: interception.data, + }) + dragEnterSent = true + } else { + await sendInput(contents, 'Input.dispatchMouseEvent', { + type: 'mouseMoved', + x, + y, + button: 'left', + buttons: 1, + }) + } + } + try { + pressed = true + await sendInput(contents, 'Input.dispatchMouseEvent', { + type: 'mousePressed', + x: from.x, + y: from.y, + button: 'left', + buttons: 1, + clickCount: 1, + }) + // Small first nudge so libraries with a start threshold (commonly 3-8px) + // register the drag before the pointer sweeps across the page. + await dragMove(from.x + Math.sign(to.x - from.x || 1) * 4, from.y + 2) + await sleep(stepDelayMs) + const stepCount = Math.max(2, steps) + for (let step = 1; step <= stepCount; step++) { + const progress = step / stepCount + await dragMove(from.x + (to.x - from.x) * progress, from.y + (to.y - from.y) * progress) + await sleep(stepDelayMs) + } + // Hold over the target so drop zones running enter/over animations settle + // before the release lands. + await sleep(120) + if (interception.intercepted && interception.data) { + await sendInput(contents, 'Input.dispatchDragEvent', { + type: 'drop', + x: to.x, + y: to.y, + data: interception.data, + }) + // Blink ended the intercepted drag session itself; a trailing + // mouseReleased would be a stray click on the drop target. + pressed = false + } else { + await sendInput(contents, 'Input.dispatchMouseEvent', { + type: 'mouseReleased', + x: to.x, + y: to.y, + button: 'left', + buttons: 0, + clickCount: 1, + }) + pressed = false + } + return { nativeDragIntercepted: interception.intercepted } + } finally { + if (pressed && !contents.isDestroyed()) { + if (interception.intercepted && interception.data) { + await sendInput(contents, 'Input.dispatchDragEvent', { + type: 'dragCancel', + x: to.x, + y: to.y, + data: interception.data, + }).catch(() => {}) + } else { + await sendInput(contents, 'Input.dispatchMouseEvent', { + type: 'mouseReleased', + x: to.x, + y: to.y, + button: 'left', + buttons: 0, + clickCount: 1, + }).catch(() => {}) + } + } + dragInterceptionsByContents.delete(contents) + if (interceptEnabled && !contents.isDestroyed()) { + await send(contents, 'Input.setInterceptDrags', { enabled: false }).catch(() => {}) + } + } +} + /** * Inserts text at the focused element's selection (replacing it) through the * native IME path — works in plain fields and code editors alike. diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index f98083e1add..9ebb90a76df 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -1121,6 +1121,20 @@ describe('credential protection', () => { expect(cdpCalls(contents, 'Input.dispatchKeyEvent').length).toBeGreaterThan(0) }) + it('still dispatches input after the user has interacted with the visible tab', async () => { + const contents = await openPage() + respondWith(contents, { activeElementSecrecy: 'safe', readActiveElementState: {} }) + // User interaction claims the visible tab for panel-level ownership + // (popups, close protection) but must never block agent input. + session.claimActiveTabForUser() + expect(session.automationTabClaimedByUser()).toBe(true) + + const result = await driver.executeTool('chat-test', 'browser_press_key', { key: 'a' }) + + expect(result.ok).toBe(true) + expect(cdpCalls(contents, 'Input.dispatchKeyEvent').length).toBeGreaterThan(0) + }) + it('sends the keystroke when nothing sensitive is focused', async () => { const contents = await openPage() respondWith(contents, { activeElementSecrecy: 'safe', readActiveElementState: {} }) @@ -1646,7 +1660,44 @@ describe('credential protection', () => { expect(mainFrame.executeJavaScript).not.toHaveBeenCalled() }) - it('reports navigation that remains obstructed by a DOM dialog', async () => { + // A dialog that was ALREADY open before the click is not obstructing the + // navigation it survived — reporting it made every SPA route change under a + // persistent role=dialog (cookie banner, side drawer, picker) read as a + // failed click. Only a dialog that arrives with the navigation obstructs it. + // targetChanged can only fire when pageActionState was given an elementId. + // Tools without one listed it in their effect formula for a long time, where + // it was silently always false — coverage that read as real. This pins the + // dependency so the next tool that adds the term has to earn it. + it('cannot observe a target change for a tool that passes no elementId', async () => { + const contents = await openPage() + let actionReads = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'readActiveElementState')) return Promise.resolve({}) + if (isPageCall(expression, 'readPageActionState')) { + actionReads++ + // No targetState in either sample: that is what a call without an + // elementId returns. + return Promise.resolve({ + url: 'https://example.com/a', + title: 'A', + focus: 'body', + mutationRevision: actionReads === 1 ? 0 : 3, + dialogs: [], + popups: [], + scroll: [0], + }) + } + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_press_key', { key: 'Enter' }) + + expect(result).toMatchObject({ ok: true }) + const effect = (result as { result?: { effect?: Record } }).result?.effect + expect(effect?.targetChanged).toBe(false) + }) + + it('ignores a dialog that was already open before the click', async () => { const contents = await openPage() let actionReads = 0 vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { @@ -1681,13 +1732,52 @@ describe('credential protection', () => { const result = await driver.executeTool('chat-test', 'browser_click', { elementId: 0 }) + expect(result).toMatchObject({ + ok: true, + result: { effectObserved: true, obstructedAfterNavigation: false, dialogs: ['Search'] }, + }) + }) + + it('reports navigation obstructed by a dialog that opened with it', async () => { + const contents = await openPage() + let actionReads = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'clickElement')) { + return Promise.resolve({ dispatched: false, x: 24, y: 48, element: 'Search result' }) + } + if (isPageCall(expression, 'readActiveElementState')) return Promise.resolve({}) + if (isPageCall(expression, 'readPageActionState')) { + actionReads++ + return Promise.resolve( + actionReads === 1 + ? { + url: 'https://example.com/search', + title: 'Search', + focus: 'body', + mutationRevision: 0, + dialogs: [], + scroll: [0], + } + : { + url: 'https://example.com/channel/eng-bugs', + title: 'eng-bugs', + focus: 'body', + mutationRevision: 1, + dialogs: ['Open in the Slack app?'], + scroll: [0], + } + ) + } + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_click', { elementId: 0 }) + expect(result).toMatchObject({ ok: true, result: { - effectObserved: true, obstructedAfterNavigation: true, - dialogs: ['Search'], - note: expect.stringContaining('dialog is still open'), + note: expect.stringContaining('Open in the Slack app?'), }, }) }) @@ -1732,4 +1822,355 @@ describe('credential protection', () => { 'Element ids are not valid in this tab. Call browser_snapshot and use an id from that result.', }) }) + + /** Fires every instrumentation listener registered for a WebContents event. */ + function emitContentsEvent( + contents: Awaited>, + event: string, + ...args: unknown[] + ): void { + for (const [name, listener] of vi.mocked(contents.on).mock.calls) { + if (name === event) (listener as (...listenerArgs: unknown[]) => void)({}, ...args) + } + } + + it('keeps element ids across a same-document (SPA) navigation', async () => { + const contents = await openPage() + respondWith(contents, {}) + + emitContentsEvent(contents, 'did-navigate-in-page') + const result = await driver.executeTool('chat-test', 'browser_click', { elementId: 0 }) + + expect(result).toMatchObject({ ok: true, result: { dispatched: true } }) + }) + + it('still invalidates element ids on a cross-document navigation', async () => { + const contents = await openPage() + respondWith(contents, {}) + + emitContentsEvent(contents, 'did-navigate') + const result = await driver.executeTool('chat-test', 'browser_click', { elementId: 0 }) + + expect(result).toEqual({ + ok: false, + error: + 'Element ids are not valid in this tab. Call browser_snapshot and use an id from that result.', + }) + }) + + it('tolerates same-document URL churn during a keypress', async () => { + const contents = await openPage() + let urlReads = 0 + vi.mocked(contents.getURL).mockImplementation(() => + ++urlReads === 1 ? 'https://example.com/channel-a' : 'https://example.com/channel-b' + ) + respondWith(contents, { + activeElementSecrecy: 'safe', + readActiveElementState: {}, + readPageActionState: { + url: 'https://example.com/channel-a', + title: 'Example', + focus: 'body', + mutationRevision: 0, + dialogs: [], + scroll: [0], + }, + }) + + const result = await driver.executeTool('chat-test', 'browser_press_key', { key: 'Escape' }) + + expect(result.ok).toBe(true) + expect(cdpCalls(contents, 'Input.dispatchKeyEvent').length).toBeGreaterThan(0) + }) + + it('aborts a keypress when a cross-document navigation lands mid-flight', async () => { + const contents = await openPage() + let navigated = false + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'activeElementSecrecy')) return Promise.resolve('safe') + if (isPageCall(expression, 'readActiveElementState')) return Promise.resolve({}) + if (isPageCall(expression, 'readPageActionState')) { + if (!navigated) { + navigated = true + emitContentsEvent(contents, 'did-navigate') + } + return Promise.resolve({ + url: 'https://example.com/login', + title: 'Example', + focus: 'body', + mutationRevision: 0, + dialogs: [], + scroll: [0], + }) + } + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_press_key', { key: 'Escape' }) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/active tab or page changed/) + expect(cdpCalls(contents, 'Input.dispatchKeyEvent')).toHaveLength(0) + }) + + it('waits for a late-mounting editor before typing', async () => { + const contents = await openPage() + let focusReads = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'focusElementForTyping')) { + focusReads++ + return Promise.resolve( + focusReads === 1 + ? { error: 'not-editable' } + : { focused: true, kind: 'contenteditable', x: 24, y: 48 } + ) + } + if (isPageCall(expression, 'activeElementSecrecy')) return Promise.resolve('safe') + if (isPageCall(expression, 'readActiveElementState')) { + return Promise.resolve({ activeElement: 'div', valueLength: 5 }) + } + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_type', { + elementId: 0, + text: 'hello', + }) + + expect(result.ok).toBe(true) + expect(focusReads).toBeGreaterThan(1) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(1) + }) + + it('reprobes a transiently stale click target before giving up', async () => { + const contents = await openPage() + let clickReads = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'clickElement')) { + clickReads++ + return Promise.resolve( + clickReads === 1 + ? { error: 'stale' } + : { dispatched: false, x: 24, y: 48, element: 'Channel row' } + ) + } + if (isPageCall(expression, 'readActiveElementState')) return Promise.resolve({}) + if (isPageCall(expression, 'readPageActionState')) return Promise.resolve({}) + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_click', { elementId: 0 }) + + expect(result).toMatchObject({ ok: true, result: { dispatched: true } }) + expect(clickReads).toBeGreaterThan(1) + }) + + it('clicks a coordinate point with native input and reports the target', async () => { + const contents = await openPage() + respondWith(contents, { + describePointTarget: { + found: true, + element: 'button "Send"', + editable: false, + secret: false, + fileInput: false, + cursor: 'pointer', + }, + readActiveElementState: {}, + readPageActionState: {}, + }) + + const result = await driver.executeTool('chat-test', 'browser_click_at', { x: 120, y: 240 }) + + expect(result).toMatchObject({ + ok: true, + result: { + dispatched: true, + trusted: true, + clickedAt: { x: 120, y: 240 }, + target: 'button "Send"', + }, + }) + const presses = cdpCalls(contents, 'Input.dispatchMouseEvent').filter( + ([, event]) => (event as { type?: string }).type === 'mousePressed' + ) + expect(presses).toHaveLength(1) + }) + + it('double-clicks a coordinate point as a rising clickCount sequence', async () => { + const contents = await openPage() + respondWith(contents, { + describePointTarget: { found: true, element: 'canvas', editable: false }, + readActiveElementState: {}, + readPageActionState: {}, + }) + + const result = await driver.executeTool('chat-test', 'browser_click_at', { + x: 10, + y: 20, + clickCount: 2, + }) + + expect(result).toMatchObject({ ok: true, result: { clickCount: 2 } }) + const counts = cdpCalls(contents, 'Input.dispatchMouseEvent') + .filter(([, event]) => (event as { type?: string }).type === 'mousePressed') + .map(([, event]) => (event as { clickCount?: number }).clickCount) + expect(counts).toEqual([1, 2]) + }) + + it('refuses a coordinate click on a file input', async () => { + const contents = await openPage() + respondWith(contents, { + describePointTarget: { found: true, element: 'input', fileInput: true }, + }) + + const result = await driver.executeTool('chat-test', 'browser_click_at', { x: 5, y: 5 }) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/file input/) + expect(cdpCalls(contents, 'Input.dispatchMouseEvent')).toHaveLength(0) + }) + + it('rejects a coordinate click outside the viewport with mapping guidance', async () => { + const contents = await openPage() + respondWith(contents, { + describePointTarget: { error: 'outside-viewport' }, + }) + + const result = await driver.executeTool('chat-test', 'browser_click_at', { x: 9999, y: 5 }) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/divide image pixels by its scale/) + }) + + it('inserts text into the focused editable at the caret', async () => { + const contents = await openPage() + respondWith(contents, { + activeElementSecrecy: 'safe', + describeFocusedEditable: { editable: true, kind: 'contenteditable' }, + readActiveElementState: { activeElement: 'div', valueLength: 12 }, + readPageActionState: {}, + }) + + const result = await driver.executeTool('chat-test', 'browser_insert_text', { + text: 'hello world', + }) + + expect(result).toMatchObject({ + ok: true, + result: { dispatched: true, trusted: true, kind: 'contenteditable', insertedChars: 11 }, + }) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(1) + }) + + it('refuses insertion when nothing editable holds focus', async () => { + const contents = await openPage() + respondWith(contents, { + activeElementSecrecy: 'safe', + describeFocusedEditable: { editable: false, reason: 'none' }, + }) + + const result = await driver.executeTool('chat-test', 'browser_insert_text', { text: 'x' }) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/No element is focused/) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(0) + }) + + it('refuses insertion while a password field holds focus', async () => { + const contents = await openPage() + respondWith(contents, { activeElementSecrecy: 'secret' }) + + const result = await driver.executeTool('chat-test', 'browser_insert_text', { text: 'x' }) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/Refusing to act on a password field/) + expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(0) + }) + + it('drags between coordinate points through the trusted pointer pipeline', async () => { + const contents = await openPage() + respondWith(contents, { + describePointTarget: { found: true, element: 'div "Card"' }, + readActiveElementState: {}, + readPageActionState: {}, + }) + + const result = await driver.executeTool('chat-test', 'browser_drag', { + fromX: 40, + fromY: 50, + toX: 200, + toY: 260, + }) + + expect(result).toMatchObject({ + ok: true, + result: { dispatched: true, trusted: true, from: { x: 40, y: 50 }, to: { x: 200, y: 260 } }, + }) + const events = cdpCalls(contents, 'Input.dispatchMouseEvent').map( + ([, event]) => (event as { type?: string }).type + ) + expect(events[0]).toBe('mouseMoved') + expect(events).toContain('mousePressed') + expect(events[events.length - 1]).toBe('mouseReleased') + expect(cdpCalls(contents, 'Input.setInterceptDrags').length).toBeGreaterThan(0) + }) + + it('drags from a snapshot element to a coordinate target', async () => { + const contents = await openPage() + respondWith(contents, { + clickElement: { dispatched: false, x: 24, y: 48, element: 'Card "Ship it"' }, + describePointTarget: { found: true, element: 'section "Done"' }, + readActiveElementState: {}, + readPageActionState: {}, + }) + + const result = await driver.executeTool('chat-test', 'browser_drag', { + fromElementId: 0, + toX: 300, + toY: 60, + }) + + expect(result).toMatchObject({ + ok: true, + result: { dispatched: true, from: { x: 24, y: 48, element: 'Card "Ship it"' } }, + }) + }) + + it('rejects a drag whose endpoints are the same point', async () => { + const contents = await openPage() + respondWith(contents, { + describePointTarget: { found: true, element: 'div' }, + }) + + const result = await driver.executeTool('chat-test', 'browser_drag', { + fromX: 10, + fromY: 10, + toX: 10, + toY: 10, + }) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/same point/) + }) + + it('returns the screenshot scale for coordinate mapping', async () => { + const contents = await openPage() + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ + cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, + }) + } + if (method === 'Page.captureScreenshot') { + return Promise.resolve({ data: 'c2lt' }) + } + return Promise.resolve(undefined) + }) + respondWith(contents, { getViewportInfo: { width: 2048, height: 1024 } }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result).toMatchObject({ ok: true, result: { scale: 0.5 } }) + }) }) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 5c1b43c0d07..e407563906a 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -46,6 +46,8 @@ import { activeElementSecrecy, clickElement, collectSnapshot, + describeFocusedEditable, + describePointTarget, focusElementForTyping, getViewportInfo, hoverElement, @@ -174,6 +176,29 @@ function invalidateSnapshot(state = driverScopeState()): void { state.snapshotCaptureEpoch++ } +/** + * Cross-document navigation counters, bumped by tab instrumentation. A live + * SPA rewrites its URL with pushState/replaceState between a snapshot and the + * input dispatched from it, so URL equality cannot distinguish "the document + * the model saw is gone" from routine same-document churn — only a real + * document swap increments these. + */ +const crossDocumentNavigations = new WeakMap() +const crossDocumentFrameNavigations = new WeakMap>() + +function navigationEpoch(contents: WebContents): number { + return crossDocumentNavigations.get(contents) ?? 0 +} + +function frameEpochKey(processId: number, routingId: number): string { + return `${processId}:${routingId}` +} + +function frameNavigationEpoch(contents: WebContents, frame: WebFrameMain): number { + const key = frameEpochKey(frame.processId, frame.routingId) + return crossDocumentFrameNavigations.get(contents)?.get(key) ?? 0 +} + const driverScopeStates = new Map() const driverScopeAliases = new Map() const CANCELLED_TOOL_TTL_MS = 5 * 60_000 @@ -312,6 +337,7 @@ function instrumentTab(contents: WebContents): void { contents.on( 'did-navigate', inScope(() => { + crossDocumentNavigations.set(contents, navigationEpoch(contents) + 1) if (session.automationTab()?.view.webContents === contents) { invalidateSnapshot() } @@ -320,6 +346,21 @@ function instrumentTab(contents: WebContents): void { pushTabsState() }) ) + contents.on( + 'did-frame-navigate', + inScope( + (_event, _url, _httpResponseCode, _httpStatusText, _isMainFrame, processId, routingId) => { + const frames = crossDocumentFrameNavigations.get(contents) ?? new Map() + const key = frameEpochKey(processId, routingId) + frames.set(key, (frames.get(key) ?? 0) + 1) + crossDocumentFrameNavigations.set(contents, frames) + } + ) + ) + // Same-document navigation deliberately does NOT invalidate the snapshot: a + // live SPA (Slack) pushStates continuously, and invalidating here made every + // ref die between snapshot and act. Element-level identity checks and the + // in-page ref resolver keep individual actions honest instead. for (const event of [ 'did-navigate-in-page', 'page-title-updated', @@ -330,12 +371,6 @@ function instrumentTab(contents: WebContents): void { contents.on( event as 'did-navigate', inScope(() => { - if ( - event === 'did-navigate-in-page' && - session.automationTab()?.view.webContents === contents - ) { - invalidateSnapshot() - } pushPageState(contents) pushTabsState() }) @@ -851,17 +886,41 @@ function unwrapPageResult(result: unknown): unknown { `That element is covered by ${blocker}. Close or move the overlay, then take a fresh browser_snapshot.` ) } + if (code === 'nested-control') { + const blocker = String((result as { blocker?: unknown }).blocker || 'a nested control') + throw new ToolError( + `The point you targeted lands on ${blocker}, which is its own control inside that element — nothing is covering it. Take a fresh browser_snapshot and use the id of the control you actually want.` + ) + } if (code === 'suggestions-open') { throw new ToolError( 'That editable field is already focused and covered by its own suggestions popup. Use browser_type on the same element; do not dismiss the popup first.' ) } if (code === 'not-editable') { - throw new ToolError('That element is not a text input — pick an editable element.') + const tag = isRecordLike(result) ? String(result.elementTag ?? '') : '' + const role = isRecordLike(result) ? String(result.elementRole ?? '') : '' + const described = [tag ? `<${tag}>` : '', role ? `role="${role}"` : ''] + .filter(Boolean) + .join(' ') + throw new ToolError( + `That element is not a text input${described ? ` (it is ${described})` : ''} — take a fresh browser_snapshot and target the editable field itself.` + ) + } + if (code === 'outside-viewport') { + throw new ToolError( + 'That point is outside the visible viewport. Coordinates are CSS pixels within the current viewport — when reading them off a browser_screenshot, divide image pixels by its scale, and scroll the target into view first.' + ) } if (code === 'ambiguous-editable') { + const candidates = + isRecordLike(result) && Array.isArray(result.candidates) + ? result.candidates.map(String).filter(Boolean) + : [] throw new ToolError( - 'That composite control contains multiple editable fields. Take a fresh browser_snapshot and target the exact field.' + `That composite control contains multiple editable fields${ + candidates.length > 0 ? ` (${candidates.join(', ')})` : '' + }. Take a fresh browser_snapshot and target the exact field.` ) } if (code === 'different') { @@ -1037,14 +1096,18 @@ function pageTargetForElement(contents: WebContents, elementId: number): PageExe return target } -function assertActiveContents(contents: WebContents, expectedUrl?: string): void { +// The user's claim on the visible tab (visibleTabUserSelected) deliberately +// does NOT gate input here: the user clicking or typing in the panel must +// never leave the agent unable to act — hand-off is cooperative via +// browser_request_takeover, whose serialized tool slot already keeps agent +// input out while the user drives. +function assertActiveContents(contents: WebContents, expectedNavigationEpoch?: number): void { const active = session.automationTab() if ( - session.automationTabClaimedByUser() || !active || active.view.webContents !== contents || contents.isDestroyed() || - (expectedUrl !== undefined && contents.getURL() !== expectedUrl) + (expectedNavigationEpoch !== undefined && navigationEpoch(contents) !== expectedNavigationEpoch) ) { throw new ToolError('The active tab or page changed before input could be dispatched.') } @@ -1089,7 +1152,7 @@ function sameWebFrame(left: WebFrameMain, right: WebFrameMain): boolean { function assertFocusedTargetUnchanged( contents: WebContents, target: PageExecutionTarget, - expectedFrameUrl?: string + expectedFrameNavigationEpoch?: number ): void { const focused = focusedPageTarget(contents) const unchanged = @@ -1106,7 +1169,8 @@ function assertFocusedTargetUnchanged( if ( frame.isDestroyed() || frame.detached || - (expectedFrameUrl !== undefined && frame.url !== expectedFrameUrl) || + (expectedFrameNavigationEpoch !== undefined && + frameNavigationEpoch(contents, frame) !== expectedFrameNavigationEpoch) || !contents.mainFrame.framesInSubtree.some((candidate) => sameWebFrame(candidate, frame)) ) { throw new ToolError( @@ -1154,16 +1218,25 @@ async function prepareTypingSurface( target: PageExecutionTarget, elementId: number, moveFocus: boolean, - executionDeadline?: number + executionDeadline?: number, + settleGraceMs = 0 ): Promise> { const prepared = unwrapPageResult( - await execInPage( - target, - focusElementForTyping, - [elementId, moveFocus], - false, - executionDeadline - ) + settleGraceMs > 0 + ? await execInPageWithSettleGrace( + target, + focusElementForTyping, + [elementId, moveFocus], + executionDeadline, + settleGraceMs + ) + : await execInPage( + target, + focusElementForTyping, + [elementId, moveFocus], + false, + executionDeadline + ) ) if ( !isRecordLike(prepared) || @@ -1178,6 +1251,43 @@ async function prepareTypingSurface( return prepared } +const TRANSIENT_PROBE_ERRORS = new Set(['stale', 'not-visible', 'not-editable']) +const SETTLE_GRACE_MS = 1_000 +const SETTLE_PROBE_INTERVAL_MS = 250 + +/** + * First-touch page probe with a short settle grace. A live view re-rendering + * under the agent (Slack's virtualized sidebar, its late-mounting composer) + * can transiently report an element as stale, invisible, or not yet editable + * while its replacement mounts one frame later. These call sites run before + * anything is dispatched, so a bounded reprobe safely turns that churn into a + * recovery instead of a dead ref. + */ +async function execInPageWithSettleGrace( + target: PageExecutionTarget, + fn: (...args: Args) => Result, + args: Args, + executionDeadline?: number, + graceMs = SETTLE_GRACE_MS +): Promise { + const graceDeadline = Date.now() + graceMs + for (;;) { + const result = await execInPage(target, fn, args, false, executionDeadline) + const code = + isRecordLike(result) && typeof result.error === 'string' ? String(result.error) : null + if ( + code === null || + !TRANSIENT_PROBE_ERRORS.has(code) || + Date.now() + SETTLE_PROBE_INTERVAL_MS > graceDeadline || + (executionDeadline !== undefined && + Date.now() + SETTLE_PROBE_INTERVAL_MS >= executionDeadline) + ) { + return result + } + await sleep(SETTLE_PROBE_INTERVAL_MS) + } +} + function pointFromPrepared(prepared: Record): { x: number; y: number } { return { x: prepared.x as number, y: prepared.y as number } } @@ -1194,6 +1304,29 @@ async function pageActionState( return toRecord(state) } +/** + * Which signals each tool accepts as proof its action reached the page. + * + * The tools deliberately do NOT share one predicate — the differences are real, + * and flattening them would make every tool wrong in a different direction: + * + * - `browser_drag` is the only tool that trusts `domChanged`, because a drop + * that reorders a list may change nothing else observable. Everywhere else + * background churn (Slack, Gmail) would forge success for an ignored action. + * - `browser_hover` ignores `fieldChanged` and `focusChanged`: hovering does not + * type or focus, so those would only ever be someone else's effect. + * - `browser_click` / `browser_click_at` count `focusChanged` only when the + * target was editable — otherwise a click that merely moved focus reads as + * success. + * - `targetChanged` requires a `targetState`, which `pageActionState` captures + * only when given an `elementId`. Tools without one (click_at, insert_text, + * drag, press_key) cannot use it; listing it there read as coverage they did + * not have, and it was silently always false. + * + * What IS shared is this function: every signal is computed here once, so a + * tool's formula is a statement about which evidence it trusts, not a private + * re-derivation of what changed. + */ function pageEffect( beforePage: Record, afterPage: Record, @@ -1522,7 +1655,7 @@ async function captureSnapshot(contents: WebContents, notAfter?: number): Promis invalidateSnapshot(state) const captureEpoch = state.snapshotCaptureEpoch const capturedTabId = tab.id - const capturedUrl = contents.getURL() + const capturedNavigationEpoch = navigationEpoch(contents) const targets = new Map() const targetLineIndexes = new Map() const stillCurrent = (): boolean => { @@ -1532,7 +1665,9 @@ async function captureSnapshot(contents: WebContents, notAfter?: number): Promis active?.id === capturedTabId && active.view.webContents === contents && !contents.isDestroyed() && - contents.getURL() === capturedUrl + // Same-document URL drift (SPA pushState) must not abort the capture; + // only a cross-document navigation makes the collected refs meaningless. + navigationEpoch(contents) === capturedNavigationEpoch ) } @@ -1817,7 +1952,8 @@ async function executeToolInner( } } assertCurrentExecution() - const tab = session.addAutomationTab() + // The agent chose to open this page to work in, so the panel follows it. + const tab = session.addAutomationTab({ reveal: true }) const contents = tab.view.webContents if (url) { assertCurrentExecution() @@ -1920,19 +2056,21 @@ async function executeToolInner( case 'browser_screenshot': { const contents = session.requireAutomationTab().view.webContents - const dataUrl = await cdp.captureScreenshot(contents).catch(() => null) - if (dataUrl === null) { + const shot = await cdp.captureScreenshot(contents).catch(() => null) + if (shot === null) { throw new ToolError( 'Could not capture the page. Use browser_snapshot or browser_read_text instead.' ) } - if (dataUrl.length > 8_000_000) { + if (shot.dataUrl.length > 8_000_000) { throw new ToolError( 'The screenshot result was too large to return safely. Use browser_snapshot or browser_read_text instead.' ) } const viewport = await execInPage(contents, getViewportInfo, []).catch(() => null) - return { dataUrl, viewport } + // scale maps image pixels back to CSS viewport pixels for the + // coordinate tools: cssX = imageX / scale. + return { dataUrl: shot.dataUrl, viewport, scale: shot.scale } } case 'browser_extract': { @@ -1957,11 +2095,10 @@ async function executeToolInner( if (!targetFrame) { assertCurrentExecution() const first = unwrapPageResult( - await execInPage( + await execInPageWithSettleGrace( target, clickElement, [elementId, false, false], - false, executionDeadline ) ) @@ -2010,7 +2147,12 @@ async function executeToolInner( assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) const focused = unwrapPageResult( - await execInPage(target, clickElement, [elementId, false, true], false, executionDeadline) + await execInPageWithSettleGrace( + target, + clickElement, + [elementId, false, true], + executionDeadline + ) ) if (!isRecordLike(focused)) throw new ToolError('Could not resolve that click target.') prepared = focused @@ -2244,10 +2386,21 @@ async function executeToolInner( ) const navigated = observation.effect.urlChanged || topObservation.effect.urlChanged || tabChanged - const obstructedAfterNavigation = navigated && dialogs.length > 0 + // Only a dialog that ARRIVED with the navigation obstructs it. Comparing + // against the union of what was already open stops the false positive + // that fires on every SPA route change under a persistent `role=dialog` + // (a cookie banner, a side drawer, an emoji picker) — those are not + // blocking anything, and reporting them made successful clicks read as + // failures. + const dialogsBefore = new Set([ + ...(Array.isArray(beforePage.dialogs) ? beforePage.dialogs.map(String) : []), + ...(Array.isArray(beforeTopPage.dialogs) ? beforeTopPage.dialogs.map(String) : []), + ]) + const newDialogs = dialogs.filter((dialog) => !dialogsBefore.has(dialog)) + const obstructedAfterNavigation = navigated && newDialogs.length > 0 const notes: string[] = [] if (obstructedAfterNavigation) { - notes.push('The page navigated, but a dialog is still open above it.') + notes.push(`The page navigated, but a dialog opened above it (${newDialogs.join(', ')}).`) } if (!effectObserved) { notes.push( @@ -2292,7 +2445,16 @@ async function executeToolInner( // synthetic value-setter when CDP is unavailable. assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) - const initialSurface = await prepareTypingSurface(target, elementId, true, executionDeadline) + // The settle grace covers a composer whose real contenteditable mounts a + // beat after the surrounding view renders (Slack); nothing has been + // dispatched yet, so reprobing is safe. + const initialSurface = await prepareTypingSurface( + target, + elementId, + true, + executionDeadline, + SETTLE_GRACE_MS + ) assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) if (targetFrame) { @@ -2591,9 +2753,10 @@ async function executeToolInner( const requestedKey = requireStr(params, 'key') const combo = parseKeyCombo(requestedKey) const contents = session.requireAutomationTab().view.webContents - const pressedPageUrl = contents.getURL() + const pressedNavigationEpoch = navigationEpoch(contents) let target: PageExecutionTarget = focusedPageTarget(contents) - let pressedFrameUrl = target === contents ? undefined : (target as WebFrameMain).url + let pressedFrameEpoch = + target === contents ? undefined : frameNavigationEpoch(contents, target as WebFrameMain) // Pasting would move the user's clipboard into the page, where the next // snapshot reports it as an ordinary field value — clipboards routinely // hold a password copied out of a password manager. Copy and cut would @@ -2612,7 +2775,8 @@ async function executeToolInner( const dispatchTarget = focusedPageTarget(contents) if (dispatchTarget !== target) { target = dispatchTarget - pressedFrameUrl = target === contents ? undefined : (target as WebFrameMain).url + pressedFrameEpoch = + target === contents ? undefined : frameNavigationEpoch(contents, target as WebFrameMain) beforePage = await pageActionState(target, true) beforeElement = await activeElementState(target) } @@ -2648,8 +2812,8 @@ async function executeToolInner( let fallbackState: Record = {} try { assertCurrentExecution() - assertActiveContents(contents, pressedPageUrl) - assertFocusedTargetUnchanged(contents, target, pressedFrameUrl) + assertActiveContents(contents, pressedNavigationEpoch) + assertFocusedTargetUnchanged(contents, target, pressedFrameEpoch) await dispatchKeyCombo(contents, combo) } catch (error) { if (error instanceof KeyDispatchError && error.keyDownDispatched) { @@ -2661,8 +2825,8 @@ async function executeToolInner( // CDP unavailable (debugger detached): synthetic DOM fallback. It // cannot trigger default editing actions, so say so in the result. assertCurrentExecution() - assertActiveContents(contents, pressedPageUrl) - assertFocusedTargetUnchanged(contents, target, pressedFrameUrl) + assertActiveContents(contents, pressedNavigationEpoch) + assertFocusedTargetUnchanged(contents, target, pressedFrameEpoch) const fallbackSecrecy = await execInPage(target, activeElementSecrecy, []).catch( () => 'opaque' ) @@ -2673,8 +2837,8 @@ async function executeToolInner( ) } assertCurrentExecution() - assertActiveContents(contents, pressedPageUrl) - assertFocusedTargetUnchanged(contents, target, pressedFrameUrl) + assertActiveContents(contents, pressedNavigationEpoch) + assertFocusedTargetUnchanged(contents, target, pressedFrameEpoch) const fallback = unwrapPageResult( await execInPage( target, @@ -2867,16 +3031,29 @@ async function executeToolInner( const elementId = requireNum(params, 'elementId') const target = pageTargetForElement(contents, elementId) const targetFrame = frameExecutionTarget(target, contents) - const beforePage = await pageActionState(target, true, elementId) - const beforeElement = await activeElementState(target) - const beforeTopPage = targetFrame ? await pageActionState(contents, true) : beforePage - const beforeTopElement = targetFrame ? await activeElementState(contents) : beforeElement + let beforePage = await pageActionState(target, true, elementId) + let beforeElement = await activeElementState(target) + let beforeTopPage = targetFrame ? await pageActionState(contents, true) : beforePage + let beforeTopElement = targetFrame ? await activeElementState(contents) : beforeElement + // Preparing the surface scrolls the element into view, so a baseline + // taken before it always reports scrollChanged — the tool's own probe, + // not the hover's effect. That pinned every unproductive hover to + // "background churn" instead of the honest "nothing happened", and hid + // real scrolling caused by the hover itself. Re-baseline once the scroll + // has settled and before the pointer moves. + const rebaseline = async (): Promise => { + beforePage = await pageActionState(target, true, elementId) + beforeElement = await activeElementState(target) + beforeTopPage = targetFrame ? await pageActionState(contents, true) : beforePage + beforeTopElement = targetFrame ? await activeElementState(contents) : beforeElement + } let trusted = false let result: unknown if (!targetFrame) { assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) let prepared = await prepareElementSurface(target, elementId, executionDeadline, true) + await rebaseline() let stable = false for (let attempt = 0; attempt < 2; attempt++) { assertCurrentExecution() @@ -2900,6 +3077,7 @@ async function executeToolInner( assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) let surface = await prepareElementSurface(target, elementId, executionDeadline, true) + await rebaseline() assertCurrentExecution() assertElementActionCurrent(contents, elementId, target) let embedding = await assertFrameEmbeddingVisible( @@ -3000,9 +3178,332 @@ async function executeToolInner( effectObserved, ...(!effectObserved ? { - note: possibleEffectObserved - ? 'Only background DOM/title churn followed the hover; a tooltip/menu was not confirmed.' - : 'No tooltip, menu, focus, or other strong hover effect was observed.', + // A capped scan cannot claim nothing appeared: overlays are + // commonly portalled to the END of , which is exactly the + // part a truncated walk misses. Say so instead of reporting a + // partial look with full confidence. + note: + afterPage.observationTruncated === true || + afterTopPage.observationTruncated === true + ? 'This page is too large to scan completely, so a tooltip or menu that opened may not have been seen. Confirm with browser_snapshot or browser_screenshot before concluding the hover did nothing.' + : possibleEffectObserved + ? 'Only background DOM/title churn followed the hover; a tooltip/menu was not confirmed.' + : 'No tooltip, menu, focus, or other strong hover effect was observed.', + } + : {}), + } + } + + case 'browser_click_at': { + const clickedTab = session.requireAutomationTab() + const contents = clickedTab.view.webContents + const x = requireNum(params, 'x') + const y = requireNum(params, 'y') + const clickCount = num(params, 'clickCount') ?? 1 + if (![1, 2, 3].includes(clickCount)) { + throw new ToolError('clickCount must be 1 (click), 2 (double-click), or 3 (triple-click).') + } + const clickNavigationEpoch = navigationEpoch(contents) + assertCurrentExecution() + assertActiveContents(contents) + const pointTarget = unwrapPageResult( + await execInPage(contents, describePointTarget, [x, y], false, executionDeadline) + ) + if (!isRecordLike(pointTarget) || pointTarget.found !== true) { + throw new ToolError( + 'Nothing is rendered at that point. Coordinates are CSS pixels in the current viewport — when reading them off a browser_screenshot, divide image pixels by its scale.' + ) + } + if (pointTarget.fileInput === true) { + throw new ToolError( + 'Refusing to click a file input because it opens a native chooser the browser agent cannot inspect or complete. Ask the user to upload the file themselves.' + ) + } + const beforePage = await pageActionState(contents, true) + const beforeElement = await activeElementState(contents) + assertCurrentExecution() + assertActiveContents(contents, clickNavigationEpoch) + try { + await cdp.clickAt(contents, x, y, true, clickCount) + } catch (error) { + throw new ToolError( + `Native click dispatch failed (${getErrorMessage(error)}). The action was not retried because a partial pointer press may already have reached the page. Take a fresh snapshot before continuing.` + ) + } + await sleep(150) + const afterElement = await activeElementState(contents) + const afterPage = await pageActionState(contents) + const observation = pageEffect(beforePage, afterPage, beforeElement, afterElement) + const activeTab = session.automationTab() + const tabChanged = activeTab?.id !== clickedTab.id + // targetChanged is deliberately absent: this tool has no elementId, so + // pageActionState captures no targetState and the term could only ever + // be false. Listing it read as coverage this tool does not have. + const effectObserved = + observation.effect.urlChanged || + observation.effect.dialogChanged || + observation.effect.popupChanged || + (pointTarget.editable === true && observation.effect.focusChanged) || + tabChanged + const dialogs = Array.isArray(afterPage.dialogs) ? afterPage.dialogs.map(String) : [] + const notes: string[] = [] + if (pointTarget.secret === true) { + notes.push( + 'The point resolves to a password field. Focusing it is fine, but typing there is refused — call browser_request_takeover for credentials.' + ) + } + if (pointTarget.crossOriginFrame === true) { + notes.push( + 'The point lands inside an embedded frame that could not be inspected; the click was dispatched but its target is unverified.' + ) + } + if (!effectObserved) { + notes.push( + observation.possibleEffectObserved || tabChanged + ? 'Only background DOM/title churn followed the click; inspect the page before treating it as successful.' + : 'No strong observable page change followed the click; inspect the page before treating it as successful.' + ) + } + return { + dispatched: true, + trusted: true, + activation: 'native-pointer', + clickedAt: { x, y }, + clickCount, + target: pointTarget.element, + targetCursor: pointTarget.cursor, + effectObserved, + possibleEffectObserved: observation.possibleEffectObserved || tabChanged, + effect: { ...observation.effect, tabChanged }, + dialogs, + ...(tabChanged && activeTab + ? { activeTab: { tabId: activeTab.id, url: activeTab.view.webContents.getURL() } } + : {}), + ...(notes.length > 0 ? { note: notes.join(' ') } : {}), + } + } + + case 'browser_insert_text': { + const text = requireStr(params, 'text') + const submit = params.submit === true + const contents = session.requireAutomationTab().view.webContents + const insertNavigationEpoch = navigationEpoch(contents) + const target: PageExecutionTarget = focusedPageTarget(contents) + const secrecy = await execInPage(target, activeElementSecrecy, []).catch(() => 'opaque') + if (secrecy === 'secret') throw new ToolError(PASSWORD_REFUSAL) + if (secrecy === 'opaque') { + throw new ToolError( + 'Focus is inside a cross-origin frame whose contents cannot be inspected, so this ' + + 'insertion could reach a password field. Call browser_request_takeover if the user needs to type here.' + ) + } + const focusState = unwrapPageResult( + await execInPage(target, describeFocusedEditable, [], false, executionDeadline) + ) + if (!isRecordLike(focusState) || focusState.editable !== true) { + const reason = isRecordLike(focusState) ? String(focusState.reason || '') : '' + // Name the element that actually held focus. Without it the agent + // cannot tell "I focused the wrong thing" from "this tool cannot type + // here", and it retries variations of the same failing approach. + const focused = isRecordLike(focusState) + ? [ + focusState.focusedTag ? `<${String(focusState.focusedTag)}>` : '', + focusState.focusedRole ? `role="${String(focusState.focusedRole)}"` : '', + focusState.contentEditable && focusState.contentEditable !== 'unset' + ? `contenteditable="${String(focusState.contentEditable)}"` + : '', + ] + .filter(Boolean) + .join(' ') + : '' + throw new ToolError( + reason === 'none' + ? 'No element is focused. Click the field first (browser_click or browser_click_at), then insert text.' + : `The focused element does not accept text${reason ? ` (${reason})` : ''}${ + focused ? `; focus is on ${focused}` : '' + }. Click the field you want to type into, then insert text.` + ) + } + const beforePage = await pageActionState(target, true) + const beforeElement = await activeElementState(target) + // Observe the TOP document too when typing inside a frame. A submit that + // navigates the top page is invisible to a frame-scoped observation, so a + // successful send reported effectObserved: false. Newly reachable now + // that the focus check descends into frames at all. + const insertInFrame = target !== contents + const beforeTopPage = insertInFrame ? await pageActionState(contents, true) : beforePage + assertCurrentExecution() + assertActiveContents(contents, insertNavigationEpoch) + assertFocusedTargetUnchanged(contents, target) + try { + await cdp.insertText(contents, text) + } catch (error) { + throw new ToolError( + `Native text insertion failed (${getErrorMessage(error)}). Take a fresh snapshot before retrying.` + ) + } + let submitDispatched = false + if (submit) { + await sleep(25) + assertCurrentExecution() + assertActiveContents(contents, insertNavigationEpoch) + try { + await dispatchKeyCombo(contents, parseKeyCombo('Enter')) + submitDispatched = true + } catch { + // Reported below through submitDispatched: false. + } + } + await sleep(150) + const state = await activeElementState(target) + const afterPage = await pageActionState(target) + const observation = pageEffect(beforePage, afterPage, beforeElement, state) + const topObservation = insertInFrame + ? pageEffect(beforeTopPage, await pageActionState(contents, true), beforeElement, state) + : observation + // targetChanged is deliberately absent: this tool has no elementId, so + // pageActionState captures no targetState and the term could only ever + // be false. Listing it read as coverage this tool does not have. + const effectObserved = + observation.effect.fieldChanged || + observation.effect.urlChanged || + observation.effect.dialogChanged || + topObservation.effect.urlChanged || + topObservation.effect.dialogChanged + return { + dispatched: true, + trusted: true, + kind: focusState.kind, + insertedChars: text.length, + ...state, + effectObserved, + possibleEffectObserved: observation.possibleEffectObserved, + effect: observation.effect, + submitRequested: submit, + submitDispatched, + ...(focusState.kind === 'canvas' || focusState.kind === 'textbox-role' + ? { + note: 'The focused editor is canvas/model-backed, so field readback cannot confirm the text — verify visually with browser_screenshot.', + } + : !effectObserved + ? { + note: 'Insertion produced no observable field or page change; inspect the page before continuing.', + } + : {}), + } + } + + case 'browser_drag': { + const draggedTab = session.requireAutomationTab() + const contents = draggedTab.view.webContents + const dragNavigationEpoch = navigationEpoch(contents) + + const resolveEndpoint = async ( + which: 'from' | 'to' + ): Promise<{ x: number; y: number; element?: string }> => { + const elementId = num(params, `${which}ElementId`) + if (elementId !== undefined) { + const target = pageTargetForElement(contents, elementId) + if (frameExecutionTarget(target, contents)) { + throw new ToolError( + `Dragging elements inside embedded frames is not supported. Use ${which}X/${which}Y viewport coordinates instead.` + ) + } + const prepared = unwrapPageResult( + await execInPageWithSettleGrace( + target, + clickElement, + [elementId, false, false], + executionDeadline + ) + ) + if ( + !isRecordLike(prepared) || + typeof prepared.x !== 'number' || + typeof prepared.y !== 'number' + ) { + throw new ToolError(`Could not resolve the ${which} element for the drag.`) + } + return { + x: prepared.x, + y: prepared.y, + ...(typeof prepared.element === 'string' ? { element: prepared.element } : {}), + } + } + const pointX = num(params, `${which}X`) + const pointY = num(params, `${which}Y`) + if (pointX === undefined || pointY === undefined) { + throw new ToolError( + `Provide either ${which}ElementId or both ${which}X and ${which}Y for the drag ${which === 'from' ? 'source' : 'target'}.` + ) + } + const probe = unwrapPageResult( + await execInPage( + contents, + describePointTarget, + [pointX, pointY], + false, + executionDeadline + ) + ) + if (!isRecordLike(probe) || probe.found !== true) { + throw new ToolError( + `Nothing is rendered at the ${which} point. Coordinates are CSS pixels in the current viewport — when reading them off a browser_screenshot, divide image pixels by its scale.` + ) + } + return { + x: pointX, + y: pointY, + ...(typeof probe.element === 'string' ? { element: probe.element } : {}), + } + } + + assertCurrentExecution() + assertActiveContents(contents) + const from = await resolveEndpoint('from') + const to = await resolveEndpoint('to') + if (Math.abs(from.x - to.x) < 1 && Math.abs(from.y - to.y) < 1) { + throw new ToolError('The drag source and target are the same point; nothing to drag.') + } + const beforePage = await pageActionState(contents, true) + const beforeElement = await activeElementState(contents) + assertCurrentExecution() + assertActiveContents(contents, dragNavigationEpoch) + let interception: { nativeDragIntercepted: boolean } + try { + interception = await cdp.dragPointer(contents, from, to) + } catch (error) { + throw new ToolError( + `Native drag dispatch failed (${getErrorMessage(error)}). The pointer may have been mid-drag; take a fresh snapshot to see the page's current state before retrying.` + ) + } + await sleep(200) + const afterElement = await activeElementState(contents) + const afterPage = await pageActionState(contents) + const observation = pageEffect(beforePage, afterPage, beforeElement, afterElement) + // targetChanged is deliberately absent: this tool has no elementId, so + // pageActionState captures no targetState and the term could only ever be + // false. domChanged IS trusted here — unlike every other tool — because a + // drop that reorders a list may change nothing else observable. + const effectObserved = + observation.effect.domChanged || + observation.effect.urlChanged || + observation.effect.dialogChanged || + observation.effect.scrollChanged + const dialogs = Array.isArray(afterPage.dialogs) ? afterPage.dialogs.map(String) : [] + return { + dispatched: true, + trusted: true, + nativeHtml5Drag: interception.nativeDragIntercepted, + from, + to, + effectObserved, + possibleEffectObserved: observation.possibleEffectObserved, + effect: observation.effect, + dialogs, + ...(!effectObserved + ? { + note: 'No observable page change followed the drag. Verify with browser_snapshot or browser_screenshot; some drop targets only commit on their own animation frame.', } : {}), } diff --git a/apps/desktop/src/main/browser-agent/page-functions.test.ts b/apps/desktop/src/main/browser-agent/page-functions.test.ts index 52a84235db0..847364b15d7 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.test.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.test.ts @@ -5,6 +5,8 @@ import { activeElementSecrecy, clickElement, collectSnapshot, + describeFocusedEditable, + describePointTarget, focusElementForTyping, getViewportInfo, hoverElement, @@ -155,6 +157,8 @@ describe('serialization contract', () => { ['readPageText', readPageText, []], ['pageContainsText', pageContainsText, ['needle']], ['getViewportInfo', getViewportInfo, []], + ['describePointTarget', describePointTarget, [10, 10]], + ['describeFocusedEditable', describeFocusedEditable, []], ] it.each(cases)('%s is self-contained', (_name, fn, args) => { @@ -257,8 +261,11 @@ describe('secret-field detection', () => { expect(typeIntoElement(0, 'change', false)).toEqual({ error: 'readonly' }) expect(focusElementForTyping(1)).toEqual({ error: 'disabled' }) expect(typeIntoElement(1, 'change', false)).toEqual({ error: 'disabled' }) - expect(focusElementForTyping(2)).toEqual({ error: 'not-editable' }) - expect(typeIntoElement(2, 'change', false)).toEqual({ error: 'not-editable' }) + expect(focusElementForTyping(2)).toMatchObject({ error: 'not-editable', elementTag: 'input' }) + expect(typeIntoElement(2, 'change', false)).toMatchObject({ + error: 'not-editable', + elementTag: 'input', + }) }) it('detects a password field reached through a same-origin iframe', () => { @@ -379,7 +386,12 @@ describe('combobox typing surfaces', () => { for (const input of Array.from(document.querySelectorAll('input'))) visible(input) register(ambiguous, secret) - expect(focusElementForTyping(0)).toEqual({ error: 'ambiguous-editable' }) + // The candidate list is the whole point of this error — it is the only + // thing that lets the agent pick a narrower target. + expect(focusElementForTyping(0)).toMatchObject({ + error: 'ambiguous-editable', + candidates: expect.arrayContaining([expect.stringContaining('input')]), + }) expect(focusElementForTyping(1)).toEqual({ error: 'password' }) expect(typeIntoElement(1, 'nope', false)).toEqual({ error: 'password' }) }) @@ -589,8 +601,11 @@ describe('collectSnapshot', () => { }) expect(card.contains(nestedButton)).toBe(true) + // Nothing is covering the card — its own button owns the point. Reporting + // this as an obstruction told the agent to close an overlay that does not + // exist; the recovery is to target the nested control instead. expect(clickElement(ref, false)).toEqual({ - error: 'obstructed', + error: 'nested-control', blocker: 'Delete channel', }) }) @@ -769,6 +784,38 @@ describe('collectSnapshot', () => { expect(focusElementForTyping(ref)).toEqual({ error: 'stale' }) }) + it('keeps a connected ref usable after a same-document URL change', () => { + document.body.innerHTML = '' + const button = visible(document.querySelector('button') as HTMLButtonElement) + const ref = refFor(outlineOf(collectSnapshot()), 'Send') + let clicked = false + button.addEventListener('click', () => { + clicked = true + }) + + window.history.pushState({}, '', '/client/T123/C456') + + expect(clickElement(ref)).toMatchObject({ dispatched: true, refRecovered: false }) + expect(clicked).toBe(true) + }) + + it('recovers a replaced ref after a same-document URL change', () => { + document.body.innerHTML = '' + const original = visible(document.querySelector('button') as HTMLButtonElement) + const ref = refFor(outlineOf(collectSnapshot()), 'Messages') + const replacement = visible(original.cloneNode(true) as HTMLButtonElement) + let clicked = false + replacement.addEventListener('click', () => { + clicked = true + }) + + window.history.pushState({}, '', '/client/T123/C456') + original.replaceWith(replacement) + + expect(clickElement(ref)).toMatchObject({ dispatched: true, refRecovered: true }) + expect(clicked).toBe(true) + }) + it('refuses to recover a ref when replacement is ambiguous', () => { document.body.innerHTML = '' const original = visible(document.querySelector('button') as HTMLButtonElement) @@ -816,6 +863,28 @@ describe('collectSnapshot', () => { expect(clickElement(secondRef)).toMatchObject({ dispatched: true }) }) + // The exact shape of the reported failure: hovering a Slack message mounts an + // action bar, but it is a role="toolbar"/"group" — none of the three roles the + // popup scan used to match. The hover therefore observed no popup change, no + // target change, and so no effect at all, and the agent concluded hovering + // did not work and fell back to clicking pixels off screenshots. + it('sees a row action bar that mounts on hover', () => { + document.body.innerHTML = '
Hello
' + visible(document.querySelector('[data-testid="message"]') as HTMLElement) + + const before = readPageActionState(true) as { popups: string[] } + expect(before.popups).toEqual([]) + + const toolbar = visible(document.createElement('div')) + toolbar.setAttribute('role', 'toolbar') + toolbar.setAttribute('aria-label', 'Message shortcuts') + document.body.append(toolbar) + + const after = readPageActionState(false) as { popups: string[] } + expect(after.popups).toEqual(['Message shortcuts']) + expect(after.popups).not.toEqual(before.popups) + }) + it('reports a targeted control semantic disappearance after its panel closes', () => { document.body.innerHTML = ` @@ -1341,3 +1410,112 @@ describe('pressKeyOnPage', () => { expect(seen).toEqual(['a']) }) }) + +describe('describePointTarget', () => { + function pointAt(el: Element | null): void { + Object.defineProperty(document, 'elementFromPoint', { + configurable: true, + value: () => el, + }) + } + + it('describes the element at a viewport point', () => { + document.body.innerHTML = '' + pointAt(document.querySelector('button')) + + expect(describePointTarget(10, 10)).toMatchObject({ + found: true, + tag: 'button', + element: 'button "Send message"', + editable: false, + fileInput: false, + secret: false, + }) + }) + + it('flags file inputs and password fields at the point', () => { + document.body.innerHTML = '' + pointAt(document.querySelector('input')) + expect(describePointTarget(10, 10)).toMatchObject({ found: true, fileInput: true }) + + document.body.innerHTML = '' + pointAt(document.querySelector('input')) + expect(describePointTarget(10, 10)).toMatchObject({ + found: true, + secret: true, + editable: true, + }) + }) + + it('rejects points outside the viewport', () => { + expect(describePointTarget(-5, 10)).toEqual({ error: 'outside-viewport' }) + expect(describePointTarget(10, window.innerHeight + 5)).toEqual({ + error: 'outside-viewport', + }) + }) +}) + +describe('describeFocusedEditable', () => { + it('reports no focus when the body holds focus', () => { + setActiveElement(document, document.body) + expect(describeFocusedEditable()).toEqual({ editable: false, reason: 'none' }) + }) + + // The bug this pins: focus inside a same-origin frame surfaces on the outer + // document as the FRAME element, which is not an input, not contentEditable, + // not a canvas, and carries no textbox role — so a composer that press-key + // typed into fine was reported `not-editable` and insert_text refused. The + // descent here must match activeElementReadback's exactly. + it('descends a same-origin frame to the editable that really holds focus', () => { + document.body.innerHTML = '' + const frame = document.createElement('iframe') + document.body.append(frame) + const inner = frame.contentDocument as Document + inner.body.innerHTML = '
composer
' + const composer = inner.querySelector('div') as HTMLElement + Object.defineProperty(composer, 'isContentEditable', { value: true, configurable: true }) + setActiveElement(inner, composer) + setActiveElement(document, frame) + + expect(describeFocusedEditable()).toEqual({ editable: true, kind: 'contenteditable' }) + }) + + it('names the focused element when it refuses, so the agent can recover', () => { + document.body.innerHTML = '
Send
' + setActiveElement(document, document.querySelector('div')) + + expect(describeFocusedEditable()).toEqual({ + editable: false, + reason: 'not-editable', + focusedTag: 'div', + focusedRole: 'button', + contentEditable: 'unset', + }) + }) + + it('reports a writable input as insertable', () => { + document.body.innerHTML = '' + setActiveElement(document, document.querySelector('input')) + expect(describeFocusedEditable()).toEqual({ editable: true, kind: 'input:text' }) + }) + + it('reports a read-only input as not insertable', () => { + document.body.innerHTML = '' + setActiveElement(document, document.querySelector('input')) + expect(describeFocusedEditable()).toEqual({ editable: false, reason: 'readonly' }) + }) + + it('reports a focused contenteditable editor as insertable', () => { + document.body.innerHTML = '
' + const editor = document.querySelector('div') as HTMLElement + Object.defineProperty(editor, 'isContentEditable', { get: () => true }) + setActiveElement(document, editor) + expect(describeFocusedEditable()).toEqual({ editable: true, kind: 'contenteditable' }) + }) + + it('treats a focused canvas editor surface as insertable', () => { + document.body.innerHTML = '' + setActiveElement(document, document.querySelector('canvas')) + expect(describeFocusedEditable()).toEqual({ editable: true, kind: 'canvas' }) + }) +}) diff --git a/apps/desktop/src/main/browser-agent/page-functions.ts b/apps/desktop/src/main/browser-agent/page-functions.ts index f5f4cb2f9a3..15c225511f9 100644 --- a/apps/desktop/src/main/browser-agent/page-functions.ts +++ b/apps/desktop/src/main/browser-agent/page-functions.ts @@ -34,6 +34,8 @@ declare global { root: Node observer: MutationObserver revision: number + /** Roots already passed to observe(), so re-observing stays cheap. */ + observedRoots: WeakSet }> __simAgentNextElementId?: number } @@ -109,6 +111,7 @@ export function collectSnapshot(startingElementId = 0): unknown { '[onclick]', '[contenteditable="true"]', '[contenteditable=""]', + '[contenteditable="plaintext-only"]', ].join(', ') const landmarkSelector = [ 'nav', @@ -593,7 +596,7 @@ export function collectSnapshot(startingElementId = 0): unknown { /** * React commonly replaces a control's DOM node while preserving its - * semantics. Recover only when the old page URL and a strong semantic + * semantics. Recover only when the old page origin and a strong semantic * fingerprint still identify one candidate; a weak or ambiguous match is a * real stale ref, never permission to click something nearby. */ @@ -601,6 +604,19 @@ export function collectSnapshot(startingElementId = 0): unknown { const locator = locators[id] if (!locator) return null + // Origin, not full URL: a live SPA rewrites its path with pushState + // between snapshot and act (Slack does so continuously), while the + // element the model chose is often still the same mounted node. The + // origin still pins the document/frame; the role/name/attribute and + // ancestor/context signatures below pin the element itself. + const pageOriginOf = (url: string): string => { + try { + return new URL(url).origin + } catch { + return url + } + } + const stableAttributes = [ 'id', 'href', @@ -620,7 +636,7 @@ export function collectSnapshot(startingElementId = 0): unknown { const identityMatches = (candidate: Element, connected = false): boolean => { if ( candidate.tagName.toUpperCase() !== locator.tag || - pageUrlFor(candidate) !== locator.url || + pageOriginOf(pageUrlFor(candidate)) !== pageOriginOf(locator.url) || roleFor(candidate) !== locator.role ) { return false @@ -708,6 +724,29 @@ export function collectSnapshot(startingElementId = 0): unknown { if (isCurrentlyVisible(current)) return { element: current, recovered: false } } + // Past this point the original node is gone or hidden, so anything returned + // is a DIFFERENT node adopted by structural resemblance. Identity matching + // compares origins only — deliberately, so a pushState between snapshot and + // act does not kill every ref — but that same leniency let a ref to "More + // actions" on a row in one view rebind to the identical control in another + // view the app had since navigated to, and act on the wrong thing with no + // signal beyond `recovered: true`. + // + // A same-document path change means the view was swapped, so resemblance is + // no longer evidence of sameness. Refuse to adopt and report the ref stale: + // the caller re-snapshots, which is cheap and always correct. Revalidating + // the still-connected node above stays lenient — it is literally the node + // the model chose. + const pathOf = (url: string): string => { + try { + const parsed = new URL(url) + return `${parsed.origin}${parsed.pathname}` + } catch { + return url + } + } + if (pathOf(window.location.href) !== pathOf(locator.url)) return null + const reachable: Element[] = [] let candidateCount = 0 const collect = (root: ParentNode, depth = 0): void => { @@ -1028,7 +1067,7 @@ export function clickElement( addCandidate(el) for (const candidate of Array.from( el.querySelectorAll( - 'input, textarea, [contenteditable="true"], [contenteditable=""]' + 'input, textarea, [contenteditable="true"], [contenteditable=""], [contenteditable="plaintext-only"], [role="textbox"]' ) )) { addCandidate(candidate) @@ -1087,6 +1126,22 @@ export function clickElement( if (suggestionsCoverFocusedEditable()) { return { error: 'suggestions-open', blocker: blockerLabel(blocker) } } + // A hit INSIDE the requested element is not an overlay — it is the ref + // wrapping its own control (a row containing a button, a card containing a + // link). hitBelongsToTarget rejects both cases identically, so this was + // reported as "covered by X, close or move the overlay", advice that cannot + // be followed because there is nothing to close. Name it for what it is so + // the agent retargets instead of hunting a phantom overlay. + let nested = false + for (let current = blocker; current; current = composedParent(current)) { + if (current === el) { + nested = true + break + } + } + if (nested) { + return { error: 'nested-control', blocker: blockerLabel(blocker) } + } return { error: 'obstructed', blocker: blockerLabel(blocker) } } @@ -1236,7 +1291,12 @@ export function focusElementForTyping(id: number, moveFocus = true): unknown { tag === 'TEXTAREA' || (tag === 'INPUT' && ['text', 'search', 'email', 'url', 'tel', 'number', 'password'].includes(inputType)) || - (node as HTMLElement).isContentEditable + (node as HTMLElement).isContentEditable || + // An ARIA-only textbox. The snapshot already advertises these as + // `[textbox]` with a ref, and browser_insert_text accepts them, so + // refusing here meant one tool rejecting exactly what the outline told + // the model to type into and what its sibling would have accepted. + node.getAttribute('role') === 'textbox' ) { potentialEditables.push(node as HTMLElement) } @@ -1244,14 +1304,34 @@ export function focusElementForTyping(id: number, moveFocus = true): unknown { addEditable(el) for (const candidate of Array.from( el.querySelectorAll( - 'input, textarea, [contenteditable="true"], [contenteditable=""]' + 'input, textarea, [contenteditable="true"], [contenteditable=""], [contenteditable="plaintext-only"], [role="textbox"]' ) )) { addEditable(candidate) } const editables = Array.from(new Set(potentialEditables)) - if (editables.length === 0) return { error: 'not-editable' } - if (editables.length > 1) return { error: 'ambiguous-editable' } + if (editables.length === 0) { + // Describe the element instead of only refusing it. Without this the agent + // cannot tell "wrong ref" from "this tool cannot type here" and retries + // variations of the same failing call. + return { + error: 'not-editable', + elementTag: String(el.tagName || '').toLowerCase(), + ...(el.getAttribute('role') ? { elementRole: el.getAttribute('role') } : {}), + } + } + if (editables.length > 1) { + // The candidate list is right here; discarding it left the agent unable to + // pick a narrower target, which is the only recovery this error allows. + return { + error: 'ambiguous-editable', + candidates: editables.slice(0, 5).map((field) => { + const fieldTag = String(field.tagName || '').toLowerCase() + const label = field.getAttribute('aria-label') || field.getAttribute('placeholder') || '' + return label ? `${fieldTag} "${label}"` : fieldTag + }), + } + } const editable = editables[0] const editableTag = tagFor(editable) @@ -1762,7 +1842,12 @@ export function typeIntoElement(id: number, text: string, submit: boolean): unkn candidateTag === 'TEXTAREA' || (candidateTag === 'INPUT' && ['text', 'search', 'email', 'url', 'tel', 'number', 'password'].includes(inputType)) || - (node as HTMLElement).isContentEditable + (node as HTMLElement).isContentEditable || + // An ARIA-only textbox. The snapshot already advertises these as + // `[textbox]` with a ref, and browser_insert_text accepts them, so + // refusing here meant one tool rejecting exactly what the outline told + // the model to type into and what its sibling would have accepted. + node.getAttribute('role') === 'textbox' ) { potentialEditables.push(node as HTMLElement) } @@ -1770,14 +1855,34 @@ export function typeIntoElement(id: number, text: string, submit: boolean): unkn addEditable(el) for (const candidate of Array.from( el.querySelectorAll( - 'input, textarea, [contenteditable="true"], [contenteditable=""]' + 'input, textarea, [contenteditable="true"], [contenteditable=""], [contenteditable="plaintext-only"], [role="textbox"]' ) )) { addEditable(candidate) } const editables = Array.from(new Set(potentialEditables)) - if (editables.length === 0) return { error: 'not-editable' } - if (editables.length > 1) return { error: 'ambiguous-editable' } + if (editables.length === 0) { + // Describe the element instead of only refusing it. Without this the agent + // cannot tell "wrong ref" from "this tool cannot type here" and retries + // variations of the same failing call. + return { + error: 'not-editable', + elementTag: String(el.tagName || '').toLowerCase(), + ...(el.getAttribute('role') ? { elementRole: el.getAttribute('role') } : {}), + } + } + if (editables.length > 1) { + // The candidate list is right here; discarding it left the agent unable to + // pick a narrower target, which is the only recovery this error allows. + return { + error: 'ambiguous-editable', + candidates: editables.slice(0, 5).map((field) => { + const fieldTag = String(field.tagName || '').toLowerCase() + const label = field.getAttribute('aria-label') || field.getAttribute('placeholder') || '' + return label ? `${fieldTag} "${label}"` : fieldTag + }), + } + } const editable = editables[0] const tag = String(editable.tagName || '').toUpperCase() editable.scrollIntoView({ block: 'nearest', inline: 'nearest', behavior: 'instant' }) @@ -1859,9 +1964,37 @@ export function pressKeyOnPage( .some((token) => token === 'current-password' || token === 'new-password') } - const target = (document.activeElement as HTMLElement | null) ?? document.body + // Descend to what is really focused, like every other focus reader here. + // Without this the synthetic key lands on the shadow HOST or the