feat: multi-source music support (v1.2.0)#13
Merged
nathanialhenniges merged 5 commits intomainfrom Apr 3, 2026
Merged
Conversation
- Extract PlaybackSource protocol + PlaybackSourceMode enum - Refactor MusicPlaybackMonitor → AppleMusicSource (same logic, new abstraction) - Add SystemNowPlayingSource using private MediaRemote framework (dlopen/dlsym) captures playback from Spotify, browsers, and any system Now Playing app - Add PlaybackSourceManager to switch sources and forward delegate callbacks - Add Music Source segmented picker to MusicMonitorSettingsView (Apple Events warning hidden when System mode is selected) - Update AppDelegate to use PlaybackSourceManager; Apple Music remains default - Add AppConstants for playbackSourceMode, PlaybackSourceModeChanged, SystemNowPlaying - Add PlaybackSourceManagerTests and SystemNowPlayingSourceTests (693 tests, 0 failures) - Add FEATURE_IDEAS.md with 8 future feature ideas - Sync docs changelog with v1.0.2, v1.1.0, and v1.2.0 entries - Bump version to 1.2.0, build number to 5 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (13)
WalkthroughReplaces the single MusicPlaybackMonitor with a PlaybackSource abstraction, adds AppleMusicSource and SystemNowPlayingSource, introduces PlaybackSourceManager to select/persist active source, updates settings UI to choose source, bumps app version to 1.2.0, and adds related tests and docs. Changes
Sequence DiagramssequenceDiagram
participant App as WolfWaveApp
participant Manager as PlaybackSourceManager
participant UDefaults as UserDefaults
participant AM as AppleMusicSource
participant SNP as SystemNowPlayingSource
participant Delegate as PlaybackSourceDelegate
App->>Manager: startTracking()
Manager->>UDefaults: read playbackSourceMode
alt mode == appleMusic
Manager->>AM: instantiate & set delegate
Manager->>AM: startTracking()
AM->>AM: subscribe to com.apple.Music.playerInfo
else mode == systemNowPlaying
Manager->>SNP: instantiate & set delegate
Manager->>SNP: startTracking()
SNP->>SNP: dlopen MediaRemote & register notifications
end
Manager->>Manager: store activeSource, set isTracking=true
AM-->>Delegate: playbackSource(_:didUpdateTrack:...)
SNP-->>Delegate: playbackSource(_:didUpdateTrack:...)
sequenceDiagram
participant UI as MusicMonitorSettingsView
participant Manager as PlaybackSourceManager
participant Current as ActiveSource
participant New as NewSource
participant UDefaults as UserDefaults
participant Notif as NotificationCenter
UI->>Manager: switchMode(newMode)
Manager->>Current: stopTracking() (if exists)
Current->>Current: remove observers, cancel timers
Manager->>UDefaults: persist newMode
Manager->>Notif: post PlaybackSourceModeChanged (userInfo: ["mode": newMode])
alt wasTracking == true
Manager->>New: instantiate & set delegate
Manager->>New: startTracking()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift (1)
53-86: Add accessibility attributes to the new Music Source picker section.As per coding guidelines, all UI elements should have
accessibilityLabelandaccessibilityIdentifierfor UI testing. The new picker and info row lack these.♿ Suggested accessibility additions
Picker("", selection: $playbackSourceMode) { Text("Apple Music").tag("appleMusic") Text("Any App (System)").tag("systemNowPlaying") } .pickerStyle(.segmented) +.accessibilityLabel("Music Source") +.accessibilityIdentifier("musicSourcePicker") .onChange(of: playbackSourceMode) { _, newValue in // ... }And for the info row at lines 78-85:
if playbackSourceMode == "systemNowPlaying" { HStack(spacing: 6) { Image(systemName: "info.circle") .foregroundStyle(.secondary) Text("Uses macOS system media info. No Apple Music access needed.") .font(.caption) .foregroundStyle(.secondary) } + .accessibilityElement(children: .combine) + .accessibilityLabel("Uses macOS system media info. No Apple Music access needed.") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift` around lines 53 - 86, The Music Source picker and its info row lack accessibility attributes; add accessibilityLabel and accessibilityIdentifier to the Picker bound to playbackSourceMode (use descriptive values like "Music Source Picker" and "musicSourcePicker") and to the explanatory Texts and the HStack info row (e.g., labels "Music Source Description" / "musicSourceDescription" and "System Now Playing Info" / "systemNowPlayingInfo"); also apply accessibilityLabel/identifier to the Image inside the HStack (e.g., "infoIcon" / "infoIconImage") so UI tests and assistive tech can target each element unambiguously.apps/native/WolfWaveTests/SystemNowPlayingSourceTests.swift (1)
61-71: Test name and comment are misleading.The test is named
testDelegateIsNotifiedIfFrameworkUnavailablebut doesn't assert that the delegate was actually notified. The comment on line 67 is also inaccurate—perSystemNowPlayingSource.swift:78-84, the delegate is notified instartTracking()(not init) when the framework is unavailable.Consider either:
- Renaming to
testDelegateWiringDoesNotCrash, or- Actually verifying the delegate callback:
✅ Option 2: Add assertion
func testDelegateIsNotifiedIfFrameworkUnavailable() { - // We can't force dlopen to fail, but we can verify the delegate interface works let source = SystemNowPlayingSource() let spy = StatusSpy() source.delegate = spy - // If framework loaded, startTracking works silently. - // If it didn't load, delegate should have been called with an error status synchronously in init (not tested here). - // Just verify no crash. source.startTracking() + // If framework unavailable, delegate receives "System Now Playing unavailable" + // If available, statuses may be empty or contain track info + // We can't force unavailability, so just verify no crash source.stopTracking() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/WolfWaveTests/SystemNowPlayingSourceTests.swift` around lines 61 - 71, The test named testDelegateIsNotifiedIfFrameworkUnavailable is misleading: either rename it to testDelegateWiringDoesNotCrash, or update it to actually assert the delegate callback fires when the framework is unavailable by exercising SystemNowPlayingSource.startTracking() and checking StatusSpy received the expected status/error; specifically, modify the test (or replace it) so after creating SystemNowPlayingSource and assigning StatusSpy to source.delegate you call source.startTracking(), then assert on the spy's observed property (e.g., a boolean or lastStatus field on StatusSpy) to verify the delegate method was invoked, and finally call source.stopTracking().CHANGELOG.md (1)
11-11: Minor style: "macOS system" is redundant.The "OS" in "macOS" already means "operating system," so "macOS system Now Playing API" is tautological. Consider:
-- **Multi-source music support** — choose between Apple Music (direct ScriptingBridge connection) and Any App (System), which uses the macOS system Now Playing API to capture playback from Spotify, browsers, and any other app. Setting lives in Music Monitor preferences. +- **Multi-source music support** — choose between Apple Music (direct ScriptingBridge connection) and Any App (System), which uses the macOS Now Playing API to capture playback from Spotify, browsers, and any other app. Setting lives in Music Monitor preferences.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 11, The phrase "macOS system Now Playing API" is redundant; update the changelog line that currently reads "**Multi-source music support** — choose between Apple Music (direct ScriptingBridge connection) and Any App (System), which uses the macOS system Now Playing API to capture playback from Spotify, browsers, and any other app. Setting lives in Music Monitor preferences." to use "macOS Now Playing API" (i.e., replace "macOS system Now Playing API" with "macOS Now Playing API") so the wording is concise and not tautological.apps/native/wolfwave/Monitors/PlaybackSourceManager.swift (1)
25-27: UseAppConstants.UserDefaults.playbackSourceModefor the read/write path.The manager still hardcodes
"playbackSourceMode"on both sides of persistence, which leaves the new central key as dead metadata and makes future renames easier to miss.♻️ Proposed cleanup
- let stored = UserDefaults.standard.string(forKey: "playbackSourceMode") + let stored = UserDefaults.standard.string(forKey: AppConstants.UserDefaults.playbackSourceMode) currentMode = PlaybackSourceMode(rawValue: stored ?? "") ?? .appleMusic @@ - UserDefaults.standard.set(mode.rawValue, forKey: "playbackSourceMode") + UserDefaults.standard.set(mode.rawValue, forKey: AppConstants.UserDefaults.playbackSourceMode)Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift` around lines 25 - 27, The PlaybackSourceManager is still using the hardcoded key "playbackSourceMode" when reading/writing UserDefaults in init() and the writer at line ~58; replace both occurrences to use the centralized key AppConstants.UserDefaults.playbackSourceMode so the read path (PlaybackSourceManager.init) and the write path (where currentMode is persisted) use the same constant; update any UserDefaults.standard.string(forKey: ...) and UserDefaults.standard.set(..., forKey: ...) calls in this class to reference AppConstants.UserDefaults.playbackSourceMode (keep behavior otherwise unchanged).apps/native/wolfwave/Monitors/SystemNowPlayingSource.swift (1)
12-16: Use the new shared MediaRemote constants here instead of redefining them.
AppConstantsalready exposes the framework path, notification name, and queue label for this source. Keeping local copies here creates a second place those integration details can drift.♻️ Proposed cleanup
private enum Constants { - static let frameworkPath = "/System/Library/PrivateFrameworks/MediaRemote.framework/MediaRemote" - static let nowPlayingChangedNotification = "kMRMediaRemoteNowPlayingInfoDidChangeNotification" - static let queueLabel = "com.mrdemonwolf.wolfwave.systemnowplaying" + static let frameworkPath = AppConstants.SystemNowPlaying.frameworkPath + static let nowPlayingChangedNotification = AppConstants.SystemNowPlaying.nowPlayingInfoDidChangeNotification + static let queueLabel = AppConstants.DispatchQueues.systemNowPlaying static let checkInterval: TimeInterval = 5.0 static let notificationDedupWindow: TimeInterval = 0.75 static let idleGraceWindow: TimeInterval = 2.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Monitors/SystemNowPlayingSource.swift` around lines 12 - 16, The local Constants enum in SystemNowPlayingSource (symbols: Constants, frameworkPath, nowPlayingChangedNotification, queueLabel, checkInterval) is redefining values already provided by AppConstants; replace the local copies with references to the shared values instead of hardcoding strings—use AppConstants.<appropriateName> for the framework path, notification name, and queue label (and keep or adjust checkInterval only if AppConstants exposes it), and remove the duplicated literal values so the source relies on AppConstants for those integration constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/native/wolfwave/Monitors/SystemNowPlayingSource.swift`:
- Around line 80-82: The early return in SystemNowPlayingSource's
startTracking() calls delegate?.playbackSource(... didUpdateStatus: "System Now
Playing unavailable") directly on the current thread; change this to use the
existing notifyDelegate(...) path so the unavailable-status update is dispatched
on the main-thread like other updates. Replace the synchronous delegate call in
the guard else with a call to notifyDelegate that invokes
delegate?.playbackSource(self, didUpdateStatus: ...) so the UI always receives
the failure status on the main thread.
- Around line 55-67: The dlsym results should be checked for NULL before calling
unsafeBitCast so you don't turn a NULL pointer into a non-nil Swift function
pointer; change the load logic around dlopen/dlsym so you first call
dlsym(handle, "...") for "MRMediaRemoteGetNowPlayingInfo" and
"MRMediaRemoteRegisterForNowPlayingNotifications", verify each returned pointer
is non-NULL, only then perform unsafeBitCast into
MRMediaRemoteGetNowPlayingInfoFunction and
MRMediaRemoteRegisterForNowPlayingNotificationsFunction to assign
getNowPlayingInfo and registerForNotifications, and set frameworkLoaded based on
the non-NULL dlsym results (or nil the functions if any symbol is missing).
In `@apps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift`:
- Line 26: Replace the hardcoded AppStorage key in the playbackSourceMode
property with the centralized constant: change the `@AppStorage` usage for the
property playbackSourceMode to reference
AppConstants.UserDefaults.playbackSourceMode instead of the literal
"playbackSourceMode", keeping the property name and default value behavior the
same so it continues to default to "appleMusic" when the key is absent.
---
Nitpick comments:
In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift`:
- Around line 25-27: The PlaybackSourceManager is still using the hardcoded key
"playbackSourceMode" when reading/writing UserDefaults in init() and the writer
at line ~58; replace both occurrences to use the centralized key
AppConstants.UserDefaults.playbackSourceMode so the read path
(PlaybackSourceManager.init) and the write path (where currentMode is persisted)
use the same constant; update any UserDefaults.standard.string(forKey: ...) and
UserDefaults.standard.set(..., forKey: ...) calls in this class to reference
AppConstants.UserDefaults.playbackSourceMode (keep behavior otherwise
unchanged).
In `@apps/native/wolfwave/Monitors/SystemNowPlayingSource.swift`:
- Around line 12-16: The local Constants enum in SystemNowPlayingSource
(symbols: Constants, frameworkPath, nowPlayingChangedNotification, queueLabel,
checkInterval) is redefining values already provided by AppConstants; replace
the local copies with references to the shared values instead of hardcoding
strings—use AppConstants.<appropriateName> for the framework path, notification
name, and queue label (and keep or adjust checkInterval only if AppConstants
exposes it), and remove the duplicated literal values so the source relies on
AppConstants for those integration constants.
In `@apps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift`:
- Around line 53-86: The Music Source picker and its info row lack accessibility
attributes; add accessibilityLabel and accessibilityIdentifier to the Picker
bound to playbackSourceMode (use descriptive values like "Music Source Picker"
and "musicSourcePicker") and to the explanatory Texts and the HStack info row
(e.g., labels "Music Source Description" / "musicSourceDescription" and "System
Now Playing Info" / "systemNowPlayingInfo"); also apply
accessibilityLabel/identifier to the Image inside the HStack (e.g., "infoIcon" /
"infoIconImage") so UI tests and assistive tech can target each element
unambiguously.
In `@apps/native/WolfWaveTests/SystemNowPlayingSourceTests.swift`:
- Around line 61-71: The test named testDelegateIsNotifiedIfFrameworkUnavailable
is misleading: either rename it to testDelegateWiringDoesNotCrash, or update it
to actually assert the delegate callback fires when the framework is unavailable
by exercising SystemNowPlayingSource.startTracking() and checking StatusSpy
received the expected status/error; specifically, modify the test (or replace
it) so after creating SystemNowPlayingSource and assigning StatusSpy to
source.delegate you call source.startTracking(), then assert on the spy's
observed property (e.g., a boolean or lastStatus field on StatusSpy) to verify
the delegate method was invoked, and finally call source.stopTracking().
In `@CHANGELOG.md`:
- Line 11: The phrase "macOS system Now Playing API" is redundant; update the
changelog line that currently reads "**Multi-source music support** — choose
between Apple Music (direct ScriptingBridge connection) and Any App (System),
which uses the macOS system Now Playing API to capture playback from Spotify,
browsers, and any other app. Setting lives in Music Monitor preferences." to use
"macOS Now Playing API" (i.e., replace "macOS system Now Playing API" with
"macOS Now Playing API") so the wording is concise and not tautological.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6291ab78-5f3d-44f0-97fb-a7910ab7c1e6
📒 Files selected for processing (14)
CHANGELOG.mdFEATURE_IDEAS.mdapps/docs/content/docs/changelog.mdxapps/native/WolfWaveTests/MusicPlaybackMonitorTests.swiftapps/native/WolfWaveTests/PlaybackSourceManagerTests.swiftapps/native/WolfWaveTests/SystemNowPlayingSourceTests.swiftapps/native/wolfwave.xcodeproj/project.pbxprojapps/native/wolfwave/Core/AppConstants.swiftapps/native/wolfwave/Monitors/AppleMusicSource.swiftapps/native/wolfwave/Monitors/PlaybackSource.swiftapps/native/wolfwave/Monitors/PlaybackSourceManager.swiftapps/native/wolfwave/Monitors/SystemNowPlayingSource.swiftapps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swiftapps/native/wolfwave/WolfWaveApp.swift
apps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift
Outdated
Show resolved
Hide resolved
- SystemNowPlayingSource: use AppConstants for frameworkPath, notification name, and queue label instead of local duplicates - SystemNowPlayingSource: check dlsym result for NULL before unsafeBitCast - SystemNowPlayingSource: dispatch unavailable status via notifyDelegate (main thread) - PlaybackSourceManager: use AppConstants.UserDefaults.playbackSourceMode key in both init() and switchMode(_:) instead of hardcoded string - MusicMonitorSettingsView: use AppConstants.UserDefaults.playbackSourceMode in @AppStorage; add accessibilityLabel/accessibilityIdentifier to picker and accessibilityElement to system mode info row - SystemNowPlayingSourceTests: rename misleading test to testDelegateWiringDoesNotCrash - CHANGELOG: fix "macOS system Now Playing API" → "macOS Now Playing API" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/native/wolfwave/Monitors/PlaybackSourceManager.swift (2)
33-40: Remove or update the misleading NOTE comment.The comment states "this requires a workaround — see below" but the code directly assigns
source.delegate = selfon the existential type (any PlaybackSource), which works correctly because thePlaybackSourceprotocol declaresdelegateas settable ({ get set }). The comment creates unnecessary confusion.✏️ Suggested fix
func startTracking() { stopTracking() let source: any PlaybackSource = (currentMode == .appleMusic) ? appleMusicSource : systemNowPlayingSource - source.delegate = self // NOTE: this requires a workaround — see below + source.delegate = self activeSource = source isTracking = true source.startTracking() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift` around lines 33 - 40, Remove the misleading NOTE comment in startTracking: the assignment source.delegate = self on the existential any PlaybackSource is valid because the PlaybackSource protocol's delegate is declared as get/set, so simply delete or update the comment above the line source.delegate = self in the startTracking() method of PlaybackSourceManager to avoid confusion (ensure references to activeSource, systemNowPlayingSource, and appleMusicSource remain unchanged).
77-85: Simplify delegate clearing using the protocol directly.Since
PlaybackSourcedeclaresdelegateas settable ({ get set }), you can clear it uniformly without type-casting. The current approach requires adding anotherelse ifbranch whenever a new source type is introduced, creating a maintenance burden and risk of missed cleanup.This also aligns with how line 36 already sets the delegate directly on
any PlaybackSource.♻️ Suggested simplification
// MARK: - Private Helpers private func clearActiveSourceDelegate() { - if let source = activeSource as? AppleMusicSource { - source.delegate = nil - } else if let source = activeSource as? SystemNowPlayingSource { - source.delegate = nil - } + activeSource?.delegate = nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift` around lines 77 - 85, The clearActiveSourceDelegate() currently type-checks activeSource against AppleMusicSource and SystemNowPlayingSource to nil out delegates; instead, treat activeSource as a PlaybackSource and clear its delegate directly. Update clearActiveSourceDelegate() to check activeSource != nil and set activeSource.delegate = nil (using the PlaybackSource protocol's settable delegate) so future source types don't require new branches; remove the AppleMusicSource/SystemNowPlayingSource casts and branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift`:
- Around line 33-40: Remove the misleading NOTE comment in startTracking: the
assignment source.delegate = self on the existential any PlaybackSource is valid
because the PlaybackSource protocol's delegate is declared as get/set, so simply
delete or update the comment above the line source.delegate = self in the
startTracking() method of PlaybackSourceManager to avoid confusion (ensure
references to activeSource, systemNowPlayingSource, and appleMusicSource remain
unchanged).
- Around line 77-85: The clearActiveSourceDelegate() currently type-checks
activeSource against AppleMusicSource and SystemNowPlayingSource to nil out
delegates; instead, treat activeSource as a PlaybackSource and clear its
delegate directly. Update clearActiveSourceDelegate() to check activeSource !=
nil and set activeSource.delegate = nil (using the PlaybackSource protocol's
settable delegate) so future source types don't require new branches; remove the
AppleMusicSource/SystemNowPlayingSource casts and branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 696a7697-cf6b-4f7d-bece-903f4f349a1b
📒 Files selected for processing (5)
CHANGELOG.mdapps/native/WolfWaveTests/SystemNowPlayingSourceTests.swiftapps/native/wolfwave/Monitors/PlaybackSourceManager.swiftapps/native/wolfwave/Monitors/SystemNowPlayingSource.swiftapps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift
✅ Files skipped from review due to trivial changes (3)
- apps/native/wolfwave/Views/MusicMonitor/MusicMonitorSettingsView.swift
- apps/native/WolfWaveTests/SystemNowPlayingSourceTests.swift
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/native/wolfwave/Monitors/SystemNowPlayingSource.swift
PlaybackSourceManagerTests crashed on every test with "pointer being freed was not allocated" on a background thread. Root cause: AppleMusicSource was eagerly created as a stored property in PlaybackSourceManager.init, racing with a still-in-flight backgroundQueue async block from the previous AppleMusicSourceTests teardown on CI runners. Making appleMusicSource (and systemNowPlayingSource) lazy defers allocation until startTracking() is called. No tests in PlaybackSourceManagerTests call startTracking(), so no AppleMusicSource is ever created during those tests, eliminating the race. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/native/wolfwave/Monitors/PlaybackSourceManager.swift (1)
79-85: Reduce concrete-source branching in delegate cleanup.
clearActiveSourceDelegate()is coupled to specific source types. This will need manual updates whenever a newPlaybackSourceis added. Prefer protocol-level cleanup so this stays extensible.♻️ Suggested simplification
private func clearActiveSourceDelegate() { - if let source = activeSource as? AppleMusicSource { - source.delegate = nil - } else if let source = activeSource as? SystemNowPlayingSource { - source.delegate = nil - } + activeSource?.delegate = nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift` around lines 79 - 85, clearActiveSourceDelegate() currently branches on concrete types (AppleMusicSource, SystemNowPlayingSource); instead make delegate cleanup protocol-level by adding a delegate property or clearDelegate() requirement to the PlaybackSource protocol and then call it on activeSource (e.g., (activeSource as? PlaybackSource)?.delegate = nil or (activeSource as? PlaybackSource)?.clearDelegate()). Ensure AppleMusicSource and SystemNowPlayingSource conform to the PlaybackSource protocol so no future source additions require edits to clearActiveSourceDelegate().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/native/wolfwave/Monitors/PlaybackSourceManager.swift`:
- Around line 79-85: clearActiveSourceDelegate() currently branches on concrete
types (AppleMusicSource, SystemNowPlayingSource); instead make delegate cleanup
protocol-level by adding a delegate property or clearDelegate() requirement to
the PlaybackSource protocol and then call it on activeSource (e.g.,
(activeSource as? PlaybackSource)?.delegate = nil or (activeSource as?
PlaybackSource)?.clearDelegate()). Ensure AppleMusicSource and
SystemNowPlayingSource conform to the PlaybackSource protocol so no future
source additions require edits to clearActiveSourceDelegate().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 044b0594-f8eb-4e81-b6d6-55dfd1af4930
📒 Files selected for processing (1)
apps/native/wolfwave/Monitors/PlaybackSourceManager.swift
CRITICAL BUG FIX: playbackSourceModeChanged notification had no observer in AppDelegate, so switching the Music Source picker never took effect at runtime. Added the missing observer — switchMode() is now called immediately when the user changes the picker. SOURCE DETECTION: SystemNowPlayingSource now resolves MRMediaRemoteGet- NowPlayingApplicationPID via dlsym to detect which app is playing (Spotify, Chrome, etc.). Bundle ID is surfaced via new PlaybackSourceDelegate method playbackSource(_:didDetectSourceApp:) with a default no-op extension for backward compatibility. SOURCE-AWARE DISCORD RPC: - Small icon + tooltip now show the actual playing app (Spotify logo, Apple Music logo, YouTube/Chrome icon, or generic music note) - "Open in Apple Music" button only appears when Apple Music is the source - Large image fallback uses the source-specific asset instead of always showing apple_music TWITCH COMMANDS: getCurrentSongInfo/getLastSongInfo no longer gate on isMusicAppOpen() when in system mode — Spotify/Chrome tracks now respond correctly. Non-Apple-Music sources append "via Spotify" etc. UI TEXT: Updated settings and onboarding descriptions to be source-agnostic. DISCORD ASSETS: Added discord-assets/ folder with placeholder PNGs for spotify, youtube, and music_generic. Upload these to Discord Developer Portal → Rich Presence → Art Assets (see discord-assets/README.md). Added KnownMusicApps helper in AppConstants with displayName(for:), discordAssetName(for:), isAppleMusic(_:), isBrowser(_:). Tested in KnownMusicAppsTests.swift (16 new tests). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The private MediaRemote framework is blocked by kMRMediaRemoteFrameworkErrorDomain Code=3 on macOS 26 (Tahoe). The mediaremoted daemon enforces private Apple entitlements (com.apple.private.mediaremote.nowplayinginfo) that third-party apps cannot obtain — no public API exists to read now-playing info from other apps. - Delete SystemNowPlayingSource.swift and all related tests - Remove PlaybackSourceMode.systemNowPlaying, didDetectSourceApp delegate method - Simplify PlaybackSourceManager to always use AppleMusicSource - Remove KnownMusicApps, SystemNowPlaying enums from AppConstants - Remove source picker from MusicMonitorSettingsView - Remove source-aware branding from DiscordRPCService and WolfWaveApp - Revert docs (index, features, usage, changelog) to Apple Music language - Revert entitlements: remove com.apple.mediaremoted mach-lookup exception - Update WhatsNewView with v1.1.0 highlights (pending v1.2.0 feature set) - Fix @mainactor isolation for Twitch bot callbacks (MainActor.assumeIsolated) - Move CHANGELOG v1.2.0 entry to [Unreleased] — more features coming Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tasks
nathanialhenniges
added a commit
that referenced
this pull request
Apr 3, 2026
…olish Resolves conflicts from PR #13 landing in main. Kept HEAD versions for AppDelegate extension architecture, thread-safe AppleMusicSource tracking lock, and richer FEATURE_IDEAS content; accepted origin/main additions for systemNowPlaying queue key, Last.fm/Multi-Platform feature ideas, and new PlaybackSourceManagerTests test. Co-Authored-By: Claude Sonnet 4.6 <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.
Summary
PlaybackSourceabstraction — protocol +PlaybackSourceModeenum replacing the tightly-coupledMusicPlaybackMonitorAppleMusicSource— refactored fromMusicPlaybackMonitor, identical logic, now conforms toPlaybackSource(default, zero behavior change for existing users)SystemNowPlayingSource— uses private macOSMediaRemoteframework viadlopen/dlsymto capture now-playing from any app (Spotify, Chrome, Tidal, etc.) with graceful degradation if unavailablePlaybackSourceManager— owns both sources, handles clean switching, forwardsPlaybackSourceDelegatecallbacks toAppDelegateFEATURE_IDEAS.md— 8 future feature ideas documented at project rootPlaybackSourceManagerTestsandSystemNowPlayingSourceTests)Test plan
make build)make test)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Other Additions
Documentation