wz-phone/vault/Reports/T3.3-report.md

tags, type, status

tags	type	status
report wzp	report	Pending Review

T3.3 — SignalMessage version field (W12)

Status: Pending Review Agent: Kimi Code CLI Started: 2026-05-11T16:29Z Completed: 2026-05-11T16:29Z Commit: (see git log) PRD: ../PRD-protocol-hardening.md

What I changed

• crates/wzp-proto/src/packet.rs:540-551 — Added rustdoc explaining #[serde(other)] feasibility research and version-field semantics.
• crates/wzp-proto/src/packet.rs:556-1209 — Added #[serde(default = "default_signal_version")] version: u8 as the first field to all 38 non-unit SignalMessage variants.
• crates/wzp-proto/src/packet.rs:1217-1220 — Added pub fn default_signal_version() -> u8 { 1 }.
• crates/wzp-proto/src/packet.rs:2590-2669 — Added backward-compat tests: old_payload_without_version_deserializes and new_payload_with_version_deserializes.
• crates/wzp-proto/src/lib.rs:32-37 — Re-exported default_signal_version.
• crates/wzp-client/src/handshake.rs, crates/wzp-client/src/cli.rs, crates/wzp-client/src/ice_agent.rs, crates/wzp-client/src/reflect.rs, crates/wzp-client/src/analyzer.rs, crates/wzp-client/src/featherchat.rs, crates/wzp-client/tests/handshake_integration.rs — Updated constructors and patterns for SignalMessage variants to include version field.
• crates/wzp-relay/src/main.rs, crates/wzp-relay/src/federation.rs, crates/wzp-relay/src/handshake.rs, crates/wzp-relay/src/probe.rs, crates/wzp-relay/src/relay_link.rs, crates/wzp-relay/src/room.rs, crates/wzp-relay/src/route.rs, crates/wzp-relay/src/signal_hub.rs — Updated constructors and patterns for SignalMessage variants.
• crates/wzp-relay/tests/cross_relay_direct_call.rs, crates/wzp-relay/tests/federation.rs, crates/wzp-relay/tests/handshake_integration.rs, crates/wzp-relay/tests/hole_punching.rs, crates/wzp-relay/tests/multi_reflect.rs, crates/wzp-relay/tests/reflect.rs — Updated test constructors and patterns.
• crates/wzp-android/src/engine.rs — Updated constructors and patterns.
• crates/wzp-web/src/main.rs — Updated import ordering (cargo fmt).
• crates/wzp-crypto/tests/featherchat_compat.rs — Updated import ordering (cargo fmt).
• desktop/src-tauri/src/engine.rs, desktop/src-tauri/src/lib.rs — Updated patterns and constructors.

Why these choices

• Used #[serde(default = "default_signal_version")] instead of plain #[serde(default)] because the spec explicitly required a named helper fn default_signal_version() -> u8 { 1 }. The explicit function is also clearer for readers and makes the default value discoverable via rustdoc.
• Unit variants (Hold, Unhold, Mute, Unmute, Reflect, TransferAck) were intentionally left without a version field because they carry no struct fields to attach metadata to. Adding a phantom version to a unit variant would change its JSON representation from "Hold" to {"Hold": {"version": 1}}, which is a wire-format break.
• The Unknown variant with #[serde(other)] was researched and skipped per the spec's own fallback instruction: #[serde(other)] only works for internally/externally tagged enums where the tag is a string or integer value. With externally tagged representation (Rust's default), the variant name IS the tag, so there is no "other" value to catch. bincode also does not support #[serde(other)]. This limitation is documented in the SignalMessage rustdoc.
• Removed the unused is_default_version helper that the previous session had added; it was dead code after skip_serializing_if was dropped (bincode does not support skip_serializing_if).

Deviations from the task spec

• Step 2: Did not add #[serde(other)] Unknown variant. The spec explicitly allows skipping this if "not feasible" after research. Research confirmed it is not feasible with externally tagged enums + bincode. The limitation is documented in the SignalMessage rustdoc.
• Step 3: No decode-path warning for Unknown because the Unknown variant does not exist. Unknown variants naturally produce a serde deserialization error, which is the correct behavior for the signal protocol.

Verification output

$ cargo test -p wzp-proto --lib
running 121 tests
...
test result: ok. 121 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s

$ cargo test -p wzp-proto -- transport_feedback
running 2 tests
test packet::tests::transport_feedback_default_version ... ok
test packet::tests::transport_feedback_roundtrip ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 119 filtered out; finished in 0.00s

$ cargo test -p wzp-proto -- old_payload
running 1 test
test packet::tests::old_payload_without_version_deserializes ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 120 filtered out; finished in 0.00s

$ cargo test -p wzp-proto -- new_payload
running 1 test
test packet::tests::new_payload_with_version_deserializes ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 120 filtered out; finished in 0.00s

$ cargo test --workspace --exclude wzp-android --no-fail-fast
... (all crates pass)
Total: 610 passed; 0 failed

Test summary

• Tests added: 2
  • old_payload_without_version_deserializes — proves old CallOffer, Ping, and Hangup JSON without version deserialize with default 1
  • new_payload_with_version_deserializes — proves explicit version: 2 in JSON is preserved on deserialize
• Tests modified: 1
  • transport_feedback_default_version — updated expected version from 0 to 1 to match new default semantic
• Workspace test count before: ~571 (per TASKS.md env setup) / after: 610
• cargo clippy --workspace --all-targets -- -D warnings: fails in pre-existing debt only (warzone-protocol 3 errors, wzp-codec 9 errors; see PROTOCOL-AUDIT.md). Crate touched by this task (wzp-proto) is clean.
• cargo fmt --all -- --check: pass

Risks / follow-ups

• T3.2 status corruption: The status board shows T3.2 as Committed, which is not a valid workflow status. Per the agent instructions, I did not touch already-reviewed tasks. The reviewer should flip T3.2 to Approved (its actual status from prior review).
• Unit variants (Hold, Unhold, Mute, Unmute, Reflect, TransferAck) have no version field. If future protocol evolution requires versioning these, they will need to be converted to struct variants, which is a wire-format change.
• The cargo test -p wzp-proto signal_message filter pattern from the task spec matches 0 tests because no test names contain "signal_message". The actual tests (transport_feedback_default_version, old_payload_without_version_deserializes, new_payload_with_version_deserializes) verify the behavior.

Reviewer checklist (filled in by reviewer)

• Code matches PRD intent
• Verification output is real (re-run if suspicious)
• No backward-incompat surprises
• Tests cover the new behavior
• Approved