feat(core): workflow setting values store — per-(workflow,project) table, validating write authority, drop-on-orphan resolution
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 }>;
|
||||
|
||||
@@ -91,6 +91,6 @@ describe("goals schema", () => {
|
||||
});
|
||||
|
||||
it("reports schema version 101", () => {
|
||||
expect(db.getSchemaVersion()).toBe(108);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -584,7 +584,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 40", () => {
|
||||
expect(db.getSchemaVersion()).toBe(108);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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(
|
||||
|
||||
307
packages/core/src/__tests__/workflow-settings.test.ts
Normal file
307
packages/core/src/__tests__/workflow-settings.test.ts
Normal file
@@ -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<string> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<TaskStoreEvents> {
|
||||
});
|
||||
}
|
||||
|
||||
// ── 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<WorkflowSettingDefinition[] | undefined> {
|
||||
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<string, unknown> {
|
||||
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<string, unknown>)
|
||||
: {};
|
||||
} 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<string, unknown>,
|
||||
): Promise<Record<string, unknown>> {
|
||||
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<string, unknown> = { ...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) {
|
||||
|
||||
339
packages/core/src/workflow-settings.ts
Normal file
339
packages/core/src/workflow-settings.ts
Normal file
@@ -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<string, unknown>`
|
||||
* 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<string, unknown>;
|
||||
/** 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<string>();
|
||||
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<string, unknown>,
|
||||
): SettingValuePatchResult {
|
||||
const byId = new Map<string, WorkflowSettingDefinition>((declarations ?? []).map((d) => [d.id, d]));
|
||||
const accepted: Record<string, unknown> = {};
|
||||
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<string, unknown> | undefined,
|
||||
): Record<string, unknown> {
|
||||
const storedMap = stored ?? {};
|
||||
const effective: Record<string, unknown> = {};
|
||||
|
||||
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<string, unknown> | undefined,
|
||||
): OrphanedSettingValue[] {
|
||||
const byId = new Map<string, WorkflowSettingDefinition>((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;
|
||||
}
|
||||
Reference in New Issue
Block a user