ref:main

UploadPackV2: follow-up to #27 — omit-acks branch breaks clients that sent haves #28

closed Opened by cole.christensen@gmail.com

Links

No links yet.

Problem

Follow-up to #27 / PR #18.

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.