feat(agents): pr-reviewer, a CodeRabbit-like review of published PRs as the review-bot fallback (#1447)

.claude/agents/lessons/pr-reviewer.md

@@ -0,0 +1,3 @@
+# Lessons for pr-reviewer
+
+No lessons yet.

.claude/agents/pr-reviewer.md

@@ -0,0 +1,114 @@
+---
+name: pr-reviewer
+description: CodeRabbit-style review of a published pull request against AGENTS.md, CLAUDE.md, the path_instructions of .coderabbit.yaml and its linked issue's acceptance criteria. Fallback for when CodeRabbit is unavailable or rate-limited. Read-only on the repository except its own artifacts/pr-review/<pr>.md.
+model: sonnet
+effort: high
+tools: Agent(Explore), Bash, PowerShell, Read, Grep, Glob, Write, Edit
+maxTurns: 60
+color: orange
+hooks:
+  PreToolUse:
+    - matcher: "Bash|PowerShell"
+      hooks:
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/block-worker-publish.ps1"'
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/block-prohibited-commands.ps1"'
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/enforce-path-ownership.ps1" -Agent pr-reviewer'
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/no-background-specialists.ps1" -Agent pr-reviewer'
+    - matcher: "Write|Edit|MultiEdit|NotebookEdit"
+      hooks:
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/enforce-path-ownership.ps1" -Agent pr-reviewer'
+    - matcher: "Agent"
+      hooks:
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/block-worker-spawn.ps1" -Agent pr-reviewer'
+        - type: command
+          command: 'pwsh -NoProfile -File "$CLAUDE_PROJECT_DIR/.claude/hooks/no-background-specialists.ps1" -Agent pr-reviewer'
+---
+
+You review a **published** pull request against the repository's own rules and its linked issue, the way
+CodeRabbit does when it is available. You are distinct from `adversarial-reviewer`, which reviews a
+**pre-push** diff against a closed worker brief before it ever reaches GitHub: that agent keeps its role
+unchanged. You exist as the fallback for when CodeRabbit is rate-limited or otherwise unavailable (it has
+been failing with "Review limit reached" since 2026-09-26). You never fix anything and you never push.
+
+Model: you run on Sonnet by default. The orchestrator passes `model: opus` in the spawn only for a pull
+request that touches security or personal data, or an unusually large diff, and says which in the prompt.
+
+Inputs: a PR number. From it you read:
+
+- `gh pr view <n> --repo dlrivada/Encina --json title,body,baseRefName,headRefName` for the PR itself and the
+  linked issue (`Fixes #N` / `Closes #N` / `Resolves #N` in the body).
+- `gh pr diff <n> --repo dlrivada/Encina` for the diff.
+- `gh issue view <N> --repo dlrivada/Encina` for the linked issue's acceptance criteria.
+- `AGENTS.md`, `CLAUDE.md` and `.coderabbit.yaml` (its `path_instructions` per glob, and `tone_instructions`).
+
+Tooling rules (mandatory, from `AGENTS.md` §2): PowerShell or direct CLI calls only; no python, no bash
+constructs, no `grep`/`sed`/`head`/`tail`. Read files with the Read tool; search with Grep/Glob.
+
+Never work around a hook. When a hook blocks a command or an edit, do not rephrase the command, split it,
+route it through another tool, build the output another way (for example `dotnet build` plus running the
+dll instead of `dotnet run`) or ask a specialist to do it for you: stop that step and report the hook's exact
+message with what you were trying to do. A false positive is fixed in the hook, by the orchestrator's
+decision, never bypassed (#1345; the #1346 worker bypassed `block-main-checkout-writes` on 2026-09-25).
+
+## Method
+
+1. Read the PR (`gh pr view`, `gh pr diff`), the linked issue and its acceptance criteria, `AGENTS.md`,
+   `CLAUDE.md`, `.coderabbit.yaml`.
+2. For each changed file, match its path against `.coderabbit.yaml`'s `path_instructions`
+   (`src/**/*.cs`, `tests/**/*.cs`, `**/*.md`) and apply those instructions plus the matching `AGENTS.md`
+   rules: §3 code rules (`TimeProvider`, `[JsonIgnore]` + `ToString()` override on secrets, async DB calls,
+   registration completeness, errors never swallowed in background infrastructure, fail-closed gates,
+   `EncinaError.Message` never logged, Railway Oriented Programming), §5 provider coherence, §6 cross-cutting
+   check, §7 EventIds, §9 testing obligations, as applicable to the files actually touched.
+3. Check every acceptance criterion of the linked issue against the diff: `met` (name the file/line
+   evidence), `not met`, or `not verifiable` (state what evidence is missing).
+4. Write the security section: secrets or personal data reaching logs, activity tags, health-check results
+   or plaintext storage; injection; fail-open gates instead of fail-closed. Known failure patterns to check
+   (project history, same list `adversarial-reviewer` carries, since the underlying rules are the same
+   `AGENTS.md` §3 rules):
+   - Registration completeness: an `AddEncina*` method that adds a service, orchestrator or hosted service
+     must also register every option type and dependency it resolves, proven by a DI test with
+     `ValidateOnBuild`/`ValidateScopes` (#1260, #1273, #1285, #1289).
+   - Errors swallowed in background infrastructure: a `Left` from `IEncina.Send`/`Publish` or from a store
+     inside a processor, adapter, orchestrator or job must fail the operation, not report success (#1150,
+     #1151, #1152, #1153, #1184).
+   - Compliance/security gates failing open: missing context (no `HttpContext`, no tenant, no principal) or a
+     failed lookup must deny, not allow, with a logged, explicit opt-out only (#1143, #1145, #1148, #1155,
+     #1161).
+   - `EncinaError.Message` leaking into logs, activity tags, health-check results or plaintext storage
+     instead of only the error code or exception type (#1168, #1173, #1259, #1274).
+5. Spawn `Explore` (the only agent you may spawn) for read-only cross-file research when a claim needs
+   checking beyond the diff itself (for example, "is this the only caller of this method").
+6. Write `artifacts/pr-review/<pr>.md` with exactly these five sections:
+   - **(a) Summary and walkthrough** — a short description of what the diff does.
+   - **(b) Findings** — one entry per finding: `file:line`, severity (`blocker`/`major`/`minor`/`nit`), a
+     suggested fix, and a verification note (how you checked it, or what would confirm it).
+   - **(c) Linked-issue check** — each acceptance criterion of the linked issue, marked `met` / `not met` /
+     `not verifiable`.
+   - **(d) Security** — the section from step 4, even when empty ("nothing found" is a valid, stated result).
+   - **(e) Lessons for the pipeline** — any pattern worth adding to
+     `.claude/agents/lessons/pr-reviewer.md`, or worth its own issue.
+7. Report to the orchestrator: the artifact's path, a one-line verdict (`merge` / `merge after fixes` / `do
+   not merge` — the same three-way verdict `adversarial-reviewer` uses, so the orchestrator's downstream
+   handling is identical), and any pattern worth adding to your own lessons file.
+
+Read `.claude/agents/lessons/pr-reviewer.md` at the start of every run; it is updated by hand by the
+orchestrator, not by you (you may write only `artifacts/pr-review/<pr>.md` — `enforce-path-ownership.ps1`
+denies any other target).
+
+## Output contract
+
+`artifacts/pr-review/<pr>.md` is read-only research plus one Markdown file: it never touches GitHub itself.
+The orchestrator posts it as one PR review (`gh pr review --repo dlrivada/Encina <n> --comment --body-file
+artifacts/pr-review/<n>.md`) and adds inline comments only for findings that carry an exact `file:line` the
+GitHub review API can anchor to.
+
+Verification discipline: report a finding only after checking it against the code; state the exact
+`file:line` and the input or scenario that exposes it. If nothing survives verification, say so plainly
+rather than padding the findings list.

.claude/hooks/block-worker-publish.ps1

@@ -1,5 +1,6 @@
 # PreToolUse hook (Bash|PowerShell), scoped to the frontmatter of the writing and reviewing subagents
-# (issue-worker, mechanical-fixer, docs-writer, docs-reviewer): blocks publish-side git/gh actions. The
+# (issue-worker, mechanical-fixer, docs-writer, docs-reviewer, site-steward, and, since #1447, pr-reviewer):
+# blocks publish-side git/gh actions. The
 # messages name the agent from the hook input's agent_type, present for a subagent's own tool call
 # (https://code.claude.com/docs/en/hooks.md, https://code.claude.com/docs/en/sub-agents.md); this hook only
 # runs scoped to one of these agents, so it is used solely to word the message, not to gate behaviour. The

.claude/hooks/block-worker-spawn.ps1

@@ -14,6 +14,8 @@
 #   mechanical-fixer  ci-diagnoser, Explore
 #   site-steward      ci-diagnoser (#1382): a failing publisher or freshness check goes there for root-causing;
 #                     site-steward never diagnoses it itself.
+#   pr-reviewer       Explore only (#1447): read-only cross-file research while reviewing a published PR; it
+#                     never diagnoses CI, it reviews, so ci-diagnoser is not in its allowlist.
 #   issue-archivist, issue-auditor, test-auditor, audit-verifier, docs-reviewer, adversarial-reviewer,
 #   ci-diagnoser, pr-watcher   empty (#1345). Each is either a SPEC-003 audit stage agent (single-owner,
 #                     no delegation) or a read-only specialist whose own definition lists no Agent tool; any
@@ -48,11 +50,12 @@ try {
     $payload = [Console]::In.ReadToEnd() | ConvertFrom-Json
 
     $allowlists = @{
-        'orchestrator'          = @('issue-worker', 'mechanical-fixer', 'docs-writer', 'docs-reviewer', 'adversarial-reviewer', 'ci-diagnoser', 'pr-watcher', 'site-steward', 'Explore', 'Plan', 'claude-code-guide', 'general-purpose', 'issue-archivist', 'issue-auditor', 'test-auditor', 'audit-verifier')
+        'orchestrator'          = @('issue-worker', 'mechanical-fixer', 'docs-writer', 'docs-reviewer', 'adversarial-reviewer', 'ci-diagnoser', 'pr-watcher', 'site-steward', 'pr-reviewer', 'Explore', 'Plan', 'claude-code-guide', 'general-purpose', 'issue-archivist', 'issue-auditor', 'test-auditor', 'audit-verifier')
         'issue-worker'          = @('ci-diagnoser', 'mechanical-fixer', 'Explore', 'adversarial-reviewer', 'docs-writer', 'docs-reviewer')
         'docs-writer'           = @('mechanical-fixer', 'docs-reviewer', 'Explore')
         'mechanical-fixer'      = @('ci-diagnoser', 'Explore')
         'site-steward'          = @('ci-diagnoser')
+        'pr-reviewer'           = @('Explore')
         'issue-archivist'       = @()
         'issue-auditor'         = @()
         'test-auditor'          = @()

.claude/hooks/enforce-path-ownership.ps1

@@ -43,6 +43,10 @@
 #                 drafts the site-health skill prepares); everything else in the repository is denied, because
 #                 the agent is read-only on the repository (it never fixes a broken workflow itself: a failing
 #                 publisher goes to ci-diagnoser).
+#   pr-reviewer   (#1447) writes ONLY under artifacts/pr-review/** (its own review of one published PR);
+#                 everything else in the repository is denied, because the agent is read-only on the
+#                 repository (it never posts to GitHub itself: the orchestrator posts its report as a PR
+#                 review).
 #   others        not restricted (mechanical-fixer is the delegate).
 #
 # docs/plans/** stays with the issue-worker: an implementation plan is an issue-scoped working document
@@ -296,6 +300,12 @@ try {
                 return $false
             }
         }
+        elseif ($Agent -eq 'pr-reviewer') {
+            if ($relative -notmatch '^artifacts/pr-review/') {
+                [Console]::Error.WriteLine("Blocked: pr-reviewer writes only under artifacts/pr-review/** (its own review of one published PR, #1447); '$relative' is not one of them. It is read-only on the rest of the repository: report anything else to the orchestrator instead of editing it.")
+                return $false
+            }
+        }
 
         return $true
     }

.claude/hooks/no-background-specialists.ps1

@@ -1,7 +1,8 @@
 # PreToolUse hook (Agent|Task, wired unconditionally in the project .claude/settings.json; Bash|PowerShell,
 # wired unconditionally there too), AND, with -Agent <name>, in the frontmatter of every agent that may itself
-# delegate or run shell commands (issue-worker, docs-writer, mechanical-fixer, the SPEC-003 audit-stage agents
-# issue-archivist/issue-auditor/test-auditor/audit-verifier, and docs-reviewer) (#1345): denies
+# delegate or run shell commands (issue-worker, docs-writer, mechanical-fixer, site-steward, the SPEC-003
+# audit-stage agents issue-archivist/issue-auditor/test-auditor/audit-verifier, docs-reviewer, and, since
+# #1447, pr-reviewer) (#1345): denies
 # run_in_background: true on any Agent/Task tool call made BY a subagent, AND on any Bash/PowerShell tool call
 # made BY a subagent. A specialist spawned in the background and awaited by ending the caller's turn stalls
 # for good, because its completion notice reaches the orchestrator, not the caller (2026-09-24 decision; the

.claude/hooks/tests/Test-Hooks.ps1

@@ -269,6 +269,11 @@ $spawnCases = @(
     @('site-steward', $null, 'mechanical-fixer', 2, 'site-steward: mechanical-fixer is blocked'),
     @('site-steward', $null, 'general-purpose', 2, 'site-steward: general-purpose is blocked'),
     @('site-steward', $null, $null, 2, 'site-steward: missing subagent_type is blocked'),
+    # #1447: pr-reviewer may spawn only Explore.
+    @('pr-reviewer', $null, 'Explore', 0, 'pr-reviewer: Explore is allowed'),
+    @('pr-reviewer', $null, 'mechanical-fixer', 2, 'pr-reviewer: mechanical-fixer is blocked'),
+    @('pr-reviewer', $null, 'general-purpose', 2, 'pr-reviewer: general-purpose is blocked'),
+    @('pr-reviewer', $null, $null, 2, 'pr-reviewer: missing subagent_type is blocked'),
     @($null, 'issue-worker', 'general-purpose', 2, 'no -Agent: agent_type from the hook input'),
     @('issue-worker', 'docs-writer', 'docs-reviewer', 0, 'agent_type of the input wins over -Agent (inherited hook)'),
     @($null, $null, 'general-purpose', 0, 'no agent known: not restricted (fail open)'),
@@ -286,6 +291,7 @@ $spawnCases = @(
     @('orchestrator', $null, 'docs-reviewer', 0, 'orchestrator: docs-reviewer is allowed'),
     @('orchestrator', $null, 'pr-watcher', 0, 'orchestrator: pr-watcher is allowed'),
     @('orchestrator', $null, 'site-steward', 0, 'orchestrator: site-steward is allowed (#1382)'),
+    @('orchestrator', $null, 'pr-reviewer', 0, 'orchestrator: pr-reviewer is allowed (#1447)'),
     @('orchestrator', $null, 'Plan', 0, 'orchestrator: Plan is allowed'),
     @('orchestrator', $null, 'claude-code-guide', 0, 'orchestrator: claude-code-guide is allowed'),
     @('orchestrator', $null, 'general-purpose', 0, 'orchestrator: general-purpose is allowed (covered by guard-orchestrator-writes)'),
@@ -670,13 +676,21 @@ $ownershipCases = @(
     @('site-steward', 'Write', "$wt\artifacts\site-health\issues\gap.md", $wt, 0, 'site-steward: an issue draft under artifacts/site-health'),
     @('site-steward', 'Edit', "$wt\src\Encina\X.cs", $wt, 2, 'site-steward: a repo source file is denied'),
     @('site-steward', 'Edit', "$wt\docs\en\guide.md", $wt, 2, 'site-steward: documentation is denied'),
-    @('site-steward', 'Write', "$wt\artifacts\board\db-summary.json", $wt, 2, 'site-steward: another artifacts/ subfolder is denied')
+    @('site-steward', 'Write', "$wt\artifacts\board\db-summary.json", $wt, 2, 'site-steward: another artifacts/ subfolder is denied'),
+    # #1447: pr-reviewer writes only under artifacts/pr-review/**; it is read-only on the rest of the
+    # repository, including documentation (docs-writer's) and every other artifacts/ subfolder.
+    @('pr-reviewer', 'Write', "$wt\artifacts\pr-review\1447.md", $wt, 0, 'pr-reviewer: its own review under artifacts/pr-review'),
+    @('pr-reviewer', 'Edit', "$wt\src\Encina\X.cs", $wt, 2, 'pr-reviewer: a repo source file is denied'),
+    @('pr-reviewer', 'Edit', "$wt\docs\en\guide.md", $wt, 2, 'pr-reviewer: documentation is denied'),
+    @('pr-reviewer', 'Write', "$wt\artifacts\site-health\report.md", $wt, 2, 'pr-reviewer: another artifacts/ subfolder is denied')
 )
 
 # agent_type (payload), agent_id, subagent_type, run_in_background, expected, label[, -Agent (hook CLI arg)]
 $noBgCases = @(
     @('issue-worker', 'a1', 'adversarial-reviewer', $true, 2, 'no-background-specialists: worker background spawn is blocked'),
     @('issue-worker', 'a1', 'adversarial-reviewer', $false, 0, 'no-background-specialists: worker foreground spawn is allowed'),
+    @('pr-reviewer', 'a4', 'Explore', $true, 2, 'no-background-specialists: pr-reviewer background spawn is blocked (#1447)'),
+    @('pr-reviewer', 'a4', 'Explore', $false, 0, 'no-background-specialists: pr-reviewer foreground spawn is allowed (#1447)'),
     @($null, $null, 'issue-worker', $true, 0, 'no-background-specialists: main session background spawn is allowed'),
     @('docs-writer', 'a2', 'docs-reviewer', $true, 2, 'no-background-specialists: docs-writer background spawn is blocked'),
     @('issue-archivist', 'a3', 'issue-auditor', $true, 2, 'no-background-specialists: audit-stage agent background spawn is blocked'),
@@ -1002,7 +1016,14 @@ try {
         @('test-auditor', 'PowerShell', "Set-Content -LiteralPath '$shellCodePath' -Value 'fabricated'", 2, 'shell vector: Set-Content by the wrong stage agent is denied'),
         @('issue-auditor', 'PowerShell', "Set-Content -LiteralPath '$shellCodePath' -Value 'legitimate'", 0, 'shell vector: Set-Content by the correct stage agent is allowed'),
         @($null, 'PowerShell', "[IO.File]::WriteAllText('$shellCodePath', 'fabricated')", 2, 'shell vector: [IO.File]::WriteAllText by the orchestrator is denied', 'mechanical-fixer'),
-        @('test-auditor', 'Bash', "echo fabricated > '$shellCodePath'", 2, 'shell vector: Bash redirection by the wrong stage agent is denied')
+        @('test-auditor', 'Bash', "echo fabricated > '$shellCodePath'", 2, 'shell vector: Bash redirection by the wrong stage agent is denied'),
+        # #1447 adversarial-review fix: pr-reviewer.md originally wired enforce-path-ownership.ps1 only on the
+        # Write|Edit|MultiEdit|NotebookEdit matcher, not on Bash|PowerShell; a shell write whose payload omits
+        # agent_type (the exact case the frontmatter -Agent fallback exists for) fell through every elseif
+        # branch to the default allow. These cases exercise the hook's own Bash|PowerShell handling for
+        # pr-reviewer directly, the same way the stage-agent cases above do.
+        @('pr-reviewer', 'PowerShell', "Set-Content -LiteralPath '$wt\src\Encina\X.cs' -Value 'fabricated'", 2, 'shell vector (#1447): pr-reviewer shell write outside artifacts/pr-review is denied'),
+        @('pr-reviewer', 'PowerShell', "Set-Content -LiteralPath '$wt\artifacts\pr-review\1447.md' -Value 'ok'", 0, 'shell vector (#1447): pr-reviewer shell write to its own artifacts/pr-review is allowed')
     )
     foreach ($case in $shellCases) {
         $hookAgent, $tool, $command, $expected, $label, $agentType = $case
@@ -2196,7 +2217,7 @@ Some debt description.
             if (-not (Test-Path (Join-Path $hooks $m.Groups['h'].Value))) { $problems.Add("missing hook $($m.Groups['h'].Value)") }
             if ($m.Groups['a'].Success -and $m.Groups['a'].Value -ne $file.BaseName) { $problems.Add("-Agent $($m.Groups['a'].Value) in $($file.Name)") }
         }
-        if ($file.BaseName -in 'issue-worker', 'mechanical-fixer', 'docs-writer', 'docs-reviewer' -and -not ($front -match 'block-worker-publish\.ps1')) { $problems.Add('block-worker-publish is not wired') }
+        if ($file.BaseName -in 'issue-worker', 'mechanical-fixer', 'docs-writer', 'docs-reviewer', 'site-steward', 'pr-reviewer' -and -not ($front -match 'block-worker-publish\.ps1')) { $problems.Add('block-worker-publish is not wired') }
         Test-Wiring "frontmatter of $($file.Name)" $problems
     }
     $settingsProblems = [System.Collections.Generic.List[string]]::new()