From a1a5cab8bf02db70d434e148459aea66f4619727 Mon Sep 17 00:00:00 2001 From: Wug Date: Tue, 29 Sep 2026 22:36:10 +0800 Subject: [PATCH] fix: keep installer internals out of human errors Signed-off-by: Wug --- rust/cli.rs | 59 +++- rust/computer.rs | 2 +- rust/diagnostic.rs | 90 ++++++ rust/lib.rs | 1 + rust/main.rs | 4 +- rust/operation.rs | 350 +++++++++++++++++----- rust/report.rs | 98 ++++++ rust/request.rs | 71 ++++- rust/supervisor.rs | 17 +- rust/world.rs | 37 ++- verification/test_native.py | 174 ++++++++++- verification/test_no_installer_wording.py | 29 ++ 12 files changed, 823 insertions(+), 109 deletions(-) create mode 100644 rust/diagnostic.rs diff --git a/rust/cli.rs b/rust/cli.rs index f10edfa..33c52cf 100644 --- a/rust/cli.rs +++ b/rust/cli.rs @@ -1,8 +1,9 @@ use crate::{ Error, Result, computer, config::Config, - host, operation, + diagnostic, host, operation, presence::Interaction, + report::FailureCode, request::{Reply, Request}, source, supervisor, version, }; @@ -13,6 +14,14 @@ use tokio::{ time::timeout, }; +pub fn failure_line() -> &'static str { + "The installation could not finish. Run the same install command again." +} + +pub fn preserve_top_level_failure(error: &Error) { + diagnostic::preserve_top_level_failure(error); +} + const HELP: &str = "Raft Computer installation commands install|upgrade [--version V | --channel main|alpha|NAME] [--yes] [--allow-downgrade] [--json] @@ -115,10 +124,12 @@ async fn worker(cfg: &Config) -> Result { request.validate()?; let reply = match operation::execute(cfg, &request).await { Ok(reply) => reply, - Err(Error::Locked(_)) => Reply::plain( + Err(Error::Locked(_)) => Reply::failure( &request.id, 2, "Another installation is running on this machine.", + FailureCode::OperationBusy, + "Another installation is running on this machine.", ), Err(error) => { // Inspect persistent state instead of reporting a write/read failure @@ -132,14 +143,19 @@ async fn worker(cfg: &Config) -> Result { operation: k_carrier::state::Operation { outcome: None, .. } } ); - eprintln!("Installation: {error}"); - Reply::plain( + let diagnostic = diagnostic::safe_error(&error); + Reply::failure( &request.id, if unresolved { 3 } else { 1 }, - String::from( - "The installation could not finish. Run the same install command again to check and continue.", - ), + failure_line(), + if unresolved { + FailureCode::RecoveryUnresolved + } else { + FailureCode::InstallFailed + }, + "The installation could not finish.", ) + .with_diagnostic(diagnostic) } }; // execute() has released the operation gate. Cleanup reacquires it and @@ -198,9 +214,19 @@ pub async fn run() -> Result { // OS parent identity limits the exemption to this invocation. args.request.waiting_caller = Some(parent); } else if !remote { - return Err(invalid( - "Computer caller has no waiting declaration; run the install command directly", - )); + let reply = Reply::failure( + &args.request.id, + 1, + "This command is started by Raft Computer, not run by hand. To install or upgrade, use the install command from the setup page.", + FailureCode::CallerNotComputer, + "This command is started by Raft Computer, not run by hand.", + ); + if args.json { + println!("{}", serde_json::to_string(&reply)?); + } else { + eprintln!("{}", reply.line); + } + return Ok(reply.exit_code); } } } @@ -212,3 +238,16 @@ pub async fn run() -> Result { } Ok(reply.exit_code) } + +#[cfg(test)] +mod failure_line_tests { + use super::*; + + #[test] + fn failure_line_has_one_runnable_next_step() { + assert_eq!( + failure_line(), + "The installation could not finish. Run the same install command again." + ); + } +} diff --git a/rust/computer.rs b/rust/computer.rs index 9d92469..22c43eb 100644 --- a/rust/computer.rs +++ b/rust/computer.rs @@ -101,7 +101,7 @@ fn redact_opaque_word(word: &str) -> String { } } -fn diagnostic_stderr(bytes: &[u8]) -> (String, bool) { +pub(crate) fn diagnostic_stderr(bytes: &[u8]) -> (String, bool) { let original_truncated = bytes.len() > DIAGNOSTIC_STDERR_LIMIT; let mut redacted = Vec::new(); let mut redact_next_nonempty_line = false; diff --git a/rust/diagnostic.rs b/rust/diagnostic.rs new file mode 100644 index 0000000..2a1c1ce --- /dev/null +++ b/rust/diagnostic.rs @@ -0,0 +1,90 @@ +//! Private, bounded diagnostics for failures that have no durable operation +//! receipt. Human output must never depend on or render these records. +use crate::{Error, Result, computer, config::Config}; +use k_carrier::storage::{ensure_dir, now_ms, sync_dir, write_json}; +use serde::{Deserialize, Serialize}; +use std::fs; + +const CLI_DIAGNOSTIC_LIMIT: usize = 16; + +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct CliDiagnostic { + format_version: u32, + kind: String, + diagnostic: String, + truncated: bool, + recorded_at_ms: u64, +} + +pub fn safe_error(error: &Error) -> String { + computer::diagnostic_stderr(error.to_string().as_bytes()).0 +} + +/// Preserve a last-resort launcher failure when configuration is usable. A +/// configuration error may make the state root unknowable; that case remains +/// fail-closed instead of guessing another user's or profile's directory. +pub fn preserve_top_level_failure(error: &Error) { + let Ok(cfg) = Config::load() else { + return; + }; + let _ = write_cli_diagnostic(&cfg, error); +} + +fn write_cli_diagnostic(cfg: &Config, error: &Error) -> Result<()> { + let directory = cfg.installer_dir.join("diagnostics"); + ensure_dir(&directory)?; + let raw = error.to_string(); + let (diagnostic, truncated) = computer::diagnostic_stderr(raw.as_bytes()); + let name = format!("cli-{}.json", uuid::Uuid::new_v4()); + write_json( + &directory.join(&name), + &CliDiagnostic { + format_version: 1, + kind: "top-level-error".into(), + diagnostic, + truncated, + recorded_at_ms: now_ms(), + }, + )?; + prune_cli_diagnostics(&directory) +} + +fn prune_cli_diagnostics(directory: &std::path::Path) -> Result<()> { + let mut diagnostics = Vec::new(); + for entry in fs::read_dir(directory)? { + let entry = entry?; + let name = entry.file_name(); + let Some(name) = name.to_str() else { + continue; + }; + let Some(id) = name + .strip_prefix("cli-") + .and_then(|name| name.strip_suffix(".json")) + else { + continue; + }; + if !entry.file_type()?.is_file() || uuid::Uuid::parse_str(id).is_err() { + continue; + } + let Ok(bytes) = fs::read(entry.path()) else { + continue; + }; + if bytes.len() > 65536 { + continue; + } + let Ok(diagnostic) = serde_json::from_slice::(&bytes) else { + continue; + }; + diagnostics.push((diagnostic.recorded_at_ms, entry.path())); + } + diagnostics.sort_by_key(|(recorded_at_ms, _)| *recorded_at_ms); + let excess = diagnostics.len().saturating_sub(CLI_DIAGNOSTIC_LIMIT); + for (_, path) in diagnostics.into_iter().take(excess) { + fs::remove_file(path)?; + } + if excess > 0 { + sync_dir(directory)?; + } + Ok(()) +} diff --git a/rust/lib.rs b/rust/lib.rs index 28f2e13..127e702 100644 --- a/rust/lib.rs +++ b/rust/lib.rs @@ -5,6 +5,7 @@ pub mod cleanup; pub mod cli; pub mod computer; pub mod config; +mod diagnostic; pub mod host; pub mod operation; pub mod presence; diff --git a/rust/main.rs b/rust/main.rs index 624a938..ab719e0 100644 --- a/rust/main.rs +++ b/rust/main.rs @@ -3,8 +3,8 @@ async fn main() { let code = match raft_computer_installer::cli::run().await { Ok(code) => code, Err(error) => { - eprintln!("Installation: {error}"); - println!("The installation could not finish."); + raft_computer_installer::cli::preserve_top_level_failure(&error); + eprintln!("{}", raft_computer_installer::cli::failure_line()); 1 } }; diff --git a/rust/operation.rs b/rust/operation.rs index c8a4e80..d40805d 100644 --- a/rust/operation.rs +++ b/rust/operation.rs @@ -6,7 +6,7 @@ use crate::{ config::Config, host, presence::{Interaction, Presence}, - report::{self, Outcome, Receipt}, + report::{self, FailureCode, Outcome, Receipt}, request::{Reply, Request}, runner, shell_path, source::{FrozenSource, Manifest, Source}, @@ -246,6 +246,8 @@ fn receipt( approved_by: Some(request.approved_by.clone()), outcome, exit_code: outcome.exit_code(), + reason: None, + code: None, line, next_step: None, finished_at_ms: 0, @@ -257,6 +259,7 @@ fn finish( cfg: &Config, plan: &mut Plan, outcome: Outcome, + failure: Option<(FailureCode, String)>, line: impl Into, ) -> Result { // Installation and account/workspace setup are separate product actions. @@ -264,7 +267,7 @@ fn finish( // never carry a product-provided login/setup hint into a current receipt. plan.next_step = None; let mut line = line.into(); - if outcome.exit_code() == 0 || outcome == Outcome::RolledBack { + if outcome.exit_code() == 0 { if plan.detail.get("readback").map(String::as_str) == Some("service") { line.push_str(" It is running."); } @@ -280,6 +283,10 @@ fn finish( outcome, line, ); + if let Some((code, reason)) = failure { + result.code = Some(code); + result.reason = Some(reason); + } result.detail = plan.detail.clone(); result.preserve_unresolved(plan.inherited_unresolved); let result = result.finish(cfg)?; @@ -333,23 +340,60 @@ fn transition(cfg: &Config, plan: &mut Plan, phase: Phase) -> Result<()> { save(cfg, plan) } -/// The recovery hint carries the cause and tells the user to repeat the -/// command they already have. The native installer is an internal detail: -/// its path never appears in user-facing text, and the entry scripts resume -/// an unresolved operation themselves before this line is ever shown. -fn recovery_hint_line(error: &Error) -> String { - format!( - "Installation could not finish ({error}). Run the same install command again to continue where it left off." - ) +/// The receipt's reason and the human line share product-language state; raw +/// causes stay in detail.diagnostic and never become display copy. +fn recovery_hint_copy(previous_version: Option<&str>, running: Option) -> (String, String) { + match (previous_version, running) { + (Some(previous), Some(true)) => { + let reason = format!("Raft Computer {previous} is running."); + ( + format!("{reason} You can try the upgrade again later."), + reason, + ) + } + (Some(_), Some(false)) => ( + "Raft Computer is not running. Run `raft-computer start`.".into(), + "Raft Computer is not running.".into(), + ), + _ => ( + "Raft Computer's current status couldn't be confirmed. Run the same command again to continue." + .into(), + "Raft Computer's current status couldn't be confirmed.".into(), + ), + } } -fn rollback_line(target: &str, from_version: &str, reason: Option<&str>) -> String { - match reason { - Some(reason) => { - format!("{target} failed its checks ({reason}); {from_version} was restored.") - } - None => format!("{target} failed its checks; {from_version} was restored."), +fn rollback_copy( + target: &str, + from_version: &str, + was_running: Option, + running: bool, +) -> (String, String) { + if running { + let reason = format!( + "Raft Computer {target} didn't start correctly, so {from_version} is still running." + ); + return ( + format!( + "{reason} Nothing else was changed. Run the same install command again to retry." + ), + reason, + ); + } + if was_running == Some(false) { + return ( + format!( + "Raft Computer {target} didn't start correctly, so {from_version} is still installed. It was not running before the upgrade and is still not running." + ), + format!( + "Raft Computer {target} didn't start correctly, so {from_version} is still installed and not running." + ), + ); } + let reason = format!( + "Raft Computer {target} didn't start correctly, so {from_version} is still installed but not running." + ); + (format!("{reason} Run `raft-computer start`."), reason) } async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result { @@ -359,6 +403,10 @@ async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result cfg, plan, Outcome::Failed, + Some(( + FailureCode::InterruptedBeforeStart, + "Installation was interrupted before the upgrade started.".into(), + )), "Installation was interrupted before the upgrade started. Run the same install command again.", ); } @@ -404,6 +452,11 @@ async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result cfg, plan, Outcome::Failed, + Some(( + FailureCode::InterruptedBeforeStart, + "Installation was interrupted before the upgrade transaction started." + .into(), + )), "Installation was interrupted before the upgrade transaction started. Run the same install command again.", ); } @@ -423,13 +476,17 @@ async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result cfg, plan, Outcome::Held, + Some(( + FailureCode::OperationBusy, + "Another upgrade is running on this machine.".into(), + )), "Another upgrade is running on this machine.", ); } if let Some(error) = &response.error { // Preserve the initiating failure across recovery's later outcome. plan.detail - .entry("upgradeError".into()) + .entry("diagnostic".into()) .or_insert_with(|| error.clone()); save(cfg, plan)?; } @@ -454,7 +511,7 @@ async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result k_carrier::state::Outcome::Failed => Outcome::Failed, }; if let Some(reason) = operation.reason { - plan.detail.insert("reason".into(), reason); + plan.detail.insert("diagnostic".into(), reason); } if outcome == Outcome::RolledBack && let Ok(Some(reference)) = host::latest_start_failure_diagnostic(cfg, &plan.request.id) @@ -464,7 +521,11 @@ async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result plan.detail .insert("startFailureDiagnostic".into(), reference); } + let mut running = None; + let mut was_running = None; if let Ok(state) = host::read(cfg) { + running = Some(state.running); + was_running = Some(!state.initial_processes.is_empty()); if outcome == Outcome::UpToDate && state.running && host::live_evidence(cfg).await?.version != plan.manifest.version @@ -499,30 +560,50 @@ async fn upgrade(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result } } let target = &plan.manifest.version; - let line = match outcome { + let (line, failure) = match outcome { Outcome::Promoted if plan.detail.get("readback").map(String::as_str) == Some("candidate") => { - format!( - "Upgraded {} to {target}. The service remains stopped. Run raft-computer start to start it.", - operation.from_version + ( + format!( + "Upgraded {} to {target}. The service remains stopped. Run raft-computer start to start it.", + operation.from_version + ), + None, ) } - Outcome::Promoted => format!("Upgraded {} to {target}.", operation.from_version), - Outcome::RolledBack => rollback_line( - target, - &operation.from_version, - plan.detail.get("reason").map(String::as_str), + Outcome::Promoted => ( + format!("Upgraded {} to {target}.", operation.from_version), + None, ), - Outcome::UpToDate => format!("{target} is already installed."), - Outcome::Held => { - "Upgrade was not allowed. Check the selected version and --allow-downgrade.".into() + Outcome::RolledBack => { + let (line, reason) = rollback_copy( + target, + &operation.from_version, + was_running, + running == Some(true), + ); + (line, Some((FailureCode::CandidateStartFailed, reason))) } - _ => format!( - "Could not upgrade to {target}. Run the same install command again to check and continue." + Outcome::UpToDate => (format!("{target} is already installed."), None), + Outcome::Held => ( + "Upgrade was not allowed. Check the selected version and --allow-downgrade.".into(), + Some(( + FailureCode::InstallationHeld, + "The requested upgrade was not allowed.".into(), + )), + ), + _ => ( + format!( + "Could not upgrade to {target}. Run the same install command again to check and continue." + ), + Some(( + FailureCode::InstallFailed, + format!("Raft Computer could not be upgraded to {target}."), + )), ), }; - finish(cfg, plan, outcome, line) + finish(cfg, plan, outcome, failure, line) } async fn install(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result { @@ -532,6 +613,10 @@ async fn install(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result cfg, plan, Outcome::Failed, + Some(( + FailureCode::InterruptedBeforeStart, + "Installation was interrupted before changing installed files.".into(), + )), "Installation was interrupted before changing installed files. Run the same install command again.", ); } @@ -705,7 +790,7 @@ async fn install(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result }, plan.manifest.version ); - return finish(cfg, plan, outcome, line); + return finish(cfg, plan, outcome, None, line); } Err(invalid("unexpected operation phase")) } @@ -734,8 +819,22 @@ async fn resume(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result match result { Ok(reply) => Ok(reply), Err(error) => { - plan.detail.insert("error".into(), error.to_string()); + plan.detail.insert("diagnostic".into(), error.to_string()); let untouched = plan.phase == Phase::Preparing && !error.is_uncertain(); + let (line, reason) = if untouched { + ( + "The installation could not finish. Nothing was changed. Run the same install command again." + .into(), + "The installation could not finish.".into(), + ) + } else { + recovery_hint_copy( + plan.from_version.as_deref(), + plan.from_version + .as_ref() + .and_then(|_| host::read(cfg).ok().map(|state| state.running)), + ) + }; finish( cfg, plan, @@ -744,11 +843,15 @@ async fn resume(cfg: &Config, plan: &mut Plan, recovery: bool) -> Result } else { Outcome::Unresolved }, - if untouched { - "Could not prepare the installation. Installed files were not changed.".into() - } else { - recovery_hint_line(&error) - }, + Some(( + if untouched { + FailureCode::InstallFailed + } else { + FailureCode::RecoveryUnresolved + }, + reason, + )), + line, ) } } @@ -760,9 +863,12 @@ fn reject( target: Option, from: Option, unresolved: bool, + failure: (FailureCode, String), line: &str, ) -> Result { let mut result = receipt(request, target, from, Outcome::Held, line.into()); + result.code = Some(failure.0); + result.reason = Some(failure.1); result.preserve_unresolved(unresolved); Ok(Reply::receipt(result.finish(cfg)?)) } @@ -775,10 +881,12 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { let _gate = match UpgradeLock::acquire(&cfg.installer_dir.join("gate")) { Ok(gate) => gate, Err(Error::Locked(_)) => { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 2, "Another installation is running on this machine.", + FailureCode::OperationBusy, + "Another installation is running on this machine.", )); } Err(error) => return Err(error), @@ -796,10 +904,12 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { // Current recovery inputs are checked below independently; an old // unreadable result alone does not make today's installation broken. if request.command != "status" { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 3, "This request's result is unreadable. Use status to inspect the current installation.", + FailureCode::ResultUnreadable, + "This request's result is unreadable.", )); } None @@ -813,10 +923,12 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { .is_some_and(|v| Some(v) != old.target_version.as_ref()) || (request.command != "recover" && request.command != old.operation)) { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 2, "This request ID already belongs to a different operation.", + FailureCode::RequestConflict, + "This request ID already belongs to a different operation.", )); } if request.command != "status" && old.outcome != Outcome::Unresolved { @@ -848,10 +960,12 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { .as_ref() .is_some_and(|v| v != &previous.manifest.version)) { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 2, "This request ID already belongs to a different operation.", + FailureCode::RequestConflict, + "This request ID already belongs to a different operation.", )); } let resumed = resume(cfg, &mut previous, true).await; @@ -871,10 +985,12 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { // must enter the repair decision, not repeat the same failed // recovery and then compete with its own retained claim again. write_json(&settlement, request)?; - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 3, "An earlier installation could not be recovered yet. Run the same install command again.", + FailureCode::RecoveryUnresolved, + "An earlier installation could not be recovered yet.", )); } } @@ -890,29 +1006,35 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { && saved.channel == request.channel => {} _ => { mark_damage(cfg)?; - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 3, "Recovery records are unreadable. Run the same install command again.", + FailureCode::ResultUnreadable, + "Recovery records are unreadable.", )); } } unresolved = true; } else if let Err(error) = settle_k(cfg).await { if let Error::Locked(_) = error { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 2, "Another upgrade is running on this machine.", + FailureCode::OperationBusy, + "Another upgrade is running on this machine.", )); } unresolved = true; write_json(&settlement, request)?; if error.is_uncertain() { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 3, "An earlier upgrade could not be recovered yet. Run the same install command again.", + FailureCode::RecoveryUnresolved, + "An earlier upgrade could not be recovered yet.", )); } } @@ -929,24 +1051,52 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { if answer.running { "running" } else { "stopped" } ), ), - World::Held { reason } => (2, format!("Not done: {reason}.")), + World::Held { reason, next_step } => ( + 2, + format!( + "Not done: {reason}.{}", + next_step + .as_ref() + .map(|step| format!(" {step}")) + .unwrap_or_default() + ), + ), _ => ( 3, "Installation requires repair. Run the same install command again.".into(), ), }; - let mut reply = Reply::plain(&request.id, code, line); + let mut reply = if code == 0 { + Reply::plain(&request.id, code, line) + } else { + let (failure_code, reason) = match &observed { + World::Held { reason, .. } => (FailureCode::InstallationHeld, reason.clone()), + _ => ( + FailureCode::RecoveryUnresolved, + "The installation requires repair.".into(), + ), + }; + Reply::failure(&request.id, code, line, failure_code, reason) + }; reply.world = Some(observed); return Ok(reply); } - if let World::Held { reason } = &observed { + if let World::Held { reason, next_step } = &observed { + let line = format!( + "Not done: {reason}.{}", + next_step + .as_ref() + .map(|step| format!(" {step}")) + .unwrap_or_default() + ); return reject( cfg, request, request.version.clone(), None, unresolved, - &format!("Not done: {reason}."), + (FailureCode::InstallationHeld, reason.clone()), + &line, ); } let broken = unresolved || matches!(observed, World::Broken { .. } | World::Upgrading { .. }); @@ -957,14 +1107,24 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { request.version.clone(), observed.version().map(str::to_owned), false, + ( + FailureCode::RepairNotNeeded, + "This installation does not need repair.".into(), + ), "This installation does not need repair. Use install or upgrade.", ); } if request.recovery_only && !exists(&settlement)? { - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, if unresolved { 3 } else { 1 }, - "The request was interrupted before a recoverable installation started. Run the same install command again.", + "The installation was interrupted before it started. Run the same install command again.", + if unresolved { + FailureCode::RecoveryUnresolved + } else { + FailureCode::InterruptedBeforeStart + }, + "The installation was interrupted before it started.", )); } let source = Source::new(cfg)?; @@ -991,7 +1151,10 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { "Could not resolve the requested release. Run the same install command again." .into(), ); - result.detail.insert("error".into(), error.to_string()); + result.code = Some(FailureCode::ReleaseResolutionFailed); + result.reason = + Some("The requested Raft Computer release could not be resolved.".into()); + result.detail.insert("diagnostic".into(), error.to_string()); result.preserve_unresolved(unresolved); return Ok(Reply::receipt(result.finish(cfg)?)); } @@ -1008,6 +1171,10 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { Some(manifest.version), from, unresolved, + ( + FailureCode::DowngradeNotAllowed, + "The selected version is older than the installed version.".into(), + ), "The selected version is older. Add --allow-downgrade to install it.", ); } @@ -1030,6 +1197,10 @@ pub async fn execute(cfg: &Config, request: &Request) -> Result { Some(manifest.version), from, unresolved, + ( + FailureCode::InstallationDeclined, + "The installation was declined.".into(), + ), "Installation declined.", ); } @@ -1082,34 +1253,79 @@ mod recovery_hint_tests { use super::*; #[test] - fn recovery_hint_names_the_cause_and_never_the_installer() { - let line = recovery_hint_line(&Error::Uncertain("digest mismatch".into())); + fn recovery_hint_reports_a_running_previous_version() { + let (line, reason) = recovery_hint_copy(Some("1.0.31"), Some(true)); + assert_eq!( + line, + "Raft Computer 1.0.31 is running. You can try the upgrade again later." + ); + assert_eq!(reason, "Raft Computer 1.0.31 is running."); + } + + #[test] + fn recovery_hint_reports_a_stopped_previous_version() { + let (line, reason) = recovery_hint_copy(Some("1.0.31"), Some(false)); assert_eq!( line, - "Installation could not finish (digest mismatch). Run the same install command again to continue where it left off." + "Raft Computer is not running. Run `raft-computer start`." + ); + assert_eq!(reason, "Raft Computer is not running."); + } + + #[test] + fn recovery_hint_falls_back_without_a_readable_previous_state() { + let (line, reason) = recovery_hint_copy(None, None); + assert_eq!( + line, + "Raft Computer's current status couldn't be confirmed. Run the same command again to continue." + ); + assert_eq!( + reason, + "Raft Computer's current status couldn't be confirmed." ); - assert!(!line.contains("installer"), "{line}"); - assert!(!line.contains('/'), "{line}"); } } #[cfg(test)] -mod rollback_line_tests { +mod rollback_copy_tests { use super::*; #[test] - fn rollback_line_with_reason_carries_it_inline() { + fn rollback_copy_reports_a_restored_running_service() { + let (line, reason) = rollback_copy("1.0.32", "1.0.31", Some(true), true); assert_eq!( - rollback_line("1.0.32", "1.0.31", Some("experiment probe failed: boom")), - "1.0.32 failed its checks (experiment probe failed: boom); 1.0.31 was restored.", + line, + "Raft Computer 1.0.32 didn't start correctly, so 1.0.31 is still running. Nothing else was changed. Run the same install command again to retry.", + ); + assert_eq!( + reason, + "Raft Computer 1.0.32 didn't start correctly, so 1.0.31 is still running." ); } #[test] - fn rollback_line_without_reason_is_the_legacy_verbatim_line() { + fn rollback_copy_reports_a_service_that_failed_to_return() { + let (line, reason) = rollback_copy("1.0.32", "1.0.31", Some(true), false); + assert_eq!( + line, + "Raft Computer 1.0.32 didn't start correctly, so 1.0.31 is still installed but not running. Run `raft-computer start`.", + ); + assert_eq!( + reason, + "Raft Computer 1.0.32 didn't start correctly, so 1.0.31 is still installed but not running." + ); + } + + #[test] + fn rollback_copy_preserves_an_already_stopped_service_without_a_next_step() { + let (line, reason) = rollback_copy("1.0.32", "1.0.31", Some(false), false); + assert_eq!( + line, + "Raft Computer 1.0.32 didn't start correctly, so 1.0.31 is still installed. It was not running before the upgrade and is still not running." + ); assert_eq!( - rollback_line("1.0.32", "1.0.31", None), - "1.0.32 failed its checks; 1.0.31 was restored.", + reason, + "Raft Computer 1.0.32 didn't start correctly, so 1.0.31 is still installed and not running." ); } } diff --git a/rust/report.rs b/rust/report.rs index 2cfb7df..c93619b 100644 --- a/rust/report.rs +++ b/rust/report.rs @@ -21,6 +21,63 @@ pub enum Outcome { Unresolved, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FailureCode { + CallerNotComputer, + CandidateStartFailed, + DowngradeNotAllowed, + InstallFailed, + InstallationDeclined, + InstallationHeld, + InterruptedBeforeStart, + LegacyFailure, + OperationBusy, + RecoveryUnresolved, + ReleaseResolutionFailed, + RepairNotNeeded, + RequestConflict, + ResultUnreadable, +} + +impl FailureCode { + pub const ALL: [Self; 14] = [ + Self::CallerNotComputer, + Self::CandidateStartFailed, + Self::DowngradeNotAllowed, + Self::InstallFailed, + Self::InstallationDeclined, + Self::InstallationHeld, + Self::InterruptedBeforeStart, + Self::LegacyFailure, + Self::OperationBusy, + Self::RecoveryUnresolved, + Self::ReleaseResolutionFailed, + Self::RepairNotNeeded, + Self::RequestConflict, + Self::ResultUnreadable, + ]; + + pub fn as_str(self) -> &'static str { + match self { + Self::CallerNotComputer => "caller_not_computer", + Self::CandidateStartFailed => "candidate_start_failed", + Self::DowngradeNotAllowed => "downgrade_not_allowed", + Self::InstallFailed => "install_failed", + Self::InstallationDeclined => "installation_declined", + Self::InstallationHeld => "installation_held", + Self::InterruptedBeforeStart => "interrupted_before_start", + Self::LegacyFailure => "legacy_failure", + Self::OperationBusy => "operation_busy", + Self::RecoveryUnresolved => "recovery_unresolved", + Self::ReleaseResolutionFailed => "release_resolution_failed", + Self::RepairNotNeeded => "repair_not_needed", + Self::RequestConflict => "request_conflict", + Self::ResultUnreadable => "result_unreadable", + } + } +} + impl Outcome { pub fn exit_code(self) -> u8 { match self { @@ -45,6 +102,10 @@ pub struct Receipt { pub approved_by: Option, pub outcome: Outcome, pub exit_code: u8, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reason: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub code: Option, pub line: String, pub next_step: Option, pub finished_at_ms: u64, @@ -63,6 +124,19 @@ impl Receipt { || self.line.trim().is_empty() || self.line.len() > 2048 || self.line.chars().any(char::is_control) + || self.reason.as_ref().is_some_and(|reason| { + reason.trim().is_empty() + || reason.len() > 2048 + || reason.chars().any(char::is_control) + }) + || self + .code + .is_some_and(|code| self.line.contains(code.as_str())) + || matches!( + (&self.reason, &self.code), + (Some(_), None) | (None, Some(_)) + ) + || (self.outcome.exit_code() == 0 && (self.reason.is_some() || self.code.is_some())) || self .next_step .as_ref() @@ -84,6 +158,8 @@ impl Receipt { if unresolved && self.outcome != Outcome::Repaired { self.outcome = Outcome::Unresolved; self.exit_code = 3; + self.reason = Some("An earlier installation could not be recovered.".into()); + self.code = Some(FailureCode::RecoveryUnresolved); } } @@ -126,3 +202,25 @@ pub fn read(cfg: &Config, id: &str) -> Result> { } Ok(Some(receipt)) } + +#[cfg(test)] +mod failure_code_tests { + use super::*; + + #[test] + fn fixed_failure_codes_round_trip_to_the_public_strings() { + let values: Vec = FailureCode::ALL + .into_iter() + .map(|code| serde_json::to_string(&code).unwrap()) + .collect(); + assert_eq!(values.len(), 14); + assert_eq!(values[0], "\"caller_not_computer\""); + assert_eq!(values[13], "\"result_unreadable\""); + for code in FailureCode::ALL { + assert_eq!( + serde_json::to_string(&code).unwrap(), + format!("\"{}\"", code.as_str()) + ); + } + } +} diff --git a/rust/request.rs b/rust/request.rs index 238ae16..d890d20 100644 --- a/rust/request.rs +++ b/rust/request.rs @@ -1,7 +1,7 @@ use crate::{ Result, presence::Presence, - report::{Outcome, Receipt}, + report::{FailureCode, Outcome, Receipt}, source, version, world::World, }; @@ -71,31 +71,78 @@ pub struct Reply { pub id: String, pub exit_code: u8, pub line: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reason: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub code: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diagnostic: Option, pub receipt: Option, pub world: Option, } impl Reply { pub fn plain(id: &str, exit_code: u8, line: impl Into) -> Self { + debug_assert_eq!(exit_code, 0, "non-success replies require reason and code"); + Self { + protocol_version: 1, + id: id.into(), + exit_code, + line: line.into(), + reason: None, + code: None, + diagnostic: None, + receipt: None, + world: None, + } + } + pub fn failure( + id: &str, + exit_code: u8, + line: impl Into, + code: FailureCode, + reason: impl Into, + ) -> Self { + debug_assert_ne!(exit_code, 0, "success replies cannot carry failure fields"); Self { protocol_version: 1, id: id.into(), exit_code, line: line.into(), + reason: Some(reason.into()), + code: Some(code), + diagnostic: None, receipt: None, world: None, } } pub fn receipt(receipt: Receipt) -> Self { + let (reason, code) = if receipt.exit_code == 0 { + (None, None) + } else { + ( + Some(receipt.reason.clone().unwrap_or_else(|| { + "A previous installation did not complete successfully.".into() + })), + Some(receipt.code.unwrap_or(FailureCode::LegacyFailure)), + ) + }; Self { protocol_version: 1, id: receipt.id.clone(), exit_code: receipt.exit_code, line: receipt.line.clone(), + reason, + code, + diagnostic: None, receipt: Some(receipt), world: None, } } + pub fn with_diagnostic(mut self, diagnostic: impl Into) -> Self { + self.diagnostic = Some(diagnostic.into()); + self + } fn replay_line(receipt: &Receipt) -> String { let result = match receipt.outcome { Outcome::Installed => "installed", @@ -129,6 +176,26 @@ impl Reply { || self.line.trim().is_empty() || self.line.len() > 2048 || self.line.chars().any(char::is_control) + || self.reason.as_ref().is_some_and(|reason| { + reason.trim().is_empty() + || reason.len() > 2048 + || reason.chars().any(char::is_control) + }) + || self.diagnostic.as_ref().is_some_and(|diagnostic| { + diagnostic.trim().is_empty() + || diagnostic.len() > 2048 + || diagnostic.chars().any(char::is_control) + }) + || self + .code + .is_some_and(|code| self.line.contains(code.as_str())) + || matches!( + (&self.reason, &self.code), + (Some(_), None) | (None, Some(_)) + ) + || (self.exit_code == 0 && (self.reason.is_some() || self.code.is_some())) + || (self.exit_code == 0 && self.diagnostic.is_some()) + || (self.exit_code != 0 && (self.reason.is_none() || self.code.is_none())) { return Err(invalid("invalid response")); } @@ -136,6 +203,8 @@ impl Reply { receipt.validate()?; if receipt.id != self.id || receipt.exit_code != self.exit_code + || (receipt.reason.is_some() && receipt.reason != self.reason) + || (receipt.code.is_some() && receipt.code != self.code) || (receipt.line != self.line && Self::replay_line(receipt) != self.line) { return Err(invalid("response receipt mismatch")); diff --git a/rust/supervisor.rs b/rust/supervisor.rs index 43b0b78..db2b7c0 100644 --- a/rust/supervisor.rs +++ b/rust/supervisor.rs @@ -3,6 +3,7 @@ use crate::{ Error, Result, config::Config, + report::FailureCode, request::{Reply, Request}, }; use k_carrier::{ @@ -41,10 +42,14 @@ pub async fn run(cfg: &Config, request: &Request) -> Result { "formatVersion":1,"sha256":digest,"size":bytes.len(),"request":request,"owner":owner }), )?; - let mut last = Reply::plain( + let mut last = Reply::failure( &request.id, 3, - String::from("Installation could not be settled. Run the same install command again."), + String::from( + "The installation could not be completed or recovered. Run the same install command again.", + ), + FailureCode::RecoveryUnresolved, + "The installation could not be completed or recovered.", ); for attempt in 0..3 { // Verify every execution, including retries of the retained copy. @@ -94,12 +99,14 @@ pub async fn run(cfg: &Config, request: &Request) -> Result { if reply.exit_code == 2 && next.recovery_only { // A busy recovery has not settled the original operation. // Preserve its executable/request and its unresolved status. - last = Reply::plain( + last = Reply::failure( &request.id, 3, String::from( "Recovery is blocked by another installation still running. Run the same install command again after it finishes.", ), + FailureCode::RecoveryUnresolved, + "Recovery is blocked by another installation still running.", ); } else if reply.exit_code != 3 { if reply.exit_code <= 1 && reply.receipt.is_some() { @@ -119,10 +126,12 @@ pub async fn run(cfg: &Config, request: &Request) -> Result { Ok(Ok(_)) ) { let _retained = directory.keep(); - return Ok(Reply::plain( + return Ok(Reply::failure( &request.id, 3, "The previous installation step has not exited. Recovery must wait for it.", + FailureCode::RecoveryUnresolved, + "The previous installation step has not exited.", )); } } diff --git a/rust/world.rs b/rust/world.rs index de68701..7b56d59 100644 --- a/rust/world.rs +++ b/rust/world.rs @@ -13,11 +13,23 @@ use std::{fs, io::Read, path::Path}; #[serde(tag = "kind", rename_all = "lowercase")] pub enum World { Fresh, - Adopted { version: String }, - Managed { version: String }, - Upgrading { id: String }, - Broken { reason: String }, - Held { reason: String }, + Adopted { + version: String, + }, + Managed { + version: String, + }, + Upgrading { + id: String, + }, + Broken { + reason: String, + }, + Held { + reason: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + next_step: Option, + }, } impl World { @@ -88,25 +100,22 @@ pub async fn read(cfg: &Config) -> Result { if let Some(manager) = foreign_manager(&cfg.binary)? { return Ok(World::Held { reason: format!( - "{} belongs to {manager}; remove it with that manager before retrying", + "Raft Computer at {} is managed by {manager}", cfg.binary.display() ), + next_step: Some(format!("Update it with {manager}.")), }); } if fs::symlink_metadata(&cfg.binary).is_ok_and(|m| m.file_type().is_symlink()) { return Ok(World::Held { - reason: format!( - "{} is a link owned by another manager; remove it with that manager before retrying", - cfg.binary.display() - ), + reason: "Raft Computer's installation is managed by another tool".into(), + next_step: None, }); } if fs::symlink_metadata(&cfg.sidecar).is_ok_and(|m| m.file_type().is_symlink()) { return Ok(World::Held { - reason: format!( - "{} is a link owned by another manager; remove it with that manager before retrying", - cfg.sidecar.display() - ), + reason: "Raft Computer's installation is managed by another tool".into(), + next_step: None, }); } if exists(&cfg.installer_dir.join("metadata-damage.json"))? { diff --git a/verification/test_native.py b/verification/test_native.py index d59ee5b..a91a029 100644 --- a/verification/test_native.py +++ b/verification/test_native.py @@ -12,6 +12,15 @@ import urllib.parse from harness import DIST, SUFFIX, TARGET, WINDOWS, Machine, ReleaseServer, exchange, sha, wait_for +from test_no_installer_wording import runtime_offenders + +FAILURE_CODES = { + "caller_not_computer", "candidate_start_failed", "downgrade_not_allowed", + "install_failed", "installation_declined", "installation_held", + "interrupted_before_start", "legacy_failure", "operation_busy", + "recovery_unresolved", "release_resolution_failed", "repair_not_needed", + "request_conflict", "result_unreadable", +} class InstallerContract(unittest.TestCase): @@ -52,6 +61,25 @@ def machine(self): self.addCleanup(machine.close) return machine + def assert_failure_contract(self, reply): + self.assertNotEqual(reply["exitCode"], 0) + self.assertIn(reply["code"], FAILURE_CODES) + self.assertTrue(reply["reason"].strip()) + self.assertEqual(runtime_offenders(reply["reason"]), []) + self.assertNotIn(reply["code"], reply["line"]) + if reply.get("diagnostic") is not None: + self.assertTrue(reply["diagnostic"].strip()) + self.assertNotIn(reply["diagnostic"], reply["line"]) + self.assertNotIn(reply["diagnostic"], reply["reason"]) + if reply.get("receipt") is not None: + receipt = reply["receipt"] + self.assertEqual(receipt["code"], reply["code"]) + self.assertEqual(receipt["reason"], reply["reason"]) + self.assertEqual(runtime_offenders(receipt["reason"]), []) + self.assertNotIn("reason", receipt["detail"]) + self.assertNotIn("error", receipt["detail"]) + self.assertNotIn("upgradeError", receipt["detail"]) + def test_published_matrix_main_execution_uses_single_updates_resolution(self): from unittest.mock import patch from matrix import execute, ROOT @@ -138,11 +166,18 @@ def test_waiting_cli_preserves_real_service_on_upgrade_and_rollback(self): receipt = json.loads(stdout)["receipt"] self.assertEqual(receipt["outcome"], expected_outcome) if expected_outcome == "rolled-back": - reason = receipt["detail"]["reason"] + reply = json.loads(stdout) + self.assert_failure_contract(reply) self.assertEqual( receipt["line"], - f"{target} failed its checks ({reason}); {expected_version} was restored. It is running.", + f"Raft Computer {target} didn't start correctly, so {expected_version} is still running. " + "Nothing else was changed. Run the same install command again to retry.", + ) + self.assertEqual( + receipt["reason"], + f"Raft Computer {target} didn't start correctly, so {expected_version} is still running.", ) + self.assertEqual(receipt["code"], "candidate_start_failed") record = json.loads((machine.state / "product-state.json").read_text()) self.assertEqual([p["pid"] for p in record["initialProcesses"]], [before["pid"]]) self.assertNotIn(parent.pid, record["forcedStops"]) @@ -195,10 +230,11 @@ def test_failed_candidate_start_preserves_private_bounded_diagnostic(self): self.assertNotIn("multiline-short-value", result.stdout + result.stderr) receipt = json.loads(result.stdout)["receipt"] self.assertEqual(receipt["outcome"], "rolled-back") + self.assert_failure_contract(json.loads(result.stdout)) reference = receipt["detail"]["startFailureDiagnostic"] self.assertRegex(reference, r"^diagnostics/[0-9a-f-]+\.json$") self.assertEqual( - receipt["detail"]["reason"], + receipt["detail"]["diagnostic"], "experiment probe failed: HOST_COMMAND_FAILED: probe (exit 1): " f"candidate was not started successfully; private diagnostic {reference}", ) @@ -301,7 +337,8 @@ def test_old_noninteractive_cli_is_rejected_without_stopping_it(self): env=machine.env({"RCI_FIXTURE_INSTALLER": str(self.server.installer)}), text=True, capture_output=True, timeout=30) self.assertEqual(result.returncode, 1, result.stdout + result.stderr) - self.assertIn("has no waiting declaration", result.stderr) + self.assertIn("This command is started by Raft Computer", result.stderr) + self.assertEqual(runtime_offenders(result.stdout + result.stderr), []) self.assertEqual(machine.binary.read_bytes(), before) self.assertEqual(self.server.requests, []) self.assertFalse((machine.state / "active.json").exists()) @@ -311,7 +348,12 @@ def test_waiting_marker_from_non_product_parent_is_rejected(self): machine.json(["install", "--version", "1.0.0"]) result = machine.run(["upgrade"], extra={"RAFT_COMPUTER_INSTALLER_CALLER": "waiting-cli-v1"}) self.assertEqual(result.returncode, 1) - self.assertIn("not the installed immediate parent", result.stderr) + self.assertIn( + "The installation could not finish. Run the same install command again.", + result.stderr, + ) + self.assertNotIn("installed immediate parent", result.stderr) + self.assertEqual(runtime_offenders(result.stdout + result.stderr), []) self.assertEqual(machine.self_version(), "1.0.0") @unittest.skipIf(WINDOWS, "terminal rejection case uses Unix PTY; Windows also tests noninteractive rejection") @@ -342,13 +384,56 @@ def test_terminal_does_not_substitute_for_waiting_declaration(self): self.fail("undeclared CLI rejection timed out") _, status = os.waitpid(pid, 0) self.assertEqual(os.waitstatus_to_exitcode(status), 1, captured.decode(errors="replace")) - self.assertIn("has no waiting declaration", captured.decode(errors="replace")) + rendered = captured.decode(errors="replace") + reply = json.loads(rendered.strip()) + self.assert_failure_contract(reply) + self.assertEqual(reply["code"], "caller_not_computer") + self.assertIsNone(reply["receipt"]) + self.assertIn("This command is started by Raft Computer", reply["line"]) + self.assertEqual(runtime_offenders(reply["line"] + reply["reason"]), []) finally: os.close(terminal) self.assertEqual(machine.self_version(), "1.0.0") self.assertIsNone(machine.live()) self.assertFalse((machine.state / "active.json").exists()) + def test_human_failure_output_hides_internal_failure_concepts(self): + rollback = self.machine() + rollback.json(["install", "--version", "1.0.0"]) + rollback.login_start() + result = rollback.run(["upgrade", "--version", "1.2.0"]) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn( + "Raft Computer 1.2.0 didn't start correctly, so 1.0.0 is still running. Nothing else was changed. Run the same install command again to retry.", + result.stdout, + ) + self.assertEqual(runtime_offenders(result.stdout + result.stderr), []) + + unresolved = self.machine() + unresolved.json(["install", "--version", "1.0.0"]) + unresolved.login_start() + (unresolved.k / "operation.json").write_text("broken") + result = unresolved.run(["repair", "--version", "1.2.0"]) + self.assertEqual(result.returncode, 3, result.stdout + result.stderr) + self.assertIn( + "Raft Computer's current status couldn't be confirmed. Run the same command again to continue.", + result.stdout, + ) + self.assertEqual(runtime_offenders(result.stdout + result.stderr), []) + + rejected = self.machine() + rejected.json(["install", "--version", "1.0.0"]) + result = subprocess.run( + [str(rejected.binary), "upgrade", "--version", "1.1.0"], + env=rejected.env({"RCI_FIXTURE_INSTALLER": str(self.server.installer)}), + text=True, + capture_output=True, + timeout=30, + ) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("This command is started by Raft Computer", result.stderr) + self.assertEqual(runtime_offenders(result.stdout + result.stderr), []) + def test_short_lived_protocol_replies_are_flushed(self): machine = self.machine() for _ in range(20): @@ -442,6 +527,9 @@ def test_failure_line_never_names_the_installer_even_with_hostile_home(self): machine.json(["install", "--version", "1.0.0"]) failed = machine.json(["upgrade", "--version", "1.3.0"], expected=1) self.assertEqual(failed["receipt"]["outcome"], "failed") + self.assert_failure_contract(failed) + self.assertEqual(failed["code"], "install_failed") + self.assertIn("diagnostic", failed["receipt"]["detail"]) line = failed["line"] self.assertIn("Run the same install command again", line) self.assertNotIn("installer", line.lower()) @@ -575,10 +663,39 @@ def test_foreign_manager_and_healthy_repair_are_held(self): for action in (["status"], ["install", "--version", "1.1.0"], ["repair", "--version", "1.1.0"]): result = machine.json(action, expected=2) self.assertEqual(machine.binary.read_bytes(), script) - self.assertIn(str(machine.binary), result["line"]) - self.assertIn("remove", result["line"]) + self.assertEqual( + result["line"], + f"Not done: Raft Computer at {machine.binary} is managed by a script package manager. " + "Update it with a script package manager.", + ) + self.assertEqual( + result["reason"], + f"Raft Computer at {machine.binary} is managed by a script package manager", + ) + self.assertEqual(runtime_offenders(result["line"]), []) + self.assertNotIn("remove", result["line"]) self.assertEqual(self.server.requests, []) + if not WINDOWS: + linked = self.machine() + linked.preinstall("1.0.0") + linked.sidecar = linked.install_dir / "photon_rs_bg.wasm" + linked.sidecar.unlink() + target = linked.home / "foreign-support-file" + target.write_bytes(self.server.sidecar) + linked.sidecar.symlink_to(target) + result = linked.json(["status"], expected=2) + self.assertEqual( + result["line"], + "Not done: Raft Computer's installation is managed by another tool.", + ) + self.assertEqual( + result["reason"], + "Raft Computer's installation is managed by another tool", + ) + self.assertEqual(runtime_offenders(result["line"]), []) + self.assertNotIn(str(linked.sidecar), result["line"]) + def test_broken_states_are_repaired_then_recovery_payloads_are_removed(self): def damage_artifact(machine): (machine.k / "slots/stable/artifact.bin").unlink() @@ -804,7 +921,9 @@ def test_bad_sources_fail_before_publication(self): for name, extra in cases: with self.subTest(name=name): machine = self.machine() - machine.json(["install"], expected=1, extra=extra) + failed = machine.json(["install"], expected=1, extra=extra) + self.assert_failure_contract(failed) + self.assertEqual(failed["code"], "release_resolution_failed") self.assertFalse(machine.binary.exists()) self.server.authority_lies.add("1.1.0") machine = self.machine() @@ -829,8 +948,43 @@ def test_top_level_failure_never_names_the_installer(self): result = machine.run(["status"], extra={key: "relative-home"}) self.assertEqual(result.returncode, 1, result.stdout + result.stderr) out = result.stdout + result.stderr - self.assertIn("The installation could not finish.", out) + self.assertIn( + "The installation could not finish. Run the same install command again.", + result.stderr, + ) + self.assertNotIn("user home must be absolute", out) self.assertNotIn("installer", out.lower(), out) + self.assertEqual(runtime_offenders(out), []) + + def test_top_level_failure_preserves_a_private_bounded_diagnostic(self): + machine = self.machine() + for _ in range(18): + result = machine.run(["--not-a-real-option"]) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(runtime_offenders(result.stdout + result.stderr), []) + self.assertNotIn("unknown option", result.stdout + result.stderr) + diagnostics = sorted((machine.state / "diagnostics").glob("cli-*.json")) + self.assertEqual(len(diagnostics), 16) + records = [json.loads(path.read_text()) for path in diagnostics] + self.assertTrue(all(record["formatVersion"] == 1 for record in records)) + self.assertTrue(all(record["kind"] == "top-level-error" for record in records)) + self.assertTrue(all(record["diagnostic"] == "unknown option" for record in records)) + if not WINDOWS: + self.assertTrue(all(path.stat().st_mode & 0o777 == 0o600 for path in diagnostics)) + + def test_worker_execute_failure_keeps_diagnostic_only_in_structured_output(self): + machine = self.machine() + gate = machine.state / "gate" + gate.mkdir(parents=True) + (gate / "upgrade.lock.claims").write_text("not a directory") + result = machine.run(["install", "--version", "1.0.0", "--json"]) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + reply = json.loads(result.stdout) + self.assert_failure_contract(reply) + self.assertEqual(reply["code"], "install_failed") + self.assertEqual(reply["diagnostic"], "directory is not a regular directory") + self.assertNotIn(reply["diagnostic"], result.stderr) + self.assertEqual(runtime_offenders(result.stderr), []) def test_entry_script_speaks_before_its_first_round_trip(self): # A silent terminal reads as a hang: the entry script must say what it diff --git a/verification/test_no_installer_wording.py b/verification/test_no_installer_wording.py index 55f26ec..f199bf1 100644 --- a/verification/test_no_installer_wording.py +++ b/verification/test_no_installer_wording.py @@ -26,6 +26,12 @@ } ENV_NAME = re.compile(r"\b[A-Z_]*INSTALLER_[A-Z_]+\b") # env-var names such as RAFT_COMPUTER_INSTALLER_CHANNEL STRING = re.compile(r'"(?:[^"\\]|\\.)*"') +RUNTIME_FORBIDDEN = re.compile( + r"(?i)diagnostics[/\\\\][0-9a-z._-]+\.json" + r"|(?