From 14c2c485bec4220e04eebc27652e8e7f6290942c Mon Sep 17 00:00:00 2001 From: Harshit Singh Bhandari Date: Tue, 23 Jun 2026 02:28:12 +0530 Subject: [PATCH] fix(desktop): recover login-shell env so the daemon finds zellij/credentials (#389) * fix(desktop): recover login-shell env so the daemon finds zellij/credentials A Finder/Dock launch starts the app via launchd, not a login shell, so ~/.zprofile and ~/.zshrc are never sourced. The daemon then inherits launchd's minimal env (no /opt/homebrew/bin on PATH, no exported ANTHROPIC_API_KEY, etc.), cannot exec zellij/git, and its agents cannot authenticate. Launching from a terminal masked this because the shell had already populated the env, so it only reproduced on a real Finder/Dock launch. Resolve the login-shell environment once at startup ($SHELL -ilc "printf sentinel; env -0"), adopt it as the base for daemonEnv(), and force PATH from the shell with a static floor when the probe fails (timeout/non-zero exit). The probe never blocks startup (3s SIGKILL timeout, stdin closed) and degrades to the floor rather than erroring. Windows keeps its existing behavior: its env comes from the registry/session and is inherited by GUI-launched apps, so this bug does not exist there. Pure parse/merge logic lives in shared/shell-env.ts (no node:* imports, per the daemon-attach.ts convention) with unit tests; main.ts owns the real spawn. Co-Authored-By: Claude Opus 4.8 * chore: format with prettier [skip ci] --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: github-actions[bot] --- docs/daemon-environment.md | 197 ++++++++++++++++++++++++++ frontend/src/main.ts | 84 ++++++++++- frontend/src/shared/shell-env.test.ts | 126 ++++++++++++++++ frontend/src/shared/shell-env.ts | 95 +++++++++++++ 4 files changed, 500 insertions(+), 2 deletions(-) create mode 100644 docs/daemon-environment.md create mode 100644 frontend/src/shared/shell-env.test.ts create mode 100644 frontend/src/shared/shell-env.ts diff --git a/docs/daemon-environment.md b/docs/daemon-environment.md new file mode 100644 index 000000000..a2f11f2c5 --- /dev/null +++ b/docs/daemon-environment.md @@ -0,0 +1,197 @@ +# Daemon environment: the GUI-launch PATH/credentials problem + +Status: proposed +Scope: desktop (Electron) launch of the AO daemon on macOS (and any GUI-launched +desktop platform) + +## Summary + +When the desktop app is launched from Finder/Dock/Spotlight, the daemon it spawns +inherits a stunted environment (minimal `PATH`, no shell-exported credentials). +The daemon then cannot find `zellij`/`git`/the agent CLIs, and the agents it +launches cannot see API keys. The same app launched from a terminal works, +because a terminal-started process inherits the shell's fully-populated +environment. The fix is to resolve the user's login-shell environment once at +startup and use it as the base for the daemon's environment. + +## Problem statement + +The Electron supervisor spawns the Go daemon with the environment it forwards in +`daemonEnv()` (`frontend/src/main.ts`), which is essentially `...process.env` +plus AO's telemetry defaults. The daemon, in turn, is the parent of every agent +session (it execs `zellij`, which runs `claude`/`codex`, etc.), and the agent's +`PATH` is derived from the daemon's own `PATH` +(`runtimeEnv` -> `HookPATH(m.executable, os.Getenv, ...)` in +`backend/internal/session_manager/manager.go`). + +So whatever environment the daemon receives propagates to the entire stack: + +``` +launchd (or terminal) -> Electron main -> daemon -> zellij -> agent (claude/codex) +``` + +When that environment is impoverished, everything downstream breaks. + +### Observed symptoms + +All of these were traced to the same root cause: + +- Terminal pane stuck on "Terminal disconnected - reattaching...". +- Terminal pane showing "Terminal ended ... but the session is not marked + terminated yet." +- Sessions stuck `idle` + `is_terminated = 0` in the store, never reaped, and + therefore not restorable (`Restore` requires `IsTerminated`, otherwise + `ErrNotRestorable`). +- `zellij list-sessions` showing sessions as alive-but-unreachable or dead, + depending on which socket universe was inspected. + +The unifying cause: the running, GUI-launched daemon cannot execute +`/opt/homebrew/bin/zellij` (and friends), so its liveness probes error +(`ProbeFailed`, never `ProbeDead`, so the reaper never terminates the row) and +its terminal attaches cannot spawn `zellij attach`. + +## Root cause: GUI apps do not inherit the shell environment + +On macOS, a process's environment is inherited solely from its parent. The +parent differs by launch method: + +- **Terminal launch.** The terminal starts a login/interactive shell + (`zsh -l`). That shell sources `/etc/zprofile`, `~/.zprofile`, `~/.zshrc`, + etc. Those files are the only thing that sets the rich environment: + `eval "$(/opt/homebrew/bin/brew shellenv)"` adds `/opt/homebrew/bin` to + `PATH`; `export ANTHROPIC_API_KEY=...` exports credentials. Every process + started from that terminal inherits the result. The app works. + +- **Finder/Dock/Spotlight launch.** The app is started by **launchd**, not by a + shell. launchd hands the process a fixed, minimal environment + (`PATH=/usr/bin:/bin:/usr/sbin:/sbin`, `HOME`, `USER`, `TMPDIR`, little else). + No shell runs anywhere in the chain, so no rc/profile file is ever sourced. + The homebrew `PATH` and the exported credentials simply do not exist for the + app, and `daemonEnv()` faithfully forwards that minimal env down to the daemon. + +This is deliberate on Apple's part: GUI apps are decoupled from interactive shell +configuration on purpose (it can be slow, interactive, or machine-specific). The +old `~/.MacOSX/environment.plist` escape hatch was removed years ago. This is the +single most common macOS-Electron footgun; it is why packages like `fix-path` and +`shell-env` exist. + +### Why "just forward env" is correct in principle + +Forwarding the environment is not the bug. The daemon and agents genuinely need: + +- `PATH` to resolve `zellij`, `git`, `node`, and the agent CLIs; +- `HOME` for config/credentials (`~/.gitconfig`, `~/.claude`, `~/.codex`, ssh + keys); +- shell-exported credentials (`ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GH_TOKEN`, + ...); +- locale/proxy (`LANG`, `LC_*`, `HTTPS_PROXY`); +- AO's own vars (telemetry, `AO_DATA_DIR`, `AO_RUN_FILE`, session ids). + +The bug is the _source_ of what we forward: under a GUI launch, `process.env` is +launchd's minimal env, not the shell's. The fix is to forward a _good_ base env, +not to stop forwarding. + +## Proposed solution: resolve the login-shell environment + +Do not reconstruct the shell environment by hand. Run the user's login shell +once, ask it to print its environment, and adopt that as the base for +`daemonEnv()`. + +### The mechanism + +``` +zsh -ilc 'env -0' +``` + +- `-l` (login): source `/etc/zprofile` and `~/.zprofile` (where the homebrew + `PATH` line typically lives). +- `-i` (interactive): source `~/.zshrc` (where most `export` lines live). +- `-c 'env -0'`: run one command and exit. `env` dumps the environment the shell + built after sourcing all config; `-0` separates entries with NUL bytes instead + of newlines, so values containing newlines parse unambiguously. + +The output is a faithful snapshot of "what a terminal would see." Parse it back +into key/value pairs and merge it under the existing forwarded env so explicit +overrides still win: + +``` +finalEnv = { ...shellEnv, ...process.env, AO_*: defaults } +``` + +### Worked example + +GUI-launched daemon env (before): + +``` +PATH=/usr/bin:/bin:/usr/sbin:/sbin +HOME=/Users/ +``` + +After `zsh -ilc 'env -0'` resolution: + +``` +PATH=/opt/homebrew/bin:/opt/homebrew/sbin:/usr/bin:/bin:/usr/sbin:/sbin +HOME=/Users/ +ANTHROPIC_API_KEY=sk-ant-... +GH_TOKEN=ghp_... +LANG=en_US.UTF-8 +``` + +The daemon can now resolve `/opt/homebrew/bin/zellij`, and agents inherit the +credentials. + +### Implementation details + +Place the resolution in Electron's `daemonEnv()` (`frontend/src/main.ts`), the +parent that hands env to the daemon. + +- **Resolve once, cache.** Sourcing rc files can take 100ms to >1s + (nvm/pyenv/...). Do it a single time at startup; never per-session. +- **Pick the shell robustly.** Prefer `process.env.SHELL`; under launchd it may + be absent, so fall back to the user record + (`dscl . -read /Users/$USER UserShell`), then `/bin/zsh`. Do not hardcode zsh; + honor bash/fish. +- **Isolate the payload.** Interactive shells can print banners/motd/prompts to + stdout. Bracket the real output with a sentinel and read only after it: + `zsh -ilc 'echo __AO_ENV_START__; env -0'`. +- **No stdin, with a timeout.** Run with ` termination + (`ProbeFailed` never terminates, so a daemon that cannot run `zellij` strands + sessions). diff --git a/frontend/src/main.ts b/frontend/src/main.ts index b0ef62ab8..b77e61e81 100644 --- a/frontend/src/main.ts +++ b/frontend/src/main.ts @@ -28,6 +28,7 @@ import { resolveDaemonFromPort, resolveDaemonFromRunFile, } from "./shared/daemon-attach"; +import { buildDaemonEnv, resolveShellEnv, type ShellRunner } from "./shared/shell-env"; import { DEFAULT_POSTHOG_HOST, DEFAULT_POSTHOG_PROJECT_KEY } from "./shared/posthog-config"; import { buildTelemetryBootstrap } from "./shared/telemetry"; import { createBrowserViewHost, type BrowserViewHost } from "./main/browser-view-host"; @@ -220,9 +221,21 @@ function runFilePath(): string | null { return defaultRunFilePath(process.platform, process.env, os.homedir()); } -function daemonEnv(): NodeJS.ProcessEnv { +// How long to wait for the login shell to print its env before giving up. A +// misconfigured rc that blocks (or a slow nvm/pyenv chain) must not hang startup; +// the daemon then falls back to the static PATH floor. +const SHELL_ENV_TIMEOUT_MS = 3_000; + +// The login-shell env resolved once at startup (see docs/daemon-environment.md), +// or null when the probe failed/timed out. Read synchronously by daemonEnv(). +let cachedShellEnv: Record | null = null; +// Memoize the in-flight resolution so concurrent/repeat awaits are cheap. +let shellEnvPromise: Promise | null = null; + +// Telemetry defaults stamped on the daemon env on every platform; explicit env +// always wins. +function telemetryOverrides(): Record { return { - ...process.env, AO_TELEMETRY_EVENTS: process.env.AO_TELEMETRY_EVENTS ?? "on", AO_TELEMETRY_REMOTE: process.env.AO_TELEMETRY_REMOTE ?? "posthog", AO_TELEMETRY_POSTHOG_KEY: process.env.AO_TELEMETRY_POSTHOG_KEY ?? DEFAULT_POSTHOG_PROJECT_KEY, @@ -230,6 +243,68 @@ function daemonEnv(): NodeJS.ProcessEnv { }; } +// Run the user's login shell to dump its env. stdin is ignored so an rc that +// reads input hits EOF instead of hanging; stderr is ignored to drop banner +// noise. Never rejects: resolves null on spawn error, non-zero exit, or timeout +// (SIGKILLed), so the caller degrades to the static PATH floor. +const runLoginShell: ShellRunner = (shellPath, args) => + new Promise((resolve) => { + let settled = false; + const finish = (value: string | null) => { + if (settled) return; + settled = true; + resolve(value); + }; + let child: ReturnType; + try { + child = spawn(shellPath, args, { stdio: ["ignore", "pipe", "ignore"] }); + } catch { + finish(null); + return; + } + const timer = setTimeout(() => { + child.kill("SIGKILL"); + finish(null); + }, SHELL_ENV_TIMEOUT_MS); + let stdout = ""; + // stdout may be typed Readable | null under this stdio config; guard it. + child.stdout?.on("data", (chunk: Buffer) => { + stdout += chunk.toString("utf8"); + }); + child.once("error", () => { + clearTimeout(timer); + finish(null); + }); + child.once("exit", (code) => { + clearTimeout(timer); + finish(code === 0 ? stdout : null); + }); + }); + +// Resolve the login-shell env once and cache it. No-op on Windows (the launchd +// shell split does not apply; a static PATH floor suffices). Awaited at the +// daemon-spawn chokepoint so the cache is populated before the first spawn. +function ensureShellEnv(): Promise { + if (process.platform === "win32") return Promise.resolve(); + if (!shellEnvPromise) { + shellEnvPromise = resolveShellEnv(process.env, runLoginShell).then((resolved) => { + cachedShellEnv = resolved; + if (!resolved) { + console.error("AO: could not read the login-shell environment; falling back to a static PATH floor."); + } + }); + } + return shellEnvPromise; +} + +function daemonEnv(): NodeJS.ProcessEnv { + // Windows keeps the old behavior exactly: no shell probe, no unix PATH floor. + if (process.platform === "win32") { + return { ...process.env, ...telemetryOverrides() }; + } + return buildDaemonEnv(process.env, cachedShellEnv, telemetryOverrides()); +} + function pathKey(value: string): string { const resolved = path.resolve(value); return process.platform === "win32" ? resolved.toLowerCase() : resolved; @@ -358,6 +433,11 @@ async function startDaemonInner(startEpoch: number): Promise { return daemonStatus; } + // Single chokepoint: make sure the login-shell env is resolved before the + // daemon is spawned, so a Finder/Dock launch hands the daemon a real PATH and + // shell-exported credentials rather than launchd's minimal env. + await ensureShellEnv(); + const launch = resolveDaemonLaunch( process.env, app.isPackaged, diff --git a/frontend/src/shared/shell-env.test.ts b/frontend/src/shared/shell-env.test.ts new file mode 100644 index 000000000..f2dcb0186 --- /dev/null +++ b/frontend/src/shared/shell-env.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from "vitest"; +import { + buildDaemonEnv, + FALLBACK_PATH_DIRS, + parseEnvBlock, + resolveShellEnv, + resolveShellPath, + SHELL_ENV_SENTINEL, + type ShellRunner, + withFallbackPath, +} from "./shell-env"; + +describe("parseEnvBlock", () => { + it("parses NUL-separated records after the sentinel", () => { + const stdout = `${SHELL_ENV_SENTINEL}PATH=/opt/homebrew/bin:/usr/bin\0HOME=/Users/me\0`; + expect(parseEnvBlock(stdout)).toEqual({ + PATH: "/opt/homebrew/bin:/usr/bin", + HOME: "/Users/me", + }); + }); + + it("drops banner/prompt noise printed before the sentinel", () => { + const stdout = `motd line\nWelcome\n${SHELL_ENV_SENTINEL}FOO=bar\0`; + expect(parseEnvBlock(stdout)).toEqual({ FOO: "bar" }); + }); + + it("preserves a value containing a newline", () => { + const stdout = `${SHELL_ENV_SENTINEL}MULTI=line1\nline2\0NEXT=ok\0`; + expect(parseEnvBlock(stdout)).toEqual({ MULTI: "line1\nline2", NEXT: "ok" }); + }); + + it("skips records with no '=' or a leading '='", () => { + const stdout = `${SHELL_ENV_SENTINEL}NOEQUALS\0=leading\0GOOD=value\0`; + expect(parseEnvBlock(stdout)).toEqual({ GOOD: "value" }); + }); +}); + +describe("withFallbackPath", () => { + it("appends only floor dirs not already present, preserving the existing order", () => { + // /opt/homebrew/bin and /usr/bin are already present and stay put; the rest + // of the floor is appended in floor order, with no duplicates added. + const result = withFallbackPath("/opt/homebrew/bin:/custom/bin:/usr/bin"); + expect(result).toBe( + "/opt/homebrew/bin:/custom/bin:/usr/bin:/opt/homebrew/sbin:/usr/local/bin:/bin:/usr/sbin:/sbin", + ); + }); + + it("yields the full floor for undefined input", () => { + expect(withFallbackPath(undefined)).toBe(FALLBACK_PATH_DIRS.join(":")); + }); + + it("yields the full floor for empty input", () => { + expect(withFallbackPath("")).toBe(FALLBACK_PATH_DIRS.join(":")); + }); +}); + +describe("buildDaemonEnv", () => { + const minimalProcessEnv: NodeJS.ProcessEnv = { PATH: "/usr/bin:/bin" }; + + it("lets overrides win over both shell and process env", () => { + const env = buildDaemonEnv( + { ...minimalProcessEnv, AO_TELEMETRY_EVENTS: "off" }, + { PATH: "/opt/homebrew/bin", AO_TELEMETRY_EVENTS: "shell" }, + { AO_TELEMETRY_EVENTS: "on" }, + ); + expect(env.AO_TELEMETRY_EVENTS).toBe("on"); + }); + + it("keeps a credential present only in the shell env", () => { + const env = buildDaemonEnv(minimalProcessEnv, { PATH: "/opt/homebrew/bin", ANTHROPIC_API_KEY: "sk-ant" }, {}); + expect(env.ANTHROPIC_API_KEY).toBe("sk-ant"); + }); + + it("takes PATH from the shell env (with floor) over a minimal process PATH", () => { + const env = buildDaemonEnv(minimalProcessEnv, { PATH: "/opt/homebrew/bin:/usr/bin" }, {}); + expect(env.PATH).toBe("/opt/homebrew/bin:/usr/bin:/opt/homebrew/sbin:/usr/local/bin:/bin:/usr/sbin:/sbin"); + }); + + it("still produces a PATH containing the floor when shellEnv is null", () => { + const env = buildDaemonEnv(minimalProcessEnv, null, {}); + for (const dir of FALLBACK_PATH_DIRS) { + expect(env.PATH?.split(":")).toContain(dir); + } + }); +}); + +describe("resolveShellPath", () => { + it("returns $SHELL when set", () => { + expect(resolveShellPath({ SHELL: "/bin/bash" })).toBe("/bin/bash"); + }); + + it("falls back to /bin/zsh when unset", () => { + expect(resolveShellPath({})).toBe("/bin/zsh"); + }); + + it("falls back to /bin/zsh when blank", () => { + expect(resolveShellPath({ SHELL: " " })).toBe("/bin/zsh"); + }); +}); + +describe("resolveShellEnv", () => { + it("yields the parsed map on a successful probe", async () => { + const run: ShellRunner = async () => `${SHELL_ENV_SENTINEL}PATH=/opt/homebrew/bin\0FOO=bar\0`; + expect(await resolveShellEnv({ SHELL: "/bin/zsh" }, run)).toEqual({ + PATH: "/opt/homebrew/bin", + FOO: "bar", + }); + }); + + it("returns null when the runner returns null", async () => { + const run: ShellRunner = async () => null; + expect(await resolveShellEnv({}, run)).toBeNull(); + }); + + it("returns null when the runner throws", async () => { + const run: ShellRunner = async () => { + throw new Error("spawn failed"); + }; + expect(await resolveShellEnv({}, run)).toBeNull(); + }); + + it("returns null when the parsed env lacks PATH", async () => { + const run: ShellRunner = async () => `${SHELL_ENV_SENTINEL}FOO=bar\0`; + expect(await resolveShellEnv({}, run)).toBeNull(); + }); +}); diff --git a/frontend/src/shared/shell-env.ts b/frontend/src/shared/shell-env.ts new file mode 100644 index 000000000..768c670a5 --- /dev/null +++ b/frontend/src/shared/shell-env.ts @@ -0,0 +1,95 @@ +// Recovering the login-shell environment so a Finder/Dock launch (started by +// launchd, not a shell) gets the same PATH and exported credentials a terminal +// launch would. See docs/daemon-environment.md for the root cause. +// +// Kept pure and dependency-injected (no node:* or electron imports — the +// vite-plugin-electron-renderer polyfill breaks node:* under vitest, see +// daemon-attach.ts) so the parsing/merging logic is testable directly; the real +// shell spawn lives in main.ts and is injected as a ShellRunner. + +export const SHELL_ENV_SENTINEL = "__AO_SHELL_ENV__"; + +// PATH floor: dirs a working macOS/Linux box keeps tools in, appended when the +// shell probe fails so zellij/git/agents still resolve. +export const FALLBACK_PATH_DIRS = [ + "/opt/homebrew/bin", + "/opt/homebrew/sbin", + "/usr/local/bin", + "/usr/bin", + "/bin", + "/usr/sbin", + "/sbin", +]; + +// Ask the login shell (-l sources zprofile, -i sources zshrc) to print a +// sentinel then a NUL-separated env dump (-0 keeps values with newlines intact). +export function shellEnvArgs(): string[] { + return ["-ilc", `printf '%s' '${SHELL_ENV_SENTINEL}'; env -0`]; +} + +// Slice after the sentinel (drops banner/motd/prompt noise printed before it), +// split on NUL, split each record on the first '='. +export function parseEnvBlock(stdout: string): Record { + const at = stdout.lastIndexOf(SHELL_ENV_SENTINEL); + const block = at === -1 ? stdout : stdout.slice(at + SHELL_ENV_SENTINEL.length); + const out: Record = {}; + for (const rec of block.split("\0")) { + if (rec === "") continue; + const eq = rec.indexOf("="); + if (eq <= 0) continue; // skip records with no key or a leading '=' + out[rec.slice(0, eq)] = rec.slice(eq + 1); + } + return out; +} + +// Prefer $SHELL (the user's login shell); under launchd it may be absent, so +// fall back to /bin/zsh. +export function resolveShellPath(env: Record): string { + const shell = env.SHELL?.trim(); + return shell && shell.length > 0 ? shell : "/bin/zsh"; +} + +// Append any missing floor dirs to PATH, preserving the existing order/priority +// and de-duping. +export function withFallbackPath(currentPath: string | undefined): string { + const result = (currentPath ?? "").split(":").filter(Boolean); + const present = new Set(result); + for (const dir of FALLBACK_PATH_DIRS) { + if (!present.has(dir)) { + present.add(dir); + result.push(dir); + } + } + return result.join(":"); +} + +// Base = shell env, overlaid by processEnv so Electron/AO runtime vars win, then +// PATH forced to the shell's PATH (with floor), then explicit overrides. +export function buildDaemonEnv( + processEnv: NodeJS.ProcessEnv, + shellEnv: Record | null, + overrides: Record, +): NodeJS.ProcessEnv { + const merged: NodeJS.ProcessEnv = { ...(shellEnv ?? {}), ...processEnv }; + merged.PATH = withFallbackPath(shellEnv?.PATH ?? processEnv.PATH); + return { ...merged, ...overrides }; +} + +export type ShellRunner = (shellPath: string, args: string[]) => Promise; + +// Run the probe via an injected runner (main.ts supplies the real spawn). +// Returns null on any failure or if the result lacks PATH; the caller then falls +// back to the static floor. +export async function resolveShellEnv( + env: Record, + run: ShellRunner, +): Promise | null> { + try { + const stdout = await run(resolveShellPath(env), shellEnvArgs()); + if (stdout == null) return null; + const parsed = parseEnvBlock(stdout); + return parsed.PATH ? parsed : null; + } catch { + return null; + } +}