From 7d809d54562463d5f3d63ddd3fa12bdac274c3f8 Mon Sep 17 00:00:00 2001 From: Mike Schwarz <171890339+mvschwarz@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:10:35 -0700 Subject: [PATCH] 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. --- packages/daemon/src/domain/review/gather.ts | 23 +++++++++++--------- packages/daemon/test/review-routes.test.ts | 24 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/daemon/src/domain/review/gather.ts b/packages/daemon/src/domain/review/gather.ts index 0d6d020fa..91b3ad55b 100644 --- a/packages/daemon/src/domain/review/gather.ts +++ b/packages/daemon/src/domain/review/gather.ts @@ -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 }>(); 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((acc, r) => (!acc || r.ts_updated > acc.ts_updated ? r : acc), null); + const newest = (rows: QitemRow[]) => rows.reduce((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 }>(); const view = this.participantView(); diff --git a/packages/daemon/test/review-routes.test.ts b/packages/daemon/test/review-routes.test.ts index 64d5e5c09..1087d7f68 100644 --- a/packages/daemon/test/review-routes.test.ts +++ b/packages/daemon/test/review-routes.test.ts @@ -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" });