diff --git a/crates/harness-runtime/src/facts.rs b/crates/harness-runtime/src/facts.rs index 4f69ab3..77c78f4 100644 --- a/crates/harness-runtime/src/facts.rs +++ b/crates/harness-runtime/src/facts.rs @@ -136,6 +136,24 @@ impl Harness { }) } + /// Whether this build can put a program on disk from bytes it can verify. + /// + /// Not the same question as "does this harness have a `software` field". + /// Pi's is `Some`, and its delivery is a package manager -- the product is + /// installable, just not by fetching an artifact whose digest was fixed in + /// advance. Offering it `software` and `rollback` would be offering commands + /// that can only refuse. + #[must_use] + pub const fn installs_a_program(&self) -> bool { + matches!( + self.software, + Some(Software { + delivery: Delivery::Artifacts(_), + .. + }) + ) + } + /// The projection this provider owns, which is what its identity is over. /// /// Exactly `native_namespaces`. It is a method rather than a field access so diff --git a/crates/harness-runtime/src/human.rs b/crates/harness-runtime/src/human.rs index d445fb4..8f4a1e7 100644 --- a/crates/harness-runtime/src/human.rs +++ b/crates/harness-runtime/src/human.rs @@ -54,6 +54,18 @@ pub enum Command { /// The setup to apply. setup: String, }, + /// Report which versions of the product are installed, and which is exposed. + Software { + /// The program directory to read. + prefix: PathBuf, + }, + /// Point the exposed command back at a version already on disk. + Rollback { + /// The program directory to change. + prefix: PathBuf, + /// The version to expose. Named, never inferred. + to: Option, + }, /// Re-apply whatever setup is already recorded, repairing drift. Reinstall { /// The directory to repair. @@ -109,6 +121,8 @@ pub fn is_human_command(name: &str) -> bool { | "remove" | "adopt" | "diff" + | "software" + | "rollback" ) } @@ -135,6 +149,8 @@ where struct Arguments { target: Option, backup: Option, + prefix: Option, + to: Option, positional: Vec, } @@ -144,29 +160,46 @@ impl Arguments { let mut parsed = Self { target: None, backup: None, + prefix: None, + to: None, positional: Vec::new(), }; let mut index = 0; while index < rest.len() { let Some(token) = rest.get(index) else { break }; match token.as_str() { - "--target" | "--backup" => { + "--target" | "--backup" | "--prefix" | "--to" => { let Some(value) = rest.get(index + 1) else { return Err(local(format!("{token} has no value"))); }; if value.starts_with("--") { return Err(local(format!("{token} has no value"))); } - if token == "--target" { - if parsed.target.is_some() { - return Err(local("--target was given twice")); + match token.as_str() { + "--target" => { + if parsed.target.is_some() { + return Err(local("--target was given twice")); + } + parsed.target = Some(PathBuf::from(value)); + } + "--prefix" => { + if parsed.prefix.is_some() { + return Err(local("--prefix was given twice")); + } + parsed.prefix = Some(PathBuf::from(value)); + } + "--to" => { + if parsed.to.is_some() { + return Err(local("--to was given twice")); + } + parsed.to = Some(value.clone()); } - parsed.target = Some(PathBuf::from(value)); - } else { - if parsed.backup.is_some() { - return Err(local("--backup was given twice")); + _ => { + if parsed.backup.is_some() { + return Err(local("--backup was given twice")); + } + parsed.backup = Some(value.clone()); } - parsed.backup = Some(value.clone()); } index += 2; } @@ -188,6 +221,12 @@ impl Arguments { .ok_or_else(|| local(format!("{name} requires --target "))) } + fn prefix(&self, name: &str) -> Result { + self.prefix + .clone() + .ok_or_else(|| local(format!("{name} requires --prefix "))) + } + fn setup(&self, name: &str) -> Result { match self.positional.as_slice() { [only] => Ok(only.clone()), @@ -210,11 +249,30 @@ impl Arguments { if self.backup.is_some() && name != "restore" { return Err(local(format!("--backup is not an argument of {name}"))); } + if self.prefix.is_some() && !matches!(name, "software" | "rollback") { + return Err(local(format!("--prefix is not an argument of {name}"))); + } + if self.to.is_some() && name != "rollback" { + return Err(local(format!("--to is not an argument of {name}"))); + } match name { "list" => { self.no_setup(name)?; Ok(Command::List) } + "software" => { + self.no_setup(name)?; + Ok(Command::Software { + prefix: self.prefix(name)?, + }) + } + "rollback" => { + self.no_setup(name)?; + Ok(Command::Rollback { + prefix: self.prefix(name)?, + to: self.to, + }) + } "status" => { self.no_setup(name)?; Ok(Command::Status { @@ -293,7 +351,133 @@ pub fn run(harness: &Harness, command: Command) -> Result<()> { Command::Restore { target, backup } => restore(harness, &target, backup), Command::Adopt { target } => adopt_target(harness, &target), Command::Remove { target } => remove(harness, &target), + Command::Software { prefix } => software(harness, &prefix), + Command::Rollback { prefix, to } => rollback(harness, &prefix, to.as_deref()), + } +} + +/// What the program directory holds, and which version answers to the command. +/// +/// Install and update are not here, and their absence is the design rather than +/// an omission: both need bytes fetched over a network this program never +/// touches, so they are the three-phase exchange the wire already carries. +/// Rollback and this reading need no network at all, which is exactly why they +/// can be commands someone types. +fn software(harness: &Harness, prefix: &Path) -> Result<()> { + let declared = declared_software(harness)?; + let present = setup_core::software::Present::under(prefix, declared.command); + + if present.versions.is_empty() { + println!( + "No version of {} is installed under {}.", + declared.command, + prefix.display() + ); + return Ok(()); + } + + println!("{} under {}:", declared.command, prefix.display()); + println!(); + for version in &present.versions { + let mark = if present.exposed.as_ref() == Some(version) { + "* " + } else { + " " + }; + println!(" {mark}{version}"); + } + println!(); + match &present.exposed { + Some(version) => println!( + " {}/bin/{} runs {version}", + prefix.display(), + declared.command + ), + // Worth saying rather than leaving blank: the versions are on disk and + // nothing answers to the command, which is a different situation from + // having nothing installed. + None => println!( + " Nothing is exposed: {}/bin/{} names no installed version.", + prefix.display(), + declared.command + ), + } + if present.versions.len() > 1 { + println!(); + println!( + " Change it with: rollback --to --prefix {}", + prefix.display() + ); + } + Ok(()) +} + +/// Point the exposed command back at a version that is already on disk. +fn rollback(harness: &Harness, prefix: &Path, to: Option<&str>) -> Result<()> { + let declared = declared_software(harness)?; + let present = setup_core::software::Present::under(prefix, declared.command); + + // Named, never inferred. There is no record of what was previous -- only + // what is on disk -- and these version strings do not order reliably: + // cursor's `2026.08.11-e8db854` sorts by string, not by release. Choosing + // "the one before" would invent an ordering the vendor never promised. + let Some(version) = to else { + return Err(local(if present.versions.is_empty() { + format!( + "rollback requires --to , and {} holds none", + prefix.display() + ) + } else { + format!( + "rollback requires --to ; {} holds {}", + prefix.display(), + present.versions.join(", ") + ) + })); + }; + + if present.exposed.as_deref() == Some(version) { + println!( + "{} already runs {version}; nothing to do.", + declared.command + ); + return Ok(()); + } + + let rolled = setup_core::software::rollback(&declared, prefix, version)?; + println!( + "{} now runs {}.", + rolled.executable.display(), + rolled.version + ); + if let Some(previous) = present.exposed { + println!(" it ran {previous} before this, and that tree is still here"); } + Ok(()) +} + +/// The software this build installs, or the reason it installs none. +/// +/// Two different absences, and they are worth separating. A harness with no +/// `software` at all does not offer the lifecycle. A harness whose delivery is a +/// package manager does -- the product is installable, just not by fetching +/// bytes whose digest was fixed in advance -- and answering "nothing is +/// installed under this prefix" would suggest this build could put something +/// there. It cannot, and pi is the one that would have been told so. +fn declared_software(harness: &Harness) -> Result { + let declared = harness.software.ok_or_else(|| { + local(format!( + "{} configures {} and does not install it", + harness.provider_id, harness.product + )) + })?; + if let setup_core::software::Delivery::Manager { tool, reason } = declared.delivery { + return Err(local(format!( + "{} is delivered by {tool}: {reason}", + declared.command + ))); + } + Ok(declared) } fn local(detail: impl Into) -> Error { @@ -783,6 +967,102 @@ fn short(digest: &str) -> String { mod tests { #![allow(clippy::unwrap_used, clippy::panic)] + /// `rollback` names its version and never infers one. + /// + /// There is no record of what was previous -- only what is on disk -- and + /// these version strings do not order reliably: cursor's + /// `2026.08.11-e8db854` sorts by string, not by release. Choosing "the one + /// before" would invent an ordering the vendor never promised, and pointing + /// a command at the wrong build is the class of mistake this program + /// refuses everywhere else. + #[test] + fn rollback_requires_the_version_it_is_going_to() { + let parsed = parse(["rollback", "--prefix", "/tmp/prefix"]).unwrap(); + assert_eq!( + parsed, + Command::Rollback { + prefix: std::path::PathBuf::from("/tmp/prefix"), + to: None + } + ); + let with = parse(["rollback", "--to", "1.2.2", "--prefix", "/tmp/prefix"]).unwrap(); + assert_eq!( + with, + Command::Rollback { + prefix: std::path::PathBuf::from("/tmp/prefix"), + to: Some("1.2.2".to_owned()) + } + ); + } + + /// The two new flags belong to the two new commands and nowhere else. + #[test] + fn prefix_and_to_are_refused_on_commands_that_do_not_take_them() { + for tokens in [ + vec![ + "install", "baseline", "--target", "/tmp/t", "--prefix", "/tmp/p", + ], + vec!["restore", "--target", "/tmp/t", "--to", "1.2.2"], + vec!["status", "--target", "/tmp/t", "--to", "1.2.2"], + ] { + let error = parse(tokens.clone()).unwrap_err(); + assert!( + error.detail().contains("is not an argument of"), + "{tokens:?} was accepted: {}", + error.detail() + ); + } + } + + /// Two different absences, and the wrong one is misleading. + /// + /// Pi's `software` is `Some` and its delivery is npm, so before this was + /// separated `software --prefix` answered *no version of pi is installed + /// under /tmp/x* -- which reads as an invitation to install one. This build + /// cannot, and says so in the same words `plan-operation` uses. + #[test] + fn a_product_delivered_by_a_package_manager_says_so_rather_than_looking_empty() { + let mut managed = crate::wire::tests_support::TEST; + managed.software = Some(setup_core::software::Software { + version: "1.0.0", + command: "managed", + delivery: setup_core::software::Delivery::Manager { + tool: "npm", + reason: "its closure is resolved at install time", + }, + unsupported: &[], + }); + assert!(!managed.installs_a_program()); + + let error = declared_software(&managed).unwrap_err(); + assert!(error.detail().contains("npm"), "{}", error.detail()); + assert!( + !error.detail().contains("no version"), + "it read as an empty prefix: {}", + error.detail() + ); + + // And a build that installs nothing at all is a third answer again. + let mut none = crate::wire::tests_support::TEST; + none.software = None; + assert!(!none.installs_a_program()); + assert!( + declared_software(&none) + .unwrap_err() + .detail() + .contains("does not install it") + ); + } + + /// Both new commands take a program directory, and neither guesses one. + #[test] + fn software_and_rollback_require_a_prefix() { + for name in ["software", "rollback"] { + let error = parse([name]).unwrap_err(); + assert!(error.detail().contains("--prefix"), "{}", error.detail()); + } + } + use crate::wire::tests_support::TEST; use super::*; diff --git a/crates/harness-runtime/src/lib.rs b/crates/harness-runtime/src/lib.rs index e5991ff..de51fb4 100644 --- a/crates/harness-runtime/src/lib.rs +++ b/crates/harness-runtime/src/lib.rs @@ -85,6 +85,26 @@ pub fn run(harness: &Harness, arguments: Vec) -> ExitCode { }; } + // ` --help` used to answer `--help has no value`, because the flag + // parser reads every `--flag` as taking one. A caller could not ask what a + // command takes, and each missing argument surfaced singly -- seven + // invocations to learn `plan-operation`, measured by a peer who already + // knew the shape. + // + // Read before parsing, and only outside a passthrough: everything after a + // bare `--` belongs to the product `launch` starts, where `--help` means + // something else entirely and is not ours to intercept. + if let Some(command) = provider_v3::vocabulary::Command::parse(first) { + let mine = arguments + .iter() + .take_while(|token| *token != "--") + .any(|token| token == "--help"); + if mine { + print!("{}", provider_v3::argv::render_usage(command)); + return ExitCode::SUCCESS; + } + } + let invocation = match provider_v3::argv::parse(arguments) { Ok(invocation) => invocation, Err(error) => { @@ -182,7 +202,19 @@ fn print_help(harness: &Harness) { if !harness.predecessor_state_file.is_empty() { println!(" adopt --target "); } + if harness.installs_a_program() { + println!(" software --prefix "); + println!(" rollback --to --prefix "); + } println!(); + if harness.installs_a_program() { + println!("`software` reads the program directory and `rollback` points the"); + println!("command back at a version already in it -- installing a new one leaves"); + println!("the old tree in place and moves only the command. Both need no network,"); + println!("which is why they are commands you type; install and update need bytes"); + println!("fetched between planning and applying, so they stay on the wire above."); + println!(); + } if !harness.predecessor_state_file.is_empty() { println!( "`adopt` takes over a target still carrying {},", @@ -192,8 +224,11 @@ fn print_help(harness: &Harness) { println!("type, never something install does behind you, and it deletes nothing."); println!(); } - println!("Every one takes an explicit --target. There is no default: a change"); - println!("aimed at a guessed path is a change aimed at someone else's state."); + println!("Every one takes an explicit --target, and the two program commands an"); + println!("explicit --prefix. There is no default: a change aimed at a guessed"); + println!("path is a change aimed at someone else's state. `rollback` names its"); + println!("version for the same reason -- there is no record of which was"); + println!("previous, only what is on disk."); println!(); println!("A backup is captured before every change, so `restore` always has"); println!("something to return to. Over the wire, install and replace arrive as"); diff --git a/crates/harness-runtime/src/wire.rs b/crates/harness-runtime/src/wire.rs index c3a2666..3c8bb02 100644 --- a/crates/harness-runtime/src/wire.rs +++ b/crates/harness-runtime/src/wire.rs @@ -2415,9 +2415,14 @@ mod tests { #[test] fn launch_without_a_prefix_says_where_a_program_lives() { + // Refused at the argv layer now rather than in dispatch: `--prefix` is + // one of the three arguments `launch` is defined by, so the parser + // names it before a target is ever opened. `launch --help` lists the + // same three, from the same table. let target = seeded("launch-noprefix"); - let error = refuse(args("launch", &target, &[])); + let error = argv::parse(args("launch", &target, &[])).unwrap_err(); assert!(error.detail().contains("--prefix"), "{}", error.detail()); + assert!(error.detail().contains("--help"), "{}", error.detail()); } #[test] diff --git a/crates/provider-v3/src/argv.rs b/crates/provider-v3/src/argv.rs index abfe379..5b7a86d 100644 --- a/crates/provider-v3/src/argv.rs +++ b/crates/provider-v3/src/argv.rs @@ -50,6 +50,157 @@ const BUNDLE_FLAGS: &[&str] = &[ "--bundle-size", ]; +/// What one command requires and what it accepts, stated once as data. +/// +/// This table exists because of a measurement, not a preference. A peer +/// building the consumer half spent five round-trips discovering that +/// `plan-operation` takes seven required arguments — and they already knew the +/// shape from their own conformance code. Each missing flag surfaced singly, +/// and `--help` answered `--help has no value`, because it was parsed as a flag +/// that takes one. The one question a caller could ask was met with a complaint +/// about its grammar. +/// +/// The refusals themselves were right, and they are unchanged. What was missing +/// was any way to ask. +/// +/// It is one table read by two callers — [`usage`] renders it and [`parse`] +/// checks against it — because two lists of the same requirement eventually +/// disagree. A test binds it to the parser by removing each named flag from a +/// complete invocation and requiring the refusal to name it. +pub struct Usage { + /// The command this describes. + pub command: Command, + /// Flags without which the command cannot run. + pub required: &'static [&'static str], + /// Flags the command accepts, each with why a caller would pass it. + pub optional: &'static [(&'static str, &'static str)], + /// One line on what the command does with them. + pub note: &'static str, +} + +/// The arguments one command takes. +#[must_use] +pub const fn usage(command: Command) -> Usage { + match command { + Command::ProviderInfo => Usage { + command, + required: &[], + optional: &[], + note: "Report capabilities. Takes no arguments at all, not even --json.", + }, + Command::Status => Usage { + command, + required: &["--target", "--json"], + optional: &[], + note: "Report the target's current state. Never changes it.", + }, + Command::RecoverOperation => Usage { + command, + required: &["--target", "--json"], + optional: &[], + note: "Resolve an interrupted operation. Reads the journal to know what it is resolving.", + }, + Command::ValidateBundle => Usage { + command, + required: &[ + "--target", + "--json", + "--bundle", + "--bundle-format", + "--bundle-digest", + "--artifact-digest", + "--bundle-size", + ], + optional: &[], + note: "Check a bundle against the exact claim that named it. Touches nothing.", + }, + Command::PlanOperation => Usage { + command, + required: &[ + "--target", + "--json", + "--operation", + "--provider-release-digest", + "--operation-id", + "--expires-at", + ], + optional: &[ + ( + "--prefix", + "where a program lives; required by every software_* operation", + ), + ("--backup-ref", "which slot a restore returns to"), + ("--permission-profile", "a profile this build declares"), + ( + "--software-version", + "exactly one pinned version, when not the current one", + ), + ( + "--bundle …", + "the five bundle flags, for install and replace", + ), + ], + note: "Produce a plan. Always pure: reads the target and the local disk, opens no socket.", + }, + Command::ApplyOperation => Usage { + command, + required: &[ + "--target", + "--json", + "--plan", + "--plan-digest", + "--provider-release-digest", + ], + optional: &[ + ( + "--prefix", + "where a program lives; required by every software_* operation", + ), + ( + "--software-artifact", + "one per software_artifacts entry, in the plan's order", + ), + ( + "--bundle …", + "the five bundle flags, for install and replace", + ), + ], + note: "Apply one exact plan under the target lock. --plan is the plan object, \ + written canonically -- not the envelope the planner printed around it.", + }, + Command::Launch => Usage { + command, + required: &["--target", "--json", "--prefix"], + optional: &[( + "-- ", + "everything after a bare -- goes to the product verbatim", + )], + note: "Start the exact executable a software install placed. Never a name found on PATH.", + }, + } +} + +/// Render one command's arguments for a caller who asked. +#[must_use] +pub fn render_usage(command: Command) -> String { + use std::fmt::Write as _; + let shape = usage(command); + let mut out = format!("{command}\n\n {}\n", shape.note); + if !shape.required.is_empty() { + out.push_str("\nRequired:\n"); + for flag in shape.required { + let _ = writeln!(out, " {flag}"); + } + } + if !shape.optional.is_empty() { + out.push_str("\nOptional:\n"); + for (flag, why) in shape.optional { + let _ = writeln!(out, " {flag:<22} {why}"); + } + } + out +} + /// One parsed invocation. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Invocation { @@ -226,6 +377,23 @@ where // leftovers the same way it always would. let (mine, passthrough) = Flags::split_passthrough(rest); let mut flags = Flags::parse(&mine)?; + + // Every missing argument at once, rather than the first one alphabetically. + // Learning a command used to cost one invocation per argument, and the + // count is seven for two of these. + let missing: Vec<&str> = usage(command) + .required + .iter() + .copied() + .filter(|flag| !flags.holds(flag)) + .collect(); + if !missing.is_empty() { + return Err(local(format!( + "{command} is missing {}; run `{command} --help` for what it takes", + missing.join(", ") + ))); + } + let target = PathBuf::from(flags.take_required("--target")?); if !flags.take_switch("--json") { return Err(local(format!("{command} requires --json"))); @@ -359,6 +527,18 @@ impl Flags { Ok(Self { values, switches }) } + /// Whether a flag was given, without consuming it. + /// + /// `--json` is a switch and lives in its own list; asking about it here + /// keeps the completeness check able to name it beside the others rather + /// than leaving one required argument to a separate refusal further down. + fn holds(&self, name: &str) -> bool { + if name == "--json" { + return self.switches.iter().any(|switch| switch == name); + } + self.values.contains_key(name) + } + fn take_required(&mut self, name: &str) -> Result { self.take_optional(name) .ok_or_else(|| local(format!("{name} is required"))) @@ -491,6 +671,129 @@ mod tests { tokens } + /// A complete, well-formed invocation of one command. + /// + /// Paths come from `std::env::temp_dir()` rather than `/tmp`, because **on + /// Windows a rooted path is not an absolute path**: `Path::new("/tmp")` + /// answers `is_absolute() == false` there, since absolute means a drive or + /// a UNC prefix. This project has met that once before -- a fixture passing + /// `/tmp` made a test prove the right thing on two systems and nothing at + /// all on the third -- and this test found it again on the first Windows run + /// after it was written, with `--prefix "/tmp/prefix" is not an absolute + /// path`. The product code was right both times. + fn complete(command: Command) -> Vec { + let temporary = |name: &str| { + std::env::temp_dir() + .join(name) + .to_string_lossy() + .into_owned() + }; + let mut tokens = vec![command.as_str().to_owned()]; + for flag in usage(command).required { + tokens.push((*flag).to_owned()); + if *flag == "--json" { + continue; + } + tokens.push(match *flag { + "--target" => temporary("target"), + "--operation" => "install".to_owned(), + "--bundle" | "--plan" => temporary("file"), + "--bundle-format" => "ai-stp-bundle/1".to_owned(), + "--bundle-size" => "4096".to_owned(), + "--operation-id" => "operation_00000000000000000000000".to_owned(), + "--expires-at" => "2027-01-01T00:00:00.000Z".to_owned(), + "--prefix" => temporary("prefix"), + _ => DIGEST.to_owned(), + }); + } + // `install` arrives as a bundle, so a complete plan for it carries one. + if command == Command::PlanOperation { + tokens.extend(bundle_flags()); + } + tokens + } + + /// The table and the parser must demand the same set. + /// + /// [`usage`] is read by `--help` and by the completeness refusal, and it + /// would be worth nothing if it drifted from what `parse` actually enforces + /// -- a caller would be told the truth about a command that then refused + /// something else. So every flag the table calls required is removed from a + /// complete invocation, one at a time, and the refusal must name it. + #[test] + fn every_flag_the_table_calls_required_is_one_the_parser_demands() { + for command in Command::ALL.iter().copied().filter(|c| c.takes_target()) { + let whole = complete(command); + parse(whole.clone()) + .unwrap_or_else(|e| panic!("{command}: a complete invocation was refused: {e}")); + + for flag in usage(command).required { + let mut without = Vec::new(); + let mut skip = false; + for token in &whole { + if skip { + skip = false; + continue; + } + if token == flag { + skip = *flag != "--json"; + continue; + } + without.push(token.clone()); + } + let error = parse(without) + .err() + .unwrap_or_else(|| panic!("{command} was accepted without {flag}")); + assert!( + error.detail().contains(flag), + "{command} without {flag} refused without naming it: {}", + error.detail() + ); + } + } + } + + /// The count is the point: learning a command used to cost one invocation + /// per argument. + #[test] + fn one_refusal_names_every_missing_argument() { + let error = parse(["plan-operation", "--target", "/tmp/target"]).unwrap_err(); + for flag in [ + "--json", + "--operation", + "--provider-release-digest", + "--operation-id", + "--expires-at", + ] { + assert!( + error.detail().contains(flag), + "the refusal did not name {flag}: {}", + error.detail() + ); + } + assert!( + error.detail().contains("--help"), + "it does not say how to ask" + ); + } + + /// Every command can be asked what it takes, and the answer names the same + /// flags the refusal would. + #[test] + fn every_command_can_be_asked_what_it_takes() { + for command in Command::ALL { + let rendered = render_usage(*command); + assert!(rendered.contains(command.as_str())); + assert!(!usage(*command).note.is_empty()); + for flag in usage(*command).required { + assert!( + rendered.contains(flag), + "{command} help omits its own required {flag}" + ); + } + } + } + #[test] fn provider_info_takes_neither_target_nor_json() { assert_eq!(parse(["provider-info"]).unwrap(), Invocation::ProviderInfo); @@ -634,9 +937,43 @@ mod tests { fn a_partial_bundle_is_refused_rather_than_completed() { let mut partial = bundle_flags(); partial.truncate(partial.len() - 2); // drop --bundle-size and its value - let tokens = [with_target("validate-bundle", &[]), partial].concat(); - let error = parse(tokens).unwrap_err(); - assert!(error.detail().contains("all five")); + + // On `validate-bundle` the bundle *is* the command, so the five flags + // are required and the completeness check names the missing one before + // the bundle reader is reached. Naming the exact flag is the better + // answer of the two, and it is the one a caller gets here. + let error = + parse([with_target("validate-bundle", &[]), partial.clone()].concat()).unwrap_err(); + assert!( + error.detail().contains("--bundle-size"), + "{}", + error.detail() + ); + + // On `plan-operation` a bundle is optional, so the invariant that still + // has to be stated is all-five-or-none. This is the refusal that would + // otherwise have been lost when the completeness check went in. + let plan = parse( + [ + with_target( + "plan-operation", + &[ + "--operation", + "install", + "--provider-release-digest", + DIGEST, + "--operation-id", + "operation_00000000000000000000000", + "--expires-at", + "2027-01-01T00:00:00.000Z", + ], + ), + partial, + ] + .concat(), + ) + .unwrap_err(); + assert!(plan.detail().contains("all five"), "{}", plan.detail()); } #[test] diff --git a/crates/setup-core/src/software.rs b/crates/setup-core/src/software.rs index e63cdc7..d2f81db 100644 --- a/crates/setup-core/src/software.rs +++ b/crates/setup-core/src/software.rs @@ -108,6 +108,15 @@ pub struct Present { } impl Present { + /// Where the exposed version is recorded, beside the command it describes. + /// + /// Dotted so it is hidden on Unix, and under `bin/` so it can never be read + /// as a version directory. + #[must_use] + fn marker(root: &Path, command: &str) -> PathBuf { + root.join("bin").join(format!(".{command}.version")) + } + /// Whether this build's pinned version is one of the ones already there. #[must_use] pub fn holds(&self, version: &str) -> bool { @@ -142,18 +151,50 @@ impl Present { // entry point names it. Resolving costs one syscall and is right for // both. A dangling link resolves to nothing, which is also correct: // nothing usable is exposed. - let link = root.join("bin").join(command); - let exposed = fs::canonicalize(&link) + // Recorded, not inferred -- and the reason is Windows. + // + // Resolving the link was the only reading here, and it cannot work on a + // system where `expose` does not make a link. Windows reserves symlink + // creation for privileged processes, so the exposed command is a hard + // link or a copy: canonicalizing it returns its own path, its first + // component under the root is `bin`, and the answer became "no version + // is exposed" on a prefix where one plainly was. + // + // That was not only cosmetic. `Present::exposed` is what separates an + // install from an update, so on Windows every `software_update` saw an + // empty prefix and refused as an update of nothing -- while the version + // it would have updated sat right there. Found by the three-OS matrix + // on the first Windows run of the rollback tests. + // A record is only ever believed about a command that resolves. A + // dangling link exposes nothing whatever the record says -- the record + // remembers what `expose` last pointed at, and someone can break that + // by hand afterwards. `metadata` follows links, so this is false for a + // dangling one and true for both a real file and a live link. + let usable = fs::metadata(root.join("bin").join(command)).is_ok(); + let marker = Self::marker(root, command); + let exposed = fs::read_to_string(&marker) .ok() - .zip(fs::canonicalize(root).ok()) - .and_then(|(to, base)| { - to.strip_prefix(&base).ok().and_then(|rest| { - rest.components() - .next() - .map(|first| first.as_os_str().to_string_lossy().into_owned()) - }) - }) - .filter(|name| versions.contains(name)); + .filter(|_| usable) + .map(|held| held.trim().to_owned()) + // A hand-edited or half-written marker must not name a version that + // is not there. + .filter(|name| versions.contains(name)) + .or_else(|| { + // Nothing recorded: a prefix written before this existed, or one + // someone arranged themselves. Where a real link is what is + // there, reading it is still the truth. + fs::canonicalize(root.join("bin").join(command)) + .ok() + .zip(fs::canonicalize(root).ok()) + .and_then(|(to, base)| { + to.strip_prefix(&base).ok().and_then(|rest| { + rest.components() + .next() + .map(|first| first.as_os_str().to_string_lossy().into_owned()) + }) + }) + .filter(|name| versions.contains(name)) + }); Self { versions, exposed } } @@ -173,6 +214,22 @@ pub struct Installed { } impl Software { + /// Where this build's own artifacts put the executable inside their tree. + /// + /// A *hint*, and named one deliberately: it is right for a version this + /// build installed and is only a first guess for a tree an older build + /// wrote. [`rollback`] tries it, then the flat shape, and refuses naming + /// both rather than pointing a command at a path it did not verify. + #[must_use] + pub fn member_hint(&self) -> &'static str { + match self.delivery { + Delivery::Artifacts(artifacts) => { + artifacts.first().map_or(self.command, |entry| entry.member) + } + Delivery::Manager { .. } => self.command, + } + } + /// The artifact for one platform, or the reason there is not one. /// /// # Errors @@ -352,7 +409,7 @@ pub fn install( }; let exposed = root.join("bin").join(software.command); - expose(&executable, &exposed)?; + expose(&executable, &exposed, software.version, software.command)?; Ok(Installed { version: software.version.to_owned(), @@ -392,16 +449,98 @@ pub fn remove(software: &Software, root: &Path) -> Result { .with_source(error) })?; } + // The record goes with the command it described. A marker outliving it + // would name a version nothing runs. + let _ = fs::remove_file(Present::marker(root, software.command)); Ok(true) } +/// Point the exposed command back at a version that is already on disk. +/// +/// Installing 1.0.6 leaves 1.0.5 in its own directory and moves only the +/// exposed command, so the bytes to go back to are already there. Until this +/// existed nothing pointed at them: the owner named rollback in the same +/// sentence as install, reinstall and select, and three of those four were +/// reachable. +/// +/// This is the one part of the software lifecycle that needs no network at all, +/// which is why it can be a command someone types rather than the three-phase +/// exchange install and update have to be. +/// +/// **The version is named, never inferred.** There is no record of what was +/// previous -- only what is on disk -- and these version strings do not order +/// reliably: `2026.08.11-e8db854` sorts by string, not by release. Picking "the +/// one before" would mean inventing an ordering the vendor never promised, and +/// pointing a command at the wrong build is exactly the class of mistake this +/// program refuses everywhere else. A caller who omits it is told what is here. +/// +/// # Errors +/// +/// Refuses a version that is not installed, naming the ones that are, and a +/// version tree that holds no executable this build can find. +pub fn rollback(software: &Software, root: &Path, to: &str) -> Result { + let present = Present::under(root, software.command); + if !present.versions.iter().any(|found| found == to) { + return Err(Error::new( + ReasonCode::InvalidTarget, + if present.versions.is_empty() { + format!( + "{} holds no installed version of {}", + root.display(), + software.command + ) + } else { + format!( + "{to} is not installed under {}; it holds {}", + root.display(), + present.versions.join(", ") + ) + }, + )); + } + + let version_root = root.join(to); + // Looked for rather than assumed. This build pins one version and knows + // where *its* executable sits inside the archive; an older tree was written + // by an older build, whose artifact table this one does not carry. Both + // shapes it could have used are tried, and neither is guessed at: if the + // file is not there, the refusal says where it looked. + let candidates = [ + version_root.join(software.member_hint()), + version_root.join(software.command), + ]; + let Some(executable) = candidates.iter().find(|path| path.is_file()) else { + return Err(Error::new( + ReasonCode::StateUnavailable, + format!( + "the {to} tree holds no {} executable; looked at {}", + software.command, + candidates + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(" and ") + ), + )); + }; + + let exposed = root.join("bin").join(software.command); + expose(executable, &exposed, to, software.command)?; + Ok(Installed { + version: to.to_owned(), + root: version_root, + executable: exposed, + files: 0, + }) +} + /// Point one stable path at the executable inside a versioned tree. /// /// The member is left where the archive put it. Codex's binary needs the `rg` /// and `bwrap` beside it and cursor's launcher needs its bundled `node`, so /// moving the executable out of its tree would produce a file that runs on the /// machine it was built on and nowhere else. -fn expose(executable: &Path, exposed: &Path) -> Result<()> { +fn expose(executable: &Path, exposed: &Path, version: &str, command: &str) -> Result<()> { let fail = |error: std::io::Error| { Error::new( ReasonCode::StateUnavailable, @@ -419,7 +558,7 @@ fn expose(executable: &Path, exposed: &Path) -> Result<()> { #[cfg(unix)] { - std::os::unix::fs::symlink(executable, exposed).map_err(fail) + std::os::unix::fs::symlink(executable, exposed).map_err(fail)?; } #[cfg(not(unix))] { @@ -428,8 +567,17 @@ fn expose(executable: &Path, exposed: &Path) -> Result<()> { // resort and costs a second copy of a large binary. fs::hard_link(executable, exposed) .or_else(|_| fs::copy(executable, exposed).map(|_| ())) - .map_err(fail) + .map_err(fail)?; + } + + // Which version this now runs, recorded rather than left to be inferred + // from a link that two of the three systems do not make. Written after the + // command is in place, so a marker never names a version that is not + // exposed yet. + if let Some(root) = exposed.parent().and_then(Path::parent) { + fs::write(Present::marker(root, command), version).map_err(fail)?; } + Ok(()) } #[cfg(test)] @@ -471,6 +619,121 @@ mod tests { } } + /// The bytes to go back to are already on disk: installing a new version + /// leaves the old tree in place and moves only the exposed command. Until + /// `rollback` existed nothing pointed back at them, and the owner named + /// rollback in the same sentence as install, reinstall and select. + #[test] + fn rollback_points_the_command_at_a_version_already_on_disk() { + let (at, artifact) = staged("rollback", b"#!/bin/sh\necho new\n", CODEX_MEMBER); + let root = at.join("prefix"); + let installed = install(&software(), &artifact, &at.join("artifact.tgz"), &root).unwrap(); + assert_eq!(installed.version, "1.2.3"); + + // What an update leaves behind: the previous tree, untouched. + let older = root.join("1.2.2"); + fs::create_dir_all(older.join("package/vendor/x86_64-unknown-linux-musl/bin")).unwrap(); + fs::write(older.join(CODEX_MEMBER), b"#!/bin/sh\necho old\n").unwrap(); + + let rolled = rollback(&software(), &root, "1.2.2").unwrap(); + assert_eq!(rolled.version, "1.2.2"); + + let present = Present::under(&root, "codex"); + assert_eq!(present.exposed.as_deref(), Some("1.2.2")); + assert_eq!(present.versions, vec!["1.2.2", "1.2.3"]); + + // And the version it came from is still there to go forward to. + assert!(root.join("1.2.3").join(CODEX_MEMBER).is_file()); + assert_eq!( + rollback(&software(), &root, "1.2.3").unwrap().version, + "1.2.3" + ); + assert_eq!( + Present::under(&root, "codex").exposed.as_deref(), + Some("1.2.3") + ); + } + + /// The exposed version is recorded, so it is readable where no link exists. + /// + /// This is the Windows defect written as a test that fails on Linux too. + /// `expose` makes a symlink on Unix and a hard link or a copy on Windows, + /// and the old reading resolved the link -- so on Windows the answer was + /// always "nothing is exposed", on a prefix where something plainly was. + /// + /// It was not cosmetic: `Present::exposed` is what separates an install + /// from an update, so every `software_update` on Windows saw an empty + /// prefix and refused as an update of nothing. + #[test] + fn the_exposed_version_is_readable_without_a_link_to_resolve() { + let (at, artifact) = staged("exposed-marker", b"#!/bin/sh\necho hi\n", CODEX_MEMBER); + let root = at.join("prefix"); + install(&software(), &artifact, &at.join("artifact.tgz"), &root).unwrap(); + assert_eq!( + Present::under(&root, "codex").exposed.as_deref(), + Some("1.2.3") + ); + + // Exactly what Windows leaves behind: a real file where Unix has a + // link. Nothing to resolve, and the answer must not change. + let exposed = root.join("bin").join("codex"); + let bytes = fs::read(root.join("1.2.3").join(CODEX_MEMBER)).unwrap(); + fs::remove_file(&exposed).unwrap(); + fs::write(&exposed, &bytes).unwrap(); + assert!(!exposed.symlink_metadata().unwrap().is_symlink()); + assert_eq!( + Present::under(&root, "codex").exposed.as_deref(), + Some("1.2.3"), + "the exposed version was unreadable without a link to resolve" + ); + + // A record naming a version that is not there is not believed. + fs::write(root.join("bin").join(".codex.version"), "9.9.9").unwrap(); + assert_eq!(Present::under(&root, "codex").exposed, None); + + // And removing takes the record with the command it described. + fs::write(root.join("bin").join(".codex.version"), "1.2.3").unwrap(); + remove(&software(), &root).unwrap(); + assert!(!root.join("bin").join(".codex.version").exists()); + } + + /// A version that is not there is refused, and the refusal says what is -- + /// otherwise a caller's only way to find out is to guess again. + #[test] + fn rollback_to_a_version_that_is_not_installed_names_the_ones_that_are() { + let (at, artifact) = staged("rollback-missing", b"x", CODEX_MEMBER); + let root = at.join("prefix"); + install(&software(), &artifact, &at.join("artifact.tgz"), &root).unwrap(); + + let error = rollback(&software(), &root, "9.9.9").unwrap_err(); + assert!(error.detail().contains("9.9.9"), "{}", error.detail()); + assert!(error.detail().contains("1.2.3"), "{}", error.detail()); + // Nothing moved. + assert_eq!( + Present::under(&root, "codex").exposed.as_deref(), + Some("1.2.3") + ); + } + + /// A tree an older build wrote may not put the executable where this + /// build's artifacts do. Both shapes are tried and neither is guessed at: + /// if the file is not there, the refusal says where it looked. + #[test] + fn a_version_tree_with_no_executable_is_refused_naming_where_it_looked() { + let (at, artifact) = staged("rollback-empty", b"x", CODEX_MEMBER); + let root = at.join("prefix"); + install(&software(), &artifact, &at.join("artifact.tgz"), &root).unwrap(); + fs::create_dir_all(root.join("1.2.2")).unwrap(); + + let error = rollback(&software(), &root, "1.2.2").unwrap_err(); + assert!(error.detail().contains("1.2.2"), "{}", error.detail()); + assert!(error.detail().contains("looked at"), "{}", error.detail()); + assert_eq!( + Present::under(&root, "codex").exposed.as_deref(), + Some("1.2.3") + ); + } + fn scratch(name: &str) -> PathBuf { let path = std::env::temp_dir().join(format!("setup-core-software-{name}-{}", std::process::id()));