Skip to content

Fix daemon reinstall race with a still-loaded LaunchAgent - #147

Merged
philcunliffe merged 1 commit into
masterfrom
fix/issue-128
Jun 25, 2026
Merged

Fix daemon reinstall race with a still-loaded LaunchAgent#147
philcunliffe merged 1 commit into
masterfrom
fix/issue-128

Conversation

@philcunliffe

@philcunliffephilcunliffe commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What

installLaunchAgent (macOS daemon installer) now closes the bootout→bootstrap race that made hyp daemon install fail when reinstalling over a still-loaded LaunchAgent.

Why

launchctl bootout is asynchronous — launchd may still be tearing the service down after the command returns. Bootstrapping into that half-removed state fails with Bootstrap failed: 5: Input/output error. This is issue #128.

How

  • After booting out an already-loaded agent, poll launchctl print until launchd has released the label (bounded, ~3s ceiling) before writing the new plist and bootstrapping.
  • Retry the transient EIO (error 5) on bootstrap a bounded number of times (3), re-waiting for release between attempts.
  • Genuine load/config errors are not retried — any non-EIO failure surfaces immediately as a LaunchAgentError, exactly as before.
  • The launchctl runner was already injectable; added a test-only sleep seam so the regression test runs without real waits.
  • Documented the decision in LLP 0017#reinstall-waits-for-launchd-release and annotated the installer with a matching @ref.

Verified

  • New regression test test/core/daemon-launchagent-race.test.js drives a fake launchctl with scripted result queues. 3 of its 4 cases (wait-before-bootstrap, EIO-retry-succeeds, bounded-retry-then-throw) fail on master and pass with this fix; the 4th (genuine error fails fast) passes both before and after, guarding against over-eager retry.
  • Full npm test green: 1386 pass, 1 skipped, 0 fail.
  • @ref annotations in src/core/daemon/macos.js resolve cleanly against the LLP corpus.

Fixes#128

launchctl bootout is asynchronous: launchd may still be tearing the
service down after the command returns, so reinstalling over a live agent
can bootstrap into a half-removed state and fail with
"Bootstrap failed: 5: Input/output error".
installLaunchAgent now polls `launchctl print` until launchd has released
the label after bootout, and retries the transient EIO (error 5) on
bootstrap a bounded number of times. Genuine load/config errors are not
retried and surface immediately as before. The launchctl runner is
already injectable; the test-only `sleep` seam keeps the regression test
fast. Documents the decision in LLP 0017 and annotates the code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

🤖 neutral dual-review — verdict: approve

Risk: Low — isolated, well-tested change to a single macOS daemon-install helper (installLaunchAgent); behind injectable launchctl/sleep seams, no other call sites touched. Full suite green (1386 pass / 0 fail).

Codex key points (gpt-5.5, ran): No findings. Fix is correct; regression test is non-vacuous — by inspection three of the four tests fail on origin/master (old code prints once before bootstrap, never retries transient EIO, and bootstraps only once on persistent EIO). The fake asserts the real contract at the LaunchctlAdapter boundary, not implementation details. Verified node --test + typecheck + lint + full npm test pass. VERDICT: approve.

Claude key points: Fix is correct and matches the issue/LLP 0017 intent: waitUntilUnloaded polls launchctl print after bootout (bounded ~3s) to close the bootout→bootstrap race; isTransientBootstrapError retries only EIO (error 5) with a hard cap of 1 + 3 attempts; genuine non-EIO failures fall straight out of the retry loop and throw. No infinite-loop or unbounded-wait risk. Style conforms (no semicolons, JSDoc not TS, type imports via top-level @import — no inline import()).

Test non-vacuousness — empirically verified: I ran the new test against the unpatched origin/mastermacos.js: 3 of 4 tests FAIL (reinstall-waits-for-release, retry-transient-EIO, bounded-persistent-EIO) and only the fail-fast guard passes. The assertions genuinely depend on the production fix. Genuine, not tautological.

Findings:

  • Severity: minor — waitUntilUnloaded silently returns after exhausting its 30-attempt bound with no log/diagnostic. Given the repo's log-driven-development emphasis, a debug log on "gave up waiting for launchd release" would aid field diagnosis. Non-blocking.
  • Severity: nit — the 4th test (a genuine non-transient bootstrap failure fails fast) passes on both patched and unpatched code, so it doesn't itself reproduce the bug; it's a valid guard for the fast-fail property of the new retry loop, just not a regression witness. Fine as-is.
  • Severity: nit — isTransientBootstrapError checks both exitCode === 5 and a stderr regex; the \b5:\s*Input\/output alternative is somewhat redundant with the exitCode check, but harmless belt-and-suspenders.

No blocker or major issues from either reviewer → approve. Left as draft (ready-hold handled by next tick).

@philcunliffe
philcunliffe marked this pull request as ready for review June 25, 2026 21:03
@philcunliffe
philcunliffe merged commit 26bfb88 into masterJun 25, 2026
6 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-128 branch June 25, 2026 21:03
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Daemon reinstall races a still-loaded LaunchAgent

1 participant

@philcunliffe