feat(docs): 2026 marketing redesign of landing page#17
Merged
nathanialhenniges merged 3 commits intomainfrom Apr 15, 2026
Merged
feat(docs): 2026 marketing redesign of landing page#17nathanialhenniges merged 3 commits intomainfrom
nathanialhenniges merged 3 commits intomainfrom
Conversation
|
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 44 minutes and 3 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 selected for processing (4)
WalkthroughThis PR introduces comprehensive song request functionality for Twitch chat, enabling users to request songs via Changes
Sequence DiagramsequenceDiagram
participant User as Twitch User
participant TwitchChat as TwitchChatService
participant Dispatcher as BotCommandDispatcher
participant SongCmd as SongRequestCommand
participant Service as SongRequestService
participant Resolver as SongSearchResolver
participant LinkResolver as LinkResolverService
participant MusicKit as AppleMusicController
participant Queue as SongRequestQueue
participant AppleScript as Music.app
User->>TwitchChat: !sr song name
TwitchChat->>Dispatcher: processMessage(text, context, asyncReply)
Dispatcher->>SongCmd: execute(message, context, reply)
SongCmd->>Service: processRequest(query, username, context)
Service->>Resolver: resolve(query)
alt Query is link
Resolver->>LinkResolver: resolve(url)
LinkResolver-->>Resolver: ResolveResult
else Query is text
Resolver->>MusicKit: search(query)
MusicKit-->>Resolver: SearchResult
end
Resolver-->>Service: Result.found(Song)
Service->>Queue: add(SongRequestItem)
Queue-->>Service: AddResult
Service->>MusicKit: playNow(song)
MusicKit->>AppleScript: AppleScript command
AppleScript-->>MusicKit: Playback started
Service-->>SongCmd: RequestResult.added
SongCmd->>TwitchChat: reply callback
TwitchChat-->>User: Chat message response
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: 14
🧹 Nitpick comments (15)
apps/native/wolfwave/Views/Shared/WhatsNewView.swift (2)
21-26: Use stable feature IDs instead of titles for accessibility identifiers.Because Line 93 builds identifiers from
feature.title, changing display text here changes test selectors. Add a stableidfield to each feature and derive identifiers from that.🧪 Suggested stabilization
-private let features: [(icon: String, iconColor: Color, title: String, description: String)] = [ - ("music.mic", .pink, "Song Requests", "..."), +private let features: [(id: String, icon: String, iconColor: Color, title: String, description: String)] = [ + ("songRequests", "music.mic", .pink, "Song Requests", "..."), ... ] -private func featureRow(_ feature: (icon: String, iconColor: Color, title: String, description: String)) -> some View { +private func featureRow(_ feature: (id: String, icon: String, iconColor: Color, title: String, description: String)) -> some View { ... - .accessibilityIdentifier("whatsNew.feature.\(feature.title)") + .accessibilityIdentifier("whatsNew.feature.\(feature.id)") }Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Views/Shared/WhatsNewView.swift` around lines 21 - 26, The feature list in WhatsNewView uses display titles which makes accessibility identifiers brittle; add a stable id field to each feature tuple/struct (e.g., change the array entries to include an "id" like "song_requests", "hold_mode", etc.), update the model used by WhatsNewView (the features collection) to include that id, and change the code that currently derives identifiers from feature.title (referenced in WhatsNewView and at the identifier construction site around where feature.title is used) to use feature.id when building accessibility identifiers and test selectors so identifiers remain stable when titles change.
21-26: Tighten copy length for faster scanability.The descriptions are clear, but several are long for a compact “What’s New” sheet. Consider shortening each to one crisp clause.
✂️ Suggested copy polish
-("music.mic", .pink, "Song Requests", "Viewers request songs with !sr in Twitch chat — plays through your Music.app"), +("music.mic", .pink, "Song Requests", "Viewers request songs in chat with !sr."), -("pause.fill", .orange, "Hold Mode", "Pause the queue to curate requests, then release when you're ready"), +("pause.fill", .orange, "Hold Mode", "Pause requests while you curate the queue."), -("forward.fill", .blue, "Queue Controls", "Skip, clear, or hold from the app, menu bar, or Twitch chat (mods only)"), +("forward.fill", .blue, "Queue Controls", "Skip, clear, or hold from app, menu bar, or chat."), -("list.number", .green, "Live Queue View", "See what's playing, what's next, and who requested each song"), +("list.number", .green, "Live Queue View", "See now playing, up next, and requester names."), -("music.note.list", .purple, "Fallback Playlist", "Pick an Apple Music playlist to play when the request queue runs dry"), +("music.note.list", .purple, "Fallback Playlist", "Auto-play a playlist when requests run out."), -("eye.slash", .cyan, "No Focus-Steal", "Music.app stays in the background — your stream tools keep focus"), +("eye.slash", .cyan, "No Focus-Steal", "Music.app stays in back while your stream tools stay front."),As per coding guidelines: "Write all user-facing text to be short, punchy, and jargon-free (ADHD-friendly)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Views/Shared/WhatsNewView.swift` around lines 21 - 26, In WhatsNewView.swift update the short-description strings in the tuples used by WhatsNewView (the list entries like the ("music.mic", .pink, "Song Requests", "...") tuples) to be single, punchy clauses instead of full sentences—e.g. condense each description to one fragment (3–6 words) that summarizes the feature—so replace the long fourth-field strings for Song Requests, Hold Mode, Queue Controls, Live Queue View, Fallback Playlist, and No Focus-Steal with much shorter phrases to improve scanability.apps/docs/app/(home)/page.tsx (1)
224-234: Consider a more descriptive key for the ticker items.Using array index as key works here since the list is static, but a compound key improves clarity.
💡 Optional improvement
- {[...tickerItems, ...tickerItems, ...tickerItems].map((item, i) => ( - <span key={i} className="ticker-item"> + {[...tickerItems, ...tickerItems, ...tickerItems].map((item, i) => ( + <span key={`${item}-${i}`} className="ticker-item">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/app/`(home)/page.tsx around lines 224 - 234, The map over tickerItems uses the array index as the React key which is unclear; update the key in the .map callback that renders .ticker-item to a more descriptive/compound key (e.g., derive from tickerItems' value plus the index or a unique id) so each <span key=... className="ticker-item"> uses a stable, meaningful key instead of just i; locate the map over [...tickerItems, ...tickerItems, ...tickerItems] and replace key={i} with a compound key like `${item}-${i}` or an actual unique identifier from the ticker data.apps/native/wolfwave/Services/SongRequest/SongBlocklist.swift (1)
84-96: Recover from corrupt persisted blocklists instead of failing silently.A decode failure currently leaves the bad payload in
UserDefaults, so every future launch keeps failing the same way. Logging and clearing/quarantining the stored blob would make this self-healing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/SongRequest/SongBlocklist.swift` around lines 84 - 96, The load() currently swallows JSON decode failures and leaves a corrupt blob under storageKey so subsequent loads always fail; modify load() to catch and log the decoding error (capture the thrown Error from JSONDecoder.decode), and if decoding fails take remedial action such as removing or moving the bad data (e.g. removeObject(forKey: storageKey) or copy it to a quarantine key like "corrupt_\(storageKey)") and then set entries = [] under the same lock used by save() (lock.withLock { entries = [] }) so the app can recover; also ensure save() still writes the new empty or repaired entries normally.apps/native/WolfWaveTests/OnboardingViewModelTests.swift (1)
139-159: Consider extending the full navigation cycle test to cover.appleMusicAccess.The
testFullNavigationCycleForwardAndBacktest stops at.obsWidgetand doesn't navigate to the new.appleMusicAccessstep. While the other tests cover this step individually, extending this test would provide more comprehensive round-trip coverage.♻️ Optional: Extend the navigation cycle test
viewModel.goToNextStep() XCTAssertEqual(viewModel.currentStep, .obsWidget) + viewModel.goToNextStep() + XCTAssertEqual(viewModel.currentStep, .appleMusicAccess) + + viewModel.goToPreviousStep() + XCTAssertEqual(viewModel.currentStep, .obsWidget) + viewModel.goToPreviousStep() XCTAssertEqual(viewModel.currentStep, .discordConnect)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/WolfWaveTests/OnboardingViewModelTests.swift` around lines 139 - 159, Extend the testFullNavigationCycleForwardAndBack test to include the new .appleMusicAccess step: after the current assertion for .obsWidget call viewModel.goToNextStep() and assert viewModel.currentStep == .appleMusicAccess, then continue the backward assertions by calling viewModel.goToPreviousStep() and asserting it returns to .obsWidget before continuing back to .discordConnect, .twitchConnect, and .welcome; update the sequence of viewModel.goToNextStep()/goToPreviousStep() calls and their XCTAssertEqual checks to cover the full forward-and-back round trip including .appleMusicAccess.apps/native/wolfwave/Services/Twitch/Commands/AsyncBotCommand.swift (1)
33-33: Consider marking thereplycallback as@Sendable.Since async commands may perform network requests and call
replyfrom different isolation contexts, marking the callback as@Sendablewould ensure thread-safety guarantees and satisfy Swift strict concurrency checking.♻️ Optional: Add `@Sendable` annotation
- func execute(message: String, context: BotCommandContext, reply: `@escaping` (String) -> Void) + func execute(message: String, context: BotCommandContext, reply: `@escaping` `@Sendable` (String) -> Void)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/Twitch/Commands/AsyncBotCommand.swift` at line 33, The execute signature's reply closure should be annotated as `@Sendable` to satisfy Swift concurrency safety: update the AsyncBotCommand.execute declaration so the reply parameter is declared as reply: `@Sendable` `@escaping` (String) -> Void (and propagate this change to any protocol/type declarations and callers that conform to or call AsyncBotCommand.execute), then fix any calling sites to pass a `@Sendable-compatible` closure (or wrap non-sendable captures) so the code compiles under strict concurrency checking.apps/native/wolfwave/Services/Twitch/Commands/HoldCommand.swift (1)
14-24: MissingenabledKeyandaliasesKeyproperties.Unlike
SkipCommandandClearQueueCommandwhich defineenabledKeyandaliasesKeyfor user-configurable enable/disable and custom aliases,HoldCommandlacks these properties. This means the command cannot be disabled or aliased via settings.Consider adding for consistency:
♻️ Proposed fix to add enable/alias support
var userCooldown: TimeInterval { 3.0 } + var enabledKey: String? { AppConstants.UserDefaults.songRequestHoldEnabled } + + var aliasesKey: String? { AppConstants.UserDefaults.holdCommandAliases } + // MARK: - PropertiesNote: You'd also need to add
holdCommandAliasestoAppConstants.UserDefaultsif you want alias support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/Twitch/Commands/HoldCommand.swift` around lines 14 - 24, HoldCommand is missing user-configurable keys so it can't be disabled or aliased; add the same properties used by SkipCommand/ClearQueueCommand: implement var enabledKey: String and var aliasesKey: String in the HoldCommand class, returning the appropriate UserDefaults keys (e.g. AppConstants.UserDefaults.holdCommandEnabled and AppConstants.UserDefaults.holdCommandAliases), and add the new holdCommandAliases and holdCommandEnabled entries to AppConstants.UserDefaults to expose alias and enable/disable settings.apps/native/wolfwave/Views/SongRequest/SongRequestQueueView.swift (2)
202-251: Missing accessibility identifiers on action buttons.The action buttons (Skip, Hold/Resume, Clear Queue) lack
accessibilityIdentifiermodifiers, which are required per coding guidelines for UI testing.♻️ Proposed fix to add accessibility identifiers
Button { Task { _ = await service?.skip() refreshState() } } label: { Label("Skip", systemImage: "forward.fill") .font(.system(size: 11)) } .buttonStyle(.bordered) .controlSize(.small) .disabled(nowPlaying == nil) + .accessibilityIdentifier("songRequestQueue.skipButton") Button { Task { await service?.setHold(!isHeld) refreshState() } } label: { Label(isHeld ? "Resume" : "Hold", systemImage: isHeld ? "play.fill" : "pause.fill") .font(.system(size: 11)) } .buttonStyle(.bordered) .controlSize(.small) .tint(isHeld ? .green : .orange) + .accessibilityIdentifier("songRequestQueue.holdButton") Button { showingClearConfirm = true } label: { Label("Clear Queue", systemImage: "trash") .font(.system(size: 11)) } .buttonStyle(.bordered) .controlSize(.small) .disabled(items.isEmpty && nowPlaying == nil) + .accessibilityIdentifier("songRequestQueue.clearQueueButton")As per coding guidelines: "Ensure all UI elements have
accessibilityLabelandaccessibilityIdentifierfor UI testing"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Views/SongRequest/SongRequestQueueView.swift` around lines 202 - 251, The action buttons in the actionButtons view (the Skip Button, the Hold/Resume Button and the Clear Queue Button plus the confirmation dialog action) are missing accessibility identifiers/labels; update each Button (the one that calls service?.skip(), the one that calls service?.setHold(!isHeld), and the Clear Queue Button and its destructive confirmation Button) to add .accessibilityIdentifier("<unique_id>") and .accessibilityLabel("<readable label>") (use clear ids like "skipButton", "holdResumeButton", "clearQueueButton", "confirmClearQueueButton") so UI tests can reliably find them while keeping existing behavior intact.
279-284: Prefer async/await over DispatchQueue for timer callback.Per coding guidelines, async/await should be preferred over DispatchQueue for new async work. The timer callback uses
DispatchQueue.main.async.♻️ Proposed fix using MainActor
private func startRefresh() { refreshState() refreshTimer = Timer.scheduledTimer(withTimeInterval: 2.0, repeats: true) { _ in - DispatchQueue.main.async { refreshState() } + Task { `@MainActor` in + refreshState() + } } }Based on learnings: "Use Swift 5.9+ with async/await concurrency instead of DispatchQueue for new async work"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Views/SongRequest/SongRequestQueueView.swift` around lines 279 - 284, The timer callback uses DispatchQueue.main.async; replace it with Swift concurrency by running the refresh on the main actor via Task. In startRefresh (and the scheduledTimer closure) call Task { await MainActor.run { refreshState() } } instead of DispatchQueue.main.async { refreshState() }; if refreshState must be async or touch UI, consider annotating it `@MainActor` or making it async so the await/MainActor.run usage is appropriate. Ensure refreshTimer and startRefresh keep the Timer.scheduledTimer invocation but invoke refreshState through Task+MainActor.apps/native/wolfwave/Views/SongRequest/SongRequestSettingsView.swift (1)
181-194: PreferTask.sleepoverDispatchQueue.main.asyncAfter.Per coding guidelines, async/await should be preferred over DispatchQueue for new async work.
♻️ Proposed fix using Task.sleep
.onAppear { blocklist = songBlocklist?.allEntries ?? [] musicAuthStatus = MusicAuthorization.currentStatus refreshTwitchState() - // Delayed re-check to catch late connections - DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { - refreshTwitchState() - } + // Delayed re-check to catch late connections + Task { + try? await Task.sleep(for: .seconds(1)) + await MainActor.run { refreshTwitchState() } + } }Based on learnings: "Use Swift 5.9+ with async/await concurrency instead of DispatchQueue for new async work"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Views/SongRequest/SongRequestSettingsView.swift` around lines 181 - 194, Replace the DispatchQueue delayed re-check in the .onAppear handler with an async/await Task using Task.sleep to wait 1 second and then call refreshTwitchState; specifically, inside the .onAppear block (where blocklist, musicAuthStatus = MusicAuthorization.currentStatus and refreshTwitchState() are set) start a Task that awaits Task.sleep(nanoseconds: 1_000_000_000) and then calls refreshTwitchState(), ensuring the call runs on the main actor if refreshTwitchState mutates UI state (e.g., mark the Task `@MainActor` or call await MainActor.run { refreshTwitchState() } as appropriate); leave the NotificationCenter .onReceive handling with updateTwitchState untouched.apps/native/WolfWaveTests/SongRequestServiceTests.swift (1)
304-317: Avoid the real 2.5s sleep in this unit test.This makes the suite slow and timing-sensitive. Inject the polling interval / clock, or expose a single monitoring tick so the paused-state branch can be asserted synchronously.
Based on learnings "Prioritize testing pure logic (Bot command parsing, version comparison, state machines) and mock network and system services to keep tests fast and reliable".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/WolfWaveTests/SongRequestServiceTests.swift` around lines 304 - 317, The test testAutoAdvanceDoesNotFireWhenPaused currently uses a real 2.5s sleep; change the design so the test can run synchronously by either (A) making SongRequestService accept an injectable polling interval or clock (e.g., add a pollingInterval parameter to the initializer and use that in startPlaybackMonitoring()) so the test can set a tiny interval, or (B) expose a single monitoring tick API on SongRequestService (e.g., monitorTick() or processPlaybackTick()) that startPlaybackMonitoring() calls repeatedly; update the test to call that single tick directly after setting mockController.isPlaying = false and mockController.isPaused = true and assert no dequeue/advance occurred, then call stopPlaybackMonitoring() as before.apps/native/wolfwave/Services/Twitch/Commands/BotCommandDispatcher.swift (1)
144-145: Minor: Redundantlowercased()call on trigger.The
allTriggersproperty (fromBotCommandextension in context snippet 1, line 71) already returns lowercased triggers. Thetrigger.lowercased()call on line 145 is redundant.♻️ Suggested simplification
for trigger in triggers { - let triggerLowered = trigger.lowercased() // Match: message starts with the trigger (original hasPrefix behavior) - if lowered.hasPrefix(triggerLowered) { + if lowered.hasPrefix(trigger) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/Twitch/Commands/BotCommandDispatcher.swift` around lines 144 - 145, The loop in BotCommandDispatcher that does `for trigger in triggers { let triggerLowered = trigger.lowercased() }` performs a redundant lowercasing because the triggers come from the BotCommand extension's allTriggers property which already returns lowercased strings; remove the extra `lowercased()` call and use `trigger` directly (or rename `triggerLowered` to `trigger` where it's referenced) to simplify the code and avoid unnecessary allocations while keeping references to the loop over `triggers`, `triggerLowered`, and the `allTriggers` property in mind.apps/native/wolfwave/Services/SongRequest/AppleMusicController.swift (1)
210-217: URL interpolation in AppleScript is safe here, but consider validation.The
song.urlis a system-provided Apple Music URL, so it shouldn't contain AppleScript-breaking characters. However, for defense-in-depth, you could applysanitizeForAppleScriptto the URL string as well:let urlString = sanitizeForAppleScript(musicURL.absoluteString)This is a minor hardening suggestion since MusicKit URLs are well-formed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/SongRequest/AppleMusicController.swift` around lines 210 - 217, The AppleScript URL is currently interpolated directly from song.url (musicURL.absoluteString) before calling runAppleScriptPreservingFocus; to harden it, pass the URL string through sanitizeForAppleScript (e.g., use sanitizeForAppleScript(musicURL.absoluteString)) and then interpolate that sanitized value into the script so the Music open location string is escaped safely; update the code around song.url, musicURL, and runAppleScriptPreservingFocus to use the sanitized urlString.apps/native/wolfwave/Services/SongRequest/LinkResolverService.swift (2)
66-75: URL extraction is fragile with punctuation.
extractURLsplits only on spaces, so URLs followed by punctuation (common in chat messages likecheck this out: https://open.spotify.com/track/abc!) would include the trailing!. Consider trimming common trailing punctuation:♻️ Suggested improvement
static func extractURL(from text: String) -> String? { let words = text.split(separator: " ") for word in words { - let str = String(word) + var str = String(word) + // Strip common trailing punctuation + while let last = str.last, ["!", "?", ",", ".", ";", ":"].contains(String(last)) { + str.removeLast() + } if str.hasPrefix("http://") || str.hasPrefix("https://") { return str } } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/SongRequest/LinkResolverService.swift` around lines 66 - 75, extractURL(from:) is fragile because it splits on spaces and can return URLs with trailing punctuation; replace the simple split logic with a robust URL detector: use NSDataDetector configured for .link to find the first URL in the input text and return its absoluteString (or, if you prefer to keep manual parsing, trim common trailing punctuation characters like . , ! ? : ; ) ] } and surrounding quotes from the detected token). Update the extractURL(from:) implementation to use NSDataDetector (NSTextCheckingResult) and return the detector's first match URL to avoid including trailing punctuation.
45-47:spotify.link/short URLs may resolve to non-track content.The
spotify.link/domain is Spotify's universal short link service that can redirect to albums, playlists, artists, or tracks. Treating allspotify.link/URLs as track links may cause oEmbed to return unexpected metadata (e.g., album/playlist info) or fail silently.Consider either:
- Following the redirect to get the canonical URL before checking, or
- Adding a note that only track links are reliably supported
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/wolfwave/Services/SongRequest/LinkResolverService.swift` around lines 45 - 47, The isSpotifyLink(_:) helper currently treats any spotify.link/ URL as a track, which is incorrect; update LinkResolverService.isSpotifyLink to resolve short URLs (follow HTTP redirect) and then inspect the final URL's path for the explicit track pattern (e.g., contains "open.spotify.com/track/") before returning true, or alternatively limit the check to the canonical track pattern only and add a comment/note that only track links are supported; use a simple URLSession HEAD/GET to follow redirects and perform the canonical check inside isSpotifyLink so callers get a reliable true/false for tracks.
🤖 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/docs/app/global.css`:
- Around line 419-427: The CSS uses the keyword "currentColor" with an uppercase
C in the selector .kinetic-headline .outline (specifically in
-webkit-text-stroke: 2px currentColor;), which violates stylelint's
value-keyword-case rule; update that property (and any other occurrences of the
keyword) to use lowercase "currentcolor" so the rule reads -webkit-text-stroke:
2px currentcolor; preserving all other declarations and selectors unchanged.
In `@apps/native/wolfwave.xcodeproj/project.pbxproj`:
- Around line 433-435: The Release build configuration is using
CODE_SIGN_IDENTITY = "Apple Development" which signs release artifacts with a
dev identity; locate the Release configuration block in the project.pbxproj
where CODE_SIGN_IDENTITY is set and change the value to "Developer ID
Application" so Release builds are signed with the Developer ID Application
identity (keep CODE_SIGN_ENTITLEMENTS and CODE_SIGN_STYLE unchanged and ensure
this change only applies to the Release configuration).
In `@apps/native/wolfwave/Services/SongRequest/SongBlocklist.swift`:
- Around line 55-60: In add(_ item: BlocklistItem) normalize the incoming value
by trimming whitespace/newlines and then lowercasing before doing the duplicate
check and before appending; specifically, compute a normalizedValue =
item.value.trimmingCharacters(in: .whitespacesAndNewlines) and ensure it's
non-empty (reject/return if empty) and use normalizedValue.lowercased() in the
entries.contains(...) comparison and when creating/persisting the stored
BlocklistItem so duplicates like "Taylor Swift " are caught and empty values are
not stored.
In `@apps/native/wolfwave/Services/SongRequest/SongRequestService.swift`:
- Around line 218-234: The recursive call in playNextInQueue on a generic
playback error can cause unbounded recursion or tight loops if many items fail;
modify playNextInQueue to avoid recursion by implementing a bounded retry/skip
mechanism: add a failure counter or maxRetries parameter tracked per invocation
or per queue item (e.g., attach a retryCount to the dequeued item), increment it
when musicController.playNow throws (except PlaybackError.musicAppNotRunning),
and if retryCount exceeds a configurable max (or after N consecutive failures)
either skip the item (log and continue) or stop iterating; alternatively convert
the recursive follow-up (await playNextInQueue()) into a loop that processes the
next item until successful or limit reached, and preserve current behavior for
PlaybackError.musicAppNotRunning by re-queuing with insertAtHead and
clearNowPlaying as before.
In `@apps/native/wolfwave/Services/SongRequest/SongSearchResolver.swift`:
- Around line 67-100: The logs in SongSearchResolver (inside resolve(url:) and
resolveText(_:)) currently emit raw user-controlled input (urlString and query);
update these Log.debug calls to redact PII by replacing the raw values with a
sanitized/stable identifier (e.g., a hashed or truncated fingerprint) and/or a
constant token, and use the app's structured logging helpers (emoji prefixes/PII
redaction) from Logger.swift; specifically change the Log.debug calls that
reference urlString in resolve(url:) and the one that logs query in
resolveText(_:) to log only the redacted identifier and any non-sensitive
context.
- Around line 84-88: The match arm for case .found in SongSearchResolver
currently force-unwraps artist when building searchQuery; change this to safe
optional binding (e.g., use if let artist = artist { ... } else { ... } or guard
let) to construct searchQuery without using !, then call
resolveText(searchQuery) as before; update the case .found(let title, let
artist) block to use the safe unwrap instead of the ternary with artist!.
In `@apps/native/wolfwave/Services/Twitch/Commands/QueueCommand.swift`:
- Around line 38-66: The code in QueueCommand.swift reads queue.isEmpty,
queue.nowPlaying and queue.items separately which can race with concurrent
mutations; add a single locked snapshot API on SongRequestQueue (e.g.,
SongRequestQueue.snapshot() or currentSnapshot()) that returns an immutable
struct containing nowPlaying, items (and derived isEmpty/count) while holding
the existing lock used by positions(for:). Update QueueCommand (the handler that
currently reads queue.nowPlaying and queue.items) to call this snapshot API once
and build the response from that snapshot instead of accessing queue properties
individually.
In `@apps/native/wolfwave/Views/Onboarding/OnboardingAppleMusicStepView.swift`:
- Around line 45-85: authStatus currently falls through to the request button
for the .restricted case; add an explicit else if authStatus == .restricted
branch (next to the .authorized and .denied branches) that mirrors the denied UI
pattern (e.g., a VStack/HStack with Image(systemName: "lock.shield.fill") or
similar, explanatory Text that the device policy restricts Apple Music access
and how to contact an administrator or check device management settings), ensure
the restricted UI does not show the Grant button or call
MusicAuthorization.request(), and include appropriate
accessibilityLabel/accessibilityIdentifier similar to the denied branch to keep
behavior consistent with SuccessFeedbackRow and isRequesting usage.
In `@apps/native/wolfwave/Views/Onboarding/OnboardingView.swift`:
- Around line 119-140: Add accessibility identifiers to the footer buttons so UI
tests can target them: for the Back button (inside the if !viewModel.isFirstStep
block around Button("Back")) add
.accessibilityIdentifier("onboarding_back_button") alongside the existing
.accessibilityLabel and .accessibilityHint, and for the Skip All button (inside
the if !viewModel.isLastStep block around Button("Skip All")) add
.accessibilityIdentifier("onboarding_skip_all_button") alongside its label and
hint; keep the existing calls to cancelTwitchOAuthIfNeeded() and
finishOnboarding() unchanged.
- Around line 196-197: shouldShowSkip currently reads
MusicAuthorization.currentStatus directly which SwiftUI doesn't observe; lift
the authorization into shared parent state so the footer updates when
authorization changes. Add an `@State` property (e.g.,
appleMusicAuthorizationStatus) in OnboardingView initialized from
MusicAuthorization.currentStatus, change shouldShowSkip to read that `@State`
instead of MusicAuthorization.currentStatus, pass a Binding of that state into
OnboardingAppleMusicStepView, and update that binding from the child when
calling MusicAuthorization.request() (or update the parent state in the request
completion) so both the footer (Skip button) and the child stay in sync.
In `@apps/native/WolfWaveTests/SongRequestCommandTests.swift`:
- Around line 353-379: The tests currently pass because SkipCommand and
ClearQueueCommand no-op when their songRequestService is nil; bind a stub/mocked
SongRequestService to the command instances (e.g., set the songRequestService
property on SkipCommand and ClearQueueCommand) before calling execute, and
assert that a viewer (context.isModerator/isBroadcaster false) does not invoke
the stub while a privileged user (isModerator or isBroadcaster true) does; the
stub should record whether its skip/clear method was called so you can assert
replyCalled or service method invocation for both viewer and
moderator/broadcaster cases.
- Around line 128-133: The test testDispatcherSkipsDisabledCommand currently can
pass for the wrong reason because a fresh BotCommandDispatcher may return nil
when no queue provider is wired; update the test to deterministically exercise
the enabled/disabled gate by either (a) registering a stub command/handler on
BotCommandDispatcher that returns a known non-nil result for "!queue" and then
assert that setting UserDefaults.standard.set(false, forKey:
AppConstants.UserDefaults.queueCommandEnabled) causes processMessage("!queue")
to return nil, or (b) explicitly bind a test queue provider to the dispatcher
before toggling AppConstants.UserDefaults.queueCommandEnabled so the only
variable is the enabled flag; reference BotCommandDispatcher,
processMessage(_:), testDispatcherSkipsDisabledCommand, and
AppConstants.UserDefaults.queueCommandEnabled when making the change.
In `@apps/native/WolfWaveTests/SongRequestServiceTests.swift`:
- Around line 32-33: The mock AppleMusicController currently hardcodes
search(query:) and resolve(url:) to return .notFound, preventing processRequest
tests (e.g., testRequestWhileMusicAppClosedBuffers,
testPlayNextInQueueRequeuesItemWhenMusicAppNotRunning,
testHoldBlocksAutoPlayOnRequest) from exercising buffering/hold/playback
branches; change the mock to allow configuring the search result (either by
adding an initializer parameter or a mutable property on the mock) and/or accept
an injectable resolver/stub so tests can set it to return a found song
(AppleMusicController.SearchResult.found) when needed, and update affected tests
to set that configured result before calling processRequest(...).
In `@CHANGELOG.md`:
- Line 15: The CHANGELOG.md entry lists the commands `!hold`, `!resume`, and
`!unhold` but the docs changelog only lists `!hold` and `!resume`; update the
docs changelog to include `!unhold` so both changelogs are consistent (add
`!unhold` to the same list/line that currently contains `!hold / !resume` in the
changelog.mdx entry), or alternatively remove `!unhold` from CHANGELOG.md if
that command shouldn’t be documented; ensure the final text uses the exact
command token `!unhold` so both files match.
---
Nitpick comments:
In `@apps/docs/app/`(home)/page.tsx:
- Around line 224-234: The map over tickerItems uses the array index as the
React key which is unclear; update the key in the .map callback that renders
.ticker-item to a more descriptive/compound key (e.g., derive from tickerItems'
value plus the index or a unique id) so each <span key=...
className="ticker-item"> uses a stable, meaningful key instead of just i; locate
the map over [...tickerItems, ...tickerItems, ...tickerItems] and replace
key={i} with a compound key like `${item}-${i}` or an actual unique identifier
from the ticker data.
In `@apps/native/wolfwave/Services/SongRequest/AppleMusicController.swift`:
- Around line 210-217: The AppleScript URL is currently interpolated directly
from song.url (musicURL.absoluteString) before calling
runAppleScriptPreservingFocus; to harden it, pass the URL string through
sanitizeForAppleScript (e.g., use
sanitizeForAppleScript(musicURL.absoluteString)) and then interpolate that
sanitized value into the script so the Music open location string is escaped
safely; update the code around song.url, musicURL, and
runAppleScriptPreservingFocus to use the sanitized urlString.
In `@apps/native/wolfwave/Services/SongRequest/LinkResolverService.swift`:
- Around line 66-75: extractURL(from:) is fragile because it splits on spaces
and can return URLs with trailing punctuation; replace the simple split logic
with a robust URL detector: use NSDataDetector configured for .link to find the
first URL in the input text and return its absoluteString (or, if you prefer to
keep manual parsing, trim common trailing punctuation characters like . , ! ? :
; ) ] } and surrounding quotes from the detected token). Update the
extractURL(from:) implementation to use NSDataDetector (NSTextCheckingResult)
and return the detector's first match URL to avoid including trailing
punctuation.
- Around line 45-47: The isSpotifyLink(_:) helper currently treats any
spotify.link/ URL as a track, which is incorrect; update
LinkResolverService.isSpotifyLink to resolve short URLs (follow HTTP redirect)
and then inspect the final URL's path for the explicit track pattern (e.g.,
contains "open.spotify.com/track/") before returning true, or alternatively
limit the check to the canonical track pattern only and add a comment/note that
only track links are supported; use a simple URLSession HEAD/GET to follow
redirects and perform the canonical check inside isSpotifyLink so callers get a
reliable true/false for tracks.
In `@apps/native/wolfwave/Services/SongRequest/SongBlocklist.swift`:
- Around line 84-96: The load() currently swallows JSON decode failures and
leaves a corrupt blob under storageKey so subsequent loads always fail; modify
load() to catch and log the decoding error (capture the thrown Error from
JSONDecoder.decode), and if decoding fails take remedial action such as removing
or moving the bad data (e.g. removeObject(forKey: storageKey) or copy it to a
quarantine key like "corrupt_\(storageKey)") and then set entries = [] under the
same lock used by save() (lock.withLock { entries = [] }) so the app can
recover; also ensure save() still writes the new empty or repaired entries
normally.
In `@apps/native/wolfwave/Services/Twitch/Commands/AsyncBotCommand.swift`:
- Line 33: The execute signature's reply closure should be annotated as
`@Sendable` to satisfy Swift concurrency safety: update the
AsyncBotCommand.execute declaration so the reply parameter is declared as reply:
`@Sendable` `@escaping` (String) -> Void (and propagate this change to any
protocol/type declarations and callers that conform to or call
AsyncBotCommand.execute), then fix any calling sites to pass a
`@Sendable-compatible` closure (or wrap non-sendable captures) so the code
compiles under strict concurrency checking.
In `@apps/native/wolfwave/Services/Twitch/Commands/BotCommandDispatcher.swift`:
- Around line 144-145: The loop in BotCommandDispatcher that does `for trigger
in triggers { let triggerLowered = trigger.lowercased() }` performs a redundant
lowercasing because the triggers come from the BotCommand extension's
allTriggers property which already returns lowercased strings; remove the extra
`lowercased()` call and use `trigger` directly (or rename `triggerLowered` to
`trigger` where it's referenced) to simplify the code and avoid unnecessary
allocations while keeping references to the loop over `triggers`,
`triggerLowered`, and the `allTriggers` property in mind.
In `@apps/native/wolfwave/Services/Twitch/Commands/HoldCommand.swift`:
- Around line 14-24: HoldCommand is missing user-configurable keys so it can't
be disabled or aliased; add the same properties used by
SkipCommand/ClearQueueCommand: implement var enabledKey: String and var
aliasesKey: String in the HoldCommand class, returning the appropriate
UserDefaults keys (e.g. AppConstants.UserDefaults.holdCommandEnabled and
AppConstants.UserDefaults.holdCommandAliases), and add the new
holdCommandAliases and holdCommandEnabled entries to AppConstants.UserDefaults
to expose alias and enable/disable settings.
In `@apps/native/wolfwave/Views/Shared/WhatsNewView.swift`:
- Around line 21-26: The feature list in WhatsNewView uses display titles which
makes accessibility identifiers brittle; add a stable id field to each feature
tuple/struct (e.g., change the array entries to include an "id" like
"song_requests", "hold_mode", etc.), update the model used by WhatsNewView (the
features collection) to include that id, and change the code that currently
derives identifiers from feature.title (referenced in WhatsNewView and at the
identifier construction site around where feature.title is used) to use
feature.id when building accessibility identifiers and test selectors so
identifiers remain stable when titles change.
- Around line 21-26: In WhatsNewView.swift update the short-description strings
in the tuples used by WhatsNewView (the list entries like the ("music.mic",
.pink, "Song Requests", "...") tuples) to be single, punchy clauses instead of
full sentences—e.g. condense each description to one fragment (3–6 words) that
summarizes the feature—so replace the long fourth-field strings for Song
Requests, Hold Mode, Queue Controls, Live Queue View, Fallback Playlist, and No
Focus-Steal with much shorter phrases to improve scanability.
In `@apps/native/wolfwave/Views/SongRequest/SongRequestQueueView.swift`:
- Around line 202-251: The action buttons in the actionButtons view (the Skip
Button, the Hold/Resume Button and the Clear Queue Button plus the confirmation
dialog action) are missing accessibility identifiers/labels; update each Button
(the one that calls service?.skip(), the one that calls
service?.setHold(!isHeld), and the Clear Queue Button and its destructive
confirmation Button) to add .accessibilityIdentifier("<unique_id>") and
.accessibilityLabel("<readable label>") (use clear ids like "skipButton",
"holdResumeButton", "clearQueueButton", "confirmClearQueueButton") so UI tests
can reliably find them while keeping existing behavior intact.
- Around line 279-284: The timer callback uses DispatchQueue.main.async; replace
it with Swift concurrency by running the refresh on the main actor via Task. In
startRefresh (and the scheduledTimer closure) call Task { await MainActor.run {
refreshState() } } instead of DispatchQueue.main.async { refreshState() }; if
refreshState must be async or touch UI, consider annotating it `@MainActor` or
making it async so the await/MainActor.run usage is appropriate. Ensure
refreshTimer and startRefresh keep the Timer.scheduledTimer invocation but
invoke refreshState through Task+MainActor.
In `@apps/native/wolfwave/Views/SongRequest/SongRequestSettingsView.swift`:
- Around line 181-194: Replace the DispatchQueue delayed re-check in the
.onAppear handler with an async/await Task using Task.sleep to wait 1 second and
then call refreshTwitchState; specifically, inside the .onAppear block (where
blocklist, musicAuthStatus = MusicAuthorization.currentStatus and
refreshTwitchState() are set) start a Task that awaits Task.sleep(nanoseconds:
1_000_000_000) and then calls refreshTwitchState(), ensuring the call runs on
the main actor if refreshTwitchState mutates UI state (e.g., mark the Task
`@MainActor` or call await MainActor.run { refreshTwitchState() } as appropriate);
leave the NotificationCenter .onReceive handling with updateTwitchState
untouched.
In `@apps/native/WolfWaveTests/OnboardingViewModelTests.swift`:
- Around line 139-159: Extend the testFullNavigationCycleForwardAndBack test to
include the new .appleMusicAccess step: after the current assertion for
.obsWidget call viewModel.goToNextStep() and assert viewModel.currentStep ==
.appleMusicAccess, then continue the backward assertions by calling
viewModel.goToPreviousStep() and asserting it returns to .obsWidget before
continuing back to .discordConnect, .twitchConnect, and .welcome; update the
sequence of viewModel.goToNextStep()/goToPreviousStep() calls and their
XCTAssertEqual checks to cover the full forward-and-back round trip including
.appleMusicAccess.
In `@apps/native/WolfWaveTests/SongRequestServiceTests.swift`:
- Around line 304-317: The test testAutoAdvanceDoesNotFireWhenPaused currently
uses a real 2.5s sleep; change the design so the test can run synchronously by
either (A) making SongRequestService accept an injectable polling interval or
clock (e.g., add a pollingInterval parameter to the initializer and use that in
startPlaybackMonitoring()) so the test can set a tiny interval, or (B) expose a
single monitoring tick API on SongRequestService (e.g., monitorTick() or
processPlaybackTick()) that startPlaybackMonitoring() calls repeatedly; update
the test to call that single tick directly after setting
mockController.isPlaying = false and mockController.isPaused = true and assert
no dequeue/advance occurred, then call stopPlaybackMonitoring() as before.
🪄 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: a6919454-feac-479f-a2fe-da5cddd055e0
📒 Files selected for processing (42)
CHANGELOG.mdapps/docs/app/(home)/page.tsxapps/docs/app/global.cssapps/docs/app/layout.tsxapps/docs/content/docs/changelog.mdxapps/native/WolfWaveTests/OnboardingViewModelEdgeCaseTests.swiftapps/native/WolfWaveTests/OnboardingViewModelTests.swiftapps/native/WolfWaveTests/SongRequestCommandTests.swiftapps/native/WolfWaveTests/SongRequestQueueTests.swiftapps/native/WolfWaveTests/SongRequestServiceTests.swiftapps/native/wolfwave.xcodeproj/project.pbxprojapps/native/wolfwave/Core/AppConstants+Notifications.swiftapps/native/wolfwave/Core/AppConstants.swiftapps/native/wolfwave/Core/AppDelegate+MenuBar.swiftapps/native/wolfwave/Core/AppDelegate+Services.swiftapps/native/wolfwave/Core/BlocklistItem.swiftapps/native/wolfwave/Core/SongRequestItem.swiftapps/native/wolfwave/Services/SongRequest/AppleMusicController.swiftapps/native/wolfwave/Services/SongRequest/LinkResolverService.swiftapps/native/wolfwave/Services/SongRequest/SongBlocklist.swiftapps/native/wolfwave/Services/SongRequest/SongRequestQueue.swiftapps/native/wolfwave/Services/SongRequest/SongRequestService.swiftapps/native/wolfwave/Services/SongRequest/SongSearchResolver.swiftapps/native/wolfwave/Services/Twitch/Commands/AsyncBotCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/BotCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/BotCommandContext.swiftapps/native/wolfwave/Services/Twitch/Commands/BotCommandDispatcher.swiftapps/native/wolfwave/Services/Twitch/Commands/ClearQueueCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/HoldCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/MyQueueCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/QueueCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/SkipCommand.swiftapps/native/wolfwave/Services/Twitch/Commands/SongRequestCommand.swiftapps/native/wolfwave/Services/Twitch/TwitchChatService.swiftapps/native/wolfwave/Views/Onboarding/OnboardingAppleMusicStepView.swiftapps/native/wolfwave/Views/Onboarding/OnboardingView.swiftapps/native/wolfwave/Views/Onboarding/OnboardingViewModel.swiftapps/native/wolfwave/Views/SettingsView.swiftapps/native/wolfwave/Views/Shared/WhatsNewView.swiftapps/native/wolfwave/Views/SongRequest/SongRequestQueueView.swiftapps/native/wolfwave/Views/SongRequest/SongRequestSettingsView.swiftapps/native/wolfwave/WolfWaveApp.swift
Comment on lines
433
to
435
| CODE_SIGN_ENTITLEMENTS = wolfwave/wolfwave.entitlements; | ||
| "CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development"; | ||
| CODE_SIGN_IDENTITY = "Apple Development"; | ||
| CODE_SIGN_STYLE = Automatic; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For a macOS app distributed outside the App Store with Sparkle auto-updates, what code-signing identity should the Xcode Release configuration use for archiving and notarization: Apple Development, Apple Distribution, or Developer ID Application?
💡 Result:
For a macOS app distributed outside the App Store with Sparkle auto-updates, the Xcode Release configuration should use Developer ID Application for archiving and notarization.
Citations:
- 1: https://developer.apple.com/documentation/security/notarizing-macos-software-before-distribution
- 2: https://developer.apple.com/documentation/xcode/creating-distribution-signed-code-for-the-mac
- 3: https://developer.apple.com/documentation/security/resolving-common-notarization-issues
- 4: https://developers.apple.com/developer-id
Change Release target signing identity from "Apple Development" to "Developer ID Application".
For a macOS app distributed outside the App Store with Sparkle auto-updates, using "Apple Development" in the Release configuration will produce a dev-signed app instead of a release-signed artifact suitable for archiving, notarization, and distribution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/wolfwave.xcodeproj/project.pbxproj` around lines 433 - 435, The
Release build configuration is using CODE_SIGN_IDENTITY = "Apple Development"
which signs release artifacts with a dev identity; locate the Release
configuration block in the project.pbxproj where CODE_SIGN_IDENTITY is set and
change the value to "Developer ID Application" so Release builds are signed with
the Developer ID Application identity (keep CODE_SIGN_ENTITLEMENTS and
CODE_SIGN_STYLE unchanged and ensure this change only applies to the Release
configuration).
Comment on lines
+55
to
+60
| func add(_ item: BlocklistItem) { | ||
| lock.withLock { | ||
| // Avoid duplicates | ||
| guard !entries.contains(where: { | ||
| $0.type == item.type && $0.value.lowercased() == item.value.lowercased() | ||
| }) else { return } |
There was a problem hiding this comment.
Normalize blocklist values before duplicate checks.
lowercased() alone still treats leading/trailing whitespace as distinct, so "Taylor Swift " can be saved as a separate entry and then never match normal metadata. Trim whitespace/newlines and reject empty values before storing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/wolfwave/Services/SongRequest/SongBlocklist.swift` around lines
55 - 60, In add(_ item: BlocklistItem) normalize the incoming value by trimming
whitespace/newlines and then lowercasing before doing the duplicate check and
before appending; specifically, compute a normalizedValue =
item.value.trimmingCharacters(in: .whitespacesAndNewlines) and ensure it's
non-empty (reject/return if empty) and use normalizedValue.lowercased() in the
entries.contains(...) comparison and when creating/persisting the stored
BlocklistItem so duplicates like "Taylor Swift " are caught and empty values are
not stored.
Comment on lines
+218
to
+234
| private func playNextInQueue() async { | ||
| guard let item = queue.dequeue(), let song = item.song else { return } | ||
|
|
||
| do { | ||
| try await musicController.playNow(song: song) | ||
| isPlayingFallback = false | ||
| Log.debug("SongRequestService: Now playing \"\(item.title)\" by \(item.artist) (requested by \(item.requesterUsername))", category: "SongRequest") | ||
| } catch PlaybackError.musicAppNotRunning { | ||
| // Music.app closed — put the item back at the front so it plays first when Music.app re-opens | ||
| queue.insertAtHead(item) | ||
| queue.clearNowPlaying() | ||
| Log.debug("SongRequestService: Music.app closed — \"\(item.title)\" re-queued at head", category: "SongRequest") | ||
| } catch { | ||
| Log.debug("SongRequestService: Failed to play \"\(item.title)\": \(error)", category: "SongRequest") | ||
| await playNextInQueue() | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential unbounded recursion on repeated playback failures.
If playNow throws an error other than musicAppNotRunning (line 230-233), the method recursively calls itself. If every song in the queue fails to play (e.g., due to a persistent network issue or invalid songs), this could drain the queue in a tight loop or, in extreme cases, cause a stack overflow.
Consider adding a retry limit or breaking the recursion:
🛡️ Suggested fix to prevent unbounded recursion
- private func playNextInQueue() async {
+ private func playNextInQueue(retryCount: Int = 0) async {
+ guard retryCount < 3 else {
+ Log.debug("SongRequestService: Max retries reached, stopping auto-advance", category: "SongRequest")
+ return
+ }
guard let item = queue.dequeue(), let song = item.song else { return }
do {
try await musicController.playNow(song: song)
isPlayingFallback = false
Log.debug("SongRequestService: Now playing \"\(item.title)\" by \(item.artist) (requested by \(item.requesterUsername))", category: "SongRequest")
} catch PlaybackError.musicAppNotRunning {
queue.insertAtHead(item)
queue.clearNowPlaying()
Log.debug("SongRequestService: Music.app closed — \"\(item.title)\" re-queued at head", category: "SongRequest")
} catch {
Log.debug("SongRequestService: Failed to play \"\(item.title)\": \(error)", category: "SongRequest")
- await playNextInQueue()
+ await playNextInQueue(retryCount: retryCount + 1)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/wolfwave/Services/SongRequest/SongRequestService.swift` around
lines 218 - 234, The recursive call in playNextInQueue on a generic playback
error can cause unbounded recursion or tight loops if many items fail; modify
playNextInQueue to avoid recursion by implementing a bounded retry/skip
mechanism: add a failure counter or maxRetries parameter tracked per invocation
or per queue item (e.g., attach a retryCount to the dequeued item), increment it
when musicController.playNow throws (except PlaybackError.musicAppNotRunning),
and if retryCount exceeds a configurable max (or after N consecutive failures)
either skip the item (log and continue) or stop iterating; alternatively convert
the recursive follow-up (await playNextInQueue()) into a loop that processes the
next item until successful or limit reached, and preserve current behavior for
PlaybackError.musicAppNotRunning by re-queuing with insertAtHead and
clearNowPlaying as before.
Comment on lines
+67
to
+100
| Log.debug("SongSearchResolver: Resolving link: \(urlString)", category: "SongRequest") | ||
|
|
||
| let result = await linkResolver.resolve(url: urlString) | ||
|
|
||
| switch result { | ||
| case .appleMusicURL(let url): | ||
| // Resolve Apple Music URL directly via MusicKit | ||
| let musicResult = await musicController.resolve(url: url) | ||
| switch musicResult { | ||
| case .found(let song): | ||
| return .found(song) | ||
| case .notFound: | ||
| return .linkNotFound | ||
| case .error(let message): | ||
| return .error(message) | ||
| } | ||
|
|
||
| case .found(let title, let artist): | ||
| // oEmbed gave us title/artist — search MusicKit | ||
| let searchQuery = artist != nil ? "\(title) \(artist!)" : title | ||
| Log.debug("SongSearchResolver: oEmbed resolved to: \(searchQuery)", category: "SongRequest") | ||
| return await resolveText(searchQuery) | ||
|
|
||
| case .notFound: | ||
| return .linkNotFound | ||
|
|
||
| case .error(let message): | ||
| return .error(message) | ||
| } | ||
| } | ||
|
|
||
| private func resolveText(_ query: String) async -> Result { | ||
| Log.debug("SongSearchResolver: Searching Apple Music for: \(query)", category: "SongRequest") | ||
|
|
There was a problem hiding this comment.
Redact user-controlled input before logging it.
Line 67 and Line 99 log raw URLs / search text coming straight from chat input. That will persist arbitrary user content in the app logs, including anything sensitive embedded in the query string. Please log a redacted form or a stable identifier instead.
As per coding guidelines "Use structured logging with emoji prefixes, file rotation (5MB cap), and PII redaction in Logger.swift".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/wolfwave/Services/SongRequest/SongSearchResolver.swift` around
lines 67 - 100, The logs in SongSearchResolver (inside resolve(url:) and
resolveText(_:)) currently emit raw user-controlled input (urlString and query);
update these Log.debug calls to redact PII by replacing the raw values with a
sanitized/stable identifier (e.g., a hashed or truncated fingerprint) and/or a
constant token, and use the app's structured logging helpers (emoji prefixes/PII
redaction) from Logger.swift; specifically change the Log.debug calls that
reference urlString in resolve(url:) and the one that logs query in
resolveText(_:) to log only the redacted identifier and any non-sensitive
context.
Comment on lines
+196
to
+197
| case .appleMusicAccess: | ||
| return MusicAuthorization.currentStatus != .authorized |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In SwiftUI, if a child view updates its own @StateafterMusicAuthorization.request(), will a parent sibling that reads MusicAuthorization.currentStatus directly recompute automatically, or should that authorization state be lifted into shared state?
💡 Result:
No, a parent sibling view that reads MusicAuthorization.currentStatus directly will not automatically recompute when a child view calls MusicAuthorization.request and the status changes. MusicAuthorization.currentStatus is a static property that is not observable by SwiftUI. Reading it directly in a view's body does not trigger recomputation when its value changes system-wide, as SwiftUI only tracks changes to @State, @ObservedObject, or @Bindable properties. The child's @State update only affects that child view's recomputation. Sibling views do not share state through the parent's recompute unless the parent has shared observable state that both depend on. To ensure the parent sibling updates reliably, lift the authorization state into shared parent state, such as an @State or @Observable property that is manually updated after request (using the returned status or by polling currentStatus), following Apple and community best practices for MusicKit in SwiftUI.
Citations:
- 1: https://exploringmusickit.com/musickit-permissions
- 2: https://wesleymatlock.com/musickit-in-swiftui-building-a-real-apple-music-player/
- 3: https://developer.apple.com/documentation/swiftui/managing-user-interface-state
- 4: https://medium.com/%40wesleymatlock/musickit-in-swiftui-building-a-real-apple-music-player-without-losing-your-mind-5e70a7f1ce88
🏁 Script executed:
# First, locate the OnboardingView.swift file
find . -name "OnboardingView.swift" -type fRepository: MrDemonWolf/wolfwave
Length of output: 124
🏁 Script executed:
# Read the OnboardingView.swift file around lines 196-197 and get context
wc -l apps/native/wolfwave/Views/Onboarding/OnboardingView.swiftRepository: MrDemonWolf/wolfwave
Length of output: 126
🏁 Script executed:
# Read the relevant section with more context (around lines 196-197)
sed -n '180,220p' apps/native/wolfwave/Views/Onboarding/OnboardingView.swiftRepository: MrDemonWolf/wolfwave
Length of output: 1167
🏁 Script executed:
# Search for shouldShowSkip definition
rg "shouldShowSkip" apps/native/wolfwave/Views/Onboarding/ -A 5 -B 2Repository: MrDemonWolf/wolfwave
Length of output: 1732
🏁 Script executed:
# Search for OnboardingAppleMusicStepView to understand its structure
rg "OnboardingAppleMusicStepView" apps/native/wolfwave/Views/Onboarding/ -A 10 -B 2Repository: MrDemonWolf/wolfwave
Length of output: 3901
🏁 Script executed:
# Find the footer usage to understand how it reads the state
rg "footer|shouldShowSkip" apps/native/wolfwave/Views/Onboarding/OnboardingView.swift -B 2 -A 2Repository: MrDemonWolf/wolfwave
Length of output: 529
🏁 Script executed:
# Read the full OnboardingAppleMusicStepView.swift
cat -n apps/native/wolfwave/Views/Onboarding/OnboardingAppleMusicStepView.swiftRepository: MrDemonWolf/wolfwave
Length of output: 4471
🏁 Script executed:
# Search for MusicAuthorization.request() in the onboarding views
rg "MusicAuthorization.request" apps/native/wolfwave/Views/Onboarding/ -A 5 -B 2Repository: MrDemonWolf/wolfwave
Length of output: 1014
🏁 Script executed:
# Check the overall structure of OnboardingView to see the footer and step view hierarchy
sed -n '1,100p' apps/native/wolfwave/Views/Onboarding/OnboardingView.swiftRepository: MrDemonWolf/wolfwave
Length of output: 3540
🏁 Script executed:
# Check the navigationBar section where shouldShowSkip is used
sed -n '120,180p' apps/native/wolfwave/Views/Onboarding/OnboardingView.swiftRepository: MrDemonWolf/wolfwave
Length of output: 2467
🏁 Script executed:
# Check the footer/navigation bar more specifically
sed -n '140,165p' apps/native/wolfwave/Views/Onboarding/OnboardingView.swiftRepository: MrDemonWolf/wolfwave
Length of output: 995
Lift Apple Music auth into shared parent state so the footer updates reliably.
The shouldShowSkip computed property reads MusicAuthorization.currentStatus directly, which SwiftUI does not track for changes. When OnboardingAppleMusicStepView calls MusicAuthorization.request() and updates its local @State, the parent's footer (Skip button) will not recompute automatically. The button can remain disabled even after authorization succeeds until an unrelated state change triggers the parent to refresh.
Move the authorization status into shared parent state (e.g., @State in OnboardingView) and pass it down to the child view as a binding so both the footer and step view stay in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/wolfwave/Views/Onboarding/OnboardingView.swift` around lines 196
- 197, shouldShowSkip currently reads MusicAuthorization.currentStatus directly
which SwiftUI doesn't observe; lift the authorization into shared parent state
so the footer updates when authorization changes. Add an `@State` property (e.g.,
appleMusicAuthorizationStatus) in OnboardingView initialized from
MusicAuthorization.currentStatus, change shouldShowSkip to read that `@State`
instead of MusicAuthorization.currentStatus, pass a Binding of that state into
OnboardingAppleMusicStepView, and update that binding from the child when
calling MusicAuthorization.request() (or update the parent state in the request
completion) so both the footer (Skip button) and the child stay in sync.
Comment on lines
+128
to
+133
| func testDispatcherSkipsDisabledCommand() { | ||
| UserDefaults.standard.set(false, forKey: AppConstants.UserDefaults.queueCommandEnabled) | ||
| let dispatcher = BotCommandDispatcher() | ||
| let result = dispatcher.processMessage("!queue") | ||
| XCTAssertNil(result) | ||
| } |
There was a problem hiding this comment.
This does not actually isolate the enabled/disabled gate.
A fresh BotCommandDispatcher can still return nil for !queue when no queue provider is wired, so this passes even if queueCommandEnabled is ignored. Please use a stub command with a deterministic response, or bind the queue before asserting the disabled path.
Based on learnings "Prioritize testing pure logic (Bot command parsing, version comparison, state machines) and mock network and system services to keep tests fast and reliable".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/WolfWaveTests/SongRequestCommandTests.swift` around lines 128 -
133, The test testDispatcherSkipsDisabledCommand currently can pass for the
wrong reason because a fresh BotCommandDispatcher may return nil when no queue
provider is wired; update the test to deterministically exercise the
enabled/disabled gate by either (a) registering a stub command/handler on
BotCommandDispatcher that returns a known non-nil result for "!queue" and then
assert that setting UserDefaults.standard.set(false, forKey:
AppConstants.UserDefaults.queueCommandEnabled) causes processMessage("!queue")
to return nil, or (b) explicitly bind a test queue provider to the dispatcher
before toggling AppConstants.UserDefaults.queueCommandEnabled so the only
variable is the enabled flag; reference BotCommandDispatcher,
processMessage(_:), testDispatcherSkipsDisabledCommand, and
AppConstants.UserDefaults.queueCommandEnabled when making the change.
Comment on lines
+353
to
+379
| func testSkipCommandSilentlyIgnoresNonPrivileged() { | ||
| let command = SkipCommand() | ||
| var replyCalled = false | ||
| command.execute( | ||
| message: "!skip", | ||
| context: BotCommandContext( | ||
| userID: "1", username: "viewer", | ||
| isModerator: false, isBroadcaster: false, | ||
| isSubscriber: false, messageID: "m" | ||
| ) | ||
| ) { _ in replyCalled = true } | ||
| XCTAssertFalse(replyCalled) | ||
| } | ||
|
|
||
| func testClearQueueCommandSilentlyIgnoresNonPrivileged() { | ||
| let command = ClearQueueCommand() | ||
| var replyCalled = false | ||
| command.execute( | ||
| message: "!clearqueue", | ||
| context: BotCommandContext( | ||
| userID: "1", username: "viewer", | ||
| isModerator: false, isBroadcaster: false, | ||
| isSubscriber: false, messageID: "m" | ||
| ) | ||
| ) { _ in replyCalled = true } | ||
| XCTAssertFalse(replyCalled) | ||
| } |
There was a problem hiding this comment.
Bind a service before asserting the privilege check.
SkipCommand and ClearQueueCommand can no-op when songRequestService is nil, so these tests pass even if context.isPrivileged is never consulted. Inject a stub service and compare viewer vs moderator/broadcaster behavior instead.
Based on learnings "Prioritize testing pure logic (Bot command parsing, version comparison, state machines) and mock network and system services to keep tests fast and reliable".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/WolfWaveTests/SongRequestCommandTests.swift` around lines 353 -
379, The tests currently pass because SkipCommand and ClearQueueCommand no-op
when their songRequestService is nil; bind a stub/mocked SongRequestService to
the command instances (e.g., set the songRequestService property on SkipCommand
and ClearQueueCommand) before calling execute, and assert that a viewer
(context.isModerator/isBroadcaster false) does not invoke the stub while a
privileged user (isModerator or isBroadcaster true) does; the stub should record
whether its skip/clear method was called so you can assert replyCalled or
service method invocation for both viewer and moderator/broadcaster cases.
Comment on lines
+32
to
+33
| func search(query: String) async -> AppleMusicController.SearchResult { .notFound } | ||
| func resolve(url: URL) async -> AppleMusicController.SearchResult { .notFound } |
There was a problem hiding this comment.
Make the mock search path configurable.
With search(query:) and resolve(url:) hardcoded to .notFound, the processRequest(...) tests below never reach buffering / hold / playback branches. That makes cases like testRequestWhileMusicAppClosedBuffers, testPlayNextInQueueRequeuesItemWhenMusicAppNotRunning, and testHoldBlocksAutoPlayOnRequest pass even if the post-search logic is broken. Please make the mock return a found song for those scenarios, or inject a stub resolver that does.
Based on learnings "Prioritize testing pure logic (Bot command parsing, version comparison, state machines) and mock network and system services to keep tests fast and reliable".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/WolfWaveTests/SongRequestServiceTests.swift` around lines 32 -
33, The mock AppleMusicController currently hardcodes search(query:) and
resolve(url:) to return .notFound, preventing processRequest tests (e.g.,
testRequestWhileMusicAppClosedBuffers,
testPlayNextInQueueRequeuesItemWhenMusicAppNotRunning,
testHoldBlocksAutoPlayOnRequest) from exercising buffering/hold/playback
branches; change the mock to allow configuring the search result (either by
adding an initializer parameter or a mutable property on the mock) and/or accept
an injectable resolver/stub so tests can set it to return a found song
(AppleMusicController.SearchResult.found) when needed, and update affected tests
to set that configured result before calling processRequest(...).
| - **`!queue` / `!myqueue`** — show the full request queue or a viewer's own requests in chat. | ||
| - **`!skip` / `!next`** — mod/broadcaster-only command to skip the current request. | ||
| - **`!clearqueue`** — mod/broadcaster-only command to wipe the queue (with in-app confirmation dialog). | ||
| - **`!hold` / `!resume` / `!unhold`** — mod/broadcaster-only hold mode; new requests buffer without auto-playing so the streamer can curate before releasing the queue. |
There was a problem hiding this comment.
Minor inconsistency: !unhold command documented here but not in changelog.mdx.
This file lists !hold / !resume / !unhold but the docs changelog (apps/docs/content/docs/changelog.mdx line 18) only lists !hold / !resume. Consider aligning both changelogs for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 15, The CHANGELOG.md entry lists the commands `!hold`,
`!resume`, and `!unhold` but the docs changelog only lists `!hold` and
`!resume`; update the docs changelog to include `!unhold` so both changelogs are
consistent (add `!unhold` to the same list/line that currently contains `!hold /
!resume` in the changelog.mdx entry), or alternatively remove `!unhold` from
CHANGELOG.md if that command shouldn’t be documented; ensure the final text uses
the exact command token `!unhold` so both files match.
Editorial tech-luxe aesthetic — kinetic hero, liquid-glass now-playing frame, infinite integration ticker, asymmetric bento features with live Twitch chat mock, diagonal 3-step flow, terminal dev card, giant CTA. - layout.tsx: add JetBrains Mono + Instrument Serif fonts - global.css: mesh bg, grid lines, noise, kinetic headline, liquid-glass, ticker marquee, shimmer btn, bento cards, terminal window, reduced-motion - page.tsx: full rewrite into 6 sections, v2.0 song-requests teaser Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use dorny/paths-filter to detect native changes. When a PR only touches apps/docs/** or apps/marketing/**, the xcodebuild step is skipped and the job reports pass. Unblocks docs-only PRs while the song-requests test malloc crash is investigated separately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nathanialhenniges
force-pushed
the
claude/agitated-chandrasekhar
branch
from
f53aba5 to
6e58283
Compare
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
YOUR MUSIC, live everywhere.) using Unbounded + Instrument Serif italic + animated gradient clip, mesh background with grid lines + noise overlay, SVG live-wire waveform, mono kicker metadata row, liquid-glass now-playing frame with corner brackets and 3-output badges, staggered page-load reveal.APPLE MUSIC · TWITCH · DISCORD · OBS · SONG REQUESTS · WEBSOCKET API · MENU BAR · OPEN SOURCE) with mask-fade edges.Press play. We handle the rest.at kinetic size, shimmer primary button, mono proof pills.Why
The previous landing read like a 2023 SaaS template — centered hero, 4 equal feature cards, generic 3-step grid. With v1.2.0 shipped and v2.0 song requests incoming, the docs landing needs to sell WolfWave in the first two seconds and feel distinctively 2026. No new dependencies — pure CSS + existing stack (Fumadocs + Tailwind v4).
Files
apps/docs/app/(home)/page.tsx— full rewrite into 6 sectionsapps/docs/app/global.css— +~400 lines (mesh, grid-lines, noise, kinetic, liquid-glass, ticker, shimmer, bento, terminal, reduced-motion)apps/docs/app/layout.tsx— add JetBrains Mono + Instrument Serif fonts, expose as--font-mono+--font-serifTest plan
bun run build --filter docspasses (verified locally — 30 pages, clean)bun run types:checkpasses (verified locally)/— hero renders with fluid kinetic headline, mesh bg, now-playing pulses/docslink still resolves and Fumadocs shell untouchedprefers-reduced-motiondisables all motion🤖 Generated with Claude Code
Summary by CodeRabbit