supervisor: bounded per-module terminal ring so a missed exit is countable

[iceteaSA]

Closes #12. Built to the shape you specified: timestamped exit records held across respawns, served by their own op, with an eviction count so a returning observer knows its backfill is a suffix.

What landed

• supervisor.terminals — its own op, not inflating supervisor.list. Same size-discipline argument as supervisor.stderr_tail.
• TerminalDisposition — Stopped / Disabled / Failed / Restarting, recorded at the point the supervisor chooses it rather than inferred afterwards.
• dropped on eviction — the field that turns "can't count what I missed" into a number.
• last_exit_ms on SupervisorEntry regardless, so every existing reader gets ordering without adopting the new op.
• ck module terminals <id>, rendering the dropped count.
• In-memory, lost on daemon restart. That boundary holds — durable history stays with the observer layer per Lifecycle event seam + bounded post-mortem surface for supervised work (companion to #1/#2) #4.

drain_child_to_state also observes terminals

I briefed this at on_child_exit and that was incomplete. drain_child_to_state reaches a terminal on five more paths — restart, reload, health restart, stop, disable — and none of them were in my scope. They record their disposition now too.

Worth stating because it changes what the feature is: had it shipped covering only on_child_exit, the ring would have held crashes while a reload or an operator stop left no record. An observer would then read "no terminals" for a module that had been deliberately cycled, which is the same absence-reads-as-health defect this issue exists to fix. Credit to the implementer for checking rather than taking my list.

The daemon-incarnation signal

The response carries daemon_started_at_ms. An empty ring immediately after daemon start otherwise reads as "nothing has died" when it means "I cannot tell you what died" — reproducing the defect one layer out. With it, an observer can tell a fresh daemon from a quiet one and re-baseline.

Today's OOM is the case that motivated it: three modules died, the successor cold-started, and nothing on the wire said an epoch had passed.

No shared ring core with StderrRing

You asked whether the two rings should share a bounded-ring core. They shouldn't. Of five dimensions, four don't transfer:

	stderr ring	terminal ring
bounded deque + dropped	yes	yes
total byte cap	load-bearing (unbounded line length)	meaningless
per-line truncation	load-bearing	meaningless
three-state capture	load-bearing	meaningless
in-band boundary marker	needed	the record is the boundary

Strip those and the shared core is a VecDeque with a length cap and a counter — about fifteen lines. The specific risk is that StderrRing's three-state capture enum gets flattened by a consumer that doesn't need it, and collapsing three world-states into two was the original defect in this area. stderr_tail.rs is untouched here.

The golden fixture is generated through the real handler

It spawns a module, crashes it twice, waits for two terminals, builds a real frame, and runs handle_control_frame — the serialized response is the fixture. Not hand-constructed, for the reason that cost a review round on #10: a fixture built from the same misconception as the implementation goes green while the implementation is wrong.

git diff --stat origin/master -- crates/subc-control/tests/golden/
 catalog_list.json                    | 3 ++-   op string
 server_describe.json                 | 3 ++-   op string
 server_describe_with_counters.json   | 3 ++-   op string
 supervisor_list.json                 | 1 +     last_exit_ms
 supervisor_entry.json                | 1 +     last_exit_ms
 supervisor_terminals.json            | 17 +++  new

Every delta attributable to exactly one cause. Nothing else moved.

Two process notes

I re-introduced the /bin/sh defect and caught it before you did. Two new test spawns used PathBuf::from("/bin/sh") — the exact thing that blocked #10 on Windows. I fixed that PR by extending fake-aft-stub, then wrote this brief without mentioning it. Both now use the stub with FAKE_AFT_EXIT_CODE; neither test needed shell behaviour beyond an exit code. git diff origin/master -- crates/ | grep '^+' | grep /bin/sh returns nothing.

Versions bumped — subc-control 0.2.0, subc-core 0.4.0, Cargo.lock committed. I ran check-wire-crate-versions.sh myself this time rather than discovering it in review.

Gates

cargo test --workspace              31 binaries, 0 failures
cargo fmt --check                   clean
cargo clippy --all-targets          clean
cargo check --workspace --locked    clean
check-wire-crate-versions.sh        6 examined, 0 violations

Both process-spawning tests pass 10/10 in a loop. Windows I can't verify — same ask as #10, whenever the twin is convenient.

---

^{Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.}

---

Summary by cubic

Adds a bounded per-module terminal history and exposes it via a new supervisor.terminals op so observers can see and count missed exits. Previously only the latest exit was visible and operator-driven cycles could leave no record; now every terminal exit records its timestamp and the supervisor’s chosen disposition.

• API: adds supervisor.terminals with daemon_started_at_ms, ordered entries, and a dropped count for evictions; supervisor.list now includes last_exit_ms for ordering without adopting the new op.
• Coverage: records terminals on crash and on restart, reload, health restart, stop, and disable paths, not just on_child_exit.
• CLI: adds ck module terminals <id> (JSON or human-readable), showing retained count and dropped.
• Wire/version: bumps subc-control to 0.2.0 and subc-core to 0.4.0; subc-client-rs now depends on subc-control 0.2.
• Migration: observers needing exit history should call supervisor.terminals; existing readers can use last_exit_ms immediately.
• Persistence: terminal history is in-memory and resets on daemon restart; use daemon_started_at_ms to distinguish a fresh daemon from a quiet one.

^{Written for commit 542fee0. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 18 files

<sub>You're on the cubic free plan with 10 free PR reviews remaining this month. Upgrade for unlimited reviews.

Re-trigger cubic</sub>

[ualtinok]

Reviewed in full — the ring, the recording sites, the op, the CLI verb, and the closure/golden updates. The shape is exactly the #12 agreement and better in two places worth naming:

1. Module-owned, not child-owned — the doc comment gives the load-bearing reason (a replacement process retains the exits that caused it to exist), and it's the same argument that placed the stderr ring. Consistent doctrine.
2. The read path bypasses the command channel — terminal_history() reads the Arc'd ring directly rather than round-tripping through SupervisorCommand. Hours before you pushed this, we had a production incident (aft P1) where the supervision task died and every command-channel op returned CommandClosed while the operator most needed exit history. Under that exact wedge, supervisor.terminals would still have served. That property is accidental in your PR and should be deliberate: add one sentence to the op's doc comment stating that reads must stay off the command channel so history survives supervision-task death — that's now a proven failure mode, not a hypothetical.

Also right: disposition recorded at the choice point (not inferred), dropped as a saturating count, zero-size clamped to one with the lying-by-omission reason stated, poison-recovered locks matching house style, last_exit_ms on SupervisorEntry so existing readers get ordering for free, and the closure test carrying the new op.

Merge order and one version-cascade consequence: #21 (already revised, twin running) merges first. This PR bumps subc-control to 0.2.0 from master's 0.1.1 — after #21 lands you'll need to rebase and re-resolve the version (whatever #21 leaves subc-control at, yours goes one above it, and Cargo.lock regenerates). Same class as your four-crate cascade on #21 — flagging so the second rebase is a mechanical fixup rather than a surprise refusal from the version-check CI.

Twin for the full matrix is already running as PR #25 (same flow as #10/#18) — Windows verdict comes from there. On twin green post-rebase, this merges and closes #12.

[ualtinok]

Twin (PR #25) ran green on the full matrix at 542fee0 — Windows included, so your same-ask-as-#10 is answered: both process-spawning tests pass there. #21 is now merged, so this is ready for the rebase + subc-control version re-resolution from my review note; push it and I re-fire the twin on the new head, then merge on green.

[iceteaSA]

Rebased onto merged master, version cascade re-resolved, your doc-comment ask added — and the rebase surfaced a latent trap I shipped in #10 that is on master right now. Head is d0f9e6b; re-fire the twin whenever convenient.

Rebase

Three conflicts, all version-cascade, all mechanical as you predicted:

• subc-core/Cargo.toml and Cargo.lock — mine (0.4.0) over master's 0.3.1
• subc-client-rs/Cargo.toml — a genuine merge rather than a side pick: subc-control = "0.2" from mine, subc-protocol = "0.11" from manifest: retire the scheduled-task vocabulary #21

subc-control landed on 0.2.0 against master's 0.1.1, so no re-resolution was needed there after all.

Verified the rebase preserved the patch rather than assuming it — diffed the pre-rebase patch against the post-rebase patch:

before:  18 files changed, 559 insertions(+), 15 deletions(-)
after:   18 files changed, 559 insertions(+), 15 deletions(-)
only differences:  -version = "0.3.0" → -version = "0.3.1"
                   subc-protocol "0.10" → "0.11"   (context line)

Your doc-comment ask, taken with the reason stated

Added to the op's doc comment. I wrote it as a requirement rather than a note, since your aft P1 makes it one:

The read MUST stay off the supervisor command channel — it reads the module's shared ring directly. This is a requirement, not an optimisation: when the supervision task itself dies, every command-channel op returns CommandClosed, and that is precisely the moment an operator needs the exit history most. A history reachable only through the machinery whose death you are diagnosing is unreachable when it matters.

That the property was accidental in my PR and load-bearing in your incident is the useful part — worth it being enforced by a comment someone has to argue with before changing.

The finding: --lib alone leaves fake-aft-stub unbuilt

Running gates post-rebase, cargo test -p subc-core --lib failed. Baselined on clean master before forming a hypothesis, and it reproduces there — 10/10 deterministic, in a test from #10:

control::tests::supervisor_stderr_tail_converts_a_real_truncated_ring_entry_to_prefix_only_wire_data
panicked: Spawn { program: ".../target/debug/fake-aft-stub", source: Os { code: 2, kind: NotFound } }

Cause: cargo test -p subc-core builds [[bin]] targets so the stub exists (153 passed, 0 failed); cargo test -p subc-core --lib builds only the library test and leaves it unbuilt. The doc comment I shipped on fake_aft_stub_path() asserted the opposite —

Cargo still builds every [[bin]] target before running the library's tests, so the binary is on disk

— which is true for the first invocation and false for the second. A comment stating something the adjacent behaviour contradicts.

CI never sees it (full cargo test), your twin never saw it, my gates never saw it. It only bites someone running --lib on a tree where the stub was never built — which is a normal thing to do, and the raw NotFound gives no hint that a binary needs building.

Fixed at the single source, both copies of the helper: assert existence with an actionable message.

fake-aft-stub not built at <path>: run `cargo test -p subc-core` (which builds [[bin]]
targets) rather than `cargo test -p subc-core --lib` (which does not)

Deliberately a panic, not a skip-if-absent. A test that quietly passes when it could not run reports health it never verified — which is the defect class this whole arc has been about, and I would rather not introduce it into the fix for it.

My branch had inherited and extended the trap: three affected tests, one from #10 and two new here. Corrected the false doc comment too.

Gates at d0f9e6b

cargo test --workspace              31 binaries, 0 failures
cargo fmt --check                   clean
check-wire-crate-versions.sh        6 examined, 0 violations

One thing I have not chased: bootstrap::tests::held_start_lock_blocks_second_acquire_until_release failed once under full-workspace parallel load and then passed 10/10 in isolation. Unrelated file, not touched by this branch. Flagging rather than filing — one occurrence is not a rate, and I would rather report it honestly than either ignore it or dress it up as a finding.

[ualtinok]

One more rebase needed, and the cause this time is my side moving under you — with a mechanism note worth having since it cost me three wrong retrigger probes tonight:

**Your rebase at d0f9e6b was correct when you pushed it; master then gained the ErrorBody.detail wire change (c100618d), which bumped subc-control 0.1.1 → 0.1.2 and subc-protocol to 0.12.0 (plus subc-client-rs 0.3.1, subc-transport 0.5.1). Your branch's version cascade now conflicts again: your subc-control 0.2.0 stands (it supersedes 0.1.2 — keep it), but the Cargo.toml/Cargo.lock conflict resolution and the protocol repin need a second pass. Same mechanical shape as the first rebase.

The mechanism note: while your PR is in the conflicted state, GitHub Actions creates NO pull_request run at all — no merge ref exists, so there is no suite, no "skipped" run, nothing to find. I spent three probes (reopen, empty commit, fresh twin) reading it as an event-delivery failure before checking mergeable: CONFLICTING. If your CI checks ever seem to silently not fire on a PR here: check mergeability first.

Also heads-up for the rebase: ErrorBody gained a detail: Option<serde_json::Value> field — if your branch constructs any ErrorBody literals, they need detail: None or the new ErrorBody::new(code, message) constructor (E0063 otherwise). Your terminal-ring code likely doesn't touch it, but the compiler will say.

On your --lib/fake-aft-stub finding from the last push: verified against clean master, reproduces exactly as you describe, and your fix shape (panic with the actionable message, deliberately not skip-if-absent — "a test that quietly passes when it could not run reports health it never verified") is precisely this repo's doctrine; the comment-contradicting-adjacent-behavior catch is a class we track. Twin re-fires on your new head; merge on green.

[iceteaSA]

Rebased onto 81dc691. Head is 4b46948, and I checked the thing your mechanism note says to check first:

$ gh api repos/cortexkit/subconscious/pulls/23 --jq '.mergeable, .mergeable_state'
true
clean

So the twin has a merge ref to run against this time. Fire when convenient.

Resolution

Same three conflicts, same shape. subc-control stayed at my 0.2.0 — it supersedes master's 0.1.2, as you said. subc-protocol repinned 0.11 → 0.12 in subc-client-rs/Cargo.toml, which was the only genuine merge again (my subc-control = "0.2" on one line, your subc-protocol = "0.12" on the next).

ErrorBody was a non-event here — my diff constructs none, so no E0063. I checked before rebasing rather than finding out from the compiler:

$ git diff origin/master...HEAD -- crates/ | grep '^+' | grep -c ErrorBody
0

Patch preserved, verified the same way as last time rather than assumed:

before:  18 files changed, 597 insertions(+), 20 deletions(-)
after:   18 files changed, 597 insertions(+), 20 deletions(-)
only differences:  -version = "0.1.1" → "0.1.2"
                   subc-protocol "0.11" → "0.12"   (context line)

Gates at 4b46948: 35 test binaries (up from 31 — master's adapter slice landed), 0 failures, fmt clean, --locked clean, version gate 6/0.

On the mechanism note — worth pinning somewhere durable

while your PR is in the conflicted state, GitHub Actions creates NO pull_request run at all — no merge ref exists, so there is no suite, no "skipped" run, nothing to find

That one cost you three probes and it's exactly the shape this repo keeps turning up: an absence that reads as a failure of the thing you're looking at. A missing run looks like broken event delivery, the same way an empty terminal ring looks like "nothing died" and a green --lib suite looked like a passing stub test. In each case the honest signal was one level out from where the search was pointed.

The discriminator you found — check mergeable before investigating CI — is the cheap one, same as git status before diagnosing a build. If docs/hunting-loop-briefing.md is where that class lives, it's worth a line; I'd write it as no run at all is a state, not a missing state.

Thanks for verifying the --lib finding against clean master rather than taking my word — that's the second time this week a claim of mine got independently reproduced before being acted on, and both times it was the right call.