diff --git a/server/index.js b/server/index.js index 8022b70..25bd6ac 100644 --- a/server/index.js +++ b/server/index.js @@ -2231,6 +2231,84 @@ function runStartupMigrations(cfg) { } +// #992: after a restart, respawn agents for projects that were MID-BATCH. +// #972's shutdown kills agent PTYs cleanly on `pm2 restart` / local restart, +// but nothing respawned them — so a project driving an active batch stayed +// `missing` (no dev progress, no reviews) until an operator manually restarted +// each of head/dev/re1/re2. That bit every v2.5.x upgrade. Here we restore ONLY +// active-batch projects; idle projects still spawn agents on demand +// (terminal-connect / batch-start) exactly as before. +// +// "Active batch" comes from the SAME getOrComputeBatchProgress + +// isBatchActiveFromProgress source the /api/batch-active route and the +// auto-reseed gate use — no new batch/pulse logic. Opt out via config.json +// `restart_respawn: { enabled: false }` (defaults on). +// +// Fail-SAFE direction is the inverse of auto-reseed's fail-CLOSED gate: if we +// can't prove a batch is active (the check throws or returns null), we do +// NOTHING and never spawn, so an idle project is never disturbed. Dependencies +// are injected (getProgress / isActiveFromProgress / spawnAgentPty / log) +// exactly like autoReseedOnStartup so tests need neither gh nor a real pty. +async function respawnActiveBatchAgents(cfg, opts = {}) { + const log = opts.log || ((m) => console.log(m)); + const getProgress = opts.getProgress || routes.getOrComputeBatchProgress; + const isActiveFromProgress = opts.isActiveFromProgress || routes.isBatchActiveFromProgress; + const spawn = opts.spawnAgentPty || spawnAgentPty; + const sessions = opts.agentSessions || agentSessions; + const decisions = []; + + if (cfg && cfg.restart_respawn && cfg.restart_respawn.enabled === false) { + log("[respawn] disabled via config (restart_respawn.enabled: false)"); + return { decisions }; + } + + const projects = (cfg?.projects || []).filter((p) => p && p.id && p.working_dir); + for (const project of projects) { + let active; + try { + const progress = await getProgress(project.id); + active = isActiveFromProgress(progress); + } catch (err) { + // Fail-safe: an unknowable batch state means DON'T spawn (never disturb + // a possibly-idle project). Logged so the skip is observable. + decisions.push({ projectId: project.id, action: "skip", reason: `batch-state check threw: ${err.message}` }); + log(`[respawn] ${project.id}: skipped — batch state unknown (${err.message})`); + continue; + } + if (!active) { // false (no active batch) OR null (unknown) → leave alone + decisions.push({ projectId: project.id, action: "skip", reason: active === null ? "batch state unknown" : "no active batch" }); + continue; + } + + // Reuse the project's configured agent keys (covers legacy layouts); fall + // back to the canonical four. spawnAgentPty resolves cwd per agent and + // returns {ok:false} for an unknown one, so a stray key can't crash boot. + const agentKeys = project.agents && typeof project.agents === "object" + ? Object.keys(project.agents) + : ["head", "re1", "re2", "dev"]; + const restored = []; + for (const agentId of agentKeys) { + const key = `${project.id}/${agentId}`; + // Idempotent: never double-spawn an agent already live (e.g. one a + // terminal-connect raced in first). + const existing = sessions.get(key); + if (existing && isPtyAlive(existing.term)) continue; + try { + const r = await spawn(project.id, agentId, { suppressLifecycleMsg: true }); + if (r && r.ok) restored.push(agentId); + else log(`[respawn] ${key}: spawn failed: ${(r && r.error) || "unknown error"}`); + } catch (err) { + log(`[respawn] ${key}: spawn threw: ${err.message}`); + } + } + if (restored.length > 0) { + log(`[respawn] ${project.id}: active batch — restored agents: ${restored.join(", ")}`); + } + decisions.push({ projectId: project.id, action: "respawned", agents: restored }); + } + return { decisions }; +} + if (!process.env.QUADWORK_SKIP_LISTEN) { // #974: a second `quadwork start` (or anything already bound to PORT) makes // server.listen emit 'error'; with no handler Node throws the raw EADDRINUSE @@ -2295,6 +2373,15 @@ if (!process.env.QUADWORK_SKIP_LISTEN) { console.error(`[reseed] auto-reseed failed: ${err.message}`); } + // #992: restore agents for any project mid-batch (see fn comment). Runs + // AFTER auto-reseed (which defers active-batch projects, so their seeds are + // untouched) and must never block boot. + try { + await respawnActiveBatchAgents(startupCfg); + } catch (err) { + console.error(`[respawn] restart respawn failed: ${err.message}`); + } + if (startupCfg.butler && startupCfg.butler.enabled && startupCfg.butler.auto_start) { const result = spawnButlerPty(); if (result.ok) console.log(`[butler] auto-started (PID: ${result.pid})`); @@ -2348,3 +2435,4 @@ function shutdown() { module.exports = { shutdown, buildAgentArgs, buildAgentEnv, isPtyAlive, watchdogCheck, markSessionExited }; module.exports.agentSessions = agentSessions; // #972: test seam for shutdown() PTY cleanup +module.exports.respawnActiveBatchAgents = respawnActiveBatchAgents; // #992: startup respawn (DI'd for tests) diff --git a/server/restartRespawn.test.js b/server/restartRespawn.test.js new file mode 100644 index 0000000..41ee113 --- /dev/null +++ b/server/restartRespawn.test.js @@ -0,0 +1,161 @@ +"use strict"; + +// #992: coverage for respawnActiveBatchAgents — the startup step that restores +// agents for projects that were MID-BATCH when the server restarted, while +// leaving idle projects untouched. +// +// respawnActiveBatchAgents takes a dependency-injection seam (getProgress / +// isActiveFromProgress / spawnAgentPty / agentSessions / log — defaults wire to +// the real module fns), so we drive it with stubbed side-effect-free deps and +// the REAL isBatchActiveFromProgress (via an injected getProgress that returns +// crafted progress payloads) so the actual active-batch determination is +// exercised, not mocked. spawnAgentPty is a spy — no real pty. +// +// QUADWORK_SKIP_LISTEN + a temp HOME let us require the server module for the +// exported helper without starting the server (see watchdog.test.js). Plain +// node:assert script — auto-discovered by the #836 runner. + +const fs = require("fs"); +const os = require("os"); +const path = require("path"); + +const TMP_HOME = path.join(os.tmpdir(), `quadwork-respawn-test-${process.pid}`); +process.env.HOME = TMP_HOME; +process.env.QUADWORK_SKIP_LISTEN = "1"; +fs.mkdirSync(path.join(TMP_HOME, ".quadwork"), { recursive: true }); +fs.writeFileSync(path.join(TMP_HOME, ".quadwork", "config.json"), JSON.stringify({ projects: [] })); + +const assert = require("node:assert/strict"); +const { respawnActiveBatchAgents } = require("./index"); +const routes = require("./routes"); + +// The real active-batch predicate — crafted progress payloads flow through it. +const ACTIVE = { items: [{ id: 1 }], complete: false }; // → true +const IDLE_COMPLETE = { items: [{ id: 1 }], complete: true }; // → false +const IDLE_CLEARED = { liveActiveBatchCleared: true, items: [{ id: 1 }] }; // → false +// null progress → isBatchActiveFromProgress returns null (unknown) + +// Sanity: our payloads mean what we think under the REAL predicate. +assert.equal(routes.isBatchActiveFromProgress(ACTIVE), true); +assert.equal(routes.isBatchActiveFromProgress(IDLE_COMPLETE), false); +assert.equal(routes.isBatchActiveFromProgress(IDLE_CLEARED), false); +assert.equal(routes.isBatchActiveFromProgress(null), null); + +function spy() { + const calls = []; + const fn = async (projectId, agentId, opts) => { calls.push([projectId, agentId, opts]); return { ok: true, pid: 4242 }; }; + return { fn, calls }; +} + +(async () => { + // ── 1. Active-batch project → all 4 agents respawned, with suppressed + // lifecycle msg; decision recorded. ── + { + const { fn: spawn, calls } = spy(); + const cfg = { projects: [{ id: "act", working_dir: "/tmp/act", agents: { head: {}, re1: {}, re2: {}, dev: {} } }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => ACTIVE, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 4, "active batch → 4 agents spawned"); + assert.deepEqual(calls.map((c) => c[1]).sort(), ["dev", "head", "re1", "re2"], "spawned head/re1/re2/dev"); + assert.equal(calls[0][0], "act", "spawn called with projectId"); + assert.ok(calls.every((c) => c[2] && c[2].suppressLifecycleMsg === true), "lifecycle msg suppressed on restore"); + assert.equal(out.decisions[0].action, "respawned"); + assert.deepEqual(out.decisions[0].agents.sort(), ["dev", "head", "re1", "re2"]); + } + + // ── 2. Idle project (batch complete) → NOT spawned (unchanged behavior). ── + { + const { fn: spawn, calls } = spy(); + const cfg = { projects: [{ id: "idle", working_dir: "/tmp/idle", agents: { head: {}, re1: {}, re2: {}, dev: {} } }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => IDLE_COMPLETE, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 0, "idle (complete) project → no agents spawned"); + assert.equal(out.decisions[0].action, "skip"); + assert.equal(out.decisions[0].reason, "no active batch"); + } + + // ── 2b. Idle project (Active Batch section explicitly cleared) → NOT spawned. ── + { + const { fn: spawn, calls } = spy(); + const cfg = { projects: [{ id: "cleared", working_dir: "/tmp/cleared" }] }; + await respawnActiveBatchAgents(cfg, { + getProgress: async () => IDLE_CLEARED, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 0, "cleared active-batch → no agents spawned"); + } + + // ── 3. Unknown batch state (null progress) → fail-SAFE, do NOTHING. ── + { + const { fn: spawn, calls } = spy(); + const cfg = { projects: [{ id: "unk", working_dir: "/tmp/unk" }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => null, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 0, "null progress → never spawns (fail-safe)"); + assert.equal(out.decisions[0].reason, "batch state unknown"); + } + + // ── 4. getProgress throws → skip that project, never spawn, never crash. ── + { + const { fn: spawn, calls } = spy(); + const cfg = { projects: [{ id: "err", working_dir: "/tmp/err" }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => { throw new Error("gh down"); }, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 0, "throwing batch-state check → no spawn"); + assert.match(out.decisions[0].reason, /batch-state check threw: gh down/); + } + + // ── 5. Config opt-out (restart_respawn.enabled:false) → nothing spawned. ── + { + const { fn: spawn, calls } = spy(); + const cfg = { restart_respawn: { enabled: false }, projects: [{ id: "act", working_dir: "/tmp/act" }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => ACTIVE, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 0, "opt-out → no agents spawned even for an active batch"); + assert.equal(out.decisions.length, 0, "opt-out → no per-project decisions"); + } + + // ── 6. Idempotent: an already-live agent is NOT re-spawned; the rest are. ── + { + const { fn: spawn, calls } = spy(); + // A session whose term.pid is our own pid → isPtyAlive returns true. + const sessions = new Map([["act/head", { term: { pid: process.pid } }]]); + const cfg = { projects: [{ id: "act", working_dir: "/tmp/act", agents: { head: {}, re1: {}, re2: {}, dev: {} } }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => ACTIVE, spawnAgentPty: spawn, agentSessions: sessions, log: () => {}, + }); + assert.deepEqual(calls.map((c) => c[1]).sort(), ["dev", "re1", "re2"], "already-live head NOT re-spawned; re1/re2/dev are"); + assert.deepEqual(out.decisions[0].agents.sort(), ["dev", "re1", "re2"]); + } + + // ── 7. A failing spawn is logged, doesn't crash, and isn't counted restored. ── + { + const calls = []; + const spawn = async (pid, aid) => { calls.push(aid); return aid === "re1" ? { ok: false, error: "boom" } : { ok: true }; }; + const logs = []; + const cfg = { projects: [{ id: "act", working_dir: "/tmp/act", agents: { head: {}, re1: {}, re2: {}, dev: {} } }] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => ACTIVE, spawnAgentPty: spawn, agentSessions: new Map(), log: (m) => logs.push(m), + }); + assert.ok(!out.decisions[0].agents.includes("re1"), "failed spawn not counted as restored"); + assert.ok(logs.some((m) => /re1: spawn failed: boom/.test(m)), "failed spawn is logged"); + } + + // ── 8. Projects without working_dir / id are ignored (no throw). ── + { + const { fn: spawn, calls } = spy(); + const cfg = { projects: [{ id: "nowd" }, { working_dir: "/tmp/x" }, null] }; + const out = await respawnActiveBatchAgents(cfg, { + getProgress: async () => ACTIVE, spawnAgentPty: spawn, agentSessions: new Map(), log: () => {}, + }); + assert.equal(calls.length, 0, "projects missing id/working_dir are skipped"); + assert.equal(out.decisions.length, 0); + } + + console.log("restartRespawn.test.js: all assertions passed (8 cases)"); + process.exit(0); +})().catch((err) => { console.error(err); process.exit(1); });