diff --git a/docs/adr/0014-session-ref-frame-lifetime.md b/docs/adr/0014-session-ref-frame-lifetime.md index 15254111c..799a052b2 100644 --- a/docs/adr/0014-session-ref-frame-lifetime.md +++ b/docs/adr/0014-session-ref-frame-lifetime.md @@ -36,8 +36,9 @@ remain compatible with selector-based replay, and add no automatic capture or pe ## Decision The terms introduced below are now the implemented model; the ref-frame vocabulary is promoted into -`CONTEXT.md`. The coarse `snapshotRefsStale` marker still backs read-only staleness warnings and is -removed once per-platform enforcement is confirmed by live evidence (migration step 8). +`CONTEXT.md`. The superseded coarse `snapshotRefsStale` marker has been removed (migration step 8): +read-only ref staleness is now derived from frame state — a plain ref warns once the frame has expired, +and a read-only capture no longer marks refs stale because it does not expire the frame. ### Ref frames are separate from operational observations @@ -62,9 +63,9 @@ the emitted subset, including ancestors, siblings, overlays, and the viewport. T the evidence tree, bounds partial authority. The frame and latest observation share immutable capture data when they originate from the same capture; neither transition deep-copies the tree. -The existing `snapshotGeneration`/`snapshotRefsStale` implementation evolves behind one ref-frame -module. The public name `refsGeneration` and the `@e12~s42` grammar remain unchanged for wire -compatibility. +The existing `snapshotGeneration` implementation evolves behind one ref-frame module; the +`snapshotRefsStale` marker it originally paired with has since been removed (migration step 8). The +public name `refsGeneration` and the `@e12~s42` grammar remain unchanged for wire compatibility. ### Frame transitions @@ -221,12 +222,12 @@ resolver rather than pretending all subcommands behave alike. The registry is the exhaustive source of truth. The following table is non-exhaustive guidance for classifying representative actions; it must not become a second prose registry: -| Effect | Commands/actions | -| --- | --- | -| `may-invalidate` | press, click, fill, longpress, type, focus, scroll, swipe, gesture, back, home, `tv-remote`, orientation, open/relaunch, trigger/push delivery, settings changes, install/reinstall, React Native overlay dismissal, and lifecycle operations that can replace the visible surface | -| Conditional resolver | keyboard status preserves while dismiss/return/input invalidate; alert get/wait preserve while accept/dismiss invalidate; find reads preserve while click/fill/focus/type delegate to their leaf mutation | -| `delegated` | batch, replay, and test/suite orchestrators; each nested leaf owns its transition | -| `preserve` | snapshots and other observation, assertion, screenshot, recording, trace, logs, events, network inspection, performance, inventory, capability, lease, and transport-management operations unless a selected subaction directly manipulates the visible surface | +| Effect | Commands/actions | +| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `may-invalidate` | press, click, fill, longpress, type, focus, scroll, swipe, gesture, back, home, `tv-remote`, orientation, open/relaunch, trigger/push delivery, settings changes, install/reinstall, React Native overlay dismissal, and lifecycle operations that can replace the visible surface | +| Conditional resolver | keyboard status preserves while dismiss/return/input invalidate; alert get/wait preserve while accept/dismiss invalidate; find reads preserve while click/fill/focus/type delegate to their leaf mutation | +| `delegated` | batch, replay, and test/suite orchestrators; each nested leaf owns its transition | +| `preserve` | snapshots and other observation, assertion, screenshot, recording, trace, logs, events, network inspection, performance, inventory, capability, lease, and transport-management operations unless a selected subaction directly manipulates the visible surface | Clipboard reads and writes preserve the frame because pasteboard state alone does not change element identity. A later paste/type action is independently invalidating. @@ -437,11 +438,12 @@ Each step lands green and independently useful: evidence; promote the implemented vocabulary into `CONTEXT.md`; then remove the superseded coarse stale marker. -Implementation status: steps 1–7 have landed — the ref-frame module and observation split (1), the -complete daemon classification and gate (2), the pre-side-effect seam at every leaf (3), correct -complete/partial publication with bounded scope, MCP pin retention, and pinned partial CLI text (4), -Android freshness decoupled from positional ref authorization (5), the cross-platform contract and -provider evidence (6), and fail-closed admission enforcement across platforms with typed reasons (7). +Implementation status: all migration steps have landed — the ref-frame module and observation split +(1), the complete daemon classification and gate (2), the pre-side-effect seam at every leaf (3), +correct complete/partial publication with bounded scope, MCP pin retention, and pinned partial CLI text +(4), Android freshness decoupled from positional ref authorization (5), the cross-platform contract and +provider evidence (6), fail-closed admission enforcement across platforms with typed reasons (7), and +the docs/vocabulary promotion plus removal of the superseded coarse `snapshotRefsStale` marker (8). Fresh live evidence has exercised nearly every production seam — Apple runtime-ref, direct/native selector, generation-pin, generic, and lifecycle paths; Android helper freshness (including proven non-retarget) and Android existing-session relaunch; and a real provider-backed interaction plus @@ -455,8 +457,7 @@ regressions (`android-system-dialog-ref-frame.test.ts` proves recovery expires t tap; `interaction-android-recovery-abort.test.ts` proves an outstanding ref action then aborts with the shared `ref_frame_expired` rejection and no dispatch), the seam is enforced in code identically to the verified paths, and it is recorded here as a documented, accepted evidence gap rather than an open -release blocker. Every other enabled seam is proven on hardware, so step 8's removal of the superseded -coarse `snapshotRefsStale` marker is unblocked. +release blocker. Every other enabled seam is proven on hardware. PR #1241 landed independently as a compatible transitional fix. It rejects a known iOS stale-marker case before this full lifecycle is implemented; it does not own the architecture migration. diff --git a/src/commands/interaction/runtime/selector-read-shared.ts b/src/commands/interaction/runtime/selector-read-shared.ts index ea0664d9d..89edbd6ef 100644 --- a/src/commands/interaction/runtime/selector-read-shared.ts +++ b/src/commands/interaction/runtime/selector-read-shared.ts @@ -20,6 +20,15 @@ export type CapturedSnapshot = { export type SelectorSnapshotOptions = SelectorSnapshotInput; +/** + * Resolve the snapshot a `@ref` READ binds against. ADR 0014: a ref resolves + * against the AUTHORIZED frame tree (`refFrameSnapshot`), never the latest + * operational observation — so an internal read-only capture that replaced the + * observation cannot let a plain `@eN` resolve a different element by positional + * coincidence. Missing frame evidence fails (the ref is simply not found in the + * retained tree) rather than falling through to a newer observation. Only used + * by ref reads; selector reads capture fresh through `captureSelectorSnapshot`. + */ export async function requireSnapshotSession( runtime: AgentDeviceRuntime, requestedName: string | undefined, @@ -27,10 +36,11 @@ export async function requireSnapshotSession( const sessionName = requestedName ?? 'default'; const session = await runtime.sessions.get(sessionName); if (!session) throw new AppError('SESSION_NOT_FOUND', 'No active session. Run open first.'); - if (!session.snapshot) { + const frameTree = session.refFrameSnapshot ?? session.snapshot; + if (!frameTree) { throw new AppError('INVALID_ARGS', 'No snapshot in session. Run snapshot first.'); } - return { sessionName, session, snapshot: session.snapshot }; + return { sessionName, session, snapshot: frameTree }; } export async function captureSelectorSnapshot( diff --git a/src/commands/interaction/runtime/settle.ts b/src/commands/interaction/runtime/settle.ts index 8c48ef767..28117d5e7 100644 --- a/src/commands/interaction/runtime/settle.ts +++ b/src/commands/interaction/runtime/settle.ts @@ -322,8 +322,10 @@ function resolveSettleHint( // The settle loop itself captures with updateSession: false (a capture that // later stalls must not race a session write past the response). The FINAL // capture is stored so follow-up snapshots/selectors see the latest surface. -// Only settled captures issue a diff/ref payload; unsettled stored captures -// conservatively mark prior refs stale through the runtime session writer. +// Only settled captures issue a diff/ref payload; an unsettled stored capture +// replaces the observation without issuing refs — it does NOT touch the ref +// frame. Read staleness is frame-derived: the frame expires at a side-effect +// seam (ADR 0014), not because a fresh observation was stored here. // Sparse-quality captures are not stored (mirroring captureSelectorSnapshot) // and therefore issue no refs. The fetched session is returned alongside // `stored` so the caller can read `appBundleId` for settle-chrome scoping diff --git a/src/daemon/__tests__/session-snapshot.test.ts b/src/daemon/__tests__/session-snapshot.test.ts index 57e3b670c..52dced0d6 100644 --- a/src/daemon/__tests__/session-snapshot.test.ts +++ b/src/daemon/__tests__/session-snapshot.test.ts @@ -32,7 +32,8 @@ test('setSessionSnapshot advances the generation on every tree replacement (#107 const seeded = session.snapshotGeneration!; expect(seeded).toBeGreaterThanOrEqual(100_000); expect(seeded).toBeLessThan(1_000_000); - expect(session.snapshotRefsStale).toBe(true); + // ADR 0014: replacing the observation does NOT touch the ref frame. + expect(session.refFrameState).toBeUndefined(); // Storing the SAME snapshot object again is not a replacement. setSessionSnapshot(session, first); @@ -51,7 +52,6 @@ test('a reopened session reseeds so pins from a previous lifetime do not silentl // Reopen: a fresh session object restarts the counter with a NEW seed. const secondLifetime = makeSession(); setSessionSnapshot(secondLifetime, makeSnapshot()); - secondLifetime.snapshotRefsStale = false; // Probabilistic, not identity-based: the seeds collide with ~1/900000 // probability (an accepted residual risk, documented on the field). @@ -66,56 +66,85 @@ test('a reopened session reseeds so pins from a previous lifetime do not silentl ).toContain(`minted from snapshot s${oldGeneration}`); }); -test('resolveRefStalenessWarning: pinned-current clean, pinned-stale precise, plain coarse', () => { +test('resolveRefStalenessWarning: frame expiry is checked before the epoch (ADR 0014 evidence #17)', () => { const session = makeSession(); session.snapshotGeneration = 15; - session.snapshotRefsStale = true; + session.refFrameGeneration = 15; - // Pinned to the stored generation: the pin proves the ref matches the tree, - // so the coarse marker is overruled. + // Expired frame: ANY read is stale, even a pin matching the epoch — a matching + // pin proves identity within the retained frame, not that the UI is current. + session.refFrameState = 'expired'; + expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 15 })).toBe( + STALE_SNAPSHOT_REFS_WARNING, + ); + expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined })).toBe( + STALE_SNAPSHOT_REFS_WARNING, + ); + + // Active frame: a pin matching the epoch and a plain ref are both clean; a pin + // from another epoch gets the precise generation warning. + session.refFrameState = 'active'; expect( resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 15 }), ).toBeUndefined(); - + expect( + resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined }), + ).toBeUndefined(); expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 12 })).toBe( - 'Ref @e37 was minted from snapshot s12 but the session tree is now s15 — re-run snapshot -i.', + "Ref @e37 was minted from snapshot s12 but the session's ref frame is now s15 — re-run snapshot -i.", ); +}); - expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined })).toBe( - STALE_SNAPSHOT_REFS_WARNING, - ); +test('resolveRefStalenessWarning names the frozen frame epoch, not the bumped observation generation (ADR 0014)', () => { + const session = makeSession(); + // A frame was issued at generation 15. + session.snapshotGeneration = 15; + session.refFrameGeneration = 15; + session.refFrameState = 'active'; + + // A read-only capture replaces the observation and advances the observation + // counter WITHOUT re-issuing the frame — the frame epoch stays frozen at 15. + setSessionSnapshot(session, makeSnapshot()); + expect(session.snapshotGeneration).toBe(16); + expect(session.refFrameGeneration).toBe(15); - session.snapshotRefsStale = false; + // A pin matching the FROZEN frame epoch is clean, even though the observation + // generation has since advanced past it. expect( - resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined }), + resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 15 }), ).toBeUndefined(); + + // A pin from another epoch names the frame epoch (s15), never the bumped + // observation generation (s16). + expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 12 })).toBe( + "Ref @e37 was minted from snapshot s12 but the session's ref frame is now s15 — re-run snapshot -i.", + ); }); test('resolveRefStalenessWarning treats a missing stored generation as s0', () => { const session = makeSession(); expect(resolveRefStalenessWarning({ session, ref: 'e2', mintedGeneration: 3 })).toBe( - 'Ref @e2 was minted from snapshot s3 but the session tree is now s0 — re-run snapshot -i.', + "Ref @e2 was minted from snapshot s3 but the session's ref frame is now s0 — re-run snapshot -i.", ); expect(resolveRefStalenessWarning({ session, ref: '@e2', mintedGeneration: 0 })).toBeUndefined(); }); -test('markSessionPartialRefsIssued: an empty result leaves all state untouched (ADR 0014)', () => { +test('markSessionPartialRefsIssued: an empty result leaves all frame state untouched (ADR 0014)', () => { const session = makeSession(); // A useful prior frame exists. - session.snapshotRefsStale = true; session.refFrameState = 'active'; session.refFrameScope = new Set(['e1']); session.refFrameGeneration = 7; - // An empty partial publication (no refs) must not supersede that authority or - // even clear the coarse marker. + // An empty partial publication (no refs) must not supersede that authority. markSessionPartialRefsIssued(session, []); - expect(session.snapshotRefsStale).toBe(true); + expect(session.refFrameState).toBe('active'); expect(session.refFrameScope).toEqual(new Set(['e1'])); expect(session.refFrameGeneration).toBe(7); // A non-empty result supersedes with exactly its bodies. + session.snapshotGeneration = 9; markSessionPartialRefsIssued(session, ['@e5~s7', 'e6']); - expect(session.snapshotRefsStale).toBe(false); expect(session.refFrameScope).toEqual(new Set(['e5', 'e6'])); + expect(session.refFrameGeneration).toBe(9); }); diff --git a/src/daemon/handlers/__tests__/find.test.ts b/src/daemon/handlers/__tests__/find.test.ts index d80ebb56e..0251b538b 100644 --- a/src/daemon/handlers/__tests__/find.test.ts +++ b/src/daemon/handlers/__tests__/find.test.ts @@ -751,33 +751,6 @@ test('handleFindCommands wait captures fresh snapshots while polling', async () expect(mockDispatch).toHaveBeenCalledTimes(2); }); -test('handleFindCommands click re-issues a fresh ref and clears the stale-refs marker (#1076)', async () => { - const sessionName = 'default'; - const session = makeSession(sessionName); - // As set by an earlier selector-resolution capture that replaced the tree. - session.snapshotRefsStale = true; - - const { response, session: storedSession } = await runFindClickScenario({ - positionals: ['Increment', 'click'], - nodes: [ - { - index: 0, - type: 'Button', - label: 'Increment', - hittable: true, - rect: { x: 50, y: 0, width: 100, height: 100 }, - depth: 0, - }, - ], - session, - }); - - expect(response.ok).toBe(true); - // The response returns a ref minted from the freshly stored snapshot, so - // the marker clears before the internal click @ref sub-invocation runs. - expect(storedSession.snapshotRefsStale).toBe(false); -}); - test('handleFindCommands click omits refsGeneration — a mutating find never issues a pinnable ref (ADR 0014)', async () => { const sessionName = 'default'; const session = makeSession(sessionName); diff --git a/src/daemon/handlers/__tests__/interaction-settle.test.ts b/src/daemon/handlers/__tests__/interaction-settle.test.ts index d5a54ad2d..a4018d432 100644 --- a/src/daemon/handlers/__tests__/interaction-settle.test.ts +++ b/src/daemon/handlers/__tests__/interaction-settle.test.ts @@ -6,12 +6,13 @@ import type { CommandFlags } from '../../../core/dispatch.ts'; import type { SnapshotBackend } from '../../../kernel/snapshot.ts'; import { buildSnapshotState } from '../snapshot-capture.ts'; import { setSessionSnapshot } from '../../session-snapshot.ts'; +import { activateCompleteRefFrame } from '../../ref-frame.ts'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; // #1101 --settle daemon response shape: the settle payload (diff + settled + // refsGeneration) rides the wire response through the shared builder, and a -// diff-carrying settle response is ref-issuing (clears snapshotRefsStale). +// diff-carrying settle response is ref-issuing (activates a partial frame). // Quiet windows are tuned down (--settle-quiet 25) so no test waits real time // beyond a few poll ticks. @@ -92,8 +93,9 @@ async function emulateCaptureSnapshotForSession( function seedSession(sessionName: string, sessionStore: ReturnType) { const session = makeIosSession(sessionName); setSessionSnapshot(session, buildSnapshotState({ nodes: BEFORE_NODES, backend: 'xctest' }, {})); - // The seed emulates a snapshot response that issued these refs. - session.snapshotRefsStale = false; + // The seed emulates a snapshot response that issued these refs: a complete, + // active ref frame (ADR 0014). + activateCompleteRefFrame(session); sessionStore.set(sessionName, session); return session; } @@ -153,7 +155,7 @@ function expectInvalidArgs( return (response.error ?? {}) as Record; } -test('press --settle responds with the settled diff, refsGeneration, and clears the stale marker', async () => { +test('press --settle responds with the settled diff, refsGeneration, and activates a partial ref frame', async () => { const sessionStore = makeSessionStore(); const sessionName = 'settle-press'; seedSession(sessionName, sessionStore); @@ -186,9 +188,9 @@ test('press --settle responds with the settled diff, refsGeneration, and clears expect(added).toEqual({ kind: 'added', text: expect.stringContaining('Welcome!'), ref: 'e2' }); const session = sessionStore.get(sessionName) as SessionState; - // The settle response handed the settled tree's refs to the client: the - // coarse marker clears and the payload carries the stored generation. - expect(session.snapshotRefsStale).toBe(false); + // The settle response handed the settled tree's refs to the client: it + // activated a partial frame and the payload carries the stored generation. + expect(session.refFrameState).toBe('active'); expect(settle.refsGeneration).toBe(session.snapshotGeneration); // The settled tree became the stored session snapshot. expect(session.snapshot?.nodes.some((node) => node.label === 'Welcome!')).toBe(true); @@ -264,9 +266,7 @@ test('press --settle rejects an expired-frame ref before dispatch or observation const sessionStore = makeSessionStore(); const sessionName = 'settle-stale-ref'; const session = seedSession(sessionName, sessionStore); - // ADR 0014: a device action since the snapshot expired the ref frame; the - // coarse marker rides along but the frame lifecycle is what rejects. - session.snapshotRefsStale = true; + // ADR 0014: a device action since the snapshot expired the ref frame. session.refFrameState = 'expired'; sessionStore.set(sessionName, session); mockDispatch.mockRejectedValue( @@ -294,7 +294,7 @@ test('press --settle rejects an expired-frame ref before dispatch or observation expect(String(response.error.details?.hint)).toMatch(/refs were issued/); } expect(mockCaptureSnapshotForSession).not.toHaveBeenCalled(); - expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true); + expect(sessionStore.get(sessionName)?.refFrameState).toBe('expired'); }); test('a settle observation without a diff leaves ref staleness untouched', async () => { @@ -330,8 +330,9 @@ test('a settle observation without a diff leaves ref staleness untouched', async expect(settle.settled).toBe(false); expect(settle.diff).toBeUndefined(); expect(settle.hint).toMatch(/Settle observation unavailable/); - // No refs were issued: the resolution capture left the marker stale. - expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true); + // The press mutated (expiring the frame) and no partial frame was published, + // so the frame stays expired — refs are stale until a fresh snapshot. + expect(sessionStore.get(sessionName)?.refFrameState).toBe('expired'); }); test('bare timeout without --settle stays compatible', async () => { diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index cd1667943..99dc18ea7 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -6,11 +6,7 @@ import type { CommandFlags } from '../../../core/dispatch.ts'; import { attachRefs, type SnapshotBackend } from '../../../kernel/snapshot.ts'; import { AppError } from '../../../kernel/errors.ts'; import { buildSnapshotState } from '../snapshot-capture.ts'; -import { - markSessionSnapshotRefsIssued, - setSessionSnapshot, - STALE_SNAPSHOT_REFS_WARNING, -} from '../../session-snapshot.ts'; +import { setSessionSnapshot, STALE_SNAPSHOT_REFS_WARNING } from '../../session-snapshot.ts'; import { activateCompleteRefFrame, expireRefFrame } from '../../ref-frame.ts'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; import { @@ -113,7 +109,7 @@ async function emulateCaptureSnapshotForSession( )) as { nodes?: never[]; truncated?: boolean; backend?: SnapshotBackend }; const snapshot = buildSnapshotState(snapshotData ?? {}, effectiveFlags); // Mirror the real captureSnapshotForSession: session snapshot writes go - // through setSessionSnapshot so snapshotRefsStale tracking (#1076) applies. + // through setSessionSnapshot so the generation advances (#1076 versioned refs). setSessionSnapshot(session, snapshot); sessionStore.set(session.name, session); return snapshot; @@ -3248,8 +3244,9 @@ function makeStaleRefSession(sessionName: string): SessionState { createdAt: Date.now(), backend: 'xctest', }; - // As if the snapshot command just returned these refs to the client. - session.snapshotRefsStale = false; + // As if the snapshot command just returned these refs to the client: a + // complete, active ref frame (ADR 0014). + activateCompleteRefFrame(session); return session; } @@ -3286,7 +3283,6 @@ test('press selector then press @ref rejects refs that outlived the stored snaps throw new Error(`selector press failed: ${JSON.stringify(selectorPress)}`); } expect(selectorPress.data?.warning).toBeUndefined(); - expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true); // ADR 0014: the selector press crossed the side-effect seam and expired the // frame, so the ref that outlived it is rejected before dispatch. @@ -3312,19 +3308,13 @@ test('a ref press crosses the ADR 0014 side-effect seam and expires the ref fram command === 'snapshot' ? { nodes: makeTwoButtonNodes(), backend: 'xctest' } : {}, ); - // A freshly issued frame is active (undefined === active). - expect(sessionStore.get(sessionName)?.refFrameState).toBeUndefined(); + // A freshly issued complete frame is active. + expect(sessionStore.get(sessionName)?.refFrameState).toBe('active'); const press = await runInteraction(sessionStore, sessionName, 'press', ['@e1']); expect(press?.ok).toBe(true); // The transition is wired at the leaf seam. expect(sessionStore.get(sessionName)?.refFrameState).toBe('expired'); - - // ADR 0014: a PARTIAL publication (find/settle/divergence, via - // markSessionSnapshotRefsIssued) clears the coarse marker but must NOT - // re-authorize the complete frame — only a complete snapshot does. - markSessionSnapshotRefsIssued(sessionStore.get(sessionName)!); - expect(sessionStore.get(sessionName)?.refFrameState).toBe('expired'); }); test('ADR 0014 evidence #1: a second ref mutation rejects (bare and pinned) until a fresh observation re-authorizes', async () => { @@ -3393,7 +3383,7 @@ test('press @ref directly after refs were issued does not warn', async () => { } }); -test('re-issuing refs clears the stale marker so press @ref does not warn', async () => { +test('re-issuing a complete frame lets press @ref succeed again without warning', async () => { const sessionStore = makeSessionStore(); const sessionName = 'reissued-refs-no-warning'; const session = makeStaleRefSession(sessionName); @@ -3406,16 +3396,13 @@ test('re-issuing refs clears the stale marker so press @ref does not warn', asyn 'label=Continue', ]); expect(selectorPress?.ok).toBe(true); - expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true); // The selector press expired the frame (ADR 0014 seam). expect(sessionStore.get(sessionName)?.refFrameState).toBe('expired'); // Simulate the snapshot command re-issuing the complete ref namespace: it - // clears the marker AND re-activates a complete frame (through - // buildNextSnapshotSession; covered in snapshot-handler tests). Clearing the - // coarse marker alone would leave the frame expired. + // re-activates a complete frame (through buildNextSnapshotSession; covered in + // snapshot-handler tests). Without it the frame would stay expired. const stored = sessionStore.get(sessionName)!; - stored.snapshotRefsStale = false; activateCompleteRefFrame(stored); sessionStore.set(sessionName, stored); @@ -3444,9 +3431,7 @@ test('fill @ref rejects after a device action expired the frame', async () => { createdAt: Date.now(), backend: 'xctest', }; - // ADR 0014: an unobserved device action expired the frame; the coarse marker - // rides along and supplies the hint. - session.snapshotRefsStale = true; + // ADR 0014: an unobserved device action expired the frame. expireRefFrame(session); sessionStore.set(sessionName, session); mockDispatch.mockRejectedValue( @@ -3467,11 +3452,59 @@ test('fill @ref rejects after a device action expired the frame', async () => { expect(mockDispatch).not.toHaveBeenCalled(); }); -test('get text @ref warns while refs are stale', async () => { +test('ADR 0014 evidence #17: get text @ref reads the retained frame tree, not a newer observation', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'read-frame-tree'; + // Frame tree: @e1 = Continue, @e2 = Cancel. + const session = makeStaleRefSession(sessionName); + // A read-only capture replaced the OBSERVATION with a divergent tree where the + // same index means a different element. The frame tree is untouched. + setSessionSnapshot(session, { + nodes: attachRefs([ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'XCUIElementTypeButton', + label: 'Different', + rect: { x: 10, y: 20, width: 100, height: 40 }, + enabled: true, + hittable: true, + }, + ] as never), + createdAt: Date.now(), + backend: 'xctest', + }); + sessionStore.set(sessionName, session); + mockDispatch.mockRejectedValue(new Error('get text @ref must not recapture')); + + // Resolves against the frame tree's @e2 (Continue), never the observation's + // positional @e2 (Different) — no fall-through by positional coincidence. + const response = await runInteraction(sessionStore, sessionName, 'get', ['text', '@e2']); + expect(response?.ok).toBe(true); + if (response?.ok) { + expect(response.data?.ref).toBe('e2'); + expect(String(response.data?.text)).toContain('Continue'); + expect(String(response.data?.text)).not.toContain('Different'); + } + + // Missing frame evidence FAILS rather than resolving a newer observation. + const missing = await runInteraction(sessionStore, sessionName, 'get', ['text', '@e9']); + expect(missing?.ok).toBe(false); + if (missing && !missing.ok) { + expect(missing.error.code).toBe('COMMAND_FAILED'); + expect(missing.error.message).toMatch(/not found/i); + } + expect(mockDispatch).not.toHaveBeenCalled(); +}); + +test('get text @ref warns while the frame is expired (retained evidence still resolves)', async () => { const sessionStore = makeSessionStore(); const sessionName = 'stale-ref-get-text'; const session = makeStaleRefSession(sessionName); - session.snapshotRefsStale = true; + // A device action expired the frame; the read still resolves against the + // retained frame tree and stays fail-open with a warning (ADR 0014). + expireRefFrame(session); sessionStore.set(sessionName, session); mockDispatch.mockRejectedValue( new Error('dispatch should not be called for snapshot-derived get text'), @@ -3487,14 +3520,11 @@ test('get text @ref warns while refs are stale', async () => { // --- Versioned @ref pins (#1076 follow-up) --- -test('press with a pinned ref matching the stored generation is clean even while the coarse marker is set', async () => { +test('press with a pinned ref matching the current frame epoch is clean', async () => { const sessionStore = makeSessionStore(); const sessionName = 'pinned-current-clean'; const session = makeStaleRefSession(sessionName); session.snapshotGeneration = 5; - // Coarse marker set (e.g. a --verify evidence capture stored the SAME tree - // shape again) — the pin proves the client's ref came from this generation. - session.snapshotRefsStale = true; sessionStore.set(sessionName, session); const response = await runInteraction(sessionStore, sessionName, 'press', ['@e1~s5']); @@ -3517,7 +3547,7 @@ test('press with a pinned ref from an older generation rejects with the precise if (response && !response.ok) { expect(response.error.code).toBe('COMMAND_FAILED'); expect(response.error.details?.hint).toBe( - 'Ref @e1 was minted from snapshot s12 but the session tree is now s15 — re-run snapshot -i.', + "Ref @e1 was minted from snapshot s12 but the session's ref frame is now s15 — re-run snapshot -i.", ); } expect(mockDispatch).not.toHaveBeenCalled(); @@ -3542,7 +3572,8 @@ test('fill with a pinned stale ref rejects; pinned current is clean', async () = backend: 'xctest', }; session.snapshotGeneration = 3; - session.snapshotRefsStale = true; + // Re-issue the frame over the overridden snapshot so its source tree matches. + activateCompleteRefFrame(session); sessionStore.set(sessionName, session); const stale = await runInteraction(sessionStore, sessionName, 'fill', ['@e1~s2', 'hello']); @@ -3550,7 +3581,7 @@ test('fill with a pinned stale ref rejects; pinned current is clean', async () = if (stale && !stale.ok) { expect(stale.error.code).toBe('COMMAND_FAILED'); expect(stale.error.details?.hint).toBe( - 'Ref @e1 was minted from snapshot s2 but the session tree is now s3 — re-run snapshot -i.', + "Ref @e1 was minted from snapshot s2 but the session's ref frame is now s3 — re-run snapshot -i.", ); } expect(mockDispatch).not.toHaveBeenCalled(); @@ -3562,6 +3593,65 @@ test('fill with a pinned stale ref rejects; pinned current is clean', async () = } }); +test("ADR 0014 blocker-2: a mutating find's internal fill from an expired frame carries no stale-ref warning", async () => { + // Removing the coarse marker (step 8) means an expired frame now derives read + // staleness. A mutating `find fill` re-resolves the locator itself and re-enters + // the fill leaf with a LOCATOR-minted ref (`internal.findResolvedTarget`) — the + // caller never consumed a `@ref`, so the public find response must not claim + // stale refs even though the frame is expired. + const sessionStore = makeSessionStore(); + const sessionName = 'find-internal-fill-expired'; + const session = makeStaleRefSession(sessionName); + session.snapshot = { + nodes: attachRefs([ + { + index: 0, + type: 'XCUIElementTypeTextField', + label: 'Email', + rect: { x: 10, y: 20, width: 200, height: 40 }, + enabled: true, + hittable: true, + }, + ]), + createdAt: Date.now(), + backend: 'xctest', + }; + // Re-issue the frame over the overridden snapshot, then expire it as if a prior + // device side effect changed the screen. + activateCompleteRefFrame(session); + expireRefFrame(session); + sessionStore.set(sessionName, session); + mockDispatch.mockResolvedValue({ filled: true }); + + // Contrast: a user-supplied `@ref` against the expired frame rejects before + // dispatch (it consumed a stale ref). + const userRef = await runInteraction(sessionStore, sessionName, 'fill', ['@e1', 'hello']); + expect(userRef?.ok).toBe(false); + if (userRef && !userRef.ok) { + expect(userRef.error.details?.reason).toBe('ref_frame_expired'); + } + + // The mutating find's internal dispatch (the exact request find.ts hands the + // leaf) bypasses admission AND attaches no stale-ref warning. + const internal = await handleInteractionCommands({ + req: { + token: 't', + session: sessionName, + command: 'fill', + positionals: ['@e1', 'hello'], + flags: {}, + internal: { findResolvedTarget: true }, + }, + sessionName, + sessionStore, + contextFromFlags, + }); + expect(internal?.ok).toBe(true); + if (internal?.ok) { + expect(internal.data?.warning).toBeUndefined(); + } +}); + test('get text with a pinned stale ref gets the precise warning', async () => { const sessionStore = makeSessionStore(); const sessionName = 'pinned-get-text'; @@ -3576,22 +3666,28 @@ test('get text with a pinned stale ref gets the precise warning', async () => { expect(response?.ok).toBe(true); if (response?.ok) { expect(response.data?.warning).toBe( - 'Ref @e1 was minted from snapshot s2 but the session tree is now s4 — re-run snapshot -i.', + "Ref @e1 was minted from snapshot s2 but the session's ref frame is now s4 — re-run snapshot -i.", ); expect(response.data?.ref).toBe('e1'); } }); -test('ADR 0014: a read-only capture that set the coarse marker does not invalidate a mutation ref', async () => { - // Evidence #6: an internal read-only capture advances the operational - // observation (and the coarse marker) but does NOT expire the frame, so a - // plain ref from the still-active frame is admitted and dispatches — the old - // coarse-marker mutation block was the ADR's false positive. +test('ADR 0014 evidence #6: a read-only capture does not invalidate a mutation ref', async () => { + // An internal read-only capture advances the operational observation (and the + // generation counter) but does NOT expire the frame, so a plain ref from the + // still-active frame is admitted and dispatches — the old coarse-marker + // mutation block was the ADR's false positive. const sessionStore = makeSessionStore(); - const sessionName = 'plain-ref-coarse'; + const sessionName = 'read-capture-preserves'; const session = makeStaleRefSession(sessionName); - session.snapshotGeneration = 7; - session.snapshotRefsStale = true; + // Simulate a read-only capture (e.g. --verify evidence) replacing the + // observation: it bumps the generation but leaves the frame active. + setSessionSnapshot(session, { + nodes: attachRefs(makeTwoButtonNodes() as never), + createdAt: Date.now(), + backend: 'xctest', + }); + expect(session.refFrameState).toBe('active'); sessionStore.set(sessionName, session); mockDispatch.mockResolvedValue({ pressed: true }); @@ -3641,7 +3737,6 @@ test('after a session reopen, a pin from the previous lifetime rejects (reseeded // one replacement deep (a per-lifetime count from 1 would collide here). const reopened = makeStaleRefSession(sessionName); setSessionSnapshot(reopened, { ...reopened.snapshot! }); - reopened.snapshotRefsStale = false; sessionStore.set(sessionName, reopened); // Probabilistic (~1/900000 collision) — accepted residual risk. expect(reopened.snapshotGeneration).not.toBe(oldGeneration); diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index 3dc9cdb5e..1c20a9bb6 100644 --- a/src/daemon/handlers/__tests__/snapshot-handler.test.ts +++ b/src/daemon/handlers/__tests__/snapshot-handler.test.ts @@ -402,7 +402,7 @@ test('snapshot on iOS runs when the session tracks an app', async () => { ); }); -test('snapshot clears the stale-refs marker; diff leaves client refs stale (#1076)', async () => { +test('snapshot re-activates a complete frame; diff preserves it (ADR 0014)', async () => { const sessionStore = makeSessionStore(); const sessionName = 'android-stale-refs-marker'; const session = makeSession(sessionName, androidDevice); @@ -411,8 +411,8 @@ test('snapshot clears the stale-refs marker; diff leaves client refs stale (#107 createdAt: Date.now(), backend: 'android', }; - // As set by a selector-resolution capture that replaced the stored tree. - session.snapshotRefsStale = true; + // A prior device action expired the frame. + session.refFrameState = 'expired'; sessionStore.set(sessionName, session); mockDispatch.mockResolvedValue({ nodes: [{ index: 0, depth: 0, type: 'android.widget.Button', label: 'Fresh' }], @@ -427,9 +427,10 @@ test('snapshot clears the stale-refs marker; diff leaves client refs stale (#107 sessionStore, }); - // The snapshot response hands every stored node's ref to the client. + // The snapshot response hands every stored node's ref to the client: it + // re-activates a complete frame, so refs are current again. expect(snapshotResponse?.ok).toBe(true); - expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(false); + expect(sessionStore.get(sessionName)?.refFrameState).toBe('active'); const diffResponse = await handleSnapshotCommands({ req: { @@ -444,10 +445,10 @@ test('snapshot clears the stale-refs marker; diff leaves client refs stale (#107 sessionStore, }); - // diff replaces the stored tree but only returns a summary, so the refs the - // client holds go stale again. + // diff replaces the observation but is a read (summary response): it preserves + // the authorized frame rather than expiring it. expect(diffResponse?.ok).toBe(true); - expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true); + expect(sessionStore.get(sessionName)?.refFrameState).toBe('active'); }); // #1076 versioned refs — shared harness for the refsGeneration tests below. diff --git a/src/daemon/handlers/find.ts b/src/daemon/handlers/find.ts index 1f6616a39..30ef8faa9 100644 --- a/src/daemon/handlers/find.ts +++ b/src/daemon/handlers/find.ts @@ -24,7 +24,6 @@ import { errorResponse, noActiveSessionError } from './response.ts'; import { recordSessionAction } from './handler-utils.ts'; import { stripInternalInteractionFlags } from '../interaction-outcome-policy.ts'; import { dispatchFindReadOnlyViaRuntime } from '../selector-runtime.ts'; -import { markSessionSnapshotRefsIssued } from '../session-snapshot.ts'; import { createSelectorCaptureRuntime } from '../selector-capture-runtime.ts'; import { isSparseSnapshotQualityVerdict, @@ -156,15 +155,6 @@ export async function handleFindCommands(params: { const ref = `@${resolvedNode.ref}`; const actionFlags = { ...(req.flags ?? {}), noRecord: true }; const match: ResolvedMatch = { node, resolvedNode, ref, nodes, actionFlags }; - if (session) { - // #1076 clear choke point: every action below returns `match.ref`, minted - // from the snapshot the capture runtime just stored, so the client leaves - // with a ref that matches the stored tree. Clearing here also keeps the - // internal `click/fill @ref` sub-invocations from attaching a spurious - // stale-refs warning to the find response. - markSessionSnapshotRefsIssued(session); - sessionStore.set(sessionName, session); - } return dispatchFindAction(ctx, match, action, value); } diff --git a/src/daemon/handlers/interaction-touch-response.ts b/src/daemon/handlers/interaction-touch-response.ts index 9f507d303..01ee6c95d 100644 --- a/src/daemon/handlers/interaction-touch-response.ts +++ b/src/daemon/handlers/interaction-touch-response.ts @@ -67,13 +67,12 @@ export function buildInteractionResponseData(params: { */ extra?: Record; /** - * Staleness warning for the consumed `@ref` argument (#1076), resolved by - * `resolveRefStalenessWarning` (src/daemon/session-snapshot.ts): the coarse - * STALE_SNAPSHOT_REFS_WARNING for plain refs while `snapshotRefsStale` is - * set, or the precise pinned-generation warning for `@e12~s3` refs whose - * generation no longer matches the stored tree. iOS stale mutations are - * rejected before reaching this builder; remaining paths append it to the - * response warning. + * Staleness warning for the consumed `@ref` argument (ADR 0014), resolved by + * `resolveRefStalenessWarning` (src/daemon/session-snapshot.ts): the + * STALE_SNAPSHOT_REFS_WARNING for a plain ref once the frame has expired, or + * the precise pinned-generation warning for a `@e12~s3` ref whose epoch no + * longer matches the frame. Stale mutations are rejected before reaching this + * builder; remaining paths append it to the response warning. */ staleRefsWarning?: string; /** diff --git a/src/daemon/handlers/interaction-touch.ts b/src/daemon/handlers/interaction-touch.ts index 49b2d7e95..2e6b74124 100644 --- a/src/daemon/handlers/interaction-touch.ts +++ b/src/daemon/handlers/interaction-touch.ts @@ -135,12 +135,14 @@ async function dispatchTargetedTouchViaRuntime( : parseTouchTarget(req.positionals ?? [], commandLabel); if (!parsedTarget.ok) return parsedTarget.response; // Staleness relative to what the client knew when it sent this @ref — read - // BEFORE any internal recapture (Android freshness refresh, --verify) flips - // the flag or advances the generation as a side effect of this same command - // (#1076). Pinned refs (`@e12~s3`) get a precise generation-mismatch - // warning; plain refs keep the coarse marker warning. + // BEFORE any internal recapture (Android freshness refresh, --verify) advances + // the generation as a side effect of this same command. Pinned refs + // (`@e12~s3`) get a precise generation-mismatch warning; a plain ref warns + // while the frame is expired. A mutating `find`'s internal dispatch supplies a + // locator-minted ref (`internal.findResolvedTarget`), so it carries no + // user-facing staleness — the caller never consumed a `@ref` (ADR 0014). const staleRefsWarning = - parsedTarget.target.kind === 'ref' + parsedTarget.target.kind === 'ref' && req.internal?.findResolvedTarget !== true ? resolveRefStalenessWarning({ session, ref: parsedTarget.target.ref, @@ -315,12 +317,11 @@ async function buildTargetedTouchResponsePayloads(params: { /** * #1101 `--settle`: a settle observation carrying a diff hands the client refs * minted from the freshly stored settled tree (added lines carry them), which - * makes the response ref-issuing like snapshot/find (#1076): the coarse - * `snapshotRefsStale` marker clears (the same accepted coarse blessing as - * find's single re-issued ref) and the stored tree's generation rides inside - * the settle payload for MCP per-ref pinning. Without a diff — never captured, - * or sparse-quality capture that was not stored — nothing was issued and the - * staleness machinery is left untouched. + * makes the response ref-issuing like snapshot/find: it activates a PARTIAL + * frame (ADR 0014) authorizing exactly those bodies, and the stored tree's + * generation rides inside the settle payload for MCP per-ref pinning. Without a + * diff — never captured, or sparse-quality capture that was not stored — nothing + * was issued and the frame is left as the press's leaf seam expired it. */ function settleRefsGenerationIssue( session: SessionState, @@ -643,11 +644,16 @@ async function prepareFillRefTarget( refGeneration: number | undefined, ): Promise<{ response?: DaemonResponse; staleRefsWarning?: string }> { if (target.kind !== 'ref') return {}; - const staleRefsWarning = resolveRefStalenessWarning({ - session, - ref: target.ref, - mintedGeneration: refGeneration, - }); + // A mutating `find`'s internal dispatch supplies a locator-minted ref, so the + // public response must not claim the caller consumed a stale `@ref` (ADR 0014). + const staleRefsWarning = + params.req.internal?.findResolvedTarget === true + ? undefined + : resolveRefStalenessWarning({ + session, + ref: target.ref, + mintedGeneration: refGeneration, + }); const invalidRefFlagsResponse = params.refSnapshotFlagGuardResponse('fill', params.req.flags); if (invalidRefFlagsResponse) return { response: invalidRefFlagsResponse, staleRefsWarning }; const admissionResponse = params.req.internal?.findResolvedTarget diff --git a/src/daemon/ref-frame.ts b/src/daemon/ref-frame.ts index b63f7d0e5..6d191148d 100644 --- a/src/daemon/ref-frame.ts +++ b/src/daemon/ref-frame.ts @@ -6,17 +6,16 @@ import type { SessionState } from './types.ts'; * * A session owns at most one **ref frame**: the namespace whose refs a caller * may use to target a mutation. This module is the single owner of the frame's - * transitions and of the admission decision. It is introduced behind the - * existing `snapshotGeneration` (the frame epoch) and `snapshotRefsStale` (the - * coarse client-stale marker) fields, which keep their wire-visible names - * (`refsGeneration`, the `@e12~s42` pin grammar) for compatibility. + * transitions and of the admission decision. The frame epoch reuses the existing + * `snapshotGeneration`/`refsGeneration` counter and the `@e12~s42` pin grammar + * for wire compatibility. * * The frame is expired at the device side-effect seam, carries a non-`all` * issuance scope after a partial publication, and its admission matrix is * enforced fail-closed on every platform before dispatch (ADR 0014 steps 3–7). - * The wire names `refsGeneration` and the `@e12~s42` pin grammar are unchanged; - * the coarse `snapshotRefsStale` marker still backs read-only staleness warnings - * until migration step 8 removes it. + * Read-only ref staleness is now derived from frame state (an expired frame + * warns; an active one does not) rather than a coarse client-stale marker, which + * migration step 8 removed. */ /** diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index 36406af6e..86cebbe25 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -153,9 +153,11 @@ export async function dispatchGetViaRuntime( }); if (!resolvedRuntime.ok) return resolvedRuntime.response; - // #1076: get @ref reads from the stored snapshot; warn when that tree was - // replaced since the client last received refs — coarse marker for plain - // refs, precise generation mismatch for pinned `@e12~s3` refs. + // #1076 + ADR 0014: a get @ref binds against the retained ref-frame evidence, + // so it never silently retargets to a newer positional tree. Its warning is + // frame-derived: once the ref frame has expired any ref gets the frame-derived + // warning, else a pinned `@e12~s3` ref whose epoch no longer matches gets the + // precise generation-mismatch warning. const staleRefsWarning = target.target.kind === 'ref' ? resolveRefStalenessWarning({ @@ -256,11 +258,13 @@ export async function dispatchWaitViaRuntime( }); if (directResponse) return directResponse; } - // #1076: wait @ref re-resolves the ref against fresh polling captures; warn - // when the stored tree already drifted from the refs the client holds — - // coarse marker for plain refs, precise generation mismatch for pinned - // `@e12~s3` refs. The pin is split off HERE so the runtime and recording - // only ever see the plain `@e12` form. + // #1076 + ADR 0014: a wait @ref names an element from the retained ref-frame + // evidence, and its staleness is frame-derived rather than a property of the + // live polling capture the condition is checked against. Once the ref frame + // has expired any ref gets the frame-derived warning, else a pinned `@e12~s3` + // ref whose epoch no longer matches gets the precise generation-mismatch + // warning. The pin is split off HERE so the runtime and recording only ever + // see the plain `@e12` form. let waitParsed = parsed; let staleRefsWarning: string | undefined; if (parsed.kind === 'ref') { diff --git a/src/daemon/session-snapshot.ts b/src/daemon/session-snapshot.ts index c45e6f72d..b92eb9c29 100644 --- a/src/daemon/session-snapshot.ts +++ b/src/daemon/session-snapshot.ts @@ -1,50 +1,29 @@ import { randomInt } from 'node:crypto'; import type { SnapshotState } from '../kernel/snapshot.ts'; -import { refFrameEpoch } from './ref-frame.ts'; +import { refFrameEpoch, refFrameState } from './ref-frame.ts'; import type { SessionState } from './types.ts'; /** - * Warning attached to responses of commands that consume an `@ref` argument - * while `session.snapshotRefsStale` is true (#1076). Read-only consumers remain - * warn-only. iOS ref mutations reject stale refs before dispatch (#1239). + * Warning attached to a read of an `@ref` argument once the ref frame has + * expired (ADR 0014): a device side effect changed the screen since the refs + * were issued, so the resolved value may not reflect the current UI. Read-only + * consumers stay fail-open with this warning while the retained frame still + * resolves the ref; mutations reject an expired-frame ref before dispatch. */ export const STALE_SNAPSHOT_REFS_WARNING = - 'The session snapshot changed since your refs were issued — @refs may now point at different elements. Re-run snapshot -i to refresh refs.'; + 'The UI may have changed since these refs were issued, so they no longer represent current device state. Take a new snapshot before relying on or interacting with them.'; /** * The single daemon-side write choke point for replacing a session's stored - * snapshot outside the snapshot/diff command (which builds its next session in - * `buildNextSnapshotSession`, src/daemon/snapshot-runtime.ts, and manages - * `snapshotRefsStale` there because its response DOES hand refs to the client). - * - * Every caller of this function replaces the tree WITHOUT returning the new - * refs to the client, so the stored refs the client holds become positionally - * unreliable and `snapshotRefsStale` is set (#1076 honest marker): - * - selector-capture-runtime.ts — find/get/is/wait selector captures - * - selector-runtime-backend.ts — selector runtime session writes (get/wait) - * - handlers/interaction-runtime.ts — press/click/fill selector-resolution and - * --verify evidence captures routed through the interaction runtime - * - handlers/interaction-snapshot.ts — Android ref-freshness refreshes and - * recording reference-frame captures - * - request-generic-dispatch.ts — screenshot --overlay-refs capture (the - * overlay burns in at most a scored subset of refs, so it does NOT count as - * issuing the full ref set and stays conservative-stale) - * - * Cleared (set false) only where the client demonstrably receives the new - * refs: the snapshot command response (buildNextSnapshotSession), find - * responses that return a ref minted from the freshly stored tree - * (handlers/find.ts, dispatchFindReadOnlyViaRuntime in selector-runtime.ts), - * interaction --settle responses whose settled diff carries refs minted - * from the freshly stored settled tree (settleRefsGenerationIssue in - * handlers/interaction-touch.ts — the same accepted coarse blessing as find's - * single re-issued ref; per-ref precision is the MCP pin layer's job), and - * replay divergence reports whose screen digest hands out refs from the - * freshly stored post-failure tree (captureDivergenceObservation in - * handlers/session-replay-divergence.ts, ADR 0012 migration step 2). + * snapshot outside the snapshot/diff command (`buildNextSnapshotSession`, + * src/daemon/snapshot-runtime.ts). It advances the observation generation but + * does NOT touch the ref frame: replacing the latest observation is an + * operational read, so it never expires, reactivates, or reindexes the + * authorized frame (ADR 0014). Frame lifetime is owned solely by + * `src/daemon/ref-frame.ts` and the partial-issuance writer below. */ export function setSessionSnapshot(session: SessionState, snapshot: SnapshotState): void { if (session.snapshot !== snapshot) { - session.snapshotRefsStale = true; // #1076 versioned refs: every tree replacement advances the session's // snapshot generation, so refs pinned to an earlier generation // (`@e12~s3`) can be diagnosed precisely. @@ -71,20 +50,6 @@ export function nextSnapshotGeneration(current: number | undefined): number { return current === undefined ? randomInt(100_000, 1_000_000) : current + 1; } -/** - * The response being returned hands the stored snapshot's refs to the client. - * - * ADR 0014: this clears the coarse client marker but must NOT re-authorize a - * complete frame — restoring broad `all`-scope mutation authority from a partial - * result is the ADR hole. Only a complete namespace (the snapshot command, via - * `buildNextSnapshotSession`) re-activates a complete frame. A partial - * publication that wants to authorize its bounded ref set calls - * {@link markSessionPartialRefsIssued} instead. - */ -export function markSessionSnapshotRefsIssued(session: SessionState): void { - session.snapshotRefsStale = false; -} - /** Plain ref body: strip a leading `@` and any `~s` generation suffix. */ function normalizeRefBody(ref: string): string { const withoutAt = ref.startsWith('@') ? ref.slice(1) : ref; @@ -107,10 +72,10 @@ export function markSessionPartialRefsIssued(session: SessionState, refs: Iterab if (body.length > 0) scope.add(body); } // ADR 0014: an empty partial result does not supersede existing authority — it - // leaves ALL session state untouched, including the coarse marker. Build the - // scope before touching anything so a no-ref result is a true no-op. + // leaves ALL session state untouched, including the ref frame fields set + // below. Build the scope before touching anything so a no-ref result is a + // true no-op. if (scope.size === 0) return; - session.snapshotRefsStale = false; session.refFrameState = 'active'; session.refFrameScope = scope; // ADR 0014: retain the tree this partial result published from as the frame's @@ -126,31 +91,37 @@ export function markSessionPartialRefsIssued(session: SessionState, refs: Iterab } /** - * Warning for a ref pinned to a generation (`@e12~s3`) that no longer matches - * the stored tree's generation. Unlike STALE_SNAPSHOT_REFS_WARNING it is - * PRECISE: the pin proves which tree minted the ref, so the mismatch is a - * fact, not a conservative marker. + * Warning for a ref pinned to a generation (`@e12~s3`) whose epoch no longer + * matches the session's current ref-frame epoch (`refFrameEpoch`) — NOT the + * latest observation generation. A read-only capture bumps the observation + * counter without re-issuing the frame, so the two can diverge; the warning + * names the frame epoch the pin is actually compared against. Unlike + * STALE_SNAPSHOT_REFS_WARNING it is PRECISE: the pin proves which frame minted + * the ref, so the mismatch is a fact, not a conservative marker. */ function buildPinnedStaleRefWarning(params: { ref: string; mintedGeneration: number; - currentGeneration: number; + currentFrameEpoch: number; }): string { const plainRef = params.ref.startsWith('@') ? params.ref.slice(1) : params.ref; - return `Ref @${plainRef} was minted from snapshot s${params.mintedGeneration} but the session tree is now s${params.currentGeneration} — re-run snapshot -i.`; + return `Ref @${plainRef} was minted from snapshot s${params.mintedGeneration} but the session's ref frame is now s${params.currentFrameEpoch} — re-run snapshot -i.`; } /** - * Staleness warning for a command consuming an `@ref` argument (#1076): - * - pinned ref (`@e12~s3`) matching the stored generation → no warning, even - * while the coarse `snapshotRefsStale` marker is set (the pin proves the - * client's ref came from the stored tree); - * - pinned ref with any other generation → the precise pinned warning; - * - plain ref → the coarse #1093 marker behavior, unchanged. + * Staleness warning for a command consuming an `@ref` argument (ADR 0014). + * Frame expiry is checked FIRST, matching the admission order (evidence #17): an + * expired frame means a device side effect changed the screen since issuance, so + * a read is stale even when a pin matches the epoch — a matching pin proves + * identity within the retained frame, not that the UI is current. + * - frame expired → the coarse staleness warning (any ref); + * - active frame, pinned ref with a different epoch → the precise pinned warning; + * - active frame, pin matching the epoch or a plain ref → no warning. A read-only + * capture no longer marks refs stale, because it does not expire the frame. * * This resolver is advisory; command handlers may enforce stronger freshness - * policy. In particular, iOS ref mutations reject a stale ref before dispatch - * (#1239). + * policy. In particular, a ref mutation rejects an expired-frame ref before + * dispatch. */ export function resolveRefStalenessWarning(params: { session: SessionState | undefined; @@ -158,13 +129,15 @@ export function resolveRefStalenessWarning(params: { mintedGeneration: number | undefined; }): string | undefined { const { session, ref, mintedGeneration } = params; + if (session && refFrameState(session) === 'expired') return STALE_SNAPSHOT_REFS_WARNING; if (mintedGeneration !== undefined) { - // ADR 0014: compare against the FRAME epoch (frozen at issuance), not the - // observation counter — a read-only capture that bumped `snapshotGeneration` - // must not make a valid pin from the issuing frame look stale. - const currentGeneration = session ? (refFrameEpoch(session) ?? 0) : 0; - if (mintedGeneration === currentGeneration) return undefined; - return buildPinnedStaleRefWarning({ ref, mintedGeneration, currentGeneration }); + // Compare against the FRAME epoch (frozen at issuance), not the observation + // counter — a read-only capture that bumped `snapshotGeneration` must not + // make a valid pin from the issuing frame look stale. + const currentFrameEpoch = session ? (refFrameEpoch(session) ?? 0) : 0; + if (mintedGeneration !== currentFrameEpoch) { + return buildPinnedStaleRefWarning({ ref, mintedGeneration, currentFrameEpoch }); + } } - return session?.snapshotRefsStale === true ? STALE_SNAPSHOT_REFS_WARNING : undefined; + return undefined; } diff --git a/src/daemon/snapshot-runtime.ts b/src/daemon/snapshot-runtime.ts index ebd39d59c..6244f38fc 100644 --- a/src/daemon/snapshot-runtime.ts +++ b/src/daemon/snapshot-runtime.ts @@ -251,13 +251,6 @@ function buildNextSnapshotSession(params: { keepCurrentSnapshot, refScopedSnapshot, }); - // #1076 honest marker (see setSessionSnapshot in session-snapshot.ts): - // - kept current snapshot (empty ref-scoped result): tree unchanged, flag unchanged; - // - snapshot command: the response hands every node's ref to the client → cleared; - // - diff: tree replaced but the response is a summary → client refs go stale. - nextSession.snapshotRefsStale = keepCurrentSnapshot - ? current?.snapshotRefsStale - : !params.issuesRefsToClient; // #1076 versioned refs: this path bypasses setSessionSnapshot, so it advances // the generation itself whenever the stored tree is replaced (snapshot AND // diff — diff's summary response leaves client refs pinned to the previous diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 2b7095a4e..b987326a8 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -214,20 +214,6 @@ export type SessionState = { appBundleId?: string; appName?: string; snapshot?: SnapshotState; - /** - * Honest-marker for stale client refs (#1076): true when the stored session - * snapshot was replaced by a capture whose response did NOT hand the new refs - * to the client (selector-resolution captures, wait/find polling captures, - * verify-evidence captures, ...). Commands that consume `@ref` arguments while - * this is true must surface staleness — refs are positional indexes into the - * latest tree, so they may silently resolve to different elements. iOS - * mutations reject before dispatch; read-only and non-iOS consumers attach a - * warning. Cleared only where - * the client demonstrably receives the new refs (snapshot responses, find - * responses that return a ref). Set/cleared at the choke points documented in - * `setSessionSnapshot` (src/daemon/session-snapshot.ts). - */ - snapshotRefsStale?: boolean; /** * Monotonically increasing generation of the stored session snapshot (#1076 * versioned refs). Incremented every time the stored tree is REPLACED — at diff --git a/test/integration/provider-scenarios/versioned-refs.test.ts b/test/integration/provider-scenarios/versioned-refs.test.ts index def8178cb..4400941f7 100644 --- a/test/integration/provider-scenarios/versioned-refs.test.ts +++ b/test/integration/provider-scenarios/versioned-refs.test.ts @@ -139,7 +139,7 @@ test('Provider-backed integration rejects stale pinned @refs and accepts current assert.equal(pinnedStaleDetails.currentGeneration, g2); assert.equal( pinnedStaleData.hint, - `Ref @e2 was minted from snapshot s${g1} but the session tree is now s${g2} — re-run snapshot -i.`, + `Ref @e2 was minted from snapshot s${g1} but the session's ref frame is now s${g2} — re-run snapshot -i.`, ); // A pin to the current generation is admitted.