fix(ci): cover the third way the trusted copy dirties the tree

Round 2 of the review loop found the case the previous commit missed. The
tree-cleanliness fix handled untracked `.review-tooling/` and tracked
`.claude` files the copy modifies or deletes, but not files the trusted copy
lands that are NOT TRACKED at the PR head — every `.claude/**` file added
after that branch was cut. `git diff --name-only` never lists untracked
paths, so the assume-unchanged pass structurally cannot reach them.

This fires on this PR's own merge: `next` tracks none of `.claude/review.yml`
or the two agents, so every already-open PR would get three `??` entries, a
dirty tree, and a review silently downgraded to diff-only extraction. Fixed
by also excluding `/.claude/`, which hides untracked copies while still
reporting tracked modifications — verified in a scratch repo reproducing the
bug, the fix, and that the assume-unchanged pass is still required.

Also from round 2:

- Document that restoring a clean tree re-enables review-cli's
  `verifyFindings` pass, dead in CI until now because it is gated on
  workspaceShape == 'working-tree'. It resolves cited files from the
  workspace, where `.claude` is the pre-PR copy, so findings against
  `.claude/**` files a PR adds are dropped as "file not readable at HEAD".
  Logged rather than silent, reachable only by reviewer-tuning PRs, and not
  to be "fixed" by skipping the swap.
- Correct the `!cancelled()` rationale. It claimed a successor replaces the
  👀; none does, because only a `pull_request` run can cancel this one and
  such a run has an empty comment_id, so it never reaches that step. The
  stale ack is the deliberate trade against a false ❌.
- Fix a wrong claim in triage.guidance: `auto-merge-dependabot` gates on the
  job RESULT, not the verdict, and `post` has no non-zero exit path, so
  REQUEST_CHANGES does not by itself stop a bump. Reviewers are told to say
  so explicitly in the finding instead.

Verified: actionlint clean, zizmor unchanged at 8, review.yml parses.

Author: wkoutre

.claude/review.yml

@@ -166,8 +166,12 @@ triage:
 
     - dependency-upgrade-reviewer — dependabot/renovate PRs and any diff
       touching package.json, bun.lock, or bunfig.toml. Dependency PRs are
-      reviewed here (not skipped) because auto-merge gates on the review,
-      so this reviewer's verdict decides whether a bump lands.
+      reviewed here rather than skipped because `auto-merge-dependabot`
+      needs this job to run and succeed before it enables auto-merge, and
+      because a bump nobody reads is the whole risk. Be accurate rather
+      than lenient, but note the gate is the job RESULT, not the verdict:
+      REQUEST_CHANGES does not by itself stop a bump, so say plainly in the
+      finding when a bump should not land.
 
     - patterns-reviewer — non-trivial TypeScript under .github/scripts/
       or packages/, especially anything that would become a shared

.github/workflows/CLAUDE.md

@@ -544,10 +544,13 @@ That hits the two reviewers this repo adds hardest, because both are told to loo
 Two mitigations, both needed:
 
 - `echo '/.review-tooling/' >> .git/info/exclude` hides the untracked tooling directory.
+- `echo '/.claude/' >> .git/info/exclude` for the files the trusted copy lands that are **not tracked at the PR head** — every `.claude/**` file added after that branch was cut. `git diff --name-only` never lists untracked paths, so the next mitigation structurally cannot reach these. This one bites hardest on the merge of the PR that introduces `.claude/` at all, because every already-open PR then gets `??` entries.
 - `git update-index --assume-unchanged` on the `.claude` paths the trusted copy reverted. `.git/info/exclude` cannot hide **tracked** files, and swapping in the trusted `.claude` makes any PR that edits `.claude/**` dirty — which would re-trigger the fallback for exactly the PRs most likely to be tuning the reviewers.
 
 The step ends by asserting `git status --porcelain` is empty and warns if it is not, because the failure is otherwise undetectable from the outside.
 
+**A second-order consequence of making that work, accepted deliberately.** review-cli's post-synthesis `verifyFindings` pass is gated on `workspaceShape == 'working-tree'`, so it was effectively dead in this repo's CI while the tree was always dirty. With the tree clean it runs, and it resolves cited files from the workspace — where `.claude` is now the pre-PR copy. On a PR that _adds_ a `.claude/**` file, a finding against that file is dropped as "file not readable at HEAD"; on one that lengthens a file, a finding past the trusted copy's EOF is dropped as "beyond file end". Both drops are logged rather than silent, and only reviewer-tuning PRs can reach them. Do not "fix" this by skipping the swap: that hands config and agent prompts back to the PR author, which is the trust inversion the swap exists to close. A real carve-out needs an upstream change.
+
 **Steps that shell out to the CLI are gated on `steps.install-review-cli.outputs.bin-path != ''`.** That output is empty whenever the install step never ran, which is exactly what happens when an earlier step fails. Without the gate, `Post` and the reaction/reply steps still execute, resolve `"$REVIEW_CLI_BIN/review-cli"` to `/review-cli`, and fail with exit 127 — replacing the real error in the log with a meaningless one.
 
 **Gotcha — the `triage` gate must read `.claude/review.yml`.** review-cli's upstream workflow template runs the gate with `--skip-config` to avoid a checkout. Do not copy that here. `--skip-config` passes **no** policy, which is not the same as "the CLI's built-in defaults":

.github/workflows/claude-code-review.yml

@@ -487,6 +487,17 @@ jobs:
           # sibling workflows), so the degradation is invisible and total.
           echo '/.review-tooling/' >> .git/info/exclude
 
+          # `/.claude/` too, for the third way this step dirties the tree.
+          # The trusted copy lands files that may not be TRACKED at the PR
+          # head — every `.claude/**` file added after that branch was cut,
+          # which on this migration's merge day means review.yml and both
+          # agents for every already-open PR. Those arrive as `??` entries,
+          # and `git diff --name-only` never lists untracked paths, so the
+          # assume-unchanged pass below structurally cannot reach them.
+          # Excluding the directory hides only untracked files; tracked
+          # modifications still show, so that pass is still needed.
+          echo '/.claude/' >> .git/info/exclude
+
           for f in review.yml \
                    agents/workflow-security-reviewer.md \
                    agents/plugin-conventions-reviewer.md; do
@@ -515,6 +526,18 @@ jobs:
               sys.exit('model.default missing — every reviewer would fall back to the CLI default model')
           PY
 
+          # Known consequence of the swap, accepted: restoring the clean tree
+          # also re-enables review-cli's post-synthesis `verifyFindings` pass,
+          # which is gated on workspaceShape == 'working-tree' and was
+          # therefore dead in CI while the tree was always dirty. It resolves
+          # cited files from this workspace, where `.claude` is now the
+          # pre-PR copy — so on a PR that ADDS a `.claude/**` file, a finding
+          # against it is dropped as "file not readable at HEAD", and on one
+          # that lengthens a file, a finding past the trusted copy's EOF is
+          # dropped as "beyond file end". Both drops are logged, not silent,
+          # and only reviewer-tuning PRs can hit them. A carve-out needs an
+          # upstream change; do not "fix" it by skipping the swap, which
+          # would hand config and agent prompts back to the PR author.
           rm -rf .claude
           cp -R .review-tooling/.claude .claude
 
@@ -780,7 +803,14 @@ jobs:
       # success/failure branch below would render as ❌. Concurrency cancels
       # this run whenever a push supersedes a comment-triggered review, so
       # without this the requester gets a ❌ on a review that a successor is
-      # about to complete. The successor owns the terminal signal; stay quiet.
+      # about to complete.
+      #
+      # Note what skipping does NOT do: no successor replaces the 👀. Only a
+      # `pull_request` run can cancel this one (`cancel-in-progress` is scoped
+      # to that event), and such a run has an empty `comment_id`, so it never
+      # reaches this step. The ack stays until someone clears it. That is the
+      # deliberate trade — a stale 👀 is better than a false ❌, and the reply
+      # step below carries the real "Superseded" signal.
       - name: Update reaction (✅ / ❌) on comment trigger
         if: |
           always() && !cancelled() &&