Public: anyone can read this; the crew can change it.
The crew's now list says the reviewer is paused when its seat is set to 0, not that there is no reviewer #8778
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/8733-the-crew-s-now-list-says-the-reviewer-is → main 5 commits fork 3 threads · 3 unresolved
8 files
changed +93
−14
src/actions/crew_now.ts +3 −1
| @@ -9,6 +9,8 @@ import { getIssueById, listOpenAgentPullsVisibleTo } from "../core/issues.ts"; | ||
| 9 | 9 | import { waitsForPerson } from "../crew/review.ts"; |
| 10 | 10 | import { getPull } from "../core/pulls.ts"; |
| 11 | 11 | import { ownerFactRepoIds } from "../core/repos.ts"; |
| 12 | import { reviewSays } from "../crew/review_says.ts"; | |
| 13 | import { reviewerPaused } from "../crew/reviewer_paused.ts"; | |
| 12 | 14 | import { plannerDeclines } from "../crew/autonomy.ts"; |
| 13 | 15 | import { listActiveRuns } from "../core/runs.ts"; |
| 14 | 16 | import { getUsersByIds } from "../core/users.ts"; |
| @@ -71,7 +73,7 @@ export function crewNow(ctx: Ctx, user: User) { | ||
| 71 | 73 | on: `#${issue.number} ${issue.title}`, |
| 72 | 74 | why: "review", |
| 73 | 75 | state: "waiting for your review", |
| 74 | says: wait.why === "lapsed" ? "the reviewer did not answer" : "no reviewer on the crew", | |
| 76 | says: reviewSays(wait, reviewerPaused(repo)), | |
| 75 | 77 | since: wait.since, |
| 76 | 78 | costUsd: 0, |
| 77 | 79 | }, |
src/actions/owner_waits.ts +2 −1
| @@ -10,6 +10,7 @@ import { getIssuesByIds, type Issue } from "../core/issues.ts"; | ||
| 10 | 10 | import { joinMany } from "../core/pulls.ts"; |
| 11 | 11 | import type { Repo } from "../core/repos.ts"; |
| 12 | 12 | import { plannerDeclines } from "../crew/autonomy.ts"; |
| 13 | import { reviewerPaused } from "../crew/reviewer_paused.ts"; | |
| 13 | 14 | import { reviewersOf, waitsForPerson } from "../crew/review.ts"; |
| 14 | 15 | import type { Database } from "bun:sqlite"; |
| 15 | 16 | |
| @@ -51,7 +52,7 @@ export function reviewWaits(db: Database, repos: readonly Repo[]): OwnerWait[] { | ||
| 51 | 52 | const w = waitsForPerson(db, repo.id, p); |
| 52 | 53 | return w === undefined ? [] : [w]; |
| 53 | 54 | }); |
| 54 | const paused = repo.reviewerSlots === 0; | |
| 55 | const paused = reviewerPaused(repo); | |
| 55 | 56 | const none = reviewersOf(db, repo.id).length === 0; |
| 56 | 57 | if (!none && !paused && waits.length === 0) return []; |
| 57 | 58 | return [ |
src/crew/review.ts +2 −1
| @@ -29,6 +29,7 @@ import { reviewSummariesFor, reviewSummary } from "../core/reviews.ts"; | ||
| 29 | 29 | import { checksForHead } from "../core/runs.ts"; |
| 30 | 30 | import { getTeam } from "../core/teams.ts"; |
| 31 | 31 | import { createLog } from "../kernel/log.ts"; |
| 32 | import { reviewerPaused } from "./reviewer_paused.ts"; | |
| 32 | 33 | import { queueAgentRun } from "./dispatch.ts"; |
| 33 | 34 | import { type Handover, handoverOf, handoversOf } from "./guard.ts"; |
| 34 | 35 | |
| @@ -449,7 +450,7 @@ export async function sendToReview( | ||
| 449 | 450 | if (getHold(app.db, pull.id) !== undefined) return "held"; |
| 450 | 451 | const reviewers = reviewersOf(app.db, repo.id); |
| 451 | 452 | // A paused reviewer reads like an empty seat: the pull waits for a person, and says so. |
| 452 | if (reviewers.length === 0 || repo.reviewerSlots === 0) return "none"; | |
| 453 | if (reviewers.length === 0 || reviewerPaused(repo)) return "none"; | |
| 453 | 454 | // The reviewer's slots bound every route, the teams' too. The most (PILLAR_MAX) is no cap: the sweep already bounds what it sends. |
| 454 | 455 | if (repo.reviewerSlots < PILLAR_MAX && reviewsInFlight(app.db, repo.id) >= repo.reviewerSlots) return "wait"; |
| 455 | 456 | if (teamsOfPull(app.db, pull).length > 0) { |
src/crew/review_says.ts added +16
| @@ -0,0 +1,16 @@ | ||
| 1 | /** | |
| 2 | * Why an agent's green pull waits for a person, in words: one place, so the | |
| 3 | * asks, the decision cards, the crew's now list and the MCP cannot say it | |
| 4 | * differently. `paused` is the reviewer seat at 0 (`reviewerPaused`). | |
| 5 | */ | |
| 6 | import type { ReviewWait } from "./review.ts"; | |
| 7 | ||
| 8 | /** The reason alone, without its date. */ | |
| 9 | export function reviewSays(wait: ReviewWait | undefined, paused: boolean): string { | |
| 10 | if (wait?.why === "rounds") return "the reviewer asked for changes three times"; | |
| 11 | if (wait?.why === "lapsed" && paused) return "the reviewer is paused"; | |
| 12 | if (wait?.why === "lapsed" && wait.lateCheckMinutes !== undefined) | |
| 13 | return `review ran late: the check took ${wait.lateCheckMinutes} min, and the reviewer did not answer`; | |
| 14 | if (wait?.why === "lapsed") return "the reviewer did not answer"; | |
| 15 | return "no reviewer on the crew"; | |
| 16 | } |
src/crew/reviewer_paused.ts added +4
| @@ -0,0 +1,4 @@ | ||
| 1 | /** Whether the reviewer seat is set to 0: on the crew, starting nothing. Every reading of "paused" goes through it. */ | |
| 2 | import type { Repo } from "../core/repos.ts"; | |
| 3 | ||
| 4 | export const reviewerPaused = (repo: Pick<Repo, "reviewerSlots">): boolean => repo.reviewerSlots === 0; |
src/web/studio/asks.ts +5 −9
| @@ -59,6 +59,8 @@ import { breakerParked, WAITING_NOTES } from "../../crew/breaker.ts"; | ||
| 59 | 59 | import { CHECK_FAILED_LIMIT } from "../../crew/dispatch.ts"; |
| 60 | 60 | import { handoversOf, passReasonOf, type Handover } from "../../crew/guard.ts"; |
| 61 | 61 | import { conflictsOf, isStalePull, mergesAgainSince, staleGreen } from "../../crew/stale_asks.ts"; |
| 62 | import { reviewSays } from "../../crew/review_says.ts"; | |
| 63 | import { reviewerPaused } from "../../crew/reviewer_paused.ts"; | |
| 62 | 64 | import { reviewWaitsFor, type ReviewWait } from "../../crew/review.ts"; |
| 63 | 65 | import type { Ctx } from "../../http/context.ts"; |
| 64 | 66 | import { NO_LONGER_ASKS } from "../../actions/asks_words.ts"; |
| @@ -1015,21 +1017,15 @@ function lineOf(c: AskCandidate): string { | ||
| 1015 | 1017 | return `"${c.pull?.title ?? ""}" had three reviews asking for changes and waits for you`; |
| 1016 | 1018 | const title = c.pull?.title ?? ""; |
| 1017 | 1019 | if (c.kind === "review") |
| 1018 | return `"${title}" waits for your review: ${reviewWhy(c.reviewWait)}`; | |
| 1020 | return `"${title}" waits for your review: ${reviewWhy(c.reviewWait, reviewerPaused(c.repo))}`; | |
| 1019 | 1021 | return c.kind === "stopped" |
| 1020 | 1022 | ? `the crew stopped on "${title}"` |
| 1021 | 1023 | : `"${title}" is green, waiting for your look`; |
| 1022 | 1024 | } |
| 1023 | 1025 | |
| 1024 | 1026 | /** Why an agent's change waits for a person's review, in a clause. */ |
| 1025 | export const reviewWhy = (wait: ReviewWait | undefined): string => | |
| 1026 | wait?.why === "rounds" | |
| 1027 | ? `the reviewer asked for changes three times, since ${wait.since.slice(0, 10)}` | |
| 1028 | : wait?.why === "lapsed" && wait.lateCheckMinutes !== undefined | |
| 1029 | ? `review ran late: the check took ${wait.lateCheckMinutes} min, and the reviewer did not answer, since ${wait.since.slice(0, 10)}` | |
| 1030 | : wait?.why === "lapsed" | |
| 1031 | ? `the reviewer did not answer, since ${wait.since.slice(0, 10)}` | |
| 1032 | : `no reviewer on the crew, since ${wait?.since.slice(0, 10) ?? "it went green"}`; | |
| 1027 | export const reviewWhy = (wait: ReviewWait | undefined, paused = false): string => | |
| 1028 | `${reviewSays(wait, paused)}, since ${wait?.since.slice(0, 10) ?? "it went green"}`; | |
| 1033 | 1029 | |
| 1034 | 1030 | /** A collapsed row in a line: how many, and of what. */ |
| 1035 | 1031 | export const ROW_PLURAL: Readonly<Record<DecisionKind, (n: number) => string>> = { |
src/web/studio/decisions.ts +3 −2
| @@ -20,6 +20,7 @@ import { btn } from "../ui.ts"; | ||
| 20 | 20 | import { thumbLook } from "../pages/looks.ts"; |
| 21 | 21 | import { changeHref } from "./nouns.ts"; |
| 22 | 22 | import { testsClause } from "./words.ts"; |
| 23 | import { reviewerPaused } from "../../crew/reviewer_paused.ts"; | |
| 23 | 24 | import { reviewWhy, scoredFor, waitingOn, type Decision, type Scored } from "./asks.ts"; |
| 24 | 25 | |
| 25 | 26 | /** Dollars as a person reads them: cents when small, whole when not. */ |
| @@ -156,8 +157,8 @@ export function decisionsFor( | ||
| 156 | 157 | repo: c.repo, |
| 157 | 158 | pull: c.pull, |
| 158 | 159 | who: c.who, |
| 159 | says: `${c.who}'s "${c.pull.title}" is green and waits for a review: ${reviewWhy(c.reviewWait)}. An agent's change never ships unreviewed; ship it, or say what is off.`, | |
| 160 | short: `green; waits for a review — ${reviewWhy(c.reviewWait)}`, | |
| 160 | says: `${c.who}'s "${c.pull.title}" is green and waits for a review: ${reviewWhy(c.reviewWait, reviewerPaused(c.repo))}. An agent's change never ships unreviewed; ship it, or say what is off.`, | |
| 161 | short: `green; waits for a review — ${reviewWhy(c.reviewWait, reviewerPaused(c.repo))}`, | |
| 161 | 162 | }; |
| 162 | 163 | if (c.kind === "look" && c.pull) { |
| 163 | 164 | const picture = pictureOf(c.repo, c.pull); |
test/seat-slots.test.ts +58
| @@ -13,6 +13,10 @@ import { createApp, type App } from "../src/app.ts"; | ||
| 13 | 13 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; |
| 14 | 14 | import { commitToBranch, createRepo, updateRepoSettings } from "../src/actions/repos.ts"; |
| 15 | 15 | import { setCrewRole } from "../src/core/agents.ts"; |
| 16 | import { crewNow } from "../src/actions/crew_now.ts"; | |
| 17 | import { reviewSays } from "../src/crew/review_says.ts"; | |
| 18 | import { askLinesFor } from "../src/web/studio/asks.ts"; | |
| 19 | import { getPull } from "../src/core/pulls.ts"; | |
| 16 | 20 | import { reviewWaits, waitLine } from "../src/actions/owner_waits.ts"; |
| 17 | 21 | import { checkAutonomy, getRepoById, type Repo } from "../src/core/repos.ts"; |
| 18 | 22 | import { openPull } from "../src/actions/pulls.ts"; |
| @@ -185,6 +189,60 @@ test("a paused reviewer is said to be paused in what waits for the owner", () => | ||
| 185 | 189 | app.db.query("UPDATE repo SET reviewer_slots = 8 WHERE id = ?1").run(repo.id); |
| 186 | 190 | }); |
| 187 | 191 | |
| 192 | test("a lapsed review says why on the now list, the MCP and the asks: paused, unanswered, or no reviewer", async () => { | |
| 193 | // A green agent pull, its check long done, one review sent and never answered. | |
| 194 | const mason = getUserByName(app.db, "mason")!; | |
| 195 | ok(await commitToBranch(app, alice, fresh(), { branch: "feature/lapse", base: "main", message: "lapse", files: [{ path: "lapse.ts", content: "export const n = 1;\n" }] })); | |
| 196 | const opened = ok(await openPull(app, alice, fresh(), { title: "lapse", baseBranch: "main", headBranch: "feature/lapse" })); | |
| 197 | app.db.query("UPDATE issue SET author_id = ?2 WHERE id = ?1").run(opened.id, mason.id); | |
| 198 | const pull = getPull(app.db, repo.id, opened.number)!; | |
| 199 | const check = createRun(app.db, { repoId: repo.id, kind: "check", issueId: pull.id, env: `pull-${pull.number}`, commitOid: pull.headOid, reason: "pull" }); | |
| 200 | claimRun(app.db, check.id); | |
| 201 | finishRun(app.db, check.id, { state: "ok", summary: "green" }); | |
| 202 | app.db.query("INSERT OR IGNORE INTO app (id, repo_id, slug, access, preview_data) VALUES (?1, ?2, 'yard', 'public', 'copy')").run(`app-${repo.id}`, repo.id); | |
| 203 | ok(await updateRepoSettings(app, alice, fresh(), { reviewerSlots: 8 })); | |
| 204 | expect(await sendToReview(app, fresh(), getPull(app.db, repo.id, pull.number)!)).toBe("sent"); | |
| 205 | for (const run of app.db.query<{ id: string }, [string]>("SELECT id FROM run WHERE issue_id = ?1 AND reason = 'review' AND state = 'queued'").all(pull.id)) { | |
| 206 | claimRun(app.db, run.id); | |
| 207 | finishRun(app.db, run.id, { state: "failed", summary: "reviewed: no verdict was posted" }); | |
| 208 | } | |
| 209 | const old = new Date(Date.now() - 30 * 24 * 3_600_000).toISOString(); | |
| 210 | app.db.query("UPDATE run SET created_at = ?2, finished_at = ?2 WHERE issue_id = ?1").run(pull.id, old); | |
| 211 | app.db.query("UPDATE issue SET author_id = ?2 WHERE id = ?1").run(pull.id, mason.id); | |
| 212 | ||
| 213 | const says = async () => { | |
| 214 | const rows = crewNow({ app } as never, alice).flatMap((r) => (r.why === "review" && "says" in r ? [r] : [])); | |
| 215 | expect(rows).toHaveLength(1); | |
| 216 | const viaMcp = (await mcpTool(servers.url, admin, "now", {})).value as { why: string; says?: string }[]; | |
| 217 | expect(viaMcp.find((r) => r.why === "review")?.says).toBe(rows[0]!.says); | |
| 218 | const ask = askLinesFor(app.db, alice).find((l) => l.id === `review:${pull.id}`); | |
| 219 | return { now: rows[0]!.says, ask: ask?.line ?? "" }; | |
| 220 | }; | |
| 221 | const judge = getUserByName(app.db, "judge")!; | |
| 222 | ||
| 223 | const live = await says(); | |
| 224 | expect(live.now).toBe("the reviewer did not answer"); | |
| 225 | expect(live.ask).toContain("the reviewer did not answer"); | |
| 226 | ||
| 227 | app.db.query("UPDATE repo SET reviewer_slots = 0 WHERE id = ?1").run(repo.id); | |
| 228 | const paused = await says(); | |
| 229 | expect(paused.now).toBe("the reviewer is paused"); | |
| 230 | expect(paused.ask).toContain("the reviewer is paused"); | |
| 231 | app.db.query("UPDATE repo SET reviewer_slots = 8 WHERE id = ?1").run(repo.id); | |
| 232 | ||
| 233 | setCrewRole(app.db, repo.id, judge.id, "builder"); | |
| 234 | const none = await says(); | |
| 235 | expect(none.now).toBe("no reviewer on the crew"); | |
| 236 | expect(none.ask).toContain("no reviewer on the crew"); | |
| 237 | setCrewRole(app.db, repo.id, judge.id, "reviewer"); | |
| 238 | }); | |
| 239 | ||
| 240 | test("three change requests are worded as such, never as an empty crew", () => { | |
| 241 | const rounds = { why: "rounds", since: "2026-10-01T00:00:00Z" } as const; | |
| 242 | expect(reviewSays(rounds, false)).toBe("the reviewer asked for changes three times"); | |
| 243 | expect(reviewSays(rounds, true)).toBe("the reviewer asked for changes three times"); | |
| 244 | }); | |
| 245 | ||
| 188 | 246 | test("over the MCP: read, plan, apply, and refuse an agent or a bad number", async () => { |
| 189 | 247 | const read = await crew(admin, {}); |
| 190 | 248 | expect(read.value.slots.map((s: { role: string }) => s.role)).toEqual(["planner", "builder", "reviewer"]); |
new above
offline
This tests the word functions only. Nothing runs crewNow, the MCP now tool or the ask/decision lines, so a revert of the call sites passes. Add a lapsed pull on a seat-0 repo and assert the output of those surfaces.