fix(ci): post no comment when no scope matches the changed files

In routed aggregate ("single") mode, ecr ci posted an empty approve
comment with a zero-row scope table on every PR whose changed files
matched no scope. Now it posts nothing and deletes a stale aggregate
comment from an earlier run instead. The unmatched-files warning stays
in the job log. Per-scope mode already behaved this way.

The partial-run merge moves into an exported pure mergePartialAggregate
so the empty/carry-over boundary is unit-tested.

llp/0007-cli-commands-and-ci.explainer.md

@@ -51,9 +51,9 @@ One directory subtlety is load-bearing: the run-log / patch-workspace directory
 
 Whether a PR gets reviewed at all is a config-not-YAML decision. The trigger policy (`skipLabel` / `label` / `all`) lives in `config.jsonc`'s `review.*` block, not in per-repo workflow YAML, because labels are write-gated to maintainers while `config.jsonc` historically could be read from the PR ref — keeping the skip decision immune to a PR author editing config to dodge review [observed: commit `1e9aea2`].
 
-Label matching is exact-string, never substring: `ai-review:skip` must not satisfy an `ai-review` opt-in check. `shouldReview` compares whole label names (and a `label:<agent>` prefix variant), a fix for an earlier `join()`+substring form [observed: commit `1e9aea2`; `src/commands/ci.ts:690-706` `shouldReview`; test `src/__tests__/ci-trigger.test.ts`]. Labels are re-fetched live via `gh pr view` rather than trusted from the possibly-stale event payload, and a fetch failure returns `[]` so a label-read hiccup can never silently SKIP a PR that should be reviewed — the default leans toward reviewing [observed: `src/commands/ci.ts:648-652,653-681`].
+Label matching is exact-string, never substring: `ai-review:skip` must not satisfy an `ai-review` opt-in check. `shouldReview` compares whole label names (and a `label:<agent>` prefix variant), a fix for an earlier `join()`+substring form [observed: commit `1e9aea2`; `src/commands/ci.ts:720-736` `shouldReview`; test `src/__tests__/ci-trigger.test.ts`]. Labels are re-fetched live via `gh pr view` rather than trusted from the possibly-stale event payload, and a fetch failure returns `[]` so a label-read hiccup can never silently SKIP a PR that should be reviewed — the default leans toward reviewing [observed: `src/commands/ci.ts:678-682,683-711`].
 
-A manual `/review` (the `--force` flag, or `GITHUB_EVENT_NAME === "issue_comment"`, which only reaches `ci` through the comment-command workflow) bypasses ONLY the trigger gate. Break-glass and the auth lock are separate checks that still apply [observed: `src/commands/ci.ts:716-721` `shouldBypassTriggerGate`, `730-747` `passesTriggerGate`; commit `69b841c`].
+A manual `/review` (the `--force` flag, or `GITHUB_EVENT_NAME === "issue_comment"`, which only reaches `ci` through the comment-command workflow) bypasses ONLY the trigger gate. Break-glass and the auth lock are separate checks that still apply [observed: `src/commands/ci.ts:746-751` `shouldBypassTriggerGate`, `760-777` `passesTriggerGate`; commit `69b841c`].
 
 Break-glass is checked exactly once per CI run, on the root/aggregate comment marker, before any scope fan-out begins — not per-scope [observed: `src/commands/ci.ts:490-508`]. One check keeps the GitHub API calls down (rate-limit hygiene) and keeps the opt-out semantics repo-wide rather than per-team.
 
@@ -70,7 +70,9 @@ The gating and identity rules are all "root config wins":
 Two rules protect against the fan-out silently losing or mis-timing work:
 
 - The passes budget divides across active scopes with a per-scope floor (`minScopeMinutes`). Scopes run sequentially, so N × floor is real wall-clock time; when the floor forces the total past `totalPassesMinutes`, the floor is kept and a loud warning is emitted, not a hard failure [observed: `src/commands/ci.ts:527-539`; commit `08026ea`].
-- A partial `--scopes` re-run in aggregate ("single") mode is authoritative ONLY for the named scopes: it merges the other scopes' prior results out of the existing aggregate comment's stored state, so re-running one scope does not wipe the rest from the PR [observed: `src/commands/ci.ts:590-608`]. Mode-switch cleanup (deleting the other comment mode's stale comments) runs once on a full run but is skipped entirely for a partial run, which must never touch other scopes' live comments [observed: `src/commands/ci.ts:609-617,637-643`].
+- A partial `--scopes` re-run in aggregate ("single") mode is authoritative ONLY for the named scopes: it merges the other scopes' prior results out of the existing aggregate comment's stored state, so re-running one scope does not wipe the rest from the PR [observed: `src/commands/ci.ts:593-604,661-679`]. Mode-switch cleanup (deleting the other comment mode's stale comments) runs once on a full run but is skipped entirely for a partial run, which must never touch other scopes' live comments [observed: `src/commands/ci.ts:615-622,644-650`].
+
+When no scope has anything to report — no changed file matched any scope, and (on a partial run) no prior scope results carry over from the stored aggregate state — the aggregate ("single") mode posts NOTHING: it deletes any stale aggregate comment from an earlier run instead of posting an empty "approve" review with a zero-row scope table [observed: `src/commands/ci.ts:605-616`, gate on `finalResults.length === 0` → `aggregate.clear()`]. A review of nothing is not a review, and a repo whose `routing.jsonc` deliberately scopes only part of the tree (no `**/*` catch-all) would otherwise get a contentless comment on every out-of-scope PR. The unmatched-files warning still goes to stderr in the job log, so coverage gaps stay visible to maintainers without notifying the PR author. Per-scope mode already behaved this way (a scope with no result gets its comment cleared, and no comment is ever created).
 
 A scope whose config directory does not exist at the TRUSTED base commit — i.e. it is new in this very PR — is reviewed with the ROOT config instead of failing the whole run; the scope's own (PR-authored, untrusted-for-this-run) config activates only after merge [observed: `src/commands/ci.ts:541-559` `hasScopeConfig` fallback; commit `601b19a`; test `src/__tests__/trust-separation.test.ts`]. The known cost: a scope that is genuinely misconfigured (missing `config.jsonc` by mistake) is indistinguishable from an intentionally-new one — both fall back to the root config with a stderr note, never a hard CI failure.
 

src/__tests__/ci-aggregate.test.ts

@@ -0,0 +1,49 @@
+import { test, expect } from "bun:test";
+
+import { mergePartialAggregate } from "../commands/ci.js";
+import type { ScopeReviewResult } from "../core/render.js";
+
+function result(scope: string, summary = `review of ${scope}`): ScopeReviewResult {
+  return {
+    scope,
+    isDefault: false,
+    review: { decision: "approve", findings: [], summary, incomplete: [] },
+  };
+}
+
+const NAMES = ["api", "web", "docs"];
+
+test("fresh results win over prior state for the scopes in the filter", () => {
+  const merged = mergePartialAggregate(
+    [result("api", "fresh")],
+    [result("api", "stale"), result("web")],
+    ["api"],
+    NAMES,
+  );
+  expect(merged.map((entry) => entry.scope)).toEqual(["api", "web"]);
+  expect(merged[0]!.review.summary).toBe("fresh");
+});
+
+test("a filtered scope with no fresh result drops (the partial run is authoritative for it)", () => {
+  const merged = mergePartialAggregate([], [result("api", "stale")], ["api"], NAMES);
+  expect(merged).toEqual([]);
+});
+
+test("prior results outside the filter carry over so nothing posted is not lost", () => {
+  const merged = mergePartialAggregate([], [result("web"), result("docs")], ["api"], NAMES);
+  expect(merged.map((entry) => entry.scope)).toEqual(["web", "docs"]);
+});
+
+test("results come back in manifest order; scopes no longer in the manifest drop out", () => {
+  const merged = mergePartialAggregate(
+    [result("docs"), result("api")],
+    [result("removed-scope")],
+    ["api", "docs"],
+    NAMES,
+  );
+  expect(merged.map((entry) => entry.scope)).toEqual(["api", "docs"]);
+});
+
+test("no fresh results and no prior state merge to empty (ci posts nothing for it)", () => {
+  expect(mergePartialAggregate([], [], ["api"], NAMES)).toEqual([]);
+});

src/commands/ci.ts

@@ -595,18 +595,23 @@ async function runRoutedCi(
       // the other scopes' previous results out of the existing aggregate comment's
       // state so re-running one scope doesn't silently discard the rest.
       const prior = (await aggregate.readState())?.scopes ?? [];
-      const byName = new Map(results.map((result) => [result.scope, result]));
-      for (const previous of prior) {
-        if (!scopesFilter.includes(previous.scope) && !byName.has(previous.scope)) {
-          byName.set(previous.scope, previous);
-        }
-      }
-      // Manifest order; scopes no longer in the manifest drop out.
-      finalResults = manifest.scopes
-        .map((scope) => byName.get(scope.name))
-        .filter((entry): entry is ScopeReviewResult => entry != null);
+      finalResults = mergePartialAggregate(
+        results,
+        prior,
+        scopesFilter,
+        manifest.scopes.map((scope) => scope.name),
+      );
+    }
+    if (finalResults.length === 0) {
+      // No scope has anything to report (no changed file matched any scope, and no
+      // prior scope results carry over): a review of nothing is not a review, so
+      // post nothing and delete a stale aggregate comment from an earlier run
+      // instead of leaving it up. The unmatched-files warning stays in the job log.
+      // @ref LLP 0007#routed-ci-fan-out — zero active scopes → no comment
+      await aggregate.clear();
+    } else {
+      await aggregate.reportAggregate(finalResults, resolution.unmatched);
     }
-    await aggregate.reportAggregate(finalResults, resolution.unmatched);
     // Clean up any per-scope comments from a previous per-scope run. A partial run
     // only ever touches the named scopes' comments.
     for (const scope of manifest.scopes) {
@@ -616,7 +621,9 @@ async function runRoutedCi(
       await reporterFor(scopedCommentTag(rootTag, scope.name)).clear();
     }
     process.stderr.write(
-      `CI reviewer: posted aggregate review for ${finalResults.length} scope(s).\n`,
+      finalResults.length === 0
+        ? "CI reviewer: no scope matched the changed files; nothing posted.\n"
+        : `CI reviewer: posted aggregate review for ${finalResults.length} scope(s).\n`,
     );
   } else {
     for (const scope of manifest.scopes) {
@@ -645,6 +652,29 @@ async function runRoutedCi(
   }
 }
 
+/**
+ * Merge a partial run's fresh results (--scopes) with the prior aggregate
+ * comment's stored state, in manifest order. Fresh results win for their scope;
+ * scopes outside the filter keep their previous result; scopes no longer in the
+ * manifest drop out. Pure so it's unit-testable.
+ */
+export function mergePartialAggregate(
+  results: ScopeReviewResult[],
+  prior: ScopeReviewResult[],
+  scopesFilter: string[],
+  manifestScopeNames: string[],
+): ScopeReviewResult[] {
+  const byName = new Map(results.map((result) => [result.scope, result]));
+  for (const previous of prior) {
+    if (!scopesFilter.includes(previous.scope) && !byName.has(previous.scope)) {
+      byName.set(previous.scope, previous);
+    }
+  }
+  return manifestScopeNames
+    .map((name) => byName.get(name))
+    .filter((entry): entry is ScopeReviewResult => entry != null);
+}
+
 /**
  * Current PR labels via gh (more authoritative than the possibly-stale event
  * payload); on failure, returns [] so a label-read hiccup never silently skips a