From b3109feb4647dd064173253a7df96f53c5abc218 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 18:38:02 +0200 Subject: [PATCH] [F020] kanban: confirm admin-override drags that skip lifecycle preconditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A drag on the operator kanban routes the status move through the admin status-override, which bypasses the in-band lifecycle validator entirely. That override is intentional (it's how an operator recovers a wedged task) but it also let a careless drag skip material preconditions silently — completing a task with no open PR, QA-bypassing, finishing docs on a task whose docs aren't complete. Leave the override intact but make the bypass explicit: compute the preconditions the dragged move would skip (open PR, docs complete, self-verified + commits + progress for submit-qa, visible non-terminal subtasks for coordination-root targets) and, when any are skipped, hold the move behind a confirmation dialog that lists exactly what's being skipped. Precision over recall — only warn on what the panel can verify from the task and its in-list children; never fabricate a 'satisfied' claim, and stay silent on benign transitions that gate on nothing we can check. The admin status-override capability is preserved (Confirm still fires it); this only surfaces the bypass instead of letting it happen silently. Does not touch the master-merge invariant — the board's updateTask is the operator override, not the Main-PM merge path. --- .../__tests__/bypass-preconditions.test.ts | 133 ++++++++++++ .../__tests__/kanban-board-bypass.test.tsx | 192 ++++++++++++++++++ .../kanban/core/bypass-preconditions.ts | 82 ++++++++ .../components/kanban/core/kanban-board.tsx | 84 ++++++++ 4 files changed, 491 insertions(+) create mode 100644 panel/src/components/kanban/core/__tests__/bypass-preconditions.test.ts create mode 100644 panel/src/components/kanban/core/__tests__/kanban-board-bypass.test.tsx create mode 100644 panel/src/components/kanban/core/bypass-preconditions.ts diff --git a/panel/src/components/kanban/core/__tests__/bypass-preconditions.test.ts b/panel/src/components/kanban/core/__tests__/bypass-preconditions.test.ts new file mode 100644 index 00000000..ec4ac2cd --- /dev/null +++ b/panel/src/components/kanban/core/__tests__/bypass-preconditions.test.ts @@ -0,0 +1,133 @@ +import { describe, it, expect } from "vitest"; +import { TaskStatus, Team, TaskType, type Task } from "@/types"; +import { skippedPreconditions } from "../bypass-preconditions"; + +function buildTask(overrides: Partial = {}): Task { + return { + id: "t1", + title: "A task", + description: "desc", + acceptance_criteria: ["a"], + status: TaskStatus.IN_PROGRESS, + priority: 2, + sequence: 0, + team: Team.BACKEND, + created_by: "ceo", + assigned_to: null, + parent_task_id: null, + dependency_ids: [], + blocker_ids: [], + created_at: "2026-01-01T00:00:00Z", + updated_at: null, + claimed_at: null, + started_at: null, + completed_at: null, + target_date: null, + estimated_complexity: "M" as never, + nature: "feature" as never, + task_type: TaskType.CODE, + project_id: "p1", + docs_complete: false, + pr_created: false, + pm_approvals: {}, + plan: null, + checkpoints: [], + progress_updates: [], + commits: [], + self_verified: false, + qa_verified: null, + sessions: [], + branch_name: null, + pr_number: null, + pr_url: null, + ...overrides, + } as unknown as Task; +} + +describe("skippedPreconditions — F020 (what an admin-override drag skips)", () => { + it("flags a PR-less drag to awaiting_qa with the submit-qa gate's missing checks", () => { + const task = buildTask({ pr_number: null, self_verified: false }); + const skipped = skippedPreconditions(task, TaskStatus.AWAITING_QA, [task]); + expect(skipped).toEqual([ + "no open PR", + "not self-verified", + "no commits linked", + "no progress updates", + ]); + }); + + it("stays silent when a task meets the awaiting_qa preconditions (no false alarm)", () => { + const task = buildTask({ + pr_number: 42, + self_verified: true, + commits: [{ sha: "abc", message: "m", timestamp: "t" } as never], + progress_updates: [{ note: "p" } as never], + }); + expect(skippedPreconditions(task, TaskStatus.AWAITING_QA, [task])).toEqual( + [], + ); + }); + + it("flags a PR-less, undocumented drag to completed", () => { + const task = buildTask({ pr_number: null, docs_complete: false }); + const skipped = skippedPreconditions(task, TaskStatus.COMPLETED, [task]); + expect(skipped).toContain("no open PR"); + expect(skipped).toContain("documentation not marked complete"); + }); + + it("flags non-terminal subtasks when completing a coordination root", () => { + const parent = buildTask({ id: "root", pr_number: 7, docs_complete: true }); + const children = [ + buildTask({ + id: "c1", + parent_task_id: "root", + status: TaskStatus.COMPLETED, + }), + buildTask({ + id: "c2", + parent_task_id: "root", + status: TaskStatus.IN_PROGRESS, + }), + ]; + const skipped = skippedPreconditions(parent, TaskStatus.COMPLETED, [ + parent, + ...children, + ]); + expect(skipped).toContain("1 non-terminal subtask(s)"); + }); + + it("does not fabricate a subtask warning when no children are visible", () => { + // The board's task list is team-filtered; a root's children may be in other + // teams and absent here. Precision over recall: don't claim "subtasks + // terminal" we can't verify — and don't false-alarm either. + const parent = buildTask({ id: "root", pr_number: 7, docs_complete: true }); + expect( + skippedPreconditions(parent, TaskStatus.COMPLETED, [parent]), + ).toEqual([]); + }); + + it("stays silent for benign transitions that skip no material precondition", () => { + // pending -> claimed, in_progress -> blocked, etc. don't gate on PR/docs. + const task = buildTask({ status: TaskStatus.PENDING }); + expect(skippedPreconditions(task, TaskStatus.CLAIMED, [task])).toEqual([]); + const inProgress = buildTask({ status: TaskStatus.IN_PROGRESS }); + expect( + skippedPreconditions(inProgress, TaskStatus.BLOCKED, [inProgress]), + ).toEqual([]); + }); + + it("flags a PR-less drag to awaiting_documentation (pass-qa gate needs a PR)", () => { + const task = buildTask({ pr_number: null }); + expect( + skippedPreconditions(task, TaskStatus.AWAITING_DOCUMENTATION, [task]), + ).toContain("no open PR"); + }); + + it("flags missing docs for a drag to awaiting_pm_review", () => { + const task = buildTask({ pr_number: 9, docs_complete: false }); + const skipped = skippedPreconditions(task, TaskStatus.AWAITING_PM_REVIEW, [ + task, + ]); + expect(skipped).toEqual(["documentation not marked complete"]); + }); +}); diff --git a/panel/src/components/kanban/core/__tests__/kanban-board-bypass.test.tsx b/panel/src/components/kanban/core/__tests__/kanban-board-bypass.test.tsx new file mode 100644 index 00000000..42430f67 --- /dev/null +++ b/panel/src/components/kanban/core/__tests__/kanban-board-bypass.test.tsx @@ -0,0 +1,192 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { TaskStatus, Team, TaskType, type Task } from "@/types"; + +// Capture the board's onDragEnd so the test can synthesize a drop without +// driving the real dnd-kit pointer sensor (painful in jsdom). +const dragRef = vi.hoisted(() => ({ + onDragEnd: null as ((e: unknown) => void) | null, +})); + +vi.mock("@dnd-kit/core", () => ({ + DndContext: ({ + onDragEnd, + children, + }: { + onDragEnd: (e: unknown) => void; + children: React.ReactNode; + }) => { + dragRef.onDragEnd = onDragEnd; + return <>{children}; + }, + DragOverlay: ({ children }: { children: React.ReactNode }) => <>{children}, + PointerSensor: () => null, + useSensor: () => null, + useSensors: () => [], +})); + +const { mutateAsync, refetch, tasksRef } = vi.hoisted(() => ({ + mutateAsync: vi.fn().mockResolvedValue(undefined), + refetch: vi.fn().mockResolvedValue(undefined), + tasksRef: { current: [] as Task[] }, +})); + +vi.mock("@/hooks/use-tasks", () => ({ + useTasks: () => ({ data: tasksRef.current, isLoading: false, refetch }), + useTaskLifecycle: () => ({ + claim: { mutateAsync: vi.fn() }, + start: { mutateAsync: vi.fn() }, + block: { mutateAsync: vi.fn() }, + unblock: { mutateAsync: vi.fn() }, + pause: { mutateAsync: vi.fn() }, + resume: { mutateAsync: vi.fn() }, + verify: { mutateAsync: vi.fn() }, + submitQa: { mutateAsync: vi.fn() }, + passQa: { mutateAsync: vi.fn() }, + failQa: { mutateAsync: vi.fn() }, + complete: { mutateAsync: vi.fn() }, + }), + useUpdateTask: () => ({ mutateAsync, isPending: false }), +})); + +// Avoid rendering the real columns/cards — the bypass gate lives in the board's +// drag handler, not in the column children. +vi.mock("../kanban-column", () => ({ KanbanColumn: () => null })); +vi.mock("../kanban-card", () => ({ KanbanCard: () => null })); +vi.mock("@/components/tasks/task-detail/task-action-dialogs", () => ({ + RequiredNotesDialog: () => null, +})); + +import { KanbanBoard } from "../kanban-board"; + +function buildTask(overrides: Partial = {}): Task { + return { + id: "t1", + title: "A task", + description: "desc", + acceptance_criteria: ["a"], + status: TaskStatus.IN_PROGRESS, + priority: 2, + sequence: 0, + team: Team.BACKEND, + created_by: "ceo", + assigned_to: null, + parent_task_id: null, + dependency_ids: [], + blocker_ids: [], + created_at: "2026-01-01T00:00:00Z", + updated_at: null, + claimed_at: null, + started_at: null, + completed_at: null, + target_date: null, + estimated_complexity: "M" as never, + nature: "feature" as never, + task_type: TaskType.CODE, + project_id: "p1", + docs_complete: false, + pr_created: false, + pm_approvals: {}, + plan: null, + checkpoints: [], + progress_updates: [], + commits: [], + self_verified: false, + qa_verified: null, + sessions: [], + branch_name: null, + pr_number: null, + pr_url: null, + ...overrides, + } as unknown as Task; +} + +const COLUMNS = [ + { id: "pending", status: TaskStatus.PENDING, title: "Pending", color: "" }, + { + id: "in_progress", + status: TaskStatus.IN_PROGRESS, + title: "In Progress", + color: "", + }, + { id: "completed", status: TaskStatus.COMPLETED, title: "Done", color: "" }, +]; + +function drop(activeId: string, overId: TaskStatus) { + dragRef.onDragEnd?.({ active: { id: activeId }, over: { id: overId } }); +} + +describe("KanbanBoard — admin-override bypass confirmation (F020)", () => { + beforeEach(() => { + mutateAsync.mockClear(); + refetch.mockClear(); + tasksRef.current = []; + }); + afterEach(() => { + vi.clearAllMocks(); + }); + + it("confirms before a drag that skips lifecycle preconditions, then fires the override on confirm", async () => { + tasksRef.current = [ + buildTask({ id: "t1", status: TaskStatus.IN_PROGRESS, pr_number: null }), + ]; + render(); + + // Drag a PR-less task straight to Done — completing with no open PR skips + // the in-band gate. The board must NOT fire the override silently; it must + // surface what's skipped and wait for an explicit confirm. + drop("t1", TaskStatus.COMPLETED); + + // The board holds the move and surfaces what's skipped — it must NOT fire + // the admin status-override silently. + await screen.findByText(/no open pr/i); + expect( + screen.getByText(/documentation not marked complete/i), + ).toBeInTheDocument(); + expect(mutateAsync).not.toHaveBeenCalled(); + + // Confirm the override — now the admin status-override fires. + fireEvent.click(screen.getByRole("button", { name: /override & move/i })); + + await waitFor(() => expect(mutateAsync).toHaveBeenCalledTimes(1)); + expect(mutateAsync).toHaveBeenCalledWith({ + taskId: "t1", + updates: { status: TaskStatus.COMPLETED }, + }); + }); + + it("does not fire the override when the confirmation is cancelled", async () => { + tasksRef.current = [ + buildTask({ id: "t1", status: TaskStatus.IN_PROGRESS, pr_number: null }), + ]; + render(); + + drop("t1", TaskStatus.COMPLETED); + await screen.findByText(/no open pr/i); + + fireEvent.click(screen.getByRole("button", { name: /cancel/i })); + + await waitFor(() => + expect(screen.queryByText(/no open pr/i)).not.toBeInTheDocument(), + ); + expect(mutateAsync).not.toHaveBeenCalled(); + }); + + it("fires the move directly when the drag skips no material precondition", async () => { + // pending -> claimed gates on nothing the panel can check — no dialog. + tasksRef.current = [ + buildTask({ id: "t1", status: TaskStatus.PENDING, pr_number: null }), + ]; + render(); + + drop("t1", TaskStatus.CLAIMED); + + await waitFor(() => expect(mutateAsync).toHaveBeenCalledTimes(1)); + expect(mutateAsync).toHaveBeenCalledWith({ + taskId: "t1", + updates: { status: TaskStatus.CLAIMED }, + }); + // No bypass confirmation should ever have been surfaced. + expect(screen.queryByText(/override & move/i)).not.toBeInTheDocument(); + }); +}); diff --git a/panel/src/components/kanban/core/bypass-preconditions.ts b/panel/src/components/kanban/core/bypass-preconditions.ts new file mode 100644 index 00000000..f2d419a3 --- /dev/null +++ b/panel/src/components/kanban/core/bypass-preconditions.ts @@ -0,0 +1,82 @@ +import { TaskStatus, type Task } from "@/types"; + +// A drag on the operator kanban routes the status move through the admin +// status-override, which bypasses the in-band lifecycle validator entirely. +// That override is intentional — it's how an operator recovers a task wedged +// in a state with no valid in-band move. But it also lets a careless drag skip +// material preconditions silently (completing a task with no PR, QA-bypassing +// a task with no PR, finishing docs on a task whose docs aren't complete). +// +// This returns the human-readable preconditions the dragged move would skip, +// computed only from what the panel can see reliably on the task (and its +// in-list children). Precision over recall: we never claim a precondition is +// satisfied when we can't verify it, and we never false-alarm on a transition +// that gates on nothing we can check. An empty list means the drag skips no +// material precondition the panel can detect — proceed without a prompt. + +// Targets whose in-band entry requires an open PR. +const PR_REQUIRED: ReadonlySet = new Set([ + TaskStatus.AWAITING_QA, + TaskStatus.AWAITING_DOCUMENTATION, + TaskStatus.NEEDS_REVISION, + TaskStatus.AWAITING_PR_REVIEW, + TaskStatus.AWAITING_CEO_APPROVAL, + TaskStatus.COMPLETED, +]); + +// Targets whose in-band entry requires the documentation phase to be complete. +const DOCS_REQUIRED: ReadonlySet = new Set([ + TaskStatus.AWAITING_PM_REVIEW, + TaskStatus.COMPLETED, +]); + +// Coordination-root targets: the in-band entry requires all subtasks terminal. +const SUBTASKS_TERMINAL_REQUIRED: ReadonlySet = new Set([ + TaskStatus.AWAITING_PM_REVIEW, + TaskStatus.AWAITING_CEO_APPROVAL, + TaskStatus.COMPLETED, +]); + +const TERMINAL: ReadonlySet = new Set([ + TaskStatus.COMPLETED, + TaskStatus.CANCELLED, +]); + +export function skippedPreconditions( + task: Task, + newStatus: TaskStatus, + allTasks: Task[], +): string[] { + const skipped: string[] = []; + + if (PR_REQUIRED.has(newStatus) && task.pr_number == null) { + skipped.push("no open PR"); + } + + if (DOCS_REQUIRED.has(newStatus) && !task.docs_complete) { + skipped.push("documentation not marked complete"); + } + + // The submit-qa gate (verifying -> awaiting_qa) additionally requires + // self-verification, at least one linked commit, and a progress update. + if (newStatus === TaskStatus.AWAITING_QA) { + if (!task.self_verified) skipped.push("not self-verified"); + if (task.commits.length === 0) skipped.push("no commits linked"); + if (task.progress_updates.length === 0) skipped.push("no progress updates"); + } + + // Coordination-root targets require every subtask in a terminal state. The + // board's task list is team-filtered, so a root's children may live in other + // teams and be absent here — only warn on children we can actually see that + // are not terminal; never fabricate a "subtasks terminal" claim we can't + // verify. + if (SUBTASKS_TERMINAL_REQUIRED.has(newStatus)) { + const children = allTasks.filter((t) => t.parent_task_id === task.id); + const nonTerminal = children.filter((c) => !TERMINAL.has(c.status)); + if (nonTerminal.length > 0) { + skipped.push(`${nonTerminal.length} non-terminal subtask(s)`); + } + } + + return skipped; +} diff --git a/panel/src/components/kanban/core/kanban-board.tsx b/panel/src/components/kanban/core/kanban-board.tsx index 0b229a6c..c6d0919d 100644 --- a/panel/src/components/kanban/core/kanban-board.tsx +++ b/panel/src/components/kanban/core/kanban-board.tsx @@ -25,6 +25,17 @@ import { import { useState } from "react"; import { KanbanCard } from "./kanban-card"; import { RequiredNotesDialog } from "@/components/tasks/task-detail/task-action-dialogs"; +import { skippedPreconditions } from "./bypass-preconditions"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; type NotesActionKind = "pass-qa" | "fail-qa" | "complete"; @@ -33,6 +44,16 @@ interface PendingNotesAction { taskId: string; } +// A drag that would skip material lifecycle preconditions (completing with no +// PR, QA-bypassing, finishing docs that aren't complete, …) is held for an +// explicit override confirmation. The admin status-override is intentional +// and stays intact — this only makes the bypass visible instead of silent. +interface PendingOverride { + task: Task; + newStatus: TaskStatus; + skipped: string[]; +} + interface ColumnConfig { id: string; status: TaskStatus; @@ -67,6 +88,8 @@ export function KanbanBoard({ const [activeTask, setActiveTask] = useState(null); const [pendingNotesAction, setPendingNotesAction] = useState(null); + const [pendingOverride, setPendingOverride] = + useState(null); const [activeColumnIndex, setActiveColumnIndex] = useState(0); const sensors = useSensors( @@ -125,6 +148,19 @@ export function KanbanBoard({ } } + // A drag on this board routes the status move through the admin + // status-override, which bypasses the in-band lifecycle validator. When + // that override would skip material preconditions the panel can detect + // (no open PR, docs not complete, not self-verified, non-terminal + // subtasks, …), hold the move for an explicit confirmation that surfaces + // exactly what's being skipped. The override stays — this only makes the + // bypass visible instead of silent. + const skipped = skippedPreconditions(task, newStatus, tasks || []); + if (skipped.length > 0) { + setPendingOverride({ task, newStatus, skipped }); + return; + } + try { await updateTask.mutateAsync({ taskId, @@ -137,6 +173,23 @@ export function KanbanBoard({ } }; + const handleOverrideConfirm = async () => { + if (!pendingOverride) return; + const { task, newStatus } = pendingOverride; + try { + await updateTask.mutateAsync({ + taskId: task.id, + updates: { status: newStatus }, + }); + toast.success(`Task moved to ${newStatus.replace(/_/g, " ")}`); + refetch(); + } catch { + toast.error("Failed to move task"); + } finally { + setPendingOverride(null); + } + }; + const handleAction = async (action: string, taskId: string) => { try { switch (action) { @@ -381,6 +434,37 @@ export function KanbanBoard({ {...notesDialogConfig[pendingNotesAction.kind]} /> )} + + { + if (!open) setPendingOverride(null); + }} + > + + + + Override lifecycle preconditions? + + + This move routes through the admin status-override, which skips + the in-band lifecycle gate. The following preconditions would be + skipped: + + +
    + {pendingOverride?.skipped.map((item) => ( +
  • {item}
    • + ))} +
+ + Cancel + + Override & move + + + + ); }