FN-7734: generalize lock-retry and store-close pattern across all fn task subcommands
Applies the FN-7731 retryOnLock + closeProjectStore pattern to the ~26 runTask* handlers in task.ts so every 'fn task' subcommand retries a momentarily locked board database and exits promptly instead of hanging or leaking a store handle. - Generalize retryOnLock/closeProjectStore usage across all runTask* handlers in packages/cli/src/commands/task.ts - Honor FUSION_CLI_LOCK_RETRY_MS for configurable retry timing - Close both cached and uncached CWD-fallback stores on exit - Multi-step flows (create/retry/delete/merge/imports) retry each discrete write independently instead of retrying the whole flow - Add task-lock-retry.test.ts covering the new retry/close behavior across subcommands - Update docs/cli-reference.md and task.test.ts for the new behavior - Add changeset fn-7734-task-cmd-lock-retry-generalized.md (patch) Files changed: .../fn-7734-task-cmd-lock-retry-generalized.md | 7 + docs/cli-reference.md | 18 +- .../src/commands/__tests__/task-lock-retry.test.ts | 282 ++++ packages/cli/src/commands/__tests__/task.test.ts | 9 +- packages/cli/src/commands/task.ts | 1541 +++++++++++--------- 5 files changed, 1173 insertions(+), 684 deletions(-) Fusion-Task-Id: FN-7734 Fusion-Task-Lineage: cd5c864b-8a33-4962-803e-bbb353a41085 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7734-task-cmd-lock-retry-generalized.md
Normal file
7
.changeset/fn-7734-task-cmd-lock-retry-generalized.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: All `fn task` subcommands now retry a momentarily locked board database and exit promptly instead of hanging or leaking.
|
||||
category: fix
|
||||
dev: Generalizes the FN-7731 CLI retryOnLock + closeProjectStore pattern across the ~26 runTask* handlers in packages/cli/src/commands/task.ts; honors FUSION_CLI_LOCK_RETRY_MS; closes both cached and uncached CWD-fallback stores; multi-step flows (create/retry/delete/merge/imports) retry each discrete write independently instead of the whole flow.
|
||||
@@ -577,14 +577,26 @@ fn task logs FN-001 --follow --limit 50 --type tool
|
||||
- unavailable-node policy value
|
||||
- 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
|
||||
Every `fn task` subcommand that touches the board retries on lock (FN-7731,
|
||||
generalized to all subcommands in FN-7734): 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
|
||||
the command retries 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.
|
||||
`TaskStore` is always closed on exit (success, not-found, or lock-exhaustion)
|
||||
so the CLI process exits promptly, for both a registered/cached project
|
||||
store and the uncached CWD-fallback resolution branch.
|
||||
|
||||
Multi-step commands (`fn task create`, `fn task retry`, `fn task delete`,
|
||||
`fn task merge`, the GitHub/GitLab bulk-import commands) retry each discrete
|
||||
board write independently rather than retrying the whole flow, so a lock
|
||||
error on a later step never redoes an already-committed earlier write (e.g.
|
||||
double-creating a task). Long-lived/interactive commands (`fn task plan`,
|
||||
interactive GitHub import, `fn task logs --follow`) keep their interactive
|
||||
prompt loop or tail session un-retried by design but still close the
|
||||
resolved store on every exit path, including on `Ctrl+C`.
|
||||
|
||||
### Execution and status
|
||||
|
||||
|
||||
@@ -428,3 +428,285 @@ describe("runTaskShow / runTaskMove — mocked-store lock exhaustion, not-found,
|
||||
logSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
// ── FN-7734: generalized coverage across the remaining `fn task` subcommands ──
|
||||
//
|
||||
// FNXC:CliBoardMutation 2026-07-09-00:00 (FN-7734):
|
||||
// Extends the FN-7731 pattern proven above for `runTaskShow`/`runTaskMove` to
|
||||
// representative commands from each Step-1-audit class: `runTaskUpdate`
|
||||
// (single-call board-mutation via `withBoardWrite`), `runTaskComments`
|
||||
// (single-call board-read via `withBoardWrite`), and `runTaskDelete`
|
||||
// (MULTI-STEP mutation via `resolveBoardContext`/`retryBoardCall` — existence
|
||||
// check, interactive confirm, terminal delete). Reproduces the Symptom
|
||||
// Verification invariant: (1) a lock released within the window succeeds
|
||||
// without surfacing `database is locked`; (2) a lock that never clears fails
|
||||
// fast with a clear, actionable, non-zero-exit error within a short bound
|
||||
// (fake timers, no real long waits per FN-5048); (3) a not-found error does
|
||||
// NOT retry-loop; (4) the resolved store is closed/evicted from
|
||||
// `storeCache` on success, not-found, and exhaustion paths, for BOTH the
|
||||
// cached (`resolveProject` mock below models a registered/cached store) and
|
||||
// the uncached CWD-fallback branch (`resolveProject` rejects, so
|
||||
// `getBoardCommandContext` falls through to the `asLocalProjectContext`
|
||||
// wrapper around a fresh, uncached `TaskStore`).
|
||||
describe("FN-7734: generalized retry+teardown across representative fn task subcommands", () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.doUnmock("../../project-context.js");
|
||||
vi.restoreAllMocks();
|
||||
delete process.env.FUSION_CLI_LOCK_RETRY_MS;
|
||||
});
|
||||
|
||||
/** Cached/registered-project store resolution branch (mirrors the existing mocked-store helper above). */
|
||||
async function loadWithCachedStore(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 };
|
||||
}
|
||||
|
||||
/** Uncached CWD-fallback store resolution branch: `resolveProject` rejects for both the explicit-name and default-project paths, forcing `getBoardCommandContext`'s catch branch (`asLocalProjectContext` wrapping a fresh store). */
|
||||
async function loadWithUncachedFallbackStore(store: Record<string, unknown>) {
|
||||
const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise<void> } }) => {
|
||||
await context.store.close?.().catch(() => {});
|
||||
});
|
||||
const resolveProject = vi.fn().mockRejectedValue(new Error("no registered project"));
|
||||
vi.doMock("../../project-context.js", () => ({
|
||||
resolveProject,
|
||||
closeProjectStore,
|
||||
}));
|
||||
vi.doMock("@fusion/core", async () => {
|
||||
const actual = await vi.importActual<typeof import("@fusion/core")>("@fusion/core");
|
||||
return {
|
||||
...actual,
|
||||
TaskStore: class {
|
||||
async init() {}
|
||||
async close() {
|
||||
await store.close?.();
|
||||
}
|
||||
constructor() {
|
||||
return new Proxy(store, {
|
||||
get(target, prop) {
|
||||
if (prop === "init") return async () => {};
|
||||
return (target as Record<string, unknown>)[prop as string];
|
||||
},
|
||||
});
|
||||
}
|
||||
},
|
||||
};
|
||||
});
|
||||
const mod = await import("../task.js");
|
||||
return { mod, closeProjectStore, resolveProject };
|
||||
}
|
||||
|
||||
describe("runTaskUpdate (single-call board-mutation)", () => {
|
||||
it("retries through a transient lock and succeeds once it clears, closing the store", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "5000";
|
||||
const lockError = new Error("database is locked");
|
||||
const updateStep = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(lockError)
|
||||
.mockResolvedValueOnce({ id: "FN-20", steps: [{ name: "step0", status: "done" }] });
|
||||
const store = { updateStep, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithCachedStore(store);
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
const promise = mod.runTaskUpdate("FN-20", "0", "done");
|
||||
for (let i = 0; i < 10 && updateStep.mock.calls.length < 2; i++) {
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
}
|
||||
await promise;
|
||||
|
||||
expect(updateStep).toHaveBeenCalledTimes(2);
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
logSpy.mockRestore();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("fails fast on lock-exhaustion with an actionable error, non-zero exit, and closes the store", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "500";
|
||||
const updateStep = vi.fn().mockRejectedValue(new Error("database is locked"));
|
||||
const store = { updateStep, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithCachedStore(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.runTaskUpdate("FN-21", "0", "done");
|
||||
const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/);
|
||||
for (let i = 0; i < 10 && updateStep.mock.calls.length < 2; i++) {
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
}
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
await assertion;
|
||||
|
||||
expect(updateStep.mock.calls.length).toBeGreaterThan(1);
|
||||
const printed = errorSpy.mock.calls.flat().join("\n");
|
||||
expect(printed).not.toMatch(/^\s*database is locked\s*$/im);
|
||||
expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i);
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
|
||||
exitSpy.mockRestore();
|
||||
errorSpy.mockRestore();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("a not-found error does not retry-loop and closes the store (uncached CWD-fallback branch)", async () => {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "5000";
|
||||
const updateStep = vi.fn().mockRejectedValue(new Error("Task FN-22 not found"));
|
||||
const store = { updateStep, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithUncachedFallbackStore(store);
|
||||
|
||||
await expect(mod.runTaskUpdate("FN-22", "0", "done")).rejects.toThrow("Task FN-22 not found");
|
||||
expect(updateStep).toHaveBeenCalledTimes(1);
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("runTaskComments (single-call board-read)", () => {
|
||||
it("retries through a transient lock and succeeds once it clears, closing the store", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "5000";
|
||||
const lockError = new Error("SQLITE_BUSY: database is locked");
|
||||
const getTask = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(lockError)
|
||||
.mockResolvedValueOnce({ id: "FN-23", comments: [{ id: "c1", author: "user", text: "hi", createdAt: new Date().toISOString() }] });
|
||||
const store = { getTask, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithCachedStore(store);
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
const promise = mod.runTaskComments("FN-23");
|
||||
for (let i = 0; i < 10 && getTask.mock.calls.length < 2; i++) {
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
}
|
||||
await promise;
|
||||
|
||||
expect(getTask).toHaveBeenCalledTimes(2);
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
logSpy.mockRestore();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("the happy path (no lock contention, uncached CWD-fallback branch) adds no retry latency and closes the store once", async () => {
|
||||
const getTask = vi.fn().mockResolvedValue({ id: "FN-24", comments: [] });
|
||||
const store = { getTask, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithUncachedFallbackStore(store);
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
await mod.runTaskComments("FN-24");
|
||||
|
||||
expect(getTask).toHaveBeenCalledTimes(1);
|
||||
expect(closeProjectStore).toHaveBeenCalledTimes(1);
|
||||
logSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("runTaskDelete (MULTI-STEP mutation: existence check + confirm + terminal delete)", () => {
|
||||
it("retries the terminal delete write through a transient lock without redoing the existence check, and closes the store", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "5000";
|
||||
const getTask = vi.fn().mockResolvedValue({ id: "FN-25" });
|
||||
const lockError = new Error("database is locked");
|
||||
const deleteTask = vi.fn().mockRejectedValueOnce(lockError).mockResolvedValueOnce(undefined);
|
||||
const store = { getTask, deleteTask, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithCachedStore(store);
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
const promise = mod.runTaskDelete("FN-25", true);
|
||||
for (let i = 0; i < 10 && deleteTask.mock.calls.length < 2; i++) {
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
}
|
||||
await promise;
|
||||
|
||||
// Existence check ran exactly once — a LATER step's lock error must not
|
||||
// redo an earlier, already-succeeded step.
|
||||
expect(getTask).toHaveBeenCalledTimes(1);
|
||||
expect(deleteTask).toHaveBeenCalledTimes(2);
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
logSpy.mockRestore();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("fails fast on lock-exhaustion during the terminal delete with an actionable error, non-zero exit, and closes the store", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "500";
|
||||
const getTask = vi.fn().mockResolvedValue({ id: "FN-26" });
|
||||
const deleteTask = vi.fn().mockRejectedValue(new Error("database is locked"));
|
||||
const store = { getTask, deleteTask, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithCachedStore(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.runTaskDelete("FN-26", true);
|
||||
const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/);
|
||||
for (let i = 0; i < 10 && deleteTask.mock.calls.length < 2; i++) {
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
}
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
await assertion;
|
||||
|
||||
expect(deleteTask.mock.calls.length).toBeGreaterThan(1);
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
|
||||
exitSpy.mockRestore();
|
||||
errorSpy.mockRestore();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("a not-found error at the existence-check step does not retry-loop, and closes the store (uncached CWD-fallback branch)", async () => {
|
||||
process.env.FUSION_CLI_LOCK_RETRY_MS = "5000";
|
||||
const getTask = vi.fn().mockRejectedValue(new Error("Task FN-27 not found"));
|
||||
const deleteTask = vi.fn();
|
||||
const store = { getTask, deleteTask, close: vi.fn().mockResolvedValue(undefined) };
|
||||
const { mod, closeProjectStore } = await loadWithUncachedFallbackStore(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(() => {});
|
||||
|
||||
await expect(mod.runTaskDelete("FN-27", true)).rejects.toThrow(/process\.exit\(1\)/);
|
||||
|
||||
expect(getTask).toHaveBeenCalledTimes(1);
|
||||
expect(deleteTask).not.toHaveBeenCalled();
|
||||
expect(closeProjectStore).toHaveBeenCalled();
|
||||
|
||||
exitSpy.mockRestore();
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -980,7 +980,14 @@ describe("project-aware task command behavior", () => {
|
||||
expect.any(Function),
|
||||
);
|
||||
expect(sigintHandlers).toHaveLength(1);
|
||||
expect(() => sigintHandlers[0]()).toThrow("process.exit");
|
||||
// FNXC:CliBoardMutation 2026-07-09-00:00 (FN-7734): the SIGINT handler
|
||||
// now closes the resolved board store BEFORE exiting (previously it
|
||||
// called `process.exit(0)` synchronously with no teardown), so invoking
|
||||
// it no longer throws synchronously — it fires the close and exits once
|
||||
// that settles. Await a tick and assert `process.exit(0)` was reached.
|
||||
sigintHandlers[0]();
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
expect(exitSpy).toHaveBeenCalledWith(0);
|
||||
promise.catch(() => {});
|
||||
|
||||
expect(resolveProject).toHaveBeenCalledWith("demo-project");
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user