diff --git a/tasks/implementation/core-consumer-associate.md b/tasks/implementation/core-consumer-associate.md new file mode 100644 index 0000000..729f8eb --- /dev/null +++ b/tasks/implementation/core-consumer-associate.md @@ -0,0 +1,79 @@ +--- +id: implementation/core-consumer-associate +name: "Core — consumer ASSOCIATE: AssociateSession + e2e tests" +status: pending +depends_on: [implementation/core-consumer-connect, implementation/core-producer-associate] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Implement the UDP ASSOCIATE consumer session in `src/consumer.rs` +per consumer.md §The associate session (POC #2's shape): the typed +client that opens the channel, receives the sentinel, and speaks the +framed datagram stage. + +Deliverables (port from `/workspace/alksocks-udp-poc/src/consumer.rs`, +adapted to the crate's wire codec): + +1. **`AssociateSession::associate(client)`**: opens the channel, + negotiates (NoAuth), sends ASSOCIATE, reads the reply — + **receives the sentinel BND.ADDR and never uses it for + addressing** (the handle is the channel; ADR-003). +2. **`send_datagram(dst, bytes)`**: frames once + (`[len: u16 BE][addr + data]` via the crate's wire codec — the + framing-once mirror) and writes the frame. +3. **`recv_datagram()`** returns `(source_addr, bytes)` per + datagram — **the incremental reader state (mid-frame position + + batched-datagram pending queue) persists across calls**: a + datagram split across channel chunks survives a caller-driven + receive loop (POC #2's finding — the first cut dropped the reader + state per call; the 2-byte-prefix chunk test exposed it). Write + the persistence test (the POC's chunk-splitting echo shape). +4. **`shutdown()`** half-closes the stream — the mux's implicit-EOF + sentinel reaches the producer's stage as EOF and the association + ends (association lifetime = channel lifetime). +5. **Teardown ownership**: the session owns the channel; on Drop it + closes/joins (alktunnels ADR-005 posture). No `Clone` + (compile-asserted). +6. **Tests** (end-to-end through the harness): associate → + send/recv round-trips (multiple in-order datagrams, one + association); the split-datagram persistence test; empty-payload + datagram round-trip; shutdown → the producer's stage ends and a + fresh associate succeeds immediately (the POC's test 4, from the + consumer side); Drop teardown (drop mid-stream → the producer's + stage sees EOF, the connection stays healthy); CONNECT + + ASSOCIATE on the same registration (the regression guard + extends). + +## Acceptance Criteria + +- [ ] `AssociateSession` with `associate`/`send_datagram`/ + `recv_datagram`/`shutdown` per the POC shape +- [ ] Reader state persists across `recv_datagram` calls (the + split-datagram test) +- [ ] Framing-once mirror (send frames exactly once; recv decodes + incrementally) +- [ ] Teardown ownership on Drop; no `Clone` (compile-asserted) +- [ ] E2E tests through the harness (round-trips, shutdown, Drop + teardown, regression guard) +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/consumer.md §The associate session +- docs/architecture/decisions/003-udp-handle-swap.md (the consumer + mirror invariants) +- POC source: `/workspace/alksocks-udp-poc/src/consumer.rs` + (`UdpAssociation` — the reader-state finding's origin) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-consumer-bind.md b/tasks/implementation/core-consumer-bind.md new file mode 100644 index 0000000..543835c --- /dev/null +++ b/tasks/implementation/core-consumer-bind.md @@ -0,0 +1,72 @@ +--- +id: implementation/core-consumer-bind +name: "Core — consumer BIND: BindSession (reply#2 reader) + e2e tests" +status: pending +depends_on: [implementation/core-consumer-connect, implementation/core-producer-bind] +scope: narrow +risk: medium +impact: component +level: implementation +--- + +## Description + +Implement the BIND consumer session in `src/consumer.rs` per +consumer.md §The BIND client (POC #5's `read_reply2` sketch): the +wrapper-aware client that opens the channel, sends BIND, reads +reply#1, awaits the inbound accept, and reads reply#2 before riding +the pass-through data plane. + +Deliverables (port from `/workspace/alksocks-bind-poc/src/consumer.rs`, +adapted to the crate's error surface): + +1. **`BindSession::bind(client, target)`**: opens the channel, + negotiates, sends the BIND command — the forked client + `request()` reads reply#1 (carrying BND.ADDR). +2. **The reply#2 reader (`await_inbound`)**: reply#2 is hand-read + with the forked public `read_address` codec (reply#1's codec + applied again — **no second codec**, ADR-011's rationale). The + session then rides the pass-through data plane + (`BindDataPlane`-shaped: `into_inner()`/`get_socket_mut()`). +3. **Error surface**: a refused BIND (the command gate's 0x02, or a + factory failure's reply#1 code) surfaces as a typed session + error carrying the RFC reply code; accept-stage failures surface + at `await_inbound` as reply#2's mapped code. +4. **Teardown ownership**: the session owns the channel; on Drop it + closes/joins. No `Clone` (compile-asserted). +5. **Tests** (end-to-end through the harness): bind → reply#1 + (BND.ADDR as the harness accept advertises) → reply#2 (the peer + address) → pass-through echo both directions; the command-gate + refusal surfaced as a typed error (a `socks5:open`-only identity + cannot bind); factory-failure and accept-stage-failure mappings + surfaced; quiet-wait garbage from the client side ends the + session; the three-command regression guard from the consumer + side. + +## Acceptance Criteria + +- [ ] `BindSession` with `bind`/`await_inbound`/data-plane accessors + per the POC shape +- [ ] Reply#2 read via the forked `read_address` codec (no second + codec) +- [ ] Typed error surface (reply-code-carrying session errors) +- [ ] Teardown ownership on Drop; no `Clone` (compile-asserted) +- [ ] E2E tests through the harness (round-trip, gate refusal, + failure mappings, regression guard) +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/consumer.md §The BIND client +- docs/architecture/decisions/007-bind-in-base.md (the client-side + phase reading) +- POC source: `/workspace/alksocks-bind-poc/src/consumer.rs` + (`BindReply1`/`BindDataPlane`/`read_reply2`) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-consumer-connect.md b/tasks/implementation/core-consumer-connect.md new file mode 100644 index 0000000..74af9a4 --- /dev/null +++ b/tasks/implementation/core-consumer-connect.md @@ -0,0 +1,77 @@ +--- +id: implementation/core-consumer-connect +name: "Core — consumer half: Socks5ChannelSession (CONNECT) + e2e tests" +status: pending +depends_on: [implementation/core-producer-connect] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Implement the CONNECT consumer session in `src/consumer.rs` per +consumer.md §The Channels-Native Sessions: `Socks5ChannelSession` — +the typed client that opens `channels/socks5/sub` and drives the +client side of the RFC conversation in-stream (POC #1's +`use_stream` shape). + +Deliverables (port from `/workspace/alksocks-connect-poc/src/ +consumer.rs`, adapted to the crate's params/error surface): + +1. **`Socks5ChannelSession::connect(client, target)`**: opens the + channel via `ChannelClient::open_channel(OP_SOCKS5_OPEN, params, + SOCKS5_ALPN)` (trivial params — ADR-001), then drives the forked + client typestate's `use_stream` path over the channel's + `BiStream` (generic over the backing stream — the POC #1 fit: + `Socks5Stream` impls `AsyncRead + AsyncWrite` itself, so the + session over a `BiStream` is the intended use). +2. **Target construction** via `ToTargetAddr` (IP-string forms parse + to IP targets; domain form passes through unresolved — ADR-012). +3. **Post-reply payload pipe**: after the reply, the session is a + raw byte pipe — expose `split()` (read/write halves) and + `into_inner()` (the underlying `Socks5Stream` for + advanced use). 0 B framing (CONNECT). +4. **Teardown ownership** (alktunnels ADR-005 posture, inherited): + the session owns the channel; on Drop it closes/joins. No + `Clone` (compile-asserted — aliasing a session would alias its + teardown; the alktunnels `compile_fail` doctest shape). +5. **Error surface**: open failures are the typed + `SocksOpenError` reasons (ADR-049 §4 — a refused session never + yields a phantom channel); a refused target is an RFC reply code + surfaced through the session type (consumer.md §Error surface). +6. **Tests** (end-to-end through the harness): connect → echo + round-trip (payload bytes both directions); refused dial → the + RFC reply code surfaced as a typed session error; FORBIDDEN open + → typed open error, no session; half-close semantics (dropping + the write half EOFs the producer's pump cleanly); domain-form + target passes through unresolved (assert the wire bytes, the + echo dial ignores the target). + +## Acceptance Criteria + +- [ ] `Socks5ChannelSession` with `connect`/`split`/`into_inner` + per the POC shape +- [ ] Teardown ownership on Drop; no `Clone` (compile-asserted) +- [ ] Typed error surface (open errors vs reply-code refusals) +- [ ] E2E tests through the harness (round-trip, refusals, + half-close, domain pass-through) +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/consumer.md §The Channels-Native Sessions +- docs/architecture/decisions/003-udp-handle-swap.md (the framing + mirror — CONNECT is the 0 B case) +- POC source: `/workspace/alksocks-connect-poc/src/consumer.rs` +- alktunnels template: `/workspace/@alkdev/alktunnels/src/consumer.rs` + (the teardown-ownership + no-Clone posture) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-crate-deps.md b/tasks/implementation/core-crate-deps.md new file mode 100644 index 0000000..5e722c1 --- /dev/null +++ b/tasks/implementation/core-crate-deps.md @@ -0,0 +1,81 @@ +--- +id: implementation/core-crate-deps +name: Core — crate dependencies + module skeleton (alkcall wiring) +status: pending +depends_on: [] +scope: narrow +risk: low +impact: project +level: implementation +--- + +## Description + +Add the dependency surface the wrapper layers need and the module +skeleton so the batch-1 core tasks have their targets. This is the +batch-1 analogue of the vendor scaffolding task: the crate gains +`alkcall` (its substrate — first consumption of the 0.8.0 surface) and +the module files named in overview.md §Module Map, born wasm-clean. + +Deliverables: + +1. **`Cargo.toml` `[dependencies]`** — add to the existing trio + (tokio wasm-clean subset, tracing, thiserror): + - `alkcall = "0.8.0"` (pinned per AGENTS.md convention 11 — bump + deliberately, never transitively) + - `serde = { version = "1", features = ["derive"] }`, + `serde_json = "1"` (the params schema + spec builder) + - `bytes = "1"` (the wire codec's buffer types) + - `futures = { version = "0.3", default-features = false, + features = ["std"] }` (BoxFuture for the seam callbacks — the + alktunnels precedent; verify the wasm-clean subset compiles) + - tokio stays `default-features = false` with the wasm-clean + subset (`rt`, `sync`, `io-util`, `macros`, `time`) — **never + `features = ["full"]`** (AGENTS.md convention 4). No + `tokio/net`, no `socket2` — those ride `local` in batch 2. + - `[features]` stays `default = []`, `local = []`, + `composed = []` — declared empty by design (ADR-009); populated + by their modules' batch-2 tasks. +2. **`src/lib.rs`** — keep `#![forbid(unsafe_code)]` and the crate + docs; declare the module skeleton per overview.md §Module Map: + `params`, `wire`, `producer`, `consumer`, `egress`, `error` — + one-line stub files, each with a module doc naming its filling + task (the vendor-scaffolding precedent). `rfc1928` stays as is. +3. **Re-export posture**: `lib.rs` re-exports public API per + convention 15 — for now the module declarations only; each + implementation task adds its own re-exports. + +## Acceptance Criteria + +- [ ] `cargo test` passes +- [ ] `cargo clippy --all-targets -- -D warnings` clean +- [ ] `cargo fmt --check` clean +- [ ] `cargo check --target wasm32-unknown-unknown` clean — the wasm + gate stays green with alkcall + serde + bytes + futures in the + graph (alkcall 0.8.0's default crate is wasm-clean; verify, do + not assume) +- [ ] `cargo clippy --target wasm32-unknown-unknown -- -D warnings` + clean +- [ ] No `tokio/net`, `socket2`, `mio` in the default dependency + graph (`cargo tree` spot-check) +- [ ] `alkcall` pinned at `"0.8.0"` (not a range) +- [ ] Module skeleton matches overview.md §Module Map (module names + and file layout) + +## References + +- docs/architecture/overview.md §Dependencies, §Module Map +- docs/architecture/decisions/009-feature-gates.md (the feature + layout; no `net` feature) +- AGENTS.md conventions 4 (tokio subset), 11 (alkcall pin), 15 + (module structure) +- Sibling posture: `/workspace/@alkdev/alktunnels/Cargo.toml` + (alkcall 0.8.0 consumer; the futures/bytes/serde dep set) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-integration-suite.md b/tasks/implementation/core-integration-suite.md new file mode 100644 index 0000000..0ac1dc7 --- /dev/null +++ b/tasks/implementation/core-integration-suite.md @@ -0,0 +1,86 @@ +--- +id: implementation/core-integration-suite +name: Core — cross-cutting integration suite + docs sync +status: pending +depends_on: [implementation/core-consumer-associate, implementation/core-consumer-bind] +scope: moderate +risk: medium +impact: phase +level: implementation +--- + +## Description + +The batch-1 closing task: the cross-cutting integration suite that +pins the whole protocol core's behavior in one place, plus the docs +sync that records what landed. The per-command tasks own their +verticals; this task owns the cross-cutting invariants and the +spec-vs-implementation reconciliation. + +Deliverables: + +1. **Cross-cutting integration tests** (`tests/`): + - **One registration, all three commands** (the POC #2/#5 + regression guard, now complete): CONNECT + ASSOCIATE + BIND all + served on the same `channels/socks5/sub` op, interleaved + sessions on one connection. + - **Concurrency**: multiple concurrent sessions of mixed command + types over one channels connection (the mux/demux under load — + the POCs tested sequentially; this pins concurrency). + - **Teardown semantics matrix**: client half-close ends a CONNECT + data plane cleanly (the producer's pump exits, the connection + stays healthy); ASSOCIATE EOF ends the association; BIND + client-gone teardown releases the accept; a fresh session + succeeds immediately after each teardown (no connection + poisoning). + - **Identity/scope matrix**: `socks5:open`-only (CONNECT + + ASSOCIATE yes, BIND 0x02), `socks5:bind`-only (BIND yes, + CONNECT/ASSOCIATE FORBIDDEN at the open gate), both-scopes + (everything) — the two-grants-never-implied posture (ADR-006) + pinned end to end. + - **The readiness contract** re-pinned at the integration level + (the producer-associate task owns the unit-level test; this + pins it under concurrent load). +2. **Docs sync** (the architecture-sync check): + - `docs/architecture/README.md` Current State + the doc statuses + reflect the landed core (the rfc1928 doc's "remaining layers + follow the decomposition phase" line updated). + - Any spec drift the implementation surfaced (naming, shapes, + the wire.md/bast.md residue already fixed in the wire-codec + task) reconciled in the same change — never silently diverged. + - `lib.rs` crate docs updated to describe the landed surface + (producer + consumer + wire + seams), pointing at the specs. +3. **Full verification sweep** (the batch gate's input): + `cargo test`, `cargo test --all-features` (features are still + empty — verify the flag combination compiles), clippy + `--all-targets -- -D warnings`, `cargo fmt --check`, both wasm + checks, and `cargo doc --no-deps` (docs changed). + +## Acceptance Criteria + +- [ ] The three-command regression + concurrency + teardown-matrix + + identity-matrix tests all pass +- [ ] Docs reconciled (README Current State; any drift fixed) +- [ ] The full verification sweep is green (test, all-features, + clippy, fmt, both wasm checks, doc) +- [ ] No `local`/`composed` code exists yet (the batch boundary + holds — the default crate is protocol-only) + +## References + +- docs/architecture/wire.md §Sentinels and lifecycle (the teardown + matrix's normative source) +- docs/architecture/decisions/006-access-control.md (the identity + matrix) +- POC regression guards: `udp_composed_egress_round_trip`-adjacent + tests in `/workspace/alksocks-udp-poc`, test 7 in + `/workspace/alksocks-bind-poc` +- docs/sdd_process.md (the architecture-sync-check posture) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-params-errors.md b/tasks/implementation/core-params-errors.md new file mode 100644 index 0000000..f3d8948 --- /dev/null +++ b/tasks/implementation/core-params-errors.md @@ -0,0 +1,88 @@ +--- +id: implementation/core-params-errors +name: Core — params schema + scope constants + typed error surface +status: pending +depends_on: [implementation/core-crate-deps] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Implement `src/params.rs` and `src/error.rs` per wire.md §The Open +Op, ADR-001, and ADR-006 — the wire-stable surface (the one-way door) +and the typed open-error vocabulary. Template: +`/workspace/@alkdev/alktunnels/src/params.rs` (the sibling's spec +builder + scope-constant posture). + +Deliverables: + +1. **`src/params.rs`**: + - Constants: `OP_SOCKS5_OPEN = "channels/socks5/sub"`, + `SOCKS5_ALPN = "alk/socks5"`, `SOCKS5_OPEN_SCOPE = + "socks5:open"`, `SOCKS5_BIND_SCOPE = "socks5:bind"` (ADR-001/ + ADR-006; the scope-string convention `: + `; stable once published). + - `socks5_open_spec() -> OperationSpec` — `Sub`-typed, external, + channel-open marker on `SOCKS5_ALPN`, **trivial params**: + input schema `{type: object}`, `additionalProperties: false` + (empty params stay valid forever — ADR-001's additive-only + extension rule); reply schema `{"channel_id": integer, + minimum 1}`; `required_scopes = ["socks5:open"]`. Follow the + alktunnels spec-builder shape exactly (the registry validates + against these schemas). + - The trivial params struct (`Socks5Params {}` — serde + `deny_unknown_fields`, empty) for the consumer's open call. +2. **`src/error.rs`** — the typed open-error surface (alkcall + ADR-049 §4): + - `SocksOpenError` — the consumer-side open-failure vocabulary + mapping the `channel:open_failed` reasons (`dial_failed` / + `unknown_resource` / `resource_shortage` / `handler_error` / + `timeout`) plus the registry's `FORBIDDEN`/`INVALID_INPUT` + classes, shaped on alktunnels' `TunnelOpenError` + + `establishment_reason` helper. + - The session-level error type for the consumer sessions + (`SocksSessionError`-shaped: open errors vs in-stream + `SocksError` vs I/O) — the exact variant set is the task's + judgment; keep it faithful to what the sessions actually + surface (a refused session never yields a phantom channel; a + refused target is an RFC reply code surfaced through the + session type — consumer.md §Error surface). +3. **lib.rs re-exports** for the new public surface (convention 15). + +Tests: spec-builder shape assertions (op id, ALPN marker, scopes, +schemas — the trivial-params schema rejects unknown fields), params +serde round-trip (empty object valid; unknown field rejected), the +error-reason mapping table. + +## Acceptance Criteria + +- [ ] Constants + spec builder per ADR-001/ADR-006 exactly (op id, + ALPN, both scopes, trivial schemas) +- [ ] Input schema rejects unknown fields (the loud-rejection + posture) +- [ ] Typed open-error surface maps the ADR-049 §4 reasons +- [ ] lib.rs re-exports the public surface +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/wire.md §The Open Op (the normative op shape) +- docs/architecture/decisions/001-alpn-and-open-op.md (trivial + params; the one-way door) +- docs/architecture/decisions/006-access-control.md (the two scopes) +- docs/architecture/decisions/011-inline-vendor.md §4 (the typed + open-error surface) +- Template: `/workspace/@alkdev/alktunnels/src/params.rs` + (`tunnel_open_spec`, the scope constants, `establishment_reason`) +- POC sketch: `/workspace/alksocks-connect-poc/src/params.rs` + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-producer-associate.md b/tasks/implementation/core-producer-associate.md new file mode 100644 index 0000000..7ee7c69 --- /dev/null +++ b/tasks/implementation/core-producer-associate.md @@ -0,0 +1,93 @@ +--- +id: implementation/core-producer-associate +name: "Core — producer ASSOCIATE: the select!-driven datagram stage" +status: pending +depends_on: [implementation/core-wire-codec, implementation/core-producer-connect] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Implement the UDP ASSOCIATE producer path in `src/producer.rs` per +wire.md §UDP ASSOCIATE and ADR-003: replace the dispatch arm's +refusal with the datagram stage (POC #2's validated scope). + +Deliverables (port from `/workspace/alksocks-udp-poc/src/producer.rs` ++ `egress.rs`, adapted to the crate's wire codec and seam types): + +1. **The ASSOCIATE arm**: `reply_success(sentinel)` — the sentinel + `127.0.0.1:0` BND.ADDR (pure RFC-compatibility surface; the + channel is the relay handle, ADR-003) — then the datagram stage. +2. **The datagram stage** — `select!`-driven (channel reads ∥ egress + replies), with the POC's three load-bearing findings as + invariants: + - **The concurrent select shape** (not poll-then-block — the + poll-then-block window loses replies; the concurrent shape is + also the honest full-duplex shape). + - **The egress readiness contract**: `next_reply` is `Pending` + when nothing is ready — an instantly-ready `Ok(None)` starves a + biased select into livelock. **Write the contract test**: an + egress returning instant `Ok(None)` must not starve the stage's + channel branch (the POC's load-bearing API finding, re-pinned + by this suite per ADR-004's "re-pinned by the crate's test + suite"). + - **Framing applied exactly once, by the wire layer**: the stage + reads frames via `DatagramReader` (channel → egress `send` + gets the payload) and writes frames via `frame()` (egress + `next_reply`'s inner bytes → the channel). The egress never + sees a length prefix. + - **Association lifetime = channel lifetime**: channel EOF ends + the stage; no extra control vocabulary. A undecodable frame + (garbage ATYP) fails the association loud + (`udp_undecodable_frame_ends_association`). +3. **The `EgressFactory` lifecycle**: called once per ASSOCIATE; the + stage owns the egress; stage exit drops it. Ownership rides the + handle structurally — no `_` bindings of owning values (the POC's + teardown-at-birth witness). +4. **Missing-egress refusal**: a registration without an + `EgressFactory` refuses ASSOCIATE RFC-faithfully + (`CommandNotSupported`-class) — never a panic (ADR-008). +5. **Tests** (via the harness + raw RFC client): associate → + sentinel reply → datagram round-trip through the echo egress + (in-order, multiple datagrams, one association); empty-payload + datagram legal (not EOF); undecodable frame ends the association + loud; channel EOF ends the association and the connection stays + healthy (a fresh associate succeeds immediately after — the POC's + test 4); the readiness-contract test; missing-egress refusal; + CONNECT still served on the same registration (the regression + guard). + +## Acceptance Criteria + +- [ ] The datagram stage per the three POC invariants (concurrent + select, readiness contract, framing-once) +- [ ] Sentinel BND.ADDR in the reply; association lifetime = channel + lifetime (EOF teardown tested) +- [ ] Readiness-contract test present (instant-Ok(None) egress does + not starve the stage) +- [ ] Missing-egress refusal RFC-faithful +- [ ] CONNECT regression guard passes (one registration, both + commands) +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/wire.md §UDP ASSOCIATE (the normative stage) +- docs/architecture/decisions/003-udp-handle-swap.md (the handle + swap; the readiness + framing invariants) +- POC sources: `/workspace/alksocks-udp-poc/src/producer.rs` + (`serve_udp_associate`), `/workspace/alksocks-udp-poc/src/ + egress.rs` (the readiness findings' origin) +- docs/architecture/decisions/008-provider-defaults.md (the + missing-callback posture) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-producer-bind.md b/tasks/implementation/core-producer-bind.md new file mode 100644 index 0000000..6af8a9f --- /dev/null +++ b/tasks/implementation/core-producer-bind.md @@ -0,0 +1,103 @@ +--- +id: implementation/core-producer-bind +name: "Core — producer BIND: the phase model + the socks5:bind command gate" +status: pending +depends_on: [implementation/core-producer-connect] +scope: moderate +risk: medium +impact: component +level: implementation +--- + +## Description + +Implement the TCP BIND producer path in `src/producer.rs` per +wire.md §BIND and ADR-007: replace the dispatch arm's refusal with +the BIND phase model (POC #5's validated scope), plus the +`socks5:bind` command gate (ADR-006). + +Deliverables (port from `/workspace/alksocks-bind-poc/src/producer.rs` ++ `accept.rs`, adapted to the crate's seam types): + +1. **The command gate**: at BIND dispatch, require `socks5:bind` + from the per-call opener identity (the open-op `AuthContext`'s + identity — CF-005/CF-006). An identity without it gets the + RFC-faithful `ConnectionNotAllowed` (0x02) refusal while CONNECT + still serves the same identity (POC #5 test 4). The gate is the + open-op identity seam consumed at command-dispatch time — no new + ops, no params change. +2. **The BIND phase model**: + - **reply#1 carries BND.ADDR** — the `BindAcceptFactory` is + called first; its `BindAccept.bind_addr` is reply#1's carrier + (the local variant's real listener; the sentinel on the + wrapper-aware in-band path when no factory is wired — ADR-007 + variant 3). + - **The quiet wait**: `select!`-single-shot with exactly two + exits — the one inbound accept (the `BindAccept` future), or + the control stream's EOF/garbage/error (the client-gone + teardown riding the accept future's drop). **Not a loop** (the + POC's `never_loop` finding — exactly two exits). Any byte in + the quiet wait is protocol garbage → the session ends with no + data plane. + - **reply#2 carries the peer address** (the accepted handle's + peer; sentinel `0.0.0.0:0` wrapper-aware). + - **Phase 2 is raw pass-through** (`pump_bidi` inline — the + CONNECT shape) between the channel and the accepted halves. +3. **Failure mapping is RFC-faithful end to end**: factory failure + (pre-reply#1) → reply#1 with the mapped code; accept-stage + failure (post-reply#1) → reply#2 with the mapped code, no data + plane. No phantom listeners, no phantom data planes. +4. **The establishment bound covers only the open-op round trip** + (ADR-007): the accept wait lives in the teardown-governed pump + phase — the quiet wait must not run under the establisher's + 10s bound (FTP-length BIND waits stop resolving as spurious + establishment refusals). +5. **Missing-accept refusal**: a registration without a + `BindAcceptFactory` serves the wrapper-aware in-band default + (sentinel reply#1 + sentinel reply#2 — ADR-007 variant 3) or + refuses — the ADR names variant 3 as the in-band default when + neither provider is wired; implement variant 3 as the + no-factory default and document the choice. +6. **Tests** (via the harness + raw RFC client): BIND → reply#1 → + (the harness accept resolves) → reply#2 with the peer → + pass-through echo both directions; the command gate (a + `socks5:open`-only identity gets 0x02 on BIND while CONNECT + still works — the POC's test 4); quiet-wait garbage byte ends + the session with no data plane; client-gone teardown (EOF + mid-wait drops the accept future — a fresh dial to the + advertised address fails); factory failure → reply#1 mapped + code; accept-stage failure → reply#2 mapped code, no data plane; + CONNECT + ASSOCIATE regression guard (one registration, all + three commands). + +## Acceptance Criteria + +- [ ] The command gate: `socks5:bind` required at BIND dispatch; + `ConnectionNotAllowed` refusal; CONNECT unaffected (tested) +- [ ] The phase model: reply#1 → quiet wait (single-shot, two + exits) → accept → reply#2 → `pump_bidi` pass-through +- [ ] Two-stage failure mapping (factory → reply#1; accept-stage → + reply#2), tested +- [ ] The accept wait is NOT under the establishment bound +- [ ] No-factory default = wrapper-aware variant 3 (documented) +- [ ] Three-command regression guard passes +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/wire.md §BIND (the normative phases) +- docs/architecture/decisions/007-bind-in-base.md (the phase model + + the three variants) +- docs/architecture/decisions/006-access-control.md (the command + gate) +- POC sources: `/workspace/alksocks-bind-poc/src/producer.rs`, + `/workspace/alksocks-bind-poc/src/accept.rs` (the quiet-wait + + failure-mapping origin) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-producer-connect.md b/tasks/implementation/core-producer-connect.md new file mode 100644 index 0000000..6cb01eb --- /dev/null +++ b/tasks/implementation/core-producer-connect.md @@ -0,0 +1,102 @@ +--- +id: implementation/core-producer-connect +name: "Core — producer half: establisher + dispatch + registration + CONNECT" +status: pending +depends_on: [implementation/core-seam-types, implementation/core-params-errors, implementation/core-test-harness] +scope: moderate +risk: medium +impact: phase +level: implementation +--- + +## Description + +Implement the producer half of `src/producer.rs` per producer.md: +the session-gate establisher, the command-dispatch pump handler, the +registration surface, and the CONNECT data plane (POC #1's validated +scope). This is the crate's core task — the open op all three +commands ride. + +Deliverables (port from `/workspace/alksocks-connect-poc/src/ +producer.rs` + `/workspace/alksocks-udp-poc/src/producer.rs`, adapted +to the crate's seam types and alkcall 0.8.0): + +1. **The session-gate establisher** (`socks5_session_establisher`): + validates/records the opener identity (the CF-005/CF-006 seam — + ADR-005) and applies session-level policy. Runs + `Establishment::default()`-shaped (no plan payload — the dial + happens post-establishment, so there is nothing to thread + establisher→handler; POC #1's wire note). Optional + `identity_witness` probe seam (the alktunnels posture) for tests. +2. **The pump handler** (`make_socks5_pump_handler(dial, egress, + accept)`): `accept_bi()` → drive the forked typestate over the + `BiStream` (`accept_no_auth → read_command`) → dispatch: + - **CONNECT** → invoke `DialFn(target)` at the command-read + interception point; success: `reply_success` then `pump_bidi` + **awaited inline** (R-02 — the returned `JoinHandle` tracks the + data-plane lifetime; early return = teardown-at-birth). Refusal: + `reply_error` with the mapped RFC code (dial-failure → + `ConnectionRefused`; scope-class → `ConnectionNotAllowed`) — no + phantom data plane. + - **UDP ASSOCIATE / TCP BIND** → this task lands the dispatch + arms as RFC-faithful refusals (`CommandNotSupported`); the + associate/bind producer tasks replace them. (The dispatch + skeleton exists from day one so the regression guard — one + registration, all three commands — is testable early.) + - The handler is substrate-agnostic by construction — it sees + boxed halves, never a socket type. +3. **`register_socks5_openable`**: spec + establisher + pump via + `register_openable_with_establisher` (`timeout: None` = the 10s + default), scope-gated `socks5:open` (the registry runs the ACL + before the establisher). Missing callbacks = those commands + refuse RFC-faithfully (`CommandNotSupported`-class), never a + panic (ADR-008). +4. **`serve_socks5_over`** — the substrate-free protocol entry + point (the POC shape): generic over `T: AsyncRead + AsyncWrite + + Unpin`, driving the typestate and dispatching. This is what makes + the producer testable without channels (the differential-suite + pattern) and reusable by the batch-2 local backend. +5. **Tests** (via the harness + the raw RFC client helper): CONNECT + echo round-trip through `pump_bidi`; dial refusal → 0x05 in the + reply, no phantom data plane; FORBIDDEN for a scope-less identity + (fails closed, no phantom channel); per-call opener identity + witnessed (CF-006); ASSOCIATE/BIND refuse `CommandNotSupported` + (the pre-vertical posture); half-close teardown (client EOF ends + the session cleanly). + +## Acceptance Criteria + +- [ ] Establisher: session-gate shape (`Establishment::default()`, + identity recorded; witness seam optional) +- [ ] Pump handler: typestate-driven dispatch; `pump_bidi` inline + for CONNECT; no substrate types leak +- [ ] `register_socks5_openable` wires spec + establisher + pump + + injected seams; missing-callback commands refuse + RFC-faithfully +- [ ] `serve_socks5_over` is substrate-free and directly testable +- [ ] Integration tests: round-trip, refusal mapping (0x05), + FORBIDDEN fails closed, identity witness, refusal arms for + ASSOCIATE/BIND, half-close teardown +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/producer.md (the normative shapes; the wiring + requirement) +- docs/architecture/decisions/001/004/005/006 (op, seams, auth, + gates) +- POC sources: `/workspace/alksocks-connect-poc/src/producer.rs` + (CONNECT + establisher), + `/workspace/alksocks-udp-poc/src/producer.rs` (the dispatch + skeleton + registration) +- alkcall ADR-049 (establishment; R-02 inline-await contract), + `/workspace/@alkdev/alktunnels/src/producer.rs` (the 0.8.0 + registration template) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-seam-types.md b/tasks/implementation/core-seam-types.md new file mode 100644 index 0000000..858ae6d --- /dev/null +++ b/tasks/implementation/core-seam-types.md @@ -0,0 +1,106 @@ +--- +id: implementation/core-seam-types +name: Core — the three seam types (DialFn / Egress / BindAccept families) +status: pending +depends_on: [implementation/core-crate-deps] +scope: moderate +risk: low +impact: component +level: implementation +--- + +## Description + +Implement `src/egress.rs` per producer.md §The Three Seams and +ADR-004: the three I/O-plane seam types as function-shaped callbacks +with their error vocabularies. The protocol layer never names a +substrate type — these types are the inversion point; the batch-2 +providers (local/composed) and the test harness all construct them. + +Deliverables (port from the POCs — the shapes are POC-validated): + +1. **The dial seam (CONNECT)** — from + `/workspace/alksocks-connect-poc/src/producer.rs` + + `/workspace/alksocks-udp-poc/src/producer.rs`: + - `TargetHandle { read, write }` — boxed halves + (`Box`, + `Box`; `+ Sync` per the + ChannelPlan payload bound). Ownership rides the handle + structurally (ADR-004). + - `DialFn` — `Arc BoxFuture<'static, + Result> + Send + Sync>`. + - `SocksDialError` with faithful `to_reply_error()` mapping + (refused → `ConnectionRefused`, not-allowed → + `ConnectionNotAllowed`, unreachable → `NetworkUnreachable`, + general → `GeneralFailure` — the POC's mapping, RFC-faithful + per AGENTS.md convention 2). +2. **The egress seam (ASSOCIATE)** — from + `/workspace/alksocks-udp-poc/src/egress.rs`: + - `Egress` trait: `send(addr, payload)` (payload-only — the frame + codec lives on the wire side) + `next_reply()` returning *inner* + bytes (`addr + data`). + - **The readiness contract is load-bearing API surface** (POC #2's + structural finding): `next_reply` must be `Pending` when no + reply is ready — an instantly-ready `Ok(None)` starves a biased + select into livelock. Document it on the trait; the contract + test lands with the producer-associate task. + - `EgressFactory` — called once per ASSOCIATE; the stage owns the + egress; stage exit drops it. + - `EgressError` with `to_reply_error()` mapping. +3. **The accept seam (BIND)** — from + `/workspace/alksocks-bind-poc/src/accept.rs`: + - `BindAccept { bind_addr: SocketAddr, accept: BoxFuture<'static, + Result> }` — reply#1's + BND.ADDR carrier + the one-inbound-accept future. **The accept + future owns its resources end-to-end**; the quiet wait's + `select!` drop IS the client-gone teardown (ADR-004/ADR-007). + - `BindTargetHandle` — accepted halves + peer address; ownership + rides the handle structurally (the third teardown-at-birth + witness — no `_` bindings of owning values). + - `BindAcceptFactory` — called once per BIND with the requested + target. + - `BindAcceptError` (accept-stage, post-reply#1 → reply#2) and + `BindAcceptFactoryError` (pre-reply#1 → reply#1), each with + faithful `to_reply_error()` mapping. +4. **No trait for the seams themselves** (ADR-004): DialFn / + EgressFactory / BindAcceptFactory stay function-shaped; `Egress` + is the one trait (the per-association instance is a returned + object, the POC shape). Note the re-evaluation threshold in the + module docs (three real trait impls). + +Tests: unit tests for the error→reply-code mapping tables (all +variants), the `egress_factory` sync-lift convenience constructor, +and compile-shape assertions (the handles are `Send`; the factories +are `Send + Sync`). The readiness-contract behavioral test lands +with the producer-associate task (it needs the stage). + +## Acceptance Criteria + +- [ ] All three seam families implemented with the POC shapes +- [ ] Every error type's `to_reply_error()` mapping table-tested + (all variants, RFC-faithful codes) +- [ ] Readiness contract documented on `Egress::next_reply` +- [ ] Ownership-rides-the-handle shapes (no `_` bindings; handles + own their payloads structurally) +- [ ] No substrate types named anywhere in the module +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + +## References + +- docs/architecture/producer.md §The Three Seams (the contracts + table) +- docs/architecture/decisions/004-dial-callback-shape.md (no trait; + ownership rides the handle) +- POC sources: `/workspace/alksocks-connect-poc/src/producer.rs`, + `/workspace/alksocks-udp-poc/src/egress.rs`, + `/workspace/alksocks-bind-poc/src/accept.rs` +- docs/architecture/decisions/007-bind-in-base.md (the accept + variants' shapes) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-test-harness.md b/tasks/implementation/core-test-harness.md new file mode 100644 index 0000000..8a6855e --- /dev/null +++ b/tasks/implementation/core-test-harness.md @@ -0,0 +1,115 @@ +--- +id: implementation/core-test-harness +name: Core — integration test harness (duplex transport + identities + in-process seams) +status: pending +depends_on: [implementation/core-seam-types, implementation/core-params-errors] +scope: moderate +risk: medium +impact: phase +level: implementation +--- + +## Description + +Build `tests/harness.rs` — the producer↔consumer wiring over a +`tokio::io::duplex` carrying the channels 8-byte chunk format — so +every subsequent core task tests end-to-end without the `local` +feature. This is the alkcall-0.8.0-first-consumption task: the POCs +ran 0.7.1; the harness proves the 0.8.0 establishment surface +(`register_openable_with_establisher`, `Establishment`, +`ChannelClient::open_channel`) works for this crate's shape before +the producer task builds on it. Template: +`/workspace/@alkdev/alktunnels/tests/harness.rs` (the sibling's +harness — same topology, adapted to this crate's seams). + +Topology (the alktunnels harness shape): + +- **producer** (connect side of the channels connection — runs + `ChannelClient::from_connection_with_serving`): serves the socks5 + open op + generic channel ops on its serving registry. Its manager + allocates the channel IDs (`ChannelSide::Connect` — the serving + side allocates; the initiator adopts). +- **consumer** (accept side): the `ChannelsAdapter` + capturing + install hook. The hook captures the consumer's channel-0 + `CallConnection` and `ChannelManager`. +- **The POC #1 wiring requirement is load-bearing here** (producer.md + §Harness wiring requirement): the accept-side install hook must + call `channel0_conn.set_identity(resolved)` before building the + `CallConnection` — without it, scope-gated opens fail closed with + `FORBIDDEN "authentication required"` (correct behavior, implicit + wiring). The harness makes the requirement explicit and tested. + +Deliverables: + +1. **Identities**: `consumer_identity()` (`socks5:open` only), + `bind_identity()` (`socks5:bind` only — the command-gate probe), + `both_scopes_identity()` (both grants, explicitly — never implied, + ADR-006), plus a `TestIdProvider` resolving them (the alktunnels + harness shape). +2. **In-process seam closures** (no sockets — the protocol layer is + substrate-agnostic, so the harness dials in-process): + - `echo_dial()` — a `DialFn` returning duplex-halves wired to an + in-process echo server (the POC #1 shape); plus + `refusing_dial()` (a failing dial for the refusal-mapping + tests). + - `echo_egress()` — an `EgressFactory` whose egress echoes + datagrams back (the POC #2 shape, readiness-contract-clean: + parks on a channel `recv()`, `Pending` when nothing is ready). + - `echo_accept()` — a `BindAcceptFactory` whose accept resolves + immediately with in-process halves (the POC #5 shape); plus a + failing variant. +3. **The producer registration helper**: a `spawn_producer(...)` + function wiring `register_socks5_openable` (or its task-6 + successor) with the harness seams — the single place the + producer-side wiring lives, so the per-command tasks only write + tests. +4. **A raw RFC client helper**: a hand-rolled minimal RFC 1928 + client over any `AsyncRead + AsyncWrite` (greeting → method + selection → command with target → reply read) — the differential + suite's established pattern — so producer tasks test without + depending on the consumer session types (which land later). +5. **One smoke test** proving the full loop: harness up → producer + registered → consumer opens a channel → raw RFC CONNECT + conversation → echo round-trip. (This test needs a minimal + CONNECT producer path; if the producer task hasn't landed, stub + the handler with the POC #1 `serve_socks5_over` shape inline in + the harness — the producer task then replaces it. Alternatively + sequence this task to land immediately before the producer task + and let the smoke test be the producer task's first test — the + implementer's judgment; the acceptance bar is the harness + compiling with the identities + seams + helpers tested.) + +## Acceptance Criteria + +- [ ] Harness topology works over alkcall 0.8.0 (the establishment + surface consumed successfully — the first-consumption risk + retired) +- [ ] Identity wiring explicit: `set_identity` in the install hook; + a scope-less identity gets `FORBIDDEN` (fails closed) — tested +- [ ] All three identities + all in-process seam closures unit-tested +- [ ] The raw RFC client helper round-trips a greeting + command + + reply against a hand-rolled server over a duplex +- [ ] No `local` feature, no sockets — everything in-process +- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean + (the harness is dev-only — verify it stays out of the wasm + build) + +## References + +- Template: `/workspace/@alkdev/alktunnels/tests/harness.rs` (the + topology + identity shapes) +- POC harnesses: `/workspace/alksocks-connect-poc/src/harness.rs`, + `/workspace/alksocks-udp-poc/src/harness.rs`, + `/workspace/alksocks-bind-poc/src/harness.rs` +- docs/architecture/producer.md §Harness wiring requirement (the + `set_identity` requirement) +- docs/architecture/decisions/006-access-control.md (the scope + identities) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/core-wire-codec.md b/tasks/implementation/core-wire-codec.md new file mode 100644 index 0000000..83983de --- /dev/null +++ b/tasks/implementation/core-wire-codec.md @@ -0,0 +1,110 @@ +--- +id: implementation/core-wire-codec +name: Core — the ASSOCIATE datagram frame codec (src/wire.rs) +status: pending +depends_on: [implementation/core-crate-deps] +scope: narrow +risk: low +impact: component +level: implementation +--- + +## Description + +Implement `src/wire.rs` per wire.md §The Data Plane and bast.md: the +first-party `[len: u16 BE][addr + data]` frame codec for the UDP +ASSOCIATE datagram stage (ADR-003), re-implemented in first-party +code shaped on alktunnels' length codec (`/workspace/@alkdev/ +alktunnels/src/wire.rs` — `frame_datagram`/`DatagramReader` — same +F-2 behavior, but the payload here is the RFC 1928 address encoding +plus data, not a bare payload). + +The codec is the crate's one framed stage (CONNECT/BIND are raw +pass-through — no framing). Framing is applied **exactly once, by +this layer**: `Egress::next_reply` returns *inner* bytes +(`addr + data`); the producer's frame writer adds the single length +prefix (the double-framing bug class POC #2 caught is an +API-design invariant this module's docs must state). + +Deliverables: + +1. **`frame(addr: &TargetAddr, payload: &[u8]) -> Result`** — one frame per datagram: `[len: u16 BE][addr + + data]` where `addr + data` is `TargetAddr::to_be_bytes()` (ATYP + + addr + port) followed by the payload. +2. **The joint bound (the load-bearing constraint)**: reject + `Oversize` at frame time whenever `len(addr) + len(payload) > + 65535`. The UDP payload bound alone (65507) does NOT fit u16 + unconditionally — a 65507-byte datagram to a 255-character domain + is 65766 frame bytes. Max data: 65276 for domain-form + destinations, 65516 for IPv6, 65528 for IPv4. No unchecked u16 + arithmetic on the payload bound (wire.md's explicit requirement). + Expose `addr_encoding_len(addr)` (7 v4 / 19 v6 / 259 max domain) + so callers can compute the joint bound. +3. **`DatagramReader`** — the incremental decoder: feed it whatever + `read()` yields; it reassembles frames across chunk boundaries + (split frames, batched frames, partial headers). Returns + `(TargetAddr, Bytes)` per completed frame — it decodes the addr + sub-layout (ATYP determines the address length) and splits + `addr + data`. Infallible decode direction (the oversize bound is + enforced at frame time); `is_mid_frame()` diagnostic for teardown. +4. **The F-2 invariant, correctly stated**: a **zero-length payload** + is a legal empty datagram — never EOF. (`len == 0` is unreachable + on this wire since `addr` is always ≥ 7 bytes; the invariant that + matters is payload-level. See the doc-sync deliverable.) +5. **Doc-sync (wire.md + bast.md residue)**: both docs say "`len = 0` + is a legal empty datagram" while also pinning that `len` covers + `addr + data` (joint bound) — `len = 0` is unreachable since + `addr` ≥ 7 bytes. Correct both docs to state the operative + invariant at payload level ("a zero-length payload is a legal + empty datagram, never EOF — the F-2 invariant") in the same + change; do not silently diverge. +6. **EOF layering note in the module docs**: EOF is the + channels-level sentinel on the `BiStream`, outside this codec; + the codec never emits zero-length reads, so the layers never + collide (the alktunnels wire.rs module-doc posture). + +Tests: round-trip (v4/v6/domain), empty payload legal + not EOF, +split across awkward chunks, batched frames in one chunk, partial +header waits, joint-bound rejection (the 65507 + 255-char-domain +case), oversize at frame time, mid-frame state observable, garbage +ATYP decode error (the `udp_undecodable_frame_ends_association` +shape — the decode error type must distinguish a bad ATYP so the +producer stage can fail the association loud). + +## Acceptance Criteria + +- [ ] `frame`/`DatagramReader`/`addr_encoding_len` implemented per + the joint bound; no unchecked u16 arithmetic +- [ ] Zero-length payload round-trips as a legal empty datagram +- [ ] Split/batch/partial-header reassembly tests pass +- [ ] Joint-bound rejection test (65507-byte payload + 255-char + domain → `Oversize`) passes +- [ ] Garbage-ATYP decode surfaces a typed error (not a panic) +- [ ] wire.md + bast.md residue corrected (payload-level F-2 + statement) in the same change +- [ ] `cargo test` green; clippy `--all-targets -- -D warnings` + clean; `cargo fmt --check` clean +- [ ] `cargo check --target wasm32-unknown-unknown` + + `cargo clippy --target wasm32-unknown-unknown -- -D warnings` + clean + +## References + +- docs/architecture/wire.md §The Data Plane (the frame + the joint + bound arithmetic) +- docs/architecture/bast.md (the BAST contract for the frame) +- docs/architecture/decisions/003-udp-handle-swap.md (framing-once, + F-2) +- Template: `/workspace/@alkdev/alktunnels/src/wire.rs` (the length + codec + its test matrix — shape on it, do not depend on it) +- POC sketch: `/workspace/alksocks-udp-poc/src/datagram.rs` + (`frame`/`inner`/`unframe`/`addr_encoding_len`) + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file diff --git a/tasks/implementation/review-protocol-core.md b/tasks/implementation/review-protocol-core.md new file mode 100644 index 0000000..6ded7d2 --- /dev/null +++ b/tasks/implementation/review-protocol-core.md @@ -0,0 +1,68 @@ +--- +id: review-protocol-core +name: Review — the protocol core before any assembly-layer work +status: pending +depends_on: [implementation/core-integration-suite] +scope: narrow +risk: low +impact: project +level: review +--- + +## Description + +Review checkpoint before batch 2 (the assembly layer: `local` +providers, front doors, `composed` providers). Everything in batch 2 +consumes the seams and the session types, so a core defect — a seam +contract drift, a teardown hole, a wasm regression — would propagate +into every downstream task. This review is the gate between the core +group and the assembly layer, and the point where batch 2 gets +decomposed (its task shapes may adjust to what the core actually +landed). + +Review against the architecture, in this order: + +1. **Seam contracts** (ADR-004, producer.md §The Three Seams): the + readiness contract (`next_reply` `Pending` when nothing is ready — + the contract test exists and pins it), ownership-rides-the-handle + (no `_` bindings of owning values; handles own their payloads + structurally), the accept future owning its resources end-to-end + (the quiet-wait drop IS the client-gone teardown), framing-once + (the egress speaks inner bytes; the wire layer frames once). +2. **Wire fidelity** (wire.md, ADR-003/ADR-007): the datagram frame + (joint bound, F-2 at payload level), the phase model (no mode + byte; post-reply garbage refused structurally in all three + commands), the sentinel surfaces (ASSOCIATE's `127.0.0.1:0`, + BIND's variant-3 zeros), association lifetime = channel lifetime. +3. **Access control** (ADR-006): the open gate (`socks5:open` at the + registry), the command gate (`socks5:bind` at BIND dispatch), + two grants never implied; refusals RFC-faithful everywhere + (convention 2 — never collapsed into generic errors). +4. **Teardown/lifecycle** (wire.md §Sentinels, alkcall ADR-050): + `pump_bidi` awaited inline (R-02 — no spawn-and-forget); the + teardown matrix tests actually pin the semantics; no + teardown-at-birth paths (the third-witness pattern held). +5. **Convention compliance** (AGENTS.md): no `unwrap`/`expect`/ + panics outside tests; `thiserror` surfaces; no comments in code + except doc comments + pinned non-obvious-correctness notes; + naming; module structure (one module per file, `lib.rs` + re-exports); producer/consumer vocabulary (no server/client + framing outside RFC vocabulary). +6. **Wasm-clean invariant** (ADR-009): both wasm checks green; no + `net`-shaped feature; tokio on the wasm-clean subset; the harness + and tests stay out of the wasm build. +7. **POC-fidelity spot-check**: for two or three of the trickiest + behaviors (the readiness contract, the quiet-wait two-exit + semantics, the split-datagram reader persistence), diff the + crate's behavior against the POC crates' findings — the POCs are + the empirical record; drift is a finding. +8. **Docs sync**: the specs match what landed (the integration + task's reconciliation verified; any residue is a finding). + +## Notes + +> To be filled by implementation agent + +## Summary + +> To be filled on completion \ No newline at end of file