mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-10 10:36:35 +00:00
fix(kanban): enrich LocalTransport subprocess env so kanban dispatcher can spawn workers
GUI-launched Scarf inherits macOS's launch-services PATH (`/usr/bin:/bin:/usr/sbin:/sbin`). Scarf itself finds `hermes` via absolute-path resolution in `HermesPathSet.hermesBinaryCandidates`, but when the kanban dispatcher (a child of Scarf) tries to spawn a worker, the worker inherits the same stripped PATH and Hermes's spawn machinery prints `\`hermes\` executable not found on PATH. Install Hermes Agent or activate its venv before running the kanban dispatcher.` — recording `outcome=spawn_failed` on the run. `LocalTransport` now mirrors `SSHTransport.environmentEnricher`: adds an `environmentEnricher: (() -> [String: String])?` static, and applies it to every subprocess. `scarfApp.swift` wires it at launch to the same `HermesFileService.enrichedEnvironment()` login-shell probe (`zsh -l -i` → `zsh -l` fallback) the SSH transport already uses, so subprocesses see `~/.local/bin`, `/opt/homebrew/bin`, and the user's credential env vars. Defense-in-depth: `subprocessEnvironment(forExecutable:)` always prepends the executable's own directory to PATH if missing — covers early-startup paths and test harnesses where the enricher hasn't been wired yet. Two new tests in `KanbanModelsTests` lock in: 1. The fallback (no enricher → executable's dir lands on PATH) 2. The enricher win for PATH + the empty-string-aware copy semantics for credential env vars (process env happens to set `ANTHROPIC_API_KEY=""` as an empty string in some environments; the enricher's non-empty value must still take effect) Release notes for v2.7.5 updated to document the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -60,6 +60,8 @@ A diagnostic round driving real tasks end-to-end exposed a connected bug pattern
|
|||||||
|
|
||||||
- **`hermes kanban assignees` empty-state was leaking into the picker.** The CLI prints a literal sentinel `(no assignees — create a profile with hermes -p <name> setup)` when the table is empty; the parser was tokenizing it on whitespace and offering `(no` as a profile in the menu. Parser now skips the sentinel, validates each candidate against `^[a-zA-Z0-9_-]+$`, and falls back cleanly to the active local profile when the table is empty.
|
- **`hermes kanban assignees` empty-state was leaking into the picker.** The CLI prints a literal sentinel `(no assignees — create a profile with hermes -p <name> setup)` when the table is empty; the parser was tokenizing it on whitespace and offering `(no` as a profile in the menu. Parser now skips the sentinel, validates each candidate against `^[a-zA-Z0-9_-]+$`, and falls back cleanly to the active local profile when the table is empty.
|
||||||
|
|
||||||
|
- **`spawn_failed` from "executable not found on PATH"** — most subtle of the lot. macOS GUI apps inherit a launch-services PATH (`/usr/bin:/bin:/usr/sbin:/sbin`) that doesn't include `~/.local/bin` (where pipx installs `hermes`) or `/opt/homebrew/bin`. Scarf was finding `hermes` for its own invocation via the absolute-path resolver in `HermesPathSet.hermesBinaryCandidates`, but when the dispatcher then spawned a worker process, that worker inherited Scarf's GUI PATH and couldn't find `hermes` by name — recording an `outcome=spawn_failed` run with the exact "executable not found on PATH" message. `LocalTransport` now grows an `environmentEnricher` static (mirroring `SSHTransport.environmentEnricher`) wired by `scarfApp.swift` to the same `HermesFileService.enrichedEnvironment()` login-shell probe the SSH transport uses. Every local subprocess Scarf spawns now sees the user's full PATH and credential env, so a spawned-from-Scarf hermes can spawn its children by name without reaching for absolute paths. Defense-in-depth: `subprocessEnvironment(forExecutable:)` also unconditionally prepends the executable's parent directory to PATH, so the fix works even if the enricher hasn't been wired (early startup, tests).
|
||||||
|
|
||||||
### Migrating from 2.7.1
|
### Migrating from 2.7.1
|
||||||
|
|
||||||
Sparkle will offer the update automatically. No config migration, no schema changes — `~/.hermes/kanban.db` is shared across all Hermes clients and Scarf only reads/writes through the documented CLI surface. Existing Scarf projects pick up the new project Kanban tab on first open; the tenant slug is minted lazily on first kanban interaction inside the project, so projects with no kanban activity stay byte-identical until the user opens the tab.
|
Sparkle will offer the update automatically. No config migration, no schema changes — `~/.hermes/kanban.db` is shared across all Hermes clients and Scarf only reads/writes through the documented CLI surface. Existing Scarf projects pick up the new project Kanban tab on first open; the tenant slug is minted lazily on first kanban interaction inside the project, so projects with no kanban activity stay byte-identical until the user opens the tab.
|
||||||
|
|||||||
@@ -25,6 +25,63 @@ public struct LocalTransport: ServerTransport {
|
|||||||
self.contextID = contextID
|
self.contextID = contextID
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MARK: - Environment enrichment
|
||||||
|
|
||||||
|
/// Injection point for local-subprocess environment enrichment.
|
||||||
|
/// Mirrors `SSHTransport.environmentEnricher` — the Mac app wires
|
||||||
|
/// this at launch to `HermesFileService.enrichedEnvironment()`,
|
||||||
|
/// which probes the user's login shell for PATH + credential env
|
||||||
|
/// vars. Without it, GUI-launched Scarf hands subprocesses a
|
||||||
|
/// stripped `/usr/bin:/bin:/usr/sbin:/sbin` PATH and child
|
||||||
|
/// `hermes` invocations from inside spawned workers fail with
|
||||||
|
/// `executable not found on PATH`.
|
||||||
|
///
|
||||||
|
/// Set once at app launch (startup is single-threaded). Tests may
|
||||||
|
/// inject a stub. iOS leaves this `nil` because LocalTransport
|
||||||
|
/// doesn't run subprocesses there.
|
||||||
|
nonisolated(unsafe) public static var environmentEnricher: (@Sendable () -> [String: String])?
|
||||||
|
|
||||||
|
/// Build the environment dict for a single subprocess. Process
|
||||||
|
/// env wins for keys it has; the enricher fills gaps + always
|
||||||
|
/// owns PATH (which is the whole point of running it). The
|
||||||
|
/// executable's parent directory is appended as a final fallback
|
||||||
|
/// so `runProcess` works even before the enricher has been wired
|
||||||
|
/// (during very early startup, in tests, etc.).
|
||||||
|
nonisolated static func subprocessEnvironment(forExecutable executable: String) -> [String: String] {
|
||||||
|
var env = ProcessInfo.processInfo.environment
|
||||||
|
if let enricher = Self.environmentEnricher {
|
||||||
|
let extra = enricher()
|
||||||
|
for (key, value) in extra where !value.isEmpty {
|
||||||
|
if key == "PATH" {
|
||||||
|
// Enricher always wins for PATH — that's the
|
||||||
|
// whole reason the enricher exists. The GUI
|
||||||
|
// process PATH is the broken thing we're
|
||||||
|
// replacing.
|
||||||
|
env[key] = value
|
||||||
|
} else if (env[key] ?? "").isEmpty {
|
||||||
|
// For other keys (credential env, locale, etc.)
|
||||||
|
// an explicit non-empty value in the GUI
|
||||||
|
// environment wins; an empty or absent value
|
||||||
|
// gets filled by the shell-harvested copy.
|
||||||
|
env[key] = value
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Always make sure the executable's own directory is on PATH —
|
||||||
|
// covers the case where the enricher hasn't been wired (tests,
|
||||||
|
// pre-launch helpers) but a child process still tries to spawn
|
||||||
|
// its sibling tools by bare name.
|
||||||
|
let dir = (executable as NSString).deletingLastPathComponent
|
||||||
|
if !dir.isEmpty {
|
||||||
|
let currentPATH = env["PATH"] ?? "/usr/bin:/bin:/usr/sbin:/sbin"
|
||||||
|
let parts = currentPATH.split(separator: ":").map(String.init)
|
||||||
|
if !parts.contains(dir) {
|
||||||
|
env["PATH"] = "\(dir):\(currentPATH)"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return env
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: - Files
|
// MARK: - Files
|
||||||
|
|
||||||
public func readFile(_ path: String) throws -> Data {
|
public func readFile(_ path: String) throws -> Data {
|
||||||
@@ -116,6 +173,17 @@ public struct LocalTransport: ServerTransport {
|
|||||||
let proc = Process()
|
let proc = Process()
|
||||||
proc.executableURL = URL(fileURLWithPath: executable)
|
proc.executableURL = URL(fileURLWithPath: executable)
|
||||||
proc.arguments = args
|
proc.arguments = args
|
||||||
|
// Hand subprocesses an environment that includes the user's
|
||||||
|
// login-shell PATH. Without this, `hermes` (pipx-installed at
|
||||||
|
// `~/.local/bin/hermes`) ends up running with macOS's GUI
|
||||||
|
// launch-services PATH (`/usr/bin:/bin:/usr/sbin:/sbin`), and
|
||||||
|
// when Hermes itself shells out to spawn a worker (e.g. the
|
||||||
|
// kanban dispatcher invoking `hermes` by name from a Python
|
||||||
|
// subprocess), it returns "executable not found on PATH" and
|
||||||
|
// the run records `outcome=spawn_failed`. Mirrors the SSH
|
||||||
|
// transport's environmentEnricher hook and is wired by
|
||||||
|
// `scarfApp.swift` at launch.
|
||||||
|
proc.environment = Self.subprocessEnvironment(forExecutable: executable)
|
||||||
let stdoutPipe = Pipe()
|
let stdoutPipe = Pipe()
|
||||||
let stderrPipe = Pipe()
|
let stderrPipe = Pipe()
|
||||||
let stdinPipe = Pipe()
|
let stdinPipe = Pipe()
|
||||||
|
|||||||
@@ -53,6 +53,49 @@ import Foundation
|
|||||||
// tokenized the sentinel and emitted `(no` as a profile name,
|
// tokenized the sentinel and emitted `(no` as a profile name,
|
||||||
// which surfaced in the Mac inspector's assignee dropdown.
|
// which surfaced in the Mac inspector's assignee dropdown.
|
||||||
|
|
||||||
|
// MARK: - LocalTransport subprocess environment
|
||||||
|
|
||||||
|
@Test func localTransportSubprocessEnvIncludesExecutableDir() {
|
||||||
|
// GUI-launched Scarf would otherwise hand subprocesses
|
||||||
|
// `/usr/bin:/bin:/usr/sbin:/sbin`, which doesn't include
|
||||||
|
// `~/.local/bin` — so when Hermes's kanban dispatcher
|
||||||
|
// spawns a worker by bare name, it fails with
|
||||||
|
// `executable not found on PATH` and the run records
|
||||||
|
// `outcome=spawn_failed`. Unblock by always making sure
|
||||||
|
// the directory of the executable we're launching is on
|
||||||
|
// PATH for the child.
|
||||||
|
let previous = LocalTransport.environmentEnricher
|
||||||
|
defer { LocalTransport.environmentEnricher = previous }
|
||||||
|
LocalTransport.environmentEnricher = nil
|
||||||
|
|
||||||
|
let env = LocalTransport.subprocessEnvironment(
|
||||||
|
forExecutable: "/Users/alanwizemann/.local/bin/hermes"
|
||||||
|
)
|
||||||
|
let path = env["PATH"] ?? ""
|
||||||
|
#expect(path.contains("/Users/alanwizemann/.local/bin"))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test func localTransportSubprocessEnvLetsEnricherWinPATH() {
|
||||||
|
let previous = LocalTransport.environmentEnricher
|
||||||
|
defer { LocalTransport.environmentEnricher = previous }
|
||||||
|
LocalTransport.environmentEnricher = {
|
||||||
|
// Simulate a login-shell probe returning a fuller PATH +
|
||||||
|
// some credential env. The enricher's PATH must override
|
||||||
|
// the GUI-process PATH.
|
||||||
|
return [
|
||||||
|
"PATH": "/opt/homebrew/bin:/usr/local/bin:/Users/me/.local/bin",
|
||||||
|
"ANTHROPIC_API_KEY": "sk-test-fake"
|
||||||
|
]
|
||||||
|
}
|
||||||
|
let env = LocalTransport.subprocessEnvironment(
|
||||||
|
forExecutable: "/usr/local/bin/hermes"
|
||||||
|
)
|
||||||
|
// Enricher's PATH wins (PATH is the whole point of running it).
|
||||||
|
#expect(env["PATH"]?.contains("/opt/homebrew/bin") == true)
|
||||||
|
// Credential env is forwarded (process env didn't have it).
|
||||||
|
#expect(env["ANTHROPIC_API_KEY"] == "sk-test-fake")
|
||||||
|
}
|
||||||
|
|
||||||
@Test func parseAssigneeTableSkipsNoAssigneesSentinel() {
|
@Test func parseAssigneeTableSkipsNoAssigneesSentinel() {
|
||||||
// Use the same parser via its public stand-in: round-trip
|
// Use the same parser via its public stand-in: round-trip
|
||||||
// through a fixture that decodes via JSON would skip the
|
// through a fixture that decodes via JSON would skip the
|
||||||
|
|||||||
@@ -56,6 +56,17 @@ struct ScarfApp: App {
|
|||||||
// owns the agent there.
|
// owns the agent there.
|
||||||
SSHTransport.environmentEnricher = { HermesFileService.enrichedEnvironment() }
|
SSHTransport.environmentEnricher = { HermesFileService.enrichedEnvironment() }
|
||||||
|
|
||||||
|
// Same enrichment for LocalTransport. Without this, GUI-launched
|
||||||
|
// Scarf hands every local subprocess (hermes acp, hermes kanban
|
||||||
|
// dispatch, sqlite3, etc.) macOS's stripped launch-services PATH
|
||||||
|
// — `/usr/bin:/bin:/usr/sbin:/sbin` — and child invocations
|
||||||
|
// (notably the kanban dispatcher's `hermes` worker spawn) fail
|
||||||
|
// with `executable not found on PATH`, recording an
|
||||||
|
// `outcome=spawn_failed` run on the task. The login-shell probe
|
||||||
|
// populates PATH with `~/.local/bin`, Homebrew, etc., matching
|
||||||
|
// what a Terminal session sees.
|
||||||
|
LocalTransport.environmentEnricher = { HermesFileService.enrichedEnvironment() }
|
||||||
|
|
||||||
// Warm up the login-shell env probe off-main at launch. Without
|
// Warm up the login-shell env probe off-main at launch. Without
|
||||||
// this, the first MainActor caller (chat preflight, OAuth flow,
|
// this, the first MainActor caller (chat preflight, OAuth flow,
|
||||||
// signal-cli detect, etc.) blocks for 5-8 seconds while
|
// signal-cli detect, etc.) blocks for 5-8 seconds while
|
||||||
|
|||||||
Reference in New Issue
Block a user