fix: W5 re-enabled a login item the user had turned off (v1.44 follow-up) - #391
Merged
Merged
Conversation
…-up) I shipped the same bug class as #382, one release later. `enableLaunchAtLoginAtFirstValueIfNeeded` asked `SMAppService.mainApp.status == .enabled`. That single comparison collapses two different worlds: .notRegistered — never registered. Fine to enable. .requiresApproval — registered, and the user then switched it OFF in System Settings. Absolutely not fine to enable. Both answer `false`, so the second one sailed through every remaining guard. `userTouchedToggle` cannot save it. That key is new in v1.44, so NOBODY upgrading from v1.43 has it — which is exactly the shape of #382: a newly added flag cannot speak for the install base that predates it. The state has to be read from the system instead of from a marker we only just started writing. Concrete failure: a v1.43 user who turned CLI Pulse off in System Settings → Login Items upgrades, opens the app, a collector returns numbers, and the app silently turns itself back on. My own comment three lines above called that "how an app earns a one-star review". `FanDaemonInstaller.state()` in this same package has distinguished all four `SMAppService.Status` cases since v1.39. I had the precedent in the repo and used a bare `== .enabled` anyway. Fixed by giving `decide` a three-state `LoginItemState` and a new `.skipAndRememberUserChoice` outcome, which also persists `userTouchedToggleKey` so the answer settles instead of being re-derived from the system on every refresh pass. Mutation-verified: restoring the naive predicate reddens exactly `testAUserWhoDisabledItInSystemSettingsIsNotOverridden` and leaves the other 14 green. Note the remaining blind spot, since it is a real limit rather than an oversight: a user who toggled it on and then off *inside the app* on v1.43 lands on `.notRegistered`, indistinguishable from never having touched it. The toggle is off by default, so reaching that state takes two deliberate trips into Settings — a much smaller population than the System Settings case, and undetectable without a marker that release never wrote. CLIPulseCore 2274 passed / 0 failed (default), 2309 / 0 (-DDEVID_BUILD); macOS + iOS + watchOS all BUILD SUCCEEDED. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
I shipped the same bug class as #382, one release later
enableLaunchAtLoginAtFirstValueIfNeededaskedSMAppService.mainApp.status == .enabled. That single comparison collapses two different worlds:.notRegistered.requiresApprovalBoth answer
false, so the second sailed through every remaining guard.userTouchedTogglecannot save it. That key is new in v1.44, so nobody upgrading from v1.43 has it — exactly the shape of #382: a newly added flag cannot speak for the install base that predates it. The state has to be read from the system, not from a marker we only just started writing.Concrete failure: a v1.43 user who turned CLI Pulse off in System Settings → Login Items upgrades, opens the app, a collector returns numbers, and the app silently turns itself back on.
My own comment three lines above called that "how an app earns a one-star review".
FanDaemonInstaller.state()in this same package has distinguished all fourSMAppService.Statuscases since v1.39. The precedent was in the repo and I used a bare== .enabledanyway.Fix
decidenow takes a three-stateLoginItemStateand gained.skipAndRememberUserChoice, which also persistsuserTouchedToggleKey— so the answer settles instead of being re-derived from the system on every refresh pass.Mutation-verified: restoring the naive predicate reddens exactly
testAUserWhoDisabledItInSystemSettingsIsNotOverriddenand leaves the other 14 green.Remaining blind spot (a real limit, not an oversight)
A user who toggled it on and then off inside the app on v1.43 lands on
.notRegistered, indistinguishable from never having touched it. The toggle is off by default, so reaching that state takes two deliberate trips into Settings — a much smaller population than the System Settings case, and undetectable without a marker that release never wrote.Verification
-DDEVID_BUILD🤖 Generated with Claude Code