diff --git a/packages/core/src/__tests__/task-fields.test.ts b/packages/core/src/__tests__/task-fields.test.ts new file mode 100644 index 0000000000..a35e0c855d --- /dev/null +++ b/packages/core/src/__tests__/task-fields.test.ts @@ -0,0 +1,455 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import { + validateCustomFieldPatch, + applyFieldDefaults, + reconcileFieldsOnWorkflowChange, +} from "../task-fields.js"; +import type { WorkflowFieldDefinition, WorkflowIr } from "../workflow-ir-types.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +/** + * U11 / KTD-13 — custom task fields: validation authority, defaults, + * reconciliation, and the store-level write authority. + * + * The pure functions in task-fields.ts are the single validation core; the + * store delegates to them for updateTask/updateTaskCustomFields and for + * workflow-switch / definition-edit reconciliation. These tests cover both. + */ + +// ── Field-definition fixtures ──────────────────────────────────────────────── + +const F = (over: Partial & { id: string; type: WorkflowFieldDefinition["type"] }): WorkflowFieldDefinition => ({ + name: over.id, + ...over, +}); + +const enumOpts = [ + { value: "high", label: "High" }, + { value: "low", label: "Low" }, +]; + +const ALL_TYPES: WorkflowFieldDefinition[] = [ + F({ id: "s", type: "string" }), + F({ id: "tx", type: "text" }), + F({ id: "n", type: "number" }), + F({ id: "b", type: "boolean" }), + F({ id: "e", type: "enum", options: enumOpts }), + F({ id: "m", type: "multi-enum", options: enumOpts }), + F({ id: "d", type: "date" }), + F({ id: "u", type: "url" }), +]; + +// ── Pure validation: every type ────────────────────────────────────────────── + +describe("validateCustomFieldPatch — per-type validate/reject", () => { + it("string/text accept strings, reject non-strings", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { s: "hi", tx: "yo" }).ok).toBe(true); + const r = validateCustomFieldPatch(ALL_TYPES, { s: 5 }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); + + it("number accepts finite numbers, rejects NaN/Infinity/non-number", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { n: 3 }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { n: 0 }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { n: Number.NaN }).ok).toBe(false); + expect(validateCustomFieldPatch(ALL_TYPES, { n: Number.POSITIVE_INFINITY }).ok).toBe(false); + expect(validateCustomFieldPatch(ALL_TYPES, { n: "3" }).ok).toBe(false); + }); + + it("boolean accepts booleans only", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { b: true }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { b: "true" }).ok).toBe(false); + }); + + it("date accepts parseable ISO strings, rejects garbage", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { d: "2026-06-04" }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { d: "2026-06-04T12:00:00Z" }).ok).toBe(true); + expect(validateCustomFieldPatch(ALL_TYPES, { d: "not-a-date" }).ok).toBe(false); + expect(validateCustomFieldPatch(ALL_TYPES, { d: 20260604 }).ok).toBe(false); + }); + + it("url accepts URL-parseable strings, rejects bad", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { u: "https://example.com/x" }).ok).toBe(true); + const r = validateCustomFieldPatch(ALL_TYPES, { u: "not a url" }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); +}); + +describe("validateCustomFieldPatch — enum membership", () => { + it("accepts a declared option, rejects a non-member with enum-violation", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { e: "high" }).ok).toBe(true); + const r = validateCustomFieldPatch(ALL_TYPES, { e: "medium" }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.rejection.code).toBe("enum-violation"); + expect(r.rejection.fieldId).toBe("e"); + } + }); + it("rejects a non-string enum value with type-mismatch", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { e: 1 }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); +}); + +describe("validateCustomFieldPatch — multi-enum subsets + dupes", () => { + it("accepts a subset of options", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: ["high"] }); + expect(r.ok).toBe(true); + if (r.ok) expect(r.normalized.m).toEqual(["high"]); + }); + it("accepts the empty array", () => { + expect(validateCustomFieldPatch(ALL_TYPES, { m: [] }).ok).toBe(true); + }); + it("rejects a non-member with enum-violation", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: ["high", "medium"] }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("enum-violation"); + }); + it("rejects duplicate members", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: ["high", "high"] }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("enum-violation"); + }); + it("rejects a non-array", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { m: "high" }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("type-mismatch"); + }); +}); + +describe("validateCustomFieldPatch — unknown field & no-fields", () => { + it("rejects a patch key naming no declared field", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { nope: 1 }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.rejection.code).toBe("unknown-field"); + expect(r.rejection.fieldId).toBe("nope"); + } + }); + it("rejects any non-empty patch when no fields are defined (no-fields-defined)", () => { + const r = validateCustomFieldPatch(undefined, { anything: 1 }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.rejection.code).toBe("no-fields-defined"); + const r2 = validateCustomFieldPatch([], { x: 1 }); + expect(r2.ok).toBe(false); + if (!r2.ok) expect(r2.rejection.code).toBe("no-fields-defined"); + }); + it("accepts an EMPTY patch even with no fields defined", () => { + expect(validateCustomFieldPatch(undefined, {}).ok).toBe(true); + expect(validateCustomFieldPatch([], {}).ok).toBe(true); + }); + it("treats null/undefined patch values as delete sentinels (normalized to null)", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { s: null, n: undefined }); + expect(r.ok).toBe(true); + if (r.ok) expect(r.normalized).toEqual({ s: null, n: null }); + }); +}); + +// ── Defaults ────────────────────────────────────────────────────────────── + +describe("applyFieldDefaults", () => { + const fields: WorkflowFieldDefinition[] = [ + F({ id: "req", type: "string", required: true, default: "x" }), + F({ id: "reqNoDefault", type: "string", required: true }), + F({ id: "optDefault", type: "number", default: 7 }), + ]; + it("fills required field defaults absent from current", () => { + expect(applyFieldDefaults(fields, {})).toEqual({ req: "x" }); + }); + it("does not override an existing value", () => { + expect(applyFieldDefaults(fields, { req: "kept" })).toEqual({ req: "kept" }); + }); + it("ignores non-required defaults and required-without-default", () => { + const out = applyFieldDefaults(fields, {}); + expect(out).not.toHaveProperty("optDefault"); + expect(out).not.toHaveProperty("reqNoDefault"); + }); +}); + +// ── Reconciliation ────────────────────────────────────────────────────────── + +describe("reconcileFieldsOnWorkflowChange", () => { + it("keeps same-id type-compatible values, orphans removed ids", () => { + const oldF = [F({ id: "a", type: "string" }), F({ id: "gone", type: "number" })]; + const newF = [F({ id: "a", type: "string" })]; + const { kept, orphaned } = reconcileFieldsOnWorkflowChange(oldF, newF, { a: "v", gone: 1 }); + expect(kept).toEqual({ a: "v" }); + expect(orphaned).toEqual({ gone: 1 }); + }); + + it("orphans a value when the new type is incompatible", () => { + const oldF = [F({ id: "a", type: "string" })]; + const newF = [F({ id: "a", type: "number" })]; + const { kept, orphaned } = reconcileFieldsOnWorkflowChange(oldF, newF, { a: "still-a-string" }); + expect(kept).toEqual({}); + expect(orphaned).toEqual({ a: "still-a-string" }); + }); + + it("keeps an enum value still in the new options, orphans one no longer present", () => { + const oldF = [F({ id: "e", type: "enum", options: enumOpts })]; + const newF = [F({ id: "e", type: "enum", options: [{ value: "high", label: "H" }] })]; + expect(reconcileFieldsOnWorkflowChange(oldF, newF, { e: "high" }).kept).toEqual({ e: "high" }); + expect(reconcileFieldsOnWorkflowChange(oldF, newF, { e: "low" }).orphaned).toEqual({ e: "low" }); + }); +}); + +// ── Store authority integration ────────────────────────────────────────────── + +describe("store: updateTaskCustomFields + updateTask integration (U11)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name = "wf"): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + async function taskWithFields(fields: WorkflowFieldDefinition[]) { + const def = await (store as any).createWorkflowDefinition({ name: "WF", ir: irWith(fields) }); + const t = await store.createTask({ description: "field task" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + return { task: t, workflowId: def.id as string }; + } + + it("happy path: validates, merges, persists, returns ok", async () => { + const { task } = await taskWithFields([ + F({ id: "sev", type: "enum", options: enumOpts }), + F({ id: "pts", type: "number" }), + ]); + const r = await (store as any).updateTaskCustomFields(task.id, { sev: "high", pts: 5 }); + expect(r.ok).toBe(true); + const got = await store.getTask(task.id); + expect(got?.customFields).toEqual({ sev: "high", pts: 5 }); + }); + + it("reject path: returns a typed rejection, does not mutate", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + const r = await (store as any).updateTaskCustomFields(task.id, { pts: "not-a-number" }); + expect(r.ok).toBe(false); + expect(r.rejection.code).toBe("type-mismatch"); + expect(r.rejection.fieldId).toBe("pts"); + const got = await store.getTask(task.id); + expect(got?.customFields).toEqual({}); + }); + + it("unknown-field rejection on an undeclared key", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + const r = await (store as any).updateTaskCustomFields(task.id, { nope: 1 }); + expect(r.ok).toBe(false); + expect(r.rejection.code).toBe("unknown-field"); + }); + + it("default workflow (zero fields) rejects cleanly with no-fields-defined", async () => { + const t = await store.createTask({ description: "default wf" }); + const r = await (store as any).updateTaskCustomFields(t.id, { anything: 1 }); + expect(r.ok).toBe(false); + expect(r.rejection.code).toBe("no-fields-defined"); + }); + + it("emits task:updated on a successful write", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + let emitted = 0; + (store as any).on("task:updated", () => { + emitted += 1; + }); + const r = await (store as any).updateTaskCustomFields(task.id, { pts: 1 }); + expect(r.ok).toBe(true); + expect(emitted).toBeGreaterThanOrEqual(1); + }); + + it("null patch value deletes the stored value", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" }), F({ id: "x", type: "number" })]); + await (store as any).updateTaskCustomFields(task.id, { pts: 1, x: 2 }); + await (store as any).updateTaskCustomFields(task.id, { pts: null }); + const got = await store.getTask(task.id); + expect(got?.customFields).toEqual({ x: 2 }); + }); + + it("updateTask with an invalid customFields patch throws CustomFieldRejectionError", async () => { + const { task } = await taskWithFields([F({ id: "pts", type: "number" })]); + await expect(store.updateTask(task.id, { customFields: { pts: "bad" } })).rejects.toThrow(/pts/); + }); + + it("applies required+default fields at workflow selection", async () => { + const def = await (store as any).createWorkflowDefinition({ + name: "Defaults", + ir: irWith([F({ id: "tier", type: "string", required: true, default: "bronze" })]), + }); + const t = await store.createTask({ description: "defaults" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ tier: "bronze" }); + }); +}); + +describe("store: workflow switch reconciliation (U11)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name: string): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + it("keeps same-id compatible values and orphans the rest (orphan-not-delete)", async () => { + const wfA = await (store as any).createWorkflowDefinition({ + name: "A", + ir: irWith([F({ id: "shared", type: "string" }), F({ id: "onlyA", type: "number" })], "A"), + }); + const wfB = await (store as any).createWorkflowDefinition({ + name: "B", + ir: irWith([F({ id: "shared", type: "string" }), F({ id: "onlyB", type: "boolean" })], "B"), + }); + const t = await store.createTask({ description: "switch" }); + await (store as any).selectTaskWorkflow(t.id, wfA.id); + await (store as any).updateTaskCustomFields(t.id, { shared: "v", onlyA: 3 }); + + await (store as any).selectTaskWorkflow(t.id, wfB.id); + const got = await store.getTask(t.id); + // shared kept; onlyA orphaned but RETAINED in storage (never destroyed). + expect(got?.customFields).toEqual({ shared: "v", onlyA: 3 }); + }); +}); + +describe("store: updateWorkflowDefinition field-type change coercion (U11)", () => { + const harness = createTaskStoreTestHarness(); + let store: ReturnType; + + const irWith = (fields: WorkflowFieldDefinition[], name = "WF"): WorkflowIr => + ({ + version: "v2", + name, + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields, + }) as unknown as WorkflowIr; + + beforeEach(async () => { + await harness.beforeEach(); + store = harness.store(); + }); + afterEach(async () => { + await harness.afterEach(); + }); + + async function fieldedTaskAndWf(fields: WorkflowFieldDefinition[]) { + const def = await (store as any).createWorkflowDefinition({ name: "WF", ir: irWith(fields) }); + const t = await store.createTask({ description: "edit" }); + await (store as any).selectTaskWorkflow(t.id, def.id); + return { workflowId: def.id as string, taskId: t.id as string }; + } + + it("rejects an incompatible type change with occupants and no coerce", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + await (store as any).updateTaskCustomFields(taskId, { x: "hello" }); + await expect( + store.updateWorkflowDefinition(workflowId, { ir: irWith([F({ id: "x", type: "number" })]) }), + ).rejects.toThrow(/IncompatibleFieldChange|incompatibl/i); + }); + + it("coerce:keep-orphaned retains the now-incompatible value", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + await (store as any).updateTaskCustomFields(taskId, { x: "hello" }); + await store.updateWorkflowDefinition(workflowId, { + ir: irWith([F({ id: "x", type: "number" })]), + coerce: "keep-orphaned", + }); + const got = await store.getTask(taskId); + expect(got?.customFields).toEqual({ x: "hello" }); + }); + + it("coerce:drop discards the now-incompatible value", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([F({ id: "x", type: "string" })]); + await (store as any).updateTaskCustomFields(taskId, { x: "hello" }); + await store.updateWorkflowDefinition(workflowId, { + ir: irWith([F({ id: "x", type: "number" })]), + coerce: "drop", + }); + const got = await store.getTask(taskId); + expect(got?.customFields).toEqual({}); + }); + + it("removing a field outright orphans (never blocks, value retained)", async () => { + const { workflowId, taskId } = await fieldedTaskAndWf([ + F({ id: "x", type: "string" }), + F({ id: "y", type: "string" }), + ]); + await (store as any).updateTaskCustomFields(taskId, { x: "a", y: "b" }); + await store.updateWorkflowDefinition(workflowId, { ir: irWith([F({ id: "x", type: "string" })]) }); + const got = await store.getTask(taskId); + // y orphaned but retained. + expect(got?.customFields).toEqual({ x: "a", y: "b" }); + }); +}); + +// ── JSON round-trip stability ──────────────────────────────────────────────── + +describe("custom-field values JSON round-trip", () => { + it("normalized values survive a JSON round-trip unchanged", () => { + const r = validateCustomFieldPatch(ALL_TYPES, { + s: "x", + n: 1.5, + b: false, + e: "low", + m: ["high", "low"], + d: "2026-06-04", + u: "https://x.test/", + }); + expect(r.ok).toBe(true); + if (r.ok) { + expect(JSON.parse(JSON.stringify(r.normalized))).toEqual(r.normalized); + } + }); +}); diff --git a/packages/core/src/__tests__/workflow-step-instances.test.ts b/packages/core/src/__tests__/workflow-step-instances.test.ts index 1d301698b9..8968f58452 100644 --- a/packages/core/src/__tests__/workflow-step-instances.test.ts +++ b/packages/core/src/__tests__/workflow-step-instances.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import type { WorkflowRunStepInstance } from "../types.js"; +import type { WorkflowIr } from "../workflow-ir-types.js"; import { createTaskStoreTestHarness } from "./store-test-helpers.js"; /** @@ -174,28 +175,87 @@ describe("workflow_run_step_instances CRUD (U4, KTD-6)", () => { }); }); -describe("tasks.customFields raw JSON round-trip (U4 groundwork for KTD-13)", () => { +describe("tasks.customFields JSON round-trip under a fielded workflow (U11/KTD-13)", () => { + // U11 behavior change vs. U4: customFields is no longer an opaque whole-object + // round-trip — every write is now validated against the task's workflow field + // schema through the single store authority (task-fields.ts). The default + // workflow declares no fields, so the original U4 tests (which wrote arbitrary + // keys onto a default-workflow task) would now be rejected with + // `no-fields-defined`. They are reworked here to attach a workflow that + // declares the fields under test, and `updateTask` is now a MERGE-with-delete + // patch (not whole-object replacement). The zero-fields rejection path is + // covered in task-fields.test.ts. const harness = createTaskStoreTestHarness(); let store: ReturnType; + // A v2 workflow declaring the fields exercised below. + const fieldedIr = (): WorkflowIr => + ({ + version: "v2", + name: "fielded", + columns: [ + { id: "todo", name: "todo", traits: [] }, + { id: "in-progress", name: "in-progress", traits: [] }, + { id: "done", name: "done", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [{ from: "start", to: "end" }], + fields: [ + { + id: "severity", + name: "Severity", + type: "enum", + options: [ + { value: "high", label: "High" }, + { value: "low", label: "Low" }, + ], + }, + { id: "points", name: "Points", type: "number" }, + { id: "flagged", name: "Flagged", type: "boolean" }, + { + id: "tags", + name: "Tags", + type: "multi-enum", + options: [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ], + }, + { id: "keep", name: "Keep", type: "string" }, + { id: "a", name: "A", type: "number" }, + { id: "b", name: "B", type: "number" }, + ], + }) as unknown as WorkflowIr; + + let workflowId: string; + beforeEach(async () => { await harness.beforeEach(); store = harness.store(); + const def = await (store as any).createWorkflowDefinition({ name: "Fielded", ir: fieldedIr() }); + workflowId = def.id; }); afterEach(async () => { await harness.afterEach(); }); + async function fieldedTask(description: string) { + const t = await store.createTask({ description }); + await (store as any).selectTaskWorkflow(t.id, workflowId); + return t; + } + it("a freshly created task has no customFields (legacy-shape default)", async () => { const t = await store.createTask({ description: "no fields" }); const got = await store.getTask(t.id); - // Stored default is '{}' which parses to an empty object; the row→Task map - // surfaces that as an empty object, distinguishable from later writes. expect(got?.customFields).toEqual({}); }); - it("round-trips a customFields object through updateTask → getTask", async () => { - const t = await store.createTask({ description: "fielded" }); + it("round-trips a validated customFields object through updateTask → getTask", async () => { + const t = await fieldedTask("fielded"); await store.updateTask(t.id, { customFields: { severity: "high", points: 3, flagged: true, tags: ["a", "b"] }, }); @@ -203,17 +263,25 @@ describe("tasks.customFields raw JSON round-trip (U4 groundwork for KTD-13)", () expect(got?.customFields).toEqual({ severity: "high", points: 3, flagged: true, tags: ["a", "b"] }); }); - it("updateTask treats customFields as a whole-object opaque patch (replaces, not merges)", async () => { - const t = await store.createTask({ description: "replace" }); + it("updateTask MERGES the customFields patch (U11 change from U4's whole-object replace)", async () => { + const t = await fieldedTask("merge"); await store.updateTask(t.id, { customFields: { a: 1, b: 2 } }); await store.updateTask(t.id, { customFields: { a: 9 } }); const got = await store.getTask(t.id); - // Whole-object replacement: `b` is gone. (Merge/validation is a later unit.) - expect(got?.customFields).toEqual({ a: 9 }); + // U11 merge semantics: `b` survives, `a` is overwritten. (U4 replaced wholesale.) + expect(got?.customFields).toEqual({ a: 9, b: 2 }); + }); + + it("null in the patch deletes that field's value", async () => { + const t = await fieldedTask("delete"); + await store.updateTask(t.id, { customFields: { a: 1, b: 2 } }); + await store.updateTask(t.id, { customFields: { a: null } }); + const got = await store.getTask(t.id); + expect(got?.customFields).toEqual({ b: 2 }); }); it("leaves customFields untouched when an unrelated field is updated", async () => { - const t = await store.createTask({ description: "untouched" }); + const t = await fieldedTask("untouched"); await store.updateTask(t.id, { customFields: { keep: "me" } }); await store.updateTask(t.id, { summary: "an unrelated change" }); const got = await store.getTask(t.id); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 67272ebed3..da2bad2e74 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -151,9 +151,11 @@ export type { ColumnCapacity } from "./workflow-capacity.js"; export { OccupiedColumnsError, InvalidRehomeTargetError, + IncompatibleFieldChangeError, resolveEntryColumnId, resolveSwitchReconciliation, computeRemovedOccupiedColumns, + computeIncompatibleFieldChanges, assertRehomeTargetValid, setReconciliationAbort, runReconciliationAbort, @@ -162,9 +164,24 @@ export { export type { SwitchReconciliation, ColumnOccupancy, + IncompatibleFieldChange, ReconciliationAbort, ReconciliationAbortContext, } from "./workflow-reconciliation.js"; +export { + validateCustomFieldPatch, + applyFieldDefaults, + reconcileFieldsOnWorkflowChange, + makeCustomFieldRejection, + CustomFieldRejectionError, + CUSTOM_FIELD_REJECTION_CODES, +} from "./task-fields.js"; +export type { + CustomFieldRejection, + CustomFieldRejectionCode, + CustomFieldPatchResult, + FieldReconciliation, +} from "./task-fields.js"; export { readTransitionPending, writeTransitionPending, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index f4ddcf46e7..6716e8f0ed 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -21,6 +21,8 @@ import { OccupiedColumnsError, assertRehomeTargetValid, computeRemovedOccupiedColumns, + computeIncompatibleFieldChanges, + IncompatibleFieldChangeError, resolveEntryColumnId, resolveSwitchReconciliation, runReconciliationAbort, @@ -43,7 +45,14 @@ import { reconcileHooksRemaining, } from "./transition-pending.js"; import { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; -import type { WorkflowIr, WorkflowIrColumn } from "./workflow-ir-types.js"; +import type { WorkflowIr, WorkflowIrColumn, WorkflowFieldDefinition } from "./workflow-ir-types.js"; +import { + validateCustomFieldPatch, + applyFieldDefaults, + reconcileFieldsOnWorkflowChange, + CustomFieldRejectionError, + type CustomFieldRejection, +} from "./task-fields.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"; @@ -6974,6 +6983,58 @@ export class TaskStore extends EventEmitter { return this.withTaskLock(id, () => this.updateTaskUnlocked(id, updates, runContext)); } + /** + * Merge a validated/normalized custom-field patch into the existing values. + * `null` in the patch deletes that field's value (the delete sentinel from + * {@link validateCustomFieldPatch}); any other value overwrites. Returns a new + * object (never mutates the input) so the caller assigns it onto the task. + */ + private mergeCustomFieldPatch( + current: Record | undefined, + patch: Record, + ): Record { + const next: Record = { ...(current ?? {}) }; + for (const [key, value] of Object.entries(patch)) { + if (value === null) { + delete next[key]; + } else { + next[key] = value; + } + } + return next; + } + + /** + * Single write authority for custom task fields (U11 / KTD-13). + * + * Resolves the task's workflow field definitions, validates `patch` against + * them via {@link validateCustomFieldPatch}, merges the normalized result into + * `Task.customFields` (delete-on-null), persists through the standard update + * path, and emits `task:updated` like every other task mutation. A workflow + * with no fields (e.g. the default) rejects any non-empty patch with + * `no-fields-defined`. Returns a typed result rather than throwing so callers + * (agent tools, HTTP routes) can surface the field path/code directly. + */ + async updateTaskCustomFields( + taskId: string, + patch: Record, + runContext?: RunMutationContext, + ): Promise<{ ok: true; task: Task } | { ok: false; rejection: CustomFieldRejection }> { + return this.withTaskLock(taskId, async () => { + const defs = this.resolveTaskCustomFieldDefsSync(taskId); + const result = validateCustomFieldPatch(defs, patch); + if (!result.ok) { + return { ok: false as const, rejection: result.rejection }; + } + // Pass the validated PATCH through (with null delete-sentinels) — the + // merge-with-delete happens once, inside updateTaskUnlocked, against the + // freshly-read task. Pre-merging here would lose the delete semantics on + // the second merge. + const task = await this.updateTaskUnlocked(taskId, { customFields: result.normalized }, runContext); + return { ok: true as const, task }; + }); + } + /** * The body of {@link updateTask} WITHOUT acquiring the per-task lock. Callers * that already hold `withTaskLock(id)` — e.g. workflow-selection mutations @@ -7082,10 +7143,19 @@ export class TaskStore extends EventEmitter { } } if (updates.steps !== undefined) task.steps = updates.steps; - // U4/KTD-13 groundwork: round-trip customFields as an opaque whole-object - // patch. The typed validation/write authority (updateTaskCustomFields) - // lands in a later unit; for now updateTask just persists what it is given. - if (updates.customFields !== undefined) task.customFields = updates.customFields; + // U11/KTD-13: customFields writes are validated against the task's workflow + // field schema through the single authority (task-fields.ts). The patch is + // merged into the existing values (delete-on-null), mirroring + // updateTaskCustomFields. Backward-compat note: U4 round-tripped the object + // opaquely; the field system now enforces type/enum/unknown-id rules, so a + // write against a workflow with no fields (the default) is rejected with a + // typed CustomFieldRejectionError rather than silently persisted. + if (updates.customFields !== undefined) { + const defs = this.resolveTaskCustomFieldDefsSync(id); + const result = validateCustomFieldPatch(defs, updates.customFields); + if (!result.ok) throw new CustomFieldRejectionError(result.rejection); + task.customFields = this.mergeCustomFieldPatch(task.customFields, result.normalized); + } if (updates.currentStep !== undefined) task.currentStep = updates.currentStep; if (updates.status === null) { task.status = undefined; @@ -12293,6 +12363,58 @@ ${stepsSection}`; pendingRehome = { rehomeTo: updates.rehomeTo, occupantTaskIds }; } } + + // U11/KTD-13: when the IR changes custom field types incompatibly for tasks + // that already hold values, block with a typed IncompatibleFieldChangeError + // unless `coerce` is supplied. Removed/added fields never block (removal + // orphans). Flag-independent: fields are orthogonal to the columns flag. + // Reconciliation runs per occupant task AFTER the IR save commits. + let pendingFieldReconcile: + | { oldFields: WorkflowFieldDefinition[]; newFields: WorkflowFieldDefinition[]; occupantTaskIds: string[]; coerce?: "drop" | "keep-orphaned" } + | undefined; + if (updates.ir !== undefined) { + const existingForFields = await this.getWorkflowDefinition(id); + if (!existingForFields) throw new Error(`Workflow '${id}' not found`); + const nextIrForFields = parseWorkflowIr(updates.ir); + const oldFields: WorkflowFieldDefinition[] = + existingForFields.ir.version === "v2" ? (existingForFields.ir.fields ?? []) : []; + const newFields: WorkflowFieldDefinition[] = + nextIrForFields.version === "v2" ? (nextIrForFields.fields ?? []) : []; + const fieldsChanged = + JSON.stringify(oldFields) !== JSON.stringify(newFields); + if (fieldsChanged) { + const occupantTaskIds = this.listWorkflowOccupantTaskIds(id, false); + const occupantsByField = new Map(); + const occupantsWithFields: string[] = []; + for (const taskId of occupantTaskIds) { + const row = this.db.prepare("SELECT customFields FROM tasks WHERE id = ?").get(taskId) as + | { customFields: string | null } + | undefined; + const values = row?.customFields + ? (fromJson>(row.customFields) ?? {}) + : {}; + if (Object.keys(values).length === 0) continue; + occupantsWithFields.push(taskId); + for (const key of Object.keys(values)) { + occupantsByField.set(key, (occupantsByField.get(key) ?? 0) + 1); + } + } + const incompatible = computeIncompatibleFieldChanges( + existingForFields.ir, + nextIrForFields, + occupantsByField, + ); + if (incompatible.length > 0 && updates.coerce === undefined) { + throw new IncompatibleFieldChangeError(id, incompatible); + } + pendingFieldReconcile = { + oldFields, + newFields, + occupantTaskIds: occupantsWithFields, + coerce: updates.coerce, + }; + } + } const saved = await this.withConfigLock(async () => { const existing = await this.getWorkflowDefinition(id); if (!existing) throw new Error(`Workflow '${id}' not found`); @@ -12341,6 +12463,23 @@ ${stepsSection}`; }); } } + + // U11/KTD-13: now that the new field schema is committed, reconcile each + // occupant task's stored values against it (orphan-not-delete by default; + // coerce:"drop" discards orphans). Each runs under its own task lock. + if (pendingFieldReconcile) { + const dropOrphans = pendingFieldReconcile.coerce === "drop"; + for (const taskId of pendingFieldReconcile.occupantTaskIds) { + await this.withTaskLock(taskId, () => + this.reconcileTaskCustomFieldsForSchema( + taskId, + pendingFieldReconcile!.oldFields, + pendingFieldReconcile!.newFields, + dropOrphans, + ), + ); + } + } return saved; } @@ -12898,6 +13037,16 @@ ${stepsSection}`; return list; } + /** + * Resolve the custom-field definitions (KTD-13) governing a task, via its + * workflow selection. v1 IR and the default workflow declare none → `[]`. + * Pure DB read, safe inside transactions. + */ + private resolveTaskCustomFieldDefsSync(taskId: string): WorkflowFieldDefinition[] { + const ir = this.resolveTaskWorkflowIrSync(taskId); + return ir.version === "v2" ? (ir.fields ?? []) : []; + } + private resolveTaskWorkflowIrSync(taskId: string): WorkflowIr { const selection = this.getTaskWorkflowSelection(taskId); const workflowId = selection?.workflowId; @@ -13130,6 +13279,12 @@ ${stepsSection}`; // prior selection's rows, so a mid-flight failure never leaves the task // referencing already-deleted step ids. const priorSelection = this.getTaskWorkflowSelection(taskId); + // U11/KTD-13: capture the OLD field schema (from the prior selection's IR) + // before the selection row flips, so we can reconcile existing field values + // against the NEW workflow's schema below. + const oldFieldDefs = this.resolveTaskCustomFieldDefsSync(taskId); + const newFieldDefs: WorkflowFieldDefinition[] = + def.ir.version === "v2" ? (def.ir.fields ?? []) : []; const ids = await this.materializeWorkflowSteps(workflowId, inputs); try { await this.updateTaskUnlocked(taskId, { enabledWorkflowSteps: ids }); @@ -13155,10 +13310,56 @@ ${stepsSection}`; } this.workflowStepsCache = null; } + + // U11/KTD-13: reconcile custom field values against the NEW workflow's + // schema. Same-id, type-compatible values are kept; incompatible/removed + // ids are orphaned — but RETAINED in storage (orphan-not-delete) so a later + // switch back, or the orphaned-fields disclosure, can still surface them. + // Then fill defaults for the new workflow's required+default fields that + // are absent. The merged object is written DIRECTLY (bypassing the + // validating patch path) because orphaned ids are by definition unknown to + // the new schema and would otherwise be rejected. + await this.reconcileTaskCustomFieldsForSchema(taskId, oldFieldDefs, newFieldDefs); + return ids; }); } + /** + * U11/KTD-13: reconcile a task's stored custom field values when its governing + * field schema changes (workflow switch or definition edit). Values are + * partitioned by {@link reconcileFieldsOnWorkflowChange}; orphans are retained + * (never destroyed). Required+default fields absent from the result are filled. + * Writes the merged values directly onto task.json — orphaned ids are unknown + * to the new schema, so this deliberately bypasses the validating patch path. + * Assumes the caller already holds the per-task lock. + */ + private async reconcileTaskCustomFieldsForSchema( + taskId: string, + oldFieldDefs: WorkflowFieldDefinition[], + newFieldDefs: WorkflowFieldDefinition[], + dropOrphans = false, + ): Promise { + const dir = this.taskDir(taskId); + const task = await this.readTaskJson(dir); + const current = task.customFields ?? {}; + const { kept, orphaned } = reconcileFieldsOnWorkflowChange(oldFieldDefs, newFieldDefs, current); + // Default (keep-orphaned): storage keeps everything (kept ∪ orphaned). + // coerce:"drop" discards the orphaned values entirely. + const base = dropOrphans ? { ...kept } : { ...kept, ...orphaned }; + const reconciled = applyFieldDefaults(newFieldDefs, base); + // Skip the write when nothing changed (no defaults added, same keys/values). + const unchanged = + Object.keys(reconciled).length === Object.keys(current).length && + Object.entries(reconciled).every(([k, v]) => current[k] === v); + if (unchanged) return; + task.customFields = reconciled; + task.updatedAt = new Date().toISOString(); + await this.atomicWriteTaskJson(dir, task); + if (this.isWatching) this.taskCache.set(taskId, { ...task }); + this.emitTaskLifecycleEventSafely("task:updated", [task]); + } + /** * U5 (R20) workflow switch: select a workflow for a task and, when the * `workflowColumns` flag is ON, reconcile the card's board column against the diff --git a/packages/core/src/task-fields.ts b/packages/core/src/task-fields.ts new file mode 100644 index 0000000000..ce4c0cd019 --- /dev/null +++ b/packages/core/src/task-fields.ts @@ -0,0 +1,362 @@ +/** + * Custom task field validation & reconciliation authority (U11 / KTD-13). + * + * Workflows declare typed custom task fields ({@link WorkflowFieldDefinition}); + * task values live in `tasks.customFields` (a JSON object keyed by field id). + * This module is the single, side-effect-free validation core that the store + * write authority (`updateTaskCustomFields` / `updateTask`) delegates to. It + * mirrors the `TransitionRejection` style: a flat, JSON-safe typed rejection + * with a machine-stable `code`, the offending `fieldId`, and a non-localized + * `detail` string for audit/logs. + * + * Three operations: + * - {@link validateCustomFieldPatch} — validate a `Record` + * patch against a field schema, normalizing accepted values. `null`/`undefined` + * in the patch is a delete sentinel for that field (always accepted). + * - {@link applyFieldDefaults} — fill `default` for required fields absent from + * the current values (task create / workflow selection). + * - {@link reconcileFieldsOnWorkflowChange} — partition existing values into + * `kept` (same id, type-compatible) and `orphaned` (everything else) when a + * workflow's fields change or the task switches workflows. Orphans are + * RETAINED in storage — this only computes the partition so the UI can render + * the orphaned-fields disclosure. + */ + +import type { + WorkflowFieldDefinition, + WorkflowFieldType, +} from "./workflow-ir-types.js"; + +// --------------------------------------------------------------------------- +// Typed rejection (TransitionRejection-style: flat, JSON-safe, no class) +// --------------------------------------------------------------------------- + +/** + * Reason codes for a rejected custom-field write. Stable string literals — they + * cross the agent-tool / HTTP boundary and are matched by surfaces for copy, so + * they must not change without migrating consumers. + */ +export type CustomFieldRejectionCode = + | "no-fields-defined" + | "unknown-field" + | "type-mismatch" + | "enum-violation"; + +/** The full, immutable set of custom-field rejection codes. */ +export const CUSTOM_FIELD_REJECTION_CODES: readonly CustomFieldRejectionCode[] = [ + "no-fields-defined", + "unknown-field", + "type-mismatch", + "enum-violation", +] as const; + +/** + * A typed custom-field rejection. Flat and JSON-safe by construction — mirrors + * {@link import("./transition-types.js").TransitionRejection}. + * + * - `code` — machine-stable {@link CustomFieldRejectionCode}. + * - `fieldId` — the offending field id (the patch key that failed). + * - `detail` — non-localized diagnostic context for audit/logs. + */ +export interface CustomFieldRejection { + code: CustomFieldRejectionCode; + fieldId: string; + detail: string; +} + +/** Result of validating a custom-field patch. Discriminated on `ok`. */ +export type CustomFieldPatchResult = + | { ok: true; normalized: Record } + | { ok: false; rejection: CustomFieldRejection }; + +/** Construct a {@link CustomFieldRejection}. */ +export function makeCustomFieldRejection( + code: CustomFieldRejectionCode, + fieldId: string, + detail: string, +): CustomFieldRejection { + return { code, fieldId, detail }; +} + +/** + * Thrown by the throw-based write paths (`updateTask` with a `customFields` + * patch) when validation rejects. `updateTaskCustomFields` returns the typed + * rejection instead; this wrapper exists for the legacy throw contract so a bad + * `updateTask` write fails loudly rather than silently round-tripping an invalid + * value (the U4 opaque behavior). Carries the structured rejection so HTTP/agent + * surfaces can recover the field path and code. + */ +export class CustomFieldRejectionError extends Error { + readonly rejection: CustomFieldRejection; + constructor(rejection: CustomFieldRejection) { + super(`custom field '${rejection.fieldId}' rejected (${rejection.code}): ${rejection.detail}`); + this.name = "CustomFieldRejectionError"; + this.rejection = rejection; + } +} + +// --------------------------------------------------------------------------- +// Per-type value validation +// --------------------------------------------------------------------------- + +/** True iff `value` is a non-empty option-value member of `field.options`. */ +function isEnumMember(field: WorkflowFieldDefinition, value: string): boolean { + return (field.options ?? []).some((o) => o.value === value); +} + +/** + * Validate (and normalize) a single non-null value against a field's type. + * Returns the normalized value on success, or a rejection. The caller has + * already resolved the field definition. + */ +function validateValue( + field: WorkflowFieldDefinition, + value: unknown, +): { ok: true; value: unknown } | { ok: false; rejection: CustomFieldRejection } { + const reject = ( + code: CustomFieldRejectionCode, + detail: string, + ): { ok: false; rejection: CustomFieldRejection } => ({ + ok: false, + rejection: makeCustomFieldRejection(code, field.id, detail), + }); + + switch (field.type) { + case "string": + case "text": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' expects a string, got ${typeof value}`); + } + return { ok: true, value }; + } + case "number": { + if (typeof value !== "number" || !Number.isFinite(value)) { + return reject( + "type-mismatch", + `field '${field.id}' expects a finite number, got ${typeof value === "number" ? String(value) : typeof value}`, + ); + } + return { ok: true, value }; + } + case "boolean": { + if (typeof value !== "boolean") { + return reject("type-mismatch", `field '${field.id}' expects a boolean, got ${typeof value}`); + } + return { ok: true, value }; + } + case "enum": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' (enum) expects a string option value, got ${typeof value}`); + } + if (!isEnumMember(field, value)) { + return reject("enum-violation", `field '${field.id}' value '${value}' is not a declared option`); + } + return { ok: true, value }; + } + case "multi-enum": { + if (!Array.isArray(value)) { + return reject("type-mismatch", `field '${field.id}' (multi-enum) expects an array, got ${typeof value}`); + } + const seen = new Set(); + for (const item of value) { + if (typeof item !== "string") { + return reject("type-mismatch", `field '${field.id}' (multi-enum) members must be strings`); + } + if (!isEnumMember(field, item)) { + return reject("enum-violation", `field '${field.id}' member '${item}' is not a declared option`); + } + if (seen.has(item)) { + return reject("enum-violation", `field '${field.id}' has duplicate member '${item}'`); + } + seen.add(item); + } + return { ok: true, value: [...value] as string[] }; + } + case "date": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' (date) expects an ISO date string, got ${typeof value}`); + } + const ms = Date.parse(value); + if (Number.isNaN(ms)) { + return reject("type-mismatch", `field '${field.id}' value '${value}' is not a parseable date`); + } + return { ok: true, value }; + } + case "url": { + if (typeof value !== "string") { + return reject("type-mismatch", `field '${field.id}' (url) expects a string, got ${typeof value}`); + } + try { + // eslint-disable-next-line no-new + new URL(value); + } catch { + return reject("type-mismatch", `field '${field.id}' value '${value}' is not a valid URL`); + } + return { ok: true, value }; + } + default: { + // Exhaustiveness guard — an unknown type cannot validate. + const _exhaustive: never = field.type; + return reject("type-mismatch", `field '${field.id}' has unsupported type '${String(_exhaustive)}'`); + } + } +} + +// --------------------------------------------------------------------------- +// Patch validation authority +// --------------------------------------------------------------------------- + +/** + * Validate a custom-field `patch` against a workflow's field `fields`. + * + * - A `null`/`undefined` patch value is a DELETE sentinel: the field's stored + * value should be removed. It is always accepted (even for required fields — + * required is not a write-time gate this round, KTD-13) and surfaces in + * `normalized` as `null` so the caller can apply the delete uniformly. + * - A non-null value is validated/normalized per the field's type. + * - A patch key that names no declared field → `unknown-field`. + * - When `fields` is undefined/empty and the patch carries any key → the whole + * patch is rejected `no-fields-defined` (the default workflow declares no + * fields; nothing can be written). An empty patch against no fields is `ok`. + * + * Validation is fail-fast: the first offending key produces the rejection. + */ +export function validateCustomFieldPatch( + fields: WorkflowFieldDefinition[] | undefined, + patch: Record, +): CustomFieldPatchResult { + const keys = Object.keys(patch); + const byId = new Map((fields ?? []).map((f) => [f.id, f])); + + if (byId.size === 0) { + if (keys.length === 0) return { ok: true, normalized: {} }; + return { + ok: false, + rejection: makeCustomFieldRejection( + "no-fields-defined", + keys[0]!, + "the resolved workflow declares no custom fields; no values may be written", + ), + }; + } + + const normalized: Record = {}; + for (const key of keys) { + const value = patch[key]; + const field = byId.get(key); + if (!field) { + return { + ok: false, + rejection: makeCustomFieldRejection( + "unknown-field", + key, + `field '${key}' is not declared by the task's workflow`, + ), + }; + } + // null/undefined = delete this field's value. + if (value === null || value === undefined) { + normalized[key] = null; + continue; + } + const res = validateValue(field, value); + if (!res.ok) return res; + normalized[key] = res.value; + } + return { ok: true, normalized }; +} + +// --------------------------------------------------------------------------- +// Defaults at create / workflow selection +// --------------------------------------------------------------------------- + +/** + * Fill `default` values for REQUIRED fields that are absent from `current`. + * Returns a NEW merged object (does not mutate `current`); existing values win. + * Non-required fields and fields without a declared `default` are left absent. + * + * Used at task create / workflow selection so a workflow with required+default + * fields lands sensible initial values. Defaults are taken on trust from the + * (already-validated-at-save) field schema. + */ +export function applyFieldDefaults( + fields: WorkflowFieldDefinition[] | undefined, + current: Record | undefined, +): Record { + const out: Record = { ...(current ?? {}) }; + for (const field of fields ?? []) { + if (!field.required) continue; + if (field.default === undefined) continue; + if (Object.prototype.hasOwnProperty.call(out, field.id) && out[field.id] !== undefined) { + continue; + } + out[field.id] = field.default; + } + return out; +} + +// --------------------------------------------------------------------------- +// Reconciliation on workflow edit / switch +// --------------------------------------------------------------------------- + +/** Two field types are "enum-kind" siblings (enum / multi-enum). */ +function isEnumKind(type: WorkflowFieldType): boolean { + return type === "enum" || type === "multi-enum"; +} + +/** + * A stored value for `field` is type-compatible with a new field definition iff + * the new value re-validates cleanly. For enum-kind fields, compatibility also + * requires the value still be a member of the new options (handled by + * re-validation). This is the same gate {@link validateValue} applies on write, + * so "kept" values are guaranteed re-writable under the new schema. + */ +function valueCompatible(newField: WorkflowFieldDefinition, value: unknown): boolean { + if (value === null || value === undefined) return true; + return validateValue(newField, value).ok; +} + +/** Partition of existing values produced by {@link reconcileFieldsOnWorkflowChange}. */ +export interface FieldReconciliation { + /** Values whose id survives in the new schema AND remain type-compatible. */ + kept: Record; + /** + * Values that no longer fit: id removed from the new schema, or the type + * changed incompatibly (including an enum value no longer in the new options). + * RETAINED in storage — listed here only so the UI can render them under the + * orphaned-fields disclosure. + */ + orphaned: Record; +} + +/** + * Reconcile stored `values` when a workflow's field schema changes (edit) or a + * task switches workflows. Same-id values are KEPT when the new field is + * type-compatible (same type, or both enum-kind with the value still a member — + * enforced by re-validation); everything else is ORPHANED. + * + * Storage keeps EVERYTHING — this function only computes the partition. Callers + * persist `{...kept, ...orphaned}` (i.e. the original values, unchanged) and use + * `orphaned` purely for UI disclosure. `oldFields` is accepted for symmetry and + * future heuristics; the decision is driven entirely by `newFields` + the value. + */ +export function reconcileFieldsOnWorkflowChange( + oldFields: WorkflowFieldDefinition[] | undefined, + newFields: WorkflowFieldDefinition[] | undefined, + values: Record | undefined, +): FieldReconciliation { + void oldFields; // reserved for future migration heuristics; intentionally unused + const newById = new Map((newFields ?? []).map((f) => [f.id, f])); + const kept: Record = {}; + const orphaned: Record = {}; + + for (const [id, value] of Object.entries(values ?? {})) { + const newField = newById.get(id); + if (newField && valueCompatible(newField, value)) { + kept[id] = value; + } else { + orphaned[id] = value; + } + } + return { kept, orphaned }; +} diff --git a/packages/core/src/workflow-definition-types.ts b/packages/core/src/workflow-definition-types.ts index 026f544684..60aee809e4 100644 --- a/packages/core/src/workflow-definition-types.ts +++ b/packages/core/src/workflow-definition-types.ts @@ -48,4 +48,14 @@ export interface WorkflowDefinitionUpdate { * the `workflowColumns` flag is ON. */ rehomeTo?: string; + /** + * U11/KTD-13: when an IR update changes a custom field's type incompatibly for + * tasks that already hold a value under that field, the update is blocked with + * a typed {@link import("./workflow-reconciliation.js").IncompatibleFieldChangeError} + * unless `coerce` is supplied. `"drop"` discards the now-incompatible stored + * values; `"keep-orphaned"` retains them as orphans (rendered under the + * orphaned-fields disclosure). Removing a field outright always orphans (never + * blocks). Mirrors the `rehomeTo` conflict-resolution posture for columns. + */ + coerce?: "drop" | "keep-orphaned"; } diff --git a/packages/core/src/workflow-reconciliation.ts b/packages/core/src/workflow-reconciliation.ts index 382d3645fa..e417c79a9e 100644 --- a/packages/core/src/workflow-reconciliation.ts +++ b/packages/core/src/workflow-reconciliation.ts @@ -32,7 +32,12 @@ * is independently testable and reused identically across switch/edit/delete. */ -import type { WorkflowIr, WorkflowIrV2, WorkflowIrColumn } from "./workflow-ir-types.js"; +import type { + WorkflowIr, + WorkflowIrV2, + WorkflowIrColumn, + WorkflowFieldDefinition, +} from "./workflow-ir-types.js"; import { resolveColumnFlags } from "./trait-registry.js"; import { workflowHasColumn } from "./workflow-transitions.js"; @@ -181,6 +186,90 @@ export function assertRehomeTargetValid(nextIr: WorkflowIr, rehomeTo: string): v } } +// ── Custom-field schema-evolution reconciliation (U11/KTD-13) ──────────────── + +/** A field whose type changed incompatibly while tasks hold values under it. */ +export interface IncompatibleFieldChange { + fieldId: string; + fromType: string; + toType: string; + /** Number of tasks (under this workflow) currently holding a value for it. */ + occupantCount: number; +} + +/** + * Thrown by the workflow update path when an IR edit changes one or more custom + * fields' types incompatibly for tasks that already hold a value, and no + * `coerce` option was supplied. Mirrors {@link OccupiedColumnsError}: a typed, + * conflict-signaling error the surface maps to a 409 prompting for a coercion + * choice (`drop` | `keep-orphaned`). + */ +export class IncompatibleFieldChangeError extends Error { + readonly workflowId: string; + readonly changes: IncompatibleFieldChange[]; + constructor(workflowId: string, changes: IncompatibleFieldChange[]) { + const summary = changes + .map((c) => `${c.fieldId} (${c.fromType}→${c.toType}, ${c.occupantCount})`) + .join(", "); + super( + `Workflow '${workflowId}' edit changes field type(s) incompatibly: ${summary}. ` + + `Supply coerce ("drop" | "keep-orphaned") to proceed.`, + ); + this.name = "IncompatibleFieldChangeError"; + this.workflowId = workflowId; + this.changes = changes; + } +} + +/** The v2 fields of an IR, or `[]` when absent (v1 or undeclared). */ +function fieldsOf(ir: WorkflowIr): WorkflowFieldDefinition[] { + const v2 = ir as WorkflowIrV2; + return Array.isArray(v2.fields) ? v2.fields : []; +} + +/** Enum-kind sibling check (enum / multi-enum). */ +function sameEnumKind(a: string, b: string): boolean { + const enumKind = (t: string) => t === "enum" || t === "multi-enum"; + return enumKind(a) && enumKind(b); +} + +/** + * Compute which custom fields change type INCOMPATIBLY between `existingIr` and + * `nextIr` AND still have occupant tasks holding a value. A type is compatible + * with itself; enum↔multi-enum is treated as compatible-shape (values are + * re-validated against the new options at reconcile time — a value dropped by + * the new options orphans individually, not via a hard block). A field removed + * outright is NOT a conflict (removal always orphans, never blocks). Returns one + * entry per blocking change in the existing IR's field order. + * + * `occupantsByField` maps a field id to the count of tasks (under this workflow) + * currently holding a value for it. + */ +export function computeIncompatibleFieldChanges( + existingIr: WorkflowIr, + nextIr: WorkflowIr, + occupantsByField: Map, +): IncompatibleFieldChange[] { + const nextById = new Map(fieldsOf(nextIr).map((f) => [f.id, f])); + const changes: IncompatibleFieldChange[] = []; + for (const oldField of fieldsOf(existingIr)) { + const next = nextById.get(oldField.id); + if (!next) continue; // removed → orphan, not a block + if (next.type === oldField.type) continue; // identical type → fine + if (sameEnumKind(oldField.type, next.type)) continue; // enum↔multi-enum → soft + const occupantCount = occupantsByField.get(oldField.id) ?? 0; + if (occupantCount > 0) { + changes.push({ + fieldId: oldField.id, + fromType: oldField.type, + toType: next.type, + occupantCount, + }); + } + } + return changes; +} + // ── Abort-on-switch DI seam (core stays engine-free) ───────────────────────── // // A workflow switch must abort the card's in-flight processing BEFORE the move