From e42bf1e37ce925b34977ef295d34d43f820846d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 02:59:01 +0000 Subject: [PATCH] fix(daemon): Allow a single daemon per run-agent dir A CLI whose 1s socket probe timed out against a busy daemon spawned another one, which unlinked the live socket and reattached the same sessions. Stray daemons kept writing to SQLite, so the event loop hit SQLITE_BUSY_SNAPSHOT and marked live sessions failed with "database is locked". One machine had 10 daemons running. The daemon entry now takes an exclusive SQLite lock on daemon.lock before opening the database. The kernel drops it on any exit, SIGKILL included, so there is no stale lock to clean. A losing daemon logs and exits 0, and the spawning CLI keeps polling the running one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R6o1wJTTJWcHm9mZMGvDTn --- .../features/daemon-single-instance/spec.md | 42 +++++++++ src/config/paths.ts | 1 + src/daemon/daemon.ts | 13 +++ src/daemon/instance-lock.ts | 50 +++++++++++ tests/daemon-instance-lock.test.ts | 90 +++++++++++++++++++ 5 files changed, 196 insertions(+) create mode 100644 .specs/features/daemon-single-instance/spec.md create mode 100644 src/daemon/instance-lock.ts create mode 100644 tests/daemon-instance-lock.test.ts 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!); + }); +});