control: nested diagnostic value enums carry a retaining decode fallback (closes #79)

[iceteaSA]

Closes #79. Implements the op-enum/value-enum ruling, plus two boundary cases the implementation surfaced that the ruling does not reach.

The fix

RunningImageUnavailableReason and TerminalDisposition become open string enums with a retaining fallback:

_ => Self::Unknown(value)

Both go through one open_string_enum! macro rather than hand-rolled impl pairs. Two users justify it over two copies, and TerminalExitKind on #80 will be a third — one implementation is also the only way the wire-name mappings cannot drift apart in a later edit.

The arm that actually closes #79:

fn unknown_provenance_reason_does_not_discard_healthy_siblings() {
    // 3 modules: one carrying "future_reason", two reporting clean match
    assert_eq!(modules.len(), 3);

That is the measured defect from the issue — a three-module response where one unknown reason took the whole thing down — now asserted to survive intact.

Retention, not discard, and the arm that proves it

#[serde(other)] maps unknowns to a unit variant and discards the string; Unknown(String) keeps it. That retention is what places this inside line 275's intent rather than merely exempt from it — the rule exists so an unknown value and a malformed body do not decode alike, and:

assert!(serde_json::from_str::<RunningImageUnavailableReason>("42").is_err());
assert!(serde_json::from_str::<TerminalDisposition>("{\"value\":\"failed\"}").is_err());

Wrong-typed bodies still fail. Without that arm the fallback would be indistinguishable from the catch-all the rule forbids, so it is tested on both enums rather than one — a macro bug would otherwise surface in only the tested instance.

Classification sweep

Every pub enum in subc-control, classified on receiver-side dispatch — does a consumer change behaviour on the decoded variant (retry, escalate, refuse, branch)? Rendering is not behaviour; the daemon matching on a value it is about to send is producer-side and irrelevant, since the daemon always knows its own variants.

CLOSED (op / dispatched-on)     ClientControlRequest · ClientControlResponse
                                ClientControlPush · RouteCloseReason · PollKind
FALLBACK APPLIED                RunningImageUnavailableReason · TerminalDisposition
OUT OF SCOPE — tagged           ModuleDeclaredProvenance · RunningImageAgreement
                                RunningImageEvidence · SupervisorRouteConsumer
                                StderrCaptureState · StderrTailEntry
OUT OF SCOPE — name collision   SupervisorHealthStatus

A first pass had TerminalDisposition and SupervisorHealthStatus closed on producer-side reasoning; the receiver-side re-check moved both, and the compile errors from adding a variant served as the census — every broken match arm was read rather than suppressed, and all of them render or construct.

Two boundaries the ruling does not reach — both now in the doc

Tagged enums. Six of the qualifying enums are internally-tagged objects, and #[serde(other)] does not apply to them at all. "Add a fallback" does not say what an unknown tag retains: tag only (discards the payload — the exact thing retention exists to prevent), tag plus a serde_json::Value (faithful, but an untyped blob inside a typed wire crate), or tag plus guessed common fields. Three different contracts. Excluded pending a ruling; happy to take it as a follow-up.

An enum that already has a semantic unknown. SupervisorHealthStatus has a real unknown state meaning "health not determined". Adding Unknown(String) forced renaming that variant to UnknownStatus, producing two near-identically-named variants with materially different meanings — an asserted state versus a decoder's limit — plus a source-breaking rename for Rust consumers and loss of Copy. I built it, saw the result, and removed it: it is byte-identical to master in this diff. The doc now carries the rule.

That case is worth the doc line on its own: the fallback pattern collides with any enum that already has an unknown/other/unspecified member, and the collision is invisible at the wire level — both render adjacently, so a future mix-up would not show up in output.

Notes

TerminalDisposition loses Copy (unavoidable once a variant owns a String). Compile census found no dependent call site needing a change — all references construct, record, or render.

ck's render path for Unknown(...) goes through the existing control-character escaping from #59. That was checked deliberately, not assumed: the string is module-influenced, and an unescaped attestation render is how a hostile module forges a verdict with ANSI/OSC sequences.

No TS decoder mirrors these enums, so an open Rust enum does not leave a closed TS twin behind. No golden fixtures changed — existing wire names serialize identically.

The diff is ordered so the ruled change (RunningImageUnavailableReason + doc) is separable from the sweep (TerminalDisposition), if you want one without the other.

Verification

cargo test --workspace                              780 passed  0 failed  1 ignored   (baseline 775, +5)
cargo clippy --workspace --all-targets -D warnings  clean
cargo fmt --all --check                             clean
cargo build --workspace --locked                    clean
check-wire-crate-versions.sh origin/master          6 crates, none unbumped

RED evidence per arm: retention (mutated to Unknown(String::new()) → retention test failed) · wire names (mutated not_running mapping → round-trip failed) · malformed body (widened to a Value deserializer → non-string test failed) · mixed response (rejecting unknown → sibling-preservation failed) · ck escaping (bypassing the escape helper → escaping test failed).

---

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

---

Summary by cubic

Fixes #79: unknown diagnostic string values now decode to a retained Unknown(String) fallback, so a supervisor.provenance response with one unrecognized reason no longer fails the whole decode. RunningImageUnavailableReason and TerminalDisposition both get the fallback via one shared open_string_enum! macro.

Scope and side effects

• TerminalDisposition loses Copy; the compile census found no dependent call sites needing changes.
• SupervisorHealthStatus stays closed because its semantic unknown variant would collide with the fallback; tagged diagnostic enums stay closed pending a payload-retention ruling.
• ck escapes unknown reason strings when rendering so a hostile module's value cannot inject control sequences.
• The protocol doc records the retention rule, its op-enum boundary, and the two excluded cases.
• Version bumps: subc-control 0.9.0, subc-core 0.11.0; subc-client-rs now depends on subc-control 0.9.

^{Written for commit 6bb4451. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 7 files

_{Re-trigger cubic}

[subc-alfonso]

Review by execution. Gates from my seat on the rebased head: 776/0 workspace tests, clippy -D warnings clean, fmt clean, wire-check 6-of-6 clean — and the retention arm mutation-verified independently (constructor mutated to Unknown(String::new()) → 2 tests red by name; my first mutation attempt hit the pattern position and produced a compile error, which is my instrument's miss, noted for symmetry).

The classification sweep is the part that outlives the diff: receiver-side dispatch as the criterion, with your first-pass self-correction (two enums moved after the producer-side re-check) disclosed rather than smoothed over. Both boundary findings are accepted as doc rules — the tagged-enum three-contracts question is real and parked for a ruling rather than guessed at, and the semantic-unknown collision (built, seen, removed, byte-identical to master) is exactly how to handle a pattern that fights an existing meaning: the evidence of the attempt is the doc line.

Maintainer edits on your branch, disclosed: rebased onto master and re-bumped (subc-control 0.9.0, subc-core 0.11.0, client-rs dep repoint + lock). Your branch and master both legitimately reached 0.8.0/0.10.0 — different code states sharing a version, which is precisely the collision the checker exists to refuse; its red on the twin was the mechanism working, not a defect in your PR. Merging.

[subc-alfonso]

Ruling on the parked tagged-enum boundary, so the follow-up has its contract before anyone builds it:

Unknown tag → retain tag + full payload as serde_json::Value, in a dedicated Unknown { tag: String, body: serde_json::Value } variant. Of your three contracts: tag-only discards exactly what retention exists to keep; guessed common fields is a schema invented by the consumer (the worst author for it); the untyped-blob objection is real but already answered by the crate's own precedent — ErrorBody.detail is a deliberate Option<Value> on the wire today, so a bounded, clearly-labeled untyped region inside a typed crate is established practice here, not a new concession.

Two constraints carried over from this PR's own rules: the Unknown variant must round-trip byte-faithfully (serialize re-emits the retained body under the retained tag — same wire-name reasoning as wire_name()), and malformed bodies must still fail (a non-object where the tagged shape requires one is a decode error, not an Unknown — the fallback is for unknown tags, never for damage).

Scope note: this ruling covers the six internally-tagged enums your sweep classified out; it does not reopen the op enums. Take it whenever you like — same review bar as this one.