From 685705c12013c22c9ada5d415e6bbeabe36d5dc8 Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Sat, 30 May 2026 17:17:49 +0200 Subject: [PATCH 1/3] Add attach-aware zmx session listing parser zmx ls --short emits only the session name, so the reaper cannot tell whether another instance still holds a client. Parse the full ls format through a pure ZmxSessionListParser (name=/clients=, supa- prefix filter, clients=nil for unreachable lines) behind a listSessionsWithClients field that returns nil on probe failure, so an unprobeable session is never reaped. The reaper consumes this exclusively, so the name-only listSessions is gone. --- supacode/Clients/Zmx/ZmxClient.swift | 62 ++++++++++++++++++++++------ supacodeTests/ZmxClientTests.swift | 52 +++++++++++++++++++++++ 2 files changed, 102 insertions(+), 12 deletions(-) diff --git a/supacode/Clients/Zmx/ZmxClient.swift b/supacode/Clients/Zmx/ZmxClient.swift index 2b4a3d6fd..2c1384637 100644 --- a/supacode/Clients/Zmx/ZmxClient.swift +++ b/supacode/Clients/Zmx/ZmxClient.swift @@ -29,10 +29,11 @@ struct ZmxClient: Sendable { /// Tear down a session. No-op on missing. Bounded by a 5-second timeout so a /// stuck daemon can't hold the close path indefinitely. var killSession: @Sendable (_ sessionID: String) async -> Void - /// Returns all live Supacode session names (`supa-`) the daemon currently - /// hosts. Empty when zmx is unbundled or the daemon is unreachable. Used at - /// launch to reap sessions whose owning surface no longer exists. - var listSessions: @Sendable () async -> [String] + /// Returns each live Supacode session with its attached-client count, or nil + /// when the probe failed/timed out. nil means UNKNOWN (never reap); `[]` means + /// a successful empty listing. A `clients` of nil marks a session whose count + /// is unknown (err/status line), which the reaper must also spare. + var listSessionsWithClients: @Sendable () async -> [ZmxSessionListParser.Entry]? } /// Cached probe result so we log the bypass reason exactly once per process @@ -202,13 +203,11 @@ extension ZmxClient { killSession: { sessionID in _ = await runZmx(["kill", sessionID]) }, - listSessions: { - guard let stdout = await runZmx(["ls", "--short"], captureStdout: true) else { return [] } - return - stdout - .split(whereSeparator: \.isNewline) - .map { $0.trimmingCharacters(in: .whitespaces) } - .filter { $0.hasPrefix(ZmxSessionID.prefix) && !$0.isEmpty } + listSessionsWithClients: { + // nil from runZmx is the UNKNOWN signal (spawn error / timeout / non-zero + // exit); preserve it so the reaper never kills against a failed probe. + guard let stdout = await runZmx(["ls"], captureStdout: true) else { return nil } + return ZmxSessionListParser.parse(stdout) } ) }() @@ -218,7 +217,7 @@ extension ZmxClient { isBundled: { false }, wrapCommand: { _, _ in nil }, killSession: { _ in }, - listSessions: { [] } + listSessionsWithClients: { [] } ) } @@ -234,6 +233,45 @@ extension DependencyValues { } } +/// Pure parser for zmx's full (`ls`, non-`--short`) tab-delimited listing. +/// Each line is `[→ | ]name=\tk=v\t...`; a healthy session carries +/// `clients=`, an unreachable one carries `err=`/`status=` (no count). +nonisolated enum ZmxSessionListParser { + struct Entry: Equatable, Sendable { + var name: String + /// nil when the count is unknown (err/status line); the reaper spares these. + var clients: Int? + } + + static func parse(_ stdout: String) -> [Entry] { + stdout + .split(whereSeparator: \.isNewline) + .compactMap { line -> Entry? in + // Strip the current-session arrow / leading indent before tokenizing. + var trimmed = Substring(line) + if trimmed.hasPrefix("→ ") { + trimmed = trimmed.dropFirst(2) + } + // Non-current sessions are indented with a literal leading space run. + while trimmed.first?.isWhitespace == true { + trimmed = trimmed.dropFirst() + } + let fields = trimmed.split(separator: "\t") + var values: [Substring: Substring] = [:] + for field in fields { + guard let separator = field.firstIndex(of: "=") else { continue } + let key = field[field.startIndex..` lands at 41, leaving /// headroom for a longer custom `ZMX_DIR`. diff --git a/supacodeTests/ZmxClientTests.swift b/supacodeTests/ZmxClientTests.swift index c51e91120..48e720a99 100644 --- a/supacodeTests/ZmxClientTests.swift +++ b/supacodeTests/ZmxClientTests.swift @@ -129,6 +129,58 @@ struct ZmxSocketBudgetTests { } } +@MainActor +struct ZmxSessionListParserTests { + @Test func parsesClientsZero() { + let entries = ZmxSessionListParser.parse("name=supa-abc\tpid=123\tclients=0\tcreated=0\n") + #expect(entries == [.init(name: "supa-abc", clients: 0)]) + } + + @Test func parsesClientsPositive() { + let entries = ZmxSessionListParser.parse("name=supa-abc\tpid=123\tclients=2\tcreated=0\n") + #expect(entries == [.init(name: "supa-abc", clients: 2)]) + } + + @Test func errOrStatusLineYieldsNilClients() { + let entries = ZmxSessionListParser.parse( + "name=supa-abc\terr=ConnectionRefused\tstatus=cleaning up\n" + ) + #expect(entries == [.init(name: "supa-abc", clients: nil)]) + } + + @Test func stripsCurrentSessionArrowPrefix() { + let entries = ZmxSessionListParser.parse("→ name=supa-abc\tpid=1\tclients=1\tcreated=0\n") + #expect(entries == [.init(name: "supa-abc", clients: 1)]) + } + + @Test func stripsLeadingIndentOnNonCurrentSessions() { + let entries = ZmxSessionListParser.parse(" name=supa-abc\tclients=0\tpid=1\tcreated=0\n") + #expect(entries == [.init(name: "supa-abc", clients: 0)]) + } + + @Test func filtersNonSupaSessions() { + let entries = ZmxSessionListParser.parse( + """ + name=dev\tpid=1\tclients=2\tcreated=0 + name=supa-abc\tpid=2\tclients=0\tcreated=0 + """ + ) + #expect(entries == [.init(name: "supa-abc", clients: 0)]) + } + + @Test func dropsBlankAndMalformedLines() { + let entries = ZmxSessionListParser.parse( + """ + + garbage with no equals + name=supa-keep\tpid=9\tclients=3\tcreated=0 + + """ + ) + #expect(entries == [.init(name: "supa-keep", clients: 3)]) + } +} + @MainActor struct ZmxClientNoopTests { /// The default test impl is a no-op so existing TestStore tests are unaffected From 6a12ae2223c91d3dbb40a7ba947f3e56a8490294 Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Sat, 30 May 2026 17:17:49 +0200 Subject: [PATCH 2/3] Add serialized off-main layout merge writer Per-mutation persistence cannot run the whole-dict encode and atomic write synchronously on the main actor. LayoutsIncrementalWriter is a single FIFO actor that re-reads layouts.json, splices in only the keys a flush carries (positive snapshot or explicit delete tombstone), and writes atomically through the temp+rename storage. It skips the write when the splice leaves the dict unchanged so high-frequency projection ticks do not churn the file. A transient read failure aborts the flush so siblings are not clobbered, while corrupt bytes are rotated aside to layouts.json.corrupt- and persistence recovers, mirroring SidebarPersistenceKey. LayoutsKey.save becomes a no-op so the actor is the sole disk writer. --- .../SettingsFilePersistence.swift | 8 +- .../Support/SupaLogger.swift | 8 + .../LayoutsIncrementalWriter.swift | 145 ++++++++++++++++++ .../BusinessLogic/LayoutsPersistenceKey.swift | 16 +- .../LayoutsIncrementalWriterTests.swift | 138 +++++++++++++++++ 5 files changed, 299 insertions(+), 16 deletions(-) create mode 100644 supacode/Features/Terminal/BusinessLogic/LayoutsIncrementalWriter.swift create mode 100644 supacodeTests/LayoutsIncrementalWriterTests.swift diff --git a/SupacodeSettingsShared/BusinessLogic/SettingsFilePersistence.swift b/SupacodeSettingsShared/BusinessLogic/SettingsFilePersistence.swift index 81c111c7a..6c5987533 100644 --- a/SupacodeSettingsShared/BusinessLogic/SettingsFilePersistence.swift +++ b/SupacodeSettingsShared/BusinessLogic/SettingsFilePersistence.swift @@ -58,10 +58,6 @@ extension SettingsFileStorage { } } -nonisolated enum SettingsFileStorageError: Error { - case missing -} - nonisolated final class InMemorySettingsFileStorage: @unchecked Sendable { private let lock = NSLock() private var dataByURL: [URL: Data] = [:] @@ -70,7 +66,9 @@ nonisolated final class InMemorySettingsFileStorage: @unchecked Sendable { lock.lock() defer { lock.unlock() } guard let data = dataByURL[url] else { - throw SettingsFileStorageError.missing + // Mirror real-disk semantics so callers that distinguish "file absent" + // (fresh start) from a read failure see the same `fileReadNoSuchFile`. + throw CocoaError(.fileReadNoSuchFile) } return data } diff --git a/SupacodeSettingsShared/Support/SupaLogger.swift b/SupacodeSettingsShared/Support/SupaLogger.swift index 8a97d3827..3d00385c9 100644 --- a/SupacodeSettingsShared/Support/SupaLogger.swift +++ b/SupacodeSettingsShared/Support/SupaLogger.swift @@ -36,4 +36,12 @@ public nonisolated struct SupaLogger: Sendable { logger.warning("\(message, privacy: .public)") #endif } + + public func error(_ message: String) { + #if DEBUG + print("[\(category)] \(message)") + #else + logger.error("\(message, privacy: .public)") + #endif + } } diff --git a/supacode/Features/Terminal/BusinessLogic/LayoutsIncrementalWriter.swift b/supacode/Features/Terminal/BusinessLogic/LayoutsIncrementalWriter.swift new file mode 100644 index 000000000..bb739fd7e --- /dev/null +++ b/supacode/Features/Terminal/BusinessLogic/LayoutsIncrementalWriter.swift @@ -0,0 +1,145 @@ +import Dependencies +import Foundation +import SupacodeSettingsShared + +/// Serialized off-main writer for incremental layout persistence. Every flush +/// re-reads `layouts.json` from disk, splices in only the per-worktree keys it +/// carries, then writes the whole dict back through the atomic temp+rename +/// `settingsFileStorage.save`. Being an actor makes the read-modify-write a +/// FIFO critical section: a positive snapshot and a delete tombstone for the +/// same key can't interleave, and concurrent keys from separate flushes both +/// survive (last-writer-wins per key, not whole-dict). +/// +/// There is no flock / NSFileCoordinator: a second Supacode instance writing +/// the same file concurrently is a dev-only scenario and accepted as +/// last-writer-wins. The in-memory `@Shared(.layouts)` dict stays the source of +/// truth on main; this actor only owns the encode + disk merge. +actor LayoutsIncrementalWriter { + /// One per-worktree change to splice into the on-disk dict. `.delete` is an + /// explicit tombstone: absence from a flush means "leave the disk key alone", + /// so a pruned worktree must be carried as `.delete`, never as omission. + enum Change: Sendable { + case snapshot(TerminalLayoutSnapshot) + case delete + } + + private static let logger = SupaLogger("Layouts") + private let storage: SettingsFileStorage + private let url: URL + /// Guards the read-modify-write so the off-actor `flushSync` (on-quit) and the + /// actor-routed flush/delete paths mutually exclude. The actor still owns FIFO + /// ordering of the live path; this only prevents a lost update against the + /// single off-actor entrant. + private let writeLock = NSLock() + + init( + storage: SettingsFileStorage, + url: URL = SupacodePaths.layoutsURL + ) { + self.storage = storage + self.url = url + } + + /// Re-reads the on-disk dict, applies `changes`, and writes the result. + /// Keys not present in `changes` are preserved from disk untouched. + func flush(_ changes: [String: Change]) { + applyAndWrite(changes) + } + + /// Synchronous variant for the on-quit terminal write, where the run loop is + /// tearing down and there's no chance to await the actor. The atomic temp+rename + /// `storage.save` makes the off-actor write safe as the process's final flush. + nonisolated func flushSync(_ changes: [String: Change]) { + applyAndWrite(changes) + } + + private nonisolated func applyAndWrite(_ changes: [String: Change]) { + guard !changes.isEmpty else { return } + writeLock.lock() + defer { writeLock.unlock() } + guard var dict = readFromDisk() else { + // Abort rather than splice our keys into an empty dict and clobber every other worktree's layout. + Self.logger.error( + "Aborting incremental layout flush: on-disk layouts failed to decode; preserving file for recovery.") + return + } + let original = dict + for (key, change) in changes { + switch change { + case .snapshot(let snapshot): + dict[key] = snapshot + case .delete: + dict.removeValue(forKey: key) + } + } + // Skip the write when the splice is a no-op. onTabProjectionChanged fires on + // notification / focus / zoom deltas that aren't part of the snapshot, so an + // agent tool-call storm would otherwise churn the file with identical bytes. + guard dict != original else { return } + write(dict) + } + + /// Returns the on-disk dict, `[:]` when the file is absent (fresh start) or + /// when corrupt bytes were rotated aside, or `nil` on a present-but-unreadable + /// file (transient/permission error) so the caller aborts rather than clobbers it. + private nonisolated func readFromDisk() -> [String: TerminalLayoutSnapshot]? { + let data: Data + do { + data = try storage.load(url) + } catch { + // Only an absent file is a fresh start; a present-but-unreadable file must abort so we don't clobber siblings. + guard Self.isFileAbsent(error) else { + Self.logger.error("Failed to read layouts during incremental merge: \(error)") + return nil + } + return [:] + } + do { + return try JSONDecoder().decode([String: TerminalLayoutSnapshot].self, from: data) + } catch { + // Corrupt bytes: rotate aside and start fresh rather than refuse to save + // forever. Mirrors SidebarPersistenceKey; the bytes are kept for recovery. + Self.logger.error("Failed to decode layouts during incremental merge: \(error)") + Self.renameCorruptFile(at: url) + return [:] + } + } + + /// True only when the read failed because the file does not exist. + private static func isFileAbsent(_ error: Error) -> Bool { + if let cocoa = error as? CocoaError, cocoa.code == .fileReadNoSuchFile { return true } + if let posix = error as? POSIXError, posix.code == .ENOENT { return true } + return false + } + + /// Moves a corrupt `layouts.json` aside to `layouts.json.corrupt-` so + /// the next save starts fresh instead of aborting forever. The storage dep only + /// exposes load/save, so the rename goes through FileManager; a missing or + /// already-renamed file returns quietly and the caller proceeds to the fresh dict. + private nonisolated static func renameCorruptFile(at url: URL) { + let fileManager = FileManager.default + guard fileManager.fileExists(atPath: url.path(percentEncoded: false)) else { return } + let formatter = ISO8601DateFormatter() + formatter.formatOptions = [.withInternetDateTime] + let timestamp = formatter.string(from: Date()).replacing(":", with: "-") + let destination = url.deletingLastPathComponent() + .appending(path: "\(url.lastPathComponent).corrupt-\(timestamp)", directoryHint: .notDirectory) + do { + try fileManager.moveItem(at: url, to: destination) + } catch { + Self.logger.warning( + "Failed to rename corrupt layouts file to \(destination.lastPathComponent): \(error).") + } + } + + private nonisolated func write(_ dict: [String: TerminalLayoutSnapshot]) { + do { + let encoder = JSONEncoder() + encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + let data = try encoder.encode(dict) + try storage.save(data, url) + } catch { + Self.logger.warning("Failed to write incremental layouts: \(error)") + } + } +} diff --git a/supacode/Features/Terminal/BusinessLogic/LayoutsPersistenceKey.swift b/supacode/Features/Terminal/BusinessLogic/LayoutsPersistenceKey.swift index 764c8285b..15f38dce5 100644 --- a/supacode/Features/Terminal/BusinessLogic/LayoutsPersistenceKey.swift +++ b/supacode/Features/Terminal/BusinessLogic/LayoutsPersistenceKey.swift @@ -42,20 +42,14 @@ nonisolated struct LayoutsKey: SharedKey { } func save( - _ value: [String: TerminalLayoutSnapshot], + _: [String: TerminalLayoutSnapshot], context _: SaveContext, continuation: SaveContinuation ) { - @Dependency(\.settingsFileStorage) var storage - do { - let encoder = JSONEncoder() - encoder.outputFormatting = [.prettyPrinted, .sortedKeys] - let data = try encoder.encode(value) - try storage.save(data, SupacodePaths.layoutsURL) - continuation.resume() - } catch { - continuation.resume(throwing: error) - } + // No-op: `LayoutsIncrementalWriter` is the sole disk writer for `layouts.json`. + // `@Shared(.layouts)` stays the in-memory source of truth; persisting here too + // would race the actor's per-key merge with a whole-dict last-writer-wins clobber. + continuation.resume() } } diff --git a/supacodeTests/LayoutsIncrementalWriterTests.swift b/supacodeTests/LayoutsIncrementalWriterTests.swift new file mode 100644 index 000000000..333f11821 --- /dev/null +++ b/supacodeTests/LayoutsIncrementalWriterTests.swift @@ -0,0 +1,138 @@ +import Dependencies +import Foundation +import SupacodeSettingsShared +import Testing + +@testable import supacode + +@MainActor +struct LayoutsIncrementalWriterTests { + private func snapshot(dir: String) -> TerminalLayoutSnapshot { + TerminalLayoutSnapshot( + tabs: [ + TerminalLayoutSnapshot.TabSnapshot( + id: nil, + title: "Terminal 1", + customTitle: nil, + icon: nil, + tintColor: nil, + layout: .leaf( + TerminalLayoutSnapshot.SurfaceSnapshot(id: nil, workingDirectory: dir) + ), + focusedLeafIndex: 0 + ) + ], + selectedTabIndex: 0 + ) + } + + private func readDict(_ storage: SettingsFileStorage, _ url: URL) -> [String: TerminalLayoutSnapshot] { + guard let data = try? storage.load(url) else { return [:] } + return (try? JSONDecoder().decode([String: TerminalLayoutSnapshot].self, from: data)) ?? [:] + } + + @Test func separateFlushesBothSurvive() async { + let storage = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + let writer = LayoutsIncrementalWriter(storage: storage, url: url) + + await writer.flush(["w1": .snapshot(snapshot(dir: "/w1"))]) + await writer.flush(["w2": .snapshot(snapshot(dir: "/w2"))]) + + let dict = readDict(storage, url) + #expect(Set(dict.keys) == ["w1", "w2"]) + } + + @Test func deleteRemovesOnlyTargetKey() async { + let storage = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + let writer = LayoutsIncrementalWriter(storage: storage, url: url) + + await writer.flush([ + "w1": .snapshot(snapshot(dir: "/w1")), + "w2": .snapshot(snapshot(dir: "/w2")), + ]) + await writer.flush(["w1": .delete]) + + let dict = readDict(storage, url) + #expect(Set(dict.keys) == ["w2"]) + } + + @Test func snapshotOverwritesSameKeyButPreservesOthers() async { + let storage = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + let writer = LayoutsIncrementalWriter(storage: storage, url: url) + + await writer.flush([ + "w1": .snapshot(snapshot(dir: "/old")), + "w2": .snapshot(snapshot(dir: "/w2")), + ]) + await writer.flush(["w1": .snapshot(snapshot(dir: "/new"))]) + + let dict = readDict(storage, url) + #expect(dict["w2"] != nil) + let leaf = dict["w1"]?.tabs.first?.layout + if case .leaf(let surface) = leaf { + #expect(surface.workingDirectory == "/new") + } else { + Issue.record("Expected a leaf layout for w1") + } + } + + @Test func identicalReflushSkipsTheWrite() async { + let inner = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + let saveCount = LockIsolated(0) + let storage = SettingsFileStorage( + load: { try inner.load($0) }, + save: { data, target in + if target == url { saveCount.withValue { $0 += 1 } } + try inner.save(data, target) + } + ) + let writer = LayoutsIncrementalWriter(storage: storage, url: url) + + await writer.flush(["w1": .snapshot(snapshot(dir: "/w1"))]) + // Re-splicing the same snapshot is a no-op; the second flush must not write. + await writer.flush(["w1": .snapshot(snapshot(dir: "/w1"))]) + + #expect(saveCount.value == 1) + #expect(Set(readDict(storage, url).keys) == ["w1"]) + } + + @Test func corruptFileIsRotatedAsideAndPersistenceRecovers() async throws { + let dir = URL(fileURLWithPath: NSTemporaryDirectory()) + .appending(path: "LayoutsWriterCorrupt-\(UUID().uuidString)", directoryHint: .isDirectory) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + let url = dir.appending(path: "layouts.json", directoryHint: .notDirectory) + // Seed garbage so the decode fails on the next merge read. + try Data("not json".utf8).write(to: url) + + let storage = SettingsFileStorage( + load: { try Data(contentsOf: $0) }, + save: { data, target in try data.write(to: target, options: .atomic) } + ) + let writer = LayoutsIncrementalWriter(storage: storage, url: url) + await writer.flush(["w1": .snapshot(snapshot(dir: "/w1"))]) + + // Self-healed: the new key persisted instead of the flush aborting forever. + #expect(readDict(storage, url)["w1"] != nil) + // The corrupt bytes were preserved under a rotated name, not overwritten. + let rotated = try FileManager.default + .contentsOfDirectory(atPath: dir.path(percentEncoded: false)) + .filter { $0.hasPrefix("layouts.json.corrupt-") } + #expect(rotated.count == 1) + } + + @Test func emptyChangesIsNoOp() async { + let storage = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + let writer = LayoutsIncrementalWriter(storage: storage, url: url) + + await writer.flush(["w1": .snapshot(snapshot(dir: "/w1"))]) + await writer.flush([:]) + + #expect(Set(readDict(storage, url).keys) == ["w1"]) + } +} From cca69ff7c797defdd2d8e7206c2f094961fb6994 Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Sat, 30 May 2026 17:17:49 +0200 Subject: [PATCH 3/3] Persist terminal layouts incrementally and reap zmx sessions by attach state Layouts were only written on quit, so a second instance launched mid-session pruned tabs the first instance had just opened and terminated their sessions. Drive a per-worktree debounced markLayoutDirty off the settled tab callbacks (create, close, projection drift, removal, rename, selection), capturing the freshest snapshot plus live agent records at fire time and merging off main; prune deletes immediately so a queued save cannot resurrect a removed worktree, and the on-quit synchronous flush stays the terminal write. reapOrphanSessions and the orphan subset of terminateAllSessions now spare any session that still has a client attached (or an unknown count), so a live instance keeps its sessions. A tab rename now routes through WorktreeTerminalState so a custom title persists incrementally instead of only at quit, while the layout-restore path keeps seeding setCustomTitle directly. Also fixes the close-last-surface-via-close_surface path that returned without emitting a projection, leaving the closed surface's session orphaned. --- supacode/App/supacodeApp.swift | 14 +- .../WorktreeTerminalManager.swift | 156 +++++++- .../Models/WorktreeTerminalState.swift | 15 + .../Views/WorktreeTerminalTabsView.swift | 2 +- supacodeTests/AgentPresence+TestHelpers.swift | 51 ++- ...erminalManagerLayoutPersistenceTests.swift | 358 ++++++++++++++++++ .../WorktreeTerminalManagerReaperTests.swift | 120 ++++++ .../WorktreeTerminalManagerTests.swift | 3 + 8 files changed, 690 insertions(+), 29 deletions(-) create mode 100644 supacodeTests/WorktreeTerminalManagerLayoutPersistenceTests.swift create mode 100644 supacodeTests/WorktreeTerminalManagerReaperTests.swift diff --git a/supacode/App/supacodeApp.swift b/supacode/App/supacodeApp.swift index af7eb2e20..21a9e4abd 100644 --- a/supacode/App/supacodeApp.swift +++ b/supacode/App/supacodeApp.swift @@ -51,8 +51,13 @@ final class SupacodeAppDelegate: NSObject, NSApplicationDelegate { private var bufferedDeeplinkURLs: [URL] = [] func applicationWillTerminate(_ notification: Notification) { - // Embed agent records so badges survive relaunch (agents only emit - // session_start once per process lifetime). + // Drop the queued debounce timers; an already-started async flush has no + // cancellation checkpoint and still completes, but the writer's lock plus the + // atomic temp+rename keep this terminal write from tearing. The on-quit save + // embeds agent records so badges survive relaunch (agents only emit + // session_start once per process lifetime), and a second concurrent instance + // overwriting the file is an accepted dev-only last-writer-wins window. + terminalManager?.cancelPendingLayoutSaves() let agentsBySurface = appStore?.state.agentPresence.agentsBySurface() ?? [:] terminalManager?.saveAllLayoutSnapshots(agentsBySurface: agentsBySurface) } @@ -168,6 +173,11 @@ struct SupacodeApp: App { _store = State(initialValue: appStore) appDelegate.appStore = appStore appDelegate.terminalManager = terminalManager + // Source live agent badge records for incremental layout captures; the [:] + // default would clobber badges that share a surface key on every save. + terminalManager.currentAgentsBySurface = { [weak appStore] in + appStore?.state.agentPresence.agentsBySurface() ?? [:] + } Self.configureSocketHandlers(terminalManager: terminalManager, store: appStore) } diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 74a669727..a92ea2e7f 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -30,6 +30,24 @@ final class WorktreeTerminalManager { private let hookEventSleep: @Sendable (Duration) async throws -> Void @ObservationIgnored @Dependency(\.zmxClient) private var zmxClient @ObservationIgnored @Dependency(\.analyticsClient) private var analyticsClient + /// Serialized off-main writer that merges per-worktree layout changes into + /// `layouts.json` without clobbering keys it isn't carrying. Built from the + /// dependency context at init so async flushes use the same storage the test + /// or app configured, not whatever context happens to be current at flush. + @ObservationIgnored private let layoutsWriter: LayoutsIncrementalWriter + /// Per-worktree debounce timers for incremental layout saves. + @ObservationIgnored private var layoutDirtyTasks: [Worktree.ID: Task] = [:] + /// Per-worktree in-flight positive flush Tasks. A delete awaits the live one + /// for its key so `.delete` always lands on the writer after the `.snapshot`, + /// preventing a stale positive flush from resurrecting a pruned worktree. + @ObservationIgnored private var layoutFlushTasks: [Worktree.ID: Task] = [:] + /// Sleeps the incremental-save debounce window; injected so tests drive it. + @ObservationIgnored private let layoutDebounceSleep: @Sendable (Duration) async throws -> Void + /// Debounce window before an incremental layout snapshot is flushed. + private static let layoutDebounceDuration: Duration = .seconds(1) + /// Reads the freshest `agentsBySurface` at flush time so incremental captures + /// embed live badge records instead of the empty default. + var currentAgentsBySurface: (() -> [UUID: [TerminalLayoutSnapshot.SurfaceAgentRecord]])? /// Holds `.idle` long enough to collapse PostToolUse/PreToolUse busy/idle alternation /// into a sustained busy; stays sub-perceptible for the badge clearing at end-of-session. private static let idleHookDebounceDuration: Duration = .milliseconds(400) @@ -54,6 +72,9 @@ final class WorktreeTerminalManager { ) { self.runtime = runtime self.hookEventSleep = { duration in try await clock.sleep(for: duration) } + self.layoutDebounceSleep = { duration in try await clock.sleep(for: duration) } + @Dependency(\.settingsFileStorage) var settingsFileStorage + self.layoutsWriter = LayoutsIncrementalWriter(storage: settingsFileStorage) let resolvedServer = socketServer ?? AgentHookSocketServer() guard resolvedServer.socketPath != nil else { self.socketServer = nil @@ -66,6 +87,8 @@ final class WorktreeTerminalManager { isolated deinit { for task in pendingIdleHookEvents.values { task.cancel() } + for task in layoutDirtyTasks.values { task.cancel() } + for task in layoutFlushTasks.values { task.cancel() } } private func configureSocketServer(_ server: AgentHookSocketServer) { @@ -300,7 +323,7 @@ final class WorktreeTerminalManager { guard id != selectedWorktreeID else { return } if let previousID = selectedWorktreeID, let previousState = states[previousID] { previousState.setAllSurfacesOccluded() - saveLayoutSnapshot?(previousID, previousState.captureLayoutSnapshot()) + markLayoutDirty(worktreeID: previousID) } selectedWorktreeID = id terminalLogger.info("Selected worktree \(id ?? "nil")") @@ -408,10 +431,15 @@ final class WorktreeTerminalManager { state.onTabCreated = { [weak self] in self?.emit(.tabCreated(worktreeID: worktree.id)) self?.emitProjection(for: worktree.id) + self?.markLayoutDirty(worktreeID: worktree.id) } state.onTabClosed = { [weak self] in self?.emit(.tabClosed(worktreeID: worktree.id)) self?.emitProjection(for: worktree.id) + self?.markLayoutDirty(worktreeID: worktree.id) + } + state.onTabRenamed = { [weak self] in + self?.markLayoutDirty(worktreeID: worktree.id) } state.onFocusChanged = { [weak self] surfaceID in self?.emit(.focusChanged(worktreeID: worktree.id, surfaceID: surfaceID)) @@ -431,9 +459,11 @@ final class WorktreeTerminalManager { } state.onTabProjectionChanged = { [weak self] projection in self?.emit(.tabProjectionChanged(worktreeID: worktree.id, projection)) + self?.markLayoutDirty(worktreeID: worktree.id) } state.onTabRemoved = { [weak self] tabID in self?.emit(.tabRemoved(worktreeID: worktree.id, tabID: tabID)) + self?.markLayoutDirty(worktreeID: worktree.id) } state.onTabProgressDisplayChanged = { [weak self] tabID, display in self?.emit(.tabProgressDisplayChanged(worktreeID: worktree.id, tabID: tabID, display: display)) @@ -484,8 +514,10 @@ final class WorktreeTerminalManager { } for (id, state) in removed { // Clear instead of resaving: archived / deleted worktrees should leave - // no trace in `layouts.json`. - saveLayoutSnapshot?(id, nil) + // no trace in `layouts.json`. The explicit delete bypasses the debounce + // and cancels any queued positive save so a pruned worktree can't be + // resurrected by an in-flight snapshot. + deleteLayoutSnapshot(worktreeID: id) state.closeAllSurfaces() // Signals the reducer to drop any orphan `terminalTabs` entries and // recently-removed-tab records for this worktree so a same-session @@ -503,6 +535,73 @@ final class WorktreeTerminalManager { killZmxSessions(prunedSessionIDs) } + /// Schedules a debounced incremental layout save for `worktreeID`. Coalesces + /// a burst of mutations into one write; the snapshot is captured at fire time + /// (freshest tree + agent records), mutated into the in-memory `@Shared` dict + /// on main, then merged into `layouts.json` off main. + func markLayoutDirty(worktreeID: Worktree.ID) { + layoutDirtyTasks[worktreeID]?.cancel() + layoutDirtyTasks[worktreeID] = Task { [weak self, layoutDebounceSleep] in + try? await layoutDebounceSleep(Self.layoutDebounceDuration) + guard !Task.isCancelled else { return } + self?.flushLayoutSnapshot(worktreeID: worktreeID) + } + } + + /// Fires after the debounce window: captures the freshest snapshot for + /// `worktreeID`, updates the in-memory `@Shared` dict on main, then queues the + /// off-main per-key merge. Its only caller is `markLayoutDirty`. + private func flushLayoutSnapshot(worktreeID: Worktree.ID) { + layoutDirtyTasks[worktreeID] = nil + guard let state = states[worktreeID] else { return } + let agents = currentAgentsBySurface?() ?? [:] + // A nil snapshot (no remaining tabs) clears the key rather than persisting + // an empty layout, matching the on-disk "no trace" semantics for emptiness. + let snapshot = state.captureLayoutSnapshot(agentsBySurface: agents) + saveLayoutSnapshot?(worktreeID, snapshot) + let change: LayoutsIncrementalWriter.Change = snapshot.map { .snapshot($0) } ?? .delete + let writer = layoutsWriter + let task = Task { [weak self] in + await writer.flush([worktreeID: change]) + self?.layoutFlushTasks[worktreeID] = nil + } + layoutFlushTasks[worktreeID] = task + } + + /// Removes `worktreeID` from disk immediately, bypassing the debounce and + /// cancelling any queued positive save so a stale snapshot can't resurrect a + /// removed worktree. Awaits any in-flight positive flush for the key first so + /// the `.delete` always reaches the writer after the `.snapshot`. + private func deleteLayoutSnapshot(worktreeID: Worktree.ID) { + layoutDirtyTasks[worktreeID]?.cancel() + layoutDirtyTasks[worktreeID] = nil + saveLayoutSnapshot?(worktreeID, nil) + let inflightFlush = layoutFlushTasks[worktreeID] + let writer = layoutsWriter + // We await inflightFlush so the .delete lands after any in-flight positive + // flush; prune also drops the id from states synchronously before any later + // saveAllLayoutSnapshots, so no positive snapshot is re-emitted. + let task = Task { [weak self] in + await inflightFlush?.value + await writer.flush([worktreeID: .delete]) + self?.layoutFlushTasks[worktreeID] = nil + } + layoutFlushTasks[worktreeID] = task + } + + /// Cancels every queued incremental save. Called before the on-quit + /// synchronous flush becomes the terminal write. + func cancelPendingLayoutSaves() { + for task in layoutDirtyTasks.values { task.cancel() } + layoutDirtyTasks.removeAll() + // Best-effort cancel: an already-started flush has no cancellation + // checkpoint in `applyAndWrite`, so it runs to completion. The writer's lock + // plus the atomic temp+rename keep the on-quit write from tearing; the worst + // case is a stale-but-valid key set on the next launch, never a corrupt file. + for task in layoutFlushTasks.values { task.cancel() } + layoutFlushTasks.removeAll() + } + /// Tears down persistent zmx sessions for worktrees that just left the keep set. /// Parallel kill so a single stuck daemon doesn't pin the executor for /// `subprocessTimeout * N` (the bound is now one timeout regardless of N). @@ -567,18 +666,33 @@ final class WorktreeTerminalManager { /// would survive forever. func terminateAllSessions() async { let trackedSurfaceIDs = states.values.flatMap(\.allSurfaceIDs) - let trackedSessionIDs = trackedSurfaceIDs.map(ZmxSessionID.make(surfaceID:)) + let trackedSessionIDs = Set(trackedSurfaceIDs.map(ZmxSessionID.make(surfaceID:))) for state in states.values { state.closeAllSurfaces() } emitHasAnyTerminalSurfaceIfNeeded() - let liveSessions = await zmxClient.listSessions() - let allSessions = Array(Set(trackedSessionIDs).union(liveSessions)) + // This instance's tracked sessions are always killed. The orphan subset + // (live and untracked) is attach-aware: spared when a client is attached or + // the count is unknown, so a concurrently-running instance keeps its + // sessions. Orphan reaping is therefore eventually consistent: the last + // instance to quit with no live clients sweeps what remains. + let liveSessions = await zmxClient.listSessionsWithClients() + let orphanSessions: [String] + if let liveSessions { + orphanSessions = liveSessions.filter { entry in + !trackedSessionIDs.contains(entry.name) && entry.clients == 0 + } + .map(\.name) + } else { + // nil = UNKNOWN probe; still force-kill tracked, but skip the orphan sweep. + terminalLogger.info("Skipping quit-time orphan sweep: zmx session probe unavailable") + orphanSessions = [] + } + let allSessions = Array(trackedSessionIDs.union(orphanSessions)) guard !allSessions.isEmpty else { return } - let orphanCount = Set(allSessions).subtracting(trackedSessionIDs).count analyticsClient.capture( "terminal_persistence_session_killed", - ["reason": "user_quit", "count": allSessions.count, "orphan_count": orphanCount] + ["reason": "user_quit", "count": allSessions.count, "orphan_count": orphanSessions.count] ) let client = zmxClient await withTaskGroup(of: Void.self) { group in @@ -589,11 +703,22 @@ final class WorktreeTerminalManager { } /// Reaps `supa-*` sessions zmx hosts that no persisted layout claims; - /// catches orphans from crashes / force-quits. + /// catches orphans from crashes / force-quits. Attach-aware: a session with + /// a live client (another Supacode instance or a manual `zmx attach`) is + /// spared, and a failed probe reaps nothing. func reapOrphanSessions(knownSurfaceIDs: Set) async { - let liveSessions = await zmxClient.listSessions() + guard let liveSessions = await zmxClient.listSessionsWithClients() else { + // nil = UNKNOWN (probe failed / timed out); never reap on no signal. + terminalLogger.info("Skipping orphan reap: zmx session probe unavailable") + return + } let knownSessionIDs = Set(knownSurfaceIDs.map(ZmxSessionID.make(surfaceID:))) - let orphans = Set(liveSessions).subtracting(knownSessionIDs) + // Only reap orphans we positively know have zero attached clients; spare + // clients>0 (in use) and clients==nil (unknown count). + let orphans = liveSessions.filter { entry in + !knownSessionIDs.contains(entry.name) && entry.clients == 0 + } + .map(\.name) guard !orphans.isEmpty else { return } terminalLogger.info("Reaping \(orphans.count) orphan zmx session(s)") analyticsClient.capture( @@ -672,9 +797,16 @@ final class WorktreeTerminalManager { assertionFailure("saveLayoutSnapshot closure not configured.") return } + // The actor is the sole disk writer (`LayoutsKey.save` is a no-op), so the + // on-quit terminal write goes through `flushSync` while still updating the + // in-memory `@Shared` dict via `saveLayoutSnapshot` for any live readers. + var changes: [Worktree.ID: LayoutsIncrementalWriter.Change] = [:] for (id, state) in states { - saveLayoutSnapshot(id, state.captureLayoutSnapshot(agentsBySurface: agentsBySurface)) + let snapshot = state.captureLayoutSnapshot(agentsBySurface: agentsBySurface) + saveLayoutSnapshot(id, snapshot) + changes[id] = snapshot.map { .snapshot($0) } ?? .delete } + layoutsWriter.flushSync(changes) } func surfaceBackgroundColorScheme() -> ColorScheme { diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 79988c656..58af7eb5b 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -127,6 +127,9 @@ final class WorktreeTerminalState { var onNotificationIndicatorChanged: (() -> Void)? var onTabCreated: (() -> Void)? var onTabClosed: (() -> Void)? + /// Fires when the user renames a tab. Manager forwards to the layout-persist + /// sink so a custom title survives relaunch without waiting for quit. + var onTabRenamed: (() -> Void)? var onFocusChanged: ((UUID) -> Void)? var onTaskStatusChanged: ((WorktreeTaskStatus) -> Void)? var onBlockingScriptCompleted: ((BlockingScriptKind, Int?, TerminalTabID?) -> Void)? @@ -638,6 +641,14 @@ final class WorktreeTerminalState { onTabClosed?() } + /// User-initiated rename. Routes through the manager so the new title (or its + /// removal on an empty commit) persists incrementally, unlike the restore path + /// which seeds `setCustomTitle` directly from a snapshot. + func renameTab(_ tabId: TerminalTabID, title: String) { + tabManager.setCustomTitle(tabId, title: title) + onTabRenamed?() + } + func closeOtherTabs(keeping tabId: TerminalTabID) { let ids = tabManager.tabs.map(\.id).filter { $0 != tabId } for id in ids { @@ -1930,6 +1941,10 @@ final class WorktreeTerminalState { } } emitTaskStatusIfChanged() + // Closing the last surface via `close_surface` removes the tab here but + // skips the `closeTab` projection path; emit one so `onTabRemoved` fires + // and the layout persistence sink observes the tab going away. + emitTabProjection(for: tabId) return } updateTree(newTree, for: tabId) diff --git a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift index 5bb94f67e..692846697 100644 --- a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift +++ b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift @@ -52,7 +52,7 @@ struct WorktreeTerminalTabsView: View { state.dismissSplitZoom(for: tabId) }, renameTab: { tabId, newTitle in - state.tabManager.setCustomTitle(tabId, title: newTitle) + state.renameTab(tabId, title: newTitle) }, ) .transition(.move(edge: .top).combined(with: .opacity)) diff --git a/supacodeTests/AgentPresence+TestHelpers.swift b/supacodeTests/AgentPresence+TestHelpers.swift index 0e7e634f1..07c64b92b 100644 --- a/supacodeTests/AgentPresence+TestHelpers.swift +++ b/supacodeTests/AgentPresence+TestHelpers.swift @@ -3,20 +3,28 @@ import Foundation @testable import supacode -/// Test-only harness around an `AgentPresenceFeature.State`. Drains the -/// manager's event stream and routes `agentHookEventReceived` / +/// Test-only harness around an `AgentPresenceFeature.State`. A background task +/// drains the manager's event stream and routes `agentHookEventReceived` / /// `surfacesClosed` events into the reducer so callers can drive the manager /// via `server.onEvent(...)` and then await `harness.drain()` to settle -/// presence on the same loop tick. +/// presence before asserting. @MainActor final class PresenceTestHarness { var state = AgentPresenceFeature.State() private let reducer = AgentPresenceFeature() - private var continuation: AsyncStream.Continuation? private var stream: AsyncStream? private var consumeTask: Task? + /// Bumped each time the consume task reduces a stream event. + private var processedCount = 0 + /// Bumped each time the consume task is about to wait for the next event, i.e. + /// it has drained everything buffered so far. + private var parkCount = 0 func send(_ action: AgentPresenceFeature.Action) { + reduce(action) + } + + private func reduce(_ action: AgentPresenceFeature.Action) { _ = reducer.reduce(into: &state, action: action) } @@ -29,13 +37,24 @@ final class PresenceTestHarness { send(.livenessSweepResult(snapshot: snapshot, alive: alive)) } - /// Pumps any events buffered on the manager's stream into the reducer and - /// returns. Tests call this after `server.onEvent(...)` so presence state - /// settles before assertions. + /// Settles presence after `server.onEvent(...)` / `clock.advance(...)`. Each + /// pass runs `megaYield` (flushing the consume task plus any clock-awoken + /// manager emit, e.g. an idle debounce resuming after `clock.advance`) and + /// returns once the consumer has parked again with no reduction in the final + /// pass, i.e. it observed and drained everything this call produced. The cap + /// keeps a genuinely quiet stream from looping forever. func drain() async { - // Yield repeatedly so the consume task drains every buffered event before - // returning to the test thread. - for _ in 0..<16 { await Task.yield() } + guard consumeTask != nil else { return } + var settled = 0 + for _ in 0..<64 { + let parksBefore = parkCount + let processedBefore = processedCount + await Task.megaYield(count: 10_000) + let parkedAgain = parkCount > parksBefore + let quiet = processedCount == processedBefore + settled = parkedAgain && quiet ? settled + 1 : 0 + if settled >= 2 { return } + } } func attach(to manager: WorktreeTerminalManager) { @@ -43,19 +62,23 @@ final class PresenceTestHarness { self.stream = stream consumeTask?.cancel() consumeTask = Task { - for await event in stream { + var iterator = stream.makeAsyncIterator() + while true { + self.parkCount += 1 + guard let event = await iterator.next() else { return } switch event { case .agentHookEventReceived(let payload): - self.send(.hookEventReceived(payload)) + self.reduce(.hookEventReceived(payload)) case .surfacesClosed(let ids): if ids.count == 1, let id = ids.first { - self.send(.surfaceClosed(id)) + self.reduce(.surfaceClosed(id)) } else { - self.send(.surfacesClosed(ids)) + self.reduce(.surfacesClosed(ids)) } default: continue } + self.processedCount += 1 } } } diff --git a/supacodeTests/WorktreeTerminalManagerLayoutPersistenceTests.swift b/supacodeTests/WorktreeTerminalManagerLayoutPersistenceTests.swift new file mode 100644 index 000000000..c2581627c --- /dev/null +++ b/supacodeTests/WorktreeTerminalManagerLayoutPersistenceTests.swift @@ -0,0 +1,358 @@ +import Clocks +import Dependencies +import Foundation +import SupacodeSettingsShared +import Testing + +@testable import supacode + +@MainActor +@Suite(.serialized) +struct LayoutPersistenceManagerTests { + /// Counts writer saves and mirrors the in-memory `@Shared(.layouts)` mutation + /// the app performs, so a test can assert both coalescing and final on-disk + /// state from one storage. + private struct Harness { + let manager: WorktreeTerminalManager + let clock: TestClock + let saveCount: LockIsolated + let storage: SettingsFileStorage + let url: URL + /// When non-nil, the next save whose payload still contains `worktreeID` + /// (a positive snapshot) blocks on this semaphore so the test can hold the + /// positive flush Task in-flight while it triggers the prune. + let gate: LockIsolated<(worktreeID: String, semaphore: DispatchSemaphore)?> + /// Flips true once the gated positive save is blocked, proving the flush + /// Task is suspended inside `writer.flush` (i.e. `layoutFlushTasks[id]` is + /// still non-nil, before the spawning Task can clear it). + let gateEngaged: LockIsolated + } + + private func makeHarness() -> Harness { + let clock = TestClock() + let saveCount = LockIsolated(0) + let gate = LockIsolated<(worktreeID: String, semaphore: DispatchSemaphore)?>(nil) + let gateEngaged = LockIsolated(false) + let inner = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + let storage = SettingsFileStorage( + load: { try inner.load($0) }, + save: { data, target in + if target == url { saveCount.withValue { $0 += 1 } } + // Block a positive snapshot write so the spawning flush Task stays + // in-flight; a delete payload (key absent) passes straight through. + if target == url, + let active = gate.value, + let dict = try? JSONDecoder().decode([String: TerminalLayoutSnapshot].self, from: data), + dict[active.worktreeID] != nil + { + gate.setValue(nil) + gateEngaged.setValue(true) + active.semaphore.wait() + } + try inner.save(data, target) + } + ) + let manager = withDependencies { + $0.settingsFileStorage = storage + } operation: { + WorktreeTerminalManager(runtime: GhosttyRuntime(), clock: clock) + } + // Mirror the app's in-memory dict mutation; the writer's storage is the + // on-disk source of truth these tests assert against. + manager.saveLayoutSnapshot = { _, _ in } + return Harness( + manager: manager, + clock: clock, + saveCount: saveCount, + storage: storage, + url: url, + gate: gate, + gateEngaged: gateEngaged + ) + } + + private func readDict(_ harness: Harness) -> [String: TerminalLayoutSnapshot] { + guard let data = try? harness.storage.load(harness.url) else { return [:] } + return (try? JSONDecoder().decode([String: TerminalLayoutSnapshot].self, from: data)) ?? [:] + } + + /// Awaits a condition by pumping the off-main actor flush onto the executor; + /// bounded so a never-true predicate can't hang the suite. No `Task.sleep`. + /// `megaYield` runs detached background work between checks so the writer + /// actor's flush actually lands even under a saturated cooperative pool. + private func waitUntil(_ predicate: () -> Bool) async { + for _ in 0..<200 { + if predicate() { return } + await Task.megaYield() + } + } + + /// Yields enough for the latest debounce task to register its sleep with the + /// TestClock, then advances past the window so the flush fires. + private func settleThenAdvance(_ clock: TestClock) async { + await Task.megaYield() + await clock.advance(by: .seconds(1)) + } + + private func makeWorktree(id: String = "/tmp/repo/wt-1") -> Worktree { + Worktree( + id: id, + name: URL(fileURLWithPath: id).lastPathComponent, + detail: "detail", + workingDirectory: URL(fileURLWithPath: id), + repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") + ) + } + + @Test func debounceCoalescesBurstIntoOneWrite() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + + // Several structural mutations within the window must coalesce to one flush. + _ = state.createTab(focusing: false) + _ = state.createTab(focusing: false) + _ = state.createTab(focusing: false) + + #expect(harness.saveCount.value == 0) + await settleThenAdvance(harness.clock) + await waitUntil { harness.saveCount.value == 1 } + #expect(harness.saveCount.value == 1) + #expect(readDict(harness)[worktree.id] != nil) + } + + @Test func pruneDeletesAndCancelsQueuedSave() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + _ = state.createTab(focusing: false) + + // Seed disk with a prior snapshot so the delete has something to remove. + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[worktree.id] != nil } + + // Queue a positive save, then prune before the window elapses. + _ = state.createTab(focusing: false) + harness.manager.prune(keeping: []) + await waitUntil { readDict(harness)[worktree.id] == nil } + + // Pin the save count once the delete has flushed; a resurrecting positive + // write would bump it, so gating on a positive increment proves whether the + // queued save was actually cancelled rather than merely lagging. + let savesAfterDelete = harness.saveCount.value + await harness.clock.advance(by: .seconds(1)) + await waitUntil { harness.saveCount.value > savesAfterDelete } + #expect(harness.saveCount.value == savesAfterDelete) + #expect(readDict(harness)[worktree.id] == nil) + } + + @Test func pruneAfterDebounceFiresDoesNotResurrect() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + _ = state.createTab(focusing: false) + + // Seed disk with a prior snapshot so the delete has something to remove. + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[worktree.id] != nil } + + // Arm the gate so the positive snapshot write blocks mid-flush, pinning the + // flush Task in-flight. Then queue the positive save and fire the debounce. + let release = DispatchSemaphore(value: 0) + let gatedID = worktree.id + harness.gate.setValue((worktreeID: gatedID, semaphore: release)) + _ = state.createTab(focusing: false) + await settleThenAdvance(harness.clock) + + // The positive flush Task is provably suspended inside `writer.flush` (save + // blocked), so `layoutFlushTasks[id]` is still non-nil. Prune now captures + // that in-flight Task and must await it before deleting. + await waitUntil { harness.gateEngaged.value } + #expect(harness.gateEngaged.value) + harness.manager.prune(keeping: []) + + // Release the positive flush; the delete chained behind it must win. + release.signal() + await waitUntil { readDict(harness)[worktree.id] == nil } + #expect(readDict(harness)[worktree.id] == nil) + } + + @Test func cancelPendingLayoutSavesDropsQueuedFlush() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + _ = state.createTab(focusing: false) + + harness.manager.cancelPendingLayoutSaves() + await harness.clock.advance(by: .seconds(1)) + + // Queue and flush a control worktree's save AFTER the cancel. Once the + // control write lands, the executor has run long enough that the cancelled + // save would have fired too, so its continued absence proves suppression + // rather than mere lag. + let control = makeWorktree(id: "/tmp/repo/wt-control") + let controlState = harness.manager.state(for: control) + _ = controlState.createTab(focusing: false) + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[control.id] != nil } + + #expect(readDict(harness)[control.id] != nil) + #expect(harness.saveCount.value == 1) + #expect(readDict(harness)[worktree.id] == nil) + } + + @Test func mergePreservesSiblingKeyWrittenByAnotherFlush() async { + let harness = makeHarness() + let wt1 = makeWorktree(id: "/tmp/repo/wt-1") + let wt2 = makeWorktree(id: "/tmp/repo/wt-2") + + let state1 = harness.manager.state(for: wt1) + _ = state1.createTab(focusing: false) + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[wt1.id] != nil } + + let state2 = harness.manager.state(for: wt2) + _ = state2.createTab(focusing: false) + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[wt2.id] != nil } + + let dict = readDict(harness) + #expect(Set(dict.keys) == [wt1.id, wt2.id]) + } + + @Test func incrementalCaptureEmbedsLiveAgentRecords() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + guard let tabID = state.createTab(focusing: false), + let surface = state.splitTree(for: tabID).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + let record = TerminalLayoutSnapshot.SurfaceAgentRecord(agent: "claude", pids: [42], activity: "busy") + harness.manager.currentAgentsBySurface = { [surface.id: [record]] } + // Mark dirty again now that the agent reader is wired. + harness.manager.markLayoutDirty(worktreeID: worktree.id) + + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[worktree.id]?.tabs.first?.layout != nil } + + let leaf = readDict(harness)[worktree.id]?.tabs.first?.layout + guard case .leaf(let persisted) = leaf else { + Issue.record("Expected a leaf layout") + return + } + #expect(persisted.agents == [record]) + } + + @Test func presentButUnreadableFileAbortsAndPreservesSiblings() async { + let clock = TestClock() + let inner = SettingsFileStorage.inMemory() + let url = SupacodePaths.layoutsURL + // Flips on after the two siblings are seeded so the third flush's read fails + // with a non-absent error, exercising the abort branch. + let failLoad = LockIsolated(false) + let loadFailCount = LockIsolated(0) + let storage = SettingsFileStorage( + load: { target in + if target == url, failLoad.value { + loadFailCount.withValue { $0 += 1 } + throw CocoaError(.fileReadCorruptFile) + } + return try inner.load(target) + }, + save: { data, target in try inner.save(data, target) } + ) + let manager = withDependencies { + $0.settingsFileStorage = storage + } operation: { + WorktreeTerminalManager(runtime: GhosttyRuntime(), clock: clock) + } + manager.saveLayoutSnapshot = { _, _ in } + + // Raw reader bypasses the throwing flag so assertions see the real disk state. + let readRaw: () -> [String: TerminalLayoutSnapshot] = { + guard let data = try? inner.load(url) else { return [:] } + return (try? JSONDecoder().decode([String: TerminalLayoutSnapshot].self, from: data)) ?? [:] + } + + let wt1 = makeWorktree(id: "/tmp/repo/wt-1") + let wt2 = makeWorktree(id: "/tmp/repo/wt-2") + let state1 = manager.state(for: wt1) + _ = state1.createTab(focusing: false) + await Task.megaYield() + await clock.advance(by: .seconds(1)) + await waitUntil { readRaw()[wt1.id] != nil } + let state2 = manager.state(for: wt2) + _ = state2.createTab(focusing: false) + await Task.megaYield() + await clock.advance(by: .seconds(1)) + await waitUntil { readRaw()[wt2.id] != nil } + #expect(Set(readRaw().keys) == [wt1.id, wt2.id]) + + // Make the merge read fail, then flush a third key. The writer must abort + // rather than splice into an empty dict and clobber the two siblings. + failLoad.setValue(true) + let wt3 = makeWorktree(id: "/tmp/repo/wt-3") + let state3 = manager.state(for: wt3) + _ = state3.createTab(focusing: false) + await Task.megaYield() + await clock.advance(by: .seconds(1)) + await waitUntil { loadFailCount.value > 0 } + #expect(loadFailCount.value > 0) + + let dict = readRaw() + #expect(Set(dict.keys) == [wt1.id, wt2.id]) + #expect(dict[wt3.id] == nil) + } + + @Test func renamePersistsCustomTitleIncrementally() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + guard let tabID = state.createTab(focusing: false) else { + Issue.record("Expected a tab") + return + } + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[worktree.id] != nil } + + state.renameTab(tabID, title: "Renamed") + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[worktree.id]?.tabs.first?.customTitle == "Renamed" } + + #expect(readDict(harness)[worktree.id]?.tabs.first?.customTitle == "Renamed") + } + + @Test func onQuitFlushSyncPersistsLiveStatesAsTerminalWrite() { + let harness = makeHarness() + let wt1 = makeWorktree(id: "/tmp/repo/wt-1") + let wt2 = makeWorktree(id: "/tmp/repo/wt-2") + _ = harness.manager.state(for: wt1).createTab(focusing: false) + _ = harness.manager.state(for: wt2).createTab(focusing: false) + + // Mirror the quit path: drop queued debounce saves, then the synchronous + // on-quit flush must land every live state as the terminal on-disk write. + harness.manager.cancelPendingLayoutSaves() + harness.manager.saveAllLayoutSnapshots() + + #expect(Set(readDict(harness).keys) == [wt1.id, wt2.id]) + } + + @Test func capturedSnapshotRoundTripsThroughDisk() async { + let harness = makeHarness() + let worktree = makeWorktree() + let state = harness.manager.state(for: worktree) + _ = state.createTab(focusing: false) + _ = state.createTab(focusing: false) + + await settleThenAdvance(harness.clock) + await waitUntil { readDict(harness)[worktree.id] != nil } + + let persisted = readDict(harness)[worktree.id] + let inMemory = state.captureLayoutSnapshot() + #expect(persisted == inMemory) + } +} diff --git a/supacodeTests/WorktreeTerminalManagerReaperTests.swift b/supacodeTests/WorktreeTerminalManagerReaperTests.swift new file mode 100644 index 000000000..b4226e1e4 --- /dev/null +++ b/supacodeTests/WorktreeTerminalManagerReaperTests.swift @@ -0,0 +1,120 @@ +import Dependencies +import Foundation +import Testing + +@testable import supacode + +@MainActor +struct WorktreeTerminalManagerReaperTests { + /// Builds a manager whose injected zmx client records every kill and serves + /// the supplied `ls` listing (nil = probe failed). + private func makeManager( + listing: [ZmxSessionListParser.Entry]?, + killed: LockIsolated<[String]> + ) -> WorktreeTerminalManager { + withDependencies { + $0.zmxClient = ZmxClient( + executableURL: { nil }, + isBundled: { true }, + wrapCommand: { _, _ in nil }, + killSession: { id in killed.withValue { $0.append(id) } }, + listSessionsWithClients: { listing } + ) + } operation: { + WorktreeTerminalManager(runtime: GhosttyRuntime()) + } + } + + private func session(for surfaceID: UUID) -> String { + ZmxSessionID.make(surfaceID: surfaceID) + } + + @Test func reapSparesAttachedOrphanEvenWhenUnknown() async { + let attached = UUID() + let idle = UUID() + let killed = LockIsolated<[String]>([]) + let manager = makeManager( + listing: [ + .init(name: session(for: attached), clients: 1), + .init(name: session(for: idle), clients: 0), + ], + killed: killed + ) + + await manager.reapOrphanSessions(knownSurfaceIDs: []) + + #expect(killed.value == [session(for: idle)]) + } + + @Test func reapSkipsErrAndUnreachableSessions() async { + let unreachable = UUID() + let killed = LockIsolated<[String]>([]) + let manager = makeManager( + listing: [.init(name: session(for: unreachable), clients: nil)], + killed: killed + ) + + await manager.reapOrphanSessions(knownSurfaceIDs: []) + + #expect(killed.value.isEmpty) + } + + @Test func reapKillsNothingWhenProbeUnavailable() async { + let idle = UUID() + let killed = LockIsolated<[String]>([]) + let manager = makeManager(listing: nil, killed: killed) + + await manager.reapOrphanSessions(knownSurfaceIDs: [idle]) + + #expect(killed.value.isEmpty) + } + + @Test func terminateKillsTrackedEvenWithLiveClientsButSparesUntrackedInUse() async { + let killed = LockIsolated<[String]>([]) + let listing = LockIsolated<[ZmxSessionListParser.Entry]?>([]) + let manager = withDependencies { + $0.zmxClient = ZmxClient( + executableURL: { nil }, + isBundled: { true }, + wrapCommand: { _, _ in nil }, + killSession: { id in killed.withValue { $0.append(id) } }, + listSessionsWithClients: { listing.value } + ) + } operation: { + WorktreeTerminalManager(runtime: GhosttyRuntime()) + } + + let worktree = makeWorktree() + let state = manager.state(for: worktree) + guard let tabID = state.createTab(focusing: false), + let surface = state.splitTree(for: tabID).root?.leftmostLeaf() + else { + Issue.record("Expected a tab and surface") + return + } + let trackedSurfaceID = surface.id + let trackedSession = session(for: trackedSurfaceID) + let untrackedSession = session(for: UUID()) + // The tracked session reports clients>0 (must still die) and an untracked + // session is in use (must be spared). + listing.setValue([ + .init(name: trackedSession, clients: 2), + .init(name: untrackedSession, clients: 3), + ]) + + await manager.terminateAllSessions() + + #expect(killed.value == [trackedSession]) + } + + private func makeWorktree(id: String = "/tmp/repo/wt-1") -> Worktree { + let name = URL(fileURLWithPath: id).lastPathComponent + return Worktree( + id: id, + name: name, + detail: "detail", + workingDirectory: URL(fileURLWithPath: id), + repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") + ) + } +} diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index bb8e0b999..51a1bdc5a 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -303,6 +303,9 @@ struct WorktreeTerminalManagerTests { server.onEvent?(makeHookEvent(.busy, surfaceID: surface.id)) server.onEvent?(makeHookEvent(.idle, surfaceID: surface.id)) + // Settle the stream-delivered events before the direct close so a buffered + // busy can't resurrect activity after the surface is gone. + await presence.drain() presence.send(.surfaceClosed(surface.id)) await clock.advance(by: .milliseconds(500)) await presence.drain()