fix(claude-review): post no review-count comment when the cap is disabled

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

@@ -148,3 +148,88 @@ for (const lane of LANES) {
     }
   });
 }
+
+// Runs a review-count step's github-script body against a recording client.
+const AsyncFunction = Object.getPrototypeOf(async () => {}).constructor;
+
+async function runCountStep(stepName, env) {
+  const source = stepSource(laneSource("claude-review.yml"), stepName);
+  const lines = source
+    .slice(source.indexOf("script: |\n") + "script: |\n".length)
+    .split("\n");
+  const end = lines.findIndex(
+    (line) => line !== "" && !line.startsWith(" ".repeat(12)),
+  );
+  const script = lines
+    .slice(0, end === -1 ? undefined : end)
+    .map((line) => line.slice(12))
+    .join("\n");
+  const calls = [];
+  const outputs = {};
+  const record = (name) => async () => {
+    calls.push(name);
+    return { data: {} };
+  };
+  const github = {
+    paginate: async () => {
+      calls.push("listComments");
+      return [];
+    },
+    rest: {
+      issues: {
+        listComments: record("listComments"),
+        createComment: record("createComment"),
+        updateComment: record("updateComment"),
+      },
+    },
+  };
+  const core = {
+    setOutput: (name, value) => {
+      outputs[name] = value;
+    },
+    info: () => {},
+    notice: () => {},
+    warning: () => {},
+  };
+  const set = { PR_NUMBER: "42", ...env };
+  const previous = Object.fromEntries(
+    Object.keys(set).map((key) => [key, process.env[key]]),
+  );
+  Object.assign(process.env, set);
+  try {
+    await new AsyncFunction("core", "github", "context", script)(core, github, {
+      repo: { owner: "melodic-software", repo: "consumer" },
+    });
+  } finally {
+    for (const [key, value] of Object.entries(previous)) {
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
+  }
+  return { calls, outputs };
+}
+
+test("a disabled cap (0) neither reads nor posts the review-count comment", async () => {
+  const lookup = await runCountStep("Check the per-PR review count", {
+    MAX_REVIEWS_PER_PR: "0",
+  });
+  assert.deepEqual(lookup.calls, []);
+  assert.equal(lookup.outputs.capped, "false");
+  const upsert = await runCountStep("Update the review-count status comment", {
+    MAX_REVIEWS_PER_PR: "0",
+    PRIOR_COUNT: "0",
+  });
+  assert.deepEqual(upsert.calls, []);
+});
+
+test("a positive cap still reads and posts the review-count comment", async () => {
+  const lookup = await runCountStep("Check the per-PR review count", {
+    MAX_REVIEWS_PER_PR: "5",
+  });
+  assert.deepEqual(lookup.calls, ["listComments"]);
+  const upsert = await runCountStep("Update the review-count status comment", {
+    MAX_REVIEWS_PER_PR: "5",
+    PRIOR_COUNT: "0",
+  });
+  assert.deepEqual(upsert.calls, ["createComment"]);
+});

.github/workflows/claude-review.yml

@@ -261,9 +261,9 @@ on:
           Maximum successful reviews per PR before the lane name-stable-skips
           further runs (caps spend on long-lived PRs). Counted via a visible
           per-PR status comment ("Claude has reviewed this PR N times") that
-          is upserted after each successful review — the comment doubles as
-          the human "was this reviewed" signal. Deleting the comment resets
-          the count (fail-open by design). Zero or negative disables the cap.
+          is upserted after each successful review. Deleting the comment
+          resets the count (fail-open by design). Zero or negative disables
+          the cap and the comment: the lane neither reads nor posts it.
           Soft cap: concurrent runs for different heads read the counter
           before either writes it, so a burst can briefly exceed the cap by
           the number of concurrent heads.
@@ -567,7 +567,8 @@ jobs:
           pull-number: ${{ steps.resolve-pr.outputs.number }}
           head-sha: ${{ steps.resolve-pr.outputs.head-sha }}
 
-      # Review-count cap (max-reviews-per-pr). The counter is the visible
+      # Review-count cap (max-reviews-per-pr). A disabled cap (<= 0) skips the
+      # lookup, so the lane never reads or writes the comment. The counter is the visible
       # status comment this lane upserts after each successful review — chosen
       # over the runs API (which counts every advisory success including skips
       # and needs an `actions: read` grant no caller makes) and over a hidden
@@ -590,10 +591,7 @@ jobs:
             const prNumber = Number(process.env.PR_NUMBER);
             core.setOutput("capped", "false");
             core.setOutput("count", "0");
-            if (!Number.isInteger(prNumber) || prNumber <= 0) return;
-            // The lookup runs even when the cap is disabled: the upsert step
-            // reads count and comment-id from here, so skipping it would
-            // degrade the upsert to an insert-per-review.
+            if (!capEnabled || !Number.isInteger(prNumber) || prNumber <= 0) return;
             let count = 0;
             try {
               const comments = await github.paginate(github.rest.issues.listComments, {
@@ -620,7 +618,7 @@ jobs:
               return;
             }
             core.setOutput("count", String(count));
-            if (capEnabled && count >= max) {
+            if (count >= max) {
               core.notice(
                 `Skipping review: this PR already received ${count} reviews ` +
                   `(max-reviews-per-pr=${max}). Delete the count comment to re-enable.`,
@@ -1345,7 +1343,8 @@ jobs:
           mode: clear-on-success
           pull-number: ${{ steps.resolve-pr.outputs.number }}
 
-      # The visible counter the review-count gate reads. Upserted only after a
+      # The visible counter the review-count gate reads, posted only while the
+      # cap is enabled (max-reviews-per-pr > 0). Upserted only after a
       # 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
@@ -1366,6 +1365,11 @@ jobs:
           HEAD_REPO: ${{ steps.resolve-pr.outputs.head-repo }}
         with:
           script: |
+            const max = Number(process.env.MAX_REVIEWS_PER_PR);
+            if (!(Number.isFinite(max) && max > 0)) {
+              core.info("max-reviews-per-pr is disabled; posting no count comment.");
+              return;
+            }
             const prNumber = Number(process.env.PR_NUMBER);
             if (!Number.isInteger(prNumber) || prNumber <= 0) {
               core.info("No pull request in this event; nothing to count.");
@@ -1380,14 +1384,10 @@ jobs:
               return;
             }
             const count = (Number(process.env.PRIOR_COUNT) || 0) + 1;
-            const max = Number(process.env.MAX_REVIEWS_PER_PR);
-            const capNote =
-              Number.isFinite(max) && max > 0
-                ? ` The lane skips further automatic reviews after ${max}; deleting this comment resets the count.`
-                : " Deleting this comment resets the count.";
             const body =
               `<!-- claude-review-count:${count} -->\n` +
-              `Claude has reviewed this PR ${count} time${count === 1 ? "" : "s"}.${capNote}`;
+              `Claude has reviewed this PR ${count} time${count === 1 ? "" : "s"}. ` +
+              `The lane skips further automatic reviews after ${max}; deleting this comment resets the count.`;
             const commentId = Number(process.env.PRIOR_COMMENT_ID);
             try {
               if (Number.isInteger(commentId) && commentId > 0) {

README.md

@@ -896,9 +896,9 @@ spent — so they do not retry on one.
 **Review-count cap (code-review lane only).** `claude-review.yml` stops
 reviewing a PR after `max-reviews-per-pr` successful reviews, capping spend on
 long-lived PRs. The counter is a **visible** per-PR status comment upserted
-after each successful review — failed and skipped runs never inflate it — which
-doubles as the human "was this reviewed" signal;
-deleting it resets the count, which is fail-open by design. A capped run is a
+after each successful review — failed and skipped runs never inflate it;
+deleting it resets the count, which is fail-open by design. A cap of zero or
+less disables both the cap and the comment. A capped run is a
 name-stable skip, not a red check. Treat it as a soft cap: concurrent runs for
 different heads read the counter before either writes it, so a burst can
 briefly exceed it by the number of concurrent heads.