diff --git a/docs/deployment.md b/docs/deployment.md index 4c18e27..6888246 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -147,6 +147,24 @@ node "$launcher" doctor Use the external transport flags on these setup commands too, if applicable. For a read-only PATH diagnostic, run the package's `bin/check-install.js` through Node from your shell, outside npm lifecycle scripts. Shell aliases still require the shell checks above. Correct PATH or remove only the verified obsolete npm installation, then open a new terminal and recheck the version. +### Recover a stale OMP installation path + +After switching OMP installation channels, PinkCollab may still point to the old launcher. Confirm the new installation works, inspect the saved entry, then run setup to approve a replacement: + +```sh +omp --version +pinkcollab doctor +pinkcollab setup +``` + +- Setup keeps a working saved launcher. If it has failed, setup offers a validated PATH replacement; confirmation defaults to **No**. To select a launcher explicitly, use `pinkcollab setup --omp omp` or `pinkcollab setup --omp "/absolute/path/to/omp"`. +- For automation, provide `--non-interactive --workspace --omp `. +- For external HTTPS, add your existing `--transport external --public-url https://your-host` options to setup. +- Finish active work first: a required Gateway restart ends active OMP runtimes but preserves provider credentials, pairings, and saved sessions. +- If restart fails, the approved configuration remains saved. Resolve the service error, then run `pinkcollab service restart`; do not delete configuration or pairings. + +If recovery still fails, locate the intended launcher with `type -a omp` on macOS/Linux or `Get-Command omp -All` in PowerShell, verify its `--version`, and select it with `setup --omp `. + ## Pairing and access ```sh diff --git a/gateway/Cargo.toml b/gateway/Cargo.toml index a6f60f1..3842212 100644 --- a/gateway/Cargo.toml +++ b/gateway/Cargo.toml @@ -36,7 +36,7 @@ url = "2" [target.'cfg(windows)'.dependencies] windows-service = "0.8" -windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_System_Console", "Win32_System_Diagnostics_ToolHelp", "Win32_System_JobObjects", "Win32_System_SystemServices", "Win32_System_Threading", "Win32_UI_Shell"] } +windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Console", "Win32_System_Diagnostics_ToolHelp", "Win32_System_JobObjects", "Win32_System_SystemServices", "Win32_System_Threading", "Win32_UI_Shell"] } [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/gateway/src/admin.rs b/gateway/src/admin.rs index 8181c98..affe4f4 100644 --- a/gateway/src/admin.rs +++ b/gateway/src/admin.rs @@ -256,6 +256,38 @@ pub async fn status(dir: &Path) -> Result<()> { ); Ok(()) } +fn doctor_lease_count(dir: &Path) -> Result { + let path = dir.join("pinkcollab.db"); + if !path.try_exists()? { + return Ok(0); + } + let db = + rusqlite::Connection::open_with_flags(path, rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY)?; + let has_leases: bool = db.query_row( + "SELECT EXISTS(SELECT 1 FROM sqlite_master WHERE type='table' AND name='runtime_leases')", + [], + |row| row.get(0), + )?; + if !has_leases { + return Ok(0); + } + Ok(db.query_row("SELECT COUNT(*) FROM runtime_leases", [], |row| row.get(0))?) +} + +async fn doctor_omp_version(path: &Path) -> bool { + let version = omp::version(&workspace::display(path)).await; + if version == "unavailable" { + println!(" version: unavailable (--version failed or timed out)"); + false + } else if version.trim().is_empty() { + println!(" version: unavailable (--version returned empty output)"); + false + } else { + println!(" version: {version}"); + true + } +} + /// Full diagnostics continue through independent failures, including an invalid config. pub async fn doctor(dir: &Path) -> Result<()> { let mut problems = Vec::new(); @@ -281,18 +313,42 @@ pub async fn doctor(dir: &Path) -> Result<()> { }; println!("OMP"); let executable = config.as_ref().map_or("omp", |c| c.omp.as_str()); - match config::resolve_executable(executable) { + println!(" configured entry: {executable}"); + let configured_path = config::resolve_executable(executable); + let configured_valid = match &configured_path { Some(path) => { - let version = omp::version(&workspace::display(&path)).await; - println!(" executable: {}\n version: {version}", path.display()); - if version == "unavailable" { - problems.push("OMP"); - } + println!(" executable: {}", path.display()); + doctor_omp_version(path).await } None => { - println!(" executable: {executable} (unavailable)"); - problems.push("OMP"); + println!(" resolution: unavailable (missing or not executable)"); + false + } + }; + if !configured_valid { + problems.push("OMP"); + match config::resolve_executable("omp") { + Some(path) => { + println!(" PATH candidate: {}", path.display()); + if configured_path + .as_ref() + .is_some_and(|configured| config::same_executable_entry(configured, &path)) + { + println!( + " PATH candidate validation: invalid (same entry already failed --version)" + ); + println!( + " Fix this launcher or select a different entry with `pinkcollab setup --omp `." + ); + } else if doctor_omp_version(&path).await { + println!(" PATH candidate validation: valid"); + } else { + println!(" PATH candidate validation: invalid"); + } + } + None => println!(" PATH candidate: no executable omp found"), } + println!(" Recovery: run `pinkcollab setup --omp omp` after validating OMP on PATH."); } let remote = config .as_ref() @@ -437,8 +493,8 @@ pub async fn doctor(dir: &Path) -> Result<()> { " clients: {}\n sessions: {}", counts.clients, counts.sessions ); - match Store::open(dir).and_then(|s| s.runtime_leases()) { - Ok(leases) => println!(" leases: {}", leases.len()), + match doctor_lease_count(dir) { + Ok(leases) => println!(" leases: {leases}"), Err(error) => { println!(" leases: unavailable ({error:#})"); problems.push("Database leases"); diff --git a/gateway/src/config.rs b/gateway/src/config.rs index f56a9ed..d4fe42a 100644 --- a/gateway/src/config.rs +++ b/gateway/src/config.rs @@ -132,6 +132,49 @@ pub fn resolve_executable(name: &str) -> Option { .and_then(|candidate| std::path::absolute(candidate).ok()) }) } +/// Compares launcher entries without following their final symlink/reparse point. +/// Windows filesystem identity handles casing and aliases without folding distinct files +/// in case-sensitive directories. Other platforms retain lexical entry comparison. +pub fn same_executable_entry(left: &Path, right: &Path) -> bool { + if left == right { + return true; + } + #[cfg(windows)] + { + use std::os::windows::{fs::OpenOptionsExt, io::AsRawHandle}; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_FLAG_OPEN_REPARSE_POINT, FILE_ID_INFO, FileIdInfo, GetFileInformationByHandleEx, + }; + + fn identity(path: &Path) -> Option<(u64, [u8; 16])> { + let file = std::fs::OpenOptions::new() + .access_mode(0) + .custom_flags(FILE_FLAG_OPEN_REPARSE_POINT) + .open(path) + .ok()?; + // FileIdInfo initializes the entire output structure on success. + let mut info: FILE_ID_INFO = unsafe { std::mem::zeroed() }; + let success = unsafe { + GetFileInformationByHandleEx( + file.as_raw_handle(), + FileIdInfo, + (&raw mut info).cast(), + std::mem::size_of::() as u32, + ) + }; + (success != 0).then_some((info.VolumeSerialNumber, info.FileId.Identifier)) + } + + identity(left) + .zip(identity(right)) + .is_some_and(|(left, right)| left == right) + } + #[cfg(not(windows))] + { + false + } +} + fn executable(path: &Path) -> bool { #[cfg(unix)] { diff --git a/gateway/src/main.rs b/gateway/src/main.rs index 4e7b65f..73ca282 100644 --- a/gateway/src/main.rs +++ b/gateway/src/main.rs @@ -48,6 +48,9 @@ enum Commands { /// Explicit HTTPS Gateway root URL for an externally managed reverse proxy. #[arg(long)] public_url: Option, + /// Select and validate OMP explicitly; use "omp" to rediscover it on PATH. + #[arg(long)] + omp: Option, }, /// Manage the background Gateway. #[command(display_order = 3)] @@ -135,12 +138,14 @@ async fn main() -> Result<()> { non_interactive, transport, public_url, + omp, } => match setup::run( &cli.data_dir, &workspace, non_interactive, transport, public_url.as_deref(), + omp.as_deref(), ) .await { diff --git a/gateway/src/setup.rs b/gateway/src/setup.rs index 3e01cfa..3b5b118 100644 --- a/gateway/src/setup.rs +++ b/gateway/src/setup.rs @@ -126,12 +126,79 @@ async fn enter(control: &Control, prompt: &str) -> Result { .await } +async fn validated_omp(control: &Control, reference: &str) -> Result<(PathBuf, String)> { + let executable = config::resolve_executable(reference) + .with_context(|| format!("OMP entry {reference:?} is missing or not executable"))?; + let version = control + .wait(async { Ok(omp::version(&workspace::display(&executable)).await) }) + .await?; + ensure!( + version != "unavailable" && !version.is_empty(), + "OMP entry {reference:?} failed `--version` (failed to start, nonzero exit, timeout, or empty output)" + ); + Ok((executable, version)) +} + +async fn select_omp( + control: &Control, + configured: &str, + explicit: Option<&str>, + non_interactive: bool, +) -> Result<(PathBuf, String)> { + if let Some(reference) = explicit { + return validated_omp(control, reference).await.context(OMP_HELP); + } + let configured_path = config::resolve_executable(configured); + let failure = match validated_omp(control, configured).await { + Ok(selected) => return Ok(selected), + Err(error) => { + control.checkpoint().await?; + error + } + }; + println!("Configured OMP: {} ({failure:#})", safe_text(configured)); + let candidate_path = config::resolve_executable("omp"); + ensure!( + !configured_path + .as_ref() + .zip(candidate_path.as_ref()) + .is_some_and(|(configured, candidate)| config::same_executable_entry( + configured, candidate + )), + "{failure:#}\nPATH points to the same failed OMP entry; no replacement is available. Fix this launcher or use pinkcollab setup --omp ." + ); + let candidate = validated_omp(control, "omp") + .await + .with_context(|| format!("{failure:#}\n{OMP_HELP}\n\nCheck PATH, then run pinkcollab setup --omp omp (with your workspace and transport options)."))?; + println!( + "PATH candidate: {}\nVersion: {}", + safe_text(&workspace::display(&candidate.0)), + safe_text(&candidate.1) + ); + ensure!( + !non_interactive, + "OMP migration requires authorization. Run pinkcollab setup --omp omp with --non-interactive, --workspace and your transport options. Configuration was not changed." + ); + let answer = enter( + control, + "Use this OMP instead of the configured entry? [y/N] ", + ) + .await?; + ensure!( + matches!(answer.trim().to_ascii_lowercase().as_str(), "y" | "yes"), + "OMP migration declined. Configuration was not changed." + ); + // Revalidate after the prompt; the launcher may have changed while awaiting approval. + validated_omp(control, &workspace::display(&candidate.0)).await +} + pub async fn run( dir: &Path, requested: &[PathBuf], non_interactive: bool, transport: Transport, public_url: Option<&str>, + explicit_omp: Option<&str>, ) -> Result<()> { transport.validate(public_url)?; ensure!( @@ -182,14 +249,8 @@ pub async fn run( "Setup cancelled. Choose a directory with --workspace ." ); } - let executable = config::resolve_executable(&config.omp).context(OMP_HELP)?; - let version = control - .wait(async { Ok(omp::version(&workspace::display(&executable)).await) }) - .await?; - ensure!( - version != "unavailable" && !version.is_empty(), - "{OMP_HELP}" - ); + let (executable, version) = + select_omp(&control, &config.omp, explicit_omp, non_interactive).await?; println!("✓ OMP {}", safe_text(&version)); control.checkpoint().await?; let save_required = merge(&mut config, roots, &executable)? || !exists; @@ -498,13 +559,6 @@ mod tests { assert_eq!(config.max_sessions, 3); } #[test] - fn missing_omp_has_actionable_message() { - let error = config::resolve_executable("/missing/pinkcollab-test/omp") - .context(OMP_HELP) - .unwrap_err(); - assert!(error.to_string().contains("omp --version")); - } - #[test] fn pairing_wait_detects_new_devices_and_expiry_without_printing_credentials() { let dir = tempfile::tempdir().unwrap(); let store = Store::open(dir.path()).unwrap(); diff --git a/gateway/tests/executable_resolution.rs b/gateway/tests/executable_resolution.rs index 34d6d99..ed7e8ea 100644 --- a/gateway/tests/executable_resolution.rs +++ b/gateway/tests/executable_resolution.rs @@ -223,3 +223,103 @@ fn pathext_defaults_when_unset() { fn already_suffixed_name_is_found_on_path() { isolated("already_suffixed", Some(".CMD;.EXE")); } + +#[cfg(windows)] +#[test] +fn doctor_probes_case_variant_of_failed_launcher_only_once() { + use pinkcollab_gateway::config::Config; + + let root = tempfile::tempdir().unwrap(); + let bin = root.path().join("bin"); + let data = root.path().join("data"); + std::fs::create_dir(&bin).unwrap(); + std::fs::create_dir(&data).unwrap(); + let launcher = bin.join("omp.cmd"); + let probes = root.path().join("probes"); + std::fs::write( + &launcher, + format!( + "@echo off\r\necho probe>>\"{}\"\r\nexit /b 23\r\n", + probes.display() + ), + ) + .unwrap(); + let config = Config { + omp: launcher.to_string_lossy().into_owned(), + workspaces: vec![root.path().to_owned()], + public_url: "https://proxy.example".into(), + ..Config::default() + }; + let config_path = data.join("config.yaml"); + let before = serde_yaml::to_string(&config).unwrap(); + std::fs::write(&config_path, &before).unwrap(); + let modified = config_path.metadata().unwrap().modified().unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_pinkcollab-gateway")) + .arg("--data-dir") + .arg(&data) + .arg("doctor") + .env("PATH", &bin) + .env("PATHEXT", ".COM;.EXE;.BAT;.CMD") + .env("USERPROFILE", root.path()) + .env("HOME", root.path()) + .env("APPDATA", root.path().join("roaming")) + .env("LOCALAPPDATA", root.path().join("local")) + .output() + .unwrap(); + assert!(!output.status.success()); + assert_eq!(std::fs::read_to_string(&probes).unwrap().lines().count(), 1); + assert_eq!(std::fs::read_to_string(&config_path).unwrap(), before); + assert_eq!( + config_path.metadata().unwrap().modified().unwrap(), + modified + ); + assert!(!data.join("pinkcollab.db").exists()); +} + +#[test] +fn launcher_entry_comparison_keeps_distinct_files_separate() { + use pinkcollab_gateway::config::same_executable_entry; + + let root = tempfile::tempdir().unwrap(); + let first = root.path().join("omp.exe"); + let second = root.path().join("replacement.exe"); + std::fs::write(&first, b"same contents").unwrap(); + std::fs::write(&second, b"same contents").unwrap(); + assert!(same_executable_entry(&first, &first)); + assert!(!same_executable_entry(&first, &second)); + assert!(!same_executable_entry( + &first, + &root.path().join("missing.exe") + )); +} + +#[cfg(windows)] +#[test] +fn launcher_entry_comparison_handles_unicode_case_variants() { + use pinkcollab_gateway::config::same_executable_entry; + + let root = tempfile::tempdir().unwrap(); + let entry = root.path().join("安装-ä.exe"); + std::fs::write(&entry, b"launcher").unwrap(); + assert!(same_executable_entry( + &entry, + &root.path().join("安装-Ä.EXE") + )); +} + +#[cfg(unix)] +#[test] +fn launcher_entry_comparison_does_not_merge_symlinks_to_one_target() { + use pinkcollab_gateway::config::same_executable_entry; + + let root = tempfile::tempdir().unwrap(); + let target = root.path().join("target"); + let first = root.path().join("omp"); + let second = root.path().join("replacement"); + std::fs::write(&target, b"launcher").unwrap(); + std::os::unix::fs::symlink(&target, &first).unwrap(); + std::os::unix::fs::symlink(&target, &second).unwrap(); + assert!(same_executable_entry(&first, &first)); + assert!(!same_executable_entry(&first, &second)); + assert!(!same_executable_entry(&first, &target)); +} diff --git a/gateway/tests/onboarding.rs b/gateway/tests/onboarding.rs index e2f18af..6ea387a 100644 --- a/gateway/tests/onboarding.rs +++ b/gateway/tests/onboarding.rs @@ -35,6 +35,269 @@ fn versioned_omp(home: &Path) -> PathBuf { std::os::unix::fs::symlink(&versioned, &entry).unwrap(); entry } + +fn saved_omp_config(home: &Path, data: &Path, omp: &Path) -> Config { + std::fs::create_dir_all(data).unwrap(); + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let config = Config { + listen: listener.local_addr().unwrap(), + omp: omp.to_string_lossy().into_owned(), + public_url: "https://proxy.example".into(), + name: "Keep this name".into(), + omp_args: vec!["--keep-argument".into()], + max_sessions: 17, + workspaces: vec![home.canonicalize().unwrap()], + ..Config::default() + }; + std::fs::write( + data.join("config.yaml"), + serde_yaml::to_string(&config).unwrap(), + ) + .unwrap(); + config +} + +fn config_snapshot(data: &Path) -> (Vec, std::time::SystemTime) { + let path = data.join("config.yaml"); + ( + std::fs::read(&path).unwrap(), + path.metadata().unwrap().modified().unwrap(), + ) +} + +fn migration_setup(home: &Path, data: &Path) -> Command { + let mut command = cli(home, data); + command + .args([ + "setup", + "--non-interactive", + "--transport", + "external", + "--public-url", + "https://proxy.example", + "--workspace", + ]) + .arg(home); + command +} + +#[test] +fn stale_omp_noninteractive_requires_authorization() { + let home = tempfile::tempdir().unwrap(); + tools(home.path()); + let data = home.path().join("data"); + let stale = home.path().join("removed-installation/omp"); + saved_omp_config(home.path(), &data, &stale); + let before = config_snapshot(&data); + let output = migration_setup(home.path(), &data).output().unwrap(); + assert!(!output.status.success()); + let diagnostic = format!( + "{} {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(diagnostic.contains("setup --omp omp"), "{diagnostic}"); + assert!(diagnostic.contains(stale.to_str().unwrap()), "{diagnostic}"); + assert_eq!(config_snapshot(&data), before); + assert!(!data.join("pinkcollab.db").exists()); + assert!(!home.path().join("Library").exists()); + assert!(!home.path().join(".config").exists()); +} + +#[test] +fn failed_configured_omp_is_probed_once_when_path_matches() { + for bare_name in [false, true] { + for doctor in [false, true] { + let home = tempfile::tempdir().unwrap(); + tools(home.path()); + let entry = versioned_omp(home.path()); + executable( + &home.path().join("bin/omp-0.1.0"), + r#"printf '%s\n' "$1" >> "$HOME/omp-probes"; exit 23"#, + ); + let data = home.path().join("data"); + saved_omp_config( + home.path(), + &data, + if bare_name { Path::new("omp") } else { &entry }, + ); + let before = config_snapshot(&data); + let output = if doctor { + cli(home.path(), &data).arg("doctor").output().unwrap() + } else { + migration_setup(home.path(), &data).output().unwrap() + }; + assert!(!output.status.success()); + assert_eq!( + std::fs::read_to_string(home.path().join("omp-probes")).unwrap(), + "--version\n", + "doctor={doctor}, bare_name={bare_name}" + ); + assert_eq!(config_snapshot(&data), before); + } + } +} + +#[test] +fn stale_omp_invalid_candidates_leave_config_and_service_untouched() { + for candidate in [ + None, + Some("exit 23"), + Some("printf ' \\n'"), + Some("exec /bin/sleep 30"), + ] { + let home = tempfile::tempdir().unwrap(); + tools(home.path()); + match candidate { + Some(script) => executable(&home.path().join("bin/omp"), script), + None => std::fs::remove_file(home.path().join("bin/omp")).unwrap(), + } + let data = home.path().join("data"); + saved_omp_config(home.path(), &data, &home.path().join("removed/omp")); + let before = config_snapshot(&data); + #[cfg(not(target_os = "linux"))] + let definition = home + .path() + .join("Library/LaunchAgents/dev.pinkcollab.gateway.plist"); + #[cfg(target_os = "linux")] + let definition = home.path().join(".config/systemd/user/pinkcollab.service"); + #[cfg(target_os = "macos")] + let base = home.path().join("Library/Application Support/PinkCollab"); + #[cfg(target_os = "linux")] + let base = home.path().join(".local/share/pinkcollab"); + std::fs::create_dir_all(base.join("bin")).unwrap(); + std::fs::write( + base.join("bin/data-dir"), + data.canonicalize().unwrap().to_string_lossy().as_bytes(), + ) + .unwrap(); + executable(&home.path().join("bin/id"), "echo 501"); + std::fs::create_dir_all(definition.parent().unwrap()).unwrap(); + std::fs::write(&definition, b"existing service definition").unwrap(); + let definition_before = ( + std::fs::read(&definition).unwrap(), + definition.metadata().unwrap().modified().unwrap(), + ); + // Any service-manager invocation is recorded, including an accidental restart. + for tool in ["systemctl", "launchctl"] { + executable( + &home.path().join("bin").join(tool), + r#"printf '%s\n' "$*" >> "$HOME/service-commands" +case "$1 $2" in + "print "*) exit 1 ;; + "--user show") echo inactive ;; + *) echo unexpected-mutation >> "$HOME/service-commands"; exit 99 ;; +esac"#, + ); + } + let absolute = home.path().join("bin/omp"); + for override_reference in [None, Some("omp"), Some(absolute.to_str().unwrap())] { + let mut command = migration_setup(home.path(), &data); + if let Some(reference) = override_reference { + command.args(["--omp", reference]); + } + let started = std::time::Instant::now(); + let output = command.output().unwrap(); + assert!(!output.status.success(), "{candidate:?}"); + let diagnostic = String::from_utf8_lossy(&output.stderr); + assert!(diagnostic.contains("OMP"), "{diagnostic}"); + assert!( + diagnostic.contains(if candidate.is_none() { + "missing or not executable" + } else { + "failed `--version`" + }), + "{diagnostic}" + ); + assert!( + started.elapsed() < std::time::Duration::from_secs(15), + "{candidate:?}" + ); + assert_eq!(config_snapshot(&data), before, "{candidate:?}"); + assert_eq!( + ( + std::fs::read(&definition).unwrap(), + definition.metadata().unwrap().modified().unwrap() + ), + definition_before, + "{candidate:?}" + ); + let calls = std::fs::read_to_string(home.path().join("service-commands")).unwrap(); + assert!(!calls.contains("unexpected-mutation"), "{calls}"); + assert!(!data.join("pinkcollab.db").exists(), "{candidate:?}"); + } + } +} + +#[test] +fn valid_pinned_omp_does_not_execute_path_candidate() { + let home = tempfile::tempdir().unwrap(); + tools(home.path()); + let pinned = home.path().join("pinned-omp"); + executable(&pinned, "echo pinned-version"); + executable( + &home.path().join("bin/omp"), + r#"echo probed > "$HOME/path-omp-probed"; exit 99"#, + ); + let data = home.path().join("data"); + let expected = saved_omp_config(home.path(), &data, &pinned); + let output = migration_setup(home.path(), &data).output().unwrap(); + // Missing fake service manager prevents completion after OMP preflight. + assert!(!output.status.success()); + assert_eq!(Config::load(&data).unwrap().omp, expected.omp); + assert!(!home.path().join("path-omp-probed").exists()); +} + +#[test] +fn stale_omp_doctor_reports_candidate_without_migrating() { + let home = tempfile::tempdir().unwrap(); + tools(home.path()); + let data = home.path().join("data"); + let stale = home.path().join("removed/omp"); + saved_omp_config(home.path(), &data, &stale); + let before = config_snapshot(&data); + let output = cli(home.path(), &data).arg("doctor").output().unwrap(); + assert!(!output.status.success()); + let text = format!( + "{} {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(text.contains(stale.to_str().unwrap()), "{text}"); + assert!( + text.contains(home.path().join("bin/omp").to_str().unwrap()), + "{text}" + ); + assert!(text.contains("0.1.0"), "{text}"); + assert!(text.contains("setup --omp omp"), "{text}"); + assert_eq!(config_snapshot(&data), before); + assert!(!data.join("pinkcollab.db").exists()); + // Diagnostics must not migrate an older database either. + let database = data.join("pinkcollab.db"); + let db = rusqlite::Connection::open(&database).unwrap(); + db.execute_batch("CREATE TABLE clients (id TEXT); PRAGMA user_version=1;") + .unwrap(); + drop(db); + let database_before = ( + std::fs::read(&database).unwrap(), + database.metadata().unwrap().modified().unwrap(), + ); + let output = cli(home.path(), &data).arg("doctor").output().unwrap(); + assert!(!output.status.success()); + assert_eq!(config_snapshot(&data), before); + assert!( + !String::from_utf8_lossy(&output.stderr).contains("Database leases"), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + ( + std::fs::read(&database).unwrap(), + database.metadata().unwrap().modified().unwrap(), + ), + database_before + ); +} #[test] fn doctor_reports_all_sections_even_when_config_and_tools_fail() { let home = tempfile::tempdir().unwrap(); @@ -57,7 +320,6 @@ fn doctor_reports_all_sections_even_when_config_and_tools_fail() { ] { assert!(text.contains(section), "{text}"); } - assert!(text.contains("omp (unavailable)")); assert!(text.contains("backend: Stopped")); } #[test] @@ -486,6 +748,9 @@ impl HealthFixture { while !server_stop.load(Ordering::Relaxed) { match listener.accept() { Ok((mut socket, _)) => { + // macOS accepted sockets inherit the listener's nonblocking mode. + // Use blocking reads with the timeout below, not an immediate WouldBlock. + socket.set_nonblocking(false).unwrap(); socket .set_read_timeout(Some(std::time::Duration::from_secs(1))) .unwrap(); @@ -540,11 +805,25 @@ exit 99"#, let health = HealthFixture::new(); let data = home.path().join("data"); let store = Store::open(&data).unwrap(); + let pairing = store.new_pairing().unwrap(); + let (client_id, _) = store.pair(&pairing, "Existing phone").unwrap(); + let db = rusqlite::Connection::open(data.join("pinkcollab.db")).unwrap(); + db.execute( + "INSERT INTO session_records (id,host_id,cwd,title,metadata_revision,created_at,updated_at,engine_session_ref) VALUES ('sess_existing',?1,?2,'Preserved task',3,'2026-01-01T00:00:00Z','2026-01-02T00:00:00Z','existing-transcript')", + rusqlite::params![store.host_id().unwrap(), home.path().to_str().unwrap()], + ).unwrap(); + drop(db); + let pinned = home.path().join("pinned-omp"); + executable(&pinned, "echo pinned-version"); let mut expected = Config { listen: health.address, public_url: "https://old-host.ts.net".into(), name: "Custom gateway name".into(), - omp: home.path().join("bin/omp").to_string_lossy().into_owned(), + omp: home + .path() + .join("removed-installation/omp") + .to_string_lossy() + .into_owned(), omp_args: vec!["--custom-argument".into()], max_sessions: 17, workspaces: vec![home.path().canonicalize().unwrap()], @@ -583,6 +862,7 @@ exit 99"#, r#"printf '%s\n' "systemctl $*" >> "$HOME/service-commands" case "$2" in show) echo active ;; + stop) [ ! -f "$HOME/restart-fails" ] || exit 23 ;; start) while IFS= read -r line; do case "$line" in public_url:*) printf '%s\n' "$line" >> "$HOME/service-commands" ;; esac @@ -594,7 +874,7 @@ esac"#, r#"printf '%s\n' "launchctl $*" >> "$HOME/service-commands" case "$1" in print) [ ! -s "$HOME/service-stopped" ] || exit 1; echo 'state = running' ;; - bootout) echo stopped > "$HOME/service-stopped" ;; + bootout) [ ! -f "$HOME/restart-fails" ] || exit 23; echo stopped > "$HOME/service-stopped" ;; bootstrap) while IFS= read -r line; do case "$line" in public_url:*) printf '%s\n' "$line" >> "$HOME/service-commands" ;; esac @@ -603,13 +883,26 @@ case "$1" in esac"#, ); let mut unchanged_config = None; - for url in [ - "https://proxy.example:8443", - "https://replacement.example", - "https://replacement.example", - ] { + for (index, (url, reference)) in [ + ("https://proxy.example:8443", Some("omp")), + ("https://proxy.example:8443", Some(pinned.to_str().unwrap())), + ("https://proxy.example:8443", None), + ("https://replacement.example", None), + ("https://replacement.example", None), + ] + .into_iter() + .enumerate() + { let before = health.requests.load(std::sync::atomic::Ordering::Relaxed); - let output = cli(home.path(), &data) + let before_config = config_snapshot(&data); + let before_definition = ( + std::fs::read(&definition).unwrap(), + definition.metadata().unwrap().modified().unwrap(), + ); + let before_calls = + std::fs::read_to_string(home.path().join("service-commands")).unwrap_or_default(); + let mut command = cli(home.path(), &data); + command .args([ "setup", "--non-interactive", @@ -619,9 +912,11 @@ esac"#, url, "--workspace", ]) - .arg(home.path()) - .output() - .unwrap(); + .arg(home.path()); + if let Some(reference) = reference { + command.args(["--omp", reference]); + } + let output = command.output().unwrap(); assert!( output.status.success(), "{} {}", @@ -630,6 +925,62 @@ esac"#, ); assert!(health.requests.load(std::sync::atomic::Ordering::Relaxed) > before); expected.public_url = url.into(); + expected.omp = if index == 0 { + home.path().join("bin/omp").to_string_lossy().into_owned() + } else { + pinned.to_string_lossy().into_owned() + }; + let calls = std::fs::read_to_string(home.path().join("service-commands")).unwrap(); + let new_calls = &calls[before_calls.len()..]; + #[cfg(target_os = "macos")] + let restarts = new_calls + .lines() + .filter(|line| line.starts_with("launchctl bootout ")) + .count(); + #[cfg(target_os = "linux")] + let restarts = new_calls + .lines() + .filter(|line| *line == "systemctl --user stop pinkcollab") + .count(); + let expected_restarts = usize::from(!matches!(index, 2 | 4)); + assert_eq!( + restarts, expected_restarts, + "iteration {index}: {new_calls}" + ); + #[cfg(target_os = "macos")] + let starts = new_calls + .lines() + .filter(|line| line.starts_with("launchctl bootstrap")) + .count(); + #[cfg(target_os = "linux")] + let starts = new_calls + .lines() + .filter(|line| *line == "systemctl --user start pinkcollab") + .count(); + // systemctl start is idempotent but still invoked after installation; + // launchctl start skips bootstrap when the service is already loaded. + #[cfg(target_os = "linux")] + let expected_starts = expected_restarts + 1; + #[cfg(target_os = "macos")] + let expected_starts = expected_restarts; + assert_eq!(starts, expected_starts, "iteration {index}: {new_calls}"); + if index == 1 || index == 2 { + assert_eq!( + ( + std::fs::read(&definition).unwrap(), + definition.metadata().unwrap().modified().unwrap() + ), + before_definition, + "config-only migration must not rewrite the service definition" + ); + } + if index == 2 || index == 4 { + assert_eq!( + config_snapshot(&data), + before_config, + "repeat setup rewrote config" + ); + } assert_eq!( serde_yaml::to_string(&Config::load(&data).unwrap()).unwrap(), serde_yaml::to_string(&expected).unwrap() @@ -641,7 +992,17 @@ esac"#, .query_row("SELECT COUNT(*) FROM pairing", [], |row| row.get(0)) .unwrap(); assert_eq!(tokens, 0, "noninteractive setup created a pairing token"); - assert!(store.clients().unwrap().is_empty()); + let clients = store.clients().unwrap(); + assert_eq!(clients.len(), 1); + assert_eq!(clients[0].id, client_id); + assert_eq!(clients[0].name, "Existing phone"); + let session = store.v2_session("sess_existing").unwrap().unwrap(); + assert_eq!(session.title, "Preserved task"); + assert_eq!(session.metadata_revision, 3); + assert_eq!( + session.engine_session_ref.as_deref(), + Some("existing-transcript") + ); if url == "https://replacement.example" { let bytes = std::fs::read(data.join("config.yaml")).unwrap(); if let Some(previous) = &unchanged_config { @@ -699,4 +1060,61 @@ esac"#, store .pair(payload["token"].as_str().unwrap(), "External phone") .unwrap(); + // Authorized config is saved before restarting; a failed restart is not success. + let replacement = home.path().join("replacement-omp"); + executable(&replacement, "echo replacement-version"); + std::fs::write(home.path().join("restart-fails"), b"fail").unwrap(); + let before_calls = std::fs::read_to_string(home.path().join("service-commands")).unwrap(); + let before_definition = ( + std::fs::read(&definition).unwrap(), + definition.metadata().unwrap().modified().unwrap(), + ); + let output = cli(home.path(), &data) + .args([ + "setup", + "--non-interactive", + "--transport", + "external", + "--public-url", + "https://replacement.example", + "--workspace", + ]) + .arg(home.path()) + .arg("--omp") + .arg(&replacement) + .output() + .unwrap(); + assert!( + !output.status.success(), + "failed service restart was reported as success" + ); + assert_eq!( + Config::load(&data).unwrap().omp, + replacement.to_str().unwrap() + ); + assert_eq!( + ( + std::fs::read(&definition).unwrap(), + definition.metadata().unwrap().modified().unwrap() + ), + before_definition + ); + let calls = std::fs::read_to_string(home.path().join("service-commands")).unwrap(); + let new_calls = &calls[before_calls.len()..]; + #[cfg(target_os = "macos")] + { + assert!(new_calls.contains("launchctl bootout"), "{new_calls}"); + assert!(!new_calls.contains("launchctl bootstrap"), "{new_calls}"); + } + #[cfg(target_os = "linux")] + { + assert!( + new_calls.contains("systemctl --user stop pinkcollab"), + "{new_calls}" + ); + assert!( + !new_calls.contains("systemctl --user start pinkcollab"), + "{new_calls}" + ); + } }