Skip to content
This repository was archived by the owner on Jul 24, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/flapping-cap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { describe, it, expect } from "vitest";
import {
classify,
classifyFailedAttempt,
clearParkForFreshSupervisor,
commandFingerprint,
FLAPPING_STATUS,
effectiveLimit,
Expand Down Expand Up @@ -197,3 +198,42 @@ describe("classifyFailedAttempt — a recovery attempt that never produced a lea
if (parked.kind === "flap") expect(writtenTags(parked.tags)[TAG.status]).toBe(FLAPPING_STATUS);
});
});

describe("clearParkForFreshSupervisor — a fresh foreground `convoy up` restores the FULL fleet (parking-recovery)", () => {
it("ACCEPTANCE: a PARKED member is un-parked — status cleared AND the counter zeroed (relaunchable again)", () => {
// The reproduced bug: an outage drives the cap to its limit → the agent parks → and a fresh supervisor,
// reading the persisted `status=flapping`, would `skip` it forever. A deliberate bring-up must not inherit
// that. Both fields reset: clearing status alone is not enough — a counter still at the cap re-parks on
// the very next fast fail.
const cleared = clearParkForFreshSupervisor(tags({ status: FLAPPING_STATUS, consecutiveFastFails: LIMIT }));
expect(cleared).not.toBeNull();
expect(cleared?.status).toBeNull();
expect(cleared?.consecutiveFastFails).toBe(0);
});

it("resets a NON-parked member with prior fails too — 'regardless of prior fail count' (Nathan mandate)", () => {
const cleared = clearParkForFreshSupervisor(tags({ status: null, consecutiveFastFails: LIMIT - 1 }));
expect(cleared?.consecutiveFastFails).toBe(0);
expect(cleared?.status).toBeNull();
});

it("is a NO-OP for a clean member (no park, counter 0) — the caller writes no tag needlessly", () => {
expect(clearParkForFreshSupervisor(tags({ status: null, consecutiveFastFails: 0 }))).toBeNull();
});

it("preserves the rest of the strategy state — only status + counter are touched", () => {
const before = tags({ status: FLAPPING_STATUS, consecutiveFastFails: LIMIT, commandHash: HASH_A, lastRespawnAt: at(500), fastFailLimitOverride: 5, fastFailWindowOverride: 120 });
const cleared = clearParkForFreshSupervisor(before);
expect(cleared?.commandHash).toBe(HASH_A);
expect(cleared?.lastRespawnAt).toEqual(at(500));
expect(cleared?.fastFailLimitOverride).toBe(5);
expect(cleared?.fastFailWindowOverride).toBe(120);
});

it("the written tags drop the park status and carry a zeroed counter (what up() persists to disk)", () => {
const cleared = clearParkForFreshSupervisor(tags({ status: FLAPPING_STATUS, consecutiveFastFails: LIMIT }));
const written = writtenTags(cleared!);
expect(written[TAG.status]).toBeUndefined(); // no park written — up() also REMOVES the on-disk status tag
expect(written[TAG.consecutive]).toBe("0");
});
});
23 changes: 23 additions & 0 deletions src/flapping-cap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,29 @@ export function classifyFailedAttempt(input: {
return { kind: "respawn", tags: { ...tags, lastRespawnAt: now, consecutiveFastFails: nextCounter, commandHash: currentHash, status: null } };
}

/** A fresh FOREGROUND supervisor gives every member a clean cap budget (convoy parking-recovery, 2026-07-22).
*
* The bug this fixes: `strategy.status=flapping` and the fast-fail counter PERSIST to the session's tags
* (the on-disk supervision contract), so they outlive the supervisor that wrote them. A mass outage drives
* the cap to its limit → the agents park → and then a FRESH `convoy up`, reading those stale tags, hits the
* `isFlapping(...) → skip` gate in `classify` and NEVER relaunches them. The reconstructed incident: a
* bring-up after an outage brought back only some of the fleet; the rest stayed parked from a prior
* supervisor's give-up and had to be hand-launched.
*
* A foreground `convoy up` is a DELIBERATE bring-up — the operator gesture that says "restore the fleet" —
* so it must not inherit a prior supervisor's verdict. This clears the park (status) AND zeroes the counter,
* regardless of prior fail count, giving each member a fresh budget; the cap still re-accrues tick-to-tick
* WITHIN this supervisor's watch (the real crash-loop protection). Returns the reset tags, or null when
* nothing needs clearing (not parked, counter already 0) so the caller writes no tag needlessly.
*
* The `--once` shepherd cron does NOT call this: it runs every few minutes, so un-parking there would
* relaunch a genuinely broken agent on every tick — parking MUST stay durable across `--once`. This
* reset is scoped to the rare, intentional foreground bring-up. Pure → unit-testable. */
export function clearParkForFreshSupervisor(tags: StrategyTags): StrategyTags | null {
if (tags.status !== FLAPPING_STATUS && tags.consecutiveFastFails === 0) return null;
return { ...tags, status: null, consecutiveFastFails: 0 };
}

/** Classify one permanent-and-gone session (spec §5.3). Pure: same inputs → same decision. */
export function classify(input: {
session: string;
Expand Down
186 changes: 186 additions & 0 deletions src/up-parking-recovery.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
// PARKING-RECOVERY (Nathan mandate, convoy incident 2026-07-22) — a supervisor bring-up after a mass
// outage MUST restore the FULL fleet. The bug: `strategy.status=flapping` + the fast-fail counter persist
// to a session's tags, so an outage that drives the cap to its limit PARKS the agents, and then a fresh
// `convoy up`, reading those stale tags, `skip`s them forever (classify: `isFlapping → skip`). The
// reconstructed incident: a bring-up brought back only part of the fleet; the rest stayed parked from a
// prior supervisor's give-up and had to be hand-launched.
//
// The fix (see up.ts FRESH-SUPERVISOR UN-PARK + flapping-cap.ts clearParkForFreshSupervisor): a foreground
// `convoy up` is a DELIBERATE bring-up, so at startup it clears the park + zeroes the counter for permanent
// members (regardless of prior fail count); the cap re-accrues tick-to-tick within THIS supervisor's watch.
// The `--once` shepherd cron does NOT un-park (it runs every few minutes — un-parking there would relaunch
// a genuinely broken agent every tick), so parking stays durable for it.
//
// This proves it end to end: it stands up a PARKED, gone-but-recorded agent (via convoy's own spawn path
// plus the same `strategy.*` tags a prior supervisor would have written), then asserts a fresh FOREGROUND
// up UN-PARKS and RELAUNCHES it, while a fresh `--once` up leaves it parked. Process-level (real daemons +
// a real `convoy up`), scoped to a throwaway XDG_STATE_HOME. Lives in the vitest gate (test.yml), not the
// hermetic nix flake check.

import { afterEach, describe, expect, it } from "vitest";
import { spawn, spawnSync, type ChildProcess } from "node:child_process";
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { updateTags } from "@compoundingtech/pty/client";
import { gone, PtyHost, processAlive, spawnFromPtyFile } from "./host.ts";
import { FLAPPING_STATUS, TAG } from "./flapping-cap.ts";

const repoRoot = join(dirname(fileURLToPath(import.meta.url)), "..");
const bin = join(repoRoot, "bin", "convoy");
const sleep = (ms: number): Promise<void> => new Promise((r) => setTimeout(r, ms));

let home = "";
let net = "";
let host: ChildProcess | null = null;
const savedPtyRoot = process.env["PTY_ROOT"];

function childEnv(): NodeJS.ProcessEnv {
return { ...process.env, XDG_STATE_HOME: home, ST_ROOT: "", PTY_ROOT: "" };
}

function freshNet(): void {
home = mkdtempSync(join(tmpdir(), "cvy-park-"));
net = join(home, "convoy", "default");
mkdirSync(join(net, "catalog"), { recursive: true });
mkdirSync(join(net, "smalltalk"), { recursive: true });
}

/** Stand up one permanent agent whose harness EXITS quickly (`sleep 1`), so it lands in the gone-but-
* recorded state a real crashed agent occupies — the shape the park tags attach to and a replay relaunches. */
async function spawnAgent(id: string): Promise<void> {
const workspace = join(net, "agents", id);
mkdirSync(join(workspace, ".convoy"), { recursive: true });
writeFileSync(
join(workspace, ".convoy", "pty.toml"),
`prefix = "${id}"\n\n[sessions.claude]\nid = "${id}"\ncommand = "sleep 1"\n\n[sessions.claude.tags]\nstrategy = "permanent"\nrole = "agent"\n\n[sessions.claude.env]\nST_AGENT = "${id}"\n`,
);
const { spawned, failed } = await spawnFromPtyFile(workspace, net);
if (failed.length > 0 || spawned.length === 0) throw new Error(`spawn ${id} failed: ${JSON.stringify({ spawned, failed })}`);
}

/** Poll until the agent is in the real CRASHED shape: gone-but-recorded with a DEAD pid. The harness
* exits, and ~0.5s later the pty daemon writes its exit record and shuts down (pid clears) — only then is
* the pid dead, so `convoy up` treats it as a genuine death to RESPAWN rather than a transient-gone to
* ADOPT (the adopt-alive guard: reported-gone but pid-alive → adopt, never respawn). */
async function waitCrashed(id: string, timeoutMs = 12000): Promise<void> {
const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
const s = (await new PtyHost(net).sessions()).find((x) => x.name === id);
if (s && gone(s) && !processAlive(s.pid)) return;
await sleep(150);
}
throw new Error(`${id} never reached the crashed (gone + dead-pid) state`);
}

/** Write the park a prior supervisor would have left: status=flapping at the cap. */
function park(id: string): void {
updateTags(id, { [TAG.status]: FLAPPING_STATUS, [TAG.consecutive]: "3" });
}

/** The persisted strategy.status tag for an agent (undefined once cleared). */
async function statusTag(id: string): Promise<string | undefined> {
const s = (await new PtyHost(net).sessions()).find((x) => x.name === id);
return s?.tags[TAG.status];
}

function lockedHostPid(): number | null {
try {
const pid = Number.parseInt(readFileSync(join(net, "convoy.pid"), "utf8").trim(), 10);
return Number.isInteger(pid) && processAlive(pid) ? pid : null;
} catch {
return null;
}
}

/** Start a foreground `convoy up --json`, wait until it is hosting, run for `runMs` (long enough for the
* startup un-park + the immediate first reconcile), then kill it. Returns the captured JSONL stdout. */
async function runForegroundUp(runMs: number): Promise<string> {
const child = spawn(process.execPath, [bin, "up", net, "--json"], { env: childEnv(), stdio: ["ignore", "pipe", "pipe"] });
host = child;
let stdout = "";
child.stdout?.on("data", (d: Buffer) => (stdout += d.toString()));
const deadline = Date.now() + 15000;
while (Date.now() < deadline && lockedHostPid() !== child.pid) {
if (child.exitCode !== null) throw new Error(`convoy up exited early (code ${child.exitCode})`);
await sleep(50);
}
await sleep(runMs);
const exited = new Promise<void>((r) => child.once("exit", () => r()));
child.kill("SIGTERM");
await exited;
host = null;
return stdout;
}

/** Parse a JSONL stream into records. */
function records(stream: string): Array<{ type?: string; session?: string; spawned?: string[] }> {
const out: Array<{ type?: string; session?: string; spawned?: string[] }> = [];
for (const line of stream.split("\n")) {
if (!line.trim()) continue;
try {
out.push(JSON.parse(line));
} catch {
/* human line leaked to stdout? ignore */
}
}
return out;
}

afterEach(() => {
if (host) {
try {
host.kill("SIGKILL");
} catch {
/* ignore */
}
host = null;
}
try {
spawnSync(process.execPath, [bin, "down", net, "--force"], { env: childEnv() });
} catch {
/* ignore */
}
if (home) rmSync(home, { recursive: true, force: true });
if (savedPtyRoot === undefined) delete process.env["PTY_ROOT"];
else process.env["PTY_ROOT"] = savedPtyRoot;
});

describe("parking recovery — a fresh foreground `convoy up` restores a PARKED agent (Nathan mandate)", () => {
it("ACCEPTANCE: a fresh FOREGROUND up UN-PARKS and RELAUNCHES a parked gone agent (regardless of fail count)", async () => {
freshNet();
const id = "prk-alpha";
await spawnAgent(id);
await waitCrashed(id);
park(id);
expect(await statusTag(id), "the agent must be parked before the bring-up").toBe(FLAPPING_STATUS);

const out = records(await runForegroundUp(2500));

// It was UN-PARKED (the startup pass cleared the persisted park)...
expect(out.some((r) => r.type === "unpark" && r.session === id), "a fresh foreground up must emit an unpark for the parked agent").toBe(true);
// ...and then RELAUNCHED (the reconcile respawned it once un-parking made it eligible — a still-parked
// agent would have been skipped and never respawned/replayed).
const relaunched = out.some((r) => (r.type === "respawn" && r.session === id) || (r.type === "replay" && (r.spawned ?? []).includes(id)));
expect(relaunched, "a fresh foreground up must relaunch the un-parked agent").toBe(true);
// The persisted park is gone from disk.
expect(await statusTag(id), "the flapping status tag must be cleared on disk after the bring-up").not.toBe(FLAPPING_STATUS);
}, 45000);

it("CONTROL: a `--once` bring-up does NOT un-park — parking stays durable for the shepherd cron", async () => {
freshNet();
const id = "prk-beta";
await spawnAgent(id);
await waitCrashed(id);
park(id);

const r = spawnSync(process.execPath, [bin, "up", net, "--once", "--json"], { env: childEnv(), encoding: "utf8" });
expect(r.status, `--once should exit 0\nstderr:\n${r.stderr}`).toBe(0);

// No un-park was emitted, and the park is still on disk — `--once` must respect it (else it would
// relaunch a genuinely broken agent every few minutes).
expect(records(r.stdout).some((rec) => rec.type === "unpark"), "--once must NOT un-park anything").toBe(false);
expect(await statusTag(id), "--once must leave the park intact").toBe(FLAPPING_STATUS);
}, 45000);
});
28 changes: 28 additions & 0 deletions src/up.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { defaultConvoyNetwork, isNetworkName, networkDirForName, networkDirOfStR
import {
classify,
classifyFailedAttempt,
clearParkForFreshSupervisor,
effectiveLimit,
effectiveWindow,
isFlapping,
Expand Down Expand Up @@ -359,6 +360,33 @@ export async function up(opts: UpOptions): Promise<number> {
const notify = opts.notify ?? [];
const dingTargets = (crashed: SupervisedSession, sessions: readonly SupervisedSession[]): string[] => crashDingTargets(crashed, sessions, notify, busIdOf);

// FRESH-SUPERVISOR UN-PARK (parking-recovery, 2026-07-22). A foreground `convoy up` is a DELIBERATE
// bring-up — after a mass outage it MUST restore the FULL fleet, not inherit a prior supervisor's
// give-up. `strategy.status=flapping` + the fast-fail counter persist to each session's tags, so a
// parked agent stays parked across a restart (classify's `isFlapping → skip`), and a bring-up brought
// back only part of the fleet — the rest had to be hand-launched. So, ONCE at startup, clear the park
// and zero the counter for permanent members (regardless of prior fail count); the cap re-accrues
// tick-to-tick within THIS supervisor's watch. A fully-gone parked agent (no session record left) is
// relaunched by the catalog pass instead — this handles the gone-but-recorded ones the cap would skip.
//
// `--once` (the shepherd cron) SKIPS this: it runs every few minutes, so un-parking there would
// relaunch a genuinely broken agent every tick. Parking must stay durable across `--once`.
if (opts.once !== true) {
const startupNow = new Date();
for (const s of await host.sessions()) {
if (!isPermanent(s)) continue;
const cleared = clearParkForFreshSupervisor(parseStrategyTags(s.tags));
if (!cleared) continue;
host.removeTag(s.name, TAG.status); // updateTags MERGES — the park must be removed, not just overwritten
host.setTags(s.name, writtenTags(cleared)); // consecutive-fast-fails → 0
state.set(s.name, cleared);
emit(
{ type: "unpark", identity: logicalId(s), session: s.name, ts: isoString(startupNow) },
`[convoy-up] fresh supervisor — cleared parked/flapping state for ${logicalId(s)} session=${s.name}; giving it a fresh cap budget`,
);
}
}

const tick = async (): Promise<void> => {
const now = new Date();
// Manifest replay relaunches EVERY limb of an agent, so it must happen at most once per agent per
Expand Down
Loading