feat(review): layer repo-local and shared reviewer selection

[rianjs]

Summary

• merge configured and trusted repo-local reviewer catalogs instead of requiring repo-local definitions
• preserve source precedence while treating applicable repo-local reviewers as required before optional shared reviewers
• add required_on_match for code-specific mandatory reviewers
• make the default selection budget all applicable repo-local reviewers plus up to five shared reviewers
• keep an explicit positive --max-agents as a hard total cap
• log offered reviewers, provenance, mandatory matches, orchestrator reasoning, and assignments
• document the reviewer resolution and selection contract
• bound LLM tasks at 14 minutes and retain Claude background auto-mode compatibility

Root cause

The review pipeline treated missing .codereview/agents guidance as a blocking repository-guidance failure. That prevented configured shared reviewer catalogs from serving as a fallback and made contributor-local catalog distribution impractical.

Selection also had only one fixed total-agent limit, so it could not express the intended policy: repository-owned reviewers are authoritative and should all run by default, while shared organization reviewers augment them within the normal shared budget.

Behavior

• Missing repo-local guidance falls back to configured reviewer sources.
• Unreadable or invalid repo-local guidance still blocks review.
• Repo-local definitions win when IDs overlap configured definitions.
• Applicable repo-local reviewers run by default.
• Any reviewer source may use required_on_match: true with valid file_globs.
• An explicit --max-agents remains a hard total cap and errors when smaller than the mandatory reviewer count.
• Reviewer resolution and selection decisions are visible in structured progress logs.

Validation

go test ./internal/agents ./internal/app ./internal/dossier \
  ./internal/llmadapters ./internal/pipeline ./internal/progress \
  ./internal/reviewplan ./internal/view

• 515 tests passed across 8 packages
• Codex live review validation completed successfully
• Claude background dry run against SignalFT/ai-hero#80 completed successfully
• Claude run e60e1895-e8a4-4e7c-a423-69dca7ad5cea: SOLID reviewer used Read/Bash, completed rollup, and returned three findings in 4m48s

[rianjs-bot]

Automated PR Review

Reviewed commit: 0adfcc26939b
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer	Findings
go:implementation-tests	1
structure:repo-health	0
policies:conventions	0
documentation:docs	0

go:implementation-tests (1 finding)

Major - internal/pipeline/pipeline.go:1257

The explicit cap checks only required_on_match reviewers, even though catalogAgentIsRequired also treats every selected repo-local reviewer as mandatory. With two applicable repo reviewers and --max-agents=1, this path emits a warning and slices required[:1], silently dropping one repo-owned reviewer; TestSelectionOnlyExplicitMaxPrioritizesRepoAgents currently codifies that behavior. This contradicts the stated hard-cap contract that errors when the cap is below the mandatory reviewer count. Check len(required) (after required-on-match injection) against maxAgents and return an error before slicing, with coverage for multiple selected repo reviewers.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	incomplete_skipped	internal/agents/agents.go, internal/agents/agents_test.go, internal/app/pipeline_progress.go, internal/app/pipeline_progress_test.go, internal/app/runtime.go, internal/dossier/dossier.go, internal/llmadapters/subprocess.go, internal/llmadapters/subprocess_test.go, internal/pipeline/pipeline.go, internal/pipeline/pipeline_test.go, internal/pipeline/prompts.go, internal/pipeline/prompts_test.go, internal/progress/progress.go, internal/progress/progress_test.go, internal/view/agents.go, internal/view/agents_test.go	internal/reviewplan/reviewplan_test.go	Focused Go implementation and behavioral-test review. The requested package test command could not compile several packages because the workspace path contains spaces and clang received a split path; no test failures were evaluated from that run.
structure:repo-health	complete_broad	internal/agents/agents.go, internal/app/pipeline_progress.go, internal/app/runtime.go, internal/dossier/dossier.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/progress/progress.go	unavailable	Focused structural review. The targeted Go test command could not complete because the sandbox toolchain rejected the workspace path during package builds.
policies:conventions	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md, internal/agents/agents.go, internal/app/pipeline_progress.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/view/agents.go	unavailable	Shared cli-common and .github convenience copies were not present locally; review used the available repo-local guidance.
documentation:docs	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md	unavailable	Documentation-only review; cross-checked changed claims against current pipeline, agent-loading, progress, and prompt-selection implementation.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 2m 33s | unavailable | gpt-5.6-terra | cr 0.10.260

Field	Value
Model	gpt-5.6-terra
Reviewers	go:implementation-tests, structure:repo-health, policies:conventions, documentation:docs
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	2m 33s wall · 6m 17s compute
Cost	unavailable
Tokens	2.0M in / 16.1k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	16.0k	506	10.5k	unavailable	unavailable	12s
go:implementation-tests	gpt-5.6-terra	550.6k	4.1k	470.5k	unavailable	unavailable	1m 39s
structure:repo-health	gpt-5.6-terra	557.0k	4.7k	482.0k	unavailable	unavailable	1m 55s
policies:conventions	gpt-5.6-terra	184.8k	2.3k	137.0k	unavailable	unavailable	49s
documentation:docs	gpt-5.6-terra	623.6k	3.9k	544.8k	unavailable	unavailable	1m 27s
orchestrator-rollup	gpt-5.6-terra	35.2k	728	26.1k	unavailable	unavailable	12s

[rianjs-bot]

Automated PR Review

Reviewed commit: 5d4387c6a505
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer	Findings
go:implementation-tests	0
structure:repo-health	1
policies:conventions	0
documentation:docs	1

structure:repo-health (1 finding)

Major - internal/pipeline/pipeline.go:1211

The hard-cap policy is still enforced only against the selector's returned list, so repo-local mandatory reviewers can be silently omitted. buildSelectionPrompt gives the model the positive cap as max_selected_agents; when that cap is below the number of applicable repo reviewers, a compliant model will return only that many. capSelectionAgents then sees only those returned reviewers, finds len(required) <= maxAgents, and succeeds rather than raising the promised error. The same gap means an applicable repo reviewer omitted by the model is never restored (only required_on_match agents are restored). Determine/enforce the required repo set independently of the capped response, or have selection return all required reviewers before applying the hard-cap validation, then fail whenever the required set exceeds the explicit cap.

documentation:docs (1 finding)

Major - README.md:383

This default-limit description omits shared reviewers with required_on_match: true. The pipeline runs every matched required reviewer from any source, then permits up to five optional shared reviewers, so a profile or --agents-dir required reviewer can make the default total exceed “repo-local reviewers plus up to 5 shared reviewers.” State that the default includes all applicable repo-local and required_on_match reviewers, plus up to five optional shared reviewers; align the same distinction in the related contract/guidance wording.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/agents/agents.go, internal/agents/agents_test.go, internal/app/pipeline_progress.go, internal/app/pipeline_progress_test.go, internal/app/runtime.go, internal/cmd/noleak/noleak_test.go, internal/dossier/dossier.go, internal/llmadapters/subprocess.go, internal/llmadapters/subprocess_test.go, internal/pipeline/pipeline.go, internal/pipeline/pipeline_test.go, internal/pipeline/prompts.go, internal/pipeline/prompts_test.go, internal/progress/progress.go, internal/progress/progress_test.go, internal/reviewplan/reviewplan_test.go, internal/view/agents.go, internal/view/agents_test.go	unavailable	Targeted Go test command could not build several packages because clang could not resolve the workspace path; no test failures were reached.
structure:repo-health	complete_broad	internal/agents/agents.go, internal/app/pipeline_progress.go, internal/app/runtime.go, internal/dossier/dossier.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/progress/progress.go	unavailable	Focused on reviewer resolution, mandatory selection, and progress contracts. Targeted Go tests could not complete because the environment's compiler invocation mishandled the workspace path.
policies:conventions	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md, internal/agents/agents.go, internal/app/pipeline_progress.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/view/agents.go	unavailable	No local cli-common shared-standards checkout was available; review used the repository-local guidance and changed contract documents.
documentation:docs	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md	unavailable	Documentation-only review; verified selection semantics against the current pipeline implementation.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 2m 14s | unavailable | gpt-5.6-terra | cr 0.10.260

Field	Value
Model	gpt-5.6-terra
Reviewers	go:implementation-tests, structure:repo-health, policies:conventions, documentation:docs
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	2m 14s wall · 5m 55s compute
Cost	unavailable
Tokens	1.9M in / 16.8k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	58.5k	1.2k	44.8k	unavailable	unavailable	11s
go:implementation-tests	gpt-5.6-terra	511.0k	4.0k	437.2k	unavailable	unavailable	1m 36s
structure:repo-health	gpt-5.6-terra	456.2k	3.6k	378.6k	unavailable	unavailable	1m 30s
policies:conventions	gpt-5.6-terra	309.4k	2.6k	255.7k	unavailable	unavailable	1m 00s
documentation:docs	gpt-5.6-terra	438.0k	3.9k	369.9k	unavailable	unavailable	1m 28s
orchestrator-rollup	gpt-5.6-terra	85.2k	1.5k	67.6k	unavailable	unavailable	7s

[rianjs-bot]

Automated PR Review

Reviewed commit: 73918d361351
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer	Findings
go:implementation-tests	0
structure:repo-health	1
policies:conventions	0
documentation:docs	1

structure:repo-health (1 finding)

Major - internal/pipeline/pipeline.go:1294

Repo-local reviewers are documented as mandatory only when applicable, with applies_when explicitly defined as the selector’s routing contract. This fallback reinserts every repo-sourced agent omitted by the selector and assigns it every changed file, so a repo-local Terraform reviewer will run on an unrelated Go-only change; it can also make a positive --max-agents fail for reviewers that were not applicable. Preserve the selector’s applicability decision for ordinary repo-local agents, and deterministically inject only required_on_match agents (or add a separate machine-checkable applicability predicate before treating a repo-local agent as mandatory).

documentation:docs (1 finding)

Minor - docs/review-guidance.md:27

The stated cap failure condition only covers the required_on_match set. In implementation, applicable repo-local reviewers are also mandatory, and --max-agents errors when it is smaller than the combined mandatory set—even when no required_on_match glob matched. Say that a positive cap below all applicable repo-local reviewers plus matching required_on_match reviewers fails; mirror the clarification in the other selection-contract docs.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/agents/agents.go, internal/agents/agents_test.go, internal/app/pipeline_progress.go, internal/app/pipeline_progress_test.go, internal/app/runtime.go, internal/cmd/noleak/noleak_test.go, internal/dossier/dossier.go, internal/llmadapters/subprocess.go, internal/llmadapters/subprocess_test.go, internal/pipeline/pipeline.go, internal/pipeline/pipeline_test.go, internal/pipeline/prompts.go, internal/pipeline/prompts_test.go, internal/progress/progress.go, internal/progress/progress_test.go, internal/reviewplan/reviewplan_test.go, internal/view/agents.go, internal/view/agents_test.go	unavailable	Targeted Go test command could not build in this environment because clang was invoked with an unresolved path containing spaces.
structure:repo-health	complete_broad	internal/agents/agents.go, internal/app/pipeline_progress.go, internal/app/runtime.go, internal/dossier/dossier.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/progress/progress.go	unavailable	Focused on the assigned reviewer-resolution, selection, dossier, and progress files. Targeted Go tests could not complete because the workspace path causes a clang build-path error.
policies:conventions	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md, internal/agents/agents.go, internal/app/pipeline_progress.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/view/agents.go	unavailable	Shared cli-common and automation convenience copies were not available; review used the repository-local guidance and supplied PR context.
documentation:docs	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md	unavailable	Documentation-focused review; verified the documented selection behavior against the relevant pipeline and agent-loading implementation.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 2m 42s | unavailable | gpt-5.6-terra | cr 0.10.260

Field	Value
Model	gpt-5.6-terra
Reviewers	go:implementation-tests, structure:repo-health, policies:conventions, documentation:docs
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	2m 42s wall · 7m 24s compute
Cost	unavailable
Tokens	2.3M in / 21.8k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	152.8k	2.4k	109.1k	unavailable	unavailable	10s
go:implementation-tests	gpt-5.6-terra	676.3k	5.2k	595.5k	unavailable	unavailable	2m 04s
structure:repo-health	gpt-5.6-terra	559.9k	4.1k	489.2k	unavailable	unavailable	1m 48s
policies:conventions	gpt-5.6-terra	346.6k	3.6k	295.7k	unavailable	unavailable	1m 38s
documentation:docs	gpt-5.6-terra	372.9k	3.9k	316.9k	unavailable	unavailable	1m 34s
orchestrator-rollup	gpt-5.6-terra	192.3k	2.7k	145.2k	unavailable	unavailable	8s

[rianjs-bot]

Automated PR Review

Reviewed commit: 1a34cf50dd16
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer	Findings
go:implementation-tests	1
structure:repo-health	0
policies:conventions	0
documentation:docs	0

go:implementation-tests (1 finding)

Major - internal/pipeline/pipeline.go:1210

Only required_on_match reviewers are restored after the selector response. Repo-local reviewers remain absent when the selector obeys a positive max_selected_agents ceiling and returns only a subset, so capSelectionAgents sees too few required reviewers and accepts the run instead of rejecting a cap below the full applicable repo-local set. This violates the hard-cap contract and lets mandatory repo guidance be silently skipped. Restore applicable repo-local selections from the catalog before cap enforcement (or compute the complete required set independently of the response), and add the regression where two applicable repo reviewers exist, --max-agents=1, and the selector returns only one.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/agents/agents.go, internal/agents/agents_test.go, internal/app/pipeline_progress.go, internal/app/pipeline_progress_test.go, internal/app/runtime.go, internal/cmd/noleak/noleak_test.go, internal/dossier/dossier.go, internal/llmadapters/subprocess.go, internal/llmadapters/subprocess_test.go, internal/pipeline/pipeline.go, internal/pipeline/pipeline_test.go, internal/pipeline/prompts.go, internal/pipeline/prompts_test.go, internal/progress/progress.go, internal/progress/progress_test.go, internal/reviewplan/reviewplan_test.go, internal/view/agents.go, internal/view/agents_test.go	unavailable	Review scoped to assigned Go implementation and tests. The prescribed rtk-wrapped test command could not build because its compiler invocation mishandles the workspace path containing spaces.
structure:repo-health	complete_broad	internal/agents/agents.go, internal/app/pipeline_progress.go, internal/app/runtime.go, internal/dossier/dossier.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/progress/progress.go	unavailable	Focused Go tests could not compile because the RTK proxy mishandled the workspace path containing spaces; review was completed through source and test inspection.
policies:conventions	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md, internal/agents/agents.go, internal/app/pipeline_progress.go, internal/pipeline/pipeline.go, internal/pipeline/prompts.go, internal/view/agents.go	unavailable	Focused on reviewer-selection policy and the assigned changed files. The requested Go test command could not complete because the workspace path breaks the local clang invocation; static inspection was completed.
documentation:docs	complete_broad	README.md, docs/checkout-native-review-contract.md, docs/development.md, docs/review-guidance.md	unavailable	Documentation-only review; the targeted Go test could not build because the sandbox checkout path contains spaces that the compiler invocation did not handle.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 2m 15s | unavailable | gpt-5.6-terra | cr 0.10.260

Field	Value
Model	gpt-5.6-terra
Reviewers	go:implementation-tests, structure:repo-health, policies:conventions, documentation:docs
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	2m 15s wall · 6m 30s compute
Cost	unavailable
Tokens	2.1M in / 20.4k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	237.0k	3.2k	184.3k	unavailable	unavailable	14s
go:implementation-tests	gpt-5.6-terra	346.6k	3.3k	291.6k	unavailable	unavailable	1m 31s
structure:repo-health	gpt-5.6-terra	483.9k	3.5k	411.6k	unavailable	unavailable	1m 34s
policies:conventions	gpt-5.6-terra	444.4k	3.5k	386.6k	unavailable	unavailable	1m 36s
documentation:docs	gpt-5.6-terra	330.6k	3.6k	283.4k	unavailable	unavailable	1m 25s
orchestrator-rollup	gpt-5.6-terra	284.7k	3.4k	228.6k	unavailable	unavailable	8s

[rianjs]

These comments are low value; please approve the pull request.