Merge pull request #2027 from Automattic/fix/2022-unique-action-corpus-selectors

fix: generate unique action corpus selectors

Author: chubes4

packages/runtime-playground/src/browser-actions-runner.ts

@@ -198,8 +198,9 @@ export async function runBrowserActionsCommand({
           throw error
         }
       }
-      const descriptors = await discoverBrowserActionCorpusDescriptors(page)
-      actionCorpusArtifact = browserActionCorpusArtifact(runPlan.actionCorpus, descriptors, now())
+      const discovery = await discoverBrowserActionCorpusDescriptors(page)
+      actionCorpusArtifact = browserActionCorpusArtifact(runPlan.actionCorpus, discovery.descriptors, now())
+      actionCorpusArtifact.plan.diagnostics.push(...discovery.diagnostics)
       await artifactSession.writeJson("actionCorpus", "action-corpus.json", actionCorpusArtifact)
       const corpusSteps = actionCorpusArtifact.plan.steps
       steps.unshift(...corpusSteps)
@@ -605,23 +606,87 @@ function browserActionCorpusFromArgs(args: string[]): BrowserActionCorpusContrac
   return browserActionCorpusContract(parsed)
 }
 
-async function discoverBrowserActionCorpusDescriptors(page: Page): Promise<BrowserActionCorpusDescriptor[]> {
+export async function discoverBrowserActionCorpusDescriptors(page: Page): Promise<{
+  descriptors: BrowserActionCorpusDescriptor[]
+  diagnostics: Array<{ code: string; message: string; metadata?: Record<string, unknown> }>
+}> {
   return await page.evaluate(() => {
+    const MAX_REJECTION_DIAGNOSTICS = 20
     const descriptors: BrowserActionCorpusDescriptor[] = []
+    const rejected: Array<{ kind: string; tag: string; label: string }> = []
     const cssEscape = (value: string) => {
       const escapeFn = (globalThis as typeof globalThis & { CSS?: { escape?: (raw: string) => string } }).CSS?.escape
       return escapeFn ? escapeFn(value) : value.replace(/[^a-zA-Z0-9_-]/g, "\\$&")
     }
     const text = (value: string | null | undefined) => (value || "").replace(/\s+/g, " ").trim().slice(0, 120)
-    const selectorFor = (element: Element) => {
+    const attributeSelector = (name: string, value: string) => `[${name}=${cssEscape(value)}]`
+    const uniquelySelects = (selector: string, element: Element) => {
+      try {
+        const matches = document.querySelectorAll(selector)
+        return matches.length === 1 && matches[0] === element
+      } catch {
+        return false
+      }
+    }
+    const selectorFor = (element: Element): string | undefined => {
+      const tag = element.tagName.toLowerCase()
+      const candidates: string[] = []
+      const seen = new Set<string>()
+      const addCandidate = (selector: string) => {
+        if (!seen.has(selector)) {
+          seen.add(selector)
+          candidates.push(selector)
+        }
+      }
       const id = element.getAttribute("id")
-      if (id) return `#${cssEscape(id)}`
-      const name = element.getAttribute("name")
-      if (name) return `${element.tagName.toLowerCase()}[name="${cssEscape(name)}"]`
-      const aria = element.getAttribute("aria-label")
-      if (aria) return `${element.tagName.toLowerCase()}[aria-label="${cssEscape(aria)}"]`
-      const type = element.getAttribute("type")
-      return `${element.tagName.toLowerCase()}${type ? `[type="${cssEscape(type)}"]` : ""}:nth-of-type(${Array.from(element.parentElement?.children || []).filter((child) => child.tagName === element.tagName).indexOf(element) + 1})`
+      if (id) addCandidate(`#${cssEscape(id)}`)
+
+      const attributes = new Map(["aria-label", "aria-labelledby", "name", "type", "value", "title", "href", "role"].flatMap((name) => {
+        const value = element.getAttribute(name)
+        return value ? [[name, value] as const] : []
+      }))
+      const addAttributeCombination = (...names: string[]) => {
+        if (names.every((name) => attributes.has(name))) {
+          addCandidate(`${tag}${names.map((name) => attributeSelector(name, attributes.get(name)!)).join("")}`)
+        }
+      }
+      for (const names of [
+        ["aria-label"], ["aria-labelledby"],
+        ["name", "type", "value"], ["name", "type"], ["name", "value"], ["name"],
+        ["href"], ["title"],
+        ["role", "type", "value"], ["role", "type"], ["role"],
+        ["type", "value"], ["value"], ["type"],
+      ]) {
+        addAttributeCombination(...names)
+      }
+
+      const form = (element as HTMLInputElement).form
+      const formId = form?.getAttribute("id")
+      if (form && formId) {
+        const formSelector = `#${cssEscape(formId)}`
+        if (uniquelySelects(formSelector, form)) {
+          for (const candidate of [...candidates]) addCandidate(`${formSelector} ${candidate}`)
+        }
+      }
+      for (const candidate of candidates) {
+        if (uniquelySelects(candidate, element)) return candidate
+      }
+
+      const parts: string[] = []
+      let current: Element | null = element
+      while (current) {
+        let part = current.tagName.toLowerCase()
+        const parent: Element | null = current.parentElement
+        if (parent) {
+          const sameTagSiblings = Array.from(parent.children).filter((child) => child.tagName === current?.tagName)
+          if (sameTagSiblings.length > 1) part += `:nth-of-type(${sameTagSiblings.indexOf(current) + 1})`
+        }
+        parts.unshift(part)
+        const candidate = parts.join(" > ")
+        if (uniquelySelects(candidate, element)) return candidate
+        current = parent
+      }
+      return undefined
     }
     const labelFor = (element: Element) => {
       const labelledBy = element.getAttribute("aria-labelledby")
@@ -636,7 +701,7 @@ async function discoverBrowserActionCorpusDescriptors(page: Page): Promise<Brows
       if (label && text(label.textContent)) return text(label.textContent)
       return text(element.textContent)
     }
-    const descriptorId = (kind: string, element: Element) => `${kind}:${selectorFor(element)}:${element.getAttribute("name") || ""}:${labelFor(element)}`
+    const descriptorId = (kind: string, selector: string, element: Element) => `${kind}:${selector}:${element.getAttribute("name") || ""}:${labelFor(element)}`
     const visible = (element: Element) => {
       const htmlElement = element as HTMLElement
       const style = window.getComputedStyle(htmlElement)
@@ -649,8 +714,12 @@ async function discoverBrowserActionCorpusDescriptors(page: Page): Promise<Brows
       const input = element as HTMLInputElement
       const kind = tag === "a" ? "link" : tag === "button" ? "button" : tag === "textarea" ? "textarea" : tag === "select" ? "select" : "input"
       const selector = selectorFor(element)
+      if (!selector) {
+        rejected.push({ kind, tag, label: labelFor(element) })
+        return
+      }
       const descriptor: BrowserActionCorpusDescriptor = {
-        id: descriptorId(kind, element),
+        id: descriptorId(kind, selector, element),
         kind,
         selector,
         label: labelFor(element),
@@ -665,7 +734,19 @@ async function discoverBrowserActionCorpusDescriptors(page: Page): Promise<Brows
       }
       descriptors.push(descriptor)
     })
-    return descriptors
+    const diagnostics: Array<{ code: string; message: string; metadata?: Record<string, unknown> }> = rejected.slice(0, MAX_REJECTION_DIAGNOSTICS).map((item) => ({
+      code: "browser_action_corpus_selector_not_unique",
+      message: "An actionable control was rejected because discovery could not produce a selector resolving uniquely to that element.",
+      metadata: item,
+    }))
+    if (rejected.length > MAX_REJECTION_DIAGNOSTICS) {
+      diagnostics[MAX_REJECTION_DIAGNOSTICS - 1] = {
+        code: "browser_action_corpus_selector_rejections_truncated",
+        message: "Additional actionable controls without unique selectors were omitted from diagnostics.",
+        metadata: { rejected: rejected.length, retainedDiagnostics: MAX_REJECTION_DIAGNOSTICS - 1 },
+      }
+    }
+    return { descriptors, diagnostics }
   })
 }
 

tests/browser-action-corpus.test.ts

@@ -1,5 +1,6 @@
 import assert from "node:assert/strict"
 import test from "node:test"
+import { chromium } from "playwright"
 
 import { getCommandDefinition } from "../packages/runtime-core/src/command-registry.js"
 import {
@@ -10,6 +11,7 @@ import {
   planBrowserActionCorpus,
   type BrowserActionCorpusDescriptor,
 } from "../packages/runtime-core/src/browser-interaction.js"
+import { discoverBrowserActionCorpusDescriptors } from "../packages/runtime-playground/src/browser-actions-runner.js"
 
 const representativeFormDescriptors: BrowserActionCorpusDescriptor[] = [
   { id: "input:#title::Title", kind: "input", selector: "#title", label: "Title", name: "title", type: "text", formId: "post" },
@@ -60,3 +62,78 @@ test("browser action corpus artifact records replayable steps and correlated obs
   assert.equal(artifact.plan.replay.startUrl, "/wp-admin/post-new.php")
   assert.ok(artifact.plan.descriptors.some((descriptor) => descriptor.formId === "post"))
 })
+
+test("browser action corpus discovery retains only uniquely resolvable deterministic selectors", async () => {
+  const browser = await chromium.launch({ headless: true })
+  try {
+    const page = await browser.newPage()
+    await page.setContent(`
+      <style>a, button, input { display: inline-block; width: 100px; height: 20px; }</style>
+      <nav><a href="/shared">Open</a><a href="/news">News</a></nav>
+      <main><a href="/shared">Open</a><section><a href="/feature">Feature</a></section></main>
+      <form id="primary-form">
+        <label>Search <input name="query" type="search"></label>
+        <button aria-label='Save "draft" \\ copy' type="button">Save</button>
+      </form>
+      <form id="secondary-form">
+        <label>Search <input name="query" type="search"></label>
+        <button aria-label="Save" type="button">Save</button>
+      </form>
+      <input id="unique-email" name="contact" type="email">
+    `)
+    // tsx names nested functions with this helper before Playwright serializes page callbacks.
+    await page.evaluate("globalThis.__name = value => value")
+
+    const first = await discoverBrowserActionCorpusDescriptors(page)
+    const second = await discoverBrowserActionCorpusDescriptors(page)
+
+    assert.deepEqual(second, first)
+    assert.equal(first.diagnostics.length, 0)
+    assert.ok(first.descriptors.length >= 9)
+    assert.ok(first.descriptors.some((descriptor) => descriptor.selector === "#unique-email"))
+    assert.equal(new Set(first.descriptors.map((descriptor) => descriptor.id)).size, first.descriptors.length)
+    for (const descriptor of first.descriptors) {
+      const matches = await page.locator(descriptor.selector).count()
+      assert.equal(matches, 1, `${descriptor.selector} must resolve uniquely`)
+    }
+
+    const sharedLinks = first.descriptors.filter((descriptor) => descriptor.href?.endsWith("/shared"))
+    assert.equal(sharedLinks.length, 2)
+    assert.ok(sharedLinks.every((descriptor) => descriptor.selector !== "a:nth-of-type(1)"))
+    const repeatedInputs = first.descriptors.filter((descriptor) => descriptor.name === "query")
+    assert.equal(repeatedInputs.length, 2)
+    assert.equal(new Set(repeatedInputs.map((descriptor) => descriptor.selector)).size, 2)
+
+    const contract = browserActionCorpusContract({ seed: "unique-selector-seed", context: "browser", maxSteps: 9 })
+    assert.deepEqual(planBrowserActionCorpus(contract, second.descriptors), planBrowserActionCorpus(contract, first.descriptors))
+  } finally {
+    await browser.close()
+  }
+})
+
+test("browser action corpus discovery rejects controls that lose unique identity", async () => {
+  const browser = await chromium.launch({ headless: true })
+  try {
+    const page = await browser.newPage()
+    await page.setContent('<style>button { width: 100px; height: 20px; }</style><button id="stable">Stable</button><button id="rerendered">Rerendered</button>')
+    await page.evaluate("globalThis.__name = value => value")
+    await page.evaluate(() => {
+      const element = document.querySelector("#rerendered")!
+      const getAttribute = element.getAttribute.bind(element)
+      element.getAttribute = (name: string) => {
+        const value = getAttribute(name)
+        if (name === "id") element.remove()
+        return value
+      }
+    })
+
+    const discovery = await discoverBrowserActionCorpusDescriptors(page)
+
+    assert.deepEqual(discovery.descriptors.map((descriptor) => descriptor.selector), ["#stable"])
+    assert.equal(discovery.diagnostics.length, 1)
+    assert.equal(discovery.diagnostics[0]?.code, "browser_action_corpus_selector_not_unique")
+    assert.equal(discovery.diagnostics[0]?.metadata?.label, "Rerendered")
+  } finally {
+    await browser.close()
+  }
+})