fix(ci): distinguish review-signal queue timeout (#3823)

Author: louistrue

.github/workflows/pr-review-signal.yml

@@ -137,9 +137,11 @@ jobs:
       #      the rest -- so there is nothing to optimise and no
       #      ceiling: queue depth grows with how many PRs are open, which is
       #      when this gate is under load. A budget cannot bound that, so it
-      #      WILL breach again; the gate then says "within the poll budget"
-      #      rather than reporting a bare absence, and the remedy is a re-run,
-      #      because nothing failed.
+      #      WILL breach again. A breach while the rollup is still moving is an
+      #      explicit LANE_PUBLICATION_TIMEOUT advisory, not MISSING_LANES: five
+      #      runs in #3810 crossed 2400 s and published every named lane later.
+      #      The remedy is a re-run; the independent dirty-PR scan remains the
+      #      eventual failing signal for lanes that genuinely never appear.
       #
       #      Measuring from run creation is the CONSERVATIVE direction: this
       #      job's own deadline starts later still, after its pickup and

scripts/check-pr-review-signal.mjs

@@ -139,7 +139,7 @@
  *     has re-reviewed the newest push is transient GitHub state, not a fact
  *     about the diff.
  *
- * FAIL-CLOSED, EVERY PATH. `gh` missing, `gh` erroring, unparseable JSON, an
+ * FAIL-CLOSED ON READABLE FACTS. `gh` missing, `gh` erroring, unparseable JSON, an
  * empty rollup, a head SHA that will not resolve, a reviewer that passed with no
  * description, a job name this parser cannot expand, a reviews walk that did not
  * complete, a review whose `commit_id` is unreadable -- each exits non-zero with
@@ -191,7 +191,11 @@ const DEFAULT_CONFIG = join(SCRIPTS_DIR, 'pr-review-signal.config.json');
  * makes `now() >= deadline` false forever: the poll would spin until the job's
  * own job timeout killed it, printing nothing at all. That is the exact
  * "no output, no verdict" shape this gate exists to reject, so an unreadable
- * duration is an error rather than a silently infinite one. Zero and negatives
+ * duration is an error rather than a silently infinite one. Exhausting that
+ * duration while the rollup is still moving is reported as an explicit
+ * LANE_PUBLICATION_TIMEOUT advisory: absence has not become evidence yet, and
+ * the independent push/scheduled dirty-PR scan remains the eventual backstop.
+ * Zero and negatives
  * go the same way: a zero budget is a gate that never waits, and a zero poll
  * interval is a busy loop against the API.
  *
@@ -653,6 +657,24 @@ export function evaluate({
         'normal for a fork and is reported without failing:',
     );
     for (const n of missing) lines.push(`      - ${n}`);
+  } else if (timedOut) {
+    // A moving rollup at the deadline is UNKNOWN, not MISSING. Five live runs
+    // in #3810 crossed the 2400 s budget while Build packages + WASM was still
+    // queued; every named lane appeared later and largely passed. Rendering
+    // that queue condition as MISSING_LANES makes a red check indistinguishable
+    // from settled absence. The dirty-PR scan independently rechecks actual
+    // missing lanes on main pushes and schedule, so this is reported loudly
+    // without pretending the code failed.
+    lines.push(
+      `⚠️  LANE_PUBLICATION_TIMEOUT: ${missing.length} of ${required.length} required lane(s) ` +
+        'had not appeared before the poll budget expired, while the rollup was still unsettled. ' +
+        'This is an unknown queue state, not evidence that the lanes will never run:',
+    );
+    for (const n of missing) lines.push(`      - ${n}`);
+    lines.push(
+      '   Re-run this signal for an immediate answer. The independent silent-PR scan will fail ' +
+        'on settled missing lanes without turning hosted-runner delay into a code failure.',
+    );
   } else {
     ok = false;
     lines.push(

scripts/check-pr-review-signal.test.mjs

@@ -20,6 +20,7 @@ import { dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
 
 import { expandJobNames, pullRequestBranchFilterKeys } from './lib/pr-review-signal.mjs';
+import { evaluate } from './check-pr-review-signal.mjs';
 
 const HERE = dirname(fileURLToPath(import.meta.url));
 const REPO_ROOT = join(HERE, '..');
@@ -98,6 +99,26 @@ const FATAL = () => ['--config', cfgWith({ reviewVerdictSeverity: 'fail' }, 'fat
 const LANE = (name, state = 'success') => ({ name, state });
 const HEALTHY = ['Typecheck', 'Lint', 'Node tests'];
 
+test('#3810: a poll-budget breach is visibly unknown, not a false MISSING_LANES failure', () => {
+  const cfg = JSON.parse(readFileSync(CONFIG, 'utf8'));
+  const result = evaluate({
+    required: HEALTHY,
+    aliases: new Map(),
+    lanes: [LANE('Typecheck', 'in_progress')],
+    reviewChecks: [],
+    reviews: [],
+    headSha: ANY_HEAD,
+    headCommittedAt: ANY_HEAD_COMMITTED_AT,
+    isFork: false,
+    cfg,
+    timedOut: true,
+    baseRefName: 'main',
+  });
+  assert.equal(result.ok, true);
+  assert.match(result.lines.join('\n'), /LANE_PUBLICATION_TIMEOUT/);
+  assert.doesNotMatch(result.lines.join('\n'), /MISSING_LANES/);
+});
+
 // -------------------------------------------------------------- happy path
 
 test('GREEN: every required lane present and no reviewer claims a verdict it lacks', () => {