diff --git a/.agents/skills/add-column-type/SKILL.md b/.agents/skills/add-column-type/SKILL.md index e9a11707e43..1c54b12cb3a 100644 --- a/.agents/skills/add-column-type/SKILL.md +++ b/.agents/skills/add-column-type/SKILL.md @@ -149,7 +149,7 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not, - [ ] `migrateCellsTo` / `migrateCellsFrom` added if the stored bytes change - [ ] New metadata keys added to `TYPE_SPECIFIC_COLUMN_KEYS` + `FOREIGN_METADATA_VERB` - [ ] Unit tests for `coerce` / `isCompatibleWith` round-trips, verified to fail without the code -- [ ] Docs row added to `apps/docs/content/docs/en/tables/index.mdx` +- [ ] Docs row added to `apps/docs/content/docs/tables/index.mdx` ## Final Validation (Required) diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md index eea2d054ccf..d3d8e8f64ea 100644 --- a/.agents/skills/add-integration/SKILL.md +++ b/.agents/skills/add-integration/SKILL.md @@ -576,7 +576,7 @@ bun run deployment-config:check bun run docs:check ``` -This creates `apps/docs/content/docs/en/integrations/{service}.mdx` — one page per service carrying the block's Actions and, if it has one, its Triggers section. Never hand-edit generated pages; the only editable region is the `{/* MANUAL-CONTENT */}` block (see `scripts/README.md`). +This creates `apps/docs/content/docs/integrations/{service}.mdx` — one page per service carrying the block's Actions and, if it has one, its Triggers section. Never hand-edit generated pages; the only editable region is the `{/* MANUAL-CONTENT */}` block (see `scripts/README.md`). The docs generator refreshes `packages/deployment-config/src/integrations.json`, and the deployment config generator projects service-account provider IDs from that catalog plus the canonical OAuth diff --git a/.agents/skills/add-model/SKILL.md b/.agents/skills/add-model/SKILL.md index df17836a790..d418773ff2e 100644 --- a/.agents/skills/add-model/SKILL.md +++ b/.agents/skills/add-model/SKILL.md @@ -153,7 +153,7 @@ If the entry has `capabilities.thinking` or `capabilities.reasoningEffort`, it a - **Anthropic-family (`anthropic`, `azure-anthropic`) thinking models MUST declare `capabilities.thinking.streamed`** (`'full' | 'summary' | 'none'`). Verify against Anthropic's current thinking-display and streaming docs: visible thinking returned by the API is summarized, including when Sim opts models whose default display is `omitted` into `display: 'summarized'` on agent-events runs, so current Claude thinking models use `'summary'`. Use `'full'` only if future official API docs explicitly guarantee raw thinking deltas. `bun run agent-stream-docs:check` (CI) fails if the field is missing. - Other families usually omit the field and inherit the provider default in `getThinkingStreamVisibility` (Gemini/OpenAI → summaries; Bedrock/Meta → none; OpenAI-compatible vendors with documented reasoning fields → full deltas). Set it explicitly only when the model deviates from its family. -- After inserting the entry, run `bun run agent-stream-docs:generate` and commit the regenerated `apps/docs/content/docs/en/workflows/blocks/agent.mdx` — CI diffs it. +- After inserting the entry, run `bun run agent-stream-docs:generate` and commit the regenerated `apps/docs/content/docs/workflows/blocks/agent.mdx` — CI diffs it. - Include the `streamed` value (with its source URL) in the verification report when set. ### Wrong family entirely? diff --git a/.github/actions/docker-build/action.yml b/.github/actions/docker-build/action.yml index b8727501f52..72d90e8e8bb 100644 --- a/.github/actions/docker-build/action.yml +++ b/.github/actions/docker-build/action.yml @@ -21,10 +21,13 @@ inputs: required: true max-cache-size-mb: description: >- - Layer cache to retain after the post-job prune, in MB. Must stay above one + Layer cache to retain after this action prunes, in MB. Must stay above one build's working set (base + dependency layers + RUN --mount=type=cache dirs) or every build evicts what the next one needs. Falls back to the - small-image default below when empty. + small-image default in the prune step when empty — the fallback lives there + rather than here because callers pass this from a matrix field, and an unset + matrix key arrives as the empty string, which counts as "provided" and would + bypass an input `default:` entirely. required: false # Registry logins must precede this action. provenance/sbom stay off: attestation @@ -49,24 +52,16 @@ runs: PLATFORMS: ${{ inputs.platforms }} run: echo "value=${GITHUB_REPOSITORY##*/}/${FILE#./}/${PLATFORMS//\//-}" >> "$GITHUB_OUTPUT" - # max-cache-size-mb is what bounds the disk: BuildKit's default GC is - # time-based only (layers unused for 8 days), and setup-docker-builder skips - # pruning altogether when the value is empty. On a repo that builds this - # often nothing ever ages out, so the disks grew without limit — - # app.Dockerfile/linux-amd64 reached 351 GB inside a day, and realtime, whose - # image is under 300 MB, sat at 249 GB. Sticky disks bill at ~$0.51/GB-month, - # so that was real money for layers no build would ever read again. - # - # The fallback is here rather than an input `default:` because callers pass - # this from a matrix field, and an unset matrix key arrives as the empty - # string — which counts as "provided", so a `default:` would never apply and - # a row that forgot the field would silently go back to unbounded growth. + # This action does NOT bound the disk — see the prune step below. BuildKit's + # own GC is time-based only (layers unused for 8 days), and these disks are + # mounted many times a day, so nothing ever ages out: app.Dockerfile/linux-amd64 + # reached 351 GB inside a day of being created, and realtime, whose image is + # under 300 MB, sat at 249 GB. Sticky disks bill at ~$0.51/GB-month. - name: Set up Blacksmith builder if: inputs.provider == '' || inputs.provider == 'blacksmith' uses: useblacksmith/setup-docker-builder@a5256a73e30f09e37e3eceb8ca36043d17621d24 # v2 with: cache-key: ${{ steps.cache-key.outputs.value }} - max-cache-size-mb: ${{ inputs.max-cache-size-mb || '25600' }} - name: Build and push (Blacksmith) if: inputs.provider == '' || inputs.provider == 'blacksmith' @@ -80,6 +75,94 @@ runs: provenance: false sbom: false + # Bound the layer cache ourselves. setup-docker-builder v1 took a + # max-cache-size-mb input and pruned in its own post step, but the v2 rewrite + # dropped it — and GitHub only WARNS on an unknown composite input, so passing + # it to v2 silently did nothing for a day while the app disk sat at 200+ GB. + # + # This is v1's command verbatim (its dist/index.js pruneBuildkitCache), against + # the fixed address v2 itself uses for `buildctl du` and `debug workers`: + # sudo buildctl --addr tcp://127.0.0.1:1234 prune --all --keep-storage + # + # Note buildctl's --all is NOT `docker buildx prune --all`. Here it means + # "include internal/frontend references" (cache/manager.go: without it, records + # typed internal or frontend, and any ref shared with an external source, are + # skipped). It does not wipe the cache, and --keep-storage still caps what is + # retained -- it maps straight onto the modern MaxUsedSpace field, so it is the + # buildctl spelling of --max-used-space rather than a deprecated alias. + # `RUN --mount=type=cache` dirs are typed exec.cachemount and are reclaimed + # either way; --all is here because it is what v1 used and it prunes strictly + # more. Runs before the builder's post step, which is what commits the disk. + # + # Warn rather than fail: a cache that is too large is not worth failing a + # deploy over. The du either side is what makes a silent no-op visible — the + # failure mode that hid the v2 input regression in the first place. + - name: Prune the layer cache + if: (inputs.provider == '' || inputs.provider == 'blacksmith') && !cancelled() + shell: bash + env: + KEEP_MB: ${{ inputs.max-cache-size-mb || '25600' }} + run: | + addr='tcp://127.0.0.1:1234' + + # A zero or non-numeric value is NOT a no-op. buildctl parses + # --keep-storage as a float, and BuildKit's cache manager treats + # keepBytes==0 as "no cap" (`gcMode := opt.keepBytes != 0`), pruning + # everything eligible rather than trimming to a limit. A typo such as + # '40GB' — valid in turbo.json, but this flag is a bare MB number — would + # silently empty the cache and make every later build cold, costing far + # more than the storage it saves. Refuse instead. + if ! [[ "$KEEP_MB" =~ ^[1-9][0-9]*$ ]]; then + echo "::warning::max-cache-size-mb must be a positive whole number of MB, got '${KEEP_MB}' — skipping prune rather than risk wiping the cache" + exit 0 + fi + + # Print the whole Total line rather than picking a column: buildctl's du + # table is whitespace-aligned and its layout is not a stable contract. + # + # The trailing `|| true` is load-bearing. Composite steps run under + # `bash -e -o pipefail`, where `cur="$(total)"` takes the substitution's + # exit status, so a failing du would abort the step and fail the build -- + # `echo "$(total)"` survives but the assignment in the settle loop does + # not. buildctl exiting non-zero here is entirely plausible: deleting a + # sticky disk out from under a running job makes buildkitd panic inside + # DiskUsage, and grep also exits 1 whenever the table has no Total line. + # Cache hygiene must never be able to fail a deploy. + total() { sudo buildctl --addr "$addr" du 2>/dev/null | grep -iE '^total:' | tr -s ' \t' ' ' || true; } + echo "before prune -> $(total)" + + if sudo buildctl --addr "$addr" prune --all --keep-storage "$KEEP_MB"; then + # buildctl prune returns BEFORE buildkitd has finished deleting + # (moby/buildkit#1198). The builder's post step then SIGTERMs buildkitd + # and SIGKILLs it after 30s (shutdownBuildkitd: `const a=3e4`); on + # SIGKILL it sets sigkillUsed and SKIPS the sticky disk commit, throwing + # away this run's cache and risking a corrupt bbolt metadata DB. So wait + # for du to stop moving before handing back. Bounded — this is hygiene, + # not correctness, and the steady-state trim settles almost at once. + prev=''; stable=0 + for _ in $(seq 1 60); do + cur="$(total)" + # An empty reading means du FAILED, never that the cache is empty: + # buildctl prints its `Total:` line unconditionally (cmd/buildctl + # diskusage.go), so an empty cache still reports `Total: 0B`. Without + # the -n guard the initial prev='' matched two empty readings and the + # loop exited after ~2s -- precisely when du is failing and the prune + # is most likely still deleting. Treat it as unstable and wait out the + # bound instead. + if [ -n "$cur" ] && [ "$cur" = "$prev" ]; then + stable=$((stable + 1)) + [ "$stable" -ge 2 ] && break + else + stable=0 + fi + prev="$cur" + sleep 2 + done + echo "after prune -> $(total) (keep-storage ${KEEP_MB} MB)" + else + echo "::warning::Layer cache prune failed; this sticky disk is unbounded for this run" + fi + - name: Set up Docker Buildx if: inputs.provider != '' && inputs.provider != 'blacksmith' uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa1be99ea97..c81d49b83f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -630,7 +630,7 @@ jobs: with: filters: | docs: - - 'apps/docs/content/docs/en/**' + - 'apps/docs/content/docs/**' - 'apps/sim/scripts/process-docs.ts' - 'apps/sim/lib/chunkers/**' diff --git a/.gitignore b/.gitignore index 6e9bdf2c99c..4371d6d73f7 100644 --- a/.gitignore +++ b/.gitignore @@ -86,7 +86,6 @@ start-collector.sh ## Helm Chart Tests helm/sim/test -i18n.cache ## Claude Code .claude/launch.json diff --git a/apps/desktop/src/main/browser-agent/cdp.test.ts b/apps/desktop/src/main/browser-agent/cdp.test.ts index 2f21c1ea6d9..f2cb1f6c0d7 100644 --- a/apps/desktop/src/main/browser-agent/cdp.test.ts +++ b/apps/desktop/src/main/browser-agent/cdp.test.ts @@ -548,6 +548,8 @@ describe('browser-agent screenshot capture', () => { expect(shot).toEqual({ dataUrl: `data:image/jpeg;base64,${Buffer.from('resized').toString('base64')}`, scale: 0.5, + viewport: { width: 2048, height: 1024 }, + imageSize: { width: 1024, height: 512 }, }) }) @@ -558,7 +560,12 @@ describe('browser-agent screenshot capture', () => { 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 }) + expect(shot).toEqual({ + dataUrl: 'data:image/jpeg;base64,c2lt', + scale: 0.5, + viewport: { width: 2048, height: 1024 }, + imageSize: { width: 1024, height: 512 }, + }) }) it('returns the raw capture when the image cannot be decoded', async () => { @@ -566,6 +573,102 @@ describe('browser-agent screenshot capture', () => { const shot = await captureScreenshot(contents) - expect(shot).toEqual({ dataUrl: 'data:image/jpeg;base64,c2lt', scale: 0.5 }) + expect(shot).toEqual({ + dataUrl: 'data:image/jpeg;base64,c2lt', + scale: 0.5, + viewport: { width: 2048, height: 1024 }, + imageSize: null, + }) }) + + it('does not expose deprecated device-pixel metrics as a CSS viewport', async () => { + const { contents } = captureFixture({ width: 1024, height: 512 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + + const shot = await captureScreenshot(contents) + + expect(shot.viewport).toBeNull() + expect(shot.imageSize).toEqual({ width: 1024, height: 512 }) + }) + + it('accepts stable finite scroll offsets around the capture', async () => { + const { contents } = captureFixture({ width: 1024, height: 512 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ + cssLayoutViewport: { + clientWidth: 2048, + clientHeight: 1024, + pageX: 12, + pageY: 34, + }, + }) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + + await expect(captureScreenshot(contents)).resolves.toMatchObject({ + viewport: { width: 2048, height: 1024 }, + imageSize: { width: 1024, height: 512 }, + }) + }) + + it.each([ + [ + 'dimensions', + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 } }, + { cssLayoutViewport: { clientWidth: 1024, clientHeight: 512 } }, + ], + [ + 'metric units', + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 } }, + { layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }, + ], + [ + 'horizontal scroll offset', + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 0, pageY: 20 } }, + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 10, pageY: 20 } }, + ], + [ + 'vertical scroll offset', + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 10, pageY: 20 } }, + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 10, pageY: 30 } }, + ], + [ + 'offset validity', + { cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024, pageX: 0, pageY: 0 } }, + { + cssLayoutViewport: { + clientWidth: 2048, + clientHeight: 1024, + pageX: 0, + pageY: Number.NaN, + }, + }, + ], + ['availability', {}, {}], + ])( + 'rejects a capture when viewport %s change during CDP capture', + async (_label, before, after) => { + const { contents } = captureFixture({ width: 1024, height: 512 }) + let metricsRead = 0 + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + metricsRead++ + return Promise.resolve(metricsRead === 1 ? before : after) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + + await expect(captureScreenshot(contents)).rejects.toThrow(/viewport changed/) + } + ) }) diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index 36dffe4da0d..75cf1adbc80 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -382,6 +382,71 @@ const SCREENSHOT_CAPTURE_QUALITY = 90 interface CdpViewport { clientWidth: number clientHeight: number + pageX?: number + pageY?: number +} + +interface ScreenshotViewportMetrics extends ScreenshotSize { + pageX: number | null + pageY: number | null + unit: 'css' | 'device' +} + +interface ScreenshotSize { + width: number + height: number +} + +export interface ScreenshotCapture { + dataUrl: string + scale: number + viewport: ScreenshotSize | null + imageSize: ScreenshotSize | null +} + +function screenshotViewportMetrics( + metrics: { + cssLayoutViewport?: CdpViewport + layoutViewport?: CdpViewport + } | null +): ScreenshotViewportMetrics | null { + const viewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport + const width = viewport?.clientWidth ?? 0 + const height = viewport?.clientHeight ?? 0 + if (width <= 0 || height <= 0) return null + const pageX = viewport?.pageX + const pageY = viewport?.pageY + const hasPagePosition = pageX !== undefined || pageY !== undefined + if ( + hasPagePosition && + (pageX === undefined || + pageY === undefined || + !Number.isFinite(pageX) || + !Number.isFinite(pageY)) + ) { + return null + } + return { + width, + height, + pageX: pageX ?? null, + pageY: pageY ?? null, + unit: metrics?.cssLayoutViewport ? 'css' : 'device', + } +} + +function sameScreenshotViewport( + before: ScreenshotViewportMetrics | null, + after: ScreenshotViewportMetrics | null +): boolean { + if (!before || !after) return false + return ( + before.unit === after.unit && + before.width === after.width && + before.height === after.height && + before.pageX === after.pageX && + before.pageY === after.pageY + ) } /** @@ -401,17 +466,18 @@ interface CdpViewport { * (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<{ dataUrl: string; scale: number }> { +export async function captureScreenshot(contents: WebContents): Promise { const metrics = await send<{ cssLayoutViewport?: CdpViewport layoutViewport?: CdpViewport }>(contents, 'Page.getLayoutMetrics').catch(() => null) - const viewport = metrics?.cssLayoutViewport ?? metrics?.layoutViewport - const width = viewport?.clientWidth ?? 0 - const height = viewport?.clientHeight ?? 0 + const captureViewport = screenshotViewportMetrics(metrics) + const width = captureViewport?.width ?? 0 + const height = captureViewport?.height ?? 0 + const cssWidth = metrics?.cssLayoutViewport?.clientWidth ?? 0 + const cssHeight = metrics?.cssLayoutViewport?.clientHeight ?? 0 + const cssViewport = cssWidth > 0 && cssHeight > 0 ? { width: cssWidth, height: cssHeight } : null const scale = width > 0 && height > 0 ? Math.min(1, MAX_SCREENSHOT_EDGE / Math.max(width, height)) : 1 @@ -419,26 +485,32 @@ export async function captureScreenshot( format: 'jpeg', quality: SCREENSHOT_CAPTURE_QUALITY, }) + const metricsAfterCapture = await send<{ + cssLayoutViewport?: CdpViewport + layoutViewport?: CdpViewport + }>(contents, 'Page.getLayoutMetrics').catch(() => null) + if (!sameScreenshotViewport(captureViewport, screenshotViewportMetrics(metricsAfterCapture))) { + throw new Error('The page viewport changed or could not be verified during screenshot capture') + } 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 === 0 || size.height === 0) { + return { dataUrl: captured, scale, viewport: cssViewport, imageSize: null } + } if (size.width === targetWidth && size.height === targetHeight) { - return { dataUrl: captured, scale } + return { dataUrl: captured, scale, viewport: cssViewport, imageSize: size } } const resized = image.resize({ width: targetWidth, height: targetHeight, quality: 'good' }) return { dataUrl: `data:image/jpeg;base64,${resized.toJPEG(SCREENSHOT_QUALITY).toString('base64')}`, scale, + viewport: cssViewport, + imageSize: { width: targetWidth, height: targetHeight }, } } diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index 84613c62d1a..ade81aff505 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -import { BrowserWindow, Menu } from 'electron' +import { BrowserWindow, Menu, nativeImage } from 'electron' import * as cdp from '@/main/browser-agent/cdp' import * as driverModule from '@/main/browser-agent/driver' import * as session from '@/main/browser-agent/session' @@ -1088,6 +1088,22 @@ describe('executeTool', () => { }) }) +describe('browserToolWatchdogMs', () => { + it.each([ + ['number', 30_000, 35_000], + ['numeric string', '30000', 35_000], + ['absent', undefined, 15_000], + ['non-numeric', 'soon', 15_000], + ['zero', 0, 15_000], + ['negative', -5_000, 15_000], + ['above the wait clamp', 500_000, 125_000], + ])('normalizes browser_wait_for timeout (%s)', (_label, timeoutMs, expected) => { + const params = timeoutMs === undefined ? {} : { timeoutMs } + + expect(driverModule.browserToolWatchdogMs('browser_wait_for', params)).toBe(expected) + }) +}) + /** * Trusted CDP input never enters the page, so a focused credential field can * only be ruled out in the driver. These cover that seam; the page-side @@ -1159,6 +1175,15 @@ describe('credential protection', () => { .mock.calls.filter(([called]) => called === method) } + function mockScreenshotImage(size: { width: number; height: number } | null): void { + vi.mocked(nativeImage.createFromBuffer).mockReturnValueOnce({ + isEmpty: vi.fn(() => size === null), + getSize: vi.fn(() => size ?? { width: 0, height: 0 }), + resize: vi.fn(() => ({ toJPEG: vi.fn(() => Buffer.from('resized')) })), + toJPEG: vi.fn(() => Buffer.alloc(0)), + } as unknown as ReturnType) + } + it('refuses a keystroke while a password field holds focus', async () => { const contents = await openPage() respondWith(contents, { activeElementSecrecy: 'secret' }) @@ -1167,6 +1192,8 @@ describe('credential protection', () => { expect(result.ok).toBe(false) expect(result.error).toMatch(/Refusing to act on a password field/) + expect(result.error).toMatch(/visible browser/) + expect(result.error).not.toContain('browser_request_takeover') expect(cdpCalls(contents, 'Input.dispatchKeyEvent')).toHaveLength(0) }) @@ -1532,6 +1559,24 @@ describe('credential protection', () => { }) }) + it('rejects an unsupported browser_scroll direction instead of treating it as down', async () => { + const contents = await openPage() + + const result = await driver.executeTool('chat-test', 'browser_scroll', { + direction: 'sideways', + }) + + expect(result).toMatchObject({ + ok: false, + error: 'Scroll direction must be "up" or "down".', + }) + expect( + vi + .mocked(contents.executeJavaScript) + .mock.calls.some(([expression]) => isPageCall(String(expression), 'scrollPage')) + ).toBe(false) + }) + it('confirms a click when the requested target changes semantic state', async () => { const contents = await openPage() let actionReads = 0 @@ -2179,6 +2224,83 @@ describe('credential protection', () => { expect(cdpCalls(contents, 'Input.insertText')).toHaveLength(1) }) + it('reports top-page effects observed after inserting text in a child frame', async () => { + const contents = await openPage() + const mainFrame = { + frameTreeNodeId: 1, + detached: false, + isDestroyed: vi.fn(() => false), + origin: 'https://example.com', + parent: null, + framesInSubtree: [] as unknown[], + } + const childFrame = { + frameTreeNodeId: 2, + detached: false, + isDestroyed: vi.fn(() => false), + origin: 'https://mail-widget.example', + parent: mainFrame, + url: 'https://mail-widget.example/compose', + } + mainFrame.framesInSubtree = [mainFrame, childFrame] + Object.defineProperty(contents, 'mainFrame', { configurable: true, value: mainFrame }) + Object.defineProperty(contents, 'focusedFrame', { configurable: true, value: childFrame }) + let topPageReads = 0 + vi.mocked(contents.executeJavaScript).mockImplementation((expression: string) => { + if (isPageCall(expression, 'readPageActionState')) { + topPageReads++ + return Promise.resolve({ + url: + topPageReads === 1 ? 'https://example.com/compose' : 'https://example.com/message/sent', + title: 'Mail', + focus: 'iframe', + mutationRevision: topPageReads, + dialogs: [], + scroll: [0], + }) + } + return Promise.resolve(undefined) + }) + const isolatedFrameEval = vi + .spyOn(cdp, 'evaluateInIsolatedFrame') + .mockImplementation((_contents, _frame, expression) => { + if (isPageCall(expression, 'activeElementSecrecy')) return Promise.resolve('safe') + if (isPageCall(expression, 'describeFocusedEditable')) { + return Promise.resolve({ editable: true, kind: 'input' }) + } + if (isPageCall(expression, 'readActiveElementState')) { + return Promise.resolve({ activeElement: 'input', valueLength: 4 }) + } + if (isPageCall(expression, 'readPageActionState')) { + return Promise.resolve({ + url: 'https://mail-widget.example/compose', + title: 'Compose', + focus: 'input', + mutationRevision: 0, + dialogs: [], + scroll: [0], + }) + } + return Promise.resolve(undefined) + }) + + try { + const result = await driver.executeTool('chat-test', 'browser_insert_text', { text: 'sent' }) + + expect(result.ok, result.error).toBe(true) + expect(result).toMatchObject({ + ok: true, + result: { + effectObserved: true, + possibleEffectObserved: true, + effect: { urlChanged: true }, + }, + }) + } finally { + isolatedFrameEval.mockRestore() + } + }) + it('refuses insertion when nothing editable holds focus', async () => { const contents = await openPage() respondWith(contents, { @@ -2272,6 +2394,7 @@ describe('credential protection', () => { it('returns the screenshot scale for coordinate mapping', async () => { const contents = await openPage() + mockScreenshotImage({ width: 1024, height: 512 }) vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { if (method === 'Page.getLayoutMetrics') { return Promise.resolve({ @@ -2287,6 +2410,229 @@ describe('credential protection', () => { const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) - expect(result).toMatchObject({ ok: true, result: { scale: 0.5 } }) + expect(result).toMatchObject({ + ok: true, + result: { + scale: 0.5, + viewport: { + url: 'https://example.com/login', + title: 'Example', + width: 2048, + height: 1024, + }, + }, + }) + expect( + vi + .mocked(contents.executeJavaScript) + .mock.calls.some(([expression]) => isPageCall(String(expression), 'getViewportInfo')) + ).toBe(false) + }) + + it('uses the in-page CSS viewport when CDP exposes only deprecated device metrics', async () => { + const contents = await openPage() + mockScreenshotImage({ width: 1024, height: 512 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) + } + if (method === 'Page.captureScreenshot') { + return Promise.resolve({ data: 'c2lt' }) + } + return Promise.resolve(undefined) + }) + respondWith(contents, { + getViewportInfo: { + url: 'https://example.com/login', + title: 'Example', + width: 1024, + height: 512, + }, + }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result).toMatchObject({ + ok: true, + result: { + scale: 1, + viewport: { + url: 'https://example.com/login', + title: 'Example', + width: 1024, + height: 512, + }, + }, + }) + if ( + !result.ok || + typeof result.result !== 'object' || + result.result === null || + !('scale' in result.result) || + typeof result.result.scale !== 'number' + ) { + throw new Error('browser_screenshot did not return a numeric coordinate scale') + } + expect(1024 / result.result.scale).toBe(1024) + expect( + vi + .mocked(contents.executeJavaScript) + .mock.calls.some(([expression]) => isPageCall(String(expression), 'getViewportInfo')) + ).toBe(true) + }) + + it('accepts stable truncated page identity with deprecated device metrics', async () => { + const contents = await openPage() + const fullUrl = `https://example.com/${'u'.repeat(5000)}` + const fullTitle = `Example ${'t'.repeat(600)}` + vi.mocked(contents.getURL).mockReturnValue(fullUrl) + vi.mocked(contents.getTitle).mockReturnValue(fullTitle) + mockScreenshotImage({ width: 1024, height: 512 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + respondWith(contents, { + getViewportInfo: { + url: fullUrl.slice(0, 4096), + title: fullTitle.slice(0, 500), + width: 1024, + height: 512, + }, + }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result).toMatchObject({ + ok: true, + result: { + scale: 1, + viewport: { + url: fullUrl.slice(0, 4096), + title: fullTitle.slice(0, 500), + width: 1024, + height: 512, + }, + }, + }) + }) + + it('rejects an undecodable screenshot instead of returning an unverified scale', async () => { + const contents = await openPage() + mockScreenshotImage(null) + 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 result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/verify the screenshot dimensions/) }) + + it('rejects a screenshot when no CSS viewport can be established', async () => { + const contents = await openPage() + mockScreenshotImage({ width: 1024, height: 512 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ layoutViewport: { clientWidth: 2048, clientHeight: 1024 } }) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + respondWith(contents, { getViewportInfo: null }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/verify the page viewport/) + }) + + it('rejects coordinate mapping when the viewport changes during capture', async () => { + const contents = await openPage() + mockScreenshotImage({ width: 1024, height: 256 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ layoutViewport: { clientWidth: 1024, clientHeight: 256 } }) + } + if (method === 'Page.captureScreenshot') return Promise.resolve({ data: 'c2lt' }) + return Promise.resolve(undefined) + }) + respondWith(contents, { + getViewportInfo: { + url: 'https://example.com/login', + title: 'Example', + width: 1024, + height: 512, + }, + }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/viewport changed while the screenshot was captured/) + }) + + it('rejects a screenshot when the document navigates during capture', async () => { + const contents = await openPage() + mockScreenshotImage({ width: 1024, height: 512 }) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ + cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, + }) + } + if (method === 'Page.captureScreenshot') { + emitContentsEvent(contents, 'did-navigate') + return Promise.resolve({ data: 'c2lt' }) + } + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/page changed while its screenshot was being captured/) + }) + + it.each(['url', 'title'] as const)( + 'rejects a screenshot when the page %s changes during capture', + async (identityField) => { + const contents = await openPage() + mockScreenshotImage({ width: 1024, height: 512 }) + const initialUrl = contents.getURL() + const initialTitle = contents.getTitle() + let currentUrl = initialUrl + let currentTitle = initialTitle + vi.mocked(contents.getURL).mockImplementation(() => currentUrl) + vi.mocked(contents.getTitle).mockImplementation(() => currentTitle) + vi.mocked(contents.debugger.sendCommand).mockImplementation((method: string) => { + if (method === 'Page.getLayoutMetrics') { + return Promise.resolve({ + cssLayoutViewport: { clientWidth: 2048, clientHeight: 1024 }, + }) + } + if (method === 'Page.captureScreenshot') { + if (identityField === 'url') currentUrl = 'https://example.com/changed' + else currentTitle = 'Changed title' + return Promise.resolve({ data: 'c2lt' }) + } + return Promise.resolve(undefined) + }) + + const result = await driver.executeTool('chat-test', 'browser_screenshot', {}) + + expect(result.ok).toBe(false) + expect(result.error).toMatch(/page changed while its screenshot was being captured/) + } + ) }) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index d8ce0631824..216ecd958de 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -24,6 +24,7 @@ import { type BrowserPanelAction, type BrowserTabsState, type BrowserToolName, + normalizeBrowserWaitForTimeoutMs, } from '@sim/browser-protocol' import type { BrowserDownloadsState, BrowserToolbarCommand } from '@sim/desktop-bridge' import { createLogger } from '@sim/logger' @@ -71,8 +72,6 @@ const logger = createLogger('BrowserAgentDriver') const NAVIGATION_TIMEOUT_MS = 25_000 const NAVIGATION_SETTLE_MS = 400 -const DEFAULT_WAIT_FOR_TIMEOUT_MS = 10_000 -const MAX_WAIT_FOR_TIMEOUT_MS = 120_000 const TAKEOVER_POLL_MS = 1_500 /** * Hard ceiling on any single tool execution (takeover excepted): whatever @@ -718,10 +717,9 @@ function num(params: Record, key: string): number | undefined { } /** - * Native execution must time out before the renderer gives up (30s default, - * 45s navigation, requested wait + 15s). Otherwise the abandoned native - * promise keeps owning the serialized queue and every later browser action - * times out behind it. + * Bounds native execution after a call reaches the head of its serialized + * scope queue. The renderer separately budgets authorization, queueing, and + * bridge delivery around this watchdog. */ export function browserToolWatchdogMs( tool: BrowserToolName, @@ -738,10 +736,7 @@ export function browserToolWatchdogMs( return NAVIGATION_TOOL_WATCHDOG_MS } if (tool === 'browser_wait_for') { - const requested = Math.min( - num(params, 'timeoutMs') ?? DEFAULT_WAIT_FOR_TIMEOUT_MS, - MAX_WAIT_FOR_TIMEOUT_MS - ) + const requested = normalizeBrowserWaitForTimeoutMs(params.timeoutMs) return requested + WAIT_FOR_TOOL_WATCHDOG_GRACE_MS } return DEFAULT_TOOL_WATCHDOG_MS @@ -889,12 +884,11 @@ function sanitizeBrowserResult( /** * Covers focusing, clicking, and typing: the agent has no legitimate reason to - * reach a credential field, and takeover is the sanctioned path when a task - * needs one. + * reach a credential field. The user can enter credentials directly in the + * visible embedded browser before the agent resumes from a fresh snapshot. */ const PASSWORD_REFUSAL = - 'Refusing to act on a password field. Call browser_request_takeover so the user ' + - 'can enter their credentials themselves.' + 'Refusing to act on a password field. Ask the user to enter their credentials in the visible browser, then take a fresh browser_snapshot.' /** Maps sentinel `{ error: ... }` results from injected functions to ToolErrors. */ function unwrapPageResult(result: unknown): unknown { @@ -2069,10 +2063,7 @@ async function executeToolInner( case 'browser_wait_for': { const text = str(params, 'text') - const timeoutMs = Math.min( - num(params, 'timeoutMs') ?? DEFAULT_WAIT_FOR_TIMEOUT_MS, - MAX_WAIT_FOR_TIMEOUT_MS - ) + const timeoutMs = normalizeBrowserWaitForTimeoutMs(params.timeoutMs) const startedAt = Date.now() if (!text) { await sleep(timeoutMs) @@ -2136,22 +2127,102 @@ async function executeToolInner( } case 'browser_screenshot': { - const contents = session.requireAutomationTab().view.webContents - const shot = await cdp.captureScreenshot(contents).catch(() => null) - if (shot === null) { + const capturedTab = session.requireAutomationTab() + const contents = capturedTab.view.webContents + const capturedNavigationEpoch = navigationEpoch(contents) + const capturedUrl = contents.getURL() + const capturedTitle = contents.getTitle() + const capturedViewportUrl = capturedUrl.slice(0, 4096) + const capturedViewportTitle = capturedTitle.slice(0, 500) + const captureIsCurrent = (): boolean => { + const activeTab = session.automationTab() + return ( + activeTab?.id === capturedTab.id && + activeTab.view.webContents === contents && + !contents.isDestroyed() && + navigationEpoch(contents) === capturedNavigationEpoch && + contents.getURL() === capturedUrl && + contents.getTitle() === capturedTitle + ) + } + const assertCaptureIsCurrent = (): void => { + if (captureIsCurrent()) return + throw new ToolError( + 'The page changed while its screenshot was being captured. Retry browser_screenshot before using image coordinates.' + ) + } + const shot = await cdp.captureScreenshot(contents).catch((error) => { + logger.warn('Browser screenshot capture failed', { error: getErrorMessage(error) }) + return null + }) + if (!shot) { throw new ToolError( 'Could not capture the page. Use browser_snapshot or browser_read_text instead.' ) } + assertCaptureIsCurrent() 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) + if (!shot.imageSize) { + throw new ToolError( + 'Could not verify the screenshot dimensions. Retry browser_screenshot or use browser_snapshot instead.' + ) + } + const viewport = shot.viewport + ? { + url: capturedViewportUrl, + title: capturedViewportTitle, + ...shot.viewport, + } + : await execInPage(contents, getViewportInfo, []).catch(() => null) + assertCaptureIsCurrent() + if ( + !shot.viewport && + isRecordLike(viewport) && + (viewport.url !== capturedViewportUrl || viewport.title !== capturedViewportTitle) + ) { + throw new ToolError( + 'The page changed while its screenshot viewport was being verified. Retry browser_screenshot before using image coordinates.' + ) + } + let scale = shot.scale + const viewportWidth = + isRecordLike(viewport) && typeof viewport.width === 'number' ? viewport.width : 0 + const viewportHeight = + isRecordLike(viewport) && typeof viewport.height === 'number' ? viewport.height : 0 + if ( + !Number.isFinite(viewportWidth) || + !Number.isFinite(viewportHeight) || + viewportWidth <= 0 || + viewportHeight <= 0 + ) { + throw new ToolError( + 'Could not verify the page viewport for this screenshot. Retry browser_screenshot or use browser_snapshot instead.' + ) + } + if (!shot.viewport) { + const widthScale = shot.imageSize.width / viewportWidth + const heightScale = shot.imageSize.height / viewportHeight + const scaleDelta = Math.abs(widthScale - heightScale) + if ( + !Number.isFinite(widthScale) || + !Number.isFinite(heightScale) || + widthScale <= 0 || + heightScale <= 0 || + scaleDelta > Math.max(widthScale, heightScale) * 0.02 + ) { + throw new ToolError( + 'The page viewport changed while the screenshot was captured. Retry browser_screenshot before using image coordinates.' + ) + } + scale = widthScale + } // scale maps image pixels back to CSS viewport pixels for the // coordinate tools: cssX = imageX / scale. - return { dataUrl: shot.dataUrl, viewport, scale: shot.scale } + return { dataUrl: shot.dataUrl, viewport, scale } } case 'browser_extract': { @@ -2928,8 +2999,8 @@ async function executeToolInner( if (secrecy === 'opaque' && !safeForOpaqueFocus) { throw new ToolError( 'Focus is inside a cross-origin frame whose contents cannot be inspected, so this ' + - 'keystroke could mutate or activate a password field. Call browser_request_takeover if the ' + - 'user needs to type here.' + 'keystroke could mutate or activate a password field. Ask the user to type in the visible ' + + 'browser, then take a fresh browser_snapshot.' ) } let trusted = true @@ -3051,6 +3122,10 @@ async function executeToolInner( } case 'browser_scroll': { + const direction = requireStr(params, 'direction') + if (direction !== 'up' && direction !== 'down') { + throw new ToolError('Scroll direction must be "up" or "down".') + } const contents = session.requireAutomationTab().view.webContents const elementId = num(params, 'elementId') const target = @@ -3071,7 +3146,7 @@ async function executeToolInner( await execInPage( target, scrollPage, - [requireStr(params, 'direction'), num(params, 'amount'), elementId], + [direction, num(params, 'amount'), elementId], false, executionDeadline ) @@ -3379,7 +3454,7 @@ async function executeToolInner( 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.' + 'The point resolves to a password field. Focusing it is fine, but typing there is refused — ask the user to enter credentials in the visible browser, then take a fresh browser_snapshot.' ) } if (pointTarget.crossOriginFrame === true) { @@ -3424,7 +3499,8 @@ async function executeToolInner( 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.' + 'insertion could reach a password field. Ask the user to type in the visible browser, then ' + + 'take a fresh browser_snapshot.' ) } const focusState = unwrapPageResult( @@ -3491,15 +3567,15 @@ async function executeToolInner( const topObservation = insertInFrame ? pageEffect(beforeTopPage, await pageActionState(contents, true), beforeElement, state) : observation + const effect: Record = { + ...observation.effect, + urlChanged: observation.effect.urlChanged || topObservation.effect.urlChanged, + dialogChanged: observation.effect.dialogChanged || topObservation.effect.dialogChanged, + } // 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 + const effectObserved = effect.fieldChanged || effect.urlChanged || effect.dialogChanged return { dispatched: true, trusted: true, @@ -3507,8 +3583,9 @@ async function executeToolInner( insertedChars: text.length, ...state, effectObserved, - possibleEffectObserved: observation.possibleEffectObserved, - effect: observation.effect, + possibleEffectObserved: + observation.possibleEffectObserved || topObservation.possibleEffectObserved, + effect, submitRequested: submit, submitDispatched, ...(focusState.kind === 'canvas' || focusState.kind === 'textbox-role' @@ -3676,6 +3753,7 @@ export async function executeTool( toolCallId?: string, authorizationBoundary?: BrowserToolQueueBoundary ): Promise<{ ok: boolean; result?: unknown; error?: string }> { + const queuedAt = Date.now() const resolvedScopeId = resolveDriverScopeId(scopeId) if (session.isBrowserScopeSuspended(resolvedScopeId)) { return { @@ -3688,6 +3766,8 @@ export async function executeTool( const invocationEpoch = ++state.toolInvocationEpoch const queueCancellationEpoch = state.toolQueueCancellationEpoch const run = async () => { + const queueWaitMs = Date.now() - queuedAt + const executionStartedAt = Date.now() if ( (authorizationBoundary && !isBrowserToolQueueBoundaryCurrent(authorizationBoundary)) || queueCancellationEpoch !== state.toolQueueCancellationEpoch || @@ -3702,7 +3782,12 @@ export async function executeTool( }) state.activeToolCancel = cancelActiveExecution return await session.withBrowserScope(resolvedScopeId, async () => { - logger.info('Executing browser tool', { tool, scopeId: resolvedScopeId }) + logger.info('Executing browser tool', { + tool, + toolCallId, + scopeId: resolvedScopeId, + queueWaitMs, + }) const keepHiddenPageActive = tool !== 'browser_request_takeover' if (keepHiddenPageActive) { session.setAutomationActive(true) @@ -3732,7 +3817,15 @@ export async function executeTool( invalidateSnapshot(state) } }) - return withNotices(await Promise.race([guardedExecution, cancellation])) + const result = withNotices(await Promise.race([guardedExecution, cancellation])) + logger.info('Browser tool completed', { + tool, + toolCallId, + scopeId: resolvedScopeId, + queueWaitMs, + executionMs: Date.now() - executionStartedAt, + }) + return result } finally { if (keepHiddenPageActive) { session.setAutomationActive(false) @@ -3757,7 +3850,13 @@ export async function executeTool( invalidateSnapshot(state) } const message = String(sanitizeBrowserResult(getErrorMessage(error), undefined, 0, 'error')) - logger.warn('Browser tool failed', { tool, error: message }) + logger.warn('Browser tool failed', { + tool, + toolCallId, + scopeId: resolvedScopeId, + totalMs: Date.now() - queuedAt, + error: message, + }) return { ok: false, error: message } } } diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index b8bfc8cc8c2..118288347a2 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -799,6 +799,45 @@ describe('registerIpcHandlers', () => { cancelActive.mockRestore() }) + it('rejects browser tools whose server authorization exceeds its execution budget', async () => { + const { invoke } = collectHandlers() + const executeTool = vi.spyOn(browserDriver, 'executeTool') + const authorizationController = new AbortController() + const timeout = vi.spyOn(AbortSignal, 'timeout').mockReturnValue(authorizationController.signal) + const fetchAuthorization = vi.fn((_url: string, request?: RequestInit) => { + const signal = request?.signal + return new Promise((_resolve, reject) => { + signal?.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + }) + const delayedEvent = { + senderFrame: { url: `${APP}/workspace/ws1` }, + sender: { session: { fetch: fetchAuthorization } }, + } + + const execution = invoke.get('browser-agent:execute-tool')?.( + delayedEvent, + 'tool-stalled-authorization', + 'browser_snapshot', + {}, + 'chat-stalled-authorization' + ) + authorizationController.abort(new DOMException('timed out', 'TimeoutError')) + + await expect(execution).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('authorized pending Copilot tool call'), + }) + expect(fetchAuthorization).toHaveBeenCalledWith( + `${APP}/api/desktop/tool/authorize`, + expect.objectContaining({ signal: expect.any(AbortSignal) }) + ) + expect(timeout).toHaveBeenCalledWith(8_000) + expect(executeTool).not.toHaveBeenCalled() + timeout.mockRestore() + executeTool.mockRestore() + }) + it('rejects a browser tool when the renderer claims a different scope than authorization', async () => { const { invoke } = collectHandlers() const handler = invoke.get('browser-agent:execute-tool') @@ -821,6 +860,40 @@ describe('registerIpcHandlers', () => { }) }) + it('rejects a retired browser tool even if authorization echoes it', async () => { + const { invoke } = collectHandlers() + const executeTool = vi.spyOn(browserDriver, 'executeTool') + const authorizedEvent = { + senderFrame: { url: `${APP}/workspace/ws1` }, + sender: { + session: { + fetch: vi.fn(async () => + Response.json({ + chatId: 'chat-1', + toolName: 'browser_request_takeover', + args: { reason: 'Legacy handoff' }, + }) + ), + }, + }, + } + + await expect( + invoke.get('browser-agent:execute-tool')?.( + authorizedEvent, + 'tool-retired', + 'browser_request_takeover', + {}, + 'chat-1' + ) + ).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('authorized pending Copilot tool call'), + }) + expect(executeTool).not.toHaveBeenCalled() + executeTool.mockRestore() + }) + it('rejects a browser tool authorized after its scope cancellation boundary', async () => { const { invoke } = collectHandlers() const executeHandler = invoke.get('browser-agent:execute-tool') diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index a2a3936d061..8221c022fdf 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -7,7 +7,7 @@ import { type BrowserPanelSnapshot, isBrowserDataKind, isBrowserTheme, - isBrowserToolName, + isCurrentBrowserToolName, } from '@sim/browser-protocol' import { type DesktopNotificationPayload, @@ -94,6 +94,7 @@ const logger = createLogger('DesktopIpc') /** Workspace/chat ids are opaque tokens; anything else never reaches a URL. */ const ID_PATTERN = /^[A-Za-z0-9_-]{1,128}$/ const TERMINAL_WRITE_CHUNK_CHARACTERS = 64 * 1024 +const DESKTOP_TOOL_AUTHORIZATION_TIMEOUT_MS = 8_000 function writeTerminalText( terminal: TerminalRegistry, @@ -503,6 +504,7 @@ async function fetchDesktopToolAuthorization( if (typeof toolCallId !== 'string' || toolCallId.length < 1 || toolCallId.length > 256) { return null } + const startedAt = Date.now() try { const response = await event.sender.session.fetch( `${deps.appOrigin()}/api/desktop/tool/authorize`, @@ -511,9 +513,17 @@ async function fetchDesktopToolAuthorization( credentials: 'include', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ toolCallId }), + signal: AbortSignal.timeout(DESKTOP_TOOL_AUTHORIZATION_TIMEOUT_MS), } ) - if (!response.ok) return null + if (!response.ok) { + logger.warn('Desktop tool authorization was rejected', { + toolCallId, + status: response.status, + durationMs: Date.now() - startedAt, + }) + return null + } const authorization = (await response.json()) as { chatId?: unknown toolName?: unknown @@ -527,6 +537,10 @@ async function fetchDesktopToolAuthorization( authorization.args === null || Array.isArray(authorization.args) ) { + logger.warn('Desktop tool authorization returned a malformed response', { + toolCallId, + durationMs: Date.now() - startedAt, + }) return null } return { @@ -534,7 +548,12 @@ async function fetchDesktopToolAuthorization( toolName: authorization.toolName, args: authorization.args as Record, } - } catch { + } catch (error) { + logger.warn('Desktop tool authorization failed', { + toolCallId, + durationMs: Date.now() - startedAt, + error: getErrorMessage(error), + }) return null } } @@ -825,7 +844,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { typeof scope !== 'string' || typeof toolCallId !== 'string' || typeof tool !== 'string' || - !isBrowserToolName(tool) + !isCurrentBrowserToolName(tool) ) { return { ok: false, error: `Unknown browser tool: ${String(tool)}` } } @@ -1893,7 +1912,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { authorization.chatId !== requestedScope || typeof requestedTool !== 'string' || authorization.toolName !== requestedTool || - !isBrowserToolName(authorization.toolName) + !isCurrentBrowserToolName(authorization.toolName) ) { return { ok: false, diff --git a/apps/docs/app/[lang]/[[...slug]]/page.tsx b/apps/docs/app/[[...slug]]/page.tsx similarity index 80% rename from apps/docs/app/[lang]/[[...slug]]/page.tsx rename to apps/docs/app/[[...slug]]/page.tsx index 87436d3f664..02f782880e6 100644 --- a/apps/docs/app/[lang]/[[...slug]]/page.tsx +++ b/apps/docs/app/[[...slug]]/page.tsx @@ -1,6 +1,5 @@ import type React from 'react' import { highlight } from 'fumadocs-core/highlight' -import type { Root } from 'fumadocs-core/page-tree' import { findNeighbour } from 'fumadocs-core/page-tree' import type { ApiPageProps } from 'fumadocs-openapi/ui' import { createAPIPage } from 'fumadocs-openapi/ui' @@ -16,13 +15,11 @@ import { APIExampleSelector } from '@/components/ui/api-example-selector' import { CodeBlock } from '@/components/ui/code-block' import { Heading } from '@/components/ui/heading' import { ResponseSection } from '@/components/ui/response-section' -import { i18n } from '@/lib/i18n' import { getApiSpecContent, getAuthenticatedCodeSamples, openapi } from '@/lib/openapi' import { simShikiOptions } from '@/lib/shiki-theme' import { type PageData, source } from '@/lib/source' import { DOCS_BASE_URL } from '@/lib/urls' -const SUPPORTED_LANGUAGES: Set = new Set(i18n.languages) const BASE_URL = DOCS_BASE_URL /** @@ -40,37 +37,6 @@ function isContentHeading(item: { url: string }): boolean { return !ONWARD_NAV_SLUG.test(item.url) } -const OG_LOCALE_MAP: Record = { - en: 'en_US', - es: 'es_ES', - fr: 'fr_FR', - de: 'de_DE', - ja: 'ja_JP', - zh: 'zh_CN', -} - -function resolveLangAndSlug(params: { slug?: string[]; lang: string }) { - const isValidLang = SUPPORTED_LANGUAGES.has(params.lang) - const lang = isValidLang ? params.lang : 'en' - const slug = isValidLang ? params.slug : [params.lang, ...(params.slug ?? [])] - return { lang, slug } -} - -/** - * Strips a leading `/{lang}` path segment from a page URL. Unlike a naive - * `String.replace`, this only removes the locale when it is actually the - * first path segment — a plain substring replace would also match `/en` - * inside unrelated slugs (e.g. `/platform/enterprise`, `/integrations/enrich`, - * `/platform/self-hosting/environment-variables`), corrupting canonical and - * hreflang URLs for those pages. - */ -function stripLocalePrefix(url: string, lang: string): string { - const prefix = `/${lang}` - if (url === prefix) return '' - if (url.startsWith(`${prefix}/`)) return url.slice(prefix.length) - return url -} - /** * Renders the API reference's request and response samples through the docs' own `CodeBlock` * rather than fumadocs-openapi's built-in one, so those blocks get the emcn copy control @@ -122,10 +88,9 @@ const APIPage = createAPIPage(openapi, { }, }) -export default async function Page(props: { params: Promise<{ slug?: string[]; lang: string }> }) { - const params = await props.params - const { lang, slug } = resolveLangAndSlug(params) - const page = source.getPage(slug, lang) +export default async function Page(props: { params: Promise<{ slug?: string[] }> }) { + const { slug } = await props.params + const page = source.getPage(slug) if (!page) notFound() const data = page.data as unknown as PageData & { @@ -139,9 +104,7 @@ export default async function Page(props: { params: Promise<{ slug?: string[]; l const isAcademy = slug?.[0] === 'academy' const isCli = slug?.[0] === 'cli' - const pageTreeRecord = source.pageTree as Record - const pageTree = pageTreeRecord[lang] ?? pageTreeRecord.en ?? Object.values(pageTreeRecord)[0] - const rawNeighbours = pageTree ? findNeighbour(pageTree, page.url) : null + const rawNeighbours = findNeighbour(source.pageTree, page.url) // Academy, API Reference, and CLI are self-contained sections; keep prev/next // inside the section instead of spilling into the main documentation tree. // Match both the section's pages (`//...`) and its index (`/`). @@ -173,11 +136,6 @@ export default async function Page(props: { params: Promise<{ slug?: string[]; l let currentPath = '' urlParts.forEach((part: string, index: number) => { - if (index === 0 && SUPPORTED_LANGUAGES.has(part)) { - currentPath = `/${part}` - return - } - currentPath += `/${part}` const name = part @@ -218,7 +176,6 @@ export default async function Page(props: { params: Promise<{ slug?: string[]; l title={data.title} description={data.description || ''} url={`${BASE_URL}${page.url}`} - lang={lang} breadcrumb={breadcrumbs} /> -}) { - const params = await props.params - const { lang, slug } = resolveLangAndSlug(params) - const page = source.getPage(slug, lang) +export async function generateMetadata(props: { params: Promise<{ slug?: string[] }> }) { + const { slug } = await props.params + const page = source.getPage(slug) if (!page) notFound() const data = page.data as unknown as PageData @@ -377,13 +330,7 @@ export async function generateMetadata(props: { url: fullUrl, siteName: 'Sim Documentation', type: 'article', - locale: OG_LOCALE_MAP[lang] ?? 'en_US', - alternateLocale: i18n.languages.reduce((locales, l) => { - if (l !== lang) { - locales.push(OG_LOCALE_MAP[l] ?? 'en_US') - } - return locales - }, []), + locale: 'en_US', images: [ { url: ogImageUrl, @@ -406,15 +353,6 @@ export async function generateMetadata(props: { canonical: fullUrl, alternates: { canonical: fullUrl, - languages: { - 'x-default': `${BASE_URL}${stripLocalePrefix(page.url, lang)}`, - en: `${BASE_URL}${stripLocalePrefix(page.url, lang)}`, - es: `${BASE_URL}/es${stripLocalePrefix(page.url, lang)}`, - fr: `${BASE_URL}/fr${stripLocalePrefix(page.url, lang)}`, - de: `${BASE_URL}/de${stripLocalePrefix(page.url, lang)}`, - ja: `${BASE_URL}/ja${stripLocalePrefix(page.url, lang)}`, - zh: `${BASE_URL}/zh${stripLocalePrefix(page.url, lang)}`, - }, }, } } diff --git a/apps/docs/app/[lang]/layout.tsx b/apps/docs/app/[lang]/layout.tsx deleted file mode 100644 index 90079d4025f..00000000000 --- a/apps/docs/app/[lang]/layout.tsx +++ /dev/null @@ -1,129 +0,0 @@ -import type { ReactNode } from 'react' -import { defineI18nUI } from 'fumadocs-ui/i18n' -import { DocsLayout } from 'fumadocs-ui/layouts/docs' -import { RootProvider } from 'fumadocs-ui/provider/next' -import { Inter } from 'next/font/google' -import { ThemeProvider } from 'next-themes' -import { - SidebarFolder, - SidebarItem, - SidebarSeparator, -} from '@/components/docs-layout/sidebar-components' -import { Footer } from '@/components/footer/footer' -import { Navbar } from '@/components/navbar/navbar' -import { SimWordmark } from '@/components/ui/sim-logo' -import { i18n } from '@/lib/i18n' -import { serializeJsonLd } from '@/lib/json-ld' -import { source } from '@/lib/source' -import { DOCS_BASE_URL } from '@/lib/urls' -import { season } from '@/app/fonts/season' -import '../global.css' - -const inter = Inter({ - subsets: ['latin'], - variable: '--font-geist-sans', - display: 'swap', -}) - -const { provider } = defineI18nUI(i18n, { - translations: { - en: { - displayName: 'English', - }, - es: { - displayName: 'Español', - }, - fr: { - displayName: 'Français', - }, - de: { - displayName: 'Deutsch', - }, - ja: { - displayName: '日本語', - }, - zh: { - displayName: '简体中文', - }, - }, -}) - -type LayoutProps = { - children: ReactNode - params: Promise<{ lang: string }> -} - -const SUPPORTED_LANGUAGES: Set = new Set(i18n.languages) - -export default async function Layout({ children, params }: LayoutProps) { - const { lang: rawLang } = await params - const lang = SUPPORTED_LANGUAGES.has(rawLang) ? rawLang : 'en' - - const structuredData = { - '@context': 'https://schema.org', - '@type': 'WebSite', - name: 'Sim Documentation', - description: - 'Documentation for Sim — the open-source AI workspace where teams build, deploy, and manage AI agents. Connect 1,000+ integrations and every major LLM.', - url: DOCS_BASE_URL, - publisher: { - '@type': 'Organization', - name: 'Sim', - url: 'https://sim.ai', - logo: { - '@type': 'ImageObject', - url: `${DOCS_BASE_URL}/static/logo.png`, - }, - }, - inLanguage: lang, - } - - return ( - - -