From 3d3bb5e9bfc23c2a0bb5a350bbf76d4f208f119a Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Thu, 29 Oct 2026 07:27:59 +0000 Subject: [PATCH] test(mapping): land the channel-not-held guard's wire rows on all three served rows (w4-channel-not-held-rows) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - git row: a second exec on a served git channel answers channel_failure inline — the serve rides on, no second serving path; race-free by construction (advertisement drained, serve parked at the gated list_refs) - tty rows: the pipe row (second exec) and the shell row (second shell request) — the same inline decline shape, channel still usable - the rows assert the decline event's exact request vocabulary (class request, reason channel-not-held, route git/pipe/absent, debug level, self-describing build) — the class's vocabulary coverage - tests/common/event_capture.rs: the observe-suite's capture convention shared across the mapping binaries - test-only: no production change; the guard held its documented shape in every featured build (verified default / git / session / session,git / tunnel / sftp / --all-features) Verified: cargo test (+ per-feature + --all-features) all green · cargo clippy --all-targets -- -D warnings ✓ · cargo clippy --all-features --all-targets -- -D warnings ✓ · cargo fmt --check ✓ · cargo doc --no-deps 0 warnings · taskgraph validate ✓ (35 tasks) --- .../w4-channel-not-held-rows.md | 66 +++++- tests/common/event_capture.rs | 195 ++++++++++++++++++ tests/common/mod.rs | 6 + tests/git_mapping.rs | 87 ++++++++ tests/tty_mapping.rs | 125 ++++++++++- 5 files changed, 470 insertions(+), 9 deletions(-) create mode 100644 tests/common/event_capture.rs diff --git a/tasks/implementation/w4-channel-not-held-rows.md b/tasks/implementation/w4-channel-not-held-rows.md index 7b54ab7..ab79ed7 100644 --- a/tasks/implementation/w4-channel-not-held-rows.md +++ b/tasks/implementation/w4-channel-not-held-rows.md @@ -1,7 +1,7 @@ --- id: w4-channel-not-held-rows name: Wave 4 — channel-not-held guard wire rows (wave-3 gate booked item 2) -status: pending +status: completed depends_on: [w4-git-exit-close] scope: narrow risk: low @@ -40,12 +40,13 @@ than silently widening scope): ## Acceptance Criteria -- [ ] Wire rows for the channel-not-held decline class on all three +- [x] Wire rows for the channel-not-held decline class on all three rows (git / tty pipe / shell), per build, through the harness -- [ ] The rows assert the wire shape + the tracing event class/reason -- [ ] No production code change (or a stop-and-booked finding - recorded for the gate — nothing silent) -- [ ] `cargo test` (+ per-feature + `--all-features`), clippy ×2 +- [x] The rows assert the wire shape + the tracing event class/reason +- [x] No production code change (no guard gap exposed — nothing to + book; the guard's shape held on the wire in every featured + build) +- [x] `cargo test` (+ per-feature + `--all-features`), clippy ×2 (`-D warnings`), fmt, `cargo doc --no-deps` pass ## References @@ -59,8 +60,57 @@ than silently widening scope): ## Notes -> To be filled by implementation agent +- **Test-only, as planned** — no production change; no guard gap + surfaced (the landing verified the guard's documented shape on the + wire in every build that carries each row). +- **Rows landed** (one per served row, each with the inline-decline + wire shape — `channel_failure` while the first serving path rides + on — plus the channel-stays-usable witness and the tracing event's + exact request-decline vocabulary `src/observe.rs` carries): + - `tests/git_mapping.rs` (cfg `git`): a second exec on a served git + channel — the decline event carries `exec_route = "git"`, and the + declined re-exec never spawns a second serving path (the doubles' + resolve count stays at one). Race-free by construction: the + advertisement drains first and the serve parks at the doubles' + gated `list_refs` behind it, so the wire is quiescent for the + decline. + - `tests/tty_mapping.rs` (cfg `session`), two rows: + the pipe row (second exec on a served pipe channel — event + carries `exec_route = "pipe"`) and the shell row (second shell + request on a served shell channel — the event carries **no** + `exec_route` field, pinning the absent-field shape on the wire). + The exit-`None` shim script keeps the serve live and the wire + quiescent. +- **The per-build matrix**: the rows join the cfg-pinned convention — + git-build rows run in every build carrying `git`, session rows in + every build carrying `session` (verified across default / `git` / + `session` / `session,git` / `tunnel` / `sftp` / `--all-features`). +- **Shared capture helper**: `tests/common/event_capture.rs` (new) — + the observe-suite's recorded hand-rolled global-subscriber + convention shared across the mapping binaries; `await_event` + field-matching (parallel rows never order against each other) and + the exact-field-set assertion (`assert_request_decline`) — the + rows double as this class's vocabulary coverage, including the + `debug` level and the build self-description. +- **No `dispatch_core.rs` rows added** — that binary builds no seam, + so its git row declines at the config gate *before* the store-take; + the guard is unreachable there (the reference names the per-build + convention the rows join, which the mapping binaries carry). ## Summary -> To be filled on completion \ No newline at end of file +The channel-not-held guard's wire rows landed on all three served +rows: `second_exec_on_a_served_git_channel_declines_channel_not_held` +(`tests/git_mapping.rs`), and the pipe/shell pair +(`tests/tty_mapping.rs`) — each pinning the inline decline shape +(`channel_failure`, no close, the first serve rides on, the channel +stays usable, no second serving path) plus the decline event's exact +vocabulary (class `request`, reason `channel-not-held`, route +`git`/`pipe`/absent, `debug` level, self-describing build). No +production change was needed — the guard held its documented shape in +every featured build. `tests/common/event_capture.rs` shares the +observe-suite's capture convention across the mapping binaries. +Verified: full suite + per-feature builds (default, `git`, `session`, +`session,git`, `tunnel`, `sftp`) + `--all-features` all green; +clippy ×2 (`-D warnings`), `cargo fmt --check`, `cargo doc --no-deps` +clean. \ No newline at end of file diff --git a/tests/common/event_capture.rs b/tests/common/event_capture.rs new file mode 100644 index 0000000..7e6e381 --- /dev/null +++ b/tests/common/event_capture.rs @@ -0,0 +1,195 @@ +//! The decline-row tracing capture (`w4-channel-not-held-rows`): a +//! minimal hand-rolled global [`tracing::Subscriber`] recording every +//! event's level + field pairs, the mapping binaries' decline-class +//! rows assert the [`crate`'s event vocabulary](alkssh's observe +//! module) against. The recorded test convention (see +//! `tests/observe.rs`'s module docs) applies verbatim: no +//! `tracing-subscriber` dependency (the hand-rolled shape needs none), +//! and the **global** default is the only capture shape that works — +//! the server side runs on spawned tasks, so a thread-local default +//! never sees their events. +//! +//! Rows install once per binary with [`install`], then match events by +//! field pairs ([`await_event`]) — parallel rows never order against +//! each other — and pin the decline event's exact vocabulary with +//! [`assert_request_decline`] (the exact-field-set assertion is the +//! safety posture: no dynamic extra fields exist). + +#![allow(dead_code)] + +use std::sync::{Arc, Mutex, OnceLock}; +use std::time::{Duration, Instant}; + +/// One captured event: the level and the field pairs, in emit order. +#[derive(Debug, Clone)] +pub struct Captured { + pub level: String, + pub fields: Vec<(String, String)>, +} + +impl Captured { + fn field(&self, name: &str) -> Option<&str> { + self.fields + .iter() + .find(|(n, _)| n == name) + .map(|(_, v)| v.as_str()) + } + + fn has(&self, want: &[(&str, &str)]) -> bool { + want.iter() + .all(|(n, v)| self.field(n).is_some_and(|got| got == *v)) + } +} + +struct FieldVisitor(Vec<(String, String)>); + +impl tracing::field::Visit for FieldVisitor { + fn record_str(&mut self, field: &tracing::field::Field, value: &str) { + self.0.push((field.name().to_owned(), value.to_owned())); + } + + fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { + self.0.push((field.name().to_owned(), format!("{value:?}"))); + } +} + +#[derive(Default)] +struct GlobalCapture(Mutex>); + +impl tracing::Subscriber for GlobalCapture { + fn enabled(&self, _metadata: &tracing::Metadata<'_>) -> bool { + true + } + + fn new_span(&self, _span: &tracing::span::Attributes<'_>) -> tracing::Id { + tracing::Id::from_u64(1) + } + + fn record(&self, _span: &tracing::Id, _values: &tracing::span::Record<'_>) {} + + fn record_follows_from(&self, _span: &tracing::Id, _follows: &tracing::Id) {} + + fn event(&self, event: &tracing::Event<'_>) { + let mut visitor = FieldVisitor(Vec::new()); + event.record(&mut visitor); + self.0 + .lock() + .unwrap_or_else(|e| e.into_inner()) + .push(Captured { + level: event.metadata().level().to_string(), + fields: visitor.0, + }); + } + + fn enter(&self, _span: &tracing::Id) {} + fn exit(&self, _span: &tracing::Id) {} + fn clone_span(&self, _id: &tracing::Id) -> tracing::Id { + tracing::Id::from_u64(1) + } +} + +/// The installed capture — one per binary (the [`OnceLock`] makes the +/// exactly-once shape explicit). +fn capture() -> &'static Arc { + static CAPTURE: OnceLock> = OnceLock::new(); + CAPTURE.get_or_init(|| { + let capture = Arc::new(GlobalCapture::default()); + tracing::subscriber::set_global_default(Arc::clone(&capture)) + .expect("the capture suite installs the global capture exactly once"); + capture + }) +} + +/// Installs the global capture once per binary. Call it before a row's +/// first wire exchange, so no event is emitted below the +/// not-yet-installed global. +pub fn install() { + let _ = capture(); +} + +/// Waits until an event matching every field pair appears, then +/// returns it — the spawned-task surfaces land their events +/// asynchronously (the deadline loop is the observe-suite convention). +pub async fn await_event(want: &[(&str, &str)]) -> Captured { + let capture = capture(); + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let events = capture.0.lock().unwrap_or_else(|e| e.into_inner()).clone(); + if let Some(event) = events.into_iter().find(|e| e.has(want)) { + return event; + } + if Instant::now() > deadline { + panic!("no tracing event matching {want:?} within the window"); + } + tokio::time::sleep(Duration::from_millis(10)).await; + } +} + +/// The expected `feature_build` value, derived from `cfg!` +/// independently of the crate's match — divergence breaks this loudly. +pub fn expected_build() -> &'static str { + match ( + cfg!(feature = "session"), + cfg!(feature = "git"), + cfg!(feature = "tunnel"), + cfg!(feature = "sftp"), + ) { + (false, false, false, false) => "none", + (true, false, false, false) => "session", + (false, true, false, false) => "git", + (false, false, true, false) => "tunnel", + (false, false, false, true) => "sftp", + (true, true, false, false) => "session+git", + (true, false, true, false) => "session+tunnel", + (true, false, false, true) => "session+sftp", + (false, true, true, false) => "git+tunnel", + (false, true, false, true) => "git+sftp", + (false, false, true, true) => "tunnel+sftp", + (true, true, true, false) => "session+git+tunnel", + (true, true, false, true) => "session+git+sftp", + (true, false, true, true) => "session+tunnel+sftp", + (false, true, true, true) => "git+tunnel+sftp", + (true, true, true, true) => "session+git+tunnel+sftp", + } +} + +/// The request-decline vocabulary assertion: a captured request-level +/// decline carries exactly the vocabulary (sorted-name-set comparison — +/// nothing more; the dynamic-free shape is the safety posture), the +/// build self-describing, the message shape, and the `debug` level (a +/// decline is the door's normal, honest posture). `exec_route` present +/// iff the row was a routed exec. +pub fn assert_request_decline(event: &Captured, exec_route: bool) { + assert_eq!( + event.level, "DEBUG", + "a decline emits at debug — the vocabulary's level posture" + ); + assert_eq!( + event.field("feature_build"), + Some(expected_build()), + "every event is self-describing per build" + ); + assert_eq!( + event.field("message"), + Some("decline: channel request rejected"), + "the request-level decline message shape" + ); + let mut names: Vec<&str> = event.fields.iter().map(|(n, _)| n.as_str()).collect(); + names.sort_unstable(); + let mut expected: Vec<&str> = vec![ + "channel_id", + "decline_class", + "feature_build", + "message", + "reason", + "row", + ]; + if exec_route { + expected.push("exec_route"); + } + expected.sort_unstable(); + assert_eq!( + names, expected, + "a request decline carries exactly the vocabulary, nothing else" + ); +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 5c94ed3..7be0f38 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -66,6 +66,12 @@ pub mod git_doubles; #[cfg(feature = "session")] pub mod tty_doubles; +/// The decline-row tracing capture (`w4-channel-not-held-rows`) — the +/// mapping binaries' decline-class rows assert the event vocabulary +/// through it (the module's own doc records the convention: +/// `tests/common/event_capture.rs`). +pub mod event_capture; + use std::collections::HashMap; use alkcall::core::auth::{AuthToken, Identity, IdentityProvider}; diff --git a/tests/git_mapping.rs b/tests/git_mapping.rs index 67b34c9..76f9837 100644 --- a/tests/git_mapping.rs +++ b/tests/git_mapping.rs @@ -21,6 +21,7 @@ mod serving { use russh::client::Msg; use russh::{Channel, ChannelMsg}; + use super::common::event_capture as ec; use super::common::git_doubles as gd; use super::common::harness::{ DataGate, LoopbackSession, authenticate_publickey, await_close, drive, @@ -864,6 +865,92 @@ mod serving { } session.unmount().await; } + + // ------------------------------------------------------------------ + // The channel-not-held guard (`w4-channel-not-held-rows`): a second + // exec on an already-served channel declines request-level — the + // store-and-take handoff holds a channel exactly once, the wave-3 + // gate's booked finding 2. + // ------------------------------------------------------------------ + + /// - *Contract stamp:* mapping.md §Request-level rows (the git + /// row's store-and-take handoff: the `channel-not-held` class — + /// a second exec on an already-served channel answers + /// `channel_failure`, inline, the first serve rides on — no + /// second serving path ever spawns) + the event vocabulary + /// (`src/observe.rs`: class `request`, reason + /// `channel-not-held`, route `git`; the advertisement drains + /// first and the serve parks at the gated `list_refs` behind it, + /// so the wire is quiescent for the decline — race-free by + /// construction). + #[tokio::test] + async fn second_exec_on_a_served_git_channel_declines_channel_not_held() { + ec::install(); + let doubles = Arc::new(gd::MemGit::new()); + doubles.install(gd::RepoSetup::private_granted( + "owner/demo", + "git-rider", + vec![], + )); + let serve_gate = doubles.install_gate("owner/demo"); + let session = git_session(&doubles).await; + let mut channel = route_git(&session, "git-upload-pack '/owner/demo'").await; + let mut framer = PktFramer::new(); + loop { + match next_packet(&mut channel, &mut framer, "the advertisement").await { + Packet::Flush => break, + Packet::Payload(_) => {} + } + } + + for _ in 0..200 { + if doubles.resolved_ids().len() == 1 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert_eq!( + doubles.resolved_ids(), + vec!["owner/demo".to_owned()], + "the first serve resolved and parks at the gated list_refs behind \ + its advertisement — the stored channel is taken, the wire quiescent" + ); + + request_exec(&channel, "git-upload-pack '/owner/demo'") + .await + .expect("the second exec rides the wire"); + match next_message(&mut channel).await { + Some(ChannelMsg::Failure) => {} + other => { + panic!("a second exec on a served channel answers channel_failure, got {other:?}") + } + } + + let event = ec::await_event(&[ + ("row", "exec_request"), + ("decline_class", "request"), + ("reason", "channel-not-held"), + ("exec_route", "git"), + ]) + .await; + ec::assert_request_decline(&event, true); + assert_eq!( + doubles.resolved_ids().len(), + 1, + "the declined re-exec never spawned a second serving path" + ); + + request_subsystem(&channel, "sftp") + .await + .expect("the alive channel carries another request"); + match next_message(&mut channel).await { + Some(ChannelMsg::Failure) => {} + other => panic!("the inline decline leaves the channel usable, got {other:?}"), + } + + serve_gate.release(); + session.unmount().await; + } } #[cfg(unix)] diff --git a/tests/tty_mapping.rs b/tests/tty_mapping.rs index f5b0ce2..0023215 100644 --- a/tests/tty_mapping.rs +++ b/tests/tty_mapping.rs @@ -25,9 +25,11 @@ mod serving { use russh::client::Msg; use russh::{Channel, ChannelMsg, Sig}; + use super::common::event_capture as ec; use super::common::harness::{ LoopbackSession, authenticate_publickey, drive, next_message, open_session_channel, - request_exec, request_pty, request_shell, serve_config as harness_serve_config, + request_exec, request_pty, request_shell, request_subsystem, + serve_config as harness_serve_config, }; use super::common::tty_doubles as td; use super::common::{MemHostKeyPolicy, MemIdentityProvider, fixture, identity}; @@ -405,6 +407,127 @@ mod serving { session.unmount().await; } + // ------------------------------------------------------------------ + // The channel-not-held guard (`w4-channel-not-held-rows`): a second + // request on an already-served channel declines request-level — the + // store-and-take handoff holds a channel exactly once, the wave-3 + // gate's booked finding 2. + // ------------------------------------------------------------------ + + /// - *Contract stamp:* mapping.md §Request-level rows (the exec + /// row's store-and-take handoff: the `channel-not-held` class — + /// a second exec on an already-served pipe channel answers + /// `channel_failure`, inline, the first serve rides on) + the + /// event vocabulary (`src/observe.rs`: class `request`, reason + /// `channel-not-held`, route `pipe`; the exit-`None` script + /// keeps the serve live and the wire quiescent, so the row is + /// race-free by construction). + #[tokio::test] + async fn second_exec_on_a_served_pipe_channel_declines_channel_not_held() { + ec::install(); + let shim = Arc::new(td::TtyShim::new()); + shim.set_script(td::ShimScript { + stdout: vec![], + stderr: vec![], + exit: None, + }); + let session = session_with(shim_seam(&shim)).await; + + let mut channel = open_session_channel(&session.client) + .await + .expect("the session channel opens"); + request_exec(&channel, "uptime") + .await + .expect("the first exec routed"); + expect_success(&mut channel, "the served pipe exec").await; + + request_exec(&channel, "uptime") + .await + .expect("the second exec rides the wire"); + match next_message(&mut channel).await { + Some(ChannelMsg::Failure) => {} + other => { + panic!("a second exec on a served channel answers channel_failure, got {other:?}") + } + } + + let event = ec::await_event(&[ + ("row", "exec_request"), + ("decline_class", "request"), + ("reason", "channel-not-held"), + ("exec_route", "pipe"), + ]) + .await; + ec::assert_request_decline(&event, true); + + request_subsystem(&channel, "sftp") + .await + .expect("the alive channel carries another request"); + match next_message(&mut channel).await { + Some(ChannelMsg::Failure) => {} + other => panic!("the inline decline leaves the channel usable, got {other:?}"), + } + + session.unmount().await; + } + + /// - *Contract stamp:* mapping.md §Request-level rows (the + /// `shell_request` row's store-and-take handoff: the + /// `channel-not-held` class — a second shell request on an + /// already-served shell channel answers `channel_failure`, + /// inline, the first serve rides on) + the event vocabulary + /// (`src/observe.rs`: class `request`, reason + /// `channel-not-held`; the lifecycle row carries no + /// `exec_route` — absent-field shape pinned on the wire). + #[tokio::test] + async fn second_shell_request_on_a_served_shell_channel_declines_channel_not_held() { + ec::install(); + let shim = Arc::new(td::TtyShim::new()); + shim.set_script(td::ShimScript { + stdout: vec![], + stderr: vec![], + exit: None, + }); + let session = session_with(shim_seam(&shim)).await; + + let mut channel = open_session_channel(&session.client) + .await + .expect("the session channel opens"); + request_shell(&channel) + .await + .expect("the first shell request routed"); + expect_success(&mut channel, "the served shell row").await; + + request_shell(&channel) + .await + .expect("the second shell request rides the wire"); + match next_message(&mut channel).await { + Some(ChannelMsg::Failure) => {} + other => panic!( + "a second shell request on a served channel answers channel_failure, \ + got {other:?}" + ), + } + + let event = ec::await_event(&[ + ("row", "shell_request"), + ("decline_class", "request"), + ("reason", "channel-not-held"), + ]) + .await; + ec::assert_request_decline(&event, false); + + request_subsystem(&channel, "sftp") + .await + .expect("the alive channel carries another request"); + match next_message(&mut channel).await { + Some(ChannelMsg::Failure) => {} + other => panic!("the inline decline leaves the channel usable, got {other:?}"), + } + + session.unmount().await; + } + #[tokio::test] async fn stdin_bytes_and_the_eof_sentinel_reach_the_backend() { let shim = Arc::new(td::TtyShim::new());