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