diff --git a/.gitattributes b/.gitattributes index 309296bbc..2175e3a17 100644 --- a/.gitattributes +++ b/.gitattributes @@ -19,3 +19,6 @@ crates/workshop/look/**/*.png binary # The event-log schema canary pins the version-1 file exactly as the log # writer emits it (LF); autocrlf must not rewrite it on Windows checkouts. crates/workshop/server/tests/it/observer/*.jsonl text eol=lf +# The Gateway discovery contract's golden fixtures are compared byte for +# byte against what the writer emits (LF); a checkout must not rewrite them. +crates/workshop/gateway-discovery/fixtures/*.json text eol=lf diff --git a/AGENTS.md b/AGENTS.md index 5dc8f37e6..2ee0d6b47 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ Seven terms have exactly one meaning each, everywhere in this repository: code c - Behavior changes ship with tests in the same change. Preserve product and behavior tests during refactors. - A Cargo feature gates a real constraint such as a toolchain requirement or heavy native build. It does not describe product shape. Feature-disabled builds must not leak optional types into core paths. - Runtime and serve paths never compile native dependencies or invoke build tools. -- Long-running gateway work reports through `gateway-progress`, a private gateway family crate: a producer begins an activity with a text, replaces the text as work moves, and drops the guard when done. Consumers outside the family read only the `Progress` wire type from `gateway-api-types`. +- Long-running gateway work reports through `gateway-progress`, a private gateway family crate: a producer begins an activity with a text, replaces the text as work moves, and drops the guard when done. Consumers outside the family read only the same `{ busy, text }` wire shape, which each side declares for itself: the Gateway sends it, and a Host that shows it pins the field names with a golden JSON test. - Unsafe code stays in its explicitly owned boundary. - Comments explain a non-obvious constraint, ordering requirement, or workaround. Every platform or external-bug workaround cites its upstream issue URL in the explanatory comment. - JSON that reaches a recorder or a replay comparison round-trips exactly - `to_value`, `to_string`, `from_str` yield an identical value, object keys stay canonical (sorted), numbers must be finite. Exact parsing (`float_roundtrip`) carries that guarantee; a value derived and then logged is additionally rounded to its meaningful precision at the source. diff --git a/Cargo.lock b/Cargo.lock index 4db2beda0..d9b32d1d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -699,6 +699,7 @@ version = "0.0.0" dependencies = [ "anyhow", "build-ceiling", + "gateway-api-discovery", "proc-macro2", "pulldown-cmark", "rustdoc-types", @@ -706,6 +707,7 @@ dependencies = [ "syn 3.0.3", "tempfile", "toml 1.1.4+spec-1.1.0", + "workshop-gateway-discovery", ] [[package]] @@ -8578,7 +8580,6 @@ version = "0.3.0" dependencies = [ "anyhow", "build-ceiling", - "gateway-api-discovery", "reqwest", "serde", "serde_json", @@ -8623,8 +8624,6 @@ dependencies = [ "axum", "build-ceiling", "futures-util", - "gateway-api-discovery", - "gateway-api-types", "reqwest", "serde", "serde_json", @@ -8634,11 +8633,27 @@ dependencies = [ "tokio-tungstenite", "tracing", "url", + "workshop-gateway-discovery", "workshop-protocol", "workshop-registry", "workshop-support", ] +[[package]] +name = "workshop-gateway-discovery" +version = "0.3.0" +dependencies = [ + "build-ceiling", + "libc", + "serde", + "serde_json", + "shared-error-source", + "tempfile", + "thiserror 2.0.19", + "windows-sys 0.61.2", + "workshop-gateway-discovery", +] + [[package]] name = "workshop-menu" version = "0.0.0" @@ -8699,7 +8714,6 @@ dependencies = [ "build-ceiling", "build-ui", "futures-util", - "gateway-api-discovery", "harness", "harness-gateway-client", "plugin-mcp", @@ -8712,7 +8726,6 @@ dependencies = [ "rust-embed", "serde", "serde_json", - "shared-loopback", "socket2", "tempfile", "thiserror 2.0.19", diff --git a/Cargo.toml b/Cargo.toml index 038868896..f9f13b214 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [workspace] resolver = "3" -members = ["crates/*", "crates/runvil-internal/types", "crates/runvil-internal/engine", "crates/runvil-internal/lua", "crates/runvil-internal/parser", "crates/runvil-internal/vfs", "crates/runvil-internal/model-client", "crates/gateway/app", "crates/gateway/cloud-providers", "crates/gateway/config", "crates/gateway/config-ui", "crates/gateway/local", "crates/gateway/logging", "crates/gateway/progress", "crates/gateway/protocol", "crates/gateway/routing", "crates/gateway/web-search", "crates/gateway/stt/api", "crates/gateway/stt/engine", "crates/gateway/stt/backend-whisper", "crates/gateway/stt/whisper-ffi", "crates/workshop/desktop", "crates/workshop/server", "crates/workshop/server-api", "crates/workshop/gateway", "crates/workshop/menu", "crates/workshop/protocol", "crates/workshop/registry", "crates/workshop/status", "crates/workshop/support", "crates/workshop/user-state", "crates/workshop/workspace", "crates/workshop/run-log", "crates/workshop/agents", "crates/harness-internal/runner", "crates/harness-internal/plugin-loader", "crates/harness-internal/test-plugin"] +members = ["crates/*", "crates/runvil-internal/types", "crates/runvil-internal/engine", "crates/runvil-internal/lua", "crates/runvil-internal/parser", "crates/runvil-internal/vfs", "crates/runvil-internal/model-client", "crates/gateway/app", "crates/gateway/cloud-providers", "crates/gateway/config", "crates/gateway/config-ui", "crates/gateway/local", "crates/gateway/logging", "crates/gateway/progress", "crates/gateway/protocol", "crates/gateway/routing", "crates/gateway/web-search", "crates/gateway/stt/api", "crates/gateway/stt/engine", "crates/gateway/stt/backend-whisper", "crates/gateway/stt/whisper-ffi", "crates/workshop/desktop", "crates/workshop/server", "crates/workshop/server-api", "crates/workshop/gateway", "crates/workshop/gateway-discovery", "crates/workshop/menu", "crates/workshop/protocol", "crates/workshop/registry", "crates/workshop/status", "crates/workshop/support", "crates/workshop/user-state", "crates/workshop/workspace", "crates/workshop/run-log", "crates/workshop/agents", "crates/harness-internal/runner", "crates/harness-internal/plugin-loader", "crates/harness-internal/test-plugin"] # crates/shared-ui is not a Rust crate: it is the TypeScript+CSS package # the Gateway config UI consumes (the Workshop uses its fork in # crates/workshop/look), so the crates/* glob skips it. @@ -44,6 +44,7 @@ shared-error-source = { path = "crates/shared-error-source" } shared-loopback = { path = "crates/shared-loopback" } gateway-protocol = { path = "crates/gateway/protocol" } gateway-api-discovery = { path = "crates/gateway-api-discovery" } +workshop-gateway-discovery = { path = "crates/workshop/gateway-discovery" } harness = { path = "crates/harness" } harness-gateway-client = { path = "crates/harness-gateway-client" } harness-runner = { path = "crates/harness-internal/runner" } diff --git a/crates/README.md b/crates/README.md index 5dc77cfed..e67690392 100644 --- a/crates/README.md +++ b/crates/README.md @@ -1,3 +1,3 @@ # Crates -This directory holds the workspace's crates. The crates at this level are the public and shared layer: the Engine's public API, the Harness and the pieces a Host plugs into it, the vocabulary and discovery code the gateway shares with its clients, shared middleware and error wrappers, and the build tooling. Each product family keeps its private crates in a container directory of its own, and code in one family reaches another family only through a crate at this level. +This directory holds the workspace's crates. The crates at this level are the public and shared layer: the Engine's public API, the Harness and the pieces a Host plugs into it, the vocabulary and protocol code each product family publishes, shared middleware and error wrappers, and the build tooling. Each product family keeps its private crates in a container directory of its own, and code in one family reaches another family only through a crate at this level. diff --git a/crates/build-xtask/Cargo.toml b/crates/build-xtask/Cargo.toml index 907190f34..d86fd22d6 100644 --- a/crates/build-xtask/Cargo.toml +++ b/crates/build-xtask/Cargo.toml @@ -17,6 +17,13 @@ toml.workspace = true [dev-dependencies] tempfile.workspace = true +# The cross-half discovery conformance check (tests/discovery_conformance.rs): +# it runs the Gateway's real writer and Workshop's forked reader against each +# other while both halves still live in this workspace. This is the one +# place allowed to name both, and it disappears when the Gateway moves to +# its own repository. +gateway-api-discovery = { workspace = true, features = ["test-fixtures"] } +workshop-gateway-discovery = { workspace = true, features = ["test-fixtures"] } [build-dependencies] build-ceiling.workspace = true diff --git a/crates/build-xtask/src/product.rs b/crates/build-xtask/src/product.rs index b136e52c2..deb4ed1b5 100644 --- a/crates/build-xtask/src/product.rs +++ b/crates/build-xtask/src/product.rs @@ -9,9 +9,7 @@ //! workshop, or Harness crates. //! - `gateway`/`gateway-*` crates must not depend on runvil, //! workshop, or Harness crates. -//! - `workshop`/`workshop-*` crates must not depend on gateway crates, -//! except the family's public pair (`gateway-api-types`, -//! `gateway-api-discovery`). +//! - `workshop`/`workshop-*` crates must not depend on gateway crates. //! - `harness`/`harness-*` crates must not depend on gateway, shared, or //! workshop crates; outside their own family they may name only //! `runvil` and `runvil-plugin`, which rules out `build-*` @@ -141,9 +139,6 @@ const SERVER: &str = "workshop-server"; /// The runvil-family crates outside crates may depend on directly: /// the Engine facade and the Plugin contract. const PUBLIC_RUNVIL: [&str; 2] = ["runvil", "runvil-plugin"]; -/// The gateway family's public pair: the only gateway crates workshop -/// crates may name. -const PUBLIC_GATEWAY: [&str; 2] = ["gateway-api-types", "gateway-api-discovery"]; /// The Harness family's facade: the one outside crate permitted into /// `crates/harness-internal/`. const HARNESS_FACADE: &str = "harness"; @@ -204,7 +199,6 @@ fn boundary_breach(package: &CrateInfo, dep: &CrateInfo) -> Option { return Some(reason.to_owned()); } let (from, to) = (family(&package.package), family(&dep.package)); - let public_gateway = PUBLIC_GATEWAY.contains(&dep.package.as_str()); let family_rule = match (from, to) { (Family::Runvil, Family::Gateway | Family::Workshop | Family::Harness) => { Some("runvil crates must not depend on gateway, workshop, or harness crates") @@ -212,7 +206,7 @@ fn boundary_breach(package: &CrateInfo, dep: &CrateInfo) -> Option { (Family::Gateway, Family::Runvil | Family::Workshop | Family::Harness) => { Some("gateway crates must not depend on runvil, workshop, or harness crates") } - (Family::Workshop, Family::Gateway) if !public_gateway => { + (Family::Workshop, Family::Gateway) => { Some("workshop crates must not depend on gateway crates") } (Family::Harness, Family::Workshop) => { diff --git a/crates/build-xtask/src/product/harness_tests.rs b/crates/build-xtask/src/product/harness_tests.rs index f36a75c5b..6a8777ee7 100644 --- a/crates/build-xtask/src/product/harness_tests.rs +++ b/crates/build-xtask/src/product/harness_tests.rs @@ -6,7 +6,9 @@ use super::product_boundary_violations; use super::test_support::write_crate; #[test] -fn a_harness_crate_depending_on_the_gateway_public_types_is_reported() { +fn a_harness_crate_depending_on_a_gateway_crate_is_reported() { + // No gateway crate is reachable from the Harness family, the former + // `gateway-api-types` exception included. let root = tempfile::TempDir::new().expect("tempdir"); write_crate( root.path(), @@ -20,7 +22,7 @@ fn a_harness_crate_depending_on_the_gateway_public_types_is_reported() { assert!( violations[0].starts_with("harness-sessions depends on gateway-api-types:") && violations[0].contains("harness crates must not depend on gateway crates"), - "the gateway public pair is closed to harness crates: {violations:?}" + "every gateway crate is closed to harness crates: {violations:?}" ); } diff --git a/crates/build-xtask/src/product/tests.rs b/crates/build-xtask/src/product/tests.rs index f207946fa..8761828f9 100644 --- a/crates/build-xtask/src/product/tests.rs +++ b/crates/build-xtask/src/product/tests.rs @@ -311,27 +311,31 @@ fn an_unparseable_manifest_is_reported() { } #[test] -fn a_workshop_crate_depending_on_the_public_gateway_pair_passes() { - let root = tempfile::TempDir::new().expect("tempdir"); - write_crate( - root.path(), - "workshop-server", - "workshop-server", - "[dependencies]\ngateway-api-types = { path = \"../gateway-api-types\" }\n\ - gateway-api-discovery = { path = \"../gateway-api-discovery\" }\n", - ); - write_crate(root.path(), "gateway-api-types", "gateway-api-types", ""); - write_crate( - root.path(), +fn a_workshop_crate_depending_on_a_gateway_crate_is_reported() { + // The Workshop family has no gateway dependency at all: Workshop reaches + // a running Gateway through its own discovery and HTTP clients, never + // through a Gateway crate. The two former exceptions are covered. + for gateway in [ + "gateway-api-types", "gateway-api-discovery", - "gateway-api-discovery", - "", - ); - let violations = product_boundary_violations(root.path()); - assert!( - violations.is_empty(), - "the public gateway pair is legal for workshop crates: {violations:?}" - ); + "gateway-routing", + ] { + let root = tempfile::TempDir::new().expect("tempdir"); + write_crate( + root.path(), + "workshop-server", + "workshop-server", + &format!("[dependencies]\n{gateway} = {{ path = \"../{gateway}\" }}\n"), + ); + write_crate(root.path(), gateway, gateway, ""); + let violations = product_boundary_violations(root.path()); + assert_eq!(violations.len(), 1, "{gateway}: {violations:?}"); + assert!( + violations[0].starts_with(&format!("workshop-server depends on {gateway}:")) + && violations[0].contains("workshop crates must not depend on gateway crates"), + "{gateway} is refused for a workshop crate: {violations:?}" + ); + } } #[test] diff --git a/crates/build-xtask/src/tidy.rs b/crates/build-xtask/src/tidy.rs index 4358eda1a..819951e33 100644 --- a/crates/build-xtask/src/tidy.rs +++ b/crates/build-xtask/src/tidy.rs @@ -30,6 +30,7 @@ type Tiered = (&'static str, &'static str); /// Tier 0: vocabulary crates. No internal `workshop-*` dependencies. const VOCABULARY: &[Tiered] = &[ + ("workshop-gateway-discovery", "gateway-discovery"), ("workshop-protocol", "protocol"), ("workshop-registry", "registry"), ("workshop-support", "support"), diff --git a/crates/build-xtask/src/unsafe_allowlist-tests.rs b/crates/build-xtask/src/unsafe_allowlist-tests.rs index 92bfff770..a424b54a4 100644 --- a/crates/build-xtask/src/unsafe_allowlist-tests.rs +++ b/crates/build-xtask/src/unsafe_allowlist-tests.rs @@ -1,5 +1,5 @@ //! The unsafe allowlist over this workspace, plus fixtures that each way of -//! relaxing `unsafe_code` outside the six owned crates fails at its line, +//! relaxing `unsafe_code` outside the owned crates fails at its line, //! text that only spells the attribute passes, and an empty scan fails. use std::path::Path; @@ -39,13 +39,14 @@ fn the_workspace_relaxes_unsafe_code_only_in_its_owned_crates() { } #[test] -fn the_six_owned_crates_may_relax_unsafe_code_and_text_that_spells_it_passes() { +fn the_owned_crates_may_relax_unsafe_code_and_text_that_spells_it_passes() { let root = tempfile::TempDir::new().expect("tempdir"); for dir in [ "gateway/app", "gateway/stt/whisper-ffi", "gateway-api-discovery", "workshop/desktop", + "workshop/gateway-discovery", "harness-internal/plugin-loader", "harness-internal/test-plugin", ] { diff --git a/crates/build-xtask/src/unsafe_allowlist.rs b/crates/build-xtask/src/unsafe_allowlist.rs index 4df424faa..5bb0c7ec4 100644 --- a/crates/build-xtask/src/unsafe_allowlist.rs +++ b/crates/build-xtask/src/unsafe_allowlist.rs @@ -1,5 +1,5 @@ //! The unsafe allowlist: the workspace lint table denies `unsafe_code`, -//! and only the six crates that own an unsafe boundary may relax it. Any +//! and only the crates that own an unsafe boundary may relax it. Any //! other `.rs` file that expects or allows `unsafe_code`, directly or //! through `cfg_attr` at any depth, is a violation. Runs as part of //! `cargo test -p build-xtask` and `cargo xtask tidy`. @@ -18,11 +18,12 @@ use syn::{Meta, Token}; /// The crate directories, relative to the workspace root, that own an /// unsafe boundary and may relax `unsafe_code` in the module that holds it. -const OWNED: [&str; 6] = [ +const OWNED: [&str; 7] = [ "crates/gateway/app", "crates/gateway/stt/whisper-ffi", "crates/gateway-api-discovery", "crates/workshop/desktop", + "crates/workshop/gateway-discovery", "crates/harness-internal/plugin-loader", "crates/harness-internal/test-plugin", ]; diff --git a/crates/build-xtask/tests/discovery_conformance.rs b/crates/build-xtask/tests/discovery_conformance.rs new file mode 100644 index 000000000..41367ce14 --- /dev/null +++ b/crates/build-xtask/tests/discovery_conformance.rs @@ -0,0 +1,357 @@ +//! Conformance between the two halves of the Gateway discovery contract. +//! +//! While both halves still live in this workspace, run the Gateway's real +//! writer and Workshop's forked reader against each other. This catches a +//! disagreement in the field set, the serialization, the unknown-field +//! tolerance, or the validation rule - the drift the fork's own unit tests +//! cannot see, because each half only checks its own idea of the contract. +//! +//! This is the strongest check available and it disappears when the Gateway +//! moves to its own repository, at which point `CONTRACT.md` and the golden +//! fixtures are what is left. `build-xtask` is the one crate the product +//! rules let depend into both families, which is why the check lives here +//! rather than in either half. + +#![expect( + clippy::expect_used, + reason = "test helpers panic on setup failure, which is the desired behavior" +)] + +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use workshop_gateway_discovery::{ + GatewayDiscoveryFile, Resolution, StaleReason, launch_or_attach, resolve, +}; + +/// The environment variable naming the run directory the child serves in. +const CHILD_RUN_DIR_ENV: &str = "RUNVIL_DISCOVERY_CONFORMANCE_RUN_DIR"; + +/// The child's shutdown marker file, written when it observes the end of +/// its serving loop. +const CHILD_READY: &str = "gateway-ready"; + +/// The image name the reader accepts. +#[cfg(windows)] +const GATEWAY_IMAGE_NAME: &str = "runvil-gateway.exe"; +/// The image name the reader accepts. +#[cfg(not(windows))] +const GATEWAY_IMAGE_NAME: &str = "runvil-gateway"; + +/// The writer's file, with the pid and port the fixture child supplies. +fn writer_file(pid: u32, port: u16) -> gateway_api_discovery::GatewayDiscoveryFile { + gateway_api_discovery::GatewayDiscoveryFile { + port, + api_key: "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b".to_owned(), + pid, + epoch: 1_757_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-09-03T12:00:00Z".to_owned(), + } +} + +/// The reader's view of the same file. +fn reader_file(pid: u32, port: u16) -> GatewayDiscoveryFile { + GatewayDiscoveryFile { + port, + api_key: "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b".to_owned(), + pid, + epoch: 1_757_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-09-03T12:00:00Z".to_owned(), + } +} + +/// A named fixture child: a copy of this test binary running under the +/// Gateway's image name, writing the Gateway writer's file and answering +/// the two probes. +struct WriterChild { + child: std::process::Child, + port: u16, + _directory: tempfile::TempDir, +} + +impl WriterChild { + /// Starts the child in `run_dir` and waits for its readiness marker. + fn spawn(run_dir: &Path) -> Self { + let directory = tempfile::TempDir::new().expect("create the image directory"); + let executable = directory.path().join(GATEWAY_IMAGE_NAME); + std::fs::copy( + std::env::current_exe().expect("locate the test executable"), + &executable, + ) + .expect("copy the test executable under the Gateway image name"); + let mut child = std::process::Command::new(&executable) + .args([ + "--exact", + "the_writer_child_process", + "--ignored", + "--nocapture", + ]) + .env(CHILD_RUN_DIR_ENV, run_dir) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("start the named child"); + + let ready = run_dir.join(CHILD_READY); + let deadline = Instant::now() + Duration::from_secs(20); + while !ready.is_file() { + assert!( + child.try_wait().expect("observe the child").is_none(), + "the writer child exited before publishing a file" + ); + assert!( + Instant::now() < deadline, + "the writer child did not publish a file within the deadline" + ); + std::thread::sleep(Duration::from_millis(10)); + } + let published = gateway_api_discovery::GatewayDiscoveryFile::read(run_dir) + .expect("read the child's file") + .expect("the child published a file"); + Self { + child, + port: published.port, + _directory: directory, + } + } + + fn pid(&self) -> u32 { + self.child.id() + } + + /// Stops the child and waits for it to exit. + fn stop(&mut self) { + let _ = self.child.kill(); + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if self + .child + .try_wait() + .expect("observe the writer child") + .is_some() + { + return; + } + assert!(Instant::now() < deadline, "the writer child did not stop"); + std::thread::sleep(Duration::from_millis(10)); + } + } +} + +impl Drop for WriterChild { + fn drop(&mut self) { + if self.child.try_wait().ok().flatten().is_none() { + let _ = self.child.kill(); + let _ = self.child.wait(); + } + } +} + +/// The reader, run against a real Gateway-written file from a real process +/// under the Gateway image name. This is the whole gauntlet: parse, process +/// identity, boot identity, one-connection health and bearer proof, and the +/// second process observation. +#[test] +fn the_workshop_reader_attaches_to_a_gateway_written_file_end_to_end() { + let run = tempfile::TempDir::new().expect("tempdir"); + let child = WriterChild::spawn(run.path()); + + let resolution = resolve(run.path()).expect("resolve"); + + assert_eq!( + resolution, + Resolution::Attach(reader_file(child.pid(), child.port)), + "the reader attaches to the process the Gateway writer described" + ); + assert!( + run.path().join("gateway.json").exists(), + "a live file is left in place" + ); +} + +#[test] +fn the_workshop_reader_rejects_a_gateway_written_file_whose_key_is_wrong() { + // The child accepts one key; the file carries another. Everything but + // the bearer proof passes, so this pins that step on real bytes. + let run = tempfile::TempDir::new().expect("tempdir"); + let child = WriterChild::spawn(run.path()); + gateway_api_discovery::GatewayDiscoveryFile { + api_key: "a-key-the-child-never-accepts".to_owned(), + ..writer_file(child.pid(), child.port) + } + .write_to(run.path()) + .expect("rewrite the file with the wrong key"); + + let resolution = resolve(run.path()).expect("resolve"); + + assert_eq!( + resolution, + Resolution::Stale(StaleReason::KeyRejected), + "a Gateway-written file with a stale key is stale on authority" + ); + assert!( + !run.path().join("gateway.json").exists(), + "the reader cleaned the stale file" + ); +} + +#[test] +fn the_workshop_reader_rejects_a_gateway_written_file_after_the_process_dies() { + let run = tempfile::TempDir::new().expect("tempdir"); + let mut child = WriterChild::spawn(run.path()); + child.stop(); + + let resolution = resolve(run.path()).expect("resolve"); + + assert_eq!( + resolution, + Resolution::Stale(StaleReason::ProcessDead), + "a dead pid is stale on process" + ); +} + +#[test] +fn the_workshop_launch_election_attaches_to_a_gateway_written_file() { + // The launch race's holder re-validates before launching; that is the + // path a Workshop boot takes when a Gateway is already running. + let run = tempfile::TempDir::new().expect("tempdir"); + let child = WriterChild::spawn(run.path()); + + let decision = launch_or_attach(run.path(), Duration::from_secs(5)).expect("the race settles"); + match decision { + workshop_gateway_discovery::LaunchDecision::Attach(file) => { + assert_eq!(file, reader_file(child.pid(), child.port)); + } + _ => panic!("a live Gateway-written file must attach, not launch: {decision:?}"), + } +} + +#[test] +fn both_halves_serialize_the_same_bytes() { + // A field rename on either side, or a change to one side's serde + // attributes, changes the bytes and fails here. + let gateway_dir = tempfile::TempDir::new().expect("tempdir"); + writer_file(4242, 8081) + .write_to(gateway_dir.path()) + .expect("the Gateway writer writes its file"); + let from_gateway = std::fs::read_to_string(gateway_dir.path().join("gateway.json")) + .expect("read the Gateway's bytes"); + + let workshop_dir = tempfile::TempDir::new().expect("tempdir"); + reader_file(4242, 8081) + .write_to(workshop_dir.path()) + .expect("the Workshop test double writes its file"); + let from_workshop = std::fs::read_to_string(workshop_dir.path().join("gateway.json")) + .expect("read the Workshop bytes"); + + assert_eq!( + from_gateway, from_workshop, + "both halves of the contract produce the same file" + ); +} + +#[test] +fn both_halves_agree_on_which_files_are_invalid() { + // The validation rule is one rule, not two: a file one half refuses for + // its fields must be refused by the other for the same reason. + let dir = tempfile::TempDir::new().expect("tempdir"); + std::fs::write( + dir.path().join("gateway.json"), + br#"{"port":0,"api_key":"k","pid":1,"epoch":1,"version":"0","started_at":"t"}"#, + ) + .expect("write the zero-port file"); + + let gateway = gateway_api_discovery::GatewayDiscoveryFile::read(dir.path()) + .expect_err("the Gateway writer's reader refuses a zero port"); + let workshop = + GatewayDiscoveryFile::read(dir.path()).expect_err("the Workshop reader refuses it"); + + let gateway_reason = match gateway { + gateway_api_discovery::SidecarError::Invalid { reason, .. } => reason, + other => panic!("the Gateway names an invalid file, got {other}"), + }; + let workshop_reason = match workshop { + workshop_gateway_discovery::SidecarError::Invalid { reason, .. } => reason, + other => panic!("the Workshop reader names an invalid file, got {other}"), + }; + assert_eq!( + gateway_reason, workshop_reason, + "both halves name the same broken invariant" + ); +} + +#[test] +fn the_reader_tolerates_a_gateway_rewrite() { + // The Gateway rewrites gateway.json as it moves through boot. A reader + // that raced the rewrite must never see a truncated file: the atomic + // write is what guarantees that, and this reads the result. + let dir = tempfile::TempDir::new().expect("tempdir"); + for epoch in [1_757_000_000_u64, 1_757_000_001, 1_757_000_002] { + gateway_api_discovery::GatewayDiscoveryFile { + epoch, + ..writer_file(4242, 8081) + } + .write_to(dir.path()) + .expect("the Gateway rewrites its file"); + let read = GatewayDiscoveryFile::read(dir.path()) + .expect("the reader accepts a rewritten file") + .expect("the file exists"); + assert_eq!(read.epoch, epoch, "the newest write is what reads back"); + } +} + +/// The child half: writes the Gateway writer's file and answers `GET +/// /health` and the bearer probe on `GET /v1/models` until it is killed. +#[test] +#[ignore = "runs only as a named child process"] +fn the_writer_child_process() { + use std::io::{Read as _, Write as _}; + use std::net::TcpListener; + + let Some(run_dir) = std::env::var_os(CHILD_RUN_DIR_ENV) else { + return; + }; + let run_dir = PathBuf::from(run_dir); + let listener = TcpListener::bind("127.0.0.1:0").expect("bind the fixture gateway"); + let port = listener.local_addr().expect("fixture address").port(); + let file = writer_file(std::process::id(), port); + file.write_to(&run_dir) + .expect("the Gateway writer publishes its file"); + std::fs::write(run_dir.join(CHILD_READY), b"ready").expect("announce readiness"); + + for stream in listener.incoming() { + let Ok(mut stream) = stream else { + continue; + }; + let expected_key = file.api_key.clone(); + std::thread::spawn(move || { + // The validation proof sends both requests on one connection, + // so the fixture keeps answering on this one until the peer + // closes. + let mut buffer = [0_u8; 4096]; + loop { + let Ok(read) = stream.read(&mut buffer) else { + return; + }; + if read == 0 { + return; + } + let request = String::from_utf8_lossy(&buffer[..read]).into_owned(); + let health = request.starts_with("GET /health "); + let accepted = health + || request.contains(&format!("Authorization: Bearer {expected_key}\r\n")); + let response = if accepted { + "HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}" + } else { + "HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\n\r\n" + }; + if stream.write_all(response.as_bytes()).is_err() { + return; + } + } + }); + } +} diff --git a/crates/gateway/app/tests/it/realtime_stt/capture.rs b/crates/gateway/app/tests/it/realtime_stt/capture.rs index 62d0ca0eb..5e436adf7 100644 --- a/crates/gateway/app/tests/it/realtime_stt/capture.rs +++ b/crates/gateway/app/tests/it/realtime_stt/capture.rs @@ -3,8 +3,8 @@ //! `/v1/realtime` route at real-time pace and records every server event it //! receives, for measuring what a dictation shows and when. //! -//! The session negotiates the hypothesis and ranges includes the way -//! Workshop does, and the gateway loads the packaged interim and final +//! The session negotiates the hypothesis include the way Workshop does, +//! and the gateway loads the packaged interim and final //! `[[stt_model]]` pair at its default `[stt]` window and interval. The clip //! is appended in 100 ms chunks on a wall-clock schedule, and the commit //! follows the last chunk. @@ -30,7 +30,6 @@ use super::{Socket, audio_samples, connect, server}; const CAPTURE_VARIABLE: &str = "RUNVIL_REALTIME_CAPTURE"; const HYPOTHESIS_INCLUDE: &str = "item.input_audio_transcription.hypothesis"; -const RANGES_INCLUDE: &str = "item.input_audio_transcription.hypothesis.ranges"; const HYPOTHESIS: &str = "conversation.item.input_audio_transcription.hypothesis"; const COMPLETED: &str = "conversation.item.input_audio_transcription.completed"; const FAILED: &str = "conversation.item.input_audio_transcription.failed"; @@ -132,7 +131,7 @@ async fn send(sink: &mut Sink, event: &Value) { .expect("client event sends"); } -/// Requests hypotheses with ranges in the session update Workshop sends. +/// Requests hypotheses in the session update Workshop sends. pub(super) async fn negotiate(sink: &mut Sink, recorder: &mut Recorder) { recorder.expect("session.created").await; send( @@ -148,7 +147,7 @@ pub(super) async fn negotiate(sink: &mut Sink, recorder: &mut Recorder) { "transcription": {"model": "realtime-transcribe", "prompt": ""}, "turn_detection": null }}, - "include": [HYPOTHESIS_INCLUDE, RANGES_INCLUDE] + "include": [HYPOTHESIS_INCLUDE] } }), ) @@ -156,8 +155,8 @@ pub(super) async fn negotiate(sink: &mut Sink, recorder: &mut Recorder) { let updated = recorder.expect("session.updated").await; assert_eq!( updated["session"]["include"], - json!([HYPOTHESIS_INCLUDE, RANGES_INCLUDE]), - "the route negotiates hypotheses with ranges" + json!([HYPOTHESIS_INCLUDE]), + "the route negotiates hypotheses" ); } diff --git a/crates/gateway/stt/api/src/realtime/input.rs b/crates/gateway/stt/api/src/realtime/input.rs index 48c2b4982..a4a318975 100644 --- a/crates/gateway/stt/api/src/realtime/input.rs +++ b/crates/gateway/stt/api/src/realtime/input.rs @@ -56,10 +56,6 @@ impl InputSnapshot { pub(crate) const fn include_hypothesis(&self) -> bool { !matches!(self.include, HypothesisInclude::Off) } - - pub(crate) const fn include_ranges(&self) -> bool { - matches!(self.include, HypothesisInclude::Ranges) - } } /// The terms every decode of a take is guided by: the generation's diff --git a/crates/gateway/stt/api/src/realtime/session/route-tests.rs b/crates/gateway/stt/api/src/realtime/session/route-tests.rs index 7e3bafefb..3c4fba5ae 100644 --- a/crates/gateway/stt/api/src/realtime/session/route-tests.rs +++ b/crates/gateway/stt/api/src/realtime/session/route-tests.rs @@ -18,7 +18,6 @@ use crate::realtime::session::InterimEpoch; mod interim; const HYPOTHESIS: &str = "item.input_audio_transcription.hypothesis"; -const RANGES: &str = "item.input_audio_transcription.hypothesis.ranges"; #[test] fn sample_milliseconds_are_exact_across_the_prior_multiplication_overflow() { @@ -159,29 +158,6 @@ fn a_new_input_after_clear_or_commit_starts_again_at_revision_one() { ); } -#[test] -fn range_fields_appear_only_for_a_ranges_input_and_report_the_finalized_text() { - let mut base = hypothesis_session(); - let epoch = base.begin_interim().expect("epoch begins"); - let event = accept_event(&mut base, epoch, 0..16_000, "alpha beta") - .expect("base hypothesis is emitted"); - assert!(event.get("finalized_through_ms").is_none()); - assert!(event.get("finalized_seq").is_none()); - - let mut ranges = session_including(&[HYPOTHESIS, RANGES]); - ranges - .input() - .expect("input exists") - .take() - .record_finalized_through("ask not", Some(16_000)); - let epoch = ranges.begin_interim().expect("epoch begins"); - let event = accept_event(&mut ranges, epoch, 16_000..32_000, "what you") - .expect("ranges hypothesis is emitted"); - assert_eq!(event["finalized"], "ask not"); - assert_eq!(event["finalized_through_ms"], 1_000); - assert_eq!(event["finalized_seq"], 1); -} - fn accept_delta( session: &mut Session, epoch: InterimEpoch, @@ -413,27 +389,24 @@ fn a_landed_final_streams_its_appended_words_to_a_plain_client() { } #[test] -fn only_a_ranges_session_advances_the_revision_for_a_range_only_change() { - for (include, expected) in [(&[HYPOTHESIS][..], 2), (&[HYPOTHESIS, RANGES][..], 3)] { - let mut session = session_including(include); - let epoch = session.begin_interim().expect("epoch begins"); - assert_eq!( - accept(&mut session, epoch, 0..16_000, "alpha beta"), - Some(1) - ); - assert_eq!( - accept(&mut session, epoch, 0..24_000, "alpha beta"), - Some(2) - ); - session - .input() - .expect("input exists") - .take() - .record_finalized_through("", None); - assert_eq!( - accept(&mut session, epoch, 0..32_000, "alpha beta"), - Some(expected), - "{include:?}" - ); - } +fn an_unchanged_hypothesis_keeps_its_revision_for_a_new_window() { + let mut session = hypothesis_session(); + let epoch = session.begin_interim().expect("epoch begins"); + assert_eq!( + accept(&mut session, epoch, 0..16_000, "alpha beta"), + Some(1) + ); + assert_eq!( + accept(&mut session, epoch, 0..24_000, "alpha beta"), + Some(2) + ); + session + .input() + .expect("input exists") + .take() + .record_finalized_through("", None); + assert_eq!( + accept(&mut session, epoch, 0..32_000, "alpha beta"), + Some(2) + ); } diff --git a/crates/gateway/stt/api/src/realtime/session/route.rs b/crates/gateway/stt/api/src/realtime/session/route.rs index 4eac7ce8f..b13d6761b 100644 --- a/crates/gateway/stt/api/src/realtime/session/route.rs +++ b/crates/gateway/stt/api/src/realtime/session/route.rs @@ -5,7 +5,7 @@ use crate::audio::AudioError; use crate::realtime::input::UncommittedInput; use crate::realtime::result_mailbox::ItemResult; use crate::realtime::session::state::InterimTaskOutput; -use crate::realtime::wire::{HypothesisRanges, ServerEvent}; +use crate::realtime::wire::ServerEvent; use crate::take::{FinalizedRange, InterimSnapshot, token_spans}; use gateway_stt_engine::{DecodeMode, DecodeRequest, EnginePolicy, TranscribeError}; @@ -210,31 +210,22 @@ impl Session { self.hypothesis_window_end = end; } let input = self.input.as_ref().ok_or(SessionError::NoInput)?; - let include_ranges = input.snapshot().include_ranges(); if !input.snapshot().include_hypothesis() { return Ok(self.standard_delta(item_id, snapshot)); } - let shown = ( - snapshot, - include_ranges.then(|| HypothesisRanges { - finalized_through_ms: sample_millis(finalized.through_samples), - finalized_seq: finalized.seq, - }), - ); let changed = self .last_hypothesis .as_ref() - .map_or(!shown.0.is_empty(), |last| last != &shown); + .map_or(!snapshot.is_empty(), |last| last != &snapshot); if changed { self.hypothesis_revision = self .hypothesis_revision .checked_add(1) .ok_or(SessionError::EpochExhausted)?; - self.last_hypothesis = Some(shown.clone()); + self.last_hypothesis = Some(snapshot.clone()); } else if window.is_none() || self.last_hypothesis.is_none() { return Ok(None); } - let (snapshot, ranges) = shown; let end = self.hypothesis_window_end; let (audio_start, audio_end) = window.unwrap_or((end, end)); Ok(Some(ServerEvent::hypothesis( @@ -244,7 +235,6 @@ impl Session { snapshot, sample_millis(audio_start), sample_millis(audio_end), - ranges, ))) } diff --git a/crates/gateway/stt/api/src/realtime/session/state.rs b/crates/gateway/stt/api/src/realtime/session/state.rs index a4994b84c..094b56804 100644 --- a/crates/gateway/stt/api/src/realtime/session/state.rs +++ b/crates/gateway/stt/api/src/realtime/session/state.rs @@ -6,7 +6,7 @@ use crate::realtime::input::UncommittedInput; use crate::realtime::item::{CommittedItem, FinalizationError}; use crate::realtime::registry::SessionRegistration; use crate::realtime::result_mailbox::{MailboxError, ResultMailbox}; -use crate::realtime::wire::{EffectiveSession, HypothesisRanges, IdGenerator}; +use crate::realtime::wire::{EffectiveSession, IdGenerator}; use crate::take::{InterimSnapshot, TakeFailure}; use gateway_stt_engine::{DecodeOutput, TranscribeError}; use std::collections::HashMap; @@ -79,7 +79,7 @@ pub(crate) struct Session { pub(super) standard_interim_sent: String, pub(super) standard_interim_committed: String, pub(super) hypothesis_revision: u64, - pub(super) last_hypothesis: Option<(InterimSnapshot, Option)>, + pub(super) last_hypothesis: Option, /// Final outcomes applied to the take when its latest update was composed. pub(super) shown_finalized_seq: u64, /// End of the latest accepted interim window, where an update for landed diff --git a/crates/gateway/stt/api/src/realtime/session/tests.rs b/crates/gateway/stt/api/src/realtime/session/tests.rs index 67a918134..27e6592f2 100644 --- a/crates/gateway/stt/api/src/realtime/session/tests.rs +++ b/crates/gateway/stt/api/src/realtime/session/tests.rs @@ -36,21 +36,6 @@ fn update(prompt: &str, include: bool) -> String { .to_string() } -fn update_with_ranges(prompt: &str) -> String { - serde_json::json!({ - "type": "session.update", - "session": { - "type": "transcription", - "audio": {"input": {"transcription": {"prompt": prompt}}}, - "include": [ - "item.input_audio_transcription.hypothesis", - "item.input_audio_transcription.hypothesis.ranges" - ] - } - }) - .to_string() -} - fn session() -> Session { let registration = SessionRegistry::default() .register() @@ -124,32 +109,6 @@ fn first_successful_append_freezes_configuration_until_clear() { assert!(!input.snapshot().include_hypothesis()); } -#[test] -fn first_successful_append_freezes_the_ranges_include_until_clear() { - let mut session = session(); - session - .update_text(&update_with_ranges("first")) - .expect("ranges update applies"); - session - .append_base64(&encoded(&[1, 2, 3])) - .expect("first append succeeds"); - - session - .update_text(&update("second", true)) - .expect("base hypothesis update applies"); - let input = session.input().expect("input remains"); - assert!(input.snapshot().include_hypothesis()); - assert!(input.snapshot().include_ranges()); - - session.clear().expect("clear succeeds"); - session - .append_base64(&encoded(&[4, 5, 6])) - .expect("next input appends"); - let input = session.input().expect("replacement input exists"); - assert!(input.snapshot().include_hypothesis()); - assert!(!input.snapshot().include_ranges()); -} - #[test] fn failed_first_append_does_not_capture_a_snapshot() { let mut session = session(); diff --git a/crates/gateway/stt/api/src/realtime/wire.rs b/crates/gateway/stt/api/src/realtime/wire.rs index e46915bba..f15b5342a 100644 --- a/crates/gateway/stt/api/src/realtime/wire.rs +++ b/crates/gateway/stt/api/src/realtime/wire.rs @@ -13,7 +13,7 @@ pub(in crate::realtime) use client::parse_client_event; reason = "no code in the realtime module reads these re-exports" )] pub(in crate::realtime) use server::{ - ConversationItem, DurationUsage, EffectiveSession, HypothesisRanges, ServerEvent, WireError, + ConversationItem, DurationUsage, EffectiveSession, ServerEvent, WireError, }; pub(in crate::realtime) use vocabulary::{ ClientError, ClientEvent, HypothesisInclude, IdGenerator, diff --git a/crates/gateway/stt/api/src/realtime/wire/client.rs b/crates/gateway/stt/api/src/realtime/wire/client.rs index 98118b266..e2d006dd0 100644 --- a/crates/gateway/stt/api/src/realtime/wire/client.rs +++ b/crates/gateway/stt/api/src/realtime/wire/client.rs @@ -4,7 +4,7 @@ use serde_json::{Map, Value}; use super::vocabulary::{ AUDIO_RATE, AUDIO_TYPE, ClientError, ClientEvent, Correlation, HYPOTHESIS_INCLUDE, - HYPOTHESIS_RANGES_INCLUDE, HypothesisInclude, MODEL, SESSION_TYPE, SessionPatch, + HypothesisInclude, MODEL, SESSION_TYPE, SessionPatch, }; pub(in crate::realtime) fn parse_client_event(text: &str) -> Result { @@ -350,16 +350,9 @@ fn parse_include( let message = match tokens.as_slice() { [] => return Ok(HypothesisInclude::Off), [Some(HYPOTHESIS_INCLUDE)] => return Ok(HypothesisInclude::Snapshots), - [Some(HYPOTHESIS_INCLUDE), Some(HYPOTHESIS_RANGES_INCLUDE)] - | [Some(HYPOTHESIS_RANGES_INCLUDE), Some(HYPOTHESIS_INCLUDE)] => { - return Ok(HypothesisInclude::Ranges); - } - [Some(HYPOTHESIS_RANGES_INCLUDE)] => { - format!("Include value {HYPOTHESIS_RANGES_INCLUDE} requires {HYPOTHESIS_INCLUDE}") - } _ => match tokens .iter() - .find(|token| !matches!(token, Some(HYPOTHESIS_INCLUDE | HYPOTHESIS_RANGES_INCLUDE))) + .find(|token| !matches!(token, Some(HYPOTHESIS_INCLUDE))) { Some(token) => format!( "Unsupported include value {}", diff --git a/crates/gateway/stt/api/src/realtime/wire/server-events.rs b/crates/gateway/stt/api/src/realtime/wire/server-events.rs index f84dd0cf2..fe51d3db4 100644 --- a/crates/gateway/stt/api/src/realtime/wire/server-events.rs +++ b/crates/gateway/stt/api/src/realtime/wire/server-events.rs @@ -1,8 +1,7 @@ //! Constructors building server events from session and item outcomes. use super::{ - ConversationItem, DurationUsage, EffectiveSession, HypothesisRanges, InputAudioContent, - ServerEvent, WireError, + ConversationItem, DurationUsage, EffectiveSession, InputAudioContent, ServerEvent, WireError, }; use crate::realtime::result_mailbox::{ItemFailure, ItemResult}; use crate::realtime::wire::vocabulary::{OptionalNullable, RequiredNullable}; @@ -33,7 +32,6 @@ impl ServerEvent { snapshot: InterimSnapshot, audio_start_ms: u64, audio_end_ms: u64, - ranges: Option, ) -> Self { let (transcript, finalized, agreed, tentative) = snapshot.into_parts(); Self::TranscriptionHypothesis { @@ -47,8 +45,6 @@ impl ServerEvent { tentative, audio_start_ms, audio_end_ms, - finalized_through_ms: ranges.map(|ranges| ranges.finalized_through_ms), - finalized_seq: ranges.map(|ranges| ranges.finalized_seq), } } diff --git a/crates/gateway/stt/api/src/realtime/wire/server.rs b/crates/gateway/stt/api/src/realtime/wire/server.rs index 66b3e5538..a0558500e 100644 --- a/crates/gateway/stt/api/src/realtime/wire/server.rs +++ b/crates/gateway/stt/api/src/realtime/wire/server.rs @@ -8,9 +8,9 @@ use serde_json::Value; use super::client::parse_client_event; use super::vocabulary::{ - AUDIO_RATE, AUDIO_TYPE, ClientError, ClientEvent, HYPOTHESIS_INCLUDE, - HYPOTHESIS_RANGES_INCLUDE, HypothesisInclude, MODEL, OptionalNullable, RequiredNullable, - SESSION_OBJECT, SESSION_TYPE, deserialize_present, deserialize_required_nullable, + AUDIO_RATE, AUDIO_TYPE, ClientError, ClientEvent, HYPOTHESIS_INCLUDE, HypothesisInclude, MODEL, + OptionalNullable, RequiredNullable, SESSION_OBJECT, SESSION_TYPE, + deserialize_required_nullable, }; #[path = "server-events.rs"] @@ -95,10 +95,6 @@ impl EffectiveSession { candidate.include = match include { HypothesisInclude::Off => Vec::new(), HypothesisInclude::Snapshots => vec![HYPOTHESIS_INCLUDE.to_owned()], - HypothesisInclude::Ranges => vec![ - HYPOTHESIS_INCLUDE.to_owned(), - HYPOTHESIS_RANGES_INCLUDE.to_owned(), - ], }; } *self = candidate; @@ -113,12 +109,6 @@ impl EffectiveSession { pub(in crate::realtime) fn hypothesis_include(&self) -> HypothesisInclude { if self.include.is_empty() { HypothesisInclude::Off - } else if self - .include - .iter() - .any(|value| value == HYPOTHESIS_RANGES_INCLUDE) - { - HypothesisInclude::Ranges } else { HypothesisInclude::Snapshots } @@ -140,7 +130,7 @@ impl EffectiveSession { .map(String::as_str) .collect::>() .as_slice(), - [] | [HYPOTHESIS_INCLUDE] | [HYPOTHESIS_INCLUDE, HYPOTHESIS_RANGES_INCLUDE] + [] | [HYPOTHESIS_INCLUDE] ) { return Err(anyhow!("invalid effective transcription session")); @@ -212,29 +202,11 @@ pub(crate) enum ServerEvent { tentative: String, audio_start_ms: u64, audio_end_ms: u64, - #[serde( - default, - skip_serializing_if = "Option::is_none", - deserialize_with = "deserialize_present" - )] - finalized_through_ms: Option, - #[serde( - default, - skip_serializing_if = "Option::is_none", - deserialize_with = "deserialize_present" - )] - finalized_seq: Option, }, #[serde(rename = "error")] Error { event_id: String, error: WireError }, } -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -pub(in crate::realtime) struct HypothesisRanges { - pub(in crate::realtime) finalized_through_ms: u64, - pub(in crate::realtime) finalized_seq: u64, -} - #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct ConversationItem { @@ -376,13 +348,6 @@ impl ServerEvent { { Err(anyhow!("invalid hypothesis snapshot")) } - Self::TranscriptionHypothesis { - finalized_through_ms, - finalized_seq, - .. - } if finalized_through_ms.is_some() != finalized_seq.is_some() => { - Err(anyhow!("hypothesis range fields must appear together")) - } _ => Ok(()), } } diff --git a/crates/gateway/stt/api/src/realtime/wire/tests.rs b/crates/gateway/stt/api/src/realtime/wire/tests.rs index a597d899c..4f1a12aa7 100644 --- a/crates/gateway/stt/api/src/realtime/wire/tests.rs +++ b/crates/gateway/stt/api/src/realtime/wire/tests.rs @@ -21,7 +21,6 @@ const WIRE_INVALID_CASES: &[&str] = &[ "missing_session_type", "non_null_noise_reduction", "non_null_turn_detection", - "ranges_include_without_hypothesis", "repeated_include", "session_audio_unknown_field", "session_input_unknown_field", @@ -161,63 +160,20 @@ fn mixed_valid_and_invalid_session_updates_change_no_effective_state() { } #[test] -fn ranges_include_extends_the_hypothesis_include_in_either_order() { +fn the_hypothesis_include_update_produces_the_updated_session() { let clients = fixture("client-events.json"); let sessions = fixture("effective-sessions.json"); let mut effective = EffectiveSession::new("sess_canonical".to_owned()); - effective - .apply_update_text(&clients["session_update_ranges"].to_string()) - .unwrap_or_else(|error| panic!("ranges update applies: {error:?}")); - assert_eq!( - serde_json::to_value(&effective) - .unwrap_or_else(|error| panic!("ranges session serializes: {error}")), - sessions["ranges"] - ); - - let mut reversed = clients["session_update_ranges"].clone(); - reversed["session"]["include"] = serde_json::json!([ - "item.input_audio_transcription.hypothesis.ranges", - "item.input_audio_transcription.hypothesis" - ]); - let mut reversed_effective = EffectiveSession::new("sess_canonical".to_owned()); - reversed_effective - .apply_update_text(&reversed.to_string()) - .unwrap_or_else(|error| panic!("reversed ranges update applies: {error:?}")); - assert_eq!(reversed_effective, effective); - effective .apply_update_text(&clients["session_update"].to_string()) .unwrap_or_else(|error| panic!("base update applies: {error:?}")); assert_eq!( serde_json::to_value(&effective) .unwrap_or_else(|error| panic!("updated session serializes: {error}")), - sessions["updated"], - "the base token alone drops the ranges extension" + sessions["updated"] ); } -#[test] -fn hypothesis_range_fields_appear_together_and_are_never_null() { - let servers = fixture("server-events.json"); - let ranges = &servers["transcription_hypothesis_ranges"]; - for field in ["finalized_through_ms", "finalized_seq"] { - let mut lone = ranges.clone(); - lone.as_object_mut() - .unwrap_or_else(|| panic!("ranges hypothesis is an object")) - .remove(field); - assert!( - ServerEvent::from_value(lone).is_err(), - "the other range field cannot appear without {field}" - ); - let mut null = ranges.clone(); - null[field] = Value::Null; - assert!( - ServerEvent::from_value(null).is_err(), - "{field} is a number when present" - ); - } -} - #[test] fn canonical_server_events_round_trip_with_exact_shapes() { let fixture = fixture("server-events.json"); diff --git a/crates/gateway/stt/api/src/realtime/wire/vocabulary.rs b/crates/gateway/stt/api/src/realtime/wire/vocabulary.rs index 787e0a910..0f93276c3 100644 --- a/crates/gateway/stt/api/src/realtime/wire/vocabulary.rs +++ b/crates/gateway/stt/api/src/realtime/wire/vocabulary.rs @@ -13,15 +13,12 @@ pub(super) const AUDIO_TYPE: &str = "audio/pcm"; pub(super) const AUDIO_RATE: u32 = 24_000; pub(super) const MODEL: &str = "realtime-transcribe"; pub(super) const HYPOTHESIS_INCLUDE: &str = "item.input_audio_transcription.hypothesis"; -pub(super) const HYPOTHESIS_RANGES_INCLUDE: &str = - "item.input_audio_transcription.hypothesis.ranges"; /// The hypothesis event shape a session negotiated through `session.include`. #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub(crate) enum HypothesisInclude { Off, Snapshots, - Ranges, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -208,15 +205,6 @@ where RequiredNullable::deserialize(deserializer) } -/// Reads an optional field that may be omitted but is never `null`. -pub(super) fn deserialize_present<'de, D, T>(deserializer: D) -> Result, D::Error> -where - D: serde::Deserializer<'de>, - T: Deserialize<'de>, -{ - T::deserialize(deserializer).map(Some) -} - #[derive(Debug, Default)] pub(super) enum OptionalNullable { #[default] diff --git a/crates/gateway/stt/api/src/take/interim.rs b/crates/gateway/stt/api/src/take/interim.rs index d119e58d7..6464ccb5d 100644 --- a/crates/gateway/stt/api/src/take/interim.rs +++ b/crates/gateway/stt/api/src/take/interim.rs @@ -47,11 +47,9 @@ impl Take { } } -/// Absolute sample watermark of finalized text and the count of final -/// outcomes applied to the take. +/// The count of final outcomes applied to the take. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct FinalizedRange { - pub(crate) through_samples: u64, pub(crate) seq: u64, } diff --git a/crates/gateway/stt/api/src/take/live_prefix.rs b/crates/gateway/stt/api/src/take/live_prefix.rs index 375b84652..39da8a189 100644 --- a/crates/gateway/stt/api/src/take/live_prefix.rs +++ b/crates/gateway/stt/api/src/take/live_prefix.rs @@ -70,7 +70,6 @@ impl LivePrefixSnapshot { pub(super) const fn finalized_range(&self) -> FinalizedRange { FinalizedRange { - through_samples: self.finalized_samples, seq: self.applied_outcomes, } } diff --git a/crates/gateway/stt/api/src/take/state/tests-live-prefix.rs b/crates/gateway/stt/api/src/take/state/tests-live-prefix.rs index 295bec130..08f04a3ec 100644 --- a/crates/gateway/stt/api/src/take/state/tests-live-prefix.rs +++ b/crates/gateway/stt/api/src/take/state/tests-live-prefix.rs @@ -112,13 +112,9 @@ fn the_live_prefix_counts_applied_final_outcomes_beside_the_finalized_watermark( &[], &ShownHypotheses::default(), ); - assert_eq!( - state.live_prefix_snapshot().finalized_range(), - FinalizedRange { - through_samples: 16_000, - seq: 1 - } - ); + let snapshot = state.live_prefix_snapshot(); + assert_eq!(snapshot.finalized_range(), FinalizedRange { seq: 1 }); + assert_eq!(snapshot.finalized_samples(), 16_000); state.record_final_outcome( FinalRangeOutcome::forced( @@ -128,13 +124,16 @@ fn the_live_prefix_counts_applied_final_outcomes_beside_the_finalized_watermark( &[], &ShownHypotheses::default(), ); + let snapshot = state.live_prefix_snapshot(); + assert_eq!( + snapshot.finalized_range(), + FinalizedRange { seq: 2 }, + "pending forced text counts as applied" + ); assert_eq!( - state.live_prefix_snapshot().finalized_range(), - FinalizedRange { - through_samples: 16_000, - seq: 2 - }, - "pending forced text counts as applied without moving the finalized watermark" + snapshot.finalized_samples(), + 16_000, + "pending forced text does not move the finalized watermark" ); state.record_final_outcome( @@ -143,12 +142,11 @@ fn the_live_prefix_counts_applied_final_outcomes_beside_the_finalized_watermark( &ShownHypotheses::default(), ); assert!(state.pending_failure().is_some()); + let snapshot = state.live_prefix_snapshot(); assert_eq!( - state.live_prefix_snapshot().finalized_range(), - FinalizedRange { - through_samples: 16_000, - seq: 2 - }, + snapshot.finalized_range(), + FinalizedRange { seq: 2 }, "a rejected outcome is not counted" ); + assert_eq!(snapshot.finalized_samples(), 16_000); } diff --git a/crates/gateway/stt/api/tests/fixtures/realtime/client-events.json b/crates/gateway/stt/api/tests/fixtures/realtime/client-events.json index cff8c96ee..934df03c4 100644 --- a/crates/gateway/stt/api/tests/fixtures/realtime/client-events.json +++ b/crates/gateway/stt/api/tests/fixtures/realtime/client-events.json @@ -33,16 +33,5 @@ ] }, "event_id": "client_update_1" - }, - "session_update_ranges": { - "type": "session.update", - "session": { - "type": "transcription", - "include": [ - "item.input_audio_transcription.hypothesis", - "item.input_audio_transcription.hypothesis.ranges" - ] - }, - "event_id": "client_update_ranges" } } diff --git a/crates/gateway/stt/api/tests/fixtures/realtime/effective-sessions.json b/crates/gateway/stt/api/tests/fixtures/realtime/effective-sessions.json index a5d8f2e16..eabdbd567 100644 --- a/crates/gateway/stt/api/tests/fixtures/realtime/effective-sessions.json +++ b/crates/gateway/stt/api/tests/fixtures/realtime/effective-sessions.json @@ -40,28 +40,5 @@ "include": [ "item.input_audio_transcription.hypothesis" ] - }, - "ranges": { - "id": "sess_canonical", - "object": "realtime.transcription_session", - "type": "transcription", - "audio": { - "input": { - "format": { - "type": "audio/pcm", - "rate": 24000 - }, - "noise_reduction": null, - "transcription": { - "model": "realtime-transcribe", - "prompt": "" - }, - "turn_detection": null - } - }, - "include": [ - "item.input_audio_transcription.hypothesis", - "item.input_audio_transcription.hypothesis.ranges" - ] } } diff --git a/crates/gateway/stt/api/tests/fixtures/realtime/invalid-sequences.json b/crates/gateway/stt/api/tests/fixtures/realtime/invalid-sequences.json index 2a56e5094..2d3c16a3d 100644 --- a/crates/gateway/stt/api/tests/fixtures/realtime/invalid-sequences.json +++ b/crates/gateway/stt/api/tests/fixtures/realtime/invalid-sequences.json @@ -125,12 +125,6 @@ "effective_session_after": "default", "keeps_connection_usable": true }, - "ranges_include_without_hypothesis": { - "input": { "message": { "type": "session.update", "session": { "type": "transcription", "include": ["item.input_audio_transcription.hypothesis.ranges"] }, "event_id": "client_ranges_alone" } }, - "expected_error": { "event_id": "evt_ranges_alone", "type": "error", "error": { "type": "invalid_request_error", "code": "unsupported_include", "message": "Include value item.input_audio_transcription.hypothesis.ranges requires item.input_audio_transcription.hypothesis", "param": "session.include", "event_id": "client_ranges_alone" } }, - "effective_session_after": "default", - "keeps_connection_usable": true - }, "repeated_include": { "input": { "message": { "type": "session.update", "session": { "type": "transcription", "include": ["item.input_audio_transcription.hypothesis", "item.input_audio_transcription.hypothesis"] }, "event_id": "client_repeated_include" } }, "expected_error": { "event_id": "evt_repeated_include", "type": "error", "error": { "type": "invalid_request_error", "code": "unsupported_include", "message": "session.include repeats an include value", "param": "session.include", "event_id": "client_repeated_include" } }, diff --git a/crates/gateway/stt/api/tests/fixtures/realtime/server-events.json b/crates/gateway/stt/api/tests/fixtures/realtime/server-events.json index 5cc484710..d3eddd325 100644 --- a/crates/gateway/stt/api/tests/fixtures/realtime/server-events.json +++ b/crates/gateway/stt/api/tests/fixtures/realtime/server-events.json @@ -149,20 +149,5 @@ "tentative": "ld", "audio_start_ms": 0, "audio_end_ms": 1250 - }, - "transcription_hypothesis_ranges": { - "event_id": "evt_transcription_hypothesis_ranges", - "type": "conversation.item.input_audio_transcription.hypothesis", - "item_id": "item_alpha", - "content_index": 0, - "revision": 3, - "transcript": "Hello, world", - "finalized": "Hello", - "agreed": ", wor", - "tentative": "ld", - "audio_start_ms": 0, - "audio_end_ms": 1250, - "finalized_through_ms": 400, - "finalized_seq": 1 } } diff --git a/crates/gateway/stt/api/tests/fixtures/realtime/valid-sequences.json b/crates/gateway/stt/api/tests/fixtures/realtime/valid-sequences.json index 5d094dbef..44228ba3c 100644 --- a/crates/gateway/stt/api/tests/fixtures/realtime/valid-sequences.json +++ b/crates/gateway/stt/api/tests/fixtures/realtime/valid-sequences.json @@ -71,21 +71,6 @@ "completion is authoritative" ] }, - "hypothesis_ranges_negotiation": { - "events": [ - { "direction": "client", "message": { "event_id": "client_hypothesis_ranges", "type": "session.update", "session": { "type": "transcription", "include": ["item.input_audio_transcription.hypothesis", "item.input_audio_transcription.hypothesis.ranges"] } } }, - { "direction": "server", "message": { "event_id": "evt_hypothesis_ranges_updated", "type": "session.updated", "session": { "id": "sess_hypothesis_ranges", "object": "realtime.transcription_session", "type": "transcription", "audio": { "input": { "format": { "type": "audio/pcm", "rate": 24000 }, "noise_reduction": null, "transcription": { "model": "realtime-transcribe", "prompt": "" }, "turn_detection": null } }, "include": ["item.input_audio_transcription.hypothesis", "item.input_audio_transcription.hypothesis.ranges"] } } }, - { "direction": "client", "message": { "type": "input_audio_buffer.append", "audio": "AAABAP//" } }, - { "direction": "server", "message": { "event_id": "evt_hypothesis_ranges_1", "type": "conversation.item.input_audio_transcription.hypothesis", "item_id": "item_hypothesis_ranges", "content_index": 0, "revision": 1, "transcript": "Hello", "finalized": "", "agreed": "Hel", "tentative": "lo", "audio_start_ms": 0, "audio_end_ms": 250, "finalized_through_ms": 0, "finalized_seq": 0 } }, - { "direction": "server", "message": { "event_id": "evt_hypothesis_ranges_2", "type": "conversation.item.input_audio_transcription.hypothesis", "item_id": "item_hypothesis_ranges", "content_index": 0, "revision": 2, "transcript": "Hello!", "finalized": "Hello", "agreed": "", "tentative": "!", "audio_start_ms": 0, "audio_end_ms": 300, "finalized_through_ms": 250, "finalized_seq": 1 } }, - { "direction": "server", "message": { "event_id": "evt_hypothesis_ranges_done", "type": "conversation.item.input_audio_transcription.completed", "item_id": "item_hypothesis_ranges", "content_index": 0, "transcript": "Hello!", "usage": { "type": "duration", "seconds": 0.25 } } } - ], - "invariants": [ - "range fields appear only when both include tokens are negotiated", - "the extended event is the base hypothesis event plus finalized_through_ms and finalized_seq", - "finalized_seq counts the final outcomes applied to the item" - ] - }, "producer_hypothesis_ownership": { "events": [ { "direction": "client", "message": { "event_id": "client_producer_hypothesis", "type": "session.update", "session": { "type": "transcription", "include": ["item.input_audio_transcription.hypothesis"] } } }, diff --git a/crates/gateway/stt/api/tests/it/realtime_fixtures.rs b/crates/gateway/stt/api/tests/it/realtime_fixtures.rs index fb03460df..804bd7020 100644 --- a/crates/gateway/stt/api/tests/it/realtime_fixtures.rs +++ b/crates/gateway/stt/api/tests/it/realtime_fixtures.rs @@ -90,12 +90,8 @@ fn assert_session(value: &Value, context: &str) { matches!( include.as_slice(), [] | ["item.input_audio_transcription.hypothesis"] - | [ - "item.input_audio_transcription.hypothesis", - "item.input_audio_transcription.hypothesis.ranges" - ] ), - "{context} include is empty, the hypothesis token, or the hypothesis token then ranges" + "{context} include is empty or the hypothesis token" ); let audio = object( @@ -211,26 +207,6 @@ fn assert_server_event_fields(value: &Value, context: &str) { "conversation.item.input_audio_transcription.failed" => { &["content_index", "error", "event_id", "item_id", "type"][..] } - "conversation.item.input_audio_transcription.hypothesis" - if event.contains_key("finalized_through_ms") - || event.contains_key("finalized_seq") => - { - &[ - "agreed", - "audio_end_ms", - "audio_start_ms", - "content_index", - "event_id", - "finalized", - "finalized_seq", - "finalized_through_ms", - "item_id", - "revision", - "tentative", - "transcript", - "type", - ][..] - } "conversation.item.input_audio_transcription.hypothesis" => &[ "agreed", "audio_end_ms", @@ -254,11 +230,6 @@ fn assert_server_event_fields(value: &Value, context: &str) { if let Some(content_index) = event.get("content_index") { assert_eq!(content_index, 0, "{context}.content_index is zero"); } - for field in ["finalized_through_ms", "finalized_seq"] { - if let Some(value) = event.get(field) { - assert!(value.as_u64().is_some(), "{context}.{field} is unsigned"); - } - } if let Some(previous) = event.get("previous_item_id") { assert!( previous.is_null() || previous.as_str().is_some_and(|id| !id.is_empty()), diff --git a/crates/gateway/stt/api/tests/it/realtime_fixtures/events.rs b/crates/gateway/stt/api/tests/it/realtime_fixtures/events.rs index fdbcf3fc9..f445521a1 100644 --- a/crates/gateway/stt/api/tests/it/realtime_fixtures/events.rs +++ b/crates/gateway/stt/api/tests/it/realtime_fixtures/events.rs @@ -22,7 +22,6 @@ const CLIENT_CASES: &[&str] = &[ "input_audio_buffer_clear", "input_audio_buffer_commit", "session_update", - "session_update_ranges", ]; const SERVER_CASES: &[&str] = &[ @@ -38,7 +37,6 @@ const SERVER_CASES: &[&str] = &[ "transcription_delta", "transcription_failed", "transcription_hypothesis", - "transcription_hypothesis_ranges", ]; #[test] @@ -120,25 +118,10 @@ fn canonical_realtime_events_are_complete_strict_and_round_trip() { "session_update.session.audio.input", ); - let ranges_update = object( - &clients["session_update_ranges"]["session"], - "session_update_ranges.session", - ); - assert_exact_keys( - ranges_update, - &["include", "type"], - "session_update_ranges.session", - ); - let sessions = fixture("effective-sessions.json"); - assert_exact_cases( - &sessions, - &["default", "ranges", "updated"], - "effective sessions", - ); + assert_exact_cases(&sessions, &["default", "updated"], "effective sessions"); assert_session(&sessions["default"], "default session"); assert_session(&sessions["updated"], "updated session"); - assert_session(&sessions["ranges"], "ranges session"); let servers = fixture("server-events.json"); assert_exact_cases(&servers, SERVER_CASES, "server event cases"); @@ -194,17 +177,6 @@ fn canonical_realtime_events_are_complete_strict_and_round_trip() { .expect("hypothesis end is unsigned"); assert!(start <= end, "hypothesis span is half-open and ordered"); - let mut extended = object(&servers["transcription_hypothesis_ranges"], "ranges").clone(); - let mut base = hypothesis.clone(); - for field in ["event_id", "finalized_through_ms", "finalized_seq"] { - extended.remove(field); - base.remove(field); - } - assert_eq!( - extended, base, - "the ranges hypothesis is the base hypothesis plus the two range fields" - ); - let mut server_ids = HashSet::new(); for event in servers.values() { let id = event["event_id"] diff --git a/crates/gateway/stt/api/tests/it/realtime_fixtures/sequences.rs b/crates/gateway/stt/api/tests/it/realtime_fixtures/sequences.rs index d0145923f..3d3238860 100644 --- a/crates/gateway/stt/api/tests/it/realtime_fixtures/sequences.rs +++ b/crates/gateway/stt/api/tests/it/realtime_fixtures/sequences.rs @@ -13,7 +13,6 @@ const VALID_SEQUENCE_CASES: &[&str] = &[ "durable_lineage", "first_event_readiness", "hypothesis_negotiation", - "hypothesis_ranges_negotiation", "immediate_commit_and_provisional_promotion", "optional_client_ids_and_error_correlation", "overlapping_items_reverse_completion", @@ -46,7 +45,6 @@ const INVALID_SEQUENCE_CASES: &[&str] = &[ "missing_session_type", "non_null_noise_reduction", "non_null_turn_detection", - "ranges_include_without_hypothesis", "repeated_include", "session_audio_unknown_field", "session_input_unknown_field", diff --git a/crates/workshop/desktop/Cargo.toml b/crates/workshop/desktop/Cargo.toml index fd3f06d9f..a5d295c91 100644 --- a/crates/workshop/desktop/Cargo.toml +++ b/crates/workshop/desktop/Cargo.toml @@ -27,8 +27,8 @@ workshop-server-api.workspace = true reqwest.workspace = true serde.workspace = true serde_json.workspace = true -# The health wait comes from the shared gateway-discovery-file seam. -gateway-api-discovery.workspace = true +# The health wait comes from the shared gateway-discovery-file seam, which +# workshop-server-api re-exports from workshop-gateway. tauri.workspace = true tauri-plugin-dialog.workspace = true tauri-plugin-opener.workspace = true @@ -61,10 +61,6 @@ default = [] [dev-dependencies] tempfile.workspace = true workshop-server-api = { workspace = true, features = ["test-fixtures"] } -# The test-fixtures feature exposes `resolve_for_test`, so the boot -# decision tests run the real liveness gauntlet against the test binary's -# own process image. -gateway-api-discovery = { workspace = true, features = ["test-fixtures"] } [lints] workspace = true diff --git a/crates/workshop/desktop/src/gateway/boot.rs b/crates/workshop/desktop/src/gateway/boot.rs index fec29d768..cd46247f5 100644 --- a/crates/workshop/desktop/src/gateway/boot.rs +++ b/crates/workshop/desktop/src/gateway/boot.rs @@ -5,10 +5,10 @@ use std::process::ExitStatus; use std::sync::{Arc, OnceLock}; use anyhow::Context as _; -use gateway_api_discovery::{ +use workshop_server_api::Config; +use workshop_server_api::discovery::{ CancellationToken, GatewayDiscoveryFile, Resolution, SidecarError, ValidatedConnection, }; -use workshop_server_api::Config; use super::identity::GatewayAttachment; use super::supervisor::{RecoveryCandidate, RecoveryOwnership, launch_and_attach_cancellable}; @@ -71,14 +71,19 @@ pub(crate) fn ensure_gateway(config: &Config) -> anyhow::Result validated_attachment(file), GatewayPlan::ConfigOnly => Ok(GatewayAttachment::Config), GatewayPlan::Fail => Err(no_gateway_error()), diff --git a/crates/workshop/desktop/src/gateway/identity.rs b/crates/workshop/desktop/src/gateway/identity.rs index ce1211080..57e889ac1 100644 --- a/crates/workshop/desktop/src/gateway/identity.rs +++ b/crates/workshop/desktop/src/gateway/identity.rs @@ -1,6 +1,6 @@ //! Local Gateway attachment and validated process identity. -use gateway_api_discovery::ValidatedConnection; +use workshop_server_api::discovery::ValidatedConnection; use super::supervisor::RecoveryCandidate; diff --git a/crates/workshop/desktop/src/gateway/supervisor.rs b/crates/workshop/desktop/src/gateway/supervisor.rs index 7e57f29fa..9953d43c9 100644 --- a/crates/workshop/desktop/src/gateway/supervisor.rs +++ b/crates/workshop/desktop/src/gateway/supervisor.rs @@ -4,7 +4,9 @@ use std::path::Path; use std::time::Duration; use anyhow::Context as _; -use gateway_api_discovery::{CancellationToken, Resolution, SidecarError, ValidatedConnection}; +use workshop_server_api::discovery::{ + CancellationToken, Resolution, SidecarError, ValidatedConnection, +}; use super::boot; use super::identity::GatewayAttachment; @@ -73,8 +75,8 @@ pub(crate) fn supervise( let Some(initial) = attachment.sidecar_identity().cloned() else { return Ok(None); }; - let run_dir = - gateway_api_discovery::default_run_dir().context("locate the sidecar run directory")?; + let run_dir = workshop_server_api::discovery::default_run_dir() + .context("locate the sidecar run directory")?; let exe_dir = std::env::current_exe() .context("locate the executable")? .parent() @@ -85,7 +87,7 @@ pub(crate) fn supervise( GatewaySupervisor::spawn_with_publication(supervisor_publication, move |cancellation| { run_supervision( RecoveryIdentity::Stable(initial), - |_, cancellation| match gateway_api_discovery::resolve_cancellable( + |_, cancellation| match workshop_server_api::discovery::resolve_cancellable( &run_dir, cancellation, ) { diff --git a/crates/workshop/desktop/src/gateway/supervisor/launch.rs b/crates/workshop/desktop/src/gateway/supervisor/launch.rs index 73e2526a1..0e824aa2d 100644 --- a/crates/workshop/desktop/src/gateway/supervisor/launch.rs +++ b/crates/workshop/desktop/src/gateway/supervisor/launch.rs @@ -6,7 +6,7 @@ use std::sync::{Arc, OnceLock}; use std::time::{Duration, Instant}; use anyhow::Context as _; -use gateway_api_discovery::{ +use workshop_server_api::discovery::{ CancellationToken, GatewayDiscoveryFile, LaunchDecision, Resolution, SidecarError, }; @@ -56,7 +56,7 @@ pub(in crate::gateway) fn launch_and_attach_cancellable( run_dir, exe, cancellation, - gateway_api_discovery::launch_or_attach_cancellable, + workshop_server_api::discovery::launch_or_attach_cancellable, |exe, _| boot::spawn_detached(exe, Arc::clone(&exit)), |run_dir, timeout, cancellation| { wait_for_launched_file_cancellable(run_dir, timeout, cancellation, || { @@ -128,8 +128,8 @@ fn wait_for_launched_file_cancellable( timeout, cancellation, &SystemClock, - gateway_api_discovery::wait_for_health_cancellable, - gateway_api_discovery::resolve_cancellable, + workshop_server_api::discovery::wait_for_health_cancellable, + workshop_server_api::discovery::resolve_cancellable, exited, ) } @@ -149,8 +149,11 @@ pub(in crate::gateway) fn wait_for_launched_file_cancellable_with anyhow::Result where Clock: WaitClock, - Health: - FnMut(&str, Duration, &CancellationToken) -> Result<(), gateway_api_discovery::HealthError>, + Health: FnMut( + &str, + Duration, + &CancellationToken, + ) -> Result<(), workshop_server_api::discovery::HealthError>, Resolve: FnMut(&Path, &CancellationToken) -> Result, Exited: FnMut() -> Option, { @@ -197,7 +200,7 @@ where Ok(_) => {} } } - Err(gateway_api_discovery::HealthError::Cancelled) => { + Err(workshop_server_api::discovery::HealthError::Cancelled) => { anyhow::bail!("the launched gateway wait was cancelled"); } Err(error) => { @@ -212,7 +215,7 @@ where anyhow::bail!( "the launched gateway exited ({status}) without writing a gateway discovery \ file; see the gateway log at {}", - gateway_api_discovery::gateway_log_path(state_dir).display() + workshop_server_api::discovery::gateway_log_path(state_dir).display() ); } if cancellation.is_cancelled() { diff --git a/crates/workshop/desktop/src/gateway/supervisor/lifecycle.rs b/crates/workshop/desktop/src/gateway/supervisor/lifecycle.rs index b774344ef..6eb41e47f 100644 --- a/crates/workshop/desktop/src/gateway/supervisor/lifecycle.rs +++ b/crates/workshop/desktop/src/gateway/supervisor/lifecycle.rs @@ -3,7 +3,7 @@ use std::time::{Duration, Instant}; use anyhow::Context as _; -use gateway_api_discovery::CancellationToken; +use workshop_server_api::discovery::CancellationToken; use super::signals::{Completion, StopSignal}; diff --git a/crates/workshop/desktop/src/gateway/supervisor/recovery.rs b/crates/workshop/desktop/src/gateway/supervisor/recovery.rs index 1a5a1031b..609b81a21 100644 --- a/crates/workshop/desktop/src/gateway/supervisor/recovery.rs +++ b/crates/workshop/desktop/src/gateway/supervisor/recovery.rs @@ -2,7 +2,7 @@ use std::time::{Duration, Instant}; -use gateway_api_discovery::{ShutdownError, ValidatedConnection}; +use workshop_server_api::discovery::{ShutdownError, ValidatedConnection}; use super::super::identity::same_gateway_identity; use super::SupervisedGatewayIdentity; @@ -61,7 +61,7 @@ impl RecoveryCandidate { debug_assert_eq!(self.child_pid, self.validated.pid()); self.published = true; let deadline = Instant::now() + LATE_CHILD_SHUTDOWN_BUDGET; - gateway_api_discovery::request_shutdown_before(&self.validated, deadline) + workshop_server_api::discovery::request_shutdown_before(&self.validated, deadline) } } @@ -80,7 +80,7 @@ impl Drop for RecoveryCandidate { .spawn(move || { let deadline = Instant::now() + LATE_CHILD_SHUTDOWN_BUDGET; if let Err(error) = - gateway_api_discovery::request_shutdown_before(&validated, deadline) + workshop_server_api::discovery::request_shutdown_before(&validated, deadline) { // The detached signal has no error return channel, so // diagnostics are the only place this cleanup failure diff --git a/crates/workshop/desktop/src/gateway/tests.rs b/crates/workshop/desktop/src/gateway/tests.rs index 3cbdc8fd8..1c3c4ab07 100644 --- a/crates/workshop/desktop/src/gateway/tests.rs +++ b/crates/workshop/desktop/src/gateway/tests.rs @@ -6,7 +6,9 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use gateway_api_discovery::{GatewayDiscoveryFile, Resolution, SidecarError, ValidatedConnection}; +use workshop_server_api::discovery::{ + GatewayDiscoveryFile, Resolution, SidecarError, ValidatedConnection, +}; use super::boot as gateway_boot; use super::supervisor::{ @@ -71,13 +73,15 @@ fn probe_own_image(run_dir: &Path) -> Result { .expect("the exe has a file name") .to_string_lossy() .into_owned(); - gateway_api_discovery::resolve_for_test(run_dir, &image) + workshop_server_api::discovery::resolve_for_test(run_dir, &image) } /// Plants an unreadable discovery-file path before resolving. fn probe_read_failure(run_dir: &Path) -> Result { - std::fs::create_dir(gateway_api_discovery::gateway_discovery_file_path(run_dir)) - .expect("plant the unreadable file"); + std::fs::create_dir(workshop_server_api::discovery::gateway_discovery_file_path( + run_dir, + )) + .expect("plant the unreadable file"); probe_own_image(run_dir) } diff --git a/crates/workshop/desktop/src/gateway/tests/boot-launch-wait.rs b/crates/workshop/desktop/src/gateway/tests/boot-launch-wait.rs index 89a574575..4a2b4ab92 100644 --- a/crates/workshop/desktop/src/gateway/tests/boot-launch-wait.rs +++ b/crates/workshop/desktop/src/gateway/tests/boot-launch-wait.rs @@ -5,7 +5,9 @@ use std::path::Path; use std::process::ExitStatus; use std::time::{Duration, Instant}; -use gateway_api_discovery::{CancellationToken, GatewayDiscoveryFile, HealthError, ProbeError}; +use workshop_server_api::discovery::{ + CancellationToken, GatewayDiscoveryFile, HealthError, ProbeError, +}; use super::super::{fixture_gateway, live_file, probe_own_image}; use crate::gateway::supervisor::{ @@ -127,7 +129,7 @@ fn launch_wait(run_dir: &Path, budget: Duration) -> anyhow::Result anyhow::Result<( let server = workshop_server_api::spawn(config).context("start the in-process workshop server")?; let attachment = attachment.reconcile_publication(server.initial_gateway_identity().cloned()); - match gateway_api_discovery::wait_for_health(server.url(), HEALTH_TIMEOUT) + match workshop_server_api::discovery::wait_for_health(server.url(), HEALTH_TIMEOUT) .context("wait for the in-process workshop server") { Ok(()) => { diff --git a/crates/workshop/gateway-discovery/CONTRACT.md b/crates/workshop/gateway-discovery/CONTRACT.md new file mode 100644 index 000000000..2eecdfeca --- /dev/null +++ b/crates/workshop/gateway-discovery/CONTRACT.md @@ -0,0 +1,198 @@ +# The Gateway discovery contract + +The Gateway and Workshop meet through a file plus three HTTP calls. Nothing else is shared. This document is the whole of that meeting point: the file format, the two lock files, the stale rule, the shutdown route, the three calls, and the fixtures that pin them. + +Both sides are written against this text. Workshop's reader is [`workshop-gateway-discovery`](src/lib.rs), and the Gateway's writer is the `gateway-api-discovery` crate, which is the same protocol spelled once from the other side. When the two disagree, this document is right and one of them has a bug. + +- Names in the file: `gateway.json`, `gateway.json.lock`, `gateway.instance.lock`, `gateway.log` +- Endpoints: `GET /health`, `GET /v1/models`, `POST /shutdown` +- Address: `127.0.0.1:{port}`, never `localhost` +- Contract fixtures: [`fixtures/`](fixtures/) + +## The discovery file + +The Gateway writes `gateway.json` into the run directory once it has bound its loopback port. A reader that finds a live file attaches to that Gateway instead of starting a second one. This is the Jupyter `connection file` pattern. + +### The six fields + +```json +{ + "port": 8081, + "api_key": "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b", + "pid": 4242, + "epoch": 1757000000, + "version": "0.2.0", + "started_at": "2026-09-03T12:00:00Z" +} +``` + +- `port` (integer, `u16`): the port the Gateway bound. A reader connects to `127.0.0.1:{port}`. Must not be `0`. +- `api_key` (string): the bearer key every authenticated request carries. Must not be empty and must not contain a carriage return or a line feed, because it is written into an `Authorization` header line. +- `pid` (integer, `u32`): the Gateway process id, used by stale detection. Must not be `0`. +- `epoch` (integer, `u64`): the boot instant in unix seconds. `0` means "not recorded", and a file with `0` carries no boot identity, so it reads as stale. +- `version` (string): the Gateway crate version, for diagnostics only. +- `started_at` (string): the same boot instant as an RFC 3339 timestamp, for diagnostics and for boot identity comparison. A whitespace-only value means "not recorded". + +### Unknown fields + +A reader must tolerate fields it does not know. serde's derived `Deserialize` ignores them, which is what both sides rely on, so a newer Gateway can add a field without breaking an older Workshop. + +### Validation + +A file is valid when `port` is nonzero, `api_key` is nonempty and holds no line break, and `pid` is nonzero. A file that fails validation is not "absent": it is stale with reason `Invalid`, and the reader deletes it. + +A file carries a **boot identity** when `epoch` is nonzero and `started_at` is not blank. A file with valid fields but no boot identity is stale with reason `BootIdentityInvalid`. + +### JSON Schema + +The schema below is the machine-readable form of the field table. It is written by hand and kept beside the fixtures rather than generated, because the contract is the input to both sides, not an output of either. + +```json +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://runvil.example/schemas/gateway.json", + "title": "Gateway discovery file", + "type": "object", + "required": ["port", "api_key", "pid", "epoch", "version", "started_at"], + "additionalProperties": true, + "properties": { + "port": { "type": "integer", "minimum": 1, "maximum": 65535 }, + "api_key": { "type": "string", "minLength": 1, "pattern": "^[^\\r\\n]*$" }, + "pid": { "type": "integer", "minimum": 1, "maximum": 4294967295 }, + "epoch": { "type": "integer", "minimum": 0 }, + "version": { "type": "string" }, + "started_at": { "type": "string" } + } +} +``` + +`additionalProperties` is `true` on purpose: unknown fields are tolerated. The `api_key` pattern is the "no line break" rule. `epoch` allows `0` even though a zero epoch makes the file stale, because the field is present-and-informational while the boot identity rule decides staleness. + +## The run directory and the two locks + +### Where things live + +- The run directory is `/.runvil/run`, where `` is the user profile directory the OS reports. +- The state directory is `/.runvil`, the run directory's parent. +- `gateway.json`, `gateway.json.lock`, and `gateway.instance.lock` all sit in the run directory, beside each other. +- `gateway.log` sits in `/.runvil/logs/gateway.log`, beside the run directory rather than in it, because the Gateway starts logging before it loads its configuration, so a Gateway that exits on its configuration still records why. + +### The atomic owner-only write + +The Gateway must never leave a truncated `gateway.json`. A reader racing a writer must see either the whole previous file or the whole new one. So every write goes: + +1. to a uniquely named sibling temp file, `gateway.json.-.rv-tmp`, so two concurrent writers never fill the same temp; +2. created with mode `0600` on Unix, so the bearer key never exists in a looser file and the rename carries that mode to the target; +3. synced to disk; +4. renamed over `gateway.json`, which is atomic within a directory. + +A failed write removes the temp and leaves the target's previous contents. On Windows there are no mode bits, and the file relies on the user profile's ACL, which already restricts it to the owner. + +Clean shutdown removes the file with `remove_if_mine`: it removes `gateway.json` only when the file's `pid` is the remover's own, so a shutting-down Gateway cannot delete a replacement's file. A missing file is not an error. An unreadable or foreign file is left for the next reader's stale cleanup. + +### `gateway.json.lock` - the launch election + +The problem it solves: two readers find no live Gateway at the same moment and both launch one. The Jupyter phantom-server bug class. The lock settles the race so exactly one launches. + +- It is an OS advisory lock on an ordinary file in the run directory. Whoever holds it is the launcher. +- The holder **re-validates** `gateway.json` after taking the lock before deciding to launch, because a previous winner may have written the file and released the lock in between. +- While another process holds it, a loser polls every 25 ms. As soon as the winner's file goes live the loser attaches to it. +- A loser never deletes anything. Cleanup of a stale file is the lock holder's privilege, because only the holder knows no launch is in flight. +- A loser that waits out its budget without an attachable Gateway reports a launch timeout. +- Dropping the file handle releases the lock; there is no pid file and no explicit unlock. + +### `gateway.instance.lock` - process-lifetime ownership + +A different question from the launch election: is a Gateway process already alive on this machine? + +- Every Gateway process acquires this lock before it performs any startup side effect, and holds it until the process exits. +- It is separate from `gateway.json.lock` because a Workshop parent may hold the launch election while the spawned Gateway acquires this lease. One lock cannot answer both questions. +- Acquisition is non-blocking. "Another process owns it" is a normal answer, not an error. +- The OS releases it when the owning process dies, so a killed Gateway frees it with no cleanup path. + +## The stale rule + +A reader attaches only to a file that proves a live, authorized Gateway **right now**. Everything else is stale: the file is deleted and the reader relaunches or falls back to explicit configuration. + +The rule, in order. Each step's failure is the reason recorded. + +1. **Read.** No file: nothing to attach to and nothing to clean. The file exists but cannot be read: an I/O error, not staleness. The file is not valid JSON, or fails field validation: stale, `Invalid`. The file is deleted. +2. **One process boot, first observation.** Look up the pid's image and start marker. No such process: stale, `ProcessDead`. The image's file name is not `runvil-gateway` (`runvil-gateway.exe` on Windows, case-insensitively): stale, `ImageMismatch`. This is the reused-pid case. +3. **Boot identity.** The file must carry one; see above. Missing: stale, `BootIdentityInvalid`. +4. **One connection, two proofs.** Open one TCP connection to `127.0.0.1:{port}` and, on that same connection, send `GET /health` and then the bearer proof on `GET /v1/models`. Same connection matters: authority cannot come from a listener that replaced the endpoint after the health answer. Failures: + - health did not answer `200`: stale, `HealthFailed`; + - the socket was lost after health succeeded: stale, `HealthFailed`, and the reader does not reconnect, because a new connection could reach a different endpoint; + - the bearer answer was not 2xx: stale, `KeyRejected`. +5. **One process boot, second observation.** Re-read the pid's image and start marker and require them identical to step 2's. The pid died or was reused: stale, `ProcessChanged`. + +The two observations bracket the network proof on purpose. That closes the health-to-bearer replacement gap and rejects pid reuse during the interval. The result is a point-in-time proof; it makes no claim that the process is still live after validation returns. + +A reader that must not delete (a launch-race loser) runs the same rule but treats the answer as a yes/no and leaves the file alone. + +### The stale reasons + +Every reader-visible reason, with what it means for an operator. + +- `Invalid` - the file was not valid JSON or failed field validation. Something wrote it badly, or it was truncated by a writer that is not following the atomic write. +- `ProcessDead` - the recorded pid names no process. The Gateway exited without withdrawing its file, typically a crash. +- `ImageMismatch` - the pid is alive but runs a different program, so the pid was reused. The recorded Gateway is gone. +- `BootIdentityInvalid` - the file carries no boot identity, so the reader cannot tell one boot from another on the same pid and port. +- `ProcessChanged` - the process identity changed while validation ran. The recorded Gateway died mid-proof. +- `HealthFailed` - the process is the right image but does not answer `GET /health` with `200`. It is wedged or still starting. +- `KeyRejected` - the process is the right image and is healthy, but refused the file's bearer key. The file is from a different boot than the listener. + +`HealthFailed` and `KeyRejected` are different operator problems: a wedged Gateway versus a stale key. Keep them distinct. + +## `POST /shutdown` and the log + +### The shutdown route + +A reader holding a validated connection asks the Gateway to exit by posting its bearer key to `POST /shutdown`. + +- The request is `POST /shutdown HTTP/1.0` with `Host: 127.0.0.1:{port}` and `Authorization: Bearer {api_key}`. +- A 2xx answer means the Gateway accepted and is draining. The caller returns without waiting for the process to die; the launch election's instance lock is what actually reports the process going away. +- Any other answer is a rejection, reported with its status line. +- A delivery failure is an I/O error, including connect refused and read timeout. +- The request carries an absolute deadline where the caller has one, so cleanup of a child that never answers cannot outlive its budget. + +The bearer is never in an error message or in `Debug` output. A rejected shutdown whose status line echoes the bearer must not surface the key. + +### The log path + +`/.runvil/logs/gateway.log`. A reader that sees a launched Gateway exit without writing a file names this path in the error, because that log is the only place the Gateway's own reason appears. + +## The three HTTP calls + +All three are raw HTTP over `std::net::TcpStream`: no HTTP client crate, no TLS, no chunked encoding, no keep-alive beyond the one validation connection. + +### `GET /health` + +- Request: `GET /health HTTP/1.0`, `Host: 127.0.0.1:{port}`, blank line. +- Success: the status code is `200`. The body is drained and ignored. +- Used two ways: as a readiness poll while a launched Gateway comes up, and as the first half of the validation proof. +- The readiness poll retries every 25 ms up to its budget, with a per-attempt connect and read timeout so one hung attempt cannot eat the whole budget. A failed attempt does not end the poll; the budget does, reporting the last attempt's error. + +### `GET /v1/models` + +- Request: `GET /v1/models HTTP/1.0`, `Host: 127.0.0.1:{port}`, `Authorization: Bearer {api_key}`, blank line. +- Success: a 2xx status. The body is drained and ignored. +- This is the authority proof: a 2xx says the bearer in the file is the bearer the listener accepts. It is sent on the same connection as the health request, in that order. + +### `POST /shutdown` + +Described above. + +### The `Host` header rule + +Every one of the three sends `Host: 127.0.0.1:{port}`, the address the caller actually connected to. The Gateway's loopback `Host` allowlist refuses anything else, so sending `localhost` (or a bare host with no port) fails the allowlist and reads as a rejection that has nothing to do with the key. Never send `localhost`. + +## Golden fixtures + +The `fixtures/` directory holds committed `gateway.json` bytes. They are the drift alarm: a change to the writer's serialization that a reader would misread shows up here. + +- [`fixtures/gateway.json`](fixtures/gateway.json) - a canonical file, the shape the Gateway writes. +- [`fixtures/gateway-unknown-field.json`](fixtures/gateway-unknown-field.json) - the same file with one field a reader does not know, which must still parse. +- [`fixtures/gateway-no-boot-identity.json`](fixtures/gateway-no-boot-identity.json) - a valid-fields file with a zero epoch and blank start time, which parses but carries no boot identity. +- [`fixtures/gateway-invalid-port.json`](fixtures/gateway-invalid-port.json) - a zero port, which parses and fails validation. + +The tests that read these fixtures live beside the reader. The Gateway's own writer is pinned by the same fixtures from its side. diff --git a/crates/workshop/gateway-discovery/Cargo.toml b/crates/workshop/gateway-discovery/Cargo.toml new file mode 100644 index 000000000..56d2d1a42 --- /dev/null +++ b/crates/workshop/gateway-discovery/Cargo.toml @@ -0,0 +1,46 @@ +[package] +name = "workshop-gateway-discovery" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +publish = false + +description = "The Gateway discovery contract as Workshop reads it: gateway.json, the launch election, stale detection, and authenticated shutdown" + +[dependencies] +serde.workspace = true +serde_json.workspace = true +shared-error-source = { workspace = true, features = ["json"] } +thiserror.workspace = true + +# Process-image lookup for stale detection (src/sys/macos.rs): proc_pidpath. +[target.'cfg(target_os = "macos")'.dependencies] +libc.workspace = true + +# Process-image lookup for stale detection (src/sys/windows.rs): +# OpenProcess + QueryFullProcessImageNameW. +[target.'cfg(target_os = "windows")'.dependencies] +windows-sys = { workspace = true, features = [ + "Win32_Foundation", + "Win32_System_Threading", +] } + +[features] +# The writer half, which Workshop ships only in test builds: a test double +# writes a contract-shaped `gateway.json` and holds a process lease. No +# production Workshop code writes the file; the Gateway owns that. +test-fixtures = [] + +[dev-dependencies] +tempfile.workspace = true +# The crate's own `test-fixtures` feature, so `cargo test -p +# workshop-gateway-discovery` compiles the writer half its unit tests drive +# without `--all-features`. +workshop-gateway-discovery = { path = ".", features = ["test-fixtures"] } + +[build-dependencies] +build-ceiling.workspace = true + +[lints] +workspace = true diff --git a/crates/workshop/gateway-discovery/build.rs b/crates/workshop/gateway-discovery/build.rs new file mode 100644 index 000000000..f62713c6f --- /dev/null +++ b/crates/workshop/gateway-discovery/build.rs @@ -0,0 +1,5 @@ +//! Runs the compile-time file-size check on this crate. + +fn main() -> Result<(), build_ceiling::Violations> { + build_ceiling::check() +} diff --git a/crates/workshop/gateway-discovery/fixtures/gateway-invalid-port.json b/crates/workshop/gateway-discovery/fixtures/gateway-invalid-port.json new file mode 100644 index 000000000..ca9da3b23 --- /dev/null +++ b/crates/workshop/gateway-discovery/fixtures/gateway-invalid-port.json @@ -0,0 +1,8 @@ +{ + "port": 0, + "api_key": "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b", + "pid": 4242, + "epoch": 1757000000, + "version": "0.2.0", + "started_at": "2026-09-03T12:00:00Z" +} diff --git a/crates/workshop/gateway-discovery/fixtures/gateway-no-boot-identity.json b/crates/workshop/gateway-discovery/fixtures/gateway-no-boot-identity.json new file mode 100644 index 000000000..4ad8c3fbf --- /dev/null +++ b/crates/workshop/gateway-discovery/fixtures/gateway-no-boot-identity.json @@ -0,0 +1,8 @@ +{ + "port": 8081, + "api_key": "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b", + "pid": 4242, + "epoch": 0, + "version": "0.2.0", + "started_at": "" +} diff --git a/crates/workshop/gateway-discovery/fixtures/gateway-unknown-field.json b/crates/workshop/gateway-discovery/fixtures/gateway-unknown-field.json new file mode 100644 index 000000000..0ada599d4 --- /dev/null +++ b/crates/workshop/gateway-discovery/fixtures/gateway-unknown-field.json @@ -0,0 +1,11 @@ +{ + "port": 8081, + "api_key": "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b", + "pid": 4242, + "epoch": 1757000000, + "version": "0.2.0", + "started_at": "2026-09-03T12:00:00Z", + "future_field": { + "nested": [1, 2, 3] + } +} diff --git a/crates/workshop/gateway-discovery/fixtures/gateway.json b/crates/workshop/gateway-discovery/fixtures/gateway.json new file mode 100644 index 000000000..bce7df716 --- /dev/null +++ b/crates/workshop/gateway-discovery/fixtures/gateway.json @@ -0,0 +1,8 @@ +{ + "port": 8081, + "api_key": "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b", + "pid": 4242, + "epoch": 1757000000, + "version": "0.2.0", + "started_at": "2026-09-03T12:00:00Z" +} diff --git a/crates/workshop/gateway-discovery/src/atomic.rs b/crates/workshop/gateway-discovery/src/atomic.rs new file mode 100644 index 000000000..0b18716ca --- /dev/null +++ b/crates/workshop/gateway-discovery/src/atomic.rs @@ -0,0 +1,137 @@ +//! Crash-safe, owner-only file writes for the gateway discovery file: each write +//! lands in a uniquely named sibling temp file, is synced to disk, and is +//! renamed over the target, so a crash at any moment leaves either the old +//! contents or the new, never a truncation. Writes are owner-only because +//! the gateway discovery file holds the bearer key. +//! +//! This module exists only under the `test-fixtures` feature. Production +//! Workshop never writes the file; the Gateway owns that half of the +//! contract, and this is Workshop's test double copying its rule. + +use std::fs::{self, OpenOptions}; +use std::io::{self, Write as _}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +/// The suffix on every atomic-write temp file. +const TEMP_SUFFIX: &str = ".rv-tmp"; + +/// Process-wide counter making each temp name unique, so two concurrent +/// writes to the same target never interleave inside one temp file: each +/// writer fills its own temp completely and the last rename wins whole. +static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); + +/// The unique sibling temp path for one write to `path`, or `None` when +/// `path` has no file name to derive it from. +fn temp_path(path: &Path) -> Option { + let mut name = path.file_name()?.to_os_string(); + let counter = TEMP_COUNTER.fetch_add(1, Ordering::Relaxed); + name.push(format!(".{}-{counter}{TEMP_SUFFIX}", std::process::id())); + Some(path.with_file_name(name)) +} + +/// Writes `bytes` to `path` atomically with owner-only permissions: the +/// bytes land in a unique sibling temp file created with mode `0600` on +/// Unix (the rename preserves it, so the target never exists with looser +/// permissions), are synced to disk, and the temp is renamed over `path`. +/// On failure the temp file is removed and the target keeps its previous +/// contents. +/// +/// On Windows there are no mode bits; the file sits under the user +/// profile, whose ACL already restricts it to the owner. Best-effort by +/// design. +pub(crate) fn write_atomic_owner_only(path: &Path, bytes: &[u8]) -> io::Result<()> { + let Some(temp) = temp_path(path) else { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "write target has no file name", + )); + }; + let result = write_temp_then_rename(&temp, path, bytes); + if result.is_err() { + // Best-effort: when the create itself failed there is nothing to + // remove. + let _ = fs::remove_file(&temp); + } + result +} + +/// The Unix write: the temp file is born owner-only. +#[cfg(unix)] +fn write_temp_then_rename(temp: &Path, path: &Path, bytes: &[u8]) -> io::Result<()> { + use std::os::unix::fs::OpenOptionsExt as _; + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(temp)?; + file.write_all(bytes)?; + file.sync_data()?; + fs::rename(temp, path) +} + +/// The Windows write: no mode bits exist; the user profile's ACL is the +/// permission boundary. +#[cfg(not(unix))] +fn write_temp_then_rename(temp: &Path, path: &Path, bytes: &[u8]) -> io::Result<()> { + let mut file = OpenOptions::new().write(true).create_new(true).open(temp)?; + file.write_all(bytes)?; + file.sync_data()?; + fs::rename(temp, path) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The sorted file names in `dir`. + fn dir_names(dir: &Path) -> Vec { + let mut names: Vec = fs::read_dir(dir) + .expect("the test directory is listable") + .map(|entry| { + entry + .expect("the test entry is readable") + .file_name() + .to_string_lossy() + .into_owned() + }) + .collect(); + names.sort(); + names + } + + #[test] + fn a_write_lands_whole_and_leaves_no_temp_behind() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let target = dir.path().join("gateway.json"); + write_atomic_owner_only(&target, b"one").expect("the first write succeeds"); + assert_eq!(fs::read(&target).expect("readable"), b"one"); + write_atomic_owner_only(&target, b"two").expect("the overwrite succeeds"); + assert_eq!(fs::read(&target).expect("readable"), b"two"); + assert_eq!(dir_names(dir.path()), ["gateway.json"]); + } + + #[test] + fn a_failed_rename_removes_the_temp_and_reports_the_error() { + let dir = tempfile::TempDir::new().expect("tempdir"); + // A directory in the target's place fails the rename after the + // temp is fully written, exercising the cleanup path. + let target = dir.path().join("gateway.json"); + fs::create_dir(&target).expect("directory in the target's place"); + write_atomic_owner_only(&target, b"payload") + .expect_err("renaming over a directory must fail"); + assert_eq!(dir_names(dir.path()), ["gateway.json"]); + assert!(target.is_dir(), "the failed write touched nothing"); + } + + #[cfg(unix)] + #[test] + fn the_written_file_is_owner_only() { + use std::os::unix::fs::PermissionsExt as _; + let dir = tempfile::TempDir::new().expect("tempdir"); + let target = dir.path().join("gateway.json"); + write_atomic_owner_only(&target, b"payload").expect("the write succeeds"); + let mode = target.metadata().expect("metadata").permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "the bearer key file is owner-only"); + } +} diff --git a/crates/workshop/gateway-discovery/src/cancellation.rs b/crates/workshop/gateway-discovery/src/cancellation.rs new file mode 100644 index 000000000..5879303d5 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/cancellation.rs @@ -0,0 +1,149 @@ +//! Cooperative cancellation for synchronous sidecar work. + +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Condvar, Mutex, PoisonError}; +use std::time::Duration; + +/// A clonable cancellation signal for finite sidecar operations. +/// +/// Cancellation wakes retry waits immediately. [`run_if_active`](Self::run_if_active) +/// also provides a linearization point for effects such as process launch and +/// authoritative publication: cancellation first wakes a cancellable effect +/// already in progress, then waits for it, and no effect starts after +/// cancellation returns. +#[derive(Clone, Debug, Default)] +pub struct CancellationToken { + state: Arc, +} + +#[derive(Debug, Default)] +struct CancellationState { + cancelled: AtomicBool, + effect: Mutex<()>, + waiter: Mutex<()>, + wake: Condvar, +} + +impl CancellationToken { + /// Creates an active cancellation token. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Cancels the token and wakes every retry wait. + pub fn cancel(&self) { + let waiter = self + .state + .waiter + .lock() + .unwrap_or_else(PoisonError::into_inner); + self.state.cancelled.store(true, Ordering::SeqCst); + self.state.wake.notify_all(); + drop(waiter); + + // Wait for an operation that already crossed the effect gate. The + // cancellation flag and wake happen first, so a cancellable operation + // inside the gate can finish instead of deadlocking with `cancel`. + drop( + self.state + .effect + .lock() + .unwrap_or_else(PoisonError::into_inner), + ); + } + + /// Whether cancellation has been requested. + #[must_use] + pub fn is_cancelled(&self) -> bool { + self.state.cancelled.load(Ordering::SeqCst) + } + + /// Waits for cancellation or until `timeout` elapses. + /// + /// Returns `true` when the token is cancelled. + #[must_use] + pub fn wait_timeout(&self, timeout: Duration) -> bool { + if self.is_cancelled() { + return true; + } + let waiter = self + .state + .waiter + .lock() + .unwrap_or_else(PoisonError::into_inner); + if self.is_cancelled() { + return true; + } + let (_waiter, _) = self + .state + .wake + .wait_timeout_while(waiter, timeout, |()| !self.is_cancelled()) + .unwrap_or_else(PoisonError::into_inner); + self.is_cancelled() + } + + /// Runs `operation` only while the token remains active. + /// + /// Cancellation and effect admission are mutually exclusive. Cancellation + /// is signalled before waiting for an admitted operation, so that operation + /// may call [`wait_timeout`](Self::wait_timeout) and stop within its bound. + /// The gate is not reentrant: `operation` must not call `cancel` or + /// `run_if_active` on this token. + pub fn run_if_active(&self, operation: impl FnOnce() -> T) -> Option { + let effect = self + .state + .effect + .lock() + .unwrap_or_else(PoisonError::into_inner); + if self.is_cancelled() { + return None; + } + let result = operation(); + drop(effect); + Some(result) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::mpsc; + use std::time::Instant; + + #[test] + fn cancellation_wakes_an_operation_inside_the_effect_gate() { + let cancellation = CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let effects = Arc::new(AtomicUsize::new(0)); + let worker_effects = Arc::clone(&effects); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + worker_cancellation.run_if_active(|| { + entered.send(()).expect("announce the gated operation"); + if !worker_cancellation.wait_timeout(Duration::from_secs(1)) { + worker_effects.fetch_add(1, Ordering::SeqCst); + } + }); + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("the operation enters the effect gate"); + + let started = Instant::now(); + cancellation.cancel(); + worker.join().expect("the cancelled operation joins"); + + assert!( + started.elapsed() < Duration::from_millis(250), + "cancellation wakes an operation already inside the effect gate" + ); + assert_eq!( + effects.load(Ordering::SeqCst), + 0, + "the cancelled operation performs no later effect" + ); + } +} diff --git a/crates/workshop/gateway-discovery/src/error.rs b/crates/workshop/gateway-discovery/src/error.rs new file mode 100644 index 000000000..c87bb990f --- /dev/null +++ b/crates/workshop/gateway-discovery/src/error.rs @@ -0,0 +1,133 @@ +//! The gateway-discovery-file and launch-lock error type. +//! +//! [`SidecarError`] is what the file and lock operations return; the health +//! probe has its own [`crate::HealthError`]. + +use std::path::PathBuf; +use std::time::Duration; + +// The JSON cause behind a `SidecarError` variant. A caller that needs the +// JSON error itself names `shared_error_source` directly; this crate does not +// re-export the wrapper, so there is one name for the cause across the +// workspace rather than one per crate. +use shared_error_source::JsonSource; + +/// A failure of a gateway-discovery-file or launch-lock operation. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum SidecarError { + /// The caller cancelled the sidecar operation. + #[error("the sidecar operation was cancelled")] + Cancelled, + + /// The run directory could not be created. + #[error("create the run directory {path}")] + CreateDir { + /// The directory that could not be created. + path: PathBuf, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The gateway discovery file existed but could not be read. + #[error("read {path}")] + Read { + /// The file that could not be read. + path: PathBuf, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The gateway discovery file was not valid JSON. + #[error("parse {path}")] + Parse { + /// The file that could not be parsed. + path: PathBuf, + /// The underlying JSON error. + #[source] + source: JsonSource, + }, + + /// The gateway discovery file failed validation. + #[error("invalid gateway discovery file {path}: {reason}")] + Invalid { + /// The file that failed validation. + path: PathBuf, + /// The broken invariant. + reason: String, + }, + + /// The gateway discovery file could not be serialized for writing. + #[cfg(feature = "test-fixtures")] + #[error("serialize the gateway discovery file")] + Serialize { + /// The underlying JSON error. + #[source] + source: JsonSource, + }, + + /// The atomic write of the gateway discovery file failed. + #[cfg(feature = "test-fixtures")] + #[error("write {path}")] + Write { + /// The file that could not be written. + path: PathBuf, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The gateway discovery file could not be removed. + #[error("remove {path}")] + Remove { + /// The file that could not be removed. + path: PathBuf, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The launch lock could not be opened or taken. + #[error("lock {path}")] + Lock { + /// The lock file that failed. + path: PathBuf, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The launch race produced no attachable gateway within the budget. + #[error("no launch winner became attachable within {timeout:?}")] + LaunchTimeout { + /// The budget that elapsed. + timeout: Duration, + }, +} + +#[cfg(test)] +mod tests { + use super::{JsonSource, SidecarError}; + use std::error::Error as _; + use std::path::PathBuf; + + #[test] + fn the_parse_variant_reaches_the_json_error_through_the_shared_wrapper() { + let Err(json) = serde_json::from_str::("nope") else { + panic!("`nope` must not parse as a u32"); + }; + let error = SidecarError::Parse { + path: PathBuf::from("gateway.json"), + source: json.into(), + }; + let Some(cause) = error.source() else { + panic!("the parse variant returns its JSON cause from source()"); + }; + let Some(wrapper) = cause.downcast_ref::() else { + panic!("the JSON cause is the shared JsonSource"); + }; + assert!(wrapper.as_inner().is_syntax()); + } +} diff --git a/crates/workshop/gateway-discovery/src/file-tests.rs b/crates/workshop/gateway-discovery/src/file-tests.rs new file mode 100644 index 000000000..e58efa520 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/file-tests.rs @@ -0,0 +1,238 @@ +//! The `gateway.json` reader and its golden fixtures: the six fields, the +//! tolerated unknown field, the boot identity, and validation. + +use super::*; + +/// A canonical valid file, matching `fixtures/gateway.json`. +fn valid_file() -> GatewayDiscoveryFile { + GatewayDiscoveryFile { + port: 8081, + api_key: "0e1f8f0d8f8c4e0f9a1c2b3d4e5f6a7b".to_owned(), + pid: 4242, + epoch: 1_757_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-09-03T12:00:00Z".to_owned(), + } +} + +/// One committed contract fixture's bytes. +fn fixture(name: &str) -> String { + let path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("fixtures") + .join(name); + std::fs::read_to_string(&path) + .unwrap_or_else(|error| panic!("read the contract fixture {}: {error}", path.display())) +} + +/// Writes one fixture into a fresh run directory and parses it back. +fn parse_fixture(name: &str) -> Result, SidecarError> { + let dir = tempfile::TempDir::new().expect("tempdir"); + fs::write( + crate::paths::gateway_discovery_file_path(dir.path()), + fixture(name), + ) + .expect("write the fixture"); + GatewayDiscoveryFile::read(dir.path()) +} + +#[test] +fn the_golden_fixture_parses_to_the_canonical_file() { + let parsed = parse_fixture("gateway.json").expect("the golden fixture is valid"); + assert_eq!(parsed, Some(valid_file())); +} + +#[test] +#[cfg(feature = "test-fixtures")] +fn the_writer_reproduces_the_golden_fixture_byte_for_byte() { + // The committed bytes are the drift alarm for the serialization: a + // change to the field order, the pretty-printing, or a field name + // shows up here rather than in a Gateway that no longer attaches. The + // writer emits no trailing newline and the committed file carries one + // because a text file should end in a newline, so the comparison is + // against the fixture without it. + let dir = tempfile::TempDir::new().expect("tempdir"); + valid_file().write_to(dir.path()).expect("write"); + let written = + fs::read_to_string(crate::paths::gateway_discovery_file_path(dir.path())).expect("read"); + let golden = fixture("gateway.json"); + assert_eq!( + written, + golden.trim_end_matches('\n'), + "the writer's bytes are the committed contract bytes" + ); +} + +#[test] +fn an_unknown_field_is_tolerated() { + let parsed = parse_fixture("gateway-unknown-field.json") + .expect("a newer Gateway's extra field must not break this reader"); + assert_eq!( + parsed, + Some(valid_file()), + "the known fields read unchanged" + ); +} + +#[test] +fn a_file_without_a_boot_identity_parses_but_carries_none() { + let parsed = parse_fixture("gateway-no-boot-identity.json") + .expect("valid fields parse") + .expect("the file exists"); + assert!( + !parsed.has_boot_identity(), + "a zero epoch and a blank start time is no boot identity" + ); +} + +#[test] +fn a_zero_port_is_a_validation_error() { + let error = + parse_fixture("gateway-invalid-port.json").expect_err("a zero port names no endpoint"); + let SidecarError::Invalid { reason, .. } = &error else { + panic!("a zero port is a validation error, got {error}"); + }; + assert_eq!(reason, "port must not be 0"); +} + +#[test] +fn the_field_names_and_types_are_the_contract() { + // Pins the wire names and types independently of the fixture file, so + // a rename or a type change fails here even if nobody updates the + // fixture. The future OpenAPI spec describes the same two fields for + // the admin status route; this is the discovery file's six. + let value = serde_json::to_value(valid_file()).expect("the file is representable"); + let object = value.as_object().expect("the file is a JSON object"); + let mut names: Vec<&str> = object.keys().map(String::as_str).collect(); + names.sort_unstable(); + assert_eq!( + names, + ["api_key", "epoch", "pid", "port", "started_at", "version"], + "the contract names exactly these six fields" + ); + assert!(object["port"].is_u64(), "port is an integer"); + assert!(object["api_key"].is_string(), "api_key is a string"); + assert!(object["pid"].is_u64(), "pid is an integer"); + assert!(object["epoch"].is_u64(), "epoch is an integer"); + assert!(object["version"].is_string(), "version is a string"); + assert!(object["started_at"].is_string(), "started_at is a string"); +} + +#[test] +fn validation_rejects_the_three_attach_fields() { + let mut file = valid_file(); + assert_eq!(file.validation_error(), None); + file.port = 0; + assert_eq!(file.validation_error(), Some("port must not be 0")); + file = valid_file(); + file.api_key.clear(); + assert_eq!(file.validation_error(), Some("api_key must not be empty")); + file = valid_file(); + file.api_key = "key\r\ninjected: value".to_owned(); + assert_eq!( + file.validation_error(), + Some("api_key must not contain line breaks") + ); + file = valid_file(); + file.pid = 0; + assert_eq!(file.validation_error(), Some("pid must not be 0")); +} + +#[test] +fn debug_redacts_bearer_and_untrusted_metadata() { + let secret = "capability-secret"; + let mut file = valid_file(); + file.api_key = secret.to_owned(); + file.version = format!("version-{secret}\r\n"); + file.started_at = format!("started-{secret}\t"); + + let debug = format!("{file:?}"); + assert!(!debug.contains(secret), "debug output redacts the bearer"); + assert!( + !debug.contains(['\r', '\n', '\t']), + "untrusted metadata cannot inject debug output" + ); + assert!( + debug.contains(&file.port.to_string()), + "the endpoint remains visible" + ); +} + +#[test] +fn read_returns_none_when_no_file_exists() { + let dir = tempfile::TempDir::new().expect("tempdir"); + assert_eq!(GatewayDiscoveryFile::read(dir.path()).expect("read"), None); +} + +#[test] +fn read_rejects_corrupt_json() { + let dir = tempfile::TempDir::new().expect("tempdir"); + fs::write( + crate::paths::gateway_discovery_file_path(dir.path()), + b"not json", + ) + .expect("write fixture"); + let error = GatewayDiscoveryFile::read(dir.path()).expect_err("corrupt JSON must fail"); + assert!( + matches!(error, SidecarError::Parse { .. }), + "a corrupt file is a parse error: {error}" + ); +} + +#[test] +fn read_rejects_an_unreadable_path() { + let dir = tempfile::TempDir::new().expect("tempdir"); + fs::create_dir(crate::paths::gateway_discovery_file_path(dir.path())) + .expect("a directory in the file's place"); + let error = GatewayDiscoveryFile::read(dir.path()).expect_err("a directory is not a file"); + assert!( + matches!(error, SidecarError::Read { .. }), + "an unreadable path is a read error, not staleness: {error}" + ); +} + +#[test] +#[cfg(feature = "test-fixtures")] +fn write_refuses_an_invalid_file() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let mut file = valid_file(); + file.api_key.clear(); + let error = file + .write_to(dir.path()) + .expect_err("an invalid file must not be written"); + assert!(matches!(error, SidecarError::Invalid { .. })); + assert!( + !crate::paths::gateway_discovery_file_path(dir.path()).exists(), + "nothing was written" + ); +} + +#[test] +#[cfg(feature = "test-fixtures")] +fn write_creates_a_missing_run_directory() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let run_dir = dir.path().join("nested").join("run"); + valid_file().write_to(&run_dir).expect("write"); + assert!(crate::paths::gateway_discovery_file_path(&run_dir).exists()); +} + +#[test] +#[cfg(feature = "test-fixtures")] +fn remove_if_mine_removes_only_the_owning_pid() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let file = valid_file(); + file.write_to(dir.path()).expect("write"); + let path = crate::paths::gateway_discovery_file_path(dir.path()); + + remove_if_mine(dir.path(), 9999).expect("a foreign pid is tolerated"); + assert!(path.exists(), "a foreign pid's removal spares the file"); + + remove_if_mine(dir.path(), file.pid).expect("the owning pid removes"); + assert!(!path.exists(), "the owning pid's removal deletes the file"); +} + +#[test] +#[cfg(feature = "test-fixtures")] +fn remove_if_mine_tolerates_a_missing_file() { + let dir = tempfile::TempDir::new().expect("tempdir"); + remove_if_mine(dir.path(), 4242).expect("a missing file is not an error"); +} diff --git a/crates/workshop/gateway-discovery/src/file.rs b/crates/workshop/gateway-discovery/src/file.rs new file mode 100644 index 000000000..d004b5f38 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/file.rs @@ -0,0 +1,184 @@ +//! The `gateway.json` type: what the Gateway writes after a successful bind +//! and what a reader validates before attaching. + +use std::fmt; +use std::fs; +use std::io; +use std::path::Path; + +use serde::{Deserialize, Serialize}; + +use crate::error::SidecarError; +use crate::paths::gateway_discovery_file_path; + +/// The on-disk gateway discovery file naming a running gateway's loopback +/// port and bearer key. +/// +/// Readers must tolerate unknown fields: a newer gateway may write fields +/// an older reader does not know, and serde ignores them. This is the six +/// fields the contract names; see `CONTRACT.md`. +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct GatewayDiscoveryFile { + /// The gateway's bound port; readers connect to `127.0.0.1:{port}`. + pub port: u16, + /// The bearer key the gateway expects. + pub api_key: String, + /// The gateway process id. + pub pid: u32, + /// The gateway's boot instant in unix seconds. + pub epoch: u64, + /// The gateway crate version. + pub version: String, + /// RFC 3339 rendering of the boot instant. + pub started_at: String, +} + +impl fmt::Debug for GatewayDiscoveryFile { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("GatewayDiscoveryFile") + .field("port", &self.port) + .field("api_key", &"[REDACTED]") + .field("pid", &self.pid) + .field("epoch", &self.epoch) + .field("version", &"[REDACTED]") + .field("started_at", &"[REDACTED]") + .finish() + } +} + +impl GatewayDiscoveryFile { + /// Whether the connection names one particular Gateway boot. + pub(crate) fn has_boot_identity(&self) -> bool { + self.epoch != 0 && !self.started_at.trim().is_empty() + } + + /// The reason the file fails validation, or `None` when it is valid. + /// + /// Validation covers the fields attach depends on: a real port, a + /// real key, a real pid. The remaining fields are informational. + #[must_use] + pub fn validation_error(&self) -> Option<&'static str> { + if self.port == 0 { + return Some("port must not be 0"); + } + if self.api_key.is_empty() { + return Some("api_key must not be empty"); + } + if self.api_key.contains(['\r', '\n']) { + return Some("api_key must not contain line breaks"); + } + if self.pid == 0 { + return Some("pid must not be 0"); + } + None + } + + /// Reads and validates the gateway discovery file in `run_dir`, + /// returning `None` when no file exists. + /// + /// # Errors + /// Returns [`SidecarError::Read`] when the file exists but cannot be + /// read, [`SidecarError::Parse`] when it is not valid JSON, and + /// [`SidecarError::Invalid`] when it fails validation. + pub fn read(run_dir: &Path) -> Result, SidecarError> { + let path = gateway_discovery_file_path(run_dir); + let raw = match fs::read_to_string(&path) { + Ok(raw) => raw, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => { + return Err(SidecarError::Read { + path, + source: error, + }); + } + }; + let file: GatewayDiscoveryFile = + serde_json::from_str(&raw).map_err(|source| SidecarError::Parse { + path: path.clone(), + source: source.into(), + })?; + if let Some(reason) = file.validation_error() { + return Err(SidecarError::Invalid { + path, + reason: reason.to_owned(), + }); + } + Ok(Some(file)) + } +} + +/// The writer half of the contract, compiled only for Workshop's test +/// doubles. The Gateway owns the production writer. +#[cfg(feature = "test-fixtures")] +impl GatewayDiscoveryFile { + /// Writes the gateway discovery file into `run_dir`, creating the + /// directory when missing. The write is atomic (temp sibling, sync, + /// rename) and the file lands owner-only; see `CONTRACT.md` for the + /// permission contract. + /// + /// # Errors + /// Returns [`SidecarError::Invalid`] when the file fails validation, + /// [`SidecarError::CreateDir`] when the run directory cannot be + /// created, [`SidecarError::Serialize`] when the file cannot be + /// serialized, and [`SidecarError::Write`] when the atomic write + /// fails. + pub fn write_to(&self, run_dir: &Path) -> Result<(), SidecarError> { + let path = gateway_discovery_file_path(run_dir); + if let Some(reason) = self.validation_error() { + return Err(SidecarError::Invalid { + path, + reason: reason.to_owned(), + }); + } + fs::create_dir_all(run_dir).map_err(|source| SidecarError::CreateDir { + path: run_dir.to_owned(), + source, + })?; + let bytes = serde_json::to_vec_pretty(self).map_err(|source| SidecarError::Serialize { + source: source.into(), + })?; + crate::atomic::write_atomic_owner_only(&path, &bytes) + .map_err(|source| SidecarError::Write { path, source }) + } +} + +/// Removes the gateway discovery file in `run_dir` when it still belongs to +/// process `pid`, so a shutting-down writer withdraws itself from discovery +/// but can never delete a replacement's file. A missing file is not an +/// error; an unreadable or foreign file is left alone for the next +/// [`crate::resolve`] to clean. +/// +/// A test-double seam: the Gateway's own writer removes its file when it +/// exits, and Workshop's doubles need the same withdrawal. +/// +/// # Errors +/// Returns [`SidecarError::Remove`] when the file is this process's own +/// but cannot be removed. +#[cfg(feature = "test-fixtures")] +pub fn remove_if_mine(run_dir: &Path, pid: u32) -> Result<(), SidecarError> { + let path = gateway_discovery_file_path(run_dir); + match fs::read_to_string(&path) { + Ok(raw) => { + let owned = serde_json::from_str::(&raw) + .is_ok_and(|file| file.pid == pid); + if !owned { + return Ok(()); + } + } + // A missing or unreadable file is left alone. + Err(_) => return Ok(()), + } + match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(SidecarError::Remove { + path, + source: error, + }), + } +} + +#[cfg(test)] +#[path = "file-tests.rs"] +mod tests; diff --git a/crates/workshop/gateway-discovery/src/health.rs b/crates/workshop/gateway-discovery/src/health.rs new file mode 100644 index 000000000..27e8d59fb --- /dev/null +++ b/crates/workshop/gateway-discovery/src/health.rs @@ -0,0 +1,335 @@ +//! Readiness and key probes: raw loopback HTTP/1.x over +//! `std::net::TcpStream`, enough to read a status line, with no HTTP +//! client dependency. +//! +//! The `Host` header is the bound loopback address, matching the gateway's +//! loopback `Host` allowlist. + +use std::io::{Read, Write as _}; +use std::net::{SocketAddr, TcpStream}; +use std::time::{Duration, Instant}; + +use crate::CancellationToken; + +mod connection; + +#[cfg(test)] +use connection::probe_connection; +pub(crate) use connection::{probe_connection_cancellable, probe_connection_until}; + +/// Delay between probes while the server comes up. +const RETRY_INTERVAL: Duration = Duration::from_millis(25); + +/// Per-attempt connect and read timeout, so one hung attempt cannot eat +/// the whole budget. +const ATTEMPT_TIMEOUT: Duration = Duration::from_secs(2); + +/// Per-attempt I/O budget while a supervisor may be cancelled. +const CANCELLABLE_ATTEMPT_TIMEOUT: Duration = Duration::from_millis(100); +/// Maximum accepted response head for the two-request validation proof. +const RESPONSE_HEAD_LIMIT: usize = 16 * 1024; +/// Maximum body drained between the health and bearer responses. +const RESPONSE_BODY_LIMIT: usize = 64 * 1024; + +/// A failure of [`wait_for_health`]. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum HealthError { + /// The caller cancelled the health wait. + #[error("the health wait was cancelled")] + Cancelled, + + /// The URL is not an `http://` URL. + #[error("health probe needs an http:// URL, got {url}")] + NotHttp { + /// The offending URL. + url: String, + }, + + /// No 200 answer arrived within the budget. + #[error("the server did not answer {url}/health within {timeout:?}")] + Timeout { + /// The probed base URL. + url: String, + /// The budget that elapsed. + timeout: Duration, + /// The last attempt's failure. + #[source] + source: ProbeError, + }, +} + +/// One probe attempt failed. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum ProbeError { + /// The connect, write, or read failed. + #[error("{operation}")] + Io { + /// The failed operation. + operation: &'static str, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The status line was not a 200. + #[error("unexpected health response: {status_line}")] + UnexpectedStatus { + /// The response's first line. + status_line: String, + }, +} + +/// The outcome of one bearer-key probe. +#[cfg(test)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum KeyProbe { + /// A 2xx answer: the key is accepted. + Accepted, + /// A non-2xx answer: the key is rejected (or the route is gone). + Rejected, + /// The server could not be queried at all. + Unreachable, +} + +/// The combined readiness and authority proof for one connection. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ConnectionProbe { + /// Health answered and the bearer was accepted. + Accepted, + /// The caller cancelled validation. + Cancelled, + /// Health failed, including a bearer probe that became unreachable. + HealthFailed, + /// Health answered but the bearer was rejected. + KeyRejected, +} + +/// Connects within the one absolute attempt deadline. +fn connect_until(address: &SocketAddr, deadline: Instant) -> Result { + let timeout = remaining(deadline)?; + TcpStream::connect_timeout(address, timeout).map_err(|source| ProbeError::Io { + operation: "connect", + source, + }) +} + +/// Applies the remaining absolute attempt budget to reads and writes. +fn configure_stream(stream: &TcpStream, deadline: Instant) -> Result<(), ProbeError> { + let timeout = remaining(deadline)?; + stream + .set_read_timeout(Some(timeout)) + .map_err(|source| ProbeError::Io { + operation: "configure the read timeout", + source, + })?; + stream + .set_write_timeout(Some(timeout)) + .map_err(|source| ProbeError::Io { + operation: "configure the write timeout", + source, + }) +} + +fn remaining(deadline: Instant) -> Result { + deadline + .checked_duration_since(Instant::now()) + .filter(|remaining| !remaining.is_zero()) + .ok_or_else(|| ProbeError::Io { + operation: "validation deadline elapsed", + source: std::io::Error::from(std::io::ErrorKind::TimedOut), + }) +} + +/// Polls `GET {base_url}/health` until it answers 200 or `timeout` +/// elapses. +/// +/// # Errors +/// Returns [`HealthError::NotHttp`] when `base_url` is not an +/// `http://host:port` URL, and [`HealthError::Timeout`] when the endpoint +/// does not answer 200 within `timeout`. +pub fn wait_for_health(base_url: &str, timeout: Duration) -> Result<(), HealthError> { + wait_for_health_cancellable_with( + base_url, + timeout, + &CancellationToken::new(), + ATTEMPT_TIMEOUT, + probe_health_until, + ) +} + +/// Polls `GET {base_url}/health` until it answers 200, the timeout elapses, +/// or the caller cancels the wait. +/// +/// # Errors +/// Returns [`HealthError::Cancelled`] when `cancellation` is signalled, plus +/// the URL and timeout failures documented by [`wait_for_health`]. +pub fn wait_for_health_cancellable( + base_url: &str, + timeout: Duration, + cancellation: &CancellationToken, +) -> Result<(), HealthError> { + wait_for_health_cancellable_with( + base_url, + timeout, + cancellation, + CANCELLABLE_ATTEMPT_TIMEOUT, + probe_health_until, + ) +} + +fn wait_for_health_cancellable_with( + base_url: &str, + timeout: Duration, + cancellation: &CancellationToken, + attempt_timeout: Duration, + mut probe: impl FnMut(&str, Instant) -> Result<(), ProbeError>, +) -> Result<(), HealthError> { + wait_for_health_cancellable_with_start( + base_url, + timeout, + cancellation, + attempt_timeout, + || {}, + |address, deadline| probe(address, deadline), + ) +} + +fn wait_for_health_cancellable_with_start( + base_url: &str, + timeout: Duration, + cancellation: &CancellationToken, + attempt_timeout: Duration, + mut before_probe: impl FnMut(), + mut probe: impl FnMut(&str, Instant) -> Result<(), ProbeError>, +) -> Result<(), HealthError> { + let address = base_url + .strip_prefix("http://") + .ok_or_else(|| HealthError::NotHttp { + url: base_url.to_owned(), + })?; + let deadline = Instant::now() + timeout; + loop { + let attempt_deadline = (Instant::now() + attempt_timeout).min(deadline); + before_probe(); + let Some(attempt) = cancellation.run_if_active(|| probe(address, attempt_deadline)) else { + return Err(HealthError::Cancelled); + }; + match attempt { + Ok(()) => return Ok(()), + Err(error) => { + if cancellation.is_cancelled() { + return Err(HealthError::Cancelled); + } + if Instant::now() >= deadline { + return Err(HealthError::Timeout { + url: base_url.to_owned(), + timeout, + source: error, + }); + } + if cancellation.wait_timeout(RETRY_INTERVAL) { + return Err(HealthError::Cancelled); + } + } + } + } +} + +fn probe_health_until(address: &str, deadline: Instant) -> Result<(), ProbeError> { + let head = request_head_until(address, "GET", "/health", None, deadline)?; + let status_ok = head + .split_whitespace() + .nth(1) + .is_some_and(|code| code == "200"); + if status_ok { + return Ok(()); + } + Err(ProbeError::UnexpectedStatus { + status_line: head.lines().next().unwrap_or("").to_owned(), + }) +} + +/// Issues one `GET {path}` presenting `api_key` as the bearer token and +/// classifies the answer. +#[cfg(test)] +fn probe_bearer(address: &str, path: &str, api_key: &str) -> KeyProbe { + match request_head(address, "GET", path, Some(api_key)) { + Ok(head) => { + let accepted = head + .split_whitespace() + .nth(1) + .and_then(|code| code.parse::().ok()) + .is_some_and(|code| (200..300).contains(&code)); + if accepted { + KeyProbe::Accepted + } else { + KeyProbe::Rejected + } + } + Err(_) => KeyProbe::Unreachable, + } +} + +/// One HTTP/1.0 request returning the response head. The `Host` header +/// is set to the bound address, never `localhost`: the gateway's loopback +/// `Host` allowlist refuses anything else. +pub(crate) fn request_head( + address: &str, + method: &str, + path: &str, + bearer: Option<&str>, +) -> Result { + request_head_with_timeout(address, method, path, bearer, ATTEMPT_TIMEOUT) +} + +fn request_head_with_timeout( + address: &str, + method: &str, + path: &str, + bearer: Option<&str>, + timeout: Duration, +) -> Result { + request_head_until(address, method, path, bearer, Instant::now() + timeout) +} + +pub(crate) fn request_head_until( + address: &str, + method: &str, + path: &str, + bearer: Option<&str>, + deadline: Instant, +) -> Result { + let socket = address + .parse::() + .map_err(|source| ProbeError::Io { + operation: "parse the loopback address", + source: std::io::Error::new(std::io::ErrorKind::InvalidInput, source), + })?; + let mut stream = connect_until(&socket, deadline)?; + let mut request = format!("{method} {path} HTTP/1.0\r\nHost: {address}\r\n"); + if let Some(key) = bearer { + request.push_str("Authorization: Bearer "); + request.push_str(key); + request.push_str("\r\n"); + } + request.push_str("\r\n"); + configure_stream(&stream, deadline)?; + stream + .write_all(request.as_bytes()) + .map_err(|source| ProbeError::Io { + operation: "write the request", + source, + })?; + let mut buffer = [0u8; 256]; + configure_stream(&stream, deadline)?; + let read = stream.read(&mut buffer).map_err(|source| ProbeError::Io { + operation: "read the status line", + source, + })?; + Ok(String::from_utf8_lossy(&buffer[..read]).into_owned()) +} + +#[cfg(test)] +mod tests; diff --git a/crates/workshop/gateway-discovery/src/health/connection.rs b/crates/workshop/gateway-discovery/src/health/connection.rs new file mode 100644 index 000000000..335772b8f --- /dev/null +++ b/crates/workshop/gateway-discovery/src/health/connection.rs @@ -0,0 +1,282 @@ +//! The one-connection health and bearer proof. + +use std::io::{Read, Write as _}; +use std::net::{SocketAddr, TcpStream}; +use std::time::{Duration, Instant}; + +use super::{ + ATTEMPT_TIMEOUT, CANCELLABLE_ATTEMPT_TIMEOUT, ConnectionProbe, ProbeError, RESPONSE_BODY_LIMIT, + RESPONSE_HEAD_LIMIT, RETRY_INTERVAL, configure_stream, connect_until, +}; +use crate::CancellationToken; + +/// Proves that one address is healthy and accepts the presented bearer. +/// Both requests use one TCP connection, so authority cannot come from a +/// listener that replaced the endpoint after the health response. +#[cfg(test)] +pub(super) fn probe_connection( + address: &str, + bearer_path: &str, + bearer: &str, + health_budget: Duration, +) -> ConnectionProbe { + probe_connection_until(address, bearer_path, bearer, Instant::now() + health_budget) +} + +/// Proves one connection within the caller's absolute deadline. +pub(crate) fn probe_connection_until( + address: &str, + bearer_path: &str, + bearer: &str, + deadline: Instant, +) -> ConnectionProbe { + probe_connection_with( + address, + bearer_path, + bearer, + deadline, + &CancellationToken::new(), + ATTEMPT_TIMEOUT, + ) +} + +/// Proves one connection while observing supervisor cancellation. +pub(crate) fn probe_connection_cancellable( + address: &str, + bearer_path: &str, + bearer: &str, + health_budget: Duration, + cancellation: &CancellationToken, +) -> ConnectionProbe { + probe_connection_with( + address, + bearer_path, + bearer, + Instant::now() + health_budget, + cancellation, + CANCELLABLE_ATTEMPT_TIMEOUT, + ) +} + +fn probe_connection_with( + address: &str, + bearer_path: &str, + bearer: &str, + deadline: Instant, + cancellation: &CancellationToken, + attempt_timeout: Duration, +) -> ConnectionProbe { + probe_connection_with_probe( + address, + bearer_path, + bearer, + deadline, + cancellation, + attempt_timeout, + probe_connection_once, + ) +} + +pub(super) fn probe_connection_with_probe( + address: &str, + bearer_path: &str, + bearer: &str, + deadline: Instant, + cancellation: &CancellationToken, + attempt_timeout: Duration, + mut probe: impl FnMut(&str, &str, &str, Instant) -> ConnectionAttempt, +) -> ConnectionProbe { + loop { + let attempt_deadline = (Instant::now() + attempt_timeout).min(deadline); + let Some(attempt) = + cancellation.run_if_active(|| probe(address, bearer_path, bearer, attempt_deadline)) + else { + return ConnectionProbe::Cancelled; + }; + match attempt { + ConnectionAttempt::Accepted => return ConnectionProbe::Accepted, + ConnectionAttempt::KeyRejected => return ConnectionProbe::KeyRejected, + ConnectionAttempt::ProofInterrupted => return ConnectionProbe::HealthFailed, + ConnectionAttempt::HealthFailed if Instant::now() >= deadline => { + return ConnectionProbe::HealthFailed; + } + ConnectionAttempt::HealthFailed if cancellation.wait_timeout(RETRY_INTERVAL) => { + return ConnectionProbe::Cancelled; + } + ConnectionAttempt::HealthFailed => {} + } + } +} + +/// One coherent proof attempt over one TCP connection. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum ConnectionAttempt { + Accepted, + HealthFailed, + KeyRejected, + ProofInterrupted, +} + +/// Checks health and bearer acceptance over one socket. Once health has +/// succeeded, any socket loss fails the proof instead of reconnecting to a +/// potentially different endpoint. +pub(super) fn probe_connection_once( + address: &str, + bearer_path: &str, + bearer: &str, + deadline: Instant, +) -> ConnectionAttempt { + let Ok(socket) = address.parse::() else { + return ConnectionAttempt::HealthFailed; + }; + let Ok(mut stream) = connect_until(&socket, deadline) else { + return ConnectionAttempt::HealthFailed; + }; + if write_request(&mut stream, address, "/health", None, false, deadline).is_err() { + return ConnectionAttempt::HealthFailed; + } + let Ok(health_head) = read_framed_response_head(&mut stream, deadline) else { + return ConnectionAttempt::HealthFailed; + }; + if response_status(&health_head) != Some(200) { + return ConnectionAttempt::HealthFailed; + } + if write_request( + &mut stream, + address, + bearer_path, + Some(bearer), + true, + deadline, + ) + .is_err() + { + return ConnectionAttempt::ProofInterrupted; + } + let Ok(bearer_head) = read_framed_response_head(&mut stream, deadline) else { + return ConnectionAttempt::ProofInterrupted; + }; + if response_status(&bearer_head).is_some_and(|code| (200..300).contains(&code)) { + ConnectionAttempt::Accepted + } else { + ConnectionAttempt::KeyRejected + } +} + +/// Writes one GET request, retaining or closing the connection as directed. +fn write_request( + stream: &mut TcpStream, + address: &str, + path: &str, + bearer: Option<&str>, + close: bool, + deadline: Instant, +) -> Result<(), ProbeError> { + let connection = if close { "close" } else { "keep-alive" }; + let mut request = + format!("GET {path} HTTP/1.1\r\nHost: {address}\r\nConnection: {connection}\r\n"); + if let Some(key) = bearer { + request.push_str("Authorization: Bearer "); + request.push_str(key); + request.push_str("\r\n"); + } + request.push_str("\r\n"); + configure_stream(stream, deadline)?; + stream + .write_all(request.as_bytes()) + .map_err(|source| ProbeError::Io { + operation: "write the validation request", + source, + }) +} + +/// Reads one response head and drains its fixed-length body so the next +/// response starts at a framing boundary on the same socket. +fn read_framed_response_head( + stream: &mut TcpStream, + deadline: Instant, +) -> Result { + let mut response = Vec::with_capacity(512); + let header_end = loop { + if let Some(end) = response.windows(4).position(|bytes| bytes == b"\r\n\r\n") { + break end + 4; + } + if response.len() >= RESPONSE_HEAD_LIMIT { + return Err(ProbeError::UnexpectedStatus { + status_line: "".to_owned(), + }); + } + let mut buffer = [0_u8; 512]; + let available = RESPONSE_HEAD_LIMIT - response.len(); + let chunk = available.min(buffer.len()); + configure_stream(stream, deadline)?; + let read = stream + .read(&mut buffer[..chunk]) + .map_err(|source| ProbeError::Io { + operation: "read the validation response", + source, + })?; + if read == 0 { + return Err(ProbeError::Io { + operation: "read the validation response", + source: std::io::Error::from(std::io::ErrorKind::UnexpectedEof), + }); + } + response.extend_from_slice(&buffer[..read]); + }; + let head = String::from_utf8_lossy(&response[..header_end]).into_owned(); + let content_length = + response_content_length(&head).ok_or_else(|| ProbeError::UnexpectedStatus { + status_line: "".to_owned(), + })?; + if content_length > RESPONSE_BODY_LIMIT { + return Err(ProbeError::UnexpectedStatus { + status_line: "".to_owned(), + }); + } + let body_already_read = response.len() - header_end; + if body_already_read > content_length { + return Err(ProbeError::UnexpectedStatus { + status_line: "".to_owned(), + }); + } + if body_already_read < content_length { + let mut remaining = content_length - body_already_read; + let mut buffer = [0_u8; 512]; + while remaining > 0 { + let chunk_len = remaining.min(buffer.len()); + configure_stream(stream, deadline)?; + let read = stream + .read(&mut buffer[..chunk_len]) + .map_err(|source| ProbeError::Io { + operation: "read the validation response body", + source, + })?; + if read == 0 { + return Err(ProbeError::Io { + operation: "read the validation response body", + source: std::io::Error::from(std::io::ErrorKind::UnexpectedEof), + }); + } + remaining -= read; + } + } + Ok(head) +} + +/// Parses a decimal Content-Length from a response head. +fn response_content_length(head: &str) -> Option { + head.lines().find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse().ok()) + .flatten() + }) +} + +/// Parses the three-digit response status. +fn response_status(head: &str) -> Option { + head.split_whitespace() + .nth(1) + .and_then(|code| code.parse::().ok()) +} diff --git a/crates/workshop/gateway-discovery/src/health/tests-connection.rs b/crates/workshop/gateway-discovery/src/health/tests-connection.rs new file mode 100644 index 000000000..5898742e3 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/health/tests-connection.rs @@ -0,0 +1,301 @@ +//! Unit tests for the one-connection health and bearer proof. + +use super::*; + +use super::super::connection::{ + ConnectionAttempt, probe_connection_once, probe_connection_with_probe, +}; + +#[test] +fn connection_proof_cannot_mix_health_and_bearer_across_endpoints() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind replacement fixture"); + let address = listener + .local_addr() + .expect("replacement fixture address") + .to_string(); + let (accepted, received) = mpsc::channel(); + std::thread::spawn(move || { + let (mut health, _) = listener.accept().expect("accept health probe"); + let mut buffer = [0_u8; 1024]; + let read = health.read(&mut buffer).expect("read health probe"); + assert!( + String::from_utf8_lossy(&buffer[..read]).starts_with("GET /health "), + "the first endpoint receives health" + ); + health + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .expect("answer health"); + drop(health); + + listener + .set_nonblocking(true) + .expect("make replacement observation bounded"); + let deadline = Instant::now() + Duration::from_millis(500); + let mut connection_count = 1; + while Instant::now() < deadline { + match listener.accept() { + Ok((mut bearer, _)) => { + connection_count += 1; + let read = bearer.read(&mut buffer).expect("read bearer probe"); + assert!( + String::from_utf8_lossy(&buffer[..read]) + .contains("Authorization: Bearer accepted\r\n"), + "the replacement endpoint accepts the bearer" + ); + bearer + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .expect("answer bearer"); + break; + } + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + std::thread::sleep(Duration::from_millis(5)); + } + Err(error) => panic!("observe replacement connection: {error}"), + } + } + accepted + .send(connection_count) + .expect("report accepted connections"); + }); + + assert_eq!( + probe_connection( + &address, + "/v1/models", + "accepted", + Duration::from_millis(250) + ), + ConnectionProbe::HealthFailed, + "one capability cannot combine health from one socket with authority from another" + ); + assert_eq!( + received + .recv_timeout(Duration::from_secs(2)) + .expect("fixture reports its connection count"), + 1, + "validation never reconnects after health succeeds" + ); +} + +#[test] +fn a_real_connect_attempt_obeys_one_absolute_deadline() { + let started = Instant::now(); + let result = probe_connection_once( + "192.0.2.1:9", + "/v1/models", + "key", + Instant::now() + Duration::from_millis(100), + ); + + assert_eq!(result, ConnectionAttempt::HealthFailed); + assert!( + started.elapsed() < Duration::from_millis(500), + "connect_timeout bounds a stalled or unreachable route" + ); +} + +#[test] +fn slow_drip_response_head_cannot_refresh_the_attempt_budget() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind slow-head fixture"); + let address = listener + .local_addr() + .expect("slow-head address") + .to_string(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept slow-head probe"); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + for byte in b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}" { + if stream.write_all(&[*byte]).is_err() { + return; + } + std::thread::sleep(Duration::from_millis(20)); + } + }); + + let started = Instant::now(); + let result = probe_connection_once( + &address, + "/v1/models", + "key", + Instant::now() + CANCELLABLE_ATTEMPT_TIMEOUT, + ); + + assert_eq!(result, ConnectionAttempt::HealthFailed); + assert!( + started.elapsed() < Duration::from_millis(500), + "one absolute deadline bounds a slow-drip response head" + ); +} + +#[test] +fn slow_drip_response_body_cannot_refresh_the_attempt_budget() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind slow-body fixture"); + let address = listener + .local_addr() + .expect("slow-body address") + .to_string(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept slow-body probe"); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + if stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 16\r\n\r\n") + .is_err() + { + return; + } + for byte in b"0123456789abcdef" { + if stream.write_all(&[*byte]).is_err() { + return; + } + std::thread::sleep(Duration::from_millis(20)); + } + }); + + let started = Instant::now(); + let result = probe_connection_once( + &address, + "/v1/models", + "key", + Instant::now() + CANCELLABLE_ATTEMPT_TIMEOUT, + ); + + assert_eq!(result, ConnectionAttempt::HealthFailed); + assert!( + started.elapsed() < Duration::from_millis(500), + "one absolute deadline bounds a slow-drip response body" + ); +} + +#[test] +fn an_unbounded_declared_body_is_rejected_without_draining() { + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n", + RESPONSE_BODY_LIMIT + 1 + ); + let listener = TcpListener::bind("127.0.0.1:0").expect("bind oversized-body fixture"); + let address = listener + .local_addr() + .expect("oversized-body address") + .to_string(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept oversized-body probe"); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + let _ = stream.write_all(response.as_bytes()); + std::thread::sleep(Duration::from_secs(1)); + }); + + let started = Instant::now(); + let result = probe_connection_once( + &address, + "/v1/models", + "key", + Instant::now() + CANCELLABLE_ATTEMPT_TIMEOUT, + ); + + assert_eq!(result, ConnectionAttempt::HealthFailed); + assert!( + started.elapsed() < Duration::from_millis(250), + "an oversized declared body is rejected before its bytes arrive" + ); +} + +#[test] +fn cancellation_joins_a_real_slow_drip_probe_without_retrying() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind cancelled-drip fixture"); + let address = listener + .local_addr() + .expect("cancelled-drip address") + .to_string(); + let connections = Arc::new(AtomicUsize::new(0)); + let server_connections = Arc::clone(&connections); + let (entered, blocked) = mpsc::channel(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept cancelled-drip probe"); + server_connections.fetch_add(1, Ordering::SeqCst); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + entered.send(()).expect("announce real slow-drip probe"); + for byte in b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}" { + if stream.write_all(&[*byte]).is_err() { + return; + } + std::thread::sleep(Duration::from_millis(20)); + } + }); + let cancellation = CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let worker = std::thread::spawn(move || { + probe_connection_cancellable( + &address, + "/v1/models", + "key", + Duration::from_secs(30), + &worker_cancellation, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("the real probe begins its slow response"); + + let started = Instant::now(); + cancellation.cancel(); + let result = worker.join().expect("the real probe worker joins"); + + assert_eq!(result, ConnectionProbe::Cancelled); + assert!( + started.elapsed() < Duration::from_millis(500), + "the attempt deadline bounds joined cancellation during real I/O" + ); + assert_eq!( + connections.load(Ordering::SeqCst), + 1, + "cancellation starts no later network probe" + ); +} + +#[test] +fn cancellation_at_validation_probe_start_prevents_the_probe() { + let cancellation = CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let probe_cancellation = worker_cancellation.clone(); + let probes = Arc::new(AtomicUsize::new(0)); + let worker_probes = Arc::clone(&probes); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + probe_connection_with_probe( + "127.0.0.1:1", + "/v1/models", + "key", + Instant::now() + Duration::from_secs(30), + &worker_cancellation, + CANCELLABLE_ATTEMPT_TIMEOUT, + |_, _, _, _| { + entered + .send(()) + .expect("announce validation probe boundary"); + if probe_cancellation.wait_timeout(Duration::from_secs(30)) { + ConnectionAttempt::HealthFailed + } else { + worker_probes.fetch_add(1, Ordering::SeqCst); + ConnectionAttempt::Accepted + } + }, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("validation pauses immediately before probe admission"); + + cancellation.cancel(); + let result = worker.join().expect("validation worker joins"); + + assert_eq!(result, ConnectionProbe::Cancelled); + assert_eq!( + probes.load(Ordering::SeqCst), + 0, + "no validation probe starts after cancellation returns" + ); +} diff --git a/crates/workshop/gateway-discovery/src/health/tests.rs b/crates/workshop/gateway-discovery/src/health/tests.rs new file mode 100644 index 000000000..b4aede42f --- /dev/null +++ b/crates/workshop/gateway-discovery/src/health/tests.rs @@ -0,0 +1,220 @@ +//! Unit tests for the health wait and the bearer probe. + +use super::*; + +use std::net::TcpListener; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, mpsc}; + +#[path = "tests-connection.rs"] +mod connection; + +/// Answers every connection with a canned response until the test +/// stops it. +fn spawn_stub_server(response: &'static [u8]) -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind stub server"); + let address = listener.local_addr().expect("stub server address"); + std::thread::spawn(move || { + while let Ok((mut stream, _)) = listener.accept() { + let _ = stream.write_all(response); + } + }); + format!("http://{address}") +} + +#[test] +fn a_200_answer_satisfies_the_probe() { + let base_url = spawn_stub_server(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"); + wait_for_health(&base_url, Duration::from_secs(5)).expect("the stub answers 200"); +} + +#[test] +fn a_non_200_answer_is_retried_until_timeout() { + let base_url = + spawn_stub_server(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\n\r\n"); + let error = wait_for_health(&base_url, Duration::from_millis(150)) + .expect_err("a 503 never satisfies the probe"); + assert!( + matches!(error, HealthError::Timeout { .. }), + "a 503 past the budget is a timeout" + ); + let message = error.to_string(); + assert!( + message.contains("did not answer"), + "the error names the timeout: {message}" + ); +} + +#[test] +fn a_dead_port_times_out() { + // Port 1 is never listening, so every connect fails fast. + let error = wait_for_health("http://127.0.0.1:1", Duration::from_millis(150)) + .expect_err("a dead port never satisfies the probe"); + assert!( + matches!(error, HealthError::Timeout { .. }), + "a dead port past the budget is a timeout" + ); + let message = error.to_string(); + assert!( + message.contains("did not answer"), + "the error names the timeout: {message}" + ); +} + +#[test] +fn a_non_http_url_is_rejected() { + let error = wait_for_health("ftp://127.0.0.1:7910", Duration::from_millis(10)) + .expect_err("a non-http URL must fail"); + assert!( + matches!(error, HealthError::NotHttp { .. }), + "a non-http URL is rejected before any probe" + ); + assert!( + error.to_string().contains("http://"), + "the error names the scheme requirement: {error}" + ); +} + +#[test] +fn the_probe_sends_the_bound_address_as_host() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind recording server"); + let address = listener.local_addr().expect("recording server address"); + let (requests, received) = mpsc::channel(); + std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut buffer = [0u8; 1024]; + if let Ok(read) = stream.read(&mut buffer) { + let _ = requests.send(String::from_utf8_lossy(&buffer[..read]).into_owned()); + } + let _ = stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"); + } + }); + wait_for_health(&format!("http://{address}"), Duration::from_secs(5)) + .expect("the stub answers 200"); + let request = received + .recv_timeout(Duration::from_secs(5)) + .expect("the probe's request arrived"); + assert!( + request.contains(&format!("Host: {address}\r\n")), + "the Host header is the bound address, not localhost: {request:?}" + ); +} + +#[test] +fn a_bearer_probe_accepts_a_2xx_and_rejects_a_401() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind key fixture"); + let address = listener + .local_addr() + .expect("key fixture address") + .to_string(); + std::thread::spawn(move || { + while let Ok((mut stream, _)) = listener.accept() { + let mut buffer = [0u8; 1024]; + let Ok(read) = stream.read(&mut buffer) else { + continue; + }; + let request = String::from_utf8_lossy(&buffer[..read]); + let response = if request.contains("Authorization: Bearer right\r\n") { + &b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"[..] + } else { + &b"HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\n\r\n"[..] + }; + let _ = stream.write_all(response); + } + }); + assert_eq!( + probe_bearer(&address, "/v1/models", "right"), + KeyProbe::Accepted + ); + assert_eq!( + probe_bearer(&address, "/v1/models", "wrong"), + KeyProbe::Rejected + ); + assert_eq!( + probe_bearer("127.0.0.1:1", "/v1/models", "right"), + KeyProbe::Unreachable + ); +} + +#[test] +fn cancellation_at_health_probe_start_prevents_the_probe() { + let cancellation = CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let probes = Arc::new(AtomicUsize::new(0)); + let worker_probes = Arc::clone(&probes); + let (entered, blocked) = mpsc::channel(); + let (release, released) = mpsc::channel(); + let worker = std::thread::spawn(move || { + wait_for_health_cancellable_with_start( + "http://127.0.0.1:1", + Duration::from_secs(30), + &worker_cancellation, + CANCELLABLE_ATTEMPT_TIMEOUT, + || { + entered.send(()).expect("announce health probe boundary"); + released.recv().expect("release health probe boundary"); + }, + |_, _| { + worker_probes.fetch_add(1, Ordering::SeqCst); + Ok(()) + }, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("health wait pauses immediately before probe admission"); + + cancellation.cancel(); + release.send(()).expect("release health probe boundary"); + let result = worker.join().expect("health worker joins"); + + assert!(matches!(result, Err(HealthError::Cancelled))); + assert_eq!( + probes.load(Ordering::SeqCst), + 0, + "no health probe starts after cancellation returns" + ); +} + +#[test] +fn cancellation_joins_a_blocked_health_wait_without_another_probe() { + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let probes = Arc::new(AtomicUsize::new(0)); + let worker_probes = Arc::clone(&probes); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + wait_for_health_cancellable_with( + "http://127.0.0.1:1", + Duration::from_secs(30), + &worker_cancellation, + CANCELLABLE_ATTEMPT_TIMEOUT, + |_, _| { + worker_probes.fetch_add(1, Ordering::SeqCst); + entered.send(()).expect("announce blocked health probe"); + let _ = worker_cancellation.wait_timeout(Duration::from_secs(30)); + Err(ProbeError::UnexpectedStatus { + status_line: "blocked".to_owned(), + }) + }, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("the health phase blocks deterministically"); + + let started = Instant::now(); + cancellation.cancel(); + let result = worker.join().expect("the health worker joins"); + + assert!( + started.elapsed() < Duration::from_millis(250), + "cancellation bounds the blocked health wait" + ); + assert!(matches!(result, Err(HealthError::Cancelled))); + assert_eq!( + probes.load(Ordering::SeqCst), + 1, + "no probe starts after cancellation" + ); +} diff --git a/crates/workshop/gateway-discovery/src/lib.rs b/crates/workshop/gateway-discovery/src/lib.rs new file mode 100644 index 000000000..f55febb03 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/lib.rs @@ -0,0 +1,90 @@ +//! The Gateway discovery contract, as Workshop reads it. +//! +//! Workshop attaches to an already-running Gateway instead of starting a +//! second one. It does that by reading the `gateway.json` file the Gateway +//! writes into the run directory (`/.runvil/run`) after a successful +//! bind: the file holds the loopback port, the bearer key, the pid, the boot +//! epoch, the version, and the start time. This crate is the reader half of +//! that protocol - read, validate, resolve, settle a launch race, wait for +//! health, and ask the Gateway to exit - and it is synchronous and +//! runtime-agnostic: no tokio, axum, or reqwest. +//! +//! The contract itself is written down in `CONTRACT.md` at the crate root, +//! which is the authority for both halves. The Gateway's writer lives in the +//! Gateway family; this crate never names it. +//! +//! The flow: +//! +//! 1. A reader ([`resolve`]) attaches only when the file is live: the pid +//! is alive, one OS process boot with a `runvil-gateway` image brackets a +//! same-socket health and bearer proof, and the file includes a boot +//! identity. Anything else is stale and the file is deleted. +//! [`ValidatedConnection`] holds that point-in-time proof without +//! exposing a forgeable constructor. +//! 2. Launch races take [`launch_or_attach`]: the `gateway.json.lock` +//! advisory lock elects one parent launcher; losers attach to the winner. +//! 3. A reader holding a [`ValidatedConnection`] asks the Gateway to exit +//! with [`request_shutdown`], which posts its bearer key to +//! `POST /shutdown`. +//! +//! URLs normalize to a literal `127.0.0.1`, never `localhost`, and probes +//! send the bound address as the `Host` header, matching the Gateway's +//! loopback `Host` allowlist. +//! +//! # Writing the file +//! +//! Production Workshop never writes `gateway.json`: the Gateway owns that +//! half, and a second writer would let Workshop forge a live endpoint. +//! Workshop's test doubles do need to write one, so the write and remove +//! functions and the process-lifetime lease are compiled only under the +//! `test-fixtures` feature. +//! +//! ## Invariants +//! +//! - Tier: vocabulary; may depend on no other `workshop-*` crate. It needs +//! no async runtime, so Workshop can hold the contract without adopting +//! one here. `cargo test -p build-xtask` enforces the product and +//! container boundaries. +//! - The discovery file, the two lock file names, the stale rule, and the +//! three HTTP calls are exactly what `CONTRACT.md` says. A behavior change +//! here is a contract change, and `CONTRACT.md` changes with it. +//! - A bearer key is never written to logs or `Debug` output. Every error +//! path that could echo a rejected request's status line redacts it. +//! - Reading the file tolerates fields this crate does not know, so a newer +//! Gateway never breaks an older Workshop. +//! - No writer ships in a production build: the file is the Gateway's to +//! write. + +#[cfg(feature = "test-fixtures")] +mod atomic; +mod cancellation; +mod error; +mod file; +mod health; +mod lock; +mod paths; +mod shutdown; +mod stale; +mod sys; +mod validated; + +pub use crate::cancellation::CancellationToken; +pub use crate::error::SidecarError; +pub use crate::file::GatewayDiscoveryFile; +#[cfg(feature = "test-fixtures")] +pub use crate::file::remove_if_mine; +pub use crate::health::{HealthError, ProbeError, wait_for_health, wait_for_health_cancellable}; +#[cfg(feature = "test-fixtures")] +pub use crate::lock::GatewayInstanceLease; +pub use crate::lock::{LaunchDecision, LaunchLock, launch_or_attach, launch_or_attach_cancellable}; +pub use crate::paths::{ + GATEWAY_DISCOVERY_FILE_NAME, GATEWAY_LOG_FILE_NAME, INSTANCE_LOCK_FILE_NAME, LOCK_FILE_NAME, + default_run_dir, gateway_discovery_file_path, gateway_log_path, instance_lock_file_path, + lock_file_path, run_dir, +}; +pub use crate::shutdown::{ShutdownError, request_shutdown, request_shutdown_before}; +#[cfg(any(test, feature = "test-fixtures"))] +#[doc(hidden)] +pub use crate::stale::resolve_for_test; +pub use crate::stale::{Resolution, StaleReason, is_running, resolve, resolve_cancellable}; +pub use crate::validated::{ValidatedConnection, ValidationError}; diff --git a/crates/workshop/gateway-discovery/src/lock-tests.rs b/crates/workshop/gateway-discovery/src/lock-tests.rs new file mode 100644 index 000000000..d5642dab8 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/lock-tests.rs @@ -0,0 +1,230 @@ +//! Unit tests for the launch-race lock and the Gateway instance lease. + +use super::*; + +use std::io::{Read, Write as _}; +use std::net::TcpListener; +use std::sync::mpsc; + +/// The test process's own image name, so the pid and image checks +/// pass and the loser reaches the probe path. +fn own_image_name() -> String { + std::env::current_exe() + .expect("current exe") + .file_name() + .expect("the exe has a file name") + .to_string_lossy() + .into_owned() +} + +/// A fixture gateway answering health and the presented key with 200. +fn fixture_gateway() -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fixture"); + let port = listener.local_addr().expect("fixture address").port(); + std::thread::spawn(move || { + while let Ok((mut stream, _)) = listener.accept() { + for _ in 0..2 { + let mut buffer = [0u8; 1024]; + if stream.read(&mut buffer).is_err() { + break; + } + if stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .is_err() + { + break; + } + } + } + }); + port +} + +/// A live gateway discovery file on the fixture gateway, owned by the test +/// process. +fn live_file(port: u16) -> GatewayDiscoveryFile { + GatewayDiscoveryFile { + port, + api_key: "key".to_owned(), + pid: std::process::id(), + epoch: 1_757_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-09-03T12:00:00Z".to_owned(), + } +} + +#[test] +fn an_empty_run_dir_elects_a_launcher() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let decision = launch_or_attach_named(dir.path(), "irrelevant", Duration::from_secs(1)) + .expect("the race settles"); + assert!( + matches!(decision, LaunchDecision::Launch(_)), + "no file means this caller launches" + ); + assert!( + lock_file_path(dir.path()).exists(), + "the lock file was created" + ); +} + +#[test] +fn process_ownership_is_separate_from_parent_launch_election() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let lease = GatewayInstanceLease::try_acquire(dir.path()) + .expect("attempt the process lease") + .expect("the first process acquires its lease"); + let decision = launch_or_attach_named(dir.path(), "irrelevant", Duration::from_millis(100)) + .expect("the independent parent launch election settles"); + assert!( + matches!(decision, LaunchDecision::Launch(_)), + "holding the process lease never contends with LaunchLock" + ); + assert!( + GatewayInstanceLease::try_acquire(dir.path()) + .expect("contend for process ownership") + .is_none(), + "a second process lease cannot coexist" + ); + drop(lease); + assert!( + GatewayInstanceLease::try_acquire(dir.path()) + .expect("reacquire released process ownership") + .is_some(), + "dropping the handle releases ownership" + ); +} + +#[test] +fn a_live_file_attaches_without_contesting_the_lock() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let file = live_file(fixture_gateway()); + file.write_to(dir.path()).expect("write"); + + let decision = launch_or_attach_named(dir.path(), &own_image_name(), Duration::from_secs(5)) + .expect("the race settles"); + match decision { + LaunchDecision::Attach(attached) => assert_eq!(attached, file), + LaunchDecision::Launch(_) => panic!("a live gateway must be attached, not relaunched"), + } +} + +#[test] +fn the_race_loser_attaches_to_the_winners_file() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let image = own_image_name(); + // The winner holds the lock with no file written yet. + let LaunchDecision::Launch(winner) = + launch_or_attach_named(dir.path(), &image, Duration::from_secs(5)) + .expect("the first caller wins the lock") + else { + panic!("an empty run dir elects a launcher"); + }; + + // The loser waits; the winner's file appears mid-wait. + let loser_dir = dir.path().to_owned(); + let loser = std::thread::spawn(move || { + launch_or_attach_named(&loser_dir, &image, Duration::from_secs(10)) + .expect("the loser's race settles") + }); + std::thread::sleep(Duration::from_millis(100)); + let file = live_file(fixture_gateway()); + file.write_to(dir.path()).expect("the winner writes"); + + match loser.join().expect("the loser thread ran") { + LaunchDecision::Attach(attached) => assert_eq!(attached, file), + LaunchDecision::Launch(_) => { + panic!("the loser must attach to the winner, not launch a second gateway") + } + } + drop(winner); +} + +#[test] +fn a_loser_becomes_the_launcher_when_the_winner_dies_silent() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let image = own_image_name(); + { + let LaunchDecision::Launch(winner) = + launch_or_attach_named(dir.path(), &image, Duration::from_secs(5)) + .expect("the first caller wins the lock") + else { + panic!("an empty run dir elects a launcher"); + }; + // The winner dies (or gives up) without writing a file. + drop(winner); + } + + let decision = launch_or_attach_named(dir.path(), &image, Duration::from_secs(5)) + .expect("the race settles"); + assert!( + matches!(decision, LaunchDecision::Launch(_)), + "a dead winner's lock passes to the next launcher" + ); +} + +#[test] +fn a_loser_times_out_when_the_winner_stays_silent() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let image = own_image_name(); + let LaunchDecision::Launch(_winner) = + launch_or_attach_named(dir.path(), &image, Duration::from_secs(5)) + .expect("the first caller wins the lock") + else { + panic!("an empty run dir elects a launcher"); + }; + + let error = launch_or_attach_named(dir.path(), &image, Duration::from_millis(150)) + .expect_err("a silent winner starves the loser"); + assert!( + matches!(error, SidecarError::LaunchTimeout { .. }), + "the loser reports the timeout: {error}" + ); +} + +#[test] +fn cancellation_joins_a_blocked_launch_race_without_launching() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let image = own_image_name(); + let LaunchDecision::Launch(winner) = + launch_or_attach_named(dir.path(), &image, Duration::from_secs(5)) + .expect("the first caller wins the lock") + else { + panic!("an empty run dir elects a launcher"); + }; + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let run_dir = dir.path().to_owned(); + let worker_image = image.clone(); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + launch_or_attach_named_cancellable_with( + &run_dir, + &worker_image, + Duration::from_secs(30), + &worker_cancellation, + |delay| { + entered.send(()).expect("announce blocked launch race"); + worker_cancellation.wait_timeout(delay) + }, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("the launch-race phase blocks deterministically"); + + let started = Instant::now(); + cancellation.cancel(); + let result = worker.join().expect("the launch-race worker joins"); + + assert!( + started.elapsed() < Duration::from_millis(250), + "cancellation bounds the blocked launch race" + ); + assert!(matches!(result, Err(SidecarError::Cancelled))); + assert!( + !crate::paths::gateway_discovery_file_path(dir.path()).exists(), + "the cancelled loser never publishes or launches" + ); + drop(winner); +} diff --git a/crates/workshop/gateway-discovery/src/lock.rs b/crates/workshop/gateway-discovery/src/lock.rs new file mode 100644 index 000000000..0bf4122f1 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/lock.rs @@ -0,0 +1,275 @@ +//! The launch-race lock: `gateway.json.lock` beside the gateway discovery file, +//! electing one launcher when two readers find no live gateway at the +//! same moment. The loser attaches to the winner. +//! +//! The protocol: the lock holder re-validates the gateway discovery file before +//! deciding, because a previous winner may have written it and released +//! the lock; a loser never deletes anything, it waits for the winner's +//! file to go live and attaches. Cleanup is the lock holder's privilege. +//! The OS-file-lock pattern mirrors gateway-local's `lock_artifact`: an +//! `OpenOptions` create plus a `std::fs::File` advisory lock, released by +//! dropping the handle. + +use std::fs::{File, OpenOptions, TryLockError}; +use std::path::Path; +use std::time::{Duration, Instant}; + +use crate::error::SidecarError; +use crate::paths::lock_file_path; +use crate::stale::{self, GATEWAY_IMAGE_NAME, Resolution}; +use crate::{CancellationToken, GatewayDiscoveryFile}; + +/// Delay between lock retries while a winner finishes its launch. +const RETRY_INTERVAL: Duration = Duration::from_millis(25); + +/// The held launch lock. The holder launches the gateway; dropping the +/// guard releases the lock. +#[derive(Debug)] +pub struct LaunchLock { + // The handle owns the OS lock; dropping it releases. + _file: File, +} + +/// The Gateway process-lifetime ownership lease, which only a test double +/// needs: production Workshop never claims to be a Gateway. +/// +/// This lock is distinct from [`LaunchLock`]: a Workshop parent may hold the +/// launch election while the spawned Gateway acquires this lease. The file +/// handle owns the operating-system lock, so dropping it or terminating its +/// process releases ownership without pid files or explicit cleanup. +#[cfg(feature = "test-fixtures")] +#[derive(Debug)] +pub struct GatewayInstanceLease { + // The handle owns the OS lock; dropping it releases. + _file: File, +} + +#[cfg(feature = "test-fixtures")] +impl GatewayInstanceLease { + /// Attempts to acquire process-lifetime Gateway ownership without + /// blocking. `Ok(None)` means another process owns the lease. + /// + /// # Errors + /// Returns [`SidecarError::CreateDir`] when the run directory cannot be + /// created, or [`SidecarError::Lock`] when the lease file cannot be opened + /// or an unexpected operating-system lock error occurs. + pub fn try_acquire(run_dir: &Path) -> Result, SidecarError> { + std::fs::create_dir_all(run_dir).map_err(|source| SidecarError::CreateDir { + path: run_dir.to_owned(), + source, + })?; + let lock_path = crate::paths::instance_lock_file_path(run_dir); + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .map_err(|source| SidecarError::Lock { + path: lock_path.clone(), + source, + })?; + match file.try_lock() { + Ok(()) => Ok(Some(Self { _file: file })), + Err(TryLockError::WouldBlock) => Ok(None), + Err(TryLockError::Error(source)) => Err(SidecarError::Lock { + path: lock_path, + source, + }), + } + } +} + +/// What [`launch_or_attach`] decided. +#[derive(Debug)] +#[non_exhaustive] +pub enum LaunchDecision { + /// This caller holds the launch lock and no live gateway exists: + /// launch one. The lock stays held until the returned guard drops, so + /// a concurrent racer attaches to the launched gateway instead of + /// launching its own. + Launch(LaunchLock), + /// A live gateway already exists: attach to it. + Attach(GatewayDiscoveryFile), +} + +/// Settles a launch race in `run_dir`: returns [`LaunchDecision::Launch`] +/// with the held lock when no live gateway exists, or +/// [`LaunchDecision::Attach`] when one does - either already running, or +/// launched by the race winner while this caller waited. A loser waits up +/// to `timeout` for the winner's gateway to become attachable, and takes +/// the lock itself when the winner dies without writing one. +/// +/// # Errors +/// Returns [`SidecarError::CreateDir`] when the run directory cannot be +/// created, [`SidecarError::Lock`] when the lock file cannot be opened or +/// an unexpected lock error occurs, [`SidecarError::LaunchTimeout`] when +/// no winner became attachable within `timeout`, and the [`crate::resolve`] +/// errors when the lock holder's own re-validation fails. +pub fn launch_or_attach(run_dir: &Path, timeout: Duration) -> Result { + launch_or_attach_named(run_dir, GATEWAY_IMAGE_NAME, timeout) +} + +/// Settles a launch race while observing caller cancellation. +/// +/// # Errors +/// Returns [`SidecarError::Cancelled`] when cancellation wins, plus the +/// file, lock, and timeout failures documented by [`launch_or_attach`]. +pub fn launch_or_attach_cancellable( + run_dir: &Path, + timeout: Duration, + cancellation: &CancellationToken, +) -> Result { + launch_or_attach_named_cancellable(run_dir, GATEWAY_IMAGE_NAME, timeout, cancellation) +} + +/// [`launch_or_attach`] against a caller-named process image, so tests +/// can run the full liveness gauntlet from a test binary, which is never +/// named `runvil-gateway`. +fn launch_or_attach_named( + run_dir: &Path, + image_name: &str, + timeout: Duration, +) -> Result { + std::fs::create_dir_all(run_dir).map_err(|source| SidecarError::CreateDir { + path: run_dir.to_owned(), + source, + })?; + let lock_path = lock_file_path(run_dir); + let lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .map_err(|source| SidecarError::Lock { + path: lock_path.clone(), + source, + })?; + let deadline = Instant::now() + timeout; + loop { + match lock.try_lock() { + Ok(()) => { + // The lock is held; a previous winner may have written a + // live file before releasing, so re-validate before + // launching. As the holder, cleanup is ours. + return Ok(match stale::resolve_named(run_dir, image_name)? { + Resolution::Attach(file) => LaunchDecision::Attach(file), + Resolution::Absent | Resolution::Stale(_) => { + LaunchDecision::Launch(LaunchLock { _file: lock }) + } + }); + } + Err(TryLockError::WouldBlock) => { + // Another process holds the lock and is mid-launch. Attach + // as soon as its file goes live; never delete here. + if let Ok(Some(file)) = GatewayDiscoveryFile::read(run_dir) + && stale::is_live(&file, image_name) + { + return Ok(LaunchDecision::Attach(file)); + } + if Instant::now() >= deadline { + return Err(SidecarError::LaunchTimeout { timeout }); + } + std::thread::sleep(RETRY_INTERVAL); + } + Err(TryLockError::Error(source)) => { + return Err(SidecarError::Lock { + path: lock_path.clone(), + source, + }); + } + } + } +} + +fn launch_or_attach_named_cancellable( + run_dir: &Path, + image_name: &str, + timeout: Duration, + cancellation: &CancellationToken, +) -> Result { + launch_or_attach_named_cancellable_with(run_dir, image_name, timeout, cancellation, |delay| { + cancellation.wait_timeout(delay) + }) +} + +fn launch_or_attach_named_cancellable_with( + run_dir: &Path, + image_name: &str, + timeout: Duration, + cancellation: &CancellationToken, + mut wait: impl FnMut(Duration) -> bool, +) -> Result { + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + std::fs::create_dir_all(run_dir).map_err(|source| SidecarError::CreateDir { + path: run_dir.to_owned(), + source, + })?; + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + let lock_path = lock_file_path(run_dir); + let lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .map_err(|source| SidecarError::Lock { + path: lock_path.clone(), + source, + })?; + let deadline = Instant::now() + timeout; + loop { + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + match lock.try_lock() { + Ok(()) => { + let resolution = + stale::resolve_named_cancellable(run_dir, image_name, cancellation)?; + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + return Ok(match resolution { + Resolution::Attach(file) => LaunchDecision::Attach(file), + Resolution::Absent | Resolution::Stale(_) => { + LaunchDecision::Launch(LaunchLock { _file: lock }) + } + }); + } + Err(TryLockError::WouldBlock) => { + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + if let Ok(Some(file)) = GatewayDiscoveryFile::read(run_dir) + && stale::is_live_cancellable(&file, image_name, cancellation)? + { + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + return Ok(LaunchDecision::Attach(file)); + } + if Instant::now() >= deadline { + return Err(SidecarError::LaunchTimeout { timeout }); + } + if wait(RETRY_INTERVAL) { + return Err(SidecarError::Cancelled); + } + } + Err(TryLockError::Error(source)) => { + return Err(SidecarError::Lock { + path: lock_path.clone(), + source, + }); + } + } + } +} + +#[cfg(test)] +#[path = "lock-tests.rs"] +mod tests; diff --git a/crates/workshop/gateway-discovery/src/paths.rs b/crates/workshop/gateway-discovery/src/paths.rs new file mode 100644 index 000000000..f5bee0bf2 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/paths.rs @@ -0,0 +1,103 @@ +//! Run-directory, gateway-discovery-file, and gateway-log paths: the one +//! place that knows the `/.runvil/run` and +//! `/.runvil/logs` layout. + +use std::path::{Path, PathBuf}; + +/// The gateway discovery file's name inside the run directory. +pub const GATEWAY_DISCOVERY_FILE_NAME: &str = "gateway.json"; + +/// The gateway log's name inside the state directory's `logs` directory. +pub const GATEWAY_LOG_FILE_NAME: &str = "gateway.log"; + +/// The launch lock's name, beside the gateway discovery file. +pub const LOCK_FILE_NAME: &str = "gateway.json.lock"; + +/// The process-lifetime Gateway instance lock's name. +pub const INSTANCE_LOCK_FILE_NAME: &str = "gateway.instance.lock"; + +/// The run directory under the state dir: `/.runvil/run`. +#[must_use] +pub fn run_dir(home: &Path) -> PathBuf { + home.join(".runvil").join("run") +} + +/// This process's default run directory: the user profile's +/// `.runvil/run`. +/// +/// Returns `None` when the user profile directory cannot be located. +#[must_use] +pub fn default_run_dir() -> Option { + std::env::home_dir().map(|home| run_dir(&home)) +} + +/// The gateway discovery file inside `run_dir`. +#[must_use] +pub fn gateway_discovery_file_path(run_dir: &Path) -> PathBuf { + run_dir.join(GATEWAY_DISCOVERY_FILE_NAME) +} + +/// The launch lock inside `run_dir`. +#[must_use] +pub fn lock_file_path(run_dir: &Path) -> PathBuf { + run_dir.join(LOCK_FILE_NAME) +} + +/// The process-lifetime Gateway instance lock inside `run_dir`. Named here +/// for the contract's sake and used by the test-double lease. +#[must_use] +pub fn instance_lock_file_path(run_dir: &Path) -> PathBuf { + run_dir.join(INSTANCE_LOCK_FILE_NAME) +} + +/// The gateway log under `state_dir`, the directory that holds the run +/// directory: `/.runvil/logs/gateway.log`. The gateway starts +/// logging there before it loads its configuration, so a gateway that exits +/// on its configuration still records why. +#[must_use] +pub fn gateway_log_path(state_dir: &Path) -> PathBuf { + state_dir.join("logs").join(GATEWAY_LOG_FILE_NAME) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_run_dir_sits_under_the_profile_runvil_dir() { + assert_eq!( + run_dir(Path::new("home")), + Path::new("home").join(".runvil").join("run") + ); + } + + #[test] + fn the_gateway_discovery_file_and_locks_sit_beside_each_other() { + let dir = Path::new("run"); + assert_eq!( + gateway_discovery_file_path(dir), + Path::new("run").join("gateway.json") + ); + assert_eq!( + lock_file_path(dir), + Path::new("run").join("gateway.json.lock") + ); + assert_eq!( + instance_lock_file_path(dir), + Path::new("run").join("gateway.instance.lock") + ); + } + + #[test] + fn the_gateway_log_sits_in_a_logs_directory_beside_the_run_directory() { + let run = run_dir(Path::new("home")); + let state_dir = run.parent().expect("the run directory has a parent"); + assert_eq!( + gateway_log_path(state_dir), + Path::new("home") + .join(".runvil") + .join("logs") + .join("gateway.log") + ); + } +} diff --git a/crates/workshop/gateway-discovery/src/shutdown.rs b/crates/workshop/gateway-discovery/src/shutdown.rs new file mode 100644 index 000000000..877192e3e --- /dev/null +++ b/crates/workshop/gateway-discovery/src/shutdown.rs @@ -0,0 +1,225 @@ +//! The `POST /shutdown` request: how a reader asks the gateway to exit. +//! +//! The workshop shell's quit-everything menu item is the caller: it posts +//! the current validated connection's bearer key to the gateway's shutdown +//! route, which answers `202 Accepted` and drains. Raw HTTP/1.0 over +//! `TcpStream` like the health probe, matching the crate's dependency diet - +//! no HTTP client. + +use crate::health::{self, ProbeError}; +use crate::{GatewayDiscoveryFile, ValidatedConnection}; +use std::time::Instant; + +/// The shutdown route's path on the gateway. +const SHUTDOWN_PATH: &str = "/shutdown"; + +/// A failure of [`request_shutdown`]. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum ShutdownError { + /// The request could not be delivered or the answer read. + #[error("{operation}")] + Io { + /// The failed operation. + operation: &'static str, + /// The underlying I/O error. + #[source] + source: std::io::Error, + }, + + /// The gateway answered with a non-2xx status: the key was rejected + /// or the route is gone. + #[error("unexpected shutdown response: {status_line}")] + Rejected { + /// The response's first line. + status_line: String, + }, +} + +impl From for ShutdownError { + fn from(error: ProbeError) -> Self { + match error { + ProbeError::Io { operation, source } => Self::Io { operation, source }, + ProbeError::UnexpectedStatus { status_line } => Self::Rejected { status_line }, + } + } +} + +/// Posts `POST /shutdown` to the validated local Gateway, presenting its +/// bearer key. A 2xx answer means the Gateway accepted and is draining; +/// the caller exits without waiting for the process to die. +/// +/// # Errors +/// Returns [`ShutdownError::Io`] when the gateway cannot be connected or +/// its answer cannot be read, and [`ShutdownError::Rejected`] when the +/// answer's status is not 2xx. +pub fn request_shutdown(connection: &ValidatedConnection) -> Result<(), ShutdownError> { + request_shutdown_file(connection.gateway_discovery_file()) +} + +/// Posts authenticated shutdown without allowing network I/O to outlive the +/// caller's absolute deadline. +/// +/// # Errors +/// Returns the same delivery and rejection errors as [`request_shutdown`]. +pub fn request_shutdown_before( + connection: &ValidatedConnection, + deadline: Instant, +) -> Result<(), ShutdownError> { + request_shutdown_file_before(connection.gateway_discovery_file(), deadline) +} + +fn request_shutdown_file(file: &GatewayDiscoveryFile) -> Result<(), ShutdownError> { + let address = format!("127.0.0.1:{}", file.port); + let head = health::request_head(&address, "POST", SHUTDOWN_PATH, Some(&file.api_key))?; + accepted(&head) +} + +fn request_shutdown_file_before( + file: &GatewayDiscoveryFile, + deadline: Instant, +) -> Result<(), ShutdownError> { + let address = format!("127.0.0.1:{}", file.port); + let head = health::request_head_until( + &address, + "POST", + SHUTDOWN_PATH, + Some(&file.api_key), + deadline, + )?; + accepted(&head) +} + +fn accepted(head: &str) -> Result<(), ShutdownError> { + let status = head + .split_whitespace() + .nth(1) + .and_then(|code| code.parse::().ok()); + if status.is_some_and(|code| (200..300).contains(&code)) { + return Ok(()); + } + Err(ShutdownError::Rejected { + status_line: status.map_or_else(|| "".to_owned(), |code| code.to_string()), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + use std::io::{Read, Write as _}; + use std::net::TcpListener; + use std::sync::mpsc; + use std::time::Duration; + + /// A gateway discovery file naming the fixture gateway's port. + fn file(port: u16) -> GatewayDiscoveryFile { + GatewayDiscoveryFile { + port, + api_key: "key".to_owned(), + pid: 4242, + epoch: 1_757_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-09-03T12:00:00Z".to_owned(), + } + } + + /// A fixture gateway recording each request and answering with the + /// canned response. + fn fixture_gateway(response: &'static [u8]) -> (u16, mpsc::Receiver) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fixture"); + let port = listener.local_addr().expect("fixture address").port(); + let (requests, received) = mpsc::channel(); + std::thread::spawn(move || { + while let Ok((mut stream, _)) = listener.accept() { + let mut buffer = [0u8; 1024]; + if let Ok(read) = stream.read(&mut buffer) { + let _ = requests.send(String::from_utf8_lossy(&buffer[..read]).into_owned()); + } + let _ = stream.write_all(response); + } + }); + (port, received) + } + + #[test] + fn an_accepted_shutdown_posts_the_route_with_the_files_key() { + let (port, received) = + fixture_gateway(b"HTTP/1.1 202 Accepted\r\nContent-Length: 0\r\n\r\n"); + request_shutdown_file(&file(port)).expect("the gateway accepted"); + let request = received + .recv_timeout(Duration::from_secs(5)) + .expect("the request arrived"); + assert!( + request.starts_with("POST /shutdown HTTP/1.0\r\n"), + "the request is a POST to the shutdown route: {request:?}" + ); + assert!( + request.contains("Authorization: Bearer key\r\n"), + "the file's key is the bearer token: {request:?}" + ); + assert!( + request.contains(&format!("Host: 127.0.0.1:{port}\r\n")), + "the Host header is the bound address, not localhost: {request:?}" + ); + } + + #[test] + fn a_refused_shutdown_is_rejected() { + let (port, _received) = + fixture_gateway(b"HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\n\r\n"); + let error = request_shutdown_file(&file(port)).expect_err("a 401 is a refusal"); + assert!( + matches!(error, ShutdownError::Rejected { .. }), + "a non-2xx answer is a rejection: {error}" + ); + } + + #[test] + fn a_reflected_bearer_is_absent_from_shutdown_errors() { + let secret = "capability-secret"; + let response = format!("HTTP/1.1 401 rejected-{secret}\r\nContent-Length: 0\r\n\r\n"); + let response: &'static [u8] = Box::leak(response.into_bytes().into_boxed_slice()); + let (port, _received) = fixture_gateway(response); + let mut connection = file(port); + connection.api_key = secret.to_owned(); + + let error = request_shutdown_file(&connection).expect_err("a 401 is a refusal"); + assert!( + !format!("{error:?} {error}").contains(secret), + "bearer values never enter error diagnostics" + ); + } + + #[test] + fn a_dead_gateway_is_an_io_error() { + // Port 1 is never listening, so the connect fails fast. + let error = request_shutdown_file(&file(1)).expect_err("a dead port cannot answer"); + assert!( + matches!(error, ShutdownError::Io { .. }), + "an undelivered request is an I/O error: {error}" + ); + } + + #[test] + fn an_absolute_deadline_bounds_an_unresponsive_shutdown_target() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind slow shutdown fixture"); + let port = listener.local_addr().expect("fixture address").port(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept shutdown"); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + std::thread::sleep(Duration::from_secs(1)); + }); + let started = std::time::Instant::now(); + + let error = request_shutdown_file_before(&file(port), started + Duration::from_millis(100)) + .expect_err("an unresponsive child exceeds the cleanup deadline"); + + assert!(matches!(error, ShutdownError::Io { .. })); + assert!( + started.elapsed() < Duration::from_millis(500), + "late-child shutdown uses one absolute network deadline" + ); + } +} diff --git a/crates/workshop/gateway-discovery/src/stale-tests.rs b/crates/workshop/gateway-discovery/src/stale-tests.rs new file mode 100644 index 000000000..4a7700c5e --- /dev/null +++ b/crates/workshop/gateway-discovery/src/stale-tests.rs @@ -0,0 +1,389 @@ +//! Unit tests for stale detection and stale-file cleanup. + +use super::*; + +use std::io::{Read, Write as _}; +use std::net::TcpListener; +use std::sync::mpsc; + +use crate::paths::gateway_discovery_file_path; + +/// The test process's own image name, so the pid and image checks +/// pass and the test reaches the probe under test. +fn own_image_name() -> String { + std::env::current_exe() + .expect("current exe") + .file_name() + .expect("the exe has a file name") + .to_string_lossy() + .into_owned() +} + +/// A gateway discovery file pointing at the test process itself. +fn live_file(port: u16, api_key: &str) -> GatewayDiscoveryFile { + GatewayDiscoveryFile { + port, + api_key: api_key.to_owned(), + pid: std::process::id(), + epoch: 1_757_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-09-03T12:00:00Z".to_owned(), + } +} + +/// A pid guaranteed dead: a short-lived child, reaped and dropped so +/// no handle keeps the process object alive. +fn dead_pid() -> u32 { + let mut child = std::process::Command::new(std::env::current_exe().expect("current exe")) + .arg("--list") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn a short-lived child"); + let pid = child.id(); + child.wait().expect("the child exits"); + drop(child); + pid +} + +/// A fixture gateway: answers `GET /health` with 200 and the key +/// probe with 200 only when the bearer matches `expected_key`. +fn fixture_gateway(expected_key: &'static str) -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fixture"); + let port = listener.local_addr().expect("fixture address").port(); + std::thread::spawn(move || { + while let Ok((mut stream, _)) = listener.accept() { + for _ in 0..2 { + let mut buffer = [0u8; 1024]; + let Ok(read) = stream.read(&mut buffer) else { + break; + }; + let request = String::from_utf8_lossy(&buffer[..read]); + let accepted = request.starts_with("GET /health ") + || request.contains(&format!("Authorization: Bearer {expected_key}\r\n")); + let response = if accepted { + &b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"[..] + } else { + &b"HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\n\r\n"[..] + }; + if stream.write_all(response).is_err() { + break; + } + } + } + }); + port +} + +#[test] +fn a_dead_pid_is_stale_and_cleaned() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let file = live_file(1, "key"); + let file = GatewayDiscoveryFile { + pid: dead_pid(), + ..file + }; + file.write_to(dir.path()).expect("write"); + + let resolution = resolve(dir.path()).expect("resolve"); + assert_eq!(resolution, Resolution::Stale(StaleReason::ProcessDead)); + assert!( + !gateway_discovery_file_path(dir.path()).exists(), + "the stale file was deleted" + ); +} + +#[test] +fn an_alive_pid_with_a_foreign_image_is_stale() { + let dir = tempfile::TempDir::new().expect("tempdir"); + // The test binary is never named runvil-gateway, so the real + // image name must reject the test process's own pid. + live_file(1, "key").write_to(dir.path()).expect("write"); + + let resolution = resolve(dir.path()).expect("resolve"); + assert_eq!(resolution, Resolution::Stale(StaleReason::ImageMismatch)); + assert!( + !gateway_discovery_file_path(dir.path()).exists(), + "the stale file was deleted" + ); +} + +#[test] +fn a_silent_health_endpoint_is_stale() { + let dir = tempfile::TempDir::new().expect("tempdir"); + // Port 1 never listens, so every probe is refused until the + // liveness budget elapses. + live_file(1, "key").write_to(dir.path()).expect("write"); + + let resolution = resolve_named(dir.path(), &own_image_name()).expect("resolve"); + assert_eq!(resolution, Resolution::Stale(StaleReason::HealthFailed)); + assert!( + !gateway_discovery_file_path(dir.path()).exists(), + "the stale file was deleted" + ); +} + +#[test] +fn a_transiently_silent_health_endpoint_is_not_stale() { + let dir = tempfile::TempDir::new().expect("tempdir"); + // The first probe connection is hung up without a response; only + // a retried probe learns the gateway is live. + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fixture"); + let port = listener.local_addr().expect("fixture address").port(); + std::thread::spawn(move || { + let mut first = true; + while let Ok((mut stream, _)) = listener.accept() { + if std::mem::take(&mut first) { + drop(stream); + continue; + } + for _ in 0..2 { + let mut buffer = [0u8; 1024]; + if stream.read(&mut buffer).is_err() { + break; + } + if stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .is_err() + { + break; + } + } + } + }); + let file = live_file(port, "key"); + file.write_to(dir.path()).expect("write"); + + let resolution = resolve_named(dir.path(), &own_image_name()).expect("resolve"); + assert_eq!( + resolution, + Resolution::Attach(file), + "one dropped probe must not reject a live gateway" + ); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "a live file is left in place" + ); +} + +#[test] +fn a_rejected_key_is_stale_and_cleaned() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let port = fixture_gateway("right"); + live_file(port, "wrong") + .write_to(dir.path()) + .expect("write"); + + let resolution = resolve_named(dir.path(), &own_image_name()).expect("resolve"); + assert_eq!(resolution, Resolution::Stale(StaleReason::KeyRejected)); + assert!( + !gateway_discovery_file_path(dir.path()).exists(), + "the stale file was deleted" + ); +} + +#[test] +fn a_fully_live_file_attaches() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let port = fixture_gateway("right"); + let file = live_file(port, "right"); + file.write_to(dir.path()).expect("write"); + + let resolution = resolve_named(dir.path(), &own_image_name()).expect("resolve"); + assert_eq!(resolution, Resolution::Attach(file)); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "a live file is left in place" + ); +} + +#[test] +fn cancellation_during_resolve_leaves_the_gateway_discovery_file_untouched() { + let dir = tempfile::TempDir::new().expect("tempdir"); + live_file(1, "key").write_to(dir.path()).expect("write"); + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let run_dir = dir.path().to_owned(); + let image_name = own_image_name(); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + resolve_named_cancellable_with( + &run_dir, + &image_name, + &worker_cancellation, + |_, _, cancellation| { + entered.send(()).expect("announce blocked resolve"); + let _ = cancellation.wait_timeout(std::time::Duration::from_secs(30)); + Err(crate::ValidationError::Cancelled) + }, + ) + }); + blocked + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("the resolve phase blocks deterministically"); + + let started = std::time::Instant::now(); + cancellation.cancel(); + let result = worker.join().expect("the resolve worker joins"); + + assert!( + started.elapsed() < std::time::Duration::from_millis(250), + "cancellation bounds the blocked resolve" + ); + assert!(matches!(result, Err(SidecarError::Cancelled))); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "cancellation never classifies or deletes the gateway discovery file" + ); +} + +#[test] +fn cancellation_immediately_before_invalid_file_removal_preserves_the_file() { + let dir = tempfile::TempDir::new().expect("tempdir"); + fs::write(gateway_discovery_file_path(dir.path()), b"not json").expect("write invalid fixture"); + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let run_dir = dir.path().to_owned(); + let (entered, blocked) = mpsc::channel(); + let (release, released) = mpsc::channel(); + let removals = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let worker_removals = std::sync::Arc::clone(&removals); + let worker = std::thread::spawn(move || { + resolve_named_cancellable_with_effects( + &run_dir, + "unused", + &worker_cancellation, + |_, _, _| -> Result { + panic!("an invalid file never reaches validation") + }, + || { + entered.send(()).expect("announce invalid-file removal"); + released.recv().expect("release invalid-file removal"); + }, + |run_dir| { + worker_removals.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + remove_stale(run_dir) + }, + ) + }); + blocked + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("resolution pauses immediately before invalid-file removal"); + + cancellation.cancel(); + release.send(()).expect("release invalid-file removal"); + let result = worker.join().expect("resolve worker joins"); + + assert!(matches!(result, Err(SidecarError::Cancelled))); + assert_eq!( + removals.load(std::sync::atomic::Ordering::SeqCst), + 0, + "invalid-file cleanup cannot begin after cancellation returns" + ); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "the cancelled invalid file remains untouched" + ); +} + +#[test] +fn cancellation_immediately_before_failed_validation_removal_preserves_the_file() { + let dir = tempfile::TempDir::new().expect("tempdir"); + live_file(1, "key").write_to(dir.path()).expect("write"); + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let run_dir = dir.path().to_owned(); + let (entered, blocked) = mpsc::channel(); + let (release, released) = mpsc::channel(); + let removals = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let worker_removals = std::sync::Arc::clone(&removals); + let worker = std::thread::spawn(move || { + resolve_named_cancellable_with_effects( + &run_dir, + "unused", + &worker_cancellation, + |_, _, _| Err(ValidationError::Stale(StaleReason::HealthFailed)), + || { + entered.send(()).expect("announce stale-file removal"); + released.recv().expect("release stale-file removal"); + }, + |run_dir| { + worker_removals.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + remove_stale(run_dir) + }, + ) + }); + blocked + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("resolution pauses immediately before stale-file removal"); + + cancellation.cancel(); + release.send(()).expect("release stale-file removal"); + let result = worker.join().expect("resolve worker joins"); + + assert!(matches!(result, Err(SidecarError::Cancelled))); + assert_eq!( + removals.load(std::sync::atomic::Ordering::SeqCst), + 0, + "failed-validation cleanup cannot begin after cancellation returns" + ); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "the cancelled stale file remains untouched" + ); +} + +#[test] +fn is_running_reports_a_live_gateway_without_touching_the_file() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let port = fixture_gateway("right"); + live_file(port, "right") + .write_to(dir.path()) + .expect("write"); + + assert!( + is_running_named(dir.path(), &own_image_name()), + "a fully live file reads as running" + ); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "the read-only check never deletes" + ); +} + +#[test] +fn is_running_leaves_a_stale_file_in_place() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let file = GatewayDiscoveryFile { + pid: dead_pid(), + ..live_file(1, "key") + }; + file.write_to(dir.path()).expect("write"); + + assert!( + !is_running_named(dir.path(), &own_image_name()), + "a dead pid reads as not-running" + ); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "stale-file deletion is the prospective owner's privilege" + ); +} + +#[test] +fn is_running_reads_absent_and_corrupt_files_as_not_running() { + let dir = tempfile::TempDir::new().expect("tempdir"); + assert!( + !is_running_named(dir.path(), &own_image_name()), + "no gateway discovery file reads as not-running" + ); + fs::write(gateway_discovery_file_path(dir.path()), b"not json").expect("write fixture"); + assert!( + !is_running_named(dir.path(), &own_image_name()), + "a corrupt file reads as not-running and is left alone" + ); + assert!( + gateway_discovery_file_path(dir.path()).exists(), + "the corrupt file was not deleted" + ); +} diff --git a/crates/workshop/gateway-discovery/src/stale.rs b/crates/workshop/gateway-discovery/src/stale.rs new file mode 100644 index 000000000..be5cc1fdd --- /dev/null +++ b/crates/workshop/gateway-discovery/src/stale.rs @@ -0,0 +1,258 @@ +//! Stale detection: decide whether a gateway discovery file names a live +//! gateway, and remove it when it does not. +//! +//! A file is live when one OS process boot with a `runvil-gateway` +//! image is unchanged across a same-socket health and bearer proof, and +//! the file includes a boot identity. Anything else is stale - the Jupyter +//! phantom-server bug class - and the file is deleted so the next reader +//! relaunches instead of retrying a corpse. + +use std::fs; +use std::io; +use std::path::Path; + +use crate::error::SidecarError; +use crate::paths::gateway_discovery_file_path; +pub(crate) use crate::validated::GATEWAY_IMAGE_NAME; +use crate::validated::{ValidatedConnection, ValidationError}; +use crate::{CancellationToken, GatewayDiscoveryFile}; + +/// What [`resolve`] found in the run directory. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum Resolution { + /// A live gateway: attach with these parameters. + Attach(GatewayDiscoveryFile), + /// No gateway discovery file exists: nothing to attach to, nothing to clean. + Absent, + /// A gateway discovery file existed but was stale; it was removed. + Stale(StaleReason), +} + +/// Why a gateway discovery file was judged stale. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[non_exhaustive] +pub enum StaleReason { + /// The file was not valid JSON or failed validation. + #[error("the gateway discovery file is invalid")] + Invalid, + /// The pid is dead. + #[error("the recorded gateway process is dead")] + ProcessDead, + /// The pid is alive but its image is not a `runvil-gateway` + /// binary (a reused pid). + #[error("the recorded pid belongs to another process image")] + ImageMismatch, + /// The gateway discovery file has no usable boot identity. + #[error("the gateway discovery file has no usable boot identity")] + BootIdentityInvalid, + /// The pid changed process boot while validation was in progress. + #[error("the recorded process identity changed during validation")] + ProcessChanged, + /// The health endpoint did not answer 200. + #[error("the recorded gateway does not answer its health probe")] + HealthFailed, + /// The bearer key was rejected. + #[error("the gateway discovery file bearer was rejected")] + KeyRejected, +} + +/// Resolves the gateway discovery file in `run_dir`: attach parameters for a +/// live gateway, or stale-file cleanup plus the reason. +/// +/// # Errors +/// Returns [`SidecarError::Read`] when the file exists but cannot be +/// read, and [`SidecarError::Remove`] when a stale file cannot be +/// deleted. +pub fn resolve(run_dir: &Path) -> Result { + resolve_named(run_dir, GATEWAY_IMAGE_NAME) +} + +/// Resolves the gateway discovery file while observing caller cancellation. +/// +/// Cancellation never classifies or deletes the current file. +/// +/// # Errors +/// Returns [`SidecarError::Cancelled`] when cancellation wins, plus the +/// read and remove failures documented by [`resolve`]. +pub fn resolve_cancellable( + run_dir: &Path, + cancellation: &CancellationToken, +) -> Result { + resolve_named_cancellable(run_dir, GATEWAY_IMAGE_NAME, cancellation) +} + +/// [`resolve`] against a caller-named process image. Test builds only. +/// +/// This exists for resolution tests that run in process and write a +/// `gateway.json` naming the test binary's own pid. A test binary is never +/// named `runvil-gateway`, so such a file would read as `ImageMismatch` +/// before the resolution logic under test was reached. +/// +/// It is not the way to exercise the real process-image step. Tests that +/// want the full gauntlet use the image-copy helper in `test_gateway.rs` +/// instead, which spawns a child named `runvil-gateway` and calls +/// [`resolve`] with no seam at all. +#[cfg(any(test, feature = "test-fixtures"))] +#[doc(hidden)] +pub fn resolve_for_test(run_dir: &Path, image_name: &str) -> Result { + resolve_named(run_dir, image_name) +} + +/// [`resolve`] against a caller-named process image, so tests can run the +/// full liveness gauntlet from a test binary, which is never named +/// `runvil-gateway`. +pub(crate) fn resolve_named(run_dir: &Path, image_name: &str) -> Result { + let file = match GatewayDiscoveryFile::read(run_dir) { + Ok(Some(file)) => file, + Ok(None) => return Ok(Resolution::Absent), + Err(SidecarError::Parse { .. } | SidecarError::Invalid { .. }) => { + remove_stale(run_dir)?; + return Ok(Resolution::Stale(StaleReason::Invalid)); + } + Err(error) => return Err(error), + }; + match ValidatedConnection::validate_named(file, image_name) { + Ok(validated) => Ok(Resolution::Attach(validated.into_gateway_discovery_file())), + Err(reason) => { + remove_stale(run_dir)?; + Ok(Resolution::Stale(reason)) + } + } +} + +pub(crate) fn resolve_named_cancellable( + run_dir: &Path, + image_name: &str, + cancellation: &CancellationToken, +) -> Result { + resolve_named_cancellable_with( + run_dir, + image_name, + cancellation, + ValidatedConnection::validate_named_cancellable, + ) +} + +fn resolve_named_cancellable_with( + run_dir: &Path, + image_name: &str, + cancellation: &CancellationToken, + validate: impl FnOnce( + GatewayDiscoveryFile, + &str, + &CancellationToken, + ) -> Result, +) -> Result { + resolve_named_cancellable_with_effects( + run_dir, + image_name, + cancellation, + validate, + || {}, + remove_stale, + ) +} + +fn resolve_named_cancellable_with_effects( + run_dir: &Path, + image_name: &str, + cancellation: &CancellationToken, + validate: impl FnOnce( + GatewayDiscoveryFile, + &str, + &CancellationToken, + ) -> Result, + mut before_remove: impl FnMut(), + mut remove: impl FnMut(&Path) -> Result<(), SidecarError>, +) -> Result { + if cancellation.is_cancelled() { + return Err(SidecarError::Cancelled); + } + let file = match GatewayDiscoveryFile::read(run_dir) { + Ok(Some(file)) => file, + Ok(None) if cancellation.is_cancelled() => return Err(SidecarError::Cancelled), + Ok(None) => return Ok(Resolution::Absent), + Err(SidecarError::Parse { .. } | SidecarError::Invalid { .. }) => { + before_remove(); + remove_stale_if_active(run_dir, cancellation, &mut remove)?; + return Ok(Resolution::Stale(StaleReason::Invalid)); + } + Err(error) => return Err(error), + }; + match validate(file, image_name, cancellation) { + Ok(validated) => Ok(Resolution::Attach(validated.into_gateway_discovery_file())), + Err(ValidationError::Cancelled) => Err(SidecarError::Cancelled), + Err(ValidationError::Stale(reason)) => { + before_remove(); + remove_stale_if_active(run_dir, cancellation, &mut remove)?; + Ok(Resolution::Stale(reason)) + } + } +} + +fn remove_stale_if_active( + run_dir: &Path, + cancellation: &CancellationToken, + remove: &mut impl FnMut(&Path) -> Result<(), SidecarError>, +) -> Result<(), SidecarError> { + cancellation + .run_if_active(|| remove(run_dir)) + .unwrap_or(Err(SidecarError::Cancelled)) +} + +/// Whether the gateway discovery file in `run_dir` names a live gateway right +/// now, with no cleanup: the read-only check a diagnostics report runs. +/// Stale-file deletion is the prospective owner's privilege, so a stale +/// file reads as not-running and stays on disk for the next launch to +/// clean. +#[must_use] +pub fn is_running(run_dir: &Path) -> bool { + is_running_named(run_dir, GATEWAY_IMAGE_NAME) +} + +/// [`is_running`] against a caller-named process image, so a test binary - +/// never named `runvil-gateway` - can run the full liveness gauntlet. +fn is_running_named(run_dir: &Path, image_name: &str) -> bool { + match GatewayDiscoveryFile::read(run_dir) { + Ok(Some(file)) => is_live(&file, image_name), + // A missing, unreadable, or invalid file reads as not-running. + Ok(None) | Err(_) => false, + } +} + +/// Whether the file's gateway is live right now, with no cleanup: the +/// check a launch-race loser runs, since deleting is the lock holder's +/// privilege. +pub(crate) fn is_live(file: &GatewayDiscoveryFile, image_name: &str) -> bool { + ValidatedConnection::validate_named(file.clone(), image_name).is_ok() +} + +pub(crate) fn is_live_cancellable( + file: &GatewayDiscoveryFile, + image_name: &str, + cancellation: &CancellationToken, +) -> Result { + match ValidatedConnection::validate_named_cancellable(file.clone(), image_name, cancellation) { + Ok(_) => Ok(true), + Err(ValidationError::Stale(_)) => Ok(false), + Err(ValidationError::Cancelled) => Err(SidecarError::Cancelled), + } +} + +/// Deletes the stale gateway discovery file, tolerating a concurrent deletion. +fn remove_stale(run_dir: &Path) -> Result<(), SidecarError> { + let path = gateway_discovery_file_path(run_dir); + match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(SidecarError::Remove { + path, + source: error, + }), + } +} + +#[cfg(test)] +#[path = "stale-tests.rs"] +mod tests; diff --git a/crates/workshop/gateway-discovery/src/sys.rs b/crates/workshop/gateway-discovery/src/sys.rs new file mode 100644 index 000000000..046f28b9e --- /dev/null +++ b/crates/workshop/gateway-discovery/src/sys.rs @@ -0,0 +1,58 @@ +//! Process identity lookup for stale detection: one shim per platform, +//! each answering "what binary does this pid run, and which process boot +//! owns the pid", so pid reuse cannot join separate validation observations. + +use std::path::PathBuf; + +/// An OS-observed process boot, stable for one lifetime and different +/// when the pid is reused. +#[derive(Clone, PartialEq, Eq)] +pub(crate) struct ProcessIdentity { + /// The process executable observed for this boot. + pub(crate) image: PathBuf, + /// Platform start marker: FILETIME, proc start ticks, or timeval. + started: u128, +} + +impl ProcessIdentity { + /// Assembles one platform observation. + pub(crate) const fn new(image: PathBuf, started: u128) -> Self { + Self { image, started } + } + + /// Assembles a deterministic identity for validation regressions. + #[cfg(test)] + pub(crate) const fn for_test(image: PathBuf, started: u128) -> Self { + Self::new(image, started) + } +} + +#[cfg(target_os = "linux")] +mod linux; +#[cfg(target_os = "macos")] +#[expect( + unsafe_code, + reason = "proc_pidpath and proc_pidinfo are raw C APIs with no safe wrappers" +)] +mod macos; +#[cfg(windows)] +#[expect( + unsafe_code, + reason = "process identity uses raw Win32 handle and query APIs" +)] +mod windows; + +#[cfg(target_os = "linux")] +pub(crate) use linux::process_identity; +#[cfg(target_os = "macos")] +pub(crate) use macos::process_identity; +#[cfg(windows)] +pub(crate) use windows::process_identity; + +/// Every other platform fails closed: no process identity means the +/// gateway discovery file is always stale, so a reader relaunches rather than +/// attaching to an unverified process. +#[cfg(not(any(windows, target_os = "linux", target_os = "macos")))] +pub(crate) fn process_identity(_pid: u32) -> Option { + None +} diff --git a/crates/workshop/gateway-discovery/src/sys/linux.rs b/crates/workshop/gateway-discovery/src/sys/linux.rs new file mode 100644 index 000000000..7b06f1c49 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/sys/linux.rs @@ -0,0 +1,22 @@ +//! Linux process identity lookup: `/proc//exe` supplies the image and +//! field 22 of `/proc//stat` supplies the kernel start tick. + +use super::ProcessIdentity; + +/// The image and start tick for process `pid`, or `None` when the process +/// disappears, changes identity during observation, or `/proc` is unreadable. +pub(crate) fn process_identity(pid: u32) -> Option { + let first_start = process_start(pid)?; + let image = std::fs::read_link(format!("/proc/{pid}/exe")).ok()?; + let second_start = process_start(pid)?; + (first_start == second_start).then(|| ProcessIdentity::new(image, u128::from(first_start))) +} + +/// Reads Linux `/proc//stat` field 22. The command field is enclosed +/// in parentheses and may itself contain spaces or closing parentheses, +/// so fields are counted only after its final delimiter. +fn process_start(pid: u32) -> Option { + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; + let after_command = stat.get(stat.rfind(')')? + 1..)?; + after_command.split_whitespace().nth(19)?.parse().ok() +} diff --git a/crates/workshop/gateway-discovery/src/sys/macos.rs b/crates/workshop/gateway-discovery/src/sys/macos.rs new file mode 100644 index 000000000..598c294da --- /dev/null +++ b/crates/workshop/gateway-discovery/src/sys/macos.rs @@ -0,0 +1,96 @@ +//! macOS process identity lookup via libproc: `proc_pidpath` supplies the +//! image and `PROC_PIDTBSDINFO` supplies the process start timeval. + +use std::ffi::OsString; +use std::mem::{MaybeUninit, size_of}; +use std::os::unix::ffi::OsStringExt as _; +use std::path::PathBuf; + +use super::ProcessIdentity; + +/// Buffer size for `proc_pidpath`: `PROC_PIDPATHINFO_MAXSIZE` from +/// `libproc.h` (4 * MAXPATHLEN). +const PROC_PIDPATHINFO_MAXSIZE: u32 = 4096; +/// `PROC_PIDTBSDINFO` from `libproc.h`. +const PROC_PIDTBSDINFO: i32 = 3; + +/// The stable prefix and start fields of Darwin's `proc_bsdinfo`. +#[repr(C)] +struct ProcBsdInfo { + pbi_flags: u32, + pbi_status: u32, + pbi_xstatus: u32, + pbi_pid: u32, + pbi_ppid: u32, + pbi_uid: u32, + pbi_gid: u32, + pbi_ruid: u32, + pbi_rgid: u32, + pbi_svuid: u32, + pbi_svgid: u32, + rfu_1: u32, + pbi_comm: [libc::c_char; 16], + pbi_name: [libc::c_char; 32], + pbi_nfiles: u32, + pbi_pgid: u32, + pbi_pjobc: u32, + e_tdev: u32, + e_tpgid: u32, + pbi_nice: i32, + pbi_start_tvsec: u64, + pbi_start_tvusec: u64, +} + +/// The image and start timeval for process `pid`, or `None` when the +/// process disappears, changes identity during observation, or refuses +/// either query. +pub(crate) fn process_identity(pid: u32) -> Option { + let first_start = process_start(pid)?; + let image = process_image_path(pid)?; + let second_start = process_start(pid)?; + (first_start == second_start).then(|| ProcessIdentity::new(image, first_start)) +} + +/// Reads the kernel's path for one live process. +fn process_image_path(pid: u32) -> Option { + let pid = i32::try_from(pid).ok()?; + let mut buffer = vec![0u8; PROC_PIDPATHINFO_MAXSIZE as usize]; + // SAFETY: `buffer` is a live allocation of exactly + // PROC_PIDPATHINFO_MAXSIZE bytes and the size handed over matches it; + // proc_pidpath writes at most that many bytes and returns the count + // written, or a value <= 0 on error. + let written = + unsafe { libc::proc_pidpath(pid, buffer.as_mut_ptr().cast(), PROC_PIDPATHINFO_MAXSIZE) }; + if written <= 0 { + return None; + } + let written = usize::try_from(written).ok()?; + buffer.truncate(written); + Some(PathBuf::from(OsString::from_vec(buffer))) +} + +/// Reads the process start timeval through `PROC_PIDTBSDINFO`. +fn process_start(pid: u32) -> Option { + let pid = i32::try_from(pid).ok()?; + let buffer_size = i32::try_from(size_of::()).ok()?; + let mut info = MaybeUninit::::uninit(); + // SAFETY: `info` points to writable storage of exactly `buffer_size` + // bytes. A full-size success initializes the complete structure before + // `assume_init`; every other result returns without reading it. + let written = unsafe { + libc::proc_pidinfo( + pid, + PROC_PIDTBSDINFO, + 0, + info.as_mut_ptr().cast(), + buffer_size, + ) + }; + if written != buffer_size { + return None; + } + // SAFETY: the full-size `proc_pidinfo` success above initialized every + // byte of the `ProcBsdInfo` output structure. + let info = unsafe { info.assume_init() }; + Some(u128::from(info.pbi_start_tvsec) << 64 | u128::from(info.pbi_start_tvusec)) +} diff --git a/crates/workshop/gateway-discovery/src/sys/windows.rs b/crates/workshop/gateway-discovery/src/sys/windows.rs new file mode 100644 index 000000000..58731f16a --- /dev/null +++ b/crates/workshop/gateway-discovery/src/sys/windows.rs @@ -0,0 +1,79 @@ +//! Windows process identity lookup: one process handle supplies the image +//! and creation FILETIME, so pid reuse cannot join two observations. + +use std::ffi::OsString; +use std::os::windows::ffi::OsStringExt as _; +use std::path::PathBuf; + +use windows_sys::Win32::Foundation::{CloseHandle, FILETIME, HANDLE}; +use windows_sys::Win32::System::Threading::{ + GetProcessTimes, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, QueryFullProcessImageNameW, +}; + +use super::ProcessIdentity; + +/// The image and creation time of process `pid`, or `None` when the +/// process is dead or refuses a limited query. +pub(crate) fn process_identity(pid: u32) -> Option { + // SAFETY: `OpenProcess` takes a valid access mask and pid; the + // returned handle is either null (checked) or a live process handle + // that `CloseHandle` below releases exactly once. + let handle = unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid) }; + if handle.is_null() { + return None; + } + let identity = query_identity(handle); + // SAFETY: `handle` is the live process handle returned by the + // `OpenProcess` above, closed exactly once here. + unsafe { + CloseHandle(handle); + } + identity +} + +/// Reads one coherent image and creation time from an open process handle. +fn query_identity(handle: HANDLE) -> Option { + let image = query_image_path(handle)?; + let mut creation = FILETIME { + dwLowDateTime: 0, + dwHighDateTime: 0, + }; + let mut exit = creation; + let mut kernel = creation; + let mut user = creation; + // SAFETY: all pointers name initialized writable FILETIME values, and + // `handle` remains open for the complete query. + let ok = unsafe { + GetProcessTimes( + handle, + &raw mut creation, + &raw mut exit, + &raw mut kernel, + &raw mut user, + ) + }; + if ok == 0 { + return None; + } + let started = u128::from(creation.dwHighDateTime) << 32 | u128::from(creation.dwLowDateTime); + Some(ProcessIdentity::new(image, started)) +} + +/// Reads the image path from an open process handle. +fn query_image_path(handle: HANDLE) -> Option { + // The long-path ceiling: QueryFullProcessImageNameW fails rather than + // truncates when the buffer is too small, so one max-sized buffer + // needs no grow loop. + const BUFFER_LEN: u32 = 32768; + let mut buffer = vec![0u16; BUFFER_LEN as usize]; + let mut size = BUFFER_LEN; + // SAFETY: `buffer` is valid for `size` UTF-16 code units and `size` + // starts at its length; on success the call writes the path length + // back into `size`, never exceeding the buffer. + let ok = unsafe { QueryFullProcessImageNameW(handle, 0, buffer.as_mut_ptr(), &raw mut size) }; + if ok == 0 { + return None; + } + let length = usize::try_from(size).ok()?; + Some(PathBuf::from(OsString::from_wide(buffer.get(..length)?))) +} diff --git a/crates/workshop/gateway-discovery/src/validated-tests.rs b/crates/workshop/gateway-discovery/src/validated-tests.rs new file mode 100644 index 000000000..f9c790922 --- /dev/null +++ b/crates/workshop/gateway-discovery/src/validated-tests.rs @@ -0,0 +1,373 @@ +//! Unit tests for building a validated Gateway connection. + +use super::*; + +use std::io::{Read, Write as _}; +use std::net::TcpListener; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, mpsc}; + +use crate::GatewayDiscoveryFile; + +fn own_image_name() -> String { + std::env::current_exe() + .expect("current exe") + .file_name() + .expect("the exe has a file name") + .to_string_lossy() + .into_owned() +} + +fn connection(port: u16, api_key: &str) -> GatewayDiscoveryFile { + GatewayDiscoveryFile { + port, + api_key: api_key.to_owned(), + pid: std::process::id(), + epoch: 1_778_000_000, + version: "0.2.0".to_owned(), + started_at: "2026-05-05T12:00:00Z".to_owned(), + } +} + +fn dead_pid() -> u32 { + let mut child = std::process::Command::new(std::env::current_exe().expect("current exe")) + .arg("--list") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn a short-lived child"); + let pid = child.id(); + child.wait().expect("the child exits"); + pid +} + +fn fixture_gateway(expected_key: &'static str) -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fixture"); + let port = listener.local_addr().expect("fixture address").port(); + std::thread::spawn(move || { + while let Ok((mut stream, _)) = listener.accept() { + for _ in 0..2 { + let mut buffer = [0_u8; 1024]; + let Ok(read) = stream.read(&mut buffer) else { + break; + }; + let request = String::from_utf8_lossy(&buffer[..read]); + let accepted = request.starts_with("GET /health ") + || request.contains(&format!("Authorization: Bearer {expected_key}\r\n")); + let response = if accepted { + &b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}"[..] + } else { + &b"HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\n\r\n"[..] + }; + if stream.write_all(response).is_err() { + break; + } + } + } + }); + port +} + +#[test] +fn a_wrong_process_image_cannot_create_a_capability() { + let file = connection(1, "key"); + + assert_eq!( + ValidatedConnection::validate_named(file, "not-the-test-binary"), + Err(StaleReason::ImageMismatch) + ); +} + +#[test] +fn a_dead_process_cannot_create_a_capability() { + let file = GatewayDiscoveryFile { + pid: dead_pid(), + ..connection(1, "key") + }; + + assert_eq!( + ValidatedConnection::validate_named(file, &own_image_name()), + Err(StaleReason::ProcessDead) + ); +} + +#[test] +fn a_stale_boot_identity_cannot_create_a_capability() { + let missing_epoch = GatewayDiscoveryFile { + epoch: 0, + ..connection(1, "key") + }; + assert_eq!( + ValidatedConnection::validate_named(missing_epoch, &own_image_name()), + Err(StaleReason::BootIdentityInvalid) + ); + + let missing_start = GatewayDiscoveryFile { + started_at: String::new(), + ..connection(1, "key") + }; + assert_eq!( + ValidatedConnection::validate_named(missing_start, &own_image_name()), + Err(StaleReason::BootIdentityInvalid) + ); +} + +#[test] +fn an_invalid_raw_connection_cannot_create_a_capability() { + let file = GatewayDiscoveryFile { + port: 0, + ..connection(1, "key") + }; + + assert_eq!( + ValidatedConnection::validate_named(file, &own_image_name()), + Err(StaleReason::Invalid) + ); +} + +#[test] +fn failed_health_cannot_create_a_capability() { + let file = connection(1, "key"); + + assert_eq!( + ValidatedConnection::validate_named(file, &own_image_name()), + Err(StaleReason::HealthFailed) + ); +} + +#[test] +fn an_absolute_validation_deadline_bounds_a_slow_owner() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind slow owner"); + let port = listener.local_addr().expect("slow owner address").port(); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept validation"); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + std::thread::sleep(Duration::from_secs(1)); + }); + let timeout = Duration::from_millis(100); + let started = Instant::now(); + + let result = ValidatedConnection::validate_named_before( + connection(port, "key"), + &own_image_name(), + started + timeout, + ); + + assert_eq!(result, Err(StaleReason::HealthFailed)); + assert!( + started.elapsed() < Duration::from_millis(500), + "the caller's deadline caps the real network proof" + ); +} + +#[test] +fn a_rejected_bearer_cannot_create_a_capability() { + let port = fixture_gateway("accepted"); + let file = connection(port, "rejected"); + + assert_eq!( + ValidatedConnection::validate_named(file, &own_image_name()), + Err(StaleReason::KeyRejected) + ); +} + +#[test] +fn a_new_boot_with_the_same_port_and_key_creates_a_distinct_capability() { + let port = fixture_gateway("stable-key"); + let original = + ValidatedConnection::validate_named(connection(port, "stable-key"), &own_image_name()) + .expect("the original connection validates"); + let replacement_file = GatewayDiscoveryFile { + epoch: original.epoch() + 1, + started_at: "2026-05-05T12:00:01Z".to_owned(), + ..connection(port, "stable-key") + }; + let replacement = ValidatedConnection::validate_named(replacement_file, &own_image_name()) + .expect("the replacement connection validates"); + + assert_eq!(replacement.port(), original.port()); + assert_eq!(replacement.api_key(), original.api_key()); + assert!(!replacement.same_boot(&original)); +} + +#[test] +fn debug_output_never_contains_the_bearer() { + let secret = "capability-secret"; + let port = fixture_gateway(secret); + let mut raw = connection(port, secret); + raw.version = format!("version-{secret}\r\n"); + raw.started_at = format!("started-{secret}\t"); + let validated = ValidatedConnection::validate_named(raw, &own_image_name()) + .expect("the connection validates"); + + let debug = format!("{validated:?}"); + assert!(!debug.contains(secret), "debug output redacts the bearer"); + assert!( + !debug.contains(['\r', '\n', '\t']), + "untrusted metadata cannot inject debug output" + ); + assert!(debug.contains(&port.to_string()), "the endpoint is visible"); +} + +#[test] +fn a_process_boot_change_during_the_network_proof_is_rejected() { + let image = std::path::PathBuf::from(own_image_name()); + let before = ProcessIdentity::for_test(image.clone(), 41); + let after = ProcessIdentity::for_test(image, 42); + let mut observations = [Some(before), Some(after)].into_iter(); + + assert_eq!( + validate_with( + connection(8081, "key"), + &own_image_name(), + |_| observations.next().flatten(), + |_, _, _| ConnectionProbe::Accepted, + ), + Err(StaleReason::ProcessChanged), + "a reused pid cannot complete a mixed proof" + ); +} + +#[test] +fn cancellation_inside_the_first_identity_probe_prevents_observation() { + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let image_name = own_image_name(); + let identity = ProcessIdentity::for_test(std::path::PathBuf::from(&image_name), 41); + let observations = Arc::new(AtomicUsize::new(0)); + let worker_observations = Arc::clone(&observations); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + validate_cancellable_with( + connection(8081, "key"), + &image_name, + &worker_cancellation, + |_| { + entered.send(()).expect("announce identity probe"); + if !worker_cancellation.wait_timeout(Duration::from_secs(30)) { + worker_observations.fetch_add(1, Ordering::SeqCst); + } + Some(identity.clone()) + }, + |_, _, _, _| ConnectionProbe::Accepted, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("the first process identity probe blocks deterministically"); + + let started = std::time::Instant::now(); + cancellation.cancel(); + let result = worker.join().expect("the validation worker joins"); + + assert!( + started.elapsed() < Duration::from_millis(250), + "cancellation wakes the in-progress process identity probe" + ); + assert!(matches!(result, Err(ValidationError::Cancelled))); + assert_eq!( + observations.load(Ordering::SeqCst), + 0, + "no process observation occurs after cancellation" + ); +} + +#[test] +fn cancellation_during_validation_prevents_the_second_identity_observation() { + let cancellation = crate::CancellationToken::new(); + let worker_cancellation = cancellation.clone(); + let image_name = own_image_name(); + let identity = ProcessIdentity::for_test(std::path::PathBuf::from(&image_name), 41); + let observations = Arc::new(AtomicUsize::new(0)); + let worker_observations = Arc::clone(&observations); + let (entered, blocked) = mpsc::channel(); + let worker = std::thread::spawn(move || { + validate_cancellable_with( + connection(8081, "key"), + &image_name, + &worker_cancellation, + |_| { + worker_observations.fetch_add(1, Ordering::SeqCst); + Some(identity.clone()) + }, + |_, _, _, cancellation| { + entered.send(()).expect("announce blocked validation"); + let _ = cancellation.wait_timeout(Duration::from_secs(30)); + ConnectionProbe::Accepted + }, + ) + }); + blocked + .recv_timeout(Duration::from_secs(1)) + .expect("the validation phase blocks deterministically"); + + let started = std::time::Instant::now(); + cancellation.cancel(); + let result = worker.join().expect("the validation worker joins"); + + assert!( + started.elapsed() < Duration::from_millis(250), + "cancellation bounds the blocked validation" + ); + assert!(matches!(result, Err(ValidationError::Cancelled))); + assert_eq!( + observations.load(Ordering::SeqCst), + 1, + "validation performs no post-cancel process probe" + ); +} + +#[test] +fn forged_file_identity_cannot_alias_a_reused_process() { + let raw = connection(8081, "key"); + let image = std::path::PathBuf::from(own_image_name()); + let first_identity = ProcessIdentity::for_test(image.clone(), 41); + let second_identity = ProcessIdentity::for_test(image, 42); + let first = validate_with( + raw.clone(), + &own_image_name(), + |_| Some(first_identity.clone()), + |_, _, _| ConnectionProbe::Accepted, + ) + .expect("the first coherent proof validates"); + let second = validate_with( + raw, + &own_image_name(), + |_| Some(second_identity.clone()), + |_, _, _| ConnectionProbe::Accepted, + ) + .expect("the replacement's coherent proof validates"); + + assert!( + !first.same_boot(&second), + "identical attacker-controlled file fields cannot forge process identity" + ); +} + +#[test] +fn validation_errors_never_contain_the_bearer_or_metadata() { + let secret = "capability-secret"; + let mut raw = connection(8081, secret); + raw.version = format!("version-{secret}\r\n"); + raw.started_at = format!("started-{secret}\t"); + let image = std::path::PathBuf::from(own_image_name()); + let identity = ProcessIdentity::for_test(image, 41); + let error = validate_with( + raw, + &own_image_name(), + |_| Some(identity.clone()), + |_, _, _| ConnectionProbe::KeyRejected, + ) + .expect_err("the bearer is rejected"); + + let debug = format!("{error:?}"); + let display = format!("{error}"); + for rendered in [debug, display] { + assert!(!rendered.contains(secret), "errors redact the bearer"); + assert!( + !rendered.contains(['\r', '\n', '\t']), + "untrusted metadata cannot inject errors" + ); + } +} diff --git a/crates/workshop/gateway-discovery/src/validated.rs b/crates/workshop/gateway-discovery/src/validated.rs new file mode 100644 index 000000000..41ff81a4f --- /dev/null +++ b/crates/workshop/gateway-discovery/src/validated.rs @@ -0,0 +1,364 @@ +//! A live Gateway connection whose process and authority have been +//! validated. + +use std::ffi::OsStr; +use std::fmt; +use std::path::Path; +use std::time::{Duration, Instant}; + +use crate::health::{self, ConnectionProbe}; +use crate::stale::StaleReason; +use crate::sys::{ProcessIdentity, process_identity}; +use crate::{CancellationToken, GatewayDiscoveryFile}; + +/// The image file name a live Gateway process must have. +#[cfg(windows)] +pub(crate) const GATEWAY_IMAGE_NAME: &str = "runvil-gateway.exe"; +/// The image file name a live Gateway process must have. +#[cfg(not(windows))] +pub(crate) const GATEWAY_IMAGE_NAME: &str = "runvil-gateway"; + +/// The bearer-gated route used to prove the presented key is accepted. +const KEY_PROBE_PATH: &str = "/v1/models"; + +/// Budget for proving health without rejecting one transient failure. +const LIVENESS_BUDGET: Duration = Duration::from_secs(2); + +/// A cancellable validation failure. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[non_exhaustive] +pub enum ValidationError { + /// The caller cancelled validation. + #[error("connection validation was cancelled")] + Cancelled, + /// The connection failed one of the liveness or authority checks. + #[error(transparent)] + Stale(#[from] StaleReason), +} + +/// A Gateway connection proven live and authorized at construction time. +/// +/// Safe code outside this crate cannot construct the capability directly. +/// Construction succeeds only after checking the process image, boot +/// identity, health endpoint, and bearer acceptance. +/// +/// Validation observes the OS process boot immediately before and after +/// the one TCP connection that makes both network checks. This closes the +/// health-to-bearer replacement gap and rejects pid reuse during that +/// interval. The capability is a point-in-time proof and makes no claim +/// that the process remains live after validation returns. +/// +/// The bearer is deliberately absent from [`Debug`](fmt::Debug) output. +/// +/// No test-fixture feature exposes another production-capability +/// constructor. +/// +/// The crate-private named validator is equally unavailable. +#[derive(Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct ValidatedConnection { + connection: GatewayDiscoveryFile, + process_identity: ProcessIdentity, +} + +impl ValidatedConnection { + /// Validates a raw gateway discovery file against the production Gateway image. + /// + /// # Errors + /// Returns the first [`StaleReason`] that prevents the raw connection + /// from proving a live, authorized Gateway boot. + pub fn validate(connection: GatewayDiscoveryFile) -> Result { + Self::validate_named(connection, GATEWAY_IMAGE_NAME) + } + + /// Validates a raw connection against the production Gateway image + /// without allowing the network proof to outlive `deadline`. + /// + /// # Errors + /// Returns the first [`StaleReason`] that prevents the raw connection + /// from proving a live, authorized Gateway boot before `deadline`. + pub fn validate_before( + connection: GatewayDiscoveryFile, + deadline: Instant, + ) -> Result { + Self::validate_named_before(connection, GATEWAY_IMAGE_NAME, deadline) + } + + /// Validates a raw connection while observing caller cancellation. + /// + /// # Errors + /// Returns [`ValidationError::Cancelled`] when cancellation wins, or + /// [`ValidationError::Stale`] when a liveness or authority check fails. + pub fn validate_cancellable( + connection: GatewayDiscoveryFile, + cancellation: &CancellationToken, + ) -> Result { + Self::validate_named_cancellable(connection, GATEWAY_IMAGE_NAME, cancellation) + } + + pub(crate) fn validate_named( + connection: GatewayDiscoveryFile, + image_name: &str, + ) -> Result { + validate_with( + connection, + image_name, + process_identity, + |address, bearer, deadline| { + health::probe_connection_until(address, KEY_PROBE_PATH, bearer, deadline) + }, + ) + } + + fn validate_named_before( + connection: GatewayDiscoveryFile, + image_name: &str, + deadline: Instant, + ) -> Result { + validate_before_with( + connection, + image_name, + deadline, + process_identity, + |address, bearer, deadline| { + health::probe_connection_until(address, KEY_PROBE_PATH, bearer, deadline) + }, + ) + } + + pub(crate) fn validate_named_cancellable( + connection: GatewayDiscoveryFile, + image_name: &str, + cancellation: &CancellationToken, + ) -> Result { + validate_cancellable_with( + connection, + image_name, + cancellation, + process_identity, + |address, bearer, budget, cancellation| { + health::probe_connection_cancellable( + address, + KEY_PROBE_PATH, + bearer, + budget, + cancellation, + ) + }, + ) + } + + /// The validated Gateway's loopback port. + #[must_use] + pub const fn port(&self) -> u16 { + self.connection.port + } + + /// The validated Gateway process identifier. + #[must_use] + pub const fn pid(&self) -> u32 { + self.connection.pid + } + + /// The validated Gateway boot epoch. + #[must_use] + pub const fn epoch(&self) -> u64 { + self.connection.epoch + } + + /// The validated Gateway version. + #[must_use] + pub fn version(&self) -> &str { + &self.connection.version + } + + /// The validated Gateway boot timestamp. + #[must_use] + pub fn started_at(&self) -> &str { + &self.connection.started_at + } + + /// Whether both capabilities name the same validated Gateway boot. + #[must_use] + pub fn same_boot(&self, other: &Self) -> bool { + self.process_identity == other.process_identity + && self.pid() == other.pid() + && self.epoch() == other.epoch() + && self.started_at() == other.started_at() + } + + /// The validated bearer required to build an authorized consumer. + /// + /// Callers must keep this value out of diagnostics. The capability's + /// own [`Debug`](fmt::Debug) implementation always redacts it. + #[doc(hidden)] + #[must_use] + pub fn api_key(&self) -> &str { + &self.connection.api_key + } + + pub(crate) fn gateway_discovery_file(&self) -> &GatewayDiscoveryFile { + &self.connection + } + + pub(crate) fn into_gateway_discovery_file(self) -> GatewayDiscoveryFile { + self.connection + } +} + +impl fmt::Debug for ValidatedConnection { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("ValidatedConnection") + .field("port", &self.port()) + .field("pid", &self.pid()) + .field("epoch", &self.epoch()) + .field("version", &"[REDACTED]") + .field("started_at", &"[REDACTED]") + .field("api_key", &"[REDACTED]") + .finish() + } +} + +/// Performs one validation while allowing deterministic observation and +/// network seams in unit tests. Production passes only the platform process +/// observer and the private shared probe. +fn validate_with( + connection: GatewayDiscoveryFile, + image_name: &str, + observe_process: impl FnMut(u32) -> Option, + prove_connection: impl FnOnce(&str, &str, Instant) -> ConnectionProbe, +) -> Result { + validate_before_with( + connection, + image_name, + Instant::now() + LIVENESS_BUDGET, + observe_process, + prove_connection, + ) +} + +fn validate_before_with( + connection: GatewayDiscoveryFile, + image_name: &str, + deadline: Instant, + mut observe_process: impl FnMut(u32) -> Option, + prove_connection: impl FnOnce(&str, &str, Instant) -> ConnectionProbe, +) -> Result { + if Instant::now() >= deadline { + return Err(StaleReason::HealthFailed); + } + if connection.validation_error().is_some() { + return Err(StaleReason::Invalid); + } + let Some(before) = observe_process(connection.pid) else { + return Err(StaleReason::ProcessDead); + }; + if !image_name_matches(&before.image, image_name) { + return Err(StaleReason::ImageMismatch); + } + if !connection.has_boot_identity() { + return Err(StaleReason::BootIdentityInvalid); + } + if Instant::now() >= deadline { + return Err(StaleReason::HealthFailed); + } + let address = format!("127.0.0.1:{}", connection.port); + match prove_connection(&address, &connection.api_key, deadline) { + ConnectionProbe::Cancelled | ConnectionProbe::HealthFailed => { + return Err(StaleReason::HealthFailed); + } + ConnectionProbe::KeyRejected => return Err(StaleReason::KeyRejected), + ConnectionProbe::Accepted => {} + } + let Some(after) = observe_process(connection.pid) else { + return Err(StaleReason::ProcessChanged); + }; + if before != after { + return Err(StaleReason::ProcessChanged); + } + Ok(ValidatedConnection { + connection, + process_identity: before, + }) +} + +fn validate_cancellable_with( + connection: GatewayDiscoveryFile, + image_name: &str, + cancellation: &CancellationToken, + mut observe_process: impl FnMut(u32) -> Option, + prove_connection: impl FnOnce(&str, &str, Duration, &CancellationToken) -> ConnectionProbe, +) -> Result { + if cancellation.is_cancelled() { + return Err(ValidationError::Cancelled); + } + if connection.validation_error().is_some() { + return Err(StaleReason::Invalid.into()); + } + let Some(before_observation) = cancellation.run_if_active(|| observe_process(connection.pid)) + else { + return Err(ValidationError::Cancelled); + }; + let Some(before) = before_observation else { + return Err(StaleReason::ProcessDead.into()); + }; + if cancellation.is_cancelled() { + return Err(ValidationError::Cancelled); + } + if !image_name_matches(&before.image, image_name) { + return Err(StaleReason::ImageMismatch.into()); + } + if !connection.has_boot_identity() { + return Err(StaleReason::BootIdentityInvalid.into()); + } + let address = format!("127.0.0.1:{}", connection.port); + let proof = prove_connection(&address, &connection.api_key, LIVENESS_BUDGET, cancellation); + if cancellation.is_cancelled() || proof == ConnectionProbe::Cancelled { + return Err(ValidationError::Cancelled); + } + match proof { + ConnectionProbe::HealthFailed => return Err(StaleReason::HealthFailed.into()), + ConnectionProbe::KeyRejected => return Err(StaleReason::KeyRejected.into()), + ConnectionProbe::Accepted => {} + ConnectionProbe::Cancelled => return Err(ValidationError::Cancelled), + } + let Some(after_observation) = cancellation.run_if_active(|| observe_process(connection.pid)) + else { + return Err(ValidationError::Cancelled); + }; + let Some(after) = after_observation else { + return Err(StaleReason::ProcessChanged.into()); + }; + if cancellation.is_cancelled() { + return Err(ValidationError::Cancelled); + } + if before != after { + return Err(StaleReason::ProcessChanged.into()); + } + Ok(ValidatedConnection { + connection, + process_identity: before, + }) +} + +fn image_name_matches(image: &Path, expected: &str) -> bool { + let Some(name) = image.file_name() else { + return false; + }; + image_file_name_matches(name, expected) +} + +#[cfg(windows)] +fn image_file_name_matches(name: &OsStr, expected: &str) -> bool { + name.to_string_lossy().eq_ignore_ascii_case(expected) +} + +#[cfg(not(windows))] +fn image_file_name_matches(name: &OsStr, expected: &str) -> bool { + name == OsStr::new(expected) +} + +#[cfg(test)] +#[path = "validated-tests.rs"] +mod tests; diff --git a/crates/workshop/gateway/Cargo.toml b/crates/workshop/gateway/Cargo.toml index cbee31f52..86ac736dd 100644 --- a/crates/workshop/gateway/Cargo.toml +++ b/crates/workshop/gateway/Cargo.toml @@ -7,18 +7,19 @@ license.workspace = true repository.workspace = true [features] -test-fixtures = ["dep:tempfile", "workshop-support/test-fixtures"] +test-fixtures = [ + "dep:tempfile", + "workshop-gateway-discovery/test-fixtures", + "workshop-support/test-fixtures", +] [dependencies] arc-swap.workspace = true futures-util.workspace = true -# The gateway's public wire vocabulary: the progress snapshot the -# `GET /admin/progress` stream sends. -gateway-api-types.workspace = true reqwest.workspace = true serde.workspace = true serde_json.workspace = true -gateway-api-discovery.workspace = true +workshop-gateway-discovery.workspace = true thiserror.workspace = true tokio.workspace = true tokio-tungstenite.workspace = true @@ -31,16 +32,15 @@ tempfile = { workspace = true, optional = true } [dev-dependencies] axum.workspace = true -# The test-fixtures feature exposes `resolve_for_test`, so the resolution -# tests can run the real liveness gauntlet against the test binary's own -# process image. -gateway-api-discovery = { workspace = true, features = ["test-fixtures"] } tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } # The gateway's tests call `workshop_support::fixtures::serve`; enabling the # feature here (not only through the crate's own `test-fixtures` feature) # keeps `cargo test -p workshop-gateway` compiling without `--all-features`. workshop-support = { workspace = true, features = ["test-fixtures"] } +# The discovery fork's `test-fixtures` feature supplies the `gateway.json` +# writer and the process lease Workshop's test doubles need. +workshop-gateway-discovery = { workspace = true, features = ["test-fixtures"] } [build-dependencies] build-ceiling.workspace = true diff --git a/crates/workshop/gateway/src/binding.rs b/crates/workshop/gateway/src/binding.rs index d90596870..ed4ff4d7b 100644 --- a/crates/workshop/gateway/src/binding.rs +++ b/crates/workshop/gateway/src/binding.rs @@ -28,7 +28,7 @@ pub struct GatewaySnapshot { /// Monotonic generation assigned before this snapshot is published. generation: u64, /// Proven local gateway boot, absent for an explicitly configured endpoint. - identity: Option, + identity: Option, } impl fmt::Debug for GatewaySnapshot { @@ -123,7 +123,7 @@ impl GatewayBinding { pub fn new_with_identity( base_url: &str, api_key: &str, - identity: Option, + identity: Option, ) -> Result { Ok(Self::from_snapshot(build_snapshot( base_url, api_key, identity, @@ -189,7 +189,7 @@ impl GatewayBinding { &self, base_url: &str, api_key: &str, - identity: Option, + identity: Option, ) -> Result<(), GatewayPublicationError> { let snapshot = build_snapshot(base_url, api_key, identity)?; self.publish_snapshot(snapshot) @@ -275,7 +275,7 @@ impl GatewayUpdater { #[cfg(any(test, feature = "test-fixtures"))] pub fn replace_sidecar( &self, - connection: &gateway_api_discovery::ValidatedConnection, + connection: &crate::discovery::ValidatedConnection, ) -> Result<(), GatewayPublicationError> { self.binding.replace_with_identity( &format!("http://127.0.0.1:{}", connection.port()), @@ -318,7 +318,7 @@ impl GatewayUpdater { fn build_snapshot( base_url: &str, api_key: &str, - identity: Option, + identity: Option, ) -> Result { Ok(GatewaySnapshot { client: GatewayClient::new(base_url, api_key)?, diff --git a/crates/workshop/gateway/src/binding/publication.rs b/crates/workshop/gateway/src/binding/publication.rs index 3031cf9ba..113b03e05 100644 --- a/crates/workshop/gateway/src/binding/publication.rs +++ b/crates/workshop/gateway/src/binding/publication.rs @@ -13,15 +13,15 @@ impl GatewayBinding { &self, base_url: &str, api_key: &str, - identity: gateway_api_discovery::ValidatedConnection, - cancellation: &gateway_api_discovery::CancellationToken, + identity: crate::discovery::ValidatedConnection, + cancellation: &crate::discovery::CancellationToken, ) -> Result { self.replace_with_identity_cancellable_with_wait( base_url, api_key, identity, cancellation, - gateway_api_discovery::CancellationToken::wait_timeout, + crate::discovery::CancellationToken::wait_timeout, ) } @@ -29,9 +29,9 @@ impl GatewayBinding { &self, base_url: &str, api_key: &str, - identity: gateway_api_discovery::ValidatedConnection, - cancellation: &gateway_api_discovery::CancellationToken, - mut wait: impl FnMut(&gateway_api_discovery::CancellationToken, Duration) -> bool, + identity: crate::discovery::ValidatedConnection, + cancellation: &crate::discovery::CancellationToken, + mut wait: impl FnMut(&crate::discovery::CancellationToken, Duration) -> bool, ) -> Result { if cancellation.is_cancelled() { return if self.publication_closed() { @@ -82,8 +82,8 @@ impl GatewayUpdater { /// replacement publication. pub fn replace_sidecar_cancellable( &self, - connection: &gateway_api_discovery::ValidatedConnection, - cancellation: &gateway_api_discovery::CancellationToken, + connection: &crate::discovery::ValidatedConnection, + cancellation: &crate::discovery::CancellationToken, ) -> Result { self.binding.replace_with_identity_cancellable( &format!("http://127.0.0.1:{}", connection.port()), diff --git a/crates/workshop/gateway/src/binding/shutdown.rs b/crates/workshop/gateway/src/binding/shutdown.rs index fd8a9485d..cef97ddc2 100644 --- a/crates/workshop/gateway/src/binding/shutdown.rs +++ b/crates/workshop/gateway/src/binding/shutdown.rs @@ -21,13 +21,13 @@ impl GatewaySnapshot { /// runs it off the executor. /// /// # Errors - /// Returns [`gateway_api_discovery::ShutdownError`] when the local gateway + /// Returns [`crate::discovery::ShutdownError`] when the local gateway /// refuses the request or cannot be reached. - pub fn request_shutdown(&self) -> Result { + pub fn request_shutdown(&self) -> Result { let Some(identity) = self.identity.as_ref() else { return Ok(false); }; - gateway_api_discovery::request_shutdown(identity)?; + crate::discovery::request_shutdown(identity)?; Ok(true) } } @@ -42,9 +42,9 @@ impl GatewayUpdater { /// authority. /// /// # Errors - /// Returns [`gateway_api_discovery::ShutdownError`] when the current local + /// Returns [`crate::discovery::ShutdownError`] when the current local /// gateway refuses the request or cannot be reached. - pub fn request_shutdown(&self) -> Result { + pub fn request_shutdown(&self) -> Result { self.binding.snapshot().request_shutdown() } } diff --git a/crates/workshop/gateway/src/binding/tests.rs b/crates/workshop/gateway/src/binding/tests.rs index 494f35abb..bc72a4551 100644 --- a/crates/workshop/gateway/src/binding/tests.rs +++ b/crates/workshop/gateway/src/binding/tests.rs @@ -11,7 +11,7 @@ fn validated_connection( api_key: &str, epoch: u64, started_at: &str, -) -> gateway_api_discovery::ValidatedConnection { +) -> crate::discovery::ValidatedConnection { gateway.validate(api_key, epoch, started_at) } diff --git a/crates/workshop/gateway/src/binding/tests/publication.rs b/crates/workshop/gateway/src/binding/tests/publication.rs index 13c772aa8..e3224fdbc 100644 --- a/crates/workshop/gateway/src/binding/tests/publication.rs +++ b/crates/workshop/gateway/src/binding/tests/publication.rs @@ -18,7 +18,7 @@ fn cancellation_wakes_a_replacement_contending_on_the_publication_lock() { .lock() .unwrap_or_else(PoisonError::into_inner); let worker_binding = binding.clone(); - let cancellation = gateway_api_discovery::CancellationToken::new(); + let cancellation = crate::discovery::CancellationToken::new(); let worker_cancellation = cancellation.clone(); let (entered, blocked) = std::sync::mpsc::channel(); let worker = std::thread::spawn(move || { @@ -79,7 +79,7 @@ fn publication_close_is_permanent_for_every_updater_clone() { .expect_err("a closed binding rejects every updater clone"); assert!(matches!(error, GatewayPublicationError::PublicationClosed)); } - let cancellation = gateway_api_discovery::CancellationToken::new(); + let cancellation = crate::discovery::CancellationToken::new(); cancellation.cancel(); assert!(matches!( binding diff --git a/crates/workshop/gateway/src/client/progress_stream.rs b/crates/workshop/gateway/src/client/progress_stream.rs index 3b0e74f2c..bf6f9cf65 100644 --- a/crates/workshop/gateway/src/client/progress_stream.rs +++ b/crates/workshop/gateway/src/client/progress_stream.rs @@ -12,8 +12,8 @@ use std::pin::Pin; +use super::super::progress::Progress; use futures_util::Stream; -use gateway_api_types::Progress; use super::GatewayError; diff --git a/crates/workshop/gateway/src/client/tests/progress.rs b/crates/workshop/gateway/src/client/tests/progress.rs index 1409321a5..6560e99fb 100644 --- a/crates/workshop/gateway/src/client/tests/progress.rs +++ b/crates/workshop/gateway/src/client/tests/progress.rs @@ -8,9 +8,8 @@ use super::*; use futures_util::StreamExt as _; -use gateway_api_types::Progress; - use crate::client::progress_stream::MAX_EVENT_BLOCK; +use crate::progress::Progress; /// Serializes a wire-format progress snapshot by hand, so the tests pin /// the JSON shape the gateway emits rather than the types crate's diff --git a/crates/workshop/gateway/src/lib.rs b/crates/workshop/gateway/src/lib.rs index 4e0a696bc..cb2f2864d 100644 --- a/crates/workshop/gateway/src/lib.rs +++ b/crates/workshop/gateway/src/lib.rs @@ -23,9 +23,10 @@ //! the gateway drives the menu through its //! [`MenuPush`](workshop_registry::MenuPush) face, so this crate never //! names another subsystem's bus. -//! - The gateway's progress reaches this crate as the public wire type -//! `gateway_api_types::Progress` alone: no progress machinery is -//! shared with the gateway family. +//! - The gateway's progress reaches this crate as one wire type defined in +//! `progress-wire.rs`: no progress machinery is shared with the gateway +//! family, and its field names are pinned by a golden JSON test so drift +//! is caught. mod binding; mod client; @@ -46,3 +47,14 @@ pub use handles::{GatewayHandles, GatewayTaskRegistrations, register, register_t pub use heartbeat::{GatewayHealth, Heartbeat, join_status, spawn as spawn_heartbeat}; pub use refresh::{refresh_catalog, refresh_profiles}; pub use resolve::{GatewaySource, ResolveError, ResolvedGateway, report, resolve}; + +/// The Gateway discovery contract as Workshop reads it, re-exported whole +/// from the fork that owns it. +/// +/// The desktop app may name only `workshop-server-api` among workshop crates, +/// and the server and its facade are re-exports, so this module binding is +/// what carries the contract out to the desktop app: +/// `workshop_server_api::discovery::resolve` and friends. Naming the crate +/// once here rather than listing its twenty items at each level keeps one +/// name for one thing. +pub use workshop_gateway_discovery as discovery; diff --git a/crates/workshop/gateway/src/progress-wire-tests.rs b/crates/workshop/gateway/src/progress-wire-tests.rs new file mode 100644 index 000000000..aea494815 --- /dev/null +++ b/crates/workshop/gateway/src/progress-wire-tests.rs @@ -0,0 +1,85 @@ +//! The progress wire contract, pinned by golden JSON. +//! +//! Workshop only deserializes `Progress`, so this file pins the field names +//! and their JSON types rather than a round trip. A rename on the Gateway's +//! side, or a type change here, fails one of these assertions instead of +//! silently leaving the status bar stuck. + +use super::*; + +#[test] +fn the_canonical_document_decodes_to_the_two_fields() { + let snapshot: Progress = + serde_json::from_str(r#"{"busy":true,"text":"Downloading qwen3-8b.gguf 45%"}"#) + .expect("the canonical document decodes"); + assert_eq!( + snapshot, + Progress { + busy: true, + text: "Downloading qwen3-8b.gguf 45%".to_owned(), + } + ); +} + +#[test] +fn an_idle_document_decodes_to_an_empty_text() { + let snapshot: Progress = + serde_json::from_str(r#"{"busy":false,"text":""}"#).expect("an idle document decodes"); + assert_eq!(snapshot, Progress::default()); +} + +#[test] +fn the_field_names_and_json_types_are_the_contract() { + // Pins the wire names and types through a decode of each, so a rename + // on either side fails here even if nobody updates a fixture. The + // future OpenAPI spec describes these same two fields. + // + // Both fields are required: the Gateway writes the whole snapshot on + // every event, so a missing field is a version skew the decoder should + // report rather than paper over with a default. + for document in [r#"{"busy":true}"#, r#"{"text":"Loading"}"#] { + let decoded = serde_json::from_str::(document); + assert!( + decoded.is_err(), + "both fields are required, so {document} must not decode: {decoded:?}" + ); + } + + let wrong_type = serde_json::from_str::(r#"{"busy":"yes","text":""}"#); + assert!( + wrong_type.is_err(), + "busy is a boolean, not a string: {wrong_type:?}" + ); + let wrong_text = serde_json::from_str::(r#"{"busy":false,"text":7}"#); + assert!( + wrong_text.is_err(), + "text is a string, not a number: {wrong_text:?}" + ); +} + +#[test] +fn an_unknown_field_is_tolerated() { + // A newer Gateway adding a field must not break an older Workshop. + let snapshot: Progress = + serde_json::from_str(r#"{"busy":true,"text":"Loading","fraction":0.45}"#) + .expect("an unknown field is ignored"); + assert_eq!(snapshot.text, "Loading"); +} + +#[test] +fn the_old_operation_event_shape_is_not_a_progress_snapshot() { + // A gateway emitting the retired weighted-tree operation events is a + // version skew the decoder reports, never renders. + let stale = serde_json::json!({ + "operation": 7, + "path": "local-models/ggml/download", + "label": "Download", + "state": {"Updated": {"fraction": 0.5}}, + }) + .to_string(); + let decoded = serde_json::from_str::(&stale); + assert!( + decoded.is_err(), + "a payload without busy and text is not a snapshot: {decoded:?}" + ); +} diff --git a/crates/workshop/gateway/src/progress-wire.rs b/crates/workshop/gateway/src/progress-wire.rs new file mode 100644 index 000000000..c4543b284 --- /dev/null +++ b/crates/workshop/gateway/src/progress-wire.rs @@ -0,0 +1,35 @@ +//! The gateway progress wire type, as Workshop reads it: one live activity, +//! a busy flag and a producer-owned text. +//! +//! [`Progress`] is the shape of one `GET /admin/progress` event and of the +//! progress field in `GET /admin/status`. Workshop only ever deserializes +//! it; the Gateway owns the producing side and the anti-flicker policy +//! lives in [`crate::progress`]. +//! +//! The field names and types are the contract. [`tests`] pins them against +//! a golden JSON document, so a rename or a type change on either side +//! fails here rather than in a status bar that silently stops updating. +//! +//! The text is user-visible, so a producer must never place a bearer key, +//! API key, or other credential in it. + +use serde::Deserialize; + +/// One snapshot of the Gateway's live activity. +/// +/// Deserialization tolerates fields this type does not know, which is what +/// lets a newer Gateway add one without breaking an older Workshop. There +/// is no schema version. +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] +pub struct Progress { + /// Whether any activity is live. The UI shows an indeterminate + /// barberpole while this is set. + pub busy: bool, + /// The newest live activity's text, e.g. `"Downloading qwen3-8b.gguf + /// 45%"`; empty when idle. + pub text: String, +} + +#[cfg(test)] +#[path = "progress-wire-tests.rs"] +mod tests; diff --git a/crates/workshop/gateway/src/progress.rs b/crates/workshop/gateway/src/progress.rs index 729c59f86..a9f15752d 100644 --- a/crates/workshop/gateway/src/progress.rs +++ b/crates/workshop/gateway/src/progress.rs @@ -18,6 +18,8 @@ //! dropped stream cannot flash it. mod presenter; +#[path = "progress-wire.rs"] +mod wire; use std::future::Future; use std::time::Duration; @@ -31,6 +33,7 @@ use workshop_registry::Push; use crate::binding::{GatewayBinding, GatewaySnapshot}; use crate::client::ProgressStream; use crate::heartbeat::GatewayHealth; +pub(crate) use wire::Progress; #[cfg(test)] use presenter::{MIN_VISIBLE, SHOW_DELAY}; diff --git a/crates/workshop/gateway/src/progress/presenter.rs b/crates/workshop/gateway/src/progress/presenter.rs index 8fc3b6542..1c7917869 100644 --- a/crates/workshop/gateway/src/progress/presenter.rs +++ b/crates/workshop/gateway/src/progress/presenter.rs @@ -12,12 +12,13 @@ use std::time::Duration; -use gateway_api_types::Progress; use tokio::time::Instant; use workshop_protocol::Activity; use workshop_registry::Push; +use super::Progress; + /// How long the gateway must stay busy before the barberpole appears; /// work shorter than this never disturbs the status bar. pub(super) const SHOW_DELAY: Duration = Duration::from_secs(1); diff --git a/crates/workshop/gateway/src/progress/tests-presenter.rs b/crates/workshop/gateway/src/progress/tests-presenter.rs index 589d3f74d..f7285ea43 100644 --- a/crates/workshop/gateway/src/progress/tests-presenter.rs +++ b/crates/workshop/gateway/src/progress/tests-presenter.rs @@ -6,8 +6,6 @@ use super::*; -use gateway_api_types::Progress; - /// A presenter under the production policy, its push, and the recorder. fn wired() -> (Presenter, Recorder) { (Presenter::new(Policy::DEFAULT), recorder()) diff --git a/crates/workshop/gateway/src/resolve-tests.rs b/crates/workshop/gateway/src/resolve-tests.rs index 1b6a4699a..7df81781c 100644 --- a/crates/workshop/gateway/src/resolve-tests.rs +++ b/crates/workshop/gateway/src/resolve-tests.rs @@ -5,7 +5,7 @@ use super::*; use std::io::{Read, Write as _}; use std::net::TcpListener; -use gateway_api_discovery::GatewayDiscoveryFile; +use crate::discovery::GatewayDiscoveryFile; /// The test process's own image name, so the probe's pid and image /// checks pass and the test reaches the liveness probes. @@ -21,7 +21,7 @@ fn own_image_name() -> String { /// A probe running the real liveness gauntlet against the test /// binary's own image. fn probe_own_image(run_dir: &Path) -> Result { - gateway_api_discovery::resolve_for_test(run_dir, &own_image_name()) + crate::discovery::resolve_for_test(run_dir, &own_image_name()) } /// A gateway discovery file pointing at the test process itself. @@ -127,7 +127,7 @@ fn a_stale_file_is_cleaned_and_explicit_config_wins() { assert_eq!(resolved.api_key(), "config-key"); assert_eq!(resolved.stale(), Some(StaleReason::ProcessDead)); assert!( - !gateway_api_discovery::gateway_discovery_file_path(dir.path()).exists(), + !crate::discovery::gateway_discovery_file_path(dir.path()).exists(), "the stale file was removed" ); } @@ -231,7 +231,7 @@ fn no_file_and_explicit_config_uses_the_config() { /// `gateway.json` belongs, so the read errors instead of answering. fn probe_read_failure(run_dir: &Path) -> Result { std::fs::create_dir(run_dir.join("gateway.json")).expect("the unreadable file plants"); - gateway_api_discovery::resolve_for_test(run_dir, &own_image_name()) + crate::discovery::resolve_for_test(run_dir, &own_image_name()) } #[test] diff --git a/crates/workshop/gateway/src/resolve.rs b/crates/workshop/gateway/src/resolve.rs index 53578ec1d..5c8b0fe80 100644 --- a/crates/workshop/gateway/src/resolve.rs +++ b/crates/workshop/gateway/src/resolve.rs @@ -11,7 +11,7 @@ use std::fmt; use std::path::Path; -use gateway_api_discovery::{Resolution, SidecarError, StaleReason, ValidatedConnection}; +use crate::discovery::{Resolution, SidecarError, StaleReason, ValidatedConnection}; use workshop_protocol::Activity; use workshop_registry::Push; @@ -184,15 +184,15 @@ impl ResolveError { /// config names no explicit gateway. pub fn resolve(config: &GatewayConfig) -> Result { resolve_with( - gateway_api_discovery::default_run_dir().as_deref(), + crate::discovery::default_run_dir().as_deref(), config, probe, ) } -/// The production probe: `gateway_api_discovery`'s stale-detecting resolve. +/// The production probe: the fork's stale-detecting resolve. fn probe(run_dir: &Path) -> Result { - gateway_api_discovery::resolve(run_dir) + crate::discovery::resolve(run_dir) } /// `resolve` against an explicit run directory and probe, so tests point @@ -248,7 +248,7 @@ fn resolve_with( /// Reifies the shared resolver's live result as the capability stored in /// the workshop's immutable gateway snapshot. fn validate_resolved( - file: gateway_api_discovery::GatewayDiscoveryFile, + file: crate::discovery::GatewayDiscoveryFile, ) -> Result { ValidatedConnection::validate(file) } diff --git a/crates/workshop/gateway/src/test_gateway-process.rs b/crates/workshop/gateway/src/test_gateway-process.rs index fc1a3fd51..bba435cd9 100644 --- a/crates/workshop/gateway/src/test_gateway-process.rs +++ b/crates/workshop/gateway/src/test_gateway-process.rs @@ -15,7 +15,7 @@ pub(super) fn run() { return; }; let _instance_lease = std::env::var_os(INSTANCE_LEASE_RUN_DIR_ENV).map(|run_dir| { - gateway_api_discovery::GatewayInstanceLease::try_acquire(std::path::Path::new(&run_dir)) + crate::discovery::GatewayInstanceLease::try_acquire(std::path::Path::new(&run_dir)) .expect("acquire named fixture process lease") .expect("the named fixture is the process lease owner") }); diff --git a/crates/workshop/gateway/src/test_gateway.rs b/crates/workshop/gateway/src/test_gateway.rs index e02b82f15..f8a72091b 100644 --- a/crates/workshop/gateway/src/test_gateway.rs +++ b/crates/workshop/gateway/src/test_gateway.rs @@ -7,7 +7,7 @@ use std::net::{TcpListener, TcpStream}; use std::process::{Child, Command, Stdio}; use std::time::{Duration, Instant}; -use gateway_api_discovery::{GatewayDiscoveryFile, ValidatedConnection}; +use crate::discovery::{GatewayDiscoveryFile, ValidatedConnection}; #[path = "test_gateway-process.rs"] mod process; @@ -287,7 +287,7 @@ fn received_shutdown_waits_for_a_delayed_marker() { std::thread::scope(|scope| { scope.spawn(move || { std::thread::sleep(delay); - gateway_api_discovery::request_shutdown(&validated) + crate::discovery::request_shutdown(&validated) .expect("the delayed authenticated shutdown is accepted"); }); assert!( @@ -308,7 +308,7 @@ fn a_named_fixture_releases_its_process_lease_when_terminated() { let run_dir = tempfile::TempDir::new().expect("create lease run directory"); let mut gateway = ValidatedGateway::spawn_with_instance_lease("fixture-key", run_dir.path()); assert!( - gateway_api_discovery::GatewayInstanceLease::try_acquire(run_dir.path()) + crate::discovery::GatewayInstanceLease::try_acquire(run_dir.path()) .expect("contend for the named fixture lease") .is_none(), "the live fixture owns the process lease" @@ -316,7 +316,7 @@ fn a_named_fixture_releases_its_process_lease_when_terminated() { gateway.terminate_within(Duration::from_secs(5)); assert!( - gateway_api_discovery::GatewayInstanceLease::try_acquire(run_dir.path()) + crate::discovery::GatewayInstanceLease::try_acquire(run_dir.path()) .expect("recover the named fixture lease") .is_some(), "the operating system releases the lease when the named fixture dies" diff --git a/crates/workshop/server-api/src/lib.rs b/crates/workshop/server-api/src/lib.rs index e42badf7b..5d8d4d2e8 100644 --- a/crates/workshop/server-api/src/lib.rs +++ b/crates/workshop/server-api/src/lib.rs @@ -18,7 +18,7 @@ pub use workshop_server::{ AgentsConfig, Config, GatewayConfig, GatewayPublicationError, GatewayUpdater, ServerConfig, - ServerHandle, SpawnError, Termination, spawn, + ServerHandle, SpawnError, Termination, discovery, spawn, }; /// The server's integration-test seams, forwarded to the desktop app's tests. diff --git a/crates/workshop/server/Cargo.toml b/crates/workshop/server/Cargo.toml index 54eeee66f..c0bc6705c 100644 --- a/crates/workshop/server/Cargo.toml +++ b/crates/workshop/server/Cargo.toml @@ -39,8 +39,6 @@ reqwest.workspace = true rust-embed.workspace = true serde.workspace = true serde_json.workspace = true -shared-loopback.workspace = true -gateway-api-discovery.workspace = true socket2.workspace = true thiserror.workspace = true tokio.workspace = true @@ -72,10 +70,6 @@ test-fixtures = [ [dev-dependencies] workshop-server = { path = ".", features = ["test-fixtures"] } -# The test-fixtures feature exposes `resolve_for_test`, so the resolution -# tests can run the real liveness gauntlet against the test binary's own -# process image. -gateway-api-discovery = { workspace = true, features = ["test-fixtures"] } tempfile.workspace = true tokio = { workspace = true, features = ["test-util"] } tower.workspace = true diff --git a/crates/workshop/server/src/lib.rs b/crates/workshop/server/src/lib.rs index 45ea95d78..3a907aee5 100644 --- a/crates/workshop/server/src/lib.rs +++ b/crates/workshop/server/src/lib.rs @@ -26,7 +26,8 @@ //! ## Invariants //! //! - Tier: server; may depend on: the vocabulary crates -//! (`workshop-protocol`, `workshop-registry`, `workshop-support`), +//! (`workshop-gateway-discovery`, `workshop-protocol`, +//! `workshop-registry`, `workshop-support`), //! the service crates (`workshop-gateway`, `workshop-menu`, //! `workshop-status`), the feature crates (`workshop-agents`, //! `workshop-run-log`, `workshop-user-state`, `workshop-workspace`), @@ -81,6 +82,7 @@ pub mod fixtures; pub use app::{AppState, router}; pub use serve::{ServerHandle, SpawnError, Termination, spawn}; +pub use workshop_gateway::discovery; pub use workshop_gateway::{GatewayPublicationError, GatewayUpdater, ResolvedGateway}; pub use workshop_protocol::InputResponse; pub use workshop_support::{AgentsConfig, Config, GatewayConfig, ServerConfig}; diff --git a/crates/workshop/server/src/routes/realtime-origin-tests.rs b/crates/workshop/server/src/routes/realtime-origin-tests.rs new file mode 100644 index 000000000..fe66d845f --- /dev/null +++ b/crates/workshop/server/src/routes/realtime-origin-tests.rs @@ -0,0 +1,59 @@ +//! The same-origin rule's cases: exact match, absent Origin, normalization, +//! and every mismatch that must fail closed. + +use super::*; + +#[test] +fn a_native_client_with_a_valid_request_authority_is_admitted() { + assert!(same_origin_authority_allowed(None, Some("127.0.0.1:7910"))); + assert!(!same_origin_authority_allowed(None, None)); + assert!(!same_origin_authority_allowed(None, Some("localhost:bad"))); +} + +#[test] +fn matching_normalized_authorities_are_admitted() { + for (origin, authority) in [ + ("http://127.0.0.1:7910", "127.0.0.1:7910"), + ("http://localhost:7910", "LOCALHOST:7910"), + ("http://[::1]:7910", "[::1]:7910"), + ("http://[0:0:0:0:0:0:0:1]:7910", "[::1]:7910"), + ] { + assert!( + same_origin_authority_allowed(Some(origin), Some(authority)), + "{origin} must match {authority}" + ); + } +} + +#[test] +fn mismatches_wrong_ports_and_malformed_values_are_refused() { + for (origin, authority) in [ + ("http://127.0.0.1:7910", "localhost:7910"), + ("http://localhost:7910", "localhost:7911"), + ("http://[0:0:0:0:0:0:0:1]:7910", "[::1]:7911"), + ("http://[0:0:0:0:0:0:0:2]:7910", "[::1]:7910"), + ("http://evil.example:7910", "localhost:7910"), + ("https://localhost:7910", "localhost:7910"), + ("http://localhost:bad", "localhost:7910"), + ("http://localhost:7910/path", "localhost:7910"), + ("null", "localhost:7910"), + ("", "localhost:7910"), + ] { + assert!( + !same_origin_authority_allowed(Some(origin), Some(authority)), + "{origin} must not match {authority}" + ); + } +} + +#[test] +fn a_userinfo_bearing_authority_is_refused() { + assert!(!same_origin_authority_allowed( + Some("http://user@localhost:7910"), + Some("localhost:7910") + )); + assert!(!same_origin_authority_allowed( + Some("http://localhost:7910"), + Some("user@localhost:7910") + )); +} diff --git a/crates/workshop/server/src/routes/realtime-origin.rs b/crates/workshop/server/src/routes/realtime-origin.rs new file mode 100644 index 000000000..4d0826983 --- /dev/null +++ b/crates/workshop/server/src/routes/realtime-origin.rs @@ -0,0 +1,96 @@ +//! Same-origin policy for the Workshop Realtime socket. +//! +//! A browser `Origin` on `/v1/realtime` must match the request's own +//! authority: same host (IP compared by value, domain compared ASCII +//! case-insensitively) and the same explicit port. An absent `Origin` +//! denotes a native client, which is admitted as long as the request +//! authority is present and valid. Missing or malformed values fail closed. +//! +//! The cross-site guard is the security boundary for the loopback socket +//! as a whole; this is the stricter rule for the one route a browser page +//! reaches with credentials, and it is deliberately narrower than the +//! guard. + +use axum::http::uri::Authority; + +/// Whether a Workshop WebSocket `Origin` matches its request authority. +/// +/// An absent `Origin` denotes a native client, but the request authority +/// must still be present and valid. A browser `Origin` must be an exact +/// HTTP origin whose normalized authority equals the validated request +/// authority. Missing or malformed values and host or port mismatches fail +/// closed. +pub(super) fn same_origin_authority_allowed( + origin: Option<&str>, + request_authority: Option<&str>, +) -> bool { + let Some(request_authority) = request_authority.and_then(parse_authority) else { + return false; + }; + origin.is_none_or(|origin| { + parse_http_origin_authority(origin).is_some_and(|origin_authority| { + same_origin_authority(&origin_authority, &request_authority) + }) + }) +} + +/// Compares normalized hosts while preserving explicit port equality. +fn same_origin_authority(left: &Authority, right: &Authority) -> bool { + left.port_u16() == right.port_u16() && same_authority_host(left.host(), right.host()) +} + +/// Compares IP hosts by value and domain hosts ASCII case-insensitively. +fn same_authority_host(left: &str, right: &str) -> bool { + let parse_ip = |host: &str| { + host.strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host) + .parse::() + .ok() + }; + match (parse_ip(left), parse_ip(right)) { + (Some(left), Some(right)) => left == right, + (None, None) => left.eq_ignore_ascii_case(right), + _ => false, + } +} + +/// Parses an exact HTTP origin and returns its authority. +fn parse_http_origin_authority(origin: &str) -> Option { + let (scheme, authority) = origin.split_once("://")?; + if !scheme.eq_ignore_ascii_case("http") { + return None; + } + parse_authority(authority) +} + +/// Parses an authority and rejects ports outside the `u16` range. +fn parse_authority(authority: &str) -> Option { + let port = if let Some(bracketed) = authority.strip_prefix('[') { + let close = bracketed.find(']')?; + match &bracketed[close + 1..] { + "" => None, + suffix => Some(suffix.strip_prefix(':')?), + } + } else { + match authority.split_once(':') { + Some((host, port)) if !host.is_empty() && !port.contains(':') => Some(port), + Some(_) => return None, + None => None, + } + }; + if authority.contains('@') + || port.is_some_and(|port| port.is_empty() || port.parse::().is_err()) + { + return None; + } + let authority = authority.parse::().ok()?; + if authority.host().is_empty() { + return None; + } + Some(authority) +} + +#[cfg(test)] +#[path = "realtime-origin-tests.rs"] +mod tests; diff --git a/crates/workshop/server/src/routes/realtime.rs b/crates/workshop/server/src/routes/realtime.rs index ccd71b9e3..78bbb9d70 100644 --- a/crates/workshop/server/src/routes/realtime.rs +++ b/crates/workshop/server/src/routes/realtime.rs @@ -1,4 +1,9 @@ //! Same-origin, payload-opaque relay for Gateway Realtime transcription. +//! +//! The one route a browser page reaches with credentials gets a stricter +//! Origin rule than the cross-site guard applies to the rest of the socket: +//! a browser `Origin` must match the request's own authority. That rule +//! lives in the private `origin` child module. use std::time::Duration; @@ -16,6 +21,9 @@ use tokio_tungstenite::tungstenite::protocol::CloseFrame as GatewayCloseFrame; use crate::app::AppState; use workshop_gateway::GatewayRealtimeSocket; +#[path = "realtime-origin.rs"] +mod origin; + const RELAY_IO_DEADLINE: Duration = Duration::from_millis(500); /// The Workshop endpoint mirroring Gateway Realtime transcription. @@ -58,9 +66,8 @@ fn same_origin_allowed(uri: &Uri, headers: &HeaderMap) -> bool { Err(()) => return false, }, }; - shared_loopback::workshop_same_origin_authority_allowed(origin, authority) + origin::same_origin_authority_allowed(origin, authority) } - fn single_header(headers: &HeaderMap, name: header::HeaderName) -> Result, ()> { let mut values = headers.get_all(name).iter(); let Some(first) = values.next() else { diff --git a/crates/workshop/server/src/serve.rs b/crates/workshop/server/src/serve.rs index a262f7d98..130305e74 100644 --- a/crates/workshop/server/src/serve.rs +++ b/crates/workshop/server/src/serve.rs @@ -50,7 +50,7 @@ pub enum Termination { pub struct ServerHandle { url: String, gateway: GatewayUpdater, - initial_gateway_identity: Option, + initial_gateway_identity: Option, shutdown: Option>, stopped: mpsc::Receiver, thread: Option>>, @@ -75,7 +75,9 @@ impl ServerHandle { /// Returns the validated local Gateway identity initially published into /// server state, or `None` when explicit configuration won resolution. #[must_use] - pub fn initial_gateway_identity(&self) -> Option<&gateway_api_discovery::ValidatedConnection> { + pub fn initial_gateway_identity( + &self, + ) -> Option<&workshop_gateway::discovery::ValidatedConnection> { self.initial_gateway_identity.as_ref() } diff --git a/crates/workshop/ui/test/realtime-wire-fixtures.mjs b/crates/workshop/ui/test/realtime-wire-fixtures.mjs index 302d34f4d..d512e32f4 100644 --- a/crates/workshop/ui/test/realtime-wire-fixtures.mjs +++ b/crates/workshop/ui/test/realtime-wire-fixtures.mjs @@ -32,9 +32,7 @@ const fixtureDir = path.join( const minimumCommitAudioBytes = (24_000 * 2) / 10; const hypothesisInclude = "item.input_audio_transcription.hypothesis"; -const rangesInclude = "item.input_audio_transcription.hypothesis.ranges"; const negotiableIncludes = [[], [hypothesisInclude]]; -const rangeFields = ["finalized_seq", "finalized_through_ms"]; async function fixture(name) { const parsed = JSON.parse(await readFile(path.join(fixtureDir, name), "utf8")); @@ -91,24 +89,6 @@ function pathName(path) { return path.map(String).join("."); } -function carriesRanges(event) { - if (event.type === "session.created" || event.type === "session.updated") { - return event.session.include.includes(rangesInclude); - } - return rangeFields.some((field) => field in event); -} - -function withoutRangesInclude(session) { - return { ...session, include: session.include.filter((value) => value !== rangesInclude) }; -} - -function withoutRanges(event) { - const base = structuredClone(event); - if ("session" in base) base.session = withoutRangesInclude(base.session); - for (const field of rangeFields) delete base[field]; - return base; -} - function isOptionalErrorField(path) { return ( path.length >= 2 && @@ -296,17 +276,15 @@ test("canonical Realtime event fixtures satisfy the browser wire contract", asyn ); const sessions = await fixture("effective-sessions.json"); - assert.deepEqual(sortedKeys(sessions), ["default", "ranges", "updated"]); + assert.deepEqual(sortedKeys(sessions), ["default", "updated"]); assertSession(sessions.default, "default session"); assertSession(sessions.updated, "updated session"); - assert.deepEqual(sessions.ranges.include, [hypothesisInclude, rangesInclude]); - assertSession(withoutRangesInclude(sessions.ranges), "ranges session without ranges"); const servers = await fixture("server-events.json"); for (const [name, event] of Object.entries(servers)) { assertNonemptyString(event.event_id, `${name}.event_id`); assertNonemptyString(event.type, `${name}.type`); - assertServerEventFields(carriesRanges(event) ? withoutRanges(event) : event, name); + assertServerEventFields(event, name); } assert.deepEqual(servers.session_created.session, sessions.default); assert.deepEqual(servers.session_updated.session, sessions.updated); @@ -327,32 +305,17 @@ test("canonical Realtime event fixtures satisfy the browser wire contract", asyn assert.ok(hypothesis.audio_end_ms >= hypothesis.audio_start_ms); }); -test("the production decoder rejects range fields and the ranges include", async () => { +test("the production decoder rejects an include the session did not negotiate", async () => { const servers = await fixture("server-events.json"); - const ranged = Object.keys(servers).filter((name) => carriesRanges(servers[name])); - assert.deepEqual(ranged, ["transcription_hypothesis_ranges"]); - for (const name of ranged) { - const event = servers[name]; - assert.equal(decodeRealtimeEvent(event), null, `${name} is rejected`); - const base = withoutRanges(event); - assert.deepEqual(decodeRealtimeEvent(base), base, `${name} without ranges decodes unchanged`); - for (const field of rangeFields) { - const lone = structuredClone(event); - delete lone[field]; - assert.equal(decodeRealtimeEvent(lone), null, `${name} rejects a lone range field`); - } - } - const sessions = await fixture("effective-sessions.json"); for (const include of [ - sessions.ranges.include, - [rangesInclude], - [rangesInclude, hypothesisInclude], + ["item.input_audio_transcription.logprobs"], + [hypothesisInclude, "item.input_audio_transcription.logprobs"], ]) { assert.equal( decodeRealtimeEvent({ ...servers.session_updated, - session: { ...sessions.ranges, include }, + session: { ...sessions.updated, include }, }), null, `include ${JSON.stringify(include)} is rejected`, @@ -363,7 +326,6 @@ test("the production decoder rejects range fields and the ranges include", async test("the production decoder rejects every canonical field mutation", async () => { const servers = await fixture("server-events.json"); for (const [name, event] of Object.entries(servers)) { - if (carriesRanges(event)) continue; assert.deepEqual(decodeRealtimeEvent(event), event, `${name} decodes unchanged`); for (const fieldPath of fieldPaths(event)) { const mutated = structuredClone(event); @@ -471,7 +433,6 @@ test("the production decoder rejects every canonical field mutation", async () = test("canonical Realtime sequences cover every frozen contract path", async () => { const valid = await fixture("valid-sequences.json"); - const rangedSequences = new Set(); for (const [name, sequence] of Object.entries(valid)) { assertExactKeys(sequence, ["events", "invariants"], name); assert.ok(sequence.events.length > 0, `${name} has events`); @@ -484,33 +445,16 @@ test("canonical Realtime sequences cover every frozen contract path", async () = assertNonemptyString(entry.message.event_id, `${name} event ID`); } if (entry.direction === "server") { - if (carriesRanges(entry.message)) { - rangedSequences.add(name); - const base = withoutRanges(entry.message); - assertServerEventFields(base, `${name} server event without ranges`); - assert.equal( - decodeRealtimeEvent(entry.message), - null, - `${name} range server event is rejected`, - ); - assert.deepEqual( - decodeRealtimeEvent(base), - base, - `${name} server event without ranges decodes`, - ); - } else { - assertServerEventFields(entry.message, `${name} server event`); - assert.deepEqual( - decodeRealtimeEvent(entry.message), - entry.message, - `${name} server event decodes`, - ); - } + assertServerEventFields(entry.message, `${name} server event`); + assert.deepEqual( + decodeRealtimeEvent(entry.message), + entry.message, + `${name} server event decodes`, + ); } } assertValidCommitAudio(name, sequence.events); } - assert.deepEqual([...rangedSequences], ["hypothesis_ranges_negotiation"]); assert.deepEqual( valid.hypothesis_negotiation.events .filter( diff --git a/crates/workshop/ui/test/stt-stream.mjs b/crates/workshop/ui/test/stt-stream.mjs index 5dcf50f6c..8fba4d2c2 100644 --- a/crates/workshop/ui/test/stt-stream.mjs +++ b/crates/workshop/ui/test/stt-stream.mjs @@ -1027,7 +1027,7 @@ await assertNoLeaks(lifecycle, async () => { assert.deepEqual( sockets[0].sent, [client.session_update], - "the production service requests hypotheses without finalized ranges", + "the production service requests hypotheses", ); sockets[0].message(server.session_updated); assert.equal(service.state, "ready"); @@ -1087,13 +1087,17 @@ await assertNoLeaks(lifecycle, async () => { const errors = []; service.onEvent((value) => events.push(value)); service.onError((value) => errors.push(value)); - const rangesSessionUpdated = { + const unnegotiatedSessionUpdated = { ...server.session_updated, session: { ...server.session_updated.session, - include: client.session_update_ranges.session.include, + include: [ + ...client.session_update.session.include, + "item.input_audio_transcription.logprobs", + ], }, }; + const malformedHypothesis = { ...server.transcription_hypothesis, revision: "3" }; const invalidEventGenerations = () => errors .filter(({ code }) => code === "invalid_server_event") @@ -1108,24 +1112,24 @@ await assertNoLeaks(lifecycle, async () => { assert.deepEqual( socket.sent.map(({ session }) => session.include), [client.session_update.session.include], - `generation ${generation} requests hypotheses without ranges`, + `generation ${generation} requests hypotheses`, ); - socket.message(rangesSessionUpdated); - assert.equal(service.state, "connecting", "the two-token effective session is rejected"); + socket.message(unnegotiatedSessionUpdated); + assert.equal(service.state, "connecting", "a session with an unnegotiated include is rejected"); socket.message(server.session_updated); assert.equal(service.state, "ready"); socket.message(server.transcription_delta); - socket.message(server.transcription_hypothesis_ranges); + socket.message(malformedHypothesis); socket.message(server.transcription_hypothesis); assert.deepEqual( events.map(({ event }) => event), [server.session_created, server.session_updated, server.transcription_hypothesis], - `generation ${generation} publishes no range fields and drops deltas`, + `generation ${generation} publishes the valid events and drops deltas`, ); assert.deepEqual( invalidEventGenerations(), Array.from({ length: generation }, (_, earlier) => [earlier + 1, earlier + 1]).flat(), - `generation ${generation} reports the ranges session and the range event as invalid`, + `generation ${generation} reports the unnegotiated session and the malformed hypothesis as invalid`, ); if (index === 0) { socket.close();