diff --git a/packages/core/src/__tests__/db-migrate.test.ts b/packages/core/src/__tests__/db-migrate.test.ts index b36c04e128..fa2616266d 100644 --- a/packages/core/src/__tests__/db-migrate.test.ts +++ b/packages/core/src/__tests__/db-migrate.test.ts @@ -715,7 +715,7 @@ describe("schema migration", () => { const row = db.prepare("SELECT deletedAt FROM tasks WHERE id = 'FN-legacy'").get() as { deletedAt: string | null }; expect(row.deletedAt).toBeNull(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -748,7 +748,7 @@ describe("schema migration", () => { { id: "WS-001", mode: "prompt", gateMode: "advisory" }, { id: "WS-002", mode: "script", gateMode: "advisory" }, ]); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -798,7 +798,7 @@ describe("schema migration", () => { reviewerContextRetryCount: 0, reviewerFallbackRetryCount: 0, }); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -827,7 +827,7 @@ describe("schema migration", () => { const columns = db.prepare("PRAGMA table_info(milestones)").all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("acceptanceCriteria"); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -868,7 +868,7 @@ describe("schema migration", () => { const missionColumns = db.prepare("PRAGMA table_info(missions)").all() as Array<{ name: string }>; expect(missionColumns.map((column) => column.name)).toContain("autoMerge"); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -902,7 +902,7 @@ describe("schema migration", () => { { id: "WS-002", mode: "script", enabled: 1, gateMode: "advisory" }, { id: "WS-003", mode: "prompt", enabled: 0, gateMode: "advisory" }, ]); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -939,7 +939,7 @@ describe("schema migration", () => { const indexes = db.prepare("PRAGMA index_list(mission_goals)").all() as Array<{ name: string }>; expect(indexes.some((index) => index.name === "idxMissionGoalsGoalId")).toBe(true); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -1000,7 +1000,52 @@ describe("schema migration", () => { expect(customFieldsColumn).toBeDefined(); expect(customFieldsColumn?.dflt_value).toBe("'{}'"); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); + db.close(); + }); + + it("adds workflow_settings table when migrating from schema version 108", () => { + const db = new Database(fusionDir); + db.exec("CREATE TABLE IF NOT EXISTS __meta (key TEXT PRIMARY KEY, value TEXT)"); + db.exec("INSERT INTO __meta (key, value) VALUES ('schemaVersion', '108')"); + db.exec("INSERT INTO __meta (key, value) VALUES ('lastModified', '1000')"); + db.exec(` + CREATE TABLE IF NOT EXISTS tasks ( + id TEXT PRIMARY KEY, + description TEXT NOT NULL, + "column" TEXT NOT NULL, + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + ) + `); + + db.init(); + + // The new per-(workflowId, projectId) setting-value table exists. + const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all() as Array<{ name: string }>; + expect(tables.map((row) => row.name)).toContain("workflow_settings"); + + const columns = db.prepare("PRAGMA table_info(workflow_settings)").all() as Array<{ + name: string; + pk: number; + dflt_value: string | null; + }>; + expect(columns.map((column) => column.name)).toEqual([ + "workflowId", + "projectId", + "values", + "updatedAt", + ]); + // Composite primary key over (workflowId, projectId). + expect(columns.filter((column) => column.pk > 0).map((column) => column.name).sort()).toEqual([ + "projectId", + "workflowId", + ]); + // `values` defaults to an empty JSON object. + const valuesColumn = columns.find((column) => column.name === "values"); + expect(valuesColumn?.dflt_value).toBe("'{}'"); + + expect(db.getSchemaVersion()).toBe(109); db.close(); }); }); diff --git a/packages/core/src/__tests__/db.test.ts b/packages/core/src/__tests__/db.test.ts index 8a618bd583..a27fc8f30b 100644 --- a/packages/core/src/__tests__/db.test.ts +++ b/packages/core/src/__tests__/db.test.ts @@ -334,7 +334,7 @@ describe("Database", () => { }); it("seeds schema version", () => { - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("includes tokenUsageCacheWriteTokens on freshly initialized tasks table", () => { @@ -393,7 +393,7 @@ describe("Database", () => { it("is idempotent - calling init() twice does not fail", () => { expect(() => db.init()).not.toThrow(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("does not overwrite existing config on re-init", () => { // Update the config @@ -1463,7 +1463,7 @@ describe("schema migrations", () => { db.init(); // Verify version bumped to 29 (includes v1→v2 through v26→v29) - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); // Verify new columns exist and existing data is intact const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; @@ -1488,11 +1488,11 @@ describe("schema migrations", () => { const db = new Database(fusionDir); db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); // Re-init should not fail db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); db.close(); }); @@ -1527,7 +1527,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; expect(cols.map((col) => col.name)).toContain("priority"); @@ -1568,7 +1568,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const colNames = cols.map((col) => col.name); @@ -1640,7 +1640,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const colNames = cols.map((col) => col.name); @@ -1722,13 +1722,13 @@ describe("schema migrations", () => { const kept = entries.filter(([name]) => !dropped.has(name)); const chosen = kept.length > 0 ? kept : entries.slice(0, 1); - const columnSql = chosen.map(([name, def]) => ` ${name} ${def}`).join(",\n"); + const columnSql = chosen.map(([name, def]) => ` "${name}" ${def}`).join(",\n"); legacyDb.exec(`CREATE TABLE IF NOT EXISTS ${tableName} (\n${columnSql}\n)`); } const validatorColumns = Object.entries(MIGRATION_ONLY_TABLE_SCHEMAS.mission_validator_runs) .filter(([name, definition], index) => name === "id" || (name !== "taskId" && (index % 4 !== 0 || !isSafeToDrop(definition)))) - .map(([name, def]) => ` ${name} ${def}`) + .map(([name, def]) => ` "${name}" ${def}`) .join(",\n"); legacyDb.exec(`CREATE TABLE IF NOT EXISTS mission_validator_runs (\n${validatorColumns}\n)`); @@ -1880,7 +1880,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const cols = db.prepare("PRAGMA table_info(chat_messages)").all() as Array<{ name: string }>; expect(cols.map((col) => col.name)).toContain("attachments"); @@ -1954,7 +1954,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>; expect(tables).toEqual([{ name: "agentRatings" }]); @@ -1978,7 +1978,7 @@ describe("schema migrations", () => { db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>; expect(tables).toEqual([{ name: "mission_events" }]); @@ -2082,7 +2082,7 @@ describe("schema migrations", () => { db.init(); // Verify version bumped to 29 - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); // Verify new columns exist and existing data is intact const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; @@ -2301,7 +2301,7 @@ describe("schema migrations", () => { localDb.init(); - expect(localDb.getSchemaVersion()).toBe(108); + expect(localDb.getSchemaVersion()).toBe(109); const columns = localDb.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; expect(columns.map((column) => column.name)).toContain("tokenUsageCacheWriteTokens"); @@ -2612,7 +2612,7 @@ describe("createDatabase factory", () => { const db = createDatabase(fusionDir); db.init(); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); expect(db.getLastModified()).toBeGreaterThan(0); db.close(); @@ -2766,7 +2766,7 @@ describe("migration v77 task token budget columns", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const rows = migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const names = new Set(rows.map((row) => row.name)); expect(names.has("tokenBudgetSoftAlertedAt")).toBe(true); @@ -2797,7 +2797,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => { const fresh = new Database(fusion); try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(108); + expect(fresh.getSchemaVersion()).toBe(109); const names = new Set( (fresh.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name), ); @@ -2825,7 +2825,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const names = new Set( (migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name), ); @@ -2851,7 +2851,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => { const fresh = new Database(fusion); try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(108); + expect(fresh.getSchemaVersion()).toBe(109); const table = fresh .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'") .get() as { name: string } | undefined; @@ -2885,7 +2885,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const table = migrated .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'") .get() as { name: string } | undefined; @@ -2926,7 +2926,7 @@ describe("migration v67 drops orphan project auth tables", () => { migrated = new Database(fusion); migrated.init(); - expect(migrated.getSchemaVersion()).toBe(108); + expect(migrated.getSchemaVersion()).toBe(109); const tables = migrated .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'") .all() as Array<{ name: string }>; @@ -2953,7 +2953,7 @@ describe("migration v67 drops orphan project auth tables", () => { try { fresh.init(); - expect(fresh.getSchemaVersion()).toBe(108); + expect(fresh.getSchemaVersion()).toBe(109); const tables = fresh .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'") .all() as Array<{ name: string }>; diff --git a/packages/core/src/__tests__/goals-schema.test.ts b/packages/core/src/__tests__/goals-schema.test.ts index 333a68e551..03398b9515 100644 --- a/packages/core/src/__tests__/goals-schema.test.ts +++ b/packages/core/src/__tests__/goals-schema.test.ts @@ -91,6 +91,6 @@ describe("goals schema", () => { }); it("reports schema version 101", () => { - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); }); diff --git a/packages/core/src/__tests__/insight-store.test.ts b/packages/core/src/__tests__/insight-store.test.ts index 7aa0e611ad..5d8ac9b47a 100644 --- a/packages/core/src/__tests__/insight-store.test.ts +++ b/packages/core/src/__tests__/insight-store.test.ts @@ -1000,7 +1000,7 @@ describe("Migration: pre-33 DB upgrade", () => { // Step 1: Create a fresh database at v33 (runs all migrations up to 33) const db1 = createDatabase(legacyDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(108); + expect(db1.getSchemaVersion()).toBe(109); db1.close(); // Step 2: Manually downgrade to version 32 and drop insight tables @@ -1035,7 +1035,7 @@ describe("Migration: pre-33 DB upgrade", () => { expect(tableNamesBefore).not.toContain("project_insight_runs"); // Now run init — this triggers the v32→v33 migration db3.init(); - expect(db3.getSchemaVersion()).toBe(108); + expect(db3.getSchemaVersion()).toBe(109); // Step 4: Verify insight tables exist after migration const tablesAfter = db3.prepare( @@ -1066,12 +1066,12 @@ describe("Migration: pre-33 DB upgrade", () => { try { const db1 = createDatabase(testDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(108); + expect(db1.getSchemaVersion()).toBe(109); db1.close(); const db2 = createDatabase(testDir); expect(() => db2.init()).not.toThrow(); - expect(db2.getSchemaVersion()).toBe(108); + expect(db2.getSchemaVersion()).toBe(109); db2.close(); } finally { rmSync(testDir, { recursive: true, force: true }); @@ -1085,7 +1085,7 @@ describe("Migration: pre-33 DB upgrade", () => { // Step 1: Create a fresh DB and run migrations const db1 = createDatabase(compatDir); db1.init(); - expect(db1.getSchemaVersion()).toBe(108); + expect(db1.getSchemaVersion()).toBe(109); // Step 2: Strip lifecycle and cancelledAt columns by recreating the // table without them. This simulates a DB that was created before the diff --git a/packages/core/src/__tests__/merge-request-record.test.ts b/packages/core/src/__tests__/merge-request-record.test.ts index ef2a10d136..64dc82817b 100644 --- a/packages/core/src/__tests__/merge-request-record.test.ts +++ b/packages/core/src/__tests__/merge-request-record.test.ts @@ -38,7 +38,7 @@ describe("TaskStore merge request record + completion handoff marker", () => { .all() as Array<{ name: string }>; expect(tableRows).toEqual([{ name: "completion_handoff_markers" }, { name: "merge_requests" }]); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("upserts merge request records", async () => { diff --git a/packages/core/src/__tests__/mission-store.test.ts b/packages/core/src/__tests__/mission-store.test.ts index 7410346c67..2ded53b090 100644 --- a/packages/core/src/__tests__/mission-store.test.ts +++ b/packages/core/src/__tests__/mission-store.test.ts @@ -3746,7 +3746,7 @@ describe("MissionStore", () => { describe("Loop State & Validator Run Schema (v31)", () => { it("schema version is 101 after migration", () => { - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); it("mission_features table has loop state columns", () => { diff --git a/packages/core/src/__tests__/run-audit.test.ts b/packages/core/src/__tests__/run-audit.test.ts index 3be684ea9a..ca428b6914 100644 --- a/packages/core/src/__tests__/run-audit.test.ts +++ b/packages/core/src/__tests__/run-audit.test.ts @@ -584,7 +584,7 @@ describe("Run Audit", () => { }); it("schema version is bumped to 40", () => { - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); }); }); }); diff --git a/packages/core/src/__tests__/store-merge-queue.test.ts b/packages/core/src/__tests__/store-merge-queue.test.ts index 6c09641156..96a9ea905b 100644 --- a/packages/core/src/__tests__/store-merge-queue.test.ts +++ b/packages/core/src/__tests__/store-merge-queue.test.ts @@ -60,7 +60,7 @@ describe("TaskStore merge queue", () => { expect.arrayContaining(["idx_mergeQueue_lease_ready", "idx_mergeQueue_leaseExpiresAt"]), ); - expect(store.getDatabase().getSchemaVersion()).toBe(108); + expect(store.getDatabase().getSchemaVersion()).toBe(109); }); it("migrates a legacy v88 database and preserves task rows", async () => { diff --git a/packages/core/src/__tests__/task-documents.test.ts b/packages/core/src/__tests__/task-documents.test.ts index 7352ec08bf..63e5eebfad 100644 --- a/packages/core/src/__tests__/task-documents.test.ts +++ b/packages/core/src/__tests__/task-documents.test.ts @@ -51,7 +51,7 @@ describe("TaskStore task documents", () => { expect(tableNames.has("task_documents")).toBe(true); expect(tableNames.has("task_document_revisions")).toBe(true); - expect(db.getSchemaVersion()).toBe(108); + expect(db.getSchemaVersion()).toBe(109); const index = db .prepare( diff --git a/packages/core/src/__tests__/workflow-settings.test.ts b/packages/core/src/__tests__/workflow-settings.test.ts new file mode 100644 index 0000000000..6677b6806e --- /dev/null +++ b/packages/core/src/__tests__/workflow-settings.test.ts @@ -0,0 +1,307 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import { + validateSettingValuePatch, + resolveEffectiveSettingValues, + findOrphanedSettingValues, + WorkflowSettingRejectionError, +} from "../workflow-settings.js"; +import type { WorkflowSettingDefinition, WorkflowIrV2 } from "../workflow-ir-types.js"; +import { BUILTIN_WORKFLOW_SETTINGS } from "../builtin-workflow-settings.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +const BUILTIN_CODING = "builtin:coding"; +const PROJECT = "proj-1"; + +/** A minimal valid v2 IR carrying `settings` declarations — enough to round-trip + * through `parseWorkflowIr` / `createWorkflowDefinition`. */ +function makeIrWithSettings(settings: WorkflowSettingDefinition[]): WorkflowIrV2 { + return { + version: "v2", + name: "Custom WF", + columns: [], + nodes: [ + { id: "start", kind: "start" }, + { id: "end", kind: "end" }, + ], + edges: [{ from: "start", to: "end" }], + settings, + }; +} + +const TIMEOUT_DECL: WorkflowSettingDefinition = { + id: "workflowStepTimeoutMs", + name: "Step timeout (ms)", + type: "number", + default: 360_000, +}; +const FLAG_DECL: WorkflowSettingDefinition = { + id: "runStepsInNewSessions", + name: "Run steps in new sessions", + type: "boolean", + default: false, +}; +const ENUM_DECL: WorkflowSettingDefinition = { + id: "reviewHandoffPolicy", + name: "Review handoff policy", + type: "enum", + default: "disabled", + options: [ + { value: "disabled", label: "Disabled" }, + { value: "always", label: "Always" }, + ], +}; + +// ─────────────────────────────────────────────────────────────────────────── +// Validation core (side-effect-free) +// ─────────────────────────────────────────────────────────────────────────── + +describe("validateSettingValuePatch", () => { + const decls = [TIMEOUT_DECL, FLAG_DECL, ENUM_DECL]; + + it("accepts and normalizes valid values of each type", () => { + const res = validateSettingValuePatch(decls, { + workflowStepTimeoutMs: 1000, + runStepsInNewSessions: true, + reviewHandoffPolicy: "always", + }); + expect(res.rejections).toEqual([]); + expect(res.accepted).toEqual({ + workflowStepTimeoutMs: 1000, + runStepsInNewSessions: true, + reviewHandoffPolicy: "always", + }); + }); + + it("accepts null as a delete sentinel (null-as-delete)", () => { + const res = validateSettingValuePatch(decls, { workflowStepTimeoutMs: null }); + expect(res.rejections).toEqual([]); + expect(res.accepted).toEqual({ workflowStepTimeoutMs: null }); + }); + + it("rejects an unknown setting", () => { + const res = validateSettingValuePatch(decls, { nope: 1 }); + expect(res.accepted).toEqual({}); + expect(res.rejections).toHaveLength(1); + expect(res.rejections[0]).toMatchObject({ code: "unknown-setting", settingId: "nope" }); + }); + + it("rejects a type mismatch", () => { + const res = validateSettingValuePatch(decls, { workflowStepTimeoutMs: "fast" }); + expect(res.accepted).toEqual({}); + expect(res.rejections[0]).toMatchObject({ code: "type-mismatch", settingId: "workflowStepTimeoutMs" }); + }); + + it("rejects an enum violation", () => { + const res = validateSettingValuePatch(decls, { reviewHandoffPolicy: "sometimes" }); + expect(res.accepted).toEqual({}); + expect(res.rejections[0]).toMatchObject({ code: "enum-violation", settingId: "reviewHandoffPolicy" }); + }); + + it("reports no-settings-defined for a non-null write against empty declarations", () => { + const res = validateSettingValuePatch([], { workflowStepTimeoutMs: 1 }); + expect(res.accepted).toEqual({}); + expect(res.rejections[0]).toMatchObject({ code: "no-settings-defined" }); + }); + + it("accepts a delete even against empty declarations (clears stale rows)", () => { + const res = validateSettingValuePatch([], { workflowStepTimeoutMs: null }); + expect(res.rejections).toEqual([]); + expect(res.accepted).toEqual({ workflowStepTimeoutMs: null }); + }); + + it("reports every offending key (not fail-fast)", () => { + const res = validateSettingValuePatch(decls, { + workflowStepTimeoutMs: "x", + reviewHandoffPolicy: "x", + }); + expect(res.rejections).toHaveLength(2); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Effective resolution (drop-on-orphan, KTD-6) +// ─────────────────────────────────────────────────────────────────────────── + +describe("resolveEffectiveSettingValues", () => { + it("uses the stored value when it still validates", () => { + const eff = resolveEffectiveSettingValues([TIMEOUT_DECL], { workflowStepTimeoutMs: 1000 }); + expect(eff).toEqual({ workflowStepTimeoutMs: 1000 }); + }); + + it("falls to the declaration default when unset", () => { + const eff = resolveEffectiveSettingValues([TIMEOUT_DECL], {}); + expect(eff).toEqual({ workflowStepTimeoutMs: 360_000 }); + }); + + it("drops a stored value that no longer validates (enum→number retype) and uses the default", () => { + // Stored a string under what is now a number declaration. + const retyped: WorkflowSettingDefinition = { id: "x", name: "X", type: "number", default: 42 }; + const eff = resolveEffectiveSettingValues([retyped], { x: "stale-string" }); + expect(eff).toEqual({ x: 42 }); + }); + + it("drops stored values for ids with no current declaration", () => { + const eff = resolveEffectiveSettingValues([TIMEOUT_DECL], { removedSetting: 7 }); + expect(eff).toEqual({ workflowStepTimeoutMs: 360_000 }); + }); + + it("omits a setting with neither a valid value nor a default", () => { + const noDefault: WorkflowSettingDefinition = { id: "y", name: "Y", type: "number" }; + const eff = resolveEffectiveSettingValues([noDefault], {}); + expect(eff).toEqual({}); + }); +}); + +describe("findOrphanedSettingValues", () => { + it("surfaces values dropped by resolution (id + raw value) for the editor disclosure", () => { + const retyped: WorkflowSettingDefinition = { id: "x", name: "X", type: "number", default: 42 }; + const orphans = findOrphanedSettingValues([retyped], { x: "stale-string", removed: 9 }); + expect(orphans).toEqual([ + { id: "x", value: "stale-string" }, + { id: "removed", value: 9 }, + ]); + }); + + it("ignores null/undefined stored entries", () => { + const orphans = findOrphanedSettingValues([TIMEOUT_DECL], { workflowStepTimeoutMs: null }); + expect(orphans).toEqual([]); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Store write authority (U2 scenarios) +// ─────────────────────────────────────────────────────────────────────────── + +describe("TaskStore.updateWorkflowSettingValues", () => { + const harness = createTaskStoreTestHarness(); + beforeEach(harness.beforeEach); + afterEach(harness.afterEach); + + async function createCustomWorkflow(settings: WorkflowSettingDefinition[]): Promise { + const def = await harness.store().createWorkflowDefinition({ + name: "Custom WF", + ir: makeIrWithSettings(settings), + }); + return def.id; + } + + it("persists a valid value for a custom workflow and reads it back typed", async () => { + const store = harness.store(); + const wfId = await createCustomWorkflow([TIMEOUT_DECL, FLAG_DECL]); + + await store.updateWorkflowSettingValues(wfId, PROJECT, { + workflowStepTimeoutMs: 5000, + runStepsInNewSessions: true, + }); + + const stored = store.getWorkflowSettingValues(wfId, PROJECT); + expect(stored).toEqual({ workflowStepTimeoutMs: 5000, runStepsInNewSessions: true }); + expect(typeof stored.workflowStepTimeoutMs).toBe("number"); + expect(typeof stored.runStepsInNewSessions).toBe("boolean"); + }); + + it("accepts value writes for (builtin:coding, project) while builtin declaration edits stay rejected", async () => { + const store = harness.store(); + + // R4: value write for a built-in workflow succeeds. + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { requirePrApproval: true }); + expect(store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT)).toEqual({ requirePrApproval: true }); + + // Built-in DECLARATION edits remain rejected on the separate error path (KTD-2). + await expect( + store.updateWorkflowDefinition(BUILTIN_CODING, { ir: makeIrWithSettings([TIMEOUT_DECL]) }), + ).rejects.toThrow(/Built-in workflows cannot be edited/); + }); + + it("rejects type-mismatch / unknown-setting / enum-violation and persists nothing", async () => { + const store = harness.store(); + const wfId = await createCustomWorkflow([TIMEOUT_DECL, ENUM_DECL]); + + await expect( + store.updateWorkflowSettingValues(wfId, PROJECT, { workflowStepTimeoutMs: "fast" }), + ).rejects.toBeInstanceOf(WorkflowSettingRejectionError); + await expect( + store.updateWorkflowSettingValues(wfId, PROJECT, { unknownKey: 1 }), + ).rejects.toBeInstanceOf(WorkflowSettingRejectionError); + await expect( + store.updateWorkflowSettingValues(wfId, PROJECT, { reviewHandoffPolicy: "nope" }), + ).rejects.toBeInstanceOf(WorkflowSettingRejectionError); + + // Nothing was persisted by any rejected write. + expect(store.getWorkflowSettingValues(wfId, PROJECT)).toEqual({}); + }); + + it("treats null as delete and effective resolution falls to the declaration default", async () => { + const store = harness.store(); + const wfId = await createCustomWorkflow([TIMEOUT_DECL]); + + await store.updateWorkflowSettingValues(wfId, PROJECT, { workflowStepTimeoutMs: 5000 }); + expect(store.getWorkflowSettingValues(wfId, PROJECT)).toEqual({ workflowStepTimeoutMs: 5000 }); + + await store.updateWorkflowSettingValues(wfId, PROJECT, { workflowStepTimeoutMs: null }); + const stored = store.getWorkflowSettingValues(wfId, PROJECT); + expect(stored).toEqual({}); + + const def = await store.getWorkflowDefinition(wfId); + const decls = def!.ir.version === "v2" ? def!.ir.settings : undefined; + expect(resolveEffectiveSettingValues(decls, stored)).toEqual({ workflowStepTimeoutMs: 360_000 }); + }); + + it("retype enum→number with a stale stored string: effective resolution drops it, returns default, stored row untouched", async () => { + const store = harness.store(); + // Declare an enum setting and store a valid enum value. + const wfId = await createCustomWorkflow([ENUM_DECL]); + await store.updateWorkflowSettingValues(wfId, PROJECT, { reviewHandoffPolicy: "always" }); + expect(store.getWorkflowSettingValues(wfId, PROJECT)).toEqual({ reviewHandoffPolicy: "always" }); + + // Retype the same id to a number (declaration edit via the IR save path). + const retyped: WorkflowSettingDefinition = { + id: "reviewHandoffPolicy", + name: "Review handoff policy", + type: "number", + default: 99, + }; + await store.updateWorkflowDefinition(wfId, { ir: makeIrWithSettings([retyped]) }); + + // Stored row is UNTOUCHED — the stale string survives in storage. + const stored = store.getWorkflowSettingValues(wfId, PROJECT); + expect(stored).toEqual({ reviewHandoffPolicy: "always" }); + + // Effective resolution drops the stale string and returns the new default. + expect(resolveEffectiveSettingValues([retyped], stored)).toEqual({ reviewHandoffPolicy: 99 }); + }); + + it("cascade-deletes value rows when the custom workflow is deleted", async () => { + const store = harness.store(); + const wfId = await createCustomWorkflow([TIMEOUT_DECL]); + await store.updateWorkflowSettingValues(wfId, PROJECT, { workflowStepTimeoutMs: 5000 }); + await store.updateWorkflowSettingValues(wfId, "proj-2", { workflowStepTimeoutMs: 7000 }); + + await store.deleteWorkflowDefinition(wfId); + + expect(store.getWorkflowSettingValues(wfId, PROJECT)).toEqual({}); + expect(store.getWorkflowSettingValues(wfId, "proj-2")).toEqual({}); + }); + + it("a task pinned to a deleted workflow resolves built-in values", async () => { + const store = harness.store(); + // Built-in values for the project (these survive a custom-workflow delete). + await store.updateWorkflowSettingValues(BUILTIN_CODING, PROJECT, { requirePrApproval: true }); + + const wfId = await createCustomWorkflow([TIMEOUT_DECL]); + await store.updateWorkflowSettingValues(wfId, PROJECT, { workflowStepTimeoutMs: 5000 }); + await store.deleteWorkflowDefinition(wfId); + + // The deleted workflow's rows are gone; a task pinned to it degrades to + // builtin:coding (resolver) and reads built-in declarations + built-in values. + expect(store.getWorkflowSettingValues(wfId, PROJECT)).toEqual({}); + const effective = resolveEffectiveSettingValues( + BUILTIN_WORKFLOW_SETTINGS, + store.getWorkflowSettingValues(BUILTIN_CODING, PROJECT), + ); + expect(effective.requirePrApproval).toBe(true); + // Untouched built-in keys resolve to their declaration defaults. + expect(effective.workflowStepTimeoutMs).toBe(360_000); + }); +}); diff --git a/packages/core/src/db.ts b/packages/core/src/db.ts index 6403a186f1..8caf5b9118 100644 --- a/packages/core/src/db.ts +++ b/packages/core/src/db.ts @@ -149,7 +149,7 @@ export function probeFts5(db: DatabaseSync): boolean { // ── Schema Definition ──────────────────────────────────────────────── -const SCHEMA_VERSION = 108; +const SCHEMA_VERSION = 109; export { SCHEMA_VERSION }; @@ -616,6 +616,16 @@ CREATE TABLE IF NOT EXISTS workflow_run_step_instances ( ); CREATE INDEX IF NOT EXISTS idx_workflow_run_step_instances_task_run ON workflow_run_step_instances(taskId, runId); +-- Workflow setting values per (workflowId, projectId). JSON values map; validated +-- against the named workflow's declared settings by the store write authority. +CREATE TABLE IF NOT EXISTS workflow_settings ( + workflowId TEXT NOT NULL, + projectId TEXT NOT NULL, + "values" TEXT DEFAULT '{}', + updatedAt TEXT NOT NULL, + PRIMARY KEY (workflowId, projectId) +); + -- Task documents (key-value store per task with revision tracking) CREATE TABLE IF NOT EXISTS task_documents ( id TEXT PRIMARY KEY, @@ -4291,6 +4301,26 @@ export class Database { }); } + // Migration 109: Workflow setting values (workflow-settings U2, KTD-2). + // Adds workflow_settings — one row per (workflowId, projectId) carrying a JSON + // map of setting values declared by the workflow's IR. Values are validated by + // the store write authority against the named workflow's declarations; built-in + // workflow ids are accepted for value writes even though their declarations are + // non-editable. Additive-only, idempotent (table-exists guard); no backfill. + if (version < 109) { + this.applyMigration(109, () => { + this.db.exec(` + CREATE TABLE IF NOT EXISTS workflow_settings ( + workflowId TEXT NOT NULL, + projectId TEXT NOT NULL, + "values" TEXT DEFAULT '{}', + updatedAt TEXT NOT NULL, + PRIMARY KEY (workflowId, projectId) + ); + `); + }); + } + } /** @@ -4367,7 +4397,8 @@ export class Database { */ private addColumnIfMissing(table: string, column: string, definition: string): void { if (!this.hasColumn(table, column)) { - this.db.exec(`ALTER TABLE ${table} ADD COLUMN ${column} ${definition}`); + // Quote the column identifier so reserved words (e.g. `values`) are legal. + this.db.exec(`ALTER TABLE ${table} ADD COLUMN "${column}" ${definition}`); } } @@ -4385,7 +4416,8 @@ export class Database { return; } - this.db.exec(`ALTER TABLE ${table} ADD COLUMN ${column} ${definition}`); + // Quote the column identifier so reserved words (e.g. `values`) are legal. + this.db.exec(`ALTER TABLE ${table} ADD COLUMN "${column}" ${definition}`); columns.add(column); if (cache) { cache.set(table, columns); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4d99fe1da2..4b371c3d57 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -218,6 +218,20 @@ export type { CustomFieldPatchResult, FieldReconciliation, } from "./task-fields.js"; +export { + validateSettingValuePatch, + resolveEffectiveSettingValues, + findOrphanedSettingValues, + makeWorkflowSettingRejection, + WorkflowSettingRejectionError, + WORKFLOW_SETTING_REJECTION_CODES, +} from "./workflow-settings.js"; +export type { + WorkflowSettingRejection, + WorkflowSettingRejectionCode, + SettingValuePatchResult, + OrphanedSettingValue, +} from "./workflow-settings.js"; export { readTransitionPending, writeTransitionPending, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 352b281004..330c84c8e3 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -45,7 +45,7 @@ import { reconcileHooksRemaining, } from "./transition-pending.js"; import { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; -import type { WorkflowIr, WorkflowIrColumn, WorkflowFieldDefinition } from "./workflow-ir-types.js"; +import type { WorkflowIr, WorkflowIrColumn, WorkflowFieldDefinition, WorkflowSettingDefinition } from "./workflow-ir-types.js"; import { validateCustomFieldPatch, applyFieldDefaults, @@ -53,6 +53,12 @@ import { CustomFieldRejectionError, type CustomFieldRejection, } from "./task-fields.js"; +import { + validateSettingValuePatch, + resolveEffectiveSettingValues, + WorkflowSettingRejectionError, + type WorkflowSettingRejection, +} from "./workflow-settings.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"; @@ -68,6 +74,8 @@ import type { } from "./workflow-definition-types.js"; import { compileWorkflowToSteps } from "./workflow-compiler.js"; import { BUILTIN_WORKFLOWS, getBuiltinWorkflow, isBuiltinWorkflowId } from "./builtin-workflows.js"; +import { resolveWorkflowIrById } from "./workflow-ir-resolver.js"; +import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js"; import { WORKFLOW_PARITY_OBSERVED_MUTATION, WORKFLOW_PARITY_DRIFT_MUTATION, @@ -6964,6 +6972,98 @@ export class TaskStore extends EventEmitter { }); } + // ── Workflow setting values (U2, R2/R4, KTD-2/KTD-9) ─────────────────────── + // + // Setting VALUES persist per `(workflowId, projectId)` in the `workflow_settings` + // table; declarations live in the named workflow's IR (built-in or custom). This + // is the single validating write authority: values are validated against the + // NAMED workflow's declarations (not the project's current default workflow), and + // invalid values are NEVER persisted. Built-in workflow ids are accepted for + // value writes even though built-in DECLARATIONS are non-editable + // (`updateWorkflowDefinition` still rejects built-in edits) — the two error paths + // stay distinct (KTD-2). + + /** Resolve the setting DECLARATIONS for a workflow id (built-in or custom). The + * built-in path mirrors the IR resolver (`resolveWorkflowIrById`): built-in ids + * resolve through the same code path so value writes target the same schema the + * engine resolver sees. For built-in ids whose resolved IR does not yet carry + * `settings` (the linear `BUILTIN_WORKFLOWS` graphs predate the settings + * declarations), fall back to the canonical built-in declaration catalog + * (`BUILTIN_WORKFLOW_SETTINGS`) so built-in VALUE writes succeed (R4/KTD-2). + * Returns `undefined` when the workflow is missing or declares no settings. */ + private async resolveWorkflowSettingDeclarations( + workflowId: string, + ): Promise { + const ir = await resolveWorkflowIrById(this, workflowId); + const declared = ir.version === "v2" ? ir.settings : undefined; + if (declared && declared.length > 0) return declared; + // Built-in workflows declare the full moved-key catalog (the migration parity + // anchor); their selectable graphs may not embed it yet. + if (isBuiltinWorkflowId(workflowId)) return BUILTIN_WORKFLOW_SETTINGS; + return declared; + } + + /** Read the raw stored setting-value map for `(workflowId, projectId)`. Returns + * an empty object when no row exists. Raw (pre drop-on-orphan) — callers that + * need engine-effective values run {@link resolveEffectiveSettingValues}. */ + getWorkflowSettingValues(workflowId: string, projectId: string): Record { + const row = this.db + .prepare('SELECT "values" FROM workflow_settings WHERE workflowId = ? AND projectId = ?') + .get(workflowId, projectId) as { values: string } | undefined; + if (!row) return {}; + try { + const parsed = JSON.parse(row.values) as unknown; + return parsed && typeof parsed === "object" && !Array.isArray(parsed) + ? (parsed as Record) + : {}; + } catch { + return {}; + } + } + + /** + * Write setting VALUES for `(workflowId, projectId)`. The patch is validated + * against the NAMED workflow's declarations via {@link validateSettingValuePatch}; + * on ANY rejection nothing is persisted (write-boundary contract) and a typed + * {@link WorkflowSettingRejectionError} is thrown. Accepted keys merge into the + * stored row; a `null` value deletes the key (null-as-delete). Built-in workflow + * value writes succeed (R4). + */ + async updateWorkflowSettingValues( + workflowId: string, + projectId: string, + patch: Record, + ): Promise> { + const declarations = await this.resolveWorkflowSettingDeclarations(workflowId); + const result = validateSettingValuePatch(declarations, patch); + if (result.rejections.length > 0) { + // Invalid values are NEVER persisted — fail the whole write loudly. + throw new WorkflowSettingRejectionError(result.rejections); + } + + const current = this.getWorkflowSettingValues(workflowId, projectId); + const next: Record = { ...current }; + for (const [key, value] of Object.entries(result.accepted)) { + if (value === null) { + delete next[key]; + } else { + next[key] = value; + } + } + + const now = new Date().toISOString(); + this.db + .prepare( + `INSERT INTO workflow_settings (workflowId, projectId, "values", updatedAt) + VALUES (?, ?, ?, ?) + ON CONFLICT(workflowId, projectId) + DO UPDATE SET "values" = excluded."values", updatedAt = excluded.updatedAt`, + ) + .run(workflowId, projectId, JSON.stringify(next), now); + this.db.bumpLastModified(); + return next; + } + /** * The body of {@link updateTask} WITHOUT acquiring the per-task lock. Callers * that already hold `withTaskLock(id)` — e.g. workflow-selection mutations @@ -12498,6 +12598,12 @@ ${stepsSection}`; } this.workflowDefinitionsCache = null; + // Cascade (KTD-9): delete this workflow's setting-value rows across all + // projects. Tasks pinned to the deleted workflow degrade to `builtin:coding` + // via the resolver and read built-in declarations + built-in values, so no + // unreachable orphan value rows remain. + this.db.prepare("DELETE FROM workflow_settings WHERE workflowId = ?").run(id); + // Cascade: clear the project default when it pointed at this workflow. try { if ((await this.getDefaultWorkflowId()) === id) { diff --git a/packages/core/src/workflow-settings.ts b/packages/core/src/workflow-settings.ts new file mode 100644 index 0000000000..e1d47e89c5 --- /dev/null +++ b/packages/core/src/workflow-settings.ts @@ -0,0 +1,339 @@ +/** + * Workflow setting-value validation & effective-resolution authority (U2, R2/R4). + * + * Workflows declare typed settings ({@link WorkflowSettingDefinition}); setting + * *values* live per `(workflowId, projectId)` in the `workflow_settings` table (a + * JSON object keyed by setting id). This module is the single, side-effect-free + * validation core that the store write authority + * (`updateWorkflowSettingValues`) delegates to. It mirrors `task-fields.ts`: a + * flat, JSON-safe typed rejection with a machine-stable `code`, the offending + * `settingId`, and a non-localized `detail` string for audit/logs. + * + * Two operations: + * - {@link validateSettingValuePatch} — validate a `Record` + * patch against a setting schema, normalizing accepted values. `null`/`undefined` + * in the patch is a delete sentinel for that setting (always accepted). + * - {@link resolveEffectiveSettingValues} — compose stored values + declaration + * defaults into the effective value map, implementing DROP-ON-ORPHAN (KTD-6). + * + * KTD-6 — DELIBERATE DIVERGENCE FROM `task-fields.ts`. The custom-field reconciler + * (`reconcileFieldsOnWorkflowChange`) RETAINS orphaned values and surfaces them in + * a UI disclosure — safe for display data. Workflow settings are POLICY the engine + * consumes (a retyped enum→number setting with a stale string value would feed + * garbage into execution), so effective resolution DROPS any stored value that no + * longer validates against the current declaration and falls to the declaration + * `default`. The dropped raw values never reach the engine; the editor surfaces + * them via {@link findOrphanedSettingValues} for the U6 disclosure. + */ + +import type { + WorkflowSettingDefinition, +} from "./workflow-ir-types.js"; + +// --------------------------------------------------------------------------- +// Typed rejection (TransitionRejection-style: flat, JSON-safe, no class) +// --------------------------------------------------------------------------- + +/** + * Reason codes for a rejected setting-value 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. Mirrors + * {@link import("./task-fields.js").CustomFieldRejectionCode}. + */ +export type WorkflowSettingRejectionCode = + | "no-settings-defined" + | "unknown-setting" + | "type-mismatch" + | "enum-violation"; + +/** The full, immutable set of setting-value rejection codes. */ +export const WORKFLOW_SETTING_REJECTION_CODES: readonly WorkflowSettingRejectionCode[] = [ + "no-settings-defined", + "unknown-setting", + "type-mismatch", + "enum-violation", +] as const; + +/** + * A typed setting-value rejection. Flat and JSON-safe by construction — mirrors + * {@link import("./task-fields.js").CustomFieldRejection}. + * + * - `code` — machine-stable {@link WorkflowSettingRejectionCode}. + * - `settingId` — the offending setting id (the patch key that failed). + * - `message` — non-localized diagnostic context for audit/logs. + */ +export interface WorkflowSettingRejection { + code: WorkflowSettingRejectionCode; + settingId: string; + message: string; +} + +/** Result of validating a setting-value patch. */ +export interface SettingValuePatchResult { + /** The accepted, normalized values (a `null` entry is a delete sentinel). */ + accepted: Record; + /** The rejected keys with their typed reasons. */ + rejections: WorkflowSettingRejection[]; +} + +/** Construct a {@link WorkflowSettingRejection}. */ +export function makeWorkflowSettingRejection( + code: WorkflowSettingRejectionCode, + settingId: string, + message: string, +): WorkflowSettingRejection { + return { code, settingId, message }; +} + +/** + * Thrown by the throw-based store write path when a setting-value write rejects. + * Mirrors {@link import("./task-fields.js").CustomFieldRejectionError}: carries + * the structured rejection(s) so HTTP/agent surfaces can recover the setting path + * and code. + */ +export class WorkflowSettingRejectionError extends Error { + readonly rejections: WorkflowSettingRejection[]; + constructor(rejections: WorkflowSettingRejection[]) { + const first = rejections[0]; + super( + first + ? `workflow setting '${first.settingId}' rejected (${first.code}): ${first.message}` + : "workflow setting value write rejected", + ); + this.name = "WorkflowSettingRejectionError"; + this.rejections = rejections; + } +} + +// --------------------------------------------------------------------------- +// Per-type value validation +// --------------------------------------------------------------------------- + +/** True iff `value` is an option-value member of `setting.options`. */ +function isEnumMember(setting: WorkflowSettingDefinition, value: string): boolean { + return (setting.options ?? []).some((o) => o.value === value); +} + +/** + * Validate (and normalize) a single non-null value against a setting's type. + * Returns the normalized value on success, or a rejection. The caller has already + * resolved the setting definition. + */ +function validateValue( + setting: WorkflowSettingDefinition, + value: unknown, +): { ok: true; value: unknown } | { ok: false; rejection: WorkflowSettingRejection } { + const reject = ( + code: WorkflowSettingRejectionCode, + message: string, + ): { ok: false; rejection: WorkflowSettingRejection } => ({ + ok: false, + rejection: makeWorkflowSettingRejection(code, setting.id, message), + }); + + switch (setting.type) { + case "string": + case "text": { + if (typeof value !== "string") { + return reject("type-mismatch", `setting '${setting.id}' expects a string, got ${typeof value}`); + } + return { ok: true, value }; + } + case "number": { + if (typeof value !== "number" || !Number.isFinite(value)) { + return reject( + "type-mismatch", + `setting '${setting.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", `setting '${setting.id}' expects a boolean, got ${typeof value}`); + } + return { ok: true, value }; + } + case "enum": { + if (typeof value !== "string") { + return reject("type-mismatch", `setting '${setting.id}' (enum) expects a string option value, got ${typeof value}`); + } + if (!isEnumMember(setting, value)) { + return reject("enum-violation", `setting '${setting.id}' value '${value}' is not a declared option`); + } + return { ok: true, value }; + } + case "multi-enum": { + if (!Array.isArray(value)) { + return reject("type-mismatch", `setting '${setting.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", `setting '${setting.id}' (multi-enum) members must be strings`); + } + if (!isEnumMember(setting, item)) { + return reject("enum-violation", `setting '${setting.id}' member '${item}' is not a declared option`); + } + if (seen.has(item)) { + return reject("enum-violation", `setting '${setting.id}' has duplicate member '${item}'`); + } + seen.add(item); + } + return { ok: true, value: [...value] as string[] }; + } + default: { + // Exhaustiveness guard — an unknown type cannot validate. + const _exhaustive: never = setting.type; + return reject("type-mismatch", `setting '${setting.id}' has unsupported type '${String(_exhaustive)}'`); + } + } +} + +// --------------------------------------------------------------------------- +// Patch validation authority +// --------------------------------------------------------------------------- + +/** + * Validate a setting-value `patch` against a workflow's `declarations`. + * + * - A `null`/`undefined` patch value is a DELETE sentinel: the setting's stored + * value should be removed. It is ALWAYS accepted (null-as-delete) and surfaces + * in `accepted` as `null` so the caller can apply the delete uniformly. + * - A non-null value is validated/normalized per the setting's type. + * - A patch key that names no declared setting → `unknown-setting`. + * - When `declarations` is undefined/empty and the patch carries any non-null key → + * that key is rejected `no-settings-defined`. (A delete against no declarations is + * harmless and accepted so stale rows can always be cleared.) + * + * Unlike the custom-field authority this is NOT fail-fast: every offending key is + * reported so the editor can render per-field errors while applying the rest. + */ +export function validateSettingValuePatch( + declarations: WorkflowSettingDefinition[] | undefined, + patch: Record, +): SettingValuePatchResult { + const byId = new Map((declarations ?? []).map((d) => [d.id, d])); + const accepted: Record = {}; + const rejections: WorkflowSettingRejection[] = []; + + for (const key of Object.keys(patch)) { + const value = patch[key]; + // null/undefined = delete this setting's value. Always accepted, even when the + // declaration is gone (lets the editor clear orphaned rows). + if (value === null || value === undefined) { + accepted[key] = null; + continue; + } + const setting = byId.get(key); + if (byId.size === 0) { + rejections.push( + makeWorkflowSettingRejection( + "no-settings-defined", + key, + "the named workflow declares no settings; no values may be written", + ), + ); + continue; + } + if (!setting) { + rejections.push( + makeWorkflowSettingRejection( + "unknown-setting", + key, + `setting '${key}' is not declared by the named workflow`, + ), + ); + continue; + } + const res = validateValue(setting, value); + if (!res.ok) { + rejections.push(res.rejection); + continue; + } + accepted[key] = res.value; + } + + return { accepted, rejections }; +} + +// --------------------------------------------------------------------------- +// Effective resolution (drop-on-orphan, KTD-6) +// --------------------------------------------------------------------------- + +/** A stored value re-validates cleanly against the current declaration. */ +function valueStillValid(setting: WorkflowSettingDefinition, value: unknown): boolean { + if (value === null || value === undefined) return false; + return validateValue(setting, value).ok; +} + +/** An orphaned stored entry: a value that no longer validates against the current + * declaration (type change, enum option removed, declaration deleted). Surfaced to + * the U6 editor disclosure; never fed to the engine. */ +export interface OrphanedSettingValue { + id: string; + value: unknown; +} + +/** + * Resolve the EFFECTIVE setting values for a workflow from its `declarations` and + * the raw `stored` map, implementing DROP-ON-ORPHAN (KTD-6). + * + * For each declared setting: + * - if a stored value exists AND re-validates against the current declaration → + * use the stored value; + * - otherwise (no stored value, OR a stored value that no longer validates — + * type change, enum option removed) → DROP it and use the declaration `default` + * when one is present; absent declarations contribute nothing. + * + * Stored values for ids with NO current declaration (declaration deleted) are + * dropped entirely — they cannot reach the effective map. The raw `stored` row is + * never mutated here; this is a pure read. Use {@link findOrphanedSettingValues} + * to surface the dropped entries in the editor. + */ +export function resolveEffectiveSettingValues( + declarations: WorkflowSettingDefinition[] | undefined, + stored: Record | undefined, +): Record { + const storedMap = stored ?? {}; + const effective: Record = {}; + + for (const setting of declarations ?? []) { + const has = Object.prototype.hasOwnProperty.call(storedMap, setting.id); + const raw = has ? storedMap[setting.id] : undefined; + if (has && valueStillValid(setting, raw)) { + effective[setting.id] = raw; + continue; + } + // Drop-on-orphan / unset → declaration default (when present). + if (setting.default !== undefined) { + effective[setting.id] = setting.default; + } + } + + return effective; +} + +/** + * Compute the orphaned stored entries for the U6 editor disclosure: stored ids + * that either have no current declaration, or whose stored value no longer + * validates against the current declaration. These are exactly the entries + * {@link resolveEffectiveSettingValues} drops. The raw row is untouched. + */ +export function findOrphanedSettingValues( + declarations: WorkflowSettingDefinition[] | undefined, + stored: Record | undefined, +): OrphanedSettingValue[] { + const byId = new Map((declarations ?? []).map((d) => [d.id, d])); + const orphaned: OrphanedSettingValue[] = []; + + for (const [id, value] of Object.entries(stored ?? {})) { + if (value === null || value === undefined) continue; + const setting = byId.get(id); + if (!setting || !valueStillValid(setting, value)) { + orphaned.push({ id, value }); + } + } + + return orphaned; +}