fix: correct reviewer APPROVE→COMMENT docs + enforce no-formal-approve at compile time#872
Conversation
…-approve at compile time Follow-up to #870. That PR mapped an `APPROVE` verdict to a GitHub `COMMENT` review event but left two loose ends: - Doc comments in `post-github.ts` still claimed "gate mode keeps APPROVE / REQUEST_CHANGES", contradicting the shipped behavior. Corrected to describe the actual contract (APPROVE → COMMENT always; comment mode also softens REQUEST_CHANGES → COMMENT; gate mode keeps REQUEST_CHANGES; `env.verdict` arrives pre-softened from `buildEnvelope`). - `VCS_EVENT`'s value type still permitted `"APPROVE"`. Narrowed it to `"COMMENT" | "REQUEST_CHANGES"` so the no-formal-approval invariant is a compile-time guarantee — no future edit can resurrect a formal GitHub approval without breaking the build. The narrower union is still assignable to Octokit's `createReview` event param. - Documented `applyMode`'s APPROVE/COMMENT passthrough. Adds a composition test asserting the positive expected GitHub event for every verdict-producing finding-set across both modes, plus the semantic-verdict-vs- VCS-event separation (gate + no findings → `verdict === "APPROVE"` but event `"COMMENT"`; comment-mode blocking critical → `idealVerdict === "REQUEST_CHANGES"`, `verdict === "COMMENT"`). Reviewed via multi-model consensus (GPT 5.4, Gemini 3.1 Pro, GLM-5): the type narrowing and test strengthening were panel findings incorporated during convergence. Closes #871 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR hardens the invariant that APPROVE verdicts never emit formal GitHub approval events. It narrows ChangesNo-formal-APPROVE guarantee
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
What does this PR do?
Follow-up to #870. That PR mapped an
APPROVEverdict to a GitHub COMMENT review event (so the bot never submits a formal approval that could satisfy branch protection), but left two loose ends this PR closes:Stale doc comments in
packages/opencode/src/altimate/review/post-github.tsstill claimed "ingatemode it maps the verdict to APPROVE/REQUEST_CHANGES" and "gate mode keeps APPROVE / REQUEST_CHANGES" — contradicting the shipped behavior on a security-relevant path. Corrected to describe the actual contract (APPROVE → COMMENT always; comment mode also softens REQUEST_CHANGES → COMMENT; gate mode keeps REQUEST_CHANGES;env.verdictarrives pre-softened frombuildEnvelope).The no-formal-approval invariant was only enforced at runtime.
VCS_EVENT's value type still included"APPROVE". Narrowed it to"COMMENT" | "REQUEST_CHANGES"so the invariant is now a compile-time guarantee — no future edit can resurrect a formal GitHub approval without breaking the build. The narrower union is still assignable to Octokit'screateReviewevent param.Also documents
applyMode's APPROVE/COMMENT passthrough, and adds a composition test asserting the positive expected GitHub event for every verdict-producing finding-set across both modes, plus the semantic-verdict-vs-VCS-event separation (gate + no findings →verdict === "APPROVE"but event"COMMENT"; comment-mode blocking critical →idealVerdict === "REQUEST_CHANGES",verdict === "COMMENT").The type narrowing and test strengthening were findings from a multi-model consensus review (GPT 5.4, Gemini 3.1 Pro, GLM-5) incorporated during convergence.
packages/opencode/src/altimate/review/verdict.ts— narrowedVCS_EVENTvalue type to exclude"APPROVE"; documentedapplyModepassthrough.packages/opencode/src/altimate/review/post-github.ts— corrected the two stale doc comments.packages/opencode/test/altimate/review.test.ts— added the composition / verdict-vs-event-separation test.Type of change
Issue for this PR
Closes #871
How did you verify your code works?
bun run typecheck(tsgo--noEmit) — clean. The narrowedVCS_EVENTtype compiles againstpost-github.ts'screateReview({ event })call.bun test test/altimate/review.test.ts— 66 pass, 0 fail (includes the new composition test).bun test test/altimate/review.test.ts review-ci.test.ts review-dbt-patterns.test.ts review-rule-catalog.test.ts— 118 pass, 0 fail.script/upstream/analyze.ts --markers --base origin/main --strict) — no upstream-shared files modified.prettier --checkon all three changed files — clean (only the added test block; no unrelated reformatting).Checklist
Summary by cubic
Enforces a compile-time guarantee that the reviewer never submits a formal GitHub approval and updates docs to reflect the APPROVE → COMMENT behavior. Adds tests to verify verdict-to-event mapping across modes.
packages/opencode/src/altimate/review/post-github.tsto state: APPROVE always posts COMMENT;commentmode softens REQUEST_CHANGES → COMMENT;gatemode keeps REQUEST_CHANGES;env.verdictis pre-softened.VCS_EVENTinpackages/opencode/src/altimate/review/verdict.tsto"COMMENT" | "REQUEST_CHANGES"to enforce no-formal-approve at compile time; documentedapplyModepassthrough.packages/opencode/test/altimate/review.test.tsthat checks correct GitHub events across modes, preserves ideal verdicts for audit, and ensures no path emits"APPROVE".Written for commit 2dd495a. Summary will update on new commits.
Summary by CodeRabbit
Documentation
Bug Fixes
Tests