From edff0d20d7ec55781d725acd673d25b367e66975 Mon Sep 17 00:00:00 2001 From: Fernando Frizzatti Date: Mon, 24 Aug 2026 11:53:55 -0300 Subject: [PATCH] feat(task-board): delivery lanes behind a default-off org flag Adds Approved, Merged and Post-deploy Validation between In Review and Done, gated by a default-off org flag `delivery_lanes_enabled`. Every automatic ship path reads one function, `shippedLane(flags)`, so "flag off = zero behaviour change" is a single tested fact rather than five call sites agreeing by luck. Rebased onto main after 290 commits, with four fixes folded in (see the PR description for what changed and why they were not left as follow-ups): The e2e asserted `expect(shipError).not.toMatch(...)` on a value that is `null` when the ship SUCCEEDS, so it failed precisely in the case it was written to confirm. The merged-tag sweep would have gone silent. It gated on `status = 'done'`, and with the lanes on a merged pull request lands the card on a delivery lane, so nothing would be tagged until a human finished dragging it. The archive sweep deliberately still gates on Done: a card in a delivery lane is in flight, while the tag is a statement about the pull request. `deployed` is renamed to `merged`, because that is the signal Studio has. Nothing reads a deployment; what moves a card here is GitHub reporting the pull request merged. Free today, a data migration the moment anyone enables the flag. The rebase kept both sides where #6544 had landed in between: `cardWorkLanded` decides WHETHER a card landed, `shippedLane` decides WHICH lane, and two tests written against the older reader shape were ported rather than dropped. Co-authored-by: Viktor Marinho --- apps/api/src/storage/task-board.ts | 22 ++- apps/api/src/storage/types.ts | 3 + .../archive-merged.integration.test.ts | 26 +++ .../human-rejected-done.integration.test.ts | 24 +++ apps/api/src/tools/task-board/lanes.test.ts | 109 +++++++++++++ apps/api/src/tools/task-board/lanes.ts | 67 ++++++++ apps/api/src/tools/task-board/merge-pr.ts | 6 +- .../task-board/promote-to-production.test.ts | 18 +++ .../tools/task-board/promote-to-production.ts | 22 +-- apps/api/src/tools/task-board/prs-get.ts | 23 ++- .../tools/task-board/reconcile-merged.test.ts | 21 ++- .../src/tools/task-board/reconcile-merged.ts | 19 ++- .../src/tools/task-board/review-decision.ts | 8 +- .../api/src/tools/task-board/run-reactions.ts | 20 +-- apps/api/src/tools/task-board/schema.ts | 3 + .../task-board/tag-merged.integration.test.ts | 26 +++ apps/api/src/tools/task-board/tag-merged.ts | 3 +- apps/api/src/tools/task-board/update.test.ts | 11 +- apps/api/src/tools/task-board/update.ts | 7 +- .../components/settings/review-settings.tsx | 7 + apps/web/src/i18n/en/settings.ts | 3 + apps/web/src/i18n/en/task-board.ts | 3 + apps/web/src/i18n/pt-br/settings.ts | 3 + apps/web/src/i18n/pt-br/task-board.ts | 3 + .../web/src/layouts/task-board/config.test.ts | 91 +++++++++++ apps/web/src/layouts/task-board/config.tsx | 72 ++++++++- apps/web/src/layouts/task-board/index.tsx | 25 ++- .../layouts/task-board/review-status.test.ts | 23 +++ .../src/layouts/task-board/review-status.ts | 9 ++ .../src/layouts/task-board/task-dialog.tsx | 17 +- apps/web/src/views/settings/jira.tsx | 21 ++- .../tests/task-board-delivery-lanes.spec.ts | 150 ++++++++++++++++++ packages/shared/src/organization/schema.ts | 6 + packages/shared/src/task-board.ts | 34 ++++ packages/shared/src/tools/tool-io.ts | 33 ++++ 35 files changed, 868 insertions(+), 70 deletions(-) create mode 100644 apps/api/src/tools/task-board/lanes.test.ts create mode 100644 apps/api/src/tools/task-board/lanes.ts create mode 100644 packages/e2e/tests/task-board-delivery-lanes.spec.ts diff --git a/apps/api/src/storage/task-board.ts b/apps/api/src/storage/task-board.ts index 9db4d54778..38ed1b6be0 100644 --- a/apps/api/src/storage/task-board.ts +++ b/apps/api/src/storage/task-board.ts @@ -21,7 +21,7 @@ import type { TaskBoardItemThreadRef, } from "./types"; import { generatePrefixedId } from "@decocms/shared/utils/generate-id"; -import { DEFAULT_TASK_TYPE } from "@decocms/shared/task-board"; +import { DEFAULT_TASK_TYPE, DELIVERY_LANES } from "@decocms/shared/task-board"; import { type ReviewCycleActivity, REVIEWER_KINDS, @@ -212,6 +212,14 @@ function newestIso( return new Date(Math.max(a, b)).toISOString(); } +/** Lanes a merged PR can leave a card on, and therefore where the merged-tag + * sweep has to look. `archived` is deliberately absent: a card that far along + * is history, and tagging it moves nothing. */ +const TAGGABLE_MERGED_STATUSES: TaskBoardItemStatus[] = [ + ...DELIVERY_LANES, + "done", +]; + export class TaskBoardStorage { constructor(private db: Kysely) {} @@ -752,7 +760,11 @@ export class TaskBoardStorage { const rows = await this.db .selectFrom("task_board_items as i") .select(["i.id", "i.organization_id as organizationId"]) - .where("i.status", "=", "done") + // Not just Done: with the delivery lanes on a merge lands the card on + // `merged`, and gating on Done alone would mean no card is ever tagged + // until a human drags it the rest of the way — days after the tag was + // worth seeing, which is the whole reason this sweep is not the archive's. + .where("i.status", "in", TAGGABLE_MERGED_STATUSES) .where("i.dismissed_at", "is", null) .where((eb) => eb.exists( @@ -1980,7 +1992,9 @@ export class TaskBoardStorage { } /** A null actor is a machine path; a `reason` marks a move that meant something - * other than "not done" (Rerun-from-Done stamps `reason: "rerun"`). */ + * other than "not done" (Rerun-from-Done stamps `reason: "rerun"`). + * `merged` counts as leaving Done — with the delivery lanes on, that is + * where a merged PR lands. */ async hasHumanRejectedDone( taskBoardItemId: string, organizationId: string, @@ -1997,7 +2011,7 @@ export class TaskBoardStorage { .where("a.task_board_item_id", "=", taskBoardItemId) .where("a.action", "=", "status_changed") .where(byMember, "is not", null) - .where(laneLeft, "=", "done") + .where(laneLeft, "in", ["done", "merged"]) .where(moveReason, "is", null) .limit(1) .executeTakeFirst(); diff --git a/apps/api/src/storage/types.ts b/apps/api/src/storage/types.ts index 68e385e5f5..9f6801115b 100644 --- a/apps/api/src/storage/types.ts +++ b/apps/api/src/storage/types.ts @@ -1600,6 +1600,9 @@ export type TaskBoardItemStatus = | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "done" | "archived"; diff --git a/apps/api/src/tools/task-board/archive-merged.integration.test.ts b/apps/api/src/tools/task-board/archive-merged.integration.test.ts index f4207b10ec..d0bed78be0 100644 --- a/apps/api/src/tools/task-board/archive-merged.integration.test.ts +++ b/apps/api/src/tools/task-board/archive-merged.integration.test.ts @@ -110,6 +110,32 @@ describe("auto-archive sweep", () => { ]); }); + // The delivery lanes sit BEFORE Done, so a card parked in one is still in flight. + it("never sweeps a card resting in a delivery lane", async () => { + const settled = new Date(Date.now() - 3 * DAY_MS); + const lanes = ["approved", "merged", "post_deploy_validation"] as const; + const parked = await Promise.all( + lanes.map((lane) => seed(lane, settled, true)), + ); + + const candidates = await taskBoard.listItemsAwaitingArchive( + new Date(Date.now() - DAY_MS), + 200, + ); + const ids = candidates.map((c) => c.id); + for (const id of parked) expect(ids).not.toContain(id); + + // And the write path refuses too, even when handed the id directly. + const swept = await archiveMergedForOrg(ctx, ORG, parked, async () => ({ + state: "closed" as const, + merged: true, + })); + expect(swept.archived).toBe(0); + for (const [i, id] of parked.entries()) { + expect((await taskBoard.getById(id, ORG))?.status).toBe(lanes[i]); + } + }); + it("archives a merged card, logs it, and won't archive it twice", async () => { const id = await seed("done", new Date(Date.now() - 3 * DAY_MS), true); diff --git a/apps/api/src/tools/task-board/human-rejected-done.integration.test.ts b/apps/api/src/tools/task-board/human-rejected-done.integration.test.ts index 6816b7ccbb..0f418b9bdc 100644 --- a/apps/api/src/tools/task-board/human-rejected-done.integration.test.ts +++ b/apps/api/src/tools/task-board/human-rejected-done.integration.test.ts @@ -73,6 +73,30 @@ describe("hasHumanRejectedDone", () => { expect(await taskBoard.hasHumanRejectedDone(id, ORG)).toBe(true); }); + // Merged is where a merged PR lands, so leaving it is the same veto as Done. + it("is true once a member moves the card out of Merged", async () => { + const id = await seed(); + await taskBoard.recordActivity({ + taskBoardItemId: id, + action: "status_changed", + actorId: USER, + data: { from: "merged", to: "in_review" }, + }); + expect(await taskBoard.hasHumanRejectedDone(id, ORG)).toBe(true); + }); + + // Past Merged, moving forward agrees with the ship rather than rejecting it. + it("is false for a member's move out of a later delivery lane", async () => { + const id = await seed(); + await taskBoard.recordActivity({ + taskBoardItemId: id, + action: "status_changed", + actorId: USER, + data: { from: "post_deploy_validation", to: "done" }, + }); + expect(await taskBoard.hasHumanRejectedDone(id, ORG)).toBe(false); + }); + it("ignores a machine move out of Done", async () => { const id = await seed(); await taskBoard.recordActivity({ diff --git a/apps/api/src/tools/task-board/lanes.test.ts b/apps/api/src/tools/task-board/lanes.test.ts new file mode 100644 index 0000000000..b6fc60c46d --- /dev/null +++ b/apps/api/src/tools/task-board/lanes.test.ts @@ -0,0 +1,109 @@ +/** + * The lane order and the two questions that ride on it. + * + * These are the assertions that make "flag off means nothing changed" and + * "automation never drags a card backward" checkable facts rather than a + * property five call sites happen to agree on. + */ + +import { describe, expect, it } from "bun:test"; +import { DELIVERY_LANES, shippedLane } from "@decocms/shared/task-board"; +import type { TaskBoardItemStatus } from "@/storage/types"; +import { + DELIVERY_LANE_STATUSES, + LANE_RANK, + movesForward, + SHIP_ELIGIBLE_LANES, +} from "./lanes"; + +const BOARD_ORDER: TaskBoardItemStatus[] = [ + "triage", + "todo", + "in_progress", + "in_review", + "approved", + "merged", + "post_deploy_validation", + "done", + "archived", +]; + +describe("LANE_RANK", () => { + it("is strictly increasing in board order", () => { + const ranks = BOARD_ORDER.map((s) => LANE_RANK[s]); + expect(ranks).toEqual([...ranks].sort((a, b) => a - b)); + expect(new Set(ranks).size).toBe(ranks.length); + }); + + it("puts every delivery lane between In Review and Done", () => { + for (const lane of DELIVERY_LANE_STATUSES) { + expect(LANE_RANK[lane]).toBeGreaterThan(LANE_RANK.in_review); + expect(LANE_RANK[lane]).toBeLessThan(LANE_RANK.done); + } + }); + + it("covers every lane the board knows", () => { + expect(Object.keys(LANE_RANK).sort()).toEqual([...BOARD_ORDER].sort()); + }); + + // The shared union must stay assignable to this side's lane type. + it("agrees with the shared delivery-lane names", () => { + expect(DELIVERY_LANE_STATUSES).toEqual(DELIVERY_LANES); + }); +}); + +describe("movesForward", () => { + it("advances along the board", () => { + expect(movesForward("in_review", "merged")).toBe(true); + expect(movesForward("in_review", "done")).toBe(true); + expect(movesForward("merged", "post_deploy_validation")).toBe(true); + }); + + // `prs-get`'s old enumerated guard dragged a card resting past Merged back. + it("refuses to drag a card back to an earlier lane", () => { + expect(movesForward("post_deploy_validation", "merged")).toBe(false); + expect(movesForward("done", "merged")).toBe(false); + expect(movesForward("archived", "merged")).toBe(false); + expect(movesForward("in_review", "in_progress")).toBe(false); + }); + + it("is not satisfied by staying put", () => { + for (const lane of BOARD_ORDER) { + expect(movesForward(lane, lane)).toBe(false); + } + }); +}); + +describe("shippedLane", () => { + // Every falsy shape of the flags bag must resolve to `done` — that IS the guarantee. + it("ships to Done by default", () => { + expect(shippedLane(undefined)).toBe("done"); + expect(shippedLane(null)).toBe("done"); + expect(shippedLane({})).toBe("done"); + expect(shippedLane({ delivery_lanes_enabled: false })).toBe("done"); + expect(shippedLane({ auto_merge: true })).toBe("done"); + }); + + it("ships to Merged once the org runs the delivery lanes", () => { + expect(shippedLane({ delivery_lanes_enabled: true })).toBe("merged"); + }); + + it("always names a lane a merged PR may actually move to", () => { + for (const flags of [{}, { delivery_lanes_enabled: true }]) { + expect(movesForward("in_review", shippedLane(flags))).toBe(true); + } + }); +}); + +describe("SHIP_ELIGIBLE_LANES", () => { + it("is exactly In Review and Approved", () => { + expect([...SHIP_ELIGIBLE_LANES].sort()).toEqual(["approved", "in_review"]); + }); + + // Reachable from every reviewed-but-unshipped lane, and from none that shipped. + it("excludes the lanes a shipped card rests in", () => { + for (const lane of ["merged", "post_deploy_validation", "done"] as const) { + expect(SHIP_ELIGIBLE_LANES.has(lane)).toBe(false); + } + }); +}); diff --git a/apps/api/src/tools/task-board/lanes.ts b/apps/api/src/tools/task-board/lanes.ts new file mode 100644 index 0000000000..5b05582ce2 --- /dev/null +++ b/apps/api/src/tools/task-board/lanes.ts @@ -0,0 +1,67 @@ +/** + * Board lane order, and the questions that depend on it. + * + * `LANE_RANK` is a total order over the lanes, used as a forward-only guard: + * automation moves a card ALONG the board, never back. It is an exhaustive + * `Record` over `TaskBoardItemStatus` on purpose — adding a lane is then a + * compile error here rather than a silent rank collision. + * + * Lives in its own module (rather than in `run-reactions`, where the table used + * to be) because the guard is read from `prs-get` and `promote-to-production` + * too, and those importing `run-reactions` for a constant would be a cycle. + */ + +import { DELIVERY_LANES } from "@decocms/shared/task-board"; +import type { TaskBoardItemStatus } from "@/storage/types"; + +export const LANE_RANK: Record = { + triage: 0, + todo: 1, + in_progress: 2, + in_review: 3, + approved: 4, + merged: 5, + post_deploy_validation: 6, + done: 7, + archived: 8, +}; + +/** The delivery lanes, as board statuses — the assertion that the shared + * literal union stays a subset of this side's lane vocabulary. */ +export const DELIVERY_LANE_STATUSES: TaskBoardItemStatus[] = DELIVERY_LANES; + +/** + * True when moving `from` → `to` advances the card. + * + * Every automatic move must go through this rather than enumerating the lanes + * it refuses to leave (`status !== "done" && status !== "archived"`): an + * enumeration is correct only for the lanes that existed when it was written, + * so adding one silently turns it into a path that drags cards BACKWARD. + */ +export function movesForward( + from: TaskBoardItemStatus, + to: TaskBoardItemStatus, +): boolean { + return LANE_RANK[to] > LANE_RANK[from]; +} + +/** + * Lanes the "Ship to production" button may act from: In Review (auto-merge + * off, reviewers approved) and Approved (a human parked it there deliberately). + * Without Approved, moving a card into it would lock the ship button out — the + * lane would be a dead end. + */ +export const SHIP_ELIGIBLE_LANES: ReadonlySet = new Set([ + "in_review", + "approved", +]); + +/** + * True where a merged pull request can have left the card, which is what the + * merged-tag sweep gates on. Mirrors `TAGGABLE_MERGED_STATUSES` in storage: the + * candidate query and the re-read inside the org's context have to agree, or + * the sweep picks cards it then refuses. + */ +export function isTaggableMergedStatus(status: TaskBoardItemStatus): boolean { + return status === "done" || DELIVERY_LANE_STATUSES.includes(status); +} diff --git a/apps/api/src/tools/task-board/merge-pr.ts b/apps/api/src/tools/task-board/merge-pr.ts index 709892907b..7215127909 100644 --- a/apps/api/src/tools/task-board/merge-pr.ts +++ b/apps/api/src/tools/task-board/merge-pr.ts @@ -5,6 +5,7 @@ import { allReviewersApproved, approvedButUnverified, enabledReviewerKinds, + shippedLane, } from "@decocms/shared/task-board"; import { recordTaskActivity } from "./activity"; import { reactToApprovedPrConflict } from "./conflict-reaction"; @@ -448,17 +449,18 @@ export async function retryAutoMergeIfApproved( return false; } + const shipped = shippedLane(settings?.flags); const done = await ctx.storage.taskBoard.update( item.id, orgId, - { status: "done" }, + { status: shipped }, item.updatedBy, ); await recordTaskActivity(ctx, { taskBoardItemId: item.id, action: "status_changed", actorId: null, - data: { from: item.status, to: "done" }, + data: { from: item.status, to: shipped }, }); emitTaskBoardUpdated(orgId, done); return true; diff --git a/apps/api/src/tools/task-board/promote-to-production.test.ts b/apps/api/src/tools/task-board/promote-to-production.test.ts index 63841df2a3..582d443c6f 100644 --- a/apps/api/src/tools/task-board/promote-to-production.test.ts +++ b/apps/api/src/tools/task-board/promote-to-production.test.ts @@ -42,4 +42,22 @@ describe("isReadyToShip", () => { ), ).toBe(true); }); + + // Refusing to ship from Approved would make the lane a dead end. + it("allows shipping from Approved", () => { + expect(isReadyToShip("approved", [], [])).toBe(true); + expect( + isReadyToShip( + "approved", + [approved("qa"), approved("code_review")], + ["qa", "code_review"], + ), + ).toBe(true); + }); + + it("rejects an already-shipped task", () => { + expect(isReadyToShip("merged", [], [])).toBe(false); + expect(isReadyToShip("post_deploy_validation", [], [])).toBe(false); + expect(isReadyToShip("done", [], [])).toBe(false); + }); }); diff --git a/apps/api/src/tools/task-board/promote-to-production.ts b/apps/api/src/tools/task-board/promote-to-production.ts index 72ffea21cd..4fab38dcd6 100644 --- a/apps/api/src/tools/task-board/promote-to-production.ts +++ b/apps/api/src/tools/task-board/promote-to-production.ts @@ -7,15 +7,17 @@ import { enabledReviewerKinds, type ReviewCycleActivity, type ReviewerKind, + shippedLane, } from "@decocms/shared/task-board"; import { TaskBoardItemStatusSchema } from "./schema"; +import { SHIP_ELIGIBLE_LANES } from "./lanes"; import { recordTaskActivity } from "./activity"; import { emitTaskBoardUpdated } from "./run-reactions"; import { mergeLinkedPr } from "./merge-pr"; /** - * The board only shows "Ship to production" once the task is In Review and - * every enabled reviewer approved (`reviewsSatisfiedForPromotion` on the web + * The board only shows "Ship to production" once the task is In Review or + * Approved, with every enabled reviewer approved (`reviewsSatisfiedForPromotion` on the web * side) — but that's client-side gating only. Without this check here, any * caller of this tool (a decopilot agent, a stale client, a direct MCP call) * could merge ANY task's linked PR — todo, in_progress, unreviewed — bypassing @@ -26,7 +28,7 @@ export function isReadyToShip( activity: ReviewCycleActivity[], enabled: ReviewerKind[], ): boolean { - if (status !== "in_review") return false; + if (!SHIP_ELIGIBLE_LANES.has(status)) return false; return enabled.length === 0 || allReviewersApproved(activity, enabled); } @@ -34,8 +36,9 @@ export const TASK_BOARD_PROMOTE_TO_PRODUCTION = defineTool({ name: "TASK_BOARD_PROMOTE_TO_PRODUCTION", description: "Ship a reviewed task: merge its open pull request and move the task to " + - "Done. Used by the board's 'Ship to production' button after the enabled " + - "reviewers approved, when auto-merge is off (a human does the final merge).", + "Merged (or Done, on a board without the delivery lanes). Used by the " + + "board's 'Ship to production' button after the enabled reviewers " + + "approved, when auto-merge is off (a human does the final merge).", annotations: { title: "Ship to Production", readOnlyHint: false, @@ -76,7 +79,7 @@ export const TASK_BOARD_PROMOTE_TO_PRODUCTION = defineTool({ ); if (!isReadyToShip(item.status, activity, enabled)) { throw new Error( - "Task is not ready to ship: it must be In Review with every enabled reviewer approved", + "Task is not ready to ship: it must be In Review or Approved with every enabled reviewer approved", ); } @@ -96,18 +99,19 @@ export const TASK_BOARD_PROMOTE_TO_PRODUCTION = defineTool({ return { status: item.status, merged: false }; } + const shipped = shippedLane(settings?.flags); const updated = await ctx.storage.taskBoard.update( taskBoardItemId, organizationId, - { status: "done" }, + { status: shipped }, item.updatedBy, ); - if (item.status !== "done") { + if (item.status !== shipped) { await recordTaskActivity(ctx, { taskBoardItemId, action: "status_changed", actorId: null, - data: { from: item.status, to: "done" }, + data: { from: item.status, to: shipped }, }); } emitTaskBoardUpdated(organizationId, updated); diff --git a/apps/api/src/tools/task-board/prs-get.ts b/apps/api/src/tools/task-board/prs-get.ts index 5577754c31..b935eaa8d9 100644 --- a/apps/api/src/tools/task-board/prs-get.ts +++ b/apps/api/src/tools/task-board/prs-get.ts @@ -6,12 +6,16 @@ import type { ConnectionEntity } from "@/tools/connection/schema"; import { clientFromConnection } from "@/mcp-clients"; import type { TaskBoardItemPrRef } from "@/storage/types"; import { getRepoScope } from "@decocms/shared/github-repo-scope"; -import { SUPER_AGENT_ASSIGNEE_ID } from "@decocms/shared/task-board"; +import { + shippedLane, + SUPER_AGENT_ASSIGNEE_ID, +} from "@decocms/shared/task-board"; import { retry, RetryError } from "@decocms/shared/std"; import { InMemoryMcpReadCache } from "@/mcp-clients/mcp-read-cache"; import { TaskBoardItemPrSchema } from "./schema"; import { cardWorkLanded } from "./archive-merged"; import { recordTaskActivity } from "./activity"; +import { movesForward } from "./lanes"; import { emitTaskBoardUpdated } from "./run-reactions"; import { enqueueEnabledReviewers } from "./enqueue-reviewer"; import { reactToApprovedPrConflict } from "./conflict-reaction"; @@ -1246,15 +1250,18 @@ export const TASK_BOARD_ITEM_PRS_GET = defineTool({ } // ponytail: reconcile-on-view — there's no GitHub PR webhook, so a merged PR - // only advances the card to Done when someone opens this modal. Upgrade path: + // only advances the card when someone opens this modal. Upgrade path: // a `pull_request` webhook calling the same forward move. Best-effort; a - // failure must never break the read. Forward-only: never un-does Done or Archived. + // failure must never break the read. Forward-only via `movesForward`. if (cardWorkLanded(prs)) { try { + // Inside the try: this block is best-effort and must not fail the read. + const settings = + await ctx.storage.organizationSettings.get(organizationId); + const shipped = shippedLane(settings?.flags); if ( item && - item.status !== "done" && - item.status !== "archived" && + movesForward(item.status, shipped) && !(await ctx.storage.taskBoard.hasHumanRejectedDone( taskBoardItemId, organizationId, @@ -1263,7 +1270,7 @@ export const TASK_BOARD_ITEM_PRS_GET = defineTool({ const updated = await ctx.storage.taskBoard.update( taskBoardItemId, organizationId, - { status: "done" }, + { status: shipped }, item.updatedBy, ); // Every other path that moves a card to Done (the review-decision @@ -1275,12 +1282,12 @@ export const TASK_BOARD_ITEM_PRS_GET = defineTool({ taskBoardItemId, action: "status_changed", actorId: null, - data: { from: item.status, to: "done" }, + data: { from: item.status, to: shipped }, }); emitTaskBoardUpdated(organizationId, updated); } } catch (err) { - console.error("[task-board] merge→done reconcile failed", err); + console.error("[task-board] merged-PR reconcile failed", err); } } diff --git a/apps/api/src/tools/task-board/reconcile-merged.test.ts b/apps/api/src/tools/task-board/reconcile-merged.test.ts index 85823873ef..ccb0e61d49 100644 --- a/apps/api/src/tools/task-board/reconcile-merged.test.ts +++ b/apps/api/src/tools/task-board/reconcile-merged.test.ts @@ -35,11 +35,18 @@ const item = (over: Partial = {}): TaskBoardItem => ...over, }) as TaskBoardItem; -function fakeCtx(over: { humanRejectedDone?: boolean } = {}) { +function fakeCtx( + over: { humanRejectedDone?: boolean; deliveryLanes?: boolean } = {}, +) { const updates: { status: string }[] = []; const activity: Record[] = []; const ctx = { storage: { + organizationSettings: { + get: async () => ({ + flags: { delivery_lanes_enabled: over.deliveryLanes ?? false }, + }), + }, taskBoard: { hasHumanRejectedDone: async () => over.humanRejectedDone ?? false, update: async ( @@ -60,7 +67,7 @@ function fakeCtx(over: { humanRejectedDone?: boolean } = {}) { } describe("advanceToDoneIfMerged", () => { - it("moves a card whose PR landed outside Studio, and says why", async () => { + it("moves a card whose PR landed outside Studio to Done, and says why", async () => { const { ctx, updates, activity } = fakeCtx(); expect(await advanceToDoneIfMerged(ctx, item(), [merged])).toBe(true); expect(updates).toEqual([{ status: "done" }]); @@ -74,6 +81,16 @@ describe("advanceToDoneIfMerged", () => { ]); }); + // With the lanes on a merged PR is DEPLOYED, not finished. + it("lands on Merged when the org runs the delivery lanes", async () => { + const { ctx, updates, activity } = fakeCtx({ deliveryLanes: true }); + expect(await advanceToDoneIfMerged(ctx, item(), [merged])).toBe(true); + expect(updates).toEqual([{ status: "merged" }]); + expect(activity[0]).toMatchObject({ + data: { from: "in_review", to: "merged", reason: "pr_merged" }, + }); + }); + it("leaves an unmerged card alone", async () => { const { ctx, updates } = fakeCtx(); expect(await advanceToDoneIfMerged(ctx, item(), [openPr])).toBe(false); diff --git a/apps/api/src/tools/task-board/reconcile-merged.ts b/apps/api/src/tools/task-board/reconcile-merged.ts index b210ca1c66..e16c50b519 100644 --- a/apps/api/src/tools/task-board/reconcile-merged.ts +++ b/apps/api/src/tools/task-board/reconcile-merged.ts @@ -13,21 +13,23 @@ * both gate on `status === "done"` already, so they never see these cards. * * This is the missing reconcile: if the linked PRs have all landed, move the - * card to Done and record the transition. It runs from the review sweeper (see + * card forward (Merged with the delivery lanes on, else Done) and record the + * transition. It runs from the review sweeper (see * `review-sweeper.ts`), which already visits exactly these cards on their own * five-minute interval and already knows each PR's live merged state. */ import type { StudioContext } from "@/core/studio-context"; import type { TaskBoardItem } from "@/storage/types"; +import { shippedLane } from "@decocms/shared/task-board"; import { recordTaskActivity } from "./activity"; import { cardWorkLanded, type PrLanding } from "./archive-merged"; import { emitTaskBoardUpdated } from "./run-reactions"; /** - * Move `item` to Done if its work landed on GitHub (see {@link cardWorkLanded} - * for what "landed" means once a card carries more than one PR). Returns - * whether it moved. + * Move `item` past review if its work landed on GitHub — to Merged with the + * delivery lanes on, else to Done. See {@link cardWorkLanded} for what "landed" + * means once a card carries more than one PR. Returns whether it moved. * * Takes the live PR states rather than reading GitHub itself: its one caller, * the sweeper, already reads every linked PR through the rate-limited queue @@ -57,21 +59,24 @@ export async function advanceToDoneIfMerged( return false; } + // Read only once the card is actually shipping. + const settings = await ctx.storage.organizationSettings.get(orgId); + const shipped = shippedLane(settings?.flags); const done = await ctx.storage.taskBoard.update( item.id, orgId, - { status: "done" }, + { status: shipped }, item.updatedBy, ); await recordTaskActivity(ctx, { taskBoardItemId: item.id, action: "status_changed", actorId: null, - data: { from: item.status, to: "done", reason: "pr_merged" }, + data: { from: item.status, to: shipped, reason: "pr_merged" }, }); emitTaskBoardUpdated(orgId, done); console.log( - `[task-board-review-sweeper] ${item.id}: linked PR already merged — moved to done`, + `[task-board-review-sweeper] ${item.id}: linked PR already merged — moved to ${shipped}`, ); return true; } diff --git a/apps/api/src/tools/task-board/review-decision.ts b/apps/api/src/tools/task-board/review-decision.ts index 2927cb9ed2..64cafa47e7 100644 --- a/apps/api/src/tools/task-board/review-decision.ts +++ b/apps/api/src/tools/task-board/review-decision.ts @@ -9,6 +9,7 @@ import { reviewCycleStart, reviewCycleVerdicts, type ReviewerKind, + shippedLane, } from "@decocms/shared/task-board"; import { TaskBoardItemStatusSchema } from "./schema"; import { recordTaskActivity } from "./activity"; @@ -367,19 +368,20 @@ export const TASK_BOARD_REVIEW_DECISION = defineTool({ : null; const merged = outcome?.merged === true; - // A merge ships the task → Done. + // A merge ships the task → Merged with the delivery lanes on, else Done. if (merged) { + const shipped = shippedLane(settings?.flags); const done = await ctx.storage.taskBoard.update( taskBoardItemId, organizationId, - { status: "done" }, + { status: shipped }, item.updatedBy, ); await recordTaskActivity(ctx, { taskBoardItemId, action: "status_changed", actorId: null, - data: { from: item.status, to: "done" }, + data: { from: item.status, to: shipped }, }); emitTaskBoardUpdated(organizationId, done); return { status: done.status, merged: true }; diff --git a/apps/api/src/tools/task-board/run-reactions.ts b/apps/api/src/tools/task-board/run-reactions.ts index 3667066a23..775e9494cc 100644 --- a/apps/api/src/tools/task-board/run-reactions.ts +++ b/apps/api/src/tools/task-board/run-reactions.ts @@ -35,19 +35,10 @@ import { TASK_BOARD_ITEM_UPDATED_EVENT, } from "@decocms/shared/task-board"; import { recordTaskActivity } from "./activity"; +import { LANE_RANK } from "./lanes"; import type { TaskBoardStorage } from "@/storage/task-board"; import type { TaskBoardItem, TaskBoardItemStatus } from "@/storage/types"; -/** Board lane order — a transition only moves a card forward, never back. */ -const RANK: Record = { - triage: 0, - todo: 1, - in_progress: 2, - in_review: 3, - done: 4, - archived: 5, -}; - /** Run-lifecycle funnel events (auto-fix leg of the PLG funnel). System * actions with no acting user — org identity, person processing off. * Fire-and-forget; posthog no-ops without POSTHOG_KEY. */ @@ -161,7 +152,7 @@ export async function advanceTaskBoardForRun( // repeated PR tool call, or in_progress re-fired on a DBOS retry, won't // regress a card that's already further along). Upgrade to a conditional // UPDATE ... WHERE status-rank < new-rank only if concurrency ever bites. - if (!current || RANK[status] <= RANK[current.status]) continue; + if (!current || LANE_RANK[status] <= LANE_RANK[current.status]) continue; const item = await ctx.storage.taskBoard.update( itemId, orgId, @@ -320,8 +311,9 @@ export async function reactToFailedTaskRun( for (const itemId of await taskBoard.linkedTaskIds(threadId, orgId)) { const item = await taskBoard.getById(itemId, orgId); if (!item) continue; - // The run moved the card itself and only THEN lost its stream. - if (item.status === "in_review" || item.status === "done") { + // The run moved the card itself and only THEN lost its stream. By rank, so + // a card the merged-PR reconcile pushed further still counts as delivered. + if (LANE_RANK[item.status] >= LANE_RANK.in_review) { await taskBoard .relabelDeliveredFailure(threadId, orgId, DELIVERED_FAILURE_REASON) .catch(() => {}); @@ -424,7 +416,7 @@ export async function refundUnproductiveTaskClaims( for (const taskId of await taskBoard.linkedTaskIds(threadId, orgId)) { const item = await taskBoard.getById(taskId, orgId); if (!item) continue; - if (RANK[item.status] >= RANK.in_review) continue; + if (LANE_RANK[item.status] >= LANE_RANK.in_review) continue; const stillRunning = item.threads.some( (t) => t.hasMessages && diff --git a/apps/api/src/tools/task-board/schema.ts b/apps/api/src/tools/task-board/schema.ts index acebb352b4..373ddf98f8 100644 --- a/apps/api/src/tools/task-board/schema.ts +++ b/apps/api/src/tools/task-board/schema.ts @@ -20,6 +20,9 @@ export const TaskBoardItemStatusSchema = z.enum([ "todo", "in_progress", "in_review", + "approved", + "merged", + "post_deploy_validation", "done", "archived", ]); diff --git a/apps/api/src/tools/task-board/tag-merged.integration.test.ts b/apps/api/src/tools/task-board/tag-merged.integration.test.ts index 6dd30b7f1f..81420489ed 100644 --- a/apps/api/src/tools/task-board/tag-merged.integration.test.ts +++ b/apps/api/src/tools/task-board/tag-merged.integration.test.ts @@ -96,6 +96,32 @@ describe("merged-tag sweep", () => { expect(ids).not.toContain(notDone); }); + // With the delivery lanes on a merge lands the card on `deployed`, so gating + // the sweep on Done alone tagged nothing until a human finished dragging it. + it("picks up a card a merge left in a delivery lane", async () => { + const mergedLane = await seed("merged", true); + const postDeploy = await seed("post_deploy_validation", true); + const approved = await seed("approved", true); + const archived = await seed("archived", true); + + const ids = await candidateIds(); + expect(ids).toContain(mergedLane); + expect(ids).toContain(postDeploy); + expect(ids).toContain(approved); + // Far enough along that a tag moves nothing. + expect(ids).not.toContain(archived); + }); + + it("tags a card that shipped into a delivery lane, not only a Done one", async () => { + const id = await seed("merged", true); + await tagMergedForOrg(ctx, ORG, [id], async () => ({ + state: "closed" as const, + merged: true, + })); + const tagged = await taskBoard.getById(id, ORG); + expect(tagged?.tags.map((t) => t.name)).toContain(MERGED_TAG_NAME); + }); + it("tags a merged card, logs it, and drops it from the work list", async () => { const id = await seed("done", true); diff --git a/apps/api/src/tools/task-board/tag-merged.ts b/apps/api/src/tools/task-board/tag-merged.ts index 1b0e059588..9b846e01d6 100644 --- a/apps/api/src/tools/task-board/tag-merged.ts +++ b/apps/api/src/tools/task-board/tag-merged.ts @@ -19,6 +19,7 @@ import { nextTagColor } from "@decocms/shared/task-board"; import type { StudioContext } from "@/core/studio-context"; import { recordTaskActivity } from "./activity"; import { cardWorkLanded, type PrLandingReader } from "./archive-merged"; +import { isTaggableMergedStatus } from "./lanes"; import { fetchPrLanding } from "./prs-get"; import { emitTaskBoardUpdated } from "./run-reactions"; @@ -65,7 +66,7 @@ async function tagIfMerged( prLanding: PrLandingReader, ): Promise { const item = await ctx.storage.taskBoard.getById(itemId, organizationId); - if (!item || item.status !== "done") return false; + if (!item || !isTaggableMergedStatus(item.status)) return false; const prs = await ctx.storage.taskBoard.listPrs(itemId, organizationId); const landings = await Promise.all( diff --git a/apps/api/src/tools/task-board/update.test.ts b/apps/api/src/tools/task-board/update.test.ts index 3b1c752899..a43b02b474 100644 --- a/apps/api/src/tools/task-board/update.test.ts +++ b/apps/api/src/tools/task-board/update.test.ts @@ -213,11 +213,20 @@ describe("closesOwnReview", () => { expect(closesOwnReview("archived", "in_review", true)).toBe(true); }); + // Shipping yourself past review also drops the card out of the review sweep. + it("catches a run shipping a task under review into a delivery lane", () => { + expect(closesOwnReview("approved", "in_review", true)).toBe(true); + expect(closesOwnReview("merged", "in_review", true)).toBe(true); + expect(closesOwnReview("post_deploy_validation", "in_review", true)).toBe( + true, + ); + }); + it("allows a run to complete a task that needed no code change", () => { expect(closesOwnReview("done", "in_progress", true)).toBe(false); }); - it("allows a run to move a task under review anywhere but Done/Archived", () => { + it("allows a run to move a task under review BACKWARD, or not at all", () => { expect(closesOwnReview("in_progress", "in_review", true)).toBe(false); expect(closesOwnReview(undefined, "in_review", true)).toBe(false); }); diff --git a/apps/api/src/tools/task-board/update.ts b/apps/api/src/tools/task-board/update.ts index 3c80ef1052..7442e6dc1a 100644 --- a/apps/api/src/tools/task-board/update.ts +++ b/apps/api/src/tools/task-board/update.ts @@ -140,8 +140,13 @@ export function delegatesToSuperAgent( /** Forward-only terminal lanes (see the activity comment above): once a card * lands here it's out of the review loop, same as "done" — `archived` skips - * review just as effectively as completing it does. */ + * review just as effectively as completing it does. The delivery lanes count + * too: they sit past In Review, so a run setting `merged` would escape this + * guard and drop the card out of `listItemsPendingReview`. */ const REVIEW_CLOSING_STATUSES = new Set([ + "approved", + "merged", + "post_deploy_validation", "done", "archived", ]); diff --git a/apps/web/src/components/settings/review-settings.tsx b/apps/web/src/components/settings/review-settings.tsx index efd2b47e98..9406d63348 100644 --- a/apps/web/src/components/settings/review-settings.tsx +++ b/apps/web/src/components/settings/review-settings.tsx @@ -5,6 +5,7 @@ import { Cube01, FileSearch02, GitMerge, + Rocket01, ShieldTick, Terminal, UserSquare, @@ -56,6 +57,12 @@ export function ReviewSettings() { titleKey="settings.review.autoMergeTitle" descriptionKey="settings.review.autoMergeDescription" /> + } + titleKey="settings.review.deliveryLanesTitle" + descriptionKey="settings.review.deliveryLanesDescription" + /> } diff --git a/apps/web/src/i18n/en/settings.ts b/apps/web/src/i18n/en/settings.ts index 423456ec56..03a2e7b976 100644 --- a/apps/web/src/i18n/en/settings.ts +++ b/apps/web/src/i18n/en/settings.ts @@ -473,6 +473,9 @@ export const settings = { "settings.review.autoMergeTitle": "Enable Auto-merge", "settings.review.autoMergeDescription": "When every enabled reviewer approves, merge the pull request automatically instead of waiting for a human. If a conflict blocks the merge, the Super Agent resolves it first.", + "settings.review.deliveryLanesTitle": "Show delivery lanes", + "settings.review.deliveryLanesDescription": + "Add Approved, Merged and Post-deploy Validation between In Review and Done, and land a merged pull request on Merged instead of Done. For teams whose release process continues after the merge.", "settings.review.autoAssignReportTasksTitle": "Auto-assign report tasks to the Super Agent", "settings.review.autoAssignReportTasksDescription": diff --git a/apps/web/src/i18n/en/task-board.ts b/apps/web/src/i18n/en/task-board.ts index e9ffc8613a..8c4b3e1d34 100644 --- a/apps/web/src/i18n/en/task-board.ts +++ b/apps/web/src/i18n/en/task-board.ts @@ -4,11 +4,14 @@ export const taskBoard = { "taskBoard.config.priorityMedium": "Medium", "taskBoard.config.priorityNone": "No priority", "taskBoard.config.priorityUrgent": "Urgent", + "taskBoard.config.statusApproved": "Approved", "taskBoard.config.statusArchived": "Archived", "taskBoard.config.statusBacklog": "Backlog", + "taskBoard.config.statusMerged": "Merged", "taskBoard.config.statusDone": "Done", "taskBoard.config.statusInProgress": "In Progress", "taskBoard.config.statusInReview": "In Review", + "taskBoard.config.statusPostDeployValidation": "Post-deploy Validation", "taskBoard.config.statusTodo": "To Do", "taskBoard.taskBoard.assignedToSuperAgent": "Assigned to Super Agent", "taskBoard.taskBoard.assignedToSuperAgentBy": diff --git a/apps/web/src/i18n/pt-br/settings.ts b/apps/web/src/i18n/pt-br/settings.ts index d6d077cb25..fdbe88249f 100644 --- a/apps/web/src/i18n/pt-br/settings.ts +++ b/apps/web/src/i18n/pt-br/settings.ts @@ -489,6 +489,9 @@ export const settings = { "settings.review.autoMergeTitle": "Ativar Auto-merge", "settings.review.autoMergeDescription": "Quando todos os revisores habilitados aprovam, mescla o pull request automaticamente em vez de esperar por uma pessoa. Se um conflito bloquear o merge, o Super Agent resolve antes.", + "settings.review.deliveryLanesTitle": "Mostrar as colunas de entrega", + "settings.review.deliveryLanesDescription": + "Adiciona Aprovado, Implantado e Valida\u00e7\u00e3o P\u00f3s Deploy entre Em Revis\u00e3o e Conclu\u00eddo, e faz um pull request mesclado cair em Implantado em vez de Conclu\u00eddo. Para times cujo processo de release continua depois do merge.", "settings.review.autoAssignReportTasksTitle": "Atribuir tarefas de relat\u00f3rio ao Super Agent automaticamente", "settings.review.autoAssignReportTasksDescription": diff --git a/apps/web/src/i18n/pt-br/task-board.ts b/apps/web/src/i18n/pt-br/task-board.ts index 7093b139cf..d1280dad97 100644 --- a/apps/web/src/i18n/pt-br/task-board.ts +++ b/apps/web/src/i18n/pt-br/task-board.ts @@ -6,11 +6,14 @@ export const taskBoard = { "taskBoard.config.priorityMedium": "Média", "taskBoard.config.priorityNone": "Sem prioridade", "taskBoard.config.priorityUrgent": "Urgente", + "taskBoard.config.statusApproved": "Aprovado", "taskBoard.config.statusArchived": "Arquivado", "taskBoard.config.statusBacklog": "Backlog", + "taskBoard.config.statusMerged": "Implantado", "taskBoard.config.statusDone": "Concluído", "taskBoard.config.statusInProgress": "Em Progresso", "taskBoard.config.statusInReview": "Em Revisão", + "taskBoard.config.statusPostDeployValidation": "Validação Pós Deploy", "taskBoard.config.statusTodo": "A Fazer", "taskBoard.taskBoard.assignedToSuperAgent": "Atribuído ao Super Agent", "taskBoard.taskBoard.assignedToSuperAgentBy": diff --git a/apps/web/src/layouts/task-board/config.test.ts b/apps/web/src/layouts/task-board/config.test.ts index 0430b1f6bb..735e412776 100644 --- a/apps/web/src/layouts/task-board/config.test.ts +++ b/apps/web/src/layouts/task-board/config.test.ts @@ -7,8 +7,11 @@ import { formatSprintDates, insertSortOrder, isTaskHandedToHuman, + laneVisibility, + moveTargets, runSortOrders, statusIconClassName, + STATUSES, } from "./config"; import type { TaskBoardItem } from "./config"; @@ -275,3 +278,91 @@ describe("cardNeedsAttention", () => { expect(cardNeedsAttention(asking(inReview("user-1")))).toBe(true); }); }); + +describe("moveTargets", () => { + test("offers no delivery lane to a board that doesn't run them", () => { + expect(moveTargets(false)).toEqual([ + "triage", + "todo", + "in_progress", + "in_review", + "done", + "archived", + ]); + }); + + test("offers every lane once they're on", () => { + expect(moveTargets(true)).toEqual(STATUSES); + }); +}); + +describe("laneVisibility", () => { + const shown: string[] = []; + + test("draws the delivery lanes as columns when they're on", () => { + const { lanes, hidden } = laneVisibility({ + deliveryEnabled: true, + shownLanes: shown, + occupied: [], + }); + expect(lanes).toEqual([ + "triage", + "todo", + "in_progress", + "in_review", + "approved", + "merged", + "post_deploy_validation", + "done", + // archived is hidden by default + ]); + expect(hidden).toEqual(["archived"]); + }); + + test("an empty delivery lane is absent, not hidden, when they're off", () => { + const { lanes, hidden } = laneVisibility({ + deliveryEnabled: false, + shownLanes: shown, + occupied: [], + }); + expect(lanes).toEqual([ + "triage", + "todo", + "in_progress", + "in_review", + "done", + ]); + expect(hidden).toEqual(["archived"]); + }); + + // Lanes off with work still in one: the card must stay reachable. + test("a card left in a delivery lane keeps the lane in the drawer", () => { + const { lanes, hidden, hideable } = laneVisibility({ + deliveryEnabled: false, + shownLanes: shown, + occupied: ["merged"], + }); + expect(lanes).not.toContain("merged"); + expect(hidden).toEqual(["merged", "archived"]); + expect(hideable).toContain("merged"); + }); + + test("and showing it puts the column back", () => { + const { lanes, hidden } = laneVisibility({ + deliveryEnabled: false, + shownLanes: ["merged"], + occupied: ["merged"], + }); + expect(lanes).toContain("merged"); + expect(hidden).toEqual(["archived"]); + }); + + test("a lane removed from the product can linger in the preference", () => { + const { lanes } = laneVisibility({ + deliveryEnabled: false, + shownLanes: ["a_lane_that_no_longer_exists"], + occupied: [], + }); + expect(lanes).not.toContain("a_lane_that_no_longer_exists"); + }); +}); diff --git a/apps/web/src/layouts/task-board/config.tsx b/apps/web/src/layouts/task-board/config.tsx index 68cdd4aa64..7dbbda9074 100644 --- a/apps/web/src/layouts/task-board/config.tsx +++ b/apps/web/src/layouts/task-board/config.tsx @@ -11,12 +11,15 @@ import { Lightbulb02, Loading02, PlusCircle, + Rocket01, Settings01, Shield01, + ShieldTick, + ThumbsUp, } from "@untitledui/icons"; import { Bug } from "lucide-react"; import type { StudioToolOutput as ToolOutput } from "@decocms/shared/tools/tool-io"; -import { DEFAULT_TAG_COLOR } from "@decocms/shared/task-board"; +import { DEFAULT_TAG_COLOR, DELIVERY_LANES } from "@decocms/shared/task-board"; import { isResolvedRunFailure } from "@decocms/shared/entities"; import type { Sprint } from "@decocms/shared/sprints"; import type { ComponentType } from "react"; @@ -184,6 +187,9 @@ export const STATUSES: TaskBoardItemStatus[] = [ "todo", "in_progress", "in_review", + "approved", + "merged", + "post_deploy_validation", "done", "archived", ]; @@ -219,6 +225,21 @@ export const STATUS_CONFIG: Record< icon: Eye, iconClassName: "text-warning", }, + approved: { + labelKey: "taskBoard.config.statusApproved", + icon: ThumbsUp, + iconClassName: "text-success", + }, + merged: { + labelKey: "taskBoard.config.statusMerged", + icon: Rocket01, + iconClassName: "text-primary", + }, + post_deploy_validation: { + labelKey: "taskBoard.config.statusPostDeployValidation", + icon: ShieldTick, + iconClassName: "text-warning", + }, done: { labelKey: "taskBoard.config.statusDone", icon: CheckCircle, @@ -287,6 +308,55 @@ export const TASK_TYPE_CONFIG: Record< }, }; +/** True for one of the post-merge delivery lanes. */ +export function isDeliveryLane(status: TaskBoardItemStatus): boolean { + return (DELIVERY_LANES as string[]).includes(status); +} + +/** + * Lanes a card may be MOVED to — "Move to", the status dropdown, drag targets. + * With the delivery lanes off they aren't offered, so nobody can put a card + * somewhere the org's state machine doesn't ship to. Rendering a lane's own + * label is a separate question, always answered by `STATUS_CONFIG`. + */ +export function moveTargets(deliveryEnabled: boolean): TaskBoardItemStatus[] { + return deliveryEnabled + ? STATUSES + : STATUSES.filter((s) => !isDeliveryLane(s)); +} + +/** + * Which lanes the board draws as columns, and which collapse into the "Hidden + * columns" drawer. A lane hides when it's hidden by default (`HIDDEN_STATUSES`) + * or is a delivery lane the board doesn't run; `shownLanes` overrides either. + * `occupied` is what keeps a card from getting stuck: an unrun delivery lane is + * absent while empty, but reappears in the drawer the moment a card sits in it. + */ +export function laneVisibility({ + deliveryEnabled, + shownLanes, + occupied, +}: { + deliveryEnabled: boolean; + /** `string[]`: it comes out of localStorage, which can hold a dead lane. */ + shownLanes: readonly string[]; + occupied: readonly TaskBoardItemStatus[]; +}): { + lanes: TaskBoardItemStatus[]; + hidden: TaskBoardItemStatus[]; + hideable: TaskBoardItemStatus[]; +} { + const known = STATUSES.filter( + (s) => deliveryEnabled || !isDeliveryLane(s) || occupied.includes(s), + ); + const hideable = known.filter( + (s) => + HIDDEN_STATUSES.includes(s) || (!deliveryEnabled && isDeliveryLane(s)), + ); + const hidden = hideable.filter((s) => !shownLanes.includes(s)); + return { lanes: known.filter((s) => !hidden.includes(s)), hidden, hideable }; +} + export const PRIORITIES: TaskBoardItemPriority[] = [ "none", "low", diff --git a/apps/web/src/layouts/task-board/index.tsx b/apps/web/src/layouts/task-board/index.tsx index c6708825ab..513095ae72 100644 --- a/apps/web/src/layouts/task-board/index.tsx +++ b/apps/web/src/layouts/task-board/index.tsx @@ -111,6 +111,8 @@ import { isTaskBlocked, isTaskHandedToHuman, HIDDEN_STATUSES, + laneVisibility, + moveTargets, PRIORITIES, PRIORITY_CONFIG, runSortOrders, @@ -1586,6 +1588,7 @@ function SelectionBar({ }) { const t = useT(); const { data: orgTags = [] } = useTags(); + const deliveryEnabled = useOrgFlag("delivery_lanes_enabled"); return (
@@ -1608,7 +1611,7 @@ function SelectionBar({ {t("taskBoard.taskBoard.moveToButton")} - {STATUSES.map((status) => ( + {moveTargets(deliveryEnabled).map((status) => ( onMoveTo(status)} @@ -1815,6 +1818,7 @@ function Lanes({ /** False while the task detail has the panel — see `useFlipLanes`. */ visible: boolean; }) { + const deliveryEnabled = useOrgFlag("delivery_lanes_enabled"); const [activeId, setActiveId] = useState(null); // Cards that just landed from a drop — they get the settle animation. Cleared // on drag start so dropping the same card twice replays it (a CSS animation @@ -1904,12 +1908,17 @@ function Lanes({ const laneItems = (status: TaskBoardItemStatus) => placed.filter((item) => item.status === status).sort(bySortOrder); - /** Shown-again lanes persist per person (localStorage), so pulling Archived - * onto the board survives a reload. */ - const hiddenLanes = HIDDEN_STATUSES.filter( - (status) => !preferences.shownTaskBoardLanes.includes(status), - ); - const boardLanes = STATUSES.filter((status) => !hiddenLanes.includes(status)); + /** Shown-again lanes persist per person, so pulling one onto the board + * survives a reload. */ + const { + lanes: boardLanes, + hidden: hiddenLanes, + hideable: hideableLanes, + } = laneVisibility({ + deliveryEnabled, + shownLanes: preferences.shownTaskBoardLanes, + occupied: placed.map((item) => item.status), + }); const setLaneShown = (status: TaskBoardItemStatus, shown: boolean) => setPreferences((prev) => ({ ...prev, @@ -2084,7 +2093,7 @@ function Lanes({ onTypeChange={onTypeChange} onDueDateChange={onDueDateChange} onHide={ - HIDDEN_STATUSES.includes(status) + hideableLanes.includes(status) ? () => setLaneShown(status, false) : undefined } diff --git a/apps/web/src/layouts/task-board/review-status.test.ts b/apps/web/src/layouts/task-board/review-status.test.ts index cab8be25e2..a39ccd19ab 100644 --- a/apps/web/src/layouts/task-board/review-status.test.ts +++ b/apps/web/src/layouts/task-board/review-status.test.ts @@ -3,6 +3,7 @@ import type { TaskBoardActivity } from "@/hooks/use-task-board-activity"; import { checksSummary, enabledReviewers, + laneCanShip, type ReviewerKind, reviewsSatisfiedForPromotion, } from "./review-status"; @@ -163,3 +164,25 @@ describe("checksSummary", () => { ).toEqual({ passed: 1, total: 1, tone: "ok" }); }); }); + +describe("laneCanShip", () => { + it("offers the button from the two lanes the server accepts", () => { + expect(laneCanShip("in_review")).toBe(true); + // The server ships from Approved, so hiding the button there strands the card. + expect(laneCanShip("approved")).toBe(true); + }); + + it("hides it before review and after the ship", () => { + for (const lane of [ + "triage", + "todo", + "in_progress", + "merged", + "post_deploy_validation", + "done", + "archived", + ]) { + expect(laneCanShip(lane)).toBe(false); + } + }); +}); diff --git a/apps/web/src/layouts/task-board/review-status.ts b/apps/web/src/layouts/task-board/review-status.ts index a2bd9cc842..41a20c4521 100644 --- a/apps/web/src/layouts/task-board/review-status.ts +++ b/apps/web/src/layouts/task-board/review-status.ts @@ -87,3 +87,12 @@ export function checksSummary( if (approvals.length < enabled.length) return { ...summary, tone: "pending" }; return { ...summary, tone: "ok" }; } + +/** + * Lanes the "Ship to production" button may act from. Must mirror the server's + * `SHIP_ELIGIBLE_LANES`: offering more puts a control on screen that always + * errors, offering fewer makes Approved a dead end. + */ +export function laneCanShip(status: string): boolean { + return status === "in_review" || status === "approved"; +} diff --git a/apps/web/src/layouts/task-board/task-dialog.tsx b/apps/web/src/layouts/task-board/task-dialog.tsx index 72bf73a1f1..9df3bd0eb2 100644 --- a/apps/web/src/layouts/task-board/task-dialog.tsx +++ b/apps/web/src/layouts/task-board/task-dialog.tsx @@ -90,7 +90,7 @@ import { type TaskBoardItemType, DEFAULT_TASK_TYPE, STATUS_CONFIG, - STATUSES, + moveTargets, statusIconClassName, SUPER_AGENT_ASSIGNEE_ID, tagDotColor, @@ -115,6 +115,7 @@ import { usePromoteToProduction } from "@/hooks/use-promote-to-production"; import { useResolveConflict } from "@/hooks/use-resolve-conflict"; import { enabledReviewers, + laneCanShip, reviewsSatisfiedForPromotion, } from "./review-status"; import { formatTimeAgo } from "@/lib/format-time"; @@ -426,6 +427,7 @@ function TaskBoardItemEditor({ const { org } = useProjectContext(); const { data } = useMembers(); const members = (data?.data?.members ?? []) as Member[]; + const deliveryEnabled = useOrgFlag("delivery_lanes_enabled"); const { handleCopy, copied } = useCopy(); const { handleCopy: copyLink, copied: linkCopied } = useCopy(); const { handleCopy: copyId, copied: idCopied } = useCopy(); @@ -954,7 +956,7 @@ function TaskBoardItemEditor({ - {STATUSES.map((s) => { + {moveTargets(deliveryEnabled).map((s) => { const Icon = STATUS_CONFIG[s].icon; return ( !isDeliveryLane(o.value)); +} + /** Radix Select forbids empty item values — sentinel for "not synced". */ const DONT_SYNC = "__dont_sync__"; @@ -227,6 +245,7 @@ function ColumnMappingRows({ integration }: { integration: JiraIntegration }) { const t = useT(); const upsert = useUpsertJiraIntegration(); const columns = useJiraBoardColumns(integration.boardId); + const deliveryEnabled = useOrgFlag("delivery_lanes_enabled"); // Optimistic local copy so two quick edits don't race the save round-trip and stomp each other. const [mapping, setMapping] = useState(integration.statusMapping); @@ -317,7 +336,7 @@ function ColumnMappingRows({ integration }: { integration: JiraIntegration }) { {t("settings.jira.dontSync")} - {BOARD_STATUS_OPTIONS.map((option) => ( + {boardStatusOptions(deliveryEnabled).map((option) => ( {t(option.labelKey)} diff --git a/packages/e2e/tests/task-board-delivery-lanes.spec.ts b/packages/e2e/tests/task-board-delivery-lanes.spec.ts new file mode 100644 index 0000000000..4306234821 --- /dev/null +++ b/packages/e2e/tests/task-board-delivery-lanes.spec.ts @@ -0,0 +1,150 @@ +/** + * The delivery lanes over the wire: the flag that gates them, the lanes a card + * can actually rest in, and the ship gate that has to accept Approved. + * + * The zero-behaviour-change property (a merged PR lands on Done with the flag + * off, Merged with it on) is asserted at the unit tier against `shippedLane` + * and each ship site's own test — proving it here would need a real merged pull + * request on GitHub, which this tier does not have. + */ + +import { callSelfMcpTool, findOrgId } from "../fixtures/mcp-tools"; +import { expect, test } from "../fixtures/test"; + +// Black-box wire-contract shapes (owned by this test, per e2e isolation rules). +interface TaskBoardItem { + id: string; + title: string; + status: string; +} +interface Activity { + action: string; + data: Record; +} +interface OrgSettings { + organizationId: string; + flags: Record | null; +} + +const DELIVERY_LANES = ["approved", "merged", "post_deploy_validation"]; + +test.describe("task board delivery lanes", () => { + test("the flag round-trips without disturbing its neighbours", async ({ + authedPage, + }) => { + const { page, orgSlug } = authedPage; + const request = page.context().request; + const call = (name: string, args: unknown) => + callSelfMcpTool(request, orgSlug, name, args); + const orgId = await findOrgId(request, orgSlug); + + // Unset is the default, and it reads as off. + const before = await call("ORGANIZATION_SETTINGS_GET", {}); + expect(before.flags?.delivery_lanes_enabled ?? false).toBe(false); + + await call("ORGANIZATION_SETTINGS_UPDATE", { + organizationId: orgId, + flags: { auto_merge: true }, + }); + await call("ORGANIZATION_SETTINGS_UPDATE", { + organizationId: orgId, + flags: { delivery_lanes_enabled: true }, + }); + const on = await call("ORGANIZATION_SETTINGS_GET", {}); + expect(on.flags?.delivery_lanes_enabled).toBe(true); + // Flags shallow-merge, so turning the lanes on must not clear a neighbour. + expect(on.flags?.auto_merge).toBe(true); + + // An explicit false persists — it is not the same as unset. + await call("ORGANIZATION_SETTINGS_UPDATE", { + organizationId: orgId, + flags: { delivery_lanes_enabled: false }, + }); + const off = await call("ORGANIZATION_SETTINGS_GET", {}); + expect(off.flags?.delivery_lanes_enabled).toBe(false); + }); + + test("a card moves through the delivery lanes and the timeline says so", async ({ + authedPage, + }) => { + const { page, orgSlug } = authedPage; + const request = page.context().request; + const call = (name: string, args: unknown) => + callSelfMcpTool(request, orgSlug, name, args); + + const { item } = await call<{ item: TaskBoardItem }>( + "TASK_BOARD_ITEM_CREATE", + { title: "Rides the delivery lanes", status: "in_review" }, + ); + + for (const status of DELIVERY_LANES) { + const { item: moved } = await call<{ item: TaskBoardItem }>( + "TASK_BOARD_ITEM_UPDATE", + { id: item.id, status }, + ); + expect(moved.status).toBe(status); + } + const { item: finished } = await call<{ item: TaskBoardItem }>( + "TASK_BOARD_ITEM_UPDATE", + { id: item.id, status: "done" }, + ); + expect(finished.status).toBe("done"); + + // The UI draws columns from this read, so it has to carry the lane too. + const { items } = await call<{ items: TaskBoardItem[] }>( + "TASK_BOARD_ITEM_LIST", + {}, + ); + expect(items.find((i) => i.id === item.id)?.status).toBe("done"); + + const { activity } = await call<{ activity: Activity[] }>( + "TASK_BOARD_ACTIVITY_LIST", + { taskBoardItemId: item.id }, + ); + const lanes = activity + .filter((a) => a.action === "status_changed") + .map((a) => a.data.to); + expect(lanes).toEqual([...DELIVERY_LANES, "done"]); + }); + + test("the ship gate accepts Approved and still refuses an unreviewed lane", async ({ + authedPage, + }) => { + const { page, orgSlug } = authedPage; + const request = page.context().request; + const call = (name: string, args: unknown) => + callSelfMcpTool(request, orgSlug, name, args); + const orgId = await findOrgId(request, orgSlug); + + // No reviewers enabled, so the readiness gate reduces to the lane alone. + await call("ORGANIZATION_SETTINGS_UPDATE", { + organizationId: orgId, + flags: { qa_agent_enabled: false, code_reviewer_enabled: false }, + }); + + const { item: early } = await call<{ item: TaskBoardItem }>( + "TASK_BOARD_ITEM_CREATE", + { title: "Not reviewed yet", status: "todo" }, + ); + await expect( + call("TASK_BOARD_PROMOTE_TO_PRODUCTION", { taskBoardItemId: early.id }), + ).rejects.toThrow(/not ready to ship/i); + + // The gate must let this THROUGH; it then fails on the missing pull request. + // Explicit catch, not `.rejects.not.toThrow` — that passes vacuously. + const { item: approved } = await call<{ item: TaskBoardItem }>( + "TASK_BOARD_ITEM_CREATE", + { title: "Parked in Approved", status: "approved" }, + ); + // Resolving means the gate let it through; the empty string keeps the + // matcher on a string either way, since `.not.toMatch(null)` throws rather + // than passing and would fail the very case this asserts. + const shipError = await call("TASK_BOARD_PROMOTE_TO_PRODUCTION", { + taskBoardItemId: approved.id, + }).then( + () => "", + (err: unknown) => (err instanceof Error ? err.message : String(err)), + ); + expect(shipError).not.toMatch(/not ready to ship/i); + }); +}); diff --git a/packages/shared/src/organization/schema.ts b/packages/shared/src/organization/schema.ts index 423ad2952b..43c14b520f 100644 --- a/packages/shared/src/organization/schema.ts +++ b/packages/shared/src/organization/schema.ts @@ -185,6 +185,12 @@ export const OrgFlagsSchema = z.object({ .describe( "When a report import creates a task board item without an assignee, delegate it to the Super Agent automatically instead of leaving it unassigned.", ), + delivery_lanes_enabled: z + .boolean() + .optional() + .describe( + "Board lanes for shipping: Approved, Merged and Post-deploy Validation sit between In Review and Done, and a merged pull request lands on Merged instead of Done. For teams whose release process continues after the merge. Off by default — with it off the board and the state machine behave exactly as if the lanes did not exist.", + ), }); export type OrgFlags = z.infer; diff --git a/packages/shared/src/task-board.ts b/packages/shared/src/task-board.ts index 4558b4af6c..6fc193a415 100644 --- a/packages/shared/src/task-board.ts +++ b/packages/shared/src/task-board.ts @@ -140,6 +140,40 @@ export function enabledReviewerKinds( return REVIEWER_KINDS.filter((k) => orgFlagEnabled(flags, REVIEWER_FLAG[k])); } +/** + * The lanes a release process needs AFTER the pull request merges, sitting + * between In Review and Done. A subset of both sides' status unions, so each + * assigns these names with no cast. The lane TYPE is unconditional — every + * status stays in the union, keeping `LANE_RANK`/`STATUS_CONFIG` exhaustive. + * The flag gates REACHABILITY: which lanes a board offers, where a ship lands. + */ +export type DeliveryLane = "approved" | "merged" | "post_deploy_validation"; + +export const DELIVERY_LANES: DeliveryLane[] = [ + "approved", + "merged", + "post_deploy_validation", +]; + +/** True when this org runs the delivery lanes. Off by default. */ +export function deliveryLanesEnabled( + flags: Record | null | undefined, +): boolean { + return orgFlagEnabled(flags, "delivery_lanes_enabled"); +} + +/** + * The lane a merged/shipped pull request moves its task to. Every automatic + * ship path reads this, so "flag off means no behaviour change" is one tested + * function rather than five call sites agreeing by luck. Falsy flags of every + * shape resolve to `done`; that default IS the safety property. + */ +export function shippedLane( + flags: Record | null | undefined, +): "merged" | "done" { + return deliveryLanesEnabled(flags) ? "merged" : "done"; +} + /** True when a thread title belongs to the given reviewer's run. */ export function isReviewerThreadTitle( title: string | null | undefined, diff --git a/packages/shared/src/tools/tool-io.ts b/packages/shared/src/tools/tool-io.ts index 4e1f9a499d..7a7d20f3f9 100644 --- a/packages/shared/src/tools/tool-io.ts +++ b/packages/shared/src/tools/tool-io.ts @@ -129,6 +129,7 @@ export interface StudioToolIO { coding_agent_org_mcps?: boolean | undefined; coding_agents_claude_code?: boolean | undefined; auto_assign_report_tasks_to_super_agent?: boolean | undefined; + delivery_lanes_enabled?: boolean | undefined; } | null | undefined; @@ -199,6 +200,7 @@ export interface StudioToolIO { coding_agent_org_mcps?: boolean | undefined; coding_agents_claude_code?: boolean | undefined; auto_assign_report_tasks_to_super_agent?: boolean | undefined; + delivery_lanes_enabled?: boolean | undefined; } | undefined; main_agent_id?: string | null | undefined; @@ -269,6 +271,7 @@ export interface StudioToolIO { coding_agent_org_mcps?: boolean | undefined; coding_agents_claude_code?: boolean | undefined; auto_assign_report_tasks_to_super_agent?: boolean | undefined; + delivery_lanes_enabled?: boolean | undefined; } | null | undefined; @@ -323,6 +326,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived" | undefined; priority?: "none" | "low" | "medium" | "high" | "urgent" | undefined; @@ -345,6 +351,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived"; priority: "none" | "low" | "medium" | "high" | "urgent"; type: "bug" | "feature" | "chore" | "spike" | "security"; @@ -409,6 +418,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived"; priority: "none" | "low" | "medium" | "high" | "urgent"; type: "bug" | "feature" | "chore" | "spike" | "security"; @@ -478,6 +490,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived" | undefined; priority?: "none" | "low" | "medium" | "high" | "urgent" | undefined; @@ -502,6 +517,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived"; priority: "none" | "low" | "medium" | "high" | "urgent"; type: "bug" | "feature" | "chore" | "spike" | "security"; @@ -610,6 +628,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived"; merged: boolean; }; @@ -623,6 +644,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived"; merged: boolean; }; @@ -4898,6 +4922,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived", string[] > @@ -4926,6 +4953,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived", string[] > @@ -4948,6 +4978,9 @@ export interface StudioToolIO { | "todo" | "in_progress" | "in_review" + | "approved" + | "merged" + | "post_deploy_validation" | "archived", string[] >