feat(review): let review mode convert a pull request to draft when it is genuinely unfinished - #58
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… is genuinely unfinished Add mark_draft_if_unready, off by default, for review mode: on an explicit, concrete unreadiness signal (a WIP/not-ready marker in the title, description, or commits, or a diff that is self-evidently incomplete for its own stated purpose), it sets draft: true via mcp__github__update_pull_request, never draft: false and never any other field. It never fires on ordinary review findings; a finished pull request with bugs, missing tests, or Blocker/ Should-fix comments is reviewed and blocked normally, not marked draft. It also never lets the review approve a pull request it has just converted. Shares the same mcp__github__update_pull_request grant fix_pr_metadata already uses, so no new tool surface is added when both are on. Defaults off, unlike fix_pr_metadata, because converting a genuinely-ready pull request to draft on a subverted review pulls it out of reviewers' visible queues, a more consequential mistake than only misstating its title or body.
18920c1 to
7aefb03
Compare
There was a problem hiding this comment.
Clean, well-scoped addition. Traced mark_draft_if_unready through every touchpoint — action.yml validation, the MODE != review guard, the shared mcp__github__update_pull_request allowlist grant (correctly OR'd with fix_pr_metadata so the tool isn't double-added), the "This run" facts block, review.yml's passthrough, both README locations (security notes + extended options), and prompts/review/base.md — all consistent with each other and with the stated rationale (defaults off, unlike fix_pr_metadata, because it changes PR visibility/review-request state rather than just title/body text).
No other open PR touches this area (checked #47, #41, #22, #59, #60), and no open issue tracks this, so nothing to reference. Title and description already name the change specifically and explain the why — nothing to rewrite. This is a first review of this PR, so no stale threads/reviews to clean up.
Nothing to flag — approving.
|
🗜️ Headroom context compression
|
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Adds
mark_draft_if_unready, a new review-mode input, off by default.When on, review checks for an explicit, concrete unreadiness signal on the pull request itself: a "WIP"/"do not merge"/"not ready for review" marker in the title, description, or commits, or a diff that is self-evidently incomplete for its own stated purpose (an unimplemented stub, unresolved merge-conflict markers). Only on one of those does it convert the pull request to draft, via
mcp__github__update_pull_request, settingdraft: trueand nothing else.It never fires on ordinary review findings. A finished pull request with real bugs, missing tests, or Blocker/Should-fix comments is reviewed and blocked in the normal way, not marked draft; draft is about the author evidently still being mid-work, not about code quality. It also never sets
draft: false(un-drafting stays a human decision), and it never lets the review submitAPPROVEDon a pull request it has just converted.The underlying tool grant is shared with
fix_pr_metadata(both gate the samemcp__github__update_pull_requestentry), so turning this on alongsidefix_pr_metadataadds no new tool surface. It defaults off, unlikefix_pr_metadata, because a subverted review abusing this could pull a genuinely-ready pull request out of reviewers' visible queues, a more consequential mistake than misstating a title or body — documented in the README's security notes alongside the existingfix_pr_metadataguidance.Wired through
action.yml(input, validation, prompt facts, allowlist grant) and thereview.ymlreusable workflow's passthrough inputs; no change needed to the direct-form examples since the new input defaults tofalse.