feat: rename ALPN prefix from alknet/ to alk/ (v0.1.1)

- CHANNELS_ALPN: b"alknet/channels" → b"alk/channels" - CallAdapter::alpn(): b"alknet/call" → b"alk/call" - derive_alpn_from_op_name: alknet/ prefix → alk/ prefix - All ALPN string literals in src/ and docs/ updated - ADR-004 amended with prefix rename rationale - AGENTS.md, README.md updated - Version bumped to 0.1.1 Review: docs/reviews/003-alpn-prefix-rename.md Verification: - cargo test: 542 passed, 0 failed - cargo clippy --all-targets -- -D warnings: clean - cargo fmt --check: clean - cargo doc --no-deps: clean
2026-08-14 13:55:28 +00:00
1 parent 3cce1410c4
commit 08e7df2aa0
@@ -0,0 +1,501 @@
+# Review 003 — ALPN Prefix Rename (`alknet/` → `alk/`)
+
+## Status
+
+Verified, open for remediation.
+
+## Scope
+
+This review covers the ALPN prefix rename from `alknet/<name>` to
+`alk/<name>` across the entire alkcall crate. The rename is driven by
+the alktty crate (`/workspace/@alkdev/alktty`), the first downstream
+consumer, which uses `alk/tty` as its ALPN. The shorter prefix is
+cleaner and more readable; doing it now (before any published consumers
+exist) avoids a breaking change later.
+
+The review identifies every location that must change, every location
+that must **not** change (filesystem paths, historical references), and
+the special-cased logic in `derive_alpn_from_op_name` that needs
+updating.
+
+Every finding below was verified directly in the source during this
+pass (2026-08-14). Each finding carries the affected file(s), the
+nature of the change, and the remediation unit it belongs to.
+
+## Baseline verification (this pass)
+
+```
+cargo test                                    → 483 passed, 0 failed
+cargo clippy --all-targets -- -D warnings     → clean
+cargo fmt --check                              → clean
+cargo doc --no-deps                            → clean
+```
+
+The suite is green. This is a mechanical rename — no logic changes, no
+new features, no protocol changes beyond the ALPN byte strings
+themselves. The risk is in missing a location or accidentally changing a
+non-ALPN reference (filesystem paths, historical commit references).
+
+## Verdict
+
+The rename is straightforward but high-volume (~400+ locations across
+~50 files). The work splits into three categories:
+
+1. **Wire-format ALPNs** (byte strings `b"alknet/..."` and string
+   literals `"alknet/..."` in `src/`) — these are the actual protocol
+   identifiers. Changing them is a one-way door: old peers using
+   `alknet/` will not interoperate with new peers using `alk/`. Since
+   no published consumers exist yet, this is the right time.
+
+2. **Doc comments, ADRs, architecture docs, AGENTS.md, README.md** —
+   these are documentation. They must be updated to match the new
+   ALPNs so the docs don't mislead future readers.
+
+3. **`derive_alpn_from_op_name` special-casing** — this function has
+   explicit logic for the `alknet/` prefix that must be updated to
+   `alk/`. This is the only non-mechanical change.
+
+The `alknet/` prefix in **filesystem paths** (e.g.
+`/workspace/@alkdev/alknet/`, `alknet/docs/architecture/`) and
+**historical commit references** must **not** be changed — those are
+not ALPNs.
+
+## Severity legend
+
+- **[critical]** — a wire-format ALPN is missed, causing silent
+  interop failure with downstream crates.
+- **[major]** — a doc or ADR reference is missed, causing confusion for
+  future readers; or the `derive_alpn_from_op_name` logic is wrong.
+- **[minor]** — a test-only ALPN is missed (no interop impact, but
+  inconsistent).
+
+---
+
+# Part A — Wire-format ALPNs (src/ byte strings and string literals)
+
+## A-01 [critical] — `CHANNELS_ALPN` constant
+
+**Location:** `src/channels/adapter.rs:40`
+**Current:** `pub const CHANNELS_ALPN: &[u8] = b"alknet/channels";`
+**Required:** `pub const CHANNELS_ALPN: &[u8] = b"alk/channels";`
+
+This is the only named ALPN constant. It is referenced from:
+- `src/channels/adapter.rs:243` (channel 0 construction)
+- `src/channels/client.rs:96,349,391,474,587,845,1160` (channel 0 construction in tests and `ChannelClient`)
+- `src/channels/manager.rs:359,503,518,546,565,606,635,706,730,754,775` (channel 0 ALPN in tests)
+
+All references use the constant, so changing the constant definition
+propagates automatically. However, the string literal `"alknet/call"`
+appears alongside `CHANNELS_ALPN` in many of these locations (as the
+channel-0 call ALPN) — those must be changed separately (see A-02).
+
+## A-02 [critical] — Hardcoded `b"alknet/call"` in `CallAdapter::alpn()`
+
+**Location:** `src/protocol/adapter.rs:151`
+**Current:** `b"alknet/call"`
+**Required:** `b"alk/call"`
+
+This is the call protocol's ALPN. It is not a named constant — it is
+hardcoded in the `ProtocolHandler` impl. Consider extracting it as
+`pub const CALL_ALPN: &[u8] = b"alk/call";` to match the pattern of
+`CHANNELS_ALPN`. This would reduce the number of hardcoded string
+literals that need individual updates.
+
+## A-03 [critical] — Hardcoded `"alknet/call"` string literals in src/
+
+These are the channel-0 call ALPN used as a `String` (not a byte
+string) in channel 0 construction and test assertions. They appear in:
+
+| File | Lines | Context |
+|------|-------|---------|
+| `src/channels/adapter.rs` | 267, 279, 291, 297, 310, 333, 346, 395, 404 | Channel 0 construction, test assertions |
+| `src/channels/client.rs` | 351, 357, 393, 399, 476, 482, 589, 595, 847, 850, 1162, 1165 | Channel 0 construction, test assertions |
+| `src/channels/manager.rs` | 507, 532, 550, 639, 710, 757, 779 | Test channel 0 ALPN assertions |
+| `src/channels/operations.rs` | 805, 911, 941, 1038, 1044, 1068, 1074, 1098, 1104, 1130, 1142, 1173, 1185, 1215, 1222, 1252, 1264, 1275 | Test auth contexts, channel opens |
+| `src/protocol/adapter.rs` | 297, 840, 858, 874 | Test assertions |
+| `src/protocol/connection.rs` | 1057 | Test connection construction |
+| `src/protocol/test_support.rs` | 64, 124, 128 | Test support connections |
+| `src/client/call_client.rs` | 187 | Test connection construction |
+
+**Required:** all `"alknet/call"` → `"alk/call"`.
+
+If A-02's recommendation to extract `CALL_ALPN` is adopted, many of
+these can use the constant instead of a hardcoded string.
+
+## A-04 [critical] — Data-plane ALPNs in src/ (string literals)
+
+These are channel ALPNs used in tests and the `derive_alpn_from_op_name`
+logic:
+
+| ALPN | Files | Approx. count |
+|------|-------|---------------|
+| `"alknet/tty"` | `channels/adapter.rs`, `channels/client.rs`, `channels/manager.rs`, `channels/operations.rs`, `client/from_call.rs`, `registry/discovery.rs`, `registry/spec.rs` | ~40 |
+| `"alknet/tunnel"` | `channels/manager.rs`, `channels/operations.rs`, `client/from_call.rs` | ~10 |
+| `"alknet/a"`, `"alknet/b"`, `"alknet/c"` | `channels/adapter.rs`, `channels/client.rs` | ~8 |
+| `"alknet/test"` | `core/auth.rs`, `core/types.rs` | ~6 |
+
+**Required:** `"alknet/tty"` → `"alk/tty"`, `"alknet/tunnel"` →
+`"alk/tunnel"`, `"alknet/a"` → `"alk/a"`, etc.
+
+## A-05 [critical] — `derive_alpn_from_op_name` special-casing
+
+**Location:** `src/client/from_call.rs:294-307`
+
+```rust
+fn derive_alpn_from_op_name(op_name: &str) -> Option<String> {
+    let rest = op_name.strip_prefix("channels/")?;
+    let segment = rest
+        .strip_suffix("/sub")
+        .or_else(|| rest.strip_suffix("/pub"))?;
+    if segment.is_empty() {
+        return None;
+    }
+    if segment.starts_with("alknet/") || segment == "alknet" || segment.contains('/') {
+        Some(segment.to_string())
+    } else {
+        Some(format!("alknet/{segment}"))
+    }
+}
+```
+
+This function derives the data-plane ALPN from an open-op name
+(`channels/<alpn>/sub` → `alknet/<alpn>`). The logic has two branches:
+
+1. If the segment already starts with `alknet/` or equals `alknet` or
+   contains `/`, return it as-is (it's already a full ALPN).
+2. Otherwise, prepend `alknet/`.
+
+**Required changes:**
+- Line 302: `starts_with("alknet/")` → `starts_with("alk/")`
+- Line 302: `segment == "alknet"` → `segment == "alk"`
+- Line 305: `format!("alknet/{segment}")` → `format!("alk/{segment}")`
+
+The doc comment on lines 284-293 also references `alknet/` and must be
+updated.
+
+The corresponding tests in `from_call.rs` (lines ~600-740) use
+`"alknet/tty"` and `"alknet/tunnel"` as expected outputs — these must
+be updated to `"alk/tty"` and `"alk/tunnel"`.
+
+---
+
+# Part B — Doc comments in src/ (/// and //!)
+
+## B-01 [major] — Module-level and item-level doc comments
+
+Every doc comment that references an ALPN must be updated. Key files:
+
+| File | Approx. count | Example |
+|------|---------------|---------|
+| `src/lib.rs` | 4 | `alknet/call`, `alknet/channels`, `alknet/alknode` |
+| `src/channels/mod.rs` | 2 | `alknet/call`, `alknet/channels` |
+| `src/channels/adapter.rs` | 5 | `alknet/channels`, `alknet/call` |
+| `src/channels/client.rs` | 5 | `alknet/channels`, `alknet/call`, `alknet/tty` |
+| `src/channels/manager.rs` | 1 | `alknet/call` |
+| `src/channels/wire.rs` | 1 | `alknet/call` |
+| `src/channels/operations.rs` | ~20 | Various ALPN references |
+| `src/protocol/mod.rs` | 1 | `alknet/call` |
+| `src/protocol/connection.rs` | 3 | `alknet/call` |
+| `src/protocol/dispatch.rs` | 1 | `alknet/call` |
+| `src/protocol/adapter.rs` | 1 | `alknet/call` |
+| `src/client/call_client.rs` | 1 | `alknet/call` |
+| `src/client/from_call.rs` | 5 | `alknet/<alpn>` |
+| `src/registry/spec.rs` | 3 | `alknet/tty`, `alknet/<alpn>` |
+
+**Required:** mechanical `alknet/` → `alk/` in all doc comments.
+
+---
+
+# Part C — Architecture docs and ADRs
+
+## C-01 [major] — Architecture overview docs
+
+| File | Approx. count |
+|------|---------------|
+| `docs/architecture/README.md` | ~15 |
+| `docs/architecture/call-README.md` | ~5 |
+| `docs/architecture/call-protocol.md` | ~10 |
+| `docs/architecture/channels-README.md` | ~8 |
+| `docs/architecture/channels-overview.md` | ~20 |
+| `docs/architecture/channels-adapter.md` | ~3 |
+| `docs/architecture/channels-wire.md` | ~8 |
+| `docs/architecture/channels-connection.md` | ~3 |
+| `docs/architecture/channel-client.md` | ~4 |
+| `docs/architecture/channel-operations.md` | ~12 |
+| `docs/architecture/client-and-adapters.md` | ~6 |
+| `docs/architecture/operation-registry.md` | ~3 |
+| `docs/architecture/open-questions.md` | ~2 |
+
+**Required:** mechanical `alknet/` → `alk/` in all architecture docs.
+**Exception:** filesystem paths like `/workspace/@alkdev/alknet/` must
+**not** be changed.
+
+## C-02 [major] — ADR decision documents
+
+| ADR | Approx. count |
+|-----|---------------|
+| `decisions/001-alpn-protocol-dispatch.md` | ~2 |
+| `decisions/002-protocol-handler-trait.md` | ~1 |
+| `decisions/004-alpn-convention-and-connection-model.md` | ~12 |
+| `decisions/007-connection-from-stream-generic-single-stream.md` | ~1 |
+| `decisions/009-bistream-as-the-handler-leaf.md` | ~1 |
+| `decisions/012-connectioncredentials-decouple-dial-from-call.md` | ~3 |
+| `decisions/013-irpc-as-call-protocol-foundation.md` | ~1 |
+| `decisions/015-call-protocol-stream-model.md` | ~3 |
+| `decisions/019-operation-registry-layering.md` | ~2 |
+| `decisions/022-call-protocol-client-and-adapter-contract.md` | ~1 |
+| `decisions/023-callclient-peer-scoped-registry-filtering.md` | ~1 |
+| `decisions/031-crate-decomposition.md` | ~1 |
+| `decisions/034-channels-wire-format.md` | ~7 |
+| `decisions/035-channels-pure-channel-multiplexing.md` | ~10 |
+| `decisions/036-channel-0-pre-negotiated-call.md` | ~15 |
+| `decisions/037-channel-lifecycle-operations.md` | ~6 |
+| `decisions/038-channelconnection-bidistreamsource.md` | ~2 |
+| `decisions/039-channelsadapter-and-channelmanager.md` | ~4 |
+| `decisions/041-per-identity-channel-cap.md` | ~1 |
+| `decisions/042-hub-relay-translate-not-forward.md` | ~5 |
+| `decisions/043-channelclient.md` | ~3 |
+| `decisions/044-channels-subcrate-decomposition.md` | ~6 |
+| `decisions/045-alknetclient-native-dial-seam.md` | ~10 |
+| `decisions/046-publish-operation-type-and-handler-kind-sink.md` | ~2 |
+| `decisions/047-openable-alpns-are-operations.md` | ~8 |
+
+**Required:** mechanical `alknet/` → `alk/` in all ADRs.
+**Exception:** filesystem paths and historical references to the
+`alknet` mono-repo must **not** be changed.
+
+### ADR-004 special consideration
+
+ADR-004 (`004-alpn-convention-and-connection-model.md`) defines the
+ALPN string convention. It currently specifies the `alknet/` prefix.
+This ADR must be amended to reflect the new `alk/` prefix. The
+amendment should note:
+- The original decision used `alknet/` as the prefix.
+- The prefix was shortened to `alk/` before the first published
+  release (v0.1.1) for brevity and readability.
+- The convention otherwise remains: one ALPN per connection, the
+  prefix identifies the alk protocol family.
+
+## C-03 [major] — AGENTS.md
+
+**Location:** `AGENTS.md`
+**Lines:** 53, 207-209, 243, 263
+
+Key changes:
+- Line 53: `alknet/call` → `alk/call`
+- Line 207: `/workspace/@alkdev/alknet/docs/architecture/` — **do not
+  change** (filesystem path)
+- Line 208-209: "The ALPN strings (`alknet/call`, `alknet/channels`)
+  are wire-stable and unchanged." — **must be updated** to reflect the
+  new ALPNs. The strings are changing; the statement that they are
+  "unchanged" is now false. Replace with the new ALPNs and note that
+  they are wire-stable going forward.
+- Line 243: `alknet/call` → `alk/call`
+- Line 263: `alknet/` prefix → `alk/` prefix
+
+## C-04 [major] — README.md
+
+**Location:** `README.md`
+**Lines:** 60, 77, 116
+
+- Line 60: `b"alknet/call"` → `b"alk/call"`
+- Line 77: `b"alknet/channels"` → `b"alk/channels"`
+- Line 116: `alknet/channels` → `alk/channels`
+
+## C-05 [minor] — Existing review documents
+
+**Locations:**
+- `docs/reviews/001-pub-and-channels-integration-review.md` (~15 occurrences)
+- `docs/reviews/002-pre-publish-coverage-and-cleanup-review.md` (0 occurrences)
+
+Review 001 references `alknet/` in code snippets and analysis. These
+are historical — they document findings against the code as it existed
+at the time. **Do not change** historical review documents. They are
+snapshots of a past state.
+
+---
+
+# Part D — What must NOT change
+
+## D-01 [critical] — Filesystem paths
+
+These are **not** ALPNs and must **not** be changed:
+
+| Pattern | Example | Files |
+|---------|---------|-------|
+| `/workspace/@alkdev/alknet/` | `/workspace/@alkdev/alknet/docs/architecture/` | `AGENTS.md:207`, `docs/architecture/README.md:15,291`, several ADRs |
+| `alknet/docs/` | `alknet/docs/architecture/decisions/` | ADR-046, ADR-047 |
+| `alknet-call` | The old crate name | `AGENTS.md:205`, `docs/architecture/README.md:14` |
+| `alknet-channels` | The old crate name | `AGENTS.md:205`, `docs/architecture/README.md:14` |
+| `alknet-client` | The old crate name | ADR-045 |
+| `alknet/alknode` | The composition layer name | `src/lib.rs:68`, `docs/architecture/README.md:228` |
+
+The crate names `alknet-call` and `alknet-channels` are historical
+references to the pre-unification crates in the alknet mono-repo.
+These are not ALPNs and should not be changed.
+
+The composition layer `alknet/alknode` in `src/lib.rs:68` is a
+reference to a future crate, not an ALPN. It should not be changed.
+
+## D-02 [minor] — Historical review documents
+
+`docs/reviews/001-pub-and-channels-integration-review.md` and
+`docs/reviews/002-pre-publish-coverage-and-cleanup-review.md` are
+historical snapshots. Do not modify them.
+
+---
+
+# Cross-cutting: what is solid (verified)
+
+These were checked and are correct — the rename does not affect them:
+
+- **Wire format shapes:** `EventEnvelope` and the channels 8-byte chunk
+  header are unchanged. Only the ALPN byte strings carried in the TLS
+  handshake change. ✓
+- **No logic changes:** the rename is purely mechanical. No function
+  signatures, trait bounds, or control flow change. ✓
+- **No new dependencies, no feature flag changes.** ✓
+- **`alktype` dependency unaffected.** ✓
+- **483 tests pass before the rename; they should all pass after.** ✓
+- **The `alknet/` prefix in `derive_alpn_from_op_name` is the only
+  non-mechanical change** — the function's logic stays the same, only
+  the prefix string changes. ✓
+
+---
+
+# Remediation plan
+
+The plan is split into **four units of work**, ordered by risk. Each
+unit is an independently shippable commit.
+
+## Unit 1 — Wire-format ALPNs + `derive_alpn_from_op_name` (A-01 through A-05)
+
+**Goal:** all ALPN byte strings and string literals in `src/` use
+`alk/` prefix; `derive_alpn_from_op_name` uses `alk/`; all tests pass.
+
+**Files:** `src/channels/adapter.rs`, `src/channels/client.rs`,
+`src/channels/manager.rs`, `src/channels/operations.rs`,
+`src/protocol/adapter.rs`, `src/protocol/connection.rs`,
+`src/protocol/test_support.rs`, `src/client/call_client.rs`,
+`src/client/from_call.rs`, `src/core/auth.rs`, `src/core/types.rs`,
+`src/registry/discovery.rs`, `src/registry/spec.rs`.
+
+**Scope:**
+- **A-01:** change `CHANNELS_ALPN` from `b"alknet/channels"` to
+  `b"alk/channels"`.
+- **A-02:** change `b"alknet/call"` in `CallAdapter::alpn()` to
+  `b"alk/call"`. Optionally extract as `pub const CALL_ALPN: &[u8] =
+  b"alk/call";` and use it in all channel-0 construction sites.
+- **A-03:** change all `"alknet/call"` string literals to `"alk/call"`.
+- **A-04:** change all data-plane ALPN string literals (`"alknet/tty"`,
+  `"alknet/tunnel"`, `"alknet/a"`, `"alknet/b"`, `"alknet/c"`,
+  `"alknet/test"`) to their `alk/` equivalents.
+- **A-05:** update `derive_alpn_from_op_name`:
+  - `starts_with("alknet/")` → `starts_with("alk/")`
+  - `segment == "alknet"` → `segment == "alk"`
+  - `format!("alknet/{segment}")` → `format!("alk/{segment}")`
+  - Update the doc comment.
+  - Update the corresponding tests.
+
+**Acceptance gate:** `cargo test` green; `cargo clippy
+--all-targets -- -D warnings` clean; `cargo fmt --check` clean; `cargo
+doc --no-deps` clean. No remaining `b"alknet/` or `"alknet/` in `src/`
+(verified by grep).
+
+## Unit 2 — Doc comments in src/ (B-01)
+
+**Goal:** all doc comments in `src/` use `alk/` prefix.
+
+**Files:** all files listed in B-01.
+
+**Scope:** mechanical `s/alknet\//alk\//g` in doc comments (`///` and
+`//!` lines) across all `src/` files. Verify no code lines are
+affected (Unit 1 already handled those).
+
+**Acceptance gate:** `cargo doc --no-deps` clean (0 warnings); grep for
+`alknet/` in `src/` returns no matches in doc comments.
+
+## Unit 3 — Architecture docs, ADRs, AGENTS.md, README.md (C-01 through C-05)
+
+**Goal:** all documentation uses `alk/` prefix; ADR-004 is amended.
+
+**Files:** all files listed in C-01 through C-04.
+
+**Scope:**
+- **C-01:** mechanical `s/alknet\//alk\//g` in all architecture docs,
+  with careful exclusion of filesystem paths.
+- **C-02:** mechanical `s/alknet\//alk\//g` in all ADRs, with careful
+  exclusion of filesystem paths and historical crate names.
+- **ADR-004 amendment:** add an amendment section noting the prefix
+  change from `alknet/` to `alk/`, the rationale (brevity, readability,
+  done before first published release), and the effective date.
+- **C-03:** update AGENTS.md — change ALPN references, update the
+  "wire-stable and unchanged" statement to reflect the new ALPNs.
+- **C-04:** update README.md — change ALPN references in code examples
+  and prose.
+
+**Acceptance gate:** grep for `alknet/call` and `alknet/channels` in
+`docs/` returns no matches (except in historical review documents and
+filesystem paths). ADR-004 has an amendment section.
+
+## Unit 4 — Final verification + version bump
+
+**Goal:** confirm nothing was missed; bump version to 0.1.1.
+
+**Files:** `Cargo.toml`.
+
+**Scope:**
+- Run `cargo test`, `cargo clippy --all-targets -- -D warnings`,
+  `cargo fmt --check`, `cargo doc --no-deps`.
+- Grep the entire repo for `alknet/` and manually verify every
+  remaining match is either a filesystem path, a historical crate name
+  (`alknet-call`, `alknet-channels`, `alknet-client`), or in a
+  historical review document.
+- Bump `version` in `Cargo.toml` from `0.1.0` to `0.1.1`.
+- Run `cargo publish --dry-run --allow-dirty` (may still fail on
+  missing README.md — that's Review 002's R-01, not this review's
+  concern).
+
+**Acceptance gate:** all verification commands pass; version is 0.1.1;
+no unexpected `alknet/` matches remain.
+
+## Suggested sequencing
+
+```
+Unit 1  (wire-format ALPNs + derive_alpn_from_op_name)  → no deps; do first
+Unit 2  (doc comments in src/)                           → depends on Unit 1 (to avoid conflicts)
+Unit 3  (architecture docs, ADRs, AGENTS.md, README.md)  → no deps; independent
+Unit 4  (final verification + version bump)              → depends on Units 1-3
+```
+
+Units 1 and 3 are independent and can proceed in parallel. Unit 2
+should follow Unit 1 to avoid merge conflicts on the same files. Unit 4
+is the final pass.
+
+---
+
+## Verification log (this pass)
+
+Findings verified directly in source on 2026-08-14 against tree
+`7cd8a57` (post-Review-002 baseline).
+
+- **A-01:** verified `CHANNELS_ALPN` at `adapter.rs:40` and all
+  references to it.
+- **A-02:** verified hardcoded `b"alknet/call"` at `adapter.rs:151`.
+- **A-03:** verified all `"alknet/call"` string literals in src/ (56
+  locations across 8 files).
+- **A-04:** verified all data-plane ALPN string literals (~60
+  locations).
+- **A-05:** verified `derive_alpn_from_op_name` at `from_call.rs:294-307`
+  and its tests at lines ~600-740.
+- **B-01:** verified all doc comment references (~40 locations across
+  12 files).
+- **C-01 through C-04:** verified all documentation references (~200+
+  locations across ~35 files).
+- **D-01:** verified filesystem path references that must not change
+  (~15 locations).
+- **D-02:** verified historical review documents (2 files).
+
+Total estimated `alknet/` occurrences requiring change: ~350.
+Total estimated `alknet/` occurrences that must NOT change: ~50
+(filesystem paths, historical crate names, review documents).