Skip to content

RFC: comment-only PR management and automatic review - #174

Merged
tzhouam merged 2 commits into
mainfrom
doc/rfc-pr-state-machine
Sep 22, 2026
Merged

tzhouam merged 2 commits into
mainfrom
doc/rfc-pr-state-machine

Conversation

@tzhouam

@tzhouam tzhouam commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

The PR automation can post comments and COMMENT reviews only. This revision makes that constraint structural, keeps labels in the ledger, defines ready notifications per readiness entry with a durable epoch, and extends the existing stale reminder to 14/30-day suggestions.

Includes lifecycle ready-note cutover, explicit finding rechecks, rollout/rollback rules, and an implementation checklist distinguishing shipped foundations from remaining work. Owner fallback and highest-rung backlog behavior follow the approved implementation plan.

Validation: independent gpt-6-astra-high staged review APPROVED; documentation change.

…o draft to close)

One derived PR state (10 live states + closed) projected from the ledgers the review bot already keeps, a 04:00 nightly review sweep replacing mention-only triggering, a ready gate (CI green, mergeable, reviewed, no open blocker/major explicitly rechecked as fixed via a versioned per-finding SDK result) that pages the routed owner once, a dispute scan that follows every unresolved finding's stored comment id across heads on its own deadline, one REQUEST_CHANGES review per blocking interval keyed on head + block_generation, 7/14/30-day inactivity clocks driven by deadlines as well as events and guarded by the exempt-label rule, parking and closure obligations each bound to the bot's own GitHub event and retired the moment a human overrides them, GitHub writes reconciled as a desired-versus-live artifact set with per-artifact dedupe keys, a reopen watch that answers only to the author, draft transitions via the GraphQL mutations, a stored pr_state row plus transition log, and a four-step rollback that unwinds live artifacts on open PRs, keeps reopen obligations for closed ones, and retires expired obligations before any table is dropped. Flags shadow-first; auto-close and bot REQUEST_CHANGES are flagged for repo-owner sign-off because they override #116's boundaries.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T07:07:31.045256Z daf06b8 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: daf06b88ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +306 to +309
- the author marked it disputed: a 👎 reaction on the bot's inline comment
or a reply in its thread. Disputed findings do not block; they are
counted and shown in the ready page ("2 findings disputed by the
author") so the maintainer arrives at exactly the disagreement.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require an explicit signal before treating replies as disputes

Any author reply is currently enough to make a blocker non-blocking, so a response such as “I’ll fix this” or “fixed” can dismiss the REQUEST_CHANGES review and allow a ready page before a re-review verifies the fix. Replies also cannot be withdrawn like reactions, meaning a later still_affected result remains effectively disputed. Require an explicit dispute command/reaction, or define how reply-based disputes are cleared when a re-review confirms the finding.

Useful? React with 👍 / 👎.

Comment thread doc/RFC-pr-state-machine.md Outdated
Comment on lines +224 to +225
| draft conversion + parked comment | clock ≥ 14 d, bot-managed | PR + window start + `park` | human activity → un-draft (`resume`, same window key) |
| close + closing comment | clock ≥ 30 d, bot-managed | PR + window start + `close` | author comment or new head within `REOPEN_WATCH_DAYS` → reopen, key PR + `closed_by_bot_at` + `reopen`; a bystander's comment never reopens |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep bot-owned closes desired throughout the reopen watch

Immediately after the day-30 action closes a PR, it no longer satisfies bot-managed because that term is defined as open, so the next desired-versus-live reconciliation considers this close artifact unwanted even without author activity. That conflicts with the required 90-day reopen obligation and can cause the close to be retracted or its watch state to be discarded. The desired predicate must continue to include a bot-owned closed PR until author/head activity, human override, or watch expiry.

Useful? React with 👍 / 👎.

Decided 2026-09-22 by the repo owner: the bot never writes GitHub's label
field, never converts a PR to draft, never closes or reopens one, and never
posts a blocking review. It may post comments and COMMENT reviews. That is
its whole outward surface.

This is not a permission GitHub can express. Posting a review needs
pull_requests:write, and the same permission grants labelling, drafting and
closing — there is no setting that allows one and forbids the others. So the
constraint is ours to keep, and it has to be structural: the write methods
come out of the client rather than sitting behind a flag, because a flag is
something somebody can turn on. An acceptance criterion asserts over the
client's public surface, so the constraint fails the build rather than a
review.

Every label in the design still exists, in our ledger rather than on GitHub.
The projection keeps computing each state and the labeler keeps inferring
the repository's content labels from changed files; both are stored and
shown on our own board. What changed is only where they are written down.

The three idle boundaries become notices on one ladder in the module that
already owns idle-PR comments. It has a write budget, so the 715 PRs already
past seven days arrive at ten per cycle instead of at once; idempotency keyed
by the inactivity window, so its own comment cannot retrigger it; a
timeline-verified human watermark; and ledger-backed note rows. A second
emitter would have had to rebuild all of it, and two modules commenting
about idleness on the same PRs would have been the worst outcome. Only the
highest rung whose clock has passed is sent, so a PR idle forty days gets
one notice rather than three.

The ready page is keyed on entry into the state, not on the head and not on
the PR. Keying on the head pages again for every push to a finished PR;
keying on the PR never pages again after CI broke and was fixed. It is
baselined on enable, so turning it on is not an event. A PR with no routed
owner — there are 128 — falls back to the coordinator instead of being
silently skipped.

What came out, and it is most of the original design: REQUEST_CHANGES and
everything it needed (the block_generation counter, body-edit-in-place,
dismissal ordering, one-review-per-blocking-interval), draft conversion, the
close action, the ninety-day reopen watch with its closed_by_bot_at and
close_head_sha columns, the seven bot-prefixed repository labels, the
GraphQL client that existed only for the two draft mutations, and most of
the rollback unwind — there is nothing to unwind on a PR when nothing was
done to it. Three of the five decisions that needed owner sign-off are gone,
because they were requests for authority the bot no longer has.

Stated plainly in the document: the bot can no longer act on a PR, only talk
about it, so one maintainer touch per merged PR is a ceiling rather than a
target — every state change is now a human action. What survives is the part
that was always worth most, which is that a maintainer is told when a PR
deserves them instead of finding out by reading the list. An open question
asks whether a weekly digest to one coordinator would close more PRs than
715 individual comments, which is the measurement that would settle it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tzhouam tzhouam changed the title RFC: PR state machine — maintainers join when the PR is almost ready RFC: comment-only PR management and automatic review Sep 22, 2026
@tzhouam
tzhouam merged commit d8f96b6 into main Sep 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant