feat: Add Discord Rich Presence integration via Local IPC - #452
feat: Add Discord Rich Presence integration via Local IPC#452anuragdeshpande wants to merge 14 commits into
Conversation
- Add get play queue command (returns play queue as JSON) - Add play track at index command (plays song by its 1-based index)
…ueue feat(scripting): expose play queue and allow playing track by index
# Conflicts: # Sources/Kaset/Services/Scripting/ScriptCommands.swift # Tests/KasetTests/ScriptCommandsTests.swift
- Implement DiscordLocalIPCService communicating via POSIX Unix domain sockets (discord-ipc-0...9) - Implement DiscordPresenceCoordinator reacting to track playback, changes, seek, and pauses - Add DiscordSettingsView with granular privacy toggles (title, artist, album, timestamps, artwork, listen button) - Document architectural decisions in ADR-0033 - Add comprehensive unit test suite for Discord presence payloads and lifecycle
There was a problem hiding this comment.
🟡 Changes recommended
Sandbox removal, IPC correctness, reconnect, privacy, localization, and release-verification issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Discord Rich Presence through local IPC, with playback coordination, privacy settings, tests, packaging changes, and an ADR.
Changes:
- Implements Discord IPC transport and presence payload coordination.
- Adds Discord settings, persistence, localization calls, and tests.
- Updates signing entitlements and architecture documentation.
File summaries
| File | Description |
|---|---|
Tests/KasetTests/MockDiscordPresenceService.swift |
Adds a mock presence service. |
Tests/KasetTests/DiscordPresenceTests.swift |
Tests payloads and coordinator behavior. |
Sources/Kaset/Views/DiscordSettingsView.swift |
Adds Discord controls and status UI. |
Sources/Kaset/Utilities/DiagnosticsLogger.swift |
Adds Discord logging category. |
Sources/Kaset/Services/SettingsManager.swift |
Persists Discord preferences. |
Sources/Kaset/Services/Discord/DiscordPresenceServiceProtocol.swift |
Defines presence models and transport API. |
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift |
Maps playback state to Discord activities. |
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift |
Implements socket discovery and IPC framing. |
Sources/Kaset/Resources/Kaset.sdef |
Adds an unrelated lyrics command declaration. |
Sources/Kaset/KasetApp.swift |
Initializes and exposes Discord integration. |
Scripts/build-app.sh |
Explicitly signs the main executable. |
Kaset.entitlements |
Removes App Sandbox entitlement. |
docs/adr/0033-discord-rich-presence.md |
Documents the integration architecture. |
Review details
Suppressed comments (1)
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:209
large_textalways publishes the video title whenever artwork is enabled, even when the independent title toggle is off. This leaks metadata the user explicitly disabled; use the filteredtitlevalue or omit the tooltip. The music builder has the same privacy defect at line 169.
large_text: video.title,
- Files reviewed: 13/13 changed files
- Comments generated: 19
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ements for packaging
There was a problem hiding this comment.
🔵 Needs a closer look
The sandbox transport is not viable as implemented, and unresolved IPC framing, reconnection, and privacy defects make release unsafe.
Review details
Suppressed comments (16)
Previously missed (2) — in code that hasn't changed since the last review.
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:150
- The album toggle is not independent: when
discordShowArtistis off butdiscordShowAlbumis on,albumis populated but no branch assigns it tostateText, so the enabled album is omitted.
if let artist, let album, !album.isEmpty {
stateText = "\(artist) • \(album)"
} else if let artist {
stateText = artist
}
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:324
- Discovery returns the first filesystem node rather than the first socket that accepts a connection. If
/tmp/discord-ipc-0is stale while Discord is active in$TMPDIR, all five retries select the same stale node and never reach the working socket. Iterate through candidates until one connects successfully.
let path = (dir as NSString).appendingPathComponent("discord-ipc-\(i)")
if fileManager.fileExists(atPath: path) {
self.logger.info("Discovered Discord socket at \(path)")
return path
Kaset.entitlements:31
- The App Sandbox file-path exception does not authorize Unix-domain socket operations outside the container. Since the packaged executable remains sandboxed, connecting to Discord’s socket at these paths will still be denied, so the core integration only works in unsandboxed development runs. This needs a supported helper/IPC or distribution architecture rather than a filesystem exception.
<key>com.apple.security.temporary-exception.files.absolute-path.read-write</key>
<array>
<string>/tmp/</string>
<string>/private/tmp/</string>
<string>/private/var/folders/</string>
Sources/Kaset/Resources/Kaset.sdef:74
KasetGetLyricsCommandhas no matching@objc(KasetGetLyricsCommand)implementation anywhere in the target, unlike the other SDEF commands. Invoking this newly advertised command will fail at runtime; either remove this unrelated entry or include the command implementation and tests.
<command name="get lyrics" code="Kastgtly" description="Get lyrics for the current track as JSON.">
<cocoa class="KasetGetLyricsCommand"/>
<result type="text" description="JSON string with lyrics, timed lines (if available), and currentLineIndex."/>
Sources/Kaset/Views/DiscordSettingsView.swift:15
- This and the other new Discord-specific localization keys are absent from
Localizable.xcstringsand the checked-in.lproj/Localizable.stringsmirrors, so every non-English locale falls back to English. Add all new keys to both localization sources as required by the repository’s localization convention;LocalizationCatalogParityTests.swift:69-93establishes mirror parity.
Toggle(String(localized: "Enable Discord Rich Presence"), isOn: self.$settings.discordPresenceEnabled)
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:169
- When title and album sharing are disabled but artwork remains enabled, this tooltip still sends
track.titleto Discord. That bypasses the title privacy control; the tooltip must use only already-filtered metadata.
large_text: album ?? track.title,
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:209
- This always sends the video title as the artwork tooltip even when
discordShowTitleis off, bypassing the advertised title privacy control. Use the filteredtitlevalue instead.
large_text: video.title,
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:201
- The video timestamp ignores the available
youtubePlayerService.progressandduration, always claiming playback began now and never supplying an end. Also,observePlaybackdoes not observe those fields, so video seeks do not trigger an update despite the PR’s seek-sync requirement. Build timestamps from the video clock and add seek-aware observation.
var timestamps: DiscordPresencePayload.Timestamps?
if self.settings.discordShowTimestamps {
let start = Int(Date().timeIntervalSince1970 * 1000)
timestamps = DiscordPresencePayload.Timestamps(start: start, end: nil)
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:185
- Unix stream reads are allowed to return 1–7 header bytes without EOF. Treating every short read as a disconnect drops valid fragmented frames; buffer or loop until exactly eight bytes are read, while retrying
EINTR.
var header = [UInt8](repeating: 0, count: 8)
let headerBytes = read(fd, &header, 8)
guard headerBytes == 8 else {
await self?.handleDisconnect()
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:169
- A stream
writemay return a positive short count, but this treats any nonnegative result as a complete frame, so Discord can receive a truncated packet. On Darwin, writing after the peer closes can also raiseSIGPIPEunless the socket is configured withSO_NOSIGPIPE. Configure the socket and loop until every byte is written, handlingEINTRand zero-byte writes.
let written = packet.withUnsafeBytes { ptr in
write(fd, ptr.baseAddress, packet.count)
}
if written < 0 {
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:207
- An unexpected socket EOF only changes the state to disconnected and never enters the retry state machine. In particular, active video playback has no observed progress changes to trigger another sync, so presence stays offline until another user action, contrary to the automatic-reconnect behavior documented by this PR.
private func handleDisconnect() {
self.closeConnection()
self.state = .disconnected
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:284
clearPresencedelegates toupdatePresence(nil), whose disconnected branch initiates a connection. Therefore a paused/disabled synchronization can connect to Discord merely to clear an already absent activity, and racing the Settings disable task can leave the service connected after the feature is turned off. Clearing while disconnected should be a no-op.
func clearPresence() async {
try? await self.updatePresence(nil)
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:10
- The PR identifies this as a temporary development application ID. Shipping it makes Kaset’s displayed Discord identity/assets dependent on an account the project does not control and allows that owner to revoke the integration; replace it with a project-owned Discord application ID and update the ADR before release.
static let defaultClientID = "1541148589269454989"
Tests/KasetTests/DiscordPresenceTests.swift:8
.serializedonly serializes tests within this suite. These tests mutate all Discord fields onSettingsManager.shared(and persistentUserDefaults) without restoring them, so concurrently running suites and later test runs inherit whichever values execute last. Capture and restore the settings/defaults in suite setup/teardown or inject isolated settings storage.
@Suite(.serialized, .tags(.service))
@MainActor
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:218
- When an activity update arrives while disconnected, this connects and then unconditionally returns, discarding the payload that triggered reconnection. A video can consequently reconnect successfully but expose no activity indefinitely because video progress is not observed. Cache/replay the latest payload after readiness or have the coordinator resync on connection.
// If disconnected, try to connect if user actively triggers
if case .disconnected = self.state {
await self.connect()
}
return
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:158
startis always reset to the current time instead of the playback start (now - currentPos). At 60 seconds into a track Discord therefore reports zero elapsed time, and observing every progress update continually resets that timer. Derive both timestamps from onenowvalue and the current playback position.
let start = Int(Date().timeIntervalSince1970 * 1000)
var end: Int?
if let duration = track.duration, duration > 0 {
let currentPos = self.playerService.progress
let remaining = max(0, duration - currentPos)
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Core IPC framing, lifecycle, rate handling, localization, and scripting issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (8)
Previously missed (3) — in code that hasn't changed since the last review.
Sources/Kaset/KasetApp.swift:348
- These playback callbacks duplicate the observation already started by
DiscordPresenceCoordinator.start()for the same player properties. A play/pause or track change launches two independent synchronizations and sends duplicate IPC frames, worsening Discord rate limiting. Keep a single observation owner, preferably the coordinator.
}
}
.defaultSize(width: MainWindowLayout.defaultWidth, height: MainWindowLayout.defaultHeight)
.windowResizability(.contentMinSize)
.handlesExternalEvents(matching: ["*"])
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:35
- The detached reader is canceled without closing or shutting down
socketFD. Cancellation does not interrupt a blocking POSIXread, so deallocating a service can leave both the descriptor and blocked task alive. Close/shutdown the descriptor as part of teardown before canceling the reader.
This issue also appears on line 216 of the same file.
self.clientID = clientID
}
deinit {
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:10
- This is the temporary development application ID called out in the PR description. Shipping it leaves Kaset's Discord application identity and asset configuration controlled by another developer and allows that integration to be changed or revoked independently of Kaset releases. Replace it with a project-owned Discord application ID and update the ADR before merge.
@Observable
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:82
- The service declares the connection established immediately after writing the handshake. Discord IPC requires waiting for the opcode-1
READYresponse; the read loop currently discards both opcode and body, so handshake rejection,CLOSE, RPC errors, andPINGframes are never handled. This can report a false Connected state and repeatedly send activity on an invalid session. Parse incoming frames, transition to.connectedonly afterREADY, and handle control/error opcodes.
do {
try await self.sendHandshake(fd: fd)
self.isConnected = true
self.state = .connected
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:220
handleDisconnectdoes not verify which descriptor reported the failure. A canceled reader can remain blocked afterdisconnect(), while a later connect installs a new socket; when the old reader finally fails, this method closes the new connection and starts another attempt. Pass the reader's descriptor/generation into this handler and ignore stale callbacks, while usingshutdownto wake canceled reads.
}
if readError {
await self?.handleDisconnect()
break
}
Tests/KasetTests/DiscordPresenceTests.swift:9
.serializedonly serializes tests within this suite, but these async tests mutateSettingsManager.sharedand persist those mutations without restoring them. Other suites can interleave at eachawaitand observe Discord settings changed by this suite; the repository already calls out this exact cross-suite singleton race inPlayerService.swift:26-28. Save/restore every modified setting and underlying default, or inject isolated settings storage.
@Suite(.serialized, .tags(.service))
@MainActor
struct DiscordPresenceTests {
Sources/Kaset/Resources/Kaset.sdef:74
- The scripting definition advertises
KasetGetLyricsCommand, but no matching@objccommand class exists inSources/Kaset/Services/Scripting/ScriptCommands.swiftor elsewhere. Invokingget lyricstherefore cannot dispatch to an implementation. Add the command handler and tests, or remove this unrelated dictionary entry.
<command name="get lyrics" code="Kastgtly" description="Get lyrics for the current track as JSON.">
<cocoa class="KasetGetLyricsCommand"/>
<result type="text" description="JSON string with lyrics, timed lines (if available), and currentLineIndex."/>
Sources/Kaset/Views/DiscordSettingsView.swift:15
- The Discord-specific localized literals (including the enable/source/privacy labels and connection descriptions) are absent from
Localizable.xcstringsand all checked-in.lprojmirrors, so they fall back to English. This violates the required workflow indocs/adr/0013-localization-strategy.md:30-34,71. Add those keys to the catalog and regenerate each mirror in this PR.
Section {
Toggle(String(localized: "Enable Discord Rich Presence"), isOn: self.$settings.discordPresenceEnabled)
- Files reviewed: 13/13 changed files
- Comments generated: 5
- Review effort level: Balanced
…andshake validation
…amp progress tests
There was a problem hiding this comment.
🟡 Changes recommended
IPC lifecycle, sandbox permissions, seek synchronization, localization, and scripting correctness have unresolved issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (12)
Previously missed (5) — in code that hasn't changed since the last review.
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:71
discoverAndConnectSocket()performs filesystem probes and blockingconnect(2)calls while this service is main-actor isolated. A stale socket or full listen backlog can block the app UI during startup and each retry. Run socket discovery/connection off the main actor or use nonblocking sockets with an explicit timeout, then publish state back on the actor.
self.retryCount += 1
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:53
- Canceling
readTaskdoes not interrupt its blocking POSIXread, and the loop does not re-check cancellation before callinghandleDisconnect(). A late callback from the old reader can therefore close a newly opened socket viacloseConnection()or reconnect immediately becausependingPayloadis retained, making manual Disconnect unreliable. Fence callbacks by connection generation/FD and explicitly wake the reader during shutdown.
func disconnect() async {
self.isExplicitlyDisconnected = true
self.pendingPayload = nil
self.retryTask?.cancel()
docs/adr/0033-discord-rich-presence.md:48
- The documented retry sequence does not match the implementation. Five total attempts produce only four delays (1s, 2s, 4s, 8s); after attempt 5 fails, the service enters
.errorimmediately and never waits 16s. Clarify that there are four retries, or change the retry budget/state labels to implement the listed five backoffs.
- **Automatic Reconnect**: When connection is lost or Discord starts after Kaset, the coordinator retries with exponential backoff (1s, 2s, 4s, 8s, 16s) up to 5 attempts.
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:164
- The core IPC transport has no tests: the new suite never instantiates
DiscordLocalIPCServiceor exercises frame encoding, partial reads, ping/pong, disconnects, or retries. Given the unsafe POSIX framing and retry state machine, add socket-pair/injected-transport tests before relying on this path in production.
while totalWritten < packet.count {
let written = write(fd, base.advanced(by: totalWritten), packet.count - totalWritten)
if written < 0 {
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:147
- The combined artist/album state is sent without enforcing Discord's 128-byte activity-text limit. Multiple artists plus an album can exceed that limit, causing Discord to reject the entire SET_ACTIVITY frame; the current reader also ignores the resulting error frame. Sanitize all outbound activity text by UTF-8 byte length before constructing the payload.
stateText = "\(artist) • \(album)"
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:66
- Video seeks have the same gap:
YouTubePlayerService.performSeekassignsprogressdirectly (YouTubePlayerService+Seeking.swift:53), but progress is not tracked here. A seek while playback continues therefore leaves Discord's timestamps stale. Add a seek-specific observable signal/callback and resync on it without turning every WebView progress sample into an IPC update.
_ = self.youtubePlayerService.duration
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:139
- This loop uses blocking
write(2), while handshake and presence updates invoke it from the main actor. If Discord stops draining the socket, Kaset's UI can hang indefinitely because no nonblocking mode or send timeout is configured. Move writes to dedicated I/O isolation or make the socket nonblocking and handle backpressure.
}
guard let json = try JSONSerialization.jsonObject(with: data) as? [String: Any],
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:225
- The read loop discards every normal frame (opcode 1). Discord reports READY and command failures such as invalid client IDs or rejected activities in these frames, so the service can remain
.connectedwhile no presence is being published and the UI receives no error. Decode frame events and correlate nonce responses, transitioning state or retrying on protocol errors.
var totalRead = 0
while totalRead < Int(length) {
let n = read(fd, &body[totalRead], Int(length) - totalRead)
Sources/Kaset/Views/DiscordSettingsView.swift:15
- The newly localized Discord labels are absent from both
Localizable.xcstringsand every checked-in.lprojmirror, so non-English builds fall back to English. This violates the repository's two-source localization contract documented byTests/KasetTests/LocalizationCatalogParityTests.swift:4-7. Add all strings from this view to the catalog and regenerate every mirror in the same change.
Toggle(String(localized: "Enable Discord Rich Presence"), isOn: self.$settings.discordPresenceEnabled)
Sources/Kaset/Resources/Kaset.sdef:74
- This scripting definition references
KasetGetLyricsCommand, but no matching@objc(KasetGetLyricsCommand)/NSScriptCommandimplementation exists anywhere in the repository; adjacent commands are backed by classes inScriptCommands.swift. Invokingget lyricswill therefore fail to resolve its handler. Add the command implementation and tests, or remove this unrelated declaration.
<command name="get lyrics" code="Kastgtly" description="Get lyrics for the current track as JSON.">
<cocoa class="KasetGetLyricsCommand"/>
<result type="text" description="JSON string with lyrics, timed lines (if available), and currentLineIndex."/>
Tests/KasetTests/DiscordPresenceTests.swift:371
- This assertion does not test the advertised progress offset: with
start = now - progressandend = now + duration - progress,end - startalways equals duration even if progress is ignored entirely. Assert each endpoint relative to a captured time window and the 30-second progress so timestamp regressions are detected.
title: "Time Track",
artists: [],
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:12
- This is the temporary development application ID called out in the PR description. Shipping it makes Kaset's displayed Discord application identity and registered assets depend on an external developer account that maintainers cannot control and that can be changed or revoked independently. Replace it with a maintainer-owned Discord application ID before release, or inject the production ID during packaging.
nonisolated static let defaultClientID = "1541148589269454989"
- Files reviewed: 30/30 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Socket authentication, sandbox scope, concurrent I/O, error handling, and seek synchronization remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:60
- Seek-only changes never trigger this observer: music seeks only update
PlayerService.progress(PlayerService+PlaybackControls.swift:774,787), while this tracking set watches neither progress nor a discrete seek signal. Discord therefore keeps advancing from the pre-seek timestamps until another playback event occurs, contrary to the advertised real-time seek updates. Add a discrete seek notification rather than observing every playback heartbeat.
private func observePlayback() {
withObservationTracking {
_ = self.playerService.currentTrack?.videoId
_ = self.playerService.currentTrack?.title
_ = self.playerService.isPlaying
Kaset.entitlements:32
- These directory exceptions apply even when Discord presence is disabled and grant read/write access far beyond the IPC nodes—especially the entire
/private/var/folderstree used by other applications for temporary and cached data. This materially weakens the sandbox for the whole app. Narrow the entitlement to the actual IPC boundary or isolate this integration in a narrowly entitled helper.
<string>/tmp/</string>
<string>/private/tmp/</string>
<string>/private/var/folders/</string>
<string>/var/folders/</string>
- Files reviewed: 30/30 changed files
- Comments generated: 7
- Review effort level: Balanced
…alization, seek detection, and state machine docs
There was a problem hiding this comment.
🔵 Needs a closer look
Playback transitions can remain unpublished, while IPC lifecycle and rejection handling still have correctness issues.
Review details
Suppressed comments (5)
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:94
- This condition does not detect the track/source/play-state changes named in the comment; it only detects position drift. For example,
play(video:)first resets progress to 0 while video is not yet playing, which takes the all-paused branch and clears presence; the first playing update near progress 0 then hasdelta <= 2.5, so the video presence can remain cleared until the 60-second refresh. Track identity and play-state changes need to sync immediately, with drift throttling applied only to progress-only updates.
// Only resync if there was a discrete change (seek > 2.5s, track/state change, or coarse 60s refresh)
if delta > 2.5 || elapsedRealTime > 60 || !self.playerService.isPlaying && !self.youtubePlayerService.isPlaying {
self.lastProgress = currentPos
self.lastSyncTime = now
await self.syncPresence()
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:58
- Cancelling
readTaskdoes not interrupt its synchronous POSIXread, andcloseConnection()closes the descriptor withoutshutdown. If the user reconnects before the old reader returns, the OS can reuse the same descriptor number; the stale reader then passessocketFD == fdand disconnects the new connection. Shut down the old socket to wake the read and use a connection-generation token rather than descriptor equality alone.
self.readTask?.cancel()
self.readTask = nil
self.retryCount = 0
self.closeConnection()
docs/adr/0033-discord-rich-presence.md:50
- This reset behavior is not implemented. After five failures the service is in
.error, whileupdatePresencereconnects only from.disconnected; track and privacy-setting changes merely callsyncPresence()and cannot reset retries. Document the manual Connect/master-toggle fallback described above instead.
- **Reset**: Any track play or manual settings toggle resets the attempt counter and re-triggers connection.
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:283
- Discord can reject a
SET_ACTIVITYframe while leaving IPC connected, but this handler only logs the rejection. The UI therefore continues to report “Connected & Active,” and the rejected pending payload is neither surfaced nor recovered, leaving stale or absent presence. Correlate the response nonce and expose a presence-update failure or retry policy.
private func handleDiscordError(_ message: String) {
self.logger.error("Discord IPC error frame: \(message, privacy: .public)")
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:101
- The failure reason is discarded, so a socket that is found but fails handshake validation or times out ultimately shows “Discord not detected…running.” That diagnosis is false when Discord is running and hides actionable causes such as an invalid application ID or protocol response. Preserve a localized failure category/reason for the terminal state.
private func handleConnectionFailure(reason _: String) async {
- Files reviewed: 30/30 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Socket lifecycle, reconnect behavior, and playback synchronization contain unresolved correctness issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (9)
Previously missed (6) — in code that hasn't changed since the last review.
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:423
SO_RCVTIMEOremains enabled after the handshake, but the long-lived read loop treats everyreadPacketerror as a disconnect. Once Discord is idle for two seconds,readreturnsEAGAIN/EWOULDBLOCK, so an otherwise healthy connection is closed and immediately re-established repeatedly. Limit this deadline to the handshake (or explicitly handle idle timeouts without losing framing) before starting the read loop.
setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, socklen_t(MemoryLayout<timeval>.size))
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:101
- The failure reason is discarded, so five valid socket connections with a rejected handshake (for example, an invalid client ID or malformed READY response) are ultimately reported as “Discord not detected.” Preserve the failure category and show a localized handshake/protocol error when Discord was found, reserving this message for discovery failures.
private func handleConnectionFailure(reason _: String) async {
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:77
- Discovery stops at the first socket path whose Unix
connectsucceeds, before the DiscordREADYhandshake is validated. Ifdiscord-ipc-0is an accepting stale/non-Discord endpoint while the real client is ondiscord-ipc-1, every retry selects-0again and the advertised0...9fallback never reaches the working socket. Validate the handshake per candidate and continue scanning when it fails.
guard let (fd, socketPath) = self.discoverAndConnectSocket() else {
self.logger.warning("No working Discord IPC socket found")
await self.handleConnectionFailure(reason: "Discord desktop app not running")
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:184
- Music timestamps use the model’s optional
track.durationinstead of the player’s observedplayerService.duration. The playback observer updates the latter from the WebView (PlayerService+PlaybackRestoration.swift:396-397), so tracks with missing/stale API duration omit or miscalculate the remaining timestamp even when the real duration is known. Use the active player duration (with an appropriate validated fallback) as the timing source.
if let duration = track.duration, duration > 0 {
let remaining = max(0, duration - currentPos)
end = Int((now + remaining) * 1000)
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:222
- Video title/channel metadata is forwarded without Discord’s 2–128 character validation. A one-character or oversized title/channel causes the direct
SET_ACTIVITYrequest to be rejected, because there is no SDK layer here to sanitize it. Apply the same omission/truncation policy used for music metadata before constructing this payload.
let title = self.settings.discordShowTitle ? video.title : nil
let channel = self.settings.discordShowArtist ? video.channelName : nil
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:165
- These user-controlled metadata strings are sent directly to Discord without enforcing Rich Presence’s 2–128 character limits. An empty artist list produces
state: "", while one-character or long titles/artists/albums can also makeSET_ACTIVITYreject the entire update. Normalize omitted/short values and cap composed fields before building the payload.
let title = self.settings.discordShowTitle ? track.title : nil
let artist = self.settings.discordShowArtist ? track.artistsDisplay : nil
let album = self.settings.discordShowAlbum ? track.album?.title : nil
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:288
- The descriptor number is not a connection generation.
disconnect()cancels the detached task and closes the descriptor without first shutting down its blocking read; if a reconnect reuses the same descriptor number, the stale task can later enter this handler, pass the equality check, and close the new connection. Associate callbacks with a monotonically increasing generation/token and shut down the old socket before closing it.
private func handleDisconnect(for fd: Int32) async {
guard self.socketFD == fd else { return }
self.closeConnection()
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:283
- Discord
ERRORresponses leave the service in.connected, so Settings continues to show “Connected & Active” even though the activity was rejected; the originatingupdatePresencealso already returned successfully. Track outstanding nonces and surface the matching response failure (or transition to an actionable error/recovery state) instead of only logging it.
private func handleDiscordError(_ message: String) {
self.logger.error("Discord IPC error frame: \(message, privacy: .public)")
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:91
- This condition does not actually detect the “track/state change” claimed by the comment; it only detects progress deviation, elapsed time, or total inactivity. A quick resume near the start can match the expected progress and leave the presence cleared until the 60-second refresh, and playing-track metadata replacements at
PlayerService+WebQueueSync.swift:715/:757are likewise discarded while progress is on schedule. Track media identity and play state separately and synchronize those changes immediately, retaining the delta check only for seeks.
// Only resync if there was a discrete change (seek > 2.5s, track/state change, or coarse 60s refresh)
if delta > 2.5 || elapsedRealTime > 60 || !self.playerService.isPlaying && !self.youtubePlayerService.isPlaying {
- Files reviewed: 29/29 changed files
- Comments generated: 1
- Review effort level: Balanced
…layback - Add NSWorkspace notifications to detect Discord app launch and termination - Automatically connect and sync pending presence when Discord launches - Teardown socket cleanly on Discord quit - Attempt silent reconnection on playback presence updates even if previously in error state - Add test coverage for Discord application bundle and process detection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update: Discord App Lifecycle & Silent Auto-Reconnect
|
There was a problem hiding this comment.
🟡 Changes recommended
IPC timeouts, connection lifecycle races, and playback transition handling can cause repeated disconnects or missing presence updates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:498
SO_RCVTIMEOapplies to every laterreadPacket, not only the handshake. Once Discord is idle for two seconds,readreturnsEAGAIN/EWOULDBLOCK;readPackettreats that as a fatal error and the read loop disconnects. With an active payload this creates a reconnect loop, and without one a successful manual connection drops after two seconds. Limit the timeout to handshake setup or treat idle read timeouts as non-fatal while keeping explicit socket shutdown for cancellation.
var timeout = timeval(tv_sec: 2, tv_usec: 0)
setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout, socklen_t(MemoryLayout<timeval>.size))
setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, socklen_t(MemoryLayout<timeval>.size))
Sources/Kaset/Services/Discord/DiscordPresenceCoordinator.swift:94
- This throttle does not actually recognize the discrete track/play-state changes named above. In the normal startup path, setting a track/video while it is still loading clears presence and resets the baseline to zero; the subsequent transition to playing near position 0 has
delta <= 2.5, so no activity is published until the 60-second refresh. Fast track changes and resumes near the beginning have the same failure. Track media identity and play-state transitions separately and bypass the progress throttle for those events.
// Only resync if there was a discrete change (seek > 2.5s, track/state change, or coarse 60s refresh)
if delta > 2.5 || elapsedRealTime > 60 || !self.playerService.isPlaying && !self.youtubePlayerService.isPlaying {
self.lastProgress = currentPos
self.lastSyncTime = now
await self.syncPresence()
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:293
- File-descriptor equality is not a connection generation check.
closeConnection()closes the descriptor while the detached reader may still be blocked; a reconnect can reuse the same integer before the stale reader reports its error, causing this guard to pass and close the new connection. Shut down the old socket to unblock reads and capture a monotonic connection generation in each read task so stale callbacks cannot mutate newer connections.
private func handleDisconnect(for fd: Int32) async {
guard self.socketFD == fd else { return }
self.closeConnection()
Sources/Kaset/Services/Discord/DiscordLocalIPCService.swift:289
- Discord
ERRORresponses only reach this log statement; the service remains.connected, so Settings continues to report “Connected & Active” even though Discord rejected the activity update. Correlate responses by nonce and expose the rejection through service state or an update error that the coordinator handles.
private func handleDiscordError(_ message: String) {
self.logger.error("Discord IPC error frame: \(message, privacy: .public)")
}
- Files reviewed: 29/29 changed files
- Comments generated: 1
- Review effort level: Balanced
…onnection requested - Keep Discord IPC inactive on startup when Rich Presence is disabled - Prevent workspace launch events from connecting when integration is not enabled - Add unit test for initial inactive state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I will be creating an updated merge request soon |
Summary
Adds Discord Rich Presence integration to Kaset via Discord's local desktop IPC (
discord-ipc-0...discord-ipc-9).When enabled in Settings, Kaset automatically updates Discord's activity status in real-time as songs or videos are played, paused, or seeked.
Features
NSWorkspaceapplication notifications to immediately and silently connect whenever the Discord desktop app is launched (and disconnect cleanly when terminated).withObservationTrackingonPlayerService/YouTubePlayerServiceto immediately dispatch playback updates (playing, paused, song change, seek).DiscordPresenceTests) covering serialization, state transitions, wire framing, payload generation, application bundle identification, and privacy masking.Screenshots
Discord Activity Status
Settings & Privacy Controls
Note for Maintainer