diff --git a/docs/reviews/002-pre-publish-coverage-and-cleanup-review.md b/docs/reviews/002-pre-publish-coverage-and-cleanup-review.md new file mode 100644 index 0000000..ccd95f8 --- /dev/null +++ b/docs/reviews/002-pre-publish-coverage-and-cleanup-review.md @@ -0,0 +1,591 @@ +# Review 002 — Pre-Publish Coverage, Convention, and Cleanup Review + +## Status + +Verified, open for remediation. + +## Scope + +General review of the alkcall crate at tree `7cd8a57` (post-Units 1-10 +remediation of Review 001). This review covers: + +- **Publishing readiness** — `cargo publish --dry-run` blockers, Cargo.toml + metadata, license files, missing README. +- **Code coverage** — `cargo llvm-cov` line/region/function coverage + across all 27 source files; identification of weak spots in core + protocol paths. +- **Convention compliance** — `expect`/`unwrap` in library code, + `#[allow(dead_code)]` annotations, `#[allow(clippy::...)]` attributes, + producer/consumer naming. +- **Code smells** — dead code, stub handlers, unnecessary annotations, + large functions, public API surface hygiene. + +Every finding below was **re-verified directly in the source** during +this pass (2026-08-14). Each finding carries: the convention or ADR it +drifts from (if any), a severity, the exact `file:line` reference(s), +the verification status, and the unit-of-work bucket 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 (0 warnings) +cargo publish --dry-run --allow-dirty → FAILS — missing README.md +cargo llvm-cov --summary-only → 91.97% lines, 93.17% regions +``` + +The suite is green and the architecture is solid. The review-001 +critical/major bugs (P-01 through C-08, C-12 through C-18) are all +fixed and verified. The remaining issues are coverage gaps in +post-Unit-2 code paths, convention violations, and one hard `cargo +publish` blocker. + +## Verdict + +- **Publishing readiness: one hard blocker** (missing README.md). All + other `cargo publish` requirements are met: license files present + (MIT + Apache-2.0), Cargo.toml has correct metadata (version, edition, + rust-version, license, description, repository, keywords, categories, + exclude list). No `unsafe` code, no `todo!()`/`unimplemented!()` + panicking macros, no `println!`/`eprintln!` in library code, no + hardcoded secrets. +- **Coverage: 91.97% overall is solid**, but seven files fall below 90% + line coverage. The gaps cluster in three areas: the single-stream call + mode (channel 0 multiplexed framing, added in Unit 2), the + `ChannelCore`/open-op machinery (added in Unit 3), and the + `SendStream`/`RecvStream` newtypes (vendored core types, ADR-092). + These are all post-Unit-2 code paths — the pre-Unit-2 surface is + well-covered. +- **Conventions: three `expect` calls in library code** violate + AGENTS.md §2. All are in crate-private or guarded contexts and won't + panic in practice, but the convention is explicit. One unnecessary + `#[allow(dead_code)]` annotation. No other convention violations + found. +- **Code smells: minor.** Stub handlers are intentional and documented + with OQ references. Two `#[allow(clippy::too_many_arguments)]` + annotations on functions with 9 parameters — acceptable for + builder-alternative constructors and well-structured wrappers. + +## Severity legend + +- **[critical]** — `cargo publish` fails; or a decided spec invariant is + violated in a way that corrupts data or causes silent permanent state + damage. +- **[major]** — a core protocol path has no test coverage; or a public + API type has no coverage; or a convention violation that would + surprise downstream consumers. +- **[minor]** — drift, convention violation, dead code, unnecessary + annotation, or a doc/spec inconsistency with no correctness impact. + +--- + +# Part A — Publishing Blockers + +## R-01 [critical] — Missing README.md; `cargo publish --dry-run` fails + +**Convention drift:** Cargo.toml `readme` field. +**Verified:** YES. `Cargo.toml:9` specifies `readme = "README.md"`. No +`README.md` exists at the repository root. `cargo publish --dry-run +--allow-dirty` fails with: + +``` +error: readme `README.md` does not appear to exist (relative to +`/workspace/@alkdev/alkcall`). +``` + +License files (`LICENSE-MIT`, `LICENSE-APACHE`) are present. All other +Cargo.toml metadata is correct: `version = "0.1.0"`, `edition = "2021"`, +`rust-version = "1.85"`, `license = "MIT OR Apache-2.0"`, `description`, +`repository`, `keywords`, `categories`, `exclude` list. + +**Fix:** either create a `README.md` at the repo root, or change the +`readme` field to point to an existing doc file (e.g. +`docs/architecture/README.md`), or remove the `readme` field. The user +has stated they write the README as the second-to-last step before +publishing, so the pragmatic fix is to remove the `readme` field from +Cargo.toml for now and restore it when the README is written. + +**Acceptance gate:** `cargo publish --dry-run --allow-dirty` succeeds. + +--- + +# Part B — Convention Violations + +## R-02 [minor] — `expect` in library code (3 locations) + +**Convention drift:** AGENTS.md §2 ("No `unwrap()` or `expect()` outside +tests. For poisoned `RwLock`/`Mutex`, use `unwrap_or_else(|e| +e.into_inner())`"). +**Verified:** YES. Three `expect` calls in non-test, non-test-support +library code: + +| File:Line | Code | Risk | +|-----------|------|------| +| `channels/operations.rs:415` | `channel_open.expect("checked above")` | Guarded by `is_none()` check on line 408; won't panic | +| `core/types.rs:436` | `self.stream.lock().expect("stream mutex poisoned")` | `StreamBidiStreamSource` is crate-private; lock held for microseconds | +| `core/types.rs:456` | `self.stream.lock().expect("stream mutex poisoned")` | Same as above | + +Additionally, `protocol/test_support.rs:92,99,110` has three +`expect("source mutex poisoned")` calls — these are in test-support code +(`SingleStreamSource`), which is a gray area (not `#[cfg(test)]` but +only used by tests). The `StaticIdentityProvider` in +`adapter.rs:199,211` and `dispatch.rs:1005` uses +`std::sync::Mutex::lock().unwrap()` — these are inside `#[cfg(test)]` +blocks and are fine. + +**Fix:** replace the three `expect` calls with +`unwrap_or_else(|e| e.into_inner())` per the convention. For +`operations.rs:415`, use `channel_open.as_ref().ok_or_else(|| +...)?.alpn.to_string()` to avoid the expect entirely. + +**Acceptance gate:** `grep -rn '\.expect(' src/ --include='*.rs' | grep +-v '#\[cfg(test)\]' | grep -v 'mod tests' | grep -v '#\[test\]' | grep +-v test_support` returns no matches. + +## R-03 [minor] — Unnecessary `#[allow(dead_code)]` on `generate_request_id` + +**Convention drift:** dead code annotation on a function that is +actually used. +**Verified:** YES. `registry/context.rs:147` has +`#[allow(dead_code)]` on `pub(crate) fn generate_request_id()`. The +function is called from `protocol/connection.rs` and +`protocol/dispatch.rs` — it is not dead code. The annotation is +unnecessary and misleading. + +Additionally, `protocol/adapter.rs:236` has `#[allow(dead_code)]` on +`fn internal_spec()` — this is a test helper that is genuinely unused +in the current test suite. It can be removed or kept with the +annotation; either is fine. + +**Fix:** remove the `#[allow(dead_code)]` from `context.rs:147`. + +**Acceptance gate:** `grep '#\[allow(dead_code)\]' src/registry/context.rs` +returns no matches. + +--- + +# Part C — Coverage Gaps + +## R-04 [major] — Single-stream call mode untested (connection.rs) + +**ADR drift:** ADR-036 amendment (single-stream call mode for channel 0 +multiplexed framing, added in Unit 2). +**Verified:** YES. `src/protocol/connection.rs` has 80.62% line coverage +(890/1104 lines). The uncovered regions are concentrated in the +single-stream call mode: + +| Function | Lines | Status | +|----------|-------|--------| +| `CallConnection::new_single_stream` | 121-132 | Untested | +| `CallConnection::is_single_stream` | 137-139 | Untested | +| `CallConnection::single_stream_writer` | 143-145 | Untested | +| `CallConnection::call_single_stream` | 262-291 | Untested | +| `CallConnection::subscribe_single_stream` | 375-396 | Untested | +| `CallConnection::publish_single_stream` | 522-567 | Untested | +| `read_single_stream_until_closed` | 654-659 | Untested | +| `split_single_stream` | 669-676 | Untested | +| `SharedFrameWriter` (all methods) | 51-59 | Untested directly | + +The stream-per-request path (`call_with_payload`, `subscribe_with_payload`, +`publish_with_payload` on multi-stream transports) is well-tested via +the publish end-to-end tests. The single-stream path is exercised +indirectly through the channel-0 end-to-end tests in +`channels/client.rs`, but `connection.rs` itself has no direct +single-stream tests. This is the largest coverage gap in a core protocol +file. + +**Fix:** add unit tests for the single-stream call mode: construct a +`CallConnection` with `new_single_stream`, exercise `call`, `subscribe`, +`publish`, and `abort` on the single-stream path, and assert correct +behavior. The channel-0 e2e tests in `client.rs` provide integration +coverage; these would provide unit-level coverage of the +`CallConnection` single-stream API directly. + +**Acceptance gate:** `connection.rs` line coverage ≥ 90%. + +## R-05 [major] — `ChannelsAdapter::handle()` untested (adapter.rs) + +**ADR drift:** ADR-034, ADR-036 (channels adapter accept-side entry +point). +**Verified:** YES. `src/channels/adapter.rs` has 82.13% line coverage +(170/207 lines). The `ProtocolHandler for ChannelsAdapter` impl's +`handle()` method (lines 208-253) is completely untested. This is the +main accept-side entry point for a channels connection — it installs +channel 0, spawns the mux runner, and runs the demux loop. The demux +loop itself (`run_demux_loop_for_client`) is well-tested with two good +tests (oversized chunk resync and backpressure), but the full +`handle()` flow is never exercised. + +**Fix:** add a test that constructs a `ChannelsAdapter`, calls +`handle()` with a duplex connection, and asserts that channel 0 is +installed and the demux loop processes frames. This is the accept-side +counterpart to the channel-0 e2e tests in `client.rs` (which test the +connect side). + +**Acceptance gate:** `adapter.rs` line coverage ≥ 90%. + +## R-06 [major] — `ChannelCore` / open-op machinery unit paths untested (operations.rs) + +**ADR drift:** ADR-047 §3 (per-ALPN open-op registration and +invocation). +**Verified:** YES. `src/channels/operations.rs` has 73.47% line coverage +(407/554 lines). The uncovered regions are concentrated in the +`ChannelCore` and open-op machinery added in Unit 3: + +| Function | Lines | Status | +|----------|-------|--------| +| `ChannelCore::new` | 343-345 | Untested | +| `ChannelCore::manager` | 347-349 | Untested | +| `ChannelCore::policy` | 351-353 | Untested | +| `ChannelCore::check_open` | 358-360 | Untested | +| `ChannelCore::on_close` | 364-366 | Untested | +| `ChannelCore::register_openable` | 399-449 | Untested directly | +| `opener_identity_from_context` | 459-465 | Untested | +| `run_open_wrapper` | 480-552 | Untested directly | +| `map_channel_error_to_call_error` | 558-575 | Untested | +| `make_open_handler_once` | 581-612 | Untested directly | +| `make_open_handler_stream` | 622-653 | Untested directly | +| `make_open_handler_sink` | 672-695 | Untested (stub) | +| `make_control_handler` success path | 254-267 | Untested | +| `make_close_handler` success path | 205-217 | Untested | +| `channel_control_spec` | 122-140 | Untested | + +The `register_openable` end-to-end test in `channels/client.rs` +exercises this machinery indirectly through channel 0, but the +unit-level coverage of the individual functions is missing. The +`ChannelOperations::new` constructor (line 52-54) is also untested +(only `with_default_policy` is used in tests). + +**Fix:** add unit tests for `ChannelCore` methods (`new`, `manager`, +`policy`, `check_open`, `on_close`), `opener_identity_from_context`, +`map_channel_error_to_call_error`, and the `make_open_handler_*` +functions. Test the `make_close_handler` and `make_control_handler` +success paths (channel exists, handler invoked). Test +`channel_control_spec`. + +**Acceptance gate:** `operations.rs` line coverage ≥ 85%. + +## R-07 [major] — `SendStream`/`RecvStream` newtypes untested (types.rs) + +**ADR drift:** ADR-092 (`BiStream` as the handler leaf; `SendStream` and +`RecvStream` are the public read/write half types). +**Verified:** YES. `src/core/types.rs` has 85.48% line coverage +(424/496 lines). The `SendStream` and `RecvStream` newtypes (lines +321-385) have zero test coverage: + +| Item | Lines | Status | +|------|-------|--------| +| `SendStream::from_stream` | 334-338 | Untested | +| `SendStream` `AsyncWrite` impl | 353-375 | Untested | +| `RecvStream::from_stream` | 346-350 | Untested | +| `RecvStream` `AsyncRead` impl | 377-385 | Untested | + +These are public API types that downstream crates will use. They are +thin newtypes over `tokio::io` halves, but their constructors and trait +impls should have basic smoke tests. + +Additionally, `BiStream::from_joined` (lines 261-269) — the public +constructor for joining independent read/write halves — is untested. +Downstream crates (channels reassembly) use it, but it has no direct +test coverage. + +**Fix:** add basic smoke tests for `SendStream`/`RecvStream` construction +and read/write round-trips, and a test for `BiStream::from_joined`. + +**Acceptance gate:** `types.rs` line coverage ≥ 90%. + +## R-08 [major] — `make_sink_forwarding_handler` untested (from_call.rs) + +**ADR drift:** ADR-046 §9 (sink forwarding for Pub ops in `from_call`). +**Verified:** YES. `src/client/from_call.rs` has 87.60% line coverage +(742/847 lines). The `make_sink_forwarding_handler` function (lines +458-477) has no test coverage. The `Query`/`Mutation` forwarding handler +(`make_forwarding_handler`) and the `Sub` forwarding handler +(`make_streaming_forwarding_handler`) are well-tested. The Pub +forwarding path is the only `HandlerKind` without a forwarding test. + +Additionally, the `from_call` happy path (lines 82-92) is untested — +only the error path (mock connection → `DiscoveryFailed`) is tested. +The happy path requires a live transport with `services/list` + +`services/schema` handlers registered, which is an integration test. + +**Fix:** add a test for `make_sink_forwarding_handler` that verifies it +forwards chunks and terminates on `Err` (matching the `SinkHandler` +contract). The `from_call` happy path can be deferred to an integration +test suite. + +**Acceptance gate:** `from_call.rs` line coverage ≥ 90%. + +## R-09 [minor] — `ChannelsSessionEnv` delegation methods untested (env.rs) + +**ADR drift:** ADR-047 §4 (per-connection `ChannelManager` resolution). +**Verified:** YES. `src/channels/env.rs` has 51.28% line coverage +(20/39 lines). The `OperationEnv for ChannelsSessionEnv` impl's +delegation methods (`invoke_with_policy`, `contains`, `peer_ids`, +`peer_contains`, `peer_operations`, `invoke_peer` — lines 57-98) are +completely untested. The only tested method is `channel_manager()`. + +The module doc says this type is "retained for future per-connection +routing" and "not on the open-op path," so the low coverage is +intentional. The `ChannelOperationEnv` trait and `ChannelsSessionEnv` +struct exist as integration points for future work. + +**Fix:** low priority. Either add basic delegation tests (construct a +`ChannelsSessionEnv` with a mock base `OperationEnv` and verify each +method delegates correctly) or document the intentional coverage gap +with an OQ reference. The file is small (39 lines) and the risk is low. + +**Acceptance gate:** `env.rs` line coverage ≥ 80%, or an OQ filed +documenting the intentional gap. + +## R-10 [minor] — `CallClient` accessors untested (call_client.rs) + +**Verified:** YES. `src/client/call_client.rs` has 87.10% line coverage +(108/124 lines). Two trivial accessors are untested: +`CallClient::registry` (lines 50-52) and +`CallClient::identity_provider` (lines 54-56). These are one-line +getters that return `Arc` clones. + +**Fix:** add one-line assertions that the accessors return the expected +values after construction. + +**Acceptance gate:** `call_client.rs` line coverage ≥ 90%. + +--- + +# Part D — Code Smells + +## R-11 [minor] — `#[allow(clippy::too_many_arguments)]` on large functions + +**Verified:** YES. Three locations: + +| File:Line | Function | Params | Notes | +|-----------|----------|--------|-------| +| `registry/spec.rs:210` | `OperationSpec::new` | 9 | Builder-alternative constructor; acceptable | +| `channels/operations.rs:479` | `run_open_wrapper` | 9 | Well-structured wrapper; could be refactored but not urgent | +| `registry/registration.rs:288` | `invoke_sink` | — | `#[allow(clippy::result_large_err)]` — acceptable | + +None of these are correctness issues. The `run_open_wrapper` function +(72 lines) is the largest and could benefit from being split into +smaller helpers, but it is well-structured with clear error branches. +This is a refactoring opportunity, not a bug. + +**Fix:** none required for v0.1.0. Consider refactoring `run_open_wrapper` +in a follow-up. + +## R-12 [minor] — Stub handlers return "not implemented" errors + +**Verified:** YES. Three handlers in `channels/operations.rs` are stubs +that return explicit error envelopes: + +| Handler | Lines | Error code | OQ | +|---------|-------|------------|-----| +| `make_control_handler` | 260-267 | `channel:control_not_implemented` | OQ-39 | +| `make_resources_subscribe_handler` | 284-293 | `channel:resources_subscribe_not_implemented` | OQ-40 | +| `make_open_handler_sink` | 679-694 | `channel:sink_open_not_implemented` | — | + +These are intentional — the features are decided but deferred, and the +stubs return honest errors rather than fake success. This is the correct +pattern. Not a smell, but worth noting for the feature-completeness +assessment. + +**Fix:** none required. These will be implemented when the corresponding +OQs are resolved. + +--- + +# Cross-cutting: what is solid (verified) + +These were checked and are correct — listed so the remediation plan can +focus on what actually needs work: + +- **Wire formats:** `EventEnvelope` shape and the five+one event types + match spec exactly; the channels 8-byte chunk header + `[channel_id:u32 BE][length:u32 BE]` matches spec exactly. No + `stream_type` anywhere in the channels layer. ✓ +- **`HandlerKind::Sink` validation:** Pub↔Sink both directions, all + builder methods tested. ✓ +- **Wire `op_type` strings:** `"query"`/`"mutation"`/`"sub"`/`"pub"` + emit and parse correctly. ✓ +- **`call.published` payload:** `{ "input": }` shape, correlated + by envelope `id`. ✓ +- **Gap F (`channel_open` marker):** `spec_to_json` emits, + `rebuild_spec_for` parses, well tested. ✓ +- **Per-identity cap default 256:** tested. ✓ +- **Channel 0 pre-installed, skips ledger:** correct bookkeeping. ✓ +- **Odd/even ID split:** connect side starts at 1, accept side at 2 — + no collisions. ✓ +- **Layering held:** no channels types leaked into call crate, + `OperationEnv` stayed a trait, auth stays on the one `AccessControl` + path. ✓ +- **No `unsafe` code** anywhere in the codebase. ✓ +- **No `todo!()` / `unimplemented!()` / `unreachable!()`** panicking + macros in library code. ✓ +- **No `println!` / `eprintln!`** in library code. ✓ +- **No hardcoded secrets, keys, or tokens.** ✓ +- **All review-001 critical/major bugs fixed:** Units 1-10 landed and + verified (commits `8066d08` through `7cd8a57`). ✓ +- **483 tests, 0 failures.** ✓ +- **Clippy, fmt, doc: all clean.** ✓ +- **License files present:** `LICENSE-MIT` and `LICENSE-APACHE`. ✓ +- **Cargo.toml metadata:** version, edition, rust-version, license, + description, repository, keywords, categories, exclude list — all + correct. ✓ +- **Dependencies:** all from crates.io with specific versions. `alktype` + is a declared dependency and is actively used (JSON schema validation + via `alktype::validation::build_validator`). ✓ + +--- + +# Remediation plan + +The plan is split into **five units of work**, ordered by dependency and +risk. Each unit is an independently shippable commit with its own +acceptance gate. Units are sized for a single session each. + +## Unit 1 — Publishing blocker + convention fixes (R-01, R-02, R-03) + +**Goal:** `cargo publish --dry-run` succeeds; no `expect` in library +code; no unnecessary `#[allow(dead_code)]`. +**Files:** `Cargo.toml`, `src/channels/operations.rs`, +`src/core/types.rs`, `src/registry/context.rs`. +**Scope:** +- **R-01:** remove the `readme = "README.md"` field from `Cargo.toml` + (the README will be written as the second-to-last step before + publishing; the field can be restored then). +- **R-02:** replace the three `expect` calls in library code: + - `operations.rs:415`: use `channel_open.as_ref().ok_or_else(|| + ...)?.alpn.to_string()`. + - `types.rs:436,456`: use `unwrap_or_else(|e| e.into_inner())`. +- **R-03:** remove `#[allow(dead_code)]` from `context.rs:147`. +**Acceptance gate:** `cargo publish --dry-run --allow-dirty` succeeds; +`grep -rn '\.expect(' src/ --include='*.rs' | grep -v '#\[cfg(test)\]' +| grep -v 'mod tests' | grep -v '#\[test\]' | grep -v test_support` +returns no matches; `cargo test` green; clippy/fmt/doc clean. + +## Unit 2 — Single-stream call mode tests (R-04) + +**Goal:** `connection.rs` line coverage ≥ 90%. +**Files:** `src/protocol/connection.rs`. +**Scope:** +- Add unit tests for `CallConnection::new_single_stream`, + `is_single_stream`, `single_stream_writer`. +- Add unit tests for `call_single_stream`, `subscribe_single_stream`, + `publish_single_stream` — construct a single-stream `CallConnection` + over a duplex pair, exercise each call type, assert correct + request/response correlation. +- Add unit tests for `SharedFrameWriter` (write_frame, concurrent + writes). +- Add unit tests for `read_single_stream_until_closed` and + `split_single_stream`. +- The channel-0 e2e tests in `channels/client.rs` already provide + integration coverage; these tests provide unit-level coverage of the + `CallConnection` single-stream API directly. +**Acceptance gate:** `connection.rs` line coverage ≥ 90%; `cargo test` +green; clippy/fmt/doc clean. + +## Unit 3 — Channels adapter + operations coverage (R-05, R-06) + +**Goal:** `adapter.rs` line coverage ≥ 90%; `operations.rs` line coverage +≥ 85%. +**Files:** `src/channels/adapter.rs`, `src/channels/operations.rs`. +**Scope:** +- **R-05:** add a test for `ChannelsAdapter::handle()` — construct a + `ChannelsAdapter`, call `handle()` with a duplex connection, assert + channel 0 is installed and the demux loop processes frames. This is + the accept-side counterpart to the channel-0 e2e tests in `client.rs`. +- **R-06:** add unit tests for `ChannelCore` methods (`new`, `manager`, + `policy`, `check_open`, `on_close`). +- Add unit tests for `opener_identity_from_context`, + `map_channel_error_to_call_error`. +- Add unit tests for `make_open_handler_once` and + `make_open_handler_stream` (construct with mock manager/policy, + invoke, assert response). +- Add unit tests for `make_close_handler` and `make_control_handler` + success paths (channel exists, handler invoked). +- Add a unit test for `channel_control_spec`. +- Add a unit test for `ChannelOperations::new` (the non-default-policy + constructor). +**Acceptance gate:** `adapter.rs` ≥ 90%, `operations.rs` ≥ 85%; `cargo +test` green; clippy/fmt/doc clean. + +## Unit 4 — Core types + from_call coverage (R-07, R-08, R-10) + +**Goal:** `types.rs` line coverage ≥ 90%; `from_call.rs` line coverage ≥ +90%; `call_client.rs` line coverage ≥ 90%. +**Files:** `src/core/types.rs`, `src/client/from_call.rs`, +`src/client/call_client.rs`. +**Scope:** +- **R-07:** add smoke tests for `SendStream::from_stream` and + `RecvStream::from_stream` — construct from a duplex pair, write/read a + few bytes, assert round-trip. +- Add a test for `BiStream::from_joined` — join independent read/write + halves, write on one side, read on the other. +- **R-08:** add a test for `make_sink_forwarding_handler` — construct a + mock `CallConnection`, feed a `PublishStream` with a few chunks, + assert the handler forwards them and terminates on `Err`. +- **R-10:** add assertions that `CallClient::registry` and + `CallClient::identity_provider` return the expected values. +**Acceptance gate:** `types.rs` ≥ 90%, `from_call.rs` ≥ 90%, +`call_client.rs` ≥ 90%; `cargo test` green; clippy/fmt/doc clean. + +## Unit 5 — Remaining coverage gaps + final verification (R-09, R-11, R-12) + +**Goal:** all files ≥ 80% line coverage; final verification pass. +**Files:** `src/channels/env.rs`, `src/channels/operations.rs` +(refactor opportunity). +**Scope:** +- **R-09:** either add basic delegation tests for `ChannelsSessionEnv` + (construct with a mock base `OperationEnv`, verify each method + delegates correctly) or file an OQ documenting the intentional + coverage gap. The file is small (39 lines) and the risk is low. +- **R-11:** optional — consider refactoring `run_open_wrapper` (72 lines, + 9 params) into smaller helpers. Not required for v0.1.0. +- **R-12:** no action — stub handlers are intentional and documented. +- Final verification: run `cargo llvm-cov --summary-only` and confirm + all files ≥ 80% line coverage, overall ≥ 92%. +**Acceptance gate:** all files ≥ 80% line coverage; `cargo test` green; +clippy/fmt/doc clean; `cargo publish --dry-run --allow-dirty` succeeds. + +## Suggested sequencing + +``` +Unit 1 (publishing blocker + conventions) → no deps; do first +Unit 2 (single-stream call mode tests) → no deps; independent +Unit 3 (adapter + operations coverage) → no deps; independent +Unit 4 (core types + from_call coverage) → no deps; independent +Unit 5 (remaining gaps + final verify) → depends on Units 1-4 +``` + +Units 1-4 are independent and can proceed in any order (or in parallel +across separate branches). Unit 5 is the final verification pass and +should land last. + +--- + +## Verification log (this pass) + +Findings re-verified directly in source on 2026-08-14 against tree +`7cd8a57`. Every `file:line` reference above was checked. + +- **R-01:** verified by running `cargo publish --dry-run --allow-dirty` + and confirming the README.md error. License files present. +- **R-02:** verified by grepping `src/` for `\.expect(` and `\.unwrap(` + outside `#[cfg(test)]` blocks. Three matches in library code, all in + crate-private or guarded contexts. +- **R-03:** verified by reading `context.rs:147` and confirming + `generate_request_id` is called from `connection.rs` and + `dispatch.rs`. +- **R-04 through R-10:** verified by running `cargo llvm-cov + --json --summary-only` and cross-referencing uncovered regions against + the source. Coverage percentages confirmed against the JSON output. +- **R-11, R-12:** verified by reading the source at the referenced + lines. + +No `unsafe` blocks found (verified by grep). No `todo!()` / +`unimplemented!()` / `unreachable!()` found (verified by grep). No +`println!` / `eprintln!` in library code (verified by grep). No +hardcoded secrets (verified by manual review of `Cargo.toml` and source +tree).