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

fix: convoy up double-logs; init exits 0 on failed root-agent bootstrap; checkDevTask never launches its tiers - #90

Merged
schickling-assistant merged 4 commits into
mainfrom
schickling-assistant/2026-07-20-valiant-tesla-7
Jul 21, 2026
Merged

fix: convoy up double-logs; init exits 0 on failed root-agent bootstrap; checkDevTask never launches its tiers#90
schickling-assistant merged 4 commits into
mainfrom
schickling-assistant/2026-07-20-valiant-tesla-7

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Three of the four Stream D convoy defects, one commit each, branched off main.

⚠️ Adjacent finding, read first: #89 already contains code fixes for two of these defects

#89 is described as "agent spec + convoy's VRS", but it also carries behavioural fixes in src/doctor/suite.ts and src/up.ts for Stream D defects 1 and 4. That is very likely not widely known. It changes what belongs where, so it's the first thing to decide on:

Either leave it split this way (the two PRs are conflict-free in both directions — see Verification), or pull the fifth insertion into #89 so all of suite.ts moves together and drop that commit from here. Both work; your call.


Fixed here

1. convoy up printed every supervisor log line twice — fix(up)

emit() wrote the human line to stderr and, in the non---json branch, wrote the same human line to stdout. For a plain convoy up both streams land on the same terminal, so every reconcile line appeared twice.

Reproduced live. convoy up <net> --once on a throwaway network, before the fix:

hosting /tmp/…/net (reconcile every 30s, cap 3 fails / 60s)
hosting /tmp/…/net (reconcile every 30s, cap 3 fails / 60s)
[convoy-up] catalog NOT declared to fabric sync — …
[convoy-up] catalog NOT declared to fabric sync — …
[convoy-up] once: reconciled … launched/spawned 0, adopted 0, …
[convoy-up] once: reconciled … launched/spawned 0, adopted 0, …
[convoy-up] stopping host; leaving 0 session(s) running — …
[convoy-up] stopping host; leaving 0 session(s) running — …

Each line once after the fix; --json stdout is still a clean JSONL stream.

Reasoning for the stream choice: stdout carrying a copy of the human text was never useful. A caller that wants to parse the stream passes --json (stdout = JSONL); a caller that wants to read it already has it on stderr. That is also what the existing comment declared the contract to be ("the human line always goes to stderr; stdout carries the JSONL stream when --json") — the non-json stdout write contradicted it. So non-json now leaves stdout empty; --json behaviour is byte-identical. No in-repo consumer parses up's non-json stdout (the doctor suite checks .ok; backgroundUp uses stdio: "ignore").

Routing is extracted to a pure exported emitWrites(obj, human, json) so the contract is unit-testable in convoy's existing style (cf. workerCrashed). The acceptance test asserts the human line appears exactly once across both streams.

The wiring is covered too, not just the decision (see Coverage of the wiring below). emitWrites only decides what goes where; the bug lived in the code that acted on it, and that code was unreachable from a test — re-inlining the buggy writes in up() left the suite byte-identical and tsc clean. The closure is now an exported makeEmit(json, out, err) with injectable sinks, so a test drives the real emitter and counts what each stream received.

2. convoy init exited 0 after failing to create the root agent — fix(init)

cmdInit ran the CoS bootstrap, printed CoS bootstrap did not complete on a nonzero cmdCos rc, and then fell through to an unconditional return 0. A scripted convoy init && convoy up therefore saw green and proceeded against a network with an empty catalog — the root agent it just asked for never came up.

init now propagates cmdCos's rc, and on failure replaces the cheerful ✓ Network … is ready / next: run convoy doctor tail with an honest one: the structure exists, it has no agents, here is how to retry. Declining the CoS prompt — and every non-interactive run, where the branch never executes at all — still exits 0; the structure is the deliverable there. That negative control is tested.

Reproduced by test, not live — stated plainly because it matters: the CoS bootstrap is TTY-gated (interactive && await askYesNo(...)), so there is no non-interactive repro. The decision is extracted to a pure initExitCode(cosCode) and asserted red-then-green: with the old unconditional return 0 restored in place, the two new acceptance tests fail. The wiring of that function into cmdInit — that it captures cmdCos's rc, consults initExitCode, and returns the result rather than falling through to return 0 — is now held by a source guard, so reverting cmdInit to the shipped bug reds the suite. (Earlier revisions of this PR left that hop inspection-only.)

3. checkDevTask declared three tiers and never launched them — fix(doctor)

The fifth declare-then-assert check. Its add-loop declares cos + supervisor + worker, then pollUntil waits for all three to reach available — against sessions convoy add never spawned ("NOTHING launched — the catalog is desired state. Run convoy up to reconcile"). One up --once pass launches all three.

Reproduced by inspection, not live: running it end-to-end needs real agent auth and minutes of real spawning, and the missing up is unambiguous in the source (no runConvoy call site in checkDevTask invokes up). I did verify the underlying premise empirically — convoy add writes a catalog entry and leaves host.sessions() empty (see below).

Because this is a class of bug nobody was watching for, it also adds a source-level guard: the checks can't run in a unit test, but "a check that declares agents reconciles them, before it asserts liveness" is a structural invariant.

The guard derives its list of declaring checks by scanning suite.ts, and matches against comment-stripped source. Both properties are load-bearing, and an earlier revision of this PR had neither — see Coverage of the wiring below. It still carries an exception list naming the four checks whose reconcile lands in #89, so it is green today and the gap is visible in code rather than only in a PR description. That list is now stated as a shrink-only property rather than an exact-match snapshot, so it is safe to empty without breaking the suite.


Coverage of the wiring — what an adversarial review found, and what changed

Two of this PR's own fixes could be reverted with the suite fully green. Both are closed, each with a revert applied and restored as proof.

1. The emit and init tests covered the pure function, not the wiring. Reverting up()'s emit closure to the buggy form — writing the human line to stderr and stdout — while leaving emitWrites intact left the suite byte-identical (294 passed / 4 failed) and passed tsc. Reverting the pure function instead produced 6 failures, which is what made the gap easy to miss: the tests looked strong because they were testing the wrong half. convoy init's tail had the identical shape.

2. The doctor guard was a defeatable substring match. Replacing the reconcile with // TODO: runConvoy(box, ["up", box.net, "--once"]) -- disabled left 8/8 guard tests passing with the bug fully reopened; a comment above pollUntil( satisfied the ordering check the same way. Its hardcoded check list could only ever guard checks someone remembered to list — checkFullOrg was already a sixth declaring check absent from it — and this PR's own instruction to delete the exception list when #89 lands broke the expect(covered).toEqual(["checkDevTask"]) snapshot it shipped with.

What changed

  • src/up.ts — the emit closure is now an exported makeEmit(json, out, err) with injectable sinks: the wiring itself is executed by tests.
  • src/source-guard.ts (new) — gives source guards a real tokenizer: stripComments (a guard can only be satisfied by code, never by a comment), brace-matched functionBody, and exportedAsyncFunctions. It deliberately handles the shapes that silently break a naive scanner and make every downstream guard vacuous: // inside a string, and a regex literal containing quote characters — up.ts's own /ST_AGENT\s*=\s*"([^"]+)"/. It has its own tests, including the exact defeat above.
  • src/doctor/suite.test.ts — the guard now derives declaring checks from suite.ts; accepts either legitimate reconcile form (up --once, or a background convoy up, which is how checkFullOrg reconciles — so it is covered rather than falsely flagged); asserts ordering non-vacuously (absence is -1, which sailed straight past toBeLessThan); and states the exception list as a shrink-only property instead of a snapshot, so deleting it widens coverage.
  • src/commands.test.ts — a matching wiring guard for cmdInit.

Evidence — each revert applied, suite run, then restored

Revert Before After
Re-inline the buggy emit in up() green (byte-identical, tsc clean) 2 red
makeEmit writes the human line to both streams n/a (unreachable) 2 red
cmdInit falls through to return 0 green 1 red
Reconcile replaced by a comment naming it green (8/8) 1 red
A new declaring check with no reconcile invisible 1 red, no list edit needed
Empty the exception list +1 spurious bookkeeping failure on top of the genuine ones 8 failures, all genuine (the 4 unreconciled #89 checks x 2 assertions); no bookkeeping breakage

The pre-existing 4 failures are unchanged and unrelated.


Dropped — defect 4, fully fixed in #89

Verified real at main: up's up-front batch pre-trust builds dirs solely from await host.sessions(), so a first-ever bring-up of freshly-declared agents contributes zero dirs and every agent falls through to the per-agent write inside nativeLaunch, racing exactly as if the batch weren't there — precisely the case the batch exists for. Observed directly: convoy add worker --identity repro-wk --dir …, then at the point up's pretrust block runs, host.sessions().length === 0.

#89 additionally seeds from readCatalog(root)'s host-filtered, non-retired entries via e.af.workspace. I checked the one thing that fix depends on — cmdAdd does persist --dir as workspace into the catalog toml (confirmed in the generated catalog/repro-wk.toml) — so #89's fix is sound, not a no-op. Nothing to add.

Also confirming the correction in the brief: up --once does pre-trust. The batch block sits before the do/while, so it runs on a one-shot pass. The earlier "doesn't pre-trust at all" report was wrong; the seeding source is the real defect.


Verification

Draft, and deliberately no auto-merge.

schickling-assistant and others added 3 commits July 20, 2026 21:48
`convoy up`'s emit() wrote the human line to stderr AND, in the non-json
branch, the same human line to stdout. Both streams land on the same
terminal for a plain `convoy up`, so EVERY supervisor line printed twice —
the doubled reconcile log.

stdout carrying a copy of the human text was never useful: a caller that
wants to parse the stream passes --json (stdout = JSONL), and a caller that
wants to read it already has it on stderr. So non-json now leaves stdout
empty; --json is unchanged.

Reproduced live before the fix (`convoy up <net> --once` on a throwaway
network printed all four lines twice) and after (once each; --json stdout
still a clean JSONL stream).

The stream routing is extracted to a pure `emitWrites` so the contract is
unit-testable in convoy's style (cf. workerCrashed).

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
`convoy init` printed "CoS bootstrap did not complete" on a nonzero
`cmdCos` rc and then fell through to an unconditional `return 0`. So a
scripted `convoy init && convoy up` saw green and moved on to a network
whose catalog is empty — the root agent it just asked for never came up.
A failed bootstrap must not report success.

init now propagates cmdCos's rc, and on failure replaces the cheerful
"✓ Network … is ready / next: run convoy doctor" tail with an honest line:
the structure exists, it has no agents, here is how to retry. Declining the
CoS prompt (and every non-interactive run, where the branch never executes)
still exits 0 — the structure IS the deliverable there.

The CoS branch is TTY-gated (`interactive && askYesNo(...)`), so this is
reproduced by TEST, not by a live run: the decision is extracted to a pure
`initExitCode` and asserted red-then-green (with the old unconditional
`return 0` restored, the two new acceptance tests fail).

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
…ing them

`convoy add` is declare-only — it writes a catalog entry and launches
nothing — so checkDevTask's add-loop declares cos/supervisor/worker and then
polls all three for `available` against sessions that were never spawned.
It fails by construction on every machine, and runReadinessSuite returns 0
only if EVERY check passes, so this alone reds a newcomer's first
`convoy doctor`. One `up --once` pass launches all three declared tiers.

SCOPE: this is the FIFTH of five declare-then-assert checks missing the
reconcile. PR #89 already inserts the same `up --once` into the other four
(checkTmpNetwork, checkDings, checkStateExternalization, checkExactlyOnce)
and owns those hunks, so this commit deliberately does NOT duplicate them —
it closes the one #89 missed, in a region #89 does not touch. The suite is
only fully green once both land.

Adds a source-level guard for the declare→assert seam: the checks spawn real
agents (minutes, real auth) so they can't run in a unit test, but the
invariant "a check that declares agents reconciles them, BEFORE it asserts
liveness" is structural and checkable. The guard carries a shrink-only
exception list naming the four checks whose reconcile lands in #89; whoever
merges #89 deletes the list and the guard covers all five.

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 schickling-assistant changed the title fix: convoy up double-logs every line; convoy init exits 0 on a failed root-agent bootstrap fix: convoy up double-logs; init exits 0 on failed root-agent bootstrap; checkDevTask never launches its tiers Jul 20, 2026
schickling-assistant added a commit that referenced this pull request Jul 20, 2026
…ility

The survivingLimbs suite conflicted with PR #90 for purely textual reasons —
both appended a describe block to the tail of src/up.test.ts and both edited
its import line. No semantic overlap (#90 tests emitWrites, this tests
recovery). Relocating to src/up-recovery.test.ts restores src/up.test.ts to
main's exact content, so this branch is independently mergeable in any order
against both open PRs.

Also emit a line on a failed recovery attempt that is still under the cap.
Silence until the park would have been a step back from the old
"(spawn FAILED)" line, and that is precisely the state convoy#82 showed was
invisible.

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
…omment

Two ways this PR's own fixes could be reverted with the suite green.

1. THE WIRING WAS UNTESTED. The `emitWrites`/`initExitCode` tests cover the
   pure DECISION; nothing covered the code that acts on it. Restoring the
   doubled-log bug verbatim — re-inlining `process.stdout.write(... human)`
   in `up()` — left the suite BYTE-IDENTICAL (294 passed / 4 failed) and
   `tsc` clean. `convoy init`'s tail had the same shape.

   `up()`'s emit closure is extracted to an exported `makeEmit(json, out, err)`
   with injectable sinks, so a test drives the REAL emitter and counts what
   each stream received. The last hop — that `up()` uses it and never writes
   to stdout itself — plus `cmdInit`'s (TTY-gated, so unreachable at runtime)
   rc propagation are held as source guards.

2. THE DOCTOR GUARD WAS A SUBSTRING MATCH. Replacing the reconcile with
   `// TODO: runConvoy(box, ["up", box.net, "--once"]) -- disabled` left 8/8
   guard tests passing with the bug fully reopened; a comment above
   `pollUntil(` likewise satisfied the ordering check. Its hardcoded check
   list could only guard the checks someone remembered to list — `checkFullOrg`
   was already a sixth declaring check missing from it — and the PR's own
   instruction to delete the exception list when #89 lands BROKE the
   `expect(covered).toEqual(["checkDevTask"])` snapshot it shipped with.

   New `src/source-guard.ts` gives source guards a real tokenizer:
   `stripComments` (a guard can only be satisfied by code), brace-matched
   `functionBody`, and `exportedAsyncFunctions`. It handles the shapes that
   silently break a naive scanner and make every downstream guard vacuous —
   `//` inside a string, and a regex literal containing quotes (up.ts's
   `/ST_AGENT\s*=\s*"([^"]+)"/`).

   The doctor guard now DERIVES its declaring checks from suite.ts instead of
   hardcoding them, accepts either legitimate reconcile form (`up --once` or a
   background `convoy up` — how checkFullOrg reconciles), asserts the ordering
   non-vacuously (absence is -1, which sailed past `toBeLessThan`), and states
   the exception list as a shrink-only property so DELETING it when #89 lands
   widens coverage instead of reddening the suite.

Evidence, each revert applied and restored:
  • re-inline the buggy emit in up()      → 2 red   (was: 0)
  • makeEmit writes human to both streams → 2 red   (was: n/a)
  • cmdInit falls through to `return 0`   → 1 red   (was: 0)
  • reconcile replaced by a comment       → 1 red   (was: 0, 8/8 green)
  • a new declaring check, no reconcile   → 1 red   (was: invisible)
  • delete the exception list (#89 lands) → only the 4 genuine #89 gaps,
                                            no bookkeeping breakage (was: +1)

294 → 329 passing; the same 4 pre-existing failures, unchanged. tsc clean.

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
schickling-assistant marked this pull request as ready for review July 20, 2026 23:47
schickling-assistant added a commit that referenced this pull request Jul 21, 2026
…93)

* fix(up): recover from provider death by replaying the manifest

convoy detected provider death precisely and then did nothing. Reproducing it
found the cause is not a missing feature but a CONTRACT DEADLOCK between two
repos we co-maintain:

  - pty's stateful-agent guard REFUSES `pty restart` on any `role=agent`
    session, and its error points the operator at the supervisor: "Cycle it
    through its supervisor (e.g. `convoy up`) instead."
  - convoy up's respawn primitive WAS `pty restart -y` — the refused command.

So pty deferred to convoy and convoy called back into pty's refusal. Every
permanent agent respawn failed, on every tick, forever. After a HARD death
there is additionally no daemon left to restart onto.

pty's guard is right — blindly re-running stored argv can wedge a
`claude --resume`. convoy was using the wrong primitive. Recovery now REPLAYS
THE MANIFEST: re-read `.convoy/pty.toml` (the launch spec) and cold-boot the
agent's whole limb set from it. That is precisely the supervisor-mediated path
the guard defers to — a fresh cold start, not an argv re-run, so no
conversation is pinned. `--force` was rejected: it reintroduces exactly the
wedge pty guards against.

All limbs are relaunched together and survivors are torn down first. The
manifest pins stable session ids, so spawning over a live sidecar would collide
on that id — and a sidecar reused across a provider death stays bound to a
target that no longer exists.

Also closes a hole the reproduction exposed in the flapping cap: it infers a
fast fail from the leaf's exit record, which is blind when the spawn never
happened. With respawn failing every tick no new exit record was written, so
`exitedAt` stayed EARLIER than `lastRespawnAt`, the interval went negative, and
the counter sat at 0/3 across unbounded cycles — never parking, and never
dinging (the fast-fail ding gates on >= 1). A silent infinite retry.
`classifyFailedAttempt` counts the failed ATTEMPT against the SAME cap, so a
manifest that cannot spawn parks like one that spawns and dies.

Reproduced end-to-end on an isolated network under a short temp root, and the
fix verified live: dead provider restored, stale sidecar replaced, and a
crash-looping agent parks at 3/3 instead of looping.

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

* test(up): move recovery tests to their own file for any-order mergeability

The survivingLimbs suite conflicted with PR #90 for purely textual reasons —
both appended a describe block to the tail of src/up.test.ts and both edited
its import line. No semantic overlap (#90 tests emitWrites, this tests
recovery). Relocating to src/up-recovery.test.ts restores src/up.test.ts to
main's exact content, so this branch is independently mergeable in any order
against both open PRs.

Also emit a line on a failed recovery attempt that is still under the cap.
Silence until the park would have been a step back from the old
"(spawn FAILED)" line, and that is precisely the state convoy#82 showed was
invisible.

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

* fix(up): a dead sidecar must never tear down its live provider (+ test the replay itself)

Three defects found by adversarial review of the manifest-replay recovery, all reproduced
before fixing.

1. REGRESSION — a dead ding sidecar tore down the healthy provider. Ding limbs carry
   strategy=permanent, so a dead sidecar entered the respawn branch, survivingLimbs()
   returned its LIVE PROVIDER, and replayManifest killed survivors before cold-booting —
   destroying the provider's in-progress work to recover its watcher. Worse than the bug
   replay was written to fix, and a breach of the ADOPT-ALIVE invariant. Fixed at both
   levels: survivingLimbs() returns [] for a dead sidecar, and `up` routes a dead sidecar
   beside a LIVE provider to an in-place restart of the sidecar alone (what main did
   correctly; pty's stateful-agent guard refuses role=agent, not role=ding). The routing
   is gated on provider LIVENESS, not on "is a sidecar": when both limbs are gone the
   sidecar still falls through to the single manifest replay, so it is never restarted
   in place into a collision with the pinned id replay is about to re-spawn.

   The PR's own test suite had blessed the symmetric case, so the wrong behaviour was
   asserted correct — the test is fixed too, not just the code.

2. replayManifest, the headline fix, was entirely untested: gutting it to
   `return {spawned:[],failed:[]}` left the suite byte-identical. Its correctness is
   pure effect SEQUENCING, so the two effects are now injectable (ReplayIO, defaulted to
   the real pty operations — no call site changes) and the sequence is asserted: kill
   every survivor BEFORE the spawn, propagate partial results verbatim, turn an
   unreadable manifest into a reported failure rather than an exception escaping into
   the reconcile loop. Gutting the method now fails 5 tests.

3. Partial replay collapsed to success and churned forever. `ok = spawned.length > 0`
   called it a win when the provider threw and only the ding came up — and a spawn racing
   the kill of a pinned id is exactly what throws. The success path ran,
   classifyFailedAttempt never fired, the counter stayed 0 across every tick, and since it
   persists to tags a fresh --once run inherited the same 0. Now an agent is its limbs
   TOGETHER: replaySucceeded() requires a clean sweep, so any failed limb counts as a
   failed attempt and the cap advances and parks.

Known limitation: an agent that parks after a persistently partial replay can leave the
one limb that did spawn running. The ding names the manifest as the thing to fix.

Suite: 300 -> 317 passing; the 4 pre-existing failures (doctor/hooks, launch,
network-config) are unrelated and unchanged. tsc --noEmit clean.

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

* refactor(up): extract planLimbRecovery so the sidecar-vs-replay routing is testable

The sidecar-death fix had two halves and only one was covered. `survivingLimbs`'
sidecar guard was tested, but the routing that sends a dead sidecar to an in-place
restart lived inline in the reconcile tick, which nothing drives — delete that branch
and the whole suite stayed green. Since the routing is exactly where the regression
lived, that is the wrong thing to leave untested.

The branch selection is now a pure `planLimbRecovery(s, sessions, replayed)` returning
restart | covered | replay, and the loop is a thin switch that performs the chosen
effect. Six tests cover all four shapes, including that both-limbs-dead routes the
sidecar to replay rather than an in-place restart that would collide with the pinned id
replay is about to re-spawn. Removing the sidecar-only case now fails the acceptance
test.

No behaviour change — pure extraction. Suite 317 -> 323 passing; same 4 pre-existing
unrelated failures. tsc --noEmit clean.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@schickling-assistant
schickling-assistant merged commit ba4fa7a into main Jul 21, 2026
@schickling-assistant
schickling-assistant deleted the schickling-assistant/2026-07-20-valiant-tesla-7 branch July 21, 2026 00:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant