Amendment batch (no new decisions, all doc-level): - A-4: done-round boundary set is the recognized subset — request haves filtered through common_haves, the same honest-boundary rule as the ack rounds (never honor an unverified have); amendment clause in ADR-014 §2, same rule restated in transport.md §fetch. - D-2: amendment note on ADR-007 step 3 — the per-repo check is ADR-011's authorize policy function (static ACL engine fails closed on None identity); step order unchanged. - D-3: authorized-repo marker added to both substrate input tuples in transport.md and to backend.md's public-API list (ADR-007's type-level enforcement promise is now findable from the transport spec). - N-1: advertisement ref cap is fail-closed (breach is an error, never a silent truncation) — transport.md §Limits. - N-2: RegistryError::NotFound and authorization failure collapse to the same wire error at the variant→wire mapping — transport.md §error taxonomy. - review 001: A-4/D-2/D-3/N-1/N-2 marked resolved. Verification: cargo doc --no-deps, cargo test — clean.
138 lines
6.8 KiB
Markdown
138 lines
6.8 KiB
Markdown
# ADR-014: V2 multi-round negotiation — the ack loop
|
||
|
||
## Status
|
||
|
||
Accepted (resolves OQ-02)
|
||
|
||
## Context
|
||
|
||
ADR-003 shipped v1's fetch as full-closure-on-`done` with
|
||
`fetch=wait-for-done` advertised, explicitly deferring the multi-round
|
||
ack/NAK design to OQ-02 ("lands after the done-path works end-to-end").
|
||
The gap: real clients with shared history send no-`done` rounds first
|
||
(want + have lines), and the server must answer them with a
|
||
grammatically valid `acknowledgments` section — even a done-only server
|
||
has to answer *something*. The open questions were the ack/NAK logic,
|
||
whether `wait-for-done` stays, and how the loop composes with the
|
||
GitPackGen seam and ADR-009's budgets.
|
||
|
||
Resolution method: duplex `git fetch` captures against a ground-truth
|
||
negotiation mock (git 2.43.0), cross-checked against `fetch-pack.c`
|
||
v2.43.0 (`do_fetch_pack_v2` / `process_ack` / `send_fetch_request`).
|
||
Captures and the source-derived grammar are recorded in
|
||
`docs/research/negotiation-captures.md`.
|
||
|
||
## Decision
|
||
|
||
**v1 serves the full ack loop; no `ready`; `wait-for-done` stays.**
|
||
|
||
1. **Round grammar served**: a no-`done` round (want + have lines +
|
||
flush) is answered with:
|
||
|
||
```
|
||
acknowledgments
|
||
[ACK <oid>]* — one per have we recognize as common (rule below)
|
||
[NAK] — only when no ACKs in the round
|
||
0000 FLUSH — section terminator (we never send `ready`, so
|
||
FLUSH is always our terminator — the DELIM
|
||
variant exists only for `ready` responses)
|
||
```
|
||
|
||
ACK rule: a have is acked iff the object exists in the repo's odb
|
||
AND is a commit. (Clients send commit haves; acking non-commits
|
||
would feed the client's negotiator garbage. The existence check is
|
||
also the honest-boundary rule: never ack what we cannot subtract.)
|
||
NAK placement matches upstream: absent when ACKs were sent.
|
||
|
||
2. **Done round**: pack generation runs exactly as ADR-003/ADR-004
|
||
define, with the request's haves as the boundary set:
|
||
closure(wants) − closure(haves). The client re-sends its acked
|
||
commons and its current-round haves in the done request, so the
|
||
boundary set is complete in that single request — no cross-round
|
||
server state is required (client-side guarantee, observed and
|
||
source-confirmed). An empty resulting pack (client already has
|
||
everything) is a valid zero-object packfile response.
|
||
|
||
*(Boundary set amended per review 001 A-4: the raw request's haves
|
||
include never-verified client claims; the boundary set is the
|
||
*recognized* subset — request haves filtered through
|
||
`common_haves` (§5) — applied on the done round exactly as on the
|
||
ack rounds. The ack rule and the subtraction rule are the same
|
||
honest-boundary rule at different points: we do not honor haves we
|
||
could not verify. Existence is the operative predicate for
|
||
subtraction (a non-commit have can legitimately bound traversal;
|
||
§1's is-commit refinement exists for ACK-line correctness, not for
|
||
subtraction — `gix_traverse::commit::Simple::filtered` takes the
|
||
boundary predicate directly, so verify-then-subtract is one
|
||
predicate, not a custom walk). Cost: one existence check per have
|
||
on the done round — the wire layer already accepts paying it on
|
||
every ack round; no new budget kind.)*
|
||
|
||
3. **Want-less rounds are answered empty.** A fetch round with no want
|
||
lines (captured: an up-to-date client sends an empty round before
|
||
exiting) gets an empty acknowledgments section — `acknowledgments` +
|
||
`NAK` + flush, no pack. The client exits without sending a done
|
||
round; nothing else is required. This is the degenerate no-work
|
||
shape, not an error.
|
||
|
||
4. **`wait-for-done` stays; `ready` is never sent.** The ack loop needs
|
||
no capability-text change (the acknowledgments section is grammar,
|
||
not a capability — clients send no-done rounds regardless of the
|
||
advertised fetch features, captured). Keeping `wait-for-done` honest
|
||
means: pack is sent only on `done`, never on `ready`. The efficiency
|
||
`ready` would buy is at most one round-trip per fetch (and the
|
||
everything-already-common case is bounded by the want-less round
|
||
rule above). Advertisement stays `fetch=wait-for-done` exactly as
|
||
ADR-003 pinned.
|
||
|
||
5. **The ack check is a backend seam**: `GitPackGen` gains one method —
|
||
`common_haves(repo, haves) -> recognized subset` (exists + is-commit
|
||
per have). The wire layer acks exactly its output. This keeps the
|
||
honest-boundary decision (never ack what we cannot subtract) at the
|
||
trait boundary, where an embedder with a foreign object store can
|
||
implement it, and keeps the wire layer odb-free (ADR-010).
|
||
|
||
6. **Budgets (ADR-009)**: the loop consumes max-negotiation-rounds
|
||
(tens) and max-haves-per-round (the default sits at/above the
|
||
client's legitimate stateless ceiling of 16384 — see the amended
|
||
ADR-009 table); breach ends the session per ADR-009's fail-closed
|
||
rule. A client that keeps sending rounds without progressing hits
|
||
the rounds cap; no new budget kinds are introduced.
|
||
|
||
7. **Statelessness preserved**: each http POST is one round; the client
|
||
re-sends wants + acked commons every round, so the stateless
|
||
substrate (ADR-005) needs no negotiation state, and the duplex
|
||
substrate needs none either. This is what makes the loop cheap on
|
||
both substrates.
|
||
|
||
## Consequences
|
||
|
||
- **Positive**: incremental fetches get real negotiation — the done
|
||
round's haves subtract shared history from the pack (the efficiency
|
||
the OQ existed for), and the client's negotiator stops wandering our
|
||
unknown history early (ACKs bound `in_vain` at 256 instead of letting
|
||
it run to exhaustion). The done-only fallback is the loop's degenerate
|
||
case (NAK-only responses), so the POC-proven path remains the
|
||
correctness floor. No capability change; ADR-003's advertisement text
|
||
stands.
|
||
- **Negative**: the ack loop adds one backend-trait method (freeze
|
||
inventory, OQ-03) and one wire-shape surface that must match
|
||
`fetch-pack.c`'s parser exactly (the capture log records the failure
|
||
modes of getting the section wrong). The `ready`-based early pack is
|
||
declined — a possible future optimization, additive (capability-text
|
||
change), two-way until published.
|
||
- **Neutral**: `no-done` (the client capability that would let the
|
||
server's ready replace done) is not applicable while we never send
|
||
`ready`.
|
||
|
||
## References
|
||
|
||
- `docs/research/negotiation-captures.md` (the grammar + client-behavior
|
||
record this decision is built on; includes `fetch-pack.c` v2.43.0
|
||
cross-checks)
|
||
- ADR-003 (V2-first, advertisement values — unchanged by this ADR),
|
||
ADR-004 (pack generation, haves boundary), ADR-005 (substrates),
|
||
ADR-009 (rounds/haves budgets)
|
||
- gitoxide: `gix-pack` generation pipeline (closure subtraction),
|
||
`gix_odb` existence checks (the gix impl of `common_haves`)
|
||
- transport.md §fetch, backend.md §"The trait family" (GitPackGen) |