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) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7704-agent-cmd-hang-fix.md
Normal file
7
.changeset/fn-7704-agent-cmd-hang-fix.md
Normal file
@@ -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.
|
||||
@@ -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 <id> is already paused`.
|
||||
- Invalid state transitions are rejected with `Cannot stop agent <id> — current state '<state>' cannot transition to 'paused'`.
|
||||
- On success, prints `✓ Agent <id> 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 <id> is already running (<state>)`.
|
||||
- Invalid state transitions are rejected with `Cannot start agent <id> — current state '<state>' cannot transition to 'active'`.
|
||||
- On success, prints `✓ Agent <id> 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
|
||||
|
||||
114
packages/cli/src/commands/__tests__/agent-process-exit.test.ts
Normal file
114
packages/cli/src/commands/__tests__/agent-process-exit.test.ts
Normal file
@@ -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,
|
||||
);
|
||||
});
|
||||
@@ -17,9 +17,19 @@ function makeConstructibleMock<T extends (...args: any[]) => 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);
|
||||
});
|
||||
|
||||
@@ -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<string> {
|
||||
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<AgentStore> {
|
||||
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<T>(
|
||||
operation: () => Promise<T>,
|
||||
context: { id: string; action: string },
|
||||
): Promise<T> {
|
||||
const timeoutMs = getAgentCmdTimeoutMs();
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
return await Promise.race([
|
||||
operation(),
|
||||
new Promise<never>((_, 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<AgentStore> {
|
||||
export async function runAgentStop(id: string, projectName?: string): Promise<void> {
|
||||
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<vo
|
||||
export async function runAgentStart(id: string, projectName?: string): Promise<void> {
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -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<void> {
|
||||
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<string> {
|
||||
const context = await resolveProject(projectNameFlag, cwd, globalDir);
|
||||
await closeProjectStore(context);
|
||||
return context.projectPath;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user