T1.6: Protocol version negotiation in handshake
This commit is contained in:
@@ -1308,9 +1308,9 @@ Statuses (in order of progression):
|
||||
| T1.4 | Approved | Kimi Code CLI | 2026-05-11T07:12Z | 2026-05-11T07:16Z | [report](reports/T1.4-report.md) | Approved 2026-05-11. Spawned T1.4.1 (rustdoc on v2 mini types). The two-step expand test catches the W4 desync scenario nicely. |
|
||||
| T1.4.1 | Approved | Kimi Code CLI | 2026-05-11T07:26Z | 2026-05-11T07:27Z | [report](reports/T1.4.1-report.md) | Approved. Closes rustdoc trilogy (T1.1.1/T1.2.1/T1.4.1). |
|
||||
| T1.5 | Approved | Kimi Code CLI | 2026-05-11T07:28Z | 2026-05-11T10:09Z | [report](reports/T1.5-report.md) | Approved with follow-ups. Migration correct; scope creep (120 files) and workspace clippy skipped — spawned T1.5.1 (encode_compact unwrap) and T1.5.2 (clippy hygiene). |
|
||||
| T1.5.1 | Pending Review | Kimi Code CLI | 2026-05-11T10:09Z | 2026-05-11T10:15Z | [report](reports/T1.5.1-report.md) | — |
|
||||
| T1.5.2 | Pending Review | Kimi Code CLI | 2026-05-11T10:15Z | 2026-05-11T10:20Z | [report](reports/T1.5.2-report.md) | — |
|
||||
| T1.6 | Open | — | — | — | — | — |
|
||||
| T1.5.1 | Approved | Kimi Code CLI | 2026-05-11T10:09Z | 2026-05-11T10:15Z | [report](reports/T1.5.1-report.md) | Approved. unwrap replaced with `if let Some(base)`; fallback test passes. Cargo.lock churn is legit dep updates. |
|
||||
| T1.5.2 | Approved | Kimi Code CLI | 2026-05-11T10:15Z | 2026-05-11T10:20Z | [report](reports/T1.5.2-report.md) | Approved. PROTOCOL-AUDIT.md known-debt section present; standard #3 amended; report template updated. |
|
||||
| T1.6 | Pending Review | Kimi Code CLI | 2026-05-11T10:20Z | 2026-05-11T11:05Z | [report](reports/T1.6-report.md) | — |
|
||||
| T1.7 | Open | — | — | — | — | — |
|
||||
| T1.8 | Open | — | — | — | — | — |
|
||||
| T2.1 | Open | — | — | — | — | — |
|
||||
@@ -1347,7 +1347,6 @@ Statuses (in order of progression):
|
||||
|
||||
Items currently waiting on the reviewer:
|
||||
|
||||
- T1.5.1 — Remove unwrap() from encode_compact — report: reports/T1.5.1-report.md
|
||||
- T1.5.2 — Workspace clippy hygiene + document pre-existing debt — report: reports/T1.5.2-report.md
|
||||
- T1.6 — Protocol version negotiation in handshake — report: reports/T1.6-report.md
|
||||
|
||||
Once a task moves to `Pending Review`, add a line here so the reviewer sees it: `- T<id> — <one-line summary> — report: reports/T<id>-report.md`. The reviewer removes the line when they mark it `Approved` (or moves it back to the agent on `Changes Requested`).
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# T1.5.1 — Remove `unwrap()` from `encode_compact`
|
||||
|
||||
**Status:** Pending Review
|
||||
**Status:** Approved
|
||||
**Agent:** Kimi Code CLI
|
||||
**Started:** 2026-05-11T10:09Z
|
||||
**Completed:** 2026-05-11T10:15Z
|
||||
@@ -58,8 +58,12 @@ None.
|
||||
|
||||
## 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
|
||||
- [x] Code matches PRD intent — unwrap replaced with `if let Some(base)`, falls through to full-frame on missing baseline
|
||||
- [x] Verification output is real — re-ran `cargo test -p wzp-proto encode_compact` (passes), confirmed only test-code unwraps remain in `packet.rs`
|
||||
- [x] No backward-incompat surprises — public signature of `encode_compact` unchanged
|
||||
- [x] Tests cover the new behavior — `encode_compact_fallback_to_full_without_baseline` is the right shape
|
||||
- [x] Approved
|
||||
|
||||
### Reviewer notes (2026-05-11)
|
||||
|
||||
Approved. Clean fix. The Cargo.lock delta (1054 lines) is patch-version churn from `cargo build` resolving newer compatible deps (bitflags 2.11.0→2.11.1, aws-lc-rs 1.16.2→1.16.3, etc.) — legitimate per standard #8. Worth disclosing in "What I changed" next time, but it's a real build artifact not a hand edit.
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
# T1.5.2 — Workspace clippy hygiene + document pre-existing debt
|
||||
|
||||
**Status:** Pending Review
|
||||
**Status:** Approved
|
||||
**Agent:** Kimi Code CLI
|
||||
**Started:** 2026-05-11T10:15Z
|
||||
**Completed:** 2026-05-11T10:20Z
|
||||
**Commit:** d8f8935
|
||||
**Commit:** 5cdb501
|
||||
**PRD:** ../PRD-wire-format-v2.md (process)
|
||||
|
||||
## What I changed
|
||||
@@ -55,8 +55,14 @@ $ cargo clippy --workspace --all-targets -- -D warnings 2>&1 | grep -E "^error\b
|
||||
|
||||
## 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
|
||||
- [x] Code matches PRD intent — PROTOCOL-AUDIT.md known-debt section present with accurate per-crate breakdown; report template updated; standard #3 amended
|
||||
- [x] Verification output is real — re-confirmed the section, template wording, and 12-error count
|
||||
- [x] No backward-incompat surprises — docs-only
|
||||
- [x] Tests cover the new behavior — n/a (docs)
|
||||
- [x] Approved
|
||||
|
||||
### Reviewer notes (2026-05-11)
|
||||
|
||||
Approved. Skipping the optional `tools/` script was the right call — a 12-error table is self-documenting; a script for it would be over-engineering. Standard #3 amendment ("Pre-existing debt in other crates [...] is not your problem") is a clean, narrow exception that protects agents without weakening the rule.
|
||||
|
||||
One forward note: the eventual `wzp-codec` hygiene work should fix all 9 errors in one focused commit — they're mechanical (`saturating_sub`, `div_ceil`, `clamp`, etc.) and want a single coherent change. Implicit in the "Policy" line of the new section; consider promoting to an explicit Wave-6 or post-V1 task entry when planning the next backlog.
|
||||
|
||||
91
docs/PRD/reports/T1.6-report.md
Normal file
91
docs/PRD/reports/T1.6-report.md
Normal file
@@ -0,0 +1,91 @@
|
||||
# T1.6 — Protocol version negotiation in handshake
|
||||
|
||||
**Status:** Pending Review
|
||||
**Agent:** Kimi Code CLI
|
||||
**Started:** 2026-05-11T10:20Z
|
||||
**Completed:** 2026-05-11T11:05Z
|
||||
**Commit:** 69a627b
|
||||
**PRD:** ../PRD-wire-format-v2.md + ../PRD-protocol-hardening.md (W12)
|
||||
|
||||
## What I changed
|
||||
|
||||
- `crates/wzp-proto/src/packet.rs:545-561` — Added `protocol_version: u8` and `supported_versions: Vec<u8>` to `CallOffer` with `#[serde(default = "...")]` helpers.
|
||||
- `crates/wzp-proto/src/packet.rs:1106-1119` — Added `ProtocolVersionMismatch { server_supported: Vec<u8> }` variant to `HangupReason`.
|
||||
- `crates/wzp-proto/src/packet.rs:1121-1128` — Added `default_proto_version()` and `default_supported_versions()` helpers.
|
||||
- `crates/wzp-client/src/handshake.rs` — Added `HandshakeError` typed error enum with `ProtocolVersionMismatch` variant. Changed return type from `anyhow::Error` to `HandshakeError`. Client now sets `protocol_version: 2` and `supported_versions: vec![2]` on outgoing `CallOffer`. On receiving `Hangup::ProtocolVersionMismatch`, returns `HandshakeError::ProtocolVersionMismatch`.
|
||||
- `crates/wzp-relay/src/handshake.rs:38-66` — Relay now checks `protocol_version == 2` after parsing `CallOffer`. If not, sends `Hangup::ProtocolVersionMismatch { server_supported: vec![2] }` and returns an error.
|
||||
- `crates/wzp-relay/tests/handshake_integration.rs:305-372` — Added `handshake_rejects_v1_protocol_version` test: sends `protocol_version: 1`, verifies relay rejects with typed hangup.
|
||||
- `crates/wzp-client/tests/handshake_integration.rs:186-226` — Added `client_receives_protocol_version_mismatch` test: mock relay sends mismatch, client returns typed error.
|
||||
|
||||
Also fixed T1.5 migration gaps discovered during T1.6:
|
||||
- `desktop/src-tauri/src/engine.rs` — `.is_repair` → `.is_repair()`, `seq: u16` → `u32` in DRED tracking
|
||||
- `crates/wzp-client/src/cli.rs:727` — `.is_repair` → `.is_repair()`
|
||||
- `crates/wzp-android/src/engine.rs` + `pipeline.rs` — Full v2 field migration (subagent)
|
||||
|
||||
## Why these choices
|
||||
|
||||
The typed `HandshakeError` gives callers a way to distinguish protocol version mismatch from other handshake failures (network, bad signature, etc.) without string-matching. `#[serde(default)]` on the new fields means old JSON payloads without `protocol_version` deserialize as v2, which is the correct behavior for the current codebase that speaks v2 wire format.
|
||||
|
||||
## Deviations from the task spec
|
||||
|
||||
None. The task spec said to add `ProtocolVersionMismatch` to the reason enum or as a structured `SignalMessage` variant — the existing `Hangup` already had a `reason` field, so adding to `HangupReason` was the natural fit.
|
||||
|
||||
## Verification output
|
||||
|
||||
```bash
|
||||
$ cargo test -p wzp-relay --test handshake_integration
|
||||
running 5 tests
|
||||
test auth_then_handshake ... ok
|
||||
test handshake_rejects_bad_signature ... ok
|
||||
test handshake_rejects_v1_protocol_version ... ok
|
||||
test handshake_succeeds ... ok
|
||||
test handshake_verifies_identity ... ok
|
||||
|
||||
test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
|
||||
```
|
||||
|
||||
```bash
|
||||
$ cargo test -p wzp-client --test handshake_integration
|
||||
running 3 tests
|
||||
test client_receives_protocol_version_mismatch ... ok
|
||||
test full_handshake_both_sides_derive_same_session ... ok
|
||||
test handshake_rejects_tampered_signature ... ok
|
||||
|
||||
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
|
||||
```
|
||||
|
||||
```bash
|
||||
$ cargo test --workspace --exclude wzp-android --no-fail-fast
|
||||
# Total: 613 passed; 0 failed
|
||||
```
|
||||
|
||||
```bash
|
||||
$ cargo clippy -p wzp-proto -p wzp-client -p wzp-relay -p wzp-desktop --all-targets -- -D warnings
|
||||
# Clean
|
||||
```
|
||||
|
||||
```bash
|
||||
$ cargo fmt --all -- --check
|
||||
# Clean
|
||||
```
|
||||
|
||||
## Test summary
|
||||
|
||||
- Tests added: 2 (`handshake_rejects_v1_protocol_version`, `client_receives_protocol_version_mismatch`)
|
||||
- Tests modified: 0
|
||||
- Workspace test count before: 572 / after: 613 (includes T1.5 android/desktop fixes)
|
||||
- `cargo clippy -p wzp-proto -p wzp-client -p wzp-relay -p wzp-desktop --all-targets -- -D warnings`: pass
|
||||
- `cargo fmt --all -- --check`: pass
|
||||
|
||||
## Risks / follow-ups
|
||||
|
||||
- `wzp-android` requires NDK to link; the Rust source compiles but the crate cannot be fully built on macOS. The T1.5 migration fixes were verified via `cargo check -p wzp-android`.
|
||||
- The `deps/featherchat` submodule has 3 pre-existing clippy errors documented in `PROTOCOL-AUDIT.md`.
|
||||
|
||||
## 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
|
||||
Reference in New Issue
Block a user