diff --git a/docs/architecture/backend.md b/docs/architecture/backend.md index b8e398f..e1fee65 100644 --- a/docs/architecture/backend.md +++ b/docs/architecture/backend.md @@ -1,6 +1,6 @@ --- -status: draft -last_updated: 2026-09-29 +status: reviewed +last_updated: 2026-09-30 --- # Backend traits: the storage seam @@ -68,6 +68,47 @@ the three object-storage traits (3–5: `GitRefs`, `GitPackGen`, `GitPackIngest`); a downstream with its own object store implements 3–5 and reuses 1–2 (`GitRegistry`/`GitRegistryStore`), or none of it. +### Pinned signatures (ADR-018; the freeze-inventory shapes) + +The registry pair is ADR-012 §1 as amended. The object-storage family +(ADR-018 §4 — pinned there; this section is the mirror the decomposer +implements from): + +- **`GitRefs`**: `list_refs(&RepoRecord) -> Vec` (the full + set — ref-prefix filtering is wire-side, client-driven per ls-refs + grammar) and `apply_updates(&RepoRecord, Vec, atomic: + bool) -> Vec` (one CAS transaction per call — the + atomic-correctness home, ADR-013 §7). +- **`GitPackGen`**: `generate(&RepoRecord, wants, boundary_haves, + limits, sink: Box)` (ADR-004's pipeline) and + `common_haves(&RepoRecord, haves) -> Vec` (the ADR-014 + ack/done-round seam; existence is the operative predicate here — the + is-commit refinement is the wire layer's ACK-line rule). +- **`GitPackIngest`**: `prepare(&RepoRecord, pack: Box, push_options: Option, limits) -> + PreparedPush` (prepare-only; the CAS application lives in + `GitRefs::apply_updates` — ADR-013 §6–7). +- The `repo` parameter is `&RepoRecord` (ADR-018 §2): the wire layer + resolves once (ADR-007 step 2) and the record carries + `storage_root` — the gix impl opens the odb from it + (`gix-discover` + `gix_odb::Store::at`) with no second lookup and no + handle type. +- Shared types (ADR-018 §5): `RefLine` (`oid: None` = the unborn + symref line, the N-5 rider's shape), `RefUpdate` + (`expected: None` = create, `new: None` = delete), `RefOutcome` + (the per-ref CAS result — an outcome, not an error; the reason string + is the client-displayed `ng` text), `PreparedPush` + (`unpack: Result<(), String>` + the updates; pack-level validation + failures land as the unpack ng leg, not `Err`), `PushOptions` (the + verbatim `(key, value)` pairs — ADR-013 §11 / N-4's additive pin). +- Object ids are `gix_hash::ObjectId` — always-on in the manifest; + hash-format-parameterized internally, so OQ-05's policy change + survives without a trait change. +- Errors: traits 1–2 → `RegistryError` (re-scoped below); traits 3–5 → + `StorageError` (ADR-018 §7 — the missing-object abort, malformed + input, and io variants; CAS and fsck outcomes are values, not + errors). + ## Feature model (ADR-012 §4, amends ADR-010's single-`gix` story) Two independent seams, two default-on features: @@ -127,6 +168,11 @@ Two independent seams, two default-on features: registries, network object stores) implements them natively; the gix impl's blocking work is an internal detail, not part of the seam. + - Bridge to the signatures (ADR-018 §4): parameter ownership is the + fork-prevention — the wire layer constructs the boxed + `Send + 'static` stream ends and owned vecs inside the permit, and + the impls move them into their internal `spawn_blocking` tasks. + Nothing is borrowed across the seam. ## Public API surface @@ -207,10 +253,14 @@ inventory as compat surface. ADR-016's open-op params; extensions replace, not accumulate, before publish). -### `RegistryError` (the variant set) +### `RegistryError` (the registry-family error set) -`thiserror` enum, five variants — every failure the trait family can -produce, no catch-all: +The error type of traits 1–2 (`GitRegistry`/`GitRegistryStore`) — +the scope is the registry family, not the whole trait family (the +object-storage traits carry `StorageError`, ADR-018 §7, after the +"every failure the trait family can produce" claim was found to +over-reach — review 002 R-2). Five variants — every failure the +registry family can produce, no catch-all: | Variant | Meaning | Wire mapping | |---|---|---| @@ -224,13 +274,21 @@ produce, no catch-all: so ops can report precise JSON errors (`{"error": "already_exists", …}`) while the trait surface stays `Result<_, RegistryError>` per ADR-012 §1. +`StorageError` (ADR-018 §7) is the object-storage sibling — traits 3–5: +`ObjectMissing { oid }` (the ADR-004 generation abort), `Invalid(String)` +(malformed/backend input), `Io(String)` (same stability rule as +`RegistryError::Io`). CAS-stale and fsck outcomes are deliberately +*not* variants: they are `RefOutcome`/`PreparedPush.unpack` values +(protocol reports the session survives), which is what keeps the +`no-catch-all` claim true for both error types. + ### `git/repo/*` op JSON schemas (request/response) | Op | Request | Response | Errors | |---|---|---|---| | `git/repo/create` | `{repo_id, visibility}` — storage root assigned by the store's naming rules (ADR-012 §2); grants seeded `{read, write, manage}` for the caller (ADR-015 §4); an explicit grants field is an admin-shape extension, NOT v1 | `{repo_id, visibility}` | `already_exists`, `invalid` | | `git/repo/delete` | `{repo_id}` | `{}` | `unauthorized` (the single wire code for unknown-repo ≡ not-authorized — N-2's collapse rule; `io`) | -| `git/repo/update` | `{repo_id, visibility?, grants?}` — both optional, full-record replace of the provided fields; grant mutation is the only grant-write path (ADR-015 §3) | `{repo_id, visibility, grants}` (post-write record, no storage root) | `unauthorized` (collapsed, N-2), `invalid`, `io` | +| `git/repo/update` | `{repo_id, visibility?, grants?}` — both optional; PATCH semantics (omitted fields unchanged; present fields replaced wholesale — see the update-semantics note below); grant mutation is the only grant-write path (ADR-015 §3) | `{repo_id, visibility, grants}` (post-write record, no storage root) | `unauthorized` (collapsed, N-2), `invalid`, `io` | | `git/repo/get` | `{repo_id}` (single read; list is a v2 additive op) | `{repo_id, visibility, grants}` (no storage root — ADR-008) | `unauthorized` (collapsed, N-2), `io` | - Error codes are the wire `code` strings in the table (snake_case, @@ -242,6 +300,24 @@ while the trait surface stays `Result<_, RegistryError>` per ADR-012 §1. unauthorized, ADR-008 + review 001 N-2), so no variant leaks an existence oracle; `AlreadyExists` is create-side only (the caller knows the id — discloseable without an oracle). +- **Update semantics are PATCH** (review 002 R-3 — the + "full-record replace of the provided fields" qualifier was + parseable either way, and the wrong reading is destructive: + a visibility-only update clearing grants locks the grant-holder + out): omitted fields are left unchanged, present fields are + replaced wholesale (`grants`, when present, replaces the whole + grant map — partial grant edits are read-modify-write at the + caller). The response echoes the post-write record so the caller + confirms what landed, including what was left unchanged. +- **`already_exists` disclosure posture** (review 002 R-6, recorded so + a later agent does not "fix" it into an upstream-incompatible + denial): a `git:repo:create`-scoped identity probing arbitrary ids + *can* learn which exist — that is accepted. Create is a + trusted, scope-gated, low-population surface; upstream gitea + behaves the same; the position is that id-probing at create-scope + is inside the trust boundary the scope already grants, while + resolve-side disclosure (the fetch/push path) stays collapsed per + ADR-008/N-2. - `update`'s response echoes the post-write record so grant edits confirm what landed (the last-writer-race posture is ADR-012 §Consequences, unchanged). @@ -249,6 +325,35 @@ while the trait surface stays `Result<_, RegistryError>` per ADR-012 §1. alksocks `{}`-precedent — same fail-closed extension rule as ADR-016's open-op params). +### Repo-id grammar (review 002 R-5) + +`repo_id` is the registry key on every wire and op surface, so its +grammar is pinned once here: + +- **Shape**: one or two path segments, `owner/name` — lowercase + alphanumerics and `-`/`_`, segments non-empty and at most 100 bytes + total (the `owner/name` convention matches self-hosted git and the + OQ-16 namespacing direction; a single segment is a valid legacy/ + local shape). +- **Wire handling** (ADR-008 governs): the id is an opaque registry + key — parsed against this grammar for *rejection* only, never + decomposed (the `owner` segment confers nothing; the registry index + is flat), never joined/normalized into paths. +- **Error mapping**: a grammar-invalid id is `unauthorized` on the + serving paths (collapsed with unknown-repo, ADR-008 — the id shape + leaks nothing) and `invalid` on the op paths (create/update — the + caller needs the correction). +- **The `registry-file` store's on-disk naming** derives from the id + by percent-encoding the `/` (`alkdev/alkgit` → `alkdev%2Falkgit` + under the roots dir): flat directory, no traversal, deterministic + round-trip on reload. Implementation detail (ADR-012 §2's naming + rules), stated here because the id grammar forces the choice. +- **Http mounting note** (for doors.md consumers): two-segment ids + need a two-placeholder route (`/{owner}/{repo}/info/refs` with + single-segment ids allowed too) — the door crate pins its own route + grammar when the alkhttp `git` feature lands; alkgit's contract is + only "the full id string arrives resolution-ready." + ## Design Decisions | ADR | Decision | Summary | @@ -265,6 +370,7 @@ while the trait surface stays `Result<_, RegistryError>` per ADR-012 §1. | [014](decisions/014-v2-negotiation-ack-loop.md) | Negotiation | ack loop, `common_haves` seam, no `ready` | | [016](decisions/016-native-session-preamble.md) | Native session preamble | `{repo, service}` open-op params, request-line preamble, service in the tuple | | [017](decisions/017-consumer-half-git-session.md) | Consumer half | `GitSession` typed client (`ls_refs`/`fetch`/`push`), custom alkcall Transport + gix-protocol, hand-rolled push | +| [018](decisions/018-backend-trait-signatures-and-storage-error-model.md) | Trait signatures + storage errors | object-storage trait signatures pinned, `StorageError` for traits 3–5, `&RepoRecord` repo param (`RegistryError` re-scoped to traits 1–2) | ## Open Questions diff --git a/docs/architecture/decisions/018-backend-trait-signatures-and-storage-error-model.md b/docs/architecture/decisions/018-backend-trait-signatures-and-storage-error-model.md new file mode 100644 index 0000000..c23befb --- /dev/null +++ b/docs/architecture/decisions/018-backend-trait-signatures-and-storage-error-model.md @@ -0,0 +1,240 @@ +# ADR-018: Backend trait signatures and the storage error model + +## Status + +Accepted (resolves review 002 R-1/R-2; amends the scope of ADR-012 §1's +A-2 amendment ("all five traits' signatures") and corrects backend.md's +`RegistryError` over-claim ("every failure the trait family can +produce")) + +## Context + +Review 001 A-2 pinned `#[async_trait]` on all five backend traits and +its remediation row claims "signatures per ADR-012 §1" — but ADR-012 §1 +contains only the *registry* pair (`GitRegistry`, `GitRegistryStore`). +Review 002 (R-1) found traits 3–5 (`GitRefs`, `GitPackGen`, +`GitPackIngest`) have no pinned signatures anywhere in the corpus: no +method names, no shape for the `repo` parameter, no type for the +prepared-updates handoff that `atomic` depends on, no representation +for object ids or push options. These are exactly the +implementation-fork hazards review 001's criticals describe: the wire +layer and the gix impls are separate workstreams, every invented shape +silently enters OQ-03's freeze inventory, and the trait signatures are +the first thing both touch. + +Review 002 (R-2) found the N-3 pin over-claims in the other direction: +`RegistryError`'s five variants cover the *registry* family (traits +1–2) well, but not the object-storage traits' characteristic failures +(missing-object aborts, fsck/unpack failures, CAS-denied refs). The +"no catch-all, every failure the trait family can produce" sentence +cannot hold for a family it does not describe. + +## Decision + +### 1. The registry traits stand as pinned + +`GitRegistry`/`GitRegistryStore` signatures are ADR-012 §1 as amended; +nothing here changes them. `RegistryError` is re-scoped as the error +type of the *registry family only* (see 7). + +### 2. The `repo` parameter is `&RepoRecord` on every object-storage method + +The wire layer resolves the repo once (ADR-007 step 2), holds the +`RepoRecord`, and passes it. The record carries `storage_root` — the +server-side path the gix impls open (`gix-discover` + +`gix_odb::Store::at`, backend.md) — so the trait needs no second +registry lookup, no registry handle on the object traits, and no new +handle type. The never-on-the-wire rule (ADR-008) governs *wire* +surfaces and op responses (N-3's exclusion is an op-response rule); the +serving path is in-process by definition and already flows the record. + +### 3. The object-id representation is `gix_hash::ObjectId` + +`gix-hash` is always-on in the manifest (the wire layer parses pkt-line +hex into it), so the trait family shares it rather than inventing a +parallel oid type. The type is hash-format-parameterized internally +(the enum carries the format tag), so the representation survives an +OQ-05 policy change; sha1 is the only constructed format in v1. + +### 4. Signatures + +``` +#[async_trait] +pub trait GitRefs: Send + Sync { + /// Ref listing — advertisement + ls-refs response data. + /// The full set; ref-prefix filtering is wire-side (client-driven). + async fn list_refs(&self, repo: &RepoRecord) -> Result, StorageError>; + + /// One CAS transaction per call (the atomic-correctness home, ADR-013 §7). + /// Runs the per-ref CAS; per-ref denials are outcomes (RefOutcome), not errors. + async fn apply_updates(&self, repo: &RepoRecord, updates: Vec, atomic: bool) -> Result, StorageError>; +} + +#[async_trait] +pub trait GitPackGen: Send + Sync { + /// closure(wants) − closure(boundary_haves) → pack streamed into sink + /// (ADR-004's pipeline; missing objects abort per ADR-004 — never a broken pack). + async fn generate(&self, repo: &RepoRecord, wants: Vec, boundary_haves: Vec, limits: Limits, sink: Box) -> Result<(), StorageError>; + + /// The negotiation ack seam (ADR-014 §5, §2's done-round amendment). + /// A have is recognized iff it exists in the object store; the is-commit + /// refinement is the wire layer's ACK-line rule, not this method's. + async fn common_haves(&self, repo: &RepoRecord, haves: Vec) -> Result, StorageError>; +} + +#[async_trait] +pub trait GitPackIngest: Send + Sync { + /// Unpack + index + fsck; prepares the ref updates. Never applies them + /// (the single CAS home is `GitRefs::apply_updates` — ADR-013 §6–7). + async fn prepare(&self, repo: &RepoRecord, pack: Box, push_options: Option, limits: Limits) -> Result; +} +``` + +Owned-value parameters (`Vec`, `Limits`, the boxed streams) are the +spawn_blocking-friendly shape: the gix impls move them into their +internal blocking tasks (backend.md concurrency model — the permit +stays wire-side, the threads stay impl-side). Boxed +`Send + 'static` streams are the blocking-side ends the substrate +bridges produce (the sideband sink's `io::Write` end; the request +body's `io::Read` end — the parked-channel bridges are ADR-005 +substrate structure, wire-layer-constructed). + +### 5. The shared object-storage types + +``` +/// One listed ref. `oid: None` is the unborn symref line (the N-5 rider's +/// "symref-target with no oid"; the wire layer maps it to the zero-oid +/// advertisement/ls-refs line). Advertisement projects oid+name only — +/// peeled tags and symref targets are ls-refs-arg data. +pub struct RefLine { + pub name: String, + pub oid: Option, + pub peeled: Option, + pub symref_target: Option, +} + +/// One prepared/applied ref update. `expected: None` = must-not-exist +/// (create); `new: None` = delete. Identical in the prepare handoff and +/// the CAS call — one type across the seam (R-1's fork point). +pub struct RefUpdate { + pub name: String, + pub expected: Option, + pub new: Option, +} + +/// Per-ref CAS outcome (an outcome, not an error — a stale ref is a +/// business result of receive-pack, ADR-013 §7–8). The reason string is +/// the client-displayed `ng ` text (server-chosen, verbatim). +pub struct RefOutcome { + pub name: String, + pub result: Result<(), String>, +} + +/// The prepare output. `unpack` is the report's unpack leg (ok, or the +/// `unpack ng ` text); pack-level validation failures (parse, +/// index, fsck connectivity) land here as the ng leg, not as Err — +/// the session continues to the report per ADR-013 §8. `Err` is for +/// substrate/infrastructure failure only (7). +pub struct PreparedPush { + pub unpack: Result<(), String>, + pub updates: Vec, +} + +/// Parsed push-options — `(key, value)` pairs verbatim, un-interpreted +/// (ADR-013 §11; repeated keys preserved). `None` on `prepare` until the +/// config gate opens; opening the gate later is value-additive. +pub struct PushOptions { + pub pairs: Vec<(String, String)>, +} +``` + +Ref-name validation is *not* in these types: the deny-list and the +`funny refname` mapping live in the transport layer (ADR-013 §9); the +transport consumes `PreparedPush.updates`, validates names, and builds +the `apply_updates` call. + +### 6. The atomic flag is the call's, not the list's + +`apply_updates(..., atomic: bool)`: non-atomic applies each update and +reports per-ref outcomes; atomic prepares the whole transaction and +commits it only if every update succeeds, mapping the abort to `ng + atomic push failure` per ADR-013 §10. One flag because both +behaviors share one transaction mechanism (§4 above); a per-update +policy flag would be a trait redesign for a case ADR-013 already +settles. + +### 7. The storage error model: `StorageError` for traits 3–5 + +`RegistryError` (backend.md §Registry types and schemas) is re-scoped: +it is the error type of the registry family (traits 1–2) — the "every +failure the trait family can produce" sentence now reads "every failure +the registry family can produce." The object-storage family gets its +own error type: + +``` +/// Object-storage family error (GitRefs / GitPackGen / GitPackIngest). +pub enum StorageError { + /// A requested object does not exist. Generation aborts per ADR-004 + /// (never a broken pack); fsck-level missing reachability is reported + /// through `PreparedPush.unpack`, not this variant. + ObjectMissing { oid: ObjectId }, + /// Structurally invalid backend input (malformed pack stream reaching + /// the impl, an unborn/misconfigured storage root). Session-op error. + Invalid(String), + /// Backing-store/filesystem failure (message-only — not + /// `std::io::Error`, which is not a stable surface; same rule as + /// `RegistryError::Io`). + Io(String), +} +``` + +| Variant | Wire mapping | +|---|---| +| `ObjectMissing` | sideband error band (fetch) — reason text may include the oid (the client computed it; no oracle risk) | +| `Invalid` | protocol error band (duplex) or 4xx-class body (stateless) | +| `Io` | session/op error, substrate-appropriate — same treatment as `RegistryError::Io` | + +Per-ref CAS failures are `RefOutcome` values, not errors (5 above); fsck +failures and pack-level corruption are `PreparedPush.unpack` ng legs +(5) — both are protocol reports the session survives, which is exactly +what separates them from the error type. + +## Consequences + +- **Positive:** the trait surface is fully pinned in one place + (backend.md remains authoritative per ADR-004's amendment note) — the + wire layer, gix impls, and any third-backend embedder now implement + against the same signatures; the prepared-updates handoff and the + `atomic` mechanism are type-expressible; the freeze inventory gains + real shapes instead of prose (OQ-03); `PushOptions`' additive path is + concrete. +- **Negative:** frozen Rust-adjacent shapes grow (the five types and + the method set join OQ-03's inventory — first-publish compat surface); + `RefOutcome`/`unpacked` reason strings are the client-visible text + surface, which is exactly as ADR-013 §8 planned but is now typed + here. +- **Neutral:** the manifest needs no change (`gix-hash`, + `async-trait`, and the substrate's channel/parking primitives are all + already carried); parameter ownership (boxed streams, owned vecs) is + the implementation-fork prevention for the A-6 execution model. + +## References + +- Review 002 R-1, R-2 (the triggers), R-3/R-5/R-6 (the op-surface + decisions this ADR's siblings pin in backend.md) +- Review 001 A-2 (the `#[async_trait]` amendment whose scope is + completed here), A-6 (the execution model these signatures imply), + N-4 (the `PushOptions` parameter this ADR types) +- ADR-012 §1 (the registry signatures — unchanged), §3 (ops over the + store), ADR-004 (the generation/ingestion pipelines these signatures + wrap; backend.md authoritative), ADR-013 §6–9, §11 (prepare/CAS/ + atomic/validation/push-options semantics), ADR-014 §2, §5 + (`common_haves`, done-round boundary), ADR-005 (the substrate bridges + the boxed streams terminate), ADR-002 (the session tuples carrying + the resolution these methods consume), ADR-008 (never-on-the-wire + scoping), ADR-009 (`Limits` in the signatures) +- backend.md §"The trait family" (mirror), §"Registry types and + schemas" (the re-scoped `RegistryError` + `StorageError` table), + OQ-03 (freeze inventory these shapes enter) +- gitoxide: `gix_hash::ObjectId` (the shared oid type), `gix-ref` + transactions (the CAS `apply_updates` wraps) \ No newline at end of file