Merge pull request #1704 from Runfusion/gsxdsm/engine-not-running

fix: report engine available when another fusion process owns it
This commit is contained in:
gsxdsm
2026-06-21 03:38:11 -07:00
committed by GitHub
7 changed files with 409 additions and 5 deletions

View File

@@ -0,0 +1,5 @@
---
"@runfusion/fusion": patch
---
Fix a false "engine not running" banner when another fusion process on the same machine already owns the engine. The dashboard's health check only counted engines this process started, so a second launch (e.g. `pnpm dev dashboard` alongside an already-running `fusion`) that was correctly refused the per-machine engine singleton lock reported the engine as unavailable — even though one was running. The `ProjectEngineManager` now tracks engines owned by another process (detected via `EngineAlreadyRunningError` from the singleton lock) and exposes `hasRunningEngine()`, which the dashboard health endpoint uses so the banner reflects machine-level truth. Reconciliation still retries so this process takes over if the other exits, and the "refusing to start" log is emitted once per project instead of on every reconciliation tick.

View File

@@ -82,6 +82,9 @@ The core board entity: a unit of work that moves through columns (triage, todo,
### Workflow Runtime
The authoritative task lifecycle runtime. It resolves a Task to workflow IR, walks the graph, routes node outcomes, and invokes runtime primitives for side effects. The engine substrate still owns scheduling, routing claims, persistence, concurrency, process supervision, storage, and audit plumbing; lifecycle policy lives in workflow nodes and built-in workflow IR.
### Engine Singleton Lock
A per-machine mutual-exclusion guard ensuring only one fusion process runs the engine for a given project, combining a lockfile in the project's `.fusion/` directory with a per-project loopback socket. Failure to acquire it (`EngineAlreadyRunningError`) is **positive proof an engine is already running** for that project elsewhere on the machine — not an error to swallow and not "no engine." A process refused the lock keeps that as a fact: it reports the engine as available (so UI surfaces don't claim it's down) while reconciliation keeps retrying, so it takes over if the current owner exits.
### ACP Ask Path
A one-turn read-only model ask routed through the ACP runtime rather than a CLI print mode. The runner accumulates streamed prose, may recover a trailing JSON object for structured seams, and treats abnormal ACP stop reasons as incomplete answers for validator use.

View File

@@ -0,0 +1,131 @@
---
title: "EngineAlreadyRunningError means an engine IS running, not 'no engine'"
date: "2026-06-21"
category: integration-issues
module: "packages/engine project-engine-manager + packages/dashboard server health"
problem_type: integration_issue
component: tooling
symptoms:
- "Dashboard shows a persistent 'engine not running' banner even though an engine is running"
- "Logs repeat 'Refusing to start engine for <projectId>: Another engine is already running...' every 30s reconciliation tick"
- "Reconciliation tries to start engines for N projects, then errors because each is already running"
root_cause: logic_error
resolution_type: code_fix
severity: medium
last_updated: "2026-06-21"
related_components:
- engine
- dashboard
- development_workflow
tags:
- engine-singleton-lock
- multi-process
- reconciliation
- dashboard-health
- regression
related_prs:
- "https://github.com/Runfusion/Fusion/pull/1704"
- "https://github.com/Runfusion/Fusion/pull/1699"
---
## Problem
Starting a second fusion process (e.g. `pnpm dev dashboard --no-auth` while an installed `pnpm fusion` is already running) showed a false **"engine not running"** banner — even though an engine was live on the machine for every project.
## Symptoms
- Banner visible despite a running engine.
- Repeating log line every reconciliation tick: `Refusing to start engine for <projectId>: Another engine is already running for project <projectId> on this machine (blocked by socket)`.
- The collision affects every registered project (the log "tries to start engines for two projects then refuses" maps 1:1 to the live `fusion-engine-<hash>.sock` files under `$TMPDIR`).
## What Didn't Work
- Looking only at the dashboard frontend / banner component — the banner faithfully renders `dashboardHealth.engine.available === false`; the wrong value comes from the server.
- Treating it as a warm-up race (engines still starting in the background). The banner is *permanent*, not transient: reconciliation retries forever and keeps refusing, because the lock is held by a **different process** that never exits.
- **Tempting wrong fix: make `reconcile()`/`has()` treat external engines as "owned" so it stops retrying.** This would silence the logs but break failover — the process would never re-attempt and never adopt the socket when the holder dies. `has()` (this-process-owns) and `hasRunningEngine()` (machine-level truth) must stay *separate*: reconciliation keys off `has()` and keeps retrying; the banner keys off `hasRunningEngine()`.
## Root Cause
The engine uses a **per-machine singleton lock** (`packages/engine/src/engine-singleton-lock.ts`): a lockfile under `<workingDir>/.fusion/engine.lock` plus a loopback socket `fusion-engine-<sha1(projectId)[:16]>.sock` (the hash is the first 16 hex chars of the sha1, not the full digest). Only one fusion process may own the engine for a given project. When a second process launches, `acquireEngineSingleton` correctly throws `EngineAlreadyRunningError` — **positive proof an engine is live** for that project.
But that proof was discarded. In `ProjectEngineManager.createAndStart` the catch logged "Refusing to start" and rethrew **without recording that an engine exists**. The dashboard's health check `hasDashboardEngine` (`packages/dashboard/src/server.ts`) then only counted engines *this* process owns:
```ts
// before
function hasDashboardEngine(options?: ServerOptions): boolean {
const engines = options?.engineManager?.getAllEngines?.();
return Boolean(options?.engine || (engines && engines.size > 0));
}
```
So `getAllEngines()` was empty → `/api/health` reported `engine.available: false` → banner. Three views of "is an engine running" disagreed: the singleton lock (machine truth: *yes*), reconciliation's `has()` (this-process-owns: *no*), and the banner (this-process-owns: *no*).
Surfaced by `feat: start engines by default` (#1699), which made the dashboard actively attempt engine startup instead of running UI-only.
## Solution
Track engines owned by **another** process and report them as available, while still retrying so this process can take over if the other dies.
```ts
// ProjectEngineManager
private externalEngines = new Set<string>();
hasRunningEngine(): boolean {
return this.engines.size > 0 || this.externalEngines.size > 0;
}
// in createAndStart's acquireEngineSingleton catch:
if (err instanceof EngineAlreadyRunningError) {
if (!this.externalEngines.has(projectId)) { // log once, not every tick
runtimeLog.warn(`Refusing to start engine for ${projectId}: ${err.message}`);
}
this.externalEngines.add(projectId);
}
throw err;
// after acquiring the lock (BEFORE engine.start()): this.externalEngines.delete(projectId);
// — acquiring proves no other process owns it; clearing here (not after a
// successful start()) avoids a phantom entry if start() then throws.
// in stopAll(): this.externalEngines.clear();
```
The outer fire-and-forget catches that drive startup — `reconcile()`, `startAll()`, and `onProjectAccessed()` — early-return on `EngineAlreadyRunningError` so they don't re-log `Failed to start engine…` on every 30s tick (`createAndStart` already logged it once). Only genuinely unexpected errors warn.
```ts
// dashboard server.ts — consult machine-level truth, fall back for old managers/test doubles
function hasDashboardEngine(options?: ServerOptions): boolean {
if (options?.engine) return true;
const manager = options?.engineManager;
if (!manager) return false;
if (typeof manager.hasRunningEngine === "function") return manager.hasRunningEngine();
const engines = manager.getAllEngines?.();
return Boolean(engines && engines.size > 0);
}
```
Reconciliation deliberately keeps retrying (it does NOT treat external engines as `has()`), so when the lock-holder exits this process acquires the lock and clears the external marker — verified live: killing the holder let the dev dashboard immediately adopt the socket.
## Why This Works
`EngineAlreadyRunningError` is the *only* signal that distinguishes "no engine anywhere" from "an engine runs in another process." Capturing it makes the dashboard report machine-level reality instead of process-local ownership. Automation still runs because the owning process's engine polls the shared DB; the second process is a correct UI-over-the-same-store.
## Prevention
- **Treat lock-acquisition failure as evidence, not an error to swallow.** A failed mutual-exclusion acquire (lock/socket/PID) usually proves the guarded resource *is* alive elsewhere — don't let downstream "is it running?" checks read process-local state instead of the lock's machine-level truth.
- **Keep `has()` and `hasRunningEngine()` semantically distinct.** `has()` = "this process owns/ is starting it" (drives retry/reconcile); `hasRunningEngine()` = "an engine is live on this machine, owned by anyone" (drives the banner). Merging them would silently kill failover *and* still pass the banner test, so a regression would be invisible.
- Tests added (had zero prior coverage — they assumed the manager always owns what it starts):
- `packages/engine/src/__tests__/project-engine-manager.test.ts`: reports a running engine when the lock is held elsewhere; logs the refusal once across repeated attempts; stays quiet across reconciliation ticks (no repeated `Failed to start…`); takes over and clears the marker once the lock frees; clears the marker even if the takeover `start()` throws.
- `packages/dashboard/src/__tests__/server.test.ts`: `/api/health` reports `available: true` when another process owns the engine; legacy fallback to `getAllEngines()` when `hasRunningEngine` is absent.
### Operational gotcha
`pnpm fusion` launches the full dashboard + engine and **never exits**. Piping it (e.g. `pnpm fusion 2>&1 | grep -i auth` to inspect auth output) leaves the server running in the background, orphaned, holding the engine singleton sockets indefinitely. To find/clear a stray holder:
```bash
lsof "$TMPDIR"/fusion-engine-*.sock # PID owning the engine locks
kill <pid> # releases the locks; reconciliation in any waiting process takes over
```
## See also
- [`developer-experience/browser-testing-dashboard-from-worktree-safely.md`](../developer-experience/browser-testing-dashboard-from-worktree-safely.md) — running a second dashboard process safely (dual-engine / shared-DB hazard). Its "a dev dashboard is engine-disabled" framing predates `start engines by default` (#1699); post-#1699/#1704 a second dashboard *does* attempt startup and reports machine-level availability, so prefer the machine-vs-process ownership model described here.

View File

@@ -415,6 +415,42 @@ describe("createServer health and headless mode", () => {
expect(res.body.engine).toEqual({ available: true });
});
it("reports the engine available when another fusion process owns it", async () => {
// The manager could not acquire the singleton lock (engine owned by another
// process on this machine) so it owns no engine instances, but it knows one
// is running. The banner must stay hidden.
const store = createMockStore();
const app = createServer(store, {
engineManager: {
getAllEngines: vi.fn().mockReturnValue(new Map()),
hasRunningEngine: vi.fn().mockReturnValue(true),
getEngine: vi.fn(),
} as any,
});
const res = await GET(app, "/api/health");
expect(res.status).toBe(200);
expect(res.body.engine).toEqual({ available: true });
});
it("falls back to owned engines when hasRunningEngine is unavailable", async () => {
// Backward-compat: an older engineManager / test double without
// hasRunningEngine must still report availability from getAllEngines().
const store = createMockStore();
const app = createServer(store, {
engineManager: {
getAllEngines: vi.fn().mockReturnValue(new Map([["p1", {}]])),
getEngine: vi.fn(),
} as any,
});
const res = await GET(app, "/api/health");
expect(res.status).toBe(200);
expect(res.body.engine).toEqual({ available: true });
});
it("reports degraded status when database corruption is detected", async () => {
const store = createMockStore({
getDatabaseHealth: vi.fn().mockReturnValue({

View File

@@ -479,8 +479,24 @@ export interface ServerOptions {
}
function hasDashboardEngine(options?: ServerOptions): boolean {
const engines = options?.engineManager?.getAllEngines?.();
return Boolean(options?.engine || (engines && engines.size > 0));
if (options?.engine) return true;
const manager = options?.engineManager;
if (!manager) return false;
/*
* FNXC:DashboardHealth 2026-06-21-03:30:
* Engine availability must reflect machine-level truth, not only engines
* owned by this dashboard process. `hasRunningEngine` counts engines owned
* by this process AND engines owned by another fusion process on the machine
* (detected via the singleton lock); without the latter a UI-only launch
* alongside an already-running engine shows a false "engine not running"
* banner. Fall back to the owned-engine map for older manager instances /
* test doubles.
*/
if (typeof manager.hasRunningEngine === "function") {
return manager.hasRunningEngine();
}
const engines = manager.getAllEngines?.();
return Boolean(engines && engines.size > 0);
}
type DashboardExpressApp = ReturnType<typeof express> & {

View File

@@ -37,6 +37,10 @@ vi.mock("../project-engine.js", () => {
import { ProjectEngineManager } from "../project-engine-manager.js";
import { ProjectEngine } from "../project-engine.js";
import {
acquireEngineSingleton,
EngineAlreadyRunningError,
} from "../engine-singleton-lock.js";
import type { RegisteredProject, CentralCore } from "@fusion/core";
function createMockCentralCore(projects: RegisteredProject[]): CentralCore {
@@ -702,4 +706,163 @@ describe("ProjectEngineManager", () => {
await manager.stopAll();
});
});
describe("engine owned by another fusion process", () => {
// These tests override the singleton-lock mock with persistent rejections.
// Restore the default "acquire succeeds" behaviour after each so later
// tests/describes are unaffected even if an assertion throws mid-test.
afterEach(() => {
const acquire = acquireEngineSingleton as ReturnType<typeof vi.fn>;
acquire.mockReset();
acquire.mockResolvedValue({
release: vi.fn().mockResolvedValue(undefined),
socketPath: "/tmp/test.sock",
lockFilePath: "/tmp/test.lock",
});
});
it("reports a running engine when the singleton lock is held elsewhere", async () => {
const manager = new ProjectEngineManager(centralCore);
// Another fusion process on this machine already owns the engine: the
// singleton lock cannot be acquired.
(acquireEngineSingleton as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
new EngineAlreadyRunningError("proj_aaa", "socket"),
);
await expect(manager.ensureEngine("proj_aaa")).rejects.toBeInstanceOf(
EngineAlreadyRunningError,
);
// We do not own an engine instance...
expect(manager.getEngine("proj_aaa")).toBeUndefined();
expect(manager.getAllEngines().size).toBe(0);
// ...but an engine IS running on the machine, so the dashboard must not
// show an "engine not running" banner.
expect(manager.hasRunningEngine()).toBe(true);
expect(manager.getExternalEngineIds().has("proj_aaa")).toBe(true);
});
it("logs the refusal only once across repeated reconciliation attempts", async () => {
const manager = new ProjectEngineManager(centralCore);
const acquire = acquireEngineSingleton as ReturnType<typeof vi.fn>;
acquire.mockRejectedValue(
new EngineAlreadyRunningError("proj_aaa", "socket"),
);
const warnSpy = vi.spyOn(
(await import("../logger.js")).runtimeLog,
"warn",
);
await expect(manager.ensureEngine("proj_aaa")).rejects.toBeInstanceOf(
EngineAlreadyRunningError,
);
await expect(manager.ensureEngine("proj_aaa")).rejects.toBeInstanceOf(
EngineAlreadyRunningError,
);
const refusals = warnSpy.mock.calls.filter(([msg]) =>
String(msg).includes("Refusing to start engine for proj_aaa"),
);
expect(refusals).toHaveLength(1);
warnSpy.mockRestore();
});
it("takes over and clears the external marker once the lock is free", async () => {
const manager = new ProjectEngineManager(centralCore);
const acquire = acquireEngineSingleton as ReturnType<typeof vi.fn>;
acquire.mockRejectedValueOnce(
new EngineAlreadyRunningError("proj_aaa", "socket"),
);
await expect(manager.ensureEngine("proj_aaa")).rejects.toBeInstanceOf(
EngineAlreadyRunningError,
);
expect(manager.getExternalEngineIds().has("proj_aaa")).toBe(true);
// The other process exits; the next attempt acquires the lock (the
// default mock resolves) and we take ownership.
const engine = await manager.ensureEngine("proj_aaa");
expect(engine).toBeDefined();
expect(manager.getEngine("proj_aaa")).toBeDefined();
expect(manager.getExternalEngineIds().has("proj_aaa")).toBe(false);
expect(manager.hasRunningEngine()).toBe(true);
});
it("stays quiet across reconciliation ticks for an externally-owned engine", async () => {
// Drives the reconciliation wrapper (not just a direct ensureEngine call):
// across many intervals neither the inner "Refusing to start" warning nor
// the outer "Failed to start engine..." reconciliation warning should
// repeat for the same externally-owned engine.
vi.useFakeTimers();
const manager = new ProjectEngineManager(centralCore);
const acquire = acquireEngineSingleton as ReturnType<typeof vi.fn>;
acquire.mockRejectedValue(
new EngineAlreadyRunningError("proj_aaa", "socket"),
);
const warnSpy = vi.spyOn(
(await import("../logger.js")).runtimeLog,
"warn",
);
manager.startReconciliation(1000);
// Immediate tick + several scheduled ticks.
for (let i = 0; i < 4; i++) {
await vi.advanceTimersByTimeAsync(1000);
}
manager.stopReconciliation();
const refusals = warnSpy.mock.calls.filter(([msg]) =>
String(msg).includes("Refusing to start engine for proj_aaa"),
);
const failures = warnSpy.mock.calls.filter(([msg]) =>
String(msg).includes("Failed to start engine for project proj_aaa"),
);
expect(refusals).toHaveLength(1); // deduped to first detection
expect(failures).toHaveLength(0); // reconciliation swallows expected error
expect(manager.getExternalEngineIds().has("proj_aaa")).toBe(true);
warnSpy.mockRestore();
vi.useRealTimers();
});
it("clears the external marker even if the takeover start fails", async () => {
// External owner first, then it exits so acquire succeeds — but
// engine.start() throws. The marker must be cleared (at acquire time) so
// hasRunningEngine() does not report a phantom engine that never started.
const manager = new ProjectEngineManager(centralCore);
const acquire = acquireEngineSingleton as ReturnType<typeof vi.fn>;
acquire.mockRejectedValueOnce(
new EngineAlreadyRunningError("proj_aaa", "socket"),
);
await expect(manager.ensureEngine("proj_aaa")).rejects.toBeInstanceOf(
EngineAlreadyRunningError,
);
expect(manager.getExternalEngineIds().has("proj_aaa")).toBe(true);
// Owner exits: acquire succeeds (default mock), but the engine fails to start.
(ProjectEngine as unknown as ReturnType<typeof vi.fn>).mockImplementationOnce(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function (config: any) {
return {
start: vi.fn().mockRejectedValue(new Error("boom")),
stop: vi.fn().mockResolvedValue(undefined),
getTaskStore: vi.fn().mockReturnValue({ projectId: config.projectId }),
_config: config,
};
},
);
await expect(manager.ensureEngine("proj_aaa")).rejects.toThrow("boom");
expect(manager.getExternalEngineIds().has("proj_aaa")).toBe(false);
expect(manager.getEngine("proj_aaa")).toBeUndefined();
expect(manager.hasRunningEngine()).toBe(false);
});
});
});

View File

@@ -51,6 +51,18 @@ export class ProjectEngineManager {
private engines = new Map<string, ProjectEngine>();
private starting = new Map<string, Promise<ProjectEngine>>();
private singletonLocks = new Map<string, EngineSingletonLock>();
/**
* FNXC:DashboardHealth 2026-06-21-03:30:
* Engine availability must reflect machine-level truth, not only engines this
* process owns. Projects whose engine is owned by ANOTHER fusion process on
* this machine are tracked here, populated when `acquireEngineSingleton`
* rejects with {@link EngineAlreadyRunningError} — that error is positive
* proof an engine is live for the project, just not owned by us. We keep
* retrying to start (so we take over if the other process dies), but the
* dashboard must report the engine as available rather than showing a false
* "engine not running" banner.
*/
private externalEngines = new Set<string>();
private stopped = false;
/**
@@ -110,6 +122,21 @@ export class ProjectEngineManager {
return this.engines;
}
/**
* Whether an engine is running for any project on this machine — including
* engines owned by another fusion process (detected via the singleton lock).
* Drives the dashboard's "engine available" health so a UI-only launch
* alongside an already-running engine does not show a false banner.
*/
hasRunningEngine(): boolean {
return this.engines.size > 0 || this.externalEngines.size > 0;
}
/** Project ids whose engine is owned by another fusion process on this machine. */
getExternalEngineIds(): ReadonlySet<string> {
return this.externalEngines;
}
/** Get the TaskStore for a project from its engine. */
getStore(projectId: string): TaskStore | undefined {
return this.engines.get(projectId)?.getTaskStore();
@@ -223,6 +250,9 @@ export class ProjectEngineManager {
for (const result of results) {
if (result.status === "fulfilled") {
started++;
} else if (result.reason instanceof EngineAlreadyRunningError) {
// Engine owned by another process — expected, already logged once.
continue;
} else {
failed++;
runtimeLog.warn(`Engine start failed: ${result.reason}`);
@@ -262,6 +292,7 @@ export class ProjectEngineManager {
await Promise.all(stops);
this.engines.clear();
this.starting.clear();
this.externalEngines.clear();
// Release all singleton locks so another fusion process can take over.
const releases = Array.from(this.singletonLocks.values()).map((lock) =>
@@ -295,6 +326,8 @@ export class ProjectEngineManager {
onProjectAccessed(projectId: string): void {
if (this.has(projectId)) return;
this.ensureEngine(projectId).catch((err) => {
// Expected when another process owns the engine — already logged once.
if (err instanceof EngineAlreadyRunningError) return;
const message = err instanceof Error ? err.message : String(err);
runtimeLog.warn(
`Failed to start engine for project ${projectId}: ${message}`,
@@ -383,6 +416,11 @@ export class ProjectEngineManager {
for (const project of missing) {
if (this.stopped || this.reconciliationStopped) break;
this.ensureEngine(project.id).catch((err) => {
// An engine owned by another process is expected, not a failure —
// createAndStart already logged it once and recorded it in
// externalEngines. Swallow it here so reconciliation doesn't warn
// every interval for the same externally-owned engine.
if (err instanceof EngineAlreadyRunningError) return;
const message = err instanceof Error ? err.message : String(err);
runtimeLog.warn(
`Failed to start engine for project ${project.id}: ${message}`,
@@ -427,13 +465,25 @@ export class ProjectEngineManager {
},
).catch((err) => {
if (err instanceof EngineAlreadyRunningError) {
runtimeLog.warn(
`Refusing to start engine for ${projectId}: ${err.message}`,
);
// An engine IS running for this project — another fusion process owns
// it. Record it so the dashboard reports the engine as available, and
// log only on the first detection to avoid spamming every 30s
// reconciliation tick while the other process stays alive.
if (!this.externalEngines.has(projectId)) {
runtimeLog.warn(
`Refusing to start engine for ${projectId}: ${err.message}`,
);
}
this.externalEngines.add(projectId);
}
throw err;
});
this.singletonLocks.set(projectId, singleton);
// Acquiring the singleton proves no other process owns the engine, so clear
// any prior "owned by another process" marker now — before engine.start().
// If start fails below we release the lock and a later tick retries; leaving
// the marker set here would make hasRunningEngine() report a phantom engine.
this.externalEngines.delete(projectId);
const engine = new ProjectEngine(
runtimeConfig,