fix: reject missing role keys in target-v1 annotations, correct data-flow comment

Second-review nits: the writer emits `role` unconditionally (top level,
ancestry entries, scrollRegion) — possibly as the empty string for typeless
nodes, which stays accepted — so a MISSING role key can only come from a
hand-edited/adversarial annotation and is now rejected with INVALID_ARGS
instead of silently parsing as an implicit empty role, which step-4
enforcement could otherwise match against anonymous wrapper nodes. Also
corrects the interaction-touch-response comment that overstated where the
raw node/tree flows (finalizeTouchInteraction strips it before both session
history and touch overlay telemetry).

Author: thymikee

src/daemon/handlers/interaction-touch-response.ts

@@ -126,12 +126,14 @@ export function buildInteractionResponseData(params: {
     responseData.warning = warning;
   }
   // ADR 0012 decision 3: the resolved node and the record-time tree it came
-  // from ride ONLY on `visualization` (session history / touch overlay
-  // input), never on `responseData` (the public payload) — attaching them
-  // here, not via `commonExtra`, is what keeps them off the wire.
-  // `finalizeTouchInteraction` (interaction-common.ts) turns them into the
-  // compact `targetEvidence` annotation when the session is being recorded
-  // and strips the raw tree back out either way.
+  // from ride ONLY on `visualization` — a hand-off channel to
+  // `finalizeTouchInteraction` (interaction-common.ts), which turns them into
+  // the compact `targetEvidence` annotation when the session is being
+  // recorded and strips the raw tree back out BEFORE anything downstream
+  // consumes the payload (session history via recordAction and touch overlay
+  // telemetry both receive the stripped form). They are never attached to
+  // `responseData` (the public payload) — attaching them here, not via
+  // `commonExtra`, is what keeps them off the wire.
   if ('node' in result && result.node) visualization.node = result.node;
   if ('preActionNodes' in result && result.preActionNodes) {
     visualization.preActionNodes = result.preActionNodes;

src/replay/__tests__/target-identity.test.ts

@@ -321,6 +321,51 @@ test('parser rejects a malformed rect', () => {
   );
 });
 
+// ---------------------------------------------------------------------------
+// Role presence: the writer emits `role` unconditionally (top level, every
+// ancestry entry, scrollRegion) — possibly as the empty string for a
+// typeless node, which stays accepted. A MISSING role key can only come from
+// a hand-edited/adversarial annotation and must be rejected, or step-4
+// enforcement could match anonymous wrapper nodes through an implicit
+// empty-role identity.
+// ---------------------------------------------------------------------------
+
+test('parser rejects a missing top-level role', () => {
+  assertInvalidArgs(
+    () => parseTargetAnnotationV1Payload(JSON.stringify({ verification: 'verified' })),
+    /"role" is required/,
+  );
+});
+
+test('parser rejects a missing role in an ancestry entry and in scrollRegion', () => {
+  assertInvalidArgs(
+    () =>
+      parseTargetAnnotationV1Payload(
+        JSON.stringify({
+          role: 'button',
+          ancestry: [{ label: 'Editor' }],
+          verification: 'verified',
+        }),
+      ),
+    /"ancestry\[0\]\.role" is required/,
+  );
+  assertInvalidArgs(
+    () =>
+      parseTargetAnnotationV1Payload(
+        JSON.stringify({ role: 'button', scrollRegion: { id: 'list' }, verification: 'verified' }),
+      ),
+    /"scrollRegion\.role" is required/,
+  );
+});
+
+test('parser accepts an explicit empty-string role (writer-legal for typeless nodes)', () => {
+  const parsed = parseTargetAnnotationV1Payload(
+    JSON.stringify({ role: '', ancestry: [{ role: '' }], verification: 'verified' }),
+  );
+  assert.equal(parsed.role, '');
+  assert.deepEqual(parsed.ancestry, [{ role: '' }]);
+});
+
 // ---------------------------------------------------------------------------
 // Classification core (decision 3 "Replay-time verification" paths 2-6):
 // this is the exact function the writer's record-time self-check calls, and

src/replay/target-identity.ts

@@ -205,7 +205,7 @@ export function parseTargetAnnotationV1Payload(jsonText: string): TargetAnnotati
   }
   const raw = parsed as Record<string, unknown>;
 
-  const role = parseRequiredRoleField(raw.role);
+  const role = parseRequiredRoleField(raw.role, 'role');
   const id = parseOptionalIdentifierField(raw.id, 'id');
   const label = parseOptionalLabelField(raw.label, 'label');
   const ancestry = parseAncestryField(raw.ancestry);
@@ -228,12 +228,24 @@ export function parseTargetAnnotationV1Payload(jsonText: string): TargetAnnotati
   };
 }
 
-function parseRequiredRoleField(value: unknown): string {
-  if (value === undefined) return '';
+/**
+ * `role` keys are always PRESENT in writer output — the writer emits `role`
+ * unconditionally at the top level, in every ancestry entry, and in
+ * `scrollRegion` (possibly as the empty string for a typeless node, which
+ * decision 3 explicitly allows). A MISSING role key can therefore only come
+ * from a hand-edited/adversarial annotation and is rejected: once step-4
+ * enforcement consumes this evidence, an implicitly-empty role in
+ * `matchesLocalIdentity` could match anonymous wrapper nodes and produce a
+ * false verified/rebind.
+ */
+function parseRequiredRoleField(value: unknown, field: string): string {
+  if (value === undefined) {
+    throw new AppError('INVALID_ARGS', `target-v1 "${field}" is required.`);
+  }
   if (typeof value !== 'string') {
-    throw new AppError('INVALID_ARGS', 'target-v1 "role" must be a string.');
+    throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a string.`);
   }
-  return boundField(normalizeRoleField(value), 'role');
+  return boundField(normalizeRoleField(value), field);
 }
 
 function parseOptionalIdentifierField(value: unknown, field: 'id'): string | undefined {
@@ -283,7 +295,7 @@ function parseAncestryEntry(entry: unknown, index: number): TargetAncestryEntry
     throw new AppError('INVALID_ARGS', `target-v1 "ancestry[${index}]" must be an object.`);
   }
   const record = entry as Record<string, unknown>;
-  const role = parseRequiredRoleField(record.role);
+  const role = parseRequiredRoleField(record.role, `ancestry[${index}].role`);
   const label = parseOptionalLabelField(record.label, `ancestry[${index}].label`);
   return { role, ...(label !== undefined ? { label } : {}) };
 }
@@ -302,7 +314,7 @@ function parseScrollRegionField(value: unknown): TargetScrollRegion | undefined
     throw new AppError('INVALID_ARGS', 'target-v1 "scrollRegion" must be an object.');
   }
   const record = value as Record<string, unknown>;
-  const role = parseRequiredRoleField(record.role);
+  const role = parseRequiredRoleField(record.role, 'scrollRegion.role');
   const id = parseOptionalIdentifierField(record.id, 'id');
   const label = parseOptionalLabelField(record.label, 'scrollRegion.label');
   return {