test(macos): inject plist removal failures as root - #3160
Open
tang-vu wants to merge 1 commit into
Open
Conversation
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this matters
The macOS uninstall contract is not portable to root-run release gates. Its fourth scenario tries to force
rmto fail by changing the fake~/Library/LaunchAgentsdirectory to mode0555. A uid-0 runner bypasses that directory permission, removes the plist successfully, and makes the test fail withfailed plist removal must produce a warningeven though the production uninstaller behaved correctly.This was reproduced in the clean Linux integration worktree after the first two batch fixes. Root-run containers are a normal way to execute
make test, so this false failure prevents the release gate from reaching later suites and obscures real regressions.The fixture now supplies an
rmshim that rejects exactly the designated plist path and delegates every other removal to/bin/rm. The failure is deterministic for root and non-root runners, while the existing public uninstall script still has to observe the surviving plist and emit its warning.Behavioral invariant: the uninstall test must exercise an actual plist-removal failure independent of runner privilege, while allowing all unrelated fixture and trap cleanup to proceed normally.
Overlap check
Searched open and closed PRs for
macOS uninstall plist removal warning,plist removal root test chmod, and the exact changed test file. No open PR covers the root-run fixture defect. Merged/closed #1886 introduced the production LaunchAgent cleanup and original hermetic test; closed #2074 later consolidated label coverage. This PR preserves those production semantics and only repairs how their failure boundary is injected.Changed file searched:
ods/tests/test-macos-uninstall-launchagents.sh.Regression test
The changed test executes the real
ods-uninstall.shwith a fake Darwin environment, fake HOME, and executable process/lifecycle shims. Its removal-failure scenario now proves that a refusedcom.ods.opencode-web.plistremoval warns without aborting uninstall even when the suite itself runs as uid 0.Validation
bash tests/test-macos-uninstall-launchagents.sh? all 5 scenarios passed as uid 0bash tests/test-uninstall-compose-flags.sh? all 6 checks passedbash -n tests/test-macos-uninstall-launchagents.sh? passedbash -n ods-uninstall.sh? passedgit diff --check? passedNo real macOS files or LaunchAgents were mutated; the suite is intentionally hermetic. Static/executable coverage proves cleanup orchestration and warning behavior, not a live launchd uninstall.
Tradeoffs and rollback
The shim is scoped by an exact absolute path and delegates every other
rminvocation, including final fixture cleanup. This is more explicit than relying on Unix discretionary permissions whose result changes with uid/capabilities. Rollback changes no production behavior but restores a known false failure on root-run gates.Batch compatibility
This PR was validated on synthetic integration head
9af795fc, which applies #3158 through #3167 in numeric order onupstream/main(6ff9b4fc). Combinedmake lint,make test,make smoke,make simulate, and all 418 BATS cases passed (one root-specific permission assertion skipped by design).Recommended merge order: #3158 ? #3159 ? #3160 ? #3161 ? #3162 ? #3163 ? #3164 ? #3165 ? #3166 ? #3167. The only manual reconciliation observed was the adjacent Makefile test insertion shared by #3164 and #3166; retain both
test-unix-restart-recreate-env.shandtest-chat-error-exit-parity.shlines. Production code merged automatically across the full batch.