diff --git a/docs-ai/064-agent-completion-signals/007-s3a-action.md b/docs-ai/064-agent-completion-signals/007-s3a-action.md index b151b338..d72db927 100644 --- a/docs-ai/064-agent-completion-signals/007-s3a-action.md +++ b/docs-ai/064-agent-completion-signals/007-s3a-action.md @@ -111,6 +111,35 @@ listener loss, notifier forwarding, and app composition are executable automated single-app visible-pane matrix remains a required manual/owner follow-up when a GUI session is available. +## Adversarial review + +### Round 1 — trust, launch transaction, epochs, and forwarding lifecycle + +The first independent full-diff review blocked on five valid P1 findings. All were reproduced +against the current code and corrected with focused regression coverage: + +- Codex preflight now resolves an absolute executable plus `HOME`/`CODEX_HOME` through the same + non-logging login-shell environment a Profile command uses. An unprovable/non-absolute result + degrades without injection; the app's launchd PATH/home can no longer authorize notifier + replacement. +- Peer/task cancellation is checked after every preparation await, before and after forwarding + record creation, and immediately before dispatch issuance. Capacity/failure/cancellation paths + explicitly discard unexposed records; the cancellation test deliberately returns a successful + preparation after observing cancellation and still proves no issue/launch. +- Managed-hook session identity is independent from detector hints. Detector `nil` cannot erase a + verified session, and detector-first same-process replacement remains unverified until Claude + sends the matching `SessionStart`. +- Deferred Ghostty arming now fails when `ghostty_surface_new` returns nil, resets its armed state, + and rolls back the exact tab/split registration instead of reporting a launch with no process. +- Forwarding cleanup initializes an orphan sweep at app startup and owns a clock-driven retry loop + until every retired record can take the exclusive lease; one busy first pass no longer leaves + sensitive argv indefinitely. + +The review also confirmed the peer-PID ancestry boundary, pre-input order, bounded early-event +buffer, exact cwd rejection (including memories), hidden bridge silence/deadline/`execvp`, payload +exclusion, carrier redaction, and owner/mode/no-follow/lease checks. Focused validation after the +fixes passed 89 tests plus `make check` (34 script tests). + ## Deferred scope S3b owns Copilot/Droid/Qoder adapters. S3c owns Pi/OMP/OpenCode adapters and the Active Agents exact diff --git a/supacode/App/supacodeApp.swift b/supacode/App/supacodeApp.swift index b999f1d2..db74c159 100644 --- a/supacode/App/supacodeApp.swift +++ b/supacode/App/supacodeApp.swift @@ -198,6 +198,7 @@ struct SupacodeApp: App { runtime: runtime, preferredFontSize: initialSettings.terminalFontSize ) + terminalManager.startAgentHookRuntimeMaintenance() _terminalManager = State(initialValue: terminalManager) let worktreeInfoWatcher = WorktreeInfoWatcherManager() _worktreeInfoWatcher = State(initialValue: worktreeInfoWatcher) @@ -1028,6 +1029,10 @@ struct SupacodeApp: App { terminalManager: terminalManager ) }, + cancelProfilePreparation: { request in + guard let preparation = request.preparedLaunch else { return } + terminalManager.discardPreparedAgentProfileLaunch(preparation) + }, issueDispatch: { do { let snapshot = try terminalManager.issueAgentDispatch() diff --git a/supacode/CLIService/LifecycleCommandHandler.swift b/supacode/CLIService/LifecycleCommandHandler.swift index 289e5fef..838bd144 100644 --- a/supacode/CLIService/LifecycleCommandHandler.swift +++ b/supacode/CLIService/LifecycleCommandHandler.swift @@ -96,6 +96,10 @@ enum CLIProfileLaunchFailure: Error, Equatable, Sendable { "The tab for Agent Profile “\(profile.name)” was created without a terminal surface." case .hookRegistrationFailed: "The managed signal channel for Agent Profile “\(profile.name)” could not be registered." + case .surfaceCreationFailed: + "The terminal surface for Agent Profile “\(profile.name)” could not be created." + case .preparationCancelled: + "Agent Profile “\(profile.name)” launch preparation was cancelled." } return .createFailed(message) } @@ -113,6 +117,7 @@ final class LifecycleCommandHandler: CommandHandler { @MainActor (CLIProfileLaunchRequest) async -> Result typealias ProfileLaunchProvider = @MainActor (CLIProfileLaunchRequest) -> Result + typealias CancelProfilePreparationProvider = @MainActor (CLIProfileLaunchRequest) -> Void typealias IssueDispatchProvider = @MainActor () -> Result typealias BindDispatchProvider = @@ -129,6 +134,7 @@ final class LifecycleCommandHandler: CommandHandler { private let profiles: ProfilesProvider private let prepareAgentProfile: PrepareProfileLaunchProvider private let launchAgentProfile: ProfileLaunchProvider + private let cancelProfilePreparation: CancelProfilePreparationProvider private let issueDispatch: IssueDispatchProvider private let bindDispatch: BindDispatchProvider private let cancelDispatch: CancelDispatchProvider @@ -146,6 +152,7 @@ final class LifecycleCommandHandler: CommandHandler { launchAgentProfile: @escaping ProfileLaunchProvider = { _ in .failure(.createFailed("Failed to launch the Agent Profile.")) }, + cancelProfilePreparation: @escaping CancelProfilePreparationProvider = { _ in }, issueDispatch: @escaping IssueDispatchProvider = { .failure(.capacityExceeded) }, bindDispatch: @escaping BindDispatchProvider = { _, _ in .failure(.notFound) }, cancelDispatch: @escaping CancelDispatchProvider = { _ in }, @@ -160,6 +167,7 @@ final class LifecycleCommandHandler: CommandHandler { self.profiles = profiles self.prepareAgentProfile = prepareAgentProfile self.launchAgentProfile = launchAgentProfile + self.cancelProfilePreparation = cancelProfilePreparation self.issueDispatch = issueDispatch self.bindDispatch = bindDispatch self.cancelDispatch = cancelDispatch @@ -330,8 +338,15 @@ final class LifecycleCommandHandler: CommandHandler { ) } + if Task.isCancelled { + cancelProfilePreparation(preparedRequest) + return cancelledPreparationResponse() + } let dispatchResult = issuedDispatch(prompt: launch.prompt) - if let response = dispatchResult.response { return response } + if let response = dispatchResult.response { + cancelProfilePreparation(preparedRequest) + return response + } let dispatch = dispatchResult.record let pairedRequest = CLIProfileLaunchRequest( resource: preparedRequest.resource, @@ -350,9 +365,11 @@ final class LifecycleCommandHandler: CommandHandler { createdTarget = target case .failure(.invalidArgument(let message)): if let dispatch { cancelDispatch(dispatch.id) } + cancelProfilePreparation(preparedRequest) return errorResponse(command: "create", code: CLIErrorCode.invalidArgument, message: message) case .failure(.createFailed(let message)): if let dispatch { cancelDispatch(dispatch.id) } + cancelProfilePreparation(preparedRequest) return errorResponse(command: "create", code: CLIErrorCode.createFailed, message: message) } if let dispatch { @@ -398,6 +415,14 @@ final class LifecycleCommandHandler: CommandHandler { } } + private func cancelledPreparationResponse() -> CommandResponse { + errorResponse( + command: "create", + code: CLIErrorCode.createFailed, + message: "Agent Profile launch preparation was cancelled." + ) + } + private func issuedDispatch( prompt: String? ) -> (record: DispatchPendingRecord?, response: CommandResponse?) { diff --git a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift index 98002280..2f507d20 100644 --- a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift +++ b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift @@ -259,6 +259,8 @@ nonisolated enum AgentProfileLaunchError: Error, Equatable, Sendable { case tabCreationFailed case launchedSurfaceMissing(TerminalTabID) case hookRegistrationFailed + case surfaceCreationFailed + case preparationCancelled } /// Which env variable names a profile override may set, shared by the planner diff --git a/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift b/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift index 756147af..0e585031 100644 --- a/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift +++ b/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift @@ -11,7 +11,7 @@ nonisolated struct AgentManagedHookPreparation: Equatable, Sendable { nonisolated enum AgentManagedHookPreparer { private struct CodexPreparationOptions { let promptIndex: Int? - let processEnvironment: [String: String] + let shellEnvironment: CodexShellLaunchEnvironment? let configReadProcess: CodexConfigReadProcess } @@ -23,7 +23,7 @@ nonisolated enum AgentManagedHookPreparer { plan: AgentProfileLaunchPlan, inheritedCWD: URL, resources: AgentHookResources?, - processEnvironment: [String: String], + codexShellEnvironment: CodexShellLaunchEnvironment? = nil, codexConfigReadProcess: CodexConfigReadProcess = CodexConfigReadProcess() ) async -> AgentManagedHookPreparation { guard @@ -68,7 +68,7 @@ nonisolated enum AgentManagedHookPreparer { resources: resources, options: CodexPreparationOptions( promptIndex: promptIndex, - processEnvironment: processEnvironment, + shellEnvironment: codexShellEnvironment, configReadProcess: codexConfigReadProcess ) ) @@ -117,13 +117,25 @@ nonisolated enum AgentManagedHookPreparer { resources: AgentHookResources, options: CodexPreparationOptions ) async -> AgentManagedHookPreparation { + guard let shellEnvironment = options.shellEnvironment else { + return degraded( + plan: plan, + capability: capability, + launchCWD: inheritedCWD, + message: "The effective Codex shell environment could not be resolved." + ) + } + let invocation = AgentInvocation( + executable: shellEnvironment.executableURL.path(percentEncoded: false), + arguments: plan.invocation.arguments + ) let context: CodexLaunchContext do { context = try CodexLaunchContext.capture( - invocation: plan.invocation, + invocation: invocation, inheritedCWD: inheritedCWD, dedicatedHome: plan.dedicatedHome, - environment: options.processEnvironment, + environment: shellEnvironment.processEnvironment, promptArgumentIndex: options.promptIndex ) } catch { @@ -136,12 +148,12 @@ nonisolated enum AgentManagedHookPreparer { } let resolver = CodexEffectiveNotifyResolver( bundledCLIPath: resources.bundledCLIPath, - query: options.configReadProcess.query + query: options.configReadProcess.usingExecutable(shellEnvironment.executableURL).query ) switch await resolver.resolve(context) { case .absent: return preparedCodex( - plan: plan, + invocation: invocation, capability: capability, context: context, resources: resources, @@ -152,7 +164,7 @@ nonisolated enum AgentManagedHookPreparer { ) case .present(let argv): return preparedCodex( - plan: plan, + invocation: invocation, capability: capability, context: context, resources: resources, @@ -172,7 +184,7 @@ nonisolated enum AgentManagedHookPreparer { } private static func preparedCodex( - plan: AgentProfileLaunchPlan, + invocation: AgentInvocation, capability: AgentSignalHookCapability, context: CodexLaunchContext, resources: AgentHookResources, @@ -180,7 +192,7 @@ nonisolated enum AgentManagedHookPreparer { ) -> AgentManagedHookPreparation { AgentManagedHookPreparation( preparedInvocation: CodexManagedNotifyRenderer.prepare( - invocation: plan.invocation, + invocation: invocation, bundledCLIPath: resources.bundledCLIPath, promptArgumentIndex: options.promptIndex ), diff --git a/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift b/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift index 65932487..153b6f25 100644 --- a/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift +++ b/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift @@ -48,6 +48,14 @@ nonisolated struct CodexConfigReadProcess: Sendable { self.timeout = max(0.05, timeout) } + func usingExecutable(_ executableURL: URL) -> CodexConfigReadProcess { + CodexConfigReadProcess( + executableURL: executableURL, + temporaryBaseDirectory: temporaryBaseDirectory, + timeout: timeout + ) + } + func query(_ query: CodexConfigQuery) async throws -> Data { let fileManager = FileManager.default var parserHome: URL? diff --git a/supacode/Domain/AgentRuntime/CodexEffectiveNotifyResolver.swift b/supacode/Domain/AgentRuntime/CodexEffectiveNotifyResolver.swift index f8a5ee95..258b765b 100644 --- a/supacode/Domain/AgentRuntime/CodexEffectiveNotifyResolver.swift +++ b/supacode/Domain/AgentRuntime/CodexEffectiveNotifyResolver.swift @@ -172,6 +172,9 @@ nonisolated struct CodexLaunchContext: Equatable, Sendable { codexHome = dedicatedHome } else if let configured = environment["CODEX_HOME"], !configured.isEmpty { codexHome = URL(filePath: configured, directoryHint: .isDirectory) + } else if let home = environment["HOME"], !home.isEmpty { + codexHome = URL(filePath: home, directoryHint: .isDirectory) + .appending(path: ".codex", directoryHint: .isDirectory) } else { codexHome = FileManager.default.homeDirectoryForCurrentUser .appending(path: ".codex", directoryHint: .isDirectory) diff --git a/supacode/Domain/AgentRuntime/CodexForwardingRecordStore.swift b/supacode/Domain/AgentRuntime/CodexForwardingRecordStore.swift index eccd77b7..cd73ac99 100644 --- a/supacode/Domain/AgentRuntime/CodexForwardingRecordStore.swift +++ b/supacode/Domain/AgentRuntime/CodexForwardingRecordStore.swift @@ -17,18 +17,22 @@ final class CodexForwardingRecordStore { private let retirementGrace: TimeInterval private let orphanMaximumAge: TimeInterval private let now: @MainActor () -> Date + private let retirementClock: any Clock private var retired: [URL: Date] = [:] + private var cleanupTask: Task? init( baseDirectory: URL, retirementGrace: TimeInterval = 2, orphanMaximumAge: TimeInterval = 24 * 60 * 60, - now: @escaping @MainActor () -> Date = Date.init + now: @escaping @MainActor () -> Date = Date.init, + retirementClock: any Clock = ContinuousClock() ) throws { self.baseDirectory = baseDirectory.standardizedFileURL self.retirementGrace = max(0, retirementGrace) self.orphanMaximumAge = max(0, orphanMaximumAge) self.now = now + self.retirementClock = retirementClock try Self.ensureOwnerOnlyDirectory(self.baseDirectory) sessionDirectory = self.baseDirectory.appending( path: "session-\(UUID().uuidString)", @@ -101,6 +105,7 @@ final class CodexForwardingRecordStore { func retire(_ record: CodexForwardingRecord) { retired[record.locator] = now().addingTimeInterval(retirementGrace) + scheduleCleanupIfNeeded() } func cleanupRetired() { @@ -112,6 +117,23 @@ final class CodexForwardingRecordStore { } } + private func scheduleCleanupIfNeeded() { + guard cleanupTask == nil else { return } + let clock = retirementClock + let milliseconds = max(100, Int(retirementGrace * 1_000)) + cleanupTask = Task { @MainActor [weak self] in + while let self, !retired.isEmpty { + do { + try await clock.sleep(for: .milliseconds(milliseconds)) + } catch { + break + } + cleanupRetired() + } + self?.cleanupTask = nil + } + } + func sweepOrphans() { guard let entries = try? FileManager.default.contentsOfDirectory( diff --git a/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift b/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift new file mode 100644 index 00000000..741b6ab3 --- /dev/null +++ b/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift @@ -0,0 +1,64 @@ +import Foundation + +nonisolated struct CodexShellLaunchEnvironment: Equatable, Sendable { + let executableURL: URL + let processEnvironment: [String: String] +} + +nonisolated enum CodexShellLaunchEnvironmentProbe { + private static let executableMarker = "__PROWL_CODEX_EXECUTABLE__" + private static let homeMarker = "__PROWL_CODEX_HOME_BASE__" + private static let codexHomeMarker = "__PROWL_CODEX_HOME__" + private static let script = """ + executable="$(command -v -- codex)" || exit 1 + printf '%s%s\n' '\(executableMarker)' "$executable" + printf '%s%s\n' '\(homeMarker)' "${HOME-}" + printf '%s%s\n' '\(codexHomeMarker)' "${CODEX_HOME-}" + """ + + static func resolve( + cwd: URL, + shell: ShellClient = .live, + isExecutable: (String) -> Bool = { FileManager.default.isExecutableFile(atPath: $0) } + ) async -> CodexShellLaunchEnvironment? { + guard + let output = try? await shell.runLogin( + URL(filePath: "/bin/sh", directoryHint: .notDirectory), + ["-c", script], + cwd, + log: false + ), + output.exitCode == 0, + output.stdout.utf8.count <= 16 * 1_024, + let values = parse(output.stdout), + let executable = values[executableMarker], executable.hasPrefix("/"), + isExecutable(executable), + let home = values[homeMarker], home.hasPrefix("/") + else { return nil } + + var environment = ["HOME": home] + if let codexHome = values[codexHomeMarker], !codexHome.isEmpty { + guard codexHome.hasPrefix("/") else { return nil } + environment["CODEX_HOME"] = codexHome + } + return CodexShellLaunchEnvironment( + executableURL: URL(filePath: executable, directoryHint: .notDirectory).standardizedFileURL, + processEnvironment: environment + ) + } + + private static func parse(_ output: String) -> [String: String]? { + let lines = output.split(separator: "\n", omittingEmptySubsequences: false) + guard lines.count == 4, lines.last?.isEmpty == true else { return nil } + var values: [String: String] = [:] + for line in lines.dropLast() { + let value = String(line) + guard + let marker = [executableMarker, homeMarker, codexHomeMarker].first(where: value.hasPrefix), + values[marker] == nil + else { return nil } + values[marker] = String(value.dropFirst(marker.count)) + } + return values.count == 3 ? values : nil + } +} diff --git a/supacode/Features/Terminal/BusinessLogic/AgentObservationStore.swift b/supacode/Features/Terminal/BusinessLogic/AgentObservationStore.swift index 11df5559..24a68eef 100644 --- a/supacode/Features/Terminal/BusinessLogic/AgentObservationStore.swift +++ b/supacode/Features/Terminal/BusinessLogic/AgentObservationStore.swift @@ -17,6 +17,7 @@ private struct ManagedHookRegistrationRecord { let launch: AgentHookLaunchRegistration var evidenceEpoch: UUID var processGeneration: AgentProcessGeneration? + var sessionID: String? var verified = false var pendingSignals: [PendingManagedHookSignal] = [] } @@ -291,7 +292,7 @@ final class AgentObservationStore { } if input.runtime == .claude, input.signal.event == .sessionStart, - let currentSession = record.sessionID, + let currentSession = managed.sessionID, currentSession != input.signal.sessionID { record.evidenceEpoch = UUID() @@ -301,12 +302,13 @@ final class AgentObservationStore { if record.latestSignal != nil { record.latestSignalBinding = .stale } managed.evidenceEpoch = record.evidenceEpoch managed.verified = false - } else if let currentSession = record.sessionID, + } else if let currentSession = managed.sessionID, currentSession != input.signal.sessionID { return .rejected } record.sessionID = input.signal.sessionID + managed.sessionID = input.signal.sessionID managed.verified = true let runtime = AgentProfileRuntime(rawValue: input.runtime.rawValue) ?? .claude let signal = AgentSignal( @@ -412,7 +414,7 @@ final class AgentObservationStore { managed.pendingSignals.removeAll() record.managedHook = managed record.processGeneration = processGeneration - record.sessionID = sessionID + if let sessionID { record.sessionID = sessionID } records[surfaceID] = record for pendingSignal in pending { if case .accepted(let signal, _) = recordManagedHook( @@ -426,7 +428,7 @@ final class AgentObservationStore { return update } record.processGeneration = processGeneration - record.sessionID = sessionID + if let sessionID { record.sessionID = sessionID } records[surfaceID] = record return update } diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 65488030..f26fd3da 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -11,10 +11,12 @@ private let layoutRestoreFailureMessage = "Saved terminal layout was invalid and final class WorktreeTerminalManager { private let runtime: GhosttyRuntime? private let layoutPersistence: TerminalLayoutPersistenceClient + private let skipsSurfaceCreationForTesting: Bool private let targetHandleRegistry = TerminalTargetHandleRegistry() @ObservationIgnored private let agentObservationStore: AgentObservationStore @ObservationIgnored private let agentDispatchStore: AgentDispatchStore @ObservationIgnored private let codexConfigReadProcess: CodexConfigReadProcess + @ObservationIgnored private let codexShellEnvironmentResolver: @Sendable (URL) async -> CodexShellLaunchEnvironment? @ObservationIgnored private let hookResourcesProvider: @MainActor () -> AgentHookResources? @ObservationIgnored private let forwardingRecordBaseDirectory: URL @ObservationIgnored private var codexForwardingRecordStore: CodexForwardingRecordStore? @@ -44,6 +46,9 @@ final class WorktreeTerminalManager { agentObservationBufferCapacity: Int = 64, agentDispatchStore: AgentDispatchStore = AgentDispatchStore(), codexConfigReadProcess: CodexConfigReadProcess = CodexConfigReadProcess(), + codexShellEnvironmentResolver: @escaping @Sendable (URL) async -> CodexShellLaunchEnvironment? = { + await CodexShellLaunchEnvironmentProbe.resolve(cwd: $0) + }, hookResourcesProvider: @escaping @MainActor () -> AgentHookResources? = { guard let url = SupacodePaths.bundledCLIURL else { return nil } return AgentHookResources( @@ -51,14 +56,17 @@ final class WorktreeTerminalManager { socketPath: ProwlSocket.defaultPath ) }, - forwardingRecordBaseDirectory: URL = SupacodePaths.agentHookForwardingDirectory + forwardingRecordBaseDirectory: URL = SupacodePaths.agentHookForwardingDirectory, + skipsSurfaceCreationForTesting: Bool = false ) { self.runtime = runtime self.layoutPersistence = layoutPersistence + self.skipsSurfaceCreationForTesting = skipsSurfaceCreationForTesting self.preferredFontSize = preferredFontSize self.agentObservationStore = AgentObservationStore(bufferCapacity: agentObservationBufferCapacity) self.agentDispatchStore = agentDispatchStore self.codexConfigReadProcess = codexConfigReadProcess + self.codexShellEnvironmentResolver = codexShellEnvironmentResolver self.hookResourcesProvider = hookResourcesProvider self.forwardingRecordBaseDirectory = forwardingRecordBaseDirectory baselineFontSize = runtime.defaultFontSize() @@ -108,13 +116,21 @@ final class WorktreeTerminalManager { } latestContext = context let resources = hookResourcesProvider() + let codexShellEnvironment: CodexShellLaunchEnvironment? + if context.request.plan.runtime == .codex, resources != nil { + codexShellEnvironment = await codexShellEnvironmentResolver(context.inheritedCWD) + } else { + codexShellEnvironment = nil + } + guard !Task.isCancelled else { return .failure(.preparationCancelled) } let preparation = await AgentManagedHookPreparer.prepare( plan: context.request.plan, inheritedCWD: context.inheritedCWD, resources: resources, - processEnvironment: ProcessInfo.processInfo.environment, + codexShellEnvironment: codexShellEnvironment, codexConfigReadProcess: codexConfigReadProcess ) + guard !Task.isCancelled else { return .failure(.preparationCancelled) } guard terminalState.isAgentProfileLaunchContextValid(context) else { if attempt == 0 { continue } let warning = LifecycleCommandWarning( @@ -150,6 +166,10 @@ final class WorktreeTerminalManager { } forwardingRecord = record } + if Task.isCancelled { + if let forwardingRecord { codexForwardingRecordStore?.discardUnexposed(forwardingRecord) } + return .failure(.preparationCancelled) + } let executionPlan = context.request.plan.applyingManagedHook( preparedInvocation, resources: resources, @@ -181,6 +201,11 @@ final class WorktreeTerminalManager { return .success(PreparedAgentProfileLaunch(context: latestContext, warnings: [])) } + func discardPreparedAgentProfileLaunch(_ preparation: PreparedAgentProfileLaunch) { + guard let record = preparation.context.request.plan.hookRegistration?.forwardingRecord else { return } + codexForwardingRecordStore?.discardUnexposed(record) + } + func launchPreparedAgentProfile( _ preparation: PreparedAgentProfileLaunch, in worktree: Worktree @@ -194,6 +219,10 @@ final class WorktreeTerminalManager { return result } + func startAgentHookRuntimeMaintenance() { + _ = forwardingRecordStore() + } + private func forwardingRecordStore() -> CodexForwardingRecordStore? { if let codexForwardingRecordStore { return codexForwardingRecordStore } guard @@ -217,6 +246,8 @@ final class WorktreeTerminalManager { : .tab(background: false) ) switch await prepareAgentProfileLaunch(request, in: worktree) { + case .failure where Task.isCancelled: + return case .failure: emit(.agentProfileLaunchFailed(worktreeID: worktree.id, profileName: plan.profileName)) case .success(let preparation): @@ -467,12 +498,7 @@ final class WorktreeTerminalManager { } private func retireForwardingRecord(_ record: CodexForwardingRecord) { - guard let store = codexForwardingRecordStore else { return } - store.retire(record) - Task { @MainActor [weak self] in - try? await ContinuousClock().sleep(for: .seconds(3)) - self?.codexForwardingRecordStore?.cleanupRetired() - } + codexForwardingRecordStore?.retire(record) } private func noteDispatchEvidence( @@ -597,14 +623,21 @@ final class WorktreeTerminalManager { guard snapshot.record.state == .pending else { throw AgentDispatchStoreError.alreadyTerminal } + let evidenceEpoch: UUID + if agentObservationStore.hasManagedHook(surfaceID: surfaceID) { + guard let current = agentObservationStore.currentEvidenceEpoch(surfaceID: surfaceID) else { + throw AgentDispatchStoreError.bindingMissing + } + evidenceEpoch = current + } else { + evidenceEpoch = agentObservationStore.beginDispatchEpoch(surfaceID: surfaceID) + } try agentDispatchStore.bind( dispatchID: dispatchID, binding: AgentDispatchBinding( surfaceID: surfaceID, target: target, - evidenceEpoch: agentObservationStore.hasManagedHook(surfaceID: surfaceID) - ? (agentObservationStore.currentEvidenceEpoch(surfaceID: surfaceID) ?? UUID()) - : agentObservationStore.beginDispatchEpoch(surfaceID: surfaceID) + evidenceEpoch: evidenceEpoch ) ) } @@ -685,7 +718,8 @@ final class WorktreeTerminalManager { worktree: worktree, runSetupScript: runSetupScript, defaultFontSize: preferredFontSize, - targetHandleRegistry: targetHandleRegistry + targetHandleRegistry: targetHandleRegistry, + skipsSurfaceCreationForTesting: skipsSurfaceCreationForTesting ) state.setNotificationsEnabled(notificationsEnabled) state.setCommandFinishedNotification( @@ -1207,10 +1241,12 @@ final class WorktreeTerminalManager { private init(preview: Void) { self.runtime = nil self.layoutPersistence = .liveValue + self.skipsSurfaceCreationForTesting = true self.preferredFontSize = nil self.agentObservationStore = AgentObservationStore(bufferCapacity: 64) self.agentDispatchStore = AgentDispatchStore() self.codexConfigReadProcess = CodexConfigReadProcess() + self.codexShellEnvironmentResolver = { _ in nil } self.hookResourcesProvider = { nil } self.forwardingRecordBaseDirectory = SupacodePaths.agentHookForwardingDirectory self.baselineFontSize = 13 diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState+Surfaces.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState+Surfaces.swift index e06cc082..4eff8472 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState+Surfaces.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState+Surfaces.swift @@ -373,6 +373,7 @@ extension WorktreeTerminalState { fontSize: resolvedFontSize, context: context, environment: worktree.scriptEnvironment.merging(additionalEnvironment) { _, patched in patched }, + skipsSurfaceCreationForTesting: skipsSurfaceCreationForTesting, defersSurfaceCreation: defersSurfaceCreation ) // Sending a no-op font size action marks the Ghostty surface as diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 8a1d67bd..43eedbdf 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -95,6 +95,7 @@ final class WorktreeTerminalState { let runtime: GhosttyRuntime let worktree: Worktree private let targetHandleRegistry: TerminalTargetHandleRegistry + let skipsSurfaceCreationForTesting: Bool @ObservationIgnored @SharedReader private var repositorySettings: RepositorySettings var trees: [TerminalTabID: SplitTree] = [:] @@ -285,11 +286,13 @@ final class WorktreeTerminalState { runSetupScript: Bool = false, defaultFontSize: Float32? = nil, targetHandleRegistry: TerminalTargetHandleRegistry? = nil, - titleFlushClock: any Clock = ContinuousClock() + titleFlushClock: any Clock = ContinuousClock(), + skipsSurfaceCreationForTesting: Bool = false ) { self.runtime = runtime self.worktree = worktree self.targetHandleRegistry = targetHandleRegistry ?? TerminalTargetHandleRegistry() + self.skipsSurfaceCreationForTesting = skipsSurfaceCreationForTesting self.pendingSetupScript = runSetupScript self.defaultFontSize = defaultFontSize self.tabManager = TerminalTabManager(titleFlushClock: titleFlushClock) @@ -628,13 +631,14 @@ final class WorktreeTerminalState { { applyResolvedIcon(icon, surfaceId: surface.surfaceID, tabId: surface.tabID) } - guard onAgentProfileSurfacePrepared?(surface.surfaceID, plan) != false, - let view = surfaces[surface.surfaceID], - view.armSurfaceCreation() - else { + guard onAgentProfileSurfacePrepared?(surface.surfaceID, plan) != false else { rollbackAgentProfileSurface(surface, placement: request.placement) return .failure(.hookRegistrationFailed) } + guard let view = surfaces[surface.surfaceID], view.armSurfaceCreation() else { + rollbackAgentProfileSurface(surface, placement: request.placement) + return .failure(.surfaceCreationFailed) + } wakeAgentDetection(for: view, tabId: surface.tabID) return launched } diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift index 90b6eb96..32834130 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift @@ -370,11 +370,11 @@ final class GhosttySurfaceView: NSView, Identifiable { surfaceCreationArmed = true guard !skipsSurfaceCreationForTesting else { return true } createSurface() - if let surface { - surfaceRef = runtime.registerSurface(surface) + guard let surface else { + surfaceCreationArmed = false + return false } - // Surface construction has historically been best-effort at this layer; - // lifecycle failure is reported by the surrounding state boundary. + surfaceRef = runtime.registerSurface(surface) return true } diff --git a/supacodeTests/CLILifecycleCommandHandlerTests.swift b/supacodeTests/CLILifecycleCommandHandlerTests.swift index 0332139b..91b166cb 100644 --- a/supacodeTests/CLILifecycleCommandHandlerTests.swift +++ b/supacodeTests/CLILifecycleCommandHandlerTests.swift @@ -234,6 +234,7 @@ struct CLILifecycleCommandHandlerTests { let clock = TestClock() var issued = false var launched = false + var cancelledPreparation = false let handler = LifecycleCommandHandler( resolveCreateTarget: { _ in .success(base) }, resolveCloseTarget: { _ in .success(.init(resource: .pane, target: base)) }, @@ -245,13 +246,16 @@ struct CLILifecycleCommandHandlerTests { try await clock.sleep(for: .seconds(10)) return .success(request) } catch { - return .failure(.createFailed("Cancelled.")) + // Model a production resolver that catches cancellation and returns + // an ordinary degraded/successful preparation. + return .success(request) } }, launchAgentProfile: { _ in launched = true return .success(base) }, + cancelProfilePreparation: { _ in cancelledPreparation = true }, issueDispatch: { issued = true return .failure(.capacityExceeded) @@ -280,6 +284,7 @@ struct CLILifecycleCommandHandlerTests { #expect(!issued) #expect(!launched) + #expect(cancelledPreparation) } @Test func promptedLaunchFailureCancelsIssuedDispatch() async { @@ -325,6 +330,7 @@ struct CLILifecycleCommandHandlerTests { let base = makeTarget() let profile = AgentProfile(name: "Reviewer", runtime: .claude) var didLaunch = false + var cancelledPreparation = false let handler = LifecycleCommandHandler( resolveCreateTarget: { _ in .success(base) }, resolveCloseTarget: { _ in .success(LifecycleResolvedTarget(resource: .pane, target: base)) }, @@ -335,6 +341,7 @@ struct CLILifecycleCommandHandlerTests { didLaunch = true return .success(base) }, + cancelProfilePreparation: { _ in cancelledPreparation = true }, issueDispatch: { .failure(.capacityExceeded) }, closeTab: { _, _ in true }, closePane: { _, _ in true } @@ -356,6 +363,7 @@ struct CLILifecycleCommandHandlerTests { #expect(!response.ok) #expect(response.error?.code == CLIErrorCode.dispatchCapacityExceeded) #expect(!didLaunch) + #expect(cancelledPreparation) } @Test func unpromptedProfileLaunchDoesNotIssueOrBindDispatch() async throws { diff --git a/supacodeTests/CodexForwardingRecordStoreTests.swift b/supacodeTests/CodexForwardingRecordStoreTests.swift index b35ce325..2375db23 100644 --- a/supacodeTests/CodexForwardingRecordStoreTests.swift +++ b/supacodeTests/CodexForwardingRecordStoreTests.swift @@ -1,3 +1,4 @@ +import Clocks import Darwin import Foundation import Testing @@ -48,6 +49,32 @@ struct CodexForwardingRecordStoreTests { #expect(!FileManager.default.fileExists(atPath: record.locator.path(percentEncoded: false))) } + @Test func scheduledCleanupRetriesAfterTheFirstLeaseConflict() async throws { + let base = temporaryDirectory("forward-scheduled-retire") + defer { try? FileManager.default.removeItem(at: base) } + var now = Date(timeIntervalSince1970: 100) + let clock = TestClock() + let store = try CodexForwardingRecordStore( + baseDirectory: base, + retirementGrace: 1, + now: { now }, + retirementClock: clock + ) + let record = try store.create(argv: ["/tmp/notifier"]) + let lease = try CodexForwardingRecordReader.open(record.locator) + store.retire(record) + await Task.yield() + + now.addTimeInterval(2) + await clock.advance(by: .seconds(1)) + #expect(FileManager.default.fileExists(atPath: record.locator.path(percentEncoded: false))) + + lease.close() + now.addTimeInterval(2) + await clock.advance(by: .seconds(1)) + #expect(!FileManager.default.fileExists(atPath: record.locator.path(percentEncoded: false))) + } + @Test func readerRejectsSymlinkPermissionDriftAndOversizedRecords() throws { let base = temporaryDirectory("forward-invalid") defer { try? FileManager.default.removeItem(at: base) } diff --git a/supacodeTests/CodexShellLaunchEnvironmentTests.swift b/supacodeTests/CodexShellLaunchEnvironmentTests.swift new file mode 100644 index 00000000..2133b820 --- /dev/null +++ b/supacodeTests/CodexShellLaunchEnvironmentTests.swift @@ -0,0 +1,96 @@ +import Foundation +import Testing + +@testable import supacode + +struct CodexShellLaunchEnvironmentTests { + @Test func probeUsesLoginShellAndReturnsOnlyValidatedLaunchFacts() async throws { + let cwd = URL(filePath: "/tmp/Project Space/界", directoryHint: .isDirectory) + let shell = ShellClient( + run: { _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) }, + runLoginImpl: { executable, arguments, currentDirectory, log in + #expect(executable.path(percentEncoded: false) == "/bin/sh") + #expect(arguments.first == "-c") + #expect(currentDirectory == cwd) + #expect(!log) + return ShellOutput( + stdout: """ + __PROWL_CODEX_EXECUTABLE__/opt/custom/bin/codex + __PROWL_CODEX_HOME_BASE__/Users/tester + __PROWL_CODEX_HOME__/tmp/codex-home + + """, + stderr: "ignored", + exitCode: 0 + ) + } + ) + + let environment = try #require( + await CodexShellLaunchEnvironmentProbe.resolve( + cwd: cwd, + shell: shell, + isExecutable: { $0 == "/opt/custom/bin/codex" } + ) + ) + + #expect(environment.executableURL.path(percentEncoded: false) == "/opt/custom/bin/codex") + #expect( + environment.processEnvironment == [ + "HOME": "/Users/tester", + "CODEX_HOME": "/tmp/codex-home", + ]) + } + + @Test func malformedNonAbsoluteAndFailedProbeDegrade() async { + for output in [ + ShellOutput(stdout: "not-json", stderr: "", exitCode: 0), + ShellOutput( + stdout: """ + __PROWL_CODEX_EXECUTABLE__codex + __PROWL_CODEX_HOME_BASE__/Users/tester + __PROWL_CODEX_HOME__ + + """, + stderr: "", + exitCode: 0 + ), + ShellOutput( + stdout: """ + __PROWL_CODEX_EXECUTABLE__/opt/codex + __PROWL_CODEX_HOME_BASE__/Users/tester + __PROWL_CODEX_HOME__ + + """, + stderr: "", + exitCode: 1 + ), + ] { + let shell = ShellClient( + run: { _, _, _ in output }, + runLoginImpl: { _, _, _, _ in output } + ) + #expect( + await CodexShellLaunchEnvironmentProbe.resolve( + cwd: URL(filePath: "/tmp", directoryHint: .isDirectory), + shell: shell, + isExecutable: { $0 == "/opt/codex" } + ) == nil + ) + } + } + + @Test func capturedShellHomeDefinesDefaultCodexHome() throws { + let shell = CodexShellLaunchEnvironment( + executableURL: URL(filePath: "/opt/codex"), + processEnvironment: ["HOME": "/Users/shell-home"] + ) + let context = try CodexLaunchContext.capture( + invocation: AgentInvocation(executable: "/opt/codex", arguments: []), + inheritedCWD: URL(filePath: "/tmp/project", directoryHint: .isDirectory), + environment: shell.processEnvironment + ) + + #expect(context.codexHome.path(percentEncoded: false) == "/Users/shell-home/.codex/") + } +} diff --git a/supacodeTests/ManagedAgentHookObservationTests.swift b/supacodeTests/ManagedAgentHookObservationTests.swift index 60ba4db2..ca81c4bd 100644 --- a/supacodeTests/ManagedAgentHookObservationTests.swift +++ b/supacodeTests/ManagedAgentHookObservationTests.swift @@ -86,6 +86,103 @@ struct ManagedAgentHookObservationTests { ) } + @Test func detectorNilCannotEraseVerifiedClaudeSessionOrAdmitDifferentStopSession() { + let now = Date(timeIntervalSince1970: 100) + let generation = AgentProcessGeneration(pid: 900, startedAt: now) + let store = AgentObservationStore(bufferCapacity: 8, now: { now }) + let surfaceID = UUID() + let registration = makeRegistration(runtime: .claude, cwd: "/tmp/project") + _ = store.registerManagedHook(registration, surfaceID: surfaceID) + _ = store.updateEvidenceEpoch( + surfaceID: surfaceID, + processGeneration: generation, + sessionID: nil + ) + let start = makeInput( + runtime: .claude, + token: registration.token, + nativeEvent: "SessionStart", + event: .sessionStart, + cwd: "/tmp/project", + sessionID: "session-1" + ) + #expect( + store.recordManagedHook(start, callerAncestry: [generation], surfaceID: surfaceID).isAccepted + ) + + _ = store.updateEvidenceEpoch( + surfaceID: surfaceID, + processGeneration: generation, + sessionID: nil + ) + let wrongStop = makeInput( + runtime: .claude, + token: registration.token, + nativeEvent: "Stop", + event: .turnEnded, + cwd: "/tmp/project", + sessionID: "session-2" + ) + + #expect( + store.recordManagedHook(wrongStop, callerAncestry: [generation], surfaceID: surfaceID) + == .rejected + ) + } + + @Test func detectorSessionReplacementRequiresClaudeSessionStartBeforeReverification() throws { + let now = Date(timeIntervalSince1970: 100) + let generation = AgentProcessGeneration(pid: 900, startedAt: now) + let store = AgentObservationStore(bufferCapacity: 8, now: { now }) + let surfaceID = UUID() + let registration = makeRegistration(runtime: .claude, cwd: "/tmp/project") + _ = store.registerManagedHook(registration, surfaceID: surfaceID) + _ = store.updateEvidenceEpoch(surfaceID: surfaceID, processGeneration: generation, sessionID: nil) + let first = makeInput( + runtime: .claude, + token: registration.token, + nativeEvent: "SessionStart", + event: .sessionStart, + cwd: "/tmp/project", + sessionID: "session-1" + ) + #expect(store.recordManagedHook(first, callerAncestry: [generation], surfaceID: surfaceID).isAccepted) + + _ = store.updateEvidenceEpoch( + surfaceID: surfaceID, + processGeneration: generation, + sessionID: "session-2" + ) + let stop = makeInput( + runtime: .claude, + token: registration.token, + nativeEvent: "Stop", + event: .turnEnded, + cwd: "/tmp/project", + sessionID: "session-2" + ) + #expect(store.recordManagedHook(stop, callerAncestry: [generation], surfaceID: surfaceID) == .rejected) + #expect( + store.signalsPayload(surfaceID: surfaceID, formatter: formatter, includeDiagnosticLast: true) + .channels.isEmpty + ) + + let second = makeInput( + runtime: .claude, + token: registration.token, + nativeEvent: "SessionStart", + event: .sessionStart, + cwd: "/tmp/project", + sessionID: "session-2" + ) + #expect(store.recordManagedHook(second, callerAncestry: [generation], surfaceID: surfaceID).isAccepted) + let channel = try #require( + store.signalsPayload(surfaceID: surfaceID, formatter: formatter, includeDiagnosticLast: true) + .channels.first + ) + #expect(channel.sessionID == "session-2") + } + @Test func processReplacementRevokesTrustAndReturnsForwardRecordForRetirement() { let now = Date(timeIntervalSince1970: 100) let store = AgentObservationStore(bufferCapacity: 8, now: { now }) diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 26913860..4d60151e 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -134,7 +134,10 @@ struct WorktreeTerminalManagerTests { } @Test func backgroundProfileSplitInHiddenWorktreePreservesVisibleSelection() throws { - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) + let manager = WorktreeTerminalManager( + runtime: GhosttyRuntime(), + skipsSurfaceCreationForTesting: true + ) let visibleWorktree = makeWorktree(id: "/tmp/repo/visible", name: "visible") let hiddenWorktree = makeWorktree(id: "/tmp/repo/hidden", name: "hidden") let visibleState = manager.state(for: visibleWorktree) @@ -178,10 +181,33 @@ struct WorktreeTerminalManagerTests { #expect(launched.tabID == hiddenTab) } + @Test func startupHookMaintenanceSweepsAgedCrashForwardingRecords() throws { + let base = FileManager.default.temporaryDirectory.appending( + path: "prowl-tests-forward-startup-\(UUID().uuidString)", + directoryHint: .isDirectory + ) + defer { try? FileManager.default.removeItem(at: base) } + let oldStore = try CodexForwardingRecordStore( + baseDirectory: base, + orphanMaximumAge: 60, + now: { Date(timeIntervalSince1970: 100) } + ) + let oldRecord = try oldStore.create(argv: ["/tmp/notifier"]) + let manager = WorktreeTerminalManager( + runtime: GhosttyRuntime(), + forwardingRecordBaseDirectory: base + ) + + manager.startAgentHookRuntimeMaintenance() + + #expect(!FileManager.default.fileExists(atPath: oldRecord.locator.path(percentEncoded: false))) + } + @Test func unavailableHookResourcesWarnOnceAndLaunchTheOriginalInvocation() async throws { let manager = WorktreeTerminalManager( runtime: GhosttyRuntime(), - hookResourcesProvider: { nil } + hookResourcesProvider: { nil }, + skipsSurfaceCreationForTesting: true ) let worktree = makeWorktree() let original = AgentProfileLaunchPlan( @@ -209,7 +235,10 @@ struct WorktreeTerminalManagerTests { } @Test func promptedManagedHookLaunchBindsDispatchToTheRegistrationEpoch() throws { - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) + let manager = WorktreeTerminalManager( + runtime: GhosttyRuntime(), + skipsSurfaceCreationForTesting: true + ) let worktree = makeWorktree() let state = manager.state(for: worktree) let base = AgentProfileLaunchPlan( diff --git a/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift b/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift index 7f5723b8..f1e1aa1e 100644 --- a/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift +++ b/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift @@ -177,6 +177,30 @@ struct WorktreeTerminalStateAgentProfileTests { #expect(state.surfaceView(for: launched.surfaceID)?.surfaceCreationArmed == true) } + @Test func deferredGhosttyCreationFailureRollsBackRegistrationAndSurface() { + let state = makeState(skipsSurfaceCreationForTesting: false) + var registeredSurface: UUID? + var closedSurface: UUID? + state.onAgentProfileSurfacePrepared = { surfaceID, _ in + registeredSurface = surfaceID + return true + } + state.onSurfaceClosed = { closedSurface = $0 } + + let result = state.launchAgentProfile( + AgentProfileLaunchRequest( + plan: makePlan(dedicatedHome: nil), + placement: .tab(background: false) + ) + ) + + #expect(result == .failure(.surfaceCreationFailed)) + #expect(closedSurface == registeredSurface) + #expect(state.tabManager.tabs.isEmpty) + #expect(state.surfaces.isEmpty) + #expect(state.launchProfilesBySurface.isEmpty) + } + @Test func registrationFailureRollsBackBeforeLeavingALiveSurface() { let state = makeState() state.onAgentProfileSurfacePrepared = { _, _ in false } @@ -311,7 +335,9 @@ struct WorktreeTerminalStateAgentProfileTests { #expect(state.tabManager.tabs.first?.iconLock == .script) } - private func makeState() -> WorktreeTerminalState { + private func makeState( + skipsSurfaceCreationForTesting: Bool = true + ) -> WorktreeTerminalState { WorktreeTerminalState( runtime: GhosttyRuntime(), worktree: Worktree( @@ -320,7 +346,8 @@ struct WorktreeTerminalStateAgentProfileTests { detail: "", workingDirectory: URL(fileURLWithPath: "/tmp/repo/wt-1"), repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") - ) + ), + skipsSurfaceCreationForTesting: skipsSurfaceCreationForTesting ) }