fix: clean up orphan LaunchAgent plist on bootstrap failure#15619
fix: clean up orphan LaunchAgent plist on bootstrap failure#15619superlowburn wants to merge 2 commits intoopenclaw:mainfrom
Conversation
…#14315) When running interactive onboarding, if the user cancels/denies the macOS security prompt during `launchctl bootstrap`, the plist file was left on disk and the daemon could remain partially loaded. This caused ghost processes to run alongside the intended Docker deployment. Changes: - installLaunchAgent now cleans up the plist file if bootstrap fails - Added bootout attempt to unload any partially loaded service - Added test to verify plist cleanup on bootstrap failure Fixes openclaw#14315 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Create a failing launchctl stub (fails on bootstrap command) | ||
| const stubPath = path.join(binDir, "launchctl"); | ||
| const stubScript = |
There was a problem hiding this comment.
Windows stub won’t execute
In the new bootstrap-failure test, the win32 branch writes a batch script to binDir/launchctl (no .cmd/.bat). But execLaunchctl runs execFileAsync('launchctl', { shell: true }) on Windows, which typically resolves launchctl.cmd/.bat via PATH and won’t execute an extensionless file. This makes the test non-hermetic (it may call a real launchctl/fail) on Windows; it should create a launchctl.cmd wrapper (like writeLaunchctlStub does) or otherwise ensure the stub is what gets executed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/daemon/launchd.test.ts
Line: 243:245
Comment:
**Windows stub won’t execute**
In the new bootstrap-failure test, the win32 branch writes a batch script to `binDir/launchctl` (no `.cmd`/`.bat`). But `execLaunchctl` runs `execFileAsync('launchctl', { shell: true })` on Windows, which typically resolves `launchctl.cmd`/`.bat` via `PATH` and won’t execute an extensionless file. This makes the test non-hermetic (it may call a real `launchctl`/fail) on Windows; it should create a `launchctl.cmd` wrapper (like `writeLaunchctlStub` does) or otherwise ensure the stub is what gets executed.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 052b7d5. The stub path now uses launchctl.cmd on Windows (matching the pattern in writeLaunchctlStub which creates both a .js file and a .cmd wrapper).
| // Clean up the plist file since bootstrap failed | ||
| await execLaunchctl(["bootout", domain, plistPath]).catch(() => {}); | ||
| await fs.unlink(plistPath).catch(() => {}); |
There was a problem hiding this comment.
Dead .catch() on cleanup
execLaunchctl never rejects (it catches internally and returns { code, stdout, stderr } even on failure), so await execLaunchctl([...]).catch(() => {}) is dead code and doesn’t change behavior. This is misleading in a failure/cleanup path; either drop the .catch or adjust execLaunchctl/cleanup to use a real throwing call if you intend to ignore exceptions.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/daemon/launchd.ts
Line: 439:441
Comment:
**Dead `.catch()` on cleanup**
`execLaunchctl` never rejects (it catches internally and returns `{ code, stdout, stderr }` even on failure), so `await execLaunchctl([...]).catch(() => {})` is dead code and doesn’t change behavior. This is misleading in a failure/cleanup path; either drop the `.catch` or adjust `execLaunchctl`/cleanup to use a real throwing call if you intend to ignore exceptions.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 052b7d5. Removed the dead .catch(() => {}) — execLaunchctl catches internally and returns { code, stdout, stderr } so it never rejects. The .catch(() => {}) on fs.unlink below is kept since that CAN reject.
- Remove dead `.catch(() => {})` on execLaunchctl bootout (it never rejects)
- Add `.cmd` extension for Windows launchctl stub in bootstrap-failure test
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
openclaw-barnacle
bot
added
stale
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
1 participant
Summary
Fixes #14315
launchctl bootstrapfails during onboarding (e.g., user denies macOS security prompt), the plist file was left on disk with no cleanup pathlaunchctl bootoutand removes the plist file before throwing the errorChanges
src/daemon/launchd.ts: 3 lines added — bootout + unlink on bootstrap failuresrc/daemon/launchd.test.ts: New test verifying plist cleanup on bootstrap failureTest plan
🤖 Generated with Claude Code
Greptile Overview
Greptile Summary
This PR updates the macOS LaunchAgent install flow to clean up on
launchctl bootstrapfailure: it attempts alaunchctl bootoutand unlinks the generated plist before rethrowing the bootstrap error. It also adds a regression test that stubslaunchctlto fail specifically onbootstrapand asserts the plist is removed afterward.The change is localized to
src/daemon/launchd.ts’sinstallLaunchAgentpath and extends the existing launchctl-stubbing test harness insrc/daemon/launchd.test.ts.Confidence Score: 4/5
launchctlon Windows due to writing an extensionless batch file while execution relies onshell: truePATH resolution. Also noted a misleading dead.catch()in the cleanup path.Last reviewed commit: 20dabad