fix(app): swap packet_id parity to match Go (server=Even, client=Odd)#30
Merged
Conversation
D2 introduced the per-end packet_id allocator with the parity
reversed from the Go reference: server walked Odd, client walked
Even. Go does the opposite — `server/end.go` builds the factory
with `id.NewIDCounter(id.Even)`, `client/end_options.go` uses
`id.Odd`.
In practice the reversal didn't break interop because Go's
`packet_id` is `(timestamp << 32) | counter`, so its values are
always in a different magnitude range from our small-counter
allocations and the two never collided on a wire. But the
convention divergence is not load-bearing — there's no reason to
deviate from the reference. Aligning now keeps the protocol matrix
boring ("Rust says X iff Go says X") and avoids a footgun for
future reviewers comparing the two impls side-by-side.
Changes:
EndCtx::new — server seed 1→2, client seed 2→1 (still ±2)
doc comments in end_ctx.rs reflect the corrected mapping
test rename: server_side_packet_id_is_odd_client_is_even →
_is_even_client_is_odd, with assertions flipped
3/3 interop tests + 145 workspace tests still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
D2 (#25) introduced the per-end
packet_idallocator with parity reversed from the Go reference:v1.3.0-rc.2)id.NewIDCounter(id.Even)→2, 4, 6, …1, 3, 5, …❌2, 4, 6, …✅id.NewIDCounter(id.Odd)→1, 3, 5, …2, 4, 6, …❌1, 3, 5, …✅(
server/end.go:136andclient/end_options.go:127are the upstream call sites.)Why interop still passed despite the bug
Go's actual
packet_idis(time.Now().Unix() << 32) | counter— values always sit in the high half of the u64 range. Our Rust allocator uses the small-counter form (1, 3, 5…). So Go's IDs and Rust's IDs are in different magnitude ranges and never collided on the wire even with the parity flipped. The 3 cross-language interop tests passed by accident, not by design.Aligning now removes a latent footgun — it keeps the protocol matrix boring ("Rust says X iff Go says X") and avoids a real collision the day someone tightens the allocator (e.g. switches to
(timestamp << 32) | counterto match Go's spec exactly).Changes
crates/app/src/end_ctx.rs::new— server seed1 → 2, client seed2 → 1. Still increments by 2.end_ctx.rscorrected to say "Server: even, Client: odd".server_side_packet_id_is_odd_client_is_even→…_is_even_client_is_odd, assertions flipped.Test plan
cargo test --workspace --all-targets— 145 passcargo test -p geminio --features interop --test interop— 3/3 pass (Go server + Rust client RPC, Go server + Rust client publish, Rust server + Go client RPC)cargo clippy --all-targets -- -D warningscleancargo fmt --all -- --checkclean🤖 Generated with Claude Code