From 0616f20aba25e543f73f95329f2894272a1b6989 Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:06:03 +0000 Subject: [PATCH] fix: explain why an element reference stopped resolving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #16. Element references are reissued by every inspection, so a reference from an earlier call silently stops working — including the common case where a narrow --context summary returns eight controls after a --context full returned two hundred. The agent got a bare ELEMENT_NOT_FOUND with nothing to distinguish 'this expired, re-inspect' from 'this never existed', which is the difference between recovering and retrying a dead reference. Failures now name the cause: expired, unknown, or detached. Region lookups do the same. Region references keep their existing lifetime on purpose. They stay resolvable across inspections because the documented outline-then-scope workflow depends on it, but the map holding them was never cleared and kept strong references to detached nodes for the lifetime of the page. It is now bounded at 256, oldest first. P1 documents the asymmetry rather than leaving it as folklore, and the runtime suite asserts each failure mode plus the cross-inspection region workflow. --- .../HeadlessProtocol/AgentRuntime.swift | 38 +++++++++++++++++-- apps/headless/Tests/agent-runtime.test.mjs | 25 ++++++++++++ apps/headless/docs/P1.md | 20 ++++++++++ docs/roadmap/improvements-backlog.md | 12 +++++- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/apps/headless/Sources/HeadlessProtocol/AgentRuntime.swift b/apps/headless/Sources/HeadlessProtocol/AgentRuntime.swift index da42728..6cdb9dd 100644 --- a/apps/headless/Sources/HeadlessProtocol/AgentRuntime.swift +++ b/apps/headless/Sources/HeadlessProtocol/AgentRuntime.swift @@ -7,6 +7,11 @@ if (!globalThis.__headlessAgent) { const regionRefs = new WeakMap(); let current = new Map(); let currentRegions = new Map(); + // Every reference ever handed out, so a failed lookup can say whether the + // reference expired or was never issued at all. + const issuedRefs = new Set(); + const issuedRegionRefs = new Set(); + const maximumTrackedRegions = 256; let lastMutation = performance.now(); new MutationObserver(() => { lastMutation = performance.now(); }) .observe(document, {subtree: true, childList: true, attributes: true, characterData: true}); @@ -107,6 +112,7 @@ if (!globalThis.__headlessAgent) { const refFor = element => { let ref = refs.get(element); if (!ref) { ref = `@e${nextRef++}`; refs.set(element, ref); } + issuedRefs.add(ref); current.set(ref, element); return ref; }; @@ -211,15 +217,30 @@ if (!globalThis.__headlessAgent) { const regionRefFor = element => { let ref = regionRefs.get(element); if (!ref) { ref = `@r${nextRegionRef++}`; regionRefs.set(element, ref); } + issuedRegionRefs.add(ref); currentRegions.set(ref, element); + // Region references stay resolvable across inspections so a scoped + // `--within @rN` workflow keeps working, but this map holds elements + // strongly. Drop the oldest so a long-lived page cannot grow it without + // bound, and never drop the reference just issued. + while (currentRegions.size > maximumTrackedRegions) { + const oldest = currentRegions.keys().next().value; + if (oldest === undefined || oldest === ref) break; + currentRegions.delete(oldest); + } return ref; }; const resolveRegion = reference => { if (!reference) return document; const element = currentRegions.get(reference); - if (!element || !element.isConnected || !visible(element)) { + if (!element) { + throw new Error(issuedRegionRefs.has(reference) + ? `REGION_NOT_FOUND:${reference} (expired: inspect again to refresh region references)` + : `REGION_NOT_FOUND:${reference} (unknown: no inspection has issued this reference)`); + } + if (!element.isConnected || !visible(element)) { currentRegions.delete(reference); - throw new Error(`REGION_NOT_FOUND:${reference}`); + throw new Error(`REGION_NOT_FOUND:${reference} (detached: the region is no longer visible on this page)`); } return element; }; @@ -423,7 +444,18 @@ if (!globalThis.__headlessAgent) { }; const resolve = target => { const element = current.get(target); - if (!element || !element.isConnected) throw new Error(`ELEMENT_NOT_FOUND:${target}`); + // Element references describe the most recent inspection only. Saying so + // is the difference between an agent re-inspecting and an agent retrying + // the same dead reference. + if (!element) { + throw new Error(issuedRefs.has(target) + ? `ELEMENT_NOT_FOUND:${target} (expired: element references come from the most recent inspection — inspect again to refresh)` + : `ELEMENT_NOT_FOUND:${target} (unknown: no inspection has issued this reference)`); + } + if (!element.isConnected) { + current.delete(target); + throw new Error(`ELEMENT_NOT_FOUND:${target} (detached: the element is no longer in the page)`); + } return element; }; const find = (wantedRole, wantedName) => { diff --git a/apps/headless/Tests/agent-runtime.test.mjs b/apps/headless/Tests/agent-runtime.test.mjs index d6dc8ae..efc67e1 100644 --- a/apps/headless/Tests/agent-runtime.test.mjs +++ b/apps/headless/Tests/agent-runtime.test.mjs @@ -104,6 +104,31 @@ assert.throws( /REGION_NOT_FOUND/, ); +// A region reference issued by an earlier inspection stays usable, which is +// what makes the outline-then-scope workflow possible across calls. +assert.equal( + agent.snapshot(false, false, {context: 'text', within: targetRegion.ref, limit: 2}).within, + targetRegion.ref, +); +assert.throws( + () => agent.snapshot(false, false, {context: 'text', within: '@r999999'}), + /REGION_NOT_FOUND.*unknown/, +); + +// Element references describe the most recent inspection only. A stale one has +// to say it expired, otherwise an agent cannot tell "re-inspect" from +// "this element never existed" and retries the dead reference. +const staleRef = full.elements[full.elements.length - 1].ref; +assert.match(staleRef, /^@e\d+$/); +agent.snapshot(false, false, {context: 'summary', limit: 8, budget: 700}); +assert.throws(() => agent.click({target: staleRef}), /ELEMENT_NOT_FOUND.*expired/); +assert.throws(() => agent.click({target: '@e999999'}), /ELEMENT_NOT_FOUND.*unknown/); + +// A reference from the latest inspection still resolves. +const fresh = agent.snapshot(false, false, {context: 'actions', limit: 5}); +assert(fresh.elements.length > 0, 'actions context should return executable controls'); +assert.equal(agent.click({target: fresh.elements[0].ref}).clicked, fresh.elements[0].ref); + console.log(JSON.stringify({ selectedRegion: targetRegion.ref, full: full.contextStats, diff --git a/apps/headless/docs/P1.md b/apps/headless/docs/P1.md index dac6a87..8a91662 100644 --- a/apps/headless/docs/P1.md +++ b/apps/headless/docs/P1.md @@ -69,6 +69,26 @@ focused response reports encoded bytes, estimated tokens, the applied budget, and omitted counts. Page-derived strings are marked as untrusted content. `--interactive` remains a legacy alias for the action-focused view. +### Reference lifetime + +The two reference kinds deliberately differ, because they are used differently. + +- **Element references (`@e1`)** describe the **most recent inspection only**. + Every inspection reissues them, so a reference from an earlier call stops + resolving — including when a narrow context such as `summary` returns fewer + controls than the `full` call before it. Act on a reference from the + inspection you just ran, or target by role and name instead. +- **Region references (`@r4`)** stay resolvable across inspections, so the + outline-then-scope workflow works: take `@r4` from an `outline`, then scope + repeated `text` and `actions` calls to it. They are bounded per page and drop + oldest-first. + +A reference that does not resolve says why, rather than failing anonymously: +`expired` (issued earlier, superseded by a later inspection — inspect again), +`unknown` (never issued — do not guess or invent references), or `detached` +(the element or region has left the page). All three surface as +`ELEMENT_NOT_FOUND` or `REGION_NOT_FOUND`. + ## Recording The built-in recorder captures browser pixels and writes MP4, MOV, WebM, or GIF diff --git a/docs/roadmap/improvements-backlog.md b/docs/roadmap/improvements-backlog.md index 4850d3f..a4674e6 100644 --- a/docs/roadmap/improvements-backlog.md +++ b/docs/roadmap/improvements-backlog.md @@ -72,11 +72,19 @@ loop. Add backoff + a fatal threshold. map is reset on each `snapshot()` (`HP/AgentRuntime.swift:376`), so a `--context summary` (max 8 elements) invalidates all refs from a prior `full`; the agent later gets a bare `ELEMENT_NOT_FOUND`. Meanwhile -`currentRegions` is *never* reset and grows for the page lifetime. Decide the +`currentRegions` is *never* reset and grows for the page lifetime. ~~Decide the contract (likely: refs from the latest inspection only — already the skill's teaching), then (a) make the error say *why* ("ref expired; re-inspect"), (b) reset regions consistently on navigation, (c) document in P1.md. Test: -inspect-full → inspect-summary → click stale `@eN` asserts the new error. +inspect-full → inspect-summary → click stale `@eN` asserts the new error.~~ +**Done.** The contract is now explicit and asymmetric on purpose: `@eN` is +latest-inspection-only, `@rN` stays resolvable so the outline-then-scope +workflow survives across calls. Failures name the cause — `expired`, `unknown`, +or `detached` — instead of an anonymous `ELEMENT_NOT_FOUND`. Region tracking is +bounded at 256 oldest-first rather than growing for the page lifetime, which +also releases the strong references it was holding to detached nodes. +Documented in P1 under "Reference lifetime"; covered in +`Tests/agent-runtime.test.mjs`. **A6. `--json`/`--session` stripped from anywhere in argv.** ([#17](https://github.com/LockInTime/headless/issues/17)) Global-option stripping (`HP/CLI.swift:46-49`) happens before subcommand parsing, so