diff --git a/CLAUDE.md b/CLAUDE.md index 73095105..b89ae3dc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,6 +154,8 @@ every edit; target the code you touched. Run the full suite only before opening/ - Read `AccessorySessions.md` before editing accessory terminal panes, sub-session launch/detection, terminal workspace linked-session restore, or `session_relationships`. - Session/workspace management state belongs in `SessionMetadataStore` / SQLite, not UserDefaults. +- Persistent-state paths (metadata sqlite, `approvals/`, `claims/`, `hooks/`) resolve through `AgentHubApplicationSupport.baseDirectoryURL` — never build `~/Library/Application Support/AgentHub` paths directly. Test processes are automatically sandboxed into a temp directory so no suite can touch real user state; `AGENTHUB_APP_SUPPORT_DIR` overrides explicitly. +- Workspace-state saves are gated on one successful `readWorkspaceState` per run (`canPersistWorkspaceState`). Never treat a failed read as empty state and never bypass the gate in `persistWorkspaceState` — a failed read saved back over the row is how users lose their tracked projects and monitored sessions. - Do not add `AgentHubDefaults` keys for selected repositories, monitored session IDs, session restore state, repo mappings, terminal workspace state, or terminal/dev-server process cleanup state. - `managed_processes` is the SQLite authority for app-spawned terminal/dev-server cleanup. Store only process identity/routing metadata needed for cleanup (PID, process group, process start time, kind/provider/session/project context), never prompts, full environment, terminal contents, or other sensitive runtime payloads. - Never edit, rename, reorder, or delete existing `DatabaseMigrator` migrations. Add a new `vN_*` migration for every schema change. diff --git a/app/modules/AgentHubCore/Sources/AgentHub/Services/ClaudeHookPaths.swift b/app/modules/AgentHubCore/Sources/AgentHub/Services/ClaudeHookPaths.swift index 04aabc09..f808febf 100644 --- a/app/modules/AgentHubCore/Sources/AgentHub/Services/ClaudeHookPaths.swift +++ b/app/modules/AgentHubCore/Sources/AgentHub/Services/ClaudeHookPaths.swift @@ -9,14 +9,7 @@ import Foundation public enum ClaudeHookPaths { public static var appSupportBaseURL: URL { - let fm = FileManager.default - let base = (try? fm.url( - for: .applicationSupportDirectory, - in: .userDomainMask, - appropriateFor: nil, - create: true - )) ?? URL(fileURLWithPath: NSString(string: "~/Library/Application Support").expandingTildeInPath) - return base.appendingPathComponent("AgentHub", isDirectory: true) + AgentHubApplicationSupport.baseDirectoryURL } public static var claimsDirectoryURL: URL { diff --git a/app/modules/AgentHubCore/Sources/AgentHub/Services/SessionMetadataStore.swift b/app/modules/AgentHubCore/Sources/AgentHub/Services/SessionMetadataStore.swift index b430776c..078db721 100644 --- a/app/modules/AgentHubCore/Sources/AgentHub/Services/SessionMetadataStore.swift +++ b/app/modules/AgentHubCore/Sources/AgentHub/Services/SessionMetadataStore.swift @@ -56,32 +56,36 @@ public actor SessionMetadataStore: TerminalWorkspaceStoreProtocol, AgentWorkspac // MARK: - Initialization - /// Creates a new metadata store at the default location - /// Database is stored in ~/Library/Application Support/AgentHub/session_metadata.sqlite + /// Creates a new metadata store at the default location: + /// `AgentHubApplicationSupport.baseDirectoryURL/session_metadata.sqlite` + /// (the real Application Support dir in the app; a temp sandbox under tests). public init() throws { - let appSupportURL = FileManager.default.urls( - for: .applicationSupportDirectory, - in: .userDomainMask - ).first! - - let agentHubDir = appSupportURL.appendingPathComponent("AgentHub", isDirectory: true) + let agentHubDir = AgentHubApplicationSupport.baseDirectoryURL try FileManager.default.createDirectory( at: agentHubDir, withIntermediateDirectories: true ) let dbPath = agentHubDir.appendingPathComponent("session_metadata.sqlite") - dbQueue = try DatabaseQueue(path: dbPath.path) + dbQueue = try DatabaseQueue(path: dbPath.path, configuration: Self.databaseConfiguration()) try migrator.migrate(dbQueue) } /// Creates a store with a custom database path (for testing) public init(path: String) throws { - dbQueue = try DatabaseQueue(path: path) + dbQueue = try DatabaseQueue(path: path, configuration: Self.databaseConfiguration()) try migrator.migrate(dbQueue) } + /// Waits out transient cross-process lock contention instead of surfacing + /// SQLITE_BUSY immediately — a busy read must not masquerade as empty state. + private static func databaseConfiguration() -> Configuration { + var configuration = Configuration() + configuration.busyMode = .timeout(5) + return configuration + } + // MARK: - Migrations private nonisolated var migrator: DatabaseMigrator { @@ -504,13 +508,23 @@ public actor SessionMetadataStore: TerminalWorkspaceStoreProtocol, AgentWorkspac // MARK: - Workspace State - public nonisolated func getWorkspaceStateSync(for provider: SessionProviderKind) -> SessionWorkspaceState { - (try? dbQueue.read { db in + /// Throwing read that distinguishes "no saved row" (returns an empty state) + /// from a failed read (throws). Callers deciding whether it is safe to + /// *write* workspace state must use this — treating a failed read as empty + /// and saving over it is how persisted repositories/sessions get lost. + public nonisolated func readWorkspaceState(for provider: SessionProviderKind) throws -> SessionWorkspaceState { + try dbQueue.read { db in try SessionWorkspaceStateRecord .filter(Column("provider") == provider.rawValue) .fetchOne(db)? .decodedState() - }) ?? SessionWorkspaceState() + } ?? SessionWorkspaceState() + } + + /// Display-only convenience: errors collapse to an empty state. Never use + /// this to decide whether saving workspace state is safe. + public nonisolated func getWorkspaceStateSync(for provider: SessionProviderKind) -> SessionWorkspaceState { + (try? readWorkspaceState(for: provider)) ?? SessionWorkspaceState() } public func saveWorkspaceState(_ state: SessionWorkspaceState, for provider: SessionProviderKind) async throws { diff --git a/app/modules/AgentHubCore/Sources/AgentHub/Utils/AgentHubApplicationSupport.swift b/app/modules/AgentHubCore/Sources/AgentHub/Utils/AgentHubApplicationSupport.swift new file mode 100644 index 00000000..43602f89 --- /dev/null +++ b/app/modules/AgentHubCore/Sources/AgentHub/Utils/AgentHubApplicationSupport.swift @@ -0,0 +1,49 @@ +import Foundation + +/// Resolves the base directory for AgentHub's persistent state +/// (`session_metadata.sqlite`, `approvals/`, `claims/`, `hooks/`). +/// +/// Normal app runs resolve to `~/Library/Application Support/AgentHub`. +/// Two overrides exist, in priority order: +/// +/// 1. `AGENTHUB_APP_SUPPORT_DIR` environment variable — explicit redirection +/// for E2E harnesses and the CLI. +/// 2. Test processes — any process with XCTest loaded (package test runners, +/// and the app itself when launched as a unit-test host) resolves to a +/// per-process temp sandbox. A test run must never be able to read or +/// write the user's real state: on 2026-08-13 a test suite sharing the +/// production sqlite caused the app to overwrite the user's workspace +/// state (tracked projects and monitored sessions) with a near-empty list. +public enum AgentHubApplicationSupport { + + /// Stable for the lifetime of the process. + public static let baseDirectoryURL: URL = resolveBaseDirectoryURL() + + static var isTestProcess: Bool { + NSClassFromString("XCTestCase") != nil + || ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil + || ProcessInfo.processInfo.environment["XCTestBundlePath"] != nil + || ProcessInfo.processInfo.environment["XCTestSessionIdentifier"] != nil + } + + private static func resolveBaseDirectoryURL() -> URL { + let url: URL + if let override = ProcessInfo.processInfo.environment["AGENTHUB_APP_SUPPORT_DIR"], + !override.isEmpty { + url = URL(fileURLWithPath: NSString(string: override).expandingTildeInPath, isDirectory: true) + } else if isTestProcess { + url = FileManager.default.temporaryDirectory + .appendingPathComponent("AgentHubTestSandbox-\(ProcessInfo.processInfo.processIdentifier)", isDirectory: true) + } else { + let appSupport = (try? FileManager.default.url( + for: .applicationSupportDirectory, + in: .userDomainMask, + appropriateFor: nil, + create: true + )) ?? URL(fileURLWithPath: NSString(string: "~/Library/Application Support").expandingTildeInPath) + url = appSupport.appendingPathComponent("AgentHub", isDirectory: true) + } + try? FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + return url + } +} diff --git a/app/modules/AgentHubCore/Sources/AgentHub/ViewModels/CLISessionsViewModel.swift b/app/modules/AgentHubCore/Sources/AgentHub/ViewModels/CLISessionsViewModel.swift index 180103a7..0a2b7be9 100644 --- a/app/modules/AgentHubCore/Sources/AgentHub/ViewModels/CLISessionsViewModel.swift +++ b/app/modules/AgentHubCore/Sources/AgentHub/ViewModels/CLISessionsViewModel.swift @@ -2450,9 +2450,41 @@ public final class CLISessionsViewModel { // MARK: - Persistence + /// Saves are disabled until the persisted workspace state has been read + /// successfully once this run. A failed read (locked or corrupt database) + /// must never be treated as "empty" and then written back over the user's + /// real tracked projects and monitored sessions. + @ObservationIgnored private var canPersistWorkspaceState = false + /// Test seam: lets unit tests simulate a failing workspace-state read. + @ObservationIgnored var workspaceStateReadOverride: ((SessionProviderKind) throws -> SessionWorkspaceState)? + + private func readPersistedWorkspaceState() throws -> SessionWorkspaceState { + if let workspaceStateReadOverride { + return try workspaceStateReadOverride(providerKind) + } + guard let metadataStore else { return SessionWorkspaceState() } + return try metadataStore.readWorkspaceState(for: providerKind) + } + private func restorePersistedRepositories() { Task { - let workspaceState = metadataStore?.getWorkspaceStateSync(for: providerKind) ?? SessionWorkspaceState() + var readState: SessionWorkspaceState? + for attempt in 1...3 { + do { + readState = try readPersistedWorkspaceState() + break + } catch { + AppLogger.session.error("Workspace state read failed (attempt \(attempt)/3): \(error.localizedDescription)") + if attempt < 3 { + try? await Task.sleep(for: .milliseconds(300)) + } + } + } + guard let workspaceState = readState else { + AppLogger.session.error("Workspace state unreadable; workspace-state saves stay disabled this run to protect the persisted data") + return + } + canPersistWorkspaceState = true let paths = workspaceState.selectedRepositoryPaths // Retain every saved session ID before any restore step can fail. Even if // repositories or worktrees are unavailable this launch, the IDs keep @@ -2695,6 +2727,10 @@ public final class CLISessionsViewModel { private func persistWorkspaceState() { guard let metadataStore else { return } + guard canPersistWorkspaceState else { + AppLogger.session.error("Skipped workspace-state save: persisted state has not been read successfully this run") + return + } let state = currentWorkspaceState() let providerKind = providerKind diff --git a/app/modules/AgentHubCore/Tests/AgentHubTests/WorkspaceStateSafetyTests.swift b/app/modules/AgentHubCore/Tests/AgentHubTests/WorkspaceStateSafetyTests.swift new file mode 100644 index 00000000..b55958cc --- /dev/null +++ b/app/modules/AgentHubCore/Tests/AgentHubTests/WorkspaceStateSafetyTests.swift @@ -0,0 +1,178 @@ +import Combine +import Foundation +import Testing + +@testable import AgentHubCore + +// Guards the two safety layers added after the 2026-08-13 incident where a +// test run sharing the production database caused the app to overwrite the +// user's workspace state (tracked projects + monitored sessions) with a +// near-empty list: +// +// 1. Test processes are sandboxed away from the real Application Support dir. +// 2. Workspace-state saves are disabled until the persisted state has been +// read successfully once — a failed read must never masquerade as "empty" +// and then get saved back over good data. + +@Suite("Application Support test sandbox") +struct ApplicationSupportSandboxTests { + + @Test("Test processes resolve to a temp sandbox, never the real Application Support") + func testProcessResolvesToSandbox() { + #expect(AgentHubApplicationSupport.isTestProcess) + let base = AgentHubApplicationSupport.baseDirectoryURL.path + #expect(base.contains("AgentHubTestSandbox-")) + #expect(!base.contains("/Library/Application Support/AgentHub")) + #expect(ClaudeHookPaths.appSupportBaseURL.path == base) + } + + @Test("Default-initialized metadata store lives in the sandbox") + func defaultStoreLivesInSandbox() throws { + _ = try SessionMetadataStore() + let sandboxDB = AgentHubApplicationSupport.baseDirectoryURL + .appendingPathComponent("session_metadata.sqlite") + #expect(FileManager.default.fileExists(atPath: sandboxDB.path)) + } +} + +@Suite("Workspace state save gate") +@MainActor +struct WorkspaceStateSaveGateTests { + + @Test("A failed read disables saves so persisted state survives the run") + func readFailureKeepsPersistedStateIntact() async throws { + let store = try await makeSeededStore( + repositoryPaths: ["/tmp/wsstate-a", "/tmp/wsstate-b"], + monitoredSessionIds: ["persisted-session"] + ) + let viewModel = makeSafetyFixtureViewModel(store: store) + viewModel.workspaceStateReadOverride = { _ in + throw WorkspaceStateSafetyTestError.simulatedReadFailure + } + + // Let the restore retries run out, then poke every save entry point a + // clobbering launch would hit. + try await Task.sleep(for: .milliseconds(1500)) + await viewModel.importMonitoredSessions([ + CLISession(id: "new-session", projectPath: "/tmp/wsstate-a", branchName: "main") + ]) + try await Task.sleep(for: .milliseconds(500)) + + let persisted = try store.readWorkspaceState(for: .claude) + #expect(persisted.selectedRepositoryPaths == ["/tmp/wsstate-a", "/tmp/wsstate-b"]) + #expect(persisted.monitoredSessionIds == ["persisted-session"]) + } + + @Test("A successful read enables saves and preserves restored state") + func successfulReadAllowsSaves() async throws { + let store = try await makeSeededStore( + repositoryPaths: ["/tmp/wsstate-a"], + monitoredSessionIds: [] + ) + let viewModel = makeSafetyFixtureViewModel(store: store) + + await viewModel.importMonitoredSessions([ + CLISession(id: "imported-session", projectPath: "/tmp/wsstate-a", branchName: "main") + ]) + + try await waitUntil { + (try? store.readWorkspaceState(for: .claude))?.monitoredSessionIds.contains("imported-session") == true + } + let persisted = try store.readWorkspaceState(for: .claude) + #expect(persisted.selectedRepositoryPaths.contains("/tmp/wsstate-a")) + } +} + +// MARK: - Fixture + +private enum WorkspaceStateSafetyTestError: Error { + case simulatedReadFailure +} + +private func makeSeededStore( + repositoryPaths: [String], + monitoredSessionIds: [String] +) async throws -> SessionMetadataStore { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("WorkspaceStateSafetyTests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + let store = try SessionMetadataStore(path: dir.appendingPathComponent("db.sqlite").path) + + let state = SessionWorkspaceState( + selectedRepositoryPaths: repositoryPaths, + monitoredSessionIds: monitoredSessionIds, + expansionState: [:] + ) + try await store.saveWorkspaceState(state, for: .claude) + return store +} + +@MainActor +private func makeSafetyFixtureViewModel(store: SessionMetadataStore) -> CLISessionsViewModel { + CLISessionsViewModel( + monitorService: WorkspaceSafetyMonitorService(), + fileWatcher: WorkspaceSafetyFileWatcher(), + searchService: nil, + cliConfiguration: .claudeDefault, + providerKind: .claude, + metadataStore: store, + approvalNotificationService: NoOpApprovalNotificationService() + ) +} + +@MainActor +private func waitUntil( + _ condition: @escaping () -> Bool, + timeoutAttempts: Int = 250 +) async throws { + for _ in 0..([]) + private var repositories: [SelectedRepository] = [] + + var repositoriesPublisher: AnyPublisher<[SelectedRepository], Never> { + subject.eraseToAnyPublisher() + } + + func addRepository(_ path: String) async -> SelectedRepository? { + guard !repositories.contains(where: { $0.path == path }) else { return nil } + let repository = SelectedRepository(path: path) + repositories.append(repository) + subject.send(repositories) + return repository + } + + func removeRepository(_ path: String) async { + repositories.removeAll { $0.path == path } + subject.send(repositories) + } + + func getSelectedRepositories() async -> [SelectedRepository] { repositories } + + func setSelectedRepositories(_ repositories: [SelectedRepository]) async { + self.repositories = repositories + subject.send(repositories) + } + + func refreshSessions(skipWorktreeRedetection: Bool) async {} +} + +private final class WorkspaceSafetyFileWatcher: SessionFileWatcherProtocol, @unchecked Sendable { + private let subject = PassthroughSubject() + + var statePublisher: AnyPublisher { + subject.eraseToAnyPublisher() + } + + func startMonitoring(sessionId: String, projectPath: String, sessionFilePath: String?) async {} + func stopMonitoring(sessionId: String) async {} + func getState(sessionId: String) async -> SessionMonitorState? { nil } + func refreshState(sessionId: String) async {} + func setApprovalTimeout(_ seconds: Int) async {} +}