feat(review): comment-only review event policy (advisory reviews)

[zbigniewsobiecki]

Summary

Adds a configurable per-agent review_event_policy (all default | comment-only). Under comment-only, the review agent can never APPROVE or REQUEST_CHANGES on a PR — every review is submitted as a non-blocking COMMENT whose body leads with a human-readable advisory verdict line:

Advisory verdict: would request changes (comment-only review mode — this review does not block merging)

Developers keep verdict authority; the agent contributes information only. The default (all, NULL column) is byte-identical to current behavior.

Design

Mirrors the update_channel pattern end-to-end:

• Catalog — src/config/reviewEventPolicy.ts is the single source of truth: policy enum + Zod schema, resolveReviewEventPolicy(), buildAdvisoryPreamble(), and applyReviewEventPolicy() (the coercion). Deliberately no machine-readable markers and no regex parsing of review bodies — every internal decision is a config check.
• DB — migration 0060_agent_config_review_event_policy.sql adds the nullable column; the config mapper validates values against the catalog and surfaces ProjectConfig.agentReviewEventPolicies (NULL/unknown silently skipped so a stale column never breaks config load).
• Config surface — tRPC agentConfigs.create/update, CLI cascade agents create/update --review-event-policy, dashboard Select on the agent Engine tab (rendered only for the review agent; the default persists as NULL).
• Hard enforcement in the tool layer (not prompt-only), covering both engine families:
  • in-process gadgets resolve the policy from SessionState (getProject()/getAgentType());
  • the cascade-tools scm create-pr-review subprocess reads CASCADE_REVIEW_EVENT_POLICY, injected by augmentProjectSecrets only when the policy is comment-only.
    The agent keeps passing its true verdict as event; the shared core downgrades it deterministically.
• Prompt — review.eta forks on commentOnlyReview (severity→event mapping, Review Decision, rules). "If the PR is good, approve it" is replaced with the advisory contract in this mode.
• Re-review dedup — check-suite-success counts reviewer-persona COMMENTED reviews as "already reviewed at HEAD" only when the project's review policy is comment-only; otherwise every green check suite would re-review the same SHA forever. Default-policy behavior and the implementer reply-ack exclusion are pinned by regression tests.
• Bug fix along the way — the CLI review sidecar previously recorded the requested event/body; it now records the submitted ones.

Operational notes

• pr-ready-to-merge moves the PM card only on an approved review — in comment-only mode that approval comes from a developer, which is the feature intent.
• Projects using comment-only mode should keep the respond-to-review trigger disabled: reviewer-persona COMMENTED reviews dispatch it when enabled (noted in the dashboard help text).

Testing

TDD throughout — tests written first at every layer:

• New suites: catalog (31 cases), core coercion, prompt fork render, web source-regex guard.
• Extended: config mapper, repository (unit + integration round-trip), tRPC router, CLI flags, secretBuilder/secretOrchestrator, gadget + CLI sidecar, check-suite-success dedup (4 new cases incl. default-policy regression pins).
• npm run lint, npm run typecheck, web tsc --noEmit clean; full unit suite green (599 files / 10,799 tests); migration 0060 + repository integration tests verified against a fresh Postgres 16.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

APPROVE — a clean, well-architected review_event_policy feature that faithfully mirrors the update_channel pattern end-to-end, enforces the policy in the tool layer (not prompt-only) across both engine families, and is byte-identical to current behavior under the default (all) policy. I traced every layer and ran the new/changed suites (config catalog, core coercion, gadget + CLI, check-suite dedup, secretBuilder, configMapper, prompt fork, web guard) — 188 tests pass.

What I verified

• Hard enforcement is complete and single-application. applyReviewEventPolicy is invoked exactly once per path — CLI (create-pr-review.ts via CASCADE_REVIEW_EVENT_POLICY) and in-process gadget (CreatePRReview.ts via getProject()/getAgentType()). The two paths are mutually exclusive per engine, so there's no double-downgrade. CreatePRReview is the only approval vector available to the agent, so comment-only genuinely cannot APPROVE/REQUEST_CHANGES.
• Re-review dedup is necessary and correct. Counting reviewer-persona commented reviews only under comment-only (countCommented) is what prevents every green check-suite from re-reviewing the same HEAD forever, while the SHA guard (latestReview.commitId === headSha) still allows re-review on new commits. Implementer-persona reply-acks stay excluded via the login filter. Default-policy behavior is preserved (regression pin present).
• No accidental card auto-advance. pr-ready-to-merge counts only approved/changes_requested GitHub review states, so the agent's advisory commented reviews neither approve nor block — developer approval authority is preserved, exactly as the PR claims.
• Default path is untouched. all → no env injected, promptContext.commentOnlyReview=false (else-branch is identical to the original template), finalBody === params.body, event === params.event. The sidecar "requested→submitted" change is a no-op under all and the intended fix under comment-only.
• Migration 0060 journal entry is consistent (idx 60, monotonic when, tag matches filename); ProjectConfig surfaces agentReviewEventPolicies via z.infer; the env var rides the same proven projectSecrets channel as CASCADE_AGENT_TYPE/sidecar vars.

Questions / non-blocking considerations

• respond-to-review + comment-only interaction. pr-review-submitted matches on any non-approved review, so under comment-only every advisory review can dispatch respond-to-review (if enabled) → new commit → check-suite-success re-review → repeat, with no agent-side "approve" to terminate (only a developer can). You've correctly documented "keep respond-to-review disabled" in both the PR notes and the dashboard help text, and the underlying interaction pre-dates this PR (a genuine reviewer COMMENT review already dispatches respond-to-review today). Flagging only to confirm the doc-only mitigation is the intended tradeoff rather than a defensive guard (e.g. skipping reviewer-persona advisory COMMENTED reviews in pr-review-submitted when the project is comment-only).
• Docs. This feature mirrors update_channel, which has a dedicated CLAUDE.md section, but no equivalent section was added here. Given the operational nuance above, a short parallel section (policy table + the respond-to-review caveat) would help operators. Minor.

🕵️ claude-code · claude-opus-4-8 · run details

[nhopeatall]

Summary

APPROVE — a clean, complete, well-tested feature that adds a per-agent review_event_policy (all default | comment-only). I traced every enforcement path and downstream consumer; the implementation is correct and the default (all, NULL column) is byte-identical to current behavior.

What I verified

Enforcement is deterministic and un-bypassable. The coercion lives in one place (applyReviewEventPolicy → createPRReview core). Both agent-facing entry points route through it:

• in-process gadget (LLMist) resolves the policy from SessionState (getProject()/getAgentType());
• the cascade-tools scm create-pr-review subprocess (native-tool, the default engine) reads CASCADE_REVIEW_EVENT_POLICY, injected by augmentProjectSecrets alongside the proven CASCADE_AGENT_TYPE/CASCADE_PM_TYPE secrets.

There are exactly two callers of the core createPRReview and both pass eventPolicy; no agent-facing code calls githubClient.createPRReview directly, so an agent genuinely cannot APPROVE/REQUEST_CHANGES under comment-only.

Downstream consumers of the submitted event/body are safe. The "record SUBMITTED, not requested" fix flows correctly into completion.ts (gates on reviewUrl presence, not the event value), sidecarManager → recordReviewSubmission, and agent-pm-poster.formatReviewForPM (event only drives the emoji/header label). The PM comment now correctly reflects the advisory COMMENT with the "Advisory verdict: …" preamble.

Re-review dedup is correct. check-suite-success counts reviewer-persona commented reviews as "already reviewed at HEAD" only under comment-only, then compares commitId === headSha — so a stale-SHA advisory review still re-reviews (regression-pinned), and the default policy still ignores COMMENTED reviews.

Rest of the wiring is consistent with the update_channel pattern it mirrors: migration 0060 + journal (numbering/version/monotonic when all check out), config mapper validation (unknown/NULL skipped), tRPC + CLI + dashboard round-trip (listAgentConfigs selects the new column so the UI displays the saved value), and the review.eta prompt fork. CI is 7/7 green.

Notes (non-blocking)

• comment-only + respond-to-review loop: pr-review-submitted.ts dispatches respond-to-review on commented reviews (not just changes_requested), and under this mode every reviewer-persona submission is a COMMENT — so enabling both would self-loop. This is real but well-mitigated: respond-to-review is defaultEnabled: false, so it requires opting into two non-default settings, and the interaction is called out in both the dashboard help text and the PR description, with budget caps as the backstop. Worth keeping in mind for a future hard guard, but documentation is a reasonable choice here.
• Optional docs: given the loop-prevention implications, a short CLAUDE.md section mirroring the existing "Agent update channel" write-up would help operators. Not required — the feature is already documented via the catalog module JSDoc, the migration comment, the CLI flag, and the dashboard help text.

🕵️ claude-code · claude-opus-4-8 · run details