fix(review): count only open rows as held, and let in-progress work decide doing (#2136)

The review page's agents band and roster tell the truth about what a seat holds. Only open rows (pending, claimed, in progress or blocked) count as held, so a seat's handed-off and done rows no longer show as holdings. When the seat has in-progress work, that's what it's doing. A seat reads as parked only on a blocked row that names a blocker, and only when nothing is in progress, so a closed row's leftover `blocked_on` no longer marks it parked.

The roster's "recently holding" complement uses the same set, so a seat whose rows were all handed off still appears, holding nothing. The other readers of `ACTIVE_STATES` (needs-you and the slice phase) are unchanged.

Reported by @ziadeh.

2 files, +37 -10, of which 24 are test lines.
This commit is contained in:
Mike Schwarz authored and GitHub committed 2026-10-26 18:10:35 -07:00
1 parent 63fd8fd015
commit 7d809d5456
2 files changed
+37 -10

No files matched your search

+13 -10
View File
@@ -79,6 +79,8 @@ interface QitemRow {
}
const ACTIVE_STATES = ["pending", "in-progress", "claimed", "blocked", "handed-off"];
// What a seat still holds. A handed-off row is closed: the next seat holds that work (#2129).
const HELD_STATES = ["pending", "in-progress", "claimed", "blocked"];
const LEXICAL_PARTICIPANTS: ReviewParticipants = { isPerson: isHumanSeatSessionRef, isSharedChannel: () => false };
@@ -600,9 +602,9 @@ export class ReviewGatherer {
`SELECT qitem_id, ts_created, ts_updated, destination_session, state, priority, tier, tags, ${summaryCol},
blocked_on, closure_required_at
FROM queue_items
WHERE state IN (${ACTIVE_STATES.map(() => "?").join(",")})`,
WHERE state IN (${HELD_STATES.map(() => "?").join(",")})`,
)
.all(...ACTIVE_STATES) as QitemRow[];
.all(...HELD_STATES) as QitemRow[];
const bySession = new Map<string, { rows: QitemRow[]; slices: Set<string> }>();
const view = this.participantView();
for (const r of rows) {
@@ -629,10 +631,11 @@ export class ReviewGatherer {
return [...bySession.entries()]
.sort(([a], [b]) => a.localeCompare(b))
.map(([session, e]) => {
const latest = e.rows.reduce((acc, r) => (r.ts_updated > acc.ts_updated ? r : acc), e.rows[0]!);
const parked = e.rows
.filter((r) => !!r.blocked_on)
.reduce<QitemRow | null>((acc, r) => (!acc || r.ts_updated > acc.ts_updated ? r : acc), null);
const newest = (rows: QitemRow[]) => rows.reduce<QitemRow | null>((acc, r) => (!acc || r.ts_updated > acc.ts_updated ? r : acc), null);
const latest = newest(e.rows)!;
// Live in-progress work is what the seat is doing; it is parked only when nothing is in progress (#2129).
const working = newest(e.rows.filter((r) => r.state === "in-progress"));
const parked = working ? null : newest(e.rows.filter((r) => r.state === "blocked" && !!r.blocked_on));
return {
agentName: sessionMemberLabel(session), // OPR.0.4.6.MH1 FR-8: shared contract
sessionName: session,
@@ -640,7 +643,7 @@ export class ReviewGatherer {
parkedOn: parked?.blocked_on ?? null,
idle: null, // honest-unknown: queue rows alone cannot prove liveness
idleSinceIso: null,
doing: parked?.summary ?? latest.summary,
doing: (working ?? parked ?? latest).summary,
holdsCount: e.rows.length,
lastTransitionIso: (parked ?? latest).ts_updated,
slices: [...e.slices].sort(),
@@ -659,17 +662,17 @@ export class ReviewGatherer {
if (!this.tableExists("queue_items")) return holders;
const summaryCol = this.columnExists("queue_items", "summary") ? "summary" : "NULL AS summary";
// Recently-holding: destination of a slice-tagged qitem whose latest
// update landed today but is no longer in an active state.
// update landed today but is no longer held (handed off, done or canceled).
const rows = this.db
.prepare(
`SELECT qitem_id, ts_created, ts_updated, destination_session, state, priority, tier, tags, ${summaryCol},
blocked_on, closure_required_at
FROM queue_items
WHERE ts_updated >= ?
AND state NOT IN (${ACTIVE_STATES.map(() => "?").join(",")})
AND state NOT IN (${HELD_STATES.map(() => "?").join(",")})
AND tags LIKE '%"slice:%'`,
)
.all(todayStartIso, ...ACTIVE_STATES) as QitemRow[];
.all(todayStartIso, ...HELD_STATES) as QitemRow[];
const known = new Set(holders.map((h) => h.sessionName));
const recent = new Map<string, { latest: QitemRow; slices: Set<string> }>();
const view = this.participantView();
@@ -417,6 +417,30 @@ describe("GET /api/review/rig (OPR.0.4.4.22)", () => {
expect(body.settled[0]).toMatchObject({ fromSession: "driver@r", toSession: "qa@r" });
});
it("counts only open rows as held, and in-progress work decides doing over a newer park (#2129)", async () => {
// The issue's repro: R1 blocked on another row, R2 claimed and in progress, R3 handed off to the next seat.
insert({ id: "q-r1", dest: "worker@r", state: "blocked", tags: ["slice:20-x"], summary: "R1 waiting on R0" });
db.prepare("UPDATE queue_items SET blocked_on = 'q-r0' WHERE qitem_id = 'q-r1'").run();
insert({ id: "q-r2", dest: "worker@r", tags: ["slice:20-x"], summary: "R2 being built", tsUpdated: "2026-07-04T11:00:00.000Z" });
insert({ id: "q-r3", dest: "worker@r", state: "handed-off", tags: ["slice:20-x"], summary: "R3 now with the next seat" });
const body = await (await app.request("/api/review/rig")).json();
const row = body.agents.rows.find((r: { sessionName: string }) => r.sessionName === "worker@r");
expect(row).toMatchObject({ doing: "R2 being built", holdsCount: 2 });
expect(row.stateGlyph).not.toBe("parked");
});
it("a seat whose only slice row today was handed off holds nothing and isn't parked on that row (#2129)", async () => {
// A closed row can keep its old blocked_on; it is neither held work nor a park.
insert({ id: "q-gone", dest: "passer@r", state: "handed-off", tags: ["slice:20-x"], summary: "handed to qa" });
db.prepare("UPDATE queue_items SET blocked_on = 'external:old-gate' WHERE qitem_id = 'q-gone'").run();
const body = await (await app.request("/api/review/rig")).json();
const row = body.agents.rows.find((r: { sessionName: string }) => r.sessionName === "passer@r");
expect(row).toMatchObject({ holdsCount: 0, doing: "no tracked work item" });
expect(row.stateGlyph).not.toBe("parked");
});
it("keeps human-routed qitems in NEEDS YOU without turning the human seat into an AGENTS row", async () => {
insert({ id: "q-human", dest: "human-review@kernel", tags: ["slice:20-x"], summary: "approve the demo", tier: "human-gate", state: "pending" });