From a99933c39170928c13155ac54ff200b9c495cedc Mon Sep 17 00:00:00 2001 From: happinessisreal Date: Wed, 16 Sep 2026 22:14:35 +0600 Subject: [PATCH] feat(browser): scroll nested containers in browser_scroll browser_scroll only ever called window.scrollTo/scrollBy, so it could not move an app pane whose own overflow container holds the content. Fixed-height app shells (chat transcripts, side panes, modal bodies) keep every message inside a nested scroller, so the window has no overflow to scroll and the container never receives the scroll event its lazy loading waits on. browser_scroll now accepts an optional element index or CSS selector. The action resolves the nearest scrollable ancestor of that anchor and moves that container's scrollTop, then replays a wheel gesture because a synthetic wheel event does not scroll by itself while apps commonly gate lazy loading on it. Without a target the previous page-level behavior is unchanged; a target with no scrollable container falls back to the page and says so in the status line. --- extensions/dsh-browser/src/content/actions.ts | 101 ++++++++++- .../dsh-browser/tests/actions-scroll.spec.ts | 170 ++++++++++++++++++ packages/browser/bridge-browser/src/tools.ts | 8 +- .../bridge-browser/tests/tools.spec.ts | 4 + 4 files changed, 275 insertions(+), 8 deletions(-) create mode 100644 extensions/dsh-browser/tests/actions-scroll.spec.ts diff --git a/extensions/dsh-browser/src/content/actions.ts b/extensions/dsh-browser/src/content/actions.ts index 7cdf60b74..766630b9a 100644 --- a/extensions/dsh-browser/src/content/actions.ts +++ b/extensions/dsh-browser/src/content/actions.ts @@ -393,9 +393,38 @@ async function pressAction(args: Record, ctx: ActionContext): P return withPageDelta(`Sent key "${key}".`, ctx) } -async function scrollAction(args: Record, ctx: ActionContext): Promise { - const direction = typeof args.direction === 'string' ? args.direction : '' - const amount = typeof args.amount === 'number' ? args.amount : Math.floor(window.innerHeight * 0.8) +/** Directions `browser_scroll` accepts. */ +type ScrollDirection = 'up' | 'down' | 'top' | 'bottom' + +function isScrollDirection(value: unknown): value is ScrollDirection { + return value === 'up' || value === 'down' || value === 'top' || value === 'bottom' +} + +/** + * Nearest ancestor — the element itself included — that actually scrolls + * vertically. Nested app shells (chat transcripts, side panes, modal bodies) + * keep their own overflow container, so a `window` scroll never reaches them + * and the app never receives the scroll event its lazy loading waits for. + */ +function nearestScrollable(el: Element | null): HTMLElement | null { + for (let node: Element | null = el; node !== null && node !== document.body; node = node.parentElement) { + if (!(node instanceof HTMLElement)) continue + const overflowY = getComputedStyle(node).overflowY + if (overflowY !== 'auto' && overflowY !== 'scroll' && overflowY !== 'overlay') continue + if (node.scrollHeight > node.clientHeight + 1) return node + } + return null +} + +/** Short, stable container description for the status line. */ +function describeContainer(box: HTMLElement): string { + const id = box.id === '' ? '' : `#${box.id}` + const role = box.getAttribute('role') + return `${box.tagName.toLowerCase()}${id}${role === null ? '' : `[role="${role}"]`}` +} + +/** Scroll the document itself: the only behavior this action had before targeting. */ +function scrollDocument(direction: ScrollDirection, amount: number): void { switch (direction) { case 'top': window.scrollTo({ top: 0, behavior: 'instant' }) @@ -409,11 +438,71 @@ async function scrollAction(args: Record, ctx: ActionContext): case 'down': window.scrollBy({ top: amount, behavior: 'instant' }) break - default: - throw new ActionError('bad-args', `direction must be up, down, top, or bottom; received "${direction}".`) } +} + +async function scrollAction(args: Record, ctx: ActionContext): Promise { + const direction = args.direction + if (!isScrollDirection(direction)) { + throw new ActionError('bad-args', `direction must be up, down, top, or bottom; received "${String(direction)}".`) + } + const amount = typeof args.amount === 'number' ? args.amount : Math.floor(window.innerHeight * 0.8) + const selector = typeof args.selector === 'string' && args.selector !== '' ? args.selector : undefined + const hasIndex = args.index !== undefined + if (hasIndex && selector !== undefined) { + throw new ActionError('bad-args', 'Provide either index or selector, not both.') + } + + // No target: keep the original page-level behavior exactly as it was. + if (!hasIndex && selector === undefined) { + scrollDocument(direction, amount) + await waitForPageSettled(SCROLL_SETTLE) + return withPageDelta(`Scrolled ${direction}.`, ctx) + } + + const index = hasIndex ? numberArg(args, 'index') : undefined + let anchor: Element + if (index !== undefined) { + anchor = elementOrThrow(ctx.ids, index) + } else { + let found: Element | null + try { + found = document.querySelector(selector as string) + } catch { + throw new ActionError('bad-args', `selector is not a valid CSS selector: ${String(selector)}`) + } + if (found === null) throw new ActionError('action-failed', `No element matched selector: ${String(selector)}`) + anchor = found + } + const label = index !== undefined ? `element [${index}]` : `selector ${String(selector)}` + + const box = nearestScrollable(anchor) + if (box === null) { + scrollDocument(direction, amount) + await waitForPageSettled(SCROLL_SETTLE) + return withPageDelta( + `Scrolled ${direction}; ${label} has no scrollable container, so the page scrolled instead.`, + ctx, + ) + } + + const before = box.scrollTop + if (direction === 'top') box.scrollTop = 0 + else if (direction === 'bottom') box.scrollTop = box.scrollHeight + else box.scrollTop += direction === 'up' ? -amount : amount + // Moving scrollTop relocates the container but tells listeners nothing, and a + // synthetic wheel event does not scroll by itself. Replay the gesture after + // the move so apps that gate lazy loading on `wheel` still fire. + const wheelDelta = direction === 'up' || direction === 'top' ? -amount : amount + if (typeof WheelEvent === 'function') { + box.dispatchEvent(new WheelEvent('wheel', { deltaY: wheelDelta, bubbles: true, cancelable: true })) + } + await waitForPageSettled(SCROLL_SETTLE) - return withPageDelta(`Scrolled ${direction}.`, ctx) + return withPageDelta( + `Scrolled ${direction} inside ${describeContainer(box)} (${before} to ${box.scrollTop}).`, + ctx, + ) } async function navigateAction(args: Record): Promise { diff --git a/extensions/dsh-browser/tests/actions-scroll.spec.ts b/extensions/dsh-browser/tests/actions-scroll.spec.ts new file mode 100644 index 000000000..7b8d9b489 --- /dev/null +++ b/extensions/dsh-browser/tests/actions-scroll.spec.ts @@ -0,0 +1,170 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { runAction } from '../src/content/actions.ts' +import { ElementIds } from '../src/content/ids.ts' + +const BUDGET = { maxItems: 20, maxForms: 10, maxChars: 8_000 } +const SETTLE_MS = 200 + +let scrollBy: ReturnType +let scrollTo: ReturnType + +/** + * jsdom has no layout engine: an element's scroll geometry and its resolved + * overflow are both fake here, which is exactly the two facts the container + * lookup reads. + */ +function makeScrollable(el: HTMLElement, geometry: { top: number; scrollHeight: number; clientHeight: number }): void { + let top = geometry.top + Object.defineProperty(el, 'scrollTop', { + configurable: true, + get: () => top, + set: (value: number) => { top = value }, + }) + Object.defineProperty(el, 'scrollHeight', { configurable: true, get: () => geometry.scrollHeight }) + Object.defineProperty(el, 'clientHeight', { configurable: true, get: () => geometry.clientHeight }) + el.style.overflowY = 'auto' + const real = window.getComputedStyle.bind(window) + // Only the container is answered from the inline style; every other element + // keeps the real cascade so snapshot rendering is unaffected. + vi.spyOn(window, 'getComputedStyle').mockImplementation(((target: Element, pseudo?: string | null) => + target === el ? { overflowY: 'auto' } as CSSStyleDeclaration : real(target, pseudo)) as typeof window.getComputedStyle) +} + +beforeEach(() => { + vi.useFakeTimers() + vi.spyOn(document, 'readyState', 'get').mockReturnValue('complete') + vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({ + top: 0, + left: 0, + right: 100, + bottom: 20, + width: 100, + height: 20, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect) + scrollBy = vi.fn() + scrollTo = vi.fn() + Object.defineProperty(window, 'scrollBy', { configurable: true, writable: true, value: scrollBy }) + Object.defineProperty(window, 'scrollTo', { configurable: true, writable: true, value: scrollTo }) +}) + +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + document.body.replaceChildren() +}) + +describe('browser_scroll targeting', () => { + it('scrolls the nearest scrollable container of an element index, not the page', async () => { + document.body.innerHTML = '
' + const pane = document.querySelector('#pane')! + const button = document.querySelector('button')! + makeScrollable(pane, { top: 300, scrollHeight: 1_000, clientHeight: 300 }) + const wheels: WheelEvent[] = [] + pane.addEventListener('wheel', (event) => { wheels.push(event as WheelEvent) }) + + const ids = new ElementIds() + await runAction('browser_snapshot', {}, { ids, budget: BUDGET }) + + const pending = runAction('browser_scroll', { + direction: 'down', + amount: 200, + index: ids.indexOf(button), + }, { ids, budget: BUDGET }) + await vi.advanceTimersByTimeAsync(SETTLE_MS) + const result = await pending + + expect(pane.scrollTop).toBe(500) + expect(scrollBy).not.toHaveBeenCalled() + expect(result.text).toContain('inside div#pane') + expect(result.text).toContain('(300 to 500)') + expect(wheels).toHaveLength(1) + expect(wheels[0]!.deltaY).toBe(200) + expect(wheels[0]!.bubbles).toBe(true) + }) + + it('jumps a container to its own top and bottom edges', async () => { + document.body.innerHTML = '
' + const pane = document.querySelector('#pane')! + const button = document.querySelector('button')! + makeScrollable(pane, { top: 400, scrollHeight: 1_200, clientHeight: 300 }) + + const ids = new ElementIds() + await runAction('browser_snapshot', {}, { ids, budget: BUDGET }) + const index = ids.indexOf(button) + + const toTop = runAction('browser_scroll', { direction: 'top', index }, { ids, budget: BUDGET }) + await vi.advanceTimersByTimeAsync(SETTLE_MS) + await toTop + expect(pane.scrollTop).toBe(0) + + const toBottom = runAction('browser_scroll', { direction: 'bottom', index }, { ids, budget: BUDGET }) + await vi.advanceTimersByTimeAsync(SETTLE_MS) + await toBottom + expect(pane.scrollTop).toBe(1_200) + }) + + it('accepts a CSS selector as the container anchor', async () => { + document.body.innerHTML = '
Row
' + const pane = document.querySelector('#pane')! + makeScrollable(pane, { top: 100, scrollHeight: 900, clientHeight: 300 }) + + const ids = new ElementIds() + await runAction('browser_snapshot', {}, { ids, budget: BUDGET }) + + const pending = runAction('browser_scroll', { + direction: 'down', + amount: 50, + selector: '#pane', + }, { ids, budget: BUDGET }) + await vi.advanceTimersByTimeAsync(SETTLE_MS) + const result = await pending + + expect(pane.scrollTop).toBe(150) + expect(result.text).toContain('div#pane') + }) + + it('rejects a call that names both an index and a selector', async () => { + document.body.innerHTML = '
' + const ids = new ElementIds() + await runAction('browser_snapshot', {}, { ids, budget: BUDGET }) + + await expect(runAction('browser_scroll', { + direction: 'down', + index: 0, + selector: '#pane', + }, { ids, budget: BUDGET })).rejects.toThrow(/either index or selector/) + }) + + it('falls back to the page when the target has no scrollable container', async () => { + document.body.innerHTML = '
' + const button = document.querySelector('button')! + const ids = new ElementIds() + await runAction('browser_snapshot', {}, { ids, budget: BUDGET }) + + const pending = runAction('browser_scroll', { + direction: 'down', + amount: 120, + index: ids.indexOf(button), + }, { ids, budget: BUDGET }) + await vi.advanceTimersByTimeAsync(SETTLE_MS) + const result = await pending + + expect(scrollBy).toHaveBeenCalledWith({ top: 120, behavior: 'instant' }) + expect(result.text).toContain('no scrollable container') + }) + + it('still scrolls the page when no target is given', async () => { + const ids = new ElementIds() + + const pending = runAction('browser_scroll', { direction: 'down', amount: 400 }, { ids, budget: BUDGET }) + await vi.advanceTimersByTimeAsync(SETTLE_MS) + const result = await pending + + expect(scrollBy).toHaveBeenCalledWith({ top: 400, behavior: 'instant' }) + expect(result.text).toBe('Scrolled down.') + }) +}) diff --git a/packages/browser/bridge-browser/src/tools.ts b/packages/browser/bridge-browser/src/tools.ts index 58344af42..5f000b602 100644 --- a/packages/browser/bridge-browser/src/tools.ts +++ b/packages/browser/bridge-browser/src/tools.ts @@ -180,19 +180,23 @@ function defineTools(call: Call, options: BrowserToolsOptions): ToolDefinition[] const scroll = (): ToolDefinition => defineTool({ name: 'browser_scroll', - description: 'Scroll up, down, top, or bottom; amount is optional pixels.', + description: 'Scroll up, down, top, or bottom; amount is optional pixels. Pass index or selector to scroll inside a nested scroll container (an app pane, chat transcript, or modal body) instead of the page.', parameters: { direction: { type: 'string', required: true, enum: ['up', 'down', 'top', 'bottom'], description: 'Scroll direction.' }, amount: { type: 'number', description: 'Number of pixels to scroll; ignored for top and bottom.' }, + index: { type: 'number', description: 'Element index from browser_snapshot; scrolls the nearest scrollable container of that element.' }, + selector: { type: 'string', description: 'CSS selector; scrolls the nearest scrollable container of the first match.' }, frame: FRAME_PARAMETER, }, timeoutMs: options.toolTimeoutMs, output: TEXT_OUTPUT, execute: (args, exec) => { - const a = args as { direction: 'up' | 'down' | 'top' | 'bottom'; amount?: number; frame?: number } + const a = args as { direction: 'up' | 'down' | 'top' | 'bottom'; amount?: number; index?: number; selector?: string; frame?: number } return call(exec, 'browser_scroll', { direction: a.direction, ...a.amount !== undefined ? { amount: a.amount } : {}, + ...a.index !== undefined ? { index: a.index } : {}, + ...a.selector !== undefined ? { selector: a.selector } : {}, ...a.frame !== undefined ? { frame: a.frame } : {}, }) }, diff --git a/packages/browser/bridge-browser/tests/tools.spec.ts b/packages/browser/bridge-browser/tests/tools.spec.ts index 938825ef2..9b9023e05 100644 --- a/packages/browser/bridge-browser/tests/tools.spec.ts +++ b/packages/browser/bridge-browser/tests/tools.spec.ts @@ -97,6 +97,10 @@ describe('registerBrowserTools', () => { expect(requestTool).toHaveBeenLastCalledWith('browser_scroll', { direction: 'top' }, exec.signal, 1_000) await run('browser_scroll', { direction: 'down', frame: 4 }) expect(requestTool).toHaveBeenLastCalledWith('browser_scroll', { direction: 'down', frame: 4 }, exec.signal, 1_000) + await run('browser_scroll', { direction: 'up', index: 7 }) + expect(requestTool).toHaveBeenLastCalledWith('browser_scroll', { direction: 'up', index: 7 }, exec.signal, 1_000) + await run('browser_scroll', { direction: 'bottom', selector: '#pane' }) + expect(requestTool).toHaveBeenLastCalledWith('browser_scroll', { direction: 'bottom', selector: '#pane' }, exec.signal, 1_000) await run('browser_navigate', { url: 'https://example.com' }) expect(requestTool).toHaveBeenLastCalledWith('browser_navigate', { url: 'https://example.com' }, exec.signal, 1_000)