diff --git a/.changeset/fn-7739-cli-cmd-lock-retry.md b/.changeset/fn-7739-cli-cmd-lock-retry.md new file mode 100644 index 0000000000..72e7f528e9 --- /dev/null +++ b/.changeset/fn-7739-cli-cmd-lock-retry.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: `fn backup`/`memory-backup`/`mcp`/`db vacuum` now retry a locked board database and exit promptly instead of hanging. +category: fix +dev: Applies the FN-7731/FN-7738 CLI retryOnLock + closeProjectStore/asLocalProjectContext pattern to packages/cli/src/commands/backup.ts, memory-backup.ts, mcp.ts, and db.ts; closes cached, uncached CWD-fallback, and ad-hoc MCP secrets TaskStores on every exit path; retries MCP settings writes and DB VACUUM; honors FUSION_CLI_LOCK_RETRY_MS. GlobalSettingsStore is file-backed and left unchanged. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 73fc270e4f..fd22ff8d77 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -609,6 +609,23 @@ already-completed side effect. The resolved `TaskStore` — whether resolved from the registered/default project or the uncached CWD-fallback project — is always closed on exit so the CLI process exits promptly. +FN-7739 extends the same pattern to `fn backup *` (`create`/`list`/ +`restore`/`cleanup`), `fn memory-backup *` (`create`/`list`/`restore`), +`fn mcp *` (`list`/`add`/`edit`/`remove`/`enable`/`disable`/`import`/ +`export`/`validate`), and `fn db vacuum`. `fn backup`/`fn memory-backup` +retry their `getSettings()` board read; `fn mcp` retries project-scope +`updateSettings` writes (mutations) and reads, and closes BOTH the cached +project `TaskStore` and the ad-hoc uncached secrets `TaskStore` opened by +`--secret-ref`/`--create-secret-*` resolution when no project is in scope; +`fn db vacuum` retries the VACUUM call itself (VACUUM requires an exclusive +lock, the canonical transient-lock case) and closes the resolved store +BEFORE each `process.exit()` call, since `runDbVacuum` always exits +explicitly and a pending `finally` does not run after `process.exit()`. MCP +global-scope settings live in the file-backed `GlobalSettingsStore` +(`~/.fusion/settings.json`, no SQLite handle) and are intentionally left +with no close and no lock-retry. All of the above honor the same +`FUSION_CLI_LOCK_RETRY_MS` deadline override. + ### Execution and status ```bash diff --git a/packages/cli/src/commands/__tests__/backup-lock-retry.test.ts b/packages/cli/src/commands/__tests__/backup-lock-retry.test.ts new file mode 100644 index 0000000000..b286654401 --- /dev/null +++ b/packages/cli/src/commands/__tests__/backup-lock-retry.test.ts @@ -0,0 +1,202 @@ +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Regression coverage for FN-7739 — `fn backup *` must retry the discrete + * `getSettings()` board read through a momentarily-locked SQLite board + * database instead of surfacing a raw `database is locked` error, and must + * always close the resolved `TaskStore` (cached AND the uncached + * CWD-fallback branch) so the CLI process exits promptly. Mirrors the + * FN-7731/FN-7738 `*-lock-retry.test.ts` pattern: mocked-store lock + * exhaustion/not-found/teardown coverage (fast, fake-timer based, no real + * waits per FN-5048). + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@fusion/core", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + createBackupManager: vi.fn(), + runBackupCommand: vi.fn(async () => ({ success: true, output: "backup created" })), + }; +}); + +function makeStore(overrides: Record = {}) { + return { + getSettings: vi.fn(async () => ({ autoBackupDir: ".fusion/backups" })), + fusionDir: "/proj/.fusion", + close: vi.fn().mockResolvedValue(undefined), + ...overrides, + }; +} + +async function loadWithMockedStore(store: Record, opts?: { cached?: boolean }) { + const cached = opts?.cached ?? true; + const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise } }) => { + await context.store.close?.().catch(() => {}); + }); + const context = { + projectId: cached ? "proj_test" : process.cwd(), + projectPath: cached ? "/proj" : process.cwd(), + projectName: "proj", + isRegistered: cached, + store, + }; + const resolveProject = cached + ? vi.fn().mockResolvedValue(context) + : vi.fn().mockRejectedValue(new Error("no registered project")); + const asLocalProjectContext = vi.fn(() => context); + vi.doMock("../../project-context.js", () => ({ resolveProject, closeProjectStore, asLocalProjectContext })); + const mod = await import("../backup.js"); + const { createBackupManager } = await import("@fusion/core"); + return { mod, closeProjectStore, resolveProject, createBackupManager }; +} + +describe("fn backup * — lock retry, leak/close, and not-found teardown (FN-7739)", () => { + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.doUnmock("../../project-context.js"); + vi.restoreAllMocks(); + delete process.env.FUSION_CLI_LOCK_RETRY_MS; + }); + + it("runBackupList: succeeds on first attempt (no lock contention) and closes the store once", async () => { + const store = makeStore(); + const { mod, closeProjectStore, createBackupManager } = await loadWithMockedStore(store); + (createBackupManager as ReturnType).mockReturnValue({ listBackupPairs: vi.fn(async () => []) }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runBackupList(); + + expect(store.getSettings).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + logSpy.mockRestore(); + }); + + it("runBackupList (uncached CWD-fallback): resolves via asLocalProjectContext and still closes the store", async () => { + const store = makeStore(); + const { mod, closeProjectStore, createBackupManager } = await loadWithMockedStore(store, { cached: false }); + (createBackupManager as ReturnType).mockReturnValue({ listBackupPairs: vi.fn(async () => []) }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runBackupList(); + + expect(closeProjectStore).toHaveBeenCalledTimes(1); + logSpy.mockRestore(); + }); + + it("runBackupList: retries through a transient lock error on getSettings and succeeds once it clears, then closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "5000"; + const lockError = new Error("database is locked"); + const getSettings = vi.fn().mockImplementationOnce(() => { + throw lockError; + }).mockImplementation(async () => ({ autoBackupDir: ".fusion/backups" })); + const store = makeStore({ getSettings }); + const { mod, closeProjectStore, createBackupManager } = await loadWithMockedStore(store); + (createBackupManager as ReturnType).mockReturnValue({ listBackupPairs: vi.fn(async () => []) }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const promise = mod.runBackupList(); + for (let i = 0; i < 10 && getSettings.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await promise; + + expect(getSettings.mock.calls.length).toBeGreaterThan(1); + expect(closeProjectStore).toHaveBeenCalled(); + logSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runBackupCleanup: bounded exhaustion on a persistently locked getSettings fails clearly and closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "500"; + const getSettings = vi.fn().mockImplementation(() => { + throw new Error("SQLITE_BUSY: database is locked"); + }); + const store = makeStore({ getSettings }); + 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.runBackupCleanup(); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/); + for (let i = 0; i < 10 && getSettings.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.advanceTimersByTimeAsync(1_000); + await assertion; + + expect(getSettings.mock.calls.length).toBeGreaterThan(1); + const printed = errorSpy.mock.calls.flat().join("\n"); + expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runBackupRestore: a restore failure closes the store before exiting non-zero", async () => { + const store = makeStore(); + const { mod, closeProjectStore, createBackupManager } = await loadWithMockedStore(store); + (createBackupManager as ReturnType).mockReturnValue({ + restoreBackup: vi.fn().mockRejectedValue(new Error("bad backup file")), + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + await expect(mod.runBackupRestore("missing.db")).rejects.toThrow(/process\.exit\(1\)/); + + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + logSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + it("runBackupCreate: closes the store before process.exit on both success and failure", async () => { + const store = makeStore(); + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const { runBackupCommand } = await import("@fusion/core"); + (runBackupCommand as ReturnType).mockResolvedValue({ success: true, output: "ok" }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(mod.runBackupCreate()).rejects.toThrow(/process\.exit\(0\)/); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + logSpy.mockRestore(); + }); + + it("runBackupCreate: a non-lock exception from runBackupCommand itself still closes the store (FN-7739 review fix)", async () => { + const store = makeStore(); + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const { runBackupCommand } = await import("@fusion/core"); + (runBackupCommand as ReturnType).mockRejectedValue(new Error("disk full")); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(mod.runBackupCreate()).rejects.toThrow(/disk full/); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + + logSpy.mockRestore(); + }); +}); diff --git a/packages/cli/src/commands/__tests__/backup.test.ts b/packages/cli/src/commands/__tests__/backup.test.ts index acfe564b3b..d3a7b4e5bc 100644 --- a/packages/cli/src/commands/__tests__/backup.test.ts +++ b/packages/cli/src/commands/__tests__/backup.test.ts @@ -47,10 +47,25 @@ vi.mock("@fusion/core", () => ({ cleanupOldBackups: mockCleanupOldBackups, })), runBackupCommand: mockRunBackupCommand, + isSqliteLockError: (error: unknown) => /database is locked/i.test(error instanceof Error ? error.message : String(error)), })); vi.mock("../../project-context.js", () => ({ resolveProject: mockResolveProject, + closeProjectStore: vi.fn(async (context: { store: { close?: () => Promise } }) => { + try { + await context.store.close?.(); + } catch { + // best-effort, mirrors production closeProjectStore + } + }), + asLocalProjectContext: vi.fn((store: unknown) => ({ + projectId: process.cwd(), + projectPath: process.cwd(), + projectName: "current-project", + isRegistered: false, + store, + })), })); import { TaskStore } from "@fusion/core"; diff --git a/packages/cli/src/commands/__tests__/db-lock-retry.test.ts b/packages/cli/src/commands/__tests__/db-lock-retry.test.ts new file mode 100644 index 0000000000..f42979fbe3 --- /dev/null +++ b/packages/cli/src/commands/__tests__/db-lock-retry.test.ts @@ -0,0 +1,182 @@ +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Regression coverage for FN-7739 — `fn db vacuum` must retry the VACUUM + * call through a momentarily-locked SQLite board database (VACUUM requires + * an exclusive lock — the canonical transient-lock case) instead of + * surfacing a raw `database is locked` error, and must close the resolved + * `TaskStore` (cached AND the uncached CWD-fallback branch) BEFORE every + * `process.exit()` call (both success and failure paths), since a pending + * `finally` does not run after `process.exit()`. Fast, fake-timer based, no + * real waits per FN-5048. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@fusion/core", async (importActual) => { + const actual = await importActual(); + return { ...actual }; +}); + +function makeStore(overrides: Record = {}) { + return { + getDatabase: vi.fn(), + close: vi.fn().mockResolvedValue(undefined), + ...overrides, + }; +} + +async function loadWithMockedStore(store: Record, opts?: { cached?: boolean }) { + const cached = opts?.cached ?? true; + const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise } }) => { + await context.store.close?.().catch(() => {}); + }); + const context = { + projectId: cached ? "proj_test" : process.cwd(), + projectPath: cached ? "/proj" : process.cwd(), + projectName: "proj", + isRegistered: cached, + store, + }; + const resolveProject = cached + ? vi.fn().mockResolvedValue(context) + : vi.fn().mockRejectedValue(new Error("no registered project")); + const asLocalProjectContext = vi.fn(() => context); + vi.doMock("../../project-context.js", () => ({ resolveProject, closeProjectStore, asLocalProjectContext })); + const mod = await import("../db.js"); + return { mod, closeProjectStore, resolveProject }; +} + +describe("fn db vacuum — lock retry and close-before-exit teardown (FN-7739)", () => { + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.doUnmock("../../project-context.js"); + vi.restoreAllMocks(); + delete process.env.FUSION_CLI_LOCK_RETRY_MS; + }); + + it("succeeds on first attempt (no lock contention) and closes the store before exit(0)", async () => { + const vacuum = vi.fn().mockReturnValue({ beforeSize: 100, afterSize: 50, durationMs: 5 }); + const getDatabase = vi.fn(() => ({ vacuum, getPath: () => "/proj/.fusion/fusion.db" })); + const store = makeStore({ getDatabase }); + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(mod.runDbVacuum()).rejects.toThrow(/process\.exit\(0\)/); + + expect(vacuum).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + exitSpy.mockRestore(); + logSpy.mockRestore(); + }); + + it("uncached CWD-fallback: resolves via asLocalProjectContext and still closes the store before exit", async () => { + const vacuum = vi.fn().mockReturnValue({ beforeSize: 0, afterSize: 0, durationMs: 0 }); + const getDatabase = vi.fn(() => ({ vacuum, getPath: () => "/fallback/.fusion/fusion.db" })); + const store = makeStore({ getDatabase }); + const { mod, closeProjectStore } = await loadWithMockedStore(store, { cached: false }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(mod.runDbVacuum()).rejects.toThrow(/process\.exit\(0\)/); + + expect(closeProjectStore).toHaveBeenCalledTimes(1); + exitSpy.mockRestore(); + logSpy.mockRestore(); + }); + + it("retries VACUUM through a transient lock error and succeeds once it clears, closing the store before exit(0)", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "5000"; + const lockError = new Error("database is locked"); + const vacuum = vi.fn().mockImplementationOnce(() => { + throw lockError; + }).mockReturnValue({ beforeSize: 10, afterSize: 5, durationMs: 1 }); + const getDatabase = vi.fn(() => ({ vacuum, getPath: () => "/proj/.fusion/fusion.db" })); + const store = makeStore({ getDatabase }); + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const promise = mod.runDbVacuum(); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(0\)/); + for (let i = 0; i < 10 && vacuum.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await assertion; + + expect(vacuum.mock.calls.length).toBeGreaterThan(1); + expect(closeProjectStore).toHaveBeenCalled(); + exitSpy.mockRestore(); + logSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("bounded exhaustion on a persistently locked VACUUM fails clearly, closes the store, and exits 1", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "500"; + const vacuum = vi.fn().mockImplementation(() => { + throw new Error("SQLITE_BUSY: database is locked"); + }); + const getDatabase = vi.fn(() => ({ vacuum, getPath: () => "/proj/.fusion/fusion.db" })); + const store = makeStore({ getDatabase }); + 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.runDbVacuum(); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/); + for (let i = 0; i < 10 && vacuum.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.advanceTimersByTimeAsync(1_000); + await assertion; + + expect(vacuum.mock.calls.length).toBeGreaterThan(1); + const printed = errorSpy.mock.calls.flat().join("\n"); + expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("a non-lock VACUUM error does not retry-loop and closes the store before exit(1)", async () => { + const vacuum = vi.fn().mockImplementation(() => { + throw new Error("disk I/O error"); + }); + const getDatabase = vi.fn(() => ({ vacuum, getPath: () => "/proj/.fusion/fusion.db" })); + const store = makeStore({ getDatabase }); + 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(() => {}); + + await expect(mod.runDbVacuum()).rejects.toThrow(/process\.exit\(1\)/); + + expect(vacuum).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalled(); + expect(errorSpy.mock.calls.flat().join("\n")).toContain("disk I/O error"); + exitSpy.mockRestore(); + errorSpy.mockRestore(); + }); +}); diff --git a/packages/cli/src/commands/__tests__/db.test.ts b/packages/cli/src/commands/__tests__/db.test.ts index 61f3368971..e1dea13cf3 100644 --- a/packages/cli/src/commands/__tests__/db.test.ts +++ b/packages/cli/src/commands/__tests__/db.test.ts @@ -27,10 +27,25 @@ vi.mock("@fusion/core", () => ({ init: vi.fn(), getDatabase: mockGetDatabase, })), + isSqliteLockError: (error: unknown) => /database is locked/i.test(error instanceof Error ? error.message : String(error)), })); vi.mock("../../project-context.js", () => ({ resolveProject: mockResolveProject, + closeProjectStore: vi.fn(async (context: { store: { close?: () => Promise } }) => { + try { + await context.store.close?.(); + } catch { + // best-effort, mirrors production closeProjectStore + } + }), + asLocalProjectContext: vi.fn((store: unknown) => ({ + projectId: process.cwd(), + projectPath: process.cwd(), + projectName: "current-project", + isRegistered: false, + store, + })), })); import { runDbVacuum } from "../db.ts"; diff --git a/packages/cli/src/commands/__tests__/mcp-lock-retry.test.ts b/packages/cli/src/commands/__tests__/mcp-lock-retry.test.ts new file mode 100644 index 0000000000..d1d054cf0c --- /dev/null +++ b/packages/cli/src/commands/__tests__/mcp-lock-retry.test.ts @@ -0,0 +1,234 @@ +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Regression coverage for FN-7739 — `fn mcp *` must retry the project-scope + * `updateSettings` board write through a momentarily-locked SQLite board + * database instead of surfacing a raw `database is locked` error, and must + * close BOTH the cached project `TaskStore` and the ad-hoc uncached MCP + * secrets `TaskStore` (the `getSecretsStore` CWD-fallback branch) on every + * exit path so the CLI process exits promptly. Fast, fake-timer based, no + * real waits per FN-5048. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@fusion/core", async (importActual) => { + const actual = await importActual(); + return { ...actual }; +}); + +function makeGlobalStore(overrides: Record = {}) { + return { + init: vi.fn().mockResolvedValue(undefined), + getSettings: vi.fn(async () => ({ mcpServers: { enabled: false, servers: [] } })), + updateSettings: vi.fn(async (patch: any) => patch), + ...overrides, + }; +} + +function makeProjectStore(overrides: Record = {}) { + return { + getSettingsByScope: vi.fn(async () => ({ + global: { mcpServers: { enabled: false, servers: [] } }, + project: { mcpServers: { enabled: false, servers: [] } }, + })), + updateSettings: vi.fn(async (patch: any) => patch), + getSecretsStore: vi.fn(async () => makeSecretsStore()), + close: vi.fn().mockResolvedValue(undefined), + ...overrides, + }; +} + +function makeSecretsStore(overrides: Record = {}) { + return { + getSecretMetadata: vi.fn(() => null), + listSecrets: vi.fn(() => []), + createSecret: vi.fn(async (input: any) => ({ id: "sec-1", ...input })), + ...overrides, + }; +} + +async function loadWithMocks(opts: { + projectStore?: Record | null; + globalStore?: Record; + uncachedSecretsStore?: Record; +}) { + const globalStore = opts.globalStore ?? makeGlobalStore(); + const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise } }) => { + await context.store.close?.().catch(() => {}); + }); + const asLocalProjectContext = vi.fn((store: unknown) => ({ + projectId: process.cwd(), + projectPath: process.cwd(), + projectName: "current-project", + isRegistered: false, + store, + })); + + const projectContext = opts.projectStore + ? { + projectId: "proj_test", + projectPath: "/proj", + projectName: "proj", + isRegistered: true, + store: opts.projectStore, + } + : undefined; + + const resolveProject = projectContext + ? vi.fn().mockResolvedValue(projectContext) + : vi.fn().mockRejectedValue(new Error("no registered project")); + + vi.doMock("../../project-context.js", () => ({ resolveProject, closeProjectStore, asLocalProjectContext })); + + const uncachedSecretsStoreClose = vi.fn().mockResolvedValue(undefined); + const uncachedSecretsInstance = opts.uncachedSecretsStore ?? { + init: vi.fn().mockResolvedValue(undefined), + close: uncachedSecretsStoreClose, + getSecretsStore: vi.fn(async () => makeSecretsStore()), + }; + + vi.doMock("@fusion/core", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + GlobalSettingsStore: vi.fn(function GlobalSettingsStore() { + return globalStore; + }), + TaskStore: vi.fn(function TaskStore() { + return uncachedSecretsInstance; + }), + }; + }); + + const mod = await import("../mcp.js"); + return { mod, closeProjectStore, resolveProject, globalStore, uncachedSecretsInstance, uncachedSecretsStoreClose }; +} + +describe("fn mcp * — lock retry and cached+uncached store teardown (FN-7739)", () => { + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.doUnmock("../../project-context.js"); + vi.doUnmock("@fusion/core"); + vi.restoreAllMocks(); + delete process.env.FUSION_CLI_LOCK_RETRY_MS; + }); + + it("runMcpList: succeeds on first attempt and closes the cached project store once", async () => { + const projectStore = makeProjectStore(); + const { mod, closeProjectStore } = await loadWithMocks({ projectStore }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runMcpList({ json: true }); + + expect(projectStore.getSettingsByScope).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + logSpy.mockRestore(); + }); + + it("runMcpAdd: retries project-scope updateSettings through a transient lock error and succeeds once it clears, then closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "5000"; + const lockError = new Error("database is locked"); + const updateSettings = vi.fn().mockImplementationOnce(() => { + throw lockError; + }).mockImplementation(async (patch: any) => patch); + const projectStore = makeProjectStore({ updateSettings }); + const { mod, closeProjectStore } = await loadWithMocks({ projectStore }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const promise = mod.runMcpAdd("github", { scope: "project", transport: "stdio", command: "gh-mcp" }); + for (let i = 0; i < 10 && updateSettings.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await promise; + + expect(updateSettings.mock.calls.length).toBeGreaterThan(1); + expect(closeProjectStore).toHaveBeenCalled(); + logSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runMcpEdit: bounded exhaustion on a persistently locked updateSettings fails clearly and closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "500"; + const updateSettings = vi.fn().mockImplementation(() => { + throw new Error("SQLITE_BUSY: database is locked"); + }); + const projectStore = makeProjectStore({ + getSettingsByScope: vi.fn(async () => ({ + global: { mcpServers: { enabled: false, servers: [] } }, + project: { mcpServers: { enabled: false, servers: [{ name: "github", transport: "stdio", command: "gh" }] } }, + })), + updateSettings, + }); + const { mod, closeProjectStore } = await loadWithMocks({ projectStore }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + + const promise = mod.runMcpEdit("github", { scope: "project", command: "gh2" }); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/); + for (let i = 0; i < 10 && updateSettings.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.advanceTimersByTimeAsync(1_000); + await assertion; + + expect(updateSettings.mock.calls.length).toBeGreaterThan(1); + const printed = errorSpy.mock.calls.flat().join("\n"); + expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i); + expect(closeProjectStore).toHaveBeenCalled(); + errorSpy.mockRestore(); + exitSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runMcpAdd: a not-found/validation error does not retry-loop and closes the store", async () => { + const projectStore = makeProjectStore({ + getSettingsByScope: vi.fn(async () => ({ + global: { mcpServers: { enabled: false, servers: [] } }, + project: { mcpServers: { enabled: false, servers: [{ name: "github", transport: "stdio", command: "gh" }] } }, + })), + }); + const { mod, closeProjectStore } = await loadWithMocks({ projectStore }); + + await expect(mod.runMcpAdd("github", { scope: "project", transport: "stdio", command: "gh2" })).rejects.toThrow(/already exists/); + + expect(projectStore.updateSettings).not.toHaveBeenCalled(); + expect(closeProjectStore).toHaveBeenCalled(); + }); + + it("runMcpAdd (no project — uncached secrets store): closes both the cached project store (absent) and the ad-hoc secrets store", async () => { + const uncachedSecretsStoreClose = vi.fn().mockResolvedValue(undefined); + const uncachedSecretsInstance = { + init: vi.fn().mockResolvedValue(undefined), + close: uncachedSecretsStoreClose, + getSecretsStore: vi.fn(async () => makeSecretsStore()), + }; + const { mod, closeProjectStore } = await loadWithMocks({ projectStore: null, uncachedSecretsStore: uncachedSecretsInstance }); + + // Global-scope add does not require a project and does not touch getSecretsStore + // unless secrets are involved; use env creation to exercise the ad-hoc secrets store. + await mod.runMcpAdd("github", { + scope: "global", + transport: "stdio", + command: "gh-mcp", + createEnv: ["TOKEN=raw-secret-value"], + }); + + expect(uncachedSecretsInstance.getSecretsStore).toHaveBeenCalled(); + expect(uncachedSecretsStoreClose).toHaveBeenCalled(); + // closeProjectStore is invoked for the ad-hoc secrets store via asLocalProjectContext, + // even though no cached project store exists. + expect(closeProjectStore).toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/commands/__tests__/mcp.test.ts b/packages/cli/src/commands/__tests__/mcp.test.ts index c68809718b..7f0c56382a 100644 --- a/packages/cli/src/commands/__tests__/mcp.test.ts +++ b/packages/cli/src/commands/__tests__/mcp.test.ts @@ -52,6 +52,20 @@ vi.mock("@fusion/core", async (importActual) => { }); vi.mock("../../project-context.js", () => ({ + closeProjectStore: vi.fn(async (context: { store: { close?: () => Promise } }) => { + try { + await context.store.close?.(); + } catch { + // best-effort, mirrors production closeProjectStore + } + }), + asLocalProjectContext: vi.fn((store: unknown) => ({ + projectId: process.cwd(), + projectPath: process.cwd(), + projectName: "current-project", + isRegistered: false, + store, + })), resolveProject: vi.fn(async () => ({ projectId: "proj-1", projectName: "demo", diff --git a/packages/cli/src/commands/__tests__/memory-backup-lock-retry.test.ts b/packages/cli/src/commands/__tests__/memory-backup-lock-retry.test.ts new file mode 100644 index 0000000000..15e2533cef --- /dev/null +++ b/packages/cli/src/commands/__tests__/memory-backup-lock-retry.test.ts @@ -0,0 +1,201 @@ +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * Regression coverage for FN-7739 — `fn memory-backup *` must retry the + * discrete `getSettings()` board read through a momentarily-locked SQLite + * board database instead of surfacing a raw `database is locked` error, and + * must always close the resolved `TaskStore` (cached AND the uncached + * CWD-fallback branch) so the CLI process exits promptly. Mirrors the + * FN-7731/FN-7738/FN-7739 `*-lock-retry.test.ts` pattern (fast, fake-timer + * based, no real waits per FN-5048). + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@fusion/core", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + createMemoryBackupManager: vi.fn(), + runMemoryBackupCommand: vi.fn(async () => ({ success: true, output: "memory backup created" })), + }; +}); + +function makeStore(overrides: Record = {}) { + return { + getSettings: vi.fn(async () => ({ memoryBackupSchedule: "0 3 * * *" })), + fusionDir: "/proj/.fusion", + close: vi.fn().mockResolvedValue(undefined), + ...overrides, + }; +} + +async function loadWithMockedStore(store: Record, opts?: { cached?: boolean }) { + const cached = opts?.cached ?? true; + const closeProjectStore = vi.fn(async (context: { store: { close?: () => Promise } }) => { + await context.store.close?.().catch(() => {}); + }); + const context = { + projectId: cached ? "proj_test" : process.cwd(), + projectPath: cached ? "/proj" : process.cwd(), + projectName: "proj", + isRegistered: cached, + store, + }; + const resolveProject = cached + ? vi.fn().mockResolvedValue(context) + : vi.fn().mockRejectedValue(new Error("no registered project")); + const asLocalProjectContext = vi.fn(() => context); + vi.doMock("../../project-context.js", () => ({ resolveProject, closeProjectStore, asLocalProjectContext })); + const mod = await import("../memory-backup.js"); + const { createMemoryBackupManager } = await import("@fusion/core"); + return { mod, closeProjectStore, resolveProject, createMemoryBackupManager }; +} + +describe("fn memory-backup * — lock retry, leak/close teardown (FN-7739)", () => { + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.doUnmock("../../project-context.js"); + vi.restoreAllMocks(); + delete process.env.FUSION_CLI_LOCK_RETRY_MS; + }); + + it("runMemoryBackupList: succeeds on first attempt and closes the store once", async () => { + const store = makeStore(); + const { mod, closeProjectStore, createMemoryBackupManager } = await loadWithMockedStore(store); + (createMemoryBackupManager as ReturnType).mockReturnValue({ listBackups: vi.fn(async () => []) }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runMemoryBackupList(); + + expect(store.getSettings).toHaveBeenCalledTimes(1); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + logSpy.mockRestore(); + }); + + it("runMemoryBackupList (uncached CWD-fallback): resolves via asLocalProjectContext and still closes the store", async () => { + const store = makeStore(); + const { mod, closeProjectStore, createMemoryBackupManager } = await loadWithMockedStore(store, { cached: false }); + (createMemoryBackupManager as ReturnType).mockReturnValue({ listBackups: vi.fn(async () => []) }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await mod.runMemoryBackupList(); + + expect(closeProjectStore).toHaveBeenCalledTimes(1); + logSpy.mockRestore(); + }); + + it("runMemoryBackupList: retries through a transient lock error on getSettings and succeeds once it clears, then closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "5000"; + const lockError = new Error("database is locked"); + const getSettings = vi.fn().mockImplementationOnce(() => { + throw lockError; + }).mockImplementation(async () => ({ memoryBackupSchedule: "0 3 * * *" })); + const store = makeStore({ getSettings }); + const { mod, closeProjectStore, createMemoryBackupManager } = await loadWithMockedStore(store); + (createMemoryBackupManager as ReturnType).mockReturnValue({ listBackups: vi.fn(async () => []) }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const promise = mod.runMemoryBackupList(); + for (let i = 0; i < 10 && getSettings.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await promise; + + expect(getSettings.mock.calls.length).toBeGreaterThan(1); + expect(closeProjectStore).toHaveBeenCalled(); + logSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runMemoryBackupRestore: bounded exhaustion on a persistently locked getSettings fails clearly and closes the store", async () => { + vi.useFakeTimers(); + try { + process.env.FUSION_CLI_LOCK_RETRY_MS = "500"; + const getSettings = vi.fn().mockImplementation(() => { + throw new Error("SQLITE_BUSY: database is locked"); + }); + const store = makeStore({ getSettings }); + 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.runMemoryBackupRestore("memory-2026-01-01-000000"); + const assertion = expect(promise).rejects.toThrow(/process\.exit\(1\)/); + for (let i = 0; i < 10 && getSettings.mock.calls.length < 2; i++) { + await vi.advanceTimersByTimeAsync(1_000); + } + await vi.advanceTimersByTimeAsync(1_000); + await assertion; + + expect(getSettings.mock.calls.length).toBeGreaterThan(1); + const printed = errorSpy.mock.calls.flat().join("\n"); + expect(printed).toMatch(/locked|FUSION_CLI_LOCK_RETRY_MS/i); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } finally { + vi.useRealTimers(); + } + }); + + it("runMemoryBackupRestore: a restore failure closes the store before exiting non-zero", async () => { + const store = makeStore(); + const { mod, closeProjectStore, createMemoryBackupManager } = await loadWithMockedStore(store); + (createMemoryBackupManager as ReturnType).mockReturnValue({ + restoreBackup: vi.fn().mockRejectedValue(new Error("bad backup file")), + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + await expect(mod.runMemoryBackupRestore("missing.zip")).rejects.toThrow(/process\.exit\(1\)/); + + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + logSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + it("runMemoryBackupCreate: closes the store before process.exit on success", async () => { + const store = makeStore(); + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const { runMemoryBackupCommand } = await import("@fusion/core"); + (runMemoryBackupCommand as ReturnType).mockResolvedValue({ success: true, output: "ok" }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(mod.runMemoryBackupCreate()).rejects.toThrow(/process\.exit\(0\)/); + expect(closeProjectStore).toHaveBeenCalled(); + + exitSpy.mockRestore(); + logSpy.mockRestore(); + }); + + it("runMemoryBackupCreate: a non-lock exception from runMemoryBackupCommand itself still closes the store (FN-7739 review fix)", async () => { + const store = makeStore(); + const { mod, closeProjectStore } = await loadWithMockedStore(store); + const { runMemoryBackupCommand } = await import("@fusion/core"); + (runMemoryBackupCommand as ReturnType).mockRejectedValue(new Error("disk full")); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(mod.runMemoryBackupCreate()).rejects.toThrow(/disk full/); + expect(closeProjectStore).toHaveBeenCalledTimes(1); + + logSpy.mockRestore(); + }); +}); diff --git a/packages/cli/src/commands/__tests__/memory-backup.test.ts b/packages/cli/src/commands/__tests__/memory-backup.test.ts index b174371c6f..ca72b14856 100644 --- a/packages/cli/src/commands/__tests__/memory-backup.test.ts +++ b/packages/cli/src/commands/__tests__/memory-backup.test.ts @@ -40,10 +40,25 @@ vi.mock("@fusion/core", () => ({ restoreBackup: mockRestoreBackup, })), runMemoryBackupCommand: mockRunMemoryBackupCommand, + isSqliteLockError: (error: unknown) => /database is locked/i.test(error instanceof Error ? error.message : String(error)), })); vi.mock("../../project-context.js", () => ({ resolveProject: mockResolveProject, + closeProjectStore: vi.fn(async (context: { store: { close?: () => Promise } }) => { + try { + await context.store.close?.(); + } catch { + // best-effort, mirrors production closeProjectStore + } + }), + asLocalProjectContext: vi.fn((store: unknown) => ({ + projectId: process.cwd(), + projectPath: process.cwd(), + projectName: "current-project", + isRegistered: false, + store, + })), })); import { runMemoryBackupCreate, runMemoryBackupList, runMemoryBackupRestore } from "../memory-backup.js"; diff --git a/packages/cli/src/commands/backup.ts b/packages/cli/src/commands/backup.ts index 06ab3ebcd6..71fa401590 100644 --- a/packages/cli/src/commands/backup.ts +++ b/packages/cli/src/commands/backup.ts @@ -4,15 +4,35 @@ import { runBackupCommand, TaskStore, } from "@fusion/core"; -import { resolveProject } from "../project-context.js"; +import { resolveProject, closeProjectStore, asLocalProjectContext, type ProjectContext } from "../project-context.js"; +import { retryOnLock, LockRetryExhaustedError } from "../lock-retry.js"; -async function resolveBackupStore(projectName?: string): Promise { +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * FN-7739 audit finding: `resolveBackupStore` resolves a `TaskStore` (cached + * via `resolveProject`, OR an UNCACHED `new TaskStore(process.cwd())` + * CWD-fallback) and, before this change, no `runBackup*` handler ever closed + * it — a leaked SQLite/WAL handle keeps the CLI process's event loop alive + * after the command's real work (a filesystem backup operation) is done. + * The only board interaction in this file is the `getSettings()` read used + * to build the `BackupManager`; it did not retry through a momentary + * `database is locked`. This mirrors the class FN-7731 fixed for `fn task + * show`/`move` and FN-7738 fixed for `fn branch-group`/`fn pr`; the fix + * below reuses the SAME `retryOnLock`/`closeProjectStore` helpers (no + * forked second implementation). The resolved store is closed on every exit + * path — success return, restore-failure `process.exit(1)`, and both + * `runBackupCreate` `process.exit()` paths (closed explicitly BEFORE the + * exit call, since a pending `finally` does not run after `process.exit()` + * — see project memory) — including the uncached CWD-fallback branch via + * `asLocalProjectContext`. + */ +async function resolveBackupContext(projectName?: string): Promise { try { - return (await resolveProject(projectName)).store; + return await resolveProject(projectName); } catch { const store = new TaskStore(process.cwd()); await store.init(); - return store; + return asLocalProjectContext(store); } } @@ -21,15 +41,32 @@ async function resolveBackupStore(projectName?: string): Promise { */ async function getBackupManager(projectName?: string): Promise<{ manager: BackupManager; - store: TaskStore; + context: ProjectContext; fusionDir: string; }> { - const store = await resolveBackupStore(projectName); - // Access the private fusionDir property via type assertion - const fusionDir = (store as unknown as { fusionDir: string }).fusionDir; - const settings = await store.getSettings(); - const manager = createBackupManager(fusionDir, settings); - return { manager, store, fusionDir }; + const context = await resolveBackupContext(projectName); + try { + const { store } = context; + // Access the private fusionDir property via type assertion + const fusionDir = (store as unknown as { fusionDir: string }).fusionDir; + const settings = await retryOnLock(async () => store.getSettings(), { id: "backup-settings", action: "read settings" }); + const manager = createBackupManager(fusionDir, settings); + return { manager, context, fusionDir }; + } catch (error) { + // Settings-read exhaustion must not strand the resolved store unclosed — + // close it here since the caller never receives `context` on throw. + await closeProjectStore(context); + throw error; + } +} + +async function failBackupCommand(error: unknown, context?: ProjectContext): Promise { + const message = error instanceof Error ? error.message : String(error); + console.error(message); + if (context) { + await closeProjectStore(context); + } + return process.exit(1); } /** @@ -37,19 +74,41 @@ async function getBackupManager(projectName?: string): Promise<{ * Usage: fn backup --create */ export async function runBackupCreate(projectName?: string): Promise { - const { fusionDir, store } = await getBackupManager(projectName); - const settings = await store.getSettings(); - - console.log("Creating database backup..."); - - const result = await runBackupCommand(fusionDir, settings); - - if (result.success) { - console.log(result.output); - process.exit(0); - } else { - console.error(result.output); - process.exit(1); + let context: ProjectContext | undefined; + try { + const resolved = await getBackupManager(projectName); + context = resolved.context; + const { fusionDir, context: ctx } = resolved; + const settings = await retryOnLock(async () => ctx.store.getSettings(), { id: "backup-settings", action: "read settings" }); + + console.log("Creating database backup..."); + + const result = await runBackupCommand(fusionDir, settings); + + await closeProjectStore(ctx); + if (result.success) { + console.log(result.output); + process.exit(0); + } else { + console.error(result.output); + process.exit(1); + } + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failBackupCommand(error, context); + } else if (context) { + // FNXC:CliBoardMutation 2026-07-09-00:10: + // A non-lock exception thrown after `getBackupManager` resolved (e.g. + // `runBackupCommand` itself throwing on an unexpected filesystem + // failure, rather than returning `{success:false}`) previously fell + // through to `throw error` WITHOUT closing the resolved store — the + // only close call on this path was gated behind + // `LockRetryExhaustedError`. Close it here too so every exit path + // (lock-exhaustion, generic exception, and the two explicit + // process.exit() success/failure branches above) releases the store. + await closeProjectStore(context); + } + throw error; } } @@ -58,42 +117,56 @@ export async function runBackupCreate(projectName?: string): Promise { * Usage: fn backup --list */ export async function runBackupList(projectName?: string): Promise { - const { manager } = await getBackupManager(projectName); - - const pairs = await manager.listBackupPairs(); + let context: ProjectContext | undefined; + try { + const resolved = await getBackupManager(projectName); + context = resolved.context; + const { manager } = resolved; - if (pairs.length === 0) { - console.log("No backups found."); - return; - } + const pairs = await manager.listBackupPairs(); - const totalSize = pairs.reduce((sum, pair) => sum + (pair.project?.size ?? 0) + (pair.central?.size ?? 0), 0); - const formattedTotal = formatBytes(totalSize); + if (pairs.length === 0) { + console.log("No backups found."); + return; + } - console.log("Date Size Filename"); - console.log("-".repeat(60)); + const totalSize = pairs.reduce((sum, pair) => sum + (pair.project?.size ?? 0) + (pair.central?.size ?? 0), 0); + const formattedTotal = formatBytes(totalSize); - for (const pair of pairs) { - if (pair.project) { - const date = formatListDate(pair.project.createdAt); - const pairSize = formatBytes((pair.project?.size ?? 0) + (pair.central?.size ?? 0)).padEnd(10); - const noSibling = pair.central ? "" : " (no central sibling)"; - console.log(`${date} ${pairSize} ${pair.project.filename}${noSibling}`); - if (pair.central) { - console.log(`${" ".repeat(28)}${formatBytes(pair.central.size).padEnd(10)} └─ ${pair.central.filename}`); + console.log("Date Size Filename"); + console.log("-".repeat(60)); + + for (const pair of pairs) { + if (pair.project) { + const date = formatListDate(pair.project.createdAt); + const pairSize = formatBytes((pair.project?.size ?? 0) + (pair.central?.size ?? 0)).padEnd(10); + const noSibling = pair.central ? "" : " (no central sibling)"; + console.log(`${date} ${pairSize} ${pair.project.filename}${noSibling}`); + if (pair.central) { + console.log(`${" ".repeat(28)}${formatBytes(pair.central.size).padEnd(10)} └─ ${pair.central.filename}`); + } + continue; + } + + if (pair.central) { + const date = formatListDate(pair.central.createdAt); + const size = formatBytes(pair.central.size).padEnd(10); + console.log(`${date} ${size} ${pair.central.filename} (orphan central backup)`); } - continue; } - if (pair.central) { - const date = formatListDate(pair.central.createdAt); - const size = formatBytes(pair.central.size).padEnd(10); - console.log(`${date} ${size} ${pair.central.filename} (orphan central backup)`); + console.log("-".repeat(60)); + console.log(`Total: ${formattedTotal}`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failBackupCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeProjectStore(context); } } - - console.log("-".repeat(60)); - console.log(`Total: ${formattedTotal}`); } /** @@ -101,23 +174,38 @@ export async function runBackupList(projectName?: string): Promise { * Usage: fn backup --restore */ export async function runBackupRestore(filename: string, projectName?: string): Promise { - const { manager } = await getBackupManager(projectName); - - console.log(`Restoring backup: ${filename}`); - console.log("A pre-restore backup will be created first.\n"); - + let context: ProjectContext | undefined; try { - await manager.restoreBackup(filename, { createPreRestoreBackup: true }); - if (filename.startsWith("fusion-central-")) { - console.log(`Successfully restored central database from ${filename}`); - console.log("Created pre-restore snapshot: fusion-central-pre-restore-.db"); - } else { - console.log(`Successfully restored project database from ${filename}`); - console.log("Created pre-restore snapshots: fusion-pre-restore-.db and (if paired) fusion-central-pre-restore-.db"); + const resolved = await getBackupManager(projectName); + context = resolved.context; + const { manager } = resolved; + + console.log(`Restoring backup: ${filename}`); + console.log("A pre-restore backup will be created first.\n"); + + try { + await manager.restoreBackup(filename, { createPreRestoreBackup: true }); + if (filename.startsWith("fusion-central-")) { + console.log(`Successfully restored central database from ${filename}`); + console.log("Created pre-restore snapshot: fusion-central-pre-restore-.db"); + } else { + console.log(`Successfully restored project database from ${filename}`); + console.log("Created pre-restore snapshots: fusion-pre-restore-.db and (if paired) fusion-central-pre-restore-.db"); + } + } catch (err) { + console.error(`Restore failed: ${(err as Error).message}`); + await closeProjectStore(context); + process.exit(1); + } + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failBackupCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeProjectStore(context); } - } catch (err) { - console.error(`Restore failed: ${(err as Error).message}`); - process.exit(1); } } @@ -126,16 +214,30 @@ export async function runBackupRestore(filename: string, projectName?: string): * Usage: fn backup --cleanup */ export async function runBackupCleanup(projectName?: string): Promise { - const { manager } = await getBackupManager(projectName); - - console.log("Cleaning up old backups..."); - - const deletedCount = await manager.cleanupOldBackups(); - - if (deletedCount > 0) { - console.log(`Removed ${deletedCount} old backup(s) and any paired central backup files.`); - } else { - console.log("No backups to clean up (within retention limit)."); + let context: ProjectContext | undefined; + try { + const resolved = await getBackupManager(projectName); + context = resolved.context; + const { manager } = resolved; + + console.log("Cleaning up old backups..."); + + const deletedCount = await manager.cleanupOldBackups(); + + if (deletedCount > 0) { + console.log(`Removed ${deletedCount} old backup(s) and any paired central backup files.`); + } else { + console.log("No backups to clean up (within retention limit)."); + } + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failBackupCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeProjectStore(context); + } } } diff --git a/packages/cli/src/commands/db.ts b/packages/cli/src/commands/db.ts index 2632869dbb..3c1da7da26 100644 --- a/packages/cli/src/commands/db.ts +++ b/packages/cli/src/commands/db.ts @@ -1,5 +1,6 @@ import { TaskStore } from "@fusion/core"; -import { resolveProject } from "../project-context.js"; +import { resolveProject, closeProjectStore, asLocalProjectContext, type ProjectContext } from "../project-context.js"; +import { retryOnLock, LockRetryExhaustedError } from "../lock-retry.js"; type VacuumResult = { beforeSize: number; @@ -13,13 +14,32 @@ type VacuumDatabase = { getPath?: () => string; }; -async function resolveStore(projectName?: string): Promise { +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * FN-7739 audit finding: `resolveStore` resolves a `TaskStore` (cached via + * `resolveProject`, OR an UNCACHED `new TaskStore(process.cwd())` + * CWD-fallback). Unlike `backup.ts`/`memory-backup.ts`/`mcp.ts`, + * `runDbVacuum` already calls `process.exit(0/1)` on EVERY path, so there is + * no event-loop hang leak today — but `process.exit()` does not run pending + * `finally` blocks (see project memory), so the resolved store was never + * explicitly closed either way, and a leaked-but-about-to-exit handle is + * still untidy. VACUUM requires an EXCLUSIVE database lock — the canonical + * transient-lock case this task targets (a concurrent engine/agent writer + * momentarily holding the DB). Decision recorded in the FN-7739 audit task + * document (key="audit"): wrap the VACUUM call in `retryOnLock` so it + * succeeds once a momentary writer lock clears instead of failing outright + * on one unlucky race, and close the resolved store (via + * `closeProjectStore`/`asLocalProjectContext` for the uncached branch) + * explicitly BEFORE each `process.exit()` call for tidy, deterministic + * teardown. Reuses the FN-7731/FN-7738 helpers — no forked implementation. + */ +async function resolveStoreContext(projectName?: string): Promise { try { - return (await resolveProject(projectName)).store; + return await resolveProject(projectName); } catch { const store = new TaskStore(process.cwd()); await store.init(); - return store; + return asLocalProjectContext(store); } } @@ -36,22 +56,33 @@ function formatBytes(bytes: number): string { } export async function runDbVacuum(projectName?: string): Promise { + let context: ProjectContext | undefined; let db: VacuumDatabase; let result: VacuumResult; try { - const store = await resolveStore(projectName); - db = store.getDatabase() as unknown as VacuumDatabase; + context = await resolveStoreContext(projectName); + db = context.store.getDatabase() as unknown as VacuumDatabase; - if (typeof db.vacuum === "function") { - result = await db.vacuum(); - } else { - const start = Date.now(); - db.exec?.("VACUUM"); - result = { beforeSize: 0, afterSize: 0, durationMs: Date.now() - start }; - } + result = await retryOnLock( + async () => { + if (typeof db.vacuum === "function") { + return await db.vacuum(); + } + const start = Date.now(); + db.exec?.("VACUUM"); + return { beforeSize: 0, afterSize: 0, durationMs: Date.now() - start }; + }, + { id: "db-vacuum", action: "VACUUM database" }, + ); } catch (error) { - console.error(`Database VACUUM failed: ${(error as Error).message}`); + const message = error instanceof LockRetryExhaustedError + ? error.message + : `Database VACUUM failed: ${(error as Error).message}`; + console.error(message); + if (context) { + await closeProjectStore(context); + } process.exit(1); return; } @@ -64,5 +95,8 @@ export async function runDbVacuum(projectName?: string): Promise { `VACUUM completed in ${result.durationMs}ms (${formatBytes(result.beforeSize)} -> ${formatBytes(result.afterSize)}): ${path}`, ); } + if (context) { + await closeProjectStore(context); + } process.exit(0); } diff --git a/packages/cli/src/commands/mcp.ts b/packages/cli/src/commands/mcp.ts index 944efe684b..904f48c494 100644 --- a/packages/cli/src/commands/mcp.ts +++ b/packages/cli/src/commands/mcp.ts @@ -18,7 +18,8 @@ import { type SecretScope, type Settings, } from "@fusion/core"; -import { resolveProject, type ProjectContext } from "../project-context.js"; +import { resolveProject, closeProjectStore, asLocalProjectContext, type ProjectContext } from "../project-context.js"; +import { retryOnLock, LockRetryExhaustedError } from "../lock-retry.js"; export type McpScope = "global" | "project"; export type McpTransportInput = "stdio" | "sse" | "http" | "streamable-http"; @@ -45,9 +46,34 @@ export interface McpMutationOptions extends McpSensitiveInputOptions { enabled?: boolean; } +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * FN-7739 audit finding: `loadContext` resolves an optional cached project + * `TaskStore` (via `resolveProject`), and `getSecretsStore` may separately + * build an UNCACHED `new TaskStore(process.cwd())` (when no project is in + * scope) — before this change, NEITHER was ever closed on any exit path, so + * every `runMcp*` read and mutation leaked a live SQLite/WAL handle that + * kept the CLI process's event loop alive after the command finished. Only + * the project-scope `writeScopedSettings` -> `store.updateSettings` call + * ever touches the board DB; it did not retry through a momentary + * `database is locked`. `GlobalSettingsStore` is file-backed + * (`~/.fusion/settings.json`, no SQLite handle, no `close()`) — it is + * intentionally left with no close/retry (confirmed via + * packages/core/src/global-settings.ts). `McpContext.secretsStore` caches + * the uncached ad-hoc secrets `TaskStore` per-invocation (created lazily by + * `getSecretsStore`, which may be called multiple times inside a single + * mutation's `buildSensitiveMap` loop) so it is opened at most once and + * closed exactly once via `closeMcpContext`, which closes BOTH the cached + * project store (`closeProjectStore`) and the ad-hoc secrets store + * (`asLocalProjectContext` + `closeProjectStore`) on every exit path. + * Reuses the FN-7731/FN-7738 `retryOnLock`/`closeProjectStore` helpers — no + * forked implementation. + */ interface McpContext { project?: ProjectContext; globalStore: GlobalSettingsStore; + /** Ad-hoc uncached secrets store, created lazily; closed via closeMcpContext. */ + secretsStore?: TaskStore; } const DEFAULT_SCOPE: McpScope = "project"; @@ -69,6 +95,29 @@ async function loadContext(projectName?: string, requireProject = false): Promis return { project, globalStore }; } +/** + * Close the cached project store (if resolved) AND the ad-hoc uncached + * secrets store (if one was created) on every exit path. Best-effort and + * idempotent — see `closeProjectStore`. + */ +async function closeMcpContext(context: McpContext): Promise { + if (context.project) { + await closeProjectStore(context.project); + } + if (context.secretsStore) { + await closeProjectStore(asLocalProjectContext(context.secretsStore)); + } +} + +async function failMcpCommand(error: unknown, context?: McpContext): Promise { + const message = error instanceof Error ? error.message : String(error); + console.error(message); + if (context) { + await closeMcpContext(context); + } + return process.exit(1); +} + function normalizeScope(scope?: McpScope): McpScope { if (!scope) return DEFAULT_SCOPE; if (scope !== "global" && scope !== "project") { @@ -101,10 +150,17 @@ function mcpSettings(settings?: Pick { if (scope === "global") return mcpSettings(await context.globalStore.getSettings()); const project = ensureProject(context); - const scoped = await project.store.getSettingsByScope(); + const scoped = await retryOnLock(async () => project.store.getSettingsByScope(), { id: "mcp-settings", action: "read project MCP settings" }); return mcpSettings(scoped.project); } +/** + * Global-scope writes target the file-backed `GlobalSettingsStore` (no + * SQLite handle behind it) and are NOT retried for lock. Only the + * project-scope `store.updateSettings` board write is wrapped in + * `retryOnLock` — it is the discrete SQLite interaction that can race a + * momentary engine/agent writer. + */ async function writeScopedSettings(context: McpContext, scope: McpScope, next: McpServersSettings): Promise { const validation = validateMcpServerDefinitionsDetailed(next.servers ?? [], "mcpServers.servers"); if (validation.errors.length > 0) { @@ -116,7 +172,10 @@ async function writeScopedSettings(context: McpContext, scope: McpScope, next: M return; } const project = ensureProject(context); - await project.store.updateSettings({ mcpServers: normalized } as Partial); + await retryOnLock( + async () => project.store.updateSettings({ mcpServers: normalized } as Partial), + { id: "mcp-settings", action: "write project MCP settings" }, + ); } function upsertServer(servers: McpServerDefinition[], server: McpServerDefinition): McpServerDefinition[] { @@ -146,11 +205,24 @@ function assertNoPlaintextSensitiveOptions(opts: McpSensitiveInputOptions): void } } +/** + * Return the secrets store for this context, reusing the cached project + * store when in scope, or lazily creating (and caching on `context`) the + * uncached ad-hoc `TaskStore(process.cwd())` fallback exactly once per + * invocation so repeated calls inside `buildSensitiveMap` do not open a new + * handle each time — `closeMcpContext` closes it on every exit path. + */ async function getSecretsStore(context: McpContext) { const project = context.project; - const store = project?.store ?? new TaskStore(process.cwd()); - if (!project) await store.init(); - return store.getSecretsStore(); + if (project) { + return project.store.getSecretsStore(); + } + if (!context.secretsStore) { + const store = new TaskStore(process.cwd()); + await store.init(); + context.secretsStore = store; + } + return context.secretsStore.getSecretsStore(); } async function resolveExistingSecret(context: McpContext, secretRef: string, scope: SecretScope): Promise { @@ -269,24 +341,37 @@ function serverLine(server: McpServerDefinition, source: string, effectiveNames: * Listing must show global declarations, project declarations, and the project-over-global effective result without exposing secret material. Sensitive env/header fields are summarized as Fusion secret references only. */ export async function runMcpList(opts: { projectName?: string; json?: boolean } = {}): Promise { - const context = await loadContext(opts.projectName, false); - const globalSettings = mcpSettings(await context.globalStore.getSettings()); - const projectSettings = context.project ? mcpSettings((await context.project.store.getSettingsByScope()).project) : undefined; - const effective = resolveEffectiveMcpServers({ mcpServers: globalSettings }, projectSettings ? { mcpServers: projectSettings } : null); - if (opts.json) { - console.log(JSON.stringify({ global: globalSettings.servers ?? [], project: projectSettings?.servers ?? [], effective }, null, 2)); - return; + let context: McpContext | undefined; + try { + context = await loadContext(opts.projectName, false); + const globalSettings = mcpSettings(await context.globalStore.getSettings()); + const project = context.project; + const projectSettings = project ? mcpSettings((await retryOnLock(async () => project.store.getSettingsByScope(), { id: "mcp-settings", action: "read project MCP settings" })).project) : undefined; + const effective = resolveEffectiveMcpServers({ mcpServers: globalSettings }, projectSettings ? { mcpServers: projectSettings } : null); + if (opts.json) { + console.log(JSON.stringify({ global: globalSettings.servers ?? [], project: projectSettings?.servers ?? [], effective }, null, 2)); + return; + } + console.log(); + console.log(" MCP servers"); + console.log(" " + "─".repeat(80)); + const effectiveNames = new Set(effective.map((server) => server.name)); + for (const server of globalSettings.servers ?? []) console.log(serverLine(server, "global", effectiveNames)); + for (const server of projectSettings?.servers ?? []) console.log(serverLine(server, "project", effectiveNames)); + if ((globalSettings.servers?.length ?? 0) === 0 && (projectSettings?.servers?.length ?? 0) === 0) console.log(" No MCP servers configured."); + console.log(); + console.log(` Effective: ${effective.map((server) => server.name).join(", ") || "none"}`); + console.log(); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } } - console.log(); - console.log(" MCP servers"); - console.log(" " + "─".repeat(80)); - const effectiveNames = new Set(effective.map((server) => server.name)); - for (const server of globalSettings.servers ?? []) console.log(serverLine(server, "global", effectiveNames)); - for (const server of projectSettings?.servers ?? []) console.log(serverLine(server, "project", effectiveNames)); - if ((globalSettings.servers?.length ?? 0) === 0 && (projectSettings?.servers?.length ?? 0) === 0) console.log(" No MCP servers configured."); - console.log(); - console.log(` Effective: ${effective.map((server) => server.name).join(", ") || "none"}`); - console.log(); } /** @@ -294,13 +379,25 @@ export async function runMcpList(opts: { projectName?: string; json?: boolean } * Add persists an MCP server at the chosen global/project scope and lets the shared resolver decide project-over-global behavior. Env/header/token material must be an existing Fusion secret reference or be created in SecretsStore before validation; raw values never enter settings. */ export async function runMcpAdd(name: string, opts: McpMutationOptions = {}): Promise { - const scope = normalizeScope(opts.scope); - const context = await loadContext(opts.projectName, scope === "project"); - const current = await readScopedSettings(context, scope); - if ((current.servers ?? []).some((server) => server.name === name)) throw new Error(`MCP server "${name}" already exists in ${scope} scope. Use edit to update it.`); - const server = await buildServerDefinition(context, name, opts); - await writeScopedSettings(context, scope, { enabled: true, servers: upsertServer(current.servers ?? [], server) }); - console.log(`✓ Added MCP server "${name}" to ${scope} scope`); + let context: McpContext | undefined; + try { + const scope = normalizeScope(opts.scope); + context = await loadContext(opts.projectName, scope === "project"); + const current = await readScopedSettings(context, scope); + if ((current.servers ?? []).some((server) => server.name === name)) throw new Error(`MCP server "${name}" already exists in ${scope} scope. Use edit to update it.`); + const server = await buildServerDefinition(context, name, opts); + await writeScopedSettings(context, scope, { enabled: true, servers: upsertServer(current.servers ?? [], server) }); + console.log(`✓ Added MCP server "${name}" to ${scope} scope`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } + } } /** @@ -308,14 +405,26 @@ export async function runMcpAdd(name: string, opts: McpMutationOptions = {}): Pr * Edit updates only the selected scope; project definitions override same-named globals and may be disabled locally. Secret-bearing fields are replaced only with Fusion secret references or newly created SecretsStore records. */ export async function runMcpEdit(name: string, opts: McpMutationOptions = {}): Promise { - const scope = normalizeScope(opts.scope); - const context = await loadContext(opts.projectName, scope === "project"); - const current = await readScopedSettings(context, scope); - const existing = (current.servers ?? []).find((server) => server.name === name); - if (!existing) throw new Error(`MCP server "${name}" not found in ${scope} scope.`); - const server = await buildServerDefinition(context, name, opts, existing); - await writeScopedSettings(context, scope, { enabled: true, servers: upsertServer(current.servers ?? [], server) }); - console.log(`✓ Updated MCP server "${name}" in ${scope} scope`); + let context: McpContext | undefined; + try { + const scope = normalizeScope(opts.scope); + context = await loadContext(opts.projectName, scope === "project"); + const current = await readScopedSettings(context, scope); + const existing = (current.servers ?? []).find((server) => server.name === name); + if (!existing) throw new Error(`MCP server "${name}" not found in ${scope} scope.`); + const server = await buildServerDefinition(context, name, opts, existing); + await writeScopedSettings(context, scope, { enabled: true, servers: upsertServer(current.servers ?? [], server) }); + console.log(`✓ Updated MCP server "${name}" in ${scope} scope`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } + } } /** @@ -323,23 +432,47 @@ export async function runMcpEdit(name: string, opts: McpMutationOptions = {}): P * Remove deletes only the scoped declaration. Removing a project override can reveal an inherited global declaration again because effective MCP resolution is project-over-global by server name. */ export async function runMcpRemove(name: string, opts: { projectName?: string; scope?: McpScope } = {}): Promise { - const scope = normalizeScope(opts.scope); - const context = await loadContext(opts.projectName, scope === "project"); - const current = await readScopedSettings(context, scope); - const next = removeServer(current.servers ?? [], name); - if (!next.removed) throw new Error(`MCP server "${name}" not found in ${scope} scope.`); - await writeScopedSettings(context, scope, { enabled: current.enabled ?? true, servers: next.servers }); - console.log(`✓ Removed MCP server "${name}" from ${scope} scope`); + let context: McpContext | undefined; + try { + const scope = normalizeScope(opts.scope); + context = await loadContext(opts.projectName, scope === "project"); + const current = await readScopedSettings(context, scope); + const next = removeServer(current.servers ?? [], name); + if (!next.removed) throw new Error(`MCP server "${name}" not found in ${scope} scope.`); + await writeScopedSettings(context, scope, { enabled: current.enabled ?? true, servers: next.servers }); + console.log(`✓ Removed MCP server "${name}" from ${scope} scope`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } + } } async function setEnabled(name: string, enabled: boolean, opts: { projectName?: string; scope?: McpScope } = {}): Promise { - const scope = normalizeScope(opts.scope); - const context = await loadContext(opts.projectName, scope === "project"); - const current = await readScopedSettings(context, scope); - const existing = (current.servers ?? []).find((server) => server.name === name); - if (!existing) throw new Error(`MCP server "${name}" not found in ${scope} scope.`); - await writeScopedSettings(context, scope, { enabled: current.enabled ?? true, servers: upsertServer(current.servers ?? [], { ...existing, enabled }) }); - console.log(`✓ ${enabled ? "Enabled" : "Disabled"} MCP server "${name}" in ${scope} scope`); + let context: McpContext | undefined; + try { + const scope = normalizeScope(opts.scope); + context = await loadContext(opts.projectName, scope === "project"); + const current = await readScopedSettings(context, scope); + const existing = (current.servers ?? []).find((server) => server.name === name); + if (!existing) throw new Error(`MCP server "${name}" not found in ${scope} scope.`); + await writeScopedSettings(context, scope, { enabled: current.enabled ?? true, servers: upsertServer(current.servers ?? [], { ...existing, enabled }) }); + console.log(`✓ ${enabled ? "Enabled" : "Disabled"} MCP server "${name}" in ${scope} scope`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } + } } /** @@ -363,33 +496,45 @@ export async function runMcpDisable(name: string, opts: { projectName?: string; * Claude Desktop imports must delegate parsing to the core importer. Any plaintext env/header values returned by the importer are immediately converted into SecretsStore records, then settings receive only the resulting Fusion secret references. */ export async function runMcpImport(filePath: string, opts: { projectName?: string; scope?: McpScope; yes?: boolean } = {}): Promise { - const scope = normalizeScope(opts.scope); - const context = await loadContext(opts.projectName, scope === "project"); - const resolvedPath = resolve(filePath); - if (!existsSync(resolvedPath)) throw new Error(`File not found: ${filePath}`); - const imported = importMcpServersJson(await readFile(resolvedPath, "utf-8"), { scope }); - if (imported.errors.length > 0) throw new Error(`Invalid MCP import file:\n${imported.errors.map((error) => ` - ${error}`).join("\n")}`); - console.log(); - console.log(" MCP Import Summary:"); - console.log(` Source: ${resolvedPath}`); - console.log(` Scope: ${scope}`); - console.log(` Servers: ${imported.definitions.length}`); - console.log(` Secrets to create: ${imported.secretsToCreate.length}`); - console.log(); - if (!opts.yes) throw new Error("Use --yes to confirm this import operation"); - const replacements = new Map(); - for (const secret of imported.secretsToCreate) { - replacements.set(`${secret.serverName}:${secret.field}:${secret.key}:${secret.suggestedKey}`, await createSecretRef(context, { - scope: secret.scope, - key: secret.suggestedKey, - plaintextValue: secret.plaintextValue, - description: `Imported MCP ${secret.field} ${secret.key} for ${secret.serverName}`, - })); + let context: McpContext | undefined; + try { + const scope = normalizeScope(opts.scope); + context = await loadContext(opts.projectName, scope === "project"); + const resolvedPath = resolve(filePath); + if (!existsSync(resolvedPath)) throw new Error(`File not found: ${filePath}`); + const imported = importMcpServersJson(await readFile(resolvedPath, "utf-8"), { scope }); + if (imported.errors.length > 0) throw new Error(`Invalid MCP import file:\n${imported.errors.map((error) => ` - ${error}`).join("\n")}`); + console.log(); + console.log(" MCP Import Summary:"); + console.log(` Source: ${resolvedPath}`); + console.log(` Scope: ${scope}`); + console.log(` Servers: ${imported.definitions.length}`); + console.log(` Secrets to create: ${imported.secretsToCreate.length}`); + console.log(); + if (!opts.yes) throw new Error("Use --yes to confirm this import operation"); + const replacements = new Map(); + for (const secret of imported.secretsToCreate) { + replacements.set(`${secret.serverName}:${secret.field}:${secret.key}:${secret.suggestedKey}`, await createSecretRef(context, { + scope: secret.scope, + key: secret.suggestedKey, + plaintextValue: secret.plaintextValue, + description: `Imported MCP ${secret.field} ${secret.key} for ${secret.serverName}`, + })); + } + const definitions = imported.definitions.map((server) => rewriteImportedSecretRefs(server, replacements)); + const current = await readScopedSettings(context, scope); + await writeScopedSettings(context, scope, { enabled: true, servers: [...(current.servers ?? []).filter((server) => !definitions.some((entry) => entry.name === server.name)), ...definitions] }); + console.log(`✓ Imported ${definitions.length} MCP server(s) into ${scope} scope`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } } - const definitions = imported.definitions.map((server) => rewriteImportedSecretRefs(server, replacements)); - const current = await readScopedSettings(context, scope); - await writeScopedSettings(context, scope, { enabled: true, servers: [...(current.servers ?? []).filter((server) => !definitions.some((entry) => entry.name === server.name)), ...definitions] }); - console.log(`✓ Imported ${definitions.length} MCP server(s) into ${scope} scope`); } function rewriteImportedSecretRefs(server: McpServerDefinition, replacements: Map): McpServerDefinition { @@ -411,23 +556,36 @@ function rewriteImportedSecretRefs(server: McpServerDefinition, replacements: Ma * MCP export uses the core JSON exporter so secret-backed fields stay as descriptors and are never materialized. The default export is effective project-over-global configuration; explicit scope exports preserve stored declarations. */ export async function runMcpExport(opts: { projectName?: string; scope?: McpScope | "effective"; output?: string; json?: boolean } = {}): Promise { - const context = await loadContext(opts.projectName, opts.scope === "project"); - const scope = opts.scope ?? "effective"; - const globalSettings = mcpSettings(await context.globalStore.getSettings()); - const projectSettings = context.project ? mcpSettings((await context.project.store.getSettingsByScope()).project) : undefined; - const definitions = scope === "global" - ? globalSettings.servers ?? [] - : scope === "project" - ? projectSettings?.servers ?? [] - : resolveEffectiveMcpServers({ mcpServers: globalSettings }, projectSettings ? { mcpServers: projectSettings } : null); - const exported = exportMcpServersJson(definitions); - const json = JSON.stringify(exported, null, 2); - if (opts.output) { - await writeFile(resolve(opts.output), json); - console.log(`✓ Exported MCP servers to ${resolve(opts.output)}`); - return; + let context: McpContext | undefined; + try { + context = await loadContext(opts.projectName, opts.scope === "project"); + const scope = opts.scope ?? "effective"; + const globalSettings = mcpSettings(await context.globalStore.getSettings()); + const project = context.project; + const projectSettings = project ? mcpSettings((await retryOnLock(async () => project.store.getSettingsByScope(), { id: "mcp-settings", action: "read project MCP settings" })).project) : undefined; + const definitions = scope === "global" + ? globalSettings.servers ?? [] + : scope === "project" + ? projectSettings?.servers ?? [] + : resolveEffectiveMcpServers({ mcpServers: globalSettings }, projectSettings ? { mcpServers: projectSettings } : null); + const exported = exportMcpServersJson(definitions); + const json = JSON.stringify(exported, null, 2); + if (opts.output) { + await writeFile(resolve(opts.output), json); + console.log(`✓ Exported MCP servers to ${resolve(opts.output)}`); + return; + } + console.log(json); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } } - console.log(json); } /** @@ -435,21 +593,34 @@ export async function runMcpExport(opts: { projectName?: string; scope?: McpScop * Validate is intentionally list-only until an optional MCP reachability service exists. It still uses the foundation validator so transport requirements and plaintext-secret rejection match every other MCP settings write path. */ export async function runMcpValidate(opts: { projectName?: string; scope?: McpScope | "effective"; json?: boolean } = {}): Promise { - const context = await loadContext(opts.projectName, opts.scope === "project"); - const scope = opts.scope ?? "effective"; - const globalSettings = mcpSettings(await context.globalStore.getSettings()); - const projectSettings = context.project ? mcpSettings((await context.project.store.getSettingsByScope()).project) : undefined; - const definitions = scope === "global" - ? globalSettings.servers ?? [] - : scope === "project" - ? projectSettings?.servers ?? [] - : resolveEffectiveMcpServers({ mcpServers: globalSettings }, projectSettings ? { mcpServers: projectSettings } : null); - const validation = validateMcpServerDefinitionsDetailed(definitions); - const result = { ok: validation.errors.length === 0, servers: definitions.length, errors: validation.errors }; - if (opts.json) { - console.log(JSON.stringify(result, null, 2)); - return; + let context: McpContext | undefined; + try { + context = await loadContext(opts.projectName, opts.scope === "project"); + const scope = opts.scope ?? "effective"; + const globalSettings = mcpSettings(await context.globalStore.getSettings()); + const project = context.project; + const projectSettings = project ? mcpSettings((await retryOnLock(async () => project.store.getSettingsByScope(), { id: "mcp-settings", action: "read project MCP settings" })).project) : undefined; + const definitions = scope === "global" + ? globalSettings.servers ?? [] + : scope === "project" + ? projectSettings?.servers ?? [] + : resolveEffectiveMcpServers({ mcpServers: globalSettings }, projectSettings ? { mcpServers: projectSettings } : null); + const validation = validateMcpServerDefinitionsDetailed(definitions); + const result = { ok: validation.errors.length === 0, servers: definitions.length, errors: validation.errors }; + if (opts.json) { + console.log(JSON.stringify(result, null, 2)); + return; + } + if (result.ok) console.log(`✓ ${definitions.length} MCP server definition(s) valid`); + else throw new Error(formatValidationErrors(validation.errors)); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMcpCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeMcpContext(context); + } } - if (result.ok) console.log(`✓ ${definitions.length} MCP server definition(s) valid`); - else throw new Error(formatValidationErrors(validation.errors)); } diff --git a/packages/cli/src/commands/memory-backup.ts b/packages/cli/src/commands/memory-backup.ts index b8cacff0d4..9e9351ca95 100644 --- a/packages/cli/src/commands/memory-backup.ts +++ b/packages/cli/src/commands/memory-backup.ts @@ -4,86 +4,167 @@ import { TaskStore, type ProjectSettings, } from "@fusion/core"; -import { resolveProject } from "../project-context.js"; +import { resolveProject, closeProjectStore, asLocalProjectContext, type ProjectContext } from "../project-context.js"; +import { retryOnLock, LockRetryExhaustedError } from "../lock-retry.js"; type MemoryBackupScope = "project" | "agents" | "all"; -async function resolveBackupStore(projectName?: string): Promise { +/** + * FNXC:CliBoardMutation 2026-07-09-00:00: + * FN-7739 audit finding: same shape as `backup.ts` — `resolveBackupContext` + * resolves a `TaskStore` (cached via `resolveProject`, OR an UNCACHED + * `new TaskStore(process.cwd())` CWD-fallback) that no `runMemoryBackup*` + * handler ever closed, leaking a SQLite/WAL handle that keeps the CLI + * process's event loop alive. The only board interaction is the + * `getSettings()` read; it did not retry through a momentary `database is + * locked`. Fix reuses the FN-7731/FN-7738 `retryOnLock`/`closeProjectStore` + * helpers — no forked implementation. Store closed on every exit path + * (success return, restore-failure `process.exit(1)`, and both + * `runMemoryBackupCreate` `process.exit()` paths, closed BEFORE the exit + * call per project memory), including the uncached CWD-fallback branch via + * `asLocalProjectContext`. + */ +async function resolveBackupContext(projectName?: string): Promise { try { - return (await resolveProject(projectName)).store; + return await resolveProject(projectName); } catch { const store = new TaskStore(process.cwd()); await store.init(); - return store; + return asLocalProjectContext(store); } } async function getMemoryBackupContext(projectName?: string): Promise<{ - store: TaskStore; + context: ProjectContext; fusionDir: string; settings: ProjectSettings; }> { - const store = await resolveBackupStore(projectName); - const fusionDir = (store as unknown as { fusionDir: string }).fusionDir; - const settings = await store.getSettings(); - return { store, fusionDir, settings }; + const context = await resolveBackupContext(projectName); + try { + const fusionDir = (context.store as unknown as { fusionDir: string }).fusionDir; + const settings = await retryOnLock(async () => context.store.getSettings(), { id: "memory-backup-settings", action: "read settings" }); + return { context, fusionDir, settings }; + } catch (error) { + // Settings-read exhaustion must not strand the resolved store unclosed — + // close it here since the caller never receives `context` on throw. + await closeProjectStore(context); + throw error; + } +} + +async function failMemoryBackupCommand(error: unknown, context?: ProjectContext): Promise { + const message = error instanceof Error ? error.message : String(error); + console.error(message); + if (context) { + await closeProjectStore(context); + } + return process.exit(1); } export async function runMemoryBackupCreate(options?: { projectName?: string; scope?: MemoryBackupScope }): Promise { - const { fusionDir, settings } = await getMemoryBackupContext(options?.projectName); - const effectiveSettings = options?.scope ? { ...settings, memoryBackupScope: options.scope } : settings; + let context: ProjectContext | undefined; + try { + const resolved = await getMemoryBackupContext(options?.projectName); + context = resolved.context; + const { fusionDir, settings } = resolved; + const effectiveSettings = options?.scope ? { ...settings, memoryBackupScope: options.scope } : settings; - console.log("Creating memory backup..."); - const result = await runMemoryBackupCommand(fusionDir, effectiveSettings); - if (result.success) { - console.log(result.output); - process.exit(0); + console.log("Creating memory backup..."); + const result = await runMemoryBackupCommand(fusionDir, effectiveSettings); + await closeProjectStore(context); + if (result.success) { + console.log(result.output); + process.exit(0); + } + console.error(result.output); + process.exit(1); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMemoryBackupCommand(error, context); + } else if (context) { + // FNXC:CliBoardMutation 2026-07-09-00:10: + // A non-lock exception thrown after `getMemoryBackupContext` resolved + // (e.g. `runMemoryBackupCommand` itself throwing rather than + // returning `{success:false}`) previously fell through to `throw + // error` WITHOUT closing the resolved store — the only close call on + // this path was gated behind `LockRetryExhaustedError`. Close it here + // too so every exit path releases the store. + await closeProjectStore(context); + } + throw error; } - console.error(result.output); - process.exit(1); } export async function runMemoryBackupList(projectName?: string): Promise { - const { fusionDir, settings } = await getMemoryBackupContext(projectName); - const manager = createMemoryBackupManager(fusionDir, settings); - const backups = await manager.listBackups(); + let context: ProjectContext | undefined; + try { + const resolved = await getMemoryBackupContext(projectName); + context = resolved.context; + const { fusionDir, settings } = resolved; + const manager = createMemoryBackupManager(fusionDir, settings); + const backups = await manager.listBackups(); - if (backups.length === 0) { - console.log("No memory backups found."); - return; + if (backups.length === 0) { + console.log("No memory backups found."); + return; + } + + console.log(`Found ${backups.length} memory backup(s):\n`); + console.log("Date Scope Entries Size Filename"); + console.log("-".repeat(80)); + + let totalSize = 0; + for (const backup of backups) { + totalSize += backup.size; + const date = new Date(backup.createdAt).toLocaleString(); + const scope = backup.scope.padEnd(7); + const entries = String(backup.entryCount).padEnd(7); + const size = formatBytes(backup.size).padEnd(9); + console.log(`${date} ${scope} ${entries} ${size} ${backup.filename}`); + } + + console.log("-".repeat(80)); + console.log(`Total: ${formatBytes(totalSize)}`); + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMemoryBackupCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeProjectStore(context); + } } - - console.log(`Found ${backups.length} memory backup(s):\n`); - console.log("Date Scope Entries Size Filename"); - console.log("-".repeat(80)); - - let totalSize = 0; - for (const backup of backups) { - totalSize += backup.size; - const date = new Date(backup.createdAt).toLocaleString(); - const scope = backup.scope.padEnd(7); - const entries = String(backup.entryCount).padEnd(7); - const size = formatBytes(backup.size).padEnd(9); - console.log(`${date} ${scope} ${entries} ${size} ${backup.filename}`); - } - - console.log("-".repeat(80)); - console.log(`Total: ${formatBytes(totalSize)}`); } export async function runMemoryBackupRestore(filename: string, projectName?: string): Promise { - const { fusionDir, settings } = await getMemoryBackupContext(projectName); - const manager = createMemoryBackupManager(fusionDir, settings); - - console.log(`Restoring memory backup: ${filename}`); - console.log("This may overwrite project and/or agent memory files.\n"); - + let context: ProjectContext | undefined; try { - await manager.restoreBackup(filename, { overwrite: true }); - console.log(`Successfully restored memory from ${filename}`); - } catch (err) { - console.error(`Memory restore failed: ${(err as Error).message}`); - process.exit(1); + const resolved = await getMemoryBackupContext(projectName); + context = resolved.context; + const { fusionDir, settings } = resolved; + const manager = createMemoryBackupManager(fusionDir, settings); + + console.log(`Restoring memory backup: ${filename}`); + console.log("This may overwrite project and/or agent memory files.\n"); + + try { + await manager.restoreBackup(filename, { overwrite: true }); + console.log(`Successfully restored memory from ${filename}`); + } catch (err) { + console.error(`Memory restore failed: ${(err as Error).message}`); + await closeProjectStore(context); + process.exit(1); + } + } catch (error) { + if (error instanceof LockRetryExhaustedError) { + await failMemoryBackupCommand(error, context); + } + throw error; + } finally { + if (context) { + await closeProjectStore(context); + } } }