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 d72db927..b5f59aef 100644 --- a/docs-ai/064-agent-completion-signals/007-s3a-action.md +++ b/docs-ai/064-agent-completion-signals/007-s3a-action.md @@ -140,6 +140,26 @@ buffer, exact cwd rejection (including memories), hidden bridge silence/deadline exclusion, carrier redaction, and owner/mode/no-follow/lease checks. Focused validation after the fixes passed 89 tests plus `make check` (34 script tests). +### Round 2 — races, cancellation, argv rendering, and runtime compatibility + +The second independent review accepted four additional P1 findings: + +- Login-shell environment resolution had no hard deadline or streaming output cap. It now uses a + purpose-built process runner with a one-second deadline, combined stdout/stderr bound, + cancellation, TERM-to-KILL escalation, and tests for a noisy shell plus a TERM-ignoring hang. +- Frozen menu/palette target validation checked only anchor existence. It now tracks whether focus + and cwd were inherited dynamically, re-reads both immediately after preflight, and retries or + degrades rather than launching a stale context. +- Renderer fallback inferred any final argv after `exec`/`-p` as a prompt. Only the planner-owned + prompt index can move insertion before a prompt now; arbitrary option/value argv remains exact. +- Stable settings/profile reads compared only the open descriptor. Claude and Codex now share one + bounded owner-file reader that also `lstat`s the source path and compares device/inode/type, + size, and mtime, so atomic replacement degrades safely. + +A follow-up hardening discovered while verifying the first finding also carries an explicit Profile +`PATH` override into login-shell executable resolution; the prepared runtime invocation then uses +the attested absolute executable. Focused round-2 validation passed 77 tests plus `make check`. + ## 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 db74c159..0ac025cf 100644 --- a/supacode/App/supacodeApp.swift +++ b/supacode/App/supacodeApp.swift @@ -1516,7 +1516,9 @@ struct SupacodeApp: App { title: preparation.context.request.title ), inheritedCWD: preparation.context.inheritedCWD, - anchorSurfaceID: preparation.context.anchorSurfaceID + anchorSurfaceID: preparation.context.anchorSurfaceID, + tracksFocusedAnchor: preparation.context.tracksFocusedAnchor, + tracksInheritedCWD: preparation.context.tracksInheritedCWD ), warnings: preparation.warnings ) diff --git a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift index 2f507d20..5d06602e 100644 --- a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift +++ b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift @@ -42,6 +42,8 @@ nonisolated struct AgentProfileLaunchPlan: Equatable, Sendable { /// nothing but the launch command reads them, and the real variable names /// never exist in the pane's shell. let surfaceEnvironment: [String: String] + /// Validated user overrides retained in-memory for preflight facts such as PATH. + let profileEnvironmentOverrides: [String: String] /// Dedicated home to provision before launch; nil for pure presets. let dedicatedHome: URL? /// Runtime-specific session root under the managed home. This is direct for @@ -60,6 +62,7 @@ nonisolated struct AgentProfileLaunchPlan: Equatable, Sendable { placement: AgentProfilePlacement, splitDirection: UserCustomSplitDirection, surfaceEnvironment: [String: String], + profileEnvironmentOverrides: [String: String] = [:], dedicatedHome: URL?, sessionConfigRoot: URL? = nil ) { @@ -73,6 +76,7 @@ nonisolated struct AgentProfileLaunchPlan: Equatable, Sendable { self.placement = placement self.splitDirection = splitDirection self.surfaceEnvironment = surfaceEnvironment + self.profileEnvironmentOverrides = profileEnvironmentOverrides self.dedicatedHome = dedicatedHome self.sessionConfigRoot = sessionConfigRoot ?? dedicatedHome } @@ -163,6 +167,7 @@ nonisolated struct AgentProfileLaunchPlan: Equatable, Sendable { placement: placement, splitDirection: splitDirection, surfaceEnvironment: environment, + profileEnvironmentOverrides: profileEnvironmentOverrides, dedicatedHome: dedicatedHome, sessionConfigRoot: sessionConfigRoot ) @@ -196,6 +201,7 @@ nonisolated struct AgentProfileLaunchPlan: Equatable, Sendable { placement: placement, splitDirection: splitDirection, surfaceEnvironment: environment, + profileEnvironmentOverrides: profileEnvironmentOverrides, dedicatedHome: dedicatedHome, sessionConfigRoot: sessionConfigRoot ) @@ -240,6 +246,22 @@ nonisolated struct FrozenAgentProfileLaunchContext: Equatable, Sendable { let request: AgentProfileLaunchRequest let inheritedCWD: URL let anchorSurfaceID: UUID? + let tracksFocusedAnchor: Bool + let tracksInheritedCWD: Bool + + init( + request: AgentProfileLaunchRequest, + inheritedCWD: URL, + anchorSurfaceID: UUID?, + tracksFocusedAnchor: Bool = false, + tracksInheritedCWD: Bool = false + ) { + self.request = request + self.inheritedCWD = inheritedCWD + self.anchorSurfaceID = anchorSurfaceID + self.tracksFocusedAnchor = tracksFocusedAnchor + self.tracksInheritedCWD = tracksInheritedCWD + } } nonisolated struct PreparedAgentProfileLaunch: Equatable, Sendable { @@ -501,6 +523,7 @@ nonisolated enum AgentProfileLaunchPlanner { placement: profile.placement, splitDirection: profile.splitDirection, surfaceEnvironment: surfaceEnvironment, + profileEnvironmentOverrides: overrides, dedicatedHome: dedicatedHome, sessionConfigRoot: sessionConfigRoot ) diff --git a/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift b/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift index 0e585031..398d76cf 100644 --- a/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift +++ b/supacode/Domain/AgentRuntime/AgentManagedHookPreparer.swift @@ -98,7 +98,7 @@ nonisolated enum AgentManagedHookPreparer { launchDirectory: inheritedCWD, promptArgumentIndex: promptIndex, hookCommands: hookCommands, - readFile: ClaudeSettingsStableReader.read + readFile: { ClaudeSettingsStableReader.read($0, maximumBytes: $1) } ) }.value return AgentManagedHookPreparation( diff --git a/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift b/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift index 153b6f25..c162f18e 100644 --- a/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift +++ b/supacode/Domain/AgentRuntime/CodexConfigReadProcess.swift @@ -124,34 +124,11 @@ nonisolated struct CodexConfigReadProcess: Sendable { } private func readStableProfile(_ url: URL) throws -> Data { - let path = url.path(percentEncoded: false) - let descriptor = open(path, O_RDONLY | O_NOFOLLOW) - guard descriptor >= 0 else { throw CodexConfigReadProcessError.invalidProfile } - defer { close(descriptor) } - var before = stat() - guard fstat(descriptor, &before) == 0, - (before.st_mode & S_IFMT) == S_IFREG, - before.st_uid == geteuid(), - before.st_size >= 0, - before.st_size <= 256 * 1_024 - else { - throw CodexConfigReadProcessError.invalidProfile - } - var data = Data(count: Int(before.st_size)) - var offset = 0 - while offset < data.count { - let count = data.withUnsafeMutableBytes { buffer in - read(descriptor, buffer.baseAddress?.advanced(by: offset), buffer.count - offset) - } - guard count > 0 else { throw CodexConfigReadProcessError.invalidProfile } - offset += count - } - var after = stat() - guard fstat(descriptor, &after) == 0, - before.st_ino == after.st_ino, - before.st_size == after.st_size, - before.st_mtimespec.tv_sec == after.st_mtimespec.tv_sec, - before.st_mtimespec.tv_nsec == after.st_mtimespec.tv_nsec + guard + case .stable(let data) = StableOwnerFileReader.read( + url, + maximumBytes: 256 * 1_024 + ) else { throw CodexConfigReadProcessError.invalidProfile } diff --git a/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift b/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift index 741b6ab3..b61226f1 100644 --- a/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift +++ b/supacode/Domain/AgentRuntime/CodexShellLaunchEnvironment.swift @@ -18,16 +18,22 @@ nonisolated enum CodexShellLaunchEnvironmentProbe { static func resolve( cwd: URL, - shell: ShellClient = .live, + pathOverride: String? = nil, + run: (@Sendable (URL, String) async throws -> ShellOutput)? = nil, isExecutable: (String) -> Bool = { FileManager.default.isExecutableFile(atPath: $0) } ) async -> CodexShellLaunchEnvironment? { + let execute = + run ?? { cwd, script in + try await CodexShellProbeProcess().run(cwd: cwd, script: script) + } + let effectiveScript: String + if let pathOverride { + effectiveScript = "PATH=\(AgentInvocation.shellQuote(pathOverride)); export PATH\n" + script + } else { + effectiveScript = script + } guard - let output = try? await shell.runLogin( - URL(filePath: "/bin/sh", directoryHint: .notDirectory), - ["-c", script], - cwd, - log: false - ), + let output = try? await execute(cwd, effectiveScript), output.exitCode == 0, output.stdout.utf8.count <= 16 * 1_024, let values = parse(output.stdout), diff --git a/supacode/Domain/AgentRuntime/CodexShellProbeProcess.swift b/supacode/Domain/AgentRuntime/CodexShellProbeProcess.swift new file mode 100644 index 00000000..233328e6 --- /dev/null +++ b/supacode/Domain/AgentRuntime/CodexShellProbeProcess.swift @@ -0,0 +1,166 @@ +import Foundation + +#if canImport(Darwin) + import Darwin +#elseif canImport(Glibc) + import Glibc +#endif + +nonisolated enum CodexShellProbeProcessError: Error, Equatable, Sendable { + case cancelled + case outputTooLarge + case processFailed + case timeout +} + +nonisolated struct CodexShellProbeProcess: Sendable { + private struct RunOptions { + let timeout: TimeInterval + let maximumOutputBytes: Int + let shellOverride: URL? + } + + private final class ProcessBox: @unchecked Sendable { + private let lock = NSLock() + private var process: Process? + + func install(_ process: Process) { + lock.withLock { self.process = process } + } + + func terminate() { + lock.withLock { + if process?.isRunning == true { process?.terminate() } + } + } + } + + let timeout: TimeInterval + let maximumOutputBytes: Int + let shellOverride: URL? + + init( + timeout: TimeInterval = 1, + maximumOutputBytes: Int = 16 * 1_024, + shellOverride: URL? = nil + ) { + self.timeout = max(0.05, timeout) + self.maximumOutputBytes = max(1, maximumOutputBytes) + self.shellOverride = shellOverride + } + + func run(cwd: URL, script: String) async throws -> ShellOutput { + let processBox = ProcessBox() + let task = Task.detached(priority: .userInitiated) { + try Self.runSynchronously( + cwd: cwd, + script: script, + options: RunOptions( + timeout: timeout, + maximumOutputBytes: maximumOutputBytes, + shellOverride: shellOverride + ), + processBox: processBox + ) + } + return try await withTaskCancellationHandler { + try await task.value + } onCancel: { + task.cancel() + processBox.terminate() + } + } + + private static func runSynchronously( + cwd: URL, + script: String, + options: RunOptions, + processBox: ProcessBox + ) throws -> ShellOutput { + let process = Process() + if let shellOverride = options.shellOverride { + process.executableURL = shellOverride + process.arguments = [] + } else { + let invocation = ShellClient.loginShellInvocation(userShell: defaultShellURL()) + process.executableURL = invocation.shell + process.arguments = [ + "-l", "-c", invocation.command, "--", + "/bin/sh", "-c", script, + ] + } + process.currentDirectoryURL = cwd + process.standardInput = FileHandle.nullDevice + let output = Pipe() + let errors = Pipe() + process.standardOutput = output + process.standardError = errors + processBox.install(process) + try process.run() + let descriptors = [ + output.fileHandleForReading.fileDescriptor, + errors.fileHandleForReading.fileDescriptor, + ] + let deadline = DispatchTime.now().uptimeNanoseconds + UInt64(options.timeout * 1_000_000_000) + var stdout = Data() + var stderr = Data() + defer { + stop(process) + try? output.fileHandleForReading.close() + try? errors.fileHandleForReading.close() + } + + while process.isRunning || hasReadableData(descriptors) { + if Task.isCancelled { throw CodexShellProbeProcessError.cancelled } + guard DispatchTime.now().uptimeNanoseconds < deadline else { + throw CodexShellProbeProcessError.timeout + } + var pollDescriptors = descriptors.map { pollfd(fd: $0, events: Int16(POLLIN), revents: 0) } + let status = poll(&pollDescriptors, nfds_t(pollDescriptors.count), 25) + if status < 0 { + if errno == EINTR { continue } + throw CodexShellProbeProcessError.processFailed + } + for index in pollDescriptors.indices where pollDescriptors[index].revents & Int16(POLLIN) != 0 { + var chunk = [UInt8](repeating: 0, count: 4 * 1_024) + let count = chunk.withUnsafeMutableBytes { + Darwin.read(descriptors[index], $0.baseAddress, $0.count) + } + if count > 0 { + if index == 0 { + stdout.append(contentsOf: chunk.prefix(count)) + } else { + stderr.append(contentsOf: chunk.prefix(count)) + } + guard stdout.count + stderr.count <= options.maximumOutputBytes else { + throw CodexShellProbeProcessError.outputTooLarge + } + } + } + } + guard process.terminationStatus == 0 else { throw CodexShellProbeProcessError.processFailed } + guard let stdoutText = String(data: stdout, encoding: .utf8), + let stderrText = String(data: stderr, encoding: .utf8) + else { throw CodexShellProbeProcessError.processFailed } + return ShellOutput(stdout: stdoutText, stderr: stderrText, exitCode: process.terminationStatus) + } + + private static func hasReadableData(_ descriptors: [Int32]) -> Bool { + var values = descriptors.map { pollfd(fd: $0, events: Int16(POLLIN), revents: 0) } + return poll(&values, nfds_t(values.count), 0) > 0 + } + + private static func stop(_ process: Process) { + if process.isRunning { process.terminate() } + for _ in 0..<100 where process.isRunning { usleep(1_000) } + if process.isRunning { kill(process.processIdentifier, SIGKILL) } + process.waitUntilExit() + } + + private static func defaultShellURL() -> URL { + if let shell = ProcessInfo.processInfo.environment["SHELL"], !shell.isEmpty { + return URL(filePath: shell, directoryHint: .notDirectory) + } + return URL(filePath: "/bin/zsh", directoryHint: .notDirectory) + } +} diff --git a/supacode/Domain/AgentRuntime/ManagedHookRendering.swift b/supacode/Domain/AgentRuntime/ManagedHookRendering.swift index f9f845e1..3370716c 100644 --- a/supacode/Domain/AgentRuntime/ManagedHookRendering.swift +++ b/supacode/Domain/AgentRuntime/ManagedHookRendering.swift @@ -1,11 +1,5 @@ import Foundation -#if canImport(Darwin) - import Darwin -#elseif canImport(Glibc) - import Glibc -#endif - nonisolated enum ClaudeSettingsReadResult: Equatable, Sendable { case stable(Data) case changed @@ -14,34 +8,17 @@ nonisolated enum ClaudeSettingsReadResult: Equatable, Sendable { } nonisolated enum ClaudeSettingsStableReader { - static func read(_ url: URL, maximumBytes: Int) -> ClaudeSettingsReadResult { - let descriptor = Darwin.open(url.path(percentEncoded: false), O_RDONLY | O_NOFOLLOW) - guard descriptor >= 0 else { return .unreadable } - defer { Darwin.close(descriptor) } - var before = stat() - guard fstat(descriptor, &before) == 0, - (before.st_mode & S_IFMT) == S_IFREG, - before.st_uid == geteuid(), - before.st_size >= 0 - else { return .unreadable } - guard before.st_size <= maximumBytes else { return .oversized } - var data = Data(count: Int(before.st_size)) - var offset = 0 - while offset < data.count { - let count = data.withUnsafeMutableBytes { buffer in - Darwin.read(descriptor, buffer.baseAddress?.advanced(by: offset), buffer.count - offset) - } - guard count > 0 else { return .unreadable } - offset += count + static func read( + _ url: URL, + maximumBytes: Int, + afterRead: () -> Void = {} + ) -> ClaudeSettingsReadResult { + switch StableOwnerFileReader.read(url, maximumBytes: maximumBytes, afterRead: afterRead) { + case .stable(let data): .stable(data) + case .changed: .changed + case .oversized: .oversized + case .unreadable: .unreadable } - var after = stat() - guard fstat(descriptor, &after) == 0, - before.st_ino == after.st_ino, - before.st_size == after.st_size, - before.st_mtimespec.tv_sec == after.st_mtimespec.tv_sec, - before.st_mtimespec.tv_nsec == after.st_mtimespec.tv_nsec - else { return .changed } - return .stable(data) } } @@ -123,8 +100,7 @@ nonisolated enum ClaudeHookSettingsPreparer { let insertionIndex = resolvedPromptIndex( promptArgumentIndex, - arguments: arguments, - executable: invocation.executable + arguments: arguments ) ?? arguments.endIndex arguments.insert(contentsOf: ["--settings", "{}"], at: insertionIndex) carrierIndex = insertionIndex + 1 @@ -268,14 +244,10 @@ nonisolated enum ClaudeHookSettingsPreparer { private static func resolvedPromptIndex( _ explicit: Int?, - arguments: [String], - executable: String + arguments: [String] ) -> Int? { - if let explicit, arguments.indices.contains(explicit) { return explicit } - if executable == "claude", arguments.first == "-p", arguments.count >= 2 { - return arguments.index(before: arguments.endIndex) - } - return nil + guard let explicit, arguments.indices.contains(explicit) else { return nil } + return explicit } private static func degraded(_ invocation: AgentInvocation) -> AgentHookPreparationOutcome { @@ -310,15 +282,10 @@ nonisolated enum CodexManagedNotifyRenderer { ) let notifyJSON = notifyData.flatMap { String(data: $0, encoding: .utf8) } ?? "[]" var arguments = invocation.arguments - let inferredPromptIndex: Int? = - if let promptArgumentIndex, arguments.indices.contains(promptArgumentIndex) { - promptArgumentIndex - } else if arguments.first == "exec", arguments.count >= 2 { - arguments.index(before: arguments.endIndex) - } else { - nil - } - let insertionIndex = inferredPromptIndex ?? arguments.endIndex + let explicitPromptIndex = promptArgumentIndex.flatMap { + arguments.indices.contains($0) ? $0 : nil + } + let insertionIndex = explicitPromptIndex ?? arguments.endIndex arguments.insert(contentsOf: ["-c", "notify=[]"], at: insertionIndex) return AgentHookPreparedInvocation( invocation: AgentInvocation(executable: invocation.executable, arguments: arguments), diff --git a/supacode/Domain/AgentRuntime/StableOwnerFileReader.swift b/supacode/Domain/AgentRuntime/StableOwnerFileReader.swift new file mode 100644 index 00000000..747de675 --- /dev/null +++ b/supacode/Domain/AgentRuntime/StableOwnerFileReader.swift @@ -0,0 +1,64 @@ +import Foundation + +#if canImport(Darwin) + import Darwin +#elseif canImport(Glibc) + import Glibc +#endif + +nonisolated enum StableOwnerFileReadResult: Equatable, Sendable { + case stable(Data) + case changed + case oversized + case unreadable +} + +nonisolated enum StableOwnerFileReader { + static func read( + _ url: URL, + maximumBytes: Int, + afterRead: () -> Void = {} + ) -> StableOwnerFileReadResult { + let path = url.path(percentEncoded: false) + let descriptor = Darwin.open(path, O_RDONLY | O_NOFOLLOW) + guard descriptor >= 0 else { return .unreadable } + defer { Darwin.close(descriptor) } + var before = stat() + guard fstat(descriptor, &before) == 0, + (before.st_mode & S_IFMT) == S_IFREG, + before.st_uid == geteuid(), + before.st_size >= 0 + else { return .unreadable } + guard before.st_size <= maximumBytes else { return .oversized } + + var data = Data(count: Int(before.st_size)) + var offset = 0 + while offset < data.count { + let count = data.withUnsafeMutableBytes { buffer in + Darwin.read(descriptor, buffer.baseAddress?.advanced(by: offset), buffer.count - offset) + } + guard count > 0 else { return .unreadable } + offset += count + } + afterRead() + + var descriptorAfter = stat() + var pathAfter = stat() + guard fstat(descriptor, &descriptorAfter) == 0, + lstat(path, &pathAfter) == 0, + sameSnapshot(before, descriptorAfter), + sameSnapshot(descriptorAfter, pathAfter), + (pathAfter.st_mode & S_IFMT) == S_IFREG, + pathAfter.st_uid == geteuid() + else { return .changed } + return .stable(data) + } + + private static func sameSnapshot(_ lhs: stat, _ rhs: stat) -> Bool { + lhs.st_dev == rhs.st_dev + && lhs.st_ino == rhs.st_ino + && lhs.st_size == rhs.st_size + && lhs.st_mtimespec.tv_sec == rhs.st_mtimespec.tv_sec + && lhs.st_mtimespec.tv_nsec == rhs.st_mtimespec.tv_nsec + } +} diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index f26fd3da..0b4380e5 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -16,7 +16,8 @@ final class WorktreeTerminalManager { @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 codexShellEnvironmentResolver: + @Sendable (URL, String?) async -> CodexShellLaunchEnvironment? @ObservationIgnored private let hookResourcesProvider: @MainActor () -> AgentHookResources? @ObservationIgnored private let forwardingRecordBaseDirectory: URL @ObservationIgnored private var codexForwardingRecordStore: CodexForwardingRecordStore? @@ -46,8 +47,8 @@ final class WorktreeTerminalManager { agentObservationBufferCapacity: Int = 64, agentDispatchStore: AgentDispatchStore = AgentDispatchStore(), codexConfigReadProcess: CodexConfigReadProcess = CodexConfigReadProcess(), - codexShellEnvironmentResolver: @escaping @Sendable (URL) async -> CodexShellLaunchEnvironment? = { - await CodexShellLaunchEnvironmentProbe.resolve(cwd: $0) + codexShellEnvironmentResolver: @escaping @Sendable (URL, String?) async -> CodexShellLaunchEnvironment? = { + await CodexShellLaunchEnvironmentProbe.resolve(cwd: $0, pathOverride: $1) }, hookResourcesProvider: @escaping @MainActor () -> AgentHookResources? = { guard let url = SupacodePaths.bundledCLIURL else { return nil } @@ -118,7 +119,10 @@ final class WorktreeTerminalManager { let resources = hookResourcesProvider() let codexShellEnvironment: CodexShellLaunchEnvironment? if context.request.plan.runtime == .codex, resources != nil { - codexShellEnvironment = await codexShellEnvironmentResolver(context.inheritedCWD) + codexShellEnvironment = await codexShellEnvironmentResolver( + context.inheritedCWD, + context.request.plan.profileEnvironmentOverrides["PATH"] + ) } else { codexShellEnvironment = nil } @@ -188,7 +192,9 @@ final class WorktreeTerminalManager { title: context.request.title ), inheritedCWD: context.inheritedCWD, - anchorSurfaceID: context.anchorSurfaceID + anchorSurfaceID: context.anchorSurfaceID, + tracksFocusedAnchor: context.tracksFocusedAnchor, + tracksInheritedCWD: context.tracksInheritedCWD ) return .success( PreparedAgentProfileLaunch( @@ -1246,7 +1252,7 @@ final class WorktreeTerminalManager { self.agentObservationStore = AgentObservationStore(bufferCapacity: 64) self.agentDispatchStore = AgentDispatchStore() self.codexConfigReadProcess = CodexConfigReadProcess() - self.codexShellEnvironmentResolver = { _ in nil } + self.codexShellEnvironmentResolver = { _, _ in nil } self.hookResourcesProvider = { nil } self.forwardingRecordBaseDirectory = SupacodePaths.agentHookForwardingDirectory self.baselineFontSize = 13 diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index 43eedbdf..17cbfefd 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -492,16 +492,19 @@ final class WorktreeTerminalState { ) -> Result { let anchor: UUID? let context: ghostty_surface_context_e + let tracksFocusedAnchor: Bool switch request.placement { case .tab: anchor = request.inheritanceAnchor ?? currentFocusedSurfaceId() context = GHOSTTY_SURFACE_CONTEXT_TAB + tracksFocusedAnchor = request.inheritanceAnchor == nil case .split(let requestedAnchor, _, _): guard let resolved = requestedAnchor ?? currentFocusedSurfaceId(), surfaces[resolved] != nil else { return .failure(.splitAnchorUnavailable) } anchor = resolved context = GHOSTTY_SURFACE_CONTEXT_SPLIT + tracksFocusedAnchor = requestedAnchor == nil } let inheritedCWD = request.workingDirectoryOverride @@ -523,14 +526,34 @@ final class WorktreeTerminalState { title: request.title ), inheritedCWD: inheritedCWD.standardizedFileURL, - anchorSurfaceID: anchor + anchorSurfaceID: anchor, + tracksFocusedAnchor: tracksFocusedAnchor, + tracksInheritedCWD: request.workingDirectoryOverride == nil ) ) } - func isAgentProfileLaunchContextValid(_ context: FrozenAgentProfileLaunchContext) -> Bool { - guard let anchor = context.anchorSurfaceID else { return true } - return surfaces[anchor] != nil + func isAgentProfileLaunchContextValid( + _ context: FrozenAgentProfileLaunchContext, + inheritedCWDOverride: URL? = nil + ) -> Bool { + if let anchor = context.anchorSurfaceID, surfaces[anchor] == nil { return false } + if context.tracksFocusedAnchor, currentFocusedSurfaceId() != context.anchorSurfaceID { return false } + guard context.tracksInheritedCWD else { return true } + let surfaceContext: ghostty_surface_context_e = + switch context.request.placement { + case .tab: GHOSTTY_SURFACE_CONTEXT_TAB + case .split: GHOSTTY_SURFACE_CONTEXT_SPLIT + } + let currentCWD = + inheritedCWDOverride + ?? inheritedSurfaceConfig( + fromSurfaceId: context.anchorSurfaceID, + context: surfaceContext + ).workingDirectory + ?? worktree.workingDirectory + return AgentProfileLaunchPlanner.pathString(currentCWD) + == AgentProfileLaunchPlanner.pathString(context.inheritedCWD) } /// Launches an agent profile through the deterministic A2 boundary. Explicit diff --git a/supacodeTests/AgentHookRenderingTests.swift b/supacodeTests/AgentHookRenderingTests.swift index 960ff093..24340e0a 100644 --- a/supacodeTests/AgentHookRenderingTests.swift +++ b/supacodeTests/AgentHookRenderingTests.swift @@ -96,6 +96,23 @@ struct AgentHookRenderingTests { #expect(stop.count == 1) } + @Test func stableReaderRejectsAtomicPathReplacement() throws { + let directory = FileManager.default.temporaryDirectory.appending( + path: "prowl-settings-replacement-\(UUID().uuidString)", + directoryHint: .isDirectory + ) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let settings = directory.appending(path: "settings.json", directoryHint: .notDirectory) + try Data(#"{"version":1}"#.utf8).write(to: settings) + + let result = ClaudeSettingsStableReader.read(settings, maximumBytes: 1_024) { + try? Data(#"{"version":2}"#.utf8).write(to: settings, options: .atomic) + } + + #expect(result == .changed) + } + @Test func claudeMalformedChangedAndOversizedSettingsPreserveInvocation() { let invocation = AgentInvocation(executable: "claude", arguments: ["--settings", "bad.json", "Prompt"]) let missingValue = ClaudeHookSettingsPreparer.prepare( @@ -134,6 +151,7 @@ struct AgentHookRenderingTests { let claudeOutcome = ClaudeHookSettingsPreparer.prepare( invocation: claude, launchDirectory: URL(filePath: "/tmp", directoryHint: .isDirectory), + promptArgumentIndex: 1, hookCommands: hookCommands, readFile: { _, _ in .unreadable } ) @@ -143,7 +161,8 @@ struct AgentHookRenderingTests { let codex = CodexManagedNotifyRenderer.prepare( invocation: AgentInvocation(executable: "codex", arguments: ["exec", "Prompt"]), - bundledCLIPath: "/Applications/Prowl Debug.app/Contents/Resources/prowl-cli/prowl" + bundledCLIPath: "/Applications/Prowl Debug.app/Contents/Resources/prowl-cli/prowl", + promptArgumentIndex: 1 ) #expect(codex.invocation.arguments.last == "Prompt") #expect(codex.invocation.arguments.contains("-c")) @@ -151,6 +170,33 @@ struct AgentHookRenderingTests { #expect(codex.argumentValues.values.first?.contains("dangerously-bypass-hook-trust") == false) } + @Test func unpromptedOptionValuePairsAreNeverInferredAsPrompts() throws { + let claude = ClaudeHookSettingsPreparer.prepare( + invocation: AgentInvocation(executable: "claude", arguments: ["-p", "--model", "opus"]), + launchDirectory: URL(filePath: "/tmp", directoryHint: .isDirectory), + hookCommands: hookCommands, + readFile: { _, _ in .unreadable } + ) + let preparedClaude = try #require(claude.prepared) + #expect( + Array(preparedClaude.invocation.arguments.prefix(3)) + == ["-p", "--model", "opus"] + ) + #expect(preparedClaude.invocation.arguments.suffix(2) == ["--settings", "{}"]) + + for original in [ + ["exec", "-C", "/tmp/project"], + ["exec", "--cd=/tmp/project"], + ] { + let codex = CodexManagedNotifyRenderer.prepare( + invocation: AgentInvocation(executable: "codex", arguments: original), + bundledCLIPath: "/bundle/prowl" + ) + #expect(Array(codex.invocation.arguments.prefix(original.count)) == original) + #expect(codex.invocation.arguments.suffix(2) == ["-c", "notify=[]"]) + } + } + @Test func arbitraryArgumentCarriersNeverRenderValuesIntoTerminalInput() { let invocation = AgentInvocation( executable: "claude", diff --git a/supacodeTests/CodexShellLaunchEnvironmentTests.swift b/supacodeTests/CodexShellLaunchEnvironmentTests.swift index 2133b820..fc186ffc 100644 --- a/supacodeTests/CodexShellLaunchEnvironmentTests.swift +++ b/supacodeTests/CodexShellLaunchEnvironmentTests.swift @@ -6,30 +6,25 @@ import Testing 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 + let run: @Sendable (URL, String) async throws -> ShellOutput = { currentDirectory, script in + #expect(currentDirectory == cwd) + #expect(script.contains("command -v -- codex")) + 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 - ) - } - ) + """, + stderr: "ignored", + exitCode: 0 + ) + } let environment = try #require( await CodexShellLaunchEnvironmentProbe.resolve( cwd: cwd, - shell: shell, + run: run, isExecutable: { $0 == "/opt/custom/bin/codex" } ) ) @@ -66,20 +61,36 @@ struct CodexShellLaunchEnvironmentTests { exitCode: 1 ), ] { - let shell = ShellClient( - run: { _, _, _ in output }, - runLoginImpl: { _, _, _, _ in output } - ) #expect( await CodexShellLaunchEnvironmentProbe.resolve( cwd: URL(filePath: "/tmp", directoryHint: .isDirectory), - shell: shell, + run: { _, _ in output }, isExecutable: { $0 == "/opt/codex" } ) == nil ) } } + @Test func profilePATHOverrideParticipatesInExecutableResolution() async throws { + let output = """ + __PROWL_CODEX_EXECUTABLE__/custom/bin/codex + __PROWL_CODEX_HOME_BASE__/Users/tester + __PROWL_CODEX_HOME__ + + """ + let result = await CodexShellLaunchEnvironmentProbe.resolve( + cwd: URL(filePath: "/tmp", directoryHint: .isDirectory), + pathOverride: "/custom/bin:/usr/bin", + run: { _, script in + #expect(script.contains("PATH='/custom/bin:/usr/bin'; export PATH")) + return ShellOutput(stdout: output, stderr: "", exitCode: 0) + }, + isExecutable: { $0 == "/custom/bin/codex" } + ) + + #expect(result?.executableURL.path(percentEncoded: false) == "/custom/bin/codex") + } + @Test func capturedShellHomeDefinesDefaultCodexHome() throws { let shell = CodexShellLaunchEnvironment( executableURL: URL(filePath: "/opt/codex"), diff --git a/supacodeTests/CodexShellProbeProcessTests.swift b/supacodeTests/CodexShellProbeProcessTests.swift new file mode 100644 index 00000000..c890f21a --- /dev/null +++ b/supacodeTests/CodexShellProbeProcessTests.swift @@ -0,0 +1,79 @@ +import Darwin +import Foundation +import Testing + +@testable import supacode + +struct CodexShellProbeProcessTests { + @Test func hardTimeoutKillsLoginShellThatIgnoresTermination() async throws { + let root = temporaryDirectory("shell-timeout") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let pidFile = root.appending(path: "pid", directoryHint: .notDirectory) + let shell = try executableScript( + in: root, + name: "hang.sh", + contents: """ + #!/bin/sh + trap '' TERM + printf '%s' "$$" > \(shellQuote(pidFile.path)) + while :; do sleep 1; done + """ + ) + let process = CodexShellProbeProcess( + timeout: 0.5, + maximumOutputBytes: 1_024, + shellOverride: shell + ) + + await #expect(throws: CodexShellProbeProcessError.timeout) { + try await process.run(cwd: root, script: "ignored") + } + + let pidText = try String(contentsOf: pidFile, encoding: .utf8) + let pid = try #require(pid_t(pidText)) + #expect(kill(pid, 0) == -1) + #expect(errno == ESRCH) + } + + @Test func streamingOutputBoundStopsNoisyLoginShell() async throws { + let root = temporaryDirectory("shell-output") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let shell = try executableScript( + in: root, + name: "noisy.sh", + contents: """ + #!/bin/sh + while :; do printf '0123456789abcdef'; done + """ + ) + let process = CodexShellProbeProcess( + timeout: 2, + maximumOutputBytes: 128, + shellOverride: shell + ) + + await #expect(throws: CodexShellProbeProcessError.outputTooLarge) { + try await process.run(cwd: root, script: "ignored") + } + } + + private func executableScript(in root: URL, name: String, contents: String) throws -> URL { + let url = root.appending(path: name, directoryHint: .notDirectory) + try contents.write(to: url, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: url.path) + return url + } + + private func temporaryDirectory(_ name: String) -> URL { + FileManager.default.temporaryDirectory.appending( + path: "prowl-tests-\(name)-\(UUID().uuidString)", + directoryHint: .isDirectory + ) + } + + private func shellQuote(_ value: String) -> String { + "'" + value.replacing("'", with: "'\"'\"'") + "'" + } +} diff --git a/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift b/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift index f1e1aa1e..33a64a24 100644 --- a/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift +++ b/supacodeTests/WorktreeTerminalStateAgentProfileTests.swift @@ -122,6 +122,44 @@ struct WorktreeTerminalStateAgentProfileTests { #expect(state.currentFocusedSurfaceId() == anchor.surfaceID) } + @Test func frozenDynamicProfileContextRejectsFocusAndInheritedCWDDrift() throws { + let state = makeState() + let first = try state.launchAgentProfile( + AgentProfileLaunchRequest( + plan: makePlan(dedicatedHome: nil), + placement: .tab(background: false) + ) + ).get() + _ = try state.launchAgentProfile( + AgentProfileLaunchRequest( + plan: makePlan(dedicatedHome: nil), + placement: .tab(background: false) + ) + ).get() + let request = AgentProfileLaunchRequest( + plan: makePlan(dedicatedHome: nil), + placement: .tab(background: false) + ) + let frozen = try state.freezeAgentProfileLaunchContext(request).get() + #expect(state.isAgentProfileLaunchContextValid(frozen)) + + #expect(state.focusSurface(id: first.surfaceID)) + #expect(!state.isAgentProfileLaunchContextValid(frozen)) + let cwdOnly = FrozenAgentProfileLaunchContext( + request: frozen.request, + inheritedCWD: frozen.inheritedCWD, + anchorSurfaceID: frozen.anchorSurfaceID, + tracksFocusedAnchor: false, + tracksInheritedCWD: true + ) + #expect( + !state.isAgentProfileLaunchContextValid( + cwdOnly, + inheritedCWDOverride: URL(filePath: "/tmp/repo/changed", directoryHint: .isDirectory) + ) + ) + } + @Test func explicitSplitFailureDoesNotFallBackToATab() { let state = makeState() let missingAnchor = UUID()