Skip to content

feat(inbox): a settled review comes back when someone asks for you in words - #45

Merged
jtomaszewski merged 1 commit into
mainfrom
jtomaszewski/missing-pr-comment
Aug 26, 2026
Merged

feat(inbox): a settled review comes back when someone asks for you in words#45
jtomaszewski merged 1 commit into
mainfrom
jtomaszewski/missing-pr-comment

Conversation

@jtomaszewski

Copy link
Copy Markdown
Contributor

The problem

A review you settled — marked reviewed or skipped — stays out of the inbox no
matter how many times the author pushes. That is deliberate: a row dragged back
every time someone commits is cerber overruling you. The one way back in was
somebody asking for your review again, and cerber only recognised one form of
asking: GitHub's re-request button.

People frequently don't press it. They write "@you this is ready for another
look" instead — the same ask, made the way people actually make it, and one
GitHub emits no timeline event for at all. Cerber read straight past it, so the
row sat in settled, invisible, while its author waited on a review they had
already asked for in plain words on the PR.

The fix

The poll already reads each PR's conversation, for whose-move-is-it. It now
reads the same comments for the last one that names you, and reopens the row
under the same after-settling test the button already used: an ask newer than
settledAt undoes the settle, anything older is the ask you already answered.

What doesn't count, and why:

  • Your own comments and bots. A bot that @-mentions the reviewer on every
    push would otherwise be an ask on every push.
  • @org/team. A room being addressed is not somebody asking for you
    the mirror of the rule the button check already applies to team requests,
    and for the same reason: this undoes a decision of yours, so the safe side is
    doing nothing unless somebody named you.
  • A longer login that starts with yours. @me must not match @me-bot,
    and an email address must not match at all.

Two places, because one would half-work

  • Rows GitHub is still asking about (they're in the awaiting search) read
    the button first, and the conversation only if the button didn't already
    answer. A live re-request says everything a comment could, so the second call
    is spared.
  • Rows GitHub has stopped asking about read the words alone. This case is
    not an edge: reviewing on github.com clears the request and drops the PR out
    of the awaiting search — which is precisely the state a "ready for another
    look" comment tends to arrive in. That check hangs off the state-refresh
    leash that was already spending a call on those rows.

An unsettled row still costs no GitHub call on either path, a failed read still
changes nothing, and the leash stamp is still spent before the call so an
outage can't turn every settled row into a retry storm.

Technical details

  • src/core/gh.ts — the conversation read now carries each comment's body.
    mentionsYou() matches @login case-insensitively with a not-a-login-char
    boundary; lastMentionOfYou() picks the newest qualifying comment by date
    rather than by position, since this undoes a settle and shouldn't lean on the
    API returning things in order.
  • src/server/daemon.ts — the reopen now takes an Ask, a discriminated union
    over the two ways of asking. reopenIfAskedAgain (awaiting search) and
    reopenIfAskedInWords (state refresh) both funnel into one reopen(), which
    keeps the existing atomic re-check against the on-disk artifact so a
    re-review or send started mid-read isn't overwritten. askWording() is one
    place, so the terminal line and the row's history note can't tell different
    stories about why a decision of yours was undone.
  • Only the GitHub reads are inside the try. A write that fails is a real
    fault and is no longer swallowed as "the read didn't work out".

Verification

pnpm typecheck, pnpm test (516 passing, 18 new) and pnpm build all green
locally.

Beyond the unit tests, the new code was run against the real PR conversation
that surfaced this — the row as it stood would have come back into the inbox on
the next poll, dated by the comment that asked:

settledAtOf   = 2026-08-24T15:45:44Z
last mention  = 2026-08-26T07:05:37Z  "@… this PR is now ready for re-review"
would reopen? = true -> ready

New tests cover both paths (in and out of the awaiting search), the
precedence short-circuit, mentions older than the settle, and each thing that
must not count as an ask.

Docs

SPEC.md §9.6 is rewritten for both ways of asking, with normative rules for
the mention and for where each check runs; §8.2's transition diagram, §21.4's
pseudocode and the conformance checklist follow. docs/lifecycle.md's "Asked
again" section and CLAUDE.md's daemon paragraph likewise.

Worth knowing

A sent row is still never reopened by this — sent rows track the head
instead, so a push is what brings them back. That's unchanged and deliberate.

🤖 Generated with Claude Code

… words

The reopen rule listened for GitHub's re-request button and nothing else, so
a settled row stayed invisible while its author waited — having said so on the
PR in plain words. "@you this is ready for another look" is the same ask, made
the way people actually make it, and GitHub emits no event for it at all.

The poll already reads the conversation for whose-move; it now also reads it
for the last comment naming you, and reopens under the same after-settling
test the button uses. Your own comments never count, bots never do, `@org/team`
never does, and a longer login that starts with yours is not you.

Two places, because a fix that only worked while a request was open would
half-work: rows GitHub is still asking about read the button first and the
words only if the button did not already answer, and rows it has stopped
asking about — you reviewed on github.com, which clears the request — read the
words alone, off the state-refresh leash that was already spending a call on
them. An unsettled row still costs no GitHub call.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

Copilot AI 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.

Pull request overview

This PR updates cerber’s daemon reopen logic so a review you previously settled (reviewed/skipped) can return to the inbox not only when GitHub’s re-request button is used, but also when someone explicitly @-mentions you in a PR comment asking for another look. It extends the existing polling behavior while preserving the “pushes don’t reopen settled work” principle and documents the normative rules in the spec and lifecycle docs.

Changes:

  • Add mention-based “asked again” detection by fetching PR conversation comment bodies, matching @login robustly, and selecting the newest qualifying mention by timestamp.
  • Refactor daemon reopen logic to model “ask” as a discriminated union (requested vs mentioned) and centralize reopen wording for logs/history.
  • Add unit tests for both reopen paths (awaiting-search path and out-of-search state-refresh path), including the precedence short-circuit and non-qualifying mentions (bots/self/team/longer-prefix logins), plus update SPEC/docs accordingly.

Reviewed changes

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

Show a summary per file
File Description
src/server/daemon.ts Implements the two-path reopen logic (re-request event vs @-mention), adds Ask union + centralized wording, and integrates a state-refresh reopen check.
src/server/daemon.test.ts Adds coverage for typed @-mention reopening and ensures the button short-circuit and exclusion rules behave as intended.
src/core/gh.ts Extends conversation fetching to include comment bodies and adds mentionsYou/lastMentionOfYou helpers.
src/core/gh.test.ts Adds focused tests for mentionsYou and lastMentionOfYou, and updates existing fixtures for the new body field.
SPEC.md Updates the normative spec sections to describe both ask mechanisms, their costs/leashes, and wording consistency requirements.
docs/lifecycle.md Updates lifecycle documentation to reflect both reopen mechanisms and where they run.
CLAUDE.md Updates architecture notes to reflect the new reopen behavior and evidence sources.

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

@jtomaszewski
jtomaszewski merged commit 0d0a2f1 into main Aug 26, 2026
3 checks passed
@jtomaszewski
jtomaszewski deleted the jtomaszewski/missing-pr-comment branch August 26, 2026 07:58
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 0.27.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants