control: tagged diagnostic enums retain unknown tag and body

Merged
IC
2mo ago
+950-35

Implements the tagged-enum contract ruled on #81, closing the boundary parked when that PR landed the string-enum half.

The contract, as ruled

Unknown { tag: String, body: serde_json::Value } — retain both. Tag-only would discard exactly what retention exists to keep; guessed common fields would be a schema invented by the consumer. The untyped-region objection was mine and the maintainer answered it from the crate's own precedent: ErrorBody.detail is already a deliberate Option<Value> on this wire, so a bounded labeled untyped region is established practice here rather than a new concession.

Six internally-tagged diagnostic enums, all nested in list responses:

ModuleDeclaredProvenance   RunningImageAgreement   RunningImageEvidence
SupervisorRouteConsumer    StderrCaptureState      StderrTailEntry

Classification re-verified by the receiver-side test — does a receiver change behaviour on the decoded variant? Rendering and storing are not behaviour. No receiver dispatches on any of the six; none stays closed. Op enums are untouched.

The two constraints, both load-bearing

Byte-faithful round-trip is an exact text assertion, not a structural one:

let wire = r#"{"kind":"future_consumer","detail":{"z":1}}"#;
let decoded: SupervisorRouteConsumer = serde_json::from_str(wire).unwrap();
assert_eq!(serde_json::to_string(&decoded).unwrap(), wire);

Field order and the nested object survive. RED-proved by omitting the retained body: the assertion fails tag-only against expected payload.

Malformed still fails — this is the arm that keeps the change inside the no-catch-all rule rather than exempt from it. A fallback that swallows a number or an array is the catch-all the rule forbids:

for wire in ["42", r#""future""#, "[]"] {
    assert!(serde_json::from_str::<SupervisorRouteConsumer>(wire).is_err());
    assert!(serde_json::from_str::<StderrCaptureState>(wire).is_err());
}

Two enums, so a per-enum mistake cannot hide. RED-proved by temporarily accepting non-objects.

Security: a wider surface than the string half

A Value body can carry arbitrary strings, so this is a bigger attack surface than #81's string enums. Verified rather than assumed, with real payloads — OSC 52 clipboard-write and screen-clear:

"status": "future_status\u{1b}]52;c;AAAA\u{07}",
"detail": "future detail\u{1b}[2J"
...
assert!(!rendered.bytes().any(|byte| byte < 0x20));

That final assertion is a universal claim over the whole rendered string, not a spot check for the escaped form. Both tag and body route through the existing provenance_value escaping added in #59.

Shape

Six hand-written Serialize/Deserialize impls with private derive-wire enums, not a macro. Tagged variants carry distinct fields, so a generic macro would need real token parsing and could silently drop or reorder them — six auditable impls beat one clever macro. (The string half's open_string_enum! remains the right call for its uniform shape.)

Verification

tests    792 passed, 0 failed, 1 ignored   (baseline 787 on eb49bd5b, +5)
clippy   clean -D warnings
fmt      clean
build    --locked clean
wire     6 crates examined, none changed without a bump

subc-control 0.10.0, subc-client-rs 0.11.0, subc-core 0.12.0, lock updated in the same commit. No golden fixtures changed — known wire shapes are byte-identical. No TypeScript mirror exists for these six.

Deploy note

Not urgent to land. A module restart wave is in progress on this box and hub's constraint set holds wire changes until it settles — this PR is subject to that like anything else. Review at leisure.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Makes the six diagnostic tagged enums in subc-control deserialize unknown tags into Unknown { tag, body } instead of erroring, so clients tolerate future discriminator values without discarding the payload. Known tags still round-trip byte-identically; non-object payloads still fail.

  • Replaces derived impls with hand-written Serialize/Deserialize over private wire enums; the affected types drop Eq and stay PartialEq.
  • Retains the body as recursively ordered JSON pairs taken directly from the wire, so byte-faithful round-trip holds even when an unknown value is nested inside a known ancestor (an unknown RunningImageEvidence inside a known RunningImageAgreement); serde_json preserve_order stays off.
  • Rejects duplicate discriminator fields as decode errors instead of reaching an unreachable arm.
  • ck.rs renders unknown tags with both tag and body escaped, so embedded control characters can't reach the terminal.
  • Documents the object-retention fallback rule in docs/subc-control-protocol.md.
  • Bumps subc-control to 0.11.0 and updates the subc-client-rs dependency.