fix(gate-manifest): fingerprint the forwarding statement, not the call

The waiver bound only the normalized `reason(entry.check, file, entry.rule,
entry.detail)` text, so the chain around it stayed swappable. Both shapes
reported last round were accepted again as long as they kept that call:

  BUILD_OWNERSHIP
    .filter((entry) => { entry.rule = computed; return true; })
    .map((entry) => reason(entry.check, file, entry.rule, entry.detail));

  BUILD_OWNERSHIP.map((entry) => {
    entry.mutate();
    return reason(entry.check, file, entry.rule, entry.detail);
  });

The claim a waiver makes — this rule is already in the universe — is true
because of the chain, not because of the call, so the key has to carry the
chain. It is now the whole enclosing statement plus the file and the nearest
enclosing named function.

The statement is the WIDEST one inside that function, not the tightest. The
tightest statement around `.map((entry) => { return reason(…); })` is the
bare `return`, which says nothing about the chain that produced `entry` —
fingerprinting that would leave the same hole one shape over. Climbing to the
outermost statement in the function always captures the chain and its
callbacks and stops before unrelated code.

The refusal message now prints the exact file/enclosing/statement to paste,
since a hand-written fingerprint is otherwise guesswork.

The previous negatives used `reason('lint', file, entry.rule, 'd')` — a
differently shaped call — so they never exercised the waiver bypass at all.
They now use the waived call verbatim, with a non-vacuity test asserting the
fixtures still contain it, so editing one cannot quietly turn these back into
ordinary not-a-literal cases.

Author: claude

docs/agents/testing.md

@@ -237,10 +237,16 @@ Where the two meet is worth knowing before you edit the selector. The path-categ
 each `BUILD_OWNERSHIP` entry's `rule:` — so a new ownership rule widens the universe and fails
 the gate until a representative path exercises it. Only string literals count. A rule the reader
 cannot see would slip past the reachability check while the gate stayed green, so a computed one
-is an error naming its line. The single call that forwards `entry.rule` out of the ownership loop
-is declared in `FORWARDED_SELECTOR_RULES`, keyed on the exact text of the call, rather than
-recognized by shape; `selector-rules.ts` records why the shape-recognizing version could not be
-made sound.
+is an error naming its line. The single statement that forwards `entry.rule` out of the ownership
+loop is declared in `FORWARDED_SELECTOR_RULES` rather than recognized by shape;
+`selector-rules.ts` records why the shape-recognizing version could not be made sound.
+
+That waiver is keyed on the whole enclosing **statement** — chain, callbacks and all — plus the
+file and enclosing function, not on the `reason(...)` call. The claim it makes ("this rule is
+already in the universe") is true because of the chain, so a key naming only the call would
+survive that chain being swapped for a mutating one. Mutate it, or move the statement, and the
+waiver goes inert and fails. When the reader refuses a forward it prints the exact three fields
+to paste.
 
 The gate is deterministic, offline, and needs no GitHub token — it runs from a clean checkout in
 the `Affected-check Selector` job. Branch-protection required-contexts drift is the one part

scripts/gate-manifest/check.ts

@@ -74,7 +74,7 @@ const vitestProjects = vitestProjectNames('vitest.config.ts', read('vitest.confi
 const selector = readSelectorRules(
   SELECTOR_SOURCE,
   read(SELECTOR_SOURCE),
-  FORWARDED_SELECTOR_RULES.map((entry) => entry.call),
+  FORWARDED_SELECTOR_RULES,
 );
 const selectorRules = selector.rules;
 
@@ -216,21 +216,23 @@ const staleWaivers = [
       `TRANSPARENT_WRAPPERS "${entry.file}" changes nothing — no resolved command forwards ` +
       `through it, so the waiver is inert`,
   ),
-  // A forwarded-rule waiver is applied-reachable when the call it names is really there — and
-  // only that call. Matching none means the forward is gone and the waiver now excuses nothing;
-  // matching several means one reviewed claim has silently spread to a call nobody looked at.
-  ...FORWARDED_SELECTOR_RULES.map((entry) => ({
+  // A forwarded-rule waiver is applied-reachable when the statement it fingerprints is really
+  // there — and only once. Matching none means the statement changed or moved, so the reviewed
+  // claim no longer describes live code; matching several means one claim now stands for code
+  // nobody looked at. Either way the waiver has stopped meaning what it said.
+  ...FORWARDED_SELECTOR_RULES.map((entry, index) => ({
     entry,
-    matches: selector.waiverMatches.get(entry.call) ?? 0,
+    matches: selector.waiverMatches[index] ?? 0,
   }))
     .filter(({ matches }) => matches !== 1)
     .map(({ entry, matches }) =>
       matches === 0
-        ? `FORWARDED_SELECTOR_RULES "${entry.call}" matches no reason() call in ` +
-          `${SELECTOR_SOURCE} — the forward is gone, so the waiver is inert`
-        : `FORWARDED_SELECTOR_RULES "${entry.call}" matches ${matches} calls in ` +
-          `${SELECTOR_SOURCE} — one waiver cannot stand for several forwards; make each call ` +
-          `distinguishable, or write the rules as literals`,
+        ? `FORWARDED_SELECTOR_RULES ${entry.file}#${entry.enclosing} matches no forwarding ` +
+          `statement — the statement it waives changed, moved, or is gone, so the waiver is ` +
+          `inert. It fingerprints: ${entry.statement}`
+        : `FORWARDED_SELECTOR_RULES ${entry.file}#${entry.enclosing} matches ${matches} ` +
+          `statements — one waiver cannot stand for several forwards; review each and give it ` +
+          `its own entry, or write the rules as literals`,
     ),
   ...DECLARED_EDGES.filter(
     (entry) =>

scripts/gate-manifest/path-categories.test.ts

@@ -113,7 +113,7 @@ test("the selector's real rule universe is derived, and excludes its fail-open c
   const { rules } = readSelectorRules(
     SELECTOR_SOURCE,
     fs.readFileSync(path.join(repoRoot, SELECTOR_SOURCE), 'utf8'),
-    FORWARDED_SELECTOR_RULES.map((entry) => entry.call),
+    FORWARDED_SELECTOR_RULES,
   );
   // Live selection rules, read from `reason(...)` calls and the BUILD_OWNERSHIP table.
   for (const expected of ['gate:lint', 'platform-src', 'own:swift', 'own:mcp', 'src-prod']) {

scripts/gate-manifest/selector-rules.test.ts

@@ -45,7 +45,7 @@ test("the reason factory's own return object declares no category", () => {
 // --- Nothing but a literal is derived ---------------------------------------
 
 /**
- * Every shape that six rounds of review argued about, plus the two that ended the argument.
+ * Every shape that seven rounds of review argued about.
  *
  * Earlier revisions tried to decide these case by case — is this `entry` really the ownership
  * table's? — and each answer left the next construct open. They are one test now because the
@@ -141,16 +141,12 @@ const NOT_A_LITERAL: readonly { name: string; source: string }[] = [
       });`,
   },
   {
-    // Round seven, first hole: the mutation is in an UPSTREAM callback, so a proof that only
-    // looked at the forwarding callback saw nothing wrong with the map below it.
     name: 'a mutating upstream filter callback',
     source: `${TABLE}
       const bad = BUILD_OWNERSHIP.filter((entry) => { entry.rule = computed; return true; })
         .map((entry) => reason('lint', file, entry.rule, 'd'));`,
   },
   {
-    // Round seven, second hole: `entry.mutate` is a member access whose object is `entry`, so a
-    // read/write classifier counted invoking it as a read.
     name: 'a member call on the entry before reading it',
     source: `${TABLE}
       const bad = BUILD_OWNERSHIP.map((entry) => {
@@ -167,7 +163,7 @@ const NOT_A_LITERAL: readonly { name: string; source: string }[] = [
 
 for (const { name, source } of NOT_A_LITERAL) {
   test(`fails closed: ${name}`, () => {
-    assert.throws(() => readSelectorRules('m.ts', source), /rule argument is not a string literal/);
+    assert.throws(() => readSelectorRules('m.ts', source), /not a string literal/);
   });
 }
 
@@ -182,99 +178,199 @@ test('a computed ownership-table rule fails closed too', () => {
   );
 });
 
-test('the failure names the line and quotes the call, so it can be waived or fixed', () => {
-  try {
-    readSelectorRules(
-      'm.ts',
-      "const ok = reason('lint', f, 'gate:lint', 'd');\nconst bad = reason('lint', f, X, 'd');",
-    );
-    assert.fail('expected a throw');
-  } catch (error) {
-    assert.match(String(error), /line 2/);
-    assert.match(String(error), /reason\('lint', f, X, 'd'\)/);
-    assert.match(String(error), /FORWARDED_SELECTOR_RULES/);
-  }
-});
-
 // --- Declared forwards ------------------------------------------------------
 
-const LIVE_CHAIN = `${TABLE}
-  const s = BUILD_OWNERSHIP.filter((entry) => entry.owns(file)).map((entry) =>
-    reason(entry.check, file, entry.rule, entry.detail),
-  );
-  const a = reason('lint', file, 'gate:lint', 'd');`;
+/**
+ * The live forwarding statement, on one line so its normalized fingerprint is obvious.
+ *
+ * The variants below keep this statement's inner `reason(...)` call byte for byte and change
+ * only the chain around it. That is the point of fingerprinting the statement: an entry keyed on
+ * the call alone accepts every one of them.
+ */
+const LIVE_STATEMENT =
+  'const s = BUILD_OWNERSHIP.filter((entry) => entry.owns(file)).map((entry) => ' +
+  'reason(entry.check, file, entry.rule, entry.detail));';
+
+const MUTATING_FILTER =
+  'const s = BUILD_OWNERSHIP.filter((entry) => { entry.rule = computed; return true; })' +
+  '.map((entry) => reason(entry.check, file, entry.rule, entry.detail));';
 
-const LIVE_CALL = 'reason(entry.check, file, entry.rule, entry.detail)';
+const MEMBER_CALL =
+  'const s = BUILD_OWNERSHIP.map((entry) => { entry.mutate(); ' +
+  'return reason(entry.check, file, entry.rule, entry.detail); });';
 
-test('a waiver admits exactly the call it names, and the universe stays complete', () => {
-  const derived = readSelectorRules('m.ts', LIVE_CHAIN, [LIVE_CALL]);
+function inFunction(statement: string, enclosing = 'buildOwnership'): string {
+  return `${TABLE}
+    const ${enclosing} = () => {
+      ${statement}
+      return s;
+    };
+    const a = reason('lint', file, 'gate:lint', 'd');`;
+}
+
+const LIVE_KEY = { file: 'm.ts', enclosing: 'buildOwnership', statement: LIVE_STATEMENT };
+
+test('a waiver admits exactly the statement it fingerprints, and loses no category', () => {
+  const derived = readSelectorRules('m.ts', inFunction(LIVE_STATEMENT), [LIVE_KEY]);
   // `own:swift` is still there — read from the table entry, which is the whole reason the
-  // forward is safe to declare: the waiver excuses the call, it does not drop a category.
+  // forward is safe to declare: the waiver excuses the statement, it does not drop a category.
   assert.deepEqual(derived.rules, ['gate:lint', 'own:swift']);
-  assert.equal(derived.waiverMatches.get(LIVE_CALL), 1);
+  assert.deepEqual(derived.waiverMatches, [1]);
 });
 
-test('a waiver matches across reformatting, because the text is normalized', () => {
-  const rewrapped = `${TABLE}
-    const s = BUILD_OWNERSHIP.map((entry) => reason(
-        entry.check,
-        file,
-        entry.rule,
-        entry.detail
-    ));
-    const a = reason('lint', file, 'gate:lint', 'd');`;
+test('a waiver survives reformatting, because the statement text is normalized', () => {
+  const rewrapped = inFunction(
+    `const s = BUILD_OWNERSHIP.filter((entry) => entry.owns(file)).map(
+        (entry) => reason(entry.check, file, entry.rule, entry.detail),
+      );`,
+  );
+  // Rewrapped across lines with extra indentation, but the same tokens: still one match. (The
+  // trailing comma IS a token difference, so this variant keeps the argument list as written.)
+  const normalized = LIVE_STATEMENT.replace(
+    '.map((entry) => reason(entry.check, file, entry.rule, entry.detail));',
+    '.map( (entry) => reason(entry.check, file, entry.rule, entry.detail), );',
+  );
+  const derived = readSelectorRules('m.ts', rewrapped, [{ ...LIVE_KEY, statement: normalized }]);
+  assert.deepEqual(derived.rules, ['gate:lint', 'own:swift']);
+  assert.deepEqual(derived.waiverMatches, [1]);
+});
+
+/**
+ * The round-eight regressions: the two unsafe chains, each keeping the waived call verbatim.
+ *
+ * A waiver keyed on `reason(entry.check, file, entry.rule, entry.detail)` accepted all of these.
+ * Keyed on the statement, every one is a different statement and fails closed.
+ */
+const BYPASS_ATTEMPTS: readonly { name: string; source: string }[] = [
+  { name: 'a mutating upstream filter callback', source: inFunction(MUTATING_FILTER) },
+  { name: 'a member call on the entry', source: inFunction(MEMBER_CALL) },
+  {
+    name: 'the waived statement relocated to another function',
+    source: inFunction(LIVE_STATEMENT, 'somewhereElse'),
+  },
+  {
+    name: 'the waived statement wrapped in a value-changing chain',
+    source: inFunction(
+      'const s = BUILD_OWNERSHIP.map(transform).map((entry) => ' +
+        'reason(entry.check, file, entry.rule, entry.detail));',
+    ),
+  },
+];
+
+test('the bypass fixtures really do keep the waived call, or they prove nothing', () => {
+  // Non-vacuity guard. These negatives only exercise the bypass while their inner call is byte
+  // for byte the one a call-keyed waiver would have named; edit a fixture so the call differs
+  // and they quietly degrade into ordinary "not a literal" cases that never touched the waiver.
+  const call = 'reason(entry.check, file, entry.rule, entry.detail)';
+  for (const statement of [LIVE_STATEMENT, MUTATING_FILTER, MEMBER_CALL]) {
+    assert.ok(statement.includes(call), `fixture must contain the waived call: ${statement}`);
+  }
+});
+
+for (const { name, source } of BYPASS_ATTEMPTS) {
+  test(`the waiver does not admit: ${name}`, () => {
+    assert.throws(() => readSelectorRules('m.ts', source, [LIVE_KEY]), /not a string literal/);
+  });
+}
+
+test('a forward inside a block callback fingerprints the chain, not the bare return', () => {
+  // The tightest statement around this forward is `return reason(…);`, which says nothing about
+  // the chain that produced `entry`. Fingerprinting that would leave the chain swappable — the
+  // same hole one shape over — so the key climbs to the outermost statement in the function.
+  const blockForm =
+    'const s = BUILD_OWNERSHIP.map((entry) => { return reason(entry.check, file, entry.rule, entry.detail); });';
+  const bareReturn = 'return reason(entry.check, file, entry.rule, entry.detail);';
+
   assert.throws(
-    () => readSelectorRules('m.ts', rewrapped, [LIVE_CALL]),
-    /rule argument is not a string literal/,
-    'a trailing-comma difference is a real text difference',
+    () =>
+      readSelectorRules('m.ts', inFunction(blockForm), [
+        { file: 'm.ts', enclosing: 'buildOwnership', statement: bareReturn },
+      ]),
+    /not a string literal/,
+    'a waiver naming only the return must not admit the chain around it',
   );
-  const normalized = 'reason( entry.check, file, entry.rule, entry.detail )';
-  assert.deepEqual(readSelectorRules('m.ts', rewrapped, [normalized]).rules, [
-    'gate:lint',
-    'own:swift',
+  const derived = readSelectorRules('m.ts', inFunction(blockForm), [
+    { file: 'm.ts', enclosing: 'buildOwnership', statement: blockForm },
   ]);
+  assert.deepEqual(derived.waiverMatches, [1]);
 });
 
-test('a waiver does not admit a different forward in the same file', () => {
-  const source = `${LIVE_CHAIN}
-    const other = BUILD_OWNERSHIP.map((entry) => reason('lint', file, entry.rule, 'd'));`;
+test('a waiver is bound to its file, so the same statement elsewhere is unreviewed', () => {
   assert.throws(
-    () => readSelectorRules('m.ts', source, [LIVE_CALL]),
-    /reason\('lint', file, entry\.rule, 'd'\)/,
+    () => readSelectorRules('other.ts', inFunction(LIVE_STATEMENT), [LIVE_KEY]),
+    /not a string literal/,
   );
 });
 
-test('an unmatched waiver is reported as matching nothing, so check.ts can call it inert', () => {
+test('an unmatched waiver reports zero, so check.ts can call it inert', () => {
   const derived = readSelectorRules('m.ts', "const a = reason('lint', file, 'gate:lint', 'd');", [
-    LIVE_CALL,
+    LIVE_KEY,
   ]);
-  assert.equal(derived.waiverMatches.get(LIVE_CALL), undefined);
+  assert.deepEqual(derived.waiverMatches, [0]);
 });
 
-test('a waiver covering two identical calls reports both, so check.ts can refuse it', () => {
-  // One reviewed claim must not silently spread to a second call nobody looked at.
+test('a waiver covering two identical statements reports both, so check.ts can refuse it', () => {
+  // One reviewed claim must not silently spread to a second forward nobody looked at.
+  const forward = 'use(BUILD_OWNERSHIP.map((entry) => reason(entry.check, file, entry.rule, 1)));';
   const source = `${TABLE}
-    const one = BUILD_OWNERSHIP.map((entry) => reason(entry.check, file, entry.rule, entry.detail));
-    const two = OTHER.map((entry) => reason(entry.check, file, entry.rule, entry.detail));`;
-  assert.equal(readSelectorRules('m.ts', source, [LIVE_CALL]).waiverMatches.get(LIVE_CALL), 2);
+    const buildOwnership = () => {
+      ${forward}
+      ${forward}
+    };`;
+  const derived = readSelectorRules('m.ts', source, [
+    { file: 'm.ts', enclosing: 'buildOwnership', statement: forward },
+  ]);
+  assert.deepEqual(derived.waiverMatches, [2]);
+});
+
+test('the failure hands back the exact three fields a waiver needs', () => {
+  try {
+    readSelectorRules('m.ts', inFunction(LIVE_STATEMENT));
+    assert.fail('expected a throw');
+  } catch (error) {
+    const message = String(error);
+    assert.match(message, /file: {6}m\.ts/);
+    assert.match(message, /enclosing: buildOwnership/);
+    assert.match(
+      message,
+      new RegExp(`statement: ${LIVE_STATEMENT.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}`),
+    );
+  }
 });
 
+test('the failure names the line, and top-level forwards report as such', () => {
+  try {
+    readSelectorRules(
+      'm.ts',
+      "const ok = reason('lint', f, 'gate:lint', 'd');\nconst bad = reason('lint', f, X, 'd');",
+    );
+    assert.fail('expected a throw');
+  } catch (error) {
+    assert.match(String(error), /line 2/);
+    assert.match(String(error), /enclosing: \(top level\)/);
+    assert.match(String(error), /FORWARDED_SELECTOR_RULES/);
+  }
+});
+
+// --- The real tree ----------------------------------------------------------
+
 test("the repo's real selector derives, and its one declared forward is live", () => {
-  const waived = FORWARDED_SELECTOR_RULES.map((entry) => entry.call);
   const derived = readSelectorRules(
     SELECTOR_SOURCE,
     fs.readFileSync(path.join(repoRoot, SELECTOR_SOURCE), 'utf8'),
-    waived,
+    FORWARDED_SELECTOR_RULES,
   );
   assert.ok(
     derived.rules.length >= 20,
     `expected the live selector to yield its categories, got ${derived.rules.length}`,
   );
-  // Each declared forward matches exactly one real call. check.ts fails the gate on 0 or 2+;
-  // asserting it here too means the unit lane catches a rotted waiver without the full manifest.
-  for (const call of waived) {
-    assert.equal(derived.waiverMatches.get(call), 1, `waiver "${call}" should match one call`);
-  }
+  // Each declared forward matches exactly one real statement. check.ts fails the gate on 0 or
+  // 2+; asserting it here means the unit lane catches a rotted waiver without the full manifest.
+  assert.deepEqual(
+    derived.waiverMatches,
+    FORWARDED_SELECTOR_RULES.map(() => 1),
+    'every FORWARDED_SELECTOR_RULES entry must fingerprint exactly one live statement',
+  );
 });
 
 test("the repo's real selector fails closed with the waiver removed", () => {
@@ -286,6 +382,6 @@ test("the repo's real selector fails closed with the waiver removed", () => {
         SELECTOR_SOURCE,
         fs.readFileSync(path.join(repoRoot, SELECTOR_SOURCE), 'utf8'),
       ),
-    /rule argument is not a string literal/,
+    /not a string literal/,
   );
 });