fangorn/ex_git_objectstore
public
UploadPackV2: follow-up to #27 — omit-acks branch breaks clients that sent haves #28
Links
No links yet.
Problem
The first fix over-corrected: when the client sent haves but none matched AND sent `done`, the server omitted the `acknowledgments` section entirely and went straight to `packfile`. Any client that sent at least one have enters its `process_acks` state and rejects this with:
``` fatal: expected ‘acknowledgments’, received ‘packfile’ ```
Reproduced against prod on 2026-04-18 immediately after deploying the #18 fix: `git pull` / `git fetch` from `fangorn/anvil` failed with the above.
Root cause
`build_acknowledgments(repo, haves, done)` had a branch:
```elixir done and acks == [] -> «» # omit section ```
This is wrong whenever `haves` is non-empty. Per the protocol v2 grammar, omitting the section is only valid when the client sent no haves at all.
Also, my earlier integration test didn’t actually trigger this path — the client sent zero haves, so the failing branch never fired.
Fix
`build_acknowledgments/3` always emits the section when `haves` is non-empty:
| haves | done | response |
|---|---|---|
| empty | any | omit section |
| non-empty | true | `acknowledgments / [ACKs] / ready / delim` + packfile |
| non-empty | false, no matches | `acknowledgments / NAK / flush` (multi-round continues) |
| non-empty | false, some matches | `acknowledgments / ACKs / flush` (multi-round continues) |
Tests
- Updated `test/ex_git_objectstore/protocol/upload_pack_v2_test.exs` — the state-machine assertion now requires the acks section to be present and end with `ready` when haves were sent + done. Fails against the broken code, passes against the fix.
- Updated `test/ex_git_objectstore/integration/upload_pack_v2_git_client_test.exs` — forced haves via `–negotiation-tip` + `–depth=1` and added an explicit `refute` for both regression error messages.