docs(reviews): architecture pre-decomposition review — 3 critical spec gaps
Full-corpus gate review between the completed OQ cycle (ADR-013/014) and phase-2 decomposition. Verified external API claims against real sources (alkcall AccessControl/OwnershipStore semantics, alktty template, gitoxide pins), probed the feature matrix (all four configs compile) and the trait dyn-compatibility claim (E0038 repro on 1.88 and 1.94). Findings: - A-1 (critical): ADR-012 §3's scope-OR-ownership op gate is not expressible in alkcall's AccessControl (restrictions compose as AND) — handler-side two-tier check recommended - A-2 (critical): bare async fn traits are not dyn-compatible (E0038) — ADR-012 §1 signatures need #[async_trait] + dep - A-3 (critical): native path has no pinned session preamble — open-op params carry no service, so the open-time ACL point cannot run the write-tier check and push is unservable over alk/git - A-4..A-6 (major): done-round boundary set should be the common_haves-filtered subset; consumer half (GitSession) named in five docs, specified in none; backend trait execution model unspecified - D-1..D-3 (minor): stale superseded text (vision/alk-stack Internal-ops framing, AGENTS.md OQ list, ADR-007 step-3 mechanism) - N-1..N-5: ref-cap breach rule, error-indistinguishability at the wire mapping, freeze-inventory schemas, push-options seam, ls-refs=unborn never capture-verified Non-findings record what verified sound (feature story, gitoxide API pins, deferral hygiene, cross-reference integrity). Remediation table proposes six fix-round batches; A-5 needs a user scope decision. Verification: cargo test/clippy/fmt/doc clean; check under default/no-default/sha256/all-features and MSRV 1.88 all clean; publish dry-run completes; git 2.43.0 present for integration tests.
This commit is contained in:
1 parent
d6d013e522
commit
d478361a15
1 file changed
+808
@@ -0,0 +1,808 @@
|
||||
---
|
||||
status: open
|
||||
last_updated: 2026-09-25
|
||||
reviewed_artifacts:
|
||||
- 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
|
||||
tool: 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
|
||||
reviewer: architecture pre-decomposition review (contradiction + gap + staleness pass)
|
||||
scope_note: 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`:
|
||||
|
||||
```rust
|
||||
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 | open |
|
||||
| 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 | open |
|
||||
| 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) | open |
|
||||
| A-4 | done-round boundary set unverified | ADR-014 §2 + transport.md clause: boundary = `common_haves`-filtered haves | trivial | none | open |
|
||||
| A-5 | consumer half unspecified | user scope decision, then amendment or small ADR (recommended: thin wrapper, deps carried with purpose) | small | scope | open |
|
||||
| A-6 | trait execution model unspecified | backend.md paragraph + transport.md rephrase: async traits, wire-layer permit, impl-internal spawn_blocking | small | none | open |
|
||||
| D-1 | stale Internal-ops + OQ-list text | supersession notes (vision, alk-stack, AGENTS) | trivial | none | open |
|
||||
| D-2 | ADR-007 step-3 mechanism superseded | amendment note on ADR-007 | trivial | none | open |
|
||||
| D-3 | authorized-repo marker missing from tuples | add to transport.md tuples + backend.md API list | trivial | none | open |
|
||||
| N-1 | ref-cap breach behavior | one fail-closed clause in transport.md | trivial | none | open |
|
||||
| N-2 | unknown ≡ unauthorized at wire mapping | one sentence in transport.md error taxonomy | trivial | none | open |
|
||||
| N-3 | schemas unpinned | backend.md types/schemas section | small | freeze inventory | open |
|
||||
| N-4 | push-options seam | pin additive parameter shape | trivial | none | open |
|
||||
| N-5 | `ls-refs=unborn` unverified | implementation-phase test rider (record in transport.md or a task) | trivial | none | open |
|
||||
|
||||
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)
|
||||
Reference in new issue
Block a user