convoy up/reconcile: recover missing/unhealthy dings + fix the respawn primitive (manifest-replay, not pty restart) (#82) (#94)

Two linked recovery gaps, both surfaced by tonight's mass-ding-kill (14 dings
died; agents needed a full restart):

1) host.respawn used 'pty restart -y', which is unusable for a headless
   supervisor (VERIFIED empirically in an isolated PTY_ROOT):
   - pty's stateful-agent guard makes 'pty restart' exit 1 on a role=agent
     session unless --force, so a dead permanent AGENT was never respawned --
     reconcile only bumped results.failed. This is the root of issue #82.
   - otherwise 'pty restart' tries to ATTACH after the respawn, which hangs a
     non-TTY host.
   Replace it with manifest-replay via spawnDaemon: read the session's pty.toml
   def and re-spawn the verbatim command with the durable ST_ROOT/PTY_ROOT env
   -- the primitive convoy reload/spawnFromPtyFile already use and that
   convoy-rust independently adopted (pty rm + pty up).

2) reconcile was session-centric, so a ding whose process was killed AND whose
   record was GC'd is absent from the session list (nothing to respawn), and a
   killed-but-registered ding lost its strategy=permanent tag (pty kill strips
   it) so the permanent-respawn branch skipped it -- either way a LIVE agent was
   left with a DEAD ding until a full restart. Add an agent-centric DING-HEALTH
   pass (convoy up AND up --once): for each live agent whose manifest declares a
   ding, ensure the ding process is alive; replay it from the manifest if
   missing or dead. Health today = process-alive; a richer ding health signal
   (the Rust ding) plugs in at the same check.

New host primitives: spawnManifestSession / readManifestDef / freeSession,
shared by spawnFromPtyFile + respawn + the ding-health pass. New pure
reconcile.dingHealthPlan (unit-tested, 10 cases). Live end-to-end proof against
the real pty daemon: dead ding, GC'd/missing ding, and role=agent harness
respawn all recover. Full suite 256/256 green.

Author: myobie

src/host.ts

@@ -6,11 +6,13 @@
 import { basename, dirname, join } from "node:path";
 import { randomBytes } from "node:crypto";
 import {
+  cleanupAll,
   isGone,
   listSessions,
   readPtyFile,
   spawnDaemon,
   updateTags,
+  type PtySessionDef,
   type SessionInfo,
 } from "@compoundingtech/pty/client";
 import { commandFingerprint, parseStrategyTags, type StrategyTags } from "./flapping-cap.ts";
@@ -31,25 +33,55 @@ function cleanEnv(overlay: NodeJS.ProcessEnv): Record<string, string> {
 export async function spawnFromPtyFile(dir: string, root: string | null): Promise<{ spawned: string[]; failed: string[] }> {
   if (root) process.env["PTY_ROOT"] = `${root}/pty`;
   // The manifest lives in the workspace's .convoy/ overlay; the SESSIONS still run in the workspace
-  // (cwd: dir below), which decouples the manifest location from the working dir.
+  // (cwd: dir, via spawnManifestSession), which decouples the manifest location from the working dir.
   const file = readPtyFile(join(dir, CONVOY_DIR));
-  const tomlPath = join(dir, CONVOY_DIR, "pty.toml");
   const spawned: string[] = [];
   const failed: string[] = [];
   for (const def of file.sessions) {
-    const name = def.id ?? `${def.shortName}-${randomBytes(3).toString("hex")}`;
-    const tags: Record<string, string> = { ...(def.tags ?? {}), ptyfile: tomlPath, "ptyfile.session": def.shortName };
-    const env = cleanEnv({ ...process.env, ...(def.env ?? {}), ...(root ? { ST_ROOT: stRootOf(root), PTY_ROOT: `${root}/pty` } : {}) });
     try {
-      await spawnDaemon({ name, command: "sh", args: ["-c", def.command], displayCommand: def.command, cwd: dir, displayName: def.displayName, tags, env });
-      spawned.push(name);
+      spawned.push(await spawnManifestSession(dir, def, root));
     } catch {
       failed.push(def.shortName);
     }
   }
   return { spawned, failed };
 }
 
+/** Spawn ONE session def from a pty.toml manifest via `spawnDaemon` — the port's launch-absorb, per session.
+ *  Shared by `spawnFromPtyFile` (bring up the whole manifest) and the reconcile respawn / ding-health
+ *  recovery (replay a single session). Bakes the durable ST_ROOT/PTY_ROOT so a replay never loses the network
+ *  pin; the session's own env (incl. ST_AGENT) rides in `def.env`. `workspace` is the session cwd (the manifest
+ *  lives in `<workspace>/.convoy/`). Returns the spawned session name. Does NOT free a stale record — a REPLAY
+ *  caller `freeSession()`s the id first (see `respawn` / the ding-health pass); a fresh bring-up has no record. */
+export async function spawnManifestSession(workspace: string, def: PtySessionDef, root: string | null): Promise<string> {
+  const tomlPath = join(workspace, CONVOY_DIR, "pty.toml");
+  const name = def.id ?? `${def.shortName}-${randomBytes(3).toString("hex")}`;
+  const tags: Record<string, string> = { ...(def.tags ?? {}), ptyfile: tomlPath, "ptyfile.session": def.shortName };
+  const env = cleanEnv({ ...process.env, ...(def.env ?? {}), ...(root ? { ST_ROOT: stRootOf(root), PTY_ROOT: `${root}/pty` } : {}) });
+  await spawnDaemon({ name, command: "sh", args: ["-c", def.command], displayCommand: def.command, cwd: workspace, displayName: def.displayName, tags, env });
+  return name;
+}
+
+/** Read a single session def (by its toml key / `shortName`) from the pty.toml at `ptyfile` — the
+ *  `<workspace>/.convoy/pty.toml` path stored in a session's `ptyfile` tag. The manifest is the source of
+ *  truth for the verbatim command + durable env, so a respawn REPLAYS it rather than reconstructing from
+ *  live pty metadata (which dropped the `sh -c "exec claude …"` wrapper and came back a bare shell — the
+ *  capstone LOOP-CLOSED regression). Returns null if the file/def is unreadable or absent. */
+export function readManifestDef(ptyfile: string, shortName: string): PtySessionDef | null {
+  try {
+    const file = readPtyFile(dirname(ptyfile)); // ptyfile is <ws>/.convoy/pty.toml; readPtyFile takes the dir
+    return file.sessions.find((d) => d.shortName === shortName) ?? null;
+  } catch {
+    return null;
+  }
+}
+
+/** Free a session's on-disk record + socket (pty's `cleanupAll`) so its stable id can be re-spawned cleanly.
+ *  Called before a manifest REPLAY drops the stale (gone) record. Safe on a non-existent name (no-op). */
+export function freeSession(name: string): void {
+  cleanupAll(name);
+}
+
 /** One session as convoy's host sees it, projected from pty's typed `SessionInfo` + `SessionMetadata`. */
 export interface SupervisedSession {
   name: string; // pty id (stable across respawn; survives kill)
@@ -194,13 +226,28 @@ export class PtyHost {
     updateTags(name, {}, [key]);
   }
 
-  /** Respawn a gone session IN PLACE via `pty restart -y <name>` — SIGTERM + respawn using the STORED
-   *  metadata.command, which PRESERVES the agent's real command verbatim (pty-claude's guidance).
-   *  Reconstructing it via `spawnDaemon(command, args)` loses it — an agent's `sh -c "… exec claude …"`
-   *  came back a bare shell, failing the capstone's LOOP-CLOSED gate. PTY_ROOT is pinned in the process
-   *  env by the constructor, so the CLI targets the right registry. */
+  /** Respawn a gone session by REPLAYING its pty.toml manifest via `spawnDaemon` — NOT `pty restart`.
+   *  `pty restart` is unusable for a headless supervisor (VERIFIED): pty's stateful-agent guard makes it
+   *  `exit 1` on a `role=agent` session unless `--force` (so a dead permanent AGENT was never respawned —
+   *  reconcile only bumped `failed`; the root of issue #82), and it otherwise tries to ATTACH after the
+   *  respawn, which HANGS a non-TTY host. Replaying the manifest re-runs the verbatim stored command with
+   *  the durable ST_ROOT/PTY_ROOT env — the primitive `convoy reload`/`spawnFromPtyFile` already use and
+   *  that convoy-rust independently adopted (pty rm + pty up). `freeSession` drops the stale (gone) record so
+   *  the stable id re-spawns cleanly. Returns false if the manifest/def can't be resolved or the spawn throws
+   *  (honest failure the caller surfaces — better than the old silent `pty restart` no-op). */
   async respawn(s: SupervisedSession): Promise<boolean> {
-    return (await run("pty", ["restart", "-y", s.name])).ok;
+    const ptyfile = s.tags["ptyfile"];
+    const sessionKey = s.tags["ptyfile.session"];
+    if (!ptyfile || !sessionKey) return false; // not a convoy manifest session — nothing to replay
+    const def = readManifestDef(ptyfile, sessionKey);
+    if (!def) return false;
+    try {
+      freeSession(s.name); // free the stale record + socket before re-spawning the same stable id
+      await spawnManifestSession(workspaceOfPtyfile(ptyfile), def, this.root);
+      return true;
+    } catch {
+      return false;
+    }
   }
 
   /** Stop a session (teardown). Residual `pty kill` shell — the client doesn't export a daemon-kill;

src/reconcile.test.ts

@@ -1,7 +1,8 @@
 import { describe, it, expect } from "vitest";
-import { agentBusId, reconcilePlan, type CatalogEntry } from "./reconcile.ts";
+import { agentBusId, dingHealthPlan, reconcilePlan, type CatalogEntry } from "./reconcile.ts";
 import type { SupervisedSession } from "./host.ts";
 import type { AgentFile } from "./agent-file.ts";
+import type { PtySessionDef } from "@compoundingtech/pty/client";
 
 // A session carrying its bus id in a plain "busId" tag; the injected resolver reads it (the real one reads
 // ST_AGENT out of the pty.toml). pid/status drive the gone / adopt-alive logic.
@@ -85,3 +86,82 @@ describe("reconcilePlan — desired (catalog) vs actual (sessions), host-filtere
     expect(plan.otherHost).toEqual([theirs]);
   });
 });
+
+describe("dingHealthPlan — agent-centric ding recovery (reconcile-recreates-missing/unhealthy-ding, #82)", () => {
+  const PF = "/w/.convoy/pty.toml";
+  const dingDef: PtySessionDef = { shortName: "ding", id: "silber.wk.ding", displayName: "wk-ding", command: "st ding silber.wk --identity silber.wk-claude --root /net" };
+  const declaresDing = (): PtySessionDef | null => dingDef;
+  const declaresNoDing = (): PtySessionDef | null => null;
+  const alive = (...pids: number[]) => (pid: number | null): boolean => pid !== null && pids.includes(pid);
+
+  // A harness or ding session keyed by its role + ptyfile tags. pid + status drive liveness.
+  function ss(role: string, o: { name?: string; ptyfile?: string; session?: string; pid?: number | null; status?: string } = {}): SupervisedSession {
+    const tags: Record<string, string> = { role };
+    if (o.ptyfile) tags["ptyfile"] = o.ptyfile;
+    if (o.session) tags["ptyfile.session"] = o.session;
+    return { name: o.name ?? role, cwd: null, command: "", args: [], status: (o.status ?? "running") as never, exitedAt: null, exitCode: null, pid: o.pid ?? 100, tags };
+  }
+  const harness = (o: Partial<Parameters<typeof ss>[1]> = {}): SupervisedSession => ss("agent", { name: "silber.wk", ptyfile: PF, session: "claude", pid: 100, ...o });
+  const ding = (o: Partial<Parameters<typeof ss>[1]> = {}): SupervisedSession => ss("ding", { name: "silber.wk.ding", ptyfile: PF, session: "ding", pid: 200, ...o });
+
+  it("HEALTHY: harness alive + ding session alive → no action", () => {
+    const plan = dingHealthPlan([harness(), ding()], declaresDing, alive(100, 200));
+    expect(plan).toEqual([]);
+  });
+
+  it("MISSING: harness alive, NO ding session at all (killed + GC'd) → heal, staleDing=null", () => {
+    const plan = dingHealthPlan([harness()], declaresDing, alive(100));
+    expect(plan).toHaveLength(1);
+    expect(plan[0]!.harness.name).toBe("silber.wk");
+    expect(plan[0]!.staleDing).toBeNull();
+    expect(plan[0]!.dingDef).toBe(dingDef);
+  });
+
+  it("DEAD: harness alive, ding session present but process dead → heal, staleDing=the dead ding", () => {
+    const dead = ding({ pid: 999 });
+    const plan = dingHealthPlan([harness(), dead], declaresDing, alive(100)); // 999 not alive
+    expect(plan).toHaveLength(1);
+    expect(plan[0]!.staleDing).toBe(dead);
+  });
+
+  it("gone-but-pid-ALIVE ding (transient CPU-spike report) → NOT healed (never double-spawn a live process)", () => {
+    const transient = ding({ pid: 200, status: "vanished" });
+    const plan = dingHealthPlan([harness(), transient], declaresDing, alive(100, 200));
+    expect(plan).toEqual([]);
+  });
+
+  it("NO DING DECLARED: agent's manifest has no ding (e.g. claude on MCP transport) → skip", () => {
+    const plan = dingHealthPlan([harness()], declaresNoDing, alive(100));
+    expect(plan).toEqual([]);
+  });
+
+  it("DEAD HARNESS: harness gone + pid dead → skipped (the respawn/launch paths own it, not this pass)", () => {
+    const plan = dingHealthPlan([harness({ status: "exited", pid: 999 })], declaresDing, alive()); // nothing alive
+    expect(plan).toEqual([]);
+  });
+
+  it("gone-but-ALIVE harness (adopt-alive) with a missing ding → still healed", () => {
+    const plan = dingHealthPlan([harness({ status: "vanished", pid: 100 })], declaresDing, alive(100));
+    expect(plan).toHaveLength(1);
+  });
+
+  it("non-agent session (role != agent) is ignored even if it looks ding-less", () => {
+    const web = ss("web", { name: "svc", ptyfile: PF, session: "svc", pid: 100 });
+    const plan = dingHealthPlan([web], declaresDing, alive(100));
+    expect(plan).toEqual([]);
+  });
+
+  it("harness without a ptyfile tag → skipped (nothing to replay from)", () => {
+    const noPf = ss("agent", { name: "silber.wk", session: "claude", pid: 100 }); // no ptyfile
+    const plan = dingHealthPlan([noPf], declaresDing, alive(100));
+    expect(plan).toEqual([]);
+  });
+
+  it("MULTI: agent A healthy, agent B missing its ding → only B is healed (per-agent, by ptyfile)", () => {
+    const aH = ss("agent", { name: "a", ptyfile: "/a/.convoy/pty.toml", session: "claude", pid: 1 });
+    const aD = ss("ding", { name: "a.ding", ptyfile: "/a/.convoy/pty.toml", session: "ding", pid: 2 });
+    const bH = ss("agent", { name: "b", ptyfile: "/b/.convoy/pty.toml", session: "claude", pid: 3 }); // ding missing
+    const plan = dingHealthPlan([aH, aD, bH], declaresDing, alive(1, 2, 3));
+    expect(plan.map((x) => x.harness.name)).toEqual(["b"]);
+  });
+});

src/reconcile.ts

@@ -9,6 +9,7 @@
 // src/fabric-sync.ts), and B's reconcile sees host==B + launches it. No RPC — the synced folder IS the
 // scheduler. Pure (no side effects) so it's unit-testable; up executes the plan.
 
+import type { PtySessionDef } from "@compoundingtech/pty/client";
 import type { AgentFile } from "./agent-file.ts";
 import { gone, processAlive, type SupervisedSession } from "./host.ts";
 
@@ -72,3 +73,47 @@ export function reconcilePlan(
   }
   return plan;
 }
+
+/** One ding-health repair: a LIVE harness whose ding sidecar is missing or dead, the manifest def to replay it
+ *  from, and the stale ding session (if any) to free first. */
+export interface DingHealAction {
+  harness: SupervisedSession;
+  dingDef: PtySessionDef;
+  staleDing: SupervisedSession | null;
+}
+
+/** Which LIVE agents have a missing/unhealthy ding sidecar — AGENT-centric, unlike the SESSION-centric respawn
+ *  loop. For each harness (role=agent) that is alive and whose manifest DECLARES a ding, the ding is healthy iff
+ *  a ding session exists for the same pty.toml AND its process is alive; otherwise it needs a manifest replay.
+ *  `dingDefOf` reads the manifest's ding def for a ptyfile (injected → keeps this pure/testable); null means the
+ *  agent declares no ding, so skip it. `isAlive` defaults to the real pid probe. Pure.
+ *
+ *  This is the reconcile-recreates-missing/unhealthy-ding fix (issue #82's sibling). Two ways the session loop
+ *  misses a dead ding, both leaving a LIVE agent ding-less until a full restart: (1) a ding whose process was
+ *  killed AND whose record was GC'd is absent from the session list entirely → nothing to respawn; (2) a
+ *  killed-but-registered ding lost its `strategy=permanent` tag (`pty kill` strips it) → the permanent-respawn
+ *  branch skips it. Anchoring on the LIVE harness + the declared manifest catches both. */
+export function dingHealthPlan(
+  sessions: SupervisedSession[],
+  dingDefOf: (ptyfile: string) => PtySessionDef | null,
+  isAlive: (pid: number | null) => boolean = processAlive,
+): DingHealAction[] {
+  const dingBy = new Map<string, SupervisedSession>(); // ptyfile → its ding session
+  for (const s of sessions) {
+    const pf = s.tags["ptyfile"];
+    if (pf && s.tags["ptyfile.session"] === "ding") dingBy.set(pf, s);
+  }
+  const out: DingHealAction[] = [];
+  for (const s of sessions) {
+    if (s.tags["role"] !== "agent") continue; // harness sessions only (the ding sidecar is role=ding)
+    if (gone(s) && !isAlive(s.pid)) continue; // a DEAD harness is the respawn/launch paths' job, not this one
+    const ptyfile = s.tags["ptyfile"];
+    if (!ptyfile) continue;
+    const dingDef = dingDefOf(ptyfile);
+    if (!dingDef) continue; // agent declares no ding (e.g. a claude agent on the MCP transport)
+    const ding = dingBy.get(ptyfile) ?? null;
+    if (ding && isAlive(ding.pid)) continue; // ding healthy → nothing to do
+    out.push({ harness: s, dingDef, staleDing: ding });
+  }
+  return out;
+}