[#992] Auto-respawn agents on server restart when a batch is active

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)

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); });