diff --git a/docs-ai/055-agent-profile-runtimes/001-action.md b/docs-ai/055-agent-profile-runtimes/001-action.md index 63741058..37952187 100644 --- a/docs-ai/055-agent-profile-runtimes/001-action.md +++ b/docs-ai/055-agent-profile-runtimes/001-action.md @@ -73,6 +73,28 @@ exclude false positives are recorded in the linked research document. Claude/Codex policy, preventing generic Profile expansion from silently changing an independently verified workflow. +### Review follow-up: executable-probe cost + +- Reproduced the review concern against the exact `ShellClient.runLogin` + shape (`zsh -l`, source `.zshrc`, then `exec /bin/sh -c`). Across 20 + fifteen-runtime refreshes, the original 300-login-shell fan-out took 17.37s + wall time and 51.43 CPU-seconds on the development machine. A 20-login-shell + batched equivalent took 5.63s wall time and 1.96 CPU-seconds. Direct + per-process energy sampling was unavailable because `powermetrics` requires + superuser privileges; process count and CPU time establish the avoidable + energy work without pretending to have measured watts. +- Replaced per-runtime shell tasks with one marker-delimited batch command. + Positive answers remain final for the app session; negative answers use a + five-minute TTL, preserving mid-session CLI installation detection without + reloading shell startup files on every popover open. Concurrent startup and + popover refreshes share one in-flight task. +- Kept the established Advanced-arguments trust boundary. Prowl does not parse + every runtime's evolving flags or configuration. Unknown arguments on a + Standard selection produce the neutral disclosure; an explicit + Unrestricted picker selection retains its conservative warning even if a + later last-wins Advanced flag may override the generated request. The exact + final argv remains visible in Launch Preview. + ## Research and live evidence - Captured installed versions and local `--help` contracts for all fifteen @@ -109,7 +131,10 @@ exclude false positives are recorded in the linked research document. availability probes, and Handoff admission. - The first focused TDD run failed at the expected missing runtime/capability assertions before implementation. -- `make test` passed with **2152 tests and zero failures**. The five emitted +- The executable-probe review follow-up began with three expected RED tests; + its final focused suite passed all five batching, cache, failure-degradation, + and Advanced-argument disclosure tests. +- `make test` passed with **2154 tests and zero failures**. The five emitted dependency-scan warnings are pre-existing package declaration warnings. - `make check` passed. - `make build-app` passed with zero warnings and zero errors. diff --git a/docs/components/agent-profiles.md b/docs/components/agent-profiles.md index 58549945..7e0c5f34 100644 --- a/docs/components/agent-profiles.md +++ b/docs/components/agent-profiles.md @@ -135,7 +135,11 @@ about what it can prove: recognized bypass flags (`--yolo`, when the picker says Standard; any other extra argument (including `--sandbox`/`--ask-for-approval`/`-c` overrides) shows a neutral "effective execution mode follows your extra arguments" note instead of claiming -Standard — the semantics belong to your command line. +Standard — the semantics belong to your command line. When the picker itself +requests Unrestricted, the warning deliberately remains conservative even if +later Extra Arguments may override the generated flags: Advanced arguments +are authoritative, and Prowl does not attempt to interpret every runtime's +full, evolving option and configuration surface. The editor opens with a **Profile** section (name, agent, icon), followed by **Launch Preview** — the exact rendered invocation, including the env prefix for bound profiles, using the same rendering as the real launch — then a @@ -207,10 +211,12 @@ prompt receiver in a future handoff workflow without another transport. once it answers, "not found" warns "… is not on your shell's PATH" and "found" clears any warning. Until it answers, the fallback heuristic — the runtime's default home exists iff the CLI has - ever run — warns "… may not be installed". A positive probe is cached for - the session; negatives re-probe each time the Agents popover opens, so - installing a CLI mid-session clears its warning without a relaunch. Both - signals only dim rows, never disable them. + ever run — warns "… may not be installed". One login shell batches all + pending runtime lookups. A positive answer is cached for the session; + negative answers are cached for five minutes before becoming eligible for + another background probe, so installing a CLI is still detected without + repeatedly loading shell startup files whenever the Agents popover opens. + Both signals only dim rows, never disable them. - Prowl provides no directory sharing between a bound home and the default one. Symlinking read-mostly directories (e.g. `skills/`) yourself works, but never link files the CLI rewrites (`settings.json`, `config.toml`, diff --git a/supacode/Clients/AgentProfile/AgentRuntimeAvailabilityProbe.swift b/supacode/Clients/AgentProfile/AgentRuntimeAvailabilityProbe.swift index 692ada20..ad8fcad9 100644 --- a/supacode/Clients/AgentProfile/AgentRuntimeAvailabilityProbe.swift +++ b/supacode/Clients/AgentProfile/AgentRuntimeAvailabilityProbe.swift @@ -2,11 +2,17 @@ import Dependencies import Foundation import Sharing -extension SharedReaderKey where Self == InMemoryKey<[AgentProfileRuntime: Bool]>.Default { +nonisolated struct AgentRuntimeAvailabilityProbeResult: Equatable, Sendable { + let isAvailable: Bool + let checkedAt: Date +} + +extension SharedReaderKey +where Self == InMemoryKey<[AgentProfileRuntime: AgentRuntimeAvailabilityProbeResult]>.Default { /// Session cache of the login-shell executable probe. A missing entry means /// the probe has not answered for that runtime yet this session. - static var agentRuntimeProbedAvailability: Self { - Self[.inMemory("agentRuntimeProbedAvailability"), default: [:]] + static var agentRuntimeAvailabilityProbeResults: Self { + Self[.inMemory("agentRuntimeAvailabilityProbeResults"), default: [:]] } } @@ -15,60 +21,119 @@ extension AgentProfileAvailability { /// home-directory heuristic while a runtime is unanswered. @MainActor static func launchWarning(for profile: AgentProfile) -> String? { - @Shared(.agentRuntimeProbedAvailability) var probed - return launchWarning(for: profile, probedAvailable: probed[profile.runtime]) + @Shared(.agentRuntimeAvailabilityProbeResults) var probeResults + return launchWarning( + for: profile, + probedAvailable: probeResults[profile.runtime]?.isAvailable + ) } } -/// Resolves each runtime's executable through the user's login shell — the -/// same resolution a profile launch uses, so a positive answer means the -/// launch will find the binary (docs-ai 053/005). A GUI app's own PATH is the -/// launchd default and useless for this; the login shell's rc-built PATH is -/// the truth. Results cache for the session: a positive is final, while a -/// negative or unanswered runtime re-probes on the next refresh (opening the -/// Agents popover), so installing a CLI mid-session clears its warning -/// without a relaunch. +/// Resolves runtime executables through the user's login shell — the same +/// resolution a profile launch uses, so a positive answer means the launch +/// will find the binary (docs-ai 053/005). A GUI app's own PATH is the launchd +/// default and useless for this; the login shell's rc-built PATH is the truth. +/// +/// One refresh batches every pending `command -v` into a single login shell. +/// Positive answers are final for the app session. Negative answers have a +/// short TTL so a newly installed CLI becomes visible without paying login-rc +/// startup cost every time the Agents popover opens. @MainActor enum AgentRuntimeAvailabilityProbe { + static let negativeResultLifetime: TimeInterval = 5 * 60 + + private static let outputMarker = "__PROWL_AGENT_RUNTIME_AVAILABILITY__" + private static var inFlightRefresh: Task? + static func refresh() async { - @Shared(.agentRuntimeProbedAvailability) var probed - let pending = AgentProfileRuntime.allCases.filter { probed[$0] != true } - guard !pending.isEmpty else { return } - await withTaskGroup(of: (AgentProfileRuntime, Bool?).self) { group in - for runtime in pending { - group.addTask { (runtime, await probeAvailability(of: runtime)) } - } - for await (runtime, availability) in group { - guard let availability else { continue } - $probed.withLock { $0[runtime] = availability } + if let inFlightRefresh { + await inFlightRefresh.value + return + } + + let task = Task { await performRefresh() } + inFlightRefresh = task + await task.value + inFlightRefresh = nil + } + + private static func performRefresh() async { + @Dependency(\.date.now) var now + @Shared(.agentRuntimeAvailabilityProbeResults) var probeResults + let checkedAt = now + let pending = AgentProfileRuntime.allCases.filter { runtime in + guard let result = probeResults[runtime] else { return true } + guard !result.isAvailable else { return false } + return checkedAt.timeIntervalSince(result.checkedAt) >= negativeResultLifetime + } + guard !pending.isEmpty, let answers = await probeAvailability(of: pending) else { return } + + $probeResults.withLock { results in + for (runtime, isAvailable) in answers { + results[runtime] = AgentRuntimeAvailabilityProbeResult( + isAvailable: isAvailable, + checkedAt: checkedAt + ) } } } - /// true/false when the login shell answered; nil when the probe itself - /// could not run (spawn failure) — an unanswered probe must not masquerade - /// as "not installed". - private static func probeAvailability(of runtime: AgentProfileRuntime) async -> Bool? { - guard - let executable = try? AgentRuntimeAdapterRegistry.makeStartInvocation( - AgentStartRequest(runtime: runtime, intent: .interactive) - ).executable - else { return nil } + /// Returns the complete set of login-shell answers, or nil when the probe + /// itself could not run or returned malformed output. An incomplete batch + /// must remain unanswered rather than masquerading as "not installed". + private static func probeAvailability( + of runtimes: [AgentProfileRuntime] + ) async -> [AgentProfileRuntime: Bool]? { + let probes = runtimes.compactMap { runtime -> (runtime: AgentProfileRuntime, executable: String)? in + guard + let executable = try? AgentRuntimeAdapterRegistry.makeStartInvocation( + AgentStartRequest(runtime: runtime, intent: .interactive) + ).executable + else { return nil } + return (runtime, executable) + } + guard !probes.isEmpty else { return [:] } + + let script = probes.map { probe in + let executable = AgentInvocation.shellQuote(probe.executable) + let available = AgentInvocation.shellQuote("\(outputMarker):\(probe.runtime.rawValue):1") + let unavailable = AgentInvocation.shellQuote("\(outputMarker):\(probe.runtime.rawValue):0") + return """ + if command -v -- \(executable) >/dev/null 2>&1; then + printf '%s\\n' \(available) + else + printf '%s\\n' \(unavailable) + fi + """ + }.joined(separator: "\n") + @Dependency(ShellClient.self) var shell - do { - // `command -v` is a shell builtin, so it runs in an inner /bin/sh that - // inherits the PATH the outer login shell built from the user's rc. - _ = try await shell.runLogin( + guard + let output = try? await shell.runLogin( URL(fileURLWithPath: "/bin/sh"), - ["-c", "command -v -- '\(executable)'"], + ["-c", script], nil, log: false ) - return true - } catch let error as ShellClientError { - return error.exitCode > 0 ? false : nil - } catch { - return nil + else { return nil } + + var answers: [AgentProfileRuntime: Bool] = [:] + for line in output.stdout.split(whereSeparator: \.isNewline) { + let fields = line.split(separator: ":", omittingEmptySubsequences: false) + guard + fields.count == 3, + fields[0] == outputMarker[...], + let runtime = AgentProfileRuntime(rawValue: String(fields[1])) + else { continue } + switch fields[2] { + case "1": answers[runtime] = true + case "0": answers[runtime] = false + default: continue + } } + + let expected = Set(probes.map { $0.runtime }) + guard Set(answers.keys) == expected else { return nil } + return answers } } diff --git a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift index 2f3816f8..19f52495 100644 --- a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift +++ b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift @@ -153,7 +153,11 @@ nonisolated enum AgentProfileEnvironmentPolicy { } } -/// What the editor may honestly claim about a profile's execution mode. +/// The editor's conservative execution-mode disclosure. This is not a full +/// parser for a runtime's final configuration: an explicit Unrestricted +/// picker selection keeps its warning even when later Advanced arguments may +/// override generated flags. Those arguments are authoritative user input. +/// /// CLI flag surfaces evolve (`--sandbox danger-full-access`, /// `--ask-for-approval never`, arbitrary `-c` overrides), so any recognition /// list goes stale: instead of chasing it, unrecognized extra arguments @@ -167,10 +171,10 @@ nonisolated enum AgentProfileEffectiveExecutionMode: Equatable, Sendable { nonisolated extension AgentProfile { /// Extra arguments are respected as explicit user configuration — never - /// blocked or stripped. A bypass flag the adapter's `observe` recognizes - /// (`--yolo`, `--dangerously-*`, `--permission-mode bypassPermissions`) - /// upgrades the claim to `.unrestricted`; any other extra argument defers - /// the claim entirely. + /// blocked or stripped. For a Standard selection, a bypass flag the + /// adapter's `observe` recognizes (`--yolo`, `--dangerously-*`, + /// `--permission-mode bypassPermissions`) upgrades the disclosure to + /// `.unrestricted`; any other extra argument defers it entirely. var effectiveExecutionMode: AgentProfileEffectiveExecutionMode { let selectableModes = AgentRuntimeAdapterRegistry.profileAdapter(for: runtime)?.executionModeOptions ?? [] diff --git a/supacodeTests/AgentProfileTests.swift b/supacodeTests/AgentProfileTests.swift index db322c47..5a767c9d 100644 --- a/supacodeTests/AgentProfileTests.swift +++ b/supacodeTests/AgentProfileTests.swift @@ -525,6 +525,14 @@ struct AgentProfileTests { claude.executionMode = .unrestricted #expect(claude.effectiveExecutionMode == .unrestricted) + // Advanced arguments remain authoritative and intentionally unparsed. + // Keep the explicit picker warning conservative even when a later flag + // may override the generated least-restricted request. + var cline = profile(name: "Cline", runtime: .cline) + cline.executionMode = .unrestricted + cline.extraArguments = "--auto-approve false" + #expect(cline.effectiveExecutionMode == .unrestricted) + var piProfile = profile(name: "Pi", runtime: .pi) piProfile.executionMode = .unrestricted #expect(piProfile.effectiveExecutionMode == .standard) diff --git a/supacodeTests/AgentRuntimeAvailabilityProbeTests.swift b/supacodeTests/AgentRuntimeAvailabilityProbeTests.swift index a3f61e1e..14a22eae 100644 --- a/supacodeTests/AgentRuntimeAvailabilityProbeTests.swift +++ b/supacodeTests/AgentRuntimeAvailabilityProbeTests.swift @@ -6,53 +6,149 @@ import Testing @testable import supacode +@Suite(.serialized) @MainActor struct AgentRuntimeAvailabilityProbeTests { - @Test(.dependencies) func refreshRecordsShellAnswersPerRuntime() async { + @Test(.dependencies) func refreshBatchesAllPendingRuntimesInOneLoginShell() async { + let probeCalls = LockIsolated(0) await withDependencies { + $0.date.now = Date(timeIntervalSince1970: 1_000) + $0.shellClient = shellClient(probeCalls: probeCalls, available: [.codex]) + } operation: { + @Shared(.agentRuntimeAvailabilityProbeResults) var probeResults + $probeResults.withLock { $0 = [:] } + + await AgentRuntimeAvailabilityProbe.refresh() + + #expect(probeCalls.value == 1) + #expect(probeResults[.codex]?.isAvailable == true) + #expect(probeResults[.claude]?.isAvailable == false) + } + } + + @Test(.dependencies) func refreshCachesNegativeAnswersUntilTheirTTLExpires() async { + let probeCalls = LockIsolated(0) + let now = LockIsolated(Date(timeIntervalSince1970: 1_000)) + await withDependencies { + $0.date = DateGenerator { now.value } + $0.shellClient = shellClient(probeCalls: probeCalls, available: [.codex]) + } operation: { + @Shared(.agentRuntimeAvailabilityProbeResults) var probeResults + $probeResults.withLock { $0 = [:] } + + await AgentRuntimeAvailabilityProbe.refresh() + #expect(probeCalls.value == 1) + + now.setValue(Date(timeIntervalSince1970: 1_299)) + await AgentRuntimeAvailabilityProbe.refresh() + #expect(probeCalls.value == 1) + + now.setValue(Date(timeIntervalSince1970: 1_300)) + await AgentRuntimeAvailabilityProbe.refresh() + #expect(probeCalls.value == 2) + #expect(probeResults[.codex]?.isAvailable == true) + #expect(probeResults[.codex]?.checkedAt == Date(timeIntervalSince1970: 1_000)) + #expect(probeResults[.claude]?.isAvailable == false) + #expect(probeResults[.claude]?.checkedAt == Date(timeIntervalSince1970: 1_300)) + } + } + + @Test(.dependencies) func refreshLeavesAnIncompleteBatchUnanswered() async { + let probeCalls = LockIsolated(0) + await withDependencies { + $0.date.now = Date(timeIntervalSince1970: 1_000) $0.shellClient = ShellClient( run: { _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) }, - runLoginImpl: { _, arguments, _, _ in - let command = arguments.joined(separator: " ") - if command.contains("'codex'") { - return ShellOutput(stdout: "/opt/homebrew/bin/codex", stderr: "", exitCode: 0) - } - // `command -v` answers "not found" with a non-zero exit, which the - // shell client surfaces as a thrown error. - throw ShellClientError(command: command, stdout: "", stderr: "", exitCode: 1) + runLoginImpl: { _, _, _, _ in + probeCalls.withValue { $0 += 1 } + return ShellOutput( + stdout: "\(Self.outputMarker):codex:1", + stderr: "", + exitCode: 0 + ) } ) } operation: { + @Shared(.agentRuntimeAvailabilityProbeResults) var probeResults + $probeResults.withLock { $0 = [:] } + await AgentRuntimeAvailabilityProbe.refresh() - @Shared(.agentRuntimeProbedAvailability) var probed - #expect(probed[.codex] == true) - #expect(probed[.claude] == false) + #expect(probeCalls.value == 1) + #expect(probeResults.isEmpty) } } - @Test(.dependencies) func refreshLeavesFailedProbesUnansweredAndSkipsPositives() async { + @Test(.dependencies) func refreshLeavesFailedBatchUnansweredAndSkipsPositives() async { let probeCalls = LockIsolated(0) await withDependencies { + $0.date.now = Date(timeIntervalSince1970: 1_000) $0.shellClient = ShellClient( run: { _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) }, runLoginImpl: { _, _, _, _ in probeCalls.withValue { $0 += 1 } - // A spawn-level failure (not a shell answer) must not masquerade as - // "not installed". + // A spawn-level failure must not masquerade as "not installed". throw CocoaError(.fileNoSuchFile) } ) } operation: { - @Shared(.agentRuntimeProbedAvailability) var probed - // A positive answer is final for the session: no re-probe. - $probed.withLock { $0[.codex] = true } + @Shared(.agentRuntimeAvailabilityProbeResults) var probeResults + $probeResults.withLock { + $0 = [ + .codex: AgentRuntimeAvailabilityProbeResult( + isAvailable: true, + checkedAt: Date(timeIntervalSince1970: 1_000) + ) + ] + } await AgentRuntimeAvailabilityProbe.refresh() - #expect(probeCalls.value == AgentProfileRuntime.allCases.count - 1) - #expect(probed[.codex] == true) - #expect(probed[.claude] == nil) + #expect(probeCalls.value == 1) + #expect(probeResults[.codex]?.isAvailable == true) + #expect(probeResults[.claude] == nil) } } + + private func shellClient( + probeCalls: LockIsolated, + available: Set + ) -> ShellClient { + ShellClient( + run: { _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) }, + runLoginImpl: { _, arguments, _, _ in + probeCalls.withValue { $0 += 1 } + let command = arguments.joined(separator: " ") + if command.contains(Self.outputMarker) { + return ShellOutput( + stdout: Self.probeOutput(for: command, available: available), + stderr: "", + exitCode: 0 + ) + } + + // Compatibility with the pre-batching implementation keeps this a + // behavioral RED test instead of a test that only fails to compile. + if let runtime = AgentProfileRuntime.allCases.first(where: { + command.contains("'\($0.rawValue)'") + }), available.contains(runtime) { + return ShellOutput(stdout: "/usr/local/bin/\(runtime.rawValue)", stderr: "", exitCode: 0) + } + throw ShellClientError(command: command, stdout: "", stderr: "", exitCode: 1) + } + ) + } + + nonisolated private static let outputMarker = "__PROWL_AGENT_RUNTIME_AVAILABILITY__" + + nonisolated private static func probeOutput( + for command: String, + available: Set + ) -> String { + AgentProfileRuntime.allCases.filter { runtime in + command.contains("\(outputMarker):\(runtime.rawValue):") + }.map { runtime in + "\(outputMarker):\(runtime.rawValue):\(available.contains(runtime) ? 1 : 0)" + }.joined(separator: "\n") + } }