From ecb41eaba8c94da67b434223316630ee533f41d9 Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 23 Sep 2026 14:12:52 +0200 Subject: [PATCH 1/4] Defer cask monitor start until the app is placed Homebrew runs the cask's installer script before it moves the app into /Applications, so the bundle executable the 3.2.0 agent names does not exist yet. launchd parks a job whose program is missing at load (EX_CONFIG) and never retries it; verified on real launchd that KeepAlive, PathState, and WatchPaths all leave it parked and kickstart blocks. The installer then failed with exit 8 and Homebrew reverted the upgrade after the old cask had already removed the monitor. Never ask launchd to start a program that is not on disk: the cask installer commits the agent and record without starting it, and the monitor starts when the app launches or at the next login. A loaded job without a process is now reloaded (bootout + bootstrap) instead of kickstarted, the only way to revive a parked job. The fake launchd now models the parked state, which is what hid this: it spawned any job regardless of its program. --- crates/git-same-cli/src/commands/monitor.rs | 18 +- .../src/commands/monitor_tests.rs | 49 ++++++ .../src/macos/monitor_agent/controller.rs | 50 +++++- .../macos/monitor_agent/controller_tests.rs | 161 +++++++++++++++--- .../src/macos/monitor_agent/fake.rs | 41 +++++ 5 files changed, 283 insertions(+), 36 deletions(-) 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..aa295a8 100644 --- a/crates/git-same-core/src/macos/monitor_agent/controller.rs +++ b/crates/git-same-core/src/macos/monitor_agent/controller.rs @@ -314,6 +314,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 +333,24 @@ 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)?; + 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 +471,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 +753,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 +799,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 +886,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..b208a59 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,128 @@ 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:?}"); +} + +// 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 +1266,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..dc66a7d 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,13 @@ 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, /// 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 +175,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 +198,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); @@ -236,6 +265,12 @@ impl FakeState { 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 +279,8 @@ 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); let pid = self.pids.remove(&label); if self.active.as_ref().map(|a| a.pid) == pid { self.active = None; @@ -256,6 +293,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); } From 976efb0c4f5743241ada6733d35f4680548f1662 Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 23 Sep 2026 14:12:52 +0200 Subject: [PATCH 2/4] Bump version to 3.2.1 for the cask install fix --- Cargo.lock | 6 +++--- Cargo.toml | 2 +- crates/git-same-app/tauri.conf.json | 2 +- crates/git-same-app/ui/package.json | 2 +- crates/git-same-cli/Cargo.toml | 2 +- macos/GitSameBadges/Info.plist | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) 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/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 From 88dd8294c16c643c137f1ef185740cd8e2b3c75e Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 23 Sep 2026 14:12:52 +0200 Subject: [PATCH 3/4] Document deferred monitor start after cask install The cask comment, README, and acceptance matrix still promised a monitor running straight after `brew install`. State when it actually starts (app launch, Homebrew's reopen after an upgrade, or next login) and why, and add checklist rows for upgrading with the app running or closed and for an agent loaded at login while the app was missing. --- docs/README.md | 2 ++ toolkit/homebrew/cask.rb.tmpl | 8 ++++++-- toolkit/packaging/release-checklist.md | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/README.md b/docs/README.md index d3acebd..2038746 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`, 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/toolkit/homebrew/cask.rb.tmpl b/toolkit/homebrew/cask.rb.tmpl index 1a718a5..b7ba5d1 100644 --- a/toolkit/homebrew/cask.rb.tmpl +++ b/toolkit/homebrew/cask.rb.tmpl @@ -24,8 +24,12 @@ 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. 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 | From f7516f9d82ab6e6b3087cfe90de4cadcf4d2f55e Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 23 Sep 2026 14:29:13 +0200 Subject: [PATCH 4/4] Keep Stop on failed start and await job removal Address the PR #28 review. An explicit start with the app missing re-enabled autostart and the launchd service before the new guard refused, undoing a persistent Stop so a later login loaded a job that cannot start; restore both when nothing exists to run. Wait for launchd to finish removing the job between bootout and bootstrap in reload_service, since a bootstrap into that window fails with 37 and the wrapper read the still-loaded old job as success. Qualify the README and cask comment: a Stop survives upgrades, so the monitor only starts on the next launch when monitoring is enabled. --- .../src/macos/monitor_agent/controller.rs | 33 ++++++++++++++- .../macos/monitor_agent/controller_tests.rs | 41 +++++++++++++++++++ .../src/macos/monitor_agent/fake.rs | 14 +++++++ docs/README.md | 2 +- toolkit/homebrew/cask.rb.tmpl | 7 ++-- 5 files changed, 92 insertions(+), 5 deletions(-) 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 aa295a8..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() } @@ -348,6 +364,21 @@ impl Controller { 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) } 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 b208a59..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 @@ -911,6 +911,47 @@ fn explicit_start_with_the_app_missing_names_it() { 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] 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 dc66a7d..351df85 100644 --- a/crates/git-same-core/src/macos/monitor_agent/fake.rs +++ b/crates/git-same-core/src/macos/monitor_agent/fake.rs @@ -37,6 +37,10 @@ pub struct FakeState { /// 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. @@ -230,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"); } @@ -262,6 +270,9 @@ 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"); } @@ -281,6 +292,9 @@ impl FakeState { } 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; diff --git a/docs/README.md b/docs/README.md index 2038746..4a8a334 100644 --- a/docs/README.md +++ b/docs/README.md @@ -284,7 +284,7 @@ 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`, 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. +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. diff --git a/toolkit/homebrew/cask.rb.tmpl b/toolkit/homebrew/cask.rb.tmpl index b7ba5d1..6a2cefc 100644 --- a/toolkit/homebrew/cask.rb.tmpl +++ b/toolkit/homebrew/cask.rb.tmpl @@ -26,9 +26,10 @@ cask "git-same" do app "Git-Same.app" # 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. 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 + # 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