Skip to content

Commit 972451e

Browse files
thymikeethymikee
authored
fix: fit target annotations after echo redaction (#4406)
* fix: fit target annotations after echo redaction * fix: enforce annotation ancestry limit when writing
1 parent 7887591 commit 972451e

6 files changed

Lines changed: 156 additions & 39 deletions

File tree

‎packages/ad-script/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export {
2222

2323
export {
2424
parseTargetAnnotationV1Payload,
25-
serializeTargetAnnotationV1,
25+
serializeRawTargetAnnotationV1,
2626
truncateToUtf8Bytes,
2727
utf8ByteLength,
2828
TARGET_ANNOTATION_MAX_ANCESTRY,

‎packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ test('serializeTargetAnnotationV1 uses the exact canonical field order from deci
5656
);
5757
});
5858

59+
test('writer degrades excessive ancestry for single and multi-target annotations', () => {
60+
const ancestry = Array.from({ length: TARGET_ANNOTATION_MAX_ANCESTRY + 1 }, () => ({
61+
role: 'group',
62+
}));
63+
const evidence = baseEvidence({ ancestry, rect: { x: 1, y: 2, width: 3, height: 4 } });
64+
const expected = { ...evidence, ancestry: [], verification: 'unverifiable' };
65+
delete expected.rect;
66+
delete expected.scrollRegion;
67+
assert.deepEqual(parseTargetAnnotationV1Payload(serializeTargetAnnotationV1(evidence)), expected);
68+
assert.deepEqual(
69+
parseMultiTargetAnnotationV1Payload(
70+
serializeMultiTargetAnnotationV1({ source: evidence, destination: evidence }),
71+
),
72+
{ source: expected, destination: expected },
73+
);
74+
assert.equal(evidence.ancestry.length, TARGET_ANNOTATION_MAX_ANCESTRY + 1);
75+
});
76+
5977
test('formatTargetAnnotationCommentLine emits the ASCII # agent-device:target-v1 prefix', () => {
6078
const line = formatTargetAnnotationCommentLine(baseEvidence());
6179
assert.ok(line.startsWith('# agent-device:target-v1 {'));

‎packages/ad-script/src/internal/target-annotation-serde.ts‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,18 +162,60 @@ function buildRectObject(rect: TargetRect): Record<string, unknown> {
162162
return { x: rect.x, y: rect.y, width: rect.width, height: rect.height };
163163
}
164164

165-
export function serializeTargetAnnotationV1(evidence: TargetAnnotationV1): string {
165+
export function serializeRawTargetAnnotationV1(evidence: TargetAnnotationV1): string {
166166
return JSON.stringify(buildCanonicalTargetAnnotationObject(evidence));
167167
}
168168

169+
function fitsTargetAnnotation(evidence: TargetAnnotationV1): boolean {
170+
const fields = [
171+
evidence.role,
172+
evidence.id,
173+
evidence.label,
174+
...evidence.ancestry.flatMap((entry) => [entry.role, entry.label]),
175+
evidence.scrollRegion?.role,
176+
evidence.scrollRegion?.id,
177+
evidence.scrollRegion?.label,
178+
];
179+
return (
180+
evidence.ancestry.length <= TARGET_ANNOTATION_MAX_ANCESTRY &&
181+
fields.every(
182+
(field) => field === undefined || utf8ByteLength(field) <= TARGET_ANNOTATION_MAX_FIELD_BYTES,
183+
) &&
184+
utf8ByteLength(serializeRawTargetAnnotationV1(evidence)) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES
185+
);
186+
}
187+
188+
function fitTargetAnnotation(evidence: TargetAnnotationV1): TargetAnnotationV1 {
189+
if (fitsTargetAnnotation(evidence)) return evidence;
190+
const candidate = { ...evidence, verification: 'unverifiable' as const };
191+
const degradations: readonly ((target: TargetAnnotationV1) => void)[] = [
192+
(target) => delete target.rect,
193+
(target) => delete target.scrollRegion,
194+
(target) => {
195+
target.ancestry = [];
196+
},
197+
(target) => delete target.label,
198+
];
199+
for (const degrade of degradations) {
200+
if (fitsTargetAnnotation(candidate)) return candidate;
201+
degrade(candidate);
202+
}
203+
if (fitsTargetAnnotation(candidate)) return candidate;
204+
throw new Error('target annotation cannot fit with role and id retained');
205+
}
206+
207+
export function serializeTargetAnnotationV1(evidence: TargetAnnotationV1): string {
208+
return serializeRawTargetAnnotationV1(fitTargetAnnotation(evidence));
209+
}
210+
169211
export function formatTargetAnnotationCommentLine(evidence: TargetAnnotationV1): string {
170212
return `# ${TARGET_ANNOTATION_TAG} ${serializeTargetAnnotationV1(evidence)}`;
171213
}
172214

173215
export function serializeMultiTargetAnnotationV1(evidence: MultiTargetAnnotationV1): string {
174216
return JSON.stringify({
175-
source: buildCanonicalTargetAnnotationObject(evidence.source),
176-
destination: buildCanonicalTargetAnnotationObject(evidence.destination),
217+
source: buildCanonicalTargetAnnotationObject(fitTargetAnnotation(evidence.source)),
218+
destination: buildCanonicalTargetAnnotationObject(fitTargetAnnotation(evidence.destination)),
177219
});
178220
}
179221

‎packages/daemon/src/core/__tests__/session-action-recorder.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,3 +935,66 @@ test('a literal echoed only as an object key does not make its placeholder live'
935935
expect(echo?.result?.attributes).toEqual({ '${FOO}': 'bar' });
936936
expect(echo?.livePlaceholders ?? []).toEqual([]);
937937
});
938+
test.each(['field', 'payload', 'multi'])(
939+
'echo placeholders fit the %s annotation when written',
940+
async (scenario) => {
941+
const { formatPortableActionLine, formatTargetAnnotationLines, parseReplayScriptDetailed } =
942+
await import('@agent-device/ad-script');
943+
const session = makeIosSession('default');
944+
recordAction(session, {
945+
command: 'fill',
946+
positionals: ['id="input"', 'bob'],
947+
flags: { recordAs: 'USERNAME' },
948+
result: {},
949+
});
950+
const common = {
951+
role: 'button',
952+
sibling: 0,
953+
viewportOrder: 0,
954+
verification: 'verified' as const,
955+
};
956+
const fieldEvidence = { ...common, id: 'field', label: `${'x'.repeat(253)}bob`, ancestry: [] };
957+
const payloadEvidence = {
958+
...common,
959+
id: 'payload',
960+
verification: 'unverifiable' as const,
961+
label: 'bob',
962+
ancestry: Array.from({ length: 8 }, () => ({ role: 'group', label: '\\'.repeat(220) })),
963+
scrollRegion: { role: 'group', label: '\\'.repeat(40) },
964+
rect: { x: 0, y: 0, width: 1, height: 1 },
965+
};
966+
payloadEvidence.scrollRegion.label += 'x'.repeat(
967+
4096 - Buffer.byteLength(JSON.stringify(payloadEvidence)),
968+
);
969+
expect(Buffer.byteLength(fieldEvidence.label)).toBe(256);
970+
expect(Buffer.byteLength(JSON.stringify(payloadEvidence))).toBe(4096);
971+
const target =
972+
scenario === 'multi'
973+
? { targetEvidences: { source: fieldEvidence, destination: payloadEvidence } }
974+
: { targetEvidence: scenario === 'field' ? fieldEvidence : payloadEvidence };
975+
const selected = recordAction(session, {
976+
command: scenario === 'multi' ? 'drag' : 'click',
977+
positionals: scenario === 'multi' ? ['id="field"', 'id="payload"'] : ['id="field"'],
978+
flags: {},
979+
result: {},
980+
...target,
981+
})!;
982+
const script = [
983+
...formatTargetAnnotationLines(selected),
984+
formatPortableActionLine(selected),
985+
].join('\n');
986+
const parsed = parseReplayScriptDetailed(script).actions;
987+
const evidence =
988+
scenario === 'multi' ? parsed[0]?.targetEvidences?.source : parsed[0]?.targetEvidence;
989+
if (scenario !== 'payload') {
990+
expect(evidence).toMatchObject({ id: 'field', verification: 'unverifiable' });
991+
expect(evidence?.label).toBeUndefined();
992+
}
993+
const payload = scenario === 'multi' ? parsed[0]?.targetEvidences?.destination : evidence;
994+
if (scenario !== 'field') {
995+
expect(payload).toMatchObject({ id: 'payload', verification: 'unverifiable' });
996+
expect(payload?.rect).toBeUndefined();
997+
expect(payload?.label).toBe('${USERNAME}');
998+
}
999+
},
1000+
);

‎packages/selectors/src/target-evidence.test.ts‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,29 @@ import { test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import { attachRefs } from '@agent-device/kernel/snapshot';
44
import type { RawSnapshotNode, SnapshotNode } from '@agent-device/kernel/snapshot';
5+
import type { TargetAnnotationV1 } from '@agent-device/contracts/replay';
56
import { computeTargetEvidence } from './target-evidence.ts';
67
import {
78
parseTargetAnnotationV1Payload,
8-
serializeTargetAnnotationV1,
9+
formatTargetAnnotationLines,
10+
serializeRawTargetAnnotationV1,
911
utf8ByteLength,
1012
TARGET_ANNOTATION_MAX_ANCESTRY,
1113
TARGET_ANNOTATION_MAX_FIELD_BYTES,
1214
TARGET_ANNOTATION_MAX_PAYLOAD_BYTES,
1315
} from '@agent-device/ad-script';
1416

17+
function serializeTargetAnnotationV1(evidence: TargetAnnotationV1): string {
18+
const [line] = formatTargetAnnotationLines({
19+
ts: 0,
20+
command: 'click',
21+
positionals: [],
22+
flags: {},
23+
targetEvidence: evidence,
24+
});
25+
return line!.slice('# agent-device:target-v1 '.length);
26+
}
27+
1528
function toSnapshotNodes(raw: RawSnapshotNode[]): SnapshotNode[] {
1629
return attachRefs(raw);
1730
}
@@ -252,7 +265,7 @@ test('computeTargetEvidence: escape-heavy fields degrade until the unverifiable
252265
utf8ByteLength(json) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES,
253266
`payload is ${utf8ByteLength(json)} bytes`,
254267
);
255-
assert.deepEqual(parseTargetAnnotationV1Payload(json), evidence);
268+
assert.equal(parseTargetAnnotationV1Payload(json).verification, 'unverifiable');
256269
});
257270

258271
// #4333: the downgrade drops optional fields in a fixed order and stops at the first payload that
@@ -290,8 +303,13 @@ test('computeTargetEvidence: an oversized payload drops rect, scrollRegion, ance
290303
const stops = new Set<number>();
291304
for (let labelLength = 1; labelLength <= TARGET_ANNOTATION_MAX_FIELD_BYTES; labelLength += 1) {
292305
const nodes = build(labelLength);
293-
const evidence = computeTargetEvidence({ node: nodes.at(-1)!, preActionNodes: nodes });
294-
assert.ok(evidence);
306+
const candidate = computeTargetEvidence({ node: nodes.at(-1)!, preActionNodes: nodes });
307+
assert.ok(candidate);
308+
assert.deepEqual(candidate.rect, rect);
309+
assert.deepEqual(candidate.scrollRegion, SCROLL_CONTAINER);
310+
assert.equal(candidate.label, '\u0001'.repeat(labelLength));
311+
assert.deepEqual(candidate.ancestry[0], SCROLL_CONTAINER);
312+
const evidence = parseTargetAnnotationV1Payload(serializeTargetAnnotationV1(candidate));
295313
const firstKept = OVERSIZED_DROP_ORDER.findIndex((field) => isPresent(evidence, field));
296314
const droppedCount = firstKept === -1 ? OVERSIZED_DROP_ORDER.length : firstKept;
297315
for (const field of OVERSIZED_DROP_ORDER.slice(droppedCount)) {
@@ -313,7 +331,8 @@ test('computeTargetEvidence: an oversized payload drops rect, scrollRegion, ance
313331
[lastDropped]: undropped[lastDropped],
314332
});
315333
assert.ok(
316-
utf8ByteLength(serializeTargetAnnotationV1(restored)) > TARGET_ANNOTATION_MAX_PAYLOAD_BYTES,
334+
utf8ByteLength(serializeRawTargetAnnotationV1(restored)) >
335+
TARGET_ANNOTATION_MAX_PAYLOAD_BYTES,
317336
`${lastDropped} dropped at length ${labelLength} although the payload already fit`,
318337
);
319338
}

‎packages/selectors/src/target-evidence.ts‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import {
3737
filterIdentitySet,
3838
matchesLocalIdentity,
3939
readNodeLocalIdentity,
40-
serializeTargetAnnotationV1,
40+
serializeRawTargetAnnotationV1,
4141
siblingOrdinal,
4242
utf8ByteLength,
4343
type LocalIdentity,
@@ -120,7 +120,7 @@ export function computeTargetEvidence(
120120
});
121121
return candidate;
122122
}
123-
if (ancestryLength === floor) return finalizeOversizedCandidate(candidate);
123+
if (ancestryLength === floor) return { ...candidate, verification: 'unverifiable' };
124124
}
125125
return undefined;
126126
}
@@ -146,34 +146,9 @@ function resolveRecordTimeVerification(params: {
146146
/** Size against the longest verification value so the payload fits whichever one the self-check returns. */
147147
function fitsPayloadCeiling(candidate: TargetAnnotationV1): boolean {
148148
return (
149-
utf8ByteLength(serializeTargetAnnotationV1({ ...candidate, verification: 'unverifiable' })) <=
150-
TARGET_ANNOTATION_MAX_PAYLOAD_BYTES
151-
);
152-
}
153-
154-
/**
155-
* Field caps count raw bytes and the ceiling counts escaped JSON, so dropping ancestry to its
156-
* floor can still leave an over-cap payload. Replay refuses recorded-unverifiable evidence before
157-
* it reads any other field, so each optional field drops in turn until the payload fits. A capped
158-
* field escapes to at most six times its cap, so `role` and `id` alone always fit and never drop.
159-
*/
160-
const OVERSIZED_CANDIDATE_DEGRADATIONS: readonly ((candidate: TargetAnnotationV1) => void)[] = [
161-
(candidate) => delete candidate.rect,
162-
(candidate) => delete candidate.scrollRegion,
163-
(candidate) => (candidate.ancestry = []),
164-
(candidate) => delete candidate.label,
165-
];
166-
167-
/** Decision 3's terminal fail-closed downgrade: never emit a payload the parser rejects. */
168-
function finalizeOversizedCandidate(candidate: TargetAnnotationV1): TargetAnnotationV1 {
169-
candidate.verification = 'unverifiable';
170-
for (const degrade of OVERSIZED_CANDIDATE_DEGRADATIONS) {
171-
if (fitsPayloadCeiling(candidate)) return candidate;
172-
degrade(candidate);
173-
}
174-
if (fitsPayloadCeiling(candidate)) return candidate;
175-
throw new Error(
176-
'target annotation exceeds the payload ceiling with only role and id left; the field cap no longer fits the ceiling',
149+
utf8ByteLength(
150+
serializeRawTargetAnnotationV1({ ...candidate, verification: 'unverifiable' }),
151+
) <= TARGET_ANNOTATION_MAX_PAYLOAD_BYTES
177152
);
178153
}
179154

0 commit comments

Comments
 (0)