From 7420abe80eb66908c094e31c2ff1ce6d18bee321 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 9 Jul 2026 10:06:17 -0700 Subject: [PATCH] FN-7731: add CLI-level lock retry for task show/move commands Add bounded exponential-backoff retry above the DB layer so `fn task show`/`fn task move` ride out transient SQLite lock contention instead of failing outright or hanging. - Add packages/cli/src/lock-retry.ts: retries a thunk on SQLite lock errors (via @fusion/core's isSqliteLockError) with exponential backoff capped by a wall-clock deadline (default 15s, override via FUSION_CLI_LOCK_RETRY_MS); non-lock errors propagate immediately; raises LockRetryExhaustedError on deadline exhaustion. - Wire lock-retry into packages/cli/src/commands/task.ts for the show/move task-store operations, and close the resolved TaskStore for deterministic exit. - Export isSqliteLockError from packages/core/src/index.ts for CLI reuse. - Add packages/cli/src/commands/__tests__/task-lock-retry.test.ts covering retry/backoff/deadline/error-passthrough behavior; extend task.test.ts. - Document the new behavior/env var in docs/cli-reference.md. - Add changeset (@runfusion/fusion: patch). Files changed: .changeset/fn-7731-task-cmd-lock-retry.md | 7 + docs/cli-reference.md | 9 + .../src/commands/__tests__/task-lock-retry.test.ts | 430 +++++++++++++++++++++ packages/cli/src/commands/__tests__/task.test.ts | 11 + packages/cli/src/commands/task.ts | 117 +++++- packages/cli/src/lock-retry.ts | 117 ++++++ packages/core/src/index.ts | 5 + 7 files changed, 688 insertions(+), 8 deletions(-) Fusion-Task-Id: FN-7731 Fusion-Task-Lineage: 85543164-10d6-4da9-8c3c-a84cd86827aa Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-7731-task-cmd-lock-retry.md | 7 + docs/cli-reference.md | 9 + .../__tests__/task-lock-retry.test.ts | 430 ++++++++++++++++++ .../cli/src/commands/__tests__/task.test.ts | 11 + packages/cli/src/commands/task.ts | 117 ++++- packages/cli/src/lock-retry.ts | 117 +++++ packages/core/src/index.ts | 5 + 7 files changed, 688 insertions(+), 8 deletions(-) create mode 100644 .changeset/fn-7731-task-cmd-lock-retry.md create mode 100644 packages/cli/src/commands/__tests__/task-lock-retry.test.ts create mode 100644 packages/cli/src/lock-retry.ts diff --git a/.changeset/fn-7731-task-cmd-lock-retry.md b/.changeset/fn-7731-task-cmd-lock-retry.md new file mode 100644 index 0000000000..c5645d3b40 --- /dev/null +++ b/.changeset/fn-7731-task-cmd-lock-retry.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: `fn task show`/`move` now retry through a momentarily locked board database instead of failing. +category: fix +dev: CLI-level bounded exponential backoff gated on SQLite lock errors (override via FUSION_CLI_LOCK_RETRY_MS); resolved TaskStore now closed for deterministic exit. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 2463feff5b..cea16834a9 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -577,6 +577,15 @@ fn task logs FN-001 --follow --limit 50 --type tool - unavailable-node policy value - source provenance line (`Source: `), including parent task / GitHub issue URL context when present +`fn task show`/`fn task move` retry-on-lock (FN-7731): if the board database +(`.fusion/fusion.db`) is momentarily locked by the engine or another agent, +both commands retry with bounded exponential backoff instead of failing +outright. If the lock hasn't cleared once the retry deadline (default 15s) +is reached, the command fails fast with a clear, actionable, non-zero-exit +error naming the task and operation rather than hanging. Override the +deadline with `FUSION_CLI_LOCK_RETRY_MS` (milliseconds). The resolved +`TaskStore` is always closed on exit so the CLI process exits promptly. + ### Execution and status ```bash diff --git a/packages/cli/src/commands/__tests__/task-lock-retry.test.ts b/packages/cli/src/commands/__tests__/task-lock-retry.test.ts new file mode 100644 index 0000000000..9206b6622d --- /dev/null +++ b/packages/cli/src/commands/__tests__/task-lock-retry.test.ts @@ -0,0 +1,430 @@ +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Regression coverage for FN-7731 — `fn task show`/`fn task move` must + * retry through a momentarily-locked SQLite board database instead of + * surfacing a raw `database is locked` error or hanging, and must always + * close the resolved `TaskStore` so the CLI process exits promptly. + * + * Two layers of coverage: + * 1. Unit-level tests against `retryOnLock` itself (fake timers, no real + * waits) proving the bounded-backoff/fast-fail/non-lock-passthrough + * contract in isolation. + * 2. An integration-level reproduction against a REAL `TaskStore`/SQLite + * database with a genuine external writer lock (a spawned Node + * subprocess holding `BEGIN IMMEDIATE`), driving `runTaskShow`/ + * `runTaskMove` exactly as the CLI would, proving the original + * `database is locked` symptom is gone end-to-end. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdtempSync } from "node:fs"; +import { rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; + +import { retryOnLock, LockRetryExhaustedError, DEFAULT_CLI_LOCK_RETRY_MS } from "../../lock-retry.js"; + +describe("retryOnLock", () => { + it("returns immediately on first-try success (no added latency)", async () => { + const op = vi.fn().mockResolvedValue("ok"); + const result = await retryOnLock(op, { id: "FN-1", action: "read task" }); + expect(result).toBe("ok"); + expect(op).toHaveBeenCalledTimes(1); + }); + + it("retries through a transient lock error and succeeds once it clears", async () => { + vi.useFakeTimers(); + try { + const lockError = new Error("database is locked"); + const op = vi + .fn() + .mockRejectedValueOnce(lockError) + .mockRejectedValueOnce(lockError) + .mockResolvedValueOnce("recovered"); + + const promise = retryOnLock(op, { id: "FN-2", action: "move task" }, 5_000); + // Drain backoff timers as they're scheduled without a fixed count, + // since exact intervals are an implementation detail. + for (let i = 0; i < 10 && op.mock.calls.length < 3; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + + const result = await promise; + expect(result).toBe("recovered"); + expect(op).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + + it("fails fast with an actionable error when the lock never clears within the bound", async () => { + vi.useFakeTimers(); + try { + const lockError = new Error("SQLITE_BUSY: database is locked"); + const op = vi.fn().mockRejectedValue(lockError); + + const promise = retryOnLock(op, { id: "FN-3", action: "move task" }, 1_000); + const assertion = expect(promise).rejects.toBeInstanceOf(LockRetryExhaustedError); + await vi.advanceTimersByTimeAsync(5_000); + await assertion; + + await expect(promise).rejects.toThrow(/FN-3/); + await expect(promise).rejects.toThrow(/move task/); + await expect(promise).rejects.toThrow(/FUSION_CLI_LOCK_RETRY_MS/); + } finally { + vi.useRealTimers(); + } + }); + + it("propagates a non-lock error immediately without retrying", async () => { + const notFound = new Error("Task FN-4 not found"); + const op = vi.fn().mockRejectedValue(notFound); + + await expect(retryOnLock(op, { id: "FN-4", action: "read task" }, 10_000)).rejects.toThrow( + "Task FN-4 not found", + ); + expect(op).toHaveBeenCalledTimes(1); + }); + + it("uses the default deadline when no override is supplied", () => { + expect(DEFAULT_CLI_LOCK_RETRY_MS).toBeGreaterThan(0); + }); +}); + +// ── Real-store integration reproduction ────────────────────────────────── + +function makeTmpDir(): string { + return mkdtempSync(join(tmpdir(), "fn-task-lock-retry-test-")); +} + +/** + * Spawn a subprocess that opens the given SQLite file, takes a real + * `BEGIN IMMEDIATE` writer lock, and holds it until told to release (or + * until `holdMs` elapses in timer mode). Mirrors the pattern used by + * `packages/core/src/__tests__/store-concurrent-writes.test.ts`. + */ +async function holdWriteLock( + dbPath: string, + options?: { holdMs?: number }, +): Promise<{ child: ChildProcessWithoutNullStreams; release: () => Promise }> { + const holdMs = options?.holdMs; + const releaseMode = holdMs !== undefined ? "timer" : "manual"; + const script = ` + const { DatabaseSync } = require("node:sqlite"); + const db = new DatabaseSync(${JSON.stringify(dbPath)}); + db.exec("PRAGMA busy_timeout = 0"); + db.exec("PRAGMA journal_mode = WAL"); + db.exec("BEGIN IMMEDIATE"); + process.stdout.write("LOCKED\\n"); + const release = () => { + try { db.exec("COMMIT"); } catch {} + try { db.close(); } catch {} + process.exit(0); + }; + if (${JSON.stringify(releaseMode)} === "timer") { + const signal = new Int32Array(new SharedArrayBuffer(4)); + Atomics.wait(signal, 0, 0, ${holdMs ?? 0}); + release(); + } else { + process.stdin.setEncoding("utf8"); + process.stdin.on("data", (chunk) => { + if (chunk.includes("RELEASE")) release(); + }); + } + `; + + const child = spawn(process.execPath, ["-e", script], { stdio: ["pipe", "pipe", "pipe"] }); + + const ready = new Promise((resolve, reject) => { + let stderr = ""; + child.stderr.on("data", (chunk) => { + stderr += chunk.toString(); + }); + child.stdout.on("data", (chunk) => { + if (chunk.toString().includes("LOCKED")) resolve(); + }); + child.once("exit", (code) => { + if (code !== 0) reject(new Error(`Lock helper exited early (${code}): ${stderr || "no stderr"}`)); + }); + child.once("error", reject); + }); + + await ready; + + return { + child, + release: async () => { + if (child.exitCode !== null || child.killed) return; + if (releaseMode === "timer") { + await new Promise((resolve) => child.once("exit", () => resolve())); + return; + } + child.stdin.write("RELEASE\n"); + await new Promise((resolve) => child.once("exit", () => resolve())); + }, + }; +} + +describe("fn task show / task move — real locked-store reproduction (FN-7731)", () => { + let tmpDir: string; + const originalRetryMs = process.env.FUSION_CLI_LOCK_RETRY_MS; + + beforeEach(() => { + tmpDir = makeTmpDir(); + vi.resetModules(); + }); + + afterEach(async () => { + if (originalRetryMs === undefined) { + delete process.env.FUSION_CLI_LOCK_RETRY_MS; + } else { + process.env.FUSION_CLI_LOCK_RETRY_MS = originalRetryMs; + } + await rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + }); + + it("succeeds when a real writer lock releases within the retry window", async () => { + const { TaskStore } = await import("@fusion/core"); + vi.doMock("../../project-context.js", () => ({ + resolveProject: vi.fn().mockRejectedValue(new Error("no registered project")), + closeProjectStore: async (context: { store: { close: () => Promise } }) => { + await context.store.close().catch(() => {}); + }, + })); + + const setupStore = new TaskStore(tmpDir); + await setupStore.init(); + const task = await setupStore.createTask({ description: "lock repro task" }); + await setupStore.close(); + + const dbPath = join(tmpDir, ".fusion", "fusion.db"); + // Hold the lock for a short window, well inside the overridden retry + // deadline, then release automatically (timer mode) — proving the + // retry path succeeds once the lock clears, per FN-5048 (no long real + // waits: short overridden bound + short real hold, not a slow test). + process.env.FUSION_CLI_LOCK_RETRY_MS = "8000"; + const lock = await holdWriteLock(dbPath, { holdMs: 400 }); + + try { + const { runTaskShow } = await import("../task.js"); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const cwd = process.cwd(); + process.chdir(tmpDir); + try { + await runTaskShow(task.id); + } finally { + process.chdir(cwd); + } + const printed = logSpy.mock.calls.flat().join("\n"); + expect(printed).toContain(task.id); + expect(errorSpy).not.toHaveBeenCalled(); + logSpy.mockRestore(); + errorSpy.mockRestore(); + } finally { + await lock.release().catch(() => {}); + } + }, 20_000); + + it("fails fast with a clear non-zero-exit error when the lock never releases (real busy_timeout, single attempt)", async () => { + // FNXC:CliBoardMutation 2026-07-09-00:00: + // Real SQLite's busy_timeout blocks synchronously at the C level for up + // to DEFAULT_SQLITE_BUSY_TIMEOUT_MS (5s, packages/core/src/db.ts) before + // a single attempt even returns control to JS, so a real end-to-end + // exhaustion repro cannot be made to fail fast without touching + // DB-level timeouts (forbidden by this task's scope). This test proves + // the invariant holds for ONE such blocking attempt: the raw + // `database is locked` never reaches the operator unformatted, the + // command still fails with a clear, actionable, non-zero-exit error, + // and the store is closed. Bounded exhaustion behavior across MANY fast + // attempts (the realistic CLI-layer retry shape) is covered by the + // mocked-store tests below per FN-5048 (no long real waits there). + const { TaskStore } = await import("@fusion/core"); + vi.doMock("../../project-context.js", () => ({ + resolveProject: vi.fn().mockRejectedValue(new Error("no registered project")), + closeProjectStore: async (context: { store: { close: () => Promise } }) => { + await context.store.close().catch(() => {}); + }, + })); + + const setupStore = new TaskStore(tmpDir); + await setupStore.init(); + const task = await setupStore.createTask({ description: "lock exhaustion repro task" }); + await setupStore.close(); + + const dbPath = join(tmpDir, ".fusion", "fusion.db"); + // Deadline shorter than a single DB-level busy_timeout attempt (~5s) so + // the very first retry check already sees the deadline exceeded. + process.env.FUSION_CLI_LOCK_RETRY_MS = "600"; + const lock = await holdWriteLock(dbPath); + + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + const { runTaskShow } = await import("../task.js"); + const cwd = process.cwd(); + process.chdir(tmpDir); + try { + await expect(runTaskShow(task.id)).rejects.toThrow(/process\.exit\(1\)/); + } finally { + process.chdir(cwd); + } + const printed = errorSpy.mock.calls.flat().join("\n"); + // Never a raw, un-retried "database is locked" with no context. + expect(printed).not.toMatch(/^\s*database is locked\s*$/im); + expect(printed).toMatch(/locked|retry|FUSION_CLI_LOCK_RETRY_MS/i); + expect(printed).toContain(task.id); + } finally { + exitSpy.mockRestore(); + errorSpy.mockRestore(); + await lock.release().catch(() => {}); + } + }, 20_000); +}); + +describe("runTaskShow / runTaskMove — mocked-store lock exhaustion, not-found, and teardown (FN-7731)", () => { + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.doUnmock("../../project-context.js"); + vi.restoreAllMocks(); + delete process.env.FUSION_CLI_LOCK_RETRY_MS; + }); + + async function loadWithMockedStore(store: Record) { + const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise } }) => { + await context.store.close?.().catch(() => {}); + }); + const resolveProject = vi.fn().mockResolvedValue({ + projectId: "proj_test", + projectPath: "/proj", + projectName: "proj", + isRegistered: true, + store, + }); + vi.doMock("../../project-context.js", () => ({ resolveProject, closeProjectStore })); + const mod = await import("../task.js"); + return { mod, closeProjectStore, resolveProject }; + } + + it("runTaskShow: bounded exhaustion across many fast lock retries fails clearly and closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "500"; + const getTask = vi.fn().mockRejectedValue(new Error("database is locked")); + const store = { getTask, close: vi.fn().mockResolvedValue(undefined) }; + const { mod, closeProjectStore } = await loadWithMockedStore(store); + + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const promise = mod.runTaskShow("FN-9"); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/); + for (let i = 0; i < 10 && getTask.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.advanceTimersByTimeAsync(1_000); + await assertion; + + expect(getTask.mock.calls.length).toBeGreaterThan(1); + const printed = errorSpy.mock.calls.flat().join("\n"); + expect(printed).toContain("FN-9"); + expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runTaskMove: bounded exhaustion across many fast lock retries fails clearly and closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "500"; + const moveTask = vi.fn().mockRejectedValue(new Error("SQLITE_BUSY: database is locked")); + const store = { moveTask, close: vi.fn().mockResolvedValue(undefined) }; + const { mod, closeProjectStore } = await loadWithMockedStore(store); + + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const promise = mod.runTaskMove("FN-10", "done"); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/); + for (let i = 0; i < 10 && moveTask.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.advanceTimersByTimeAsync(1_000); + await assertion; + + expect(moveTask.mock.calls.length).toBeGreaterThan(1); + const printed = errorSpy.mock.calls.flat().join("\n"); + expect(printed).toContain("FN-10"); + expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runTaskShow: a not-found error does not retry-loop and propagates clearly, store still closed", async () => { + process.env.FUSION_CLI_LOCK_RETRY_MS = "5000"; + const getTask = vi.fn().mockRejectedValue(new Error("Task FN-404 not found")); + const store = { getTask, close: vi.fn().mockResolvedValue(undefined) }; + const { mod, closeProjectStore } = await loadWithMockedStore(store); + + await expect(mod.runTaskShow("FN-404")).rejects.toThrow("Task FN-404 not found"); + expect(getTask).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalled(); + }); + + it("runTaskMove: a move-to-same-column no-op succeeds on the first attempt and closes the store", async () => { + const moveTask = vi.fn().mockResolvedValue({ id: "FN-5", column: "todo" }); + const store = { moveTask, close: vi.fn().mockResolvedValue(undefined) }; + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runTaskMove("FN-5", "todo"); + + expect(moveTask).toHaveBeenCalledTimes(1); + expect(moveTask).toHaveBeenCalledWith("FN-5", "todo"); + expect(closeProjectStore).toHaveBeenCalled(); + logSpy.mockRestore(); + }); + + it("runTaskShow: the happy path (no lock contention) adds no retry latency and closes the store once", async () => { + const getTask = vi.fn().mockResolvedValue({ + id: "FN-6", + description: "d", + column: "todo", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }); + const store = { getTask, close: vi.fn().mockResolvedValue(undefined) }; + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runTaskShow("FN-6"); + + expect(getTask).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + logSpy.mockRestore(); + }); +}); diff --git a/packages/cli/src/commands/__tests__/task.test.ts b/packages/cli/src/commands/__tests__/task.test.ts index 50b2c74dee..ec7863ab90 100644 --- a/packages/cli/src/commands/__tests__/task.test.ts +++ b/packages/cli/src/commands/__tests__/task.test.ts @@ -143,6 +143,17 @@ vi.mock("../../project-context.js", () => ({ getStore: vi.fn().mockResolvedValue({}), getDefaultProject: vi.fn().mockResolvedValue(undefined), setDefaultProject: vi.fn().mockResolvedValue(undefined), + // FNXC:CliBoardMutation 2026-07-09-00:00: FN-7731's runTaskShow/runTaskMove + // close the resolved store via closeProjectStore on every exit path; the + // real implementation is best-effort/tolerant of a store without a usable + // close(), so the mock mirrors that here rather than throwing. + closeProjectStore: vi.fn().mockImplementation(async (context: { store?: { close?: () => unknown } }) => { + try { + await context?.store?.close?.(); + } catch { + // best-effort, matches real closeProjectStore + } + }), })); import { createInterface } from "node:readline/promises"; diff --git a/packages/cli/src/commands/task.ts b/packages/cli/src/commands/task.ts index 0f90ad81a4..3cccc63706 100644 --- a/packages/cli/src/commands/task.ts +++ b/packages/cli/src/commands/task.ts @@ -12,8 +12,9 @@ import { isGhAvailable, runGhJsonAsync, } from "@fusion/core/gh-cli"; -import { resolveProject, type ProjectContext } from "../project-context.js"; +import { resolveProject, closeProjectStore, type ProjectContext } from "../project-context.js"; import { findNodeByNameOrId } from "./node.js"; +import { retryOnLock, LockRetryExhaustedError } from "../lock-retry.js"; const STEP_STATUSES: StepStatus[] = ["pending", "in-progress", "done", "skipped"]; @@ -175,6 +176,54 @@ async function getStore(projectName?: string): Promise { return (await getCommandContext(projectName)).store; } +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Resolve the FULL `ProjectContext` (not just a bare `TaskStore`, unlike + * `getStore`/`getCommandContext` above) for board read/write commands + * (`runTaskShow`/`runTaskMove`) so they can deterministically close+evict + * the store they open via `closeProjectStore` on every exit path — mirroring + * the FN-7704 `fn agent stop/start` teardown fix. Covers all three + * project-resolution branches `getCommandContext` covers (explicit + * `--project`, default-project, and CWD-detected/unregistered fallback via + * `new TaskStore(process.cwd())`), reusing `asLocalProjectContext` for the + * fallback so `closeProjectStore` always receives a well-formed context. + */ +async function getBoardCommandContext(projectName?: string): Promise { + if (projectName) { + const context = await resolveProject(projectName); + if (!context) { + throw new Error(`Project ${projectName} not found`); + } + return context; + } + + try { + const context = await resolveProject(undefined); + if (!context) { + throw new Error("No project context"); + } + return context; + } catch { + const store = new TaskStore(process.cwd()); + await store.init(); + return asLocalProjectContext(store); + } +} + +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Translate a `LockRetryExhaustedError` (or any other error) from a board + * read/write into the CLI's standard "print + exit(1)" failure shape. A + * lock-exhaustion error already carries an actionable message (task id, + * operation, and the `FUSION_CLI_LOCK_RETRY_MS` override), so it is printed + * as-is rather than re-wrapped. + */ +function failBoardCommand(error: unknown): never { + const message = error instanceof Error ? error.message : String(error); + console.error(`\n ✗ ${message}\n`); + process.exit(1); +} + async function getProjectContext(projectName?: string): Promise { if (projectName) { return resolveProject(projectName); @@ -780,7 +829,36 @@ export async function runTaskClearNode(id: string, projectName?: string) { } export async function runTaskShow(id: string, projectName?: string) { - const store = await getStore(projectName); + // FNXC:CliBoardMutation 2026-07-09-00:00: + // Wrap the ENTIRE flow — project/store resolution (`getBoardCommandContext`, + // which can itself hit `database is locked` inside `TaskStore.init()` for + // the CWD-detected/unregistered fallback branch) AND the board read — in a + // single retryable unit, not just `store.getTask`. Each attempt resolves a + // fresh context and closes it in an inner `finally` before the next retry, + // so a failed attempt never leaks a store handle. Only SQLite lock errors + // are retried (`retryOnLock`); not-found and other errors propagate + // immediately without looping. + try { + await retryOnLock( + async () => { + const context = await getBoardCommandContext(projectName); + try { + await runTaskShowWithStore(id, context.store); + } finally { + await closeProjectStore(context); + } + }, + { id, action: "read task" }, + ); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + failBoardCommand(error); + } + throw error; + } +} + +async function runTaskShowWithStore(id: string, store: TaskStore) { const task = await store.getTask(id); const settings: Partial = "getSettings" in store ? await store.getSettings() : {}; @@ -983,12 +1061,35 @@ export async function runTaskMove(id: string, column: string, projectName?: stri process.exit(1); } - const store = await getStore(projectName); - const task = await store.moveTask(id, column as Column); - - console.log(); - console.log(` ✓ Moved ${task.id} → ${columnLabel(task.column)}`); - console.log(); + // FNXC:CliBoardMutation 2026-07-09-00:00: + // Same rationale as runTaskShow above: wrap project/store resolution + // (`getBoardCommandContext`, which can itself hit `database is locked` + // inside `TaskStore.init()`) AND the write in one retryable unit, closing + // the resolved store in an inner `finally` on every attempt. Only + // `database is locked`/SQLITE_BUSY|LOCKED errors are retried; a genuinely + // invalid move (bad column, missing task) propagates immediately without + // looping. + try { + await retryOnLock( + async () => { + const context = await getBoardCommandContext(projectName); + try { + const task = await context.store.moveTask(id, column as Column); + console.log(); + console.log(` ✓ Moved ${task.id} → ${columnLabel(task.column)}`); + console.log(); + } finally { + await closeProjectStore(context); + } + }, + { id, action: "move task" }, + ); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + failBoardCommand(error); + } + throw error; + } } export async function runTaskDuplicate(id: string, projectName?: string) { diff --git a/packages/cli/src/lock-retry.ts b/packages/cli/src/lock-retry.ts new file mode 100644 index 0000000000..571e9aeb7a --- /dev/null +++ b/packages/cli/src/lock-retry.ts @@ -0,0 +1,117 @@ +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * `fn task show`/`fn task move` (FN-7731, upstream #1976) open a `TaskStore` + * and call `getTask`/`moveTask` exactly once. If the engine or another agent + * holds a SQLite writer lock on `.fusion/fusion.db` at that instant, the + * call surfaces a raw `database is locked` error (or appears to hang until + * the DB layer's own bounded `busy_timeout`/lock-recovery window in + * packages/core/src/db.ts — DEFAULT_SQLITE_BUSY_TIMEOUT_MS = 5s plus a short + * lock-recovery retry — finally gives up). That DB-level bound already + * prevents an unbounded hang at the SQLite layer, but it does not retry + * across separate `better-sqlite3`/node:sqlite statement calls, so a single + * unlucky read/write at the CLI surface still fails outright even though + * the lock typically clears within a second or two of normal engine + * activity. + * + * This module adds a CLI-level retry ABOVE that bound: it retries a thunk + * only when the error is classified as a SQLite lock error (reusing + * `@fusion/core`'s `isSqliteLockError`, the same classifier the DB layer's + * own `runWithLockRecovery` uses, so CLI and DB lock detection never drift), + * with exponential backoff capped by a total wall-clock deadline. Non-lock + * errors (not-found, invalid column, etc.) propagate immediately — they are + * never retried. On deadline exhaustion the command fails fast with a + * clear, actionable, non-zero-exit error naming the task id and operation + * instead of hanging indefinitely. The default deadline is intentionally + * generous (long enough to ride out a typical engine write) while still + * bounded, and is operator-overridable via `FUSION_CLI_LOCK_RETRY_MS` for + * constrained/CI environments. + * + * Do NOT widen `@fusion/core`'s DB-level busy_timeout/lock-recovery window + * to "fix" this — that bound is intentionally tight so a single genuinely + * stuck writer does not block the whole process; this retry belongs at the + * CLI surface, above the DB-level bound, not inside it. + */ +import { isSqliteLockError } from "@fusion/core"; + +/** Default total wall-clock deadline (ms) for the lock-retry loop. */ +export const DEFAULT_CLI_LOCK_RETRY_MS = 15_000; + +const INITIAL_BACKOFF_MS = 100; +const MAX_BACKOFF_MS = 2_000; + +/** Raised when a retried operation exhausts the bounded lock-retry deadline. */ +export class LockRetryExhaustedError extends Error { + readonly cause: unknown; + + constructor(message: string, cause: unknown) { + super(message); + this.name = "LockRetryExhaustedError"; + this.cause = cause; + } +} + +/** + * Read the operator-overridable total retry deadline from + * `FUSION_CLI_LOCK_RETRY_MS`, falling back to `DEFAULT_CLI_LOCK_RETRY_MS` + * for an unset/invalid value. + */ +export function getCliLockRetryDeadlineMs(): number { + const raw = process.env.FUSION_CLI_LOCK_RETRY_MS; + if (!raw) return DEFAULT_CLI_LOCK_RETRY_MS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : DEFAULT_CLI_LOCK_RETRY_MS; +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +export interface RetryOnLockContext { + /** Task id the operation targets, used only for the error message. */ + id: string; + /** Human-readable action name (e.g. "read task", "move task"), used only for the error message. */ + action: string; +} + +/** + * Run `operation`, retrying with bounded exponential backoff ONLY when the + * thrown error is a SQLite lock error (`isSqliteLockError`). Any other + * error (not-found, invalid column, etc.) propagates on the first attempt + * without retrying. On deadline exhaustion, throws `LockRetryExhaustedError` + * naming `context.id`/`context.action` with actionable guidance. + */ +export async function retryOnLock( + operation: () => Promise, + context: RetryOnLockContext, + totalMs: number = getCliLockRetryDeadlineMs(), +): Promise { + const deadline = Date.now() + totalMs; + let backoff = INITIAL_BACKOFF_MS; + + for (;;) { + try { + return await operation(); + } catch (error) { + if (!isSqliteLockError(error)) { + throw error; + } + + const now = Date.now(); + if (now >= deadline) { + throw new LockRetryExhaustedError( + `Timed out after ${totalMs}ms waiting to ${context.action} for ${context.id}: ` + + `the board database stayed locked (the engine or another agent is writing). ` + + `Retry the command, or raise the bound via FUSION_CLI_LOCK_RETRY_MS.`, + error, + ); + } + + const remaining = deadline - now; + const wait = Math.min(backoff, remaining, MAX_BACKOFF_MS); + await sleep(wait); + backoff = Math.min(backoff * 2, MAX_BACKOFF_MS); + } + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c470af7fcc..8c2a414cc1 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -831,6 +831,11 @@ export { // FNXC:CoreTests 2026-06-25-16:30: test-only migrated-DB snapshot hook so // cross-package suites (dashboard route tests) can amortize db.init() cost. setInMemoryTemplateSnapshot, + // FNXC:CliBoardMutation 2026-07-09-00:00: exported so the CLI-level + // lock-retry wrapper (packages/cli/src/lock-retry.ts, FN-7731) can classify + // SQLite lock errors identically to the DB layer's own runWithLockRecovery, + // instead of re-implementing (and risking drift in) the detection regex. + isSqliteLockError, } from "./db.js"; export { ProjectIdentityConflictError,