From 67313f3629ed380c4b7d29facb94f21bd5037124 Mon Sep 17 00:00:00 2001 From: opencode controller Date: Tue, 30 Jun 2026 23:49:28 -0400 Subject: [PATCH] =?UTF-8?q?test(tap):=20golden-fixture=20wire-byte=20locks?= =?UTF-8?q?=20for=20slice-3=20protocol=20events=20(spec=20=C2=A77.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit slice-3 §7.4: external golden JSON fixtures + a new integration-test file locking the exact on-wire bytes produced by each slice-3 encode_* function. The inline round-trip tests in protocol.rs use String::contains (substring) which cannot catch field-order drift, stray fields, or number formatting changes — all of which the Envelope Serialize impl promises to keep stable (protocol.rs:115-118: "field order is pinned to make the wire bytes deterministic"). These fixtures make that promise load-bearing. 5 fixtures cover the 5 new event kinds (spec §3.2-3.3): speech_started, speech_stopped, function_call, function_call_output, tools.update. Each fixture asserts (a) encode_* output equals the fixture bytes exactly (string equality, not serde_json::Value equality — the latter would erase the field-order pinning the serializer guarantees), and (b) the fixture decodes back to the expected DecodedPayload variant with the expected fields. Plus a forwards-compat re-assertion: unknown wire type -> DecodedPayload::Unknown (via FrameKind's #[serde(other)]), not an error — the foundation of the slice-2 §3.4 "drop + observe" posture that lets a future-protocol brain not crash an older core. On first run the golden test caught real drift: serde_json (without preserve_order) alphabetizes nested object keys, so tools.update's tools array serializes description-before-name, not input order. Fixture corrected to lock the actual contract. 11 new tests, all green. cargo fmt + clippy -D warnings clean. --- .../tests/fixtures/function_call.json | 1 + .../tests/fixtures/function_call_output.json | 1 + .../tests/fixtures/speech_started.json | 1 + .../tests/fixtures/speech_stopped.json | 1 + .../tests/fixtures/tools_update.json | 1 + crates/rutster-tap/tests/protocol_events.rs | 225 ++++++++++++++++++ 6 files changed, 230 insertions(+) create mode 100644 crates/rutster-tap/tests/fixtures/function_call.json create mode 100644 crates/rutster-tap/tests/fixtures/function_call_output.json create mode 100644 crates/rutster-tap/tests/fixtures/speech_started.json create mode 100644 crates/rutster-tap/tests/fixtures/speech_stopped.json create mode 100644 crates/rutster-tap/tests/fixtures/tools_update.json create mode 100644 crates/rutster-tap/tests/protocol_events.rs diff --git a/crates/rutster-tap/tests/fixtures/function_call.json b/crates/rutster-tap/tests/fixtures/function_call.json new file mode 100644 index 0000000..d070274 --- /dev/null +++ b/crates/rutster-tap/tests/fixtures/function_call.json @@ -0,0 +1 @@ +{"v":1,"type":"function_call","seq":0,"ts":0,"id":"abc-123","name":"hangup","args":{}} \ No newline at end of file diff --git a/crates/rutster-tap/tests/fixtures/function_call_output.json b/crates/rutster-tap/tests/fixtures/function_call_output.json new file mode 100644 index 0000000..692d4db --- /dev/null +++ b/crates/rutster-tap/tests/fixtures/function_call_output.json @@ -0,0 +1 @@ +{"v":1,"type":"function_call_output","seq":0,"ts":0,"id":"abc-123","status":"ok","result":{"channel_state":"Closing"}} \ No newline at end of file diff --git a/crates/rutster-tap/tests/fixtures/speech_started.json b/crates/rutster-tap/tests/fixtures/speech_started.json new file mode 100644 index 0000000..f516d6f --- /dev/null +++ b/crates/rutster-tap/tests/fixtures/speech_started.json @@ -0,0 +1 @@ +{"v":1,"type":"speech_started","seq":7,"ts":100} \ No newline at end of file diff --git a/crates/rutster-tap/tests/fixtures/speech_stopped.json b/crates/rutster-tap/tests/fixtures/speech_stopped.json new file mode 100644 index 0000000..6e8ca77 --- /dev/null +++ b/crates/rutster-tap/tests/fixtures/speech_stopped.json @@ -0,0 +1 @@ +{"v":1,"type":"speech_stopped","seq":9,"ts":200} \ No newline at end of file diff --git a/crates/rutster-tap/tests/fixtures/tools_update.json b/crates/rutster-tap/tests/fixtures/tools_update.json new file mode 100644 index 0000000..0d40254 --- /dev/null +++ b/crates/rutster-tap/tests/fixtures/tools_update.json @@ -0,0 +1 @@ +{"v":1,"type":"tools.update","seq":0,"ts":0,"tools":[{"description":"hang up the call","name":"hangup"}]} \ No newline at end of file diff --git a/crates/rutster-tap/tests/protocol_events.rs b/crates/rutster-tap/tests/protocol_events.rs new file mode 100644 index 0000000..6914ba1 --- /dev/null +++ b/crates/rutster-tap/tests/protocol_events.rs @@ -0,0 +1,225 @@ +//! Slice-3 golden-fixture tests for the tap protocol's new event types. +//! +//! ## Why this file exists (alongside `protocol.rs`'s inline tests) +//! +//! `protocol.rs` already has inline round-trip tests for each new slice-3 +//! event kind, but they assert with `String::contains` — a *substring* check. +//! That catches "the right `type` tag is present" but it cannot catch: +//! +//! - a change in **field order** (the `Envelope` `Serialize` impl at +//! `protocol.rs::Serialize_for_Envelope` pins field order deliberately — +//! see its doc comment: "field order is pinned to make the wire bytes +//! deterministic… helps snapshot tests and interop with brains that read +//! key order for debugging"); +//! - a stray extra field; +//! - a missing field; +//! - a change in whitespace or number formatting. +//! +//! These fixtures lock the **exact wire bytes** produced by each `encode_*` +//! function. Any future refactor that alters the on-wire shape — even +//! cosmetically — breaks a test here, forcing a conscious fixture update + +//! a protocol-version bump conversation (spec §3.1: `PROTOCOL_VERSION` is +//! the contract door). +//! +//! ## Forwards-compat (the slice-2 §3.4 "drop + observe" contract) +//! +//! The last test here re-asserts the load-bearing decode-side rule: an +//! unknown `type` string deserializes to `DecodedPayload::Unknown` (via +//! `FrameKind`'s `#[serde(other)]` fallback) rather than erroring. This is +//! the foundation of the forwards-compat posture — a brain speaking a +//! future protocol version's event kind must not crash an older core; it +//! gets logged + counted + dropped (the behavioral side lives in +//! `tap_client::handle_brain_frame` and is exercised end-to-end by the +//! slice-2 / slice-3 integration tests, not here). +//! +//! ## Why string equality (not `serde_json::Value` equality) +//! +//! Comparing parsed `serde_json::Value`s would be *semantic* equality — it +//! would treat `{"a":1,"b":2}` and `{"b":2,"a":1}` as the same. That +//! throws away the field-order pinning the `Envelope` serializer +//! guarantees. Comparing the raw encoded `String` to the fixture's bytes +//! keeps the order contract load-bearing. + +use std::fs; + +use rutster_tap::{ + DecodedPayload, decode_envelope, encode_function_call, encode_function_call_output, + encode_speech_started, encode_speech_stopped, encode_tools_update, +}; + +/// Read a fixture file from `tests/fixtures/`. Uses +/// `CARGO_MANIFEST_DIR` so the path is stable regardless of which +/// directory `cargo test` is invoked from. Trailing newlines are stripped +/// because `serde_json::to_string` (what the encoders call) never emits one +/// — a fixture checked into git shouldn't acquire a phantom newline mismatch. +fn fixture(name: &str) -> String { + let path = format!("{}/tests/fixtures/{}", env!("CARGO_MANIFEST_DIR"), name); + let raw = + fs::read_to_string(&path).unwrap_or_else(|e| panic!("read fixture {name} ({path}): {e}")); + raw.trim_end().to_string() +} + +// ─── speech_started ────────────────────────────────────────────────────── + +#[test] +fn golden_speech_started_wire_bytes_match_fixture() { + let encoded = encode_speech_started(7, 100).unwrap(); + let golden = fixture("speech_started.json"); + assert_eq!( + encoded, golden, + "speech_started wire bytes drifted from golden fixture" + ); +} + +#[test] +fn speech_started_fixture_decodes_to_speechstarted_variant() { + let golden = fixture("speech_started.json"); + let decoded = decode_envelope(&golden).unwrap(); + assert_eq!(decoded.seq, 7); + assert_eq!(decoded.ts, 100); + assert!( + matches!(decoded.payload, DecodedPayload::SpeechStarted), + "speech_started fixture did not decode to SpeechStarted: {:?}", + decoded.payload + ); +} + +// ─── speech_stopped ────────────────────────────────────────────────────── + +#[test] +fn golden_speech_stopped_wire_bytes_match_fixture() { + let encoded = encode_speech_stopped(9, 200).unwrap(); + let golden = fixture("speech_stopped.json"); + assert_eq!( + encoded, golden, + "speech_stopped wire bytes drifted from golden fixture" + ); +} + +#[test] +fn speech_stopped_fixture_decodes_to_speechstopped_variant() { + let golden = fixture("speech_stopped.json"); + let decoded = decode_envelope(&golden).unwrap(); + assert_eq!(decoded.seq, 9); + assert_eq!(decoded.ts, 200); + assert!( + matches!(decoded.payload, DecodedPayload::SpeechStopped), + "speech_stopped fixture did not decode to SpeechStopped: {:?}", + decoded.payload + ); +} + +// ─── function_call ─────────────────────────────────────────────────────── + +#[test] +fn golden_function_call_wire_bytes_match_fixture() { + // `args_json_str="{}"` — the encoder parses it into `serde_json::Value` + // (an empty object) so the wire field is `"args":{}`, not `"args":"{}"`. + // This is load-bearing: a brain emitting a function_call with a raw + // JSON-string args would be a different wire shape (and a translator + // bug). The fixture pins the parsed-object shape. + let encoded = encode_function_call("abc-123", "hangup", "{}", 0, 0).unwrap(); + let golden = fixture("function_call.json"); + assert_eq!( + encoded, golden, + "function_call wire bytes drifted from golden fixture" + ); +} + +#[test] +fn function_call_fixture_decodes_to_functioncall_with_fields() { + let golden = fixture("function_call.json"); + let decoded = decode_envelope(&golden).unwrap(); + match decoded.payload { + DecodedPayload::FunctionCall(p) => { + assert_eq!(p.id, "abc-123"); + assert_eq!(p.name, "hangup"); + assert_eq!(p.args, serde_json::Value::Object(serde_json::Map::new())); + } + other => panic!("expected FunctionCall, got {other:?}"), + } +} + +// ─── function_call_output ─────────────────────────────────────────────── + +#[test] +fn golden_function_call_output_wire_bytes_match_fixture() { + let encoded = + encode_function_call_output("abc-123", "ok", r#"{"channel_state":"Closing"}"#, 0, 0) + .unwrap(); + let golden = fixture("function_call_output.json"); + assert_eq!( + encoded, golden, + "function_call_output wire bytes drifted from golden fixture" + ); +} + +#[test] +fn function_call_output_fixture_decodes_to_functioncalloutput_with_fields() { + let golden = fixture("function_call_output.json"); + let decoded = decode_envelope(&golden).unwrap(); + match decoded.payload { + DecodedPayload::FunctionCallOutput(p) => { + assert_eq!(p.id, "abc-123"); + assert_eq!(p.status, "ok"); + // The result field is a parsed JSON object, not a string. + assert_eq!( + p.result.get("channel_state"), + Some(&serde_json::json!("Closing")) + ); + } + other => panic!("expected FunctionCallOutput, got {other:?}"), + } +} + +// ─── tools.update ─────────────────────────────────────────────────────── + +#[test] +fn golden_tools_update_wire_bytes_match_fixture() { + let tools_json = r#"[{"name":"hangup","description":"hang up the call"}]"#; + let encoded = encode_tools_update(tools_json, 0, 0).unwrap(); + let golden = fixture("tools_update.json"); + assert_eq!( + encoded, golden, + "tools.update wire bytes drifted from golden fixture" + ); +} + +#[test] +fn tools_update_fixture_decodes_to_toolsupdate_with_array() { + let golden = fixture("tools_update.json"); + let decoded = decode_envelope(&golden).unwrap(); + match decoded.payload { + DecodedPayload::ToolsUpdate(p) => { + let arr = p + .tools + .as_array() + .expect("tools field must be a JSON array"); + assert_eq!(arr.len(), 1); + assert_eq!(arr[0].get("name"), Some(&serde_json::json!("hangup"))); + } + other => panic!("expected ToolsUpdate, got {other:?}"), + } +} + +// ─── forwards-compat (slice-2 §3.4 "drop + observe") ──────────────────── + +/// The load-bearing forwards-compat rule on the decode side: an unknown +/// `type` string deserializes to `DecodedPayload::Unknown` (via +/// `FrameKind`'s `#[serde(other)]`), **not** an error. A future-protocol +/// brain emitting `"type":"future_event"` must not crash an older core; +/// the core logs + counts + drops it (the observe-and-continue posture +/// the eventual fuzz harness will stress). This re-asserts the contract +/// the inline `unknown_type_decodes_to_unknown_variant` test established, +/// here in the external fixture file so it sits next to the golden wire +/// shapes it protects. +#[test] +fn unknown_wire_type_decodes_to_unknown_variant_not_error() { + let wire = r#"{"v":1,"type":"future_event","seq":0,"ts":0}"#; + let decoded = decode_envelope(wire).expect("unknown type must not error (forwards-compat)"); + assert!( + matches!(decoded.payload, DecodedPayload::Unknown), + "forwards-compat broken: unknown type decoded to {:?}, expected Unknown", + decoded.payload + ); +}