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:
7
.changeset/fn-7731-task-cmd-lock-retry.md
Normal file
7
.changeset/fn-7731-task-cmd-lock-retry.md
Normal file
@@ -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.
|
||||||
@@ -577,6 +577,15 @@ fn task logs FN-001 --follow --limit 50 --type tool
|
|||||||
- unavailable-node policy value
|
- unavailable-node policy value
|
||||||
- source provenance line (`Source: <origin>`), including parent task / GitHub issue URL context when present
|
- source provenance line (`Source: <origin>`), 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
|
### Execution and status
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|||||||
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({}),
|
getStore: vi.fn().mockResolvedValue({}),
|
||||||
getDefaultProject: vi.fn().mockResolvedValue(undefined),
|
getDefaultProject: vi.fn().mockResolvedValue(undefined),
|
||||||
setDefaultProject: 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";
|
import { createInterface } from "node:readline/promises";
|
||||||
|
|||||||
@@ -12,8 +12,9 @@ import {
|
|||||||
isGhAvailable,
|
isGhAvailable,
|
||||||
runGhJsonAsync,
|
runGhJsonAsync,
|
||||||
} from "@fusion/core/gh-cli";
|
} 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 { findNodeByNameOrId } from "./node.js";
|
||||||
|
import { retryOnLock, LockRetryExhaustedError } from "../lock-retry.js";
|
||||||
|
|
||||||
const STEP_STATUSES: StepStatus[] = ["pending", "in-progress", "done", "skipped"];
|
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;
|
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> {
|
async function getProjectContext(projectName?: string): Promise<ProjectContext | undefined> {
|
||||||
if (projectName) {
|
if (projectName) {
|
||||||
return resolveProject(projectName);
|
return resolveProject(projectName);
|
||||||
@@ -780,7 +829,36 @@ export async function runTaskClearNode(id: string, projectName?: string) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export async function runTaskShow(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 task = await store.getTask(id);
|
||||||
const settings: Partial<Settings> = "getSettings" in store ? await store.getSettings() : {};
|
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);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
const store = await getStore(projectName);
|
// FNXC:CliBoardMutation 2026-07-09-00:00:
|
||||||
const task = await store.moveTask(id, column as Column);
|
// Same rationale as runTaskShow above: wrap project/store resolution
|
||||||
|
// (`getBoardCommandContext`, which can itself hit `database is locked`
|
||||||
console.log();
|
// inside `TaskStore.init()`) AND the write in one retryable unit, closing
|
||||||
console.log(` ✓ Moved ${task.id} → ${columnLabel(task.column)}`);
|
// the resolved store in an inner `finally` on every attempt. Only
|
||||||
console.log();
|
// `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) {
|
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);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -831,6 +831,11 @@ export {
|
|||||||
// FNXC:CoreTests 2026-06-25-16:30: test-only migrated-DB snapshot hook so
|
// 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.
|
// cross-package suites (dashboard route tests) can amortize db.init() cost.
|
||||||
setInMemoryTemplateSnapshot,
|
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";
|
} from "./db.js";
|
||||||
export {
|
export {
|
||||||
ProjectIdentityConflictError,
|
ProjectIdentityConflictError,
|
||||||
|
|||||||
Reference in New Issue
Block a user