From 16b28c33d71bafd2c78403a6cd55a9ef8bd2229f Mon Sep 17 00:00:00 2001 From: Akhil <11626756+buggerman@users.noreply.github.com> Date: Sun, 26 Apr 2026 23:26:39 +0200 Subject: [PATCH] Inject ScrollbackStore and ScrollbackIndex so tests can't pollute scrollback either --- Sources/Brygga/BryggaApp.swift | 6 ++- Sources/BryggaCore/IRC/IRCSession.swift | 41 +++++++++++++---- Sources/BryggaCore/Models/AppState.swift | 48 +++++++++++++++----- Tests/AppStateNavigationTests.swift | 56 ++++++++++++++++++------ Tests/IRCSessionTests.swift | 12 ++++- 5 files changed, 128 insertions(+), 35 deletions(-) diff --git a/Sources/Brygga/BryggaApp.swift b/Sources/Brygga/BryggaApp.swift index 4d44e68..40f4a97 100644 --- a/Sources/Brygga/BryggaApp.swift +++ b/Sources/Brygga/BryggaApp.swift @@ -30,7 +30,11 @@ final class BryggaAppDelegate: NSObject, NSApplicationDelegate { @main struct BryggaApp: App { @NSApplicationDelegateAdaptor(BryggaAppDelegate.self) private var appDelegate - @State private var appState = AppState(store: .shared) + @State private var appState = AppState( + store: .shared, + scrollbackStore: .shared, + scrollbackIndex: .shared, + ) var body: some Scene { Window("Brygga", id: "main") { diff --git a/Sources/BryggaCore/IRC/IRCSession.swift b/Sources/BryggaCore/IRC/IRCSession.swift index a72d4a2..80fb79f 100644 --- a/Sources/BryggaCore/IRC/IRCSession.swift +++ b/Sources/BryggaCore/IRC/IRCSession.swift @@ -52,9 +52,24 @@ public final class IRCSession { /// by token string → send timestamp. Used to derive `Server.lag`. private var pendingPings: [String: Date] = [:] - public init(server: Server, connection: IRCConnection) { + /// Per-channel JSONL scrollback store. Production code passes + /// `ScrollbackStore.shared`; tests pass a tempdir-rooted instance so a + /// `swift test` run never appends to the user's real scrollback. + private let scrollbackStore: ScrollbackStore + + /// SQLite/FTS5 search index. Same injection rule as `scrollbackStore`. + private let scrollbackIndex: ScrollbackIndex + + public init( + server: Server, + connection: IRCConnection, + scrollbackStore: ScrollbackStore, + scrollbackIndex: ScrollbackIndex, + ) { self.server = server self.connection = connection + self.scrollbackStore = scrollbackStore + self.scrollbackIndex = scrollbackIndex } // MARK: - Lifecycle @@ -336,8 +351,10 @@ public final class IRCSession { channel.scrollbackLoaded = true let sid = server.id let target = channel.name + let store = scrollbackStore + let index = scrollbackIndex Task { [weak channel] in - let msgs = await ScrollbackStore.shared.load(serverId: sid, target: target) + let msgs = await store.load(serverId: sid, target: target) guard let channel, !msgs.isEmpty else { return } await MainActor.run { channel.messages.insert(contentsOf: msgs, at: 0) @@ -346,7 +363,7 @@ public final class IRCSession { // reply, openQuery, incoming PM). `index` is idempotent on // `msg_id` so on-line writes for the same channel are safe. for msg in msgs { - await ScrollbackIndex.shared.index(msg, serverID: sid, target: target) + await index.index(msg, serverID: sid, target: target) } } } @@ -357,9 +374,11 @@ public final class IRCSession { channel.messages.append(message) let sid = server.id let target = channel.name + let store = scrollbackStore + let index = scrollbackIndex Task { - await ScrollbackStore.shared.append(serverId: sid, target: target, message: message) - await ScrollbackIndex.shared.index(message, serverID: sid, target: target) + await store.append(serverId: sid, target: target, message: message) + await index.index(message, serverID: sid, target: target) } logToDiskIfEnabled(message, target: target) } @@ -368,9 +387,11 @@ public final class IRCSession { public func recordServer(_ message: Message) { server.messages.append(message) let sid = server.id + let store = scrollbackStore + let index = scrollbackIndex Task { - await ScrollbackStore.shared.append(serverId: sid, target: "__server__", message: message) - await ScrollbackIndex.shared.index(message, serverID: sid, target: "__server__") + await store.append(serverId: sid, target: "__server__", message: message) + await index.index(message, serverID: sid, target: "__server__") } logToDiskIfEnabled(message, target: "server") } @@ -870,14 +891,16 @@ public final class IRCSession { } let sid = server.id let target = ctx.target + let store = scrollbackStore + let index = scrollbackIndex Task { for msg in novel { - await ScrollbackStore.shared.append( + await store.append( serverId: sid, target: target, message: msg, ) - await ScrollbackIndex.shared.index(msg, serverID: sid, target: target) + await index.index(msg, serverID: sid, target: target) } } } diff --git a/Sources/BryggaCore/Models/AppState.swift b/Sources/BryggaCore/Models/AppState.swift index 0dd1759..c59881c 100644 --- a/Sources/BryggaCore/Models/AppState.swift +++ b/Sources/BryggaCore/Models/AppState.swift @@ -69,8 +69,25 @@ public final class AppState { /// production data wipe. private let store: ServerStore - public init(store: ServerStore) { + /// Per-channel JSONL scrollback store. Same injection rule as `store` + /// — every constructed `IRCSession` gets this so test sessions don't + /// trickle into the user's real `scrollback/` directory. + private let scrollbackStore: ScrollbackStore + + /// FTS5 search index. Same injection rule. Read-only `search` in the + /// UI still uses `ScrollbackIndex.shared` directly because UI is + /// production-only; the injection is purely about isolating writes + /// from tests. + private let scrollbackIndex: ScrollbackIndex + + public init( + store: ServerStore, + scrollbackStore: ScrollbackStore, + scrollbackIndex: ScrollbackIndex, + ) { self.store = store + self.scrollbackStore = scrollbackStore + self.scrollbackIndex = scrollbackIndex restoreFromStore() requestNotificationPermission() } @@ -204,11 +221,13 @@ public final class AppState { // Each loaded message is also fed into the FTS5 index using the // canonical channel name — this is the cold-start backfill path, // done lazily through the existing rehydrate loop instead of a - // separate filesystem walker. `ScrollbackIndex.index` is - // idempotent on `msg_id`, so on-line writes that ran first - // won't be double-counted. + // separate filesystem walker. The index is idempotent on + // `msg_id`, so on-line writes that ran first won't be + // double-counted. + let scrollback = scrollbackStore + let index = scrollbackIndex Task { [server] in - let serverMessages = await ScrollbackStore.shared.load( + let serverMessages = await scrollback.load( serverId: server.id, target: "__server__", ) @@ -216,10 +235,10 @@ public final class AppState { server.messages.insert(contentsOf: serverMessages, at: 0) } for msg in serverMessages { - await ScrollbackIndex.shared.index(msg, serverID: server.id, target: "__server__") + await index.index(msg, serverID: server.id, target: "__server__") } for channel in server.channels { - let msgs = await ScrollbackStore.shared.load( + let msgs = await scrollback.load( serverId: server.id, target: channel.name, ) @@ -228,7 +247,7 @@ public final class AppState { channel.scrollbackLoaded = true } for msg in msgs { - await ScrollbackIndex.shared.index( + await index.index( msg, serverID: server.id, target: channel.name, @@ -313,7 +332,8 @@ public final class AppState { if selection == channelID { selection = server.id } - Task { await ScrollbackIndex.shared.clear(serverID: serverID, target: closedTarget) } + let index = scrollbackIndex + Task { await index.clear(serverID: serverID, target: closedTarget) } persist() } @@ -451,7 +471,12 @@ public final class AppState { clientCertificatePassphrase: clientCertificatePassphrase, bouncerNetID: bouncerNetID, ) - let session = IRCSession(server: server, connection: connection) + let session = IRCSession( + server: server, + connection: connection, + scrollbackStore: scrollbackStore, + scrollbackIndex: scrollbackIndex, + ) session.autoJoinChannels = autoJoinChannels session.onChannelsChanged = { [weak self] in self?.persist() } session.onHighlight = { [weak self] channel, message in @@ -606,7 +631,8 @@ public final class AppState { if selection == id { selection = nil } - Task { await ScrollbackIndex.shared.clear(serverID: id) } + let index = scrollbackIndex + Task { await index.clear(serverID: id) } persist() } } diff --git a/Tests/AppStateNavigationTests.swift b/Tests/AppStateNavigationTests.swift index 34a455d..df10f24 100644 --- a/Tests/AppStateNavigationTests.swift +++ b/Tests/AppStateNavigationTests.swift @@ -6,19 +6,40 @@ import XCTest @MainActor final class AppStateNavigationTests: XCTestCase { - /// Build a `ServerStore` rooted at a unique temp directory. Critical - /// for isolation: the production singleton would share the user's real - /// `~/Library/Application Support/Brygga/servers.json`, and any test - /// mutation that triggers `persist()` (e.g. `closePrivateMessage`) - /// would overwrite that file with the test fixture data. - private func makeStore() -> ServerStore { + /// Build a tempdir-rooted set of stores. Critical for isolation: the + /// production singletons would share the user's real + /// `~/Library/Application Support/Brygga/{servers.json,scrollback/,scrollback.sqlite}`, + /// and any test mutation that triggers a write would land there. The + /// recovery script (`Scripts/recover-scrollback.sh`) was written to + /// dig users out of exactly that hole. + private struct TestDeps { + let server: ServerStore + let scrollback: ScrollbackStore + let scrollbackIndex: ScrollbackIndex + } + + private func makeDeps() -> TestDeps { let dir = FileManager.default.temporaryDirectory .appendingPathComponent("BryggaTests-\(UUID().uuidString)", isDirectory: true) - return ServerStore(root: dir) + return TestDeps( + server: ServerStore(root: dir), + scrollback: ScrollbackStore(root: dir.appendingPathComponent("scrollback", isDirectory: true)), + scrollbackIndex: ScrollbackIndex(path: ":memory:"), + ) + } + + /// Convenience for tests that only need the `ServerStore`. + private func makeStore() -> ServerStore { + makeDeps().server } private func makeFixture() -> AppState { - let state = AppState(store: makeStore()) + let deps = makeDeps() + let state = AppState( + store: deps.server, + scrollbackStore: deps.scrollback, + scrollbackIndex: deps.scrollbackIndex, + ) state.servers.removeAll() state.selection = nil let s1 = Server(name: "ServerA", host: "a.example.org", nickname: "me") @@ -95,7 +116,12 @@ final class AppStateNavigationTests: XCTestCase { } func testEmptyStateIsNoOp() { - let state = AppState(store: makeStore()) + let deps = makeDeps() + let state = AppState( + store: deps.server, + scrollbackStore: deps.scrollback, + scrollbackIndex: deps.scrollbackIndex, + ) state.servers.removeAll() state.selection = nil state.selectAdjacentChannel(direction: 1) @@ -106,11 +132,15 @@ final class AppStateNavigationTests: XCTestCase { /// `persist()` must write to the injected store's path, not to the /// production `~/Library/Application Support/Brygga/servers.json`. func testPersistWritesToInjectedStoreNotProduction() { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("BryggaTests-\(UUID().uuidString)", isDirectory: true) - let store = ServerStore(root: dir) - let state = AppState(store: store) + let deps = makeDeps() + let state = AppState( + store: deps.server, + scrollbackStore: deps.scrollback, + scrollbackIndex: deps.scrollbackIndex, + ) state.servers.removeAll() + // Convenience handles for the assertions below. + let store = deps.server let pm = Channel(name: "carol") let owner = Server(name: "Test", host: "irc.example.org", nickname: "me") diff --git a/Tests/IRCSessionTests.swift b/Tests/IRCSessionTests.swift index b10f900..68d19ac 100644 --- a/Tests/IRCSessionTests.swift +++ b/Tests/IRCSessionTests.swift @@ -9,7 +9,17 @@ final class IRCSessionTests: XCTestCase { private func makeSession(ownNick: String = "me") -> IRCSession { let server = Server(name: "Test", host: "irc.example.org", nickname: ownNick) let connection = IRCConnection(host: "irc.example.org", nickname: ownNick) - return IRCSession(server: server, connection: connection) + // Tempdir-rooted scrollback store so `record(_:in:)` calls during + // the test can't append to the user's real `~/Library/Application + // Support/Brygga/scrollback/`. `:memory:` SQLite for the index. + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("BryggaTests-\(UUID().uuidString)", isDirectory: true) + return IRCSession( + server: server, + connection: connection, + scrollbackStore: ScrollbackStore(root: dir), + scrollbackIndex: ScrollbackIndex(path: ":memory:"), + ) } private func parse(_ line: String) -> IRCLineParserResult {