Skip to content

Commit 9057552

Browse files
thymikeethymikee
committed
refactor(replay): dedupe post-dispatch identity-mismatch shaping, trim evidence-writer complexity
Shared buildPostDispatchIdentityMismatchResponse behind the guard and wait-landmark conversions; extracted payload-ceiling helpers from computeTargetEvidence; identity-refusal conversion split out of resolveReplayStepResponse. Docs: ADR 0012 decision 3 amendment (#1349), ADR 0016 guard strengthening, help workflow/save-script text.
1 parent f666bbc commit 9057552

9 files changed

Lines changed: 256 additions & 226 deletions

File tree

‎docs/adr/0012-interactive-replay.md‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ Accepted (2026-07-10); partially implemented (last updated 2026-07-16). See [Mig
3838
- Decision 6 amendment, repair-segment default exclusion of observation-only commands and the `--record`
3939
opt-in — stage 1 (interim guidance only, #1287) and stage 2 (the semantic default-exclusion fix,
4040
this amendment) of #1271. See the amendment under decision 6 below.
41+
- Decision 3 amendment (#1349), read-only step identity — landmark-mode `target-v1` evidence and
42+
post-resolution (in-loop) verification for selector `wait`, `get`-pattern coverage for `is` (except
43+
`exists`), the `targetIdentityVerification` descriptor trait pinning the evidence-carrying command
44+
set, explicit deferral of read-only `find`/`is exists`/non-selector waits, and ADR 0016's destination
45+
guard strengthened to require a `verified` landmark annotation, with a provider-level
46+
reshuffled-screen false-pass regression. See the amendment under decision 3 below.
4147

4248
**Accepted but NOT yet implemented** (this amendment; tracked by #1235 — repair-transaction lifecycle):
4349
the R7 repair-transaction keep-alive and its distinct `resume.repairSessionHeld` signal, the ARMED →
@@ -370,6 +376,52 @@ A recorded `id` never matches a node without that id.
370376
> platform-agnostic: an RN `FlatList` row (`Cell`) that is label-less with a labeled `Text` child hits
371377
> the same substitution on iOS.
372378
379+
> **Amendment (#1349): read-only step identity — `wait` landmark verification, `is` coverage, and the
380+
> per-command verification-phase trait.** Decision 3's evidence and machinery extend to the eligible
381+
> read-only steps without changing their execution semantics. The annotation format is **unchanged**
382+
> (`target-v1`, same tuple, same bounds, same writer/parser); what is new is a per-command dispatch of
383+
> WHEN verification runs, declared on the central `CommandDescriptor` as
384+
> `targetIdentityVerification: 'pre-dispatch' | 'post-resolution'` and pinned by a completeness test —
385+
> a new evidence-carrying command must choose its phase explicitly rather than silently entering the
386+
> generic pre-dispatch path.
387+
>
388+
> - **`wait <selector>` — covered, `post-resolution`, landmark (existence) semantics.** A wait's
389+
> landmark may legitimately be absent when its step starts, so the generic pre-action verification
390+
> (paths 2-6) must never run for it; only path 1 (recorded-`unverifiable`, which consults no screen)
391+
> refuses up front. The recorded annotation instead travels into the wait's own polling loop
392+
> (`internal.replayLandmarkGuard`, the same daemon-only channel as `replayTargetGuard`), which
393+
> strengthens the success condition from "the selector matches" to "a selector match carries the
394+
> recorded identity" — the identity tier only (local identity + leaf-anchored ancestry prefix). The
395+
> positional disambiguation signals are deliberately **not** compared: a destination guard proves the
396+
> landmark exists on the ready screen, not that it kept its list position, and requiring position
397+
> would produce the inverse false-failure on a legitimately reshuffled-but-correct screen. Polling is
398+
> preserved fail-open in time and fail-closed at the deadline: a same-selector impostor never aborts
399+
> the wait (a transient look-alike mid-transition is exactly what a wait exists to wait through); at
400+
> the deadline, a poll history containing selector matches that never carried the identity surfaces
401+
> as an `identity-mismatch` divergence (`matchCount` from the last matching poll, observed identity +
402+
> first ancestry mismatch in `mismatches`) BEFORE the wait reports success, while a selector that
403+
> never matched at all remains the ordinary wait timeout (`action-failure` — that failure needs a
404+
> state repair, not an identity repair, and its `repairHint` routing is already correct).
405+
> Record time writes evidence in a **landmark mode**: the step-5 self-check is identity-set
406+
> MEMBERSHIP rather than isolation (mirroring what replay will actually verify), and a matched node
407+
> with no id and no label after #1269 demotion records **no annotation** — a role-only landmark
408+
> identity is near-vacuous, and an unannotated wait keeps its existing selector-existence semantics
409+
> instead of failing closed on evidence that never discriminated anything. ADR 0016's destination
410+
> guard consumes exactly this: a qualifying guard is a selector wait with a `verified` annotation.
411+
> - **`get` — unchanged**; already covered by the pre-dispatch path and the post-resolution guard.
412+
> - **`is` (all predicates except `exists`) — covered, `pre-dispatch`, the `get` pattern end-to-end.**
413+
> `is` resolves a unique node immediately, so pre-action verification is semantically valid; the
414+
> resolved node/tree feed record-time evidence, and dispatch threads `replayTargetGuard` into
415+
> `assertExpectedResolvedTarget` exactly like `get`. The direct-iOS `is`/`wait` fast paths are gated
416+
> off during recording and guarded replays, mirroring `get`'s existing recording gate.
417+
> - **Intentionally deferred, with tests proving no annotation is recorded and no identity check runs:**
418+
> `is exists` (existence assertion with no unique winner; wait-like semantics without the
419+
> guard-critical role), every read-only `find` variant (fuzzy-locator resolution has no
420+
> selector-chain identity token for the classifier, and publication already refuses mutating `find`
421+
> as non-verifiable), and `wait text`/`wait stable`/duration waits/`wait @ref` (no element target, or
422+
> a session-local ref that ADR 0016 already refuses to publish; `wait @ref` is rejected rather than
423+
> converted to a portable selector).
424+
373425
**Ancestry.** The chain is the nearest **K = 8** ancestors of the target, ordered **leaf→root** (nearest
374426
ancestor first), each entry `{ role, label? }` under the same normalization (`role` may be the empty
375427
string when the node has no type; `label` is omitted when empty). Truncation drops entries from the
@@ -1254,6 +1306,7 @@ not restate or amend the decisions.
12541306
| 8. Agent-supervised re-record repair, base | 6 (R1-R6) | Shipped | #1228 |
12551307
| 9. Repair-transaction lifecycle | 6 (R7) | Shipped | #1235 |
12561308
| 10. Repair-segment default exclusion + `--record` | 6 amendment | Shipped | #1271 stage 2 |
1309+
| 11. Read-only step identity (wait landmark verification, `is` coverage) | 3 amendment (#1349) | Shipped | — |
12571310

12581311
Step 4 (#1209) added the `selector-miss`/`identity-mismatch`/`identity-unverifiable` divergence kinds
12591312
and a post-resolution target guard that cross-checks the dispatched winner against the verified member.

‎docs/adr/0016-active-session-script-publication.md‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,27 @@ the serialized guard before filesystem work and never relies on repair-only bare
8787
`session save-script` refuses publication without this **destination guard** and tells the author to
8888
record one. V1 does not infer a screen identity from a snapshot or synthesize an implicit guard.
8989

90-
The initial guard is selector-level. `wait` does not yet carry recorded-landmark identity through its
91-
polling resolution, so the guard proves that an element matching its selector exists, not that it is the
92-
same landmark element observed while authoring. Authors must choose a selective destination-specific
93-
landmark; a reshuffled screen containing the same weak label elsewhere can false-pass.
94-
[#1349](https://github.com/callstack/agent-device/issues/1349) owns the identity design for waits and
95-
remaining read-only steps. It must preserve polling: ADR 0012's current pre-action `target-v1`
90+
> **Amendment (#1349, shipped).** The guard is now identity-level, not merely selector-level. A
91+
> selector wait recorded while ARMED captures landmark-mode `target-v1` evidence for the matched
92+
> element (ADR 0012's read-only-step amendment), and a qualifying destination guard is exactly a
93+
> selector wait whose annotation is `verification: "verified"` — a selector wait on an identity-empty
94+
> element (no id and no label) records no annotation and does not qualify, so publication refuses it
95+
> with a recovery hint naming a labeled/id-bearing landmark. On replay, polling is preserved: the wait
96+
> keeps polling until a selector match carries the recorded identity (local identity + leaf-anchored
97+
> ancestry prefix), and a deadline reached with only same-selector impostors fails closed as an
98+
> `identity-mismatch` `REPLAY_DIVERGENCE` before the wait reports success. The reshuffled-screen
99+
> false-pass below is covered by a provider-scenario regression (record → publish → replay against a
100+
> reshuffled tree whose same-label node sits under a different id/ancestry).
101+
102+
The original selector-level caveat, retained as context: the guard proved that an element matching its
103+
selector exists, not that it is the same landmark element observed while authoring, so a reshuffled
104+
screen containing the same weak label elsewhere could false-pass.
105+
[#1349](https://github.com/callstack/agent-device/issues/1349) owned the identity design for waits and
106+
remaining read-only steps. It preserves polling: ADR 0012's pre-action `target-v1`
96107
verification cannot be attached to a wait unchanged because a not-yet-present landmark is the expected
97-
starting condition. Any identity check for a wait happens after its selector resolves, before the wait
98-
reports success, or uses a distinct guard-specific mechanism.
108+
starting condition, so the identity check runs after the wait's selector resolves and before the wait
109+
reports success (see ADR 0012's #1349 amendment for the mechanism and the read-only-step coverage
110+
classification).
99111

100112
The phrase "last mutating action" is derived from a request-sensitive recording-effect trait on the
101113
central `CommandDescriptor`, required for every command that records session actions and guarded by a
@@ -203,7 +215,9 @@ executing that script, not the artifact being saved.
203215
- The artifact replays from a cold start, completes its destination guard, returns the live session id,
204216
and accepts a subsequent command on that session.
205217
- Every action in ADR 0012's existing target-binding command set has canonical identity evidence and no
206-
unresolved `@ref` reaches disk; the destination guard remains selector-level until #1349 lands.
218+
unresolved `@ref` reaches disk; since #1349 the destination guard additionally requires verified
219+
recorded landmark identity, with a reshuffled-screen false-pass regression proving replay fails
220+
closed.
207221
- Existing-target refusal preserves the original bytes; `--force` replaces atomically; a failed publish
208222
remains retryable.
209223
- After PUBLISHED, later ordinary actions remain usable, repeated `session save-script` fails, plain

‎src/cli/parser/cli-help.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ Reusable open-to-destination scripts:
234234
agent-device press 'id="continue"' --settle
235235
agent-device wait 'role="heading" label="Screen X"'
236236
agent-device session save-script
237-
session save-script [path] [--force] publishes the sole recorded open through the destination guard, omits close, and leaves the session active. A duration wait, wait stable, or wait @ref is not a destination guard. A second successful open aborts publication; start a fresh session to author again.
237+
session save-script [path] [--force] publishes the sole recorded open through the destination guard, omits close, and leaves the session active. The guard is a selector wait on a labeled or id-bearing landmark: its recorded identity is captured while armed, and replay verifies that identity after the wait's selector resolves, so a reshuffled screen with the same label elsewhere fails closed instead of false-passing. A duration wait, wait stable, wait @ref, or a selector wait on an unlabeled element is not a destination guard. A second successful open aborts publication; start a fresh session to author again.
238238
Recorded fill/type inputs are written literally to the .ad file. Do not record passwords, tokens, or other secrets; use pre-authenticated test state or non-secret fixture credentials until parameterized input authoring is available.
239239
240240
Snapshots and refs:

‎src/commands/interaction/runtime/selector-read.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,14 @@ import {
1010
} from '../../../runtime.ts';
1111
import { ref, selector } from './selector-read.ts';
1212
import { makeSnapshotState } from '../../../__tests__/test-utils/index.ts';
13-
import { createSelectorDevice, selectorReadSnapshot } from './__tests__/test-utils/index.ts';
13+
import {
14+
createFakeClock,
15+
createSelectorDevice,
16+
selectorReadSnapshot,
17+
} from './__tests__/test-utils/index.ts';
18+
import { computeTargetEvidence } from '../../../daemon/session-target-evidence.ts';
19+
import { WAIT_LANDMARK_MISMATCH_REASON } from '../../../replay/target-identity-node.ts';
20+
import { AppError } from '../../../kernel/errors.ts';
1421

1522
test('runtime get reads text from a selector target', async () => {
1623
const snapshot = selectorReadSnapshot();
@@ -493,11 +500,6 @@ test('runtime selector convenience methods use explicit target helpers', async (
493500
// rejected candidates into the fail-closed landmark refusal.
494501
// ---------------------------------------------------------------------------
495502

496-
import { computeTargetEvidence } from '../../../daemon/session-target-evidence.ts';
497-
import { WAIT_LANDMARK_MISMATCH_REASON } from '../../../replay/target-identity-node.ts';
498-
import { createFakeClock } from './__tests__/test-utils/index.ts';
499-
import { AppError } from '../../../kernel/errors.ts';
500-
501503
function landmarkScreen(parentLabel: string) {
502504
return makeSnapshotState([
503505
{ index: 0, depth: 0, type: 'Other', label: parentLabel },

‎src/core/command-descriptor/__tests__/parity.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
listMcpExposedCommandNames,
2222
resolveCommandRecordsSessionAction,
2323
resolveCommandRecordingEffect,
24+
resolveTargetIdentityVerification,
2425
RAW_COMMAND_DESCRIPTORS,
2526
} from '../registry.ts';
2627

@@ -390,17 +391,16 @@ test('recordingEffect resolves request-sensitive observation and mutation subcom
390391
});
391392

392393
test('targetIdentityVerification pins exactly the evidence-carrying command set (ADR 0012 / #1349)', () => {
393-
const declared = new Map(
394-
RAW_COMMAND_DESCRIPTORS.filter((descriptor) => descriptor.targetIdentityVerification).map(
395-
(descriptor) => [descriptor.name, descriptor.targetIdentityVerification],
396-
),
397-
);
394+
const declared = RAW_COMMAND_DESCRIPTORS.flatMap((descriptor) => {
395+
const phase = resolveTargetIdentityVerification(descriptor.name);
396+
return phase ? [[descriptor.name, phase] as const] : [];
397+
});
398398
// A new evidence-carrying command must choose its replay verification phase
399399
// here explicitly instead of silently entering the generic pre-dispatch
400400
// path — wait is the only command whose target may legitimately be absent
401401
// when its step starts.
402402
assert.deepEqual(
403-
[...declared.entries()].sort(([a], [b]) => a.localeCompare(b)),
403+
[...declared].sort(([a], [b]) => a.localeCompare(b)),
404404
[
405405
['click', 'pre-dispatch'],
406406
['fill', 'pre-dispatch'],

‎src/daemon/handlers/session-replay-runtime.ts‎

Lines changed: 59 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { SessionStore } from '../session-store.ts';
1818
import { expandSessionPath } from '../session-paths.ts';
1919
import { applySaveScriptRetarget } from '../session-action-recorder.ts';
2020
import { computeReplayPlanDigest } from '../../replay/plan-digest.ts';
21+
import type { TargetAnnotationV1 } from '../../replay/target-identity.ts';
2122
import { errorResponse, noActiveSessionError } from './response.ts';
2223
import { invokeReplayAction } from './session-replay-action-runtime.ts';
2324
import { tryParseSelectorChain } from '../../selectors/index.ts';
@@ -50,6 +51,7 @@ import {
5051
isReplayTargetGuardMismatchResponse,
5152
isWaitLandmarkMismatchResponse,
5253
verifyReplayActionTarget,
54+
type ReplayVerifiedTargetGuard,
5355
} from './session-replay-target-verification.ts';
5456
import { buildReplayBuiltinVars } from './session-replay-vars.ts';
5557
import {
@@ -109,17 +111,14 @@ async function resolveReplayStepResponse(
109111
if (!verification.verified) return verification.response;
110112
const guard = verification.guard;
111113
const deferredLandmark = verification.deferredLandmark;
112-
const guardedReq = guard
113-
? {
114-
...ctx.replayReq,
115-
internal: { ...ctx.replayReq.internal, replayTargetGuard: guard.expected },
116-
}
114+
const guardInternal = guard
115+
? { replayTargetGuard: guard.expected }
117116
: deferredLandmark
118-
? {
119-
...ctx.replayReq,
120-
internal: { ...ctx.replayReq.internal, replayLandmarkGuard: deferredLandmark },
121-
}
122-
: ctx.replayReq;
117+
? { replayLandmarkGuard: deferredLandmark }
118+
: undefined;
119+
const guardedReq = guardInternal
120+
? { ...ctx.replayReq, internal: { ...ctx.replayReq.internal, ...guardInternal } }
121+
: ctx.replayReq;
123122
const response = await invokeReplayAction({
124123
req: guardedReq,
125124
sessionName: ctx.sessionName,
@@ -132,42 +131,57 @@ async function resolveReplayStepResponse(
132131
tracePath: ctx.actionTracePath,
133132
invoke: ctx.invoke,
134133
});
135-
if (guard && isReplayTargetGuardMismatchResponse(response)) {
136-
return await buildReplayTargetGuardMismatchResponse({
137-
action,
138-
scope: ctx.scope,
139-
guard,
140-
failedResponse: response,
141-
sourcePath,
142-
sourceLine,
143-
replayPath: ctx.resolved,
144-
step: index + 1,
145-
sessionName: ctx.sessionName,
146-
sessionStore: ctx.sessionStore,
147-
logPath: ctx.logPath,
148-
artifactPaths,
149-
responseLevel: ctx.responseLevel,
150-
planActions: ctx.actions,
151-
planDigest: ctx.planDigest,
152-
});
134+
return await convertIdentityRefusalResponse({
135+
ctx,
136+
action,
137+
index,
138+
artifactPaths,
139+
sourcePath,
140+
sourceLine,
141+
response,
142+
guard,
143+
deferredLandmark,
144+
});
145+
}
146+
147+
/**
148+
* Converts a dispatch-time identity refusal — the post-resolution guard
149+
* mismatch, or wait's landmark timeout — into its identity-mismatch
150+
* divergence; every other response passes through unchanged.
151+
*/
152+
async function convertIdentityRefusalResponse(params: {
153+
ctx: ReplayStepContext;
154+
action: SessionAction;
155+
index: number;
156+
artifactPaths: string[];
157+
sourcePath: string;
158+
sourceLine: number;
159+
response: DaemonResponse;
160+
guard: ReplayVerifiedTargetGuard | undefined;
161+
deferredLandmark: TargetAnnotationV1 | undefined;
162+
}): Promise<DaemonResponse> {
163+
const { ctx, action, index, artifactPaths, sourcePath, sourceLine, response } = params;
164+
const mismatchParams = {
165+
action,
166+
scope: ctx.scope,
167+
failedResponse: response,
168+
sourcePath,
169+
sourceLine,
170+
replayPath: ctx.resolved,
171+
step: index + 1,
172+
sessionName: ctx.sessionName,
173+
sessionStore: ctx.sessionStore,
174+
logPath: ctx.logPath,
175+
artifactPaths,
176+
responseLevel: ctx.responseLevel,
177+
planActions: ctx.actions,
178+
planDigest: ctx.planDigest,
179+
};
180+
if (params.guard && isReplayTargetGuardMismatchResponse(response)) {
181+
return await buildReplayTargetGuardMismatchResponse({ ...mismatchParams, guard: params.guard });
153182
}
154-
if (deferredLandmark && isWaitLandmarkMismatchResponse(response)) {
155-
return await buildWaitLandmarkMismatchResponse({
156-
action,
157-
scope: ctx.scope,
158-
failedResponse: response,
159-
sourcePath,
160-
sourceLine,
161-
replayPath: ctx.resolved,
162-
step: index + 1,
163-
sessionName: ctx.sessionName,
164-
sessionStore: ctx.sessionStore,
165-
logPath: ctx.logPath,
166-
artifactPaths,
167-
responseLevel: ctx.responseLevel,
168-
planActions: ctx.actions,
169-
planDigest: ctx.planDigest,
170-
});
183+
if (params.deferredLandmark && isWaitLandmarkMismatchResponse(response)) {
184+
return await buildWaitLandmarkMismatchResponse(mismatchParams);
171185
}
172186
return response;
173187
}

0 commit comments

Comments
 (0)