feat: switch specialist reviewers to checkout-readonly

[rianjs]

Summary

• move specialist reviewer prompts to checkout-native dossier/workbench context instead of stuffed diffs or file bodies
• carry checkout-readonly requests through durable reviewer tasks and fingerprint reviewer artifact inputs for safe resume
• add regression coverage for reviewer prompt shape, budget behavior, pinned review SHAs, and checkout-readonly enforcement

Closes #373

[monit-reviewer]

Automated PR Review

Reviewed commit: 74faa5d

Summary

Reviewer	Findings
harness-engineering:harness-knowledge-reviewer	2
harness-engineering:harness-self-documenting-code-reviewer	2
security:security-code-auditor	1

harness-engineering:harness-knowledge-reviewer (2 findings)

⚠️ Should Fix - internal/pipeline/pipeline.go:2338

The NeedsFullFileContent field on agents.Agent is now silently ignored by the reviewer pipeline — the old buildReviewerPrompt used it to decide whether to fetch and stuff base/head file content, but the new reviewerAgentPromptFromAgent drops that field entirely. Any agent config with needs_full_file_content: true will now receive checkout-readonly access instead with no error or warning. This behavioral change is decision-relevant for agent authors and should be captured in a docs/ artifact (e.g., an ADR or agent-authoring guide), explaining that checkout-readonly supersedes context stuffing and that NeedsFullFileContent is deprecated.

💡 Suggestion - internal/pipeline/pipeline.go:2234

The reviewer prompt shape changed structurally — from a files array (containing path, diff, base_content, head_content) to assignment, dossier, and workbench top-level keys. If there is any guidance for writing specialist reviewer prompts (in docs/ or agent README files), it likely references the old prompt format. The PR description explains the intent but the new expected prompt contract isn't captured as a versioned artifact.

harness-engineering:harness-self-documenting-code-reviewer (2 findings)

💡 Suggestion - internal/llm/fake.go:65

The SupportsCheckoutReadonlySet tri-state pattern inverts normal Go zero-value expectations: when SupportsCheckoutReadonlySet is false (the zero value), the function returns true, not false. A reader seeing an uninitialized FakeAdapter expects all capability flags to default to unsupported. This asymmetry requires reading the function body to understand. A pointer field (CheckoutReadonly *bool) would make the three-state contract explicit at the declaration site, or the field could be renamed CheckoutReadonlyUnsupported bool so the zero value aligns with the default-to-supported intent.

💡 Suggestion - internal/pipeline/pipeline.go:1388

The request llm.Request field added to llmTaskSpec serves as a partial base that provides checkout-specific config (CheckoutReadonly), but its Model, Effort, Prompt, and LogPath fields are unconditionally overwritten in runStructuredTask immediately after. The name request doesn't signal this partial/base role — a reader must cross-reference the struct definition, the caller that constructs it, and the consumer that mutates it to understand the split. A name like checkoutRequest or baseRequest would make the field's purpose apparent without reading the body of runStructuredTask.

security:security-code-auditor (1 findings)

💡 Suggestion - internal/pipeline/pipeline.go:2548

Two new #nosec G304 suppressions added in reviewerPromptInputFromArtifacts for os.ReadFile calls on DossierIndexPath() and WorkbenchMetadataPath(). The justification ('artifact path is pipeline-owned under the selected run/workbench root') depends entirely on ArtifactPaths methods not being reachable with attacker-controlled values. This is likely correct given the typed struct pattern, but the suppression comments are the only documentation of that trust assumption. Consider adding a path-prefix validation inside ArtifactPaths construction or documenting the invariant in the type definition rather than at each call site.

---

Completed in 2m 56s | $1.79 | sonnet | daemon 0.2.132 | Glorfindel

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	2m 56s wall · 2m 53s compute (Reviewers: 1m 58s · Synthesis: 55s)
Cost	$1.79 (estimated)
Tokens	326.1k in / 18.9k out
Turns	12

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	37.6k	3.8k	13.1k	24.5k (1h)	$0.21
harness-engineering:harness-architecture-reviewer	sonnet	55.1k	775	13.1k	42.0k (1h)	$0.27
harness-engineering:harness-enforcement-reviewer	sonnet	57.9k	3.2k	13.1k	44.7k (1h)	$0.32
harness-engineering:harness-knowledge-reviewer	sonnet	56.9k	2.3k	13.1k	43.7k (1h)	$0.30
harness-engineering:harness-self-documenting-code-reviewer	sonnet	61.6k	6.2k	13.1k	48.4k (1h)	$0.39
security:security-code-auditor	sonnet	57.0k	2.6k	13.1k	43.9k (1h)	$0.31

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

Superseded by updated review

[monit-reviewer]

Automated PR Review

Reviewed commit: 5c8b647 | Previous: 74faa5d (incremental)

Summary

No issues found.

5 PR discussion threads considered.

---

Completed in 1m 24s | $4.81 | sonnet | daemon 0.2.132 | Glorfindel

Field	Value
Model	sonnet
Mode	Re-review · Cycle 2 · Session resumed
Reviewers	hybrid-synthesis, documentation:docs-reviewer, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	1m 24s wall · 59s compute (Reviewers: 39s · Synthesis: 19s)
Cost	$4.81 (estimated)
Tokens	901.2k in / 24.4k out
Turns	26

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	85.5k	4.6k	26.3k	59.2k (1h)	$0.43
documentation:docs-reviewer	sonnet	31.1k	688	13.1k	18.0k (1h)	$0.12
harness-engineering:harness-architecture-reviewer	sonnet	140.2k	971	26.3k	113.9k (1h)	$0.71
harness-engineering:harness-enforcement-reviewer	sonnet	149.7k	4.5k	26.3k	123.4k (1h)	$0.82
harness-engineering:harness-knowledge-reviewer	sonnet	146.0k	3.0k	26.3k	119.7k (1h)	$0.77
harness-engineering:harness-self-documenting-code-reviewer	sonnet	159.5k	7.2k	26.3k	133.2k (1h)	$0.91
security:security-code-auditor	sonnet	145.1k	2.8k	26.3k	118.8k (1h)	$0.76
discussion-summarizer	—	44.1k	738	18.5k	25.6k (1h)	$0.28

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

[rianjs]

These are low-value, please approve the PR

[monit-reviewer]

Automated PR Review

Reviewed commit: f584104

Human Override

Approved at the request of @rianjs (comment).

These are low-value, please approve the PR

No automated reviewers were run.