diff --git a/.specs/features/daemon-single-instance/spec.md b/.specs/features/daemon-single-instance/spec.md new file mode 100644 index 0000000..e3c0d84 --- /dev/null +++ b/.specs/features/daemon-single-instance/spec.md @@ -0,0 +1,42 @@ +# Daemon Single Instance Specification + +## Problem Statement + +Uma máquina real acumulou 10 processos `dist/daemon/daemon.js --daemon` no mesmo `~/.run-agent`. O caminho: `isDaemonRunning()` (`src/daemon/ipc.ts`) desiste do socket após 1s; o daemon usa `DatabaseSync` síncrono, então uma query/checkpoint/espera de `busy_timeout` longa bloqueia o `accept`; o CLI conclui que não há daemon e `ensureDaemonStarted()` sobe outro; `createIpcServer()` deslinka o socket vivo; o daemon antigo segue vivo, sem socket, ainda escrevendo no SQLite; o novo `recover()` reatacha as mesmas sessões. + +Com N daemons tailando o mesmo log e escrevendo no mesmo banco, o `BEGIN` deferred de `attachDriverEvents` sofre `SQLITE_BUSY_SNAPSHOT` (errcode 517, imediato, ignora `busy_timeout`), o `catch` marca a sessão `failed` com `database is locked` e sai do loop, enquanto o harness continua rodando (sessão `df94`, `codex exec resume` vivo após o "fail"). + +## Goals + +- [x] No máximo um daemon por `RUN_AGENT_DIR`, garantido pelo SO +- [x] Um daemon perdedor não migra, não reatacha e não toca no socket +- [x] Nenhum lock stale: morte por `SIGKILL` libera o lock +- [ ] (fase 2, spec própria) Loop de eventos resistente a `SQLITE_BUSY`: `BEGIN IMMEDIATE`, retry, nunca `failed` com PID vivo +- [ ] (fase 2) Reconciliador: sessão `failed` por erro de store com PID vivo é reatachada + +## Out of Scope + +| Feature | Reason | +| ------- | ------ | +| Matar daemons extras já rodando | Ação manual única (`kill -9`, não `SIGTERM`, que marcaria sessões `interrupted`) | +| Aumentar o timeout do probe no CLI | Com o lock, um spawn a mais é inofensivo: sai com 0 e o CLI continua pollando | + +## Decisions + +| Decisão | Escolha | Racional | +| ------- | ------- | -------- | +| Mecanismo | Arquivo SQLite `daemon.lock` em `locking_mode=EXCLUSIVE`, `busy_timeout=0` | Lock fcntl liberado pelo kernel na morte do processo; `node:sqlite` já é dependência; Node não expõe `flock` | +| Arquivo separado | Não usar `run-agent.db` | Lock exclusivo no banco principal bloquearia CLIs read-only (`usage`) | +| Onde | Entrada `--daemon`, antes de `new Daemon()` | O perdedor não roda migração nem `recover()` | +| Saída do perdedor | `exit 0` + linha em `daemon.log` | `ensureDaemonStarted` segue pollando e conecta no daemon existente | + +## Acceptance Criteria + +1. WHEN um daemon segura o lock THEN um segundo `acquireInstanceLock` (mesmo processo ou outro) SHALL retornar `null` +2. WHEN o dono do lock morre por `SIGKILL` THEN o próximo daemon SHALL adquirir o lock sem limpeza +3. WHEN `daemon.js --daemon` inicia com o lock ocupado THEN SHALL sair com 0 sem abrir o banco nem deslinkar o socket + +## Validation + +- `tests/daemon-instance-lock.test.ts` (AC 1, 2) +- Manual contra `dist/`: dois `daemon.js --daemon` no mesmo `RUN_AGENT_DIR` → segundo sai 0, `daemon.log` registra, socket e `daemon.pid` seguem do primeiro; após `kill -9` do primeiro, um novo sobe (AC 3) diff --git a/src/config/paths.ts b/src/config/paths.ts index e0f89de..01f73fc 100644 --- a/src/config/paths.ts +++ b/src/config/paths.ts @@ -43,6 +43,7 @@ export function getPaths() { db: path.join(base, "run-agent.db"), daemonSock: path.join(base, "daemon.sock"), daemonPid: path.join(base, "daemon.pid"), + daemonLock: path.join(base, "daemon.lock"), daemonLog: path.join(base, "daemon.log"), logsDir: path.join(base, "logs"), worktreesDir: path.join(base, "worktrees"), diff --git a/src/daemon/daemon.ts b/src/daemon/daemon.ts index 6045cbf..7ac4e16 100644 --- a/src/daemon/daemon.ts +++ b/src/daemon/daemon.ts @@ -9,6 +9,7 @@ import { EventStore } from "../store/events.js"; import { ClaimsStore } from "../store/claims.js"; import { getPaths, ensureDirs } from "../config/paths.js"; import { createIpcServer } from "./ipc.js"; +import { acquireInstanceLock, type InstanceLock } from "./instance-lock.js"; import type { IpcRequest, IpcResponse, UsageQueryParams, WebEnsureParams, WebEnsureResult } from "./protocol.js"; import { WebEnsureError, WebSupervisor, type WebSupervisorOptions } from "./web-supervisor.js"; import { getRegistry } from "../drivers/registry.js"; @@ -2025,8 +2026,20 @@ class Daemon { } } +// Held for the process lifetime (module scope so it is never collected); the +// kernel drops it on exit, so shutdown does not release it early. +let instanceLock: InstanceLock | null = null; + // Entry if (process.argv.includes("--daemon")) { + // Take the lock before opening run-agent.db: a losing daemon must not + // migrate, recover or bind the socket. Exit 0 so the spawning CLI keeps + // polling and connects to the daemon that already runs. + instanceLock = acquireInstanceLock(getPaths().daemonLock); + if (!instanceLock) { + appendDaemonLog(`pid ${process.pid}: another daemon holds ${getPaths().daemonLock}; exiting`); + process.exit(0); + } const d = new Daemon(); d.start().then( () => d.autostartWeb(), diff --git a/src/daemon/instance-lock.ts b/src/daemon/instance-lock.ts new file mode 100644 index 0000000..cbe15a5 --- /dev/null +++ b/src/daemon/instance-lock.ts @@ -0,0 +1,50 @@ +import fs from "node:fs"; +import path from "node:path"; +import { DatabaseSync } from "node:sqlite"; + +// One daemon per RUN_AGENT_DIR. Without this, a CLI whose 1s socket probe +// times out against a busy daemon spawns another one, which unlinks the +// live socket and reattaches the same sessions: N daemons then tail the +// same logs and race each other in SQLite ("database is locked"). +// +// The lock is a tiny SQLite file held in EXCLUSIVE locking mode. SQLite +// takes an fcntl lock that the kernel drops when the process dies, even on +// SIGKILL, so there is no stale pid file to judge. It is a separate file +// because an exclusive lock on run-agent.db would block read-only CLIs. +// +// Kept free of TypeScript-only syntax and relative imports: the test loads +// this file in a child process with --experimental-strip-types. + +export interface InstanceLock { + release(): void; +} + +export function acquireInstanceLock(lockPath: string): InstanceLock | null { + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + const db = new DatabaseSync(lockPath); + try { + // busy_timeout 0: a held lock must refuse now, not after a wait. + db.exec(` + PRAGMA busy_timeout = 0; + PRAGMA journal_mode = DELETE; + PRAGMA locking_mode = EXCLUSIVE; + BEGIN EXCLUSIVE; + CREATE TABLE IF NOT EXISTS owner (pid INTEGER NOT NULL, acquired_at TEXT NOT NULL); + DELETE FROM owner; + `); + db.prepare(`INSERT INTO owner (pid, acquired_at) VALUES (?, ?)`).run(process.pid, new Date().toISOString()); + // In EXCLUSIVE locking mode the lock outlives the COMMIT until close. + db.exec(`COMMIT;`); + } catch { + try { db.close(); } catch {} + return null; + } + let released = false; + return { + release(): void { + if (released) return; + released = true; + try { db.close(); } catch {} + }, + }; +} diff --git a/tests/daemon-instance-lock.test.ts b/tests/daemon-instance-lock.test.ts new file mode 100644 index 0000000..912a7f9 --- /dev/null +++ b/tests/daemon-instance-lock.test.ts @@ -0,0 +1,90 @@ +import { spawn, type ChildProcess } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { acquireInstanceLock, type InstanceLock } from "../src/daemon/instance-lock.js"; + +const lockModule = fileURLToPath(new URL("../src/daemon/instance-lock.ts", import.meta.url)); + +// Holds the lock from a separate OS process, the way a second `daemon.js` +// would, and reports once it has it. +function holdInOtherProcess(lockPath: string): Promise { + const script = ` + import { acquireInstanceLock } from ${JSON.stringify(lockModule)}; + const lock = acquireInstanceLock(${JSON.stringify(lockPath)}); + process.stdout.write(lock ? "held\\n" : "refused\\n"); + setInterval(() => {}, 1000); + `; + const child = spawn(process.execPath, ["--experimental-strip-types", "--no-warnings", "--input-type=module", "-e", script], { + stdio: ["ignore", "pipe", "inherit"], + }); + return new Promise((resolve, reject) => { + child.stdout!.once("data", (chunk: Buffer) => { + if (chunk.toString().trim() === "held") resolve(child); + else reject(new Error(`child did not get the lock: ${chunk}`)); + }); + child.once("exit", (code) => reject(new Error(`child exited early (${code})`))); + }); +} + +function exited(child: ChildProcess): Promise { + return new Promise((resolve) => { + if (child.exitCode !== null || child.signalCode !== null) resolve(); + else child.once("exit", () => resolve()); + }); +} + +describe("daemon instance lock", () => { + let tempDir: string; + let lockPath: string; + const held: InstanceLock[] = []; + const children: ChildProcess[] = []; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "run-agent-instance-lock-")); + lockPath = path.join(tempDir, "daemon.lock"); + }); + + afterEach(async () => { + for (const lock of held.splice(0)) lock.release(); + for (const child of children.splice(0)) { + child.kill("SIGKILL"); + await exited(child); + } + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it("grants the lock to the first daemon", () => { + const lock = acquireInstanceLock(lockPath); + expect(lock).not.toBeNull(); + held.push(lock!); + }); + + it("refuses a second holder in the same process", () => { + held.push(acquireInstanceLock(lockPath)!); + expect(acquireInstanceLock(lockPath)).toBeNull(); + }); + + it("refuses while another process holds it", async () => { + children.push(await holdInOtherProcess(lockPath)); + expect(acquireInstanceLock(lockPath)).toBeNull(); + }); + + it("frees the lock when the holder is SIGKILLed, with no stale file to clean", async () => { + const child = await holdInOtherProcess(lockPath); + child.kill("SIGKILL"); + await exited(child); + const lock = acquireInstanceLock(lockPath); + expect(lock).not.toBeNull(); + held.push(lock!); + }); + + it("is free again after release", () => { + acquireInstanceLock(lockPath)!.release(); + const lock = acquireInstanceLock(lockPath); + expect(lock).not.toBeNull(); + held.push(lock!); + }); +});