Skip to content
This repository was archived by the owner on Jul 24, 2026. It is now read-only.

Commit dab96f0

Browse files
key run's liveness on the DECLARATION's host, as reconcile does
`buildDeclaration` always populates `host` (`--host` ?? this machine), so keying liveness on the args-built agent file computed a this-machine bus id for an agent declared `host = otherbox` — whose catalog file is present locally via fabric sync. `convoy run --identity <that agent>` would find nothing live and launch a DUPLICATE alongside the real session. That falsified the property this design rests on: `run` and `up` must agree on what "already running" means. `up` reconciles on `entry.af.host ?? thisHost` — the declaration's host — so `run` now reads the existing declaration BEFORE computing the bus id and keys off it. The choice is a named, exported function (`livenessAgentFile`) rather than an inline `??`: it is the single point where the two verbs agree or diverge, so it deserves to be visible and directly testable. Reverting it to return the args-built file turns two tests red. Also, while reading the existing declaration up front: · reuse it for `effective` instead of re-reading the file · warn when a positional ROLE differs from the declared one — the role is not a flag, so `passedDeclarationFlags` could not see it and it was silently dropped on resume · print `session:` / `bus:` ids, so a declaration owned by another host is visible before anything launches Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ePNMmLYa7qVT3h7bRCWUJ agent-session-id: 0abcedc7-6b71-4046-9e7c-f645268c0b15 agent-tool: Claude Code agent-tool-version: 2.1.215 agent-model: claude-opus-4-8 agent-runtime-profile: /nix/store/acr8a3l2v366jgmwiq8xdrhgz1py0db5-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/sj1v5j91h8v8d1w9lca4040302lwrd6v-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
1 parent f3c7664 commit dab96f0

3 files changed

Lines changed: 86 additions & 12 deletions

File tree

src/commands.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { runFullOrgSuite, runReadinessSuite } from "./doctor/suite.ts";
3030
import { structureChecks } from "./doctor/structure.ts";
3131
import { baseFile, ensureInstalled, personasDir, personasInstalled } from "./personas.ts";
3232
import { ROLES, parseRole } from "./role.ts";
33-
import { declaredRunNotice, liveForceRefusal, passedDeclarationFlags, resolveRunAction, staleFlagsNote } from "./run.ts";
33+
import { declaredRunNotice, liveForceRefusal, livenessAgentFile, passedDeclarationFlags, resolveRunAction, staleFlagsNote } from "./run.ts";
3434
import { busAgentId, isValidModel, preflight, resolvedPersonaPath, sessionId, shortHostname, type AgentSpec, type Transport } from "./agent-spec.ts";
3535
import { HARNESSES, HARNESS_SESSION_KEYS, HARNESS_SUFFIX_RE, harnessDescriptor, harnessesInPtyToml, harnessLimitations, isHarness, type Harness } from "./harness.ts";
3636
import { claudeConfigPath, codexConfigPath, pretrustDirs, pretrustDirsCodex } from "./trust.ts";
@@ -972,7 +972,24 @@ export async function cmdRun(args: string[]): Promise<number> {
972972
// `busIdOf`), so `run` and `up` can never disagree about what "already running" means. The gone-but-pid-
973973
// alive tolerance is reconcile's too: pty can transiently report `gone` under load, and treating that as
974974
// dead would relaunch a live agent.
975-
const busId = agentBusId(af, shortHostname());
975+
// Read the EXISTING declaration first, because liveness must be keyed off it, not off this invocation's
976+
// flags. `buildDeclaration` always sets `af.host` (`--host` ?? this machine), whereas reconcile keys on
977+
// `entry.af.host ?? thisHost` — the DECLARATION's host. Keying on the args host would mean that for an
978+
// agent declared `host = dev4`, a `convoy run --identity <id>` on another box (its catalog file is present
979+
// via fabric sync) computes the wrong bus id, sees nothing live, and launches a duplicate. That would
980+
// falsify the exact property this design rests on: `run` and `up` must agree on what "already running"
981+
// means. Deriving both from the same source makes them agree by construction.
982+
let existingAf: AgentFile | null = null;
983+
if (existed) {
984+
try {
985+
existingAf = readAgentFile(path);
986+
} catch (e) {
987+
err(`agent "${identity}" is declared at ${path} but its agent file could not be read: ${e instanceof Error ? e.message : String(e)}`);
988+
return 1;
989+
}
990+
}
991+
992+
const busId = agentBusId(livenessAgentFile(existingAf, af), shortHostname());
976993
const live = (await new PtyHost(network).sessions()).filter(
977994
(s) => busIdOf(s) === busId && (!gone(s) || processAlive(s.pid)),
978995
);
@@ -987,15 +1004,14 @@ export async function cmdRun(args: string[]): Promise<number> {
9871004
// exists, and silently re-declaring from this invocation's flags would let a stray `--model` mutate a
9881005
// synced, fleet-visible agent file as a side effect of attaching to it.
9891006
const reuseExisting = action === "resume" || action === "attach";
990-
let effective = af;
1007+
const effective = reuseExisting && existingAf ? existingAf : af;
9911008
if (reuseExisting) {
992-
try {
993-
effective = readAgentFile(path);
994-
} catch (e) {
995-
err(`agent "${identity}" is declared at ${path} but its agent file could not be read: ${e instanceof Error ? e.message : String(e)}`);
996-
return 1;
997-
}
998-
const note = staleFlagsNote(identity, passedDeclarationFlags(args));
1009+
// The role is a POSITIONAL, not a flag, so `passedDeclarationFlags` cannot see it — report it
1010+
// separately rather than silently resuming `cos` as its declared role when the caller typed another.
1011+
const declaredRole = existingAf?.role;
1012+
const askedRole = positionals(args)[0];
1013+
const roleDiffers = askedRole !== undefined && declaredRole !== undefined && parseRole(askedRole) !== declaredRole;
1014+
const note = staleFlagsNote(identity, [...(roleDiffers ? [`role "${askedRole}"`] : []), ...passedDeclarationFlags(args)]);
9991015
if (note) out(note);
10001016
}
10011017

@@ -1005,6 +1021,9 @@ export async function cmdRun(args: string[]): Promise<number> {
10051021

10061022
out(`convoy run — ${identity} (${effective.harness ?? "claude"}, ${effective.role})`);
10071023
out(`workspace: ${effective.workspace ?? "(none)"}`);
1024+
// Print the host-prefixed ids: they are what `convoy up`, `pty attach`, and `st` all key on, and showing
1025+
// them makes an unexpected host (a declaration owned by another box) visible before anything launches.
1026+
out(`session: ${ref} · bus: ${busAgentId(spec)}`);
10081027

10091028
if (action === "attach") {
10101029
out(` already running (${live.map((s) => s.name).join(", ")}) — attaching, NOT restarting it.`);

src/run.test.ts

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { describe, it, expect } from "vitest";
22
import { spawnSync } from "node:child_process";
33
import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs";
4-
import { tmpdir } from "node:os";
4+
import { hostname, tmpdir } from "node:os";
55
import { dirname, join } from "node:path";
66
import { fileURLToPath } from "node:url";
77
import { COMMANDS } from "./command-table.ts";
8-
import { declaredRunNotice, liveForceRefusal, passedDeclarationFlags, resolveRunAction, staleFlagsNote } from "./run.ts";
8+
import { declaredRunNotice, liveForceRefusal, livenessAgentFile, passedDeclarationFlags, resolveRunAction, staleFlagsNote } from "./run.ts";
9+
import { agentBusId } from "./reconcile.ts";
910

1011
const root = dirname(dirname(fileURLToPath(import.meta.url)));
1112
const bin = join(root, "bin", "convoy");
@@ -93,6 +94,29 @@ describe("passedDeclarationFlags — distinguishes declaration flags from per-in
9394
});
9495
});
9596

97+
describe("livenessAgentFile — the single point where `run` agrees with `up` about what is running", () => {
98+
const declared = { identity: "x", role: "worker" as const, host: "otherbox" };
99+
const fromArgs = { identity: "x", role: "worker" as const, host: "thisbox" };
100+
101+
it("ACCEPTANCE: keys on the EXISTING declaration — `up` reconciles on `af.host`, so `run` must too", () => {
102+
// `buildDeclaration` ALWAYS populates host (--host ?? this machine), so keying on the args-built file
103+
// computes a this-machine bus id for an agent declared `host = otherbox` (catalog file arrived via
104+
// fabric sync). run would find nothing live and launch a DUPLICATE beside the real one.
105+
expect(livenessAgentFile(declared, fromArgs)).toBe(declared);
106+
expect(livenessAgentFile(declared, fromArgs).host).toBe("otherbox");
107+
});
108+
109+
it("falls back to the args-built file only when nothing is declared yet (the first-run path)", () => {
110+
expect(livenessAgentFile(null, fromArgs)).toBe(fromArgs);
111+
});
112+
113+
it("keys identically to reconcile's agentBusId for an explicitly-hosted agent", () => {
114+
const thisHost = "thisbox";
115+
expect(agentBusId(livenessAgentFile(declared, fromArgs), thisHost)).toBe(agentBusId(declared, thisHost));
116+
expect(agentBusId(livenessAgentFile(declared, fromArgs), thisHost)).toBe("otherbox.x");
117+
});
118+
});
119+
96120
describe("declaredRunNotice — states the guarantees, the exact inverse of #92's disclaimer", () => {
97121
it("ACCEPTANCE: says detaching leaves the agent RUNNING (a pty session outlives its client)", () => {
98122
const n = declaredRunNotice("fodfix", "dev3.fodfix", "dev3.fodfix");
@@ -225,6 +249,22 @@ describe("`convoy run` end to end", () => {
225249
}
226250
});
227251

252+
it("resolves an explicitly-hosted declaration to ITS host, not this machine's", () => {
253+
const h = setup();
254+
try {
255+
const catalog = catalogOf(h);
256+
mkdirSync(catalog, { recursive: true });
257+
writeFileSync(join(catalog, "elsewhere.toml"), 'identity = "elsewhere"\nrole = "worker"\nhost = "otherbox"\nworkspace = "' + h + '"\n');
258+
// No --host passed: the declaration's host must still win for the session ref and the bus id.
259+
const r = cli(["run", "--identity", "elsewhere", "--dry-run", "--no-attach"], h);
260+
expect(r.rc).toBe(0);
261+
expect(r.out).toContain("otherbox.elsewhere");
262+
expect(r.out).not.toContain(`${hostname().split(".")[0]?.toLowerCase()}.elsewhere`);
263+
} finally {
264+
teardown();
265+
}
266+
});
267+
228268
it("accepts --permanent, which #92 rejected (rc=2) as a contradiction for an undeclared session", () => {
229269
const h = setup();
230270
try {

src/run.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,21 @@ export function passedDeclarationFlags(args: readonly string[]): string[] {
111111
return DECLARATION_FLAGS.filter((f) => args.includes(f));
112112
}
113113

114+
/** Which agent file `run` must key LIVENESS on: the existing declaration whenever there is one, never the
115+
* one just built from this invocation's flags.
116+
*
117+
* This is the single point where `run` either does or does not agree with `convoy up` about what "already
118+
* running" means, so it is a named function rather than an inline `??` — the coherence claim of this whole
119+
* design reduces to this choice.
120+
*
121+
* `up`'s reconcile keys on `entry.af.host ?? thisHost` — the DECLARATION's host. `buildDeclaration` always
122+
* populates `host` (`--host` ?? this machine), so keying on the args-built file would compute a
123+
* this-machine-prefixed bus id for an agent declared `host = otherbox` (its catalog file having arrived by
124+
* fabric sync). `run` would then find nothing live and launch a DUPLICATE alongside the real one. */
125+
export function livenessAgentFile<T>(existing: T | null, fromArgs: T): T {
126+
return existing ?? fromArgs;
127+
}
128+
114129
/** The line `run` prints once the session is up, stating the guarantees it DOES have — the exact inverse
115130
* of #92's `adHocNotice`, which existed to disclaim them. Detaching is safe and is worth saying: a pty
116131
* session outlives its client, and because the agent is declared, `convoy up` also respawns it if the

0 commit comments

Comments
 (0)