diff options
| author | amankumarsingh77 <amankumarsingh7702@gmail.com> | 2026-07-28 19:50:02 +0530 |
|---|---|---|
| committer | Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> | 2026-08-02 19:12:46 +0530 |
| commit | 254de089ee468795bac175d99e09ddaec4ac1469 (patch) | |
| tree | 10ef23546ec6c5b20f6339b6d1b88769f323447e | |
| parent | ce1240fa9b3b74983cc22171469c24d0708b10ea (diff) | |
fix(domains): keep element refs stable across a mutation
interactiveDiff re-assigned refs by tree position after every fill, click and
select, so any element appearing or disappearing shifted the numbering and a ref
the agent was still holding silently resolved to a different element — while the
tools kept promising that refs survive re-renders. Refs are now carried over by
backendNodeId and only genuinely new elements draw a new number.
Report: docs/superpowers/reports/2026-07-26-bug-audit.md#f6
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| -rw-r--r-- | src/cdp/session.ts | 5 | ||||
| -rw-r--r-- | src/domains/ref-resolve.ts | 17 | ||||
| -rw-r--r-- | test/domains/ref-stability-test.ts | 80 |
3 files changed, 99 insertions, 3 deletions
diff --git a/src/cdp/session.ts b/src/cdp/session.ts index 2e0e71f..80fec16 100644 --- a/src/cdp/session.ts +++ b/src/cdp/session.ts @@ -43,6 +43,7 @@ export type CdpSession = { resolveRef(ref: string): number | undefined; /** The active tab's prior ref signatures — baseline for the post-mutation diff. */ refSignatures(): ReadonlyMap<string, string>; + refMappings(): ReadonlyMap<string, number>; call<M extends CdpMethod>(method: M, params?: ParamsOf<M>, opts?: { timeoutMs?: number }): Promise<Result<ResultOf<M>, CdpError>>; callOnTarget<M extends CdpMethod>(method: M, params: ParamsOf<M>, sessionId: string, opts?: { timeoutMs?: number }): Promise<Result<ResultOf<M>, CdpError>>; callBrowser<M extends CdpMethod>(method: M, params?: ParamsOf<M>, opts?: { timeoutMs?: number }): Promise<Result<ResultOf<M>, CdpError>>; @@ -396,6 +397,10 @@ export const createCdpSession = ( const tab = targetId ? tabs.get(targetId) : undefined; return tab?.refSig ?? new Map(); }, + refMappings() { + const tab = targetId ? tabs.get(targetId) : undefined; + return tab?.refMap ?? new Map(); + }, call(method, params, opts) { return req(method, params, sessionId, opts); }, diff --git a/src/domains/ref-resolve.ts b/src/domains/ref-resolve.ts index a80c620..822f8e8 100644 --- a/src/domains/ref-resolve.ts +++ b/src/domains/ref-resolve.ts @@ -83,14 +83,25 @@ export const interactiveDiff = async (client: BrowserClient): Promise<string> => const targets = collectInteractiveTargets(slim); // Re-publish fresh refs so later ref calls resolve against the new DOM. + const priorRefs = session.refMappings(); + const refByBackendId = new Map<number, string>(); + for (const [ref, backendId] of priorRefs) refByBackendId.set(backendId, ref); + let nextIndex = 1; + for (const ref of priorRefs.keys()) { + const digits = /^e(\d+)$/.exec(ref)?.[1]; + if (digits !== undefined) nextIndex = Math.max(nextIndex, Number(digits) + 1); + } const refMap = new Map<string, number>(); const refSig = new Map<string, string>(); - targets.forEach(({ node, backendId }, i) => { - const ref = `e${i + 1}`; + const taken = new Set<string>(); + for (const { node, backendId } of targets) { + const prior = refByBackendId.get(backendId); + const ref = prior !== undefined && !taken.has(prior) ? prior : `e${nextIndex++}`; + taken.add(ref); node.ref = ref; refMap.set(ref, backendId); refSig.set(ref, sigOf(node)); - }); + } session.setRefMap(refMap, refSig); // Compare by stable identity (role|name) rather than ref position, since refs diff --git a/test/domains/ref-stability-test.ts b/test/domains/ref-stability-test.ts new file mode 100644 index 0000000..0c5ddd4 --- /dev/null +++ b/test/domains/ref-stability-test.ts @@ -0,0 +1,80 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { createFakeClient, axNode } from "./fake-client"; +import { interactiveDiff } from "../../src/domains/ref-resolve"; +import { ok } from "../../src/util/result"; + +const SEEDED = [ + { ref: "e1", backendId: 101, sig: "textbox|Email||" }, + { ref: "e2", backendId: 102, sig: "textbox|Name||" }, + { ref: "e3", backendId: 103, sig: "button|Go||" }, +]; + +const treeWithNewButtonFirst = ok({ + nodes: [ + axNode("1", "RootWebArea", "", { childIds: ["9", "2", "3", "4"] }), + axNode("9", "button", "Dismiss", { parentId: "1", backendDOMNodeId: 999 }), + axNode("2", "textbox", "Email", { parentId: "1", backendDOMNodeId: 101 }), + axNode("3", "textbox", "Name", { parentId: "1", backendDOMNodeId: 102 }), + axNode("4", "button", "Go", { parentId: "1", backendDOMNodeId: 103 }), + ], +}); + +const treeUnchanged = ok({ + nodes: [ + axNode("1", "RootWebArea", "", { childIds: ["2", "3", "4"] }), + axNode("2", "textbox", "Email", { parentId: "1", backendDOMNodeId: 101, value: { value: "a@b.com" } }), + axNode("3", "textbox", "Name", { parentId: "1", backendDOMNodeId: 102 }), + axNode("4", "button", "Go", { parentId: "1", backendDOMNodeId: 103 }), + ], +}); + +describe("ref stability across a mutation", () => { + test("an element appearing above the others does not steal their refs", async () => { + const fake = await createFakeClient({ + refs: SEEDED, + canned: { "Accessibility.getFullAXTree": treeWithNewButtonFirst }, + }); + await interactiveDiff(fake.client); + assert.equal(fake.session.resolveRef("e1"), 101); + assert.equal(fake.session.resolveRef("e2"), 102); + assert.equal(fake.session.resolveRef("e3"), 103); + }); + + test("a newly appeared element gets a fresh ref rather than an existing one", async () => { + const fake = await createFakeClient({ + refs: SEEDED, + canned: { "Accessibility.getFullAXTree": treeWithNewButtonFirst }, + }); + const diff = await interactiveDiff(fake.client); + assert.equal(fake.session.resolveRef("e4"), 999); + assert.match(diff, /\*\[e4\] button "Dismiss"/); + }); + + test("refs are unchanged when the interactive set is unchanged", async () => { + const fake = await createFakeClient({ + refs: SEEDED, + canned: { "Accessibility.getFullAXTree": treeUnchanged }, + }); + const diff = await interactiveDiff(fake.client); + assert.equal(fake.session.resolveRef("e1"), 101); + assert.equal(fake.session.resolveRef("e2"), 102); + assert.equal(fake.session.resolveRef("e3"), 103); + assert.match(diff, /\[e1\] textbox "Email": value "" → "a@b.com"/); + }); + + test("every published ref maps to a distinct element", async () => { + const fake = await createFakeClient({ + refs: SEEDED, + canned: { "Accessibility.getFullAXTree": treeWithNewButtonFirst }, + }); + await interactiveDiff(fake.client); + const seen = new Set<number>(); + for (const ref of ["e1", "e2", "e3", "e4"]) { + const backendId = fake.session.resolveRef(ref); + assert.ok(backendId !== undefined, `${ref} resolved to nothing`); + assert.equal(seen.has(backendId), false, `${ref} duplicates another ref's element`); + seen.add(backendId); + } + }); +}); |
