fix(FN-1952): recover routines schedules merge
This commit is contained in:
@@ -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(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
const index = db
|
||||
.prepare(
|
||||
|
||||
@@ -11,7 +11,9 @@ import {
|
||||
validateBackupRetention,
|
||||
validateBackupDir,
|
||||
runBackupCommand,
|
||||
syncBackupRoutine,
|
||||
} from "./backup.js";
|
||||
import { RoutineStore } from "./routine-store.js";
|
||||
import type { ProjectSettings } from "./types.js";
|
||||
|
||||
describe("BackupManager", () => {
|
||||
@@ -438,6 +440,90 @@ describe("createBackupManager", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("syncBackupRoutine", () => {
|
||||
let tempDir: string;
|
||||
let routineStore: RoutineStore;
|
||||
|
||||
const baseSettings: ProjectSettings = {
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.useRealTimers();
|
||||
tempDir = mkdtempSync(join(tmpdir(), "kb-backup-routine-test-"));
|
||||
routineStore = new RoutineStore(tempDir);
|
||||
await routineStore.init();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("creates a command-backed routine for automatic database backups", async () => {
|
||||
const routine = await syncBackupRoutine(routineStore, {
|
||||
...baseSettings,
|
||||
autoBackupEnabled: true,
|
||||
autoBackupSchedule: "0 3 * * *",
|
||||
});
|
||||
|
||||
expect(routine).toBeDefined();
|
||||
expect(routine?.name).toBe("Database Backup");
|
||||
expect(routine?.trigger).toEqual({ type: "cron", cronExpression: "0 3 * * *" });
|
||||
expect(routine?.command).toBe("fn backup --create");
|
||||
expect(routine?.agentId).toBe("");
|
||||
expect(routine?.scope).toBe("project");
|
||||
});
|
||||
|
||||
it("updates the existing backup routine when settings change", async () => {
|
||||
await syncBackupRoutine(routineStore, {
|
||||
...baseSettings,
|
||||
autoBackupEnabled: true,
|
||||
autoBackupSchedule: "0 2 * * *",
|
||||
});
|
||||
|
||||
const updated = await syncBackupRoutine(routineStore, {
|
||||
...baseSettings,
|
||||
autoBackupEnabled: true,
|
||||
autoBackupSchedule: "30 4 * * *",
|
||||
});
|
||||
const routines = await routineStore.listRoutines();
|
||||
|
||||
expect(routines).toHaveLength(1);
|
||||
expect(updated?.trigger).toEqual({ type: "cron", cronExpression: "30 4 * * *" });
|
||||
expect(updated?.command).toBe("fn backup --create");
|
||||
expect(updated?.enabled).toBe(true);
|
||||
});
|
||||
|
||||
it("deletes the backup routine when automatic backups are disabled", async () => {
|
||||
await syncBackupRoutine(routineStore, {
|
||||
...baseSettings,
|
||||
autoBackupEnabled: true,
|
||||
autoBackupSchedule: "0 2 * * *",
|
||||
});
|
||||
|
||||
await syncBackupRoutine(routineStore, {
|
||||
...baseSettings,
|
||||
autoBackupEnabled: false,
|
||||
});
|
||||
|
||||
expect(await routineStore.listRoutines()).toEqual([]);
|
||||
});
|
||||
|
||||
it("rejects invalid backup schedules before creating a routine", async () => {
|
||||
await expect(syncBackupRoutine(routineStore, {
|
||||
...baseSettings,
|
||||
autoBackupEnabled: true,
|
||||
autoBackupSchedule: "bad-cron",
|
||||
})).rejects.toThrow("Invalid backup schedule");
|
||||
|
||||
expect(await routineStore.listRoutines()).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runBackupCommand", () => {
|
||||
let tempDir: string;
|
||||
let kbDir: string;
|
||||
|
||||
@@ -445,3 +445,53 @@ export async function syncBackupAutomation(
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Synchronizes the database backup routine with project settings.
|
||||
*
|
||||
* This is the combined routine/schedule model: cron scheduling comes from the
|
||||
* routine trigger, while the backup action is stored as a command on the same
|
||||
* record.
|
||||
*/
|
||||
export async function syncBackupRoutine(
|
||||
routineStore: import("./routine-store.js").RoutineStore,
|
||||
settings: ProjectSettings,
|
||||
): Promise<import("./routine.js").Routine | undefined> {
|
||||
const { RoutineStore } = await import("./routine-store.js");
|
||||
|
||||
const routines = await routineStore.listRoutines();
|
||||
const existingRoutine = routines.find((routine) => routine.name === BACKUP_SCHEDULE_NAME);
|
||||
|
||||
if (!settings.autoBackupEnabled) {
|
||||
if (existingRoutine) {
|
||||
await routineStore.deleteRoutine(existingRoutine.id);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const schedule = settings.autoBackupSchedule || "0 2 * * *";
|
||||
if (!RoutineStore.isValidCron(schedule)) {
|
||||
throw new Error(`Invalid backup schedule: ${schedule}`);
|
||||
}
|
||||
|
||||
const command = "fn backup --create";
|
||||
const input = {
|
||||
name: BACKUP_SCHEDULE_NAME,
|
||||
description: "Automatic database backup based on project settings",
|
||||
agentId: "",
|
||||
trigger: { type: "cron" as const, cronExpression: schedule },
|
||||
command,
|
||||
enabled: true,
|
||||
scope: "project" as const,
|
||||
};
|
||||
|
||||
if (existingRoutine) {
|
||||
return await routineStore.updateRoutine(existingRoutine.id, {
|
||||
trigger: input.trigger,
|
||||
command,
|
||||
enabled: true,
|
||||
});
|
||||
}
|
||||
|
||||
return await routineStore.createRoutine(input);
|
||||
}
|
||||
|
||||
@@ -119,7 +119,7 @@ describe("Database", () => {
|
||||
});
|
||||
|
||||
it("seeds schema version", () => {
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
});
|
||||
|
||||
it("seeds lastModified", () => {
|
||||
@@ -142,7 +142,7 @@ describe("Database", () => {
|
||||
|
||||
it("is idempotent - calling init() twice does not fail", () => {
|
||||
expect(() => db.init()).not.toThrow();
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
});
|
||||
|
||||
it("does not overwrite existing config on re-init", () => {
|
||||
@@ -749,7 +749,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29 (includes v1→v2 through v26→v29)
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -774,11 +774,11 @@ describe("schema migrations", () => {
|
||||
const db = new Database(kbDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
// Re-init should not fail
|
||||
db.init();
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -794,7 +794,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
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" }]);
|
||||
@@ -818,7 +818,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
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" }]);
|
||||
@@ -922,7 +922,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -1291,7 +1291,7 @@ describe("createDatabase factory", () => {
|
||||
const db = createDatabase(kbDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
expect(db.getLastModified()).toBeGreaterThan(0);
|
||||
|
||||
db.close();
|
||||
|
||||
@@ -59,7 +59,7 @@ export function fromJson<T>(json: string | null | undefined): T | undefined {
|
||||
|
||||
// ── Schema Definition ────────────────────────────────────────────────
|
||||
|
||||
const SCHEMA_VERSION = 35;
|
||||
const SCHEMA_VERSION = 36;
|
||||
|
||||
function normalizeTaskComments(
|
||||
steeringComments: SteeringComment[] | undefined,
|
||||
@@ -403,6 +403,9 @@ CREATE TABLE IF NOT EXISTS routines (
|
||||
description TEXT,
|
||||
triggerType TEXT NOT NULL,
|
||||
triggerConfig TEXT NOT NULL,
|
||||
command TEXT,
|
||||
steps TEXT,
|
||||
timeoutMs INTEGER,
|
||||
catchUpPolicy TEXT NOT NULL DEFAULT 'run_one',
|
||||
executionPolicy TEXT NOT NULL DEFAULT 'queue',
|
||||
catchUpLimit INTEGER DEFAULT 5,
|
||||
@@ -1085,6 +1088,9 @@ export class Database {
|
||||
description TEXT,
|
||||
triggerType TEXT NOT NULL,
|
||||
triggerConfig TEXT NOT NULL,
|
||||
command TEXT,
|
||||
steps TEXT,
|
||||
timeoutMs INTEGER,
|
||||
catchUpPolicy TEXT NOT NULL DEFAULT 'run_one',
|
||||
executionPolicy TEXT NOT NULL DEFAULT 'queue',
|
||||
catchUpLimit INTEGER DEFAULT 5,
|
||||
@@ -1449,6 +1455,15 @@ export class Database {
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
if (version < 36) {
|
||||
this.applyMigration(36, () => {
|
||||
this.addColumnIfMissing("routines", "command", "TEXT");
|
||||
this.addColumnIfMissing("routines", "steps", "TEXT");
|
||||
this.addColumnIfMissing("routines", "timeoutMs", "INTEGER");
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -137,6 +137,7 @@ export {
|
||||
validateBackupDir,
|
||||
runBackupCommand,
|
||||
syncBackupAutomation,
|
||||
syncBackupRoutine,
|
||||
BACKUP_SCHEDULE_NAME,
|
||||
} from "./backup.js";
|
||||
export type { BackupInfo, BackupOptions } from "./backup.js";
|
||||
|
||||
@@ -776,7 +776,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(35);
|
||||
expect(db1.getSchemaVersion()).toBe(36);
|
||||
db1.close();
|
||||
|
||||
// Step 2: Manually downgrade to version 32 and drop insight tables
|
||||
@@ -811,7 +811,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(35);
|
||||
expect(db3.getSchemaVersion()).toBe(36);
|
||||
|
||||
// Step 4: Verify insight tables exist after migration
|
||||
const tablesAfter = db3.prepare(
|
||||
@@ -842,12 +842,12 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
try {
|
||||
const db1 = createDatabase(testDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(35);
|
||||
expect(db1.getSchemaVersion()).toBe(36);
|
||||
db1.close();
|
||||
|
||||
const db2 = createDatabase(testDir);
|
||||
expect(() => db2.init()).not.toThrow();
|
||||
expect(db2.getSchemaVersion()).toBe(35);
|
||||
expect(db2.getSchemaVersion()).toBe(36);
|
||||
db2.close();
|
||||
} finally {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
|
||||
@@ -2544,7 +2544,7 @@ describe("MissionStore", () => {
|
||||
|
||||
describe("Loop State & Validator Run Schema (v31)", () => {
|
||||
it("schema version is 32 after migration", () => {
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
});
|
||||
|
||||
it("mission_features table has loop state columns", () => {
|
||||
|
||||
@@ -739,7 +739,7 @@ describe("RoadmapStore", () => {
|
||||
|
||||
describe("schema version", () => {
|
||||
it("schema version is 32 after init", () => {
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -109,6 +109,9 @@ export class RoutineStore extends EventEmitter<RoutineStoreEvents> {
|
||||
name: row.name,
|
||||
description: row.description || undefined,
|
||||
trigger,
|
||||
command: row.command || undefined,
|
||||
steps: fromJson<Routine["steps"]>(row.steps),
|
||||
timeoutMs: row.timeoutMs ?? undefined,
|
||||
catchUpPolicy: (row.catchUpPolicy as Routine["catchUpPolicy"]) || "run_one",
|
||||
executionPolicy: (row.executionPolicy as Routine["executionPolicy"]) || "queue",
|
||||
enabled: row.enabled === 1,
|
||||
@@ -148,10 +151,11 @@ export class RoutineStore extends EventEmitter<RoutineStoreEvents> {
|
||||
this.db.prepare(`
|
||||
INSERT OR REPLACE INTO routines (
|
||||
id, agentId, name, description, triggerType, triggerConfig,
|
||||
command, steps, timeoutMs,
|
||||
catchUpPolicy, executionPolicy, catchUpLimit, enabled,
|
||||
lastRunAt, lastRunResult, nextRunAt,
|
||||
runCount, runHistory, scope, createdAt, updatedAt
|
||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`).run(
|
||||
routine.id,
|
||||
routine.agentId,
|
||||
@@ -159,6 +163,9 @@ export class RoutineStore extends EventEmitter<RoutineStoreEvents> {
|
||||
routine.description ?? null,
|
||||
trigger.type,
|
||||
JSON.stringify(triggerConfig),
|
||||
routine.command ?? null,
|
||||
routine.steps ? JSON.stringify(routine.steps) : null,
|
||||
routine.timeoutMs ?? null,
|
||||
routine.catchUpPolicy,
|
||||
routine.executionPolicy,
|
||||
routine.catchUpLimit ?? 5,
|
||||
@@ -254,6 +261,9 @@ export class RoutineStore extends EventEmitter<RoutineStoreEvents> {
|
||||
name: input.name.trim(),
|
||||
description: input.description?.trim() || undefined,
|
||||
trigger: input.trigger,
|
||||
command: input.command?.trim() || undefined,
|
||||
steps: input.steps && input.steps.length > 0 ? input.steps : undefined,
|
||||
timeoutMs: input.timeoutMs,
|
||||
catchUpPolicy: input.catchUpPolicy ?? "run_one",
|
||||
executionPolicy: input.executionPolicy ?? "queue",
|
||||
enabled,
|
||||
@@ -316,6 +326,15 @@ export class RoutineStore extends EventEmitter<RoutineStoreEvents> {
|
||||
}
|
||||
routine.trigger = updates.trigger;
|
||||
}
|
||||
if (updates.command !== undefined) {
|
||||
routine.command = updates.command?.trim() || undefined;
|
||||
}
|
||||
if (updates.steps !== undefined) {
|
||||
routine.steps = updates.steps.length > 0 ? updates.steps : undefined;
|
||||
}
|
||||
if (updates.timeoutMs !== undefined) {
|
||||
routine.timeoutMs = updates.timeoutMs;
|
||||
}
|
||||
if (updates.catchUpPolicy !== undefined) {
|
||||
routine.catchUpPolicy = updates.catchUpPolicy;
|
||||
}
|
||||
@@ -405,17 +424,19 @@ export class RoutineStore extends EventEmitter<RoutineStoreEvents> {
|
||||
*/
|
||||
async completeRoutineExecution(
|
||||
id: string,
|
||||
meta: { completedAt: string; success: boolean; resultJson?: Record<string, unknown>; error?: string },
|
||||
meta: { completedAt: string; success: boolean; resultJson?: Record<string, unknown>; error?: string; output?: string; triggerType?: RoutineTriggerType; stepResults?: RoutineExecutionResult["stepResults"] },
|
||||
): Promise<void> {
|
||||
await this.withRoutineLock(id, async () => {
|
||||
const routine = await this.getRoutine(id);
|
||||
const result: RoutineExecutionResult = {
|
||||
routineId: id,
|
||||
success: meta.success,
|
||||
output: meta.success ? JSON.stringify(meta.resultJson ?? {}) : "",
|
||||
output: meta.output ?? (meta.success ? JSON.stringify(meta.resultJson ?? {}) : ""),
|
||||
error: meta.error,
|
||||
startedAt: routine.lastRunAt ?? meta.completedAt,
|
||||
completedAt: meta.completedAt,
|
||||
triggerType: meta.triggerType,
|
||||
stepResults: meta.stepResults,
|
||||
};
|
||||
|
||||
routine.lastRunAt = result.startedAt;
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* (cron, webhook, API, manual) with configurable execution and catch-up policies.
|
||||
*/
|
||||
|
||||
import type { AutomationRunResult } from "./automation.js";
|
||||
import type { AutomationRunResult, AutomationStep } from "./automation.js";
|
||||
|
||||
// ── Trigger Types ─────────────────────────────────────────────────────
|
||||
|
||||
@@ -115,6 +115,12 @@ export interface Routine {
|
||||
description?: string;
|
||||
/** The trigger configuration. */
|
||||
trigger: RoutineTrigger;
|
||||
/** Shell command to execute when this routine uses command action mode. */
|
||||
command?: string;
|
||||
/** Multi-step workflow to execute when present. */
|
||||
steps?: AutomationStep[];
|
||||
/** Per-routine execution timeout in milliseconds. */
|
||||
timeoutMs?: number;
|
||||
/** Catch-up policy for missed runs. Default: "run_one". */
|
||||
catchUpPolicy: RoutineCatchUpPolicy;
|
||||
/** Execution policy for concurrent runs. Default: "queue". */
|
||||
@@ -155,6 +161,12 @@ export interface RoutineCreateInput {
|
||||
description?: string;
|
||||
/** Trigger configuration. Required. */
|
||||
trigger: RoutineTrigger;
|
||||
/** Shell command action. Required when `steps` is omitted and no agent is assigned. */
|
||||
command?: string;
|
||||
/** Multi-step workflow action. When provided, `command` is ignored. */
|
||||
steps?: AutomationStep[];
|
||||
/** Per-routine execution timeout in milliseconds. */
|
||||
timeoutMs?: number;
|
||||
/** Catch-up policy. Default: "run_one". */
|
||||
catchUpPolicy?: RoutineCatchUpPolicy;
|
||||
/** Execution policy. Default: "queue". */
|
||||
@@ -173,6 +185,12 @@ export interface RoutineUpdateInput {
|
||||
description?: string;
|
||||
/** Trigger configuration. */
|
||||
trigger?: RoutineTrigger;
|
||||
/** Shell command action. */
|
||||
command?: string;
|
||||
/** Multi-step workflow action. */
|
||||
steps?: AutomationStep[];
|
||||
/** Per-routine execution timeout in milliseconds. */
|
||||
timeoutMs?: number;
|
||||
/** Catch-up policy. */
|
||||
catchUpPolicy?: RoutineCatchUpPolicy;
|
||||
/** Execution policy. */
|
||||
|
||||
@@ -465,7 +465,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 28", () => {
|
||||
expect(db.getSchemaVersion()).toBe(35);
|
||||
expect(db.getSchemaVersion()).toBe(36);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { defineConfig } from "vitest/config";
|
||||
import { availableParallelism } from "node:os";
|
||||
|
||||
const defaultMaxWorkers = Math.max(1, Math.min(2, Math.ceil(availableParallelism() / 8)));
|
||||
const defaultMaxWorkers = Math.max(1, Math.min(4, Math.ceil(availableParallelism() / 4)));
|
||||
const maxWorkers = Number.parseInt(process.env.VITEST_MAX_WORKERS ?? String(defaultMaxWorkers), 10);
|
||||
|
||||
export default defineConfig({
|
||||
|
||||
Reference in New Issue
Block a user