feat(task-board): run a column's rule when a card lands there, however it moved

apps/api/src/jira/sync.ts

@@ -25,20 +25,14 @@
  */
 
 import { orgFlagEnabled } from "@decocms/shared/organization/schema";
-import {
-  boardAutomationFor,
-  boardFor,
-  boardCan,
-  boardLanes,
-} from "@/tools/task-board/board-handler";
-import { SUPER_AGENT_ASSIGNEE_ID } from "@decocms/shared/task-board";
+import { boardFor } from "@/tools/task-board/board-handler";
 import type { StudioContext } from "@/core/studio-context";
 import type {
   OrgJiraIntegration,
   TaskBoardItem,
   TaskBoardItemPriority,
 } from "@/storage/types";
-import { reactToSuperAgentDelegation } from "@/tools/task-board/enqueue-super-agent";
+import { runColumnAutomation } from "@/tools/task-board/run-column-automation";
 import { emitTaskBoardUpdated } from "@/tools/task-board/run-reactions";
 import { laneIndex } from "@decocms/shared/jira-status-mapping";
 import {
@@ -327,63 +321,20 @@ function mapPriority(issue: JiraIssue): TaskBoardItemPriority {
   return (name && PRIORITY_MAP[name]) || "medium";
 }
 
-/** The Jira-driven agent trigger: an unassigned card that just landed in the
- *  To Do lane gets the Super Agent, running under the integration's creator
- *  (`enqueueAgentRunForTask` acts as the card's `assigned_by`). A quota
- *  rejection un-delegates — import-route precedent — instead of leaving a
- *  card assigned-but-never-running. */
-async function maybeAutoDelegate(
+/** The Jira-driven leg of the column rule: an issue whose status just changed
+ *  landed the card in a column, and that column may have a rule on it. The
+ *  rule itself is shared with the board's own drag — see
+ *  {@link runColumnAutomation}. Runs as the integration's creator, which is
+ *  what `enqueueAgentRunForTask` records as the card's `assigned_by`. */
+function maybeAutoDelegate(
   ctx: StudioContext,
   integration: OrgJiraIntegration,
   item: TaskBoardItem,
 ): Promise<TaskBoardItem> {
-  if (item.assigneeId) return item;
-  const orgId = integration.organizationId;
-  // The board decides: a column with no rule on it is uneventful. This is also
-  // what replaced `integration.autoDelegate`, which could only ever mean the
-  // Super Agent, on To Do, for an org that had Jira.
-  const automation = await boardAutomationFor(ctx, orgId, item.status);
-  if (!automation) return item;
-  // Conditional claim, not a plain update: the cron, a webhook wake-up (its
-  // debounce is per-pod) and a manual JIRA_SYNC_RUN can all be mid-sync on the
-  // same issue, and a read-then-write would dispatch two paid agent runs on it.
-  const queue = (await boardLanes(ctx, orgId)).queue;
-  if (
-    !boardCan(orgId, "todo", queue, "auto-delegating Jira issues to the agent")
-  ) {
-    return item;
-  }
-  const delegated = await ctx.storage.taskBoard.claimUnassignedForSuperAgent(
-    item.id,
-    orgId,
-    integration.createdBy,
-    JIRA_SYNC_ACTOR,
-    queue,
-  );
-  if (!delegated) return item;
-  await ctx.storage.taskBoard.recordActivity({
-    taskBoardItemId: item.id,
-    action: "assignee_changed",
-    actorId: null,
-    data: { from: null, to: SUPER_AGENT_ASSIGNEE_ID },
+  return runColumnAutomation(ctx, item, {
+    assignedBy: integration.createdBy,
+    actor: JIRA_SYNC_ACTOR,
   });
-  try {
-    await reactToSuperAgentDelegation(ctx, delegated, {
-      instruction: automation.prompt ?? undefined,
-    });
-  } catch (err) {
-    console.warn(
-      `[jira] auto-delegate of ${item.id} rejected, un-delegating:`,
-      err instanceof Error ? err.message : err,
-    );
-    return await ctx.storage.taskBoard.update(
-      item.id,
-      orgId,
-      { assigneeId: null, assignedBy: null },
-      JIRA_SYNC_ACTOR,
-    );
-  }
-  return delegated;
 }
 
 /** Import a changed issue's comments not yet on the card. The link table is

apps/api/src/storage/task-board-column-claim.integration.test.ts

@@ -0,0 +1,146 @@
+/**
+ * Real-Postgres coverage for the fence a column rule claims through.
+ *
+ * The rule lives on a column, so the claim has to be conditional on the card
+ * still sitting in THAT column. It used to be conditional on the board's queue
+ * lane instead, while the rule itself was looked up by the card's status — so
+ * a rule on any other column found itself, tried to claim somewhere else,
+ * matched nothing, and did nothing. Silently, because a lost claim and an
+ * impossible one both come back null.
+ */
+
+import { afterAll, beforeAll, describe, expect, it } from "bun:test";
+import { sql } from "kysely";
+import { SUPER_AGENT_ASSIGNEE_ID } from "@decocms/shared/task-board";
+import type { StudioDatabase } from "../database";
+import {
+  closeTestPgDatabase,
+  connectTestPgDatabase,
+  resetTestPgDatabase,
+} from "../database/test-db-pg";
+import { TaskBoardStorage } from "./task-board";
+
+const ORG = "org_column_claim";
+const USER = "user_column_claim";
+
+describe("claimUnassignedForSuperAgent (real Postgres)", () => {
+  let database: StudioDatabase;
+  let taskBoard: TaskBoardStorage;
+
+  beforeAll(async () => {
+    database = await connectTestPgDatabase();
+    await resetTestPgDatabase(database);
+    await database.db
+      .insertInto("organization")
+      .values({
+        id: ORG,
+        name: ORG,
+        slug: "org-column-claim",
+        createdAt: new Date().toISOString(),
+      })
+      .execute();
+    const now = new Date().toISOString();
+    await sql`
+      INSERT INTO "user" (id, email, "emailVerified", name, "createdAt", "updatedAt")
+      VALUES (${USER}, ${"column-claim@test"}, false, ${USER}, ${now}, ${now})
+    `.execute(database.db);
+    taskBoard = new TaskBoardStorage(database.db);
+  });
+
+  afterAll(async () => {
+    await closeTestPgDatabase(database);
+  });
+
+  const card = (status: string) =>
+    taskBoard.create({
+      organizationId: ORG,
+      title: `card in ${status}`,
+      status,
+      by: USER,
+    });
+
+  /**
+   * The bug. A tracker's column is called whatever it is called, and a team
+   * putting its rule on "QA" is the normal case — not everything starts from
+   * a queue lane.
+   */
+  it("claims a card in the column the rule is on, whatever it is called", async () => {
+    const task = await card("QA Deco");
+    const claimed = await taskBoard.claimUnassignedForSuperAgent(
+      task.id,
+      ORG,
+      USER,
+      USER,
+      "QA Deco",
+    );
+    expect(claimed?.assigneeId).toBe(SUPER_AGENT_ASSIGNEE_ID);
+    expect(claimed?.assignedBy).toBe(USER);
+  });
+
+  /** The card moved on between the read and the claim, so the rule that fired
+   *  is no longer the rule for where it is. */
+  it("refuses a card that has left that column", async () => {
+    const task = await card("Fazendo");
+    expect(
+      await taskBoard.claimUnassignedForSuperAgent(
+        task.id,
+        ORG,
+        USER,
+        USER,
+        "QA Deco",
+      ),
+    ).toBeNull();
+  });
+
+  /** What makes it a fence: exactly one of two concurrent triggers wins, so a
+   *  card cannot buy two agent runs. */
+  it("lets one of two concurrent triggers win, never both", async () => {
+    const task = await card("Fazendo");
+    const [a, b] = await Promise.all([
+      taskBoard.claimUnassignedForSuperAgent(
+        task.id,
+        ORG,
+        USER,
+        USER,
+        "Fazendo",
+      ),
+      taskBoard.claimUnassignedForSuperAgent(
+        task.id,
+        ORG,
+        USER,
+        USER,
+        "Fazendo",
+      ),
+    ]);
+    expect([a, b].filter(Boolean)).toHaveLength(1);
+  });
+
+  /** A card someone already owns is theirs; a rule never takes it. */
+  it("refuses a card that already has an assignee", async () => {
+    const task = await card("Fazendo");
+    await taskBoard.update(task.id, ORG, { assigneeId: USER }, USER);
+    expect(
+      await taskBoard.claimUnassignedForSuperAgent(
+        task.id,
+        ORG,
+        USER,
+        USER,
+        "Fazendo",
+      ),
+    ).toBeNull();
+  });
+
+  /** Null is "this board has no such column", which cannot be claimed in. */
+  it("refuses when the board has no column for the rule", async () => {
+    const task = await card("Fazendo");
+    expect(
+      await taskBoard.claimUnassignedForSuperAgent(
+        task.id,
+        ORG,
+        USER,
+        USER,
+        null,
+      ),
+    ).toBeNull();
+  });
+});

apps/api/src/storage/task-board.ts

@@ -1695,12 +1695,13 @@ export class TaskBoardStorage {
     organizationId: string,
     assignedBy: string,
     by: string,
-    /** This board's queue column — the claim's starting line. Null means the
-     *  board has no column that means "queued", so there is nothing to claim
-     *  and the caller reads it the same as losing the race. */
-    queueLane: string | null,
+    /** The column the card must still be sitting in for the claim to win —
+     *  the one whose rule is firing, not a fixed lane. An automation on any
+     *  other column used to look itself up by the card's status and then fence
+     *  on the queue lane, so it never claimed anything. */
+    fromLane: string | null,
   ): Promise<TaskBoardItem | null> {
-    if (queueLane === null) return null;
+    if (fromLane === null) return null;
     const row = await this.db
       .updateTable("task_board_items")
       .set({
@@ -1711,7 +1712,7 @@ export class TaskBoardStorage {
       })
       .where("id", "=", id)
       .where("organization_id", "=", organizationId)
-      .where("status", "=", queueLane)
+      .where("status", "=", fromLane)
       .where("assignee_id", "is", null)
       .returningAll()
       .executeTakeFirst();

apps/api/src/tools/task-board/board-handler.test.ts

@@ -5,7 +5,7 @@
 
 import { afterEach, beforeEach, describe, expect, it } from "bun:test";
 import type { BoardHandler } from "./board-handler";
-import { boardCan, shippedPatch } from "./board-handler";
+import { boardCan, canAdvance, shippedPatch } from "./board-handler";
 
 const boardWithOwner = (columnOwner: string | null): BoardHandler =>
   ({ columnOwner: () => columnOwner }) as BoardHandler;
@@ -72,3 +72,55 @@ describe("boardCan", () => {
     expect(warns).toHaveLength(3);
   });
 });
+
+describe("canAdvance", () => {
+  const board = (...keys: string[]) =>
+    keys.map((key, position) => ({
+      key,
+      title: key,
+      position,
+      role: null,
+      trackerStatuses: [],
+    }));
+
+  /**
+   * The set this replaced was `{triage, todo, in_progress}` — every lane at or
+   * before in_progress. Position reproduces it exactly on Studio's board,
+   * which is what makes swapping the two a refactor for anyone not mirroring.
+   */
+  it("matches the lanes the hardcoded set held, on Studio's board", () => {
+    const studio = board(
+      "triage",
+      "todo",
+      "in_progress",
+      "in_review",
+      "approved",
+      "merged",
+      "post_deploy_validation",
+      "done",
+      "archived",
+    );
+    const advanceable = studio
+      .map((c) => c.key)
+      .filter((key) => canAdvance(studio, key, "in_progress"));
+    expect(advanceable).toEqual(["triage", "todo", "in_progress"]);
+  });
+
+  /** The point of the change: a tracker's own order answers the same question
+   *  for columns Studio never named. */
+  it("answers by the tracker's order on a mirrored board", () => {
+    const jira = board("Backlog", "Fazendo", "Code Review", "Deploy");
+    expect(canAdvance(jira, "Backlog", "Fazendo")).toBe(true);
+    expect(canAdvance(jira, "Fazendo", "Fazendo")).toBe(true);
+    expect(canAdvance(jira, "Code Review", "Fazendo")).toBe(false);
+    expect(canAdvance(jira, "Deploy", "Fazendo")).toBe(false);
+  });
+
+  /** A card in a column the board does not have is not one to move — the same
+   *  answer every other lane decision gives for an unplaceable card. */
+  it("refuses a column this board does not have, either end", () => {
+    const jira = board("Backlog", "Fazendo");
+    expect(canAdvance(jira, "triage", "Fazendo")).toBe(false);
+    expect(canAdvance(jira, "Backlog", "in_progress")).toBe(false);
+  });
+});

apps/api/src/tools/task-board/board-handler.ts

@@ -261,6 +261,29 @@ export async function boardFor(
   });
 }
 
+/**
+ * Whether a card sitting in `from` may still be advanced to `to`.
+ *
+ * True when `from` is at or before `to` in the board's OWN order, which is
+ * what stops a re-opened PR dragging a finished card backwards.
+ *
+ * Position rather than a fixed list of lane names, because a mirrored board's
+ * columns are ordered and named by its tracker — "has it got past this yet" is
+ * a question only the board can answer. On Studio's board the answer is the
+ * same set the hardcoded list held. A column this board does not have is not
+ * advanceable at all: a card nobody can place is not one to move.
+ */
+export function canAdvance(
+  columns: readonly BoardColumn[],
+  from: string,
+  to: string,
+): boolean {
+  const at = (key: string) => columns.find((c) => c.key === key)?.position;
+  const fromAt = at(from);
+  const toAt = at(to);
+  return fromAt !== undefined && toAt !== undefined && fromAt <= toAt;
+}
+
 /** One warning per org and meaning. A board nobody configured would otherwise
  *  log on every sweep tick, which is the fastest way to make the signal
  *  worthless. Capped rather than TTL'd: the key set is bounded by orgs times

apps/api/src/tools/task-board/pr-open-board-reaction.integration.test.ts

@@ -20,6 +20,23 @@ import {
 } from "./pr-open-board-reaction";
 
 /** Studio's own board, which is what these fixtures run on. */
+const CANON_COLUMNS = [
+  "triage",
+  "todo",
+  "in_progress",
+  "in_review",
+  "approved",
+  "merged",
+  "post_deploy_validation",
+  "done",
+  "archived",
+].map((key, position) => ({
+  key,
+  title: key,
+  position,
+  role: key,
+  trackerStatuses: [],
+}));
 const CANON_LANES = {
   intake: "triage",
   queue: "todo",
@@ -60,6 +77,7 @@ describe("applyBoardDecision", () => {
       threadId: thread,
       pr: PR,
       lanes: CANON_LANES,
+      columns: CANON_COLUMNS,
       decision,
       openCards,
     });