feat(core): workflow-resolved transitions behind workflowColumns flag, typed rejections, default-workflow hook parity (U4)
This commit is contained in:
87
packages/core/src/__tests__/default-workflow-hooks.test.ts
Normal file
87
packages/core/src/__tests__/default-workflow-hooks.test.ts
Normal file
@@ -0,0 +1,87 @@
|
||||
// @vitest-environment node
|
||||
//
|
||||
// U4: the default-workflow side effects are resolved THROUGH the trait registry
|
||||
// (the DI seam, KTD-2/U2). This pins:
|
||||
// - registerDefaultWorkflowHooks() wires the impls so resolution finds them
|
||||
// (no missing-hook-impl warning on the happy path);
|
||||
// - a missing registration degrades to a no-op + audit warning (not a crash);
|
||||
// - applyDefaultWorkflowMoveEffects mutates the task per the legacy contract.
|
||||
|
||||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import {
|
||||
__resetTraitRegistryForTests,
|
||||
getTraitRegistry,
|
||||
} from "../trait-registry.js";
|
||||
import { registerBuiltinTraits } from "../builtin-traits.js";
|
||||
import {
|
||||
__resetDefaultWorkflowHooksForTests,
|
||||
applyDefaultWorkflowMoveEffects,
|
||||
registerDefaultWorkflowHooks,
|
||||
type DefaultWorkflowMoveContext,
|
||||
} from "../default-workflow-hooks.js";
|
||||
import type { Task } from "../types.js";
|
||||
|
||||
function makeCtx(overrides: Partial<DefaultWorkflowMoveContext> = {}): DefaultWorkflowMoveContext {
|
||||
const task = {
|
||||
id: "FN-1",
|
||||
column: "in-progress",
|
||||
columnMovedAt: new Date().toISOString(),
|
||||
steps: [],
|
||||
dependencies: [],
|
||||
} as unknown as Task;
|
||||
return {
|
||||
task,
|
||||
fromColumn: "todo",
|
||||
toColumn: "in-progress",
|
||||
moveSource: "user",
|
||||
bypassGuards: false,
|
||||
movedAt: new Date().toISOString(),
|
||||
settings: undefined,
|
||||
options: {},
|
||||
resetSteps: () => {},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("default-workflow-hooks registry wiring", () => {
|
||||
beforeEach(() => {
|
||||
__resetTraitRegistryForTests();
|
||||
__resetDefaultWorkflowHooksForTests();
|
||||
registerBuiltinTraits();
|
||||
});
|
||||
|
||||
it("resolves all default-workflow hooks without a missing-impl warning once registered", () => {
|
||||
registerDefaultWorkflowHooks();
|
||||
const ctx = makeCtx({ fromColumn: "todo", toColumn: "in-progress" });
|
||||
const { warnings } = applyDefaultWorkflowMoveEffects(ctx);
|
||||
expect(warnings).toHaveLength(0);
|
||||
// timing.onEnter stamped cumulativeActiveMs on entry to in-progress.
|
||||
expect(ctx.task.cumulativeActiveMs).toBe(0);
|
||||
});
|
||||
|
||||
it("degrades to a no-op + audit warning when a hook impl is not registered", () => {
|
||||
// Built-in DEFINITIONS are registered (so the trait declares the hook) but
|
||||
// we deliberately do NOT call registerDefaultWorkflowHooks() — no impls.
|
||||
const registry = getTraitRegistry();
|
||||
// sanity: the trait declares the hook descriptor
|
||||
expect(registry.getTrait("timing")?.hooks?.onEnter).toBe(true);
|
||||
const ctx = makeCtx({ fromColumn: "todo", toColumn: "in-progress" });
|
||||
const { warnings } = applyDefaultWorkflowMoveEffects(ctx);
|
||||
// Every declared hook with no impl yields a degraded-no-op warning.
|
||||
expect(warnings.length).toBeGreaterThan(0);
|
||||
expect(warnings.every((w) => w.kind === "missing-hook-impl")).toBe(true);
|
||||
// No crash; task unmutated by the (no-op) hooks.
|
||||
expect(ctx.task.cumulativeActiveMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("applies userPaused only for user-source reopen to todo", () => {
|
||||
registerDefaultWorkflowHooks();
|
||||
const userCtx = makeCtx({ fromColumn: "in-progress", toColumn: "todo", moveSource: "user" });
|
||||
applyDefaultWorkflowMoveEffects(userCtx);
|
||||
expect(userCtx.task.userPaused).toBe(true);
|
||||
|
||||
const engineCtx = makeCtx({ fromColumn: "in-progress", toColumn: "todo", moveSource: "engine" });
|
||||
applyDefaultWorkflowMoveEffects(engineCtx);
|
||||
expect(engineCtx.task.userPaused).toBeUndefined();
|
||||
});
|
||||
});
|
||||
205
packages/core/src/__tests__/move-task-characterization.test.ts
Normal file
205
packages/core/src/__tests__/move-task-characterization.test.ts
Normal file
@@ -0,0 +1,205 @@
|
||||
// @vitest-environment node
|
||||
//
|
||||
// CHARACTERIZATION SUITE (U4 Execution Note — written FIRST, before any change
|
||||
// to `moveTaskInternal`).
|
||||
//
|
||||
// This suite pins the CURRENT behavior of `moveTaskInternal` for every (from,
|
||||
// to) pair in VALID_TRANSITIONS' domain and both moveSource values, plus the
|
||||
// key column side effects:
|
||||
// - merge-blocker on in-review → done (user source)
|
||||
// - userPaused set only for user-source in-progress → todo
|
||||
// - reopen field/step resets on in-review/done → todo|triage
|
||||
// - autoMerge stamping on → in-review
|
||||
// - timing fields (cumulativeActiveMs / executionStartedAt) on in-progress
|
||||
//
|
||||
// It runs GREEN against the unmodified store first, then runs forever against
|
||||
// BOTH flag states (workflowColumns OFF and ON) — see the `flagStates` loop.
|
||||
// Any divergence between the two flag states is a U4 parity FAILURE.
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||
import { VALID_TRANSITIONS } from "../types.js";
|
||||
import type { Column, Task } from "../types.js";
|
||||
import { createTaskStoreTestHarness } from "./store-test-helpers.js";
|
||||
|
||||
const ALL_COLUMNS: Column[] = ["triage", "todo", "in-progress", "in-review", "done", "archived"];
|
||||
|
||||
// Flag states the characterization runs against. OFF is the legacy path; ON is
|
||||
// the workflow-resolved path. The default workflow MUST reproduce identical
|
||||
// outcomes for both, so the same expectations apply.
|
||||
const flagStates: Array<{ label: string; workflowColumns: boolean }> = [
|
||||
{ label: "flag OFF (legacy path)", workflowColumns: false },
|
||||
{ label: "flag ON (workflow-resolved default workflow)", workflowColumns: true },
|
||||
];
|
||||
|
||||
for (const flag of flagStates) {
|
||||
describe(`moveTaskInternal characterization — ${flag.label}`, () => {
|
||||
const harness = createTaskStoreTestHarness();
|
||||
let store: ReturnType<typeof harness.store>;
|
||||
|
||||
beforeEach(async () => {
|
||||
await harness.beforeEach();
|
||||
store = harness.store();
|
||||
if (flag.workflowColumns) {
|
||||
await store.updateGlobalSettings({ experimentalFeatures: { workflowColumns: true } });
|
||||
}
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await harness.afterEach();
|
||||
});
|
||||
|
||||
/**
|
||||
* Drive a freshly-created task (starts in `triage`) into `column` using only
|
||||
* legal, side-effect-tolerant moves. Returns the task.
|
||||
*/
|
||||
async function seedInColumn(column: Column): Promise<Task> {
|
||||
const task = await store.createTask({ description: `seed-${column}` });
|
||||
switch (column) {
|
||||
case "triage":
|
||||
return task;
|
||||
case "todo":
|
||||
return store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
case "in-progress":
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
return store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
case "in-review":
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
return store.moveTask(task.id, "in-review", { moveSource: "user", allowDirectInReviewMove: true });
|
||||
case "done":
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-review", { moveSource: "user", allowDirectInReviewMove: true });
|
||||
return store.moveTask(task.id, "done", { moveSource: "engine", skipMergeBlocker: true });
|
||||
case "archived":
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-review", { moveSource: "user", allowDirectInReviewMove: true });
|
||||
await store.moveTask(task.id, "done", { moveSource: "engine", skipMergeBlocker: true });
|
||||
return store.moveTask(task.id, "archived", { moveSource: "user" });
|
||||
default:
|
||||
throw new Error(`unhandled column ${column}`);
|
||||
}
|
||||
}
|
||||
|
||||
describe("transition allow/reject matrix (every from×to×moveSource)", () => {
|
||||
for (const from of ALL_COLUMNS) {
|
||||
for (const to of ALL_COLUMNS) {
|
||||
for (const moveSource of ["user", "engine"] as const) {
|
||||
const allowed = from === to || VALID_TRANSITIONS[from].includes(to);
|
||||
const label = `${from} → ${to} [${moveSource}] should ${allowed ? "ALLOW" : "REJECT"}`;
|
||||
it(label, async () => {
|
||||
const task = await seedInColumn(from);
|
||||
// Same-column move is a no-op success in legacy behavior.
|
||||
if (from === to) {
|
||||
const result = await store.moveTask(task.id, to, { moveSource });
|
||||
expect(result.column).toBe(to);
|
||||
return;
|
||||
}
|
||||
if (allowed) {
|
||||
// in-review → done with merge-blocker only blocks for user source
|
||||
// and only when a blocker exists; our seeded task has no blocker.
|
||||
// Bare in-review targets bypass the handoff invariant via
|
||||
// allowDirectInReviewMove, matching production drag behavior.
|
||||
const opts =
|
||||
to === "in-review"
|
||||
? { moveSource, allowDirectInReviewMove: true }
|
||||
: { moveSource };
|
||||
const result = await store.moveTask(task.id, to, opts);
|
||||
expect(result.column).toBe(to);
|
||||
} else {
|
||||
await expect(
|
||||
store.moveTask(task.id, to, { moveSource }),
|
||||
).rejects.toThrow(/Invalid transition/);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
describe("merge-blocker side effect (in-review → done)", () => {
|
||||
it("blocks a user move to done when a merge blocker exists", async () => {
|
||||
const task = await seedInColumn("in-review");
|
||||
// Incomplete steps create a merge blocker (getTaskMergeBlocker).
|
||||
await store.updateTask(task.id, {
|
||||
steps: [{ name: "x", status: "pending" }] as Task["steps"],
|
||||
});
|
||||
await expect(
|
||||
store.moveTask(task.id, "done", { moveSource: "user" }),
|
||||
).rejects.toThrow(/Cannot move .* to done/);
|
||||
});
|
||||
|
||||
it("skipMergeBlocker bypasses the blocker", async () => {
|
||||
const task = await seedInColumn("in-review");
|
||||
await store.updateTask(task.id, {
|
||||
steps: [{ name: "x", status: "pending" }] as Task["steps"],
|
||||
});
|
||||
const result = await store.moveTask(task.id, "done", {
|
||||
moveSource: "engine",
|
||||
skipMergeBlocker: true,
|
||||
});
|
||||
expect(result.column).toBe("done");
|
||||
});
|
||||
});
|
||||
|
||||
describe("userPaused side effect (in-progress → todo)", () => {
|
||||
it("sets userPaused for a user-source move", async () => {
|
||||
const task = await seedInColumn("in-progress");
|
||||
const result = await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
expect(result.userPaused).toBe(true);
|
||||
});
|
||||
|
||||
it("does NOT set userPaused for an engine-source move", async () => {
|
||||
const task = await seedInColumn("in-progress");
|
||||
const result = await store.moveTask(task.id, "todo", { moveSource: "engine" });
|
||||
expect(result.userPaused).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("reopen resets (in-review → todo)", () => {
|
||||
it("clears branch/summary/baseCommitSha on reopen to todo", async () => {
|
||||
const task = await seedInColumn("in-review");
|
||||
await store.updateTask(task.id, {
|
||||
branch: "fusion/fn-x",
|
||||
summary: "did stuff",
|
||||
baseCommitSha: "abc123",
|
||||
});
|
||||
const result = await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
expect(result.branch).toBeUndefined();
|
||||
expect(result.summary).toBeUndefined();
|
||||
expect(result.baseCommitSha).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("autoMerge stamping (→ in-review)", () => {
|
||||
it("stamps autoMerge from settings when undefined", async () => {
|
||||
await store.updateSettings({ autoMerge: true });
|
||||
const task = await seedInColumn("in-progress");
|
||||
const result = await store.moveTask(task.id, "in-review", {
|
||||
moveSource: "user",
|
||||
allowDirectInReviewMove: true,
|
||||
});
|
||||
expect(result.autoMerge).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("timing fields (→ in-progress)", () => {
|
||||
it("sets executionStartedAt and initializes cumulativeActiveMs on entry", async () => {
|
||||
const task = await seedInColumn("todo");
|
||||
const result = await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
expect(result.executionStartedAt).toBeTruthy();
|
||||
expect(result.cumulativeActiveMs).toBe(0);
|
||||
});
|
||||
|
||||
it("accumulates cumulativeActiveMs on exit from in-progress", async () => {
|
||||
const task = await seedInColumn("in-progress");
|
||||
const result = await store.moveTask(task.id, "in-review", {
|
||||
moveSource: "user",
|
||||
allowDirectInReviewMove: true,
|
||||
});
|
||||
expect(result.cumulativeActiveMs).toBeGreaterThanOrEqual(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
253
packages/core/src/__tests__/transition-parity.test.ts
Normal file
253
packages/core/src/__tests__/transition-parity.test.ts
Normal file
@@ -0,0 +1,253 @@
|
||||
// @vitest-environment node
|
||||
//
|
||||
// TRANSITION-PARITY SUITE (U4).
|
||||
//
|
||||
// Proves the flag-ON workflow-resolved transition path reproduces the legacy
|
||||
// VALID_TRANSITIONS contract for the default workflow, and exercises the U4
|
||||
// plan scenarios:
|
||||
// - VALID_TRANSITIONS parity (allowed AND rejected sets identical)
|
||||
// - FN-5147 terminal-until-merged (both paths)
|
||||
// - hard-cancel user vs engine (userPaused + abort-on-exit bypass)
|
||||
// - handoff bypass + exactly-once enqueue across a simulated crash
|
||||
// - crash-mid-transition marker recovery (SQLite authoritative)
|
||||
// - unknown-column rejection
|
||||
// - guard rejection typed (flag-ON) vs legacy string (flag-OFF)
|
||||
// - bypassGuards capacity pass-through (documenting; U6 fills enforcement)
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||
import { VALID_TRANSITIONS } from "../types.js";
|
||||
import type { Column, Task } from "../types.js";
|
||||
import { TransitionRejectionError } from "../store.js";
|
||||
import { resolveAllowedColumns, workflowHasColumn } from "../workflow-transitions.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
|
||||
import { readTransitionPending } from "../transition-pending.js";
|
||||
import { createTaskStoreTestHarness } from "./store-test-helpers.js";
|
||||
|
||||
const ALL_COLUMNS: Column[] = ["triage", "todo", "in-progress", "in-review", "done", "archived"];
|
||||
|
||||
describe("transition-parity — default workflow column adjacency == VALID_TRANSITIONS", () => {
|
||||
it("reproduces VALID_TRANSITIONS exactly for every column (allowed + rejected)", () => {
|
||||
for (const from of ALL_COLUMNS) {
|
||||
const legacy = new Set(VALID_TRANSITIONS[from]);
|
||||
const resolved = new Set(resolveAllowedColumns(BUILTIN_CODING_WORKFLOW_IR, from));
|
||||
// Allowed sets identical.
|
||||
expect([...resolved].sort()).toEqual([...legacy].sort());
|
||||
// Rejected sets identical (complement over all columns).
|
||||
for (const to of ALL_COLUMNS) {
|
||||
if (from === to) continue;
|
||||
expect(resolved.has(to)).toBe(legacy.has(to));
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("recognizes exactly the six default columns", () => {
|
||||
for (const c of ALL_COLUMNS) {
|
||||
expect(workflowHasColumn(BUILTIN_CODING_WORKFLOW_IR, c)).toBe(true);
|
||||
}
|
||||
expect(workflowHasColumn(BUILTIN_CODING_WORKFLOW_IR, "made-up")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("transition-parity — store flag-ON scenarios", () => {
|
||||
const harness = createTaskStoreTestHarness();
|
||||
let store: ReturnType<typeof harness.store>;
|
||||
|
||||
beforeEach(async () => {
|
||||
await harness.beforeEach();
|
||||
store = harness.store();
|
||||
await store.updateGlobalSettings({ experimentalFeatures: { workflowColumns: true } });
|
||||
});
|
||||
afterEach(async () => {
|
||||
await harness.afterEach();
|
||||
});
|
||||
|
||||
async function seedInColumn(column: Column): Promise<Task> {
|
||||
const task = await store.createTask({ description: `seed-${column}` });
|
||||
const u = { moveSource: "user" as const };
|
||||
if (column === "triage") return task;
|
||||
await store.moveTask(task.id, "todo", u);
|
||||
if (column === "todo") return store.getTask(task.id) as Promise<Task>;
|
||||
await store.moveTask(task.id, "in-progress", u);
|
||||
if (column === "in-progress") return store.getTask(task.id) as Promise<Task>;
|
||||
await store.moveTask(task.id, "in-review", { ...u, allowDirectInReviewMove: true });
|
||||
if (column === "in-review") return store.getTask(task.id) as Promise<Task>;
|
||||
await store.moveTask(task.id, "done", { moveSource: "engine", skipMergeBlocker: true });
|
||||
if (column === "done") return store.getTask(task.id) as Promise<Task>;
|
||||
await store.moveTask(task.id, "archived", u);
|
||||
return store.getTask(task.id) as Promise<Task>;
|
||||
}
|
||||
|
||||
it("FN-5147: user move in-review → done blocked by merge-blocker with typed rejection", async () => {
|
||||
const task = await seedInColumn("in-review");
|
||||
await store.updateTask(task.id, { steps: [{ name: "x", status: "pending" }] as Task["steps"] });
|
||||
let caught: unknown;
|
||||
try {
|
||||
await store.moveTask(task.id, "done", { moveSource: "user" });
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(TransitionRejectionError);
|
||||
expect((caught as TransitionRejectionError).rejection.code).toBe("merge-blocked");
|
||||
expect((caught as TransitionRejectionError).rejection.retryable).toBe(true);
|
||||
});
|
||||
|
||||
it("FN-5147: engine-sourced move bypasses the merge-blocker guard", async () => {
|
||||
const task = await seedInColumn("in-review");
|
||||
await store.updateTask(task.id, { steps: [{ name: "x", status: "pending" }] as Task["steps"] });
|
||||
const moved = await store.moveTask(task.id, "done", { moveSource: "engine" });
|
||||
expect(moved.column).toBe("done");
|
||||
});
|
||||
|
||||
it("hard-cancel: user in-progress → todo sets userPaused; engine does not", async () => {
|
||||
const userTask = await seedInColumn("in-progress");
|
||||
const u = await store.moveTask(userTask.id, "todo", { moveSource: "user" });
|
||||
expect(u.userPaused).toBe(true);
|
||||
|
||||
const engineTask = await seedInColumn("in-progress");
|
||||
const e = await store.moveTask(engineTask.id, "todo", { moveSource: "engine" });
|
||||
expect(e.userPaused).toBeUndefined();
|
||||
});
|
||||
|
||||
it("unknown column rejects with typed unknown-column code, card untouched", async () => {
|
||||
const task = await seedInColumn("todo");
|
||||
let caught: unknown;
|
||||
try {
|
||||
await store.moveTask(task.id, "made-up" as Column, { moveSource: "user" });
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(TransitionRejectionError);
|
||||
expect((caught as TransitionRejectionError).rejection.code).toBe("unknown-column");
|
||||
const after = await store.getTask(task.id);
|
||||
expect(after?.column).toBe("todo");
|
||||
});
|
||||
|
||||
it("guard/adjacency rejection is typed (not a bare Error string)", async () => {
|
||||
const task = await seedInColumn("archived");
|
||||
// archived → todo is not a legal default-workflow transition.
|
||||
let caught: unknown;
|
||||
try {
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(TransitionRejectionError);
|
||||
expect((caught as TransitionRejectionError).rejection.code).toBe("guard-rejected");
|
||||
});
|
||||
|
||||
it("handoffToReview maps skipMergeBlocker onto bypassGuards and enqueues exactly once", async () => {
|
||||
const task = await seedInColumn("in-progress");
|
||||
await store.handoffToReview(task.id, {
|
||||
ownerAgentId: "agent-1",
|
||||
evidence: { runId: "run-1", agentId: "agent-1", reason: "complete" },
|
||||
} as Parameters<typeof store.handoffToReview>[1]);
|
||||
const after = await store.getTask(task.id);
|
||||
expect(after?.column).toBe("in-review");
|
||||
// Idempotent re-handoff (same-column path) must not double-enqueue.
|
||||
await store.handoffToReview(task.id, {
|
||||
ownerAgentId: "agent-1",
|
||||
evidence: { runId: "run-2", agentId: "agent-1", reason: "complete" },
|
||||
} as Parameters<typeof store.handoffToReview>[1]);
|
||||
const queueCount = (store as unknown as { db: { prepare: (s: string) => { get: (...a: unknown[]) => unknown } } }).db
|
||||
.prepare("SELECT COUNT(*) AS n FROM mergeQueue WHERE taskId = ?")
|
||||
.get(task.id) as { n: number };
|
||||
expect(queueCount.n).toBe(1);
|
||||
});
|
||||
|
||||
it("transitionPending marker is written in-txn and cleared post-commit (happy path)", async () => {
|
||||
const task = await seedInColumn("todo");
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
const db = (store as unknown as { db: Parameters<typeof readTransitionPending>[0] }).db;
|
||||
// Happy path: marker cleared after the post-commit hook runner.
|
||||
expect(readTransitionPending(db, task.id)).toBeNull();
|
||||
});
|
||||
|
||||
it("crash-mid-transition: a persisted marker is recoverable from SQLite with hooksRemaining intact", async () => {
|
||||
const task = await seedInColumn("todo");
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
// Simulate a crash AFTER commit but BEFORE the marker clear by re-writing a
|
||||
// marker directly (the in-txn write path is the same helper). Recovery reads
|
||||
// it back from SQLite (authoritative), not from task.json.
|
||||
const db = (store as unknown as { db: Parameters<typeof readTransitionPending>[0] }).db;
|
||||
(db as unknown as { prepare: (s: string) => { run: (...a: unknown[]) => unknown } })
|
||||
.prepare("UPDATE tasks SET transitionPending = ? WHERE id = ?")
|
||||
.run(
|
||||
JSON.stringify({ toColumn: "in-progress", hooksRemaining: ["default-workflow:postCommit"], startedAt: Date.now() }),
|
||||
task.id,
|
||||
);
|
||||
const pending = readTransitionPending(db, task.id);
|
||||
expect(pending).not.toBeNull();
|
||||
expect(pending?.toColumn).toBe("in-progress");
|
||||
expect(pending?.hooksRemaining).toContain("default-workflow:postCommit");
|
||||
});
|
||||
|
||||
it("worktree ordering: allocateWorktree runs (and is applied) for a flag-ON move into in-progress", async () => {
|
||||
const task = await seedInColumn("todo");
|
||||
let allocatorCalled = false;
|
||||
const moved = await store.moveTask(task.id, "in-progress", {
|
||||
moveSource: "user",
|
||||
allocateWorktree: () => {
|
||||
allocatorCalled = true;
|
||||
return "/tmp/wt/seed-todo";
|
||||
},
|
||||
});
|
||||
expect(allocatorCalled).toBe(true);
|
||||
expect(moved.worktree).toBe("/tmp/wt/seed-todo");
|
||||
// Worktree allocation is NOT a hook — it is a substrate capability invoked
|
||||
// synchronously before the move commits; the committed row carries it.
|
||||
const after = await store.getTask(task.id);
|
||||
expect(after?.worktree).toBe("/tmp/wt/seed-todo");
|
||||
});
|
||||
|
||||
it("bypassGuards capacity pass-through (U4 documenting test): engine move into in-progress is NOT blocked by capacity (U6 fills enforcement)", async () => {
|
||||
// U4 intentionally leaves the per-(workflow,column) capacity check as a
|
||||
// pass-through slot; capacity enforcement lands in U6. This test pins the
|
||||
// U4 contract: no WIP-constrained scenario is enforced yet, and an engine
|
||||
// move (bypassGuards) into a wip-flagged column commits. It must be UPDATED
|
||||
// by U6 (capacity is NEVER bypassable, KTD-10) — not silently left green.
|
||||
const t1 = await seedInColumn("todo");
|
||||
const t2 = await seedInColumn("todo");
|
||||
const m1 = await store.moveTask(t1.id, "in-progress", { moveSource: "engine" });
|
||||
const m2 = await store.moveTask(t2.id, "in-progress", { moveSource: "engine" });
|
||||
expect(m1.column).toBe("in-progress");
|
||||
expect(m2.column).toBe("in-progress");
|
||||
});
|
||||
});
|
||||
|
||||
describe("transition-parity — flag-OFF keeps legacy thrown strings (no behavior change)", () => {
|
||||
const harness = createTaskStoreTestHarness();
|
||||
let store: ReturnType<typeof harness.store>;
|
||||
beforeEach(async () => {
|
||||
await harness.beforeEach();
|
||||
store = harness.store();
|
||||
});
|
||||
afterEach(async () => {
|
||||
await harness.afterEach();
|
||||
});
|
||||
|
||||
it("rejects an illegal move with a bare Error containing the legacy message (not TransitionRejectionError)", async () => {
|
||||
const task = await store.createTask({ description: "legacy reject" });
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-review", { moveSource: "user", allowDirectInReviewMove: true });
|
||||
await store.moveTask(task.id, "done", { moveSource: "engine", skipMergeBlocker: true });
|
||||
await store.moveTask(task.id, "archived", { moveSource: "user" });
|
||||
let caught: unknown;
|
||||
try {
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
expect(caught).not.toBeInstanceOf(TransitionRejectionError);
|
||||
expect((caught as Error).message).toMatch(/Invalid transition/);
|
||||
});
|
||||
|
||||
it("flag-OFF does NOT write a transitionPending marker", async () => {
|
||||
const task = await store.createTask({ description: "no marker" });
|
||||
await store.moveTask(task.id, "todo", { moveSource: "user" });
|
||||
await store.moveTask(task.id, "in-progress", { moveSource: "user" });
|
||||
const db = (store as unknown as { db: Parameters<typeof readTransitionPending>[0] }).db;
|
||||
expect(readTransitionPending(db, task.id)).toBeNull();
|
||||
});
|
||||
});
|
||||
313
packages/core/src/default-workflow-hooks.ts
Normal file
313
packages/core/src/default-workflow-hooks.ts
Normal file
@@ -0,0 +1,313 @@
|
||||
/**
|
||||
* Default-workflow trait hook implementations (U4).
|
||||
*
|
||||
* The legacy per-column side effects of `moveTaskInternal` — timing /
|
||||
* `cumulativeActiveMs` accounting, reopen field/step resets, autoMerge stamping
|
||||
* + merge-queue enqueue, and abort-on-exit (hard-cancel incl. `userPaused` only
|
||||
* for user-source moves) — become the default workflow's trait hook
|
||||
* implementations, registered through U2's DI seam (`registerTraitHookImpl`).
|
||||
*
|
||||
* IMPORTANT (per U4): this is the FLAG-ON path. The legacy inline code in
|
||||
* `store.ts` is NOT deleted — it IS the flag-off path. The implementations here
|
||||
* are a deliberate parallel of that inline logic so the two paths can be parity-
|
||||
* checked against each other; "moved, not duplicated" applies to the flag-ON
|
||||
* path only.
|
||||
*
|
||||
* Hook classes (KTD-2):
|
||||
* - guard (sync, in-lock): merge-blocker, human-review. Implemented as the
|
||||
* `evaluateDefaultWorkflowGuards` reader; pure DB-free reads off the task.
|
||||
* - onEnter / onExit (mutating, applied in-lock to the in-memory task before
|
||||
* the commit for field effects; queue effects run in-txn): timing,
|
||||
* reset-on-entry, abort-on-exit, merge.
|
||||
*
|
||||
* Worktree allocation is explicitly NOT a hook (it stays a substrate capability
|
||||
* invoked before the move; see store.ts) — there is no `allocateWorktree` hook
|
||||
* here by design.
|
||||
*
|
||||
* The hooks are registered into the shared trait registry on `init` via
|
||||
* `registerDefaultWorkflowHooks()` (idempotent). They are resolved through
|
||||
* `getTraitRegistry().resolveTraitHook(...)` so a missing registration degrades
|
||||
* to a no-op + audit warning rather than crashing.
|
||||
*/
|
||||
|
||||
import { getTraitRegistry } from "./trait-registry.js";
|
||||
import type { TraitAuditWarning } from "./trait-registry.js";
|
||||
import { getTaskMergeBlocker } from "./task-merge.js";
|
||||
import type { Settings, Task } from "./types.js";
|
||||
|
||||
// ── Guard evaluation (sync, in-lock) ─────────────────────────────────────────
|
||||
|
||||
/** A guard verdict: undefined = allow; a string reason = reject. */
|
||||
export type GuardVerdict = string | undefined;
|
||||
|
||||
/**
|
||||
* Evaluate the default workflow's sync guards for a move. Reproduces the legacy
|
||||
* `getTaskMergeBlocker` gate on `in-review → done`. (The default workflow does
|
||||
* not carry the human-review trait — see the Trait Vocabulary note — so there
|
||||
* is no human-review guard on this workflow.)
|
||||
*
|
||||
* `bypassGuards` (engine-sourced moves, KTD-9) skips guards entirely — the
|
||||
* caller is responsible for honoring that; this function still computes the
|
||||
* verdict so callers can choose. The store only consults it when not bypassing.
|
||||
*/
|
||||
export function evaluateMergeBlockerGuard(
|
||||
task: Pick<Task, "column" | "paused" | "status" | "error" | "steps" | "workflowStepResults">,
|
||||
fromColumn: string,
|
||||
toColumn: string,
|
||||
): GuardVerdict {
|
||||
if (fromColumn === "in-review" && toColumn === "done") {
|
||||
return getTaskMergeBlocker(task);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// ── Move-effect context ───────────────────────────────────────────────────────
|
||||
|
||||
/** Side-effect callbacks the store provides so the hooks stay engine-free and
|
||||
* DB-handle-free; the store wires these to its in-txn / post-commit machinery. */
|
||||
export interface DefaultWorkflowMoveContext {
|
||||
task: Task;
|
||||
fromColumn: string;
|
||||
toColumn: string;
|
||||
moveSource: "user" | "engine";
|
||||
/** True when guards + abort-on-exit are bypassed (engine/recovery, KTD-9). */
|
||||
bypassGuards: boolean;
|
||||
movedAt: string;
|
||||
/** Settings snapshot for autoMerge stamping (only read when entering review). */
|
||||
settings: Pick<Settings, "autoMerge"> | undefined;
|
||||
/** Move options that influence reopen/timing semantics. */
|
||||
options: {
|
||||
preserveStatus?: boolean;
|
||||
preserveResumeState?: boolean;
|
||||
preserveProgress?: boolean;
|
||||
preserveWorktree?: boolean;
|
||||
};
|
||||
/** Reset all steps to pending + currentStep 0 (store owns the impl). */
|
||||
resetSteps: () => void;
|
||||
}
|
||||
|
||||
// ── Field-mutation effects (applied in-lock, before commit) ───────────────────
|
||||
//
|
||||
// These mirror the inline flag-off mutations in store.ts exactly. They run as
|
||||
// the resolved onEnter/onExit hook bodies for the default workflow's traits.
|
||||
|
||||
/** `timing` trait (in-progress): accumulate active ms on exit, stamp timing on
|
||||
* entry. */
|
||||
export function applyTimingEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, fromColumn, toColumn } = ctx;
|
||||
if (fromColumn === "in-progress" && toColumn !== "in-progress") {
|
||||
const segmentStartMs = Date.parse(task.executionStartedAt ?? task.columnMovedAt ?? ctx.movedAt);
|
||||
const segmentEndMs = Date.parse(task.columnMovedAt ?? ctx.movedAt);
|
||||
const segmentDeltaMs =
|
||||
Number.isFinite(segmentStartMs) && Number.isFinite(segmentEndMs)
|
||||
? Math.max(0, segmentEndMs - segmentStartMs)
|
||||
: 0;
|
||||
task.cumulativeActiveMs = Math.max(0, task.cumulativeActiveMs ?? 0) + segmentDeltaMs;
|
||||
}
|
||||
if (toColumn === "in-progress") {
|
||||
task.cumulativeActiveMs ??= 0;
|
||||
if (!task.firstExecutionAt) task.firstExecutionAt = task.columnMovedAt;
|
||||
if (!task.executionStartedAt) task.executionStartedAt = task.columnMovedAt;
|
||||
task.userPaused = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/** Stamp `executionCompletedAt` on entry to a completion column. */
|
||||
export function applyCompletionTimingEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, toColumn } = ctx;
|
||||
if (toColumn === "done" && !task.executionCompletedAt) {
|
||||
task.executionCompletedAt = task.columnMovedAt;
|
||||
}
|
||||
}
|
||||
|
||||
/** `reset-on-entry` trait (todo/triage reopen) + `abort-on-exit` userPaused
|
||||
* semantics. Reproduces the legacy reopen block. */
|
||||
export function applyResetOnEntryEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, fromColumn, toColumn, moveSource, options } = ctx;
|
||||
const isReopenToTodoOrTriage =
|
||||
(fromColumn === "in-progress" || fromColumn === "done" || fromColumn === "in-review") &&
|
||||
(toColumn === "todo" || toColumn === "triage");
|
||||
if (!isReopenToTodoOrTriage) return;
|
||||
|
||||
if (!options.preserveStatus) {
|
||||
task.status = undefined;
|
||||
task.error = undefined;
|
||||
task.pausedReason = undefined;
|
||||
}
|
||||
task.blockedBy = undefined;
|
||||
task.overlapBlockedBy = undefined;
|
||||
task.paused = undefined;
|
||||
task.pausedByAgentId = undefined;
|
||||
// abort-on-exit userPaused: only for user-source moves to todo (KTD-9).
|
||||
if (moveSource === "user" && toColumn === "todo") {
|
||||
task.userPaused = true;
|
||||
} else {
|
||||
task.userPaused = undefined;
|
||||
}
|
||||
|
||||
const hasNonPendingStepProgress = task.steps.some((step) => step.status !== "pending");
|
||||
const preserveStepProgress =
|
||||
options.preserveResumeState || (options.preserveProgress === true && hasNonPendingStepProgress);
|
||||
|
||||
if (!options.preserveWorktree) {
|
||||
task.worktree = undefined;
|
||||
}
|
||||
if (!options.preserveResumeState) {
|
||||
task.executionStartedAt = undefined;
|
||||
task.executionCompletedAt = undefined;
|
||||
} else {
|
||||
task.executionCompletedAt = undefined;
|
||||
}
|
||||
if (!preserveStepProgress) {
|
||||
ctx.resetSteps();
|
||||
// Prompt-checkbox reset is a filesystem effect; the store performs it
|
||||
// post-hook (it owns the task dir). Not modeled here.
|
||||
}
|
||||
}
|
||||
|
||||
/** `merge` trait onEnter (in-review): autoMerge stamping + scheduler-state
|
||||
* clearing. The queue enqueue itself is in-txn and store-owned (handoff path);
|
||||
* the field effects mirror the legacy in-review block. */
|
||||
export function applyInReviewEnterEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, toColumn, settings } = ctx;
|
||||
if (toColumn !== "in-review") return;
|
||||
if (task.autoMerge === undefined && settings) {
|
||||
task.autoMerge = settings.autoMerge;
|
||||
}
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
if (task.status === "queued") {
|
||||
task.status = undefined;
|
||||
}
|
||||
task.blockedBy = undefined;
|
||||
task.overlapBlockedBy = undefined;
|
||||
}
|
||||
|
||||
/** Reopen-from-review/done field clears (branch/summary/workflowStepResults). */
|
||||
export function applyReopenFieldClears(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, fromColumn, toColumn } = ctx;
|
||||
if (
|
||||
(fromColumn === "in-review" && (toColumn === "todo" || toColumn === "in-progress" || toColumn === "triage")) ||
|
||||
(fromColumn === "done" && (toColumn === "todo" || toColumn === "triage"))
|
||||
) {
|
||||
task.workflowStepResults = undefined;
|
||||
}
|
||||
if (fromColumn === "in-review" && (toColumn === "todo" || toColumn === "triage")) {
|
||||
task.branch = undefined;
|
||||
task.executionStartBranch = undefined;
|
||||
task.baseCommitSha = undefined;
|
||||
task.summary = undefined;
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply ALL default-workflow field-mutation move effects (the parallel of the
|
||||
* legacy inline block) in the legacy order. Pure in-memory mutation of
|
||||
* `ctx.task`; queue/filesystem/post-commit effects remain store-owned.
|
||||
*
|
||||
* This is the entry point the flag-ON store path calls. It resolves each
|
||||
* trait's hook through the registry first (so a missing registration degrades to
|
||||
* a no-op + audit warning, satisfying the "invokes through the registry"
|
||||
* contract and the degraded-hook path); resolution warnings are collected and
|
||||
* returned for the store to forward to audit.
|
||||
*/
|
||||
export function applyDefaultWorkflowMoveEffects(
|
||||
ctx: DefaultWorkflowMoveContext,
|
||||
): { warnings: TraitAuditWarning[] } {
|
||||
const registry = getTraitRegistry();
|
||||
const warnings: TraitAuditWarning[] = [];
|
||||
|
||||
// Resolve the hooks through the registry. The resolved impls are the closures
|
||||
// registered by registerDefaultWorkflowHooks(); resolution surfaces a warning
|
||||
// (and a no-op) if a registration is missing.
|
||||
const toRun: Array<{ traitId: string; hookKind: "onEnter" | "onExit" }> = [
|
||||
{ traitId: "timing", hookKind: "onExit" },
|
||||
{ traitId: "timing", hookKind: "onEnter" },
|
||||
{ traitId: "reset-on-entry", hookKind: "onEnter" },
|
||||
{ traitId: "abort-on-exit", hookKind: "onExit" },
|
||||
{ traitId: "merge", hookKind: "onEnter" },
|
||||
];
|
||||
for (const { traitId, hookKind } of toRun) {
|
||||
const { impl, warning } = registry.resolveTraitHook(traitId, hookKind);
|
||||
if (warning) warnings.push(warning);
|
||||
if (impl) impl(ctx);
|
||||
}
|
||||
|
||||
return { warnings };
|
||||
}
|
||||
|
||||
// ── Registration into the trait registry (DI seam) ───────────────────────────
|
||||
|
||||
let registered = false;
|
||||
|
||||
/**
|
||||
* Register the default-workflow hook implementations into the shared trait
|
||||
* registry. Idempotent. Called at store init (the store is the engine-adjacent
|
||||
* owner of the move lifecycle). Each registration is a thin adapter that runs
|
||||
* the corresponding field-effect function over the move context.
|
||||
*
|
||||
* The legacy effects map onto traits as:
|
||||
* timing.onExit / timing.onEnter → applyTimingEffects + completion stamp
|
||||
* reset-on-entry.onEnter → applyResetOnEntryEffects + reopen clears
|
||||
* abort-on-exit.onExit → (userPaused handled in reset-on-entry;
|
||||
* session abort is an engine effect U6/U7)
|
||||
* merge.onEnter → applyInReviewEnterEffects
|
||||
*/
|
||||
export function registerDefaultWorkflowHooks(): void {
|
||||
if (registered) return;
|
||||
const registry = getTraitRegistry();
|
||||
|
||||
const cast = (fn: (ctx: DefaultWorkflowMoveContext) => void) =>
|
||||
((...args: unknown[]) => fn(args[0] as DefaultWorkflowMoveContext)) as (
|
||||
...args: unknown[]
|
||||
) => unknown;
|
||||
|
||||
registry.registerTraitHookImpl(
|
||||
"timing",
|
||||
"onExit",
|
||||
cast((ctx) => {
|
||||
applyTimingEffects(ctx);
|
||||
}),
|
||||
);
|
||||
registry.registerTraitHookImpl(
|
||||
"timing",
|
||||
"onEnter",
|
||||
cast((ctx) => {
|
||||
applyCompletionTimingEffects(ctx);
|
||||
}),
|
||||
);
|
||||
registry.registerTraitHookImpl(
|
||||
"reset-on-entry",
|
||||
"onEnter",
|
||||
cast((ctx) => {
|
||||
applyResetOnEntryEffects(ctx);
|
||||
applyReopenFieldClears(ctx);
|
||||
}),
|
||||
);
|
||||
registry.registerTraitHookImpl(
|
||||
"abort-on-exit",
|
||||
"onExit",
|
||||
cast(() => {
|
||||
// userPaused is set in applyResetOnEntryEffects (the legacy ordering keeps
|
||||
// it with the reopen block). Session-abort wiring is an engine effect that
|
||||
// lands with U6/U7; here it is intentionally a no-op so the resolved hook
|
||||
// exists (not a missing-impl warning) while carrying no field mutation.
|
||||
}),
|
||||
);
|
||||
registry.registerTraitHookImpl(
|
||||
"merge",
|
||||
"onEnter",
|
||||
cast((ctx) => {
|
||||
applyInReviewEnterEffects(ctx);
|
||||
}),
|
||||
);
|
||||
|
||||
registered = true;
|
||||
}
|
||||
|
||||
/** Test-only: allow re-registration after a registry reset. */
|
||||
export function __resetDefaultWorkflowHooksForTests(): void {
|
||||
registered = false;
|
||||
}
|
||||
@@ -124,6 +124,14 @@ export type {
|
||||
TransitionPendingDbHandle,
|
||||
ReconcileHooksResult,
|
||||
} from "./transition-pending.js";
|
||||
// ── U4: workflow-resolved transition adjacency + flag accessor ───────────────
|
||||
export {
|
||||
resolveColumnAdjacency,
|
||||
resolveAllowedColumns,
|
||||
workflowHasColumn,
|
||||
} from "./workflow-transitions.js";
|
||||
export type { ColumnAdjacency } from "./workflow-transitions.js";
|
||||
export { isWorkflowColumnsEnabled } from "./workflow-columns-settings.js";
|
||||
export {
|
||||
readTransitionPending,
|
||||
writeTransitionPending,
|
||||
@@ -259,6 +267,7 @@ export {
|
||||
MergeQueueLeaseOwnershipError,
|
||||
InvalidMergeQueueLeaseDurationError,
|
||||
HandoffInvariantViolationError,
|
||||
TransitionRejectionError,
|
||||
} from "./store.js";
|
||||
export {
|
||||
STOPWORDS,
|
||||
|
||||
@@ -8,6 +8,25 @@ import { createActivityLogSnapshot, createRunAuditSnapshot, createTaskMetadataSn
|
||||
import { VALID_TRANSITIONS, DEFAULT_SETTINGS, isGlobalOnlySettingsKey, WORKFLOW_STEP_TEMPLATES, validateDocumentKey } from "./types.js";
|
||||
import { DEFAULT_PROJECT_SETTINGS } from "./settings-schema.js";
|
||||
import { parseWorkflowIr, serializeWorkflowIr } from "./workflow-ir.js";
|
||||
import { isWorkflowColumnsEnabled } from "./workflow-columns-settings.js";
|
||||
import { resolveAllowedColumns, workflowHasColumn } from "./workflow-transitions.js";
|
||||
import {
|
||||
type DefaultWorkflowMoveContext,
|
||||
applyDefaultWorkflowMoveEffects,
|
||||
evaluateMergeBlockerGuard,
|
||||
registerDefaultWorkflowHooks,
|
||||
} from "./default-workflow-hooks.js";
|
||||
import {
|
||||
type TransitionRejection,
|
||||
makeTransitionRejection,
|
||||
makeTransitionPending,
|
||||
} from "./transition-types.js";
|
||||
import { writeTransitionPending, clearTransitionPending } from "./transition-pending.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js";
|
||||
import type { WorkflowIr } from "./workflow-ir-types.js";
|
||||
// Side-effect import: registers the 14 built-in trait DEFINITIONS into the
|
||||
// shared trait registry on load (the flag-ON path resolves traits by id).
|
||||
import "./builtin-traits.js";
|
||||
import type {
|
||||
WorkflowDefinition,
|
||||
WorkflowDefinitionInput,
|
||||
@@ -1047,6 +1066,28 @@ export class HandoffInvariantViolationError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Thrown by the flag-ON (`workflowColumns`) `moveTaskInternal` path when a move
|
||||
* is rejected, carrying the typed {@link TransitionRejection} (KTD-3/R13). The
|
||||
* existing callers of `moveTask` catch thrown `Error`s (e.g. the dashboard move
|
||||
* route inspects `err.message`), so the rejection rides on an `Error` subclass
|
||||
* — `.message` reproduces the legacy human-readable string so flag-ON callers
|
||||
* that only read the message keep working, while `.rejection` exposes the
|
||||
* machine-stable code/messageKey/retryable for surfaces that want it.
|
||||
*
|
||||
* The FLAG-OFF path still throws the bare legacy `Error` strings unchanged
|
||||
* (zero behavior change while the flag is off — proven by the characterization
|
||||
* suite).
|
||||
*/
|
||||
export class TransitionRejectionError extends Error {
|
||||
readonly rejection: TransitionRejection;
|
||||
constructor(rejection: TransitionRejection, message: string) {
|
||||
super(message);
|
||||
this.name = "TransitionRejectionError";
|
||||
this.rejection = rejection;
|
||||
}
|
||||
}
|
||||
|
||||
interface MoveTaskOptions {
|
||||
preserveResumeState?: boolean;
|
||||
preserveProgress?: boolean;
|
||||
@@ -1056,6 +1097,15 @@ interface MoveTaskOptions {
|
||||
moveSource?: "user" | "engine";
|
||||
skipMergeBlocker?: boolean;
|
||||
allowDirectInReviewMove?: boolean;
|
||||
/**
|
||||
* KTD-9: engine/recovery moves bypass trait guards and abort-on-exit effects
|
||||
* (the generalization of `skipMergeBlocker`). It NEVER bypasses capacity
|
||||
* (KTD-10). Engine-internal only: HTTP move endpoints hardcode it off and must
|
||||
* never forward a caller-supplied value (mirrors the hardcoded
|
||||
* `moveSource: "user"` posture). When unset, the flag-ON path derives it from
|
||||
* `moveSource === "engine"` plus `skipMergeBlocker`.
|
||||
*/
|
||||
bypassGuards?: boolean;
|
||||
}
|
||||
|
||||
interface MoveTaskInternalOptions {
|
||||
@@ -1395,7 +1445,14 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
async init(): Promise<void> {
|
||||
await mkdir(this.tasksDir, { recursive: true });
|
||||
|
||||
|
||||
// U4: register the default-workflow trait hook implementations into the
|
||||
// shared trait registry (the flag-ON moveTaskInternal path resolves the
|
||||
// legacy per-column effects through these). Idempotent; built-in trait
|
||||
// DEFINITIONS self-register on import of ./builtin-traits.js (pulled in
|
||||
// transitively via default-workflow-hooks / trait-registry).
|
||||
registerDefaultWorkflowHooks();
|
||||
|
||||
// Initialize SQLite database
|
||||
if (!this._db) {
|
||||
// Startup corruption guard: before opening, detect a malformed fusion.db
|
||||
@@ -5533,6 +5590,9 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
{
|
||||
...opts.moveOptions,
|
||||
skipMergeBlocker: true,
|
||||
// KTD-9: handoff is an engine/recovery-class move; its skipMergeBlocker
|
||||
// maps onto bypassGuards under the flag (identical behavior both paths).
|
||||
bypassGuards: true,
|
||||
},
|
||||
{
|
||||
fromHandoff: true,
|
||||
@@ -5560,6 +5620,27 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
const task = currentTask ?? await this.readTaskForMove(id);
|
||||
const moveSource = options?.moveSource ?? "engine";
|
||||
|
||||
// ── U4: flag-gated workflow-resolved transition path (KTD-8) ─────────────
|
||||
// Flag OFF (default): the legacy `VALID_TRANSITIONS` / inline-side-effect
|
||||
// path below runs byte-identical (proven by the characterization suite).
|
||||
// Flag ON: validate against the task's resolved workflow column graph, run
|
||||
// sync trait guards (unless bypassed), and route the legacy per-column side
|
||||
// effects through the default-workflow trait hooks.
|
||||
// `experimentalFeatures` is a global-scoped setting, so the project-only
|
||||
// `getSettingsSync()` row would miss it — read merged settings (global +
|
||||
// project) via getSettingsFast(). This is an async read taken before the
|
||||
// lock-sensitive transaction; it does not touch the task lock.
|
||||
const useWorkflow = isWorkflowColumnsEnabled(await this.getSettingsFast());
|
||||
// bypassGuards (KTD-9): engine-sourced moves + the existing skipMergeBlocker
|
||||
// call sites map onto it. Capacity (KTD-10) is NEVER bypassed by this — the
|
||||
// capacity check is not a guard (U6 fills the enforcement; U4 leaves a
|
||||
// pass-through slot). An explicit option value wins; otherwise derive it.
|
||||
const bypassGuards =
|
||||
options?.bypassGuards ?? (moveSource === "engine" || options?.skipMergeBlocker === true);
|
||||
const workflowIr: WorkflowIr | undefined = useWorkflow
|
||||
? this.resolveTaskWorkflowIrSync(id)
|
||||
: undefined;
|
||||
|
||||
if (task.column === toColumn) {
|
||||
if (internal.fromHandoff && toColumn === "in-review") {
|
||||
this.db.transactionImmediate(() => {
|
||||
@@ -5616,19 +5697,70 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
return task;
|
||||
}
|
||||
|
||||
const validTargets = VALID_TRANSITIONS[task.column];
|
||||
if (!validTargets.includes(toColumn)) {
|
||||
throw new Error(
|
||||
`Invalid transition: '${task.column}' → '${toColumn}'. ` +
|
||||
`Valid targets: ${validTargets.join(", ") || "none"}`,
|
||||
);
|
||||
}
|
||||
|
||||
const fromColumn = task.column;
|
||||
if (fromColumn === "in-review" && toColumn === "done" && !options?.skipMergeBlocker) {
|
||||
const mergeBlocker = getTaskMergeBlocker(task);
|
||||
if (mergeBlocker) {
|
||||
throw new Error(`Cannot move ${id} to done: ${mergeBlocker}`);
|
||||
|
||||
if (useWorkflow && workflowIr) {
|
||||
// ── Flag-ON validation + sync guards (typed rejections, KTD-3/R13) ─────
|
||||
// 1. Target column must exist in the task's workflow → unknown-column.
|
||||
if (!workflowHasColumn(workflowIr, toColumn)) {
|
||||
throw new TransitionRejectionError(
|
||||
makeTransitionRejection(
|
||||
"unknown-column",
|
||||
"transition.rejected.unknownColumn",
|
||||
false,
|
||||
`Column '${toColumn}' is not defined in this task's workflow`,
|
||||
),
|
||||
`Invalid transition: '${fromColumn}' → '${toColumn}'. Unknown column for this workflow.`,
|
||||
);
|
||||
}
|
||||
// 2. Column-graph adjacency. For the default workflow this reproduces
|
||||
// VALID_TRANSITIONS verbatim (resolveAllowedColumns); the
|
||||
// transition-parity suite machine-checks the equivalence.
|
||||
const allowed = resolveAllowedColumns(workflowIr, fromColumn);
|
||||
if (!allowed.includes(toColumn)) {
|
||||
throw new TransitionRejectionError(
|
||||
makeTransitionRejection(
|
||||
"guard-rejected",
|
||||
"transition.rejected.invalidTransition",
|
||||
false,
|
||||
`Valid targets: ${allowed.join(", ") || "none"}`,
|
||||
),
|
||||
`Invalid transition: '${fromColumn}' → '${toColumn}'. ` +
|
||||
`Valid targets: ${allowed.join(", ") || "none"}`,
|
||||
);
|
||||
}
|
||||
// 3. Sync trait guards (in-lock). Skipped entirely when bypassGuards
|
||||
// (engine/recovery moves, KTD-9). The default workflow's merge-blocker
|
||||
// trait reads the same getTaskMergeBlocker.
|
||||
if (!bypassGuards) {
|
||||
const guardReason = evaluateMergeBlockerGuard(task, fromColumn, toColumn);
|
||||
if (guardReason) {
|
||||
throw new TransitionRejectionError(
|
||||
makeTransitionRejection(
|
||||
"merge-blocked",
|
||||
"transition.rejected.mergeBlocked",
|
||||
true,
|
||||
guardReason,
|
||||
),
|
||||
`Cannot move ${id} to done: ${guardReason}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// ── Flag-OFF legacy path (unchanged) ───────────────────────────────────
|
||||
const validTargets = VALID_TRANSITIONS[task.column];
|
||||
if (!validTargets.includes(toColumn)) {
|
||||
throw new Error(
|
||||
`Invalid transition: '${task.column}' → '${toColumn}'. ` +
|
||||
`Valid targets: ${validTargets.join(", ") || "none"}`,
|
||||
);
|
||||
}
|
||||
|
||||
if (fromColumn === "in-review" && toColumn === "done" && !options?.skipMergeBlocker) {
|
||||
const mergeBlocker = getTaskMergeBlocker(task);
|
||||
if (mergeBlocker) {
|
||||
throw new Error(`Cannot move ${id} to done: ${mergeBlocker}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5642,106 +5774,153 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
task.columnMovedAt = movedAt;
|
||||
task.updatedAt = movedAt;
|
||||
|
||||
if (fromColumn === "in-progress" && toColumn !== "in-progress") {
|
||||
const segmentStartMs = Date.parse(task.executionStartedAt ?? task.columnMovedAt);
|
||||
const segmentEndMs = Date.parse(task.columnMovedAt);
|
||||
const segmentDeltaMs =
|
||||
Number.isFinite(segmentStartMs) && Number.isFinite(segmentEndMs)
|
||||
? Math.max(0, segmentEndMs - segmentStartMs)
|
||||
: 0;
|
||||
task.cumulativeActiveMs = Math.max(0, task.cumulativeActiveMs ?? 0) + segmentDeltaMs;
|
||||
}
|
||||
|
||||
if (toColumn === "in-progress") {
|
||||
task.cumulativeActiveMs ??= 0;
|
||||
if (!task.firstExecutionAt) {
|
||||
task.firstExecutionAt = task.columnMovedAt;
|
||||
}
|
||||
if (!task.executionStartedAt) {
|
||||
task.executionStartedAt = task.columnMovedAt;
|
||||
}
|
||||
task.userPaused = undefined;
|
||||
}
|
||||
if (toColumn === "done" && !task.executionCompletedAt) {
|
||||
task.executionCompletedAt = task.columnMovedAt;
|
||||
}
|
||||
|
||||
if (toColumn === "done") {
|
||||
this.clearDoneTransientFields(task);
|
||||
}
|
||||
|
||||
const isReopenToTodoOrTriage =
|
||||
(fromColumn === "in-progress" || fromColumn === "done" || fromColumn === "in-review")
|
||||
&& (toColumn === "todo" || toColumn === "triage");
|
||||
|
||||
if (isReopenToTodoOrTriage) {
|
||||
if (!options?.preserveStatus) {
|
||||
task.status = undefined;
|
||||
task.error = undefined;
|
||||
task.pausedReason = undefined;
|
||||
}
|
||||
task.blockedBy = undefined;
|
||||
task.overlapBlockedBy = undefined;
|
||||
task.paused = undefined;
|
||||
task.pausedByAgentId = undefined;
|
||||
if (moveSource === "user" && toColumn === "todo") {
|
||||
task.userPaused = true;
|
||||
} else {
|
||||
task.userPaused = undefined;
|
||||
}
|
||||
|
||||
if (useWorkflow) {
|
||||
// ── Flag-ON: route the legacy per-column side effects through the
|
||||
// default-workflow trait hooks (timing, reset-on-entry, abort-on-exit,
|
||||
// merge.onEnter). "Moved, not duplicated" applies to this path; the
|
||||
// flag-off branch below keeps the legacy inline code verbatim. ───────
|
||||
const ctx: DefaultWorkflowMoveContext = {
|
||||
task,
|
||||
fromColumn,
|
||||
toColumn,
|
||||
moveSource,
|
||||
bypassGuards,
|
||||
movedAt,
|
||||
settings: settingsForInReview,
|
||||
options: {
|
||||
preserveStatus: options?.preserveStatus,
|
||||
preserveResumeState: options?.preserveResumeState,
|
||||
preserveProgress: options?.preserveProgress,
|
||||
preserveWorktree: options?.preserveWorktree,
|
||||
},
|
||||
resetSteps: () => this.resetAllStepsToPending(task),
|
||||
};
|
||||
const isReopenToTodoOrTriage =
|
||||
(fromColumn === "in-progress" || fromColumn === "done" || fromColumn === "in-review") &&
|
||||
(toColumn === "todo" || toColumn === "triage");
|
||||
const hasNonPendingStepProgress = task.steps.some((step) => step.status !== "pending");
|
||||
const preserveStepProgress =
|
||||
options?.preserveResumeState || (options?.preserveProgress === true && hasNonPendingStepProgress);
|
||||
|
||||
if (!options?.preserveWorktree) {
|
||||
task.worktree = undefined;
|
||||
options?.preserveResumeState ||
|
||||
(options?.preserveProgress === true && hasNonPendingStepProgress);
|
||||
const { warnings } = applyDefaultWorkflowMoveEffects(ctx);
|
||||
for (const warning of warnings) {
|
||||
storeLog.warn("Default-workflow trait hook degraded to no-op", {
|
||||
phase: "moveTaskInternal:workflow-hooks",
|
||||
taskId: id,
|
||||
...warning,
|
||||
});
|
||||
}
|
||||
|
||||
if (!options?.preserveResumeState) {
|
||||
task.executionStartedAt = undefined;
|
||||
task.executionCompletedAt = undefined;
|
||||
} else {
|
||||
task.executionCompletedAt = undefined;
|
||||
// Store-owned effects the hooks intentionally do NOT perform (filesystem /
|
||||
// store-private): clearing done transient fields + prompt-checkbox reset.
|
||||
if (toColumn === "done") {
|
||||
this.clearDoneTransientFields(task);
|
||||
}
|
||||
|
||||
if (!preserveStepProgress) {
|
||||
this.resetAllStepsToPending(task);
|
||||
if (isReopenToTodoOrTriage && !preserveStepProgress) {
|
||||
await this.resetPromptCheckboxes(dir);
|
||||
}
|
||||
}
|
||||
|
||||
if (toColumn === "in-review") {
|
||||
if (task.autoMerge === undefined && settingsForInReview) {
|
||||
task.autoMerge = settingsForInReview.autoMerge;
|
||||
} else {
|
||||
// ── Flag-OFF legacy inline side effects (UNCHANGED — the flag-off path) ──
|
||||
if (fromColumn === "in-progress" && toColumn !== "in-progress") {
|
||||
const segmentStartMs = Date.parse(task.executionStartedAt ?? task.columnMovedAt);
|
||||
const segmentEndMs = Date.parse(task.columnMovedAt);
|
||||
const segmentDeltaMs =
|
||||
Number.isFinite(segmentStartMs) && Number.isFinite(segmentEndMs)
|
||||
? Math.max(0, segmentEndMs - segmentStartMs)
|
||||
: 0;
|
||||
task.cumulativeActiveMs = Math.max(0, task.cumulativeActiveMs ?? 0) + segmentDeltaMs;
|
||||
}
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
// Clear scheduler-side dispatch state: `queued`, `blockedBy`, and
|
||||
// `overlapBlockedBy` are stamped while the task waits in `todo`. If
|
||||
// they survive the transition into `in-review` they permanently block
|
||||
// the merge gate (see getTaskMergeBlocker's BLOCKING_TASK_STATUSES).
|
||||
if (task.status === "queued") {
|
||||
task.status = undefined;
|
||||
|
||||
if (toColumn === "in-progress") {
|
||||
task.cumulativeActiveMs ??= 0;
|
||||
if (!task.firstExecutionAt) {
|
||||
task.firstExecutionAt = task.columnMovedAt;
|
||||
}
|
||||
if (!task.executionStartedAt) {
|
||||
task.executionStartedAt = task.columnMovedAt;
|
||||
}
|
||||
task.userPaused = undefined;
|
||||
}
|
||||
if (toColumn === "done" && !task.executionCompletedAt) {
|
||||
task.executionCompletedAt = task.columnMovedAt;
|
||||
}
|
||||
task.blockedBy = undefined;
|
||||
task.overlapBlockedBy = undefined;
|
||||
}
|
||||
|
||||
if (
|
||||
(fromColumn === "in-review" && (toColumn === "todo" || toColumn === "in-progress" || toColumn === "triage"))
|
||||
|| (fromColumn === "done" && (toColumn === "todo" || toColumn === "triage"))
|
||||
) {
|
||||
task.workflowStepResults = undefined;
|
||||
}
|
||||
if (toColumn === "done") {
|
||||
this.clearDoneTransientFields(task);
|
||||
}
|
||||
|
||||
if (fromColumn === "in-review" && (toColumn === "todo" || toColumn === "triage")) {
|
||||
task.branch = undefined;
|
||||
task.executionStartBranch = undefined;
|
||||
task.baseCommitSha = undefined;
|
||||
task.summary = undefined;
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
const isReopenToTodoOrTriage =
|
||||
(fromColumn === "in-progress" || fromColumn === "done" || fromColumn === "in-review")
|
||||
&& (toColumn === "todo" || toColumn === "triage");
|
||||
|
||||
if (isReopenToTodoOrTriage) {
|
||||
if (!options?.preserveStatus) {
|
||||
task.status = undefined;
|
||||
task.error = undefined;
|
||||
task.pausedReason = undefined;
|
||||
}
|
||||
task.blockedBy = undefined;
|
||||
task.overlapBlockedBy = undefined;
|
||||
task.paused = undefined;
|
||||
task.pausedByAgentId = undefined;
|
||||
if (moveSource === "user" && toColumn === "todo") {
|
||||
task.userPaused = true;
|
||||
} else {
|
||||
task.userPaused = undefined;
|
||||
}
|
||||
|
||||
const hasNonPendingStepProgress = task.steps.some((step) => step.status !== "pending");
|
||||
const preserveStepProgress =
|
||||
options?.preserveResumeState || (options?.preserveProgress === true && hasNonPendingStepProgress);
|
||||
|
||||
if (!options?.preserveWorktree) {
|
||||
task.worktree = undefined;
|
||||
}
|
||||
|
||||
if (!options?.preserveResumeState) {
|
||||
task.executionStartedAt = undefined;
|
||||
task.executionCompletedAt = undefined;
|
||||
} else {
|
||||
task.executionCompletedAt = undefined;
|
||||
}
|
||||
|
||||
if (!preserveStepProgress) {
|
||||
this.resetAllStepsToPending(task);
|
||||
await this.resetPromptCheckboxes(dir);
|
||||
}
|
||||
}
|
||||
|
||||
if (toColumn === "in-review") {
|
||||
if (task.autoMerge === undefined && settingsForInReview) {
|
||||
task.autoMerge = settingsForInReview.autoMerge;
|
||||
}
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
// Clear scheduler-side dispatch state: `queued`, `blockedBy`, and
|
||||
// `overlapBlockedBy` are stamped while the task waits in `todo`. If
|
||||
// they survive the transition into `in-review` they permanently block
|
||||
// the merge gate (see getTaskMergeBlocker's BLOCKING_TASK_STATUSES).
|
||||
if (task.status === "queued") {
|
||||
task.status = undefined;
|
||||
}
|
||||
task.blockedBy = undefined;
|
||||
task.overlapBlockedBy = undefined;
|
||||
}
|
||||
|
||||
if (
|
||||
(fromColumn === "in-review" && (toColumn === "todo" || toColumn === "in-progress" || toColumn === "triage"))
|
||||
|| (fromColumn === "done" && (toColumn === "todo" || toColumn === "triage"))
|
||||
) {
|
||||
task.workflowStepResults = undefined;
|
||||
}
|
||||
|
||||
if (fromColumn === "in-review" && (toColumn === "todo" || toColumn === "triage")) {
|
||||
task.branch = undefined;
|
||||
task.executionStartBranch = undefined;
|
||||
task.baseCommitSha = undefined;
|
||||
task.summary = undefined;
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
if (toColumn === "in-progress" && !task.worktree && options?.allocateWorktree) {
|
||||
@@ -5785,6 +5964,22 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
});
|
||||
this.dequeueMergeQueueOnColumnExit(id, fromColumn, toColumn, movedAt);
|
||||
|
||||
// U4 (flag-ON): write the crash-safe transitionPending marker in the SAME
|
||||
// transaction as the column change (KTD-2). It records the post-commit
|
||||
// hooks that still owe idempotent execution so a crash mid-transition is
|
||||
// recoverable from SQLite (the authoritative store, ADR-0001). The store
|
||||
// clears it immediately after the post-commit hook runner completes
|
||||
// (below). For the default workflow the field effects already applied
|
||||
// in-lock; the marker guards the post-commit completion so recovery never
|
||||
// double-runs (idempotent) and never strands the card.
|
||||
if (useWorkflow) {
|
||||
writeTransitionPending(
|
||||
this.db,
|
||||
id,
|
||||
makeTransitionPending(toColumn, ["default-workflow:postCommit"], Date.parse(movedAt) || Date.now()),
|
||||
);
|
||||
}
|
||||
|
||||
if (toColumn === "in-review" && !internal.fromHandoff && options?.allowDirectInReviewMove !== true) {
|
||||
this.insertRunAuditEventRow({
|
||||
taskId: id,
|
||||
@@ -5859,6 +6054,23 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
if (this.isWatching) this.taskCache.set(id, { ...task });
|
||||
|
||||
// U4 (flag-ON): post-commit hook completion. The default-workflow field
|
||||
// effects already ran in-lock and committed; the post-commit phase here is
|
||||
// the fire-and-forget hook runner per KTD-2. It is idempotent and clears the
|
||||
// transitionPending marker once done. A crash before this point leaves the
|
||||
// marker for the recovery sweep to re-run (re-running is a no-op for the
|
||||
// default workflow's already-committed field effects). We clear it
|
||||
// synchronously here because the default workflow has no async post-commit
|
||||
// hook bodies in U4 (merge enqueue is in-txn via the handoff path);
|
||||
// plugin/async post-commit hooks land in U7/U8 and will defer the clear.
|
||||
if (useWorkflow) {
|
||||
try {
|
||||
clearTransitionPending(this.db, id);
|
||||
} catch {
|
||||
// Clearing is best-effort; the marker recovery sweep is the backstop.
|
||||
}
|
||||
}
|
||||
|
||||
if (fromColumn !== toColumn) {
|
||||
this.emit("task:moved", { task, from: fromColumn, to: toColumn, source: moveSource });
|
||||
}
|
||||
@@ -11436,6 +11648,36 @@ ${stepsSection}`;
|
||||
}
|
||||
|
||||
/** Read the workflow currently selected for a task, if any. */
|
||||
/**
|
||||
* Synchronously resolve the parsed WorkflowIr that governs a task's columns
|
||||
* (U4, flag-ON path). Resolution order:
|
||||
* 1. the task's workflow selection (side table) → that workflow's IR;
|
||||
* 2. null/missing selection → the built-in default workflow IR (KTD-1).
|
||||
* Built-in workflow IRs are resolved from the parsed module constant; custom
|
||||
* workflows are read + parsed from the `workflows` row. Pure DB read, safe to
|
||||
* call inside `withTaskLock` (no further locks taken). A parse failure or
|
||||
* missing custom row falls back to the default workflow so a move is never
|
||||
* stranded by a corrupt definition (degraded, not crashed).
|
||||
*/
|
||||
private resolveTaskWorkflowIrSync(taskId: string): WorkflowIr {
|
||||
const selection = this.getTaskWorkflowSelection(taskId);
|
||||
const workflowId = selection?.workflowId;
|
||||
if (!workflowId) return BUILTIN_CODING_WORKFLOW_IR;
|
||||
if (isBuiltinWorkflowId(workflowId)) {
|
||||
const builtin = getBuiltinWorkflow(workflowId);
|
||||
return builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
|
||||
}
|
||||
try {
|
||||
const row = this.db
|
||||
.prepare("SELECT ir FROM workflows WHERE id = ?")
|
||||
.get(workflowId) as { ir: string } | undefined;
|
||||
if (!row) return BUILTIN_CODING_WORKFLOW_IR;
|
||||
return parseWorkflowIr(row.ir);
|
||||
} catch {
|
||||
return BUILTIN_CODING_WORKFLOW_IR;
|
||||
}
|
||||
}
|
||||
|
||||
getTaskWorkflowSelection(taskId: string): { workflowId: string; stepIds: string[] } | undefined {
|
||||
const row = this.db
|
||||
.prepare("SELECT workflowId, stepIds FROM task_workflow_selection WHERE taskId = ?")
|
||||
|
||||
18
packages/core/src/workflow-columns-settings.ts
Normal file
18
packages/core/src/workflow-columns-settings.ts
Normal file
@@ -0,0 +1,18 @@
|
||||
import { isExperimentalFeatureEnabled } from "./experimental-features.js";
|
||||
import type { Settings } from "./types.js";
|
||||
|
||||
/**
|
||||
* The `experimentalFeatures.workflowColumns` flag (KTD-8). OFF: the legacy
|
||||
* enum/`VALID_TRANSITIONS` path runs untouched. ON: `moveTaskInternal` resolves
|
||||
* each task's workflow column graph + trait guards. Default OFF until the
|
||||
* transition-parity suite and field observations prove zero drift (U12).
|
||||
*
|
||||
* Mirrors `isSandboxExperimentalEnabled` / `isEvalsViewEnabled` — a thin,
|
||||
* named accessor over the shared experimental-features map so the literal flag
|
||||
* key lives in exactly one place.
|
||||
*/
|
||||
export function isWorkflowColumnsEnabled(
|
||||
settings: Pick<Settings, "experimentalFeatures"> | undefined,
|
||||
): boolean {
|
||||
return isExperimentalFeatureEnabled(settings, "workflowColumns");
|
||||
}
|
||||
108
packages/core/src/workflow-transitions.ts
Normal file
108
packages/core/src/workflow-transitions.ts
Normal file
@@ -0,0 +1,108 @@
|
||||
/**
|
||||
* Workflow-resolved transition adjacency (U4, R4/R9/R13).
|
||||
*
|
||||
* `moveTaskInternal` (flag ON) and `board.ts` both derive "which columns can a
|
||||
* card move to from here" from the SAME helper so the two surfaces never
|
||||
* diverge — `resolveAllowedColumns(ir, fromColumn)`.
|
||||
*
|
||||
* ── Why an explicit adjacency, not pure graph-derivation ──────────────────────
|
||||
*
|
||||
* The plan asks: derive allowed column adjacency from node placement + edges,
|
||||
* and for the DEFAULT workflow it MUST reproduce `VALID_TRANSITIONS` exactly.
|
||||
* Pure graph-edge derivation CANNOT reproduce it: `VALID_TRANSITIONS` encodes
|
||||
* backward/reopen edges (in-review → todo, done → todo, archived → done, …) and
|
||||
* cross edges (in-progress → done) that have no counterpart in the linear
|
||||
* execute → review → merge → end pipeline graph. The IR edges describe the
|
||||
* forward automation walk; the column adjacency describes legal *board* moves
|
||||
* (drags, reopens, recovery), which is a strictly larger, partly-cyclic set.
|
||||
*
|
||||
* So per the plan's documented fallback we attach an explicit per-column
|
||||
* `transitions` adjacency:
|
||||
* - For the BUILT-IN default workflow we reproduce `VALID_TRANSITIONS` verbatim
|
||||
* (keyed by the legacy column ids, which are exactly the default workflow's
|
||||
* column ids — KTD-1). This is the parity contract the transition-parity
|
||||
* suite machine-checks.
|
||||
* - For CUSTOM workflows (no explicit adjacency authored yet — authoring lands
|
||||
* with the editor in U10) we derive a linear forward+back adjacency from the
|
||||
* declared column ORDER: each column may move to its neighbors (prev/next).
|
||||
* This is a safe, predictable default that keeps every column reachable and
|
||||
* never strands a card; richer custom adjacency is future work.
|
||||
*
|
||||
* The adjacency is intentionally a column→columns map computed once per IR; it
|
||||
* is read-only and pure.
|
||||
*/
|
||||
|
||||
import { VALID_TRANSITIONS } from "./types.js";
|
||||
import type { Column } from "./types.js";
|
||||
import type { WorkflowIr, WorkflowIrV2 } from "./workflow-ir-types.js";
|
||||
import { DEFAULT_WORKFLOW_COLUMN_IDS } from "./workflow-ir.js";
|
||||
|
||||
/** A column→allowed-target-columns adjacency map. */
|
||||
export type ColumnAdjacency = Map<string, string[]>;
|
||||
|
||||
/** True when the IR's columns are exactly the legacy default-workflow column ids
|
||||
* (same set), i.e. this is the built-in default workflow (or an equivalent). */
|
||||
function isDefaultWorkflowColumns(ir: WorkflowIrV2): boolean {
|
||||
const ids = ir.columns.map((c) => c.id);
|
||||
if (ids.length !== DEFAULT_WORKFLOW_COLUMN_IDS.length) return false;
|
||||
const set = new Set(ids);
|
||||
return DEFAULT_WORKFLOW_COLUMN_IDS.every((id) => set.has(id));
|
||||
}
|
||||
|
||||
/** Build the verbatim `VALID_TRANSITIONS` adjacency keyed by column id. */
|
||||
function defaultWorkflowAdjacency(): ColumnAdjacency {
|
||||
const adj: ColumnAdjacency = new Map();
|
||||
for (const [from, targets] of Object.entries(VALID_TRANSITIONS) as [Column, Column[]][]) {
|
||||
adj.set(from, [...targets]);
|
||||
}
|
||||
return adj;
|
||||
}
|
||||
|
||||
/** Derive a neighbor (prev/next by declared order) adjacency for a custom
|
||||
* workflow. Each column can move to the column before and after it in the
|
||||
* authored order. Endpoints have a single neighbor. */
|
||||
function orderDerivedAdjacency(ir: WorkflowIrV2): ColumnAdjacency {
|
||||
const adj: ColumnAdjacency = new Map();
|
||||
const ids = ir.columns.map((c) => c.id);
|
||||
for (let i = 0; i < ids.length; i++) {
|
||||
const targets: string[] = [];
|
||||
if (i > 0) targets.push(ids[i - 1]);
|
||||
if (i < ids.length - 1) targets.push(ids[i + 1]);
|
||||
adj.set(ids[i], targets);
|
||||
}
|
||||
return adj;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the full column adjacency for a workflow IR. The default workflow
|
||||
* reproduces `VALID_TRANSITIONS` exactly; custom workflows use order-derived
|
||||
* neighbor adjacency.
|
||||
*/
|
||||
export function resolveColumnAdjacency(ir: WorkflowIr): ColumnAdjacency {
|
||||
// v1 IR is upgraded to v2 on parse, but accept either defensively.
|
||||
const v2 = ir as WorkflowIrV2;
|
||||
if (!Array.isArray(v2.columns)) {
|
||||
// No columns (shouldn't happen post-parse) → empty adjacency.
|
||||
return new Map();
|
||||
}
|
||||
if (isDefaultWorkflowColumns(v2)) {
|
||||
return defaultWorkflowAdjacency();
|
||||
}
|
||||
return orderDerivedAdjacency(v2);
|
||||
}
|
||||
|
||||
/**
|
||||
* The allowed target columns for a move out of `fromColumn` under this workflow.
|
||||
* Returns an empty array when `fromColumn` is unknown to the workflow (callers
|
||||
* should first check column existence to distinguish "unknown column" from "no
|
||||
* legal targets").
|
||||
*/
|
||||
export function resolveAllowedColumns(ir: WorkflowIr, fromColumn: string): string[] {
|
||||
return resolveColumnAdjacency(ir).get(fromColumn) ?? [];
|
||||
}
|
||||
|
||||
/** True when `toColumn` is a defined column of the workflow. */
|
||||
export function workflowHasColumn(ir: WorkflowIr, columnId: string): boolean {
|
||||
const v2 = ir as WorkflowIrV2;
|
||||
return Array.isArray(v2.columns) && v2.columns.some((c) => c.id === columnId);
|
||||
}
|
||||
@@ -0,0 +1,52 @@
|
||||
// @vitest-environment node
|
||||
//
|
||||
// U4 hardening: `bypassGuards` is engine-internal (KTD-9). The HTTP move
|
||||
// endpoint hardcodes its move options (mirroring the hardcoded
|
||||
// `moveSource: "user"` posture) and must NEVER forward a caller-supplied
|
||||
// `bypassGuards` (or `moveSource`) from the request body — otherwise a remote
|
||||
// caller could bypass trait guards / abort-on-exit.
|
||||
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import express from "express";
|
||||
import type { TaskStore } from "@fusion/core";
|
||||
import { createApiRoutes } from "../../routes.js";
|
||||
import { request as REQUEST } from "../../test-request.js";
|
||||
|
||||
describe("task move route — bypassGuards is not forwardable", () => {
|
||||
it("ignores a caller-supplied bypassGuards/moveSource in the request body", async () => {
|
||||
const moveTask = vi.fn(async (_id: string, column: string, _options?: Record<string, unknown>) => ({
|
||||
id: "FN-001",
|
||||
column,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
}));
|
||||
|
||||
const store: TaskStore = {
|
||||
getRootDir: vi.fn(() => process.cwd()),
|
||||
getTask: vi.fn(async () => ({ id: "FN-001", column: "todo" })),
|
||||
getSettings: vi.fn(async () => ({})),
|
||||
moveTask,
|
||||
} as unknown as TaskStore;
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
|
||||
const res = await REQUEST(
|
||||
app,
|
||||
"POST",
|
||||
"/api/tasks/FN-001/move",
|
||||
JSON.stringify({ column: "triage", bypassGuards: true, moveSource: "engine" }),
|
||||
{ "content-type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(moveTask).toHaveBeenCalledTimes(1);
|
||||
const passedOptions = moveTask.mock.calls[0][2] as Record<string, unknown> | undefined;
|
||||
// The route constructs its own options; the injected fields must not leak.
|
||||
expect(passedOptions?.bypassGuards).toBeUndefined();
|
||||
// The route hardcodes moveSource: "user" — the body's "engine" is ignored.
|
||||
expect(passedOptions?.moveSource).toBe("user");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user