Every gate check read against the cited ADR text: the key encoding holds byte-exact against ADR-002 (tags, lengths, rejects, sort order, preamble abstraction, live git-CLI interop, lowercase oid policy recorded); the trait + I/O seams + engine-state seam match ADR-003/ 010 and ADR-012 §5's reduced table; the error model's five variants and single-enum altitude reading match store-api.md/ADR-012 §6.5; the feature graph matches ADR-013 §4; all named design points audited consistent; the suite convention and backlog are traceable; family conventions clean. One inline fix: publish keyword `bao` -> `git` (excluded technology named in metadata). Wave 2 decomposition unblocked, no amendments. Verified: cargo test (26 lib + 4 conformance + 3 git-interop passed), cargo clippy --all-targets -- -D warnings, cargo clippy --all-features --all-targets -- -D warnings, cargo fmt --check, cargo check --all-features, cargo check --no-default-features --features pg-lo.
179 lines
9.4 KiB
Markdown
179 lines
9.4 KiB
Markdown
---
|
||
id: review-wave-1
|
||
name: Wave 1 review gate — the one-way-door gate
|
||
status: completed
|
||
depends_on: [core-hashing-keys, core-error-model, core-trait-and-types, suite-scaffold]
|
||
scope: narrow
|
||
risk: low
|
||
impact: phase
|
||
level: review
|
||
tags: [wave-1, review]
|
||
---
|
||
|
||
## Description
|
||
|
||
The wave-1 review gate, before wave 2 decomposes. Wave 1 is the
|
||
contract surface every later wave implements against, and it carries
|
||
the crate's declared one-way door — a shape error found here is a task
|
||
revision; found after an engine ships data it is a migration ADR. The
|
||
gate reads the code against the pinned ADR text line by line.
|
||
|
||
Gate checks:
|
||
|
||
- **Key encoding vs ADR-002, line by line** (the one-way door): byte
|
||
layout (`1 alg byte + digest`, 0x01/0x02, 0x00 reserved, 33/21
|
||
lengths), rejects-never-guesses, sort stability, the preamble-as-
|
||
abstraction derivation, the interop test's fidelity (git CLI, not
|
||
just vectors), the oid hex policy decision recorded.
|
||
- **Backend trait + seams vs ADR-003/010**: the seven-method set with
|
||
`size` metadata-only; opaque byte keys; the read-cursor abstraction
|
||
(two body families, range contract at one seam, not public API);
|
||
the staged-put contract (staging cursor, commit publishes, discard
|
||
converges); the engine-state companion trait's clause families and
|
||
per-engine applicability vs ADR-012 §5's table.
|
||
- **Error model vs store-api.md / ADR-012 §6.5**: the five variants;
|
||
the altitude discipline; the single-enum reading audited against
|
||
the spec text (the named design point from `core-error-model`).
|
||
- **Feature graph vs ADR-013 §4**: the manifest's feature set, the
|
||
pg-lo-no-implies-postgres rule, `sqlite` default-on, `local`
|
||
ungated.
|
||
- **The named design points audited as resolved** (each recorded in
|
||
its task's Notes): single-enum error reading; Backend trait
|
||
visibility; trait put-form method shapes; oid hex policy; async
|
||
trait shape (AFIT vs async_trait).
|
||
- **Suite convention**: the factory contract, stamp format, and
|
||
backlog traceable to store-api.md's invariants.
|
||
- **Family conventions**: no comments in code (doc comments on public
|
||
API fine), thiserror, tokio-only async, no unwrap/expect outside
|
||
tests, naming, module structure.
|
||
- **Gates green**: `cargo test`, `cargo clippy --all-targets -- -D
|
||
warnings`, `cargo fmt --check`, `cargo check --all-features`.
|
||
|
||
Findings are fixed inline where cheap, or recorded as wave-2
|
||
amendments where they shape the next decomposition. The gate's
|
||
completion is what unblocks wave 2's decomposition.
|
||
|
||
## Acceptance Criteria
|
||
|
||
- [x] Every gate check above read and verified against the cited ADR
|
||
text (not from memory of it)
|
||
- [x] Named design points each decided, recorded, and consistent with
|
||
the specs
|
||
- [x] Findings fixed inline or recorded as wave-2 amendments with
|
||
owners
|
||
- [x] All gates re-run green after any fixes
|
||
- [x] Wave 2 decomposition unblocked (recorded in this task's Summary)
|
||
|
||
## References
|
||
|
||
- docs/plans/implementation.md (§Wave 1, §Review gates)
|
||
- docs/architecture/decisions/002-canonical-hash-and-key-encoding.md
|
||
- docs/architecture/decisions/003-backend-contract-and-dispatch.md
|
||
- docs/architecture/decisions/010-backend-io-seams-and-fleet-state-home.md
|
||
- docs/architecture/decisions/012-pre-decomposition-consistency-rulings.md
|
||
- docs/architecture/decisions/013-family-composition-reactive-boundary-and-feature-graph.md §4
|
||
- docs/architecture/store-api.md
|
||
|
||
## Notes
|
||
|
||
Every gate check was read against the cited ADR text (the files, not
|
||
memory). Findings below; everything else passed.
|
||
|
||
- **Key encoding vs ADR-002 (the one-way door)** — verified byte-exact:
|
||
`key := <alg byte> <digest>` (src/keys.rs), 0x01/0x02 assigned,
|
||
0x00 reserved and rejected, total lengths 33/21 (encoding test),
|
||
rejects-never-guesses over the full matrix (empty/truncated/
|
||
over-long/unknown 0x00+0x03+0xFF, both widths), derived `Ord` ==
|
||
physical byte order (sort-stability test), the preamble shared by
|
||
both algorithms with only the hasher specializing (no
|
||
special-casing, src/hashing.rs), the interop test drives the live
|
||
`git hash-object` CLI for both algorithms over empty/short/binary/
|
||
75-KiB content, and the oid hex policy (lowercase-only acceptance)
|
||
is recorded in core-hashing-keys Notes and stated verbatim on
|
||
`Digest::from_git_oid_hex`. No shape errors. The door holds.
|
||
- **Backend trait + seams vs ADR-003/010/012 §5** — the seven methods
|
||
(+ `put_staged`) with opaque byte keys, `size` metadata-only,
|
||
`list` complete-and-well-formed by contract; the read cursor with
|
||
both body families serving the ADR-006 slice+digest contract at one
|
||
seam; the staged-put contract (staging cursor, commit publishes,
|
||
every failure path converging on discard). The engine-state seam's
|
||
clause families match ADR-012 §5's table: `EngineWindowState`
|
||
(postgres/sqlite/pg-lo — `gc_window`-only schema rows for the
|
||
latter two), `EngineFleetState: EngineWindowState` (postgres host
|
||
only; sqlite's pin/sweep-lock bodies correctly absent — the dead
|
||
machinery ADR-012 §5 dropped never existed here; `local`/`mem`
|
||
excluded by construction).
|
||
- **Error model vs store-api.md / ADR-012 §6.5** — the five variants
|
||
exactly; payloads only where the spec writes one (`Io(String)`);
|
||
the single-enum reading audited against the spec text and held: the
|
||
facade's "re-exposes them with `GcRefuse` added" is faithfully a
|
||
one-enum posture with the altitude discipline carried by doc
|
||
comments and construction discipline (enforcement lands with the
|
||
wave-2 facade, as scoped). `#[non_exhaustive]` correctly on
|
||
`GcAbortCause` (ADR-012 §1's non-exhaustive-match ruling, made
|
||
structural) and correctly absent from `Error` (no spec requirement).
|
||
- **Feature graph vs ADR-013 §4** — the manifest table-matches exactly
|
||
(`sqlite` default-on pulling only rusqlite; `postgres`/`mem`/`pg-lo`
|
||
default-off pulling their own clusters; `pg-lo` does not imply
|
||
`postgres`; `local` ungated; `ops` default-off). Verified
|
||
`--no-default-features --features pg-lo` resolves with rusqlite
|
||
absent.
|
||
- **Named design points** — all audited consistent with the specs:
|
||
single-enum error reading (above); `Backend` `pub(crate)` visibility
|
||
(ADR-004: no consumer ever implements it; export later is purely
|
||
additive); put-form shapes (`put` + `put_staged` with associated
|
||
cursor types, kv engines carrying the degenerate staged form —
|
||
required by store-api.md invariant 7); oid hex policy (lowercase-only);
|
||
async shape (AFIT + associated types, RTN resolvable, no
|
||
`async_trait`, no `dyn`).
|
||
- **Suite convention** — the factory contract (fresh isolated store,
|
||
idempotent teardown, `Send + Sync`), the `Contract stamp:` marker
|
||
convention, and the seven-entry invariant backlog traceable to
|
||
store-api.md §Invariants 1–7 (in-code traceability gate passing),
|
||
with the mem-first column plan and the wave-2 `Store` binding point
|
||
recorded.
|
||
- **Family conventions** — no `unwrap`/`expect`/panic outside
|
||
`#[cfg(test)]` (audited every occurrence); thiserror, tokio,
|
||
parking_lot throughout; no user-comment violations (the three
|
||
inline comments on `#[allow(dead_code)]` in facade.rs and the
|
||
scoped `#![allow(dead_code)]` module attributes document why the
|
||
suppression exists — accepted under the non-obvious-constraint
|
||
exception); naming and one-module-per-file re-export shape clean.
|
||
- **Inline fix applied** — the manifest's keyword list named `bao`,
|
||
a technology this crate explicitly excludes (ADR-002 Decision
|
||
class 3; ADR-006's whole-blob verification cap); replaced with
|
||
`git`. Cosmetic publish-metadata only; no wave-2 consequence.
|
||
- **Observations recorded, no amendment needed**: (1) the staged
|
||
commit consumes the cursor and self-discards on failure — a
|
||
cancellation mid-commit leaves residue handling to engine Drop
|
||
bodies, which the contract delegates to engine tasks (waves 2–5);
|
||
each engine's hygiene tests must cover its own cancellation story.
|
||
(2) The store's verification pass reading staged bytes through the
|
||
`StagingCursor: ReadCursor` supertrait is the one design-led
|
||
extension of the ADR-010 text; it is what makes ADR-003's
|
||
second-pass-over-staged-bytes rule expressible without the retaining
|
||
the content stream — recorded in core-trait-and-types Notes and
|
||
audited consistent.
|
||
|
||
## Summary
|
||
|
||
Wave-1 review gate complete: every gate check read against the cited
|
||
ADR text line by line and verified — the one-way-door key encoding
|
||
holds byte-exact against ADR-002 (including live git-CLI interop for
|
||
both algorithms), the trait + I/O seams + engine-state seam match
|
||
ADR-003/010 and ADR-012 §5's reduced table, the error model's five
|
||
variants and single-enum altitude reading match store-api.md/ADR-012
|
||
§6.5, the feature graph matches ADR-013 §4 (pg-lo implies nothing),
|
||
all five named design points are recorded and consistent, the suite
|
||
convention and backlog are traceable, and the family conventions are
|
||
clean. One inline fix (publish keyword `bao` → `git`; excluded
|
||
technology named in metadata). All gates re-run green post-fix:
|
||
`cargo test` (26 lib + 4 conformance + 3 git-interop), `cargo clippy
|
||
--all-targets -- -D warnings`, `cargo clippy --all-features
|
||
--all-targets -- -D warnings`, `cargo fmt --check`, `cargo check
|
||
--all-features`, and the pg-lo no-implication check. Wave 2
|
||
decomposition is unblocked — it decomposes against the contract
|
||
surface as reviewed (no amendments); the two recorded observations
|
||
(cancellation residue per engine; the staging-cursor verification
|
||
pass) are engine-task scoping notes for waves 2–5, not contract
|
||
changes. |