This repository was archived by the owner on Jul 24, 2026. It is now read-only.
convoy run declares, launches, and attaches (supersedes #92) - #98
Draft
schickling-assistant wants to merge 2 commits into
Draft
convoy run declares, launches, and attaches (supersedes #92)#98schickling-assistant wants to merge 2 commits into
convoy run declares, launches, and attaches (supersedes #92)#98schickling-assistant wants to merge 2 commits into
Conversation
`convoy run` shipped in #92 as an AD-HOC session: deliberately not declared, not reconciled, not respawned, no durable context. This replaces that with the intended semantics — `run` is exactly `convoy add`, plus a launch and an attach. The verb set becomes coherent: add declare (no launch) render materialize the overlay (no launch, no bus) up reconcile — launch what is declared (no attach) run declare, launch, and attach The deeper reason: an undeclared session has no stable identity, therefore no durable context/ directory, therefore it cannot externalize state or converge after a restart. That is a permanent second class of session. Making `run` declare means every agent gets the same guarantees and the ad-hoc/declared distinction disappears rather than being managed forever. `add` and `run` now share `buildDeclaration`, so the two verbs cannot drift. Identity is REQUIRED, as for `add`. #92 generated one (`run-b3ur0v`) when it was omitted; that is precisely the hashed-name problem that makes durable context unreachable — nobody re-derives a random name on the next cold boot. Attach/detach: detaching leaves the agent running (a pty session outlives its client), and because it is declared, `convoy up` reconciles and respawns it. `--no-attach` covers non-interactive callers. Already-exists is a decision table, NOT `add`'s flat refuse-without-force: not declared → declare + launch + attach declared, not live → RESUME from the existing declaration (the point of declaring; refusing would break it) declared, live → attach to the running session, never restart declared, not live, --force → re-declare with these flags, then launch declared, live, --force → refuse; `convoy reload` is the kill+respawn verb Removed the ad-hoc machinery rather than leaving two models: generateAdHocIdentity, adHocNotice, isAdHocIdentity, AD_HOC_PREFIX, and validateRunIdentity (whose premise inverted — collision-with-declared is now the resume case). Help text in cli.ts and the command table no longer advertise the old semantics. 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
`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
schickling-assistant
force-pushed
the
schickling-assistant/2026-07-21-run-declares
branch
from
July 21, 2026 12:12
dab96f0 to
5bd857a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This supersedes the design that shipped in #92
If you read #92, read this section first — otherwise this looks like a regression.
#92 shipped
convoy runas an ad-hoc session, and said so in the help text: "launch an AD-HOC session — NOT declared, NOT reconciled, NOT respawned, no durable context (declare it withaddif it should survive)." This PR replaces that with the intended semantics:convoy runis exactlyconvoy add, plus a launch and an attach.The verb set becomes coherent:
addrenderuprunWhy #92's reasoning does not survive
#92 argued the declared model was "not implementable honestly", on three grounds.
1. "
supervisoris required for every non-root agent, so an ephemeral declaration would fabricate one and corrupt the supervision tree." — Refuted. No such field requirement exists.parseAgentFilerequires onlyidentityandrole;supervisoris optional throughout. This was factually wrong.2. "The catalog syncs union/no-delete, so a per-run entry is permanent fleet-wide garbage." — Stops being an objection under this model. The entry is not garbage: it is a declared agent that happens to have been started interactively. Permanence is the intent, not a leak. Retirement is what
retired = trueis for.3. "
strategyaccepts onlypermanent." — Consistent rather than obstructive, since a run-created agent is a real declared agent.--permanentis now accepted byrun(#92 rejected it with rc=2).The deeper reason
An undeclared session has no stable identity → no durable
context/directory → it cannot externalize state or converge after a restart. That is a permanent second class of session, one that could never be promoted. Makingrundeclare means every agent gets the same guarantees, and the ad-hoc/declared distinction disappears rather than being managed forever.The coherence property that makes this safe
convoy up's reconcile keys liveness on the bus id<host>.<identity>(reconcile.tsagentBusId), andrunlaunches through the very samenativeLaunchthat up's reconcile uses, pinning the same identity-derived session id (writePtyToml→sessionId(spec)). So a session started byrunis adopted by a concurrently-runningconvoy up— never double-spawned. Declaring and launching in one breath is indistinguishable, to every other verb, fromaddfollowed byup.This was the load-bearing unknown and it was verified in the source before anything was built, not assumed.
Decisions you should push back on if you disagree
Identity: required, never generated
runnow requires--identity, exactly asadddoes. #92 generated one (run-b3ur0v) when it was omitted.A generated identity reintroduces precisely the hashed-name problem that makes durable context unreachable: nobody re-derives a random name on the next cold boot, so the agent can never find its own
context/again. Deriving from the role (worker-1) is worse — a counter re-derives within a parent's lifetime, so after a restartworker-2names a different agent and reads a stranger's memory (this is #88 item 6, and #92 itself cited it as the reason its discriminator was random).The error message explains the why, since #92 accepted the omission happily:
Attach / detach
Detaching leaves the agent running. This is free from pty's model (a session outlives its client) and it is the correct semantic here: the agent is declared, so
convoy upalso reconciles and respawns it. The printed notice states this rather than leaving it to be discovered — it is the exact inverse of #92'sadHocNotice, which existed to disclaim these guarantees.--no-attachcovers non-interactive callers (declare + launch, return).Already-exists: a decision table, NOT
add's refuse-without---forceThis is a deliberate divergence from "exactly like
add", and the one place a reviewer holding me to that phrase should look:--forceconvoy reload <id>The resume row is why.
addrefuses a re-declare without--force; inheriting that flatly would make re-running an identity you already declared an error — breaking the exact property this redesign exists to deliver (durable context survives a restart, so resuming is the everyday path, not a collision).The last row refuses rather than growing a second kill-and-respawn: overwriting a live agent's declaration and relaunching would spawn a second session on the same pinned session id.
convoy reloadalready does that job properly.On resume/attach the existing declaration is authoritative — a stray
--modelmust not mutate a synced, fleet-visible agent file as a side effect of attaching. Ignored flags are named out loud rather than silently dropped (staleFlagsNote), since silent ignoring is a real footgun.Two bugs this surfaced
Liveness was keyed on the wrong host.
buildDeclarationalways populateshost(--host?? this machine), so keying liveness on the args-built agent file computed a this-machine bus id for an agent declaredhost = otherbox— whose catalog file is present locally via fabric sync.convoy run --identity <that agent>would have found nothing live and launched a duplicate alongside the real session, falsifying the coherence property above.runnow reads the existing declaration before computing the bus id and keys off it, exactly as reconcile does.The choice is a named, exported function (
livenessAgentFile) rather than an inline??, because it is the single point where the two verbs agree or diverge. Reverting it to return the args-built file turns two tests red.One ordering bug
:
buildDeclarationrefused "no workspace" before knowing the agent was already declared — which would have forced callers to re-specify--diron every resume, repeating what the catalog already knows. The check is now skipped whenrunis resuming an existing declaration;addkeeps the hard requirement, since it only ever authors a declaration and has nothing to fall back on.What was removed
The ad-hoc machinery is deleted, not left dormant — two models in the codebase is the thing being fixed:
generateAdHocIdentity,adHocNotice,isAdHocIdentity,AD_HOC_PREFIXvalidateRunIdentity— note its premise inverted: it refused an identity colliding with a declared agent, and that collision is now precisely the resume case. Not dead code; wrong code.add'sidentityErrorscovers grammar and socket-budget validation.--prefixand--config-dirdropped fromrun: the declaration carries the host prefix (--host) and the harness config dir (viaenv), and a per-invocation override of either would desync the session from the agent file describing it.addandrunnow sharebuildDeclaration, so they cannot drift — enforced by a test assertingrun's flag set is exactlyadd's plus--no-attach.Help text updated in both
cli.tsandcommand-table.ts; the old wording actively advertised the superseded semantics.Tests
src/run.test.tswas rewritten rather than extended — it was entirely ad-hoc machinery. 39 tests: the decision table, the messages, command-table parity withadd, and end-to-end through the real CLI against throwaway networks.Proven red without the change: applying this test file to unmodified
maingives 35 failed / 4 passed. The 4 that pass are unchanged-behavior guards (unknown flag, unknown role, identity grammar validation, table presence). EveryACCEPTANCE:test and every behavior-change test fails on baseline.Full suite: 548 passed, with 4 pre-existing environmental failures identical to
origin/main(bash-3.2 heredoc parsing indoctor/hooks,git worktreeinlaunch/network-config). No regressions.Reviewer notes — what I did not verify
--dry-runor--no-attachagainst a throwaway network. ThenativeLaunch→pty attachhandoff is verified by construction (I confirmedwritePtyTomlpins the harness session tosessionId(spec)andspawnFromPtyFilespawns under exactly that name, so the attach ref is right) but never by spawning a real harness. Production was not disturbed.execInteractiveis new (spawnwithstdio: "inherit") becauseexec.ts'srunbuffers throughexecFile, which would break raw mode and the detach key. Its behavior under a non-TTY stdin is untested.runtime —nativeLaunchmkdirs the workspace and writes the overlay, matching whatupdoes today (launch.tscalls megarepo worktree-cutting a follow-up). Soruninherits that gap rather than fixing it; arunagainst a megarepo-configured network gets a plain directory where a worktree is intended, exactly asupdoes.