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) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
430
packages/cli/src/commands/__tests__/task-lock-retry.test.ts
Normal file
430
packages/cli/src/commands/__tests__/task-lock-retry.test.ts
Normal file
@@ -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<void> }> {
|
||||
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<void>((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<void>((resolve) => child.once("exit", () => resolve()));
|
||||
return;
|
||||
}
|
||||
child.stdin.write("RELEASE\n");
|
||||
await new Promise<void>((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<void> } }) => {
|
||||
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<void> } }) => {
|
||||
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<string, unknown>) {
|
||||
const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise<void> } }) => {
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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";
|
||||
|
||||
@@ -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<TaskStore> {
|
||||
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<ProjectContext> {
|
||||
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<ProjectContext | undefined> {
|
||||
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<Settings> = "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) {
|
||||
|
||||
117
packages/cli/src/lock-retry.ts
Normal file
117
packages/cli/src/lock-retry.ts
Normal file
@@ -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<void> {
|
||||
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<T>(
|
||||
operation: () => Promise<T>,
|
||||
context: RetryOnLockContext,
|
||||
totalMs: number = getCliLockRetryDeadlineMs(),
|
||||
): Promise<T> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user