feat(inbox): a review you skipped comes back when someone asks again

[jtomaszewski]

The problem

You skip a PR. Later its author asks you for a review again — they withdrew the request while you were looking, pushed more work, and re-requested. The PR never comes back. It sits in settled, invisible, while someone waits on you, and the only way to notice is to go looking for it by hand.

This is not the inbox being over-eager in reverse: a skip is meant to survive a push, and that rule is right. The gap is narrower. Your skip answered the review request that was open at the moment you made it. It cannot have answered one that arrived afterwards — and nothing in cerber could tell those two apart, because every read GitHub offers about a review request says only whether one is open, never when it was made.

Found the plain way: a PR requested again yesterday afternoon that was still missing from the inbox this morning.

The fix

Settling now records when. settledAt is stamped by both paths that settle a row — the cockpit's mark reviewed/skipped, and the poll's own filing. updatedAt could not stand in for it: merely opening a settled review refreshes it onto the new head, which moves that field forward long after the decision it would be dating.

Each poll then does filing's mirror image (reopenIfAskedAgain). For rows GitHub is asking about right now — only ever rows already in the awaiting search — it reads the timestamp of the last REVIEW_REQUESTED_EVENT naming you. If that is newer than the stamp, the row goes back into the inbox: ready if it has a finished draft, awaiting if it does not, with settledAt and cerber's own filed note cleared. From there the ordinary rules take over — the freshness guard re-drafts it if the head moved since the run actually read the code, and leaves the existing draft alone if it did not.

This is the only thing that reopens a reviewed/skipped row. A push still does not.

Details

• One GraphQL call, timelineItems(last:20, itemTypes:[REVIEW_REQUESTED_EVENT]) — one request regardless of how long the PR's history is, where the REST timeline would be several pages on a busy PR.
• The same leash as the existing state checks: one call per artifact per 30 minutes, capped per poll. A row that is not settled costs nothing at all — there is a test asserting GitHub is never asked about one.
• Team requests never count. stillRequested honours them because refusing to file work away is the safe side of that question; this one undoes a decision of yours, where the safe side is doing nothing unless somebody named you.
• A read that fails changes nothing — no evidence either way, next poll retries.
• Rows settled before the field existed are dated by the latest moment cerber can prove the decision came after: filed.at, else run.finishedAt (you cannot have skipped a draft that did not exist yet). Both are lower bounds, so the cost of being wrong is one row returning once — against every pre-existing settled row staying unreachable forever.
• mergeRunResult carries settledAt with the status it dates, so a settle that lands mid-run keeps both halves of itself and a forced re-review clears both.

Verification

• pnpm typecheck, pnpm test (471 tests, 20 new), pnpm build — all green.
• New tests cover the poll end to end (reopens on a newer request; leaves the skip standing on an older one; never touches a sent row; clears filed; reopens a draft-less row as awaiting; asks GitHub nothing about unsettled rows; leaves the row alone when GitHub is unreachable), the pure predicates including the milliseconds-vs-seconds comparison trap, the legacy fallbacks, lastRequestOf's filtering, the PATCH stamp, and the run-merge.
• Ran the new predicates against a real artifact on this machine — a skipped review whose author re-requested afterwards — with the real GitHub read: it resolves the legacy date from run.finishedAt, reads the later request, and would reopen it as ready.

Worth knowing

docs/lifecycle.md gains an "Asked again" section, and the transitions table, §4's guard notes and the quick answers now say plainly that a settled row can come back and for exactly one reason. CLAUDE.md's daemon paragraph matches.

No cockpit UI was added for this: the row simply reappears in the inbox, and the daemon logs your review was requested again on … — back in the inbox.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes an inbox blind spot where a PR you previously marked reviewed/skipped could remain hidden in settled even after someone later re-requested your review, by stamping the settle time (settledAt) and having the daemon poll reopen settled rows when a newer REVIEW_REQUESTED_EVENT exists.

Changes:

• Add settledAt to the artifact schema and stamp it when a row is settled (by you or by daemon filing).
• Add a daemon “reopen if asked again” check using a single GraphQL query to fetch the latest review-request timestamp for the current user.
• Add/extend tests covering reopen behavior, legacy fallback dating, and timestamp comparison correctness; update lifecycle docs to describe the new transition.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/server/index.ts	Stamps settledAt on PATCH settle actions.
src/server/guards.test.ts	Adds coverage asserting settle actions are dated.
src/server/daemon.ts	Implements reopen-on-new-request logic and legacy settle dating helpers.
src/server/daemon.test.ts	Adds end-to-end daemon tests for reopening settled rows.
src/server/chat.test.ts	Updates artifact test helper to include settledAt.
src/runner/review.ts	Exports SETTLED_BY_YOU; initializes artifacts with settledAt.
src/runner/review.test.ts	Updates artifact test helper to include settledAt.
src/runner/review-merge.test.ts	Verifies settledAt is preserved/cleared appropriately across run merges.
src/runner/chat.test.ts	Updates artifact test helper to include settledAt.
src/core/state.test.ts	Updates artifact test helper to include settledAt.
src/core/send.test.ts	Updates artifact test helper to include settledAt.
src/core/revise.test.ts	Updates artifact test helper to include settledAt.
src/core/refresh.ts	Ensures settledAt travels with user-owned status during merges.
src/core/refresh.test.ts	Updates artifact test helper to include settledAt.
src/core/gh.ts	Adds GraphQL query + helpers to fetch last review-request time for a user.
src/core/gh.test.ts	Adds unit tests for lastRequestOf.
src/core/autosend.test.ts	Updates artifact test helper to include settledAt.
src/core/artifact.ts	Adds settledAt to the artifact schema with documentation.
docs/lifecycle.md	Documents the new “asked again” transition and settledAt semantics.
CLAUDE.md	Updates daemon behavior description to include reopen-on-request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/core/gh.ts:400

• fetchLastReviewRequest only asks for the last 20 REVIEW_REQUESTED_EVENTs. If a PR has >20 review-request events (e.g. many re-requests to different reviewers), this can return null even though you were requested (just earlier than the last 20), which prevents reopenIfAskedAgain from reopening a settled row and reintroduces the “stuck in settled” behavior for busy PRs.

Consider increasing the window (e.g. to 100) so the “when was I last requested” answer remains reliable without changing the 1-call design.

      timelineItems(last:20,itemTypes:[REVIEW_REQUESTED_EVENT]){
        nodes{... on ReviewRequestedEvent{createdAt requestedReviewer{__typename ... on User{login}}}}
      }
    }

[jtomaszewski]

Second review pass came back with no new inline comments, but its suppressed one was right and is now fixed in 612e7d3.

fetchLastReviewRequest asked for the last 20 request events. That is not the same as all of them: a PR that cycled through a dozen reviewers can push the request that named you off the end, and the answer comes back null — no reopen, row stays in settled, which is precisely the bug this PR is about. Widened to last:100 (the page maximum), which costs exactly what 20 did — one call. Verified against this PR's own timeline.

The three inline threads from the first pass are answered and resolved: the filing note now clears when you settle a row yourself, the truncated lifecycle sentence is finished, and the failed-read comment no longer claims a retry the leash does not allow.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/core/gh.ts:405

• This docstring claims the GraphQL query “holds the lot in one call whatever the PR's history looks like”, but the query is explicitly limited to the last 100 events. Please soften the wording (or implement pagination) so the behavior is accurately described for very busy PRs.

 * GraphQL rather than the REST timeline on purpose: this runs on rows the queue
 * already holds, poll after poll, and the timeline of a busy PR is hundreds of
 * events across several pages. Filtered to the one event type, the last page
 * holds the lot in one call whatever the PR's history looks like.
 */

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

[github-actions]

🎉 This PR is included in version 0.25.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀