diff --git a/Cargo.lock b/Cargo.lock index 73b1ac9..cddbdf1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1589,7 +1589,7 @@ dependencies = [ [[package]] name = "git-same" -version = "3.2.0" +version = "3.2.1" dependencies = [ "anyhow", "chrono", @@ -1614,7 +1614,7 @@ dependencies = [ [[package]] name = "git-same-app" -version = "3.2.0" +version = "3.2.1" dependencies = [ "anyhow", "chrono", @@ -1635,7 +1635,7 @@ dependencies = [ [[package]] name = "git-same-core" -version = "3.2.0" +version = "3.2.1" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index c44a4be..1f48e77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ members = [ resolver = "2" [workspace.package] -version = "3.2.0" +version = "3.2.1" edition = "2021" authors = ["Manuel Gruber"] license = "MIT" diff --git a/crates/git-same-app/tauri.conf.json b/crates/git-same-app/tauri.conf.json index b8d752c..1d3d312 100644 --- a/crates/git-same-app/tauri.conf.json +++ b/crates/git-same-app/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "Git-Same", - "version": "3.2.0", + "version": "3.2.1", "identifier": "com.zaai.git-same", "build": { "beforeDevCommand": "corepack pnpm dev", diff --git a/crates/git-same-app/ui/package.json b/crates/git-same-app/ui/package.json index 0abc4e1..fce488c 100644 --- a/crates/git-same-app/ui/package.json +++ b/crates/git-same-app/ui/package.json @@ -1,7 +1,7 @@ { "name": "git-same-app-ui", "private": true, - "version": "3.2.0", + "version": "3.2.1", "type": "module", "packageManager": "pnpm@11.0.9+sha512.34ce82e6780233cf9cad8685029a8f81d2e06196c5a9bad98879f7424940c6817c4e4524fb7d38b8553ceed48b9758b8ebaf1abd3600c232c4c8cf7366086f38", "scripts": { diff --git a/crates/git-same-cli/Cargo.toml b/crates/git-same-cli/Cargo.toml index eeaacc2..8e8b6b5 100644 --- a/crates/git-same-cli/Cargo.toml +++ b/crates/git-same-cli/Cargo.toml @@ -40,7 +40,7 @@ tui = ["dep:ratatui", "dep:crossterm"] release-tools = ["dep:clap_complete", "dep:clap_mangen"] [dependencies] -git-same-core = { path = "../git-same-core", version = "=3.2.0" } +git-same-core = { path = "../git-same-core", version = "=3.2.1" } clap = { workspace = true } tokio = { workspace = true } serde = { workspace = true } diff --git a/crates/git-same-cli/src/commands/monitor.rs b/crates/git-same-cli/src/commands/monitor.rs index 3216379..fa8e358 100644 --- a/crates/git-same-cli/src/commands/monitor.rs +++ b/crates/git-same-cli/src/commands/monitor.rs @@ -229,7 +229,23 @@ fn install_agent(args: &ControlArgs, config_override: bool) -> Result { let app_path = args.app_path.as_deref().unwrap_or(Path::new("")); let retained = args.installer_copy.as_deref().unwrap_or(Path::new("")); let status = controller.install_for_cask(&staged, app_path, retained)?; - Ok(report(status)) + Ok(install_agent_report(status, app_path)) +} + +/// Homebrew runs the installer before it moves the app into place, so the +/// monitor cannot start yet. Say when it will, rather than the generic +/// "installed but not running", which reads like a failure in `brew` output. +fn install_agent_report(status: MonitorAgentStatus, app_path: &Path) -> Report { + let app_pending = !monitor_agent::source::app_main_executable(app_path).exists(); + if status.running || status.state != MonitorAgentState::Stopped || !app_pending { + return report(status); + } + Report { + headline: "Monitor installed; it starts when you open Git-Same or at next login" + .to_string(), + status: Some(status), + data: None, + } } /// Persistent Stop on macOS. Elsewhere there is no managed service, so stop diff --git a/crates/git-same-cli/src/commands/monitor_tests.rs b/crates/git-same-cli/src/commands/monitor_tests.rs index cee9785..be2e08e 100644 --- a/crates/git-same-cli/src/commands/monitor_tests.rs +++ b/crates/git-same-cli/src/commands/monitor_tests.rs @@ -54,3 +54,52 @@ fn lock_holder_is_starting_until_it_writes_its_own_status() { .message .starts_with("Monitor is running")); } + +fn stopped_status() -> MonitorAgentStatus { + MonitorAgentStatus { + installed: true, + state: MonitorAgentState::Stopped, + message: "Monitor is installed but not running".to_string(), + ..MonitorAgentStatus::unsupported() + } +} + +#[test] +fn cask_install_before_the_app_is_placed_says_when_the_monitor_starts() { + let dir = tempfile::tempdir().unwrap(); + let app = dir.path().join("Git-Same.app"); + + let report = install_agent_report(stopped_status(), &app); + + assert_eq!( + report.headline, + "Monitor installed; it starts when you open Git-Same or at next login" + ); +} + +#[test] +fn cask_install_with_the_app_in_place_keeps_the_status_message() { + let dir = tempfile::tempdir().unwrap(); + let app = dir.path().join("Git-Same.app"); + let executable = monitor_agent::source::app_main_executable(&app); + std::fs::create_dir_all(executable.parent().unwrap()).unwrap(); + std::fs::write(&executable, b"app").unwrap(); + + let report = install_agent_report(stopped_status(), &app); + + assert_eq!(report.headline, "Monitor is installed but not running"); +} + +#[test] +fn cask_install_with_monitoring_stopped_keeps_the_status_message() { + let dir = tempfile::tempdir().unwrap(); + let status = MonitorAgentStatus { + state: MonitorAgentState::Disabled, + message: "Monitoring is stopped".to_string(), + ..stopped_status() + }; + + let report = install_agent_report(status, &dir.path().join("Git-Same.app")); + + assert_eq!(report.headline, "Monitoring is stopped"); +} diff --git a/crates/git-same-core/src/macos/monitor_agent/controller.rs b/crates/git-same-core/src/macos/monitor_agent/controller.rs index abd3ed6..8a3d268 100644 --- a/crates/git-same-core/src/macos/monitor_agent/controller.rs +++ b/crates/git-same-core/src/macos/monitor_agent/controller.rs @@ -246,10 +246,26 @@ impl Controller { return Err(MonitorAgentError::ForegroundActive { pid: active.pid }); } } + let stopped_before = read_monitor_autostart(&self.paths.config).is_ok_and(|on| !on); + let disabled_before = self.launchd().is_disabled(LABEL)?; set_monitor_autostart(&self.paths.config, true) .map_err(|e| MonitorAgentError::Configuration(e.to_string()))?; self.launchd().enable(LABEL)?; - self.bring_up(Intent::Explicit, restart)?; + if let Err(error) = self.bring_up(Intent::Explicit, restart) { + // Nothing exists to run (the app is missing), which only shows + // once the recorded owner is resolved. Put the persistent Stop + // back so a later login does not load a job that cannot start. + if matches!(error, MonitorAgentError::MissingSource(_)) { + if stopped_before { + set_monitor_autostart(&self.paths.config, false) + .map_err(|e| MonitorAgentError::Configuration(e.to_string()))?; + } + if disabled_before { + self.launchd().disable(LABEL)?; + } + } + return Err(error); + } self.inspect() } @@ -314,6 +330,18 @@ impl Controller { } } + // Re-read: an `Install` above may have just changed the owner. + let program = self.program(InstallRecord::load(&self.paths.install_record)?.as_ref()); + if !program_placed(&program) { + return match intent { + // The app's next launch or the next login starts it. + Intent::Automatic => Ok(()), + Intent::Explicit => Err(MonitorAgentError::MissingSource(format!( + "'{}' does not exist; reinstall or move Git-Same.app back", + program.display() + ))), + }; + } if !launchd.gui_session_available()? { // The plist is in place; launchd starts it at the next login. return Ok(()); @@ -321,14 +349,39 @@ impl Controller { let service = launchd.service(LABEL)?; if !service.loaded { launchd.bootstrap(LABEL, &self.paths.launch_agent)?; + } else if service.pid.is_none() { + self.reload_service()?; } else if restart { launchd.kickstart(LABEL, true)?; - } else if service.pid.is_none() { - launchd.kickstart(LABEL, false)?; } self.confirm_started() } + /// Starts a loaded job that has no process. A fresh bootstrap rather than + /// `kickstart`: a job launchd parked with `EX_CONFIG` (loaded at login + /// while the app was missing) never starts from `kickstart`, which blocks + /// instead. `RunAtLoad` makes the bootstrap start it either way. + fn reload_service(&self) -> Result<()> { + let launchd = self.launchd(); + launchd.bootout(LABEL)?; + // Removal can finish after bootout returns. Bootstrapping into that + // window fails with "Operation already in progress", which the + // wrapper would read as success because the old job is still loaded. + let mut waited = Duration::ZERO; + while launchd.service(LABEL)?.loaded { + if waited >= EXIT_WAIT { + return Err(MonitorAgentError::Launchd { + operation: "bootout".to_string(), + code: None, + detail: format!("{LABEL} was still loaded after bootout"), + }); + } + self.system.sleep(POLL); + waited += POLL; + } + launchd.bootstrap(LABEL, &self.paths.launch_agent) + } + /// launchd accepted the service and reports a process. Does not wait for /// repository scanning: a long first scan is `starting`, not a failure. fn confirm_started(&self) -> Result<()> { @@ -449,16 +502,17 @@ impl Controller { } } self.wait_for_managed_exit()?; - let rendered = self.render_plist( - &staged.source.program(&self.paths.helper), - Some(staged.source.owner_kind), - )?; + let program = staged.source.program(&self.paths.helper); + let rendered = self.render_plist(&program, Some(staged.source.owner_kind))?; self.installer().activate(staged, &rendered)?; let foreground_active = !matches!( self.system.monitor_state(&self.paths.ipc), RuntimeMonitorState::Stopped ); - if start_if_possible && gui && !foreground_active { + // A cask install runs before Homebrew places the app, so the bundle + // executable is usually still missing here; the installation is + // committed and the app's next launch or the next login starts it. + if start_if_possible && gui && !foreground_active && program_placed(&program) { launchd.bootstrap(LABEL, &self.paths.launch_agent)?; self.confirm_started()?; } @@ -730,7 +784,7 @@ impl Controller { let source = source::cask_source(staged_executable, final_app_path); if self.cask_install_is_current(&source)? { - if enabled { + if enabled && program_placed(&source.source_binary) { self.bring_up_installed()?; } } else { @@ -776,7 +830,7 @@ impl Controller { if !service.loaded { launchd.bootstrap(LABEL, &self.paths.launch_agent)?; } else if service.pid.is_none() { - launchd.kickstart(LABEL, false)?; + self.reload_service()?; } self.confirm_started() } @@ -863,6 +917,15 @@ fn program_installed(program: &Path, record: Option<&InstallRecord>) -> bool { } } +/// Whether launchd may be asked to start `program`. launchd parks a job whose +/// program is missing at load (`EX_CONFIG`) and never retries it, even once +/// the file appears, so nothing bootstraps the monitor before its executable +/// is on disk. During a cask install it is not: Homebrew runs the installer +/// before it moves the app into place. +fn program_placed(program: &Path) -> bool { + is_executable(program) +} + /// Whether the installed program still matches what was recorded. Used to /// decide whether an installation needs repairing; see /// [`program_installed`] for why an absent in-place program is not damage. diff --git a/crates/git-same-core/src/macos/monitor_agent/controller_tests.rs b/crates/git-same-core/src/macos/monitor_agent/controller_tests.rs index e5f3c88..67f6bfc 100644 --- a/crates/git-same-core/src/macos/monitor_agent/controller_tests.rs +++ b/crates/git-same-core/src/macos/monitor_agent/controller_tests.rs @@ -106,6 +106,26 @@ impl Env { .is_ok_and(|plist| plist.contains(program.to_str().unwrap())) } + /// Homebrew moving the staged bundle to `app` once the installer script + /// has exited. Returns the placed bundle executable. + fn place_app(&self, staged: &Path, app: &Path) -> PathBuf { + let executable = source::app_main_executable(app); + write_executable(&executable, &std::fs::read(staged_app(staged)).unwrap()); + executable + } + + /// Git-Same.app at `app` launching, as Homebrew's reopen or the user does. + fn launch_app(&self, app: &Path) -> Result { + let executable = source::app_main_executable(app); + self.controller_for(Some(HelperSource { + owner_kind: OwnerKind::App, + owner_path: app.to_path_buf(), + source_binary: executable.clone(), + copy_from: executable, + })) + .ensure_running() + } + /// `(staged installer, final app path, retained service tool)`, matching /// what the cask passes: the installer is the staged bundle's CLI helper, /// while the program that gets installed is the staged bundle's main @@ -263,7 +283,7 @@ fn unloaded_service_is_bootstrapped() { } #[test] -fn loaded_service_without_a_process_is_kickstarted_without_killing() { +fn loaded_service_without_a_process_is_reloaded_not_kickstarted() { let env = env(); env.controller().ensure_running().unwrap(); env.system.with(|s| { @@ -274,10 +294,11 @@ fn loaded_service_without_a_process_is_kickstarted_without_killing() { env.controller().ensure_running().unwrap(); - assert_eq!( - env.system.mutating_calls(), - vec![format!("launchctl kickstart gui/501/{LABEL}")] - ); + let calls = env.system.mutating_calls(); + assert_eq!(calls.len(), 2, "{calls:?}"); + assert_eq!(calls[0], format!("launchctl bootout gui/501/{LABEL}")); + assert!(calls[1].starts_with("launchctl bootstrap gui/501 ")); + assert!(env.system.with(|s| s.active.is_some())); } #[test] @@ -726,8 +747,11 @@ fn staged_app(staged_cli: &Path) -> PathBuf { // ------------------------------------------------------------------ cask +// Homebrew runs the installer script before it moves the app into place, so +// the bundle executable the agent names does not exist yet. launchd would park +// such a job with EX_CONFIG and never start it, which is what broke 3.2.0. #[test] -fn cask_install_starts_monitoring_without_the_app() { +fn cask_install_defers_start_until_the_app_is_placed() { let env = env(); let (staged, app, tool) = env.cask_bundle(); @@ -736,7 +760,11 @@ fn cask_install_starts_monitoring_without_the_app() { .install_for_cask(&staged, &app, &tool) .unwrap(); - assert!(status.running); + assert!(!status.running); + assert!(status.installed); + assert_eq!(status.state, MonitorAgentState::Stopped); + let calls = env.system.mutating_calls(); + assert!(!calls.iter().any(|c| c.contains("bootstrap")), "{calls:?}"); assert_eq!(status.owner_kind, Some(OwnerKind::HomebrewCask)); assert_eq!(status.source.as_deref(), Some(app.to_str().unwrap())); assert_eq!(std::fs::read(&tool).unwrap(), b"cask cli v1"); @@ -787,7 +815,7 @@ fn cask_upgrade_after_a_stop_updates_the_helper_but_stays_stopped() { } #[test] -fn cask_upgrade_while_enabled_starts_the_new_helper() { +fn cask_upgrade_while_enabled_starts_the_new_build_on_the_next_app_launch() { let env = env(); let (staged, app, tool) = env.cask_bundle(); let controller = env.controller_for(None); @@ -798,49 +826,169 @@ fn cask_upgrade_while_enabled_starts_the_new_helper() { write_executable(&staged_app(&staged), b"cask helper v2"); let status = controller.install_for_cask(&staged, &app, &tool).unwrap(); + assert!(!status.running); + let placed = env.place_app(&staged, &app); + let status = env.launch_app(&app).unwrap(); assert!(status.running); - assert_eq!( - std::fs::read(staged_app(&staged)).unwrap(), - b"cask helper v2" - ); + assert_eq!(std::fs::read(placed).unwrap(), b"cask helper v2"); } #[test] -fn app_launch_right_after_a_cask_install_changes_nothing() { +fn app_launch_after_a_cask_install_starts_the_monitor() { let env = env(); let (staged, app, tool) = env.cask_bundle(); env.controller_for(None) .install_for_cask(&staged, &app, &tool) .unwrap(); // Homebrew has moved the bundle into place and reopens the app. - let installed_executable = app.join("Contents/MacOS/git-same-app"); - write_executable(&installed_executable, b"cask helper v1"); - let app_caller = HelperSource { - owner_kind: OwnerKind::App, - owner_path: app.clone(), - source_binary: installed_executable.clone(), - copy_from: installed_executable, - }; - let (pid, stamps) = (env.pid(), env.stamps()); + env.place_app(&staged, &app); + let stamps = env.stamps(); env.system.with(|s| s.calls.clear()); - env.controller_for(Some(app_caller)) - .ensure_running() - .unwrap(); + let status = env.launch_app(&app).unwrap(); - assert_eq!(env.pid(), pid); + assert!(status.running); + // The installation is already current: nothing is rewritten or copied. assert_eq!(env.stamps(), stamps); + let calls = env.system.mutating_calls(); + assert_eq!(calls.len(), 1, "{calls:?}"); + assert!(calls[0].starts_with("launchctl bootstrap gui/501 ")); + + // A second launch finds it healthy and changes nothing. + let pid = env.pid(); + env.system.with(|s| s.calls.clear()); + env.launch_app(&app).unwrap(); + assert_eq!(env.pid(), pid); assert!(env.system.mutating_calls().is_empty()); } +#[test] +fn reinstalling_before_the_app_is_placed_starts_nothing() { + let env = env(); + let (staged, app, tool) = env.cask_bundle(); + let controller = env.controller_for(None); + controller.install_for_cask(&staged, &app, &tool).unwrap(); + + controller.install_for_cask(&staged, &app, &tool).unwrap(); + + let calls = env.system.mutating_calls(); + assert!(!calls.iter().any(|c| c.contains("bootstrap")), "{calls:?}"); + assert!(env.system.with(|s| s.loaded.is_empty())); +} + +#[test] +fn automatic_ensure_with_the_app_missing_leaves_launchd_alone() { + let env = env(); + let (staged, app, tool) = env.cask_bundle(); + env.controller_for(None) + .install_for_cask(&staged, &app, &tool) + .unwrap(); + env.system.with(|s| s.calls.clear()); + + env.controller_for(None).ensure_running().unwrap(); + + assert!(env.system.mutating_calls().is_empty()); +} + +#[test] +fn explicit_start_with_the_app_missing_names_it() { + let env = env(); + let (staged, app, tool) = env.cask_bundle(); + env.controller_for(None) + .install_for_cask(&staged, &app, &tool) + .unwrap(); + env.system.with(|s| s.calls.clear()); + + let error = env.controller_for(None).start().unwrap_err(); + + assert!( + matches!(error, MonitorAgentError::MissingSource(_)), + "{error}" + ); + assert!(error.to_string().contains("git-same-app"), "{error}"); + let calls = env.system.mutating_calls(); + assert!(!calls.iter().any(|c| c.contains("bootstrap")), "{calls:?}"); +} + +#[test] +fn reload_waits_for_launchd_to_finish_removing_the_job() { + let env = env(); + env.controller().ensure_running().unwrap(); + env.system.with(|s| { + s.pids.clear(); + s.active = None; + s.bootout_lingers = 3; + }); + + let status = env.controller().ensure_running().unwrap(); + + assert!(status.running, "{status:?}"); + assert!(!env.system.with(|s| s + .calls + .iter() + .any(|c| c.contains("bootstrap") && c.contains("37")))); +} + +#[test] +fn explicit_start_with_the_app_missing_keeps_a_persistent_stop() { + let env = env(); + // Without a config file only launchd's disable records a Stop. + env.write_config(""); + let (staged, app, tool) = env.cask_bundle(); + let controller = env.controller_for(None); + controller.install_for_cask(&staged, &app, &tool).unwrap(); + controller.stop().unwrap(); + assert!(!read_monitor_autostart(&env.paths.config).unwrap()); + assert!(env.system.with(|s| s.disabled.contains(LABEL))); + + let error = controller.start().unwrap_err(); + + assert!( + matches!(error, MonitorAgentError::MissingSource(_)), + "{error}" + ); + assert!(!read_monitor_autostart(&env.paths.config).unwrap()); + assert!(env.system.with(|s| s.disabled.contains(LABEL))); +} + +// A login while the app was missing leaves launchd holding the job parked +// with EX_CONFIG. kickstart never revives that; a fresh bootstrap does. +#[test] +fn a_job_parked_while_the_app_was_missing_starts_once_it_is_back() { + let env = env(); + let (staged, app, tool) = env.cask_bundle(); + env.controller_for(None) + .install_for_cask(&staged, &app, &tool) + .unwrap(); + env.system.with(|s| { + s.loaded.insert(LABEL.to_string()); + s.parked.insert(LABEL.to_string()); + }); + env.place_app(&staged, &app); + env.system.with(|s| s.calls.clear()); + + let status = env.launch_app(&app).unwrap(); + + assert!(status.running); + let calls = env.system.mutating_calls(); + assert!(!calls.iter().any(|c| c.contains("kickstart")), "{calls:?}"); + assert_eq!(calls[0], format!("launchctl bootout gui/501/{LABEL}")); +} + #[test] fn reinstalling_the_same_cask_keeps_exactly_one_monitor() { let env = env(); let (staged, app, tool) = env.cask_bundle(); let controller = env.controller_for(None); controller.install_for_cask(&staged, &app, &tool).unwrap(); + let placed = env.place_app(&staged, &app); + env.launch_app(&app).unwrap(); let pid = env.pid(); + assert!(pid.is_some()); + // Homebrew moves the installed app aside before the installer runs; the + // running monitor keeps going from the moved bundle. + std::fs::remove_file(placed).unwrap(); controller.install_for_cask(&staged, &app, &tool).unwrap(); @@ -1159,6 +1307,8 @@ fn headless_cask_removal_signals_the_managed_monitor() { env.controller_for(None) .install_for_cask(&staged, &app, &tool) .unwrap(); + env.place_app(&staged, &app); + env.launch_app(&app).unwrap(); let pid = env.system.with(|s| s.active.as_ref().unwrap().pid); env.system.with(|s| s.gui = false); diff --git a/crates/git-same-core/src/macos/monitor_agent/fake.rs b/crates/git-same-core/src/macos/monitor_agent/fake.rs index 88301a8..351df85 100644 --- a/crates/git-same-core/src/macos/monitor_agent/fake.rs +++ b/crates/git-same-core/src/macos/monitor_agent/fake.rs @@ -30,6 +30,17 @@ pub struct FakeState { pub corrupt_copies: bool, /// launchd accepts the job but its process never appears. pub jobs_never_start: bool, + /// `Program` of each bootstrapped plist. + pub programs: HashMap, + /// Jobs whose program was missing when launchd tried to spawn them. Real + /// launchd records `EX_CONFIG` and never retries such a job, even once + /// the file appears; `kickstart` blocks instead of reviving it. Only a + /// bootout clears the state. + pub parked: HashSet, + /// How many `print` queries still report a job loaded after its bootout, + /// modelling launchd finishing the removal asynchronously. + pub bootout_lingers: u32, + lingering: HashMap, /// Scripted `codesign`. `None` keeps the default: every binary is /// unsigned, which is true of test binaries and short-circuits /// `verify_signature` before it can check anything. @@ -168,6 +179,20 @@ fn fail(code: i32, stderr: &str) -> CommandOutput { } } +/// The `Program` path of a rendered plist, XML-unescaped. +fn plist_program(plist: &str) -> Option { + let after = &plist[plist.find("Program")?..]; + let start = after.find("")? + "".len(); + let end = after[start..].find("")? + start; + let value = after[start..end] + .replace("<", "<") + .replace(">", ">") + .replace(""", "\"") + .replace("'", "'") + .replace("&", "&"); + Some(PathBuf::from(value)) +} + fn label_of(target: &str) -> String { target.rsplit('/').next().unwrap_or_default().to_string() } @@ -177,6 +202,14 @@ impl FakeState { if self.jobs_never_start { return; } + if self + .programs + .get(label) + .is_some_and(|program| !program.exists()) + { + self.parked.insert(label.to_string()); + return; + } self.next_pid += 1; let pid = self.next_pid; self.pids.insert(label.to_string(), pid); @@ -201,6 +234,10 @@ impl FakeState { }; } let label = label_of(target); + if let Some(left) = self.lingering.get_mut(&label).filter(|left| **left > 0) { + *left -= 1; + return ok(format!("{target} = {{\n\tstate = not running\n}}\n")); + } if !self.gui || !self.loaded.contains(&label) { return fail(113, "Could not find service"); } @@ -233,9 +270,18 @@ impl FakeState { if self.disabled.contains(&label) { return fail(5, "Bootstrap failed: 5: Input/output error"); } + if self.lingering.get(&label).is_some_and(|left| *left > 0) { + return fail(37, "Bootstrap failed: 37: Operation already in progress"); + } if !self.loaded.insert(label.clone()) { return fail(37, "Operation already in progress"); } + if let Some(program) = std::fs::read_to_string(args[2]) + .ok() + .and_then(|plist| plist_program(&plist)) + { + self.programs.insert(label.clone(), program); + } self.spawn(&label); ok("") } @@ -244,6 +290,11 @@ impl FakeState { if !self.loaded.remove(&label) { return fail(3, "Boot-out failed: 3: No such process"); } + self.parked.remove(&label); + self.programs.remove(&label); + if self.bootout_lingers > 0 { + self.lingering.insert(label.clone(), self.bootout_lingers); + } let pid = self.pids.remove(&label); if self.active.as_ref().map(|a| a.pid) == pid { self.active = None; @@ -256,6 +307,10 @@ impl FakeState { if !self.loaded.contains(&label) { return fail(113, "Could not find service"); } + if self.parked.contains(&label) { + // The real call blocks until our launchctl timeout. + return fail(1, "kickstart timed out: job parked with EX_CONFIG"); + } if restart || !self.pids.contains_key(&label) { self.spawn(&label); } diff --git a/docs/README.md b/docs/README.md index d3acebd..4a8a334 100644 --- a/docs/README.md +++ b/docs/README.md @@ -284,6 +284,8 @@ Finder badges need Full Disk Access. The monitor reads every repository folder, 3. The Finder extension is installed. 4. Enable badges. The app sets the extension election itself; if macOS ignores that, use the Open button to toggle Git-Same Badges in Login Items & Extensions. +After `brew install` or `brew upgrade`, unless you stopped monitoring with `gisa monitor --stop`, the monitor starts the next time you open Git-Same (Homebrew reopens it for you if it was running during the upgrade) or at your next login. Homebrew places the app only after the cask's installer runs, so the installer cannot start it. + One grant covers both the app and the monitor because the LaunchAgent runs the monitor through the app's own executable (`Git-Same.app/Contents/MacOS/git-same-app monitor`). Two things the grant never covers: `gisa` run from a terminal uses the terminal's permissions, and a development build under `target/` is a separate identity that macOS prompts for again. Useful checks: diff --git a/macos/GitSameBadges/Info.plist b/macos/GitSameBadges/Info.plist index a55e641..16e6a58 100644 --- a/macos/GitSameBadges/Info.plist +++ b/macos/GitSameBadges/Info.plist @@ -17,9 +17,9 @@ CFBundlePackageType XPC! CFBundleShortVersionString - 3.2.0 + 3.2.1 CFBundleVersion - 3.2.0 + 3.2.1 NSExtension NSExtensionPointIdentifier diff --git a/toolkit/homebrew/cask.rb.tmpl b/toolkit/homebrew/cask.rb.tmpl index 1a718a5..6a2cefc 100644 --- a/toolkit/homebrew/cask.rb.tmpl +++ b/toolkit/homebrew/cask.rb.tmpl @@ -24,8 +24,13 @@ cask "git-same" do depends_on macos: :ventura app "Git-Same.app" - # Installs the background monitor LaunchAgent and starts it when monitoring - # is enabled. The agent execs the app bundle's own main executable in + # Installs the background monitor LaunchAgent but does not start it: the + # program it names is not on disk yet (see below), and launchd parks a job + # whose program is missing at load (EX_CONFIG) and never retries it. Unless + # the user stopped monitoring (a Stop survives upgrades), the monitor starts + # when Homebrew reopens the app after an upgrade, when the user next opens + # Git-Same, or at the next login. The agent execs the app + # bundle's own main executable in # headless `monitor` mode, not the CLI helper: macOS TCC attributes a # launchd-spawned process to its bundle only when the executable is the # bundle's CFBundleExecutable, so this is what lets one Full Disk Access diff --git a/toolkit/packaging/release-checklist.md b/toolkit/packaging/release-checklist.md index bd3673a..90532ef 100644 --- a/toolkit/packaging/release-checklist.md +++ b/toolkit/packaging/release-checklist.md @@ -68,7 +68,10 @@ After every step check `gisa monitor --status`, `launchctl print gui/$(id -u)/co | Scenario | Required result | |---|---| -| Fresh signed cask install | Monitor active without opening the app. The plist `Program` is `/Git-Same.app/Contents/MacOS/git-same-app` and the managed root holds no helper copy | +| Fresh signed cask install | `brew` exits 0 and reports "Monitor installed; it starts when you open Git-Same or at next login". The agent is written but not loaded; the plist `Program` is `/Git-Same.app/Contents/MacOS/git-same-app` and the managed root holds no helper copy. Opening the app starts the monitor | +| Upgrade with the app running | Homebrew quits the app, installs, and reopens it; the monitor is running within seconds of `brew` returning, with no error | +| Upgrade with the app closed | `brew` exits 0; no monitor until the app is opened or the next login, then exactly one | +| App missing when the agent loads at login | `gisa monitor --start` fails naming the missing `git-same-app` and leaves launchd untouched; once the app is back, opening it starts the monitor (a fresh bootstrap, never `kickstart`) | | Upgrade from the 3.1.2 cask | Legacy helper copy removed, the agent re-rendered onto the bundle executable, no second monitor | | Pre-3.2 agent still installed, app launched once | Startup recovery re-renders the plist onto the bundle executable and restarts the monitor exactly once; `install.json` records the app as owner | | App upgraded while the old monitor keeps running | The app flags the build skew and restarts the installed agent on launch; `status.json` `monitor_version` matches the app afterwards |