alkgit/docs/reviews/001-architecture-pre-decomposition-review.md

status, last_updated, reviewed_artifacts, tool, reviewer, scope_note

status	last_updated	reviewed_artifacts	tool	reviewer	scope_note
open	2026-09-29	docs/architecture/README.md docs/architecture/overview.md docs/architecture/transport.md docs/architecture/backend.md docs/architecture/doors.md docs/architecture/open-questions.md docs/architecture/decisions/ (ADR-001..014) docs/research/ (vision, git-protocol, gitoxide, alk-stack, reference-policy, pocs, poc-1/2/3 findings, push-captures, negotiation-captures) docs/sdd_process.md tasks/architecture/ (all five tracker/backlog tasks) Cargo.toml src/lib.rs	manual full-corpus read + alkcall/alktty source cross-checks + gitoxide source verification + feature-matrix compile probes + dyn-compatibility probe (scratch crate, MSRV-pinned) + cargo test/clippy/fmt/doc/publish-dry-run	architecture pre-decomposition review (contradiction + gap + staleness pass)	phase-1 gate review — the entire committed architecture corpus against the sibling-crate surfaces it leans on (alkcall 0.8, alktty template) and the pinned gitoxide versions. No implementation exists to review (src/lib.rs is the skeleton); this review asks whether the spec set is internally consistent and implementable as written.

Review 001 — Architecture Pre-Decomposition Review

Purpose

The phase-1 OQ cycle is complete (OQ-04/ADR-013 and OQ-02/ADR-014 landed 2026-09-25; every wire-shape question has a capture-grounded decision) and the next step is decomposition into implementation tasks. This review is the gate between those phases: it reads the whole architecture corpus the way a decomposer and then an implementation agent would — looking for

1. Contradictions between decisions — places where two Accepted documents (or an ADR and the sibling-crate API it cites) disagree, where an implementer would have to pick a winner or guess.
2. Spec gaps that strand implementers — wire shapes, trait signatures, or gate mechanics the docs reference but never pin, where two implementation workstreams would invent incompatible answers.
3. Stale text — superseded statements left standing in docs the decomposer reads first.

Findings A-1/A-2/A-3 are the reason this review exists: each is a place where an implementation agent hits a hard failure (a compile error, an unservable wire path, an unenforceable gate) that the decomposition agent cannot resolve because the mechanism is not specified anywhere. The rest are smaller, but each would have cost a session to discover mid-implementation.

This review is docs-only by design — there is no code to review yet (the crate is the skeleton). The code-level review passes (alkty-001 / alksocks-002 style) come after the first implementation batches land.

Methodology

• Full read of every architecture document, all 14 ADRs, every research document, the SDD process, and all five tasks/architecture/ files.
• Cross-check of every load-bearing external API claim against the real sibling sources: alkcall 0.8 (AccessControl::check semantics, OwnershipStore, Identity, ChannelOpenSpec, OperationSpec, ADR-025/017/011/047 texts) and alktty (TtyBackend async-trait pattern, register_openable/establisher shape, TtySession).
• Cross-check of the pinned gitoxide APIs against the reference clone (/workspace/gitoxide, matching published 0.87.1): the ADR-013 ingestion composition (Bundle::write_to_directory_eagerly, thin_pack_base_object_lookup, PreviousValue::MustExistAndMatch), ADR-014's subtraction mechanism (gix_traverse::commit::Simple filtered walks), gix_validate::reference::name, and the gix-pack feature set claims.
• Feature-matrix probe: cargo check under all four configurations (default, --no-default-features, --features sha256, --all-features) — the ADR-012 §4 feature split and the wire-layer-without-gix claim, tested rather than trusted.
• Dyn-compatibility probe: a scratch crate (/tmp/opencode/dynasync) reproducing ADR-012 §1's literal trait shape (async fn + Arc<dyn Trait>) under both the active toolchain (1.94) and the pinned MSRV (1.88) — see A-2.
• Verification battery on the current tree (see baseline below).
• Cross-reference integrity sweep: every ADR-NNN / OQ-NN reference in docs/architecture/ resolved against existing files, including the sibling-crate ADR citations (alkcall 003/011/017/025/040/047, alknet 033/035); every deferred OQ checked for the two-half protocol (tracker entry + tasks/architecture/ task) and for circular deferral.

Verification Baseline

All commands on the reviewed tree (commit d6d013e, clean):

• cargo check (default features): clean.
• cargo check --no-default-features: clean — the wire layer compiles without gix (the ADR-010/ADR-012 §4 claim is true today).
• cargo check --features sha256 and --all-features: clean.
• cargo test: passes (0 tests — the skeleton; the suite is trivially green).
• cargo clippy --all-targets -- -D warnings: clean.
• cargo fmt --check: clean.
• cargo doc --no-deps: 0 warnings.
• cargo +1.88 check: clean (the manifest's rust-version = "1.88" is real).
• cargo publish --dry-run --allow-dirty: completes.
• git --version: 2.43.0 — the integration-test surface the ADR-013/014 captures were built against is present in this environment for implementation-phase verification.

Summary Statistics

Severity	Count	IDs
Critical	3	A-1, A-2, A-3
Major	3	A-4, A-5, A-6
Minor (stale doc)	3	D-1, D-2, D-3
Observation	5	N-1..N-5

No finding questions the architecture's direction — the pure-protocol- crate shape (ADR-010), the capture-grounded wire decisions (ADR-013/ 014), and the auth model (ADR-011) all survived the pass intact. Every finding is a mechanism gap: the WHAT is decided, a HOW the docs implicitly assumed turns out to be wrong or missing, and the resolution is an amendment or one small ADR each. The three criticals share a property: they sit exactly where a decomposer would cut tasks (the op gate, the backend trait signatures, the native session preamble), so they would each silently fork into incompatible implementations rather than fail loudly.

Finding prefixes: A (architecture/spec defect — decision or mechanism), D (documentation staleness), N (notes/observations).

---

Part A — Findings

A-1 [critical] — ADR-012 §3's two-tier op gate ("scope git:admin or ownership") is not expressible in alkcall's AccessControl, which composes restrictions as AND

Files: docs/architecture/decisions/012-registry-backing-and-ops.md §3 (the gate table and the "uses the checks AccessControl::check already composes" claim); alkcall src/registry/spec.rs:94-171 (the mechanism); docs/architecture/backend.md §"The two op kinds".

Problem: ADR-012 §3 gates the CRUD ops with a two-tier rule — git/repo/delete|update|get are allowed to "scope git:admin or ownership" — and states the rule "uses the checks AccessControl::check already composes: required_scopes for the global tier, resource_type

• ownership for the self-owned tier." I verified alkcall's check() directly: the restrictions compose as AND, in a fixed order — required_scopes are checked first and fail hard (Forbidden), then required_scopes_any, then the ownership/resource block. There is no scope-OR-ownership composition, and required_scopes_any is scope-set-OR, not scope-or-ownership. A single OperationSpec therefore cannot express the gate:

• required_scopes: ["git:admin"] + resource_type on one spec → requires both; a self-owned non-admin user is Forbidden at the registry and the handler never runs.
• ownership-only (resource_type + resource_id_path, no scopes) → a global admin who does not own the repo is Forbidden at the registry.
• The "or" cannot be recovered handler-side, because the registry gate fires before dispatch reaches the handler in both shapes.

Consequence: any of the three gate rows in the ADR-012 §3 table, built naively, either locks out the self-owned tier (the entire point of the two-tier design — "a user who creates a repo administers their own repos") or locks out the global admin. This is not a corner: it is the primary authorization path for every management op in the crate.

Options:

• (a) Handler-side two-tier check (recommended). git/repo/create keeps its pure-scope registry gate (required_scopes: ["git:repo:create"] — no ownership tier, so the static engine serves it fine). git/repo/delete|update|get carry an empty AccessControl::default() and the handler evaluates the two-tier rule itself: admin scope OR OwnershipProvider::owns(identity, "git/repo", repo_id, action), else a generic FORBIDDEN. This matches ADR-011 §4's own pattern — policy lives in alkgit core, one tested function — and keeps the two-tier rule unit-testable in-crate instead of hidden in an assembly-supplied provider. The gitea lesson is unaffected: visible-surface = authorized-surface is about enforcement living in the same place as exposure, and a handler-side check is exactly as enforcement-shaped as the registry gate (it denies with the same generic error; no existence oracle — the handler must return the same denial for unknown-repo and not-owner, per ADR-008's rule).
• (b) Ownership-provider composition. Keep the gate registry-side (resource_type + resource_id_path, no scopes) and have the assembly's OwnershipProvider report admin-scoped identities as owning everything. Mechanically clean (all enforcement in alkcall dispatch), but it hides the two-tier policy in assembly code — untestable in-crate, invisible to the architecture, and it overloads owns() semantics.
• (c) Extend alkcall's AccessControl with a scope-or-ownership mode. Cleanest long-term but it is a family-crate change on someone else's release cadence; not alkgit's decision to make unilaterally.

Fix (small): a short ADR (or an explicit amendment block on ADR-012 §3, the pattern ADR-004 used for ADR-010's layout change) recording the mechanism — recommended: option (a), with the create-op carve-out, the generic-denial requirement, and the (store, ownership) handler wiring spelled out. The ADR-012 §3 sentence "uses the checks AccessControl::check already composes" must be corrected either way — as written it certifies a composition that does not exist.

---

A-2 [critical] — ADR-012 §1's literal trait signatures (async fn in traits) do not compile as Arc<dyn GitRegistryStore>: not dyn-compatible (E0038); async-trait is absent from the manifest

Files: docs/architecture/decisions/012-registry-backing-and-ops.md §1 (the signature block), docs/architecture/backend.md §"Concurrency model" ("The traits are Send + Sync object-safe"), §3 ("thin OperationSpec+handler pairs over Arc<dyn GitRegistryStore>"), Cargo.toml (no async-trait).

Problem: ADR-012 §1 writes the registry traits with bare async fn:

pub trait GitRegistry: Send + Sync {
    async fn resolve(&self, repo_id: &str) -> Result<RepoRecord, RegistryError>;
}

backend.md simultaneously claims the traits are "Send + Sync object-safe," and ADR-012 §3 puts them behind Arc<dyn GitRegistryStore>. Bare async fn in a trait is not dyn-compatible — I reproduced this exactly with a scratch crate (/tmp/opencode/dynasync, a trait with one async fn coerced to Arc<dyn Trait>): rustc 1.94 and the pinned MSRV 1.88 both reject it with E0038 ("the trait Reg is not dyn-compatible ... method resolve is async"). The family precedent resolves it the same way every time: #[async_trait] (alktty's TtyBackend, alkcall's ProtocolHandler, both pinned at async-trait = "0.1" in their manifests) — but alkgit's manifest does not carry the dependency, and neither ADR-012 §1 nor backend.md says the word.

Consequence: the registry traits are the first thing every backend/ops/registry-file workstream touches; the first task that writes Arc<dyn GitRegistryStore> hits E0038 against a signature the ADR presents as normative. Because these signatures are the OQ-03 freeze inventory, the correction should be pinned before decomposition so no task bakes in an ad-hoc answer (hand-rolled BoxFuture signatures, a private ErasedRegistry wrapper, per-trait #[async_trait] vs manual desugaring — three workstreams, three answers, one freeze surface).

Fix (small): amend ADR-012 §1 and backend.md to #[async_trait] on all five traits (the signatures otherwise stand — Send + Sync, resolve async per ADR-011 §6, the write supertrait split per alknet ADR-035's shape), and add async-trait = "0.1" to the manifest. One conscious note for the freeze inventory: #[async_trait] desugars to Pin<Box<dyn Future + Send + 'async_trait>> — that boxed form is the published vtable shape, which is fine, but it should be pinned deliberately (OQ-03) rather than discovered at first publish. The five-trait family (GitRefs, GitPackGen, GitPackIngest too) should get the same attribute in the same amendment so all six signatures in the corpus agree.

---

A-3 [critical] — The native path has no pinned session preamble: the service dimension is missing from the session tuple, and the channels open-op params are pinned as "repo id" only — as written, push is unservable over the alk/git ALPN

Files: docs/architecture/overview.md (crate map: "repo id in open-op params"), docs/architecture/decisions/010-pure-protocol-crate.md (same pin), docs/architecture/decisions/002-front-door-blind-core.md (the session tuple), docs/architecture/decisions/005-session-substrate-types.md, docs/architecture/transport.md §substrate, docs/architecture/decisions/013-receive-pack-state-machine.md §2 (server-speaks-first on duplex push).

Problem: every door selects the git service (upload-pack vs receive-pack) before the protocol starts — http by route (/{repo}/git-upload-pack vs /{repo}/git-receive-pack, doors.md), ssh by the parsed exec command (git-upload-pack '<repo>' vs git-receive-pack '<repo>', doors.md's alkssh requirement). On the native path the carrier is underdetermined, and the two native sub-paths the crate map names each have a hole:

1. Channels path (register_openable): the docs pin the open-op params as "repo id in open-op params — the negotiation + ACL point" (ADR-010, overview crate map). Nothing carries the service, and the consequences start before the state machine even runs:
   • The open-time ACL point cannot evaluate the write tier. The reason the repo id rides the open-op params is that the open op is "the negotiation + ACL enforcement point" — ADR-007's resolve→authorize sequence runs at open time, before the channel carries protocol data. But authorize(record, identity, action) is per-action (ADR-011): fetch needs the read check, push needs the write check. Without the service in the params, the open gate cannot know which check to run — it either admits a pusher it should have rejected at the gate (deferring the write check into the session, weakening the ADR-007 posture) or over-restricts.
   • The session cannot choose its state machine. Both advertisements are server-emitted firsts on the duplex path (the V2 capability advertisement for fetch, ADR-003; the V0 ref advertisement for push — ADR-013 §2 "the server speaks first" — where "speaks first" means the advertisement precedes the client's protocol request, not the open op itself). With no service anywhere in the open-op handshake, the server cannot select which advertisement to emit. As specified, push over alk/git channels is impossible.
2. Direct-ALPN path (GitAdapter): the crate map says "POC-1 verbatim," and POC-1's shape carries the service in-band — the git-daemon request line (git-upload-pack <repo>\0host=…\0\0version=2\0 for fetch; git-receive-pack <repo>\0host=…\0 for push, poc-1-findings §2 / push-captures §request), client-sent before the server speaks. But that framing on the alk/git ALPN is alkgit-specific wire format (real git clients never touch the native path — they speak to doors), and AGENTS.md convention 9 explicitly says alkgit-specific framing "will get ADRs when it exists." No ADR pins it: whether GitAdapter parses a git-daemon-style request line, a JSON preamble, or expects the caller to pre-negotiate is unspecified. (POC-1's bridge emulated a git:// TCP server because real git was the client; GitAdapter's native clients are GitSession and alkcall-speaking embedders — a different contract that no doc defines.)

The session tuple itself lacks the dimension: ADR-002's tuple and transport.md's substrate inputs are "(peer identity, resolved repo id, duplex stream, Limits)" and "(peer identity, resolved repo id, request-reader, response-writer, Limits)" — no service/variant selector. The stateless substrate gets the service from the door's route, but the substrate input must carry it explicitly (nothing in the POST body reliably distinguishes an upload-pack POST from a receive-pack POST before parsing). GitSession (the consumer half) mirrors whatever is pinned here, so the consumer half is blocked on it too.

Fix (small — one ADR): pin the native session preamble. The shape the corpus already points at (my recommendation, but the ADR should decide):

• The channels open-op params schema is {repo, service} — JSON, service ∈ {"git-upload-pack", "git-receive-pack"}, additive- extension rule (alksocks pinned {} with additionalProperties: false for exactly this reason: extensions stay additive; here the first extension already exists at v1). The params are the open-time ACL point (ADR-010 unchanged) and the service selector — the service at open time is what lets the gate run the correct authorize action (read for fetch, write for push) per ADR-011, instead of deferring the write check into the session.
• The duplex session tuple gains the service dimension: (identity, repo, service, stream, limits) — amend ADR-002, ADR-005, and transport.md's substrate input lists (the authorized- repo marker from D-3 rides the same amendment).
• GitAdapter (direct ALPN) parses a pinned in-band preamble carrying the same fields — the git-daemon request-line grammar (POC-1 verbatim, real-git-compatible framing, service-in-first-line) is the natural candidate and needs the ADR convention 9 asks for, since it is alkgit-specific wire format on a published ALPN (freeze inventory, one-way).
• GitSession::open_via_channels sends the same params schema.

Note the version dimension does not ride the preamble: fetch is V2-only (ADR-003 — V0/V1 clients get a clear error) and push is V0-framed unconditionally (ADR-013 §1), so service fully determines the state machine. That is worth stating in the ADR — it is the reason one field suffices.

---

A-4 [major] — ADR-014 §2's done-round boundary set is "the request's haves," but haves include unverified client claims; the boundary set should be the recognized subset (the ADR's own honesty rule)

Files: docs/architecture/decisions/014-v2-negotiation-ack-loop.md §2, docs/architecture/transport.md §fetch, cross-checked against docs/research/negotiation-captures.md and upstream behavior.

Problem: ADR-014 §2 says the done round "generates closure(wants) − closure(haves) … with the request's haves as the boundary set." The request's haves are a mix of acked commons (which the server verified via common_haves in an earlier round) and new, never-verified haves the negotiator is offering. Taking the boundary set literally — subtract on the raw request haves — means the server honors unverified client claims: a client (buggy or malicious) that claims have-X without having it receives a pack missing closure(X) ∩ closure(wants) and fails client-side with "did not send all necessary objects." Self-inflicted, not a server-side hole — but it contradicts the ADR's own honesty principle ("never ack what we cannot subtract," §1/§5) at the one place subtraction actually happens, and it deviates from upstream: upload-pack marks a have uninteresting only after confirming the object exists server-side.

The mechanism to do it right is already in the corpus: §5's common_haves seam. The boundary set should be the recognized subset — common_haves(repo, request_haves) — applied on the done round exactly as on the ack rounds. Existence is the operative predicate for subtraction (a non-commit have can legitimately bound traversal); the is-commit refinement in §1 exists for ACK-line correctness, not for subtraction. I verified the traversal side supports this cheaply: gix_traverse::commit::Simple::filtered(tips, find, predicate) takes exactly a boundary predicate ("whether a commit should be included as well as whether its parents should be traversed" — gix-traverse source), so verify-then-subtract is one predicate, not a custom walk.

Fix (small): one clause in ADR-014 §2 ("the boundary set is the recognized subset — request haves filtered through common_haves — not the raw request list; the ack rule and the subtraction rule are the same honest-boundary rule at different points") and the same clause in transport.md §fetch. Cost: one existence check per have on the done round — already paid on every ack round; no new budget kind.

---

A-5 [major] — The consumer half (GitSession) is named in five documents and specified in none; its scope is the largest uncut ambiguity for decomposition

Files: docs/architecture/overview.md (crate map: "new, small (TtySession analog)"), docs/architecture/decisions/010-pure-protocol-crate.md, docs/architecture/backend.md §"Public API surface", docs/architecture/transport.md §"Public API surface", docs/architecture/doors.md, docs/architecture/decisions/012-registry-backing-and-ops.md §4 (the dependency rider), Cargo.toml (unconditional gix-protocol = "0.65" + gix-transport = "0.59").

Problem: ADR-010 makes the consumer half a load-bearing half — "GitSession typed client with connect_direct and open_via_channels constructors … the alkcall-native primitive for replication/mirroring in the alknet rewrite" — and every public-API list includes it. But no document says what it does. The reading that it is "small" like TtySession (a typed wrapper over an established byte stream) is doing unexamined work: a git client that can drive fetch/push means client-side protocol state machines (want/have negotiation, V2 request encoding, push command/pack construction, response parsing) — an entire second surface that transport.md does not cover (it is server-side only). ADR-012 §4 explicitly deferred the deciding call — "gix-protocol's response parsing or hand-roll the small client surface; gix-transport is almost certainly droppable. Implementation-time call, recorded, not an architecture commitment" — but the manifest carries both crates unconditionally today (not feature-gated, not optional), so the "pending decision" is already a compile-graph commitment.

Consequence: the decomposer either silently omits the half (leaving ADR-010's producer/consumer claim unimplemented and the manifest overweight) or invents its scope (client protocol machines nobody designed). Both are bad; the decision is cheap now.

Options:

• (a) Thin wrapper in v1 (recommended) — GitSession is what the crate-map words say: connect_direct / open_via_channels constructors, the session preamble (A-3's shape), and raw pkt-line-adjacent access sufficient for alkgit↔alkgit replication to ride it — with client protocol state machines explicitly deferred (recorded as out of scope). This matches the "TtySession analog" reading exactly (TtySession wraps streams; the tty protocol lives in the negotiation layer), keeps gix-protocol as the future client parser, and lets the manifest carry the two deps honestly as the consumer half's declared seam.
• (b) Defer the half entirely — drop gix-protocol/gix-transport from the manifest (real dependency-graph reduction; both pull nontrivial trees), amend ADR-010's crate map and both public-API sections, and record the alknet-replication primitive as a future half. Cleanest scope, but it walks back a headline ADR-010 claim.
• (c) Full client in v1 — make the consumer half real (V2 request encoding, negotiation loop, push construction). This is a second protocol surface with its own capture-testing burden; nothing in the research corpus validates it and no consumer needs it before the alknet rewrite.

Fix: a user scope decision, then either an amendment (a/b) or a small ADR; whichever is chosen, the manifest's two client-side deps become honest (carried with purpose, or dropped).

---

A-6 [major] — The backend traits' execution model is unspecified: async-trait methods on the executor vs sync methods on the blocking pool determines where ADR-009's pipeline budget lives

Files: docs/architecture/backend.md §"Concurrency model" ("impls run under the adapter's tokio context"; GitPackIngest "blocking-thread friendly"), §"The trait family" (GitPackGen/ GitPackIngest signatures), docs/architecture/transport.md §fetch ("generation runs on spawn_blocking with the owned handle moved in"), docs/architecture/decisions/009-bounded-resources-budget.md ("max concurrent blocking pipeline tasks … enforced at assembly/acceptance time"), ADR-013 §6 / ADR-014 §5.

Problem: the corpus describes the execution model three different ways without committing to one. transport.md §fetch states the POC-2 shape in the wire layer's voice ("generation runs on spawn_blocking with the owned handle moved in — store shared, handle per session") — but the wire layer is backend-trait-only (ADR-010): it holds Arc<dyn GitPackGen> and there is no "store" or "handle" in its vocabulary; store/handle/spawn_blocking is the gix impl's internal shape. backend.md says the traits are async (resolve is; A-2's amendment makes all of them #[async_trait]) and impls "run under the adapter's tokio context" while also being "blocking-thread friendly." And ADR-009's blocking-pipeline budget is "enforced at assembly/acceptance time (reject/slow-path excess concurrent generations)" — but who admits: the wire layer around the trait call, or the impl inside it? Two workstreams implementing the same trait from these texts will diverge: one puts spawn_blocking in the wire layer (which then can't, because the trait method is async and the layer is gix-free), one puts it in the impl (and then the wire layer's admission budget has nothing to count).

Consequence: ADR-009's only concurrency budget for the heaviest operations (pack gen/ingest) has no specified enforcement point, and the wire-layer spec carries gix-implementation vocabulary it structurally cannot use.

Fix (small): one paragraph in backend.md's concurrency model (referenced from transport.md), committing to the model the trait signatures already imply: trait methods are #[async_trait]; the wire layer enforces the pipeline-concurrency budget itself (a permit acquired around gen/ingest calls — ADR-009's admission point, now concrete); implementations must not block the async executor and own their internal threading (the gix impls run spawn_blocking with store-shared/handle-per-session inside the impl — POC-2's shape, restated at its true layer). transport.md §fetch's spawn_blocking sentence gets rephrased to the trait-contract version so the wire spec stops speaking gix.

---

D-1 [minor] — Stale superseded text in the docs the decomposer reads first: the "Internal ops over an admin listener" framing survives in vision.md and alk-stack.md, and AGENTS.md's lifecycle list names resolved OQs as active

Files: docs/research/vision.md (threat-model notes: "Registry management ops, if the gix feature ships any, are alkcall Visibility::Internal ops over an admin-only listener"), docs/research/alk-stack.md (gitea-lesson item 2: "Admin API = internal ops … Visibility::Internal ops over the admin interface"), AGENTS.md (Lifecycle Status: "Open architecture questions live in … (OQ-04 receive-pack, OQ-06 registry backing, OQ-08 identity model are the active ones)").

Problem: all three statements are superseded. ADR-012 §3 resolved OQ-07 by rejecting the Internal-ops framing in exactly these words ("right mechanism, wrong axis" — the ops are Visibility::External, gated by scope+ownership, registered by the assembler), and OQ-04/ OQ-06/OQ-08 are all resolved (ADR-013, ADR-012, ADR-011); the active set is OQ-03 (partially resolved) and OQ-05 (deferred). AGENTS.md further declares docs/research/ "the current source of truth for design direction," which makes the stale bullets actively misleading rather than merely historical: a decomposer told to treat research as truth reads two contradicting admin-API designs and a stale OQ list.

Fix (trivial): one-line amendments — vision.md's threat-model bullet and alk-stack.md item 2 get a supersession note pointing at ADR-012 §3 (the research docs are phase-0 records; the note marks the supersession without rewriting history), and AGENTS.md's lifecycle paragraph updates the active-OQ list to OQ-03/OQ-05.

---

D-2 [minor] — ADR-007 step 3 still names AccessControl::check as the enforcement mechanism — superseded in place by ADR-011's authorize policy function, with no note on the older ADR

Files: docs/architecture/decisions/007-acl-before-advertisement.md (step 3: "Run alkcall AccessControl::check(peer_identity) against the repo's required access"), docs/architecture/decisions/011-per-repo-authorization.md §3 (the policy function that replaced it).

Problem: ADR-011 resolved the gap in ADR-007's own context (the static ACL engine "fails closed on identity: None," so it cannot express anonymous-public fetch — verified against alkcall's check(), which returns Forbidden("authentication required") for None with restrictions). ADR-011 records the resolution, but ADR-007's decision text — the doc an implementer reads for the enforcement order — still instructs wiring the wrong mechanism. A literal implementation of ADR-007 breaks anonymous-public fetch, the deployment anchor.

Fix (trivial): an amendment note on ADR-007 step 3 ("mechanism amended by ADR-011 §3: the per-repo check is alkgit-core's authorize policy function evaluated on the registry record; the step order is unchanged") — the same pattern ADR-004 used when ADR-010 amended its layout.

---

D-3 [minor] — The authorized-repo marker ADR-007 promises is missing from the session tuples in transport.md (and backend.md's API list)

Files: docs/architecture/decisions/007-acl-before-advertisement.md ("the session entry points take an authorized-repo marker — a type the adapter constructs only after step 3 passes — so skipping the check is a type error"), docs/architecture/transport.md §substrate (inputs listed without the marker), docs/architecture/backend.md §"Public API surface" (the marker type is absent from the re-export list).

Problem: ADR-007's type-level enforcement is one of the design's better moves (skipping ACL is a compile error, not a runtime log), but the promise is unfindable from the transport spec — the substrate input tuples omit the marker, so the decomposer slicing transport tasks from transport.md will not include it, and the first session- tuple task will "fix" the signature by removing it (or never add it).

Fix (trivial): add the marker to both substrate input tuples in transport.md (and note it in backend.md's public-API list — it is crate-root surface: the door must be able to construct it). Rides the A-3 tuple amendment if that lands first.

---

Part B — Non-findings (verified correct, recorded to bound re-review)

• The feature story is real, not aspirational. All four configurations compile today on the skeleton manifest: default (gix + registry-file), --no-default-features (wire-only), --features sha256, --all-features. ADR-012 §4's feature split and the default-features = false embed claim are verified properties of the current manifest, not promises about future code. The manifest also already encodes the ADR-012 §4 facade-dropping (component crates only: gix-odb/pack/ref/object/fsck — no gix facade dep) and the sha1 compile-time pin.
• alkcall's ACL semantics verify ADR-011's premise exactly. AccessControl::check (spec.rs:94) fails closed on anonymous callers whenever restrictions exist — the precise property ADR-011 §3 cites as why the static engine cannot express anonymous-public fetch. The premise of the per-repo authorization design is confirmed against the real engine, not assumed.
• The alkcall surface the ADRs cite exists as cited: Identity.id as the stable logical id (auth.rs:15, alkcall ADR-025's decoupling), OwnershipStore::record as the async mint (ownership.rs:58, ADR-011), ChannelOpenSpec + openable-ALPNs-are-operations (spec.rs:26, ADR-047), resource_id_path for registry-side ownership extraction, AccessControl's scope/resource composition (the AND-composition that A-1 turns on). alktty's register_openable/establisher/ TtySession template matches ADR-010's producer/consumer claims piece for piece.
• The gitoxide API pins are real. Verified in the reference clone: PreviousValue::MustExistAndMatch (gix-ref transaction CAS), Bundle::write_to_directory_eagerly with thin_pack_base_object_lookup: Option<impl gix_object::Find> (ADR-013 §5/§6's thin-pack composition, exactly the named parameter), gix_validate::reference::name (ADR-013 §9), and — load-bearing for ADR-014 — gix_traverse::commit::Simple::filtered with Predicate: FnMut(&oid) -> bool, the have-boundary traversal A-4's verify-then-subtract rides. The gix-pack feature-set claims (generate, streaming-input, parallel as separate features) match the crate's Cargo.toml. One item remains honestly deferred by the docs themselves: gix-discover + gix_odb::Store::at composition is marked "Verified at implementation" in backend.md — it was not independently verified here, and the docs do not claim it was.
• ADR-013 and ADR-014 are model capture-grounded decisions. Both resolve every listed unknown against real-client behavior (raw stdio into real git receive-pack for policy ground truth; fetch-pack.c source cross-checks), record the failure modes of getting it wrong, bind prior tool decisions (ADR-004) to concrete calls, and file their own backlog riders (oq-13-cas-failfast). transport.md and backend.md are consistent with both ADRs in every detail I compared (advertisement sets, report framing, ack grammar, budget placements).
• Deferral hygiene is clean. Both remaining OQs are properly two-half-tracked: OQ-05 (deferred(scope), ecosystem need) and OQ-03 (partially resolved; the freeze inventory itself) each have the tracker entry in open-questions.md with a concrete blocked-on condition and the machine-readable tasks/architecture/ tracker task. Neither is circular — OQ-03's blocker (first-publish timing) is genuinely downstream of implementation, and OQ-05 waits on external ecosystem need. Resolved-early trackers (OQ-04, OQ-06) are correctly closed with their resolution recorded.
• Cross-reference integrity is exceptional. Every ADR-NNN and OQ-NN reference across the corpus resolves to an existing file, and every sibling-crate citation I checked (alkcall ADR-003/011/017/025/ 040/047, alknet ADR-033/035, alkcall AccessControl/OwnershipStore) says what the citing document claims it says. The superseded pair (ADR-001, ADR-006) is marked Superseded and consistently referenced as such.
• The crate manifest is coherent with the architecture beyond the two findings above: rust-version = "1.88" verified real, the feature comments state their ADR provenance, the dev-deps match the planned test surface (tokio full + test-util), and cargo publish --dry-run completes.

---

Part C — Notes and observations

N-1 [observation] — The advertisement ref-cap's breach behavior is unstated, and truncation is the tempting implementation

ADR-009's table carries "max advertisement refs" but its breach rule ("the session ends with a substrate-appropriate error") was written for the loop/size budgets; a streaming ls-refs implementation hitting the cap will be tempted to stop emitting refs and flush (a silent partial ref list — the nastiest failure mode a ref advertisement can have, since clones appear to succeed). One clause in transport.md (§Limits): the ref cap is fail-closed like every other budget — breach is an error, never a truncation.

N-2 [observation] — The unknown-repo ≡ unauthorized indistinguishability is not stated at the wire-mapping point

ADR-007 step 2 and ADR-008 pin the rule, but transport.md's error taxonomy (the place RegistryError variants get mapped onto pkt-line bands / http statuses) does not mention it. RegistryError::NotFound and an authorization failure must collapse to one wire error at that mapping — one sentence in transport.md §error taxonomy prevents the variant leak.

N-3 [observation] — The freeze inventory's schemas are unpinned: RepoRecord, RegistryError, and the git/repo/* request/response shapes

OQ-03 counts "names + schemas" in the freeze inventory, but nothing specifies the serde shape of RepoRecord (field names, the grants map's value shape, visibility representation), RegistryError's variant set, or the four ops' JSON schemas. Two workstreams would invent them independently (the ops handlers and the registry-file store both serialize the record). One backend.md section (types + schemas, even as draft) closes it before the freeze inventory is taken as complete.

N-4 [observation] — ADR-013 §11's push-options seam has no trait input to surface into

"surfaced to the ingest/refs seam as per-push metadata" — but no backend trait signature carries a per-push metadata input, and push-options are default-off in v1. Pin the additive shape now (e.g. an Option<&PushOptions>-style parameter on the GitPackIngest binding, default None until the config gate opens) so opening the gate later is a signature-additive change, not a trait redesign.

N-5 [observation] — ls-refs=unborn is advertised but unborn-HEAD serving was never capture-verified

Every POC fixture had refs; the advertisement promises ls-refs=unborn (transport.md §advertisement, POC-1-validated as an accepted capability token, not as a served behavior — serving an unborn HEAD's symref line was never exercised against real git). This is the only advertised promise in the corpus without capture evidence. One implementation test (ls-refs against an unborn repo, real client, both substrates) closes it; if it cannot be served, the honest move per ADR-003 is dropping the token.

---

What's Good

• The two newest ADRs are the strongest documents in the corpus. ADR-013 and ADR-014 are exactly what capture-grounded architecture should be: real-client walkthroughs (including raw stdio probes for policy ground truth), source cross-checks recorded with versions, every unknown from the OQ resolved with evidence, consequences stated honestly (including the accepted capability gaps), and backlog riders filed where optimization was declined.
• The structural spine (ADR-010/002/005/007/008/009) has survived four decision cycles without drift — the pure-protocol-crate correction, the feature split, and the two wire ADRs all amend around it cleanly, and the corpus's cross-reference integrity is the best I have traced in the family.
• The honest-capability discipline is consistent everywhere it appears — fetch declination by omission (POC-1-validated), the push served set matched to exactly what the state machine serves (quiet's omission reasoned through, report-status-v2's v1-shape argument capture-backed), and git-upload-archive's fixed refusal carried across every door.
• The deferral protocol worked. Both resolved-early deferrals (OQ-04, OQ-06) record why the deferral dissolved, both remaining deferrals have concrete non-circular blockers, and the two-half tracking (OQ entry + tracker task) is in place for every one of them.

---

Remediation

Nothing is remediated yet — this review is the gate output. Suggested batches for the fix round (each is one session-sized unit; the criticals are ADR-writing work, not code):

ID	Finding	Recommended fix	Effort	Risk	Status
A-1	op-gate OR not expressible in AccessControl	new ADR (or ADR-012 §3 amendment): handler-side two-tier check, create keeps static scope gate	small	none	resolved (ADR-015) — option (a) shape with the OR-term generalized to the manage grant
A-2	async fn traits not dyn-compatible	ADR-012 §1 + backend.md amendment: #[async_trait]; add async-trait = "0.1" to manifest	small	none	resolved — all five traits #[async_trait], desugared boxed form pinned in the freeze inventory (OQ-03), dep in manifest
A-3	native preamble / service dimension unpinned	new ADR: open-op params {repo, service}, session tuple + stateless entry gain the service selector, GitAdapter preamble pinned	moderate	wire-format (freeze inventory)	resolved (ADR-016) — {repo, service} open-op params (channels/git/sub, additionalProperties: false), git-daemon request line on the direct path (POC-1 verbatim, freeze inventory), service in both substrate tuples, GitSession mirrors the shapes
A-4	done-round boundary set unverified	ADR-014 §2 + transport.md clause: boundary = common_haves-filtered haves	trivial	none	resolved — boundary set is the recognized subset (common_haves-filtered), amendment clause in ADR-014 §2 + transport.md §fetch
A-5	consumer half unspecified	user scope decision, then amendment or small ADR (recommended: thin wrapper, deps carried with purpose)	small	scope	resolved (ADR-017) — superseding the thin-wrapper recommendation: GitSession is a real typed client in v1 (ls_refs/fetch/push), grounded in the two deployment use cases (the p2p replicator is the named downstream and needs the client protocol layer); fetch reuses gix-protocol over a custom alkcall Transport, push is hand-rolled to ADR-013's shapes (gitoxide has no send-pack), storage-agnostic
A-6	trait execution model unspecified	backend.md paragraph + transport.md rephrase: async traits, wire-layer permit, impl-internal spawn_blocking	small	none	resolved — backend.md concurrency model: wire layer enforces the ADR-009 permit around gen/ingest trait calls; impls own internal spawn_blocking (ADR-009/ADR-013 aligned)
D-1	stale Internal-ops + OQ-list text	supersession notes (vision, alk-stack, AGENTS)	trivial	none	resolved — supersession notes in vision.md (with ADR-015) and alk-stack.md item 2; AGENTS.md active-OQ list updated to OQ-03/05/16
D-2	ADR-007 step-3 mechanism superseded	amendment note on ADR-007	trivial	none	resolved — amendment note on ADR-007 step 3 (mechanism → ADR-011 authorize; step order unchanged)
D-3	authorized-repo marker missing from tuples	add to transport.md tuples + backend.md API list	trivial	none	resolved — marker added to both substrate input tuples (transport.md) and backend.md public-API list
N-1	ref-cap breach behavior	one fail-closed clause in transport.md	trivial	none	resolved — ref cap fail-closed clause in transport.md §Limits (breach is an error, never truncation)
N-2	unknown ≡ unauthorized at wire mapping	one sentence in transport.md error taxonomy	trivial	none	resolved — collapse rule stated at the variant→wire mapping in transport.md §error taxonomy
N-3	schemas unpinned	backend.md types/schemas section	small	freeze inventory	resolved — backend.md §"Registry types and schemas": RepoRecord serde shape (three-action grants, opaque keys, storage-root omitted from all op responses), the five-variant RegistryError set with wire mappings, and the four git/repo/* op request/response schemas (additionalProperties: false, git:repo:* error codes)
N-4	push-options seam	pin additive parameter shape	trivial	none	resolved — GitPackIngest's prepare binding carries push_options: Option<&PushOptions> (parsed (key, value) pairs, verbatim and un-interpreted; None until the config gate opens) — pinned in ADR-013 §11 + backend.md trait description, so opening the gate is value-additive, not a trait redesign
N-5	ls-refs=unborn unverified	implementation-phase test rider (record in transport.md or a task)	trivial	none	resolved (rider) — unborn-HEAD verification recorded in transport.md §ls-refs and tracker task tasks/architecture/n5-unborn-head-rider.md (unborn fixture, real client, both substrates; drop the token if it cannot be served — ADR-003)

Suggested sequencing: (1) A-2 + A-6 together (one trait-surface amendment set + manifest change — they are the same signature surface); (2) A-3 (the one new wire-format ADR; A-5's decision unblocks with it); (3) A-1 (the op-gate ADR); (4) A-4 + D-2 + D-3 + N-1 + N-2 (one doc batch — all transport/ADR amendments, no new decisions); (5) A-5 (user decision, then the doc change); (6) D-1 + N-3 + N-4 + N-5 (hygiene batch). After the batch that resolves each spec's findings, the spec docs are candidates for reviewed status per the lifecycle definition, and decomposition can start.

---

References

• ADR-010/002/005 (the structural spine A-3 amends), ADR-012 (§1/§3 — A-2/A-1's subject), ADR-013/014 (A-4's subject; verified sound), ADR-007/008/011 (the auth chain — D-2/D-3), ADR-009 (N-1, A-6's budget), ADR-003 (honest advertisement — N-5)
• alkcall src/registry/spec.rs (AccessControl::check — A-1's mechanism evidence), src/core/auth.rs, src/core/ownership.rs; alkcall ADRs 003/011/017/025/040/047 (all citations verified)
• alktty src/backend.rs/src/channels.rs/src/session.rs (the async-trait + register_openable + TtySession template)
• gitoxide reference clone /workspace/gitoxide (gix-ref PreviousValue, gix-pack Bundle::write_to_directory_eagerly, gix-traverse Simple::filtered, gix-validate reference::name, gix-pack feature set)
• docs/research/poc-1-findings.md (the in-band request-line shape — A-3's direct-ALPN evidence), push-captures.md / negotiation-captures.md (ADR-013/014's normative basis)
• The house format: alksocks docs/reviews/001-rfc1928-full-review.md (structure, severity legend, non-findings discipline), alkty docs/reviews/001-code-review.md (status conventions)