upload_pack_excluding breaks incremental (multi-round) fetch of withheld repos: pack sent mid-negotiation, git aborts with 'expected ACK/NAK'

[beardthelion]

Summary

The withheld-blob serve path answers the first git-upload-pack POST with NAK + a full self-contained pack, ignoring negotiation. That is fine for a single-round request (fresh clone: the pack arrives after done), but on a multi-round fetch it breaks: git has sent a flush-terminated have batch and is waiting for an ACK/NAK continuation, receives the pack instead, and aborts with fatal: git fetch-pack: expected ACK/NAK, got '?PACK'.

So a repo with a path-scoped withheld subtree cannot be fetched incrementally. Any client with more than ~32 commits of overlapping history (the common "pull updates into an existing clone") hits it.

Where

• crates/gitlawb-node/src/git/smart_http.rs — upload_pack_excluding builds a v0 NAK + full pack and returns it regardless of the client's want/have negotiation. Its docstring asserts "a fresh clone and an incremental fetch are both correct," which is the incorrect assumption: it holds for a single-round exchange but not a multi-round one.
• crates/gitlawb-node/src/api/repos.rs — git_upload_pack routes to upload_pack_excluding when has_path_scoped_rule(&rules) is true and withheld_blob_oids(...) is non-empty for the caller.

Reproduced against the running node

Public repo with a /secret/** mode-B visibility rule and a blob under secret/, ~50-commit shared history; client seeded with that history, server advanced by 3 commits, then an anonymous git fetch:

• Multi-round fetch: fatal: git fetch-pack: expected ACK/NAK, got '?PACK' (exit 128). GIT_TRACE_PACKET shows 51 haves across 2 flush-terminated batches (a real multi-round negotiation).
• Differential (isolation): deleting the one visibility rule makes the identical fetch succeed (exit 0). The rule is the only thing selecting the excluding path, so the failure is upload_pack_excluding, not the client or the history shape.
• Single-round path (fresh/partial clone) does not hit the ACK/NAK error; it gets past negotiation to pack processing.

Root cause

upload_pack_excluding is not a valid stateless-RPC upload-pack responder. In a v0 negotiation the server must reply to a flush-terminated (non-done) have batch with an ACK/NAK continuation and send the packfile only after the client sends done. Sending the pack on the first POST puts it on the wire where the client still expects negotiation, and fetch-pack aborts.

Fix options

1. Make upload_pack_excluding honor the negotiation (preferred): on a flush-terminated round reply NAK (continue) and send the filtered pack only on the done round, so the pack lands where the client expects it. This is the same framing the normal git upload-pack --stateless-rpc path already satisfies.
2. Force the withheld path single-round (e.g. don't advertise multi_ack, so the client sends want+done in one shot). Fragile and capability-dependent.
3. Accept and document that withheld repos support only single-round fetch (fresh clone), not incremental fetch.

Context

Surfaced while fixing #117 (the client helper's multi-round negotiation). The git-remote-gitlawb helper forwards the response and terminates cleanly, so this is a node-side defect, not a helper one. #117 lands a committed test (real_git_withheld_shaped_first_post) that drives the exact wire shape upload_pack_excluding produces through real git and records the rejection, so a fix here can verify against it.