control: nested diagnostic value enums carry a retaining decode fallback (closes #79)
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
TerminalDispositionlosesCopy; the compile census found no dependent call sites needing changes.SupervisorHealthStatusstays closed because its semanticunknownvariant would collide with the fallback; tagged diagnostic enums stay closed pending a payload-retention ruling.ckescapes 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-control0.9.0,subc-core0.11.0;subc-client-rsnow depends onsubc-control0.9.