From 31941a46f70ce130a601e20a229054185e28d66d Mon Sep 17 00:00:00 2001 From: Andrew Wheeler Date: Tue, 20 Oct 2026 14:12:31 -0600 Subject: [PATCH] Gateway - Replace custom cli parsing code with clap Replace custom CLI parsing code in gateway with Clap. Clap is a de facto standard for command line parsing in Rust and gives us simpler, smaller, and more standard CLI code. It should also make it considerably easier to change the CLI in the future (add args, more subcommands, etc). Code Changes: The workflow is basically the same as before: Main constructs an AppOptions by calling from_args(), from_args() parses the args using the Cli struct, and main uses AppOptions to decide how to run. - Add a Cli struct to define and parse the command line interface - Rename Invocation to AppOptions to better reflect what it does. Moved parse_args to AppOptions::from_args for a one-stop constructor - Add / update tests User Facing Changes: `runvil-gateway --help` now lists every option with a one-line description, and states the order in which the gateway finds its config: --config, then RUNVIL_GATEWAY_CONFIG, then the executable's directory, the current directory, and the home directory's .runvil folder (%USERPROFILE%\.runvil on Windows), where a default config is generated when none is found. `runvil-gateway diagnostics --help` shows the subcommand's own help. Help is colored on a terminal and plain when piped. A usage error prints the error, a short usage line, and a pointer to `--help` instead of the whole usage text, for example "the subcommand 'diagnostics' cannot be used with '--no-tray'" or "invalid value '../x' for '--profile ': profile name must not contain path separators". Usage errors still exit with status 1. Two behaviors change: - Any flag given twice is an error. Before, only a repeated --config was rejected, and a second --profile silently replaced the first. - -V works as a short form of --version. The flags and the rules between them are otherwise unchanged: `diagnostics` must come first and accepts only `--config`, `serve` and positional arguments are rejected, `--supervised` stays hidden and conflicts with `--print-url`, and `--login` still suppresses `--browser`. --- Cargo.lock | 84 +++++++++ Cargo.toml | 1 + crates/gateway/app/Cargo.toml | 1 + crates/gateway/app/src/cli-tests.rs | 179 +++++++++++------- crates/gateway/app/src/cli.rs | 280 ++++++++++++---------------- crates/gateway/app/src/main.rs | 57 +++--- 6 files changed, 350 insertions(+), 252 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a02d60d7d..3728ccfda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -98,12 +98,56 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" +[[package]] +name = "anstream" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "824a212faf96e9acacdbd09febd34438f8f711fb84e09a8916013cd7815ca28d" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is_terminal_polyfill", + "utf8parse", +] + [[package]] name = "anstyle" version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" +[[package]] +name = "anstyle-parse" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52ce7f38b242319f7cabaa6813055467063ecdc9d355bbb4ce0c68908cd8130e" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" +dependencies = [ + "windows-sys 0.60.2", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" +dependencies = [ + "anstyle", + "once_cell_polyfill", + "windows-sys 0.60.2", +] + [[package]] name = "antithesis_sdk" version = "0.2.9" @@ -924,6 +968,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "473c7e07f409a8d772161724aa8db6a765a2532a70f9667eeb7b49d3d02fbdca" dependencies = [ "clap_builder", + "clap_derive", ] [[package]] @@ -932,8 +977,22 @@ version = "4.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7b48fea5a88e9ae728a2dcbedbfc0e730f7d60da42e1cb049a83c9fb8b789889" dependencies = [ + "anstream", "anstyle", "clap_lex", + "strsim", +] + +[[package]] +name = "clap_derive" +version = "4.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d012d2b9d65aca7f18f4d9878a045bc17899bba951561ba5ec3c2ba1eed9a061" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 3.0.3", ] [[package]] @@ -957,6 +1016,12 @@ version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c9ea0ac24bc397ab3c98583a3c9ba74fa56b09a4449bbe172b9b1ddb016027a" +[[package]] +name = "colorchoice" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" + [[package]] name = "combine" version = "4.6.7" @@ -1966,6 +2031,7 @@ dependencies = [ "base64 0.22.1", "block2", "build-ceiling", + "clap", "dotenvy", "embed-resource", "futures-util", @@ -3193,6 +3259,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "is_terminal_polyfill" +version = "1.70.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" + [[package]] name = "itertools" version = "0.10.5" @@ -4285,6 +4357,12 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" +[[package]] +name = "once_cell_polyfill" +version = "1.70.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" + [[package]] name = "oorandom" version = "11.1.5" @@ -7642,6 +7720,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" +[[package]] +name = "utf8parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" + [[package]] name = "uuid" version = "1.24.0" diff --git a/Cargo.toml b/Cargo.toml index 6791d91e7..1c1627740 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -158,6 +158,7 @@ sha2 = "0.11" hmac = "0.13" subtle = "2.6" dotenvy = "0.15" +clap = { version = "4.6", features = ["derive"] } tar = { version = "0.4.46", default-features = false } zip = { version = "9.0.4", default-features = false, features = ["deflate-flate2"] } # Loads the packaged whisper.cpp runtime without linking it into Cargo builds. diff --git a/crates/gateway/app/Cargo.toml b/crates/gateway/app/Cargo.toml index 64ce31309..1f2497f78 100644 --- a/crates/gateway/app/Cargo.toml +++ b/crates/gateway/app/Cargo.toml @@ -28,6 +28,7 @@ build-ceiling.workspace = true # src/main.rs, enabled only by the `test-fixtures` feature. anyhow = { workspace = true, optional = true } axum.workspace = true +clap.workspace = true dotenvy.workspace = true futures-util.workspace = true # GPU name and VRAM for GET /admin/system; probes NVML at runtime, so builds diff --git a/crates/gateway/app/src/cli-tests.rs b/crates/gateway/app/src/cli-tests.rs index 0023d9352..50294befd 100644 --- a/crates/gateway/app/src/cli-tests.rs +++ b/crates/gateway/app/src/cli-tests.rs @@ -1,5 +1,8 @@ //! Tests for the binary's command-line parsing. +use clap::CommandFactory as _; +use clap::error::ErrorKind; + use super::*; fn args(items: &[&str]) -> Vec { @@ -9,6 +12,18 @@ fn args(items: &[&str]) -> Vec { .collect() } +/// The kind of the parse error for `items`; panics if they parse. +fn error_kind(items: &[&str]) -> ErrorKind { + AppOptions::from_args(args(items)) + .expect_err("the arguments are rejected") + .kind() +} + +#[test] +fn the_command_definition_is_consistent() { + Cli::command().debug_assert(); +} + #[test] fn cli_path_wins_over_env() { let path = resolve_config_path( @@ -43,7 +58,7 @@ fn neither_path_set_defers_to_boot_discovery() { #[test] fn the_root_invocation_serves_with_discovery() { - let invocation = parse_args(args(&[])).expect("the bare invocation parses"); + let invocation = AppOptions::from_args(args(&[])).expect("the bare invocation parses"); assert_eq!( invocation.serve.config_path, None, "no --config defers to boot discovery" @@ -60,25 +75,25 @@ fn the_root_invocation_serves_with_discovery() { #[test] fn the_serve_verb_is_rejected() { - let error = parse_args(args(&["serve"])).unwrap_err(); - assert!( - matches!(error, ParseError::Usage(_)), - "the removed subcommand is a usage error, never an alias: {error:?}" + assert_eq!( + error_kind(&["serve"]), + ErrorKind::InvalidSubcommand, + "the removed subcommand is a usage error, never an alias" ); } #[test] fn a_positional_config_path_is_rejected() { - let error = parse_args(args(&["gateway.toml"])).unwrap_err(); - assert!( - matches!(error, ParseError::Usage(_)), - "the config path is --config PATH, never a positional: {error:?}" + assert_eq!( + error_kind(&["gateway.toml"]), + ErrorKind::InvalidSubcommand, + "the config path is --config PATH, never a positional" ); } #[test] fn the_config_flag_sets_the_path() { - let invocation = parse_args(args(&["--config", "gateway.toml"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--config", "gateway.toml"])).expect("parse"); assert_eq!( invocation.serve.config_path, Some(PathBuf::from("gateway.toml")) @@ -87,20 +102,21 @@ fn the_config_flag_sets_the_path() { #[test] fn the_config_flag_requires_a_value() { - let error = parse_args(args(&["--config"])).unwrap_err(); - assert!(matches!(error, ParseError::Usage(_))); + assert_eq!(error_kind(&["--config"]), ErrorKind::InvalidValue); } #[test] fn the_config_flag_is_given_once() { - let error = parse_args(args(&["--config", "a.toml", "--config", "b.toml"])).unwrap_err(); - assert!(matches!(error, ParseError::Usage(_))); + assert_eq!( + error_kind(&["--config", "a.toml", "--config", "b.toml"]), + ErrorKind::ArgumentConflict + ); } #[test] fn parses_path_and_profile() { - let invocation = - parse_args(args(&["--config", "gateway.toml", "--profile", "dev"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--config", "gateway.toml", "--profile", "dev"])) + .expect("parse"); assert_eq!( invocation.serve.profile.as_ref().map(ProfileName::as_str), Some("dev") @@ -113,14 +129,14 @@ fn parses_path_and_profile() { #[test] fn the_tray_is_default_and_login_is_off() { - let invocation = parse_args(args(&["--config", "gateway.toml"])).expect("parse"); + let invocation = AppOptions::from_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 = parse_args(args(&["--no-tray"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--no-tray"])).expect("parse"); assert!(!invocation.tray); assert!(!invocation.login); } @@ -129,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 = parse_args(args(&["--login"])).expect("parse"); + let invocation = AppOptions::from_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 = parse_args(args(&["--print-url"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--print-url"])).expect("parse"); assert!(invocation.print_url); assert!( invocation.tray, @@ -147,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 = parse_args(args(&[ + let invocation = AppOptions::from_args(args(&[ "--config", "gateway.toml", "--no-tray", @@ -164,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 = parse_args(args(&["--browser"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--browser"])).expect("parse"); assert!( invocation.serve.browser, "the flag reaches the spawn hook through ServeOptions" @@ -174,7 +190,7 @@ fn browser_parses_and_reaches_the_serve_options() { #[test] fn login_wins_over_browser() { - let invocation = parse_args(args(&["--login", "--browser"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--login", "--browser"])).expect("parse"); assert!( !invocation.serve.browser, "a login launch never opens a browser" @@ -183,67 +199,70 @@ fn login_wins_over_browser() { #[test] fn missing_profile_defers_to_environment_or_state() { - let invocation = parse_args(args(&["--config", "gateway.toml"])).expect("parse"); + let invocation = AppOptions::from_args(args(&["--config", "gateway.toml"])).expect("parse"); assert!(invocation.serve.profile.is_none()); } #[test] fn invalid_profile_name_is_a_usage_error() { - let error = parse_args(args(&["--config", "gateway.toml", "--profile", ""])).unwrap_err(); - assert!(matches!(error, ParseError::Usage(_))); + assert_eq!( + error_kind(&["--config", "gateway.toml", "--profile", ""]), + ErrorKind::ValueValidation + ); } #[test] fn rejects_traversal_profile_name() { - let error = parse_args(args(&[ - "--config", - "gateway.toml", - "--profile", - "../escape", - ])) - .unwrap_err(); - assert!(matches!(error, ParseError::Usage(_))); + assert_eq!( + error_kind(&["--config", "gateway.toml", "--profile", "../escape"]), + ErrorKind::ValueValidation + ); } #[test] fn rejects_an_unknown_argument() { - let error = parse_args(args(&["frobnicate"])).unwrap_err(); - assert!(matches!(error, ParseError::Usage(_))); + assert_eq!(error_kind(&["frobnicate"]), ErrorKind::InvalidSubcommand); } #[test] fn help_is_recognized() { - let error = parse_args(args(&["--help"])).unwrap_err(); - assert_eq!(error, ParseError::Help); - let error = parse_args(args(&["-h"])).unwrap_err(); - assert_eq!(error, ParseError::Help); + for flag in ["--help", "-h"] { + let error = AppOptions::from_args(args(&[flag])).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::DisplayHelp, "{flag}"); + assert!(!error.use_stderr(), "{flag} prints to stdout and succeeds"); + } } #[test] fn version_is_recognized() { - let error = parse_args(args(&["--version"])).unwrap_err(); - assert_eq!(error, ParseError::Version); + for flag in ["--version", "-V"] { + let error = AppOptions::from_args(args(&[flag])).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::DisplayVersion, "{flag}"); + assert!(!error.use_stderr(), "{flag} prints to stdout and succeeds"); + } } #[test] fn rejects_unknown_flag() { - let error = parse_args(args(&["--profiles-dir", "x", "--profile", "dev"])).unwrap_err(); - assert!(matches!(error, ParseError::Usage(_))); + assert_eq!( + error_kind(&["--profiles-dir", "x", "--profile", "dev"]), + ErrorKind::UnknownArgument + ); } #[test] fn diagnostics_is_the_only_subcommand() { - let invocation = parse_args(args(&["diagnostics"])).expect("parses"); + let invocation = AppOptions::from_args(args(&["diagnostics"])).expect("parses"); assert_eq!(invocation.command, LaunchCommand::Diagnostics); assert_eq!(invocation.serve.config_path, None); - let invocation = parse_args(args(&[])).expect("the bare invocation parses"); + let invocation = AppOptions::from_args(args(&[])).expect("the bare invocation parses"); assert_eq!(invocation.command, LaunchCommand::Serve); } #[test] fn diagnostics_accepts_a_config_path() { let invocation = - parse_args(args(&["diagnostics", "--config", "gateway.toml"])).expect("parses"); + AppOptions::from_args(args(&["diagnostics", "--config", "gateway.toml"])).expect("parses"); assert_eq!(invocation.command, LaunchCommand::Diagnostics); assert_eq!( invocation.serve.config_path, @@ -254,26 +273,27 @@ fn diagnostics_accepts_a_config_path() { #[test] fn diagnostics_rejects_serving_flags() { for rest in ["--no-tray", "--login", "--print-url", "--browser"] { - let error = parse_args(args(&["diagnostics", rest])).unwrap_err(); - assert!( - matches!(error, ParseError::Usage(_)), - "diagnostics rejects {rest}: {error:?}" + assert_eq!( + error_kind(&["diagnostics", rest]), + ErrorKind::UnknownArgument, + "diagnostics rejects {rest}" ); } } #[test] fn diagnostics_after_a_flag_is_not_a_subcommand() { - let error = parse_args(args(&["--no-tray", "diagnostics"])).unwrap_err(); - assert!( - matches!(error, ParseError::Usage(_)), - "the subcommand must come first: {error:?}" + assert_eq!( + error_kind(&["--no-tray", "diagnostics"]), + ErrorKind::ArgumentConflict, + "the subcommand must come first" ); } #[test] fn supervised_option_parses_and_reaches_the_serve_options() { - let invocation = parse_args(args(&["--supervised", "--no-tray"])).expect("should parse"); + let invocation = + AppOptions::from_args(args(&["--supervised", "--no-tray"])).expect("should parse"); assert!(invocation.serve.supervised); assert!( @@ -284,11 +304,12 @@ fn supervised_option_parses_and_reaches_the_serve_options() { #[test] fn supervised_option_does_not_override_no_tray_option() { - let invocation = parse_args(args(&["--supervised"])).expect("should parse"); + let invocation = AppOptions::from_args(args(&["--supervised"])).expect("should parse"); assert!(invocation.tray, "--supervised should not set tray to false"); - let invocation = parse_args(args(&["--supervised", "--no-tray"])).expect("should parse"); + let invocation = + AppOptions::from_args(args(&["--supervised", "--no-tray"])).expect("should parse"); assert!( !invocation.tray, @@ -298,7 +319,7 @@ fn supervised_option_does_not_override_no_tray_option() { #[test] fn a_launch_is_not_supervised_by_default() { - let invocation = parse_args(args(&[])).expect("should parse"); + let invocation = AppOptions::from_args(args(&[])).expect("should parse"); assert!(!invocation.serve.supervised); } @@ -306,10 +327,42 @@ fn a_launch_is_not_supervised_by_default() { #[test] fn supervised_and_print_url_options_cannot_be_used_together() { let print_url = "--print-url"; - let error = parse_args(args(&["--supervised", print_url])).unwrap_err(); + let kind = error_kind(&["--supervised", print_url]); - assert!( - matches!(error, ParseError::Usage(_)), - "--supervised {print_url} cannot both be set: {error:?}" + assert_eq!( + kind, + ErrorKind::ArgumentConflict, + "--supervised {print_url} cannot both be set" ); } + +#[test] +fn diagnostics_has_its_own_help() { + let error = AppOptions::from_args(args(&["diagnostics", "--help"])).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::DisplayHelp); + let help = error.to_string(); + assert!(help.contains("--config"), "{help}"); + assert!(!help.contains("--no-tray"), "{help}"); +} + +#[test] +fn the_help_text_hides_the_supervised_flag() { + let help = Cli::command().render_long_help().to_string(); + assert!(!help.contains("--supervised"), "{help}"); +} + +#[test] +fn the_help_text_lists_all_flags() { + let help = Cli::command().render_long_help().to_string(); + for flag in [ + "--config", + "--profile", + "--no-tray", + "--login", + "--print-url", + "--browser", + "diagnostics", + ] { + assert!(help.contains(flag), "the help names {flag}: {help}"); + } +} diff --git a/crates/gateway/app/src/cli.rs b/crates/gateway/app/src/cli.rs index 3a9c5e670..921cda290 100644 --- a/crates/gateway/app/src/cli.rs +++ b/crates/gateway/app/src/cli.rs @@ -1,189 +1,145 @@ use std::{ffi::OsString, path::PathBuf}; +use clap::{Parser, Subcommand}; use gateway::{ProfileName, ServeOptions}; -pub(crate) const USAGE: &str = concat!( - "usage: runvil-gateway [--config PATH] [--profile NAME] [--no-tray] [--login] [--print-url] [--browser]\n", - " runvil-gateway diagnostics [--config PATH]\n", - " runvil-gateway --version\n", - "the config path may also be set with the RUNVIL_GATEWAY_CONFIG environment variable;\n", - "--config wins over it\n", - "with no config path, the gateway searches beside the executable, the current directory,\n", - "and the profile's .runvil directory, generating a default config on first run\n", - "diagnostics print a JSON report of the state dir, config, logs, and gateway discovery file;\n", - " never serves, rotates logs, or parses the config\n", - "--no-tray run headless (Ctrl-C driven); for servers and CI\n", - "--login the launch came from the OS autostart entry; never opens a browser\n", - "--print-url print the Settings handoff URL once bound, then serve headless;\n", - " with a gateway already running, print its URL instead\n", - "--browser open the Settings page in the default browser once bound;\n", - " the installer's first run uses this", - // --supervised is internal only, so not included in the user-facing usage message -); - -/// Why argument parsing stopped. -#[derive(Debug, PartialEq, Eq)] -pub(crate) enum ParseError { - /// `-h`/`--help` was requested. - Help, - /// `--version` was requested. - Version, - /// The arguments were invalid; the string is the operator-facing reason. - Usage(String), -} - -/// 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, -} - -/// The parsed invocation: the serve options plus how the main thread runs. +/// The parsed app options: the serve options plus how the main thread runs. #[derive(Debug)] -pub(crate) struct Invocation { +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, } -/// Parses the command line into a typed [`Invocation`]. -/// -/// The bare invocation serves; there are no subcommands. Uses `OsString` -/// operands so non-UTF-8 config 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. -pub(crate) fn parse_args( - args: impl IntoIterator, -) -> Result { - let mut args = args.into_iter(); - let _binary = args.next(); - - // `diagnostics` is the only subcommand and must come first. - let mut args = args.peekable(); - if args.peek().and_then(|arg| arg.to_str()) == Some("diagnostics") { - args.next(); - return parse_diagnostics_args(args); - } +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 mut profile: Option = None; - let mut config_path: Option = None; - let mut tray = true; - let mut login = false; - let mut print_url = false; - let mut browser = false; - let mut supervised = false; - - while let Some(arg) = args.next() { - match arg.to_str() { - Some("--config") => { - let path = args - .next() - .ok_or_else(|| ParseError::Usage("--config requires a path".to_string()))?; - if config_path.is_some() { - return Err(ParseError::Usage("--config accepts one path".to_string())); - } - config_path = Some(PathBuf::from(path)); - } - Some("--profile") => { - let name = args - .next() - .ok_or_else(|| ParseError::Usage("--profile requires a name".to_string()))?; - let name = name.into_string().map_err(|_| { - ParseError::Usage("--profile name must be valid UTF-8".to_string()) - })?; - let name = ProfileName::parse(&name) - .map_err(|error| ParseError::Usage(format!("invalid profile name: {error}")))?; - profile = Some(name); - } - Some("--no-tray") => tray = false, - Some("--login") => login = true, - Some("--print-url") => print_url = true, - Some("--browser") => browser = true, - Some("--supervised") => supervised = true, - Some("-h" | "--help") => return Err(ParseError::Help), - Some("--version") => return Err(ParseError::Version), - Some(other) if other.starts_with('-') => { - return Err(ParseError::Usage(format!("unknown flag {other}"))); - } - _ => { - return Err(ParseError::Usage(format!( - "unexpected argument {}", - arg.to_string_lossy() - ))); - } - } - } + 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, + }, - if supervised && print_url { - return Err(ParseError::Usage( - "--supervised cannot be combined with --print-url".to_string(), - )); + 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), + tray: !cli.no_tray, + login: cli.login, + print_url: cli.print_url, + }, + }) } +} - let config_path = resolve_config_path(config_path, std::env::var_os("RUNVIL_GATEWAY_CONFIG")); - - Ok(Invocation { - 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, profile) - .with_browser(browser && !login) - .with_supervised(supervised), - tray, - login, - 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 what may follow `diagnostics`: at most `--config PATH`. Every -/// other flag belongs to a serving launch and is a usage error here. -fn parse_diagnostics_args(args: impl Iterator) -> Result { - let mut args = args; - let mut config_path: Option = None; - while let Some(arg) = args.next() { - match arg.to_str() { - Some("--config") => { - let path = args - .next() - .ok_or_else(|| ParseError::Usage("--config requires a path".to_string()))?; - if config_path.is_some() { - return Err(ParseError::Usage("--config accepts one path".to_string())); - } - config_path = Some(PathBuf::from(path)); - } - Some("-h" | "--help") => return Err(ParseError::Help), - _ => { - return Err(ParseError::Usage(format!( - "diagnostics accepts only --config PATH, got {}", - arg.to_string_lossy() - ))); - } - } - } - let config_path = resolve_config_path(config_path, std::env::var_os("RUNVIL_GATEWAY_CONFIG")); - Ok(Invocation { - command: LaunchCommand::Diagnostics, - serve: ServeOptions::new(config_path, None), - tray: false, - login: false, - print_url: false, - }) +/// The command line as clap reads it +#[derive(Debug, Parser)] +#[command( + name = "runvil-gateway", + version, + about = "Runvil inference gateway: routes OpenAI-shaped chat completions to a backend", + after_help = r#"The gateway reads the first config it finds, in the following order. + --config + RUNVIL_GATEWAY_CONFIG environment variable + The directory containing the executable + The current directory + The user's home .runvil directory (~/.runvil or %USERPROFILE%\.runvil on Windows) +If none is found, a default config is generated in the .runvil directory."#, + // `diagnostics` must come first: `--no-tray diagnostics` is a usage error, not a subcommand. + args_conflicts_with_subcommands = true, + disable_help_subcommand = true +)] +#[expect( + clippy::struct_excessive_bools, + reason = "each bool is an independent command-line flag" +)] +struct Cli { + #[command(subcommand)] + command: Option, + + /// The config file + #[arg(long, value_name = "PATH", global = true)] + config: Option, + + /// The profile to serve + #[arg(long, value_name = "NAME", value_parser = ProfileName::parse)] + profile: Option, + + /// Run headless (Ctrl-C driven); for servers and CI + #[arg(long)] + no_tray: bool, + + /// The launch came from the OS autostart entry; never opens a browser + #[arg(long)] + login: bool, + + /// Print the Settings handoff URL once bound, then serve headless; with a + /// gateway already running, print its URL instead + #[arg(long)] + print_url: bool, + + /// Open the Settings page in the default browser once bound; the + /// installer's first run uses this + #[arg(long)] + browser: bool, + + /// The launch is a supervised child; internal only + #[arg(long, hide = true, conflicts_with = "print_url")] + supervised: bool, +} + +#[derive(Debug, Subcommand)] +enum SubCommand { + /// Print a JSON report of the state dir, config, logs, and gateway + /// discovery file. Does not serve, rotate logs, or parse the config + Diagnostics, } /// Resolves the config path: the `--config` flag wins, then the @@ -197,9 +153,9 @@ fn parse_diagnostics_args(args: impl Iterator) -> Result, env: Option) -> Option { - cli.or_else(|| { - let path = PathBuf::from(env?); +fn resolve_config_path(from_cli: Option, from_env: Option) -> Option { + from_cli.or_else(|| { + let path = PathBuf::from(from_env?); if path.is_file() { return Some(path); } diff --git a/crates/gateway/app/src/main.rs b/crates/gateway/app/src/main.rs index c82d3c9e4..13ee4d553 100644 --- a/crates/gateway/app/src/main.rs +++ b/crates/gateway/app/src/main.rs @@ -1,5 +1,6 @@ //! The `runvil-gateway` binary: -//! `runvil-gateway [--config PATH] [--profile NAME] [--no-tray] [--login] [--print-url] [--browser]`. +//! `runvil-gateway [--config PATH] [--profile NAME] [--no-tray] [--login] [--print-url] [--browser]`, +//! or `runvil-gateway diagnostics [--config PATH]`. //! //! This is a thin shell: it parses arguments into a typed `ServeOptions` and //! hands off to [`run_with_tray`], which owns the tokio runtime, provisioning, @@ -21,7 +22,7 @@ use tracing_subscriber::Layer as _; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; -use crate::cli::{LaunchCommand, ParseError, USAGE, parse_args}; +use crate::cli::{AppOptions, LaunchCommand}; mod cli; @@ -59,30 +60,30 @@ fn main() -> ExitCode { ); } - let invocation = match parse_args(std::env::args_os()) { - Ok(invocation) => invocation, - Err(ParseError::Help) => { - println!("{USAGE}"); - return ExitCode::SUCCESS; - } - Err(ParseError::Version) => { - println!("runvil-gateway {}", env!("CARGO_PKG_VERSION")); - return ExitCode::SUCCESS; - } - Err(ParseError::Usage(message)) => { - eprintln!("error: {message}"); - eprintln!("{USAGE}"); - return ExitCode::FAILURE; + let app_options = match AppOptions::from_args(std::env::args_os()) { + Ok(options) => options, + + // clap returns `--help` and `--version` through the error channel. + // If `exit_code()` is `0` then clap returned `Err(DisplayHelp)` or + // `Err(DisplayVersion)` as a successful response and the program should + // exit successfully. Otherwise, there was an error parsing the command + // line args and the program should exit with a failure code. + Err(error) => { + let _ = error.print(); + return match error.exit_code() { + 0 => ExitCode::SUCCESS, + _ => ExitCode::FAILURE, + }; } }; // The diagnostics report is not a boot: it runs before the handoff // check and before logging starts, and never rotates a log, parses a // config, or mutates the state directory. - if invocation.command == LaunchCommand::Diagnostics { + if app_options.command == LaunchCommand::Diagnostics { print!( "{}", - gateway::diagnostics_json(invocation.serve.config_path) + gateway::diagnostics_json(app_options.serve.config_path) ); return ExitCode::SUCCESS; } @@ -97,13 +98,15 @@ 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(&invocation.serve, OWNER_PUBLICATION_TIMEOUT) - { + let _instance_lease = match settle_gateway_startup( + &app_options.serve, + OWNER_PUBLICATION_TIMEOUT, + ) { Ok(GatewayStartup::Boot(lease)) => lease, Ok(GatewayStartup::OpenSettings(url)) => { - if invocation.print_url { + if app_options.print_url { println!("{url}"); - } else if invocation.login || invocation.serve.supervised { + } else if app_options.login || app_options.serve.supervised { // A login-triggered start never opens a browser, and a // supervised child leaves what the user sees to its // supervisor; the running gateway leaves this launch @@ -130,12 +133,12 @@ fn main() -> ExitCode { // out from under it. let logging = init_logging(); - let result = if invocation.print_url { - run_printing_url(&invocation.serve) - } else if invocation.tray { - run_with_tray(&invocation.serve) + let result = if app_options.print_url { + run_printing_url(&app_options.serve) + } else if app_options.tray { + run_with_tray(&app_options.serve) } else { - run(&invocation.serve) + run(&app_options.serve) }; let exit = match result { Ok(shutdown_reason) => {