Repository navigation
Follow the self-review policy - #515
Merged
Merged
Conversation
Developers now CR and QA their own PRs unless they ask for review, so "needs CR" no longer means someone else owes a review. derive() needs to know which PRs are the author's own job and which are waiting on people who were asked. A pull is ownReview when its author is a developer (config projects.developerTeams, or everyone when that's unset) and nothing requested a review: no person, no team, no claim. askedOf lists who was asked, with a requested GitHub team counting as everyone on the roster team of that name, and askedAt is when. Only pulls waiting on others can starve. An author's own stale stamp now owes a re-stamp, like anyone else's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The self-review policy needs a few things from the server: 1) A clean merge of master into a PR no longer voids its CR and QA stamps, the way GitHub keeps an approval across one. The commit list is only fetched when a stamp actually predates the head, to spare the API quota. A merge that resolved conflicts reads the same as a clean one; the code says so. 2) Review requests to GitHub teams (requested_teams) are stored and sent to the board, which counts one as a request to everyone on the roster team of that name. 3) "Input hints": which areas a PR's diff touches that usually deserve team input (CI, migrations, alerting, agent docs, deploy config, dependencies), from its changed paths, fetched once per head. 4) The board gets the developer roster, the server-side derive uses it, and /api/v1 adds review.own, asked_of and asked_at. 5) Stats history's time to first CR only counts someone other than the author. Migration 0033 adds the two columns; run it before this code. The dummy board seeds team requests, request times, self-stamped pulls and input hints. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With authors stamping their own PRs, "time to first CR" drops to about zero and review debt counts work nobody owes. The Stats tab now shows the time to answer a review request (median and slowest 10%, in hours), who's waiting on someone (open requests by hours, outside contributors' PRs by days), and how merged PRs were reviewed: asked, by someone else, self-reviewed, or not stamped, with a rough count of reverts and quick follow-up fixes after self-reviewed merges. Open PRs by author and repo say "in self-review" and "waiting on others"; stamps per day count only stamps on other people's PRs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A bot's PR still needs someone else's review, but derive() only knew the [bot] suffix, so with no roster configured a config-listed bot like ifixit-systems counted as reviewing its own. The review policy now carries config.bots, on the board and the server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Developers review their own PRs unless they ask, so the board stops treating every unstamped PR as someone else's job. 1) Nothing assigns review: the rotation, "your turn" alerts and cheers, and "Return the favor" are gone. 2) The review queue and Needs QA hold only PRs that asked you (by name or team), ones you said you'd review, and PRs from outside the dev team or bots. Requests lead Waiting on you, oldest first, with "asked 3h ago"; a new request sends a desktop alert. 3) Your own PRs say what you owe: Stamp CR, Stamp QA, Re-stamp, or "waiting on bob · asked 3h ago", turning into "Nudge bob" after 4 hours. "Find a QA-er", "Nudge for a review" and "in the CR queue" are gone, and so is the "Getting QA is a to-do" setting. 4) Claim reads "I'll review" and "Drop it". 5) New: "Could use your input", a closed-by-default fold of other people's self-reviews in your code regions or repos you review, with no counts; and a quiet "worth asking for review?" hint on your PRs that touch CI, migrations, alerting, agent docs, deploy config or dependencies. 6) Words across Review, My work, Team, CI, Classic, Projects stages, cheers and PR jail now match the policy, and "is:asked" finds requests of you. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Requests are answered in hours under the self-review policy, but the
"asked 3h ago" clock only lived in the state popover. Rows waiting on
someone who was asked now carry it as a flag ("asked 3h ago", or
"review asked" for a team request with no time), amber past 4 hours,
naming who was asked on hover.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A review found three holes in the self-review model: 1) A CR comment doesn't clear GitHub's review request, so a reviewer who had stamped stayed "asked": the author was told to nudge them, and everyone on a requested team was told to QA it. askedOf now drops anyone whose CR already counts. 2) Someone who stamped a self-reviewed PR without being asked was told to re-stamp after the next push. On a self-reviewed PR only the author owes a re-stamp now. 3) A team request had no time, so it never got the hours clock. The wire gains team_requests (slug and time); members inherit their team's time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1) Being asked to review means code review: a requested reviewer sees
"Review it" while it's in review, never "QA it". The author tests
their own ("Stamp QA"); Needs QA holds only claims and PRs from
outside the dev team or bots.
2) The "Starving" block in the review queue is gone. Requests of you
come first, oldest request first, then the rest of the queue.
3) "Board's clear" only counts review that's yours.
4) Stats' "Asked" no longer counts a claim.
5) Ready to merge shows your own ready PRs, plus others' only when you
were asked or said you'd review; the author merges. It replaces the
Merge row in Waiting on you.
6) A team request that names nobody on the roster is in nobody's
queue: the author sees "waiting on a review from {team}", and it
can show under Could use your input.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A review found: 1) "Update branch" on GitHub dropped every stamp until the refresh put them back, and sent reviewers a false "Re-stamp owed". The push webhook now checks the new head and skips that when it's a base merge. 2) The keep-the-stamp rule failed open: with no commit dated after the stamp, or a PR past GitHub's 250-commit list, the stamp survived any push. Both now drop it. A commit made before the stamp but pushed after still slips through; the code says so. 3) The commit list was fetched on every webhook, closed PRs included, whenever any old stamp predated the head, and a GitHub error failed the whole refresh. It's now open PRs only, newest stamp per person, cached per head, and errors fall back to the old behavior. 4) Team review requests now record when they were asked, so a team's members get the hours clock. 5) The "alerting" hint fired on any Alert* file, like AlertBanner.tsx; it now matches alerting directories and config files only. The dummy board gets dated team requests and hints on your own PRs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1) A push of real work followed by a merge of master kept its stamps until the refresh ran, and for good if the refresh failed. The push webhook now keeps stamps only when the push is that single merge, and gives up on GitHub after 3 seconds. 2) Changing the developer roster on the Projects tab didn't reach open boards, so they and the API disagreed on who self-reviews until a reconnect. Saving the roster now tells every board. 3) Stats counted the CI review bot as "reviewed by someone else", and listed requests on drafts and PRs already signed off. 4) The commit-list cache kept whole GitHub responses; it keeps only what the stamp rule reads. 5) Roadmap status used the default policy instead of the board's. 6) The "waiting for review" toast said "worth a nudge?" when nobody had been asked; it suggests asking now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A review request to a GitHub team the roster doesn't name, like @iFixit/coders, became nobody's work: no queue, no alert, no nudge. The policy says a request asks everyone listed, and the team people actually request is usually the whole dev team, so a team the roster doesn't name now asks every developer on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # frontend-v2/src/components/JailMode.test.tsx # lib/schema-check.js
A review of the self-review branch found: 1) The amber "asked 5h ago" flag stayed on after CR was met (say, by the author's own stamp), though nothing waited on the people asked any more. It now shows only while CR is still short. 2) The Waiting card dated a team member's wait from the pull's oldest request, so someone asked an hour ago through a team showed as waiting three days when another person was asked by name back then. derive now gives each asked person their own time (askedAtBy). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # lib/schema-check.js
A review noted that with no developer roster a team-only request asks nobody in particular. That's the only honest answer when nobody can be named; the comment now says so where a team request is resolved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Asking for a review was taken as proof the author is a developer. Once the reviewer stamped, they left askedOf (a CR comment doesn't clear GitHub's request), so a contractor's PR fell into nobody's queue and the board told its author to stamp CR and QA themselves. derive now says whether the author is a developer (authorIsDeveloper, also on /api/v1 as review.author_is_developer). A non-developer's PR with nobody left to answer goes back to the open queue and Needs QA, and its author isn't told to self-review. Developers' PRs work as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A review walking each PR's lifecycle found: 1) A signed-off PR from an outside contributor or a bot was on nobody's Ready lane. It's on everyone's again, reading "ready · merge it", since its author usually can't. 2) GitHub clears a review request as soon as the reviewer submits any review, with no event saying so (checked on #63534 and #63360). So after the author's next push the PR read as self-review: the author was told to stamp CR, and the reviewer wasn't asked to re-stamp. The server now reads answered requests from the PR's timeline, and a PR that ever asked someone stays theirs to answer. 3) Stats listed a claimed outside PR as waiting on a first review. 4) An own PR whose next step was "Answer the review" or "Unblock" showed in neither Review lane. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # controllers/settings.js # test/settings.test.js
The last change remembered answered requests but a review found three gaps: 1) While the reviewer owed a re-stamp, the author was still told "Stamp CR", and the PR showed under everyone's Could use your input. The author now waits on the reviewer instead. 2) A webhook that wasn't a push (a label, an edit) rebuilt the requests without the answered ones, so the PR read as self-review until the next push. A login that left the request list is now answered, since withdrawals are removed on their own. 3) An answered request never expired, so after the reviewer left the roster, or when the reviewer was a bot, every push asked them to re-stamp. Only a developer's answered request keeps the PR theirs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"Could use your input" left out any PR with a re-stamp owed, the author's own included, so a PR whose reviewer answered without a stamp dropped out of everyone's view once the author pushed over their own stamp. Only someone else's owed re-stamp takes it out now, matching the author's note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jarstelfox
marked this pull request as ready for review
October 10, 2026 02:47
An error occurred while trying to automatically change base from
pr-jail
to
master
October 10, 2026 02:48
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
1 participant
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.
🤖
closes #512
We changed our CR policy: developers review and QA their own PRs unless they ask for a review (canvas). Pulldasher still treated every unstamped PR as someone else's job. It put them in everyone's review queue, named whose turn it was after 4 days, and never told authors their own stamp went stale. This makes the board follow the policy. It's live on dev and prod already.
Changes
Also new
Important
Run
bin/migrate-missingbefore this code anywhere but dev and prod: migration 0033 adds two columns topulls.Stacked on PR jail; retarget once it merges.
Known limits
Review
Seven rounds of blind review, each fixed before the next; the last found nothing. The biggest fixes along the way: a contractor's PR no longer lands in nobody's queue, a PR that asked someone stays theirs after GitHub clears the request on review (checked against #63534 and #63360 in iFixit/ifixit), a merge of master that carries real work no longer keeps stamps, and signed-off outside and bot PRs are back on everyone's Ready lane.
QA
npm testat the root andnpx vitest runinfrontend-v2.