Multi-Lens Code Review Round

I’ll review the current branch against its intended base, first establishing the exact diff and repository guidance. I’ll use three sub-agents in parallel: principal/API design, security, and idiomatic Go/regressions; I’ll independently verify the findings and return a single merge verdict with prioritized MUSTs and SHOULDs.

The change is an uncommitted, 12-file remote-helper patch on top of main; it alters protocol body handling, push behavior, replica/discovery caching, and tests. I’m treating malformed remote input, bounded memory, cache consistency, and Git protocol compatibility as the highest-risk areas while the parallel reviews run.

The focused package tests pass once the test harness is allowed to bind localhost; the first run was blocked by the sandbox, not by the code. I’m now checking the patch’s invariants against the transport’s retry/failover behavior and running repository lint while the sub-agents finish.

One blocking regression is confirmed: the new chunk reader swallows non-EOF read failures that the old io.Copy path propagated, so a truncated pack can be posted instead of rejected locally. Lint is clean and the full unit suite is still running; no security boundary issue has surfaced.

Verdict: DO NOT MERGE

MUST

• body.go:61: RequestBody.ReadFrom swallows non-EOF reader failures. io.ReadFull can either return or discard an underlying io.ErrUnexpectedEOF, depending on block length; the code then treats it as successful completion. This regresses the prior io.Copy behavior and can post a truncated pack. Use a normal read loop that appends n bytes, accepts only io.EOF, and propagates every other error. Add partial- and full-block error regression tests.

SHOULD

• body_test.go:59: add a transport-level replay test through an actual 307 redirect, 401 retry, or replica failover. Directly invoking GetBody does not fully test the abstraction’s primary contract.

• body.go:11: correct the chunk-size documentation. 64 << 10 is 65,536 bytes; Git’s maximum pkt-line payload is 65,516 bytes, so they do not literally match.

Security review found no new credential, filesystem, subprocess, protocol-injection, or fail-open issue. The broader architecture, cache locking/pruning, replay design, and API ownership otherwise look sound.

Verification passed:

• mise run lint: zero issues
• mise run test: 17,863 passed, 13 skipped
• Targeted race tests
• git diff --check

I’ll review the current diff again, check whether the previous findings are resolved, and repeat the independent reviews across the same lenses.

The previous blocker is fixed, and the added tests cover redirect and replica-failover replay. Both independent reviews now recommend merge, with two nonblocking follow-ups: use exact io.EOF comparison to match the documented contract, and test reads that return bytes and an error together. The broader race run and lint are finishing.

merge

Reviewed the updated patch across all five lenses with two independent sub-agents. The previous blocker is fixed; replay tests and chunk-size documentation are addressed.

MUSTs: none.

SHOULDs:

• body.go:74: use err == io.EOF to match documented io.Copy semantics. errors.Is also suppresses wrapped or joined errors containing EOF.
• body_test.go:126: add cases returning bytes and an error in the same Read, including a full block. Current injected errors arrive on a subsequent read.

Validation: race tests passed for all five affected packages; repository lint reports zero issues; git diff --check passed.