Skip to content

Commit 22a3c47

Browse files
thymikeeclaude
andauthored
refactor(daemon): remove the superseded coarse snapshotRefsStale marker (ADR 0014 step 8) (#1268)
* refactor(daemon): remove the superseded coarse snapshotRefsStale marker (ADR 0014 step 8) The coarse `snapshotRefsStale` client-stale marker is fully superseded by the ref-frame model and is removed: - `setSessionSnapshot` and `buildNextSnapshotSession` no longer set/clear it — replacing the latest observation is a read that never touches the frame. - Read-only ref staleness now derives from frame state: a plain ref warns once the frame has EXPIRED (a device side effect changed the screen), and a read-only capture no longer marks refs stale because it does not expire the frame. Pinned-ref warnings keep comparing against the frozen frame epoch. - Deletes `markSessionSnapshotRefsIssued` (its only job was clearing the marker) and the `session.snapshotRefsStale` field. Migrates every test off the marker to the frame model (frame-expiry drives the read warning; complete/partial activation drives admission), and updates the ADR status + module docs to record step 8 as landed. Ships as follow-up to the merged #1257 since that PR closed before this step. Full unit-core + provider-integration green; tsc/lint/fallow/production-exports clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): resolve @ref reads from the frame tree; scope find's internal warning Address three review blockers on the coarse-marker removal (ADR 0014 step 8): 1. @ref reads now bind against the authorized frame tree (`refFrameSnapshot ?? snapshot`) in `requireSnapshotSession`, 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 instead of falling through to a newer observation. 2. A mutating find's internal leaf dispatch (`internal.findResolvedTarget`) no longer attaches a stale-ref warning in either the press or fill path — the caller never consumed a `@ref`, so the public find response must not claim it did. 3. `resolveRefStalenessWarning` checks frame expiry FIRST, matching the admission order: an expired frame is stale for any ref, even a pin that matches the epoch (a matching pin proves identity within the retained frame, not that the UI is current). Regressions: divergent observation-vs-frame trees resolve from the frame tree or fail when evidence is missing; a locator-based mutating find from an expired frame carries no stale-ref warning; the reordered resolver unit test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix: correct stale-ref warning comments and ADR-0014 present-tense marker refs The get/wait dispatch comments in selector-runtime.ts still described the superseded coarse snapshotRefsStale marker ("warn when that tree was replaced since the client last received refs") even though staleness is now derived from ref-frame expiry (ADR 0014 migration step 8). Reworded both to describe the frame-derived mechanism actually implemented by resolveRefStalenessWarning. session-snapshot.ts's early-return comment in markSessionPartialRefsIssued referenced "the coarse marker" as something still left untouched, but that field no longer exists — reworded to name the ref frame fields it actually preserves. ADR-0014's "Ref frames are separate from operational observations" section still described snapshotRefsStale as part of "the existing... implementation" in present tense, contradicting the Decision section's own note (line 39) that migration step 8 already removed it. Reworded to keep the historical mention while stating the removal. * fix: frame-lifetime wording for the stale-ref warning and read comments Address the follow-up review blocker plus the co-located terminology cleanup (ADR 0014 step 8): - STALE_SNAPSHOT_REFS_WARNING no longer claims "the session snapshot changed"; it now describes frame lifetime in terms valid for both read warnings and mutation rejection — the UI may have changed since the refs were issued, so take a new snapshot before relying on or interacting with them. The warning fires on frame expiry, including device side effects where no stored snapshot changed. - selector-runtime.ts: the get/wait @ref comments now say the read binds to the retained ref-frame evidence and its staleness is frame-derived, not a property of the stored snapshot or the live polling capture. - settle.ts: an unsettled stored capture replaces the observation without touching the ref frame; read staleness is driven by side-effect-seam expiry, not by storing a fresh observation. - interaction-settle.test.ts: renamed the settle test off the removed stale-marker language to "activates a partial ref frame" (what it asserts). Comments/test-name/warning-text only — no runtime behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): name the ref-frame epoch in the pinned-stale-ref warning The pinned-ref warning is compared against refFrameEpoch(session) — the frozen frame epoch — not the latest observation generation, and after a read-only capture those two diverge. The message still said "the session tree is now sN", which is ambiguous once the observation counter has advanced past the frame epoch. Name the ref-frame epoch instead: Ref @E12 was minted from snapshot s3 but the session's ref frame is now s15 — re-run snapshot -i. Renames the builder param to `currentFrameEpoch` and corrects its doc comment to say the pin is compared against the frame epoch, not the stored tree generation. Regression: `resolveRefStalenessWarning` names the frozen frame epoch, not the bumped observation generation — a read-only `setSessionSnapshot` advances the observation counter (15 -> 16) while the frame epoch stays frozen at 15; a pin at s15 is clean and a pin at s12 names s15, never s16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cc1a0ec commit 22a3c47

17 files changed

Lines changed: 339 additions & 277 deletions

docs/adr/0014-session-ref-frame-lifetime.md

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ remain compatible with selector-based replay, and add no automatic capture or pe
3636
## Decision
3737

3838
The terms introduced below are now the implemented model; the ref-frame vocabulary is promoted into
39-
`CONTEXT.md`. The coarse `snapshotRefsStale` marker still backs read-only staleness warnings and is
40-
removed once per-platform enforcement is confirmed by live evidence (migration step 8).
39+
`CONTEXT.md`. The superseded coarse `snapshotRefsStale` marker has been removed (migration step 8):
40+
read-only ref staleness is now derived from frame state — a plain ref warns once the frame has expired,
41+
and a read-only capture no longer marks refs stale because it does not expire the frame.
4142

4243
### Ref frames are separate from operational observations
4344

@@ -62,9 +63,9 @@ the emitted subset, including ancestors, siblings, overlays, and the viewport. T
6263
the evidence tree, bounds partial authority. The frame and latest observation share immutable capture
6364
data when they originate from the same capture; neither transition deep-copies the tree.
6465

65-
The existing `snapshotGeneration`/`snapshotRefsStale` implementation evolves behind one ref-frame
66-
module. The public name `refsGeneration` and the `@e12~s42` grammar remain unchanged for wire
67-
compatibility.
66+
The existing `snapshotGeneration` implementation evolves behind one ref-frame module; the
67+
`snapshotRefsStale` marker it originally paired with has since been removed (migration step 8). The
68+
public name `refsGeneration` and the `@e12~s42` grammar remain unchanged for wire compatibility.
6869

6970
### Frame transitions
7071

@@ -221,12 +222,12 @@ resolver rather than pretending all subcommands behave alike.
221222
The registry is the exhaustive source of truth. The following table is non-exhaustive guidance for
222223
classifying representative actions; it must not become a second prose registry:
223224

224-
| Effect | Commands/actions |
225-
| --- | --- |
226-
| `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 |
227-
| 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 |
228-
| `delegated` | batch, replay, and test/suite orchestrators; each nested leaf owns its transition |
229-
| `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 |
225+
| Effect | Commands/actions |
226+
| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
227+
| `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 |
228+
| 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 |
229+
| `delegated` | batch, replay, and test/suite orchestrators; each nested leaf owns its transition |
230+
| `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 |
230231

231232
Clipboard reads and writes preserve the frame because pasteboard state alone does not change element
232233
identity. A later paste/type action is independently invalidating.
@@ -437,11 +438,12 @@ Each step lands green and independently useful:
437438
evidence; promote the implemented vocabulary into `CONTEXT.md`; then remove the superseded coarse
438439
stale marker.
439440

440-
Implementation status: steps 1–7 have landed — the ref-frame module and observation split (1), the
441-
complete daemon classification and gate (2), the pre-side-effect seam at every leaf (3), correct
442-
complete/partial publication with bounded scope, MCP pin retention, and pinned partial CLI text (4),
443-
Android freshness decoupled from positional ref authorization (5), the cross-platform contract and
444-
provider evidence (6), and fail-closed admission enforcement across platforms with typed reasons (7).
441+
Implementation status: all migration steps have landed — the ref-frame module and observation split
442+
(1), the complete daemon classification and gate (2), the pre-side-effect seam at every leaf (3),
443+
correct complete/partial publication with bounded scope, MCP pin retention, and pinned partial CLI text
444+
(4), Android freshness decoupled from positional ref authorization (5), the cross-platform contract and
445+
provider evidence (6), fail-closed admission enforcement across platforms with typed reasons (7), and
446+
the docs/vocabulary promotion plus removal of the superseded coarse `snapshotRefsStale` marker (8).
445447
Fresh live evidence has exercised nearly every production seam — Apple runtime-ref, direct/native
446448
selector, generation-pin, generic, and lifecycle paths; Android helper freshness (including proven
447449
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
455457
tap; `interaction-android-recovery-abort.test.ts` proves an outstanding ref action then aborts with the
456458
shared `ref_frame_expired` rejection and no dispatch), the seam is enforced in code identically to the
457459
verified paths, and it is recorded here as a documented, accepted evidence gap rather than an open
458-
release blocker. Every other enabled seam is proven on hardware, so step 8's removal of the superseded
459-
coarse `snapshotRefsStale` marker is unblocked.
460+
release blocker. Every other enabled seam is proven on hardware.
460461

461462
PR #1241 landed independently as a compatible transitional fix. It rejects a known iOS stale-marker
462463
case before this full lifecycle is implemented; it does not own the architecture migration.

src/commands/interaction/runtime/selector-read-shared.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,27 @@ export type CapturedSnapshot = {
2020

2121
export type SelectorSnapshotOptions = SelectorSnapshotInput;
2222

23+
/**
24+
* Resolve the snapshot a `@ref` READ binds against. ADR 0014: a ref resolves
25+
* against the AUTHORIZED frame tree (`refFrameSnapshot`), never the latest
26+
* operational observation — so an internal read-only capture that replaced the
27+
* observation cannot let a plain `@eN` resolve a different element by positional
28+
* coincidence. Missing frame evidence fails (the ref is simply not found in the
29+
* retained tree) rather than falling through to a newer observation. Only used
30+
* by ref reads; selector reads capture fresh through `captureSelectorSnapshot`.
31+
*/
2332
export async function requireSnapshotSession(
2433
runtime: AgentDeviceRuntime,
2534
requestedName: string | undefined,
2635
): Promise<CapturedSnapshot> {
2736
const sessionName = requestedName ?? 'default';
2837
const session = await runtime.sessions.get(sessionName);
2938
if (!session) throw new AppError('SESSION_NOT_FOUND', 'No active session. Run open first.');
30-
if (!session.snapshot) {
39+
const frameTree = session.refFrameSnapshot ?? session.snapshot;
40+
if (!frameTree) {
3141
throw new AppError('INVALID_ARGS', 'No snapshot in session. Run snapshot first.');
3242
}
33-
return { sessionName, session, snapshot: session.snapshot };
43+
return { sessionName, session, snapshot: frameTree };
3444
}
3545

3646
export async function captureSelectorSnapshot(

src/commands/interaction/runtime/settle.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,10 @@ function resolveSettleHint(
322322
// The settle loop itself captures with updateSession: false (a capture that
323323
// later stalls must not race a session write past the response). The FINAL
324324
// capture is stored so follow-up snapshots/selectors see the latest surface.
325-
// Only settled captures issue a diff/ref payload; unsettled stored captures
326-
// conservatively mark prior refs stale through the runtime session writer.
325+
// Only settled captures issue a diff/ref payload; an unsettled stored capture
326+
// replaces the observation without issuing refs — it does NOT touch the ref
327+
// frame. Read staleness is frame-derived: the frame expires at a side-effect
328+
// seam (ADR 0014), not because a fresh observation was stored here.
327329
// Sparse-quality captures are not stored (mirroring captureSelectorSnapshot)
328330
// and therefore issue no refs. The fetched session is returned alongside
329331
// `stored` so the caller can read `appBundleId` for settle-chrome scoping

src/daemon/__tests__/session-snapshot.test.ts

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ test('setSessionSnapshot advances the generation on every tree replacement (#107
3232
const seeded = session.snapshotGeneration!;
3333
expect(seeded).toBeGreaterThanOrEqual(100_000);
3434
expect(seeded).toBeLessThan(1_000_000);
35-
expect(session.snapshotRefsStale).toBe(true);
35+
// ADR 0014: replacing the observation does NOT touch the ref frame.
36+
expect(session.refFrameState).toBeUndefined();
3637

3738
// Storing the SAME snapshot object again is not a replacement.
3839
setSessionSnapshot(session, first);
@@ -51,7 +52,6 @@ test('a reopened session reseeds so pins from a previous lifetime do not silentl
5152
// Reopen: a fresh session object restarts the counter with a NEW seed.
5253
const secondLifetime = makeSession();
5354
setSessionSnapshot(secondLifetime, makeSnapshot());
54-
secondLifetime.snapshotRefsStale = false;
5555

5656
// Probabilistic, not identity-based: the seeds collide with ~1/900000
5757
// 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
6666
).toContain(`minted from snapshot s${oldGeneration}`);
6767
});
6868

69-
test('resolveRefStalenessWarning: pinned-current clean, pinned-stale precise, plain coarse', () => {
69+
test('resolveRefStalenessWarning: frame expiry is checked before the epoch (ADR 0014 evidence #17)', () => {
7070
const session = makeSession();
7171
session.snapshotGeneration = 15;
72-
session.snapshotRefsStale = true;
72+
session.refFrameGeneration = 15;
7373

74-
// Pinned to the stored generation: the pin proves the ref matches the tree,
75-
// so the coarse marker is overruled.
74+
// Expired frame: ANY read is stale, even a pin matching the epoch — a matching
75+
// pin proves identity within the retained frame, not that the UI is current.
76+
session.refFrameState = 'expired';
77+
expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 15 })).toBe(
78+
STALE_SNAPSHOT_REFS_WARNING,
79+
);
80+
expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined })).toBe(
81+
STALE_SNAPSHOT_REFS_WARNING,
82+
);
83+
84+
// Active frame: a pin matching the epoch and a plain ref are both clean; a pin
85+
// from another epoch gets the precise generation warning.
86+
session.refFrameState = 'active';
7687
expect(
7788
resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 15 }),
7889
).toBeUndefined();
79-
90+
expect(
91+
resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined }),
92+
).toBeUndefined();
8093
expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 12 })).toBe(
81-
'Ref @e37 was minted from snapshot s12 but the session tree is now s15 — re-run snapshot -i.',
94+
"Ref @e37 was minted from snapshot s12 but the session's ref frame is now s15 — re-run snapshot -i.",
8295
);
96+
});
8397

84-
expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined })).toBe(
85-
STALE_SNAPSHOT_REFS_WARNING,
86-
);
98+
test('resolveRefStalenessWarning names the frozen frame epoch, not the bumped observation generation (ADR 0014)', () => {
99+
const session = makeSession();
100+
// A frame was issued at generation 15.
101+
session.snapshotGeneration = 15;
102+
session.refFrameGeneration = 15;
103+
session.refFrameState = 'active';
104+
105+
// A read-only capture replaces the observation and advances the observation
106+
// counter WITHOUT re-issuing the frame — the frame epoch stays frozen at 15.
107+
setSessionSnapshot(session, makeSnapshot());
108+
expect(session.snapshotGeneration).toBe(16);
109+
expect(session.refFrameGeneration).toBe(15);
87110

88-
session.snapshotRefsStale = false;
111+
// A pin matching the FROZEN frame epoch is clean, even though the observation
112+
// generation has since advanced past it.
89113
expect(
90-
resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: undefined }),
114+
resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 15 }),
91115
).toBeUndefined();
116+
117+
// A pin from another epoch names the frame epoch (s15), never the bumped
118+
// observation generation (s16).
119+
expect(resolveRefStalenessWarning({ session, ref: '@e37', mintedGeneration: 12 })).toBe(
120+
"Ref @e37 was minted from snapshot s12 but the session's ref frame is now s15 — re-run snapshot -i.",
121+
);
92122
});
93123

94124
test('resolveRefStalenessWarning treats a missing stored generation as s0', () => {
95125
const session = makeSession();
96126
expect(resolveRefStalenessWarning({ session, ref: 'e2', mintedGeneration: 3 })).toBe(
97-
'Ref @e2 was minted from snapshot s3 but the session tree is now s0 — re-run snapshot -i.',
127+
"Ref @e2 was minted from snapshot s3 but the session's ref frame is now s0 — re-run snapshot -i.",
98128
);
99129
expect(resolveRefStalenessWarning({ session, ref: '@e2', mintedGeneration: 0 })).toBeUndefined();
100130
});
101131

102-
test('markSessionPartialRefsIssued: an empty result leaves all state untouched (ADR 0014)', () => {
132+
test('markSessionPartialRefsIssued: an empty result leaves all frame state untouched (ADR 0014)', () => {
103133
const session = makeSession();
104134
// A useful prior frame exists.
105-
session.snapshotRefsStale = true;
106135
session.refFrameState = 'active';
107136
session.refFrameScope = new Set(['e1']);
108137
session.refFrameGeneration = 7;
109138

110-
// An empty partial publication (no refs) must not supersede that authority or
111-
// even clear the coarse marker.
139+
// An empty partial publication (no refs) must not supersede that authority.
112140
markSessionPartialRefsIssued(session, []);
113-
expect(session.snapshotRefsStale).toBe(true);
141+
expect(session.refFrameState).toBe('active');
114142
expect(session.refFrameScope).toEqual(new Set(['e1']));
115143
expect(session.refFrameGeneration).toBe(7);
116144

117145
// A non-empty result supersedes with exactly its bodies.
146+
session.snapshotGeneration = 9;
118147
markSessionPartialRefsIssued(session, ['@e5~s7', 'e6']);
119-
expect(session.snapshotRefsStale).toBe(false);
120148
expect(session.refFrameScope).toEqual(new Set(['e5', 'e6']));
149+
expect(session.refFrameGeneration).toBe(9);
121150
});

src/daemon/handlers/__tests__/find.test.ts

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -751,33 +751,6 @@ test('handleFindCommands wait captures fresh snapshots while polling', async ()
751751
expect(mockDispatch).toHaveBeenCalledTimes(2);
752752
});
753753

754-
test('handleFindCommands click re-issues a fresh ref and clears the stale-refs marker (#1076)', async () => {
755-
const sessionName = 'default';
756-
const session = makeSession(sessionName);
757-
// As set by an earlier selector-resolution capture that replaced the tree.
758-
session.snapshotRefsStale = true;
759-
760-
const { response, session: storedSession } = await runFindClickScenario({
761-
positionals: ['Increment', 'click'],
762-
nodes: [
763-
{
764-
index: 0,
765-
type: 'Button',
766-
label: 'Increment',
767-
hittable: true,
768-
rect: { x: 50, y: 0, width: 100, height: 100 },
769-
depth: 0,
770-
},
771-
],
772-
session,
773-
});
774-
775-
expect(response.ok).toBe(true);
776-
// The response returns a ref minted from the freshly stored snapshot, so
777-
// the marker clears before the internal click @ref sub-invocation runs.
778-
expect(storedSession.snapshotRefsStale).toBe(false);
779-
});
780-
781754
test('handleFindCommands click omits refsGeneration — a mutating find never issues a pinnable ref (ADR 0014)', async () => {
782755
const sessionName = 'default';
783756
const session = makeSession(sessionName);

0 commit comments

Comments
 (0)