From 3e57249d85050645320541d12855eff4f1f4d17d Mon Sep 17 00:00:00 2001 From: Andrew Wheeler Date: Wed, 28 Oct 2026 10:45:38 -0600 Subject: [PATCH 1/2] Gateway - Move AppOptions to lib Move `AppOptions` and `LaunchCommand` from cli.rs to runner.rs so they can be used by code under `lib.rs`. Note: We need to make a lib directory so it's more obvios which code is in lib's source tree vs main's. --- crates/gateway/app/src/cli-tests.rs | 52 ++++++------ crates/gateway/app/src/cli.rs | 119 ++++++++++------------------ crates/gateway/app/src/lib.rs | 3 +- crates/gateway/app/src/main.rs | 7 +- crates/gateway/app/src/runner.rs | 33 ++++++++ 5 files changed, 107 insertions(+), 107 deletions(-) diff --git a/crates/gateway/app/src/cli-tests.rs b/crates/gateway/app/src/cli-tests.rs index ab1a79844..3c4386c36 100644 --- a/crates/gateway/app/src/cli-tests.rs +++ b/crates/gateway/app/src/cli-tests.rs @@ -14,7 +14,7 @@ fn args(items: &[&str]) -> Vec { /// The kind of the parse error for `items`; panics if they parse. fn error_kind(items: &[&str]) -> ErrorKind { - AppOptions::from_args(args(items)) + parse_args(args(items)) .expect_err("the arguments are rejected") .kind() } @@ -58,7 +58,7 @@ fn neither_path_set_defers_to_boot_discovery() { #[test] fn the_root_invocation_serves_with_discovery() { - let invocation = AppOptions::from_args(args(&[])).expect("the bare invocation parses"); + let invocation = parse_args(args(&[])).expect("the bare invocation parses"); assert_eq!( invocation.serve.config_path, None, "no --config defers to boot discovery" @@ -93,7 +93,7 @@ fn a_positional_config_path_is_rejected() { #[test] fn the_config_flag_sets_the_path() { - let invocation = AppOptions::from_args(args(&["--config", "gateway.toml"])).expect("parse"); + let invocation = parse_args(args(&["--config", "gateway.toml"])).expect("parse"); assert_eq!( invocation.serve.config_path, Some(PathBuf::from("gateway.toml")) @@ -115,8 +115,8 @@ fn the_config_flag_is_given_once() { #[test] fn parses_path_and_profile() { - let invocation = AppOptions::from_args(args(&["--config", "gateway.toml", "--profile", "dev"])) - .expect("parse"); + let invocation = + parse_args(args(&["--config", "gateway.toml", "--profile", "dev"])).expect("parse"); assert_eq!( invocation.serve.profile.as_ref().map(ProfileName::as_str), Some("dev") @@ -129,14 +129,14 @@ fn parses_path_and_profile() { #[test] fn the_tray_is_default_and_login_is_off() { - let invocation = AppOptions::from_args(args(&["--config", "gateway.toml"])).expect("parse"); + let invocation = parse_args(args(&["--config", "gateway.toml"])).expect("parse"); assert!(invocation.tray, "the tray is the default main loop"); assert!(!invocation.login); } #[test] fn no_tray_selects_the_headless_loop() { - let invocation = AppOptions::from_args(args(&["--no-tray"])).expect("parse"); + let invocation = parse_args(args(&["--no-tray"])).expect("parse"); assert!(!invocation.tray); assert!(!invocation.login); } @@ -145,14 +145,14 @@ fn no_tray_selects_the_headless_loop() { fn the_autostart_command_line_parses() { // The Run-key entry is `"" --login`; a login launch must // never fail on its own command line. - let invocation = AppOptions::from_args(args(&["--login"])).expect("parse"); + let invocation = parse_args(args(&["--login"])).expect("parse"); assert!(invocation.login); assert!(invocation.tray, "a login launch still shows the tray"); } #[test] fn print_url_parses_and_leaves_the_other_flags_alone() { - let invocation = AppOptions::from_args(args(&["--print-url"])).expect("parse"); + let invocation = parse_args(args(&["--print-url"])).expect("parse"); assert!(invocation.print_url); assert!( invocation.tray, @@ -163,7 +163,7 @@ fn print_url_parses_and_leaves_the_other_flags_alone() { #[test] fn print_url_combines_with_no_tray_and_a_config_path() { - let invocation = AppOptions::from_args(args(&[ + let invocation = parse_args(args(&[ "--config", "gateway.toml", "--no-tray", @@ -180,7 +180,7 @@ fn print_url_combines_with_no_tray_and_a_config_path() { #[test] fn browser_parses_and_reaches_the_serve_options() { - let invocation = AppOptions::from_args(args(&["--browser"])).expect("parse"); + let invocation = parse_args(args(&["--browser"])).expect("parse"); assert!( invocation.serve.browser, "the flag reaches the spawn hook through ServeOptions" @@ -190,7 +190,7 @@ fn browser_parses_and_reaches_the_serve_options() { #[test] fn login_wins_over_browser() { - let invocation = AppOptions::from_args(args(&["--login", "--browser"])).expect("parse"); + let invocation = parse_args(args(&["--login", "--browser"])).expect("parse"); assert!( !invocation.serve.browser, "a login launch never opens a browser" @@ -199,7 +199,7 @@ fn login_wins_over_browser() { #[test] fn missing_profile_defers_to_environment_or_state() { - let invocation = AppOptions::from_args(args(&["--config", "gateway.toml"])).expect("parse"); + let invocation = parse_args(args(&["--config", "gateway.toml"])).expect("parse"); assert!(invocation.serve.profile.is_none()); } @@ -227,7 +227,7 @@ fn rejects_an_unknown_argument() { #[test] fn help_is_recognized() { for flag in ["--help", "-h"] { - let error = AppOptions::from_args(args(&[flag])).unwrap_err(); + let error = parse_args(args(&[flag])).unwrap_err(); assert_eq!(error.kind(), ErrorKind::DisplayHelp, "{flag}"); assert!(!error.use_stderr(), "{flag} prints to stdout and succeeds"); } @@ -236,7 +236,7 @@ fn help_is_recognized() { #[test] fn version_is_recognized() { for flag in ["--version", "-V"] { - let error = AppOptions::from_args(args(&[flag])).unwrap_err(); + let error = parse_args(args(&[flag])).unwrap_err(); assert_eq!(error.kind(), ErrorKind::DisplayVersion, "{flag}"); assert!(!error.use_stderr(), "{flag} prints to stdout and succeeds"); } @@ -252,17 +252,17 @@ fn rejects_unknown_flag() { #[test] fn diagnostics_is_the_only_subcommand() { - let invocation = AppOptions::from_args(args(&["diagnostics"])).expect("parses"); + let invocation = parse_args(args(&["diagnostics"])).expect("parses"); assert_eq!(invocation.command, LaunchCommand::Diagnostics); assert_eq!(invocation.serve.config_path, None); - let invocation = AppOptions::from_args(args(&[])).expect("the bare invocation parses"); + let invocation = parse_args(args(&[])).expect("the bare invocation parses"); assert_eq!(invocation.command, LaunchCommand::Serve); } #[test] fn diagnostics_accepts_a_config_path() { let invocation = - AppOptions::from_args(args(&["diagnostics", "--config", "gateway.toml"])).expect("parses"); + parse_args(args(&["diagnostics", "--config", "gateway.toml"])).expect("parses"); assert_eq!(invocation.command, LaunchCommand::Diagnostics); assert_eq!( invocation.serve.config_path, @@ -292,8 +292,7 @@ fn diagnostics_after_a_flag_is_not_a_subcommand() { #[test] fn supervised_option_parses_and_reaches_the_serve_options() { - let invocation = - AppOptions::from_args(args(&["--supervised", "--no-tray"])).expect("should parse"); + let invocation = parse_args(args(&["--supervised", "--no-tray"])).expect("should parse"); assert!(invocation.serve.supervised); assert!( @@ -304,12 +303,11 @@ fn supervised_option_parses_and_reaches_the_serve_options() { #[test] fn supervised_option_does_not_override_no_tray_option() { - let invocation = AppOptions::from_args(args(&["--supervised"])).expect("should parse"); + let invocation = parse_args(args(&["--supervised"])).expect("should parse"); assert!(invocation.tray, "--supervised should not set tray to false"); - let invocation = - AppOptions::from_args(args(&["--supervised", "--no-tray"])).expect("should parse"); + let invocation = parse_args(args(&["--supervised", "--no-tray"])).expect("should parse"); assert!( !invocation.tray, @@ -319,7 +317,7 @@ fn supervised_option_does_not_override_no_tray_option() { #[test] fn a_launch_is_not_supervised_by_default() { - let invocation = AppOptions::from_args(args(&[])).expect("should parse"); + let invocation = parse_args(args(&[])).expect("should parse"); assert!(!invocation.serve.supervised); } @@ -338,7 +336,7 @@ fn supervised_and_print_url_options_cannot_be_used_together() { #[test] fn diagnostics_has_its_own_help() { - let error = AppOptions::from_args(args(&["diagnostics", "--help"])).unwrap_err(); + let error = parse_args(args(&["diagnostics", "--help"])).unwrap_err(); assert_eq!(error.kind(), ErrorKind::DisplayHelp); let help = error.to_string(); assert!(help.contains("--config"), "{help}"); @@ -369,14 +367,14 @@ fn the_help_text_lists_all_flags() { #[test] fn preferred_port_parses_and_reaches_the_serve_options() { - let options = AppOptions::from_args(args(&["--preferred-port", "8081"])).expect("should parse"); + let options = parse_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"); + let options = parse_args(args(&[])).expect("the bare invocation should parse"); assert_eq!(options.serve.preferred_port, None); } diff --git a/crates/gateway/app/src/cli.rs b/crates/gateway/app/src/cli.rs index 81f5267c0..a55ea3675 100644 --- a/crates/gateway/app/src/cli.rs +++ b/crates/gateway/app/src/cli.rs @@ -1,83 +1,50 @@ use std::{ffi::OsString, path::PathBuf}; use clap::{Parser, Subcommand}; -use gateway::{ProfileName, ServeOptions}; +use gateway::{AppOptions, LaunchCommand, ProfileName, ServeOptions}; -/// The parsed app options: the serve options plus how the main thread runs. -#[derive(Debug)] -pub(crate) struct AppOptions { - /// What the launch does. - pub command: LaunchCommand, - - /// What to serve. Under [`LaunchCommand::Diagnostics`] only the config path - /// is meaningful: the report names it. - pub serve: ServeOptions, - - /// Whether the system tray occupies the main thread (default). - /// `--no-tray` keeps the headless Ctrl-C loop for servers and CI. - pub tray: bool, - - /// Whether the launch came from the OS autostart entry (`--login`). - pub login: bool, - - /// Whether to print the Settings handoff URL to stdout (`--print-url`). - /// Implies the headless loop: the flag exists for tray-less - /// environments. - pub print_url: bool, -} - -impl AppOptions { - /// Constructs an [`AppOptions`] from command line args. - /// - /// Config paths are read as `OsString`, so non-UTF-8 paths survive. - /// The config path - /// (`--config PATH`, falling back to `RUNVIL_GATEWAY_CONFIG`) stays - /// optional: with neither set, the gateway discovers or generates the - /// boot config itself. - /// `--profile NAME` is validated into a - /// [`ProfileName`] at parse time. - /// - /// `--help` and `--version` also arrive as a [`clap::Error`]; its - /// `use_stderr()` is false for them. - pub(crate) fn from_args(args: impl IntoIterator) -> Result { - let cli = Cli::try_parse_from(args)?; - let env = std::env::var_os("RUNVIL_GATEWAY_CONFIG"); - - let config_path = resolve_config_path(cli.config, env); - - Ok(match cli.command { - Some(SubCommand::Diagnostics) => Self { - command: LaunchCommand::Diagnostics, - serve: ServeOptions::new(config_path, None), - tray: false, - login: false, - print_url: false, - }, - - None => Self { - command: LaunchCommand::Serve, - // `--login`'s contract is absolute - a login launch never opens a - // browser - so it wins over `--browser`. - serve: ServeOptions::new(config_path, cli.profile) - .with_browser(cli.browser && !cli.login) - .with_supervised(cli.supervised) - .with_preferred_port(cli.preferred_port), - tray: !cli.no_tray, - login: cli.login, - print_url: cli.print_url, - }, - }) - } -} - -/// What this launch does. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum LaunchCommand { - /// Serve (the default and only serving mode). - Serve, - - /// Prints the diagnostics report and exits. - Diagnostics, +/// Parses command line args into an [`AppOptions`]. +/// +/// Config paths are read as `OsString`, so non-UTF-8 paths survive. +/// The config path +/// (`--config PATH`, falling back to `RUNVIL_GATEWAY_CONFIG`) stays +/// optional: with neither set, the gateway discovers or generates the +/// boot config itself. +/// `--profile NAME` is validated into a +/// [`ProfileName`] at parse time. +/// +/// `--help` and `--version` also arrive as a [`clap::Error`]; its +/// `use_stderr()` is false for them. +pub(crate) fn parse_args( + args: impl IntoIterator, +) -> Result { + let cli = Cli::try_parse_from(args)?; + let env = std::env::var_os("RUNVIL_GATEWAY_CONFIG"); + + let config_path = resolve_config_path(cli.config, env); + + Ok(match cli.command { + Some(SubCommand::Diagnostics) => AppOptions { + command: LaunchCommand::Diagnostics, + serve: ServeOptions::new(config_path, None), + tray: false, + login: false, + print_url: false, + }, + + None => AppOptions { + command: LaunchCommand::Serve, + // `--login`'s contract is absolute - a login launch never opens a + // browser - so it wins over `--browser`. + serve: ServeOptions::new(config_path, cli.profile) + .with_browser(cli.browser && !cli.login) + .with_supervised(cli.supervised) + .with_preferred_port(cli.preferred_port), + tray: !cli.no_tray, + login: cli.login, + print_url: cli.print_url, + }, + }) } /// The command line as clap reads it diff --git a/crates/gateway/app/src/lib.rs b/crates/gateway/app/src/lib.rs index 179990cc4..570aeb8e7 100644 --- a/crates/gateway/app/src/lib.rs +++ b/crates/gateway/app/src/lib.rs @@ -128,7 +128,8 @@ use crate::boot_load::STT_RUNTIME_UNAVAILABLE; pub use crate::diagnostics::diagnostics_json; pub use crate::relaunch::{GatewayStartup, GatewayStartupError, settle_gateway_startup}; pub use crate::runner::{ - Gateway, GatewayHandle, ProfilesContext, ServeOptions, run, run_printing_url, spawn, + AppOptions, Gateway, GatewayHandle, LaunchCommand, ProfilesContext, ServeOptions, run, + run_printing_url, spawn, }; pub use crate::shutdown::{RESTART_REQUESTED, ShutdownReason}; pub use crate::tray::run_with_tray; diff --git a/crates/gateway/app/src/main.rs b/crates/gateway/app/src/main.rs index 13ee4d553..9972e69d4 100644 --- a/crates/gateway/app/src/main.rs +++ b/crates/gateway/app/src/main.rs @@ -15,14 +15,15 @@ use std::path::PathBuf; use std::process::ExitCode; use gateway::{ - GatewayStartup, ShutdownReason, run, run_printing_url, run_with_tray, settle_gateway_startup, + GatewayStartup, LaunchCommand, ShutdownReason, run, run_printing_url, run_with_tray, + settle_gateway_startup, }; use gateway_logging::{LogConfig, LogRuntime}; use tracing_subscriber::Layer as _; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; -use crate::cli::{AppOptions, LaunchCommand}; +use crate::cli::parse_args; mod cli; @@ -60,7 +61,7 @@ fn main() -> ExitCode { ); } - let app_options = match AppOptions::from_args(std::env::args_os()) { + let app_options = match parse_args(std::env::args_os()) { Ok(options) => options, // clap returns `--help` and `--version` through the error channel. diff --git a/crates/gateway/app/src/runner.rs b/crates/gateway/app/src/runner.rs index ecde2c3f8..d37757363 100644 --- a/crates/gateway/app/src/runner.rs +++ b/crates/gateway/app/src/runner.rs @@ -135,6 +135,39 @@ impl ServeOptions { } } +/// The parsed app options: the serve options plus how the main thread runs. +#[derive(Debug)] +pub struct AppOptions { + /// What the launch does. + pub command: LaunchCommand, + + /// What to serve. Under [`LaunchCommand::Diagnostics`] only the config path + /// is meaningful: the report names it. + pub serve: ServeOptions, + + /// Whether the system tray occupies the main thread (default). + /// `--no-tray` keeps the headless Ctrl-C loop for servers and CI. + pub tray: bool, + + /// Whether the launch came from the OS autostart entry (`--login`). + pub login: bool, + + /// Whether to print the Settings handoff URL to stdout (`--print-url`). + /// Implies the headless loop: the flag exists for tray-less + /// environments. + pub print_url: bool, +} + +/// What this launch does. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LaunchCommand { + /// Serve (the default and only serving mode). + Serve, + + /// Prints the diagnostics report and exits. + Diagnostics, +} + /// Optional admin configuration path plus the active profile name. #[non_exhaustive] #[derive(Debug, Clone, Default)] From 0e37dc718a89a49c895cf9336a8b87a9b439a3ae Mon Sep 17 00:00:00 2001 From: Andrew Wheeler Date: Fri, 30 Oct 2026 14:59:45 -0600 Subject: [PATCH 2/2] Run the Gateway under a lightweight supervisor - Summary Part of the work to allow the Gateway to restart itself. For an ordinary launch, the gateway starts a supervisor function that runs the serving gateway as a child process. The supervisor will be able to read the child's exit code and restart it upon request (not implemented yet). - Code Changes Adds `crates/gateway/app/src/supervisor.rs` module. It has a single public function `run`, which manages the child gateway processes lifecycle from birth to death. Run takes and holds the launch lock while it's running, so any other launch attempts will attach to it's wrapped gateway. Launch args are passed through and the child's output is passed out to the supervisor, so the whole thing is transparent to the end user. Makes some changes to `crates/gateway-api-discovery`. This is the public interface between the gateway and the rest of Runvil. `launch_or_attach`and `launch_or_attach_cancellable` become `LaunchDecision::decide_launch` and `LaunchDecision::decide_launch_cancellable` to better reflect that they are returning the results of a decision, and not doing anything else. They're semantics are unchanged, this is a purely syntactic change. `settle_gateway_startup` now takes an `AppOptions` instead of a `ServeOptions`. `AppOptions` contains a `ServeOptions`, this just allows the function to examine the full `AppOptions`. - External changes The changes to `launch_or_attach_cancellable` require some minor changes to Workshop. - User Facing Changes Aside from the minor API changes in `gateway-api-discovery` there are no user facing changes in this commit. --- crates/gateway-api-discovery/src/file.rs | 8 ++ crates/gateway-api-discovery/src/lib.rs | 7 +- crates/gateway-api-discovery/src/lock.rs | 62 ++++---- crates/gateway-api-discovery/tests/it/main.rs | 9 +- crates/gateway/app/src/main-logging-tests.rs | 18 ++- crates/gateway/app/src/main.rs | 14 +- crates/gateway/app/src/relaunch.rs | 104 ++++++++++---- crates/gateway/app/src/supervisor.rs | 126 ++++++++++++++++ crates/gateway/app/tests/it/boot.rs | 10 +- crates/gateway/app/tests/it/boot/ownership.rs | 4 +- .../gateway/app/tests/it/boot/supervisor.rs | 136 ++++++++++++++++++ .../gateway/app/tests/it/support/process.rs | 25 ++++ .../desktop/src/gateway/supervisor/launch.rs | 3 +- .../desktop/src/gateway/tests/cancellation.rs | 6 +- .../desktop/src/gateway/tests/recovery.rs | 8 +- vibe/2026-10/2026-10-06-1-restart-to-apply.md | 21 ++- 16 files changed, 471 insertions(+), 90 deletions(-) create mode 100644 crates/gateway/app/src/supervisor.rs create mode 100644 crates/gateway/app/tests/it/boot/supervisor.rs diff --git a/crates/gateway-api-discovery/src/file.rs b/crates/gateway-api-discovery/src/file.rs index 6fbbe10f6..2886f935d 100644 --- a/crates/gateway-api-discovery/src/file.rs +++ b/crates/gateway-api-discovery/src/file.rs @@ -137,6 +137,14 @@ impl GatewayDiscoveryFile { write_atomic_owner_only(&path, &bytes) .map_err(|source| SidecarError::Write { path, source }) } + + /// Builds a string containing the local address the gateway is running on, + /// for example if the gateway is running on port `12345` this would + /// give `http://127.0.0.1:12345` + #[must_use] + pub fn local_addr(&self) -> String { + format!("http://127.0.0.1:{}", self.port) + } } /// Removes the gateway discovery file in `run_dir` when it still belongs to diff --git a/crates/gateway-api-discovery/src/lib.rs b/crates/gateway-api-discovery/src/lib.rs index 973f8df7f..e58afa4df 100644 --- a/crates/gateway-api-discovery/src/lib.rs +++ b/crates/gateway-api-discovery/src/lib.rs @@ -21,7 +21,7 @@ //! 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. -//! 3. Launch races take [`launch_or_attach`]: the `gateway.json.lock` +//! 3. Launch races take [`LaunchDecision::decide_launch`]: the `gateway.json.lock` //! advisory lock elects one parent launcher; losers attach to the winner. //! 4. Every Gateway process holds [`GatewayInstanceLease`] from before //! startup side effects until process exit, independently of that parent @@ -50,10 +50,7 @@ pub use crate::cancellation::CancellationToken; pub use crate::error::SidecarError; pub use crate::file::{GatewayDiscoveryFile, remove_if_mine}; pub use crate::health::{HealthError, ProbeError, wait_for_health, wait_for_health_cancellable}; -pub use crate::lock::{ - GatewayInstanceLease, LaunchDecision, LaunchLock, launch_or_attach, - launch_or_attach_cancellable, -}; +pub use crate::lock::{GatewayInstanceLease, LaunchDecision, LaunchLock}; 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, diff --git a/crates/gateway-api-discovery/src/lock.rs b/crates/gateway-api-discovery/src/lock.rs index cd67173d2..29e8e81c2 100644 --- a/crates/gateway-api-discovery/src/lock.rs +++ b/crates/gateway-api-discovery/src/lock.rs @@ -77,50 +77,52 @@ impl GatewayInstanceLease { } } -/// What [`launch_or_attach`] decided. +/// Represents the decision to launch a new Gateway or attach to a running one #[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) -} +impl LaunchDecision { + /// Settles a launch race in `run_dir`: returns [`Self::Launch`] + /// with the held lock when no live gateway exists, or + /// [`Self::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 decide_launch(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) + /// Settles a launch race while observing caller cancellation. + /// + /// # Errors + /// Returns [`SidecarError::Cancelled`] when cancellation wins, plus the + /// file, lock, and timeout failures documented by [`Self::decide_launch`]. + pub fn decide_launch_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 +/// [`LaunchDecision::decide_launch`] 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( diff --git a/crates/gateway-api-discovery/tests/it/main.rs b/crates/gateway-api-discovery/tests/it/main.rs index 0fdf220b5..4e1ec3421 100644 --- a/crates/gateway-api-discovery/tests/it/main.rs +++ b/crates/gateway-api-discovery/tests/it/main.rs @@ -10,8 +10,8 @@ use std::time::Duration; use gateway_api_discovery::{ GatewayDiscoveryFile, GatewayInstanceLease, LaunchDecision, Resolution, SidecarError, - StaleReason, gateway_discovery_file_path, instance_lock_file_path, launch_or_attach, - lock_file_path, resolve, wait_for_health, + StaleReason, gateway_discovery_file_path, instance_lock_file_path, lock_file_path, resolve, + wait_for_health, }; const LEASE_CHILD_RUN_DIR: &str = "RUNVIL_LEASE_CHILD_RUN_DIR"; @@ -175,7 +175,8 @@ fn resolve_cleans_a_dead_pid_file() { #[test] fn launch_or_attach_elects_one_launcher_and_times_out_the_loser() { let dir = tempfile::TempDir::new().expect("tempdir"); - let decision = launch_or_attach(dir.path(), Duration::from_secs(5)).expect("the race settles"); + let decision = LaunchDecision::decide_launch(dir.path(), Duration::from_secs(5)) + .expect("the race settles"); assert!( matches!(decision, LaunchDecision::Launch(_)), "an empty run dir elects a launcher" @@ -185,7 +186,7 @@ fn launch_or_attach_elects_one_launcher_and_times_out_the_loser() { "the lock file was created" ); - let error = launch_or_attach(dir.path(), Duration::from_millis(150)) + let error = LaunchDecision::decide_launch(dir.path(), Duration::from_millis(150)) .expect_err("a held lock with no live file starves the loser"); assert!( matches!(error, SidecarError::LaunchTimeout { .. }), diff --git a/crates/gateway/app/src/main-logging-tests.rs b/crates/gateway/app/src/main-logging-tests.rs index defb16e95..f4f66d18c 100644 --- a/crates/gateway/app/src/main-logging-tests.rs +++ b/crates/gateway/app/src/main-logging-tests.rs @@ -10,7 +10,7 @@ use std::time::Duration; use base64::Engine as _; #[cfg(feature = "stt")] use futures_util::{SinkExt as _, StreamExt as _}; -use gateway::ServeOptions; +use gateway::{AppOptions, LaunchCommand, ServeOptions}; #[cfg(feature = "stt")] use gateway::{Config, Gateway, ProfilesContext}; #[cfg(feature = "stt")] @@ -127,7 +127,13 @@ fn a_process_lease_loser_leaves_the_canonical_log_untouched() { let connection_path = gateway_api_discovery::gateway_discovery_file_path(&run_dir); std::fs::write(&connection_path, b"owner-is-still-publishing") .expect("seed an unreadable owner record"); - let options = ServeOptions::new(None, None::).with_run_dir(run_dir); + let options = AppOptions { + command: LaunchCommand::Serve, + serve: ServeOptions::new(None, None::).with_run_dir(run_dir), + tray: false, + login: false, + print_url: true, + }; let error = gateway::settle_gateway_startup(&options, Duration::from_millis(150)) .expect_err("the process lease loser times out before logging"); @@ -162,7 +168,13 @@ fn a_lease_holder_resolution_failure_leaves_the_canonical_log_untouched() { std::fs::write(&log_path, "owner-log-sentinel").expect("seed the canonical log"); let connection_path = gateway_api_discovery::gateway_discovery_file_path(&run_dir); std::fs::create_dir_all(&connection_path).expect("create unreadable connection fixture"); - let options = ServeOptions::new(None, None::).with_run_dir(run_dir); + let options = AppOptions { + command: LaunchCommand::Serve, + serve: ServeOptions::new(None, None::).with_run_dir(run_dir), + tray: false, + login: false, + print_url: true, + }; let error = gateway::settle_gateway_startup(&options, Duration::from_millis(150)) .expect_err("connection resolution fails before logging"); diff --git a/crates/gateway/app/src/main.rs b/crates/gateway/app/src/main.rs index 9972e69d4..97452947e 100644 --- a/crates/gateway/app/src/main.rs +++ b/crates/gateway/app/src/main.rs @@ -26,6 +26,7 @@ use tracing_subscriber::util::SubscriberInitExt; use crate::cli::parse_args; mod cli; +mod supervisor; /// The log filter when `RUST_LOG` is unset: the gateway crates at `info` /// (a download or a switch must say what it is doing), the chatty HTTP @@ -99,11 +100,10 @@ fn main() -> ExitCode { // loser, recovery, bind, or publication. The lease holder re-resolves the // connection record; a loser waits only for the holder's validated record // and exits through this console-only path. - let _instance_lease = match settle_gateway_startup( - &app_options.serve, - OWNER_PUBLICATION_TIMEOUT, - ) { + // An ordinary launch obtains a launch lock and hands it off to `supervisor::run` + let _instance_lease = match settle_gateway_startup(&app_options, OWNER_PUBLICATION_TIMEOUT) { Ok(GatewayStartup::Boot(lease)) => lease, + Ok(GatewayStartup::OpenSettings(url)) => { if app_options.print_url { println!("{url}"); @@ -119,10 +119,16 @@ fn main() -> ExitCode { } return ExitCode::SUCCESS; } + + Ok(GatewayStartup::Supervise(lock)) => { + return supervisor::run(&app_options, lock); + } + Ok(_) => { eprintln!("error: unrecognized gateway startup decision"); return ExitCode::FAILURE; } + Err(error) => { print_error_chain(&error); return ExitCode::FAILURE; diff --git a/crates/gateway/app/src/relaunch.rs b/crates/gateway/app/src/relaunch.rs index b0a75f89c..5aac67c79 100644 --- a/crates/gateway/app/src/relaunch.rs +++ b/crates/gateway/app/src/relaunch.rs @@ -8,10 +8,16 @@ //! A lease loser reads without cleanup until the owner publishes a validated //! record, then hands off. It never deletes stale state or initializes //! canonical logging. - +//! +//! An ordinary launch, neither `--supervised` nor `--print-url`, runs the +//! launch election instead: its winner supervises the serving Gateway and +//! holds the launch lock for its lifetime, and a loser hands off to the +//! live Gateway. use std::time::{Duration, Instant}; -use crate::runner::ServeOptions; +use gateway_api_discovery::LaunchDecision; + +use crate::AppOptions; const OWNER_POLL_INTERVAL: Duration = Duration::from_millis(25); @@ -30,6 +36,11 @@ enum Relaunch { pub enum GatewayStartup { /// This process owns the lifetime lease and may boot. Boot(gateway_api_discovery::GatewayInstanceLease), + + /// This process won the launch election and supervises the serving + /// Gateway as its child, holding the launch lock for its lifetime. + Supervise(gateway_api_discovery::LaunchLock), + /// Another validated Gateway owns the lease, so this process hands off. OpenSettings(String), } @@ -41,12 +52,15 @@ pub enum GatewayStartupError { /// No run directory exists in which process ownership can be established. #[error("no user profile directory found for the Gateway process lease")] NoRunDirectory, + /// The operating-system lease could not be opened or attempted. #[error("establish Gateway process ownership")] Lease(#[source] gateway_api_discovery::SidecarError), + /// The lease holder could not resolve existing shared connection state. #[error("resolve existing Gateway connection before startup")] Resolve(#[source] gateway_api_discovery::SidecarError), + /// The lease owner did not publish a validated record in time. #[error("the Gateway process owner published no validated connection within {timeout:?}")] #[non_exhaustive] @@ -54,6 +68,10 @@ pub enum GatewayStartupError { /// The bounded wait that elapsed. timeout: Duration, }, + + /// The launch election failed or found no attachable Gateway in time. + #[error("settle which process launches the Gateway")] + Launch(#[source] gateway_api_discovery::SidecarError), } /// Maps a gateway discovery file resolution to the relaunch decision. Only a @@ -64,7 +82,7 @@ fn decide(resolution: &gateway_api_discovery::Resolution) -> Relaunch { // The file records the real port of the loopback bind; URLs // normalize to a literal 127.0.0.1, never localhost. Relaunch::OpenSettings(crate::auth::primitives::auth_url( - &format!("http://127.0.0.1:{}", file.port), + &file.local_addr(), &file.api_key, )) } @@ -86,32 +104,50 @@ fn settings_url(connection: &gateway_api_discovery::ValidatedConnection) -> Stri /// state. A lease loser only reads and validates, waiting up to `timeout` for /// the owner to publish. It never mutates shared state. /// +/// A launch that will supervise settles the launch election instead, +/// returning [GatewayStartup::Supervise] with the held lock, or +/// [GatewayStartup::OpenSettings] for a live Gateway. +/// /// # Errors -/// Returns [`GatewayStartupError`] when no run directory is available, the -/// operating-system lease cannot be attempted, existing shared connection -/// state cannot be resolved, or the owner publishes no validated connection -/// within `timeout`. +/// Returns [`GatewayStartupError`] when: +/// - There is no run directory available +/// - The lease attempt fails at the operating-system level +/// - Existing shared connection state cannot be resolved +/// - The owner publishes no validated connection within `timeout` +/// - The launch election fails pub fn settle_gateway_startup( - options: &ServeOptions, + options: &AppOptions, timeout: Duration, ) -> Result { let run_dir = options + .serve .run_dir .clone() .or_else(gateway_api_discovery::default_run_dir) .ok_or(GatewayStartupError::NoRunDirectory)?; - match gateway_api_discovery::GatewayInstanceLease::try_acquire(&run_dir) - .map_err(GatewayStartupError::Lease)? - { - Some(lease) => { - let resolution = - gateway_api_discovery::resolve(&run_dir).map_err(GatewayStartupError::Resolve)?; - match decide(&resolution) { - Relaunch::OpenSettings(url) => Ok(GatewayStartup::OpenSettings(url)), - Relaunch::Boot => Ok(GatewayStartup::Boot(lease)), + + if options.serve.supervised || options.print_url { + return match gateway_api_discovery::GatewayInstanceLease::try_acquire(&run_dir) + .map_err(GatewayStartupError::Lease)? + { + Some(lease) => { + let resolution = gateway_api_discovery::resolve(&run_dir) + .map_err(GatewayStartupError::Resolve)?; + match decide(&resolution) { + Relaunch::OpenSettings(url) => Ok(GatewayStartup::OpenSettings(url)), + Relaunch::Boot => Ok(GatewayStartup::Boot(lease)), + } } - } - None => wait_for_owner(&run_dir, timeout), + None => wait_for_owner(&run_dir, timeout), + }; + } + + match LaunchDecision::decide_launch(&run_dir, timeout).map_err(GatewayStartupError::Launch)? { + LaunchDecision::Launch(lock) => Ok(GatewayStartup::Supervise(lock)), + + LaunchDecision::Attach(file) => Ok(GatewayStartup::OpenSettings( + crate::auth::primitives::auth_url(&file.local_addr(), &file.api_key), + )), } } @@ -150,6 +186,8 @@ fn wait_for_owner_with( #[cfg(test)] mod tests { + use crate::{LaunchCommand, ServeOptions}; + use super::*; /// A gateway discovery file as a live gateway would write it. @@ -164,6 +202,16 @@ mod tests { } } + fn app_options(serve: ServeOptions) -> AppOptions { + AppOptions { + command: LaunchCommand::Serve, + serve, + tray: false, + login: false, + print_url: true, + } + } + #[test] fn a_live_file_hands_off_its_settings_url() { let decision = decide(&gateway_api_discovery::Resolution::Attach(live_file())); @@ -212,8 +260,10 @@ mod tests { #[test] fn an_empty_run_dir_settles_to_boot_with_the_lease_held() { let temp = tempfile::TempDir::new().expect("tempdir"); - let options = ServeOptions::new(None, None::) - .with_run_dir(temp.path().to_path_buf()); + let options = app_options( + ServeOptions::new(None, None::) + .with_run_dir(temp.path().to_path_buf()), + ); assert!( matches!( settle_gateway_startup(&options, Duration::from_millis(100)) @@ -239,8 +289,10 @@ mod tests { drop(child); let file = gateway_api_discovery::GatewayDiscoveryFile { pid, ..live_file() }; file.write_to(temp.path()).expect("write fixture"); - let options = ServeOptions::new(None, None::) - .with_run_dir(temp.path().to_path_buf()); + let options = app_options( + ServeOptions::new(None, None::) + .with_run_dir(temp.path().to_path_buf()), + ); assert!( matches!( @@ -261,8 +313,10 @@ mod tests { let temp = tempfile::TempDir::new().expect("tempdir"); let connection_path = gateway_api_discovery::gateway_discovery_file_path(temp.path()); std::fs::create_dir_all(&connection_path).expect("create unreadable connection fixture"); - let options = ServeOptions::new(None, None::) - .with_run_dir(temp.path().to_path_buf()); + let options = app_options( + ServeOptions::new(None, None::) + .with_run_dir(temp.path().to_path_buf()), + ); let error = settle_gateway_startup(&options, Duration::from_millis(100)) .expect_err("an unreadable connection record blocks startup"); diff --git a/crates/gateway/app/src/supervisor.rs b/crates/gateway/app/src/supervisor.rs new file mode 100644 index 000000000..49f0e2311 --- /dev/null +++ b/crates/gateway/app/src/supervisor.rs @@ -0,0 +1,126 @@ +//! The supervisor is the process an ordinary launch runs. It runs the serving +//! gateway as its `--supervised` child, copying the child's output to its own +//! and exiting with the child's code. It holds the launch lock for its whole +//! lifetime. +//! +//! The supervisor does not start the logging system, the child gateway process +//! owns the log file. + +use std::io::{BufRead, BufReader, Read, Write}; +use std::process::{Command, ExitCode, ExitStatus, Stdio}; +use std::thread::JoinHandle; + +use gateway::AppOptions; +use gateway_api_discovery::LaunchLock; + +/// Supervisor entry point: starts and manages the child gateway process, +/// returning the process exit code when it ends. Holds the launch lock while +/// the gateway process is running, so another launch attempt attaches to the +/// live child instead of launching a new gateway. +pub(crate) fn run(options: &AppOptions, _launch_lock: LaunchLock) -> ExitCode { + let mut child = match build_child_command(options).spawn() { + Ok(child) => child, + Err(error) => { + eprintln!("error: could not start the gateway: {error}"); + return ExitCode::FAILURE; + } + }; + + // The child's stdin stays open, unused, for as long as this process + // lives. When the pipe closes, the child's watchdog stops it. + let _child_stdin = child.stdin.take(); + + let child_stdout = child.stdout.take(); + let child_stderr = child.stderr.take(); + + // Forward the child's stdout and std_error to this thread's output streams + let output_handles = [ + child_stdout.map(|pipe| OutputStream::Stdout.forward(pipe)), + child_stderr.map(|pipe| OutputStream::Stderr.forward(pipe)), + ]; + + let status = child.wait(); + + // The pipes close when the child exits; joining flushes its last lines. + for handle in output_handles.into_iter().flatten() { + let _ = handle.join(); + } + + match status { + Ok(status) => exit_code(status), + Err(error) => { + eprintln!("error: could not wait for the gateway: {error}"); + ExitCode::FAILURE + } + } +} + +/// Builds the command to launch the child Gateway: this binary with `--supervised` +/// flag set, passing currently set app options on to the child. +fn build_child_command(options: &AppOptions) -> Command { + // A missing executable path makes `spawn` fail with a clear error. + let exe = std::env::current_exe().unwrap_or_else(|_| "runvil-gateway".into()); + let mut command = Command::new(exe); + + command.arg("--supervised"); + + if !options.tray { + command.arg("--no-tray"); + } + + // Already resolved against RUNVIL_GATEWAY_CONFIG, so the child needs no + // second lookup. + if let Some(config) = &options.serve.config_path { + command.arg("--config").arg(config); + } + + if let Some(profile) = &options.serve.profile { + command.arg("--profile").arg(profile.as_str()); + } + + if options.serve.browser { + command.arg("--browser"); + } + + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + command +} + +/// Control forwarding of the child's output to the supervisor +#[derive(Clone, Copy)] +enum OutputStream { + Stdout, + Stderr, +} + +impl OutputStream { + /// Copies every line from one of the child's pipes to the matching stream + /// of this process, as bytes, so output that is not UTF-8 passes through. + fn forward(self, pipe: impl Read + Send + 'static) -> JoinHandle<()> { + std::thread::spawn(move || { + for line in BufReader::new(pipe).split(b'\n') { + let Ok(mut line) = line else { break }; + line.push(b'\n'); + + let _ = match self { + OutputStream::Stdout => std::io::stdout().write_all(&line), + OutputStream::Stderr => std::io::stderr().write_all(&line), + }; + } + }) + } +} + +/// Builds the supervisor's exit code from the child's `ExitStatus`: +/// the child processes i16 `ExitStatus::code` is translated to a u8 `ExitCode`, +/// or `ExitCode::FAILURE` for a code outside the range of u8. +fn exit_code(status: ExitStatus) -> ExitCode { + status + .code() + .and_then(|code| u8::try_from(code).ok()) + .map_or(ExitCode::FAILURE, ExitCode::from) +} diff --git a/crates/gateway/app/tests/it/boot.rs b/crates/gateway/app/tests/it/boot.rs index c09860392..a4075eabf 100644 --- a/crates/gateway/app/tests/it/boot.rs +++ b/crates/gateway/app/tests/it/boot.rs @@ -11,6 +11,7 @@ mod in_process; mod lease; mod ownership; mod restart; +mod supervisor; /// Writes the config and returns its path; the profile selects every model /// the body declares. @@ -302,10 +303,13 @@ fn the_workshop_package_smoke_profile_boots_without_cli_arguments() { let file = gateway_api_discovery::GatewayDiscoveryFile::read(&profile.join("run")) .expect("read package connection") .expect("package connection exists"); + let publisher = sysinfo::Pid::from_u32(file.pid); + let mut system = sysinfo::System::new(); + system.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[publisher]), true); assert_eq!( - file.pid, - child.id(), - "the launched Gateway owns publication" + system.process(publisher).and_then(sysinfo::Process::parent), + Some(sysinfo::Pid::from_u32(child.id())), + "the launched Gateway's supervised child owns publication" ); let _ = child.kill(); let _ = child.wait(); diff --git a/crates/gateway/app/tests/it/boot/ownership.rs b/crates/gateway/app/tests/it/boot/ownership.rs index e75b3e7e7..a22fb3a0f 100644 --- a/crates/gateway/app/tests/it/boot/ownership.rs +++ b/crates/gateway/app/tests/it/boot/ownership.rs @@ -157,11 +157,13 @@ fn simultaneous_direct_launches_leave_one_owner_and_one_clean_handoff() { #[cfg(feature = "test-fixtures")] #[test] fn workshop_launch_lock_and_direct_launch_do_not_deadlock_or_double_boot() { + use gateway_api_discovery::LaunchDecision; + let temp = tempfile::tempdir().unwrap(); let config = race_config(&temp); let run_dir = temp.path().join(".runvil").join("run"); let gateway_api_discovery::LaunchDecision::Launch(workshop_lock) = - gateway_api_discovery::launch_or_attach(&run_dir, Duration::from_secs(5)) + LaunchDecision::decide_launch(&run_dir, Duration::from_secs(5)) .expect("Workshop wins the parent launch election") else { panic!("an empty run directory elects the Workshop launcher"); diff --git a/crates/gateway/app/tests/it/boot/supervisor.rs b/crates/gateway/app/tests/it/boot/supervisor.rs new file mode 100644 index 000000000..4482fc479 --- /dev/null +++ b/crates/gateway/app/tests/it/boot/supervisor.rs @@ -0,0 +1,136 @@ +//! Test he supervisor through the production binary: an ordinary launch serves +//! through a `--supervised` child, ends with it, stops it when killed, and +//! hands a second launch off to it. + +use std::path::PathBuf; +use std::time::{Duration, Instant}; + +use gateway_api_discovery::{GatewayDiscoveryFile, ValidatedConnection}; + +use super::race_config; +use crate::support::{GatewayProcess, wait_for_connection}; + +const BOOT_TIMEOUT: Duration = Duration::from_secs(30); +const EXIT_TIMEOUT: Duration = Duration::from_secs(10); + +fn run_dir(temp: &tempfile::TempDir) -> PathBuf { + temp.path().join(".runvil").join("run") +} + +/// Polls `condition` until it holds, failing with `what` after `timeout`. +fn wait_until(timeout: Duration, what: &str, mut condition: impl FnMut() -> bool) { + let deadline = Instant::now() + timeout; + + while !condition() { + assert!(Instant::now() < deadline, "{what} within {timeout:?}"); + std::thread::sleep(Duration::from_millis(25)); + } +} + +/// Whether `pid` has exited. A zombie counts: it has exited and only waits +/// for whoever inherited it to reap it. +fn process_exited(pid: u32) -> bool { + let pid = sysinfo::Pid::from_u32(pid); + let mut system = sysinfo::System::new(); + system.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[pid]), true); + + system + .process(pid) + .is_none_or(|process| process.status() == sysinfo::ProcessStatus::Zombie) +} + +/// `POST /shutdown` stops the child, and the supervisor exits with the +/// child's 0. +#[test] +fn a_shutdown_request_ends_the_child_and_the_supervisor_with_success() { + let temp = tempfile::tempdir().unwrap(); + let config = race_config(&temp); + let mut supervisor = GatewayProcess::spawn_headless_launch(&config, temp.path(), &[]); + + let connection = wait_for_connection(&run_dir(&temp), BOOT_TIMEOUT); + let child_pid = connection.pid; + assert_ne!( + child_pid, + supervisor.id(), + "the supervisor's child should serve, not the supervisor" + ); + + let validated_connection = + ValidatedConnection::validate_before(connection, Instant::now() + EXIT_TIMEOUT) + .expect("the published connection should validate"); + + gateway_api_discovery::request_shutdown(&validated_connection) + .expect("the Gateway should accept a shutdown"); + + let status = supervisor.wait_for_exit(EXIT_TIMEOUT); + + assert!( + status.success(), + "a stopped child should end the supervisor with 0, got {status}" + ); + assert!( + process_exited(child_pid), + "the child exited before its supervisor" + ); +} + +/// Killing the supervisor closes the child's stdin, and the child's +/// watchdog stops it cleanly: it removes its discovery file, which a +/// killed process would leave behind. +#[test] +fn killing_the_supervisor_stops_its_child() { + let temp = tempfile::tempdir().unwrap(); + let config = race_config(&temp); + let mut supervisor = GatewayProcess::spawn_headless_launch(&config, temp.path(), &[]); + + let connection = wait_for_connection(&run_dir(&temp), BOOT_TIMEOUT); + let child_pid = connection.pid; + + supervisor.stop(EXIT_TIMEOUT); + + let file = gateway_api_discovery::gateway_discovery_file_path(&run_dir(&temp)); + wait_until( + EXIT_TIMEOUT, + "an orphaned child should remove its discovery file", + || !file.exists(), + ); + wait_until(EXIT_TIMEOUT, "an orphaned child should exit", || { + process_exited(child_pid) + }); +} + +/// A second ordinary launch finds the first supervisor's lock held and its +/// child live, hands off, and exits 0, leaving the first child serving. +/// `--login` keeps the handoff from opening a browser. +#[test] +fn a_second_launch_attaches_to_the_supervised_child() { + let temp = tempfile::tempdir().unwrap(); + let config = race_config(&temp); + + let mut first = GatewayProcess::spawn_headless_launch(&config, temp.path(), &[]); + + let connection = wait_for_connection(&run_dir(&temp), BOOT_TIMEOUT); + + let mut second = GatewayProcess::spawn_headless_launch(&config, temp.path(), &["--login"]); + + let status = second.wait_for_exit(EXIT_TIMEOUT); + assert!( + status.success(), + "a second launch should hand off with 0, got {status}" + ); + + let after = GatewayDiscoveryFile::read(&run_dir(&temp)) + .expect("read the gateway discovery file should not fail") + .expect("the first child should still publish"); + + assert_eq!( + after.pid, connection.pid, + "the first supervisor's child should still serve" + ); + assert!( + first.try_wait().is_none(), + "the first supervisor should keep running" + ); + + first.stop(EXIT_TIMEOUT); +} diff --git a/crates/gateway/app/tests/it/support/process.rs b/crates/gateway/app/tests/it/support/process.rs index 8a1fb20ad..c9ede5599 100644 --- a/crates/gateway/app/tests/it/support/process.rs +++ b/crates/gateway/app/tests/it/support/process.rs @@ -76,6 +76,31 @@ impl GatewayProcess { Self { child } } + /// Starts the production binary as an ordinary headless launch, which + /// supervises the serving Gateway as its `--supervised` child. Output is + /// discarded: the supervisor forwards the child's, and an unread pipe + /// would eventually block both. + pub(crate) fn spawn_headless_launch(config: &Path, home: &Path, args: &[&str]) -> Self { + let mut command = Command::new(env!("CARGO_BIN_EXE_runvil-gateway")); + command + .arg("--config") + .arg(config) + .args(["--profile", "main", "--no-tray"]) + .args(args) + .env("USERPROFILE", home) + .env("HOME", home) + .env_remove("RUST_LOG") + .env_remove("RUNVIL_PROFILE") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + + command + .spawn() + .map(|child| Self { child }) + .expect("the Gateway launch spawns") + } + fn spawn_command(config: &Path, home: &Path) -> Command { Self::command_selecting(config, home, Some("main")) } diff --git a/crates/workshop/desktop/src/gateway/supervisor/launch.rs b/crates/workshop/desktop/src/gateway/supervisor/launch.rs index 73e2526a1..5803e4cd5 100644 --- a/crates/workshop/desktop/src/gateway/supervisor/launch.rs +++ b/crates/workshop/desktop/src/gateway/supervisor/launch.rs @@ -56,7 +56,7 @@ pub(in crate::gateway) fn launch_and_attach_cancellable( run_dir, exe, cancellation, - gateway_api_discovery::launch_or_attach_cancellable, + LaunchDecision::decide_launch_cancellable, |exe, _| boot::spawn_detached(exe, Arc::clone(&exit)), |run_dir, timeout, cancellation| { wait_for_launched_file_cancellable(run_dir, timeout, cancellation, || { @@ -100,7 +100,6 @@ where drop(lock); Ok(boot::RecoveryLaunch::Launched { child_pid, file }) } - decision => anyhow::bail!("an unknown launch decision: {decision:?}"), } } diff --git a/crates/workshop/desktop/src/gateway/tests/cancellation.rs b/crates/workshop/desktop/src/gateway/tests/cancellation.rs index ee40d9a85..25e647c9b 100644 --- a/crates/workshop/desktop/src/gateway/tests/cancellation.rs +++ b/crates/workshop/desktop/src/gateway/tests/cancellation.rs @@ -5,7 +5,7 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, mpsc}; use std::time::Duration; -use gateway_api_discovery::Resolution; +use gateway_api_discovery::{LaunchDecision, Resolution}; use super::{TestIdentity, assert_bounded_supervisor_shutdown, live_file, test_identity}; use crate::gateway::supervisor::{ @@ -139,7 +139,7 @@ fn exit_joins_a_supervisor_blocked_in_health_wait_before_resolve() { #[test] fn exit_joins_a_supervisor_blocked_in_launch_race_before_spawn() { let run = tempfile::TempDir::new().expect("tempdir"); - let decision = gateway_api_discovery::launch_or_attach(run.path(), Duration::from_secs(1)) + let decision = LaunchDecision::decide_launch(run.path(), Duration::from_secs(1)) .expect("acquire a launch decision"); let run_dir = run.path().to_owned(); let (entered, blocked) = mpsc::channel(); @@ -183,7 +183,7 @@ fn exit_joins_a_supervisor_blocked_in_launch_race_before_spawn() { #[test] fn exit_joins_a_supervisor_blocked_inside_launch_without_process_creation() { let run = tempfile::TempDir::new().expect("tempdir"); - let decision = gateway_api_discovery::launch_or_attach(run.path(), Duration::from_secs(1)) + let decision = LaunchDecision::decide_launch(run.path(), Duration::from_secs(1)) .expect("acquire a launch decision"); let run_dir = run.path().to_owned(); let (entered, blocked) = mpsc::channel(); diff --git a/crates/workshop/desktop/src/gateway/tests/recovery.rs b/crates/workshop/desktop/src/gateway/tests/recovery.rs index 26276e425..77afae7b1 100644 --- a/crates/workshop/desktop/src/gateway/tests/recovery.rs +++ b/crates/workshop/desktop/src/gateway/tests/recovery.rs @@ -9,7 +9,9 @@ use std::sync::{Arc, mpsc}; use std::time::{Duration, Instant}; use anyhow::Context as _; -use gateway_api_discovery::{CancellationToken, GatewayDiscoveryFile, ValidatedConnection}; +use gateway_api_discovery::{ + CancellationToken, GatewayDiscoveryFile, LaunchDecision, ValidatedConnection, +}; use super::{ TestIdentity, assert_bounded_supervisor_shutdown, get, live_file, test_identity, @@ -337,7 +339,7 @@ fn successful_publication_disarms_late_child_cleanup() { #[test] fn launched_recovery_retains_the_spawned_pid_and_releases_launch_lock() { let run = tempfile::TempDir::new().expect("create run directory"); - let decision = gateway_api_discovery::launch_or_attach(run.path(), Duration::from_secs(1)) + let decision = LaunchDecision::decide_launch(run.path(), Duration::from_secs(1)) .expect("acquire launch election"); let file = live_file(54_375, "candidate-key"); let cancellation = CancellationToken::new(); @@ -364,7 +366,7 @@ fn launched_recovery_retains_the_spawned_pid_and_releases_launch_lock() { } assert!( matches!( - gateway_api_discovery::launch_or_attach(run.path(), Duration::from_secs(1)) + LaunchDecision::decide_launch(run.path(), Duration::from_secs(1)) .expect("reacquire launch election"), gateway_api_discovery::LaunchDecision::Launch(_) ), 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 bc42d47a9..7d2d2286b 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 @@ -12,7 +12,7 @@ todos: 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" + content: "Supervisor mode (4a-4d): lifetime launch lock, child spawn, exit-code loop, retries for restarts only (capped), .prev rollback, failure record; integration tests against the real binary" status: pending - id: config-ui content: "Config UI: Restart to Apply button, Restart now on the profile banner, rollback failure display, unsupervised fallback; harness and tests" @@ -101,7 +101,7 @@ sequenceDiagram - **Restart is an exit code, not a spawn.** A child that wants a restart shuts down gracefully and exits with `RESTART_REQUESTED`, one constant in the gateway crate (value 75) that `main` and the supervisor both name. The child never launches its successor, so the lease and port races of a self-restart do not exist. The code is private to the supervisor and its child; the supervisor never exits with it, so the value carries no meaning outside the pair beyond being distinct from 0 and 1 and below the 128+N signal range. - **No service-manager special case.** The shipped unit (`crates/gateway/app/packaging/gateway.service`) runs `runvil-gateway --config ... --profile main` with `Restart=on-failure`; that launch becomes a supervisor like any other, and the supervisor's own exit behaves as the gateway's does today: 0 on a stop, and the child's own non-zero code at once when the first boot fails or a running gateway dies, so systemd's restart policy applies unchanged and on the same schedule. A UI restart happens inside the supervisor and systemd never sees it. `--supervised` is an internal flag between the supervisor and its child, not documented for users. The unit file does not change. - **The supervisor is the same binary.** A bare launch, `--login`, and `--no-tray` run the supervisor; `--supervised` runs the serving gateway as today, tray included unless `--no-tray` is also given. The Windows Run key, the Linux autostart file, macOS `SMAppService`, the installer, and the workshop desktop's bare launch (`workshop/desktop/src/gateway/boot.rs:166-173`) all keep working with no change to what they launch. -- **The child keeps the instance lease; the supervisor uses the launch election.** The discovery file and the lease identify the serving process by pid and image (`runner.rs:1071`, `gateway-api-discovery/src/validated.rs:16-19`), so they stay with the child and `settle_gateway_startup` is untouched. The supervisor settles its own start with `launch_or_attach` (`gateway-api-discovery/src/lock.rs:106`): `Attach` is today's second-launch handoff (open Settings unless `--login`), `Launch(lock)` spawns the first child. Across a restart the supervisor takes `gateway.json.lock` before the old child exits and holds it until the new child publishes. The desktop supervisor, which sees the gap as `Missing` on its 5 s probe (`workshop/desktop/src/gateway/supervisor.rs:88-108`), then waits in its own `launch_or_attach` and attaches to the new child instead of launching a competitor. This reuses the election exactly as the lock's doc describes it ("a Workshop parent may hold the launch election while the spawned Gateway acquires this lease", `lock.rs:35-37`). +- **The child keeps the instance lease; the supervisor uses the launch election.** The discovery file and the lease identify the serving process by pid and image (`runner.rs:1071`, `gateway-api-discovery/src/validated.rs:16-19`), so they stay with the child. `settle_gateway_startup` runs both elections: it takes `AppOptions` (moved into the library for this), and for a launch that will supervise (neither `--supervised` nor `--print-url`) it settles the start with `launch_or_attach` (`gateway-api-discovery/src/lock.rs:106`) instead of the lease. `Attach` returns `OpenSettings`, today's second-launch handoff (open Settings unless `--login`); `Launch(lock)` returns the new `GatewayStartup::Supervise(lock)`, and `main` passes the lock to `supervisor::run`, which spawns the first child. A `--supervised` child or a `--print-url` launch takes the lease exactly as before. The supervisor keeps the `Launch` lock for its whole lifetime, not just its first launch: it learns of a restart only when the child exits, too late to take the lock first. Any other `launch_or_attach` finds the lock held and attaches as soon as a live discovery file exists, so while a child serves it attaches at once, and in the gap between children it waits for the new one to publish instead of launching a competitor; the desktop supervisor, which sees the gap as `Missing` on its 5 s probe (`workshop/desktop/src/gateway/supervisor.rs:88-108`), does exactly that. The OS releases the lock however the supervisor exits. This reuses the election exactly as the lock's doc describes it ("a Workshop parent may hold the launch election while the spawned Gateway acquires this lease", `lock.rs:35-37`). - **Orphan safety through a stdin pipe.** The supervisor gives the child a piped stdin; a supervised child watches it on a thread and fires `ShutdownSignal` on EOF. Killing the supervisor closes the pipe on every OS, so the child shuts down gracefully and its `llama-server` children drop through `ServerGuard::drop` (`gateway/local/src/server.rs:586-594`). No Job Object, `PR_SET_PDEATHSIG`, or new unsafe code. Orphaned `llama-server` children after a hard kill of the serving process itself are a pre-existing gap and out of scope. - **Retries belong to restarts only.** The supervisor relaunches only after a child exits with `RESTART_REQUESTED`. A first boot that fails, or a running child that exits with any other code, ends the supervisor with that child's exit code, exactly as the gateway process ends today; the desktop supervisor and systemd keep owning crash recovery. Within a restart, failed launches retry with backoff from 250 ms doubling, at most 5 launches in all; a restart that exhausts them ends the supervisor with the last child's code. - **Restart only restarts; Apply keeps the rollback copies.** `POST /admin/restart` fires the restart and does nothing else; Restart to Apply in the UI is `POST /admin/config-apply` followed by `POST /admin/restart`, a gap of one round trip inside one click. Apply copies each real file it is about to replace to `.prev`, supervised or not, unless that `.prev` already exists: the first copy since the last start is the config that booted, and a second Apply before the restart must not overwrite it. Every gateway removes its `.prev` files right after it publishes its discovery file, the moment the supervisor counts as a successful start, so neither Apply nor the supervisor branches on supervision and an unsupervised gateway's copies last only until its next start. A `.prev` that cannot be removed only warns; a later failed restart would restore it. @@ -160,16 +160,17 @@ This step only adds the option; nothing passes it yet. Step 4's supervisor is it ## 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. +- Start: `main` calls `settle_gateway_startup(&app_options, ..)` as today; for a launch that will supervise it runs `launch_or_attach` in the run directory. `OpenSettings` hands off through `main`'s existing arm (opens Settings unless `--login`). `Supervise(lock)` calls `supervisor::run(&app_options, lock)`, which spawns the child and holds the lock until it returns. +- The supervisor never starts canonical logging: the child rotates and writes the log file as today, and two processes rotating one file would collide. The supervisor's own messages go to its stderr. - `--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 `--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`). +- 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 and held open (the watchdog). Stdout and stderr piped; a reader thread per pipe copies every line to the supervisor's own stdout or stderr respectively, so a terminal or service manager sees the child's output as before, and the stderr reader also keeps the 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. + - `RESTART_REQUESTED`: relaunch, still holding the launch lock. 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`) resets the count. - Anything else, or no publication within 30 s on the first boot: no relaunch. The supervisor exits with the child's code (1 when the child died by a signal, or published nothing and was stopped), having logged the child's last stderr lines and printed them on its own stderr. - Failure record: on rollback the supervisor writes `restart-failure.json` in the run directory (`{"at", "exit", "stderr_tail"}`); the next child reads and deletes it at boot and reports it once in `/admin/status` as `restart_failure`. - Ctrl-C in headless mode stops the child through `request_shutdown` (`gateway-api-discovery/src/shutdown.rs`) and waits, then exits. -- Integration tests (`tests/it/supervisor.rs`, real binary): +- Integration tests (`tests/it/boot/supervisor.rs`, real binary): - `/admin/restart` yields a new child pid on the same port, and the supervisor pid is unchanged. - A promoted config that cannot load (unresolvable `${VAR}`) rolls back: the real file matches the previous content, the rejected edit is `.next`, `/admin/status` reports `restart_failure`. - Killing the supervisor stops the child within the drain bound and removes its discovery file. @@ -178,7 +179,11 @@ This step only adds the option; nothing passes it yet. Step 4's supervisor is it - A config that fails the first boot ends the supervisor at once with the child's exit code and no relaunch. - A running child killed from outside ends the supervisor with a non-zero code and no relaunch. - A restart whose every launch fails (the config is broken and no `.prev` exists) stops after 5 launches and ends the supervisor with the last child's code. -- Commit: `Run the gateway under a restarting supervisor` +- Commits, in order: + - 4a `Run the gateway under a supervisor`: the supervisor election in `settle_gateway_startup` (every launch except `--supervised`, `--print-url`, and `diagnostics`) with `GatewayStartup::Supervise` and its arm in `main`, the lifetime lock and handoff, the child spawn with output forwarding, and exiting with the child's code; tests for a separate child pid, `POST /shutdown` ending both with 0, killing the supervisor stopping the child, and a second launch attaching. + - 4b `Relaunch the gateway on a restart request`: the loop on `RESTART_REQUESTED`, `--preferred-port`, the publication wait, backoff and the 5-launch cap, and the exits for every other code. + - 4c `Roll back a restart that fails to boot`: `.prev` restore, the rejected edit back to `.next`, `restart-failure.json`, and `restart_failure` on `/admin/status`. + - 4d `Stop a console-less child on Ctrl-C`: `CREATE_NO_WINDOW` on Windows and Ctrl-C forwarding through `request_shutdown`. ## Step 5: Config UI @@ -206,6 +211,8 @@ This step only adds the option; nothing passes it yet. Step 4's supervisor is it - **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. +- **Drop `#[non_exhaustive]` from workspace-internal types.** The attribute is a semver guard for published crates; on `publish = false` crates such as `gateway` and `gateway-api-discovery` every consumer is in this workspace and updates in the same change. There it only forces untestable `_` arms (`main`'s "unrecognized gateway startup decision" exit) and turns off the exhaustiveness check that would point at every `match` a new variant must reach: the missing `GatewayStartup::Supervise` arm in Step 4 surfaced as three failing integration tests instead of a compile error. The cleanup removes it from the gateway crates' internal enums and structs (`error.rs` alone has 28), drops the `_` arms it required, and keeps it on the published `runvil` facade, whose surface `cargo xtask api` records. +- **The UI build scripts install their npm dependencies.** `build-ui` (behind the `workshop-server` and `gateway-config-ui` build scripts) requires a manual `npm ci` per checkout and finds esbuild in the nearest `node_modules/.bin` at or above the UI directory (`build-ui/src/lib.rs:246-263`). A stale `crates/workshop/ui/node_modules` from before the `crates/workshop/` npm workspace therefore shadows the missing workspace install, and the failure is an esbuild "Could not resolve `@workshop/look/...`" rather than the "run `npm ci`" message. A follow-up has `build-ui` run `npm ci` itself when the workspace's `node_modules` is missing or older than its `package-lock.json`, and ignore `node_modules` below the lockfile's directory. The open question is network access: an install inside a build script breaks `cargo --offline` and sandboxed builds, so it may need an opt-out (or to stay a clear error naming the command) in CI. ## Constraints