diff --git a/devlog/_plan/260911_hub_single_port/010_launchd_repair.md b/devlog/_plan/260911_hub_single_port/010_launchd_repair.md new file mode 100644 index 0000000000..15d88e421d --- /dev/null +++ b/devlog/_plan/260911_hub_single_port/010_launchd_repair.md @@ -0,0 +1,421 @@ +# PR1 — macOS launchd repair/status (issue #4236 defects 1 & 2) + +Branch `codex/260911-l4-launchd-repair`, based on `dev` (`babb76449`). First of the four-PR +hub single-port stack; the others target this branch's head in turn. + +Scope: `src/service.ts` macOS path only, plus the two shared test-safety guards the work +uncovered. Defects 3 and 4 from the issue (secondary-port misdiagnosis, `ocx status` fence +comparison) are deliberately left to PR2, which owns the loopback listener. + +Three rounds. The first is the three commits below; the second folds in a review of them, and +every fix it carries is marked in place; the third (section E) makes `ocx service restart` +actually restart, which the second round's no-op had quietly turned into a no-op of its own. In short: the new protocol was still asking the OLD +two-state `launchdJobMatchesPlist` at both decision points (so an unreadable `launchctl print` +still evicted a healthy hub, twice), the no-op pre-check compared whole-file bytes while +`buildPlist` bakes the repairing process's `PATH`, the comment justifying `kickstart -k` was +wrong about launchd re-reading the plist, and every mutating verb still addressed `gui/` +alone while the new probe reports `user/` too. + +## What shipped + +### A — `installLaunchd()` (repair must not be an outage) + +1. **No-op pre-check, on the TRI-STATE probe.** The plist is rendered BEFORE anything is + written. If the rendered bytes equal the on-disk bytes, the data-token file is unchanged, + and `probeLaunchdLoadState()` answers `loaded-current`, the function re-asserts 0600 on the + plist, refreshes install state, logs + `service is already loaded from the current plist; nothing to do.` and returns. launchd is + not touched at all. This is the headline fix: a repair of a healthy hub used to evict it + unconditionally. + + Two things the first round got wrong here, both found in review: + + - It asked `launchdJobMatchesPlist`, which reports `loaded:false` for EVERY non-zero + `launchctl print` — EPERM from a non-Aqua ssh/cron context, an unspawnable launchctl, an + undocumented status. On a healthy serving hub that read as "not loaded", so the pre-check + evicted, the verification failed the same way, the rollback evicted again and the error + ended with "IS NOT RUNNING" about a job that was up. Both checks now go through + `probeLaunchdLoadState`, and `unknown` refuses to touch launchd at all (item 9). + - It compared whole-file bytes, while `buildPlist` bakes `process.env.PATH` from whichever + process is repairing. A tray helper, `ocx update`'s child or an ssh session carries a + different PATH, so the pre-check missed and the healthy hub was evicted *and* had its + PATH narrowed. `reusePreviousPlistPathVariable()` now puts the installed PATH back when + PATH is the ONLY difference and the live job runs the exec line this install baked — the + two files then compare equal on their own terms, and the PATH the service already runs + with survives. Anything else differing means a real rewrite, PATH included. +2. **Backup + rollback.** The previous plist bytes are held in memory and copied to + `.prev` before the overwrite. On terminal failure the bytes go back, a + bootout/settle/bootstrap tries to re-register them, and the thrown error states whether + that worked. +3. **`bootstrap gui/$uid ` replaces `load -w`.** `bootout` was already + domain-explicit; `load` acts on the CALLER's bootstrap domain, so from ssh/cron/another + bootstrap context the old pair deleted the gui-domain job and registered nothing. +4. **Bounded settle after an eviction that evicted something** — up to 5 × 200 ms while + `launchctl print ` still answers 0, the launchd twin of the Windows + `SCHEDULER_SETTLE_DELAYS_MS` idea. `bootout` is asynchronous, so the old back-to-back + retry raced the same exiting job twice and added nothing. A `bootout` that exited 3 is not + settled: nothing is exiting to wait for. +5. **Success is `probeLaunchdLoadState()` answering `loaded-current`, never stderr.** It is + asked against the command this install actually baked, and `writeServiceInstallState` + runs only after it agrees. Stderr regexes are advisory routing signals now. +6. **One retry, routed by the failure.** Exit 5 / `Bootstrap failed` → `kickstart -k` (only + when the rendered bytes are already on disk, see item 10), then `enable` + a second + bootout/bootstrap (see the launchctl findings below). Exit 0 with a disagreeing probe → + one more bootout/bootstrap, because that is the silent no-op. Any other failure (malformed + plist, EPERM) throws immediately so the real stderr reaches the operator undelayed — the + property the previous code had and kept. A probe that answered `unknown` is NOT retried: a + retry is another eviction. +7. **Error text names what the probe actually found.** `not-loaded`: the job was evicted from + `gui/` and is not running (or the previous plist was restored and re-bootstrapped). + `loaded-stale`: it *is* loaded, from a different command than the plist just written — + telling that operator "nothing is listening" sends them to fix the wrong thing. Both name + `launchctl bootstrap gui/ ` as the remedy, plus `launchctl print` and + `launchctl print-disabled` to inspect. +8. **`stableLauncherEntry()` prefers the recorded launcher** when it is still an absolute + executable file, falling back to the PATH walk otherwise (defect 1g). A repair from a + context without `ocx` on PATH no longer rewrites a working launcher-form plist into the + version-pinned Bun + CLI pair. + + **This is not a macOS-only change.** `installSystemd` resolves the same function, so a + Linux repair from a PATH-less context now keeps the `ExecStart` the unit already has. The + failure it prevents there is milder (systemd reloads and restarts; it never evicts into + nothing), but the silent rewrite was identical, so the behaviour is deliberately shared + rather than branched. `tests/service/service.test.ts` covers the systemd side directly. +9. **An unverifiable launchd state refuses to act.** `unknown` from the pre-check throws + before a single file is written or a single verb is run, saying the job may be RUNNING and + naming `launchctl print gui//