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

CLAUDE.md

@@ -98,7 +98,13 @@ any of them — it names the files each rule lives in.
   `run.trigger` spares a draft you asked for after having spoken; the
   withdrawal reason is stricter still (poll-written drafts only, and it
   confirms against the PR with `fetchReviewRequests` before acting, because
-  there its only evidence is a search index's silence). Config's `daemon`
+  there its only evidence is a search index's silence). The poll also does the
+  reverse (`reopenIfAskedAgain`): a settled row comes back when someone
+  requested your review *again* after you settled it — the one thing that
+  reopens a `reviewed`/`skipped` row, since your skip answered the request that
+  was open at the time and cannot have answered a later one. It turns on
+  `settledAt` dating the decision and the last `REVIEW_REQUESTED_EVENT` naming
+  you; team requests never count, because this undoes a decision of yours. Config's `daemon`
   block is re-read every poll, so cockpit toggles apply without a restart.
   Each poll also publishes what it found (`status.awaiting`): the queue filters
   on what you settled locally, which stops matching what GitHub asks you for the

docs/lifecycle.md

@@ -24,7 +24,7 @@ Four things write the artifact, and most questions in this document are really
 
 | Writer | Where | What it may do |
 | --- | --- | --- |
-| **the poll** | `src/server/daemon.ts` | create stubs, archive, file, delete stubs, start drafts — and, with `--auto-send`, submit and mark `sent` |
+| **the poll** | `src/server/daemon.ts` | create stubs, archive, file, reopen a settled row someone asked you about again, delete stubs, start drafts — and, with `--auto-send`, submit and mark `sent` |
 | **startup** | `reconcileRunning`, `src/core/state.ts` | on boot, turn a leftover `running` into `failed` and error a pending chat turn |
 | **the runner** | `src/runner/review.ts`, `chat.ts` | fill in summary / chapters / comments / verdict |
 | **you** | the cockpit → `src/server/index.ts` | edit, mark reviewed/skipped, send, re-review, chat — and, just by opening a review, the automatic refresh that rewrites `pr`, `diff`, the comment anchors and `refresh` |
@@ -55,7 +55,13 @@ Two derived groupings drive most behaviour:
 - **`SETTLED = [sent, reviewed, skipped]`** (`web/src/inbox.ts`) — out of the
   live queue.
 - **`SETTLED_BY_YOU = [reviewed, skipped]`** (`src/runner/review.ts`) — a new
-  push must not drag these back.
+  push must not drag these back. The one thing that can is somebody requesting
+  your review *again* after you settled it — see §5, "Asked again".
+
+Settling one of those stamps `settledAt`. That field exists for exactly one
+question — did this review request come before or after your decision — and
+`updatedAt` cannot answer it, because opening a settled review refreshes it and
+moves that field forward long afterwards.
 
 `archived` is *not* a status. It is `pr.state !== "OPEN"` — merged or closed.
 It moves a row out of the live tabs and out of **settled** and **sent**, and
@@ -103,11 +109,17 @@ running ──cerber restarted─► failed   (reconcileRunning)
 does not:
 
 ```
-any unsent row ──you mark it────────────────► reviewed | skipped
-ready ──the poll finds GitHub moved past it─► reviewed  (+ `filed`)
+any unsent row ──you mark it────────────────► reviewed | skipped  (+ `settledAt`)
+ready ──the poll finds GitHub moved past it─► reviewed  (+ `filed`, `settledAt`)
 ready ──Send / `cerber send` / auto-send────► sent
 ```
 
+And the one way back out of settled without you pressing anything:
+
+```
+reviewed | skipped ──asked for again since `settledAt`──► ready | awaiting
+```
+
 Settling is the one that is not restricted to a finished draft: the cockpit
 offers skip on `awaiting`, `running` and `failed` rows too, so a PR you have
 decided about needs no draft first.
@@ -150,7 +162,10 @@ a token it checks the artifact already on disk:
 1. `force`? → run. (The cockpit's **re-review** button always forces;
    `cerber review --force` too.)
 2. Status in `SETTLED_BY_YOU` (`reviewed`, `skipped`)? → **skip**, log
-   "use --force". *This is why marking a PR reviewed survives a push.*
+   "use --force". *This is why marking a PR reviewed survives a push.* The poll
+   can still take such a row out of settled first — see "Asked again" in §5 —
+   but that is a decision about the queue, made before this guard is reached,
+   and never about spending a run on a row that is still settled.
 3. Status in `HEAD_SENSITIVE` (`ready`, `sent`) **and** the head the last run
    *read* is still the PR's head? → **skip** as up to date. (Which sha that is,
    and why it is not `pr.headSha`, is the paragraph below.)
@@ -253,6 +268,10 @@ walk `walkable()`: open, unsettled rows only.
 | GitHub moved past a finished draft | status → `reviewed`, `filed` set | `fileIfSettledElsewhere` |
 | Auto-send is on and the draft qualified | status → `sent` (the **sent** tab) | `handleAutoSend` |
 
+And the one that *adds* a row back: your review requested again after you
+settled → status → `ready`/`awaiting`, `settledAt` and `filed` cleared
+(`reopenIfAskedAgain`; see "Asked again" below).
+
 A **pure stub** is `awaiting`, with no comments and **`run === null`** — it
 does not look at chapters. The `run` block is what makes this safe: the create
 endpoint persists one before it answers, so a review you pulled in by hand is
@@ -280,6 +299,37 @@ Someone *answering* your comment files nothing — that reply is addressed to
 you. State checks are leashed to one per artifact per 30 minutes and capped
 per poll.
 
+### Asked again: the way back out of settled
+
+Filing's mirror image, and the only thing that reopens a settled row on its own
+(`reopenIfAskedAgain`, `src/server/daemon.ts`). A skip says "I am done with
+this PR", and a push does not undo that. Somebody asking you *again* is a
+different event: your skip answered the request that was open when you made it
+and cannot have answered one that came later — which is what a request
+withdrawn and then re-added is. Nothing else in GitHub's API distinguishes the
+two, so this reads the one thing that does: the timestamp of the last
+`REVIEW_REQUESTED_EVENT` naming you (`fetchLastReviewRequest`, one GraphQL call).
+
+Conditions, all of them: the row is `reviewed` or `skipped`, it was never sent,
+GitHub is asking about it *now* (only rows in the awaiting search are checked
+at all), and the request is newer than `settledAt`. Then the row goes back to
+`ready` — or `awaiting` if it has no finished draft to show — `settledAt` and
+`filed` are cleared, and the ordinary rules take it from there: the freshness
+guard in §4 re-drafts it if the head has moved since the run read it, and
+leaves the existing draft alone if it has not. Same leash as the state checks
+above: one call per artifact per 30 minutes, capped per poll.
+
+Team requests are ignored here on purpose. `stillRequested` counts them because
+refusing to *file work away* is the safe side of that question; this decides to
+undo a decision of yours, where the safe side is doing nothing unless somebody
+named you.
+
+Rows settled before `settledAt` existed are dated by the latest moment cerber
+can prove the decision came after — cerber's own `filed.at`, or `run.finishedAt`
+(you cannot have skipped a draft before it existed). Both are lower bounds, so
+the cost of being wrong is one row coming back once; the alternative is that
+every row settled before this shipped stays unreachable forever.
+
 ### "Whose move is it"
 
 Independent of status. Each poll reads the PR conversation per awaiting PR
@@ -381,16 +431,20 @@ freshness guard skipped is not re-judged or re-logged on later polls.
 (the **sent** tab — `sent` is in `SETTLED` too), it's settled (`reviewed` /
 `skipped` — the **settled** tab, and check **open requests**), the PR is
 merged/closed (**archived**), the poll is off, or GitHub isn't requesting your
-review.
+review. A settled row that GitHub *has* asked you about again since you settled
+it is meant to come back on its own — if it hasn't, the poll is off, or the
+request is older than your decision.
 
 **"Why did this PR come back?"** — The poll re-drafted it, which needs polling
 and auto-review both on and the PR still in the awaiting search. Given that:
 its status was `awaiting` or `failed`, which are retried with no push involved
 at all; or the author pushed and it was `ready` or `sent`, both head-sensitive
 — though a `sent` row also needs someone to have asked you again. Whether you
 have opened the draft since the push makes no difference: the guard compares
-against the head the run *read*, not the one the artifact mentions. Only
-`reviewed` and `skipped` never come back under any of it.
+against the head the run *read*, not the one the artifact mentions. A
+`reviewed` or `skipped` row comes back for one reason only, and it is not a
+push: somebody requested your review *again* after you settled it — see "Asked
+again" in §5.
 
 **"Why won't it re-review?"** — Both causes are the freshness guard in §4, so
 they only bind the callers that obey it (the poll, plain `cerber review`):

src/core/artifact.ts

@@ -316,6 +316,18 @@ export const ArtifactSchema = z.object({
   sent: SentInfoSchema.nullable().default(null),
   /** Set when cerber filed this draft away itself, and why. Never on a sent one. */
   filed: FiledInfoSchema.nullable().default(null),
+  /**
+   * When this review was settled — marked `reviewed` or `skipped`, by you or by
+   * cerber filing it. Null on anything not settled.
+   *
+   * A settle answers the request that was open at the time. It cannot answer
+   * one that came afterwards, so the poll needs to know which side of it a
+   * review request falls on before deciding a settled row stays settled
+   * (`askedAgainAfterSettling` in `src/server/daemon.ts`). `updatedAt` cannot
+   * stand in for this: opening a settled review refreshes it, which moves that
+   * field forward long after the decision it is meant to date.
+   */
+  settledAt: z.string().nullable().default(null),
   /** Last time this review was pulled forward onto a newer head commit. */
   refresh: RefreshInfoSchema.nullable().default(null),
   calibration: CalibrationSchema.nullable().default(null),

src/core/autosend.test.ts

@@ -36,6 +36,7 @@ function makeArtifact(overrides: Partial<Artifact> = {}): Artifact {
     sent: null,
     refresh: null,
     filed: null,
+    settledAt: null,
     calibration: null,
   chat: [],
   preChat: null,

src/core/gh.test.ts

@@ -1,6 +1,6 @@
 import { execFile } from "node:child_process";
 import { Mock, beforeEach, describe, expect, it, vi } from "vitest";
-import { classifyReply, currentLogin, latestOwnReview, resetLoginCache } from "./gh.js";
+import { classifyReply, currentLogin, lastRequestOf, latestOwnReview, resetLoginCache } from "./gh.js";
 
 // gh.ts calls `promisify(execFile)`, which honours this symbol — so the mock
 // resolves to the `{ stdout }` shape the real one does, while still recording
@@ -148,6 +148,41 @@ describe("latestOwnReview", () => {
   });
 });
 
+describe("lastRequestOf", () => {
+  const asked = (login: string, createdAt: string, typename = "User") => ({
+    createdAt,
+    requestedReviewer: { __typename: typename, login },
+  });
+
+  it("finds nothing when nobody has asked you", () => {
+    expect(lastRequestOf([], "me")).toBeNull();
+    expect(lastRequestOf([asked("them", "2026-08-20T10:00:00Z")], "me")).toBeNull();
+  });
+
+  // The whole point: a request that was withdrawn and made again is a second
+  // ask, and only its timestamp can tell it from the first.
+  it("takes the most recent ask, not the first", () => {
+    expect(
+      lastRequestOf(
+        [
+          asked("me", "2026-08-21T10:43:27Z"),
+          asked("them", "2026-08-24T11:00:00Z"),
+          asked("me", "2026-08-24T12:41:22Z"),
+        ],
+        "me",
+      ),
+    ).toBe("2026-08-24T12:41:22Z");
+  });
+
+  // A team request names the team, never you. This answer decides whether to
+  // undo a decision of yours, so it acts only on somebody naming you.
+  it("ignores requests that did not name you", () => {
+    expect(lastRequestOf([{ createdAt: "2026-08-24T12:41:22Z", requestedReviewer: { __typename: "Team" } }], "me")).toBeNull();
+    expect(lastRequestOf([asked("me", "2026-08-24T12:41:22Z", "Bot")], "me")).toBeNull();
+    expect(lastRequestOf([{ createdAt: "2026-08-24T12:41:22Z", requestedReviewer: null }], "me")).toBeNull();
+  });
+});
+
 describe("parsePrRef", () => {
   it("parses a full PR URL", () => {
     expect(parsePrRef("https://github.com/fullstackhouse/skills/pull/22")).toEqual({

src/core/gh.ts

@@ -356,6 +356,80 @@ export function stillRequested(requests: ReviewRequests, login: string): boolean
   return requests.teams.length > 0 || requests.users.includes(login);
 }
 
+/**
+ * When someone last asked *you*, by name, for a review — and nothing else.
+ *
+ * `fetchReviewRequests` answers whether a request is open; this answers when it
+ * was made, which is the only way to tell a request you already dealt with from
+ * a second one asking again. A withdrawn-then-re-added request looks identical
+ * to the original in every other read GitHub offers.
+ *
+ * Team requests are deliberately ignored. `stillRequested` counts them because
+ * refusing to file work away is the safe side of that question; this one decides
+ * to *undo* a decision of yours, where the safe side is doing nothing unless
+ * somebody named you.
+ */
+export interface RequestedReviewEvent {
+  createdAt: string;
+  requestedReviewer: { __typename?: string; login?: string } | null;
+}
+
+export function lastRequestOf(events: RequestedReviewEvent[], login: string): string | null {
+  const mine = events.filter((e) => e.requestedReviewer?.__typename === "User" && e.requestedReviewer.login === login);
+  if (mine.length === 0) return null;
+  return mine.reduce((a, b) => (b.createdAt > a.createdAt ? b : a)).createdAt;
+}
+
+// 100 is the page maximum, and it costs exactly what a smaller window would:
+// one call. It wants to be big because the recent events are not the useful
+// ones — a PR that cycled through a dozen reviewers can push the request that
+// named *you* past a short window, and the answer would come back "nobody
+// asked", silently restoring the bug this exists to fix.
+//
+// It is still a window, not a guarantee: a PR carrying more than 100 review
+// requests would lose the oldest, and that is accepted rather than paginated.
+// Paginating would cost one call per extra page on every settled row on every
+// poll, to cover a PR that does not realistically exist — and the failure mode
+// is the conservative one, a row left settled rather than one wrongly reopened.
+const LAST_REQUEST_QUERY = `query($owner:String!,$repo:String!,$number:Int!){
+  repository(owner:$owner,name:$repo){
+    pullRequest(number:$number){
+      timelineItems(last:100,itemTypes:[REVIEW_REQUESTED_EVENT]){
+        nodes{... on ReviewRequestedEvent{createdAt requestedReviewer{__typename ... on User{login}}}}
+      }
+    }
+  }
+}`;
+
+/**
+ * Ask GitHub when your review was last requested on a PR.
+ *
+ * GraphQL rather than the REST timeline on purpose: this runs on rows the queue
+ * already holds, poll after poll, and the REST timeline of a busy PR is hundreds
+ * of events across several pages — every one of them fetched to find the handful
+ * that are review requests. Filtering server-side to the one event type turns
+ * that into a single call; the window it reads is bounded, as the query above
+ * explains.
+ */
+export async function fetchLastReviewRequest(ref: PrRef, login: string): Promise<string | null> {
+  const out = await gh([
+    "api",
+    "graphql",
+    "-f",
+    `query=${LAST_REQUEST_QUERY}`,
+    "-F",
+    `owner=${ref.owner}`,
+    "-F",
+    `repo=${ref.repo}`,
+    "-F",
+    `number=${ref.number}`,
+  ]);
+  const raw = JSON.parse(out) as {
+    data?: { repository?: { pullRequest?: { timelineItems?: { nodes?: RequestedReviewEvent[] } } } };
+  };
+  return lastRequestOf(raw.data?.repository?.pullRequest?.timelineItems?.nodes ?? [], login);
+}
+
 export function searchAwaitingArgs(repoFilter?: string, limit = 50): string[] {
   const args = [
     "search",

src/core/refresh.test.ts

@@ -70,6 +70,7 @@ function makeArtifact(overrides: Partial<Artifact> = {}): Artifact {
     sent: null,
     refresh: null,
     filed: null,
+    settledAt: null,
     calibration: null,
   chat: [],
   preChat: null,

src/core/refresh.ts

@@ -86,6 +86,10 @@ export function mergeRunResult(fresh: Artifact, current: Artifact): Artifact {
   return {
     ...fresh,
     status: userOwnsStatus(current) ? current.status : fresh.status,
+    // Travels with the status it dates. A settle that landed while the run
+    // worked keeps both halves of itself, and a run that reopens the row takes
+    // the stamp away with the status.
+    settledAt: userOwnsStatus(current) ? current.settledAt : fresh.settledAt,
     sent: current.sent,
     calibration: current.calibration,
     filed: current.filed,

src/core/revise.test.ts

@@ -52,6 +52,7 @@ function artifact(over: Partial<Artifact> = {}): Artifact {
     sent: null,
     refresh: null,
     filed: null,
+    settledAt: null,
     calibration: null,
     chat: [],
     preChat: null,

src/core/send.test.ts

@@ -48,6 +48,7 @@ function makeArtifact(overrides: Partial<Artifact> = {}): Artifact {
     sent: null,
     refresh: null,
     filed: null,
+    settledAt: null,
     calibration: null,
   chat: [],
   preChat: null,

src/core/state.test.ts

@@ -42,6 +42,7 @@ function artifact(over: Partial<Artifact> = {}): Artifact {
     sent: null,
     refresh: null,
     filed: null,
+    settledAt: null,
     calibration: null,
     chat: [],
     preChat: null,

src/runner/chat.test.ts

@@ -69,6 +69,7 @@ function artifact(over: Partial<Artifact> = {}): Artifact {
     sent: null,
     refresh: null,
     filed: null,
+    settledAt: null,
     calibration: null,
     chat: [],
     preChat: null,

src/runner/review-merge.test.ts

@@ -85,6 +85,7 @@ function ready(comments: Comment[], headSha = "old-sha"): Artifact {
     run: null,
     sent: null,
     filed: null,
+    settledAt: null,
     refresh: null,
     calibration: null,
     chat: [],
@@ -180,6 +181,31 @@ describe("what a re-review does to a decision you made while it ran", () => {
     }
   });
 
+  it("keeps the settle whole — the stamp with the status", async () => {
+    // The stamp is what the poll compares a review request against. Half a
+    // settle would leave the row settled with nothing to date the decision by,
+    // and a re-request would then read as older than a skip that came after it.
+    await saveArtifact(ready([]));
+    const settledAt = "2026-08-21T10:02:00.000Z";
+    claudeThat(async () => {
+      await updateArtifactByKey(KEY, (a) => ({ ...a, status: "skipped" as const, settledAt }));
+    });
+
+    const { artifact } = await reviewPr(REF, { withSource: false });
+    expect(artifact.status).toBe("skipped");
+    expect(artifact.settledAt).toBe(settledAt);
+  });
+
+  // A forced re-review is you taking the row back, so the settle goes entirely.
+  it("clears the stamp when it does reopen a row", async () => {
+    await saveArtifact({ ...ready([]), status: "skipped" as const, settledAt: "2026-08-20T09:00:00Z" });
+    claudeThat(async () => {});
+
+    const { artifact } = await reviewPr(REF, { withSource: false, force: true });
+    expect(artifact.status).toBe("ready");
+    expect(artifact.settledAt).toBeNull();
+  });
+
   it("does not reopen one you settled when the run fails either", async () => {
     // A failure is still just a fact about the code; it does not overrule a
     // decision about the PR any more than a success does.