fix(snapshot): latch on the captured verdict, not the retained session snapshot

Review P2 on #1590: the latch seam read a diff capture's verdict back from
session.snapshot, but an empty ref-scoped capture deliberately retains the
previous stored snapshot (shouldKeepCurrentSnapshot) — so a deferred
capture could consult a retained healthy verdict, clearing the latch and
omitting the one-shot warning.

The daemon snapshot backend now fills a per-request CapturedSnapshotQuality
slot on every capture, and the seam latches on that just-captured verdict
for both snapshot and diff. New production-path regression: an empty
ref-scoped diff over a retained healthy snapshot with a deferred capture
warns once (verified red against the previous seam).

Author: thymikee

src/daemon/__tests__/snapshot-quality-latch.test.ts

@@ -11,7 +11,7 @@ import {
   applyRecoveredWarningLatch,
   resolveRecoveredWarningLatch,
 } from '../snapshot-quality-latch.ts';
-import { dispatchSnapshotViaRuntime } from '../snapshot-runtime.ts';
+import { dispatchSnapshotDiffViaRuntime, dispatchSnapshotViaRuntime } from '../snapshot-runtime.ts';
 import { SessionStore } from '../session-store.ts';
 import type { SessionState } from '../types.ts';
 
@@ -121,7 +121,8 @@ test('internal observation responses neither consume nor clear the latch', () =>
 
   const internal = applyRecoveredWarningLatch({
     session,
-    data: { snapshotQuality: deferredVerdict() },
+    data: {},
+    verdict: deferredVerdict(),
     internalObservation: true,
   });
   expect(internal.warnings).toBeUndefined();
@@ -130,17 +131,23 @@ test('internal observation responses neither consume nor clear the latch', () =>
   session.recoveredSnapshotWarningLatch = { appBundleId: 'com.example.app' };
   applyRecoveredWarningLatch({
     session,
-    data: { snapshotQuality: { state: 'healthy', backend: 'tree' } },
+    data: {},
+    verdict: { state: 'healthy', backend: 'tree' },
     internalObservation: true,
   });
   expect(session.recoveredSnapshotWarningLatch).toEqual({ appBundleId: 'com.example.app' });
 });
 
 test('sessionless responses pass through unchanged', () => {
-  const data = { snapshotQuality: deferredVerdict() };
-  expect(applyRecoveredWarningLatch({ session: undefined, data, internalObservation: false })).toBe(
-    data,
-  );
+  const data = {};
+  expect(
+    applyRecoveredWarningLatch({
+      session: undefined,
+      data,
+      verdict: deferredVerdict(),
+      internalObservation: false,
+    }),
+  ).toBe(data);
 });
 
 function scenario() {
@@ -152,7 +159,7 @@ function scenario() {
   return { sessionStore, sessionName, logPath: path.join(root, 'daemon.log') };
 }
 
-function seedCapture(verdict: SnapshotQualityVerdict) {
+function seedCapture(verdict: SnapshotQualityVerdict, label = 'Continue') {
   dispatchCommandMock.mockResolvedValue({
     backend: 'xctest',
     truncated: false,
@@ -162,7 +169,7 @@ function seedCapture(verdict: SnapshotQualityVerdict) {
         index: 0,
         depth: 0,
         type: 'Button',
-        label: 'Continue',
+        label,
         rect: { x: 0, y: 0, width: 100, height: 44 },
         hittable: true,
       },
@@ -236,6 +243,56 @@ test('a healthy public capture re-arms the one-shot warning', async () => {
   expect(responseWarnings(rearmed).filter((line) => line === FULL_WARNING)).toHaveLength(1);
 });
 
+test('an empty ref-scoped diff latches on the captured verdict, not the retained snapshot', async () => {
+  const input = scenario();
+  // The stored snapshot is healthy and carries the ref the diff will scope to;
+  // the fresh capture is deferred and contains no node matching that scope, so
+  // the empty scoped result deliberately retains the stored snapshot
+  // (`shouldKeepCurrentSnapshot`) — a seam reading the verdict back from the
+  // session would consult the retained healthy verdict and omit the warning.
+  const session = input.sessionStore.get(input.sessionName)!;
+  session.snapshot = {
+    createdAt: Date.now(),
+    snapshotQuality: { state: 'healthy', backend: 'tree' },
+    nodes: [
+      {
+        index: 0,
+        depth: 0,
+        type: 'Button',
+        ref: 'e1',
+        label: 'Continue',
+        rect: { x: 0, y: 0, width: 100, height: 44 },
+        hittable: true,
+      },
+    ],
+  };
+  // The deferred capture holds no node labeled 'Continue', so the '@e1' scope
+  // resolves to zero nodes and the retention path runs.
+  seedCapture(deferredVerdict(), 'Something else');
+
+  const diff = await dispatchSnapshotDiffViaRuntime({
+    req: {
+      command: 'diff',
+      positionals: [],
+      token: 't',
+      session: input.sessionName,
+      flags: { snapshotScope: '@e1' },
+    },
+    sessionName: input.sessionName,
+    logPath: input.logPath,
+    sessionStore: input.sessionStore,
+  });
+
+  // The retention actually happened: the stored snapshot (and its healthy
+  // verdict) survived the empty scoped capture.
+  const retained = input.sessionStore.get(input.sessionName)?.snapshot;
+  expect(retained?.nodes[0]?.label).toBe('Continue');
+  expect(retained?.snapshotQuality?.state).toBe('healthy');
+
+  expect(responseWarnings(diff).filter((line) => line === FULL_WARNING)).toHaveLength(1);
+  expect(storedLatch(input)).toEqual({ appBundleId: 'com.example.app' });
+});
+
 test('a genuine recovered render keeps later deferred captures quiet without doubling', async () => {
   const input = scenario();
   seedCapture(genuineRecoveredVerdict());

src/daemon/snapshot-quality-latch.ts

@@ -1,8 +1,5 @@
 import type { SnapshotQualityVerdict } from '@agent-device/kernel/snapshot';
-import {
-  readSnapshotQualityVerdict,
-  recoveredSnapshotQualityWarning,
-} from '../snapshot/snapshot-quality.ts';
+import { recoveredSnapshotQualityWarning } from '../snapshot/snapshot-quality.ts';
 import type { DaemonResponseData, SessionState } from './types.ts';
 
 type RecoveredWarningLatch = NonNullable<SessionState['recoveredSnapshotWarningLatch']>;
@@ -55,6 +52,16 @@ export function resolveRecoveredWarningLatch(params: {
   return { warning: recoveredSnapshotQualityWarning(verdict.backend), latch: { appBundleId } };
 }
 
+/**
+ * The verdict slot the daemon snapshot backend fills on every capture, so the
+ * latch seam sees the verdict of THE capture that produced this response.
+ * Reading it back from `session.snapshot` instead would be wrong: an empty
+ * ref-scoped capture deliberately retains the previous stored snapshot
+ * (`shouldKeepCurrentSnapshot`), so a deferred capture could consult a retained
+ * healthy verdict — clearing the latch and omitting the one-shot warning.
+ */
+export type CapturedSnapshotQuality = { value?: SnapshotQualityVerdict };
+
 /**
  * Applies the latch to a user-facing snapshot/diff response: updates the
  * session's latch state and prepends the full recovered warning when this is
@@ -65,15 +72,12 @@ export function resolveRecoveredWarningLatch(params: {
 export function applyRecoveredWarningLatch(params: {
   session: SessionState | undefined;
   data: DaemonResponseData;
+  /** The just-captured verdict (`CapturedSnapshotQuality`), never a stored one. */
+  verdict: SnapshotQualityVerdict | undefined;
   internalObservation: boolean;
 }): DaemonResponseData {
-  const { session, data, internalObservation } = params;
+  const { session, data, verdict, internalObservation } = params;
   if (internalObservation || !session) return data;
-  // The snapshot command carries the capture's verdict in the response; diff
-  // publishes only warnings, so its verdict is read from the session snapshot
-  // the capture just stored.
-  const verdict =
-    readSnapshotQualityVerdict(data.snapshotQuality) ?? session.snapshot?.snapshotQuality;
   const decision = resolveRecoveredWarningLatch({
     verdict,
     appBundleId: session.appBundleId,

src/daemon/snapshot-runtime.ts

@@ -18,7 +18,10 @@ import {
   withSessionlessRunnerCleanup,
 } from './handlers/snapshot-session.ts';
 import { activateCompleteRefFrame } from './ref-frame.ts';
-import { applyRecoveredWarningLatch } from './snapshot-quality-latch.ts';
+import {
+  applyRecoveredWarningLatch,
+  type CapturedSnapshotQuality,
+} from './snapshot-quality-latch.ts';
 import { createDaemonRuntimePolicy } from './runtime-policy.ts';
 import { createDaemonRuntimeSessionStore } from './runtime-session.ts';
 import { isInteractiveObservation } from './session-action-recorder.ts';
@@ -145,6 +148,7 @@ async function dispatchSnapshotRuntimeCommand(
   if (iosAppSessionGuard) return iosAppSessionGuard;
 
   return await withSessionlessRunnerCleanup(session, device, async () => {
+    const capturedQuality: CapturedSnapshotQuality = {};
     const runtime = createSnapshotRuntime({
       req,
       sessionName,
@@ -153,6 +157,7 @@ async function dispatchSnapshotRuntimeCommand(
       session,
       device,
       snapshotScope: resolvedScope.scope,
+      capturedQuality,
     });
     let result: Awaited<ReturnType<SnapshotRuntimeCommandParams['execute']>>;
     try {
@@ -184,6 +189,7 @@ async function dispatchSnapshotRuntimeCommand(
       data: applyRecoveredWarningLatch({
         session: sessionStore.get(sessionName),
         data: result.data,
+        verdict: capturedQuality.value,
         internalObservation: req.internal?.observationOnly === true,
       }),
     };
@@ -212,6 +218,7 @@ function createSnapshotRuntime(params: {
   session: SessionState | undefined;
   device: SessionState['device'];
   snapshotScope: string | undefined;
+  capturedQuality: CapturedSnapshotQuality;
 }) {
   const { req, sessionName, logPath, sessionStore, session, device, snapshotScope } = params;
   return createAgentDevice({
@@ -221,6 +228,7 @@ function createSnapshotRuntime(params: {
       session,
       device,
       snapshotScope,
+      capturedQuality: params.capturedQuality,
     }),
     ...createDaemonRuntimePolicy('snapshot'),
     sessions: createDaemonRuntimeSessionStore({
@@ -328,6 +336,7 @@ function createDaemonSnapshotBackend(params: {
   session: SessionState | undefined;
   device: SessionState['device'];
   snapshotScope: string | undefined;
+  capturedQuality: CapturedSnapshotQuality;
 }): AgentDeviceBackend {
   const { req, logPath, session, device, snapshotScope } = params;
   return {
@@ -341,10 +350,15 @@ function createDaemonSnapshotBackend(params: {
         logPath,
         snapshotScope,
       });
+      const annotations = snapshotCaptureAnnotationsFrom(capture);
+      // Feed the latch seam the capture's own verdict: the stored session
+      // snapshot is not a substitute (an empty ref-scoped capture retains the
+      // previous snapshot, and diff never publishes the verdict).
+      params.capturedQuality.value = annotations.quality;
       const snapshotDiagnostics = summarizeSnapshotDiagnostics(session);
       return {
         snapshot: capture.snapshot,
-        ...snapshotCaptureAnnotationsFrom(capture),
+        ...annotations,
         ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}),
         appName: session?.appBundleId ? (session.appName ?? session.appBundleId) : undefined,
         appBundleId: session?.appBundleId,