Skip to content

Commit c6acd1e

Browse files
committed
fix(daemon): print ambiguity candidates only when they can be issued (ADR 0014)
The review's P1: updateSessionSnapshot deliberately skips storing a sparse-quality capture, so candidates minted from that tree were issued against the PREVIOUS stored tree's generation -- the same wrong-node bind the issuance rule exists to prevent, arriving through the sparse path. issueSettleRefs already carries the opposite ruling beside the same primitive: what was not stored issues nothing. The second implementation of the rule had dropped that guard. Printing and issuing become one decision at the rule, keyed on the typed fact the surfaces already read: the candidate bodies may only be authorized when the capture the failing request consumed IS the tree the session stores under the generation being frozen (node identity -- the command layer stores an annotation copy of the same array). A sparse/unstored capture, a retired or sessionless ref, or a session with no generation now degrades to the count-only refusal: truthful matches count, no candidate refs, and a hint that cannot advertise an affordance the frame could not honor. An advertised-but-unusable ref is worse than no list. Both consumer seams hand the rule the capture their request consumed: the selector routes pass the capture runtime's consumedSnapshot slot through toDaemonResponse's issuance parameter, and the touch runtime owns a per-dispatch consumedCapture slot filled where its own captures land. Runner-native refusals (direct iOS) carry no candidate shape and pass through untouched. The unit door previously over-claimed a sessionless assertion it never made; it now pins retired-ref, sessionless, no-generation, and unstored-capture refusals separately, and the reviewer's end-to-end sequence gains its sparse twin. commands.md scopes the predicate claim to the uniqueness rows (exists/absent never resolve one element) and documents the count-only form.
1 parent ded07c0 commit c6acd1e

7 files changed

Lines changed: 270 additions & 33 deletions

File tree

‎src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,3 +174,62 @@ test('a plain candidate ref from the ambiguity refusal is refused, not silently
174174
}
175175
expect(mockTapPoint).not.toHaveBeenCalled();
176176
});
177+
178+
/**
179+
* The P1 the review caught (cubic + coordinator): the capture runtime
180+
* DELIBERATELY does not store a sparse-quality capture
181+
* (`selector-capture-runtime.ts` `updateSessionSnapshot`: sparse verdicts skip
182+
* the store, exactly as `issueSettleRefs` documents for the settle path). So
183+
* candidates minted from that never-stored tree cannot be issued against the
184+
* PREVIOUS stored tree's generation — a printed `@ref` would resolve against
185+
* the older tree, the same wrong-node bind this rule exists to prevent.
186+
* Printing and issuing are ONE decision: when the consumed capture was not
187+
* stored, the refusal keeps its truthful count and prints no candidate refs.
188+
*/
189+
test('an ambiguous is on a sparse capture prints no candidates and issues nothing', async () => {
190+
const sessionStore = makeSessionStore();
191+
const sessionName = 'is-ambiguity-sparse';
192+
// A stored frame from an earlier command, as in the reviewer's sequence.
193+
const session = makeStaleRefSession(sessionName);
194+
session.device = IOS_SIMULATOR;
195+
sessionStore.publish(sessionName, session);
196+
const storedNodes = session.snapshot!.nodes;
197+
198+
// The ambiguous two-button tree, but delivered with a SPARSE quality
199+
// verdict: the capture runtime refuses to store it, so the session keeps
200+
// describing the older tree. `is` itself does not gate on the verdict; the
201+
// readUnique door is reached on this tree. (The verdict's `backend` is the
202+
// acquisition backend vocabulary, not the producer.)
203+
const sparseCapture = () => ({
204+
...ambiguousFullCapture(),
205+
quality: {
206+
state: 'sparse' as const,
207+
backend: 'tree' as const,
208+
reasonCode: 'sparse-tree' as const,
209+
},
210+
});
211+
const fixture = selectorCaptureFixture({ snapshot: sparseCapture });
212+
const response = await dispatchIsViaRuntime({
213+
req: isRequest(sessionName, ['visible', 'label="Deploy"']),
214+
sessionName,
215+
sessionStore,
216+
inspectFacts: fixture.inspectFacts,
217+
bindDevice: fixture.bindDevice,
218+
});
219+
220+
expect(response?.ok).toBe(false);
221+
if (!response || response.ok) throw new Error('expected the ambiguity refusal');
222+
// Still the truth: two nodes matched and the read refuses to choose.
223+
expect(response.error.code).toBe('AMBIGUOUS_MATCH');
224+
expect(response.error.details?.matches).toBe(2);
225+
// But nothing issuable, so nothing printable: no candidate refs, no epoch,
226+
// and no hint advertising an affordance the frame cannot honor.
227+
expect(response.error.details?.candidates).toBeUndefined();
228+
expect(response.error.details?.refsGeneration).toBeUndefined();
229+
expect(String(response.error.details?.hint)).not.toMatch(/printed candidate/);
230+
// The store still describes the tree it did — the sparse capture replaced
231+
// nothing — and the pre-existing complete frame from the earlier snapshot
232+
// was left alone rather than reissued over a tree that was never stored.
233+
expect(session.snapshot!.nodes).toBe(storedNodes);
234+
expect(refFrameScope(session)).toBe('all');
235+
});

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

Lines changed: 108 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,14 @@ for (const retire of [false, true]) {
262262
}
263263

264264
// #2870 review: every response that prints candidate @refs must issue those
265-
// refs on the frame they came from. One rule, consumed by the acting refusal
265+
// refs on the frame they came from, and issuance is only valid against the
266+
// tree that generation describes. One rule, consumed by the acting refusal
266267
// (touch runtime) and the strict-read refusals (is / get attrs dispatch).
267268
test('ambiguous refusals issue their printed candidates as a partial frame', () => {
268269
const store = makeSessionStore();
269270
const session = makeSession();
270-
setSessionSnapshot(session, makeSnapshot());
271+
const captured = makeSnapshot();
272+
setSessionSnapshot(session, captured);
271273
const ref = store.publish('cwd:ambiguity:default', session);
272274
const published = publishAmbiguousMatchCandidateRefs(
273275
ref,
@@ -276,28 +278,78 @@ test('ambiguous refusals issue their printed candidates as a partial frame', ()
276278
matches: 4,
277279
candidates: ['@e2 [text] "Team Standup"', '@e5 [button] "Team Standup"'],
278280
}),
281+
// The consumed capture IS the stored tree: what the capture runtime does
282+
// for every non-sparse read, node identity included.
283+
captured,
279284
);
280285
expect(published.details?.refsGeneration).toBe(session.snapshotGeneration);
281286
expect(refFrameScope(session)).toEqual(new Set(['e2', 'e5']));
282287
});
283288

284-
test('a non-ambiguity read refusal issues nothing', () => {
289+
/**
290+
* The P1 the review caught: the capture runtime DELIBERATELY does not store a
291+
* sparse-quality capture (`updateSessionSnapshot` skips it, and `issueSettleRefs`
292+
* documents the same ruling for the settle path), so candidates minted from
293+
* that tree cannot be issued against the PREVIOUS stored tree's generation —
294+
* a printed @ref would resolve to a different node. Printing and issuing are
295+
* one decision: no stored tree, no printed candidates, and the hint drops the
296+
* "act on a printed candidate" route it could not honor.
297+
*/
298+
test('a sparse capture that was never stored issues nothing and prints no candidates', () => {
285299
const store = makeSessionStore();
286300
const session = makeSession();
287301
setSessionSnapshot(session, makeSnapshot());
302+
const ref = store.publish('cwd:ambiguity-sparse:default', session);
303+
const storedNodes = session.snapshot!.nodes;
304+
// The sparse capture the failing request consumed: a different tree, never
305+
// stored (that skip is the capture runtime's deliberate quality ruling).
306+
const sparseCapture: SnapshotState = {
307+
nodes: [],
308+
createdAt: Date.now(),
309+
backend: 'xctest',
310+
snapshotQuality: { state: 'sparse', backend: 'tree', reasonCode: 'sparse-tree' },
311+
};
312+
const published = publishAmbiguousMatchCandidateRefs(
313+
ref,
314+
store,
315+
new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', {
316+
matches: 2,
317+
candidates: ['@e2 [text] "a"', '@e3 [text] "b"'],
318+
hint: 'Narrow the selector with role/id/longer text, or act on a printed candidate with a command that takes refs, such as press.',
319+
}),
320+
sparseCapture,
321+
);
322+
expect(published.code).toBe('AMBIGUOUS_MATCH');
323+
expect(published.details?.refsGeneration).toBeUndefined();
324+
expect(published.details?.candidates).toBeUndefined();
325+
// The truthful count survives; the unusable affordance does not.
326+
expect(published.details?.matches).toBe(2);
327+
expect(String(published.details?.hint)).not.toMatch(/printed candidate/);
328+
// Nothing was issued: the frame stays untouched and the stored tree was
329+
// never replaced by the sparse capture.
330+
expect(session.refFrame).toBeUndefined();
331+
expect(session.snapshot!.nodes).toBe(storedNodes);
332+
});
333+
334+
test('a non-ambiguity read refusal issues nothing', () => {
335+
const store = makeSessionStore();
336+
const session = makeSession();
337+
const captured = makeSnapshot();
338+
setSessionSnapshot(session, captured);
288339
const ref = store.publish('cwd:ambiguity-none:default', session);
289340
const original = new AppError('COMMAND_FAILED', 'Selector did not match: label="X"');
290-
const returned = publishAmbiguousMatchCandidateRefs(ref, store, original);
341+
const returned = publishAmbiguousMatchCandidateRefs(ref, store, original, captured);
291342
expect(returned).toBe(original);
292343
// The ref-frame accessor defaults a never-issued frame to PRISTINE; the
293344
// raw slot staying unset is what "untouched" means here.
294345
expect(session.refFrame).toBeUndefined();
295346
});
296347

297-
test('ambiguity without a live session ref (retired or sessionless route) issues nothing', () => {
348+
test('ambiguity on a retired session ref issues nothing and prints no candidates', () => {
298349
const store = makeSessionStore();
299350
const session = makeSession();
300-
setSessionSnapshot(session, makeSnapshot());
351+
const captured = makeSnapshot();
352+
setSessionSnapshot(session, captured);
301353
const ref = store.publish('cwd:ambiguity-retired:default', session);
302354
store.retire(ref);
303355
const published = publishAmbiguousMatchCandidateRefs(
@@ -307,7 +359,57 @@ test('ambiguity without a live session ref (retired or sessionless route) issues
307359
matches: 2,
308360
candidates: ['@e2 [text] "a"', '@e3 [text] "b"'],
309361
}),
362+
captured,
363+
);
364+
expect(published.details?.refsGeneration).toBeUndefined();
365+
expect(published.details?.candidates).toBeUndefined();
366+
expect(published.details?.matches).toBe(2);
367+
expect(session.refFrame).toBeUndefined();
368+
});
369+
370+
/**
371+
* The sessionless shape `is` can travel (a selector route whose `lookup` found
372+
* no live ref): with no session there is no generation to freeze and no frame
373+
* to authorize, so the refusal must degrade to the count-only form too rather
374+
* than print refs nobody can act on.
375+
*/
376+
test('ambiguity without a session ref (sessionless route) issues nothing and prints no candidates', () => {
377+
const store = makeSessionStore();
378+
const published = publishAmbiguousMatchCandidateRefs(
379+
undefined,
380+
store,
381+
new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', {
382+
matches: 2,
383+
candidates: ['@e2 [text] "a"', '@e3 [text] "b"'],
384+
}),
385+
makeSnapshot(),
386+
);
387+
expect(published.code).toBe('AMBIGUOUS_MATCH');
388+
expect(published.details?.refsGeneration).toBeUndefined();
389+
expect(published.details?.candidates).toBeUndefined();
390+
expect(published.details?.matches).toBe(2);
391+
});
392+
393+
/**
394+
* A session whose lifetime never stored a tree has no generation to freeze
395+
* (both `setSessionSnapshot` and every capture runtime mint it on the first
396+
* store). Same rule as the sparse case: nothing issuable, nothing printable.
397+
*/
398+
test('ambiguity on a session with no stored snapshot issues nothing and prints no candidates', () => {
399+
const store = makeSessionStore();
400+
const session = makeSession();
401+
const ref = store.publish('cwd:ambiguity-nogen:default', session);
402+
const published = publishAmbiguousMatchCandidateRefs(
403+
ref,
404+
store,
405+
new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', {
406+
matches: 2,
407+
candidates: ['@e2 [text] "a"', '@e3 [text] "b"'],
408+
}),
409+
makeSnapshot(),
310410
);
311411
expect(published.details?.refsGeneration).toBeUndefined();
412+
expect(published.details?.candidates).toBeUndefined();
413+
expect(published.details?.matches).toBe(2);
312414
expect(session.refFrame).toBeUndefined();
313415
});

‎src/daemon/interaction/internal/interaction-runtime.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type {
88
} from '../../../backend.ts';
99
import { createCommandSurfaceAgentDevice } from '../../../command-runtime/runtime-command-surface.ts';
1010
import { getRequestSignal } from '@agent-device/host-kit/request';
11-
import type { Rect } from '@agent-device/kernel/snapshot';
11+
import type { Rect, SnapshotState } from '@agent-device/kernel/snapshot';
1212
import type { DaemonCommandContext } from '../../context.ts';
1313
import { createDaemonRuntimePolicy } from '../../runtime-policy.ts';
1414
import { buildAppleRunnerRequestOptions } from '../../apple-runner-options.ts';
@@ -40,6 +40,14 @@ export function createInteractionRuntimeForRoute(
4040
pairedGestureViewport?: Rect;
4141
touchExecutor?: BoundTouchExecutor;
4242
gestures?: BoundGestureExecutor;
43+
/**
44+
* Filled with the LAST capture this request consumed. ADR 0014 ambiguity
45+
* issuance is only valid against a tree the session stored under the
46+
* generation it freezes, and a sparse-quality capture deliberately stores
47+
* nothing — the refusal seam needs the same consumed-capture slot the
48+
* selector routes carry (`consumedSnapshot`), owned per dispatch.
49+
*/
50+
consumedCapture?: { state?: SnapshotState };
4351
},
4452
) {
4553
const ref = bindInteractionSession(params).sessionRef;
@@ -58,6 +66,7 @@ export function createInteractionRuntimeForRoute(
5866
params.contextFromFlags,
5967
options,
6068
);
69+
if (params.consumedCapture) params.consumedCapture.state = snapshot;
6170
return recordCaptureProof(params.captureProof, snapshot);
6271
},
6372
runtimeSessions: createDaemonRuntimeSessionStore({

‎src/daemon/interaction/internal/interaction-touch-runtime.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
ResolvedInteractionTarget,
88
} from '@agent-device/contracts/interaction';
99
import type { GestureReferenceFrame } from '@agent-device/contracts/scroll-gesture';
10+
import type { SnapshotState } from '@agent-device/kernel/snapshot';
1011
import { asAppError, normalizeError } from '@agent-device/kernel/errors';
1112
import { readResolvedInteractionTarget } from '../../../core/interaction-outcome.ts';
1213
import { publishAmbiguousMatchCandidateRefs } from '../../session-snapshot.ts';
@@ -70,9 +71,11 @@ export async function dispatchRuntimeInteraction<
7071
params = bindInteractionSession(params);
7172
if (!params.sessionRef) return noActiveSessionError();
7273
const session = params.sessionStore.requireCurrent(params.sessionRef);
74+
const consumedCapture: { state?: SnapshotState } = {};
7375
const runtime = createInteractionRuntimeForRoute({
7476
...params,
7577
touchExecutor: options.touchExecutor,
78+
consumedCapture,
7679
});
7780
const actionStartedAt = Date.now();
7881
try {
@@ -124,6 +127,7 @@ export async function dispatchRuntimeInteraction<
124127
params.sessionRef,
125128
params.sessionStore,
126129
asAppError(error),
130+
consumedCapture.state,
127131
);
128132
if (isAndroidEscapeError(appError)) throw appError;
129133
if (appError.code === 'AMBIGUOUS_MATCH') return appErrorResponse(appError);

‎src/daemon/selector-runtime.ts‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { asAppError } from '@agent-device/kernel/errors';
2-
import type { SnapshotNode } from '@agent-device/kernel/snapshot';
2+
import type { SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot';
33
import { absenceCaptureOptionError } from '@agent-device/selectors/absence-observation-errors';
44
import { absenceCaptureOptionRefusal } from '@agent-device/selectors/absence-observation';
55
import type { DaemonRequest, DaemonResponse } from './daemon-request.ts';
@@ -186,7 +186,11 @@ export async function dispatchGetViaRuntime(
186186
const data = toDaemonGetData(result);
187187
return staleRefsWarning ? { ...data, warning: staleRefsWarning } : data;
188188
},
189-
{ ref: resolvedRuntime.ref, sessionStore: params.sessionStore },
189+
{
190+
ref: resolvedRuntime.ref,
191+
sessionStore: params.sessionStore,
192+
consumed: params.consumedSnapshot,
193+
},
190194
);
191195
return withCaptureDisclosures({
192196
response,
@@ -255,7 +259,11 @@ export async function dispatchIsViaRuntime(
255259
);
256260
return stripSelectorChain(strippedResult);
257261
},
258-
{ ref: resolvedRuntime.ref, sessionStore: params.sessionStore },
262+
{
263+
ref: resolvedRuntime.ref,
264+
sessionStore: params.sessionStore,
265+
consumed: params.consumedSnapshot,
266+
},
259267
);
260268
return withCaptureDisclosures({
261269
response: await maybeAndroidForegroundBlockerResponse(params, response, `is ${predicate}`),
@@ -312,18 +320,30 @@ function parseGetTarget(req: DaemonRequest):
312320
* `get attrs`): ADR 0014's ambiguity-issuance rule then runs before flattening,
313321
* so a printed candidate is admitted on the frame that listed it — the same
314322
* contract the acting refusal gets through the touch runtime (#2870 review).
323+
* `consumed` is the capture runtime's slot for the tree THIS request consumed:
324+
* issuance is only valid against a capture the session actually stored under
325+
* that generation, and a sparse-quality capture deliberately stores nothing.
315326
* Routes that never print candidates (`wait`, read-only `find`) pass nothing
316327
* and keep the plain flatten.
317328
*/
318329
export async function toDaemonResponse(
319330
task: () => Promise<Record<string, unknown>>,
320-
issuance?: { ref: SessionRef | undefined; sessionStore: SessionStore },
331+
issuance?: {
332+
ref: SessionRef | undefined;
333+
sessionStore: SessionStore;
334+
consumed: { state?: SnapshotState } | undefined;
335+
},
321336
): Promise<DaemonResponse> {
322337
try {
323338
return { ok: true, data: await task() };
324339
} catch (error) {
325340
const appError = issuance
326-
? publishAmbiguousMatchCandidateRefs(issuance.ref, issuance.sessionStore, asAppError(error))
341+
? publishAmbiguousMatchCandidateRefs(
342+
issuance.ref,
343+
issuance.sessionStore,
344+
asAppError(error),
345+
issuance.consumed?.state,
346+
)
327347
: asAppError(error);
328348
return errorResponse(appError.code, appError.message, appError.details);
329349
}

0 commit comments

Comments
 (0)