diff --git a/apps/desktop/src/app/chat/right-rail/preview-act.ts b/apps/desktop/src/app/chat/right-rail/preview-act.ts index bcef505fbc..790196c2b1 100644 --- a/apps/desktop/src/app/chat/right-rail/preview-act.ts +++ b/apps/desktop/src/app/chat/right-rail/preview-act.ts @@ -8,10 +8,11 @@ * * - `executeJavaScript` injects the engine SOURCE (see * lib/preview-act/act-in-page.ts's self-containment contract) to RESOLVE - * and READ — turn '@e5' into a node, measure it, inventory the page. It is - * parked on a window global alongside the ref holder so refs survive - * between calls, and it vanishes with the page, so a navigation drops the - * refs and the engine reports that instead of clicking the wrong node. + * and READ — turn 'btn-sign-in' into a node, measure it, inventory the + * page. It is parked on a window global alongside the book of handles so + * they survive between calls, and it vanishes with the page, so a + * navigation retires them and the engine reports that instead of acting on + * the wrong node. * - `sendInputEvent` (preview-drive.ts) does the ACTING, as real Chromium * input. Script can only dispatch synthetic events, which the page can tell * apart and which never move the browser's own hover or focus target. @@ -192,7 +193,8 @@ ${preamble()} var found = act({ kind: 'elements' }); if (!found.success) { return JSON.stringify(found); } watch('hold'); - return JSON.stringify({ acted: 'held the field', elements: found.elements, title: found.title, url: found.url, success: true }); + found.acted = 'held the field'; + return JSON.stringify(found); })()` } @@ -204,7 +206,8 @@ ${preamble()} var found = act({ kind: 'elements' }); if (!found.success) { return JSON.stringify(found); } watch('strobe'); - return JSON.stringify({ acted: 'strobed the field', elements: found.elements, title: found.title, url: found.url, success: true }); + found.acted = 'strobed the field'; + return JSON.stringify(found); })()` } @@ -272,7 +275,10 @@ ${preamble()} try { var after = act({ kind: 'elements' }); watch('sweep'); + // One or the other, never both: a re-read answers with the whole + // inventory only when it is the first look at this page. result.elements = after.elements; + result.delta = after.delta; result.url = after.url; result.title = after.title; } catch (err) { diff --git a/apps/desktop/src/lib/preview-act/act-in-page.test.ts b/apps/desktop/src/lib/preview-act/act-in-page.test.ts index 5fafdb91e8..081b67c6f1 100644 --- a/apps/desktop/src/lib/preview-act/act-in-page.test.ts +++ b/apps/desktop/src/lib/preview-act/act-in-page.test.ts @@ -28,13 +28,20 @@ function inventory(holder: PreviewActHolder) { return actInPage(document, holder, { kind: 'elements' }) } +/** Take the inventory and hand back the handle for the first thing on the page. + * Tests address elements the way the agent does — by asking what they are + * called — rather than predicting what the engine will name them. */ +function firstRef(holder: PreviewActHolder) { + return inventory(holder).elements![0].ref +} + beforeEach(() => { vi.restoreAllMocks() layOutTheDocument() Element.prototype.scrollIntoView = vi.fn() // jsdom has no hit-testing at all, so the engine skips the occlusion check // here by default. Tests that stand one in must not leak it into the next. - delete (document as Document & { elementFromPoint?: unknown }).elementFromPoint + delete (document as unknown as { elementFromPoint?: unknown }).elementFromPoint }) describe('self-containment', () => { @@ -51,18 +58,22 @@ describe('self-containment', () => { const injected = new Function('return (' + actInPage.toString() + ')')() as typeof actInPage const holder: PreviewActHolder = {} - expect(injected(document, holder, { kind: 'elements' }).elements?.[0].label).toBe('Save') + const [save] = injected(document, holder, { kind: 'elements' }).elements! + + expect(save.label).toBe('Save') const clicked = vi.fn() document.getElementById('save')!.addEventListener('click', clicked) - expect(injected(document, holder, { kind: 'click', ref: '@e1' }).success).toBe(true) + expect(injected(document, holder, { kind: 'click', ref: save.ref }).success).toBe(true) expect(clicked).toHaveBeenCalledOnce() }) }) describe('elements', () => { - it('numbers the interactive nodes with browser_*-style refs', () => { + // The handle says what the thing is and which one it is, so a delta line the + // agent reads ten turns later needs no lookup to make sense of. + it('names the interactive nodes after their role and label', () => { const holder = page(` Help @@ -74,12 +85,43 @@ describe('elements', () => { expect(result.success).toBe(true) expect(result.elements?.map(e => [e.ref, e.label])).toEqual([ - ['@e1', 'Save'], - ['@e2', 'Help'], - ['@e3', 'Your name'] + ['btn-save', 'Save'], + ['lnk-help', 'Help'], + ['inp-your-name', 'Your name'] ]) }) + it('tells two elements with the same name apart', () => { + const holder = page(` + + + + `) + + expect(inventory(holder).elements?.map(e => e.ref)).toEqual(['btn-edit', 'btn-edit-1', 'btn-edit-2']) + }) + + // Handing a retired name to a different element would silently redirect a + // handle the agent is still holding. The two ids here also disagree, which is + // the page saying outright that these are different buttons — so this must + // not re-bind despite the identical label. + it('never reissues the name of an element that went away', () => { + const holder = page(` +
+ Help + Terms + `) + + expect(firstRef(holder)).toBe('btn-edit') + + document.getElementById('host')!.innerHTML = '' + + const again = inventory(holder) + + expect(again.delta?.removed).toEqual(['btn-edit']) + expect(again.delta?.added?.map(e => e.ref)).toEqual(['btn-edit-1']) + }) + it('reports role, current value, and disabled state', () => { const holder = page(` @@ -146,6 +188,7 @@ describe('elements', () => { `) + const wall = document.createElement('div') document.body.append(wall) @@ -179,35 +222,276 @@ describe('elements', () => { expect(holder.field).toEqual([document.getElementById('named'), document.getElementById('mystery')]) }) - it('prefers an identity selector so the agent can re-find the node later', () => { + // The positional `:nth-child` chain this used to fall back to was 74% of a + // real page's inventory and nothing read it. An identity selector is short + // and stable, so it stays; everything else addresses by ref. + it('carries an identity selector only, and omits it when there is none', () => { const holder = page(`
+
`) - const [byTestId, positional] = inventory(holder).elements! + const [byTestId, byId, plain] = inventory(holder).elements! expect(byTestId.selector).toBe('[data-testid="submit"]') - expect(document.querySelector(positional.selector)).toBe(document.querySelectorAll('button')[1]) + expect(byId.selector).toBe('#cancel') + expect(plain.selector).toBeUndefined() + expect(plain.ref).toBe('btn-plain') }) it('honours the cap', () => { const holder = page(Array.from({ length: 10 }, (_, i) => ``).join('')) expect(inventory(holder).elements).toHaveLength(10) - expect(actInPage(document, holder, { kind: 'elements', max: 3 }).elements).toHaveLength(3) + expect(actInPage(document, holder, { full: true, kind: 'elements', max: 3 }).elements).toHaveLength(3) + }) +}) + +// Re-sending the whole inventory after every click is what made a ten-step +// session cost several times what it needed to: the page barely moves between +// steps and the agent was charged for a fresh copy of it each time. +describe('delta', () => { + it('gives the full inventory the first time it looks at a page', () => { + const holder = page('') + const first = inventory(holder) + + expect(first.elements).toHaveLength(1) + expect(first.delta).toBeUndefined() + }) + + it('reports only what moved on every look after that', () => { + const holder = page(` +
+ + + +
+ `) + + inventory(holder) + + document.getElementById('host')!.insertAdjacentHTML('beforeend', '') + + const next = inventory(holder) + + expect(next.elements).toBeUndefined() + expect(next.delta?.added?.map(e => e.ref)).toEqual(['btn-quit']) + expect(next.delta?.same).toBe(3) + }) + + it('says nothing about a page that did not move', () => { + const holder = page('') + inventory(holder) + + const still = inventory(holder) + + expect(still.delta).toEqual({ same: 1 }) + }) + + it('reports a relabelled control as changed, keeping its handle', () => { + const holder = page(` + + Help + Terms + `) + + const ref = firstRef(holder) + + document.getElementById('cart')!.textContent = 'Added' + + const next = inventory(holder) + + expect(next.delta?.changed?.map(e => [e.ref, e.label])).toEqual([[ref, 'Added']]) + expect(actInPage(document, holder, { kind: 'click', ref }).success).toBe(true) + }) + + // `changed` fires on nearly every step of a long task, so it is the one part + // of the payload whose cost compounds. It carries the moved field and the + // handle, and nothing the agent already knows. + it('reports only the field that moved, not the whole element', () => { + const holder = page(` + + Help + Terms + `) + inventory(holder) + ;(document.getElementById('q') as HTMLInputElement).value = 'shoes' + + const [moved] = inventory(holder).delta!.changed! + + expect(moved).toEqual({ ref: 'inp-search', value: 'shoes' }) + }) + + it('reports a control becoming available on its own', () => { + const holder = page(` + + Help + Terms + `) + inventory(holder) + ;(document.getElementById('go') as HTMLButtonElement).disabled = false + + expect(inventory(holder).delta?.changed).toEqual([{ disabled: false, ref: 'btn-continue' }]) + }) + + it('falls back to the whole inventory when most of the page is new', () => { + const holder = page('
') + inventory(holder) + + document.getElementById('host')!.innerHTML = 'ABC' + + const next = inventory(holder) + + expect(next.delta).toBeUndefined() + expect(next.elements?.map(e => e.ref)).toEqual(['lnk-a', 'lnk-b', 'lnk-c']) + }) + + // An element that slid past the cap is still clickable, so calling it removed + // would be a lie the agent acts on. + it('does not report an element as removed just because it fell past the cap', () => { + const holder = page(Array.from({ length: 4 }, (_, i) => ``).join('')) + inventory(holder) + + const capped = actInPage(document, holder, { kind: 'elements', max: 2 }) + + expect(capped.delta?.removed).toBeUndefined() + expect(actInPage(document, holder, { kind: 'click', ref: 'btn-b3' }).success).toBe(true) + }) + + // The contract the whole delta exists for. Not a fixed byte count — that + // would break on any wording change — but the relationship between the two + // payloads, which is what has to hold. + it('costs a fraction of the inventory on a page that mostly held still', () => { + const holder = page(` + +
+ ${Array.from({ length: 24 }, (_, i) => ``).join('')} + +
+
+ `) + + const baseline = JSON.stringify(inventory(holder).elements) + + document.getElementById('host')!.innerHTML = '' + document.getElementById('row-3')!.textContent = 'Row action 3 (done)' + + const next = inventory(holder) + + expect(next.delta?.same).toBe(32) + // Measured at roughly 15x on this fixture; the bar is set well below that + // so a wording change does not fail the build, but a regression to + // re-sending the page would. + expect(JSON.stringify(next.delta).length * 5).toBeLessThan(baseline.length) + }) + + it('retires every handle when the page navigates', () => { + const holder = page('') + inventory(holder) + holder.url = 'https://elsewhere.example/other' + + const landed = inventory(holder) + + expect(landed.delta).toBeUndefined() + expect(landed.elements?.map(e => e.ref)).toEqual(['btn-save']) + }) +}) + +// The headline case. A framework re-render destroys the node and builds a new +// one; the agent's handle has to survive that, and it has to hear about it in +// one word rather than as a removal it must react to plus an addition it must +// re-read. +describe('rebind', () => { + /** A page with a stable nav around the part that re-renders, so the re-bind + * has to pick its candidate rather than being handed the only one going. */ + function app(inner: string) { + return page(` + +
${inner}
+ `) + } + + function rerender(inner: string) { + document.getElementById('host')!.innerHTML = inner + } + + it('keeps the handle when a re-render replaces the node', () => { + const holder = app('') + const ref = inventory(holder).elements!.find(e => e.label === 'Sign in')!.ref + const before = document.querySelector('button') + + rerender('') + + const next = inventory(holder) + + expect(document.querySelector('button')).not.toBe(before) + expect(next.delta?.rebound).toEqual([ref]) + expect(next.delta?.added).toBeUndefined() + expect(next.delta?.removed).toBeUndefined() + expect(actInPage(document, holder, { kind: 'click', ref }).success).toBe(true) + }) + + it('follows a label through a count badge appearing on it', () => { + const holder = app('Inbox') + const ref = inventory(holder).elements!.find(e => e.label === 'Inbox')!.ref + + rerender('
Inbox (3)
') + + expect(inventory(holder).delta?.rebound).toEqual([ref]) + }) + + it('will not move a handle across roles', () => { + const holder = app('') + inventory(holder) + + rerender('Continue') + + const next = inventory(holder) + + expect(next.delta?.rebound).toBeUndefined() + expect(next.delta?.removed).toEqual(['btn-continue']) + expect(next.delta?.added?.map(e => e.ref)).toEqual(['lnk-continue']) + }) + + // Two unrelated buttons trading places must not trade handles with them. + it('mints a new handle rather than guess between unrelated candidates', () => { + const holder = app('') + inventory(holder) + + rerender('') + + const next = inventory(holder) + + expect(next.delta?.rebound).toBeUndefined() + expect(next.delta?.removed).toEqual(['btn-delete-account']) + expect(next.delta?.added?.map(e => e.ref)).toEqual(['btn-upload-photo']) + }) + + // Both carry an id and the ids disagree, which is the page saying outright + // that these are two different controls however alike they read. + it('will not move a handle between elements the page marks as distinct', () => { + const holder = app('') + inventory(holder) + + rerender('') + + const next = inventory(holder) + + expect(next.delta?.rebound).toBeUndefined() + expect(next.delta?.removed).toEqual(['btn-save']) }) }) describe('click', () => { it('activates the element a ref points at', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) const clicked = vi.fn() document.getElementById('save')!.addEventListener('click', clicked) - const result = actInPage(document, holder, { kind: 'click', ref: '@e1' }) + const result = actInPage(document, holder, { kind: 'click', ref }) expect(result.success).toBe(true) expect(result.acted).toContain('Save') @@ -216,7 +500,7 @@ describe('click', () => { it('replays the pointer/mouse sequence frameworks bind to', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) const seen: string[] = [] @@ -224,7 +508,7 @@ describe('click', () => { document.getElementById('save')!.addEventListener(type, () => seen.push(type)) } - actInPage(document, holder, { kind: 'click', ref: '@e1' }) + actInPage(document, holder, { kind: 'click', ref }) expect(seen).toContain('mousedown') expect(seen).toContain('mouseup') @@ -242,9 +526,7 @@ describe('click', () => { it('refuses a disabled control instead of silently doing nothing', () => { const holder = page('') - inventory(holder) - - const result = actInPage(document, holder, { kind: 'click', ref: '@e1' }) + const result = actInPage(document, holder, { kind: 'click', ref: firstRef(holder) }) expect(result.success).toBe(false) expect(result.error).toContain('disabled') @@ -252,18 +534,17 @@ describe('click', () => { it('reports the live url so a navigation is visible to the agent', () => { const holder = page('') - inventory(holder) - expect(actInPage(document, holder, { kind: 'click', ref: '@e1' }).url).toBe(document.location.href) + expect(actInPage(document, holder, { kind: 'click', ref: firstRef(holder) }).url).toBe(document.location.href) }) }) describe('stale refs', () => { - it('names an unknown ref rather than clicking whatever sits at that index', () => { + it('names an unknown ref rather than acting on whatever is nearby', () => { const holder = page('') inventory(holder) - const result = actInPage(document, holder, { kind: 'click', ref: '@e9' }) + const result = actInPage(document, holder, { kind: 'click', ref: 'btn-imaginary' }) expect(result.success).toBe(false) expect(result.error).toContain('elements') @@ -271,18 +552,18 @@ describe('stale refs', () => { it('catches a node that was removed after the snapshot', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) document.getElementById('save')!.remove() - expect(actInPage(document, holder, { kind: 'click', ref: '@e1' }).error).toContain('removed') + expect(actInPage(document, holder, { kind: 'click', ref }).error).toContain('removed') }) it('invalidates every ref when the page navigated under them', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) holder.url = 'https://elsewhere.example/other' - expect(actInPage(document, holder, { kind: 'click', ref: '@e1' }).error).toContain('navigated') + expect(actInPage(document, holder, { kind: 'click', ref }).error).toContain('navigated') }) it('asks for a target when given neither', () => { @@ -297,14 +578,14 @@ describe('stale refs', () => { describe('type', () => { it('enters text and fires the events a controlled input listens for', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) const input = document.getElementById('who') as HTMLInputElement const events: string[] = [] input.addEventListener('input', () => events.push('input')) input.addEventListener('change', () => events.push('change')) - const result = actInPage(document, holder, { kind: 'type', ref: '@e1', text: 'Brooklyn' }) + const result = actInPage(document, holder, { kind: 'type', ref, text: 'Brooklyn' }) expect(result.success).toBe(true) expect(input.value).toBe('Brooklyn') @@ -313,7 +594,7 @@ describe('type', () => { it('bypasses the own-property shadow React installs on tracked inputs', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) // React defines its own `value` accessor on the node to track what it last // wrote, and ignores an input event that agrees with it. Writing through @@ -330,7 +611,7 @@ describe('type', () => { } }) - actInPage(document, holder, { kind: 'type', ref: '@e1', text: 'Brooklyn' }) + actInPage(document, holder, { kind: 'type', ref, text: 'Brooklyn' }) expect(shadowWrites).toEqual([]) expect(nativeValue.get!.call(input)).toBe('Brooklyn') @@ -338,21 +619,20 @@ describe('type', () => { it('writes into a contenteditable host', () => { const holder = page('
') - inventory(holder) - actInPage(document, holder, { kind: 'type', ref: '@e1', text: 'hello' }) + actInPage(document, holder, { kind: 'type', ref: firstRef(holder), text: 'hello' }) expect(document.getElementById('editor')!.textContent).toBe('hello') }) it('submits the owning form when asked', () => { const holder = page('
') - inventory(holder) + const ref = firstRef(holder) const form = document.getElementById('f') as HTMLFormElement form.requestSubmit = vi.fn() - const result = actInPage(document, holder, { kind: 'type', ref: '@e1', submit: true, text: 'cats' }) + const result = actInPage(document, holder, { kind: 'type', ref, submit: true, text: 'cats' }) expect(form.requestSubmit).toHaveBeenCalledOnce() expect(result.acted).toContain('submitted') @@ -360,29 +640,29 @@ describe('type', () => { it('refuses a target that has no text to type into', () => { const holder = page('') - inventory(holder) - expect(actInPage(document, holder, { kind: 'type', ref: '@e1', text: 'x' }).error).toContain('not a text field') + expect(actInPage(document, holder, { kind: 'type', ref: firstRef(holder), text: 'x' }).error).toContain( + 'not a text field' + ) }) }) describe('press', () => { it('sends the key to the target', () => { const holder = page('') - inventory(holder) + const ref = firstRef(holder) const keys: string[] = [] document.getElementById('q')!.addEventListener('keydown', e => keys.push((e as KeyboardEvent).key)) - expect(actInPage(document, holder, { key: 'Enter', kind: 'press', ref: '@e1' }).success).toBe(true) + expect(actInPage(document, holder, { key: 'Enter', kind: 'press', ref }).success).toBe(true) expect(keys).toEqual(['Enter']) }) it('needs a key', () => { const holder = page('') - inventory(holder) - expect(actInPage(document, holder, { kind: 'press', ref: '@e1' }).error).toContain('key') + expect(actInPage(document, holder, { kind: 'press', ref: firstRef(holder) }).error).toContain('key') }) }) @@ -426,13 +706,13 @@ describe('scroll', () => { it('scrolls a ref’d container instead of the page', () => { const holder = page('
') - inventory(holder) + const ref = firstRef(holder) const list = document.getElementById('list') as HTMLElement list.scrollBy = vi.fn() const pageScroll = vi.spyOn(window, 'scrollBy').mockImplementation(() => {}) - const result = actInPage(document, holder, { amount: 200, kind: 'scroll', ref: '@e1' }) + const result = actInPage(document, holder, { amount: 200, kind: 'scroll', ref }) expect(list.scrollBy).toHaveBeenCalledWith({ behavior: 'smooth', top: 200 }) expect(pageScroll).not.toHaveBeenCalled() diff --git a/apps/desktop/src/lib/preview-act/act-in-page.ts b/apps/desktop/src/lib/preview-act/act-in-page.ts index 0d78fccf2a..1c5428319a 100644 --- a/apps/desktop/src/lib/preview-act/act-in-page.ts +++ b/apps/desktop/src/lib/preview-act/act-in-page.ts @@ -20,16 +20,48 @@ export interface PreviewElement { disabled?: boolean /** Human-readable label (aria-label, text, placeholder, value …). */ label: string - /** Stable-for-this-snapshot handle: '@e1', '@e2', … */ + /** Durable handle for as long as this page is open: 'btn-sign-in'. Legible on + * purpose — see the ref-minting note in `actInPage`. */ ref: string /** Explicit ARIA role, else the tag name. */ role: string - /** CSS selector that resolves back to this node, for re-finding it later. */ - selector: string + /** The element's `#id` or `[data-testid]`, when it has one. Absent otherwise + * — address the element by its `ref`. */ + selector?: string /** Current value of a form control, truncated. */ value?: string } +/** An element that is still itself but no longer reads the same. + * + * Only the fields that actually moved are present. Role and selector are + * absent by construction rather than by omission: a change in either would + * mean this is a different element, which the re-bind ladder would have + * refused to match in the first place. */ +export interface PreviewElementChange { + /** Present only when the control's availability flipped. */ + disabled?: boolean + label?: string + ref: string + value?: string +} + +/** What changed on the page since the last look. Sent instead of the whole + * inventory once the agent has a baseline for the page — see `survey`. */ +export interface PreviewActDelta { + /** Elements seen for the first time, in full. */ + added?: PreviewElement[] + /** Same handle, new label/value/disabled state — and nothing else. */ + changed?: PreviewElementChange[] + /** Handles that are gone from the page. */ + removed?: string[] + /** Handles whose element was destroyed and recreated by a re-render. The + * handle still works; nothing about them needs re-reading. */ + rebound?: string[] + /** How many handles were on the page and untouched. */ + same?: number +} + /** A normalized action. `kind` is the verb; the rest is per-verb payload. */ export interface PreviewActAction { /** scroll distance in px. Defaults to ~90% of the viewport height. */ @@ -53,6 +85,8 @@ export interface PreviewActAction { /** locate: also give the target keyboard focus, for a key press that must not * be preceded by a click (which would activate the control instead). */ focus?: boolean + /** elements: answer with the whole inventory rather than a delta. */ + full?: boolean /** Cap on the returned inventory. */ max?: number ref?: string @@ -66,6 +100,11 @@ export interface PreviewActAction { export interface PreviewActResult { /** What the action landed on, for the agent's own log. */ acted?: string + /** What moved since the last look. Present INSTEAD of `elements` once the + * agent holds a baseline for this page. */ + delta?: PreviewActDelta + /** The full inventory. Sent on the first look at a page, and again whenever + * the page changed too much for a delta to be the cheaper answer. */ elements?: PreviewElement[] error?: string note?: string @@ -79,17 +118,45 @@ export interface PreviewActResult { url?: string } -/** Where the surface keeps the last snapshot between actions (a window global - * in the preview page), so '@e5' still means something on the next call. */ +/** One element the agent has a handle on, remembered across actions. */ +export interface PreviewActBinding { + el: Element + /** What it read as last time. Kept field by field rather than as one hash so + * a change can be reported as only the part that moved. */ + label: string + /** The accessible name at mint time, for re-finding this element after a + * re-render destroys and recreates its node. */ + name: string + /** Whether the control was unavailable last time. */ + off: boolean + /** Nearest-landmark path plus position among same-role siblings. */ + path: string + ref: string + role: string + /** `id` / `name` / `data-testid` / `aria-label`, if the page provides one. + * The strongest re-bind signal there is, and the only one a rewrite of the + * surrounding markup cannot disturb. */ + stable: string + value: string +} + +/** Where the surface keeps what it knows between actions (a window global in + * the preview page), so a handle still means something on the next call. */ export interface PreviewActHolder { /** Target of the action in flight, for the watch overlay to draw onto. */ aimed?: Element | null + /** Every handle minted on this page, live or not yet retired. */ + book?: PreviewActBinding[] + /** Next disambiguating suffix per ref stem, so two "Edit" buttons become + * `btn-edit` and `btn-edit-1`. Never rewound: a retired handle's name is + * not handed to a different element later in the same page. */ + coined?: Record /** The on-screen subset, for the overlay to outline. Diverges from `nodes` in * both directions: it drops what is below the fold, and it is not capped at * the inventory's size. */ field?: Element[] nodes?: Element[] - /** URL the snapshot was taken on; a navigation invalidates every ref. */ + /** URL the snapshot was taken on; a navigation retires every handle. */ url?: string } @@ -102,6 +169,11 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ // told about. Far higher, because an extra mark costs one rect read where an // extra inventory row costs tokens on every single call. const maxMarks = 600 + // How alike a remembered element and a fresh one have to be before the + // handle moves across. Below it we mint a new handle instead: a re-render + // costing the agent a re-read is a cheap mistake, and a handle silently + // pointing at the wrong button is not. + const rebindBar = 0.6 const win = doc.defaultView const here = doc.location ? doc.location.href : '' @@ -141,8 +213,107 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ return '' } - /** Identity-first selector, positional fallback — the caller re-finds nodes - * with this once the refs have gone stale. */ + /** The strongest identity signal the page offers, if it offers one. Nothing a + * re-render does to the surrounding markup disturbs these. */ + const stableOf = (el: Element): string => + el.id || + el.getAttribute('data-testid') || + el.getAttribute('name') || + el.getAttribute('aria-label') || + '' + + /** Handle stems by role, so a handle says what it is before it says which + * one. Anything unrecognised is `el`. */ + const stemOf = (role: string): string => { + if (role === 'button' || role === 'summary') { + return 'btn' + } + + if (role === 'a' || role === 'link') { + return 'lnk' + } + + if (role === 'input:search' || role === 'searchbox') { + return 'srch' + } + + if (role === 'input:checkbox' || role === 'checkbox') { + return 'chk' + } + + if (role === 'input:radio' || role === 'radio') { + return 'rdo' + } + + if (role === 'select' || role === 'combobox') { + return 'sel' + } + + if (role === 'textarea') { + return 'txt' + } + + if (role === 'switch') { + return 'sw' + } + + if (role === 'tab' || role === 'menuitem' || role === 'option') { + return role === 'menuitem' ? 'mi' : role === 'option' ? 'opt' : 'tab' + } + + // Every `input:*` that isn't one of the special cases above, plus the ARIA + // textbox. A date picker and an email field are both places text goes. + return role.indexOf('input') === 0 || role === 'textbox' ? 'inp' : 'el' + } + + /** Lowercase, hyphenated, and short enough to read at a glance. */ + const slug = (name: string): string => { + let out = '' + let dash = false + + for (let i = 0; i < name.length && out.length < 24; i++) { + const ch = name[i] + + if (/[a-zA-Z0-9]/.test(ch)) { + out += ch.toLowerCase() + dash = false + } else if (!dash && out) { + out += '-' + dash = true + } + } + + return out.replace(/-+$/, '') + } + + /** Where the element sits, coarsely: the nearest landmark plus its position + * among same-role elements inside it. Deliberately NOT the CSS selector + * below — a wrapper div appearing anywhere in the chain changes that string + * completely, which is exactly the churn a re-bind has to see through. */ + const anchorOf = (el: Element): string => { + const near = el.closest( + 'main,nav,header,footer,aside,[role="main"],[role="navigation"],[role="banner"],' + + '[role="contentinfo"],[role="complementary"],[role="search"],form[aria-label],section[aria-label]' + ) + + if (!near) { + return 'root' + } + + const named = near.getAttribute('aria-label') || '' + + return near.tagName.toLowerCase() + (named ? '#' + slug(named) : '') + } + + /** The element's own selector, when the page gives it one worth having. + * + * Deliberately identity-only. This used to fall back to a chain of up to + * eight `:nth-child` rungs, and on a real app shell that column was 74% of + * the entire inventory — the single biggest thing the agent was paying for. + * It bought nothing: nothing downstream reads it, a positional chain is + * wrong the moment a sibling appears, and re-finding a node is what the + * durable ref now does properly. An `#id` is short, stable, and the one + * case where naming the node is genuinely useful to the model. */ const selectorFor = (el: Element): string => { if (el.id) { return '#' + cssEscape(el.id) @@ -150,28 +321,7 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ const testId = el.getAttribute('data-testid') - if (testId) { - return '[data-testid="' + cssEscape(testId) + '"]' - } - - const path: string[] = [] - let node: Element | null = el - - while (node && node !== doc.body && path.length < 8) { - if (node.id) { - path.unshift('#' + cssEscape(node.id)) - - break - } - - const parent: Element | null = node.parentElement - const index = parent ? Array.prototype.indexOf.call(parent.children, node) : -1 - - path.unshift(node.tagName.toLowerCase() + (index >= 0 ? ':nth-child(' + (index + 1) + ')' : '')) - node = parent - } - - return path.join(' > ') + return testId ? '[data-testid="' + cssEscape(testId) + '"]' : '' } /** On screen right now, and worth drawing a box around. The field is strictly @@ -267,6 +417,7 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ // function after the scroll. const midX = rect.left + rect.width / 2 const midY = rect.top + rect.height / 2 + const under = (doc as Document & { elementFromPoint?: (x: number, y: number) => Element | null }) .elementFromPoint @@ -302,7 +453,9 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ return '' } - const collect = (max: number): PreviewElement[] => { + /** Walk the page and hand back what is interactable, in document order. The + * handles are assigned afterwards, by `survey`. */ + const sight = (max: number) => { const nodes: Element[] = [] const field: Element[] = [] const elements: PreviewElement[] = [] @@ -342,15 +495,25 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ // A control with neither a label nor a value is not addressable in prose // — the agent could not tell it apart from its unlabelled neighbours. + // It is also the one case a durable handle cannot be minted for, since + // there would be nothing to name it after and nothing to re-find it by, + // so dropping it here keeps every handle we DO mint anchorable. if (!label && !value) { continue } const entry: PreviewElement = { label, - ref: '@e' + (elements.length + 1), - role, - selector: selectorFor(el) + // Filled in by `survey`, which is what knows whether this element + // already has a handle. + ref: '', + role + } + + const selector = selectorFor(el) + + if (selector) { + entry.selector = selector } if ((el as HTMLInputElement).disabled) { @@ -367,12 +530,284 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ holder.nodes = nodes holder.field = field - holder.url = here - return elements + return { elements, nodes } } - /** Resolve the action's target: a ref from the last snapshot, else a selector. */ + /** What moved on an element that kept its handle, or nothing if it held + * still. Field by field, so a status line ticking over costs the agent one + * short line instead of a re-run of everything already known about it. */ + const shifted = (was: PreviewActBinding, entry: PreviewElement): PreviewElementChange | null => { + const off = !!entry.disabled + const value = entry.value || '' + + if (was.label === entry.label && was.value === value && was.off === off) { + return null + } + + const moved: PreviewElementChange = { ref: was.ref } + + if (was.label !== entry.label) { + moved.label = entry.label + } + + if (was.value !== value) { + moved.value = value + } + + if (was.off !== off) { + moved.disabled = off + } + + return moved + } + + /** Do two labels share at least half their words? Tolerates the count badge + * and the copy edit — "Inbox" against "Inbox (3)". */ + const alike = (a: string, b: string): boolean => { + if (!a || !b) { + return false + } + + const one = a.toLowerCase().split(/\s+/).filter(Boolean) + const two = b.toLowerCase().split(/\s+/).filter(Boolean) + const both = one.filter(word => two.indexOf(word) !== -1).length + const all = one.length + two.filter(word => one.indexOf(word) === -1).length + + return all > 0 && both / all >= 0.5 + } + + /** How strongly a remembered element matches one just observed, 0 to 1. + * + * Ported from anchortree's re-bind ladder (Apache-2.0), minus its geometry + * rung: a centroid is only ever worth 0.1 there, it never reaches the 0.6 + * bar on its own, and carrying coordinates through the book to buy a + * tie-break is not worth the measurement. */ + const affinity = (was: PreviewActBinding, now: PreviewActBinding): number => { + // A button is not a link, however alike the rest of it reads. + if (was.role !== now.role) { + return 0 + } + + // Two elements that BOTH carry a stable attribute and disagree are the page + // telling us outright that they are different things. + if (was.stable && now.stable) { + return was.stable === now.stable ? 1 : 0 + } + + let score = 0 + + if (was.name && was.name === now.name) { + score += 0.6 + } else if (alike(was.name, now.name)) { + score += 0.4 + } + + if (was.path && was.path === now.path) { + score += 0.3 + } + + return score + } + + /** Mint a handle. Legible on purpose: the agent reads `btn-sign-in` in a + * three-line delta on turn nine and knows what it is, where `@e42` would + * send it back to an inventory twenty thousand tokens ago. Suffixes are + * never rewound, so a retired handle's name is not later handed to a + * different element on the same page. */ + const coin = (role: string, name: string): string => { + const coined = holder.coined || (holder.coined = {}) + const named = slug(name) + const stem = stemOf(role) + (named ? '-' + named : '') + const nth = coined[stem] || 0 + + coined[stem] = nth + 1 + + return nth ? stem + '-' + nth : stem + } + + /** Look at the page and say what is there — or, once there is something to + * compare against, only what moved. + * + * Re-sending the whole inventory every action is what took a ten-step + * session from 45k to 85k tokens of context: the page barely changes between + * a scroll and a click, and the agent was being charged for a fresh copy of + * it each time. */ + const survey = (max: number): PreviewActResult => { + // A navigation is a different page. Every handle on the old one is retired + // rather than rebound onto whatever now sits in the same place. + const fresh = holder.url !== here + + if (fresh) { + holder.book = [] + holder.coined = {} + } + + const book = holder.book || (holder.book = []) + const seen = sight(max) + const claimed: Record = {} + const kept: PreviewActBinding[] = [] + const added: PreviewElement[] = [] + const changed: PreviewElementChange[] = [] + const rebound: string[] = [] + const known = new Map() + const waiting: number[] = [] + let same = 0 + + for (const bound of book) { + known.set(bound.el, bound) + } + + // Pass one: the element object itself is still the one we remember. Free, + // and it is what happens on a scroll, a hover, and most clicks. + for (let i = 0; i < seen.elements.length; i++) { + const entry = seen.elements[i] + const bound = known.get(seen.nodes[i]) + + if (!bound) { + waiting.push(i) + + continue + } + + const moved = shifted(bound, entry) + + entry.ref = bound.ref + claimed[bound.ref] = true + kept.push(bound) + + if (!moved) { + same++ + continue + } + + bound.label = entry.label + bound.name = entry.label || entry.value || '' + bound.off = !!entry.disabled + bound.value = entry.value || '' + changed.push(moved) + } + + // Pass two: whatever is left either replaced something (a framework threw + // the node away and built a new one) or is genuinely new. The pool is only + // the handles whose element is GONE, which is both the correct candidate + // set and a small one. + const pool = book.filter(bound => !claimed[bound.ref] && !doc.contains(bound.el)) + + for (const i of waiting) { + const entry = seen.elements[i] + const el = seen.nodes[i] + + const now: PreviewActBinding = { + el, + label: entry.label, + name: entry.label || entry.value || '', + off: !!entry.disabled, + path: anchorOf(el), + ref: '', + role: entry.role, + stable: stableOf(el), + value: entry.value || '' + } + + let best: PreviewActBinding | undefined + let score = 0 + + for (const bound of pool) { + if (claimed[bound.ref]) { + continue + } + + const rung = affinity(bound, now) + + // Strictly better, so a tie goes to whichever candidate the page put + // first and the same page twice re-binds the same way. + if (rung >= rebindBar && rung > score) { + best = bound + score = rung + } + } + + if (best) { + // Same handle, new node. Reported as one word rather than a removal + // and an addition, because from the agent's side nothing happened — + // its handle still works and it has nothing to re-read. + entry.ref = best.ref + best.el = el + best.label = now.label + best.name = now.name + best.off = now.off + best.path = now.path + best.stable = now.stable + best.value = now.value + claimed[best.ref] = true + kept.push(best) + rebound.push(best.ref) + + continue + } + + now.ref = coin(entry.role, now.name) + entry.ref = now.ref + claimed[now.ref] = true + kept.push(now) + added.push(entry) + } + + // A handle nobody claimed is gone ONLY if its element really left. One that + // is still on the page but fell past `max` keeps working and is simply not + // mentioned — saying "removed" about something the agent can still click + // would be worse than saying nothing. + const removed: string[] = [] + + for (const bound of book) { + if (claimed[bound.ref]) { + continue + } + + if (doc.contains(bound.el) && visible(bound.el)) { + kept.push(bound) + + continue + } + + removed.push(bound.ref) + } + + holder.book = kept + holder.url = here + + // The delta has to actually be cheaper. When half the page is new there is + // nothing to reuse, and a delta is then just the inventory with extra + // framing around it. + const churn = added.length + changed.length + + if (fresh || action.full || churn * 2 >= seen.elements.length) { + return { elements: seen.elements, success: true } + } + + const delta: PreviewActDelta = { same } + + if (added.length) { + delta.added = added + } + + if (changed.length) { + delta.changed = changed + } + + if (removed.length) { + delta.removed = removed + } + + if (rebound.length) { + delta.rebound = rebound + } + + return { delta, success: true } + } + + /** Resolve the action's target: a handle from the book, else a selector. */ const resolve = (): { el?: Element; error?: string } => { const ref = (action.ref || '').trim() @@ -381,18 +816,17 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ return { error: 'The page navigated since the last snapshot, so ' + ref + ' no longer points anywhere. Call elements again.' } } - const index = Number(ref.replace(/^@e/, '')) - 1 - const el = holder.nodes && holder.nodes[index] + const bound = (holder.book || []).filter(entry => entry.ref === ref)[0] - if (!el) { + if (!bound) { return { error: 'Unknown element ' + ref + '. Call elements to get current refs.' } } - if (!doc.contains(el)) { + if (!doc.contains(bound.el)) { return { error: ref + ' has been removed from the page since the last snapshot. Call elements again.' } } - return { el } + return { el: bound.el } } const selector = (action.selector || '').trim() @@ -434,12 +868,12 @@ export function actInPage(doc: Document, holder: PreviewActHolder, action: Previ } if (action.kind === 'elements') { - const elements = collect(Math.max(1, Math.min(action.max || maxElements, maxElements))) + const looked = survey(Math.max(1, Math.min(action.max || maxElements, maxElements))) + const empty = !looked.delta && !(looked.elements || []).length return answer({ - elements, - note: elements.length ? undefined : 'No interactive elements found — the page may still be loading.', - success: true + ...looked, + note: empty ? 'No interactive elements found — the page may still be loading.' : undefined }) } diff --git a/tests/tools/test_drive_preview_tool.py b/tests/tools/test_drive_preview_tool.py index e7c0a06db3..9c714fb359 100644 --- a/tests/tools/test_drive_preview_tool.py +++ b/tests/tools/test_drive_preview_tool.py @@ -73,13 +73,24 @@ def test_payload_forwards_only_what_was_given(): seen = {} ap.drive_preview_tool( action="type", - ref="@e5", + ref="inp-password", text="hunter2", submit=True, callback=lambda p: seen.update(p) or json.dumps({"success": True}), ) - assert seen == {"action": "type", "ref": "@e5", "text": "hunter2", "submit": True} + assert seen == {"action": "type", "ref": "inp-password", "text": "hunter2", "submit": True} + + +def test_full_asks_the_renderer_for_a_whole_inventory(): + seen = {} + ap.drive_preview_tool( + action="elements", + full=True, + callback=lambda p: seen.update(p) or json.dumps({"success": True}), + ) + + assert seen == {"action": "elements", "full": True} def test_numeric_arguments_are_validated(): diff --git a/tools/annotate_preview_tool.py b/tools/annotate_preview_tool.py index 59919394e3..71747798b7 100644 --- a/tools/annotate_preview_tool.py +++ b/tools/annotate_preview_tool.py @@ -55,7 +55,7 @@ def annotate_preview_tool( if verb in ("add", "remove") and not (ref or selector): return tool_error( f"{verb} needs a ref from drive_preview action='elements' " - "(e.g. '@e5') or a CSS selector." + "(e.g. 'btn-sign-in') or a CSS selector." ) payload = { @@ -95,7 +95,7 @@ ANNOTATE_PREVIEW_SCHEMA = { "this is how you point at something. Use it to show the user what you " "found ('here are the three cheapest'), flag what you are about to " "change before you change it, or keep your place while you work " - "elsewhere on the page. Address elements by the same '@e5' refs " + "elsewhere on the page. Address elements by the same refs " "drive_preview action='elements' hands back. action='add' outlines an " "element and gives it an optional short label; 'hold' freezes the WHOLE " "visible field at once — every element the page offers, outlined and " @@ -120,7 +120,7 @@ ANNOTATE_PREVIEW_SCHEMA = { }, "ref": { "type": "string", - "description": "Element reference from drive_preview action='elements' (e.g. '@e5').", + "description": "Element reference from drive_preview action='elements' (e.g. 'btn-sign-in').", }, "selector": { "type": "string", diff --git a/tools/drive_preview_tool.py b/tools/drive_preview_tool.py index 7bdb1207a5..a90b0e8475 100644 --- a/tools/drive_preview_tool.py +++ b/tools/drive_preview_tool.py @@ -5,15 +5,22 @@ third leg — clicking, typing, scrolling, and history — so the agent can drive the same page the user is looking at instead of narrating from the outside. -Elements are addressed by ``@e1``-style refs from ``action="elements"``, the -same shape the ``browser_*`` tools use, so what the model knows about one -transfers to the other. Refs are only valid until the page navigates; the -renderer says so rather than clicking whatever now sits at that index. +Elements are addressed by refs from ``action="elements"`` that say what they +are: ``btn-sign-in``, ``inp-email``. A ref lasts as long as the page is open, +including across a re-render that destroys and rebuilds the element, and only a +navigation retires it — the renderer says so rather than acting on whatever now +occupies the spot. + +Because the refs hold, the renderer answers with a *delta* — what appeared, +what went, what changed, and what was rebound — instead of re-sending the whole +inventory after every click. That is the cheap half of the arrangement, and it +only works because the refs are legible enough to read on their own three turns +later. Round-trips through the gateway's blocking-prompt bridge like ``read_preview``: tui_gateway emits ``preview.act.request``, the renderer injects the interaction engine into the pane's webview and answers ``preview.act.respond`` with the -outcome plus a fresh inventory. This module is just schema + a thin dispatcher +outcome plus whatever moved. This module is just schema + a thin dispatcher over the platform-injected callback. Lives in the ``desktop_ui`` toolset, which the GUI gateway enables only for @@ -54,6 +61,7 @@ def drive_preview_tool( amount: Optional[int] = None, to: Optional[str] = None, limit: Optional[int] = None, + full: Optional[bool] = None, callback: Optional[Callable] = None, ) -> str: """Dispatch one interaction to the desktop renderer and return its outcome.""" @@ -66,7 +74,7 @@ def drive_preview_tool( if verb in NEEDS_TARGET and not (ref or selector): return tool_error( - f"{verb} needs a ref from action='elements' (e.g. '@e5') or a CSS selector." + f"{verb} needs a ref from action='elements' (e.g. 'btn-sign-in') or a CSS selector." ) if verb == "type" and text is None: @@ -88,6 +96,7 @@ def drive_preview_tool( ("text", text), ("key", key), ("submit", submit), + ("full", full), ("to", to), ("amount", None if amount is None else int(amount)), ("max", None if limit is None else int(limit)), @@ -123,12 +132,21 @@ ACT_PREVIEW_SCHEMA = { "chat. This is how you USE a web app the user is looking at: log in, " "fill a form, click through a flow, page a long document. ALWAYS call " "action='elements' first to get the current inventory of clickable and " - "typable things — each carries a ref like '@e5' plus its role, label, " - "and value — then act with that ref instead of guessing a selector. " - "Every action answers with a refreshed inventory and the live url/" - "title, so a click that navigated is visible immediately and you can " - "chain the next step without re-reading. Refs are invalidated by a " - "navigation; when told they are stale, call elements again. The mouse " + "typable things — each carries a ref like 'btn-sign-in' or 'inp-email' " + "plus its role, label, and value — then act with that ref instead of " + "guessing a selector. A ref keeps working for as long as the page is " + "open, INCLUDING across a re-render that rebuilds the element, so hold " + "onto the ones you were given. " + "Every action answers with the live url/title plus what moved: the " + "first look at a page returns the full 'elements' inventory, and after " + "that you get a 'delta' instead — 'added' entries in full, 'changed' " + "entries carrying only the ref and whichever of label/value/disabled " + "actually moved, 'removed' and 'rebound' as bare ref lists, and 'same' " + "counting the refs that held. A 'rebound' ref needs NO action from you; it " + "means the page rebuilt that element and your ref already follows it. " + "Anything not mentioned in a delta is unchanged, so do not re-read the " + "page to check. Only a navigation invalidates refs; when told they are " + "stale, call elements again. The mouse " "and keyboard are real: the pointer travels to its target and the page " "sees genuine input, so hover menus open and hover-only controls work. " "Actions: 'elements' (inventory), 'click', 'hover' (move the pointer " @@ -157,7 +175,7 @@ ACT_PREVIEW_SCHEMA = { }, "ref": { "type": "string", - "description": "Element reference from the last elements call (e.g. '@e5').", + "description": "Element reference from any earlier elements call (e.g. 'btn-sign-in'). Good until the page navigates.", }, "selector": { "type": "string", @@ -185,6 +203,10 @@ ACT_PREVIEW_SCHEMA = { "type": "integer", "description": "For 'elements': cap the inventory. Defaults to the per-call maximum.", }, + "full": { + "type": "boolean", + "description": "For 'elements': re-read the whole page instead of a delta. Rarely needed.", + }, }, "required": ["action"], }, @@ -205,6 +227,7 @@ registry.register( amount=args.get("amount"), to=args.get("to"), limit=args.get("max"), + full=args.get("full"), callback=kw.get("callback"), ), emoji="🖱️", diff --git a/website/docs/reference/tools-reference.md b/website/docs/reference/tools-reference.md index 9ccfe87e25..5363e35187 100644 --- a/website/docs/reference/tools-reference.md +++ b/website/docs/reference/tools-reference.md @@ -199,7 +199,7 @@ messaging, and cron sessions. | `open_preview` | Open a web URL, localhost dev-server URL, or file path in the preview pane beside the chat in the Hermes desktop app. | — | | `close_preview` | Close the preview pane beside the chat, or one tab inside it. Omit `url` to close the whole pane; pass a URL or file path to close that tab. | — | | `read_preview` | Read what's currently shown in the preview pane of the Hermes desktop GUI — the in-app Browser's page text (URL + title + rendered text, pageable with `start`/`count`), or a file/artifact tab's identity. | — | -| `drive_preview` | Interact with the page open in the in-app browser: `elements` inventories what's clickable and typable (each with an `@e1`-style ref, role, label, and value), then `click`, `hover`, `type`, `scroll`, and `press` act on a ref, and `back`/`forward`/`reload` drive the pane's history. The pointer and keyboard are real input, so hover menus open. Every action answers with the live URL and a refreshed inventory. | — | +| `drive_preview` | Interact with the page open in the in-app browser: `elements` inventories what's clickable and typable (each with a ref that names it, like `btn-sign-in` or `inp-email`, plus role, label, and value), then `click`, `hover`, `type`, `scroll`, and `press` act on a ref, and `back`/`forward`/`reload` drive the pane's history. The pointer and keyboard are real input, so hover menus open. A ref lasts until the page navigates, including across a re-render that rebuilds the element, so after the first inventory every action answers with just a delta — what was added, removed, changed, or rebound — instead of the whole page again. | — | | `annotate_preview` | Outline an element in the in-app browser and leave the mark up until it's removed — the deliberate counterpart to the transient cues `drive_preview` draws as it works. `add` marks a ref with an optional short label, `remove` takes one down, `clear` takes them all. Marks follow their element and vanish with it, so a navigation clears them. | — | | `read_window_below` | Identify the OS window directly underneath the Hermes desktop window — app name, title, bounds (metadata only, never pixels). On macOS, other apps' titles appear only when Screen Recording is already granted; the tool never prompts for it. | — | | `focus_pane` | Reveal and focus a pane in the Hermes desktop app (chat, files, terminal, review, sessions). | — |