fix: make a knownDivergence waiver cover exactly one failure, not any failure

P1 from re-review, and a real flaw: the code did not do what its own comment
claimed. runScenario() collapsed every unexpected outcome and every invariant
failure into `misbehaved`, then turned ANY of them green if the scenario carried
a declaration. So while the #1299 scrollUntilVisible waiver is open, upstream
Maestro could start failing too — or a different invariant could break — and the
scheduled job would still report known-divergence and pass. A waiver for one bug
was silently amnesty for the next. That is the exact failure this oracle exists
to prevent, committed one commit after building the guard against it.

knownDivergence now requires an `expected` signature: both engines' outcomes plus
each declared invariant's status. The runner matches it exactly —
  - matches            -> known-divergence (green, tracked)
  - misbehaves differently -> failed (red): not the failure the waiver covers
  - stops misbehaving  -> stale-declaration (red): remove the declaration
#1299's signature pins what runs 29504440599/29510020718 actually observed:
maestro=pass, agent-device=fail, settle invariant no-data.

Tests prove unrelated failures stay red under an open waiver: upstream also
failing, our engine unexpectedly passing, a different invariant status, and a new
invariant appearing are each NOT covered. A signature where both engines pass is
rejected outright as describing no divergence.

Also retains replay-timing.ndjson as a run artifact (review evidence note): the
invariants are computed from that trace, so a report saying "tapRetries was 0"
cannot be audited once the runner is gone without it.

Author: thymikee

.github/workflows/conformance-differential.yml

@@ -139,10 +139,16 @@ jobs:
             --trace-root "${{ github.workspace }}/.agent-device" \
             ${DIFFERENTIAL_ONLY:+--only "$DIFFERENTIAL_ONLY"}
 
-      - name: Upload report
+      # Keep the trace, not just the verdict: the engine-side invariants are
+      # computed FROM replay-timing.ndjson, so without it a report saying
+      # "invariant violated: tapRetries was 0" cannot be audited or debugged
+      # after the runner is gone.
+      - name: Upload report and trace
         if: always()
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         with:
           name: conformance-differential-ios
-          path: .tmp/conformance-differential/differential-report.json
+          path: |
+            .tmp/conformance-differential/differential-report.json
+            .agent-device/**/replay-timing.ndjson
           if-no-files-found: ignore

scripts/maestro-conformance/differential/run.test.ts

@@ -5,9 +5,13 @@ import assert from 'node:assert/strict';
 import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { test } from 'node:test';
-import { DIFFERENTIAL_APP_ID, DIFFERENTIAL_SCENARIOS } from './scenarios.ts';
-import { parseRunnerArgs, selectScenarios, validateScenarios } from './run.ts';
+import { describe, test } from 'node:test';
+import {
+  DIFFERENTIAL_APP_ID,
+  DIFFERENTIAL_SCENARIOS,
+  type DivergenceSignature,
+} from './scenarios.ts';
+import { matchesSignature, parseRunnerArgs, selectScenarios, validateScenarios } from './run.ts';
 
 const CONFORMANCE_DIR = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
 
@@ -66,6 +70,64 @@ test('every knownDivergence carries a tracking issue', () => {
   assert.deepEqual(problems, [], problems.join('\n'));
 });
 
+// A waiver must cover the ONE failure it was granted for. Without an exact
+// signature it is blanket amnesty: while a gap is open the job would also
+// swallow upstream regressing or a different invariant breaking — hiding the
+// next bug behind the last one.
+describe('knownDivergence signature matching', () => {
+  const sig: DivergenceSignature = { maestro: 'pass', agentDevice: 'fail', invariants: ['no-data'] };
+  const engine = (outcome: 'pass' | 'fail') => ({
+    engine: 'maestro' as const,
+    outcome,
+    exitCode: outcome === 'pass' ? 0 : 1,
+  });
+  const inv = (status: 'held' | 'violated' | 'no-data') =>
+    ({ invariant: { kind: 'stepDurationBelow', command: 'tapOn', maxMs: 1, because: 'x' }, status, detail: '' }) as never;
+
+  test('matches the declared failure exactly', () => {
+    assert.equal(matchesSignature(sig, engine('pass'), engine('fail'), [inv('no-data')]), true);
+  });
+
+  test('upstream also failing is NOT covered by the waiver', () => {
+    // The #1299 shape is maestro=pass. If Maestro starts failing too, that is a
+    // different problem and must not ride in green on this declaration.
+    assert.equal(matchesSignature(sig, engine('fail'), engine('fail'), [inv('no-data')]), false);
+  });
+
+  test('our engine unexpectedly passing is NOT covered (declaration is stale)', () => {
+    assert.equal(matchesSignature(sig, engine('pass'), engine('pass'), [inv('no-data')]), false);
+  });
+
+  test('a different invariant outcome is NOT covered', () => {
+    assert.equal(matchesSignature(sig, engine('pass'), engine('fail'), [inv('violated')]), false);
+  });
+
+  test('a new invariant appearing is NOT covered', () => {
+    assert.equal(
+      matchesSignature(sig, engine('pass'), engine('fail'), [inv('no-data'), inv('violated')]),
+      false,
+    );
+  });
+
+  test('every declaration states its expected signature', () => {
+    for (const scenario of DIFFERENTIAL_SCENARIOS) {
+      const declared = scenario.knownDivergence;
+      if (!declared) continue;
+      assert.ok(declared.expected, `${scenario.id}: knownDivergence must declare an expected signature`);
+      assert.ok(
+        ['pass', 'fail'].includes(declared.expected.maestro) &&
+          ['pass', 'fail'].includes(declared.expected.agentDevice),
+        `${scenario.id}: signature must state both engines' outcomes`,
+      );
+      // A waiver that expects both engines to pass is not a divergence at all.
+      assert.ok(
+        !(declared.expected.maestro === 'pass' && declared.expected.agentDevice === 'pass'),
+        `${scenario.id}: a signature where both engines pass describes no divergence`,
+      );
+    }
+  });
+});
+
 test('every device flow targets the fixture app the workflow installs', () => {
   for (const scenario of DIFFERENTIAL_SCENARIOS) {
     const body = fs.readFileSync(path.join(CONFORMANCE_DIR, scenario.flow), 'utf8');

scripts/maestro-conformance/differential/run.ts

@@ -13,7 +13,11 @@ import { spawnSync } from 'node:child_process';
 import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { DIFFERENTIAL_SCENARIOS, type DifferentialScenario } from './scenarios.ts';
+import {
+  DIFFERENTIAL_SCENARIOS,
+  type DifferentialScenario,
+  type DivergenceSignature,
+} from './scenarios.ts';
 import { type InvariantResult, evaluateInvariants, readTrace } from './invariants.ts';
 
 const HERE = path.dirname(fileURLToPath(import.meta.url));
@@ -124,6 +128,24 @@ function findTimingTrace(root: string | undefined): string | undefined {
   return found.sort((a, b) => b.mtimeMs - a.mtimeMs)[0]?.file;
 }
 
+/**
+ * Does the observed failure match the one the waiver was granted for, exactly?
+ * Both engines' outcomes and every declared invariant status must line up; any
+ * deviation means this is a different failure and must stay red.
+ */
+export function matchesSignature(
+  expected: DivergenceSignature,
+  maestro: EngineResult,
+  agentDevice: EngineResult,
+  invariants: InvariantResult[],
+): boolean {
+  if (maestro.outcome !== expected.maestro) return false;
+  if (agentDevice.outcome !== expected.agentDevice) return false;
+  const expectedInvariants = expected.invariants ?? [];
+  if (expectedInvariants.length !== invariants.length) return false;
+  return expectedInvariants.every((status, index) => invariants[index]?.status === status);
+}
+
 function runScenario(scenario: DifferentialScenario, options: RunnerOptions): ScenarioReport {
   const flowPath = path.join(CONFORMANCE_DIR, scenario.flow);
   const platformArgs = options.platform ? ['--platform', options.platform] : [];
@@ -151,17 +173,25 @@ function runScenario(scenario: DifferentialScenario, options: RunnerOptions): Sc
   const misbehaved = outcomeDiverged || invariantFailed;
 
   // A declared divergence is an expected, tracked gap: it keeps the run green so
-  // the oracle is not blocked on the engine bug it just found. But a declaration
-  // that no longer reproduces FAILS — the fix PR has to delete it, which is what
-  // turns the differential into the acceptance test for its own findings.
+  // the oracle is not blocked on the engine bug it just found. But the waiver
+  // covers ONE precise failure — matched exactly below — so while the gap is
+  // open the job still catches anything else (upstream regressing, a different
+  // invariant breaking). And a declaration that no longer reproduces FAILS, so
+  // the fix PR has to delete it; that is what turns the differential into the
+  // acceptance test for its own findings.
   const declared = scenario.knownDivergence;
-  const status = declared
+  const matchesDeclared =
+    declared !== undefined && matchesSignature(declared.expected, maestro, agentDevice, invariants);
+  const status = !declared
     ? misbehaved
-      ? ('known-divergence' as const)
-      : ('stale-declaration' as const)
-    : misbehaved
       ? ('failed' as const)
-      : ('ok' as const);
+      : ('ok' as const)
+    : matchesDeclared
+      ? ('known-divergence' as const)
+      : misbehaved
+        ? // Diverged, but not the way the waiver describes: not covered.
+          ('failed' as const)
+        : ('stale-declaration' as const);
 
   return {
     id: scenario.id,

scripts/maestro-conformance/differential/scenarios.ts

@@ -39,11 +39,30 @@ export type DifferentialOutcome = 'pass' | 'fail';
  * no issue behind it is exactly how "temporarily expected" becomes permanent
  * without anyone deciding to make it so.
  */
+/**
+ * The EXACT failure a declaration covers.
+ *
+ * Without this a declaration is blanket amnesty: any failure at all would turn
+ * the scenario green, so while a gap is open the job would also swallow an
+ * unrelated regression — upstream Maestro starting to fail, or a different
+ * invariant breaking. A waiver must cover the one failure it was granted for and
+ * nothing else, so the signature is matched exactly and any deviation is red.
+ */
+export type DivergenceSignature = {
+  /** Outcome each engine is expected to produce while the gap is open. */
+  maestro: DifferentialOutcome;
+  agentDevice: DifferentialOutcome;
+  /** Expected status of each declared engine invariant, in declaration order. */
+  invariants?: Array<'held' | 'violated' | 'no-data'>;
+};
+
 export type KnownDivergence = {
   /** Why this scenario currently fails, and what it blocks. */
   reason: string;
   /** Issue tracking the fix. Required — see above. */
   tracking: string;
+  /** The precise failure this waiver covers. Anything else stays red. */
+  expected: DivergenceSignature;
 };
 
 export type DifferentialScenario = {
@@ -90,8 +109,18 @@ export const DIFFERENTIAL_SCENARIOS: DifferentialScenario[] = [
       'agent-device settled in a different order than upstream (sleep-before vs sleep-after capture) or never latches within the shared budget.',
     knownDivergence: {
       reason:
-        'Caught by this differential on its first working run: our scrollUntilVisible times out finding home-open-form where Maestro 2.5.1 scrolls to it and passes (maestro=pass, agent-device=fail). The flow cannot reach its tapOn step, so bug class 4 has no working device detector until that engine bug is fixed. Declared rather than chased inline — the instrument does not block on repairing what it just measured.',
+        'Caught by this differential on its first working run: our scrollUntilVisible times out finding home-open-form where Maestro 2.5.1 scrolls to it and passes. The flow cannot reach its tapOn step, so bug class 4 has no working device detector until that engine bug is fixed. Declared rather than chased inline — the instrument does not block on repairing what it just measured.',
       tracking: 'https://github.com/callstack/agent-device/issues/1299',
+      // Exactly what runs 29504440599 and 29510020718 observed. If upstream
+      // starts failing too, or the flow fails for a different reason, the
+      // signature stops matching and the job goes red — a waiver for THIS bug
+      // must not silently cover the next one.
+      expected: {
+        maestro: 'pass',
+        agentDevice: 'fail',
+        // The tap step is never reached, so the settle invariant has no data.
+        invariants: ['no-data'],
+      },
     },
   },
   {