fix(music): stop relaunching Music.app by addressing it via pid - #392
Conversation
An Apple event addressed by bundle identifier is auto-launched by LaunchServices, so `SBApplication(bundleIdentifier:)` and a bare `tell application "Music"` both start Music when it is not running. `LSLaunchFlags` has no opt-out. Checking "is it running?" before the send only narrows the window: the user can quit Music in the gap and the event drags it straight back. That is why this shipped twice (#203, #273) and returned both times. Address Music by pid instead, which cannot launch anything: - `AppleMusicSource` resolves `MusicProcess.pid` and uses `SBApplication(processIdentifier:)`. That initializer does NOT return nil for an unresolvable pid as its header claims; it returns a non-KVC-compliant object whose `value(forKey:)` raises `NSUnknownKeyException`, which Swift cannot catch. The read is therefore gated on `isRunning` + `responds(to:)`, both verified false for an unresolved target and true for a live one. - `AppleMusicController.timeoutWrapped` wraps every script in `if application "Music" is running`, which AppleScript answers without launching, and raises -600 when closed so callers take their existing no-information path. - New `Core/MusicProcess.swift` replaces three duplicated is-running checks and documents the invariant. A SwiftLint custom rule bans `SBApplication(bundleIdentifier:` so this cannot regress a third time. Also surface the state the fix exposes: the now-playing card said "Nothing playing right now" whether Music was closed or idle. It now distinguishes closed ("Apple Music isn't open", with an Open Music button), idle, and tracking-off, with the copy rules extracted as pure functions and tested. Settings sidebar: clearing the NSSplitView autosave was not enough because the stale 148pt width had already been restored by the time it ran, so the divider is re-pinned to the token from `SettingsWindowConfigurator`, which reaches the real split view through the window. Tooling, all found while verifying the above: - Local Debug builds sign with a stable Apple Development identity. Ad-hoc signatures derive from the binary hash, so every rebuild looked like a new app and invalidated the Keychain ACL and TCC grant, prompting for a password on each launch. CI keeps ad-hoc signing. - `check-test-results.sh` summed nested `Executed N tests` lines (18 reported as 54), its failure grep matched none of xcodebuild's real formats, and it ignored Swift Testing entirely, so a failing `@Test` reported 0 failures and exited 0. The suite is 1361 tests, not the 937 the gate could see. - `WidgetHTTPServiceTests` stopped pinning ports 59995/59993 and uses the existing `startBoundService` walker, so a lingering socket or a running WolfWave no longer fails the suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The new `sb_application_bundle_identifier` rule matched its own documentation: `MusicProcess.swift` and `AppleMusicSource.swift` both name `SBApplication(bundleIdentifier:` in comments explaining why not to use it, and the rule fired on all three at error severity. A rule that forbids naming the thing it forbids is unusable, so exclude the comment, doccomment, and string syntax kinds. Code-level uses still fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Why
WolfWave reopened Apple Music after the user quit it, and opened it unprompted while running. This shipped as fixed twice before, in #203 and #273, and came back both times.
Both previous fixes had the same shape: check
NSRunningApplication…isEmpty, then send the Apple event. That is a check-then-send race, not a fix.Root cause
An Apple event addressed by bundle identifier is auto-launched by LaunchServices. Verified live on macOS 26:
osascript -e 'application "Music" is running'→false, Music stayed closed.osascript -e 'tell application "Music" to get player state'→ returnedstoppedand launched Music.SBApplication.h:203-206states it outright: it "launches the application only when it's necessary to send it an event."LSLaunchFlags(LSOpen.h:63-71) has no don't-launch flag, solaunchFlagscannot suppress it. Any guard placed before the send only narrows the window.Fix
Address Music by pid, which cannot launch anything.
AppleMusicSourceresolvesMusicProcess.pid→SBApplication(processIdentifier:).AppleMusicController.timeoutWrappedgates every script onapplication "Music" is running(evaluated without launching), raising-600when closed so callers take their existing no-information path.Core/MusicProcess.swiftreplaces three duplicated is-running checks and carries the rationale.SBApplication(bundleIdentifier:so it cannot regress a third time.Trap worth knowing
SBApplication(processIdentifier:)does not return nil for an unresolvable pid, despite its header saying so. It returns a non-KVC-compliant object whosevalue(forKey:)raisesNSUnknownKeyException— uncatchable in Swift. Since the whole point is handling Music quitting mid-flight, the naive version traded a relaunch for a crash. The read is gated onisRunning+responds(to:), both empirically false for an unresolved target and true for a live one.Also in here
AppleMusicSource(notRunningvsnotPlaying) and was dropped at the delegate. Now three distinct states, with an Open Music button. Copy rules extracted as pure functions and tested.NSSplitViewautosave was not enough: the stale 148pt width had already been restored. The divider is re-pinned to the token fromSettingsWindowConfigurator, which reaches the real split view through the window. Verified by screenshot on fresh launch and same-session reopen.Tooling fixes found while verifying
check-test-results.shwas reporting fiction. It summed nestedExecuted N testslines (an 18-test suite reported as 54), its failure grep matched none of the formats xcodebuild emits (so a red run printed a count and no detail), and it ignored Swift Testing entirely — a failing@Testreported 0 failures and exited 0. The suite is 1361 tests, not the 937 the gate could see.WidgetHTTPServiceTestsstopped pinning ports 59995/59993 and uses the file's existingstartBoundServicewalker, so a lingering socket or a running WolfWave no longer fails the suite.Verification
make test→ 1361 tests, 0 failures (424 via Swift Testing).make lint-headersandmake lint-crash-safetyclean.Not visually verified: the "Apple Music isn't open" card. Synthetic clicks do not reach this app's windows (What's New and onboarding both ignored repeated clicks while frontmost), so it is covered by unit tests rather than a screenshot. That hit-testing behaviour may itself be worth investigating.
🤖 Generated with Claude Code