Skip to content

Commit b4fe2df

Browse files
austinywangclaude
andcommitted
fix: read Focus state fresh for every fallback sound
Replace the stale cached Focus snapshot with a fresh assertion-store read per play. The read still happens on the background utility queue (no disk I/O on the @mainactor callers); playback hops back to the main queue afterwards. An out-of-band sound a few milliseconds later is imperceptible, but a stale snapshot is not: it let the first sound after the user enables a Focus punch through, which is the canonical repro of the bug this gate exists to fix. This deletes the cache, its lock, and the in-flight flag outright - notification sounds are cooldown-throttled, so per-play reads need no caching layer. This is the green half: the staleness tests from the previous commit now pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5aa5ee7 commit b4fe2df

1 file changed

Lines changed: 17 additions & 39 deletions

File tree

Sources/TerminalNotificationStore.swift

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@ enum NotificationSoundSettings {
5757
label: "com.cmuxterm.notification-dnd-assertion",
5858
qos: .utility
5959
)
60-
private static let dndSuppressionLock = NSLock()
61-
// nil until the first background read completes. Fail-open treats nil as "not suppressed".
62-
private static var cachedDndSuppression: Bool?
63-
private static var dndRefreshInFlight = false
6460
private static let notificationSoundSupportedExtensions: Set<String> = [
6561
"aif",
6662
"aiff",
@@ -327,37 +323,17 @@ enum NotificationSoundSettings {
327323
}
328324
}
329325

330-
/// Cheap, main-actor-safe snapshot of the live Focus/DND state.
331-
///
332-
/// Reads a process-local cached `Bool` (no disk I/O) and schedules a
333-
/// background refresh for the next call. The first call returns `false`
334-
/// (fail-open) and warms the cache off-main, so the `@MainActor` playback
335-
/// path never reads `Assertions.json` synchronously. Slightly stale by
336-
/// design, which is acceptable for an out-of-band fallback sound.
337-
static func isActiveFocusSuppressionCached(
338-
assertionsFileURL: URL = defaultAssertionsFileURL
339-
) -> Bool {
340-
dndSuppressionLock.lock()
341-
let snapshot = cachedDndSuppression ?? false
342-
let needsRefresh = !dndRefreshInFlight
343-
if needsRefresh { dndRefreshInFlight = true }
344-
dndSuppressionLock.unlock()
345-
346-
if needsRefresh {
347-
dndAssertionQueue.async {
348-
let suppressed = isSuppressedByActiveFocus(assertionsFileURL: assertionsFileURL)
349-
dndSuppressionLock.lock()
350-
cachedDndSuppression = suppressed
351-
dndRefreshInFlight = false
352-
dndSuppressionLock.unlock()
353-
}
354-
}
355-
return snapshot
356-
}
357-
358326
/// Plays the user-selected notification sound unless an active macOS
359327
/// Focus / Do Not Disturb mode should silence it.
360328
///
329+
/// The Focus check reads the assertion store, which is disk I/O, so it
330+
/// runs on the background assertion queue and playback hops back to the
331+
/// main queue. The state is read fresh for every play: a cached snapshot
332+
/// would let the first sound after the user enables a Focus punch
333+
/// through, which is the exact bug this gate exists to fix. Notification
334+
/// sounds are low-frequency (cooldown-throttled), so one small file read
335+
/// per play on a utility queue is cheap.
336+
///
361337
/// `completion` runs on the main queue with whether the sound was allowed
362338
/// to play. It exists so tests can observe the gate decision; production
363339
/// callers pass nothing.
@@ -366,14 +342,16 @@ enum NotificationSoundSettings {
366342
assertionsFileURL: URL = defaultAssertionsFileURL,
367343
completion: ((_ didPlay: Bool) -> Void)? = nil
368344
) {
369-
// Honor macOS Focus / Do Not Disturb for the out-of-band fallback sound.
370-
if isActiveFocusSuppressionCached(assertionsFileURL: assertionsFileURL) {
371-
completion?(false)
372-
return
345+
dndAssertionQueue.async {
346+
let suppressed = isSuppressedByActiveFocus(assertionsFileURL: assertionsFileURL)
347+
DispatchQueue.main.async {
348+
if !suppressed {
349+
let value = defaults.string(forKey: key) ?? defaultValue
350+
playSound(value: value, defaults: defaults)
351+
}
352+
completion?(!suppressed)
353+
}
373354
}
374-
let value = defaults.string(forKey: key) ?? defaultValue
375-
playSound(value: value, defaults: defaults)
376-
completion?(true)
377355
}
378356

379357
static func previewSound(value: String, defaults: UserDefaults = .standard) {

0 commit comments

Comments
 (0)