diff --git a/Cargo.lock b/Cargo.lock index 3728ccfda..11ca21f02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1333,9 +1333,9 @@ dependencies = [ [[package]] name = "ctutils" -version = "0.4.2" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +checksum = "38d28fb6aa7dd680b3ffd1a3e64e115c177bdba63c0ae7f9cd48129ff05c4fa0" dependencies = [ "cmov", ] @@ -6985,9 +6985,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.53.1" +version = "1.54.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "202caea871b69668250d242070849eb495be178ed697a3e98aebce5bc81a0bed" +checksum = "b52fd3255258d26fac7264ab5a76b53c13367136dc011d3d6ad4f8ed070396a3" dependencies = [ "bytes", "libc", diff --git a/crates/gateway/app/Cargo.toml b/crates/gateway/app/Cargo.toml index 1f2497f78..8c5106dd8 100644 --- a/crates/gateway/app/Cargo.toml +++ b/crates/gateway/app/Cargo.toml @@ -91,10 +91,13 @@ tracing-subscriber = { workspace = true, features = ["env-filter"] } # First-run STT gating reads the installer's registry choice (src/boot.rs): # RegOpenKeyExW + RegQueryValueExW. The Windows tray backend (src/tray/) adds # the Run-key write shim and the hidden-window message loop; main() adds the -# PerMonitorV2 DPI-awareness call. +# PerMonitorV2 DPI-awareness call. Win32_Graphics_Gdi gates the message +# window's RegisterClassW and WNDCLASSW; it must be declared here rather than +# arrive by feature unification from tray-icon's copy of windows-sys. [target.'cfg(target_os = "windows")'.dependencies] windows-sys = { workspace = true, features = [ "Win32_Foundation", + "Win32_Graphics_Gdi", "Win32_System_LibraryLoader", "Win32_System_Registry", "Win32_UI_HiDpi", diff --git a/crates/gateway/app/src/cli-tests.rs b/crates/gateway/app/src/cli-tests.rs index 50294befd..ab1a79844 100644 --- a/crates/gateway/app/src/cli-tests.rs +++ b/crates/gateway/app/src/cli-tests.rs @@ -366,3 +366,40 @@ fn the_help_text_lists_all_flags() { assert!(help.contains(flag), "the help names {flag}: {help}"); } } + +#[test] +fn preferred_port_parses_and_reaches_the_serve_options() { + let options = AppOptions::from_args(args(&["--preferred-port", "8081"])).expect("should parse"); + + assert_eq!(options.serve.preferred_port, Some(8081)); +} + +#[test] +fn a_launch_has_no_preferred_port_by_default() { + let options = AppOptions::from_args(args(&[])).expect("the bare invocation should parse"); + + assert_eq!(options.serve.preferred_port, None); +} + +#[test] +fn preferred_port_requires_a_value() { + assert_eq!(error_kind(&["--preferred-port"]), ErrorKind::InvalidValue); +} + +#[test] +fn preferred_port_rejects_a_value_that_is_not_a_port() { + for value in ["http", "65536", "70000"] { + assert_eq!( + error_kind(&["--preferred-port", value]), + ErrorKind::ValueValidation, + "--preferred-port {value} is not a port" + ); + } +} + +#[test] +fn the_help_text_hides_the_preferred_port_flag() { + let help = Cli::command().render_long_help().to_string(); + + assert!(!help.contains("--preferred-port"), "{help}"); +} diff --git a/crates/gateway/app/src/cli.rs b/crates/gateway/app/src/cli.rs index 921cda290..81f5267c0 100644 --- a/crates/gateway/app/src/cli.rs +++ b/crates/gateway/app/src/cli.rs @@ -60,7 +60,8 @@ impl AppOptions { // browser - so it wins over `--browser`. serve: ServeOptions::new(config_path, cli.profile) .with_browser(cli.browser && !cli.login) - .with_supervised(cli.supervised), + .with_supervised(cli.supervised) + .with_preferred_port(cli.preferred_port), tray: !cli.no_tray, login: cli.login, print_url: cli.print_url, @@ -133,6 +134,11 @@ struct Cli { /// The launch is a supervised child; internal only #[arg(long, hide = true, conflicts_with = "print_url")] supervised: bool, + + /// The port to bind when the config asks for any free port. Best-effort only, + /// if the specified port is not available it falls back to any free port. + #[arg(long, value_name = "PORT", hide = true)] + preferred_port: Option, } #[derive(Debug, Subcommand)] diff --git a/crates/gateway/app/src/runner.rs b/crates/gateway/app/src/runner.rs index b64d474c7..ecde2c3f8 100644 --- a/crates/gateway/app/src/runner.rs +++ b/crates/gateway/app/src/runner.rs @@ -21,6 +21,7 @@ use gateway_config::{Config, ProfileName, Secret}; use crate::api_error::StartupError; use crate::{AppState, ShutdownReason}; +mod bind; mod gateway; mod thread; @@ -65,48 +66,54 @@ pub struct ServeOptions { /// profile's `.runvil` directory) and, when nothing is found, /// first-run generation into the profile location. pub config_path: Option, + /// Optional command-line profile override. pub profile: Option, + /// Directory the `gateway.json` gateway discovery file is written to after a /// successful bind; `None` uses the default run directory under the /// user profile's `.runvil` directory. pub run_dir: Option, + /// Opens the Settings handoff URL in the default browser once the /// listener is bound - the binary's `--browser`, used by the /// installer's first run. Embedders leave this `false`. pub browser: bool, + /// Run the gateway as the child of a supervisor that handles restarting logic. /// `POST /admin/restart` is available. pub supervised: bool, + + /// When running under a config that allows the gateway to bind to any free + /// port, it will prefer this one. + pub preferred_port: Option, } impl ServeOptions { /// Builds serve options from the config path and optional profile override. #[must_use] - pub fn new( - config_path: Option, - profile: impl Into>, - ) -> ServeOptions { - ServeOptions { + pub fn new(config_path: Option, profile: impl Into>) -> Self { + Self { config_path, profile: profile.into(), run_dir: None, browser: false, supervised: false, + preferred_port: None, } } /// Sets the gateway discovery file's run directory, for tests and portable /// installs; the default is the user profile's `.runvil/run`. #[must_use] - pub fn with_run_dir(mut self, run_dir: PathBuf) -> ServeOptions { + pub fn with_run_dir(mut self, run_dir: PathBuf) -> Self { self.run_dir = Some(run_dir); self } /// Sets whether to open the Settings page in the browser once bound. #[must_use] - pub fn with_browser(mut self, browser: bool) -> ServeOptions { + pub fn with_browser(mut self, browser: bool) -> Self { self.browser = browser; self } @@ -114,10 +121,18 @@ impl ServeOptions { /// Sets whether this gateway runs with a supervisor which can relaunch it /// on a restart request. #[must_use] - pub fn with_supervised(mut self, supervised: bool) -> ServeOptions { + pub fn with_supervised(mut self, supervised: bool) -> Self { self.supervised = supervised; self } + + /// Sets a preferred port to bind when the config asks for any free port. + /// Best-effort only, if the port is not available falls back to any free port. + #[must_use] + pub fn with_preferred_port(mut self, port: Option) -> Self { + self.preferred_port = port; + self + } } /// Optional admin configuration path plus the active profile name. diff --git a/crates/gateway/app/src/runner/bind-tests.rs b/crates/gateway/app/src/runner/bind-tests.rs new file mode 100644 index 000000000..44579960a --- /dev/null +++ b/crates/gateway/app/src/runner/bind-tests.rs @@ -0,0 +1,124 @@ +use std::net::SocketAddr; + +use super::*; + +/// A get a port that was is not in use. +fn free_port() -> u16 { + std::net::TcpListener::bind("127.0.0.1:0") + .expect("should bind a probe") + .local_addr() + .expect("address should exist") + .port() +} + +fn loopback(port: u16) -> SocketAddr { + SocketAddr::from(([127, 0, 0, 1], port)) +} + +/// create a tokio Runtime for a test. +fn runtime() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_io() + .build() + .expect("a runtime builds") +} + +#[test] +fn a_port_zero_config_binds_the_preferred_port() { + let runtime = runtime(); + + let preferred_port = free_port(); + + let listener = + bind_listener(loopback(0), Some(preferred_port), runtime.handle()).expect("should bind"); + + let listener_port = listener + .local_addr() + .expect("should have an address") + .port(); + + assert_eq!(listener_port, preferred_port); +} + +#[test] +fn a_configured_port_wins_over_the_preferred_port() { + let runtime = runtime(); + + let configured_port = free_port(); + let preferred_port = free_port(); + + let listener = bind_listener( + loopback(configured_port), + Some(preferred_port), + runtime.handle(), + ) + .expect("should bind"); + + let listener_port = listener + .local_addr() + .expect("should have an address") + .port(); + + assert_eq!(listener_port, configured_port); +} + +#[test] +fn a_taken_preferred_port_falls_back_to_a_free_one() { + let runtime = runtime(); + + let holder = std::net::TcpListener::bind("127.0.0.1:0").expect("should bind"); + let used_port = holder.local_addr().expect("should have an address").port(); + + let listener = + bind_listener(loopback(0), Some(used_port), runtime.handle()).expect("should bind"); + + let listener_port = listener + .local_addr() + .expect("should have an address") + .port(); + + assert_ne!(listener_port, used_port); +} + +#[test] +fn no_preferred_port_binds_a_free_port() { + let runtime = runtime(); + + let listener = bind_listener(loopback(0), None, runtime.handle()).expect("binds"); + + let listener_port = listener + .local_addr() + .expect("should have an address") + .port(); + + assert_ne!(listener_port, 0); +} + +/// The server side closing a connection first leaves it in `TIME_WAIT`, +/// which is what a gateway shutting down with clients attached does. +#[tokio::test] +async fn a_released_port_rebinds_while_its_connection_is_in_time_wait() { + let runtime = tokio::runtime::Handle::current(); + + let listener = bind_listener(loopback(0), None, &runtime).expect("should bind"); + + let port = listener + .local_addr() + .expect("should have an address") + .port(); + + let client = tokio::net::TcpStream::connect(loopback(port)) + .await + .expect("should connect"); + let (served, _) = listener.accept().await.expect("should accept"); + drop(served); + drop(listener); + drop(client); + + let rebound = bind_listener(loopback(0), Some(port), &runtime).expect("should rebind"); + + assert_eq!( + rebound.local_addr().expect("should have an address").port(), + port + ); +} diff --git a/crates/gateway/app/src/runner/bind.rs b/crates/gateway/app/src/runner/bind.rs new file mode 100644 index 000000000..b84adf337 --- /dev/null +++ b/crates/gateway/app/src/runner/bind.rs @@ -0,0 +1,72 @@ +//! Binding the gateway's listener to: +//! - The configured address, if it is set +//! - The preferred port if it is set and available +//! - Any available port + +use std::net::SocketAddr; + +use tokio::{ + net::{TcpListener, TcpSocket}, + runtime::EnterGuard, +}; + +const MAX_BACKLOG_CONNECTIONS: u32 = 1024; + +/// Binds the gateway's listener to `addr`. +/// +/// When `addr` asks for any free port (port 0) and `preferred_port` names one, +/// that port is tried first, enabling a gateway relaunched by its supervisor to +/// come back on the address its clients already hold. If the preferred port is +/// taken, a free port is bound instead and a warning is logged. `preferred_port` is +/// ignored if `addr` asks for a port other than 0. +pub(super) fn bind_listener( + addr: SocketAddr, + preferred_port: Option, + runtime: &tokio::runtime::Handle, +) -> std::io::Result { + let context = runtime.enter(); + + if addr.port() == 0 + && let Some(preferred_port) = preferred_port + { + let preferred_addr = SocketAddr::new(addr.ip(), preferred_port); + + match bind_addr(preferred_addr, &context) { + Ok(listener) => return Ok(listener), + Err(error) => tracing::warn!( + "could not bind the preferred port {preferred_port}: {error}; binding a free port instead" + ), + } + } + + bind_addr(addr, &context) +} + +/// Binds `address`, reusing a port even if its previous connections are still in +/// `TIME_WAIT`, so a restarted gateway can take back the port it just released. +/// +/// Must run inside a tokio runtime context, which the caller supplies via the +/// `context` param +fn bind_addr(address: SocketAddr, _context: &EnterGuard<'_>) -> std::io::Result { + let socket = if address.is_ipv4() { + TcpSocket::new_v4()? + } else { + TcpSocket::new_v6()? + }; + + // Unix only: on Windows `SO_REUSEADDR` lets a second socket take a port that is in use + #[cfg(unix)] + socket.set_reuseaddr(true)?; + + socket.bind(address)?; + + // backlog connections defines the maximum number of pending connections + // queued by the operating system at any given time. Connection are removed + // from the queue with `TcpListener::accept`. When the queue is full, the + // operating-system will start rejecting connections. + socket.listen(MAX_BACKLOG_CONNECTIONS) +} + +#[cfg(test)] +#[path = "bind-tests.rs"] +mod tests; diff --git a/crates/gateway/app/src/runner/thread.rs b/crates/gateway/app/src/runner/thread.rs index 70096e01f..501d9a0fa 100644 --- a/crates/gateway/app/src/runner/thread.rs +++ b/crates/gateway/app/src/runner/thread.rs @@ -6,8 +6,6 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, mpsc}; use std::thread::JoinHandle; -use tokio::net::TcpListener; - use gateway_config::{Config, Secret}; use super::{ @@ -224,7 +222,7 @@ fn serve_thread( return Ok(()); } }; - let bind = config.bind_addr(); + let bind_addr = config.bind_addr(); // Producers own their own log lines; the hub feeds the UIs only. let hub = Arc::new(gateway_progress::ProgressHub::new()); let runtime = match tokio::runtime::Builder::new_multi_thread() @@ -240,13 +238,14 @@ fn serve_thread( // The bind runs on this plain thread, apart from the serve future, so // the bound address is known before serving starts: the readiness // signal and the gateway discovery file both report the real port. - let listener = match runtime.block_on(TcpListener::bind(bind)) { - Ok(listener) => listener, - Err(error) => { - let _ = ready.send(Err(StartupError::bind(error))); - return Ok(()); - } - }; + let listener = + match super::bind::bind_listener(bind_addr, options.preferred_port, runtime.handle()) { + Ok(listener) => listener, + Err(error) => { + let _ = ready.send(Err(StartupError::bind(error))); + return Ok(()); + } + }; // The configured bind may specify port 0; the bound local_addr is the // real address the readiness signal must report. let address = match listener.local_addr() { diff --git a/vibe/2026-10/2026-10-06-1-restart-to-apply.md b/vibe/2026-10/2026-10-06-1-restart-to-apply.md index 373953dd0..bc42d47a9 100644 --- a/vibe/2026-10/2026-10-06-1-restart-to-apply.md +++ b/vibe/2026-10/2026-10-06-1-restart-to-apply.md @@ -8,9 +8,9 @@ todos: - id: restart-contract content: "Child side: --supervised flag, stdin-EOF watchdog, `RESTART_REQUESTED` exit code, Apply keeps .prev (first since boot wins), startup removes .prev, POST /admin/restart (restart only), restart_required on GET /admin/config-dirty" status: completed - - id: bind-hint - content: "--port-hint for a config that binds port 0; bind through TcpSocket with SO_REUSEADDR on Unix" - status: pending + - id: preferred-port + content: "--preferred-port for a config that binds port 0; bind through TcpSocket with SO_REUSEADDR on Unix" + status: completed - id: supervisor-headless content: "Supervisor mode: launch election, child spawn, exit-code loop, retries for restarts only (capped), .prev rollback, failure record; integration tests against the real binary" status: pending @@ -64,7 +64,7 @@ What stays the same: the tray icon (it belongs to the gateway, so it disappears flowchart TD Launch["runvil-gateway (bare, --login, --no-tray)"] --> Sup[supervisor] Desktop[workshop desktop supervisor] -->|"bare launch, unchanged"| Sup - Sup -->|"spawn --supervised --port-hint P, stdin pipe"| Child["serving gateway (tray unless --no-tray)"] + Sup -->|"spawn --supervised --preferred-port P, stdin pipe"| Child["serving gateway (tray unless --no-tray)"] Child -->|"exit 0"| SupExit[supervisor exits] Child -->|"exit RESTART_REQUESTED"| Restart[relaunch, roll back on failure] Child -->|"any other code, or first boot fails"| Exit[supervisor exits with the child's code] @@ -84,7 +84,7 @@ sequenceDiagram C1-->>UI: 202 C1->>C1: graceful shutdown, exit RESTART_REQUESTED S->>S: hold launch lock - S->>C2: spawn --supervised --port-hint P + S->>C2: spawn --supervised --preferred-port P alt C2 publishes a validated discovery file in time C2->>C2: remove .prev after publishing S->>S: release launch lock @@ -152,17 +152,17 @@ Fixes the reproduced Secrets-then-Cloud-Models failure in Problem: validation of This step only adds the option; nothing passes it yet. Step 4's supervisor is its caller, giving each relaunched child the port the previous one bound. -- `--port-hint PORT` (supervised only): when the config's bind port is 0, bind that port on the configured address; when that bind fails, fall back to port 0 and log it. A non-zero configured port ignores the hint. -- `serve_thread` binds through `tokio::net::TcpSocket` with `set_reuseaddr(true)` on Unix so a port with connections in `TIME_WAIT` rebinds at once, matching the workshop server (`crates/workshop/server/src/serve.rs:346-357`). Windows keeps the default; `SO_REUSEADDR` there allows port stealing. -- Tests: hint honored for port 0, ignored for a fixed port, fallback when the hinted port is held; rebind immediately after a served connection closes. -- Commit: `Add a --port-hint option for a port-0 bind` +- `--preferred-port PORT`, a clap argument in `cli.rs` independent of `--supervised` and hidden from `--help` like it (a public, revised form is a follow-up): when the config's bind port is 0, bind that port on the configured address; when that bind fails, fall back to port 0 and log it. A non-zero configured port ignores the preferred port. +- `ServeOptions::preferred_port` (`with_preferred_port`) carries it to `serve_thread`, which binds through `runner/bind.rs`'s `bind_listener(addr, preferred_port, runtime)`, taking the runtime handle it registers the listener with. It binds through `tokio::net::TcpSocket` with `set_reuseaddr(true)` on Unix so a port with connections in `TIME_WAIT` rebinds at once, matching the workshop server (`crates/workshop/server/src/serve.rs:346-357`). Windows keeps the default; `SO_REUSEADDR` there allows port stealing. +- Tests: the preferred port honored for port 0, ignored for a fixed port, fallback when the preferred port is held; rebind immediately after a served connection closes. +- Commit: `Add a --preferred-port option for a port-0 bind` ## Step 4: Supervisor mode - New `src/supervisor.rs` + `src/supervisor/` (spawn, loop, rollback): entered for a bare launch, `--login`, and `--no-tray`. The supervisor has no tray and no window in any mode. - Start: `launch_or_attach` in the run directory. `Attach` hands off as `relaunch.rs` does today. `Launch(lock)` spawns the child and holds the lock until the child publishes. - `--print-url` launches stay single-process and unsupervised: a terminal-driven mode with no tray, where Restart to Apply falls back to today's Apply and banner. -- Child spawn: `current_exe()` with `--supervised`; `--no-tray`, `--config`, and `--profile` passed through when the supervisor received them; `--browser` passed to the first child only, so the installer's first run still opens Settings and restarts never do; and `--port-hint` after the first boot (read from the discovery file). Stdin piped, stdout null, stderr piped and retained (last 64 lines) for the failure record. Windows adds `CREATE_NO_WINDOW` like `production_command` (`gateway/local/src/server-support.rs:102-106`). +- Child spawn: `current_exe()` with `--supervised`; `--no-tray`, `--config`, and `--profile` passed through when the supervisor received them; `--browser` passed to the first child only, so the installer's first run still opens Settings and restarts never do; and `--preferred-port` after the first boot (read from the discovery file). Stdin piped, stdout null, stderr piped and retained (last 64 lines) for the failure record. Windows adds `CREATE_NO_WINDOW` like `production_command` (`gateway/local/src/server-support.rs:102-106`). - Loop on child exit: - 0: the supervisor exits 0. - `RESTART_REQUESTED`: take the launch lock and relaunch. A launch fails when the child exits or does not publish within 30 s. The first failure with `.prev` files present rolls back as in Decisions; every failure relaunches after a backoff from 250 ms doubling, at most 5 launches per restart. Success (the child, having published, has already removed `.prev`) releases the lock and resets the count. @@ -204,6 +204,7 @@ This step only adds the option; nothing passes it yet. Step 4's supervisor is it - **Workshop profile switch.** `workshop_socket-menu.rs:228-248` restarts a sidecar with `request_shutdown`. With this plan that stops the whole supervisor, and the desktop's 5 s probe relaunches it, so it keeps working. A follow-up switches it to `POST /admin/restart`, which is faster and keeps the supervisor up; it touches workshop crates, so it is not part of this plan. - **Tray in the supervisor.** Only if the brief icon gap during a restart proves a problem (see the tray decision). - **Env-only Apply reloads the routing table.** Step 1 made a reload resolve `${VAR}` from the pending env file, but `capture_apply` reloads only when a `gateway.toml.next` exists: an Apply that changes `gateway.env` alone takes the inline path, so a new key for a remote endpoint waits for a restart, while the same key applied together with a config change works at once. Treating an env shadow as a reason to reload makes both cases live immediately for remote endpoints; `restart_required` stays true for an env shadow, because `HF_TOKEN` and the other direct readers of the process environment change only at boot. Matters only for unsupervised gateways and the moment between Apply and the restart in Restart to Apply. +- **A public port option.** `--preferred-port` ships hidden, with the supervisor as its only caller. A revised form is meant to become a documented user-facing option; its shape is decided then, and unhiding it adds it to the docs. - **`PendingEnv` in `gateway-config`.** Moving the pending-env reader into the config crate would let `save_config_shadow` and `load_pending_config` build it themselves and drop the public `lookup` parameter and `VarLookupFn`, at the cost of a `dotenvy` dependency and reversing the dominion refactor's no-env-file rule for the config crate. ## Constraints