Skip to content

Commit 5aa5ee7

Browse files
austinywangclaude
andcommitted
test: cover the Focus-gate staleness on the playback path
The Focus/DND gate added for #5650 reads a cached snapshot that only refreshes after deciding, so the first sound after the user enables a Focus still punches through — the canonical repro for the bug the gate exists to fix (and the first play after a Focus ends stays wrongly silent for one notification). Drive the real playback entry point in tests: playSelectedSound gains an injectable assertion-store URL and a main-queue completion reporting whether the sound was allowed to play (production callers pass nothing). The gate still consults the stale cache here, so the two first-play assertions fail. Per the repo's two-commit regression policy: this commit is the red half. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent cb7dddc commit 5aa5ee7

2 files changed

Lines changed: 97 additions & 4 deletions

File tree

Sources/TerminalNotificationStore.swift

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,9 @@ enum NotificationSoundSettings {
334334
/// (fail-open) and warms the cache off-main, so the `@MainActor` playback
335335
/// path never reads `Assertions.json` synchronously. Slightly stale by
336336
/// design, which is acceptable for an out-of-band fallback sound.
337-
static func isActiveFocusSuppressionCached() -> Bool {
337+
static func isActiveFocusSuppressionCached(
338+
assertionsFileURL: URL = defaultAssertionsFileURL
339+
) -> Bool {
338340
dndSuppressionLock.lock()
339341
let snapshot = cachedDndSuppression ?? false
340342
let needsRefresh = !dndRefreshInFlight
@@ -343,7 +345,7 @@ enum NotificationSoundSettings {
343345

344346
if needsRefresh {
345347
dndAssertionQueue.async {
346-
let suppressed = isSuppressedByActiveFocus()
348+
let suppressed = isSuppressedByActiveFocus(assertionsFileURL: assertionsFileURL)
347349
dndSuppressionLock.lock()
348350
cachedDndSuppression = suppressed
349351
dndRefreshInFlight = false
@@ -353,11 +355,25 @@ enum NotificationSoundSettings {
353355
return snapshot
354356
}
355357

356-
static func playSelectedSound(defaults: UserDefaults = .standard) {
358+
/// Plays the user-selected notification sound unless an active macOS
359+
/// Focus / Do Not Disturb mode should silence it.
360+
///
361+
/// `completion` runs on the main queue with whether the sound was allowed
362+
/// to play. It exists so tests can observe the gate decision; production
363+
/// callers pass nothing.
364+
static func playSelectedSound(
365+
defaults: UserDefaults = .standard,
366+
assertionsFileURL: URL = defaultAssertionsFileURL,
367+
completion: ((_ didPlay: Bool) -> Void)? = nil
368+
) {
357369
// Honor macOS Focus / Do Not Disturb for the out-of-band fallback sound.
358-
if isActiveFocusSuppressionCached() { return }
370+
if isActiveFocusSuppressionCached(assertionsFileURL: assertionsFileURL) {
371+
completion?(false)
372+
return
373+
}
359374
let value = defaults.string(forKey: key) ?? defaultValue
360375
playSound(value: value, defaults: defaults)
376+
completion?(true)
361377
}
362378

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

cmuxTests/NotificationSoundSettingsTests.swift

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,81 @@ import Testing
6969
.appendingPathComponent("Assertions.json", isDirectory: false)
7070
#expect(!NotificationSoundSettings.isSuppressedByActiveFocus(assertionsFileURL: missing))
7171
}
72+
73+
// The canonical repro for the Focus bug is: the user enables a Focus, and
74+
// the next notification must already be silent. A cached Focus snapshot
75+
// that refreshes only after deciding lets exactly that first sound punch
76+
// through, so these tests drive the real playback entry point and assert
77+
// on the per-play decision, not on the pure predicate.
78+
79+
@Test func firstPlayAfterFocusActivationIsSuppressed() async throws {
80+
let fixture = try ActiveFocusFixture()
81+
defer { fixture.cleanUp() }
82+
83+
try fixture.writeAssertions(#"{"data":[{"storeAssertionRecords":[{"a":1}]}]}"#)
84+
#expect(await fixture.playOutcome() == false)
85+
}
86+
87+
@Test func firstPlayAfterFocusEndsIsAudible() async throws {
88+
let fixture = try ActiveFocusFixture()
89+
defer { fixture.cleanUp() }
90+
91+
try fixture.writeAssertions(#"{"data":[{"storeAssertionRecords":[{"a":1}]}]}"#)
92+
#expect(await fixture.playOutcome() == false)
93+
94+
// The Focus ended: the very next play must be audible again.
95+
try fixture.writeAssertions(#"{"data":[{"storeAssertionRecords":[]}]}"#)
96+
#expect(await fixture.playOutcome() == true)
97+
}
98+
99+
@Test func playbackFailsOpenWhenAssertionStoreIsMissing() async throws {
100+
let fixture = try ActiveFocusFixture(createAssertionsFile: false)
101+
defer { fixture.cleanUp() }
102+
103+
#expect(await fixture.playOutcome() == true)
104+
}
105+
}
106+
107+
/// Drives `playSelectedSound` against a scratch assertion store and scratch
108+
/// defaults whose selected sound is "none", so the decision is observable
109+
/// without audible output.
110+
private struct ActiveFocusFixture {
111+
let directory: URL
112+
let assertionsFileURL: URL
113+
let defaults: UserDefaults
114+
private let suiteName: String
115+
116+
init(createAssertionsFile: Bool = true) throws {
117+
let fileManager = FileManager.default
118+
directory = fileManager.temporaryDirectory
119+
.appendingPathComponent("cmux-dnd-play-\(UUID().uuidString)", isDirectory: true)
120+
try fileManager.createDirectory(at: directory, withIntermediateDirectories: true)
121+
assertionsFileURL = directory.appendingPathComponent("Assertions.json", isDirectory: false)
122+
if createAssertionsFile {
123+
try Data(#"{"data":[]}"#.utf8).write(to: assertionsFileURL)
124+
}
125+
suiteName = "cmux-tests-notification-sound-\(UUID().uuidString)"
126+
defaults = try #require(UserDefaults(suiteName: suiteName))
127+
defaults.set("none", forKey: NotificationSoundSettings.key)
128+
}
129+
130+
func writeAssertions(_ json: String) throws {
131+
try Data(json.utf8).write(to: assertionsFileURL)
132+
}
133+
134+
func playOutcome() async -> Bool {
135+
await withCheckedContinuation { continuation in
136+
NotificationSoundSettings.playSelectedSound(
137+
defaults: defaults,
138+
assertionsFileURL: assertionsFileURL
139+
) { didPlay in
140+
continuation.resume(returning: didPlay)
141+
}
142+
}
143+
}
144+
145+
func cleanUp() {
146+
defaults.removePersistentDomain(forName: suiteName)
147+
try? FileManager.default.removeItem(at: directory)
148+
}
72149
}

0 commit comments

Comments
 (0)