Quality pass: data persistence / storage

Makefile

@@ -1,4 +1,4 @@
-.PHONY: build setup ghostty app release sign clean clean-all check help
+.PHONY: build setup ghostty app release sign clean clean-all check test help
 
 FRAMEWORKS_DIR := Frameworks
 XCFW := $(FRAMEWORKS_DIR)/GhosttyKit.xcframework
@@ -14,6 +14,7 @@ help:
 	@echo "  build      Full build: submodules + ghostty + swift build (default)"
 	@echo "  setup      Init submodules and check build prerequisites"
 	@echo "  check      Verify all build and runtime prerequisites"
+	@echo "  test       Run unit tests for CrowCore and CrowPersistence"
 	@echo "  ghostty    Build GhosttyKit framework"
 	@echo "  app        Swift build only (debug)"
 	@echo "  release    Release build + .app bundle"
@@ -65,6 +66,12 @@ clean:
 clean-all: clean
 	rm -rf $(FRAMEWORKS_DIR)
 
+# --- Test ---
+
+test:
+	swift test --package-path Packages/CrowCore
+	swift test --package-path Packages/CrowPersistence
+
 # --- Check ---
 
 check: setup

Packages/CrowPersistence/Sources/CrowPersistence/JSONStore.swift

@@ -22,6 +22,17 @@ public struct StoreData: Codable, Sendable {
 }
 
 /// Thread-safe JSON file store for session persistence.
+///
+/// Uses `NSLock` to serialize access to the in-memory `StoreData` and disk writes.
+/// The `nonisolated(unsafe)` annotation on `_data` is safe because all reads and writes
+/// go through the lock. An actor was not used because `mutate()` must be synchronous
+/// to support callers on the MainActor without requiring `await`.
+///
+/// On initialization, if `store.json` is corrupt (fails to decode), the file is backed up
+/// to `store.json.bak` and the store starts fresh with empty data.
+///
+/// Performs a one-time migration from the legacy "rm-ai-ide" application support directory
+/// when no "crow" directory exists yet (via `AppSupportDirectory`).
 public final class JSONStore: Sendable {
     private let fileURL: URL
     private let lock = NSLock()
@@ -60,10 +71,9 @@ public final class JSONStore: Sendable {
 
     public func mutate(_ transform: (inout StoreData) -> Void) {
         lock.lock()
+        defer { lock.unlock() }
         transform(&_data)
-        let snapshot = _data
-        lock.unlock()
-        Self.save(snapshot, to: fileURL)
+        Self.save(_data, to: fileURL)
     }
 
     private static func save(_ data: StoreData, to url: URL) {

Packages/CrowPersistence/Sources/CrowPersistence/Repositories/SessionRepository.swift

@@ -27,11 +27,13 @@ public struct SessionRepository: Sendable {
         }
     }
 
+    /// Delete a session and all related data (worktrees, links, terminals) in a single atomic mutation.
     public func delete(id: UUID) {
         store.mutate { data in
             data.sessions.removeAll { $0.id == id }
             data.worktrees.removeAll { $0.sessionID == id }
             data.links.removeAll { $0.sessionID == id }
+            data.terminals.removeAll { $0.sessionID == id }
         }
     }
 

Packages/CrowPersistence/Tests/CrowPersistenceTests/JSONStoreTests.swift

@@ -0,0 +1,143 @@
+import Foundation
+import Testing
+@testable import CrowPersistence
+@testable import CrowCore
+
+@Test func emptyStoreCreation() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let data = store.data
+    #expect(data.sessions.isEmpty)
+    #expect(data.worktrees.isEmpty)
+    #expect(data.links.isEmpty)
+    #expect(data.terminals.isEmpty)
+}
+
+@Test func mutatePersistsToFile() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let session = Session(name: "persist-test")
+    store.mutate { data in
+        data.sessions.append(session)
+    }
+
+    // Create a new store from the same directory — data should survive
+    let reloaded = JSONStore(directory: dir)
+    #expect(reloaded.data.sessions.count == 1)
+    #expect(reloaded.data.sessions.first?.name == "persist-test")
+    #expect(reloaded.data.sessions.first?.id == session.id)
+}
+
+@Test func concurrentMutatesAreConsistent() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let iterations = 50
+
+    // Dispatch concurrent mutates that each append a session
+    let group = DispatchGroup()
+    for i in 0..<iterations {
+        group.enter()
+        DispatchQueue.global().async {
+            store.mutate { data in
+                data.sessions.append(Session(name: "session-\(i)"))
+            }
+            group.leave()
+        }
+    }
+    group.wait()
+
+    #expect(store.data.sessions.count == iterations)
+
+    // Verify file on disk is also consistent
+    let reloaded = JSONStore(directory: dir)
+    #expect(reloaded.data.sessions.count == iterations)
+}
+
+@Test func corruptFileCreatesBackup() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true)
+    let storeFile = dir.appendingPathComponent("store.json")
+    try "not valid json{{{".write(to: storeFile, atomically: true, encoding: .utf8)
+
+    // Init should detect corruption, create backup, and start fresh
+    let store = JSONStore(directory: dir)
+    #expect(store.data.sessions.isEmpty)
+
+    // Backup file should exist
+    let backupFile = dir.appendingPathComponent("store.json.bak")
+    #expect(FileManager.default.fileExists(atPath: backupFile.path))
+
+    // Backup should contain the original corrupt content
+    let backupContent = try String(contentsOf: backupFile, encoding: .utf8)
+    #expect(backupContent == "not valid json{{{")
+}
+
+@Test func filePermissionsAreOwnerOnly() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    store.mutate { data in
+        data.sessions.append(Session(name: "perm-test"))
+    }
+
+    let storeFile = dir.appendingPathComponent("store.json")
+    let attributes = try FileManager.default.attributesOfItem(atPath: storeFile.path)
+    let permissions = attributes[.posixPermissions] as? Int
+    #expect(permissions == 0o600)
+}
+
+@Test func roundTripAllModelTypes() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let sessionID = UUID()
+    let session = Session(id: sessionID, name: "round-trip")
+    let worktree = SessionWorktree(
+        sessionID: sessionID, repoName: "crow", repoPath: "/path/to/crow",
+        worktreePath: "/path/to/worktree", branch: "feature/test", workspace: "RadiusMethod"
+    )
+    let link = SessionLink(
+        sessionID: sessionID, label: "Issue", url: "https://github.com/org/repo/issues/1",
+        linkType: .ticket
+    )
+    let terminal = SessionTerminal(
+        sessionID: sessionID, name: "Claude Code", cwd: "/path/to/worktree",
+        command: nil, isManaged: true
+    )
+
+    let store = JSONStore(directory: dir)
+    store.mutate { data in
+        data.sessions.append(session)
+        data.worktrees.append(worktree)
+        data.links.append(link)
+        data.terminals.append(terminal)
+    }
+
+    let reloaded = JSONStore(directory: dir)
+    #expect(reloaded.data.sessions.count == 1)
+    #expect(reloaded.data.worktrees.count == 1)
+    #expect(reloaded.data.links.count == 1)
+    #expect(reloaded.data.terminals.count == 1)
+
+    let rt = reloaded.data.terminals.first!
+    #expect(rt.name == "Claude Code")
+    #expect(rt.isManaged == true)
+    #expect(rt.sessionID == sessionID)
+
+    let rw = reloaded.data.worktrees.first!
+    #expect(rw.branch == "feature/test")
+    #expect(rw.workspace == "RadiusMethod")
+
+    let rl = reloaded.data.links.first!
+    #expect(rl.linkType == .ticket)
+    #expect(rl.label == "Issue")
+}

Packages/CrowPersistence/Tests/CrowPersistenceTests/SessionRepositoryTests.swift

@@ -0,0 +1,128 @@
+import Foundation
+import Testing
+@testable import CrowPersistence
+@testable import CrowCore
+
+@Test func saveAndFind() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let repo = SessionRepository(store: store)
+
+    let session = Session(name: "find-test", status: .active, ticketURL: "https://example.com/1")
+    repo.save(session)
+
+    let found = repo.find(id: session.id)
+    #expect(found != nil)
+    #expect(found?.name == "find-test")
+    #expect(found?.ticketURL == "https://example.com/1")
+}
+
+@Test func saveUpdatesExisting() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let repo = SessionRepository(store: store)
+
+    var session = Session(name: "original")
+    repo.save(session)
+    #expect(repo.all().count == 1)
+
+    session.name = "updated"
+    repo.save(session)
+    #expect(repo.all().count == 1)
+    #expect(repo.find(id: session.id)?.name == "updated")
+}
+
+@Test func deleteCascadesAllRelatedData() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let repo = SessionRepository(store: store)
+
+    let sessionID = UUID()
+    let session = Session(id: sessionID, name: "cascade-test")
+    repo.save(session)
+
+    // Add related data directly via store
+    store.mutate { data in
+        data.worktrees.append(SessionWorktree(
+            sessionID: sessionID, repoName: "crow", repoPath: "/repo",
+            worktreePath: "/wt", branch: "feature/x", workspace: "ws"
+        ))
+        data.links.append(SessionLink(
+            sessionID: sessionID, label: "PR", url: "https://example.com", linkType: .pr
+        ))
+        data.terminals.append(SessionTerminal(
+            sessionID: sessionID, name: "Shell", cwd: "/wt"
+        ))
+    }
+
+    // Verify data exists
+    #expect(repo.worktrees(for: sessionID).count == 1)
+    #expect(repo.links(for: sessionID).count == 1)
+    #expect(store.data.terminals.filter { $0.sessionID == sessionID }.count == 1)
+
+    // Delete should cascade to all related data
+    repo.delete(id: sessionID)
+
+    #expect(repo.find(id: sessionID) == nil)
+    #expect(repo.worktrees(for: sessionID).isEmpty)
+    #expect(repo.links(for: sessionID).isEmpty)
+    #expect(store.data.terminals.filter { $0.sessionID == sessionID }.isEmpty)
+}
+
+@Test func allReturnsSessions() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let repo = SessionRepository(store: store)
+
+    repo.save(Session(name: "session-1"))
+    repo.save(Session(name: "session-2"))
+    repo.save(Session(name: "session-3"))
+
+    let all = repo.all()
+    #expect(all.count == 3)
+    #expect(Set(all.map(\.name)) == Set(["session-1", "session-2", "session-3"]))
+}
+
+@Test func worktreesAndLinksFilterBySession() throws {
+    let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    defer { try? FileManager.default.removeItem(at: dir) }
+
+    let store = JSONStore(directory: dir)
+    let repo = SessionRepository(store: store)
+
+    let session1 = UUID()
+    let session2 = UUID()
+
+    store.mutate { data in
+        data.worktrees.append(SessionWorktree(
+            sessionID: session1, repoName: "a", repoPath: "/a",
+            worktreePath: "/wt-a", branch: "feature/a", workspace: "ws"
+        ))
+        data.worktrees.append(SessionWorktree(
+            sessionID: session2, repoName: "b", repoPath: "/b",
+            worktreePath: "/wt-b", branch: "feature/b", workspace: "ws"
+        ))
+        data.links.append(SessionLink(
+            sessionID: session1, label: "Issue", url: "https://example.com/1", linkType: .ticket
+        ))
+        data.links.append(SessionLink(
+            sessionID: session1, label: "PR", url: "https://example.com/2", linkType: .pr
+        ))
+        data.links.append(SessionLink(
+            sessionID: session2, label: "Repo", url: "https://example.com/3", linkType: .repo
+        ))
+    }
+
+    #expect(repo.worktrees(for: session1).count == 1)
+    #expect(repo.worktrees(for: session2).count == 1)
+    #expect(repo.links(for: session1).count == 2)
+    #expect(repo.links(for: session2).count == 1)
+}