fix(claude-review): count only reviews that ran (A2: repoint + gate flip)

.github/scripts/claude-review-outcome-wiring.test.cjs

@@ -0,0 +1,111 @@
+"use strict";
+
+// The review lane consumes the claude-lane-outcome composite through a
+// SHA-pinned `uses:`, so the composite's checked-in source and the version
+// the lane actually runs can diverge — exactly the window in which a gate
+// reading a not-yet-pinned output evaluates false on every run and silently
+// skips the step it guards (the review-count upsert stopped writing for one
+// PR window this way). These tests pin the wiring from both ends: the gate
+// conditions the lane must carry, and the rule that every consumed output of
+// the composite is one its source declares — checked against the pinned
+// revision when the object is reachable, and against the working tree always.
+
+const assert = require("node:assert/strict");
+const { execFileSync } = require("node:child_process");
+const fs = require("node:fs");
+const path = require("node:path");
+const test = require("node:test");
+
+const repositoryRoot = path.join(__dirname, "..", "..");
+const workflow = fs.readFileSync(
+  path.join(repositoryRoot, ".github", "workflows", "claude-review.yml"),
+  "utf8",
+);
+
+const COMPOSITE_PATH = ".github/actions/claude-lane-outcome/action.yml";
+
+function stepSource(stepName) {
+  const start = workflow.indexOf(`      - name: ${stepName}\n`);
+  assert.notEqual(start, -1, `step not found: ${stepName}`);
+  const rest = workflow.slice(start + 1);
+  const next = rest.indexOf("\n      - name: ");
+  return next === -1 ? rest : rest.slice(0, next);
+}
+
+// Every output name the composite's action.yml declares. Indentation-anchored
+// to the `outputs:` block's two-space keys so step ids and input names never
+// leak into the set.
+function declaredOutputs(actionYaml) {
+  const block = actionYaml.slice(
+    actionYaml.indexOf("\noutputs:") + 1,
+    actionYaml.indexOf("\nruns:"),
+  );
+  return new Set(
+    [...block.matchAll(/^ {2}([\w-]+):/gmu)].map((match) => match[1]),
+  );
+}
+
+const consumedOutputs = new Set(
+  [...workflow.matchAll(/steps\.review-outcome\.outputs\.([\w-]+)/gu)].map(
+    (match) => match[1],
+  ),
+);
+
+test("the count upsert and stale-comment clear gate on a review that RAN", () => {
+  // review-failed == 'false' is also true of a validation skip, which
+  // reviewed nothing; only review-ran distinguishes a review that happened.
+  for (const stepName of [
+    "Update the review-count status comment",
+    "Clear stale failure comment after successful review",
+  ]) {
+    assert.match(
+      stepSource(stepName),
+      /^ {8}if: "!cancelled\(\) && steps\.review-outcome\.outputs\.review-ran == 'true'"$/mu,
+      `'${stepName}' must gate on review-ran, not on not-failed`,
+    );
+  }
+});
+
+test("every consumed outcome output is declared by the composite's source", () => {
+  assert.ok(consumedOutputs.size > 0, "the lane must consume outcome outputs");
+  const declared = declaredOutputs(
+    fs.readFileSync(path.join(repositoryRoot, COMPOSITE_PATH), "utf8"),
+  );
+  for (const name of consumedOutputs) {
+    assert.ok(
+      declared.has(name),
+      `the lane reads steps.review-outcome.outputs.${name}, which ${COMPOSITE_PATH} does not declare`,
+    );
+  }
+});
+
+test("every consumed outcome output is declared at the pinned revision, when reachable", (t) => {
+  // The tree check above cannot see the one divergence that matters: a tree
+  // that declares an output the PINNED revision predates. Reading the
+  // composite at the pin closes that hole — but only where the pinned object
+  // exists locally (a shallow CI checkout does not carry history), so an
+  // unreachable pin skips rather than fails: this half is a local/deep-clone
+  // guard, and the tree check above is the floor that always runs.
+  const pin = workflow.match(
+    /uses: melodic-software\/ci-workflows\/\.github\/actions\/claude-lane-outcome@([0-9a-f]{40})/u,
+  );
+  assert.ok(pin, "the outcome composite must be SHA-pinned");
+  let pinnedYaml;
+  try {
+    pinnedYaml = execFileSync(
+      "git",
+      ["-C", repositoryRoot, "show", `${pin[1]}:${COMPOSITE_PATH}`],
+      { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] },
+    );
+  } catch {
+    t.skip(`pinned revision ${pin[1]} is not reachable in this clone`);
+    return;
+  }
+  const declared = declaredOutputs(pinnedYaml);
+  for (const name of consumedOutputs) {
+    assert.ok(
+      declared.has(name),
+      `the lane reads steps.review-outcome.outputs.${name}, which the composite at pinned ${pin[1]} does not declare — repoint the pin and the gates in the same commit`,
+    );
+  }
+});

.github/workflows/claude-review.yml

@@ -384,7 +384,7 @@
       # persist-credentials:false makes that fetch fail and leaves an orphan
       # tracking comment. The final strip step below scrubs the persisted token
       # to restore zizmor `artipacked` compliance. Reversible when #1236 lands.
       - name: Check out
         if: steps.freshness.outputs.superseded != 'true' && steps.review-count.outputs.capped != 'true'
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
@@ -398,7 +398,7 @@
         if: >-
           inputs.standards-ref != '' && steps.freshness.outputs.superseded != 'true' &&
           steps.review-count.outputs.capped != 'true'
         uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
         with:
           client-id: ${{ secrets.STANDARDS_REVIEW_APP_ID }}
           private-key: ${{ secrets.STANDARDS_REVIEW_APP_PRIVATE_KEY }}
@@ -801,7 +801,7 @@
         if: >-
           !cancelled() && steps.freshness.outputs.superseded != 'true' &&
           steps.review-count.outputs.capped != 'true'
-        uses: melodic-software/ci-workflows/.github/actions/claude-lane-outcome@b5d54bf7cb386b1f2c35426c6c5fb8d1686671bd # b5d54bf 2026-07-27
+        uses: melodic-software/ci-workflows/.github/actions/claude-lane-outcome@7415d4e8e33564b7997763805583e5ad1b062dbe # 7415d4e 2026-08-08
         with:
           outcome: ${{ steps.attempt.outputs.outcome }}
           execution-file: ${{ steps.attempt.outputs.execution_file }}
@@ -838,9 +838,13 @@
       # Symmetric counterpart to the failure step above: a transient infra
       # failure (e.g. an OIDC blip) can leave the marker comment behind on the
       # PR. Once a retry (rerun or new push) succeeds, clear that stale
-      # warning so it is not mistaken for a still-open problem.
+      # warning so it is not mistaken for a still-open problem. Gated on
+      # review-ran, not review-failed == 'false': a validation skip is also
+      # "not failed", but it reviewed nothing, and letting it clear the
+      # warning would erase the one PR-visible signal that review has not
+      # been happening.
       - name: Clear stale failure comment after successful review
-        if: "!cancelled() && steps.review-outcome.outputs.review-failed == 'false'"
+        if: "!cancelled() && steps.review-outcome.outputs.review-ran == 'true'"
         # Tidying a stale comment is housekeeping, and a rejected API call
         # during it must not turn a successful review red.
         continue-on-error: true
@@ -850,11 +854,14 @@
           mode: clear-on-success
 
       # The visible counter the review-count gate reads. Upserted only after a
-      # successful review (review-failed == 'false' implies the outcome step
-      # ran, so the run was neither superseded nor capped), so failed and
-      # skipped runs never inflate the count.
+      # review that actually RAN (review-ran == 'true' implies the outcome
+      # step ran, so the run was neither superseded nor capped), so failed
+      # runs, validation skips, and skipped runs never inflate the count: the
+      # comment is the human "was this reviewed" signal and it charges the
+      # max-reviews-per-pr budget, so counting a run that reviewed nothing
+      # both misinforms the reader and burns allowance on nothing.
       - name: Update the review-count status comment
-        if: "!cancelled() && steps.review-outcome.outputs.review-failed == 'false'"
+        if: "!cancelled() && steps.review-outcome.outputs.review-ran == 'true'"
         # Housekeeping: a rejected API call must not redden a successful
         # advisory review.
         continue-on-error: true