From 834a1c15846128f822ef523179a0f758a9c5961c Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Thu, 29 Oct 2026 13:51:23 +0800 Subject: [PATCH 1/2] fix: exit session-construction failures by diagnostic class The check, lookup, and serve wrappers hardcoded exit 2 for every CompilerSession::new / ToolService::new failure, so the process exit disagreed with Diagnostic.class whenever a construction diagnostic carried anything but Usage. CompilerSession::new now classifies lint-rule-error by failure kind: a missing or unreadable --rule-file path (RuleRegistryError::Io) is Usage, while invalid rule content stays SourceError. activate_pinned declares each package failure with the LintPackageError exit class that `lint package` already reports, so a bad pin keeps exit 2 at the session boundary. check and lookup accept --rule-file like lint and serve, and the failure tables pin lint-rule-error on both classes. A CLI test drives missing and invalid rule files through check, lookup, and serve. Fixes #1028 --- crates/wright-cli/src/cli.rs | 6 ++ crates/wright-cli/src/main.rs | 18 ++---- crates/wright-cli/src/serve.rs | 4 +- crates/wright-cli/src/session.rs | 23 +++++++ crates/wright-cli/tests/session_exits.rs | 61 ++++++++++++++++++ crates/wright-driver/src/lint_package.rs | 3 + crates/wright-driver/src/session/registry.rs | 13 +++- crates/wright-driver/tests/check_failures.rs | 35 +++++++++-- crates/wright-driver/tests/lookup_failures.rs | 63 +++++++++++++++---- 9 files changed, 192 insertions(+), 34 deletions(-) create mode 100644 crates/wright-cli/src/session.rs create mode 100644 crates/wright-cli/tests/session_exits.rs diff --git a/crates/wright-cli/src/cli.rs b/crates/wright-cli/src/cli.rs index dcbe12d1..88180cc0 100644 --- a/crates/wright-cli/src/cli.rs +++ b/crates/wright-cli/src/cli.rs @@ -190,6 +190,9 @@ pub(crate) struct ReportArgs { pub(crate) common: CommonArgs, #[command(flatten)] pub(crate) select: SelectArgs, + /// Load a local YAML rule file or directory (repeatable). + #[arg(long = "rule-file", value_name = "PATH")] + pub(crate) rule_file: Vec, } /// Arguments of `analyze`: shared workflow options, finding selection, and @@ -294,6 +297,9 @@ pub(crate) struct LookupArgs { /// Override the first-party OPY provider executable. #[arg(long, value_name = "PATH")] pub(crate) opy_provider: Option, + /// Load a local YAML rule file or directory (repeatable). + #[arg(long = "rule-file", value_name = "PATH")] + pub(crate) rule_file: Vec, /// Output format. #[arg(short = 'f', long, value_enum, default_value_t = OutputFormatArg::Text)] pub(crate) format: OutputFormatArg, diff --git a/crates/wright-cli/src/main.rs b/crates/wright-cli/src/main.rs index 8709c132..47e40e17 100644 --- a/crates/wright-cli/src/main.rs +++ b/crates/wright-cli/src/main.rs @@ -8,6 +8,7 @@ mod mcp; mod plain; mod present; mod serve; +mod session; mod tooldefs; mod update; mod usage_hint; @@ -176,6 +177,7 @@ fn run_workflow(command: Command) -> ExitCode { Command::Check(args) => { let mut config = config_from_common(&args.common); config.selection = selection_from_args(&args.select); + config.lint_rule_paths = args.rule_file; run_configured( "check", config, @@ -360,10 +362,7 @@ pub(crate) fn run_configured< ) -> ExitCode { let mut session = match wright_driver::CompilerSession::new(config) { Ok(session) => session, - Err(diagnostic) => { - present::terminal::report_error(presentation, "USAGE", name, &diagnostic.message); - return ExitCode::from(exit::USAGE); - } + Err(diagnostic) => return session::refuse(presentation, name, &diagnostic), }; ExitCode::from(run_command(&mut session, run, presentation, subject)) @@ -467,6 +466,7 @@ fn run_lookup(args: &cli::LookupArgs) -> ExitCode { text: String::new(), path: None, }, + lint_rule_paths: args.rule_file.clone(), opy_provider: wright_driver::OpyProviderConfig { executable: args.opy_provider.clone(), ..wright_driver::OpyProviderConfig::default() @@ -475,17 +475,11 @@ fn run_lookup(args: &cli::LookupArgs) -> ExitCode { }; let mut session = match wright_driver::CompilerSession::new(config) { Ok(session) => session, - Err(diagnostic) => { - present::terminal::report_error(pres, "USAGE", "lookup", &diagnostic.message); - return ExitCode::from(exit::USAGE); - } + Err(diagnostic) => return session::refuse(pres, "lookup", &diagnostic), }; let mut service = match wright_driver::service::ToolService::new(&mut session) { Ok(service) => service, - Err(diagnostic) => { - present::terminal::report_error(pres, "USAGE", "lookup", &diagnostic.message); - return ExitCode::from(exit::USAGE); - } + Err(diagnostic) => return session::refuse(pres, "lookup", &diagnostic), }; let request = wright_driver::service::ToolRequest::Lookup { language: args.language.as_str().to_string(), diff --git a/crates/wright-cli/src/serve.rs b/crates/wright-cli/src/serve.rs index 27f82e98..f22cd30d 100644 --- a/crates/wright-cli/src/serve.rs +++ b/crates/wright-cli/src/serve.rs @@ -130,14 +130,14 @@ pub(crate) fn run(args: ServeArgs) -> ExitCode { Ok(session) => session, Err(diagnostic) => { eprintln!("wright: {}", diagnostic.message); - return ExitCode::from(2); + return ExitCode::from(diagnostic.class.exit_code()); } }; let mut service = match ToolService::new(&mut session) { Ok(service) => service, Err(diagnostic) => { eprintln!("wright: {}", diagnostic.message); - return ExitCode::from(2); + return ExitCode::from(diagnostic.class.exit_code()); } }; diff --git a/crates/wright-cli/src/session.rs b/crates/wright-cli/src/session.rs new file mode 100644 index 00000000..475400ee --- /dev/null +++ b/crates/wright-cli/src/session.rs @@ -0,0 +1,23 @@ +//! Session- and service-construction failures (#1028): the diagnostic's +//! declared exit class sets the process exit, so every command reports +//! the word and code the diagnostic carries instead of a fixed usage +//! exit. + +use std::process::ExitCode; + +use wright_driver::Diagnostic; + +use crate::present::{self, Presentation}; + +/// Report a `CompilerSession::new`/`ToolService::new` construction +/// diagnostic and return the exit code its declared class carries. +pub(crate) fn refuse(pres: Presentation, name: &str, diagnostic: &Diagnostic) -> ExitCode { + let code = diagnostic.class.exit_code(); + present::terminal::report_error( + pres, + present::terminal::exit_word(code), + name, + &diagnostic.message, + ); + ExitCode::from(code) +} diff --git a/crates/wright-cli/tests/session_exits.rs b/crates/wright-cli/tests/session_exits.rs new file mode 100644 index 00000000..1f6e9e92 --- /dev/null +++ b/crates/wright-cli/tests/session_exits.rs @@ -0,0 +1,61 @@ +//! Session-construction exits follow the diagnostic's declared class +//! (#1028): `lint-rule-error` splits by failure kind — a missing or +//! unreadable `--rule-file` path is a usage error (2), while a file +//! that loads but is invalid content is a source error (1). Each of +//! the three session-construction surfaces must exit by the class the +//! diagnostic carries, not a fixed wrapper exit. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn wright() -> &'static str { + env!("CARGO_BIN_EXE_wright") +} + +fn run(args: &[&str]) -> std::process::Output { + Command::new(wright()) + .args(args) + .output() + .expect("wright runs") +} + +fn temp_dir() -> PathBuf { + use std::sync::atomic::{AtomicUsize, Ordering}; + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let dir = std::env::temp_dir().join(format!( + "wright-session-exits-{}-{}", + std::process::id(), + COUNTER.fetch_add(1, Ordering::SeqCst) + )); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +#[test] +fn session_construction_exits_follow_the_diagnostic_class() { + let dir = temp_dir(); + let missing = dir.join("missing.yaml"); + let invalid = dir.join("invalid.yaml"); + std::fs::write(&invalid, "id: [unclosed\n").unwrap(); + for command in [&["check"][..], &["lookup", "wait"][..], &["serve"][..]] { + for (rule_file, code) in [(&missing, 2), (&invalid, 1)] { + let args: Vec = command + .iter() + .map(|arg| arg.to_string()) + .chain([ + "--rule-file".to_string(), + rule_file.to_string_lossy().into_owned(), + ]) + .collect(); + let output = run(&args.iter().map(String::as_str).collect::>()); + assert_eq!( + output.status.code(), + Some(code), + "{args:?}: status={} stderr={}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + } + } + let _ = std::fs::remove_dir_all(Path::new(&dir)); +} diff --git a/crates/wright-driver/src/lint_package.rs b/crates/wright-driver/src/lint_package.rs index 327f2042..0b2542f6 100644 --- a/crates/wright-driver/src/lint_package.rs +++ b/crates/wright-driver/src/lint_package.rs @@ -496,6 +496,7 @@ pub(crate) fn activate_pinned( let pin_error = |message: String| { let error = LintPackageError::source(message); Diagnostic::error(error.code(), Stage::Discovery, error.to_string()) + .with_class(error.exit_class()) }; let store = packages.store(); for pin in &lint.packages { @@ -505,6 +506,7 @@ pub(crate) fn activate_pinned( Stage::Discovery, format!("lint package pin '{}': {error}", pin.repo), ) + .with_class(error.exit_class()) })?; if let Some(reference) = reference { return Err(pin_error(format!( @@ -529,6 +531,7 @@ pub(crate) fn activate_pinned( stage, format!("lint package '{source}' at {}: {error}", pin.sha), ) + .with_class(error.exit_class()) })?; let rules_dir = installed.rules_dir(); let loaded = if source.is_first_party() { diff --git a/crates/wright-driver/src/session/registry.rs b/crates/wright-driver/src/session/registry.rs index 13f866ef..f19dc8f5 100644 --- a/crates/wright-driver/src/session/registry.rs +++ b/crates/wright-driver/src/session/registry.rs @@ -1,7 +1,7 @@ -use wright_analyzer::registry::LintRegistry; +use wright_analyzer::registry::{LintRegistry, RuleRegistryError}; use crate::config::SessionConfig; -use crate::diag::{Diagnostic, Stage}; +use crate::diag::{Diagnostic, ExitClass, Stage}; pub(super) fn compose(config: &SessionConfig) -> Result { let mut registry = LintRegistry::default(); @@ -12,7 +12,14 @@ pub(super) fn compose(config: &SessionConfig) -> Result ExitClass::Usage, + _ => ExitClass::SourceError, + }; + Diagnostic::classed_error("lint-rule-error", Stage::Analysis, class, error.to_string()) })?; } if let Err(message) = config.selection.validate(®istry) { diff --git a/crates/wright-driver/tests/check_failures.rs b/crates/wright-driver/tests/check_failures.rs index 37f395ab..ee3c9b5e 100644 --- a/crates/wright-driver/tests/check_failures.rs +++ b/crates/wright-driver/tests/check_failures.rs @@ -455,10 +455,6 @@ fn check_surfaces_every_failure_kind_with_its_code_and_exit_class() { // The session machinery's own refusal kinds: construction // diagnostics (observed as the session boundary's `Err`), the // fixed-config refusal, and the `source-provider-*` load refusals. - // `lint-rule-error` is deliberately unpinned: its declared - // `SourceError` class disagrees with the CLI wrappers, which exit - // `USAGE` for every session-construction failure — the discrepancy - // awaits a separate owner decision. let Err(diagnostic) = CompilerSession::new(SessionConfig { selection: FindingSelection { rule: Some("no-such-rule".to_string()), @@ -472,6 +468,37 @@ fn check_surfaces_every_failure_kind_with_its_code_and_exit_class() { assert_eq!(diagnostic.class, ExitClass::Usage); assert_eq!(diagnostic.class.exit_code(), ExitClass::Usage.exit_code()); + // `lint-rule-error` splits by failure kind (#1028): a missing or + // unreadable `--rule-file` path is a usage error, while a file that + // loads but is invalid content is a source error. + { + let dir = temp_dir("check-rule-file"); + let Err(diagnostic) = CompilerSession::new(SessionConfig { + lint_rule_paths: vec![dir.join("missing.yaml")], + ..SessionConfig::default() + }) else { + panic!("a missing --rule-file must refuse session construction"); + }; + assert_eq!(diagnostic.code, "lint-rule-error"); + assert_eq!(diagnostic.class, ExitClass::Usage); + assert_eq!(diagnostic.class.exit_code(), ExitClass::Usage.exit_code()); + + let invalid = write_file(&dir, "invalid.yaml", "id: [unclosed\n"); + let Err(diagnostic) = CompilerSession::new(SessionConfig { + lint_rule_paths: vec![invalid], + ..SessionConfig::default() + }) else { + panic!("an invalid --rule-file must refuse session construction"); + }; + assert_eq!(diagnostic.code, "lint-rule-error"); + assert_eq!(diagnostic.class, ExitClass::SourceError); + assert_eq!( + diagnostic.class.exit_code(), + ExitClass::SourceError.exit_code() + ); + std::fs::remove_dir_all(&dir).expect("cleanup"); + } + // `session-config-changed` — the fixed-config refusal. { let dir = temp_dir("check-config"); diff --git a/crates/wright-driver/tests/lookup_failures.rs b/crates/wright-driver/tests/lookup_failures.rs index 695d1b4b..c208d7a1 100644 --- a/crates/wright-driver/tests/lookup_failures.rs +++ b/crates/wright-driver/tests/lookup_failures.rs @@ -383,16 +383,29 @@ fn lookup_surfaces_every_failure_kind_with_its_code_and_exit_class() { // The session machinery's own failure kinds at the service // boundary: construction diagnostics keep their own code and class, // and `session-config-changed` refuses `ToolService::new` itself. - // `lint-rule-error` is deliberately unpinned: its declared - // `SourceError` class disagrees with the CLI wrappers, which exit - // `USAGE` for every session-construction failure — the discrepancy - // awaits a separate owner decision. - let session_failures: &[(&str, ExitClass)] = &[ - ("invalid-selection", ExitClass::Usage), - ("session-config-changed", ExitClass::SourceError), + // `lint-rule-error` splits by failure kind (#1028): a missing or + // unreadable `--rule-file` path is `Usage`, while a file that loads + // but is invalid content is `SourceError`. + let session_failures: &[(&str, &str, ExitClass)] = &[ + ("invalid-selection", "invalid-selection", ExitClass::Usage), + ( + "session-config-changed", + "session-config-changed", + ExitClass::SourceError, + ), + ( + "lint-rule-error:missing", + "lint-rule-error", + ExitClass::Usage, + ), + ( + "lint-rule-error:invalid", + "lint-rule-error", + ExitClass::SourceError, + ), ]; - for (code, class) in session_failures { - let diagnostic = match *code { + for (name, code, class) in session_failures { + let diagnostic = match *name { "session-config-changed" => { let mut session = CompilerSession::new(SessionConfig { input: InputSpec::Path(workshop_path()), @@ -418,11 +431,35 @@ fn lookup_surfaces_every_failure_kind_with_its_code_and_exit_class() { }; diagnostic } - _ => unreachable!("{code} has no session-boundary trigger"), + "lint-rule-error:missing" => { + let dir = temp_dir("lookup-rule-file"); + let Err(diagnostic) = CompilerSession::new(SessionConfig { + lint_rule_paths: vec![dir.join("missing.yaml")], + ..SessionConfig::default() + }) else { + panic!("a missing --rule-file must refuse the session"); + }; + std::fs::remove_dir_all(&dir).expect("cleanup"); + diagnostic + } + "lint-rule-error:invalid" => { + let dir = temp_dir("lookup-rule-file"); + let path = dir.join("invalid.yaml"); + std::fs::write(&path, "id: [unclosed\n").expect("write file"); + let Err(diagnostic) = CompilerSession::new(SessionConfig { + lint_rule_paths: vec![path], + ..SessionConfig::default() + }) else { + panic!("an invalid --rule-file must refuse the session"); + }; + std::fs::remove_dir_all(&dir).expect("cleanup"); + diagnostic + } + _ => unreachable!("{name} has no session-boundary trigger"), }; - assert_eq!(diagnostic.code, *code); - assert_eq!(diagnostic.class, *class); - assert_eq!(diagnostic.class.exit_code(), class.exit_code()); + assert_eq!(diagnostic.code, *code, "{name}"); + assert_eq!(diagnostic.class, *class, "{name}"); + assert_eq!(diagnostic.class.exit_code(), class.exit_code(), "{name}"); } } From f951bb5d73ec52dc474fa2b1baa50968f7092a5a Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:32:48 +0800 Subject: [PATCH 2/2] refactor: drop narrating comments from the session exit change Remove the explanatory headers and local comments that restate the class-to-exit mapping. Code and assertions are unchanged. --- crates/wright-cli/src/session.rs | 7 ------- crates/wright-cli/tests/session_exits.rs | 7 ------- crates/wright-driver/src/session/registry.rs | 3 --- crates/wright-driver/tests/check_failures.rs | 3 --- crates/wright-driver/tests/lookup_failures.rs | 3 --- 5 files changed, 23 deletions(-) diff --git a/crates/wright-cli/src/session.rs b/crates/wright-cli/src/session.rs index 475400ee..b6bb8c3b 100644 --- a/crates/wright-cli/src/session.rs +++ b/crates/wright-cli/src/session.rs @@ -1,16 +1,9 @@ -//! Session- and service-construction failures (#1028): the diagnostic's -//! declared exit class sets the process exit, so every command reports -//! the word and code the diagnostic carries instead of a fixed usage -//! exit. - use std::process::ExitCode; use wright_driver::Diagnostic; use crate::present::{self, Presentation}; -/// Report a `CompilerSession::new`/`ToolService::new` construction -/// diagnostic and return the exit code its declared class carries. pub(crate) fn refuse(pres: Presentation, name: &str, diagnostic: &Diagnostic) -> ExitCode { let code = diagnostic.class.exit_code(); present::terminal::report_error( diff --git a/crates/wright-cli/tests/session_exits.rs b/crates/wright-cli/tests/session_exits.rs index 1f6e9e92..f8fdd7a2 100644 --- a/crates/wright-cli/tests/session_exits.rs +++ b/crates/wright-cli/tests/session_exits.rs @@ -1,10 +1,3 @@ -//! Session-construction exits follow the diagnostic's declared class -//! (#1028): `lint-rule-error` splits by failure kind — a missing or -//! unreadable `--rule-file` path is a usage error (2), while a file -//! that loads but is invalid content is a source error (1). Each of -//! the three session-construction surfaces must exit by the class the -//! diagnostic carries, not a fixed wrapper exit. - use std::path::{Path, PathBuf}; use std::process::Command; diff --git a/crates/wright-driver/src/session/registry.rs b/crates/wright-driver/src/session/registry.rs index f19dc8f5..a8c58796 100644 --- a/crates/wright-driver/src/session/registry.rs +++ b/crates/wright-driver/src/session/registry.rs @@ -12,9 +12,6 @@ pub(super) fn compose(config: &SessionConfig) -> Result ExitClass::Usage, _ => ExitClass::SourceError, diff --git a/crates/wright-driver/tests/check_failures.rs b/crates/wright-driver/tests/check_failures.rs index ee3c9b5e..b1ff7643 100644 --- a/crates/wright-driver/tests/check_failures.rs +++ b/crates/wright-driver/tests/check_failures.rs @@ -468,9 +468,6 @@ fn check_surfaces_every_failure_kind_with_its_code_and_exit_class() { assert_eq!(diagnostic.class, ExitClass::Usage); assert_eq!(diagnostic.class.exit_code(), ExitClass::Usage.exit_code()); - // `lint-rule-error` splits by failure kind (#1028): a missing or - // unreadable `--rule-file` path is a usage error, while a file that - // loads but is invalid content is a source error. { let dir = temp_dir("check-rule-file"); let Err(diagnostic) = CompilerSession::new(SessionConfig { diff --git a/crates/wright-driver/tests/lookup_failures.rs b/crates/wright-driver/tests/lookup_failures.rs index c208d7a1..d5e37922 100644 --- a/crates/wright-driver/tests/lookup_failures.rs +++ b/crates/wright-driver/tests/lookup_failures.rs @@ -383,9 +383,6 @@ fn lookup_surfaces_every_failure_kind_with_its_code_and_exit_class() { // The session machinery's own failure kinds at the service // boundary: construction diagnostics keep their own code and class, // and `session-config-changed` refuses `ToolService::new` itself. - // `lint-rule-error` splits by failure kind (#1028): a missing or - // unreadable `--rule-file` path is `Usage`, while a file that loads - // but is invalid content is `SourceError`. let session_failures: &[(&str, &str, ExitClass)] = &[ ("invalid-selection", "invalid-selection", ExitClass::Usage), (