fix(task-board): open the review cycle when the PR link loses the race

apps/api/migrations/189-task-board-review-cycle-started-at.ts

@@ -0,0 +1,71 @@
+import { type Kysely, sql } from "kysely";
+
+/**
+ * Decouple the review cycle from the board lane.
+ *
+ * The "current review cycle" — the boundary that decides which reviewer
+ * verdicts still count — used to be derived from the newest
+ * `status_changed → in_review` activity, i.e. from the LANE the card sits in.
+ * That made the lane load-bearing: a card could not be anywhere but In Review
+ * while its reviewer ran, because moving it would have reset the cycle and
+ * invalidated every verdict recorded before the move.
+ *
+ * `review_cycle_started_at` is that boundary as its own column. With it, the
+ * lane is free to say what a human actually wants to read — In Progress while
+ * an agent reviewer is still working, In Review once it is a person's turn —
+ * and the reviewer machinery keys on the column instead.
+ *
+ * Backfilled from the timeline so in-flight cycles survive the deploy:
+ * `reviewCycleStart` still falls back to the activity scan for a card that has
+ * neither (see `packages/shared/src/task-board.ts`), so a missed row degrades
+ * to the old behaviour rather than to a lost cycle.
+ */
+export async function up(db: Kysely<unknown>): Promise<void> {
+  await db.schema
+    .alterTable("task_board_items")
+    .addColumn("review_cycle_started_at", "timestamptz")
+    .execute();
+
+  // Every card whose current lane means "a review cycle is open", anchored on
+  // the transition that opened it.
+  await sql`
+    UPDATE task_board_items i
+       SET review_cycle_started_at = (
+             SELECT max(a.occurred_at)
+               FROM task_board_activity a
+              WHERE a.task_board_item_id = i.id
+                AND a.action = 'status_changed'
+                AND a.data ->> 'to' = 'in_review'
+           )
+     WHERE i.status = 'in_review'
+  `.execute(db);
+
+  // Replaces idx_task_board_items_pending_review: the sweeper's work list is no
+  // longer "cards in the In Review lane" but "cards with an open review cycle",
+  // which spans In Progress and In Review.
+  await sql`
+    CREATE INDEX idx_task_board_items_review_cycle
+      ON task_board_items (updated_at, id)
+      WHERE (review_cycle_started_at IS NOT NULL OR status = 'in_review')
+        AND assignee_id = 'super-agent'
+        AND dismissed_at IS NULL
+  `.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_task_board_items_pending_review`.execute(
+    db,
+  );
+}
+
+export async function down(db: Kysely<unknown>): Promise<void> {
+  await sql`
+    CREATE INDEX idx_task_board_items_pending_review
+      ON task_board_items (updated_at, id)
+      WHERE status = 'in_review'
+        AND assignee_id = 'super-agent'
+        AND dismissed_at IS NULL
+  `.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_task_board_items_review_cycle`.execute(db);
+  await db.schema
+    .alterTable("task_board_items")
+    .dropColumn("review_cycle_started_at")
+    .execute();
+}

apps/api/migrations/index.ts

@@ -187,6 +187,7 @@ import * as migration185jirarescanexistingcards from "./185-jira-rescan-existing
 import * as migration186jirarescanpendingflag from "./186-jira-rescan-pending-flag.ts";
 import * as migration187taskboardcommentthread from "./187-task-board-comment-thread.ts";
 import * as migration188invitationautoaccept from "./188-invitation-auto-accept.ts";
+import * as migration189taskboardreviewcyclestartedat from "./189-task-board-review-cycle-started-at.ts";
 
 /**
  * Core migrations for the Studio application.
@@ -405,6 +406,8 @@ const migrations: Record<string, Migration> = {
   "186-jira-rescan-pending-flag": migration186jirarescanpendingflag,
   "187-task-board-comment-thread": migration187taskboardcommentthread,
   "188-invitation-auto-accept": migration188invitationautoaccept,
+  "189-task-board-review-cycle-started-at":
+    migration189taskboardreviewcyclestartedat,
 };
 
 export default migrations;

apps/api/src/api/routes/decopilot/cluster-mcp-tool-hooks.ts

@@ -21,7 +21,7 @@ import type {
 } from "@/harnesses/lib/decopilot/mcp-tools";
 import { resolveArgsStorageRefs } from "./file-materializer";
 import {
-  advanceTaskBoardForRun,
+  openReviewCycleForRun,
   capturePrForRun,
   isPrCreateMcpTool,
 } from "@/tools/task-board/run-reactions";
@@ -51,10 +51,13 @@ export function buildClusterMcpToolHooks(
   return {
     resolveArgs: (input) => resolveArgsStorageRefs(input, ctx),
     onToolCalled: (event) => {
-      // A Super Agent task run just opened a PR via the GitHub MCP tool —
-      // move its card to In Review. Fire-and-forget (no-ops off a task run).
+      // A Super Agent task run just opened a PR via the GitHub MCP tool — open
+      // its card's review cycle so a reviewer picks it up. The card STAYS In
+      // Progress: an agent is still working on it, and In Review is what the
+      // board says once it is a person's turn. Fire-and-forget (no-ops off a
+      // task run).
       if (!event.isError && isPrCreateMcpTool(event.toolName)) {
-        void advanceTaskBoardForRun(ctx, "in_review", threadId);
+        void openReviewCycleForRun(ctx, threadId);
       }
       const orgId = ctx.organization?.id;
       const userId = ctx.auth?.user?.id;

apps/api/src/harnesses/decopilot/run-agent-loop.ts

@@ -44,7 +44,7 @@ import { assembleAgentTools } from "./assemble-agent-tools";
 import type { BuiltinToolParams } from "./built-in-tools";
 import { buildClusterMcpToolHooks } from "@/api/routes/decopilot/cluster-mcp-tool-hooks";
 import {
-  advanceTaskBoardForRun,
+  openReviewCycleForRun,
   capturePrForRun,
   isPrCreateBashCommand,
 } from "@/tools/task-board/run-reactions";
@@ -225,9 +225,9 @@ export async function runAgentLoop(
     : createLanguageModel(opts.provider, opts.models.thinking);
 
   // Watch each step's bash calls for `gh pr create` (or a REST fallback) and
-  // move a linked task card to In Review. The scan itself is a cheap per-step
+  // open a linked task card's review cycle. The scan itself is a cheap per-step
   // array walk, so it's unconditional — a normal run never has a `bash` call
-  // matching the PR regexes, and `advanceTaskBoardForRun` hits the DB only when
+  // matching the PR regexes, and `openReviewCycleForRun` hits the DB only when
   // one does: a link SELECT to resolve the target, then a write only if that
   // run is actually task-linked (a non-task match reads nothing to update).
   // (The GitHub MCP tool path is caught separately via
@@ -243,11 +243,7 @@ export async function runAgentLoop(
         command &&
         isPrCreateBashCommand(command)
       ) {
-        void advanceTaskBoardForRun(
-          opts.ctx,
-          "in_review",
-          opts.currentThreadId,
-        );
+        void openReviewCycleForRun(opts.ctx, opts.currentThreadId);
         // Link the PR too — its URL is in the bash call's stdout (`gh pr create`
         // prints it; a `curl … /pulls` POST returns it in the response body).
         // Only parse the matched call's own output, never every bash stdout.

apps/api/src/harnesses/sandbox-dispatch-client.ts

@@ -548,21 +548,24 @@ export class SandboxDispatchClient {
       resume: { reason: string } | null,
     ): AsyncIterable<UIMessageChunk> =>
       (async function* () {
-        // The longest silence in the run: pod boot, clone, and (interactive)
-        // install. The chat has no per-thread stream here, so this rides
-        // the org `/watch`.
-        await publishRunStatusStage({
-          streamBuffer,
-          harnessId: SANDBOX_HOSTED_HARNESS,
-          taskId: runId,
-          stage: "starting-sandbox",
-        });
         const sandbox = await ensureSandbox(
           {
             virtualMcpId,
             branch,
             // Headless loop, no preview — unless someone is watching it.
             purpose: interactive ? "interactive" : "harness-run",
+            // The longest silence in the run: pod boot, clone, and
+            // (interactive) install. The chat has no per-thread stream here, so
+            // this rides the org `/watch`. Only on a cold start — an
+            // interactive agent keeps its pod between turns, and announcing a
+            // boot on every message made a warm resume look like a re-boot.
+            onColdStart: () =>
+              publishRunStatusStage({
+                streamBuffer,
+                harnessId: SANDBOX_HOSTED_HARNESS,
+                taskId: runId,
+                stage: "starting-sandbox",
+              }),
           },
           ctx,
         );

apps/api/src/storage/task-board-advance-review.integration.test.ts

@@ -28,6 +28,7 @@ import {
 } from "../database/test-db-pg";
 import { TaskBoardStorage } from "./task-board";
 import { SqlThreadStorage } from "./threads";
+import { SUPER_AGENT_ASSIGNEE_ID } from "@decocms/shared/task-board";
 
 const ORG = "org_advance_review";
 const USER = "user_advance_review";
@@ -173,8 +174,11 @@ describe("advanceToReviewIfInProgress (real Postgres)", () => {
     expect(results.filter((r) => r !== null)).toHaveLength(1);
   });
 
-  // A repo-backed task can't dead-end In Review with no PR — on finish it stays In Progress until a PR is linked, then the finish backstop advances it.
-  it("holds a repo-backed task on finish until a PR is linked", async () => {
+  // A repo-backed task can't dead-end with no PR — on finish it stays In
+  // Progress with no review cycle until a PR is linked, and then the finish
+  // backstop OPENS the cycle. It does not move the card: a reviewer is about to
+  // work on it, and In Review is what the board says once it is a person's turn.
+  it("opens the review cycle on finish only once a PR is linked", async () => {
     const task = await taskBoard.create({
       organizationId: ORG,
       title: "repo, no PR yet",
@@ -206,11 +210,13 @@ describe("advanceToReviewIfInProgress (real Postgres)", () => {
       })
       .execute();
 
-    // No PR → stays In Progress.
+    // No PR → stays In Progress with nothing to review.
     await taskBoard.advanceLinkedTasksToReviewOnThreadFinish(thread.id, ORG);
-    expect((await taskBoard.getById(task.id, ORG))?.status).toBe("in_progress");
+    const before = await taskBoard.getById(task.id, ORG);
+    expect(before?.status).toBe("in_progress");
+    expect(before?.reviewCycleStartedAt).toBeNull();
 
-    // Link a PR → the finish backstop now advances it.
+    // Link a PR → the finish backstop opens the cycle, and the lane holds.
     await taskBoard.linkPr({
       taskBoardItemId: task.id,
       organizationId: ORG,
@@ -220,8 +226,154 @@ describe("advanceToReviewIfInProgress (real Postgres)", () => {
       repoName: "site",
     });
     await taskBoard.advanceLinkedTasksToReviewOnThreadFinish(thread.id, ORG);
+    const after = await taskBoard.getById(task.id, ORG);
+    expect(after?.status).toBe("in_progress");
+    expect(after?.reviewCycleStartedAt).not.toBeNull();
+  });
+
+  /**
+   * The duplicate-stamp bug, re-encoded against the column that replaced the
+   * activity row. Re-stamping an OPEN cycle moves its boundary forward and
+   * invalidates every verdict already recorded against it — which is exactly
+   * how 13 prod cards ended up holding an approval that could never merge.
+   * `review_cycle_started_at IS NULL` in the WHERE is the whole guard, and only
+   * a real database proves a SQL predicate.
+   */
+  it("opens a review cycle exactly once, however many callers try", async () => {
+    const { task } = await cardWithFinishedRun("one cycle only");
+
+    const winners = await Promise.all(
+      Array.from({ length: 8 }, () =>
+        taskBoard.openReviewCycleIfInProgress(task.id, ORG),
+      ),
+    );
+
+    expect(winners.filter((w) => w !== null)).toHaveLength(1);
+  });
+
+  it("re-opens a cycle only after it is closed", async () => {
+    const { task } = await cardWithFinishedRun("second round");
+    const first = await taskBoard.openReviewCycleIfInProgress(task.id, ORG);
+    expect(first).not.toBeNull();
+    const firstAt = first?.reviewCycleStartedAt;
+
+    // Still open — the boundary must not move under a standing verdict.
+    expect(
+      await taskBoard.openReviewCycleIfInProgress(task.id, ORG),
+    ).toBeNull();
+    expect((await taskBoard.getById(task.id, ORG))?.reviewCycleStartedAt).toBe(
+      firstAt as string,
+    );
+
+    await taskBoard.closeReviewCycle(task.id, ORG);
+    const second = await taskBoard.openReviewCycleIfInProgress(task.id, ORG);
+    expect(second).not.toBeNull();
+    expect(second?.reviewCycleStartedAt).not.toBe(firstAt as string);
+  });
+
+  /**
+   * The PR link can LOSE THE RACE to the thread-finish backstop: the backstop
+   * reads `listPrs` to decide repo-backed vs repo-less, and a run that links
+   * its PR moments after its thread goes terminal is read as repo-less and
+   * parked In Review. Observed at 52 seconds between the two on a real board.
+   *
+   * The old rule matched only In Progress, so the late link was a no-op, the
+   * cycle never opened, and the card sat In Review for the whole reviewer run
+   * with its verdicts falling back to the legacy activity scan. Inverted.
+   */
+  it("rescues a card the backstop parked In Review before the PR landed", async () => {
+    const { task } = await cardWithFinishedRun("late pr link");
+    await taskBoard.update(
+      task.id,
+      ORG,
+      { assigneeId: SUPER_AGENT_ASSIGNEE_ID },
+      USER,
+    );
+    await taskBoard.advanceToReviewIfInProgress(task.id, ORG, USER);
+    expect((await taskBoard.getById(task.id, ORG))?.status).toBe("in_review");
+
+    const opened = await taskBoard.openReviewCycleIfInProgress(task.id, ORG);
+
+    expect(opened?.status).toBe("in_progress");
+    expect(opened?.reviewCycleStartedAt).not.toBeNull();
+  });
+
+  // Only while the cycle is null. Once one is open the card is mid-review, and
+  // `parkReviewedCardForHuman` put it In Review on a verdict — dragging it back
+  // would undo that and re-stamp a boundary verdicts already stand on.
+  it("leaves an In Review card with a cycle already open alone", async () => {
+    const { task } = await cardWithFinishedRun("mid review");
+    await taskBoard.openReviewCycleIfInProgress(task.id, ORG);
+    await taskBoard.update(task.id, ORG, { status: "in_review" }, USER);
+
+    expect(
+      await taskBoard.openReviewCycleIfInProgress(task.id, ORG),
+    ).toBeNull();
     expect((await taskBoard.getById(task.id, ORG))?.status).toBe("in_review");
   });
+
+  // A person took the card over (`handTaskToHuman` clears the assignee); the
+  // automation does not get to pull it back into the agents' lane.
+  it("never rescues a card a human owns", async () => {
+    const { task } = await cardWithFinishedRun("human owns it");
+    await taskBoard.update(
+      task.id,
+      ORG,
+      { assigneeId: SUPER_AGENT_ASSIGNEE_ID },
+      USER,
+    );
+    await taskBoard.advanceToReviewIfInProgress(task.id, ORG, USER);
+    await taskBoard.unassignSuperAgent(task.id, ORG, USER);
+
+    expect(
+      await taskBoard.openReviewCycleIfInProgress(task.id, ORG),
+    ).toBeNull();
+    expect((await taskBoard.getById(task.id, ORG))?.status).toBe("in_review");
+  });
+
+  it("never opens a cycle on a card past the review phase", async () => {
+    const { task } = await cardWithFinishedRun("wrong lane");
+    await taskBoard.update(task.id, ORG, { status: "done" }, USER);
+
+    expect(
+      await taskBoard.openReviewCycleIfInProgress(task.id, ORG),
+    ).toBeNull();
+  });
+
+  it("is org-scoped — another org cannot open the cycle", async () => {
+    const { task } = await cardWithFinishedRun("cross-org cycle");
+
+    expect(
+      await taskBoard.openReviewCycleIfInProgress(task.id, "org_other"),
+    ).toBeNull();
+    expect(
+      (await taskBoard.getById(task.id, ORG))?.reviewCycleStartedAt,
+    ).toBeNull();
+  });
+
+  // The sweeper's work list is the open cycle, not the lane — a card whose
+  // reviewer is working reads In Progress and still has to be swept.
+  it("lists an In Progress card with an open cycle as pending review", async () => {
+    const { task } = await cardWithFinishedRun("pending while in progress");
+    await taskBoard.openReviewCycleIfInProgress(task.id, ORG);
+
+    const pending = await taskBoard.listItemsPendingReview(100);
+
+    expect(pending.map((p) => p.id)).toContain(task.id);
+  });
+
+  it("drops a card out of the work list once it ships", async () => {
+    const { task } = await cardWithFinishedRun("shipped, stop sweeping");
+    await taskBoard.openReviewCycleIfInProgress(task.id, ORG);
+    await taskBoard.update(task.id, ORG, { status: "done" }, USER);
+
+    expect(
+      (await taskBoard.listItemsPendingReview(100)).map((p) => p.id),
+    ).not.toContain(task.id);
+    expect(
+      (await taskBoard.getById(task.id, ORG))?.reviewCycleStartedAt,
+    ).toBeNull();
+  });
 });
 
 /**