diff --git a/pkg/spec/src/web-runtime.ts b/pkg/spec/src/web-runtime.ts index fb26b2c..eac9ff3 100644 --- a/pkg/spec/src/web-runtime.ts +++ b/pkg/spec/src/web-runtime.ts @@ -260,6 +260,20 @@ function queryAllElements(root: ParentNode, selector: unknown): Element[] { if (xpath) return evaluateXPathAll(xpath, root as Node); return []; } + // A selector path: every match of the first segment is searched for the rest, + // concatenated in walk order, mirroring FindAllBySelectorPath in + // internal/hierarchy. Falling through to the object branch (as this did) + // returned NOTHING for a path on web while native returned matches, so a spec + // reading state.ax.findAll([{screen}, {row}]) saw an empty list on web and + // every property over it passed by having nothing to check. + if (Array.isArray(selector)) { + const head = selector[0]; + if (head === undefined) return []; + const heads = queryAllElements(root, head); + if (selector.length === 1) return heads; + const rest = selector.slice(1); + return heads.flatMap((element) => queryAllElements(element, rest)); + } if (selector && typeof selector === "object" && !Array.isArray(selector)) { const { css, xpath } = selectorFromObject(selector as Record); if (css) return deepQueryAll(css, root); @@ -284,7 +298,37 @@ function evaluateXPathAll(xpath: string, root: Node): Element[] { return out; } -function elementHandle(element: Element): Record { +// SELECTOR_TAG is the key an ax element carries the selector it was found by, +// the same key the goja host writes (internal/verifier/bindings.go tagSelector). +// The shared serializer (runtime-entry.ts pointOf) reads it off an author +// target, so a spec's Tap({ on: state.ax.find(...) }) reaches the runner naming +// the control it acted on instead of a bare pair of coordinates. Without it +// `lastAction.on` is empty on web for exactly the actions a spec authored. +const SELECTOR_TAG = "__sanderlingSelector"; + +// selectorTag renders a selector argument in the canonical "k:v" grammar the +// hierarchy package parses, chains joined by " > ". It mirrors +// selectorStringFromJS in internal/verifier/marshal.go, so an element found by +// the same selector is labelled with the SAME string on both hosts. +function selectorTag(selector: unknown): string { + if (typeof selector === "string") return selector; + if (Array.isArray(selector)) { + return selector + .map(selectorTag) + .filter((segment) => segment !== "") + .join(" > "); + } + if (selector && typeof selector === "object") { + const source = selector as Record; + return Object.keys(source) + .filter((key) => key !== SELECTOR_TAG && source[key] !== undefined && source[key] !== null) + .map((key) => `${key}:${String(source[key])}`) + .join(" "); + } + return ""; +} + +function elementHandle(element: Element, selector: unknown): Record { const rect = element.getBoundingClientRect(); const x = Math.round(rect.left + rect.width / 2); const y = Math.round(rect.top + rect.height / 2); @@ -318,12 +362,15 @@ function elementHandle(element: Element): Record { ...datasetCopy, }, dataset: datasetCopy, - find(selector: unknown): unknown { - const child = queryElement(element, selector); - return child ? elementHandle(child) : undefined; + [SELECTOR_TAG]: selectorTag(selector), + find(childSelector: unknown): unknown { + const child = queryElement(element, childSelector); + return child ? elementHandle(child, childSelector) : undefined; }, - findAll(selector: unknown): unknown[] { - return queryAllElements(element, selector).map(elementHandle); + findAll(childSelector: unknown): unknown[] { + return queryAllElements(element, childSelector).map((child) => + elementHandle(child, childSelector), + ); }, }; } @@ -332,10 +379,12 @@ function buildAx(): unknown { return { find(selector: unknown): unknown { const element = queryElement(document, selector); - return element ? elementHandle(element) : undefined; + return element ? elementHandle(element, selector) : undefined; }, findAll(selector: unknown): unknown[] { - return queryAllElements(document, selector).map(elementHandle); + return queryAllElements(document, selector).map((element) => + elementHandle(element, selector), + ); }, }; } @@ -372,13 +421,22 @@ if (typeof globalThis.addEventListener === "function") { }); } +// lastAction is what the previous step actually did, pushed in by the Go runner +// (internal/runner, via __sanderlingSetLastAction__) before each extractor +// evaluation, in the shape internal/verifier/marshal.go builds for goja. The +// page cannot derive it: only the runner knows whether the action it picked was +// really applied, and under --generator llm the action is not picked here at +// all. Hardcoding null here, as this file used to, makes every spec property +// that reads state.lastAction vacuously true on web. +let lastAction: unknown = null; + function buildState(): unknown { return { snapshots: {}, ax: buildAx(), document, window, - lastAction: null, + lastAction, time: 0, logs: [], exceptions: capturedExceptions.slice(), @@ -424,6 +482,11 @@ const runtime = { // the host invoking the extractor/next-action callbacks. defineLockedGlobal("__sanderling__", runtime); +// The host calls this once per step, before __sanderlingExtractors__. +defineLockedGlobal("__sanderlingSetLastAction__", (value: unknown) => { + lastAction = value ?? null; +}); + // writable:false stops a page script from shadowing the runtime via plain // assignment (the realistic in-page threat). configurable:true is required so // unit tests sharing one process can reinstall a fake via defineProperty; a @@ -567,6 +630,53 @@ function expandShadowContent(elements: HTMLElement[], into: HTMLElement[]): void } } +// IDENTITY_KEYS is the ladder a target's selector is built from, mirroring +// selectorForElement in internal/verifier/worker.go: the id first (where +// Compose for Web lands a testTag), then data-testid, then the description. +// Every key here is one the goja host's selector grammar already understands, +// so the runner can re-resolve the target it names. `desc` reads the same three +// attributes, in the same order, that the hierarchy dump folds into +// content-desc (internal/driver/chrome/driver.go); reading fewer of them would +// let a selector this side calls unique resolve to a different element on the +// Go side, which re-routes the action to whatever the dump matched first. +const IDENTITY_KEYS: ReadonlyArray string]> = [ + ["id", (element) => element.id], + ["data-testid", (element) => element.dataset.testid ?? ""], + [ + "desc", + (element) => + element.getAttribute("aria-label") || + element.getAttribute("alt") || + element.getAttribute("title") || + "", + ], +]; + +// selectorsFor names each enumerated element, or leaves it unnamed. A value is +// only used when it occurs ONCE across the enumeration, so an action carrying +// the selector can never be re-resolved onto a sibling that shares the value +// (folio's Home screen has many AccountCards under one testTag). Unnamed +// elements keep the coordinates-only behaviour the web host always had. +function selectorsFor(elements: readonly HTMLElement[]): Array { + const counts = IDENTITY_KEYS.map(() => new Map()); + for (const element of elements) { + IDENTITY_KEYS.forEach(([, read], index) => { + const value = read(element); + if (!value) return; + const seen = counts[index]!; + seen.set(value, (seen.get(value) ?? 0) + 1); + }); + } + return elements.map((element) => { + for (let index = 0; index < IDENTITY_KEYS.length; index++) { + const [key, read] = IDENTITY_KEYS[index]!; + const value = read(element); + if (value && counts[index]!.get(value) === 1) return `${key}:${value}`; + } + return undefined; + }); +} + // collectTargets walks the document ONCE and reports every element with the facts // the shared eligibility rule reads. The tappable/editable membership sets are // resolved by selector first so the DOM's answer to "clickable" and "editable" @@ -576,8 +686,11 @@ function collectTargets(): TargetElement[] { const editable = new Set( (deepQueryAll(EDITABLE_SELECTOR, document) as HTMLElement[]).filter(isEditableElement), ); - return targetElements().map((element) => ({ + const elements = targetElements(); + const selectors = selectorsFor(elements); + return elements.map((element, index) => ({ ...pointOf(element), + selector: selectors[index], clickable: clickable.has(element), enabled: !(element as HTMLButtonElement).disabled, editable: editable.has(element), @@ -637,6 +750,7 @@ export const __testing__ = { evaluateExtractors, selectorFromString, selectorFromObject, + selectorTag, xpathStringLiteral, }; diff --git a/pkg/spec/test/web-dom-harness.ts b/pkg/spec/test/web-dom-harness.ts index e0d3bdb..cc0e671 100644 --- a/pkg/spec/test/web-dom-harness.ts +++ b/pkg/spec/test/web-dom-harness.ts @@ -14,6 +14,15 @@ export interface FakeElementSpec { y: number; width: number; height: number; + // id/testid/label/alt/title are how the host names a target: it builds the + // selector an action carries from them, so a fake needs them to exercise that + // naming. alt and title are the fallbacks the hierarchy dump folds into + // content-desc, which the host has to fall back to in the same order. + id?: string; + testid?: string; + label?: string; + alt?: string; + title?: string; // clickable/editable place the element in the selector sets the host queries; // the fake answers those queries directly rather than matching CSS. clickable?: boolean; @@ -28,6 +37,9 @@ export interface FakeElement extends FakeElementSpec { tagName: string; type: string; isContentEditable: boolean; + id: string; + dataset: Record; + getAttribute(name: string): string | null; scrollHeight: number; clientHeight: number; scrollWidth: number; @@ -49,6 +61,10 @@ export function fakeElement(spec: FakeElementSpec): FakeElement { tagName: spec.tag.toUpperCase(), type: spec.tag === "input" ? "text" : "", isContentEditable: editable && spec.tag !== "input" && spec.tag !== "textarea", + id: spec.id ?? "", + dataset: { testid: spec.testid }, + getAttribute: (name: string) => + ({ "aria-label": spec.label, alt: spec.alt, title: spec.title })[name] ?? null, scrollHeight: spec.overflows ? spec.height * 2 : spec.height, clientHeight: spec.height, scrollWidth: spec.width, diff --git a/pkg/spec/test/web-runtime.test.ts b/pkg/spec/test/web-runtime.test.ts index 70c1160..0e80c5b 100644 --- a/pkg/spec/test/web-runtime.test.ts +++ b/pkg/spec/test/web-runtime.test.ts @@ -128,6 +128,53 @@ test("queryTargets reports a disabled control rather than dropping it", () => { }); }); +// A target with no selector is a target no property can name. The action the +// picker builds from it carries coordinates only, so `lastAction.on` is empty +// and any property matching on WHICH control was acted upon cannot fire. +test("queryTargets names a uniquely identified target", () => { + const submit = fakeElement({ + tag: "button", x: 0, y: 0, width: 40, height: 20, clickable: true, id: "TxnSubmit", + }); + const byTestid = fakeElement({ + tag: "button", x: 0, y: 40, width: 40, height: 20, clickable: true, testid: "cancel", + }); + const byLabel = fakeElement({ + tag: "button", x: 0, y: 80, width: 40, height: 20, clickable: true, label: "Close", + }); + // alt and title are the fallbacks the hierarchy dump folds into content-desc, + // so the host has to fall back to them in the same order or a name it calls + // unique resolves to a different element on the Go side. + const byAlt = fakeElement({ tag: "img", x: 0, y: 120, width: 40, height: 20, alt: "Logo" }); + const byTitle = fakeElement({ tag: "div", x: 0, y: 160, width: 40, height: 20, title: "Help" }); + const anonymous = fakeElement({ tag: "div", x: 0, y: 200, width: 10, height: 10 }); + withFakeDocument([submit, byTestid, byLabel, byAlt, byTitle, anonymous], () => { + const targets = host.queryTargets(); + assert.equal(targets[0]!.selector, "id:TxnSubmit"); + assert.equal(targets[1]!.selector, "data-testid:cancel"); + assert.equal(targets[2]!.selector, "desc:Close"); + assert.equal(targets[3]!.selector, "desc:Logo"); + assert.equal(targets[4]!.selector, "desc:Help"); + assert.equal(targets[5]!.selector, undefined); + }); +}); + +// A repeated id (folio's Home screen renders one AccountCard testTag per +// account) names no single element, so the runner would re-resolve the action +// onto whichever sibling it found first. Better unnamed than mis-aimed. +test("queryTargets leaves duplicated identities unnamed", () => { + const first = fakeElement({ + tag: "div", x: 0, y: 0, width: 40, height: 20, clickable: true, id: "AccountCard", + }); + const second = fakeElement({ + tag: "div", x: 0, y: 40, width: 40, height: 20, clickable: true, id: "AccountCard", + }); + withFakeDocument([first, second], () => { + const targets = host.queryTargets(); + assert.equal(targets[0]!.selector, undefined); + assert.equal(targets[1]!.selector, undefined); + }); +}); + test("queryTargets caches within a tick until reset", () => { const button = fakeElement({ tag: "button", x: 0, y: 0, width: 10, height: 10, clickable: true }); withFakeDocument([button], () => { @@ -204,6 +251,36 @@ test("an uncaught cross-extractor read aborts evaluateExtractors", () => { ); }); +// state.lastAction is the one piece of state the page cannot observe for +// itself: only the runner knows which action it actually applied. While the web +// runtime hardcoded null there, a spec property gated on the last action (e.g. +// folio's submitMovesBalanceByTypedAmount, which only looks at taps on +// TxnSubmit) was vacuously true on web forever, and the run went green having +// checked nothing. +function lastActionSeenByASpec(pushed: unknown): unknown { + const setLastAction = (globalThis as Record) + .__sanderlingSetLastAction__ as (value: unknown) => void; + __testing__.extractors.length = 0; + __testing__.runtime.extract((state) => (state as { lastAction: unknown }).lastAction); + let out: Record = {}; + withState(() => { + setLastAction(pushed); + out = __testing__.evaluateExtractors(); + }); + return out[0]; +} + +test("state.lastAction carries the action the host pushed", () => { + const action = { kind: "Tap", on: "id:TxnSubmit" }; + assert.deepEqual(lastActionSeenByASpec(action), action); +}); + +test("state.lastAction is null when the host pushed nothing", () => { + // The first step of a run, and any step whose action was never applied: the + // goja host reports null there, so the web host must too. + assert.equal(lastActionSeenByASpec(null), null); +}); + // sanitize runs over every extractor's return value before it leaves the // runtime. A user extractor that returns a page object reachable from // document/window can be self-referential, carry functions, or nest deeply; @@ -338,3 +415,105 @@ test("selectorFromObject text-only selector becomes an XPath", () => { xpath: `//*[normalize-space(text())="Go"]`, }); }); + +// An ax element is labelled with the selector it was found by, in the same +// canonical grammar selectorStringFromJS emits in internal/verifier/marshal.go. +// The label is what a spec's own Tap({ on: state.ax.find(...) }) carries to the +// runner: with no label the action is coordinates only, `lastAction.on` is +// empty, and a property matching on WHICH control was tapped cannot fire. +const { selectorTag } = __testing__; + +test("selectorTag renders the selector shapes the goja host renders", () => { + assert.equal(selectorTag("testTag:TxnSubmit"), "testTag:TxnSubmit"); + assert.equal(selectorTag({ testTag: "TxnSubmit" }), "testTag:TxnSubmit"); + assert.equal( + selectorTag([{ testTag: "AddTransactionScreen" }, { testTag: "TxnSubmit" }]), + "testTag:AddTransactionScreen > testTag:TxnSubmit", + ); + assert.equal(selectorTag({ testTag: "Row", "aria-label": "first" }), "testTag:Row aria-label:first"); + assert.equal(selectorTag(undefined), ""); +}); + +// A selector path scopes the second segment to each match of the first. It +// returned nothing at all on web while returning matches on native, so folio's +// accounts/totalBalance extractors (findAll([{HomeScreen}, {AccountCard}])) +// were empty on every web step and the properties over them checked nothing. +test("ax.findAll resolves a selector path segment by segment", () => { + const rect = { left: 0, top: 0, right: 10, bottom: 10, width: 10, height: 10 }; + const node = (id: string, answers: Record = {}) => ({ + id, + tagName: "DIV", + className: "", + textContent: id, + dataset: {}, + getAttribute: () => null, + getBoundingClientRect: () => rect, + querySelectorAll: (selector: string) => answers[selector] ?? [], + }); + const cardCss = `:is([data-testid="AccountCard"], [id="AccountCard"])`; + const screenCss = `:is([data-testid="HomeScreen"], [id="HomeScreen"])`; + const cards = [node("first"), node("second")]; + const home = node("HomeScreen", { [cardCss]: cards }); + + const g = globalThis as Record; + const originalDocument = g.document; + const originalWindow = g.window; + g.document = { querySelectorAll: (selector: string) => (selector === screenCss ? [home] : []) }; + g.window = {}; + try { + __testing__.extractors.length = 0; + __testing__.runtime.extract((state) => { + const ax = (state as { ax: { findAll(s: unknown): Record[] } }).ax; + return ax + .findAll([{ testTag: "HomeScreen" }, { testTag: "AccountCard" }]) + .map((card) => card.text); + }); + const values = __testing__.evaluateExtractors(); + // Scoped to the head match: the cards come from the HomeScreen node, not + // from a document-wide sweep for AccountCard. + assert.deepEqual(values[0], ["first", "second"]); + } finally { + g.document = originalDocument; + g.window = originalWindow; + } +}); + +test("ax.find and ax.findAll label the element with its selector", () => { + const rect = { left: 0, top: 0, right: 10, bottom: 10, width: 10, height: 10 }; + const submit = { + id: "TxnSubmit", + tagName: "DIV", + className: "", + textContent: "Submit", + dataset: {}, + getAttribute: () => null, + getBoundingClientRect: () => rect, + }; + const matches = `:is([data-testid="TxnSubmit"], [id="TxnSubmit"])`; + const g = globalThis as Record; + const originalDocument = g.document; + const originalWindow = g.window; + g.document = { querySelectorAll: (selector: string) => (selector === matches ? [submit] : []) }; + g.window = {}; + try { + __testing__.extractors.length = 0; + __testing__.runtime.extract((state) => { + const ax = (state as { ax: { find(s: unknown): Record | undefined } }).ax; + return ax.find({ testTag: "TxnSubmit" }); + }); + __testing__.runtime.extract((state) => { + const ax = (state as { ax: { findAll(s: unknown): Record[] } }).ax; + return ax.findAll({ testTag: "TxnSubmit" }); + }); + const values = __testing__.evaluateExtractors(); + const found = values[0] as Record; + assert.equal(found.__sanderlingSelector, "testTag:TxnSubmit"); + // findAll passes each element through map(); passing the callback by + // reference would hand the array INDEX to the runtime as the selector. + const all = values[1] as Record[]; + assert.equal(all[0]!.__sanderlingSelector, "testTag:TxnSubmit"); + } finally { + g.document = originalDocument; + g.window = originalWindow; + } +});