From 9a3c3ee9638977142928ded7ee81fec97762fa12 Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 21 Jul 2026 14:46:24 +0200 Subject: [PATCH] fix(up): a worker crash dings its SUPERVISOR again, not just the CoS (crash-ding regression) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a worker crash, `convoy up`'s crash-ding is meant to page BOTH the CoS (the always-on backstop, via convoy.tier=cos) AND the worker's actual supervisor (via its convoy.spawner tag) — the two-recipient design from #48. The declarative arc broke the supervisor half. `agentFileToSpec` DROPPED the agent file's `supervisor` field, so it never reached the launch path; and `writeAgentFiles` stamped convoy.spawner from the LAUNCHING process's ST_AGENT — which, in the declarative flow, is the `convoy up` HOST, not the parent. So a declared worker's convoy.spawner became the host (often == the CoS, then deduped), and its real supervisor was never dinged. Fix: carry `supervisor` on AgentSpec, map af.supervisor → spec.supervisor in agentFileToSpec, and stamp convoy.spawner = spec.supervisor ?? ST_AGENT in writeAgentFiles. The declared supervisor is now authoritative; ST_AGENT stays the fallback for the imperative `convoy run` path (no declared supervisor). Surfaced by the evals crash-ding cell (cd-cos got the notice; cd-sup's inbox stayed empty). +2 regression tests: agentFileToSpec carries supervisor, and writeAgentFiles prefers the declared supervisor over the host's ST_AGENT. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MCzqQKSpPiNX2ketyubByS --- src/agent-file.test.ts | 8 ++++++++ src/agent-file.ts | 4 ++++ src/agent-spec.test.ts | 1 + src/agent-spec.ts | 6 ++++++ src/commands.ts | 2 ++ src/harness.test.ts | 1 + src/launch.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ src/launch.ts | 10 +++++++--- 8 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/agent-file.test.ts b/src/agent-file.test.ts index 4b6ee82..d831f2b 100644 --- a/src/agent-file.test.ts +++ b/src/agent-file.test.ts @@ -113,6 +113,14 @@ describe("agentFileToSpec — compile intent → AgentSpec", () => { expect(agentFileToSpec({ ...base, model: "claude-fable-5" }, { networkRoot: "/net" }).model).toBe("claude-fable-5"); expect(agentFileToSpec(base, { networkRoot: "/net" }).model).toBe(null); }); + + it("threads supervisor → spec.supervisor so a crash-ding reaches the parent (dropping it was the regression)", () => { + // The declarative flow (convoy up launches from the catalog) must carry the declared supervisor through + // to the spec — it becomes the session's convoy.spawner tag, which crashDingTargets pages on a crash. + expect(agentFileToSpec({ ...base, supervisor: "silber.cd-sup" }, { networkRoot: "/net" }).supervisor).toBe("silber.cd-sup"); + // no supervisor declared → null (launch then falls back to the launching ST_AGENT). + expect(agentFileToSpec(base, { networkRoot: "/net" }).supervisor).toBeNull(); + }); }); describe("agentFileToToml — serialize (what convoy add authors)", () => { diff --git a/src/agent-file.ts b/src/agent-file.ts index 1aece96..e0fff8f 100644 --- a/src/agent-file.ts +++ b/src/agent-file.ts @@ -312,6 +312,10 @@ export function agentFileToSpec(af: AgentFile, opts: { networkRoot: string | nul harness: af.harness ?? "claude", role: af.role, identity: af.identity, + // Carry the declared supervisor through to the spec so the launch path can stamp it as `convoy.spawner` + // (a worker crash pages its actual supervisor, not just the CoS). Dropping it here was the crash-ding + // regression: the declarative launch fell back to the HOST's ST_AGENT instead of the declared parent. + supervisor: af.supervisor ?? null, transport: af.transport ?? "ding", networkRoot: opts.networkRoot, personaOverride: af.persona ?? null, diff --git a/src/agent-spec.test.ts b/src/agent-spec.test.ts index b3ae0cf..6d2dd00 100644 --- a/src/agent-spec.test.ts +++ b/src/agent-spec.test.ts @@ -23,6 +23,7 @@ function spec(over: Partial = {}): AgentSpec { harness: "claude", role: "worker", identity: "wk1", + supervisor: null, transport: "ding", networkRoot: null, personaOverride: null, diff --git a/src/agent-spec.ts b/src/agent-spec.ts index 6465ac1..257a8e4 100644 --- a/src/agent-spec.ts +++ b/src/agent-spec.ts @@ -20,6 +20,12 @@ export interface AgentSpec { harness: Harness; role: Role; identity: string; + /** The bus id of this agent's SUPERVISOR (whoever it reports to / escalates a crash to). Comes from the + * agent file's `supervisor` in the declarative flow, or falls back to the launching ST_AGENT in the + * imperative (`convoy run`) flow. Stamped as the `convoy.spawner` session tag so `convoy up`'s crash-ding + * reaches the actual parent (crashDingTargets), not just the CoS backstop. null = no declared supervisor + * (a crash then dings the CoS only). */ + supervisor: string | null; transport: Transport; networkRoot: string | null; personaOverride: string | null; diff --git a/src/commands.ts b/src/commands.ts index f70f2e8..c37998d 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -893,6 +893,7 @@ export async function cmdCos(args: string[]): Promise { harness: "claude", role: "chief-of-staff", identity, + supervisor: null, // the CoS is the root of the supervision tree — it reports to no one transport, networkRoot: resolveNetworkRoot(optValue(args, "--network")), personaOverride: optValue(args, "--persona"), @@ -989,6 +990,7 @@ export async function cmdRun(args: string[]): Promise { harness: harnessRaw, role, identity, + supervisor: null, // ad-hoc: no declared supervisor — writeAgentFiles falls back to the runner's ST_AGENT transport, networkRoot: network, // An ad-hoc session declares no extra environment — it is the runnable core, so it takes the network diff --git a/src/harness.test.ts b/src/harness.test.ts index a9b4f02..1117a90 100644 --- a/src/harness.test.ts +++ b/src/harness.test.ts @@ -24,6 +24,7 @@ function spec(over: Partial = {}): AgentSpec { harness: "claude", role: "worker", identity: "a", + supervisor: null, transport: "ding", networkRoot: null, personaOverride: null, diff --git a/src/launch.test.ts b/src/launch.test.ts index ca1a1b0..571d16e 100644 --- a/src/launch.test.ts +++ b/src/launch.test.ts @@ -82,6 +82,7 @@ describe("writePtyToml (pinned hostname-prefixed ids, cold start)", () => { harness: "claude", role: "worker", identity: "convoy-claude", + supervisor: null, transport: "ding", networkRoot: null, personaOverride: null, @@ -387,6 +388,7 @@ describe("writeContextFiles — clean-worktree wiring (convoy must not dirty a r harness: "claude", role: "worker", identity: "wk-1", + supervisor: null, transport: "ding", networkRoot: null, personaOverride: personaPath, @@ -504,6 +506,7 @@ describe("convoy add clean-worktree — pty.toml + settings + context, EVERY aut harness: "claude", role: "worker", identity: "wk-1", + supervisor: null, transport: "ding", networkRoot, personaOverride: personaPath, @@ -578,4 +581,39 @@ describe("convoy add clean-worktree — pty.toml + settings + context, EVERY aut for (const d of [repo, stub, personaDir, netRoot]) rmSync(d, { recursive: true, force: true }); } }); + + it("stamps convoy.spawner from the DECLARED supervisor, over the launching ST_AGENT (the crash-ding fix)", () => { + const savedSt = process.env["SMALLTALK_DIR"]; + const savedAgent = process.env["ST_AGENT"]; + const repo = mkdtempSync(join(tmpdir(), "convoy-spawner-")); + const stub = mkdtempSync(join(tmpdir(), "convoy-st-stub2-")); + const personaDir = mkdtempSync(join(tmpdir(), "convoy-persona2-")); + const netRoot = mkdtempSync(join(tmpdir(), "convoy-net2-")); + try { + mkdirSync(join(stub, "examples", "claude-code", "hooks"), { recursive: true }); + writeFileSync(join(stub, "examples", "claude-code", "hooks", "session-start.sh"), "#!/bin/sh\n"); + process.env["SMALLTALK_DIR"] = stub; + const persona = join(personaDir, "worker.md"); + writeFileSync(persona, "# worker persona\n"); + mkdirSync(join(repo, ".git", "info"), { recursive: true }); // pose as a git repo (no git binary needed) + + // DECLARATIVE flow: `convoy up` (the host) does the launch, so ST_AGENT is the HOST — the declared + // supervisor must WIN, else a worker's crash never pages its actual parent (the regression). + process.env["ST_AGENT"] = "hetz.convoy-up-host"; + writeAgentFiles(repo, { ...spec(repo, persona, netRoot), supervisor: "silber.cd-sup" }); + const toml = readFileSync(join(repo, ".convoy", "pty.toml"), "utf8"); + expect(toml).toContain('"convoy.spawner" = "silber.cd-sup"'); // the declared parent, NOT the host + expect(toml).not.toContain("convoy-up-host"); + + // IMPERATIVE `convoy run` (no declared supervisor) → falls back to the launching ST_AGENT (the runner). + writeAgentFiles(repo, { ...spec(repo, persona, netRoot), supervisor: null }); + expect(readFileSync(join(repo, ".convoy", "pty.toml"), "utf8")).toContain('"convoy.spawner" = "hetz.convoy-up-host"'); + } finally { + if (savedSt === undefined) delete process.env["SMALLTALK_DIR"]; + else process.env["SMALLTALK_DIR"] = savedSt; + if (savedAgent === undefined) delete process.env["ST_AGENT"]; + else process.env["ST_AGENT"] = savedAgent; + for (const d of [repo, stub, personaDir, netRoot]) rmSync(d, { recursive: true, force: true }); + } + }); }); diff --git a/src/launch.ts b/src/launch.ts index 5c5eedf..08d0234 100644 --- a/src/launch.ts +++ b/src/launch.ts @@ -385,9 +385,13 @@ export function writeContextFiles(dir: string, spec: AgentSpec): void { export function writeAgentFiles(dir: string, spec: AgentSpec): void { writeContextFiles(dir, spec); writeHooks(dir); - // The spawner = whoever ran `convoy add` (their bus id, from ST_AGENT) — stamped so a crash-ding reaches - // this agent's actual supervisor, not the whole permanent crew. Null when a human spawns it (→ cos-only ding). - writePtyToml(dir, spec, { spawner: process.env["ST_AGENT"] ?? null }); + // The spawner = the agent's DECLARED supervisor (from its agent file), else whoever launched it (their bus + // id, from ST_AGENT) — stamped as `convoy.spawner` so a crash-ding reaches this agent's actual supervisor, + // not the whole permanent crew. The declared supervisor is authoritative: in the DECLARATIVE flow `convoy + // up` (the host) does the launch, so ST_AGENT is the HOST, not the parent — falling back to it there is + // exactly what dropped the supervisor-ding (a worker crash then only reached the CoS backstop). ST_AGENT + // stays the fallback for the imperative `convoy run` path (no declared supervisor). Null → cos-only ding. + writePtyToml(dir, spec, { spawner: spec.supervisor ?? process.env["ST_AGENT"] ?? null }); } /**