From 55dae49b37fae0f6b50fa1fe5b91c0612e80fb1b Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 8 Jul 2026 19:52:51 -0700 Subject: [PATCH] FN-7704: fix fn agent stop/start hanging up to 60s due to unclosed store handles Fix CLI process exit so `fn agent stop`/`fn agent start` no longer hang up to 60s and eventually time out on repeated retries against the same agent. - Root cause: `resolveProject()` cached an unclosed `TaskStore`, and `createAgentStore()` never closed the `AgentStore` it opened, leaving SQLite handles alive after the command's real work was done. - Add `resolveProjectPathOnly`/`closeProjectStore` helpers in `project-context.ts` so path-only callers never leak a `TaskStore`. - Explicitly close `AgentStore` on every exit/return path in `agent.ts`, since `process.exit()` skips pending `finally` blocks. - Add a bounded fast-fail timeout around the state-store write (default 10s, overridable via `FUSION_AGENT_CMD_TIMEOUT_MS`) so a genuinely stuck operation fails fast with a clear error and non-zero exit instead of hanging. - Add regression tests covering process-exit/store-closing behavior and update CLI reference docs. - Add changeset for the patch release. Files changed: .changeset/fn-7704-agent-cmd-hang-fix.md | 7 + docs/cli-reference.md | 3 + .../commands/__tests__/agent-process-exit.test.ts | 114 +++++++++++ packages/cli/src/commands/__tests__/agent.test.ts | 111 +++++++++- packages/cli/src/commands/agent.ts | 223 ++++++++++++++++----- packages/cli/src/project-context.ts | 44 ++++ 6 files changed, 444 insertions(+), 58 deletions(-) Fusion-Task-Id: FN-7704 Fusion-Task-Lineage: 4679d1a0-3ab8-48ce-86b7-5919bba805fb Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-7704-agent-cmd-hang-fix.md | 7 + docs/cli-reference.md | 3 + .../__tests__/agent-process-exit.test.ts | 114 +++++++++ .../cli/src/commands/__tests__/agent.test.ts | 111 ++++++++- packages/cli/src/commands/agent.ts | 223 ++++++++++++++---- packages/cli/src/project-context.ts | 44 ++++ 6 files changed, 444 insertions(+), 58 deletions(-) create mode 100644 .changeset/fn-7704-agent-cmd-hang-fix.md create mode 100644 packages/cli/src/commands/__tests__/agent-process-exit.test.ts diff --git a/.changeset/fn-7704-agent-cmd-hang-fix.md b/.changeset/fn-7704-agent-cmd-hang-fix.md new file mode 100644 index 0000000000..5c9aa10567 --- /dev/null +++ b/.changeset/fn-7704-agent-cmd-hang-fix.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix `fn agent stop`/`fn agent start` hanging up to 60s per retry instead of exiting. +category: fix +dev: Root cause was non-deterministic CLI process exit, not a DB lock — `resolveProject()` cached an unclosed `TaskStore` and `createAgentStore()` never closed the `AgentStore` it opened, leaving SQLite handles alive after the command's real work finished. Added `resolveProjectPathOnly`/`closeProjectStore` in `project-context.ts` so path-only callers never leak a `TaskStore`, explicit `AgentStore.close()` on every exit/return path in `agent.ts` (since `process.exit()` does not run pending `finally` blocks), and a bounded fast-fail timeout around the state-store write (default 10s, override via `FUSION_AGENT_CMD_TIMEOUT_MS`) so a genuinely stuck operation fails fast with a clear error and non-zero exit instead of hanging. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 76ac59a5db..2463feff5b 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -795,11 +795,13 @@ Pause a running/active agent by transitioning its state to `paused`. - If the agent is already paused, this is a no-op and prints `Agent is already paused`. - Invalid state transitions are rejected with `Cannot stop agent — current state '' cannot transition to 'paused'`. - On success, prints `✓ Agent stopped`. +- The command always closes its store connections and exits promptly on every path (success, already-paused, not-found, invalid-transition) — it never hangs. The underlying state-store write is bounded by a fast-fail deadline (default 10s, override via `FUSION_AGENT_CMD_TIMEOUT_MS`); if it cannot complete in time, the command prints a clear error naming the agent and operation and exits non-zero instead of hanging. Safe to drive from an automated recovery watcher. **Examples:** ```bash fn agent stop AGENT-001 fn agent stop AGENT-001 --project my-project +FUSION_AGENT_CMD_TIMEOUT_MS=5000 fn agent stop AGENT-001 # tighter fast-fail deadline ``` ### `fn agent start` @@ -817,6 +819,7 @@ Resume a paused agent by transitioning its state to `active`. - If the agent is already `active` or `running`, this is a no-op and prints `Agent is already running ()`. - Invalid state transitions are rejected with `Cannot start agent — current state '' cannot transition to 'active'`. - On success, prints `✓ Agent started`. +- Same deterministic-exit and fast-fail-timeout behavior as `fn agent stop` (see above) — the command always closes its store connections and exits promptly, and the state-store write is bounded by `FUSION_AGENT_CMD_TIMEOUT_MS` (default 10s). **Examples:** ```bash diff --git a/packages/cli/src/commands/__tests__/agent-process-exit.test.ts b/packages/cli/src/commands/__tests__/agent-process-exit.test.ts new file mode 100644 index 0000000000..5d2742d469 --- /dev/null +++ b/packages/cli/src/commands/__tests__/agent-process-exit.test.ts @@ -0,0 +1,114 @@ +/** + * FN-7704: `fn agent stop` / `fn agent start` completed their real work but + * left the CLI process's event loop alive — `resolveProject()` cached an + * unclosed `TaskStore` and `createAgentStore()` never closed the + * `AgentStore` it opened. A caller bounding the subprocess with a timeout + * (e.g. a recovery watcher) saw this as a "hang" until it force-killed the + * process at its own 60s ceiling, on every single retry. + * + * This test exercises the REAL modules end-to-end (no `@fusion/core` or + * `project-context.js` mocking) against a temp fixture `.fusion` project so + * it reproduces the actual leaked-handle condition, not just a mocked + * approximation of it. It asserts via `process.getActiveResourcesInfo()` + * that `runAgentStop`/`runAgentStart` do not grow the set of active + * (keep-alive) resources across the transition path AND the + * already-in-target-state early-return path, for BOTH commands. + */ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { TaskStore, AgentStore } from "@fusion/core"; +import { runAgentStop, runAgentStart } from "../agent.js"; + +/** Strict bound the exit-determinism assertions must land within (< 15s per FN-7704's symptom-verification contract; the original failure window was 60s). */ +const STRICT_BOUND_MS = 15_000; + +describe("fn agent stop/start — deterministic process exit (FN-7704)", () => { + let tempDir: string; + let originalCwd: string; + let agentId: string; + + beforeAll(async () => { + tempDir = mkdtempSync(join(tmpdir(), "fn-7704-agent-exit-")); + + // Bootstrap a real .fusion project dir (fusion.db) so CWD auto-detection + // in resolveProject()/resolveProjectPathOnly() resolves this temp dir as + // the project, exercising the REAL TaskStore construction/teardown path. + const bootstrapStore = new TaskStore(tempDir); + await bootstrapStore.init(); + await bootstrapStore.close(); + + // Seed a real, non-ephemeral agent (starts "active") directly via + // AgentStore so the CLI command under test operates on real state. + const seedStore = new AgentStore({ rootDir: join(tempDir, ".fusion") }); + await seedStore.init(); + const agent = await seedStore.createAgent({ name: "fn-7704-fixture-agent", role: "executor" }); + agentId = agent.id; + seedStore.close(); + + originalCwd = process.cwd(); + process.chdir(tempDir); + }, STRICT_BOUND_MS); + + afterAll(() => { + process.chdir(originalCwd); + rmSync(tempDir, { recursive: true, force: true }); + }); + + it( + "runAgentStop (transition path: active -> paused) leaves no net-new active resources", + async () => { + const before = process.getActiveResourcesInfo(); + await runAgentStop(agentId); + const after = process.getActiveResourcesInfo(); + + // FN-7704: before the fix, this call left an open AgentStore SQLite + // handle (and a cached, unclosed TaskStore from project resolution) + // registered as active resources, which is exactly what kept the CLI + // process's event loop alive past the point where the real work was + // done. After the fix, the command's own handles must all be closed + // by the time it returns. + expect(after.length).toBeLessThanOrEqual(before.length); + }, + STRICT_BOUND_MS, + ); + + it( + "runAgentStop (already-paused early-return path) leaves no net-new active resources", + async () => { + const before = process.getActiveResourcesInfo(); + // Agent is already "paused" from the previous test. + await runAgentStop(agentId); + const after = process.getActiveResourcesInfo(); + + expect(after.length).toBeLessThanOrEqual(before.length); + }, + STRICT_BOUND_MS, + ); + + it( + "runAgentStart (transition path: paused -> active) leaves no net-new active resources", + async () => { + const before = process.getActiveResourcesInfo(); + await runAgentStart(agentId); + const after = process.getActiveResourcesInfo(); + + expect(after.length).toBeLessThanOrEqual(before.length); + }, + STRICT_BOUND_MS, + ); + + it( + "runAgentStart (already-active early-return path) leaves no net-new active resources", + async () => { + const before = process.getActiveResourcesInfo(); + // Agent is already "active" from the previous test. + await runAgentStart(agentId); + const after = process.getActiveResourcesInfo(); + + expect(after.length).toBeLessThanOrEqual(before.length); + }, + STRICT_BOUND_MS, + ); +}); diff --git a/packages/cli/src/commands/__tests__/agent.test.ts b/packages/cli/src/commands/__tests__/agent.test.ts index df2793908f..65d64799b7 100644 --- a/packages/cli/src/commands/__tests__/agent.test.ts +++ b/packages/cli/src/commands/__tests__/agent.test.ts @@ -17,9 +17,19 @@ function makeConstructibleMock unknown>(impl?: T) // ── Mock AgentStore ────────────────────────────────────────────────── -const mockGetAgent = vi.fn(); -const mockUpdateAgentState = vi.fn(); -const mockInit = vi.fn().mockResolvedValue(undefined); +// FN-7704: `vi.hoisted` guarantees these mock fns exist before either +// `vi.mock` factory below runs, regardless of source order between the two +// mock blocks (a prior plain-`const` version of the project-context mock +// hit "Cannot access before initialization" because Vitest's hoisting only +// reliably hoists `mock`-prefixed consts declared ahead of the FIRST +// `vi.mock` call in the file). +const { mockGetAgent, mockUpdateAgentState, mockInit, mockClose, mockResolveProjectPathOnly } = vi.hoisted(() => ({ + mockGetAgent: vi.fn(), + mockUpdateAgentState: vi.fn(), + mockInit: vi.fn().mockResolvedValue(undefined), + mockClose: vi.fn(), + mockResolveProjectPathOnly: vi.fn().mockResolvedValue("/tmp/test-project"), +})); // AgentStore mock — vi.fn() with mockImplementation works with `new` in vitest. // We return a plain object from the constructor which becomes the instance. @@ -28,6 +38,7 @@ vi.mock("@fusion/core", () => ({ init: mockInit, getAgent: mockGetAgent, updateAgentState: mockUpdateAgentState, + close: mockClose, })), AGENT_VALID_TRANSITIONS: { idle: ["active"], @@ -40,14 +51,19 @@ vi.mock("@fusion/core", () => ({ // ── Mock project-context ───────────────────────────────────────────── -vi.mock("../project-context.js", () => ({ - resolveProject: vi.fn().mockResolvedValue({ - projectId: "test-project", - projectPath: "/tmp/test-project", - projectName: "test-project", - isRegistered: true, - store: {}, - }), +// FN-7704: this test lives in src/commands/__tests__/, so the module under +// test's "../project-context.js" import resolves to src/project-context.js +// — the mock path here must match that resolved module ("../../ +// project-context.js" from this file), not agent.ts's own relative import +// string. A prior version of this mock pointed at "../project-context.js" +// (i.e. the nonexistent src/commands/project-context.js) and silently never +// applied — getProjectPath's try/catch fallback to the REAL resolveProject +// masked it because only the mocked AgentStore mattered for these tests' +// assertions. Fixed alongside FN-7704 so the mock now actually intercepts +// the call, which enabled asserting resolveProjectPathOnly is used (i.e. +// that no TaskStore is leaked). +vi.mock("../../project-context.js", () => ({ + resolveProjectPathOnly: mockResolveProjectPathOnly, })); // ── Spies ──────────────────────────────────────────────────────────── @@ -94,6 +110,9 @@ describe("runAgentStop", () => { expect(mockUpdateAgentState).toHaveBeenCalledWith("agent-test123", "paused"); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("✓ Agent agent-test123 stopped")); + // FN-7704: store must be closed on the happy path so the CLI process + // does not keep a lingering SQLite handle alive after work is done. + expect(mockClose).toHaveBeenCalledTimes(1); }); it("should stop an active agent", async () => { @@ -103,6 +122,7 @@ describe("runAgentStop", () => { expect(mockUpdateAgentState).toHaveBeenCalledWith("agent-test123", "paused"); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("✓ Agent agent-test123 stopped")); + expect(mockClose).toHaveBeenCalledTimes(1); }); it("should report when agent is not found", async () => { @@ -111,6 +131,11 @@ describe("runAgentStop", () => { await expect(runAgentStop("agent-nonexistent")).rejects.toThrow("process.exit"); expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("agent-nonexistent not found")); expect(exitSpy).toHaveBeenCalledWith(1); + // FN-7704: store must be closed before process.exit on the not-found path. + // (closeAgentStoreSafely may run twice here: once explicitly before + // process.exit, and once more via the outer catch when the mocked + // process.exit throws to unwind the test — both are safe/idempotent.) + expect(mockClose).toHaveBeenCalled(); }); it("should report when agent is already paused", async () => { @@ -121,6 +146,8 @@ describe("runAgentStop", () => { // Should NOT call updateAgentState expect(mockUpdateAgentState).not.toHaveBeenCalled(); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("already paused")); + // FN-7704: store must be closed on the already-in-state early-return path. + expect(mockClose).toHaveBeenCalledTimes(1); }); it("should reject stopping an idle agent (invalid transition)", async () => { @@ -129,6 +156,8 @@ describe("runAgentStop", () => { await expect(runAgentStop("agent-test123")).rejects.toThrow("process.exit"); expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("cannot transition to 'paused'")); expect(exitSpy).toHaveBeenCalledWith(1); + // FN-7704: store must be closed on the invalid-transition path. + expect(mockClose).toHaveBeenCalled(); }); it("should reject stopping an error agent (invalid transition)", async () => { @@ -137,7 +166,41 @@ describe("runAgentStop", () => { await expect(runAgentStop("agent-test123")).rejects.toThrow("process.exit"); expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("cannot transition to 'paused'")); expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockClose).toHaveBeenCalled(); }); + + it("should close the store and resolve the project path without leaking a TaskStore", async () => { + await runAgentStop("agent-test123"); + + // FN-7704: agent commands must resolve the project path via + // resolveProjectPathOnly (not resolveProject) so no TaskStore this + // command never touches is left open/cached. + expect(mockResolveProjectPathOnly).toHaveBeenCalled(); + }); + + it("fast-fails with a clear error and non-zero exit when the store mutation never resolves", async () => { + vi.useFakeTimers(); + try { + // Simulate a store mutation that never resolves (e.g. a stuck/contended write). + mockUpdateAgentState.mockImplementation(() => new Promise(() => {})); + + const resultPromise = runAgentStop("agent-test123"); + // Suppress unhandled rejection warnings until we assert below. + resultPromise.catch(() => {}); + + // Advance past the default bounded fast-fail deadline (10s). + await vi.advanceTimersByTimeAsync(10_000); + + await expect(resultPromise).rejects.toThrow("process.exit"); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining(`Failed to stop agent agent-test123`)); + expect(exitSpy).toHaveBeenCalledWith(1); + // Even on the fast-fail timeout path, the store must still be closed so + // the CLI process exits promptly instead of hanging on the stuck op. + expect(mockClose).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }, 15_000); }); describe("runAgentStart", () => { @@ -156,6 +219,8 @@ describe("runAgentStart", () => { expect(mockUpdateAgentState).toHaveBeenCalledWith("agent-test123", "active"); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("✓ Agent agent-test123 started")); + // FN-7704: store must be closed on the happy path. + expect(mockClose).toHaveBeenCalledTimes(1); }); it("should start an idle agent", async () => { @@ -184,6 +249,8 @@ describe("runAgentStart", () => { await expect(runAgentStart("agent-nonexistent")).rejects.toThrow("process.exit"); expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("agent-nonexistent not found")); expect(exitSpy).toHaveBeenCalledWith(1); + // FN-7704: store must be closed on the not-found path. + expect(mockClose).toHaveBeenCalled(); }); it("should report when agent is already active", async () => { @@ -193,6 +260,8 @@ describe("runAgentStart", () => { expect(mockUpdateAgentState).not.toHaveBeenCalled(); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("already running")); + // FN-7704: store must be closed on the already-in-state early-return path. + expect(mockClose).toHaveBeenCalledTimes(1); }); it("should report when agent is already running", async () => { @@ -202,5 +271,25 @@ describe("runAgentStart", () => { expect(mockUpdateAgentState).not.toHaveBeenCalled(); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("already running")); + expect(mockClose).toHaveBeenCalledTimes(1); }); + + it("fast-fails with a clear error and non-zero exit when the store mutation never resolves", async () => { + vi.useFakeTimers(); + try { + mockUpdateAgentState.mockImplementation(() => new Promise(() => {})); + + const resultPromise = runAgentStart("agent-test123"); + resultPromise.catch(() => {}); + + await vi.advanceTimersByTimeAsync(10_000); + + await expect(resultPromise).rejects.toThrow("process.exit"); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining(`Failed to start agent agent-test123`)); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockClose).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }, 15_000); }); diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index fe47348386..030b893980 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -1,20 +1,23 @@ import { AgentStore, AGENT_VALID_TRANSITIONS } from "@fusion/core"; import type { AgentState } from "@fusion/core"; -import { resolveProject } from "../project-context.js"; +import { resolveProjectPathOnly } from "../project-context.js"; /** * Get the project path for agent operations. * Falls back to process.cwd() if no project is specified. + * + * FNXC:CliAgentControl 2026-07-08-00:00: + * Uses `resolveProjectPathOnly` (not `resolveProject`) so this never leaks a + * `TaskStore` this command has no use for. See `closeProjectStore` in + * project-context.ts for the leak this avoids. */ async function getProjectPath(projectName?: string): Promise { if (projectName) { - const context = await resolveProject(projectName); - return context.projectPath; + return await resolveProjectPathOnly(projectName); } try { - const context = await resolveProject(undefined); - return context.projectPath; + return await resolveProjectPathOnly(undefined); } catch { return process.cwd(); } @@ -30,6 +33,98 @@ async function createAgentStore(projectName?: string): Promise { return agentStore; } +/** + * FNXC:CliAgentControl 2026-07-08-00:00: + * Close the AgentStore best-effort, tolerating a store that is already + * closed (or was never fully opened). `AgentStore.close()` already tolerates + * a "database is not open" error internally; this wrapper additionally + * guards against any other unexpected close-time error so a teardown failure + * never masks the command's real result or blocks process exit. + */ +function closeAgentStoreSafely(agentStore: AgentStore): void { + try { + agentStore.close(); + } catch { + // Best-effort teardown — never let a close failure block exit. + } +} + +/** + * FNXC:CliAgentControl 2026-07-08-00:00: + * Root cause of FN-7704: `fn agent stop`/`fn agent start` did their state + * transition correctly but the CLI process never exited on its own — + * `resolveProject()` cached an unclosed `TaskStore` and `createAgentStore()` + * never closed the `AgentStore` it opened (see `agentStoreDbCache` in + * @fusion/core's agent-store.ts). SQLite-level blocking is already bounded + * (`busy_timeout` = 5s + a ~1s lock-recovery window in packages/core/src/db.ts), + * so the previously observed 60s hang was the CALLER's subprocess timeout, + * not a DB lock — the command's work finished, but a lingering handle kept + * the event loop alive so the process never exited, and a recovery watcher + * driving these commands as subprocesses would force-kill every single + * retry at the same 60s ceiling, blocking recovery entirely. + * + * The fix has two parts, both implemented in this file: + * 1. Deterministic teardown — close the `AgentStore` (via + * `closeAgentStoreSafely`) on EVERY exit path (success, already-in- + * target-state, not-found, invalid-transition, and unexpected error), + * and resolve the project path without leaking a `TaskStore` at all + * (`resolveProjectPathOnly` in project-context.ts). + * 2. A bounded fast-fail guard (`withBoundedTimeout`) around the store + * mutation itself, so that if the operation genuinely cannot complete + * quickly the CLI fails fast with a clear, actionable error and a + * non-zero exit instead of hanging until an external caller kills it. + * The default deadline is intentionally short relative to the 60s + * caller-side ceiling this bug was measured against, and is + * operator-overridable via `FUSION_AGENT_CMD_TIMEOUT_MS` for + * constrained environments. + */ +const DEFAULT_AGENT_CMD_TIMEOUT_MS = 10_000; + +function getAgentCmdTimeoutMs(): number { + const raw = process.env.FUSION_AGENT_CMD_TIMEOUT_MS; + if (!raw) return DEFAULT_AGENT_CMD_TIMEOUT_MS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_AGENT_CMD_TIMEOUT_MS; +} + +/** Raised when a store mutation exceeds the bounded fast-fail deadline. */ +export class AgentCommandTimeoutError extends Error { + constructor(message: string) { + super(message); + this.name = "AgentCommandTimeoutError"; + } +} + +/** + * Race a store mutation against a bounded deadline so a stuck/contended + * operation fails fast with a clear error instead of hanging the CLI + * process. See the FNXC:CliAgentControl block above for rationale. + */ +async function withBoundedTimeout( + operation: () => Promise, + context: { id: string; action: string }, +): Promise { + const timeoutMs = getAgentCmdTimeoutMs(); + let timer: ReturnType | undefined; + try { + return await Promise.race([ + operation(), + new Promise((_, reject) => { + timer = setTimeout(() => { + reject( + new AgentCommandTimeoutError( + `Timed out after ${timeoutMs}ms waiting for agent ${context.id} to ${context.action}. ` + + `The operation may still complete in the background; retry, or increase the deadline via FUSION_AGENT_CMD_TIMEOUT_MS.`, + ), + ); + }, timeoutMs); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} + /** * Stop (pause) a running agent. * Transitions state from running/active to paused. @@ -41,32 +136,49 @@ async function createAgentStore(projectName?: string): Promise { export async function runAgentStop(id: string, projectName?: string): Promise { const agentStore = await createAgentStore(projectName); - const agent = await agentStore.getAgent(id); - if (!agent) { - console.error(`Agent ${id} not found`); - process.exit(1); + function exitWithStore(code: number): never { + closeAgentStoreSafely(agentStore); + return process.exit(code); } - // Already paused — nothing to do - if (agent.state === "paused") { + try { + const agent = await agentStore.getAgent(id); + if (!agent) { + console.error(`Agent ${id} not found`); + exitWithStore(1); + } + + // Already paused — nothing to do + if (agent.state === "paused") { + console.log(); + console.log(` Agent ${id} is already paused`); + console.log(); + closeAgentStoreSafely(agentStore); + return; + } + + // Validate transition locally + const validTargets = AGENT_VALID_TRANSITIONS[agent.state as AgentState]; + if (!validTargets || !validTargets.includes("paused")) { + console.error(`Cannot stop agent ${id} — current state '${agent.state}' cannot transition to 'paused'`); + exitWithStore(1); + } + + try { + await withBoundedTimeout(() => agentStore.updateAgentState(id, "paused"), { id, action: "stop (transition to paused)" }); + } catch (err) { + console.error(`Failed to stop agent ${id}: ${err instanceof Error ? err.message : String(err)}`); + exitWithStore(1); + } + console.log(); - console.log(` Agent ${id} is already paused`); + console.log(` ✓ Agent ${id} stopped`); console.log(); - return; + closeAgentStoreSafely(agentStore); + } catch (err) { + closeAgentStoreSafely(agentStore); + throw err; } - - // Validate transition locally - const validTargets = AGENT_VALID_TRANSITIONS[agent.state as AgentState]; - if (!validTargets || !validTargets.includes("paused")) { - console.error(`Cannot stop agent ${id} — current state '${agent.state}' cannot transition to 'paused'`); - process.exit(1); - } - - await agentStore.updateAgentState(id, "paused"); - - console.log(); - console.log(` ✓ Agent ${id} stopped`); - console.log(); } /** @@ -76,30 +188,47 @@ export async function runAgentStop(id: string, projectName?: string): Promise { const agentStore = await createAgentStore(projectName); - const agent = await agentStore.getAgent(id); - if (!agent) { - console.error(`Agent ${id} not found`); - process.exit(1); + function exitWithStore(code: number): never { + closeAgentStoreSafely(agentStore); + return process.exit(code); } - // Already active/running — nothing to do - if (agent.state === "active" || agent.state === "running") { + try { + const agent = await agentStore.getAgent(id); + if (!agent) { + console.error(`Agent ${id} not found`); + exitWithStore(1); + } + + // Already active/running — nothing to do + if (agent.state === "active" || agent.state === "running") { + console.log(); + console.log(` Agent ${id} is already running (${agent.state})`); + console.log(); + closeAgentStoreSafely(agentStore); + return; + } + + // Validate transition locally + const validTargets = AGENT_VALID_TRANSITIONS[agent.state as AgentState]; + if (!validTargets || !validTargets.includes("active")) { + console.error(`Cannot start agent ${id} — current state '${agent.state}' cannot transition to 'active'`); + exitWithStore(1); + } + + try { + await withBoundedTimeout(() => agentStore.updateAgentState(id, "active"), { id, action: "start (transition to active)" }); + } catch (err) { + console.error(`Failed to start agent ${id}: ${err instanceof Error ? err.message : String(err)}`); + exitWithStore(1); + } + console.log(); - console.log(` Agent ${id} is already running (${agent.state})`); + console.log(` ✓ Agent ${id} started`); console.log(); - return; + closeAgentStoreSafely(agentStore); + } catch (err) { + closeAgentStoreSafely(agentStore); + throw err; } - - // Validate transition locally - const validTargets = AGENT_VALID_TRANSITIONS[agent.state as AgentState]; - if (!validTargets || !validTargets.includes("active")) { - console.error(`Cannot start agent ${id} — current state '${agent.state}' cannot transition to 'active'`); - process.exit(1); - } - - await agentStore.updateAgentState(id, "active"); - - console.log(); - console.log(` ✓ Agent ${id} started`); - console.log(); } diff --git a/packages/cli/src/project-context.ts b/packages/cli/src/project-context.ts index e0ccf6d795..f32b887595 100644 --- a/packages/cli/src/project-context.ts +++ b/packages/cli/src/project-context.ts @@ -312,3 +312,47 @@ export async function getStore( const context = await resolveProject(projectName, cwd, globalDir); return context.store; } + +/** + * FNXC:CliAgentControl 2026-07-08-00:00: + * Close a resolved project's `TaskStore` and evict it from `storeCache` when + * it is that store's owner. `resolveProject()` always constructs (and, for + * registered/CWD-detected projects, caches) a `TaskStore` even when a caller + * only needs the resolved `projectPath` — e.g. `fn agent stop/start` + * (packages/cli/src/commands/agent.ts) never touches `context.store` at all. + * An unclosed cached store keeps the underlying SQLite connection (and any + * handles it owns) alive, which can keep the CLI process's event loop alive + * past the point where the command's real work is done — the process never + * exits on its own, so a caller bounding the subprocess with a timeout (e.g. + * a recovery watcher) sees a false "hang" until it force-kills at its own + * deadline. Close+evict is best-effort and idempotent so it is safe even if + * another in-process caller already holds/closed the same cached instance. + */ +export async function closeProjectStore(context: ProjectContext): Promise { + try { + await context.store.close(); + } catch { + // Best-effort: an already-closed store (or one closed by a concurrent + // in-process caller) must not throw here. + } + if (storeCache.get(context.projectId) === context.store) { + storeCache.delete(context.projectId); + } +} + +/** + * FNXC:CliAgentControl 2026-07-08-00:00: + * Resolve only the project PATH without leaking the `TaskStore` that + * `resolveProject()` constructs internally. Use this instead of + * `resolveProject()` when a command has no use for `context.store` (see + * `closeProjectStore` above for the underlying leak this avoids). + */ +export async function resolveProjectPathOnly( + projectNameFlag?: string, + cwd: string = process.cwd(), + globalDir?: string, +): Promise { + const context = await resolveProject(projectNameFlag, cwd, globalDir); + await closeProjectStore(context); + return context.projectPath; +}