feat: reviewer discipline gate for free-tier private repos (ship + land-and-deploy)#884
Open
Tarabcak wants to merge 1 commit intogarrytan:mainfrom
Open
feat: reviewer discipline gate for free-tier private repos (ship + land-and-deploy)#884Tarabcak wants to merge 1 commit intogarrytan:mainfrom
Tarabcak wants to merge 1 commit intogarrytan:mainfrom
Conversation
Adds a coordinated pair of changes across /ship and /land-and-deploy to close the "private repo + free GitHub plan + no branch protection" loophole, where /ship would happily create a PR and /land-and-deploy would happily merge it without a human reviewer ever seeing the code. On a paid plan you'd configure branch protection and the platform would enforce review approval. On a free private plan there's no platform gate at all, so discipline has to live in the skills. ## ship/SKILL.md (Step 8: Create PR/MR) New "Reviewer resolution" sub-step before the gh pr create / glab mr create command. Resolves a reviewer in this priority order: 1. CLAUDE.md "Default reviewer: @user" line 2. CLAUDE.md "Reviewers: @user1, @user2" line 3. gh api repos/:owner/:repo/collaborators (auto-detect non-self collaborators on the repo) 4. None — solo repo, skip reviewer assignment The result is stored in $_REVIEWERS. Both the GitHub and GitLab PR creation blocks now branch on whether $_REVIEWERS is set, passing --reviewer (gh) or --reviewer (glab) when available. After PR creation, if $_REVIEWERS is non-empty, /ship prints a mandatory banner verbatim: ================================================================== REVIEW REQUESTED from: $_REVIEWERS DO NOT MERGE this PR until a reviewer approves it. Check status: gh pr view <PR#> --json reviewDecision,reviews Merge only when reviewDecision == "APPROVED" ================================================================== And /ship is now explicitly forbidden from running gh pr merge (or equivalent) in the same invocation when reviewers are set. If the user asks /ship to also merge, it refuses with a pointer at the banner: "Review required first. PR #<N> has reviewers assigned. Merge only after reviewDecision is APPROVED." ## land-and-deploy/SKILL.md (Step 3.5a-ter: HARD GATE) New 3.5a-ter sub-step that runs alongside the existing 3.5a (local AI review check) and 3.5a-bis (inline review offer). It is a HARD GATE — local AI reviews are not a substitute for human approval when a reviewer is assigned. Queries the PR's review state via: gh pr view --json reviewDecision,reviewRequests,latestReviews Gate logic: | reviewDecision | reviewRequests | Action | |--------------------|----------------|----------------------| | APPROVED | - | PASS | | CHANGES_REQUESTED | - | BLOCK (list reviewer)| | REVIEW_REQUIRED | non-empty | BLOCK (list pending) | | null | empty + collab | WARN (collab repo) | | null | empty + solo | PASS (solo repo) | The "solo repo" detection uses gh api repos/:owner/:repo/collaborators filtered against the current user's login. If $_OTHER_COLLABS == 0, treat as solo and pass; otherwise treat missing reviews as a warning/blocker per the table. The readiness report's REVIEWS section is renamed to "REVIEWS (local AI)" and a new "GITHUB REVIEW (human)" section is added showing decision / approvers / pending. If the blocker is "GitHub PR review not approved", option C ("override") is intentionally NOT available in the AskUserQuestion. The only override is an explicit `--override-review` flag passed to /land-and-deploy or an explicit dedicated AskUserQuestion bypass ("I am the reviewer and I approve this"). Default is refuse. A reminder is also added to the "Important Rules" section at the end of the file: "Never merge a PR without approved human review when a reviewer was assigned. Step 3.5a-ter is a HARD GATE — if reviewDecision != APPROVED and the PR has reviewers or the repo has other collaborators, refuse to merge." ## Why this matters Free GitHub plans on private repos have no branch protection available. The "Require pull request reviews before merging" toggle is a paid feature. So on a solo founder's private repo, there is no platform-level enforcement that the assigned reviewer ever sees the code before it merges. Without these changes, /ship + /land-and- deploy would happily auto-merge every PR with reviewers set, defeating the whole point of having a reviewer in the first place. This is the exact failure mode this change exists to prevent. ship and land-and-deploy now form a coordinated pair: ship sets the reviewer + posts the banner, land-and-deploy enforces the gate at merge time. ## Test plan - [x] /ship on a solo private repo (no other collaborators) — no reviewer assigned, no banner, /ship completes normally - [x] /ship on a multi-collaborator repo — auto-detects non-self collaborator, assigns as reviewer, prints banner, refuses to merge in the same invocation - [x] /ship with "Default reviewer: @username" line in CLAUDE.md — uses the configured username over auto-detection - [x] /land-and-deploy on a PR with reviewDecision=APPROVED — passes - [x] /land-and-deploy on a PR with reviewDecision=CHANGES_REQUESTED — blocks, lists the reviewer - [x] /land-and-deploy on a PR with pending reviewers and no decision — blocks, lists pending - [x] /land-and-deploy on a solo repo PR — passes (no collaborators to require review from) - [x] /land-and-deploy --override-review on a blocked PR — passes with override logged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the "private repo + free GitHub plan + no branch protection" loophole where
/shiphappily creates a PR with reviewers assigned and/land-and-deployhappily merges it without the human reviewer ever seeing the code. Two coordinated changes —shipsets the reviewer + posts a banner,land-and-deployenforces the gate at merge time.On a paid GitHub plan you'd configure branch protection and the platform would enforce review approval. On a free private plan the "Require pull request reviews before merging" toggle isn't available, so there is no platform-level enforcement that the assigned reviewer ever sees the code before it merges. Without this change,
/ship+/land-and-deploywill auto-merge every PR with reviewers set, defeating the whole point of having a reviewer in the first place.This is a real failure mode I hit on a private repo earlier today.
/shipcreated the PR,/land-and-deploywas about to merge it, my collaborator never saw it. That's the gap this PR fills.What changes
ship/SKILL.md— Step 8 (Create PR/MR)Adds a Reviewer resolution sub-step before
gh pr create/glab mr create. Resolves a reviewer in this priority order:CLAUDE.mdline matchingDefault reviewer: @usernameCLAUDE.mdline matchingReviewers: @user1, @user2gh api repos/:owner/:repo/collaborators(auto-detect non-self collaborators)Result is stored in
$_REVIEWERS. Both the GitHub and GitLab create-PR blocks now branch on whether$_REVIEWERSis set, passing--reviewerwhen available.After PR creation, if
$_REVIEWERSis non-empty,/shipprints a mandatory banner verbatim and is explicitly forbidden from runninggh pr mergein the same invocation:If the user asks
/shipto also merge, it refuses with: "Review required first. PR # has reviewers assigned. Merge only after reviewDecision is APPROVED."land-and-deploy/SKILL.md— new Step 3.5a-ter (HARD GATE)A new sub-step that runs alongside the existing 3.5a (local AI review check) and 3.5a-bis (inline review offer). It is a HARD GATE — local AI reviews are not a substitute for human approval when a reviewer is assigned.
Queries the PR's review state via:
Gate logic:
The "solo repo" detection uses
gh api repos/:owner/:repo/collaboratorsfiltered against the current user's login. If_OTHER_COLLABS == 0, treat as solo and pass; otherwise treat missing reviews as a warning/blocker per the table.The readiness report's REVIEWS section is renamed to REVIEWS (local AI) and a new GITHUB REVIEW (human) section is added showing decision / approvers / pending.
If the blocker is "GitHub PR review not approved", the standard option C ("override") is intentionally NOT available in the AskUserQuestion. The only override is an explicit
--override-reviewflag passed to/land-and-deployor an explicit dedicated AskUserQuestion bypass ("I am the reviewer and I approve this"). Default is refuse.A reminder is also added to the "Important Rules" section of
land-and-deploy/SKILL.md:Why a "soft" warn for the unset-decision-with-collaborators case
When
reviewDecisionisnull(no review submitted yet) but the repo has other collaborators and no reviewers were explicitly requested, the gate is a warning, not a blocker. Reasoning: it might be a solo-authored fix in a multi-person repo where review isn't needed for that specific change. Forcing a hard block here would be too aggressive and would break legitimate small fixes. The warning surfaces it in the readiness report so the user can make an explicit call.Impact on existing behavior
Solo repos with no other collaborators: zero behavior change. Reviewer resolution returns empty, no banner printed, no gate triggered,
/shipand/land-and-deploywork exactly as today.Repos with collaborators where no reviewers were ever assigned: still works, surfaces a one-line warning in the readiness report, doesn't block.
Repos with collaborators where
/shipassigned a reviewer: new behavior —/shipprints the banner and stops at PR creation,/land-and-deployblocks merge untilreviewDecision == APPROVED. This is the intended discipline.Existing CLAUDE.md
Default reviewer:lines: picked up automatically, no migration needed.Test plan
/shipon a solo private repo (no other collaborators) — no reviewer assigned, no banner,/shipcompletes normally/shipon a multi-collaborator repo — auto-detects non-self collaborator, assigns as reviewer, prints banner, refuses to merge in the same invocation/shipwithDefault reviewer: @usernameline in CLAUDE.md — uses configured username over auto-detection/land-and-deployon a PR withreviewDecision=APPROVED— passes/land-and-deployon a PR withreviewDecision=CHANGES_REQUESTED— blocks, lists the reviewer who requested changes/land-and-deployon a PR with pending reviewers and no decision — blocks, lists pending/land-and-deployon a solo repo PR — passes (no collaborators to require review from)/land-and-deploy --override-reviewon a blocked PR — passes with override loggedNotes for review
ship/SKILL.mdchange is the smaller, lower-risk one. Theland-and-deploy/SKILL.mdchange is where the actual enforcement lives.