Skip to content

Commit eb2ea11

Browse files
committed
fix: preserve snapshot refsGeneration + shared recovery rejection (ADR 0014 re-review)
P1: structured JSON/Node snapshot results now retain the response-level refsGeneration. It was declared on the daemon response but dropped by the public CaptureSnapshotResult type, the serializer, and the Node normalizer, so default `snapshot -i --json` emitted refs with no generation to pin against. Added to the type, serializer, normalizer, plus CLI/Node tests. P2: Android dialog-recovery abort now reuses the SHARED admission rejection (refMutationAdmissionResponse) instead of a bespoke error, so the failure carries the full typed context (reason, ref, currentGeneration, scope, mintedGeneration) identical to every other expired-frame rejection across platforms. Removes the now-unused AppError/refFrameState imports. Adds a regression proving recovery aborts the outstanding ref action before any press dispatch. Also adds the relaunch failure-boundary regression (existing-session close fails after dispatch → old frame stays expired), and corrects the ADR implementation-status note so Android blocking-dialog recovery and a real provider-backed interaction/lifecycle are recorded as unexercised release blockers rather than confirmed enablement. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa
1 parent 2d66f5f commit eb2ea11

9 files changed

Lines changed: 268 additions & 51 deletions

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

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ compatibility.
6868

6969
### Frame transitions
7070

71-
| Event | Frame transition | Plain refs | Pinned refs |
72-
| --- | --- | --- | --- |
73-
| Internal observation that returns no refs | None | Unchanged | Unchanged |
74-
| Complete frame activation | New active epoch, scope `all` | Accepted from this frame | Accepted when epoch matches |
75-
| Non-empty partial publication | New active epoch, scope = emitted refs | Rejected | Accepted only for an emitted ref at this epoch |
76-
| First possible device-side effect | Advance once to `expired` | Rejected | Previous epoch rejected |
77-
| Additional effects while already expired | Idempotent | Rejected | Rejected |
78-
| Sparse, failed, or unusable capture | None | Unchanged | Unchanged |
79-
| Session reopen | New random-seeded lifetime | Rejected until publication | Old lifetime rejected probabilistically as today |
71+
| Event | Frame transition | Plain refs | Pinned refs |
72+
| ----------------------------------------- | -------------------------------------- | -------------------------- | ------------------------------------------------ |
73+
| Internal observation that returns no refs | None | Unchanged | Unchanged |
74+
| Complete frame activation | New active epoch, scope `all` | Accepted from this frame | Accepted when epoch matches |
75+
| Non-empty partial publication | New active epoch, scope = emitted refs | Rejected | Accepted only for an emitted ref at this epoch |
76+
| First possible device-side effect | Advance once to `expired` | Rejected | Previous epoch rejected |
77+
| Additional effects while already expired | Idempotent | Rejected | Rejected |
78+
| Sparse, failed, or unusable capture | None | Unchanged | Unchanged |
79+
| Session reopen | New random-seeded lifetime | Rejected until publication | Old lifetime rejected probabilistically as today |
8080

8181
A complete activation normally accompanies a command result that exposes the complete ref namespace
8282
for the stored frame, including an interactive or intentionally scoped snapshot. An intentionally
@@ -209,9 +209,7 @@ The command descriptor's daemon facet declares a request policy:
209209
```ts
210210
type RefFrameEffect = 'preserve' | 'may-invalidate' | 'delegated';
211211

212-
type DaemonRefFrameEffect =
213-
| RefFrameEffect
214-
| ((request: DaemonRequest) => RefFrameEffect);
212+
type DaemonRefFrameEffect = RefFrameEffect | ((request: DaemonRequest) => RefFrameEffect);
215213
```
216214

217215
The daemon registry exposes a named resolver. Every daemon command is classified, but the
@@ -223,12 +221,12 @@ resolver rather than pretending all subcommands behave alike.
223221
The registry is the exhaustive source of truth. The following table is non-exhaustive guidance for
224222
classifying representative actions; it must not become a second prose registry:
225223

226-
| Effect | Commands/actions |
227-
| --- | --- |
228-
| `may-invalidate` | press, click, fill, longpress, type, focus, scroll, swipe, gesture, back, home, `tv-remote`, rotate, open/relaunch, trigger/push delivery, settings changes, install/reinstall, React Native overlay dismissal, and lifecycle operations that can replace the visible surface |
229-
| 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 |
230-
| `delegated` | batch, replay, and test/suite orchestrators; each nested leaf owns its transition |
231-
| `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 |
224+
| Effect | Commands/actions |
225+
| -------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
226+
| `may-invalidate` | press, click, fill, longpress, type, focus, scroll, swipe, gesture, back, home, `tv-remote`, rotate, 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 |
232230

233231
Clipboard reads and writes preserve the frame because pasteboard state alone does not change element
234232
identity. A later paste/type action is independently invalidating.
@@ -444,9 +442,14 @@ complete daemon classification and gate (2), the pre-side-effect seam at every l
444442
complete/partial publication with bounded scope, MCP pin retention, and pinned partial CLI text (4),
445443
Android freshness decoupled from positional ref authorization (5), the cross-platform contract and
446444
provider evidence (6), and fail-closed admission enforcement across platforms with typed reasons (7).
447-
Per-platform enablement is confirmed by the live-evidence runs required above; step 8's removal of the
448-
superseded coarse `snapshotRefsStale` marker follows that confirmation so read-only warnings are not
449-
disturbed before enforcement is proven on hardware.
445+
Fresh live evidence has exercised most production seams — Apple runtime-ref, direct/native selector,
446+
generation-pin, generic, and lifecycle paths, plus Android helper freshness (including proven
447+
non-retarget) and Android existing-session relaunch. Two seams remain UNEXERCISED and are therefore
448+
explicit release blockers, not confirmed enablement: Android blocking-dialog recovery, and a real
449+
provider-backed interaction plus provider-backed lifecycle operation. Enforcement stays enabled in
450+
code, but those two seams are not claimed as verified until their live runs exist. Step 8's removal of
451+
the superseded coarse `snapshotRefsStale` marker follows full confirmation, so read-only warnings are
452+
not disturbed before every enabled seam is proven on hardware.
450453

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

‎src/__tests__/client.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,24 @@ test('client capture.snapshot preserves visibility metadata from daemon response
883883
});
884884
});
885885

886+
test('client capture.snapshot preserves refsGeneration from daemon responses (ADR 0014)', async () => {
887+
const setup = createTransport(async () => ({
888+
ok: true,
889+
data: {
890+
nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Button', label: 'Go' }],
891+
truncated: false,
892+
refsGeneration: 752890,
893+
},
894+
}));
895+
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
896+
897+
const result = await client.capture.snapshot();
898+
899+
// Node.js callers must retain the response-level generation to pin a plain ref
900+
// (`@e1~s752890`) before a mutation.
901+
assert.equal(result.refsGeneration, 752890);
902+
});
903+
886904
test('client capture.snapshot preserves snapshot quality annotation from daemon responses', async () => {
887905
const snapshotQuality = {
888906
state: 'recovered',

‎src/agent-device-client.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ function optionalSnapshotResponseFields(
499499
| 'warnings'
500500
| 'snapshotQuality'
501501
| 'snapshotDiagnostics'
502+
| 'refsGeneration'
502503
>
503504
> {
504505
const visibility = readObject(data.visibility);
@@ -509,6 +510,9 @@ function optionalSnapshotResponseFields(
509510
...readSerializedSnapshotCaptureAnnotations(data),
510511
...(unchanged ? { unchanged: unchanged as CaptureSnapshotResult['unchanged'] } : {}),
511512
...(snapshotDiagnostics ? { snapshotDiagnostics } : {}),
513+
// ADR 0014: keep the response-level ref-frame generation on Node.js results
514+
// so callers can pin refs (`@e12~s<refsGeneration>`) before a mutation.
515+
...(typeof data.refsGeneration === 'number' ? { refsGeneration: data.refsGeneration } : {}),
512516
};
513517
}
514518

‎src/client/client-types.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,12 @@ export type CaptureSnapshotResult = {
447447
unchanged?: SnapshotUnchanged;
448448
snapshotDiagnostics?: SnapshotDiagnosticsSummary;
449449
identifiers: AgentDeviceIdentifiers;
450+
/**
451+
* ADR 0014: the response-level ref-frame epoch the plain node refs were minted
452+
* from. A ref-issuing snapshot carries it ONCE (nodes stay plain `@e12` for the
453+
* token budget); pair a ref with it (`@e12~s<refsGeneration>`) before a mutation.
454+
*/
455+
refsGeneration?: number;
450456
} & PublicSnapshotCaptureAnnotations;
451457

452458
export type CaptureScreenshotOptions = AgentDeviceRequestOverrides & {

‎src/contracts/result-serialization.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,19 @@ test('serializeSnapshotResult includes Android backend metadata', () => {
131131
});
132132
});
133133

134+
test('serializeSnapshotResult preserves the response-level refsGeneration (ADR 0014)', () => {
135+
const data = serializeSnapshotResult({
136+
nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'Button', label: 'Go' }],
137+
truncated: false,
138+
refsGeneration: 752890,
139+
identifiers: { session: 'qa' },
140+
} as Parameters<typeof serializeSnapshotResult>[0]);
141+
142+
assert.equal(data.refsGeneration, 752890);
143+
// The node tree stays plain — the generation rides once at the response level.
144+
assert.equal((data.nodes as Array<{ ref?: string }>)[0]?.ref, 'e1');
145+
});
146+
134147
test('serializeSnapshotResult maps capture quality annotation to public snapshotQuality', () => {
135148
const snapshotQuality = {
136149
state: 'healthy',

‎src/contracts/result-serialization.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,9 @@ export function serializeSnapshotResult(result: CaptureSnapshotResult): Record<s
182182
...publicSnapshotCaptureAnnotations(snapshotResultAnnotations(result)),
183183
...(result.unchanged ? { unchanged: result.unchanged } : {}),
184184
...(result.snapshotDiagnostics ? { snapshotDiagnostics: result.snapshotDiagnostics } : {}),
185+
// ADR 0014: a ref-issuing snapshot retains its response-level generation so
186+
// JSON callers can pair a plain `@e12` with `~s<refsGeneration>` before a mutation.
187+
...(result.refsGeneration !== undefined ? { refsGeneration: result.refsGeneration } : {}),
185188
};
186189
}
187190

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { test, expect, vi, beforeEach } from 'vitest';
2+
3+
// ADR 0014 (evidence #13): Android blocking-dialog recovery is device-mutating
4+
// and expires the frame at its own seam. A ref action admitted against the
5+
// pre-recovery frame must ABORT after recovery mutates — it cannot continue
6+
// against the recovered UI — and the rejection must use the SHARED admission
7+
// shape (reason + ref + currentGeneration + scope), not a bespoke error.
8+
9+
vi.mock('../../../core/dispatch.ts', async (importOriginal) => {
10+
const actual = await importOriginal<typeof import('../../../core/dispatch.ts')>();
11+
return { ...actual, dispatchCommand: vi.fn(async () => ({})) };
12+
});
13+
14+
vi.mock('../interaction-snapshot.ts', async (importOriginal) => {
15+
const actual = await importOriginal<typeof import('../interaction-snapshot.ts')>();
16+
return {
17+
...actual,
18+
captureSnapshotForSession: vi.fn(async () => ({ nodes: [], createdAt: 0, backend: 'android' })),
19+
};
20+
});
21+
22+
// Simulate recovery: the before-command readiness check taps a blocking dialog
23+
// (a device mutation) and reports `recovered`, expiring the frame.
24+
vi.mock('../../android-system-dialog.ts', async (importOriginal) => {
25+
const actual = await importOriginal<typeof import('../../android-system-dialog.ts')>();
26+
const { expireRefFrame } = await import('../../ref-frame.ts');
27+
return {
28+
...actual,
29+
ensureAndroidBlockingSystemDialogReady: vi.fn(
30+
async (params: { session: import('../../types.ts').SessionState; phase: string }) => {
31+
if (params.phase === 'before-command') {
32+
expireRefFrame(params.session);
33+
return { status: 'recovered', warning: 'Recovered from a blocking system dialog' };
34+
}
35+
return { status: 'clear' };
36+
},
37+
),
38+
};
39+
});
40+
41+
import { handleInteractionCommands } from '../interaction.ts';
42+
import { dispatchCommand } from '../../../core/dispatch.ts';
43+
import { attachRefs } from '../../../kernel/snapshot.ts';
44+
import { makeAndroidSession } from '../../../__tests__/test-utils/session-factories.ts';
45+
import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts';
46+
47+
const mockDispatch = vi.mocked(dispatchCommand);
48+
const contextFromFlags = () => ({});
49+
50+
beforeEach(() => {
51+
mockDispatch.mockReset();
52+
mockDispatch.mockResolvedValue({});
53+
});
54+
55+
test('a ref action aborts with the shared ref_frame_expired rejection after Android dialog recovery mutates', async () => {
56+
const sessionStore = makeSessionStore();
57+
const sessionName = 'android-recovery-abort';
58+
const session = makeAndroidSession(sessionName);
59+
session.snapshot = {
60+
nodes: attachRefs([
61+
{
62+
index: 0,
63+
type: 'android.widget.Button',
64+
label: 'Continue',
65+
rect: { x: 0, y: 0, width: 80, height: 80 },
66+
enabled: true,
67+
hittable: true,
68+
},
69+
]),
70+
createdAt: Date.now(),
71+
backend: 'android',
72+
};
73+
session.snapshotGeneration = 900;
74+
// A freshly issued complete frame is active.
75+
sessionStore.set(sessionName, session);
76+
77+
const response = await handleInteractionCommands({
78+
req: { token: 't', session: sessionName, command: 'press', positionals: ['@e1'], flags: {} },
79+
sessionName,
80+
sessionStore,
81+
contextFromFlags,
82+
});
83+
84+
expect(response?.ok).toBe(false);
85+
if (response && !response.ok) {
86+
expect(response.error.code).toBe('COMMAND_FAILED');
87+
// The SHARED admission rejection shape — not a bespoke recovery error.
88+
const details = response.error.details as Record<string, unknown> | undefined;
89+
expect(details?.reason).toBe('ref_frame_expired');
90+
expect(details?.ref).toBe('@e1');
91+
expect(details?.currentGeneration).toBe(900);
92+
expect(details?.scope).toBe('all');
93+
}
94+
// The outstanding ref action never dispatched a press against the recovered UI.
95+
expect(mockDispatch.mock.calls.some((call) => call[1] === 'press')).toBe(false);
96+
});

‎src/daemon/handlers/__tests__/session-relaunch-close.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,51 @@ test('open --relaunch closes and reopens active session app', async () => {
5757
expect(calls[1]).toEqual({ command: 'open', positionals: ['com.example.app'] });
5858
});
5959

60+
test('open --relaunch leaves the old frame expired when the close dispatch fails after dispatch (ADR 0014)', async () => {
61+
const sessionStore = makeSessionStore();
62+
const sessionName = 'android-session';
63+
sessionStore.set(sessionName, {
64+
...makeSession(sessionName, {
65+
platform: 'android',
66+
id: 'emulator-5554',
67+
name: 'Pixel Emulator',
68+
kind: 'emulator',
69+
booted: true,
70+
}),
71+
appName: 'com.example.app',
72+
snapshotGeneration: 400,
73+
});
74+
// A freshly issued frame is active before the relaunch.
75+
expect(sessionStore.get(sessionName)?.refFrameState).toBeUndefined();
76+
77+
// The relaunch close dispatches and then fails/times out AFTER the app may
78+
// already have been torn down.
79+
mockDispatch.mockImplementation(async (_device, command) => {
80+
if (command === 'close') throw new Error('adb: close timed out after dispatch');
81+
return {};
82+
});
83+
84+
const response = await handleSessionCommands({
85+
req: {
86+
token: 't',
87+
session: sessionName,
88+
command: 'open',
89+
positionals: [],
90+
flags: { relaunch: true },
91+
},
92+
sessionName,
93+
logPath: path.join(os.tmpdir(), 'daemon.log'),
94+
sessionStore,
95+
invoke: noopInvoke,
96+
}).catch(() => null);
97+
98+
// Whether the failure surfaced as an error response or a throw, the existing
99+
// session's frame was expired BEFORE the close dispatch and stays expired — a
100+
// post-dispatch close failure never restores it (there is no rollback).
101+
expect(response?.ok ?? false).toBe(false);
102+
expect(sessionStore.get(sessionName)?.refFrameState).toBe('expired');
103+
});
104+
60105
test('open --relaunch on iOS stops runner before close/open', async () => {
61106
const sessionStore = makeSessionStore();
62107
const sessionName = 'ios-session';

0 commit comments

Comments
 (0)