Pay the clippy debt in walgit-proto

[ethanstoner]

Follow-up to #15, which says the gate is red "until the existing debt is paid ... fixing is left to follow-up commits." This pays walgit-proto's share — 27 of the workspace's 74 errors — after which the crate is clean under -D warnings.

Deliberately one crate. The rest of the debt (walgit-config has 45, plus walgit-server/build.rs) is untouched here so each slice stays reviewable, and so this does not collide with #17, #19 or #20.

What is in it

19 × doc_markdown inside mod v1. prost reproduces the doc comments written in wal.proto, so the lint is judging the schema's prose rather than ours. Targeted allow on the module.

frame::encode_entry — expect on encode, which fails only on insufficient capacity, and reserve(encoded_len()) runs immediately above. Cargo.toml's own note says known-infallible cases should "say so with a targeted #[allow] or an expect() naming the invariant"; this now does both.

frame::decode_entries — indexed and sliced a buffer read from the bucket and narrowed a u64 frame length with as. Rewritten in checked operators: get(pos..), usize::try_from, get(..len), each falling out of the loop exactly where a short read already did.

time::from_system / to_system — as across the signed boundary becomes cast_signed/cast_unsigned. to_system clamps to zero first, so neither cast can change the value it carries.

On the parser rewrite — not a bug fix

I want to be exact about this, because "checked operators" invites the reading that something was broken. Nothing was.

The bounds were already established: pos = buf.len() - probe.len() + len after a remaining() >= len check cannot exceed buf.len(), and &probe[..len] was guarded by that same check. Even the u64 → usize narrowing was harmless, since a truncated length still failed the remaining() test on both 32- and 64-bit. These were theoretical panic paths that the lint flags and the code never took.

So the added test passes against the old body as well, by construction. It is not there to catch a regression that existed; it is there so the rewrite is provably behaviour-preserving, which is the only claim I can make honestly. If you would rather this PR not carry a test that cannot fail against main, say so and I will drop it.

Verification

Check	Result
cargo clippy -p walgit-proto --all-targets -- -D warnings	clean
Workspace clippy errors	74 on main → 47
cargo fmt --check -p walgit-proto	clean
cargo test -p walgit-proto	4 pass
cargo test -p walgit-wal -p walgit-store -p walgit-bundle	9 suites, all green

just clippy still fails overall, as expected — the remaining 47 live in the crates this PR does not touch. Happy to take those in the same shape if this one lands the way you want it.

[0bserver07]

Ran this here on rust 1.97.1: cargo clippy -p walgit-proto --all-targets --no-deps -- -D warnings is clean and the 4 tests pass. The decode_entries rewrite reads as equivalent to me: each early break lands where the old remaining() < len check did, and the usize::try_from case can only fire where the old cast would have failed that check anyway. I'm doing walgit-config in the same shape (see #24), so we won't step on each other.

[ethanstoner]

That split works — I'll stay off walgit-config so #24 has it clear.

One refinement on the equivalence, because it is the reason the try_from is there rather than a straight rewrite. On every target walgit builds for it is exact: usize is 64 bits, usize::try_from(u64) cannot fail, so that arm is unreachable and the behaviour is identical to the old cast.

It stops being equivalent only on a 32-bit target. There len as usize truncates, so a varint of, say, 0x1_0000_0005 becomes 5, remaining() < 5 passes, and the old code decodes a 5-byte frame from a length it misread — a silent misparse rather than a clean stop. The try_from turns that into the same incomplete-trailing-frame break as any other short read.

That is academic for this repo today — nothing in CI or the manifests targets 32-bit or wasm — so it is not an argument for the change, just a note on what the arm is actually guarding.

[ethanstoner]

That split works — this PR stays inside walgit-proto and does not touch walgit-config, so #24 should apply cleanly on top either way round.

One note on the usize::try_from case you singled out: it is only reachable on a 32-bit target. On 64-bit the old as usize cast was lossless for any u64 that had already passed the remaining() < len check, so the two are equivalent there, and on 32-bit the cast could truncate a length above u32::MAX into a small value that passed the check. That is the one place the rewrite is not purely mechanical.

[0bserver07]

Right, on 32-bit the truncated length would pass the check instead of failing it; thanks for the precise version. Split confirmed on my side too.