From aee30ecfec727126de82b1692fce0987c10fc208 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 30 Oct 2026 14:56:27 -0500 Subject: [PATCH] fix(evals): batch task search result readback across renders Co-Authored-By: Paperclip --- tests/runner-e2e/PRODUCT-JOURNEYS.md | 8 +- .../task-search-filter.test.ts | 16 +++- .../product-journeys/task-search-filter.ts | 19 ++--- tests/runner-e2e/task-search-filter.spec.ts | 76 ++++++++++++++++++- 4 files changed, 102 insertions(+), 17 deletions(-) diff --git a/tests/runner-e2e/PRODUCT-JOURNEYS.md b/tests/runner-e2e/PRODUCT-JOURNEYS.md index 59bd750a262..d92ef26d490 100644 --- a/tests/runner-e2e/PRODUCT-JOURNEYS.md +++ b/tests/runner-e2e/PRODUCT-JOURNEYS.md @@ -70,8 +70,12 @@ Unexpected browser/API errors retain only a closed `task_search__failed` code. Stage names come from the journey source; raw exception text, URLs, form values, and response bodies are not added to reporting. Existing owned evidence failures keep their specific codes and the screenshot privacy guard stays in place. -It does not depend on row order or an otherwise empty company. This selection is -manual until a separate target owner records a fresh pass and admits it to a daily +It does not depend on row order or an otherwise empty company. The browser reads +the visible links together. Per-row indexed locators can disappear +while a search replaces the old collection; a detached snapshot is retried within +the existing poll deadline. Duplicates, unknown destinations, and extra controls +remain evidence failures. This does not add a journey retry or extend its timeout. +The selection stays manual until a separate target owner records a fresh pass and admits it to a daily plan; adding the shared journey does not qualify any deployed target. Supported acquisitions are managed self-hosted (`local`), registered existing diff --git a/tests/runner-e2e/product-journeys/task-search-filter.test.ts b/tests/runner-e2e/product-journeys/task-search-filter.test.ts index d4646725f38..5753f30d727 100644 --- a/tests/runner-e2e/product-journeys/task-search-filter.test.ts +++ b/tests/runner-e2e/product-journeys/task-search-filter.test.ts @@ -16,9 +16,9 @@ vi.mock("@playwright/test", () => ({ expect: Object.assign( type Fault = "ignore-search" | "ignore-status" | "duplicate-result" | "unknown-result" | "reload-query" | "reload-status" | "back-query" | "back-status" | "wrong-route" | "reload-content" | "saved-company" | "saved-description" | "target-run" | "control-run" | "missing-run-evidence" - | "intent" | "receipt" | "missing-identifier" | "ignore-named-view"; + | "intent" | "receipt" | "missing-identifier" | "ignore-named-view" | "detached-snapshot"; type UiNode = { key: string; role?: string; name?: string; title?: string; text?: string; href?: string; - visible?: boolean; children?: UiNode[]; click?(): void; fill?(value: string): void; check?(): void; + visible?: boolean; connected?: boolean; children?: UiNode[]; click?(): void; fill?(value: string): void; check?(): void; value?(): string; checked?(): boolean }; type CollectionState = { query: string; backlog: boolean }; @@ -59,6 +59,7 @@ function fixture(fault?: Fault, shell: "coordinator" | "streamlined" | "legacy" }; const links = (): UiNode[] => displayedId || !allTasks ? [] : visibleTasks().map((task, index) => ({ key: `task-${task.id}-${index}`, role: "link", name: task.title, text: task.title, + connected: fault !== "detached-snapshot" || task.id !== "task-2", href: `/QA/issues/${task.identifier}`, click: () => openTask(task), })); const nodes = (): UiNode[] => { @@ -107,7 +108,10 @@ function fixture(fault?: Fault, shell: "coordinator" | "streamlined" | "legacy" resolve, or(other: { resolve(): UiNode[] }) { return locator(() => [...new Map([...resolve(), ...other.resolve()].map(node => [node.key, node])).values()]); }, first() { return locator(() => resolve().slice(0, 1)); }, - filter({ hasText }: { hasText: string }) { return locator(() => resolve().filter(node => node.text?.includes(hasText))); }, + filter({ hasText, visible }: { hasText?: string; visible?: boolean }) { + return locator(() => resolve().filter(node => (hasText === undefined || node.text?.includes(hasText)) + && (visible !== true || node.visible !== false))); + }, locator(selector: string) { if (selector !== "..") throw Error(`Unsupported control-relative selector: ${selector}`); return locator(() => { @@ -122,6 +126,10 @@ function fixture(fault?: Fault, shell: "coordinator" | "streamlined" | "legacy" return locator(() => flatten(resolve().flatMap(node => node.children ?? [])).filter(node => node.role === role && matches(node.name, name))); }, async all() { return resolve().map(node => locator(() => [node])); }, + async evaluateAll(read: (nodes: Array<{ isConnected: boolean; getAttribute(name: string): string | null }>) => R) { + return read(resolve().map(node => ({ isConnected: node.connected !== false, + getAttribute(name: string) { expect(name).toBe("href"); return node.href ?? null; } }))); + }, async isVisible() { return resolve().length > 0; }, async getAttribute(name: string) { expect(name).toBe("href"); return unique().href ?? null; }, async inputValue() { const read = unique().value; if (!read) throw Error("Not an input"); return read(); }, @@ -229,7 +237,7 @@ describe("taskSearchFilter browser journey", () => { expect(test.evidence.checks).toEqual([]); }); it.each(["ignore-search", "ignore-status", "duplicate-result", "unknown-result", "reload-query", "reload-status", - "back-query", "back-status", "reload-content"])("does not report success when the UI has %s", async fault => { + "back-query", "back-status", "reload-content", "detached-snapshot"])("does not report success when the UI has %s", async fault => { const test = fixture(fault); await expect(test.run()).rejects.toThrow(); expect(test.evidence.checks.some(check => check.passed)).toBe(false); diff --git a/tests/runner-e2e/product-journeys/task-search-filter.ts b/tests/runner-e2e/product-journeys/task-search-filter.ts index 2bdabcb0061..34d58499940 100644 --- a/tests/runner-e2e/product-journeys/task-search-filter.ts +++ b/tests/runner-e2e/product-journeys/task-search-filter.ts @@ -12,20 +12,21 @@ export function taskSearchBacklogCheckbox(filters: Locator) { .getByRole("checkbox", { name: /^Backlog(?:\s*Backlog)?$/ }); } -async function resultIds(page: Page, tasks: readonly SearchTask[]) { - const ids: string[] = []; - for (const link of await page.locator("main#main-content a[data-inbox-issue-link]").all()) { - if (!await link.isVisible()) continue; - const href = await link.getAttribute("href"); +export async function taskSearchResultIds(page: Page, tasks: readonly SearchTask[]) { + // Read the matched nodes together. Indexed locators from all() can disappear + // between isVisible() and getAttribute() while the search replaces its rows. + const hrefs = await page.locator("main#main-content a[data-inbox-issue-link]").filter({ visible: true }) + .evaluateAll(links => links.some(link => !link.isConnected) ? null : links.map(link => link.getAttribute("href"))); + if (hrefs === null) return []; + return hrefs.map(href => { const identifier = href ? decodeURIComponent(new URL(href, page.url()).pathname.split("/").at(-1)!) : ""; - ids.push(tasks.find(task => task.identifier === identifier || task.id === identifier)?.id ?? `unknown:${identifier}`); - } - return ids.sort(); + return tasks.find(task => task.identifier === identifier || task.id === identifier)?.id ?? `unknown:${identifier}`; + }).sort(); } async function expectResults(page: Page, tasks: readonly SearchTask[], expectedIds: readonly string[]) { let observed: string[] = []; - await expect.poll(async () => { observed = await resultIds(page, tasks); return observed; }, { timeout: 30_000 }) + await expect.poll(async () => { observed = await taskSearchResultIds(page, tasks); return observed; }, { timeout: 30_000 }) .toEqual([...expectedIds].sort()); return observed; } diff --git a/tests/runner-e2e/task-search-filter.spec.ts b/tests/runner-e2e/task-search-filter.spec.ts index 3dc8a2172f9..fbe98888693 100644 --- a/tests/runner-e2e/task-search-filter.spec.ts +++ b/tests/runner-e2e/task-search-filter.spec.ts @@ -1,5 +1,6 @@ -import { expect, test } from "@playwright/test"; -import { taskSearchBacklogCheckbox } from "./product-journeys/task-search-filter.js"; +import { expect, test, type Locator, type Page } from "@playwright/test"; +import { taskSearchBacklogCheckbox, taskSearchResultIds } from "./product-journeys/task-search-filter.js"; +import type { SearchTask } from "./product-journeys/task-search-evidence.js"; // Reproduce the observed Radix checkbox + StatusIcon + visible text label. // No app server, provider, browser profile, or target credentials are involved. @@ -31,3 +32,74 @@ for (const presentation of ["legacy", "spaced-legacy", "refreshed"] as const) { await expect(page.locator("#assignee-backlog")).not.toBeChecked(); }); } + +const searchTasks: SearchTask[] = ["target", "status-control", "search-control"].map((name, index) => ({ + id: name, identifier: `QA-${index + 1}`, companyId: "company", title: name, description: `${name} context`, + status: index === 1 ? "done" : "backlog", assigneeUserId: "board", assigneeAgentId: null, +})); +const taskLink = (task: SearchTask) => `${task.title}`; + +test("task search readback survives a search rerender that removes an observed row", async ({ page }) => { + page.setDefaultTimeout(150); // Bound only the reproduced stale-index wait. + await page.setContent(`
${searchTasks.map(taskLink).join("")}
`); + let transitions = 0; + const transition = async () => { + if (transitions++) return; + await page.locator("main#main-content").evaluate((main, html) => { main.innerHTML = html; }, searchTasks.slice(0, 2).map(taskLink).join("")); + }; + // Keep all browser operations real. Place the render between the old + // visibility/attribute reads, or immediately before the batch read. + const withTransition = (source: Locator): Locator => new Proxy(source, { + get(object, property, receiver) { + if (property === "all") return async () => (await object.all()).map((row, index) => index !== 2 ? row : new Proxy(row, { + get(element, operation, elementReceiver) { + if (operation === "isVisible") return async (...args: Parameters) => { + const visible = await element.isVisible(...args); + if (visible) await transition(); + return visible; + }; + const value = Reflect.get(element, operation, elementReceiver); + return typeof value === "function" ? value.bind(element) : value; + }, + })); + if (property === "filter") return (...args: Parameters) => withTransition(object.filter(...args)); + if (property === "evaluateAll") return async (...args: Parameters) => { + await transition(); + return object.evaluateAll(...args); + }; + const value = Reflect.get(object, property, receiver); + return typeof value === "function" ? value.bind(object) : value; + }, + }); + const observedPage = new Proxy(page, { + get(object, property, receiver) { + if (property === "locator") return (...args: Parameters) => withTransition(object.locator(...args)); + const value = Reflect.get(object, property, receiver); + return typeof value === "function" ? value.bind(object) : value; + }, + }); + await expect(taskSearchResultIds(observedPage, searchTasks)).resolves.toEqual(["status-control", "target"]); + expect(transitions).toBe(1); + await expect(page.locator("a[data-inbox-issue-link]")).toHaveCount(2); +}); + +for (const fixture of [ + { name: "extra owned control", html: searchTasks.map(taskLink).join(""), ids: ["search-control", "status-control", "target"] }, + { name: "duplicate target", html: [...searchTasks.slice(0, 2), searchTasks[0]!].map(taskLink).join(""), ids: ["status-control", "target", "target"] }, + { name: "unknown destination", html: searchTasks.slice(0, 2).map(taskLink).join("") + taskLink({ ...searchTasks[2]!, identifier: "QA-99" }), ids: ["status-control", "target", "unknown:QA-99"] }, + { name: "wrong target destination", html: taskLink(searchTasks[1]!) + taskLink({ ...searchTasks[0]!, identifier: "QA-2" }), ids: ["status-control", "status-control"] }, +]) { + test(`task search snapshot preserves invalid evidence: ${fixture.name}`, async ({ page }) => { + await page.setContent(`
${fixture.html}
`); + const ids = await taskSearchResultIds(page, searchTasks); + expect(ids).toEqual(fixture.ids); + expect(ids).not.toEqual(["status-control", "target"]); + }); +} + +test("task search snapshot retains the existing visible-link scope", async ({ page }) => { + await page.setContent(`
${searchTasks.slice(0, 2).map(taskLink).join("")} +
+ ${taskLink({ ...searchTasks[2]!, identifier: "QA-99" })}`); + expect(await taskSearchResultIds(page, searchTasks)).toEqual(["status-control", "target"]); +});