Skip to content

Commit 62f8be7

Browse files
committed
fix(snapshot): one-shot recovered warning for internally armed penalties
The deferred-capture suppression assumed the capture that armed the XCTest-channel penalty already rendered the full 'overly complex or slow accessibility tree' warning. Internal captures (selector resolution, settle observation loops, system-modal probes) can arm the penalty without any user-facing render, leaving the next public snapshot with only the structured verdict and no CLI warning line. The runner cannot tell user-facing from internal captures, so the daemon now holds a per-session one-shot latch (snapshot-quality-latch.ts) applied at the snapshot/diff response seam: a genuine recovered render sets it silently, the first public 'deferred' verdict without the latch re-renders the full warning once and sets it, a healthy public verdict clears it (the penalty window is over), and an app switch supersedes it. Internal observation responses (observationOnly) neither consume nor clear the latch. Follow-up to PR #1587 review (non-blocking hardening).
1 parent 8d526f2 commit 62f8be7

5 files changed

Lines changed: 368 additions & 6 deletions

File tree

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
import os from 'node:os';
2+
import path from 'node:path';
3+
import { expect, test, vi } from 'vitest';
4+
import type { SnapshotQualityVerdict } from '@agent-device/kernel/snapshot';
5+
import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts';
6+
import {
7+
recoveredSnapshotQualityWarning,
8+
renderSnapshotQualityWarnings,
9+
} from '../../snapshot/snapshot-quality.ts';
10+
import {
11+
applyRecoveredWarningLatch,
12+
resolveRecoveredWarningLatch,
13+
} from '../snapshot-quality-latch.ts';
14+
import { dispatchSnapshotViaRuntime } from '../snapshot-runtime.ts';
15+
import { SessionStore } from '../session-store.ts';
16+
import type { SessionState } from '../types.ts';
17+
18+
const dispatchCommandMock = vi.hoisted(() => vi.fn());
19+
20+
vi.mock('../../core/dispatch.ts', async (importOriginal) => {
21+
const actual = await importOriginal<typeof import('../../core/dispatch.ts')>();
22+
return {
23+
...actual,
24+
dispatchCommand: dispatchCommandMock,
25+
};
26+
});
27+
28+
const FULL_WARNING = recoveredSnapshotQualityWarning('private-ax');
29+
30+
function deferredVerdict(): SnapshotQualityVerdict {
31+
return {
32+
state: 'recovered',
33+
backend: 'private-ax',
34+
reason: 'XCTest-backed snapshot tiers were deferred after recent slow accessibility work',
35+
reasonCode: 'deferred',
36+
};
37+
}
38+
39+
function genuineRecoveredVerdict(): SnapshotQualityVerdict {
40+
return {
41+
state: 'recovered',
42+
backend: 'private-ax',
43+
reason: 'iOS XCTest snapshot failed while serializing the accessibility tree',
44+
reasonCode: 'ax-rejected',
45+
};
46+
}
47+
48+
test('the injected warning is the exact line a genuine recovery renders', () => {
49+
// The latch exists to restore the warning an internal arming capture never rendered;
50+
// wording drift between the two paths would defeat one-shot deduplication.
51+
expect(renderSnapshotQualityWarnings(genuineRecoveredVerdict(), [])).toEqual([FULL_WARNING]);
52+
});
53+
54+
test('resolveRecoveredWarningLatch transitions', () => {
55+
const app = 'com.example.app';
56+
57+
// First deferred verdict with no latch held: inject once and hold the latch.
58+
const first = resolveRecoveredWarningLatch({
59+
verdict: deferredVerdict(),
60+
appBundleId: app,
61+
latch: undefined,
62+
});
63+
expect(first.warning).toBe(FULL_WARNING);
64+
expect(first.latch).toEqual({ appBundleId: app });
65+
66+
// Held latch: further deferred verdicts stay quiet.
67+
const repeat = resolveRecoveredWarningLatch({
68+
verdict: deferredVerdict(),
69+
appBundleId: app,
70+
latch: first.latch,
71+
});
72+
expect(repeat.warning).toBeUndefined();
73+
expect(repeat.latch).toEqual({ appBundleId: app });
74+
75+
// A genuine recovery renders through the runtime, so it sets the latch silently.
76+
const genuine = resolveRecoveredWarningLatch({
77+
verdict: genuineRecoveredVerdict(),
78+
appBundleId: app,
79+
latch: undefined,
80+
});
81+
expect(genuine.warning).toBeUndefined();
82+
expect(genuine.latch).toEqual({ appBundleId: app });
83+
84+
// Healthy ends the penalty window; the next arming warns again.
85+
const healthy = resolveRecoveredWarningLatch({
86+
verdict: { state: 'healthy', backend: 'tree' },
87+
appBundleId: app,
88+
latch: first.latch,
89+
});
90+
expect(healthy.latch).toBeUndefined();
91+
92+
// Sparse and verdict-less captures carry no penalty signal either way.
93+
const sparse = resolveRecoveredWarningLatch({
94+
verdict: { state: 'sparse', backend: 'private-ax' },
95+
appBundleId: app,
96+
latch: first.latch,
97+
});
98+
expect(sparse.warning).toBeUndefined();
99+
expect(sparse.latch).toEqual({ appBundleId: app });
100+
const absent = resolveRecoveredWarningLatch({
101+
verdict: undefined,
102+
appBundleId: app,
103+
latch: first.latch,
104+
});
105+
expect(absent.warning).toBeUndefined();
106+
expect(absent.latch).toEqual({ appBundleId: app });
107+
108+
// The runner clears its penalty on app switch, so a latch held for another
109+
// app never suppresses the new app's first deferred verdict.
110+
const switched = resolveRecoveredWarningLatch({
111+
verdict: deferredVerdict(),
112+
appBundleId: 'com.example.other',
113+
latch: first.latch,
114+
});
115+
expect(switched.warning).toBe(FULL_WARNING);
116+
expect(switched.latch).toEqual({ appBundleId: 'com.example.other' });
117+
});
118+
119+
test('internal observation responses neither consume nor clear the latch', () => {
120+
const session = makeIosSession('default', { appBundleId: 'com.example.app' });
121+
122+
const internal = applyRecoveredWarningLatch({
123+
session,
124+
data: { snapshotQuality: deferredVerdict() },
125+
internalObservation: true,
126+
});
127+
expect(internal.warnings).toBeUndefined();
128+
expect(session.recoveredSnapshotWarningLatch).toBeUndefined();
129+
130+
session.recoveredSnapshotWarningLatch = { appBundleId: 'com.example.app' };
131+
applyRecoveredWarningLatch({
132+
session,
133+
data: { snapshotQuality: { state: 'healthy', backend: 'tree' } },
134+
internalObservation: true,
135+
});
136+
expect(session.recoveredSnapshotWarningLatch).toEqual({ appBundleId: 'com.example.app' });
137+
});
138+
139+
test('sessionless responses pass through unchanged', () => {
140+
const data = { snapshotQuality: deferredVerdict() };
141+
expect(
142+
applyRecoveredWarningLatch({ session: undefined, data, internalObservation: false }),
143+
).toBe(data);
144+
});
145+
146+
function scenario() {
147+
const root = path.join(os.tmpdir(), `agent-device-quality-latch-${crypto.randomUUID()}`);
148+
const sessionStore = new SessionStore(path.join(root, 'sessions'));
149+
const sessionName = 'default';
150+
const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' });
151+
sessionStore.set(sessionName, session);
152+
return { sessionStore, sessionName, logPath: path.join(root, 'daemon.log') };
153+
}
154+
155+
function seedCapture(verdict: SnapshotQualityVerdict) {
156+
dispatchCommandMock.mockResolvedValue({
157+
backend: 'xctest',
158+
truncated: false,
159+
quality: verdict,
160+
nodes: [
161+
{
162+
index: 0,
163+
depth: 0,
164+
type: 'Button',
165+
label: 'Continue',
166+
rect: { x: 0, y: 0, width: 100, height: 44 },
167+
hittable: true,
168+
},
169+
],
170+
});
171+
}
172+
173+
async function dispatchPublicSnapshot(input: ReturnType<typeof scenario>) {
174+
return await dispatchSnapshotViaRuntime({
175+
req: { command: 'snapshot', positionals: [], token: 't', session: input.sessionName },
176+
sessionName: input.sessionName,
177+
logPath: input.logPath,
178+
sessionStore: input.sessionStore,
179+
});
180+
}
181+
182+
function responseWarnings(response: Awaited<ReturnType<typeof dispatchSnapshotViaRuntime>>) {
183+
if (!response.ok) throw new Error('expected ok response');
184+
return (response.data?.warnings ?? []) as string[];
185+
}
186+
187+
function storedLatch(input: ReturnType<typeof scenario>): SessionState['recoveredSnapshotWarningLatch'] {
188+
return input.sessionStore.get(input.sessionName)?.recoveredSnapshotWarningLatch;
189+
}
190+
191+
test('an internally armed penalty warns once on the first public deferred snapshot', async () => {
192+
const input = scenario();
193+
seedCapture(deferredVerdict());
194+
195+
// The internal capture that armed the penalty (settle observation, selector
196+
// resolution) rendered nothing; without the latch the deferred suppression
197+
// would hide the warning from the user entirely.
198+
const internal = await dispatchSnapshotViaRuntime({
199+
req: {
200+
command: 'snapshot',
201+
positionals: [],
202+
token: 't',
203+
session: input.sessionName,
204+
internal: { observationOnly: true },
205+
},
206+
sessionName: input.sessionName,
207+
logPath: input.logPath,
208+
sessionStore: input.sessionStore,
209+
});
210+
expect(responseWarnings(internal)).not.toContain(FULL_WARNING);
211+
expect(storedLatch(input)).toBeUndefined();
212+
213+
const first = await dispatchPublicSnapshot(input);
214+
expect(responseWarnings(first).filter((line) => line === FULL_WARNING)).toHaveLength(1);
215+
expect(storedLatch(input)).toEqual({ appBundleId: 'com.example.app' });
216+
217+
const second = await dispatchPublicSnapshot(input);
218+
expect(responseWarnings(second)).not.toContain(FULL_WARNING);
219+
});
220+
221+
test('a healthy public capture re-arms the one-shot warning', async () => {
222+
const input = scenario();
223+
seedCapture(deferredVerdict());
224+
await dispatchPublicSnapshot(input);
225+
expect(storedLatch(input)).toEqual({ appBundleId: 'com.example.app' });
226+
227+
seedCapture({ state: 'healthy', backend: 'tree' });
228+
const healthy = await dispatchPublicSnapshot(input);
229+
expect(responseWarnings(healthy)).not.toContain(FULL_WARNING);
230+
expect(storedLatch(input)).toBeUndefined();
231+
232+
seedCapture(deferredVerdict());
233+
const rearmed = await dispatchPublicSnapshot(input);
234+
expect(responseWarnings(rearmed).filter((line) => line === FULL_WARNING)).toHaveLength(1);
235+
});
236+
237+
test('a genuine recovered render keeps later deferred captures quiet without doubling', async () => {
238+
const input = scenario();
239+
seedCapture(genuineRecoveredVerdict());
240+
241+
const genuine = await dispatchPublicSnapshot(input);
242+
expect(responseWarnings(genuine).filter((line) => line === FULL_WARNING)).toHaveLength(1);
243+
expect(storedLatch(input)).toEqual({ appBundleId: 'com.example.app' });
244+
245+
seedCapture(deferredVerdict());
246+
const deferred = await dispatchPublicSnapshot(input);
247+
expect(responseWarnings(deferred)).not.toContain(FULL_WARNING);
248+
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import type { SnapshotQualityVerdict } from '@agent-device/kernel/snapshot';
2+
import {
3+
readSnapshotQualityVerdict,
4+
recoveredSnapshotQualityWarning,
5+
} from '../snapshot/snapshot-quality.ts';
6+
import type { DaemonResponseData, SessionState } from './types.ts';
7+
8+
type RecoveredWarningLatch = NonNullable<SessionState['recoveredSnapshotWarningLatch']>;
9+
10+
type LatchDecision = {
11+
/** Full recovered warning to prepend to the response, when the latch was not held. */
12+
warning?: string;
13+
latch: RecoveredWarningLatch | undefined;
14+
};
15+
16+
/**
17+
* One-shot warning latch for penalty-deferred captures (PR #1587 follow-up).
18+
*
19+
* `renderSnapshotQualityWarnings` suppresses the full recovered warning for
20+
* `reasonCode: 'deferred'` on the assumption that the capture which ARMED the
21+
* XCTest-channel penalty already rendered it. Internal captures (selector
22+
* resolution, settle observation loops, system-modal probes) can arm the
23+
* penalty without any user-facing render, and the runner cannot tell the two
24+
* apart — so the daemon tracks per session whether a recovered warning actually
25+
* reached this client for the currently penalized app, and re-renders it once
26+
* when it has not.
27+
*
28+
* Transitions (evaluated only on user-facing snapshot/diff responses):
29+
* - genuine `recovered` — the same response already carries the full warning
30+
* (rendered by the runtime), so the latch is set without injecting anything;
31+
* - `deferred` with the latch not held for this app — inject the full warning
32+
* once and set the latch;
33+
* - `healthy` — the penalty window is over; clear the latch so the next arming
34+
* renders again;
35+
* - `sparse` or no verdict — no penalty signal either way; latch unchanged.
36+
*
37+
* The daemon cannot observe penalty arming directly (a 120s TTL held
38+
* runner-side), so "penalty window" is approximated by the verdicts of public
39+
* captures: as long as no public capture reports healthy, an unbroken hostile
40+
* stretch of the same app warns once.
41+
*/
42+
export function resolveRecoveredWarningLatch(params: {
43+
verdict: SnapshotQualityVerdict | undefined;
44+
appBundleId: string | undefined;
45+
latch: RecoveredWarningLatch | undefined;
46+
}): LatchDecision {
47+
const { verdict, appBundleId, latch } = params;
48+
if (!verdict) return { latch };
49+
if (verdict.state === 'healthy') return { latch: undefined };
50+
if (verdict.state !== 'recovered') return { latch };
51+
if (verdict.reasonCode !== 'deferred') {
52+
return { latch: { appBundleId } };
53+
}
54+
if (latch !== undefined && latch.appBundleId === appBundleId) return { latch };
55+
return { warning: recoveredSnapshotQualityWarning(verdict.backend), latch: { appBundleId } };
56+
}
57+
58+
/**
59+
* Applies the latch to a user-facing snapshot/diff response: updates the
60+
* session's latch state and prepends the full recovered warning when this is
61+
* the penalty window's first user-facing render. Internal observation captures
62+
* (`req.internal.observationOnly`) must pass `internalObservation: true` — they
63+
* never reach the user, so they neither consume nor clear the latch.
64+
*/
65+
export function applyRecoveredWarningLatch(params: {
66+
session: SessionState | undefined;
67+
data: DaemonResponseData;
68+
internalObservation: boolean;
69+
}): DaemonResponseData {
70+
const { session, data, internalObservation } = params;
71+
if (internalObservation || !session) return data;
72+
// The snapshot command carries the capture's verdict in the response; diff
73+
// publishes only warnings, so its verdict is read from the session snapshot
74+
// the capture just stored.
75+
const verdict =
76+
readSnapshotQualityVerdict(data.snapshotQuality) ?? session.snapshot?.snapshotQuality;
77+
const decision = resolveRecoveredWarningLatch({
78+
verdict,
79+
appBundleId: session.appBundleId,
80+
latch: session.recoveredSnapshotWarningLatch,
81+
});
82+
session.recoveredSnapshotWarningLatch = decision.latch;
83+
if (!decision.warning) return data;
84+
const warnings = Array.isArray(data.warnings) ? data.warnings : [];
85+
return { ...data, warnings: [decision.warning, ...warnings] };
86+
}

src/daemon/snapshot-runtime.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
withSessionlessRunnerCleanup,
1919
} from './handlers/snapshot-session.ts';
2020
import { activateCompleteRefFrame } from './ref-frame.ts';
21+
import { applyRecoveredWarningLatch } from './snapshot-quality-latch.ts';
2122
import { createDaemonRuntimePolicy } from './runtime-policy.ts';
2223
import { createDaemonRuntimeSessionStore } from './runtime-session.ts';
2324
import { isInteractiveObservation } from './session-action-recorder.ts';
@@ -180,7 +181,11 @@ async function dispatchSnapshotRuntimeCommand(
180181
});
181182
return {
182183
ok: true,
183-
data: result.data,
184+
data: applyRecoveredWarningLatch({
185+
session: sessionStore.get(sessionName),
186+
data: result.data,
187+
internalObservation: req.internal?.observationOnly === true,
188+
}),
184189
};
185190
});
186191
}

src/daemon/types.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,18 @@ export type SessionState = {
319319
* probabilistic (seeded), NOT identity-based.
320320
*/
321321
snapshotGeneration?: number;
322+
/**
323+
* One-shot latch: the full "overly complex or slow accessibility tree" warning has been
324+
* rendered to this session's client for the app currently under the XCTest-channel
325+
* penalty. Penalty-deferred verdicts (`reasonCode: 'deferred'`) suppress the repeated
326+
* warning in `renderSnapshotQualityWarnings`, but internal captures (selector
327+
* resolution, settle observation, system-modal probes) can arm the runner-side penalty
328+
* without any user-facing render — when the latch is not held, the first public
329+
* deferred verdict re-renders the warning once. Managed only through
330+
* `src/daemon/snapshot-quality-latch.ts`: a genuine recovered render sets it, a healthy
331+
* public verdict clears it, and an app switch supersedes it.
332+
*/
333+
recoveredSnapshotWarningLatch?: { appBundleId?: string };
322334
/** Source snapshot used to resolve repeated `snapshot -s @ref` after scoped output replaces refs. */
323335
snapshotScopeSource?: SnapshotState;
324336
/**

0 commit comments

Comments
 (0)