From 7b0bac0671aafa9e55137e2202e97d42b6becbfd Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Sat, 5 Sep 2026 16:36:22 +0000 Subject: [PATCH] docs: mark review #003 P12 + P6 + P7 + P4 + P16 resolved (c8e4610) --- docs/reviews/003-prepublish-review.md | 81 +++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 5 deletions(-) diff --git a/docs/reviews/003-prepublish-review.md b/docs/reviews/003-prepublish-review.md index 04a5fdf..ef3f159 100644 --- a/docs/reviews/003-prepublish-review.md +++ b/docs/reviews/003-prepublish-review.md @@ -1,6 +1,7 @@ --- status: open last_updated: 2026-09-05 +resolved: 1, 4, 6, 7, 12, 13, 16 (P14 closed) reviewed_artifacts: - src/lib.rs - src/wire.rs @@ -28,6 +29,7 @@ reviewer: prepublish review for v0.1.0 (subagent-assisted) base_commit: 918af40 (review #002 resolved) resolutions: - "P1 + P13 resolved 2026-09-05 (87c52e5) — see the Session-1 Resolution section" + - "P12 + P6 + P7 + P4 + P16 resolved 2026-09-05 — see the Session-2 Resolution section" --- # Code Review #003 — Prepublish Review (v0.1.0) @@ -467,19 +469,19 @@ theirs; don't replicate that nit. | P1 | pump_session deadlock ≥63 chunks | concurrent drainer + regression test | small | medium (core pump) | ✅ resolved (`87c52e5`) | | P2 | AGENTS.md ships; dead exclude entries | fix `exclude` | trivial | none | open | | P3 | stale AGENTS.md + architecture README | refresh phase/ADR/alkcall text | trivial | none | open | -| P4 | no README + `readme` field | write README (after P5/P6/P12) | medium | none | open | +| P4 | no README + `readme` field | write README (after P5/P6/P12) | medium | none | ✅ resolved (Session 2) | | P5 | scope gate after backend lookup | reorder checks | trivial | low (test updates) | open | -| P6 | write paths accept invalid chunks | validate on write; unify empty-payload shape | small | low | open | -| P7 | read-after-peek framing corruption | peeked-state guard | small | none | open | +| P6 | write paths accept invalid chunks | validate on write; unify empty-payload shape | small | low | ✅ resolved (Session 2) | +| P7 | read-after-peek framing corruption | peeked-state guard | small | none | ✅ resolved (Session 2) | | P8 | poll_shutdown missing waker | inflight-future pattern | small | low | open | | P9 | poisoned-lock `.expect()` × 5 | `into_inner()` | trivial | none | open | | P10 | thread-spawn `.expect()` × 3 | map to `TtyError` | trivial | none | open | | P11 | recv_stdout sentinel contract | terminate stream on sentinel (or fix doc) | small | low (semver-visible) | open | -| P12 | no crate-root re-exports | `pub use` primary types | trivial | none | open | +| P12 | no crate-root re-exports | `pub use` primary types | trivial | none | ✅ resolved (Session 2) | | P13 | test gaps | backpressure (>64 chunks) + boundary tests | small | none | ✅ resolved (`87c52e5`; accept-loop harness deferred) | | P14 | `Result<(), String>` on register_openable | closed — alkcall parity | none | none | ✅ closed | | P15 | minor polish batch | one batched commit | small | none | open | -| P16 | no CHANGELOG.md | start changelog with [0.1.0] | small | none | open | +| P16 | no CHANGELOG.md | start changelog with [0.1.0] | small | none | ✅ resolved (Session 2) | ## Suggested Session Breakdown @@ -532,6 +534,75 @@ Verification on the resolution tree: `cargo test` 98 lib (was 95) / `cargo test --all-features` 141 (was 138); clippy (all-targets + wasm32), fmt, wasm check, doc — all clean. +## Session-2 Resolution (P12 + P6 + P7 + P4 + P16, 2026-09-05) + +The API-shape batch (P12/P6/P7) plus the docs that use the final +surface (P4/P16), per the review's suggested session breakdown item 2. +Commit: `c8e4610`. + +- **P12 — crate-root re-exports** (`src/lib.rs`): `pub use` of the + primary types — `TtyAdapter`, `drive_session`, + `drive_session_pre_negotiated`, `TTY_OPEN_SCOPE` (adapter); + `TtyBackend`, `TtyHandle`, `TtyParams`, `TerminalParams`, + `TtyControl`, `TtyControlHandle`, `TtyError`, `BoxFuture` (backend); + `TtySession`, `TtySessionError` (session); `Chunk`, `ChunkReader`, + `ChunkWriter`, `RawError` and the five `STREAM_*` constants + + `CHUNK_HEADER_LEN` + `MAX_CHUNK_LEN` (wire); `ControlMessage` and + `signal_from_name` — the latter `#[cfg(unix)]`-gated to match its + definition (caught by the wasm check); `NegotiateRequest`, + `TerminalParamsWire`, `NegotiationError`, `NegotiationReader`, + `NegotiationWriter`, `error_response_bytes` (negotiation); + `register_openable`, `tty_open_spec`, `OP_TTY_OPEN`, `TTY_ALPN` + (channels); `LocalTtyBackend` under the `local` feature. Module + paths remain the full surface; the lib.rs doc comment gained a + "Crate-root surface" section explaining the split. +- **P6 — write-path validation** (`src/wire.rs`): all four + `ChunkWriter` methods validate via a shared `validate_header` + (stream_type ≤ 4, length ≤ `MAX_CHUNK_LEN`) before touching the + transport, reusing `RawError::InvalidStreamType`/`ChunkTooLarge`. + The length is validated as `u64` *before* the `u32` cast, so an + oversized payload fails instead of wrapping past the check — + validating the truncated value would have reintroduced the exact bug + the validation exists to catch. Empty-payload shape unified: one + `write_validated` helper emits `length == 0` + no payload bytes for + all four paths (the old `write_ctrl_*` always-write shape is gone; + wire-equivalent). Tests: reject-invalid-stream-type (nothing + written), reject-oversized (writer + all three helpers), and + exact-boundary acceptance. +- **P7 — peek-state guard** (`src/wire.rs`, `src/session.rs`): + `ChunkReader` tracks peeked state. `read_chunk()` after + `peek_stream_type()` completes the peeked chunk instead of consuming + a second header byte; a second peek is idempotent; + `read_chunk_after_peek` debug-asserts a peek happened. This let the + session's read pump drop its manual `first_byte_peeked` threading — + `read_chunk()` handles both orders. Tests: peek-then-`read_chunk` + delivers two well-framed chunks (the old shape silently read the + peeked byte's length field from the payload), and peek idempotence. +- **P4 — README + `readme` field**: `README.md` mirrors alkcall's + structure — tagline + positioning, `## Quick start` with per-role + examples (producer: backends map + `TtyAdapter` as `ProtocolHandler`; + producer-via-channels: `register_openable`; consumer: + `TtySession::connect_direct` with the full typed-method flow; + consumer-via-channels: `open_via_channels`), `## Architecture` + (producer/consumer table per convention 7, wire-format summary, + wasm + `local` note), `## Documentation`, `## License`. Examples use + the P12 root paths. `readme = "README.md"` added to `[package]`; + the dry-run package list confirms it ships. +- **P16 — CHANGELOG.md**: Keep-a-Changelog format with a full + `[0.1.0]` entry written from the commit log (wire format, backend + trait, local backend, adapter, channels integration, session + consumer, control messages, plus the Session-1/2 fixes folded into + their features), with the `[0.1.0]` release-tag link definition + (alkcall's missing-link nit not replicated). ADR references use the + actual on-disk numbering (exit code = ADR-004, cancel-cleanup = + ADR-005, framing = ADR-006, channels = ADR-008/009). + +Verification on the resolution tree: `cargo test` 104 lib (was 98) / +`cargo test --all-features` 128 lib + 19 integration = 147 (was 141); +clippy (all-targets + wasm32) `-D warnings`, fmt, wasm check, doc +(0 warnings), `cargo publish --dry-run --allow-dirty` (44 files, +702.8KiB) — all clean. + ## Notes - Line numbers refer to the tree at `918af40` (review base).