From 79bec95b06a9b4c51f0aa3cfcc558f1075dec098 Mon Sep 17 00:00:00 2001 From: Test Date: Fri, 6 Mar 2026 17:02:14 -0800 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20eliminate=20daemon-env.json=20?= =?UTF-8?q?=E2=80=94=20derive=20env=20at=20spawn=20time=20(#118)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite buildSpawnEnv() to source ~/.zshenv at spawn time instead of loading stale JSON. Remove saveEnvironment(), loadSavedEnvironment(), and ENV_FILE. Simplify buildSpawnEnv signature (drop first param). Co-Authored-By: Claude Opus 4.6 --- src/adapters/claude-code.ts | 2 +- src/adapters/codex.ts | 2 +- src/adapters/opencode.ts | 2 +- src/adapters/pi-rust.ts | 2 +- src/adapters/pi.ts | 2 +- src/daemon/server.ts | 29 +++++------ src/utils/daemon-env.test.ts | 93 ++++++++++++++++++++++++++++++++++++ src/utils/daemon-env.ts | 63 +++++++++++------------- 8 files changed, 138 insertions(+), 57 deletions(-) create mode 100644 src/utils/daemon-env.test.ts diff --git a/src/adapters/claude-code.ts b/src/adapters/claude-code.ts index ca3a515..5f1ceae 100644 --- a/src/adapters/claude-code.ts +++ b/src/adapters/claude-code.ts @@ -470,7 +470,7 @@ export class ClaudeCodeAdapter implements AgentAdapter { args.push("-p", opts.prompt); } - const env = buildSpawnEnv(undefined, opts.env); + const env = buildSpawnEnv(opts.env); const cwd = opts.cwd || process.cwd(); // Write stdout to a log file so we can extract the session ID diff --git a/src/adapters/codex.ts b/src/adapters/codex.ts index 2623ef0..d713848 100644 --- a/src/adapters/codex.ts +++ b/src/adapters/codex.ts @@ -274,7 +274,7 @@ export class CodexAdapter implements AgentAdapter { args.push("--", opts.prompt); } - const env = buildSpawnEnv(undefined, opts.env); + const env = buildSpawnEnv(opts.env); await fs.mkdir(this.sessionsMetaDir, { recursive: true }); const logPath = path.join(this.sessionsMetaDir, `launch-${Date.now()}.log`); diff --git a/src/adapters/opencode.ts b/src/adapters/opencode.ts index 9f2e6d7..1094653 100644 --- a/src/adapters/opencode.ts +++ b/src/adapters/opencode.ts @@ -362,7 +362,7 @@ export class OpenCodeAdapter implements AgentAdapter { args.push("--", opts.prompt); } - const env = buildSpawnEnv(undefined, opts.env); + const env = buildSpawnEnv(opts.env); const cwd = opts.cwd || process.cwd(); await fs.mkdir(this.sessionsMetaDir, { recursive: true }); diff --git a/src/adapters/pi-rust.ts b/src/adapters/pi-rust.ts index 0178ff4..e70d0ae 100644 --- a/src/adapters/pi-rust.ts +++ b/src/adapters/pi-rust.ts @@ -370,7 +370,7 @@ export class PiRustAdapter implements AgentAdapter { args.unshift("--append-system-prompt", text); } - const env = buildSpawnEnv(undefined, opts.env); + const env = buildSpawnEnv(opts.env); const cwd = opts.cwd || process.cwd(); // Write stdout to a log file so we can extract the session ID diff --git a/src/adapters/pi.ts b/src/adapters/pi.ts index e256886..142a1d4 100644 --- a/src/adapters/pi.ts +++ b/src/adapters/pi.ts @@ -348,7 +348,7 @@ export class PiAdapter implements AgentAdapter { args.unshift("--model", opts.model); } - const env = buildSpawnEnv(undefined, opts.env); + const env = buildSpawnEnv(opts.env); const cwd = opts.cwd || process.cwd(); // Write stdout to a log file so we can extract the session ID diff --git a/src/daemon/server.ts b/src/daemon/server.ts index 04abf3c..2c14aed 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -14,7 +14,7 @@ import { PiAdapter } from "../adapters/pi.js"; import { PiRustAdapter } from "../adapters/pi-rust.js"; import type { AgentAdapter } from "../core/types.js"; import { migrateLocks } from "../migration/migrate-locks.js"; -import { saveEnvironment } from "../utils/daemon-env.js"; + import { clearBinaryCache } from "../utils/resolve-binary.js"; import { FuseEngine } from "./fuse-engine.js"; import { LockManager } from "./lock-manager.js"; @@ -77,21 +77,18 @@ export async function startDaemon(opts: DaemonStartOpts = {}): Promise<{ // 3. Clean stale socket file await fs.rm(sockPath, { force: true }); - // 4. Save shell environment for subprocess spawning (#42) - await saveEnvironment(configDir); - - // 5. Clear binary cache on restart (#41 — pick up moved/updated binaries) + // 4. Clear binary cache on restart (#41 — pick up moved/updated binaries) clearBinaryCache(); - // 6. Run migration (idempotent) + // 5. Run migration (idempotent) await migrateLocks(configDir).catch((err) => console.error("Migration warning:", err.message), ); - // 7. Load persisted state + // 6. Load persisted state const state = await StateManager.load(configDir); - // 8. Initialize subsystems + // 7. Initialize subsystems const adapters: Record = opts.adapters || { "claude-code": new ClaudeCodeAdapter(), codex: new CodexAdapter(), @@ -115,7 +112,7 @@ export async function startDaemon(opts: DaemonStartOpts = {}): Promise<{ metrics.recordFuseExpired(); }); - // 9. Initial PID liveness cleanup for daemon-launched sessions + // 8. Initial PID liveness cleanup for daemon-launched sessions // (replaces the old validateAllSessions — much simpler, only checks launches) const initialDead = sessionTracker.cleanupDeadLaunches(); if (initialDead.length > 0) { @@ -125,15 +122,15 @@ export async function startDaemon(opts: DaemonStartOpts = {}): Promise<{ ); } - // 10. Resume fuse timers + // 9. Resume fuse timers fuseEngine.resumeTimers(); - // 11. Start periodic PID liveness check for lock cleanup (30s interval) + // 10. Start periodic PID liveness check for lock cleanup (30s interval) sessionTracker.startLaunchCleanup((deadId) => { lockManager.autoUnlock(deadId); }); - // 12. Create request handler + // 11. Create request handler const handleRequest = createRequestHandler({ sessionTracker, lockManager, @@ -145,7 +142,7 @@ export async function startDaemon(opts: DaemonStartOpts = {}): Promise<{ sockPath, }); - // 13. Start Unix socket server + // 12. Start Unix socket server const socketServer = net.createServer((conn) => { let buffer = ""; conn.on("data", (chunk) => { @@ -184,10 +181,10 @@ export async function startDaemon(opts: DaemonStartOpts = {}): Promise<{ socketServer.on("error", reject); }); - // 14. Write PID file (after socket is listening — acts as "lock acquired") + // 13. Write PID file (after socket is listening — acts as "lock acquired") await fs.writeFile(pidFilePath, String(process.pid)); - // 15. Start HTTP metrics server + // 14. Start HTTP metrics server const metricsPort = opts.metricsPort ?? 9200; const httpServer = http.createServer((req, res) => { if (req.url === "/metrics" && req.method === "GET") { @@ -218,7 +215,7 @@ export async function startDaemon(opts: DaemonStartOpts = {}): Promise<{ await fs.rm(pidFilePath, { force: true }); }; - // 16. Signal handlers + // 15. Signal handlers for (const sig of ["SIGTERM", "SIGINT"] as const) { process.on(sig, async () => { console.log(`Received ${sig}, shutting down...`); diff --git a/src/utils/daemon-env.test.ts b/src/utils/daemon-env.test.ts new file mode 100644 index 0000000..dd47a30 --- /dev/null +++ b/src/utils/daemon-env.test.ts @@ -0,0 +1,93 @@ +import { execFileSync } from "node:child_process"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, type Mock, vi } from "vitest"; +import { buildSpawnEnv, getCommonBinDirs } from "./daemon-env.js"; + +vi.mock("node:child_process", () => ({ + execFileSync: vi.fn(), +})); + +const mockedExecFileSync = execFileSync as Mock; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("getCommonBinDirs", () => { + it("returns an array of common bin directories", () => { + const dirs = getCommonBinDirs(); + expect(dirs).toBeInstanceOf(Array); + expect(dirs.length).toBeGreaterThan(0); + expect(dirs).toContain("/usr/local/bin"); + expect(dirs).toContain("/usr/bin"); + expect(dirs).toContain("/opt/homebrew/bin"); + expect(dirs.some((d) => d.includes(".cargo/bin"))).toBe(true); + }); +}); + +describe("buildSpawnEnv", () => { + it("sources ~/.zshenv and returns env with augmented PATH", () => { + const fakeEnv = `HOME=/Users/test\0PATH=/usr/bin:/bin\0EDITOR=vim\0`; + mockedExecFileSync.mockReturnValue(fakeEnv); + + const env = buildSpawnEnv(); + + expect(mockedExecFileSync).toHaveBeenCalledWith( + "/bin/zsh", + ["-c", expect.stringContaining("source")], + expect.objectContaining({ encoding: "utf-8", timeout: 5000 }), + ); + expect(env.HOME).toBe("/Users/test"); + expect(env.EDITOR).toBe("vim"); + // PATH should include original dirs + common bin dirs + expect(env.PATH).toContain("/usr/bin"); + expect(env.PATH).toContain("/opt/homebrew/bin"); + }); + + it("falls back to process.env when sourcing fails", () => { + mockedExecFileSync.mockImplementation(() => { + throw new Error("zsh not found"); + }); + + const env = buildSpawnEnv(); + + // Should still have process.env PATH augmented with common dirs + expect(env.PATH).toBeDefined(); + expect(env.PATH).toContain("/opt/homebrew/bin"); + }); + + it("applies extra env overrides", () => { + const fakeEnv = `HOME=/Users/test\0PATH=/usr/bin\0`; + mockedExecFileSync.mockReturnValue(fakeEnv); + + const env = buildSpawnEnv({ MY_VAR: "hello", PATH: "/custom/bin" }); + + expect(env.MY_VAR).toBe("hello"); + // Extra env PATH override should win + expect(env.PATH).toBe("/custom/bin"); + }); + + it("does not duplicate existing PATH entries", () => { + const home = os.homedir(); + const cargoDir = path.join(home, ".cargo", "bin"); + const fakeEnv = `PATH=/usr/bin:${cargoDir}\0`; + mockedExecFileSync.mockReturnValue(fakeEnv); + + const env = buildSpawnEnv(); + + // Count occurrences of cargoDir in PATH + const pathDirs = (env.PATH ?? "").split(":"); + const count = pathDirs.filter((d) => d === cargoDir).length; + expect(count).toBe(1); + }); + + it("handles empty output from zsh gracefully", () => { + mockedExecFileSync.mockReturnValue(""); + + const env = buildSpawnEnv(); + + // Should fall back to process.env + expect(env.PATH).toBeDefined(); + }); +}); diff --git a/src/utils/daemon-env.ts b/src/utils/daemon-env.ts index 5f50577..78f9d2c 100644 --- a/src/utils/daemon-env.ts +++ b/src/utils/daemon-env.ts @@ -1,14 +1,12 @@ -import * as fs from "node:fs/promises"; +import { execFileSync } from "node:child_process"; import * as os from "node:os"; import * as path from "node:path"; -const ENV_FILE = "daemon-env.json"; - /** * Common bin directories that should be in PATH when spawning subprocesses. * These cover the usual locations for various package managers and tools. */ -function getCommonBinDirs(): string[] { +export function getCommonBinDirs(): string[] { const home = os.homedir(); return [ path.join(home, ".local", "bin"), @@ -25,53 +23,46 @@ function getCommonBinDirs(): string[] { } /** - * Save the current process environment to disk. - * Called at daemon start time when we still have the user's shell env. + * Source ~/.zshenv (or other shell init) and capture the resulting environment. + * Returns undefined if the file doesn't exist or sourcing fails. */ -export async function saveEnvironment(configDir: string): Promise { - const envPath = path.join(configDir, ENV_FILE); +function sourceZshEnv(): Record | undefined { + const zshenv = path.join(os.homedir(), ".zshenv"); try { - const tmpPath = `${envPath}.tmp`; - await fs.writeFile(tmpPath, JSON.stringify(process.env)); - await fs.rename(tmpPath, envPath); - } catch (err) { - console.error( - `Warning: could not save environment: ${(err as Error).message}`, + const output = execFileSync( + "/bin/zsh", + ["-c", `source "${zshenv}" 2>/dev/null; env -0`], + { + encoding: "utf-8", + timeout: 5000, + env: { HOME: os.homedir(), PATH: "/usr/bin:/bin" }, + }, ); - } -} - -/** - * Load the saved environment from disk. - * Returns undefined if the env file doesn't exist or is corrupt. - */ -export async function loadSavedEnvironment( - configDir: string, -): Promise | undefined> { - const envPath = path.join(configDir, ENV_FILE); - try { - const raw = await fs.readFile(envPath, "utf-8"); - const parsed = JSON.parse(raw); - if (typeof parsed === "object" && parsed !== null) { - return parsed as Record; + const env: Record = {}; + for (const entry of output.split("\0")) { + if (!entry) continue; + const idx = entry.indexOf("="); + if (idx > 0) { + env[entry.slice(0, idx)] = entry.slice(idx + 1); + } } + return Object.keys(env).length > 0 ? env : undefined; } catch { - // File doesn't exist or is corrupt + return undefined; } - return undefined; } /** * Build an augmented environment for spawning subprocesses. - * Merges the saved daemon env with common bin paths to ensure - * binaries are findable even when the daemon is detached from the shell. + * Sources ~/.zshenv at call time to get fresh env vars, then augments + * PATH with common bin directories. Falls back to process.env if + * sourcing fails. */ export function buildSpawnEnv( - savedEnv?: Record, extraEnv?: Record, ): Record { const base: Record = {}; - const source = savedEnv || (process.env as Record); + const source = sourceZshEnv() || (process.env as Record); // Copy source env for (const [k, v] of Object.entries(source)) { From 2db39ad4743538861723da84e8b9b83f5c70e1a9 Mon Sep 17 00:00:00 2001 From: "Doink (OpenClaw)" Date: Fri, 6 Mar 2026 17:34:11 -0800 Subject: [PATCH 2/2] fix: preserve process env when sourcing zsh --- src/utils/daemon-env.test.ts | 22 ++++++++++++++++++++++ src/utils/daemon-env.ts | 13 ++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/utils/daemon-env.test.ts b/src/utils/daemon-env.test.ts index dd47a30..e7b7be2 100644 --- a/src/utils/daemon-env.test.ts +++ b/src/utils/daemon-env.test.ts @@ -45,6 +45,28 @@ describe("buildSpawnEnv", () => { expect(env.PATH).toContain("/opt/homebrew/bin"); }); + it("preserves process.env-only vars when ~/.zshenv sourcing succeeds", () => { + const processOnlyKey = "AGENTCTL_PROCESS_ONLY_TEST_VAR"; + const previous = process.env[processOnlyKey]; + process.env[processOnlyKey] = "from-process"; + mockedExecFileSync.mockReturnValue( + `HOME=/Users/test\0PATH=/usr/bin:/bin\0`, + ); + + try { + const env = buildSpawnEnv(); + + expect(env[processOnlyKey]).toBe("from-process"); + expect(env.HOME).toBe("/Users/test"); + } finally { + if (previous === undefined) { + delete process.env[processOnlyKey]; + } else { + process.env[processOnlyKey] = previous; + } + } + }); + it("falls back to process.env when sourcing fails", () => { mockedExecFileSync.mockImplementation(() => { throw new Error("zsh not found"); diff --git a/src/utils/daemon-env.ts b/src/utils/daemon-env.ts index 78f9d2c..2884a5e 100644 --- a/src/utils/daemon-env.ts +++ b/src/utils/daemon-env.ts @@ -54,17 +54,20 @@ function sourceZshEnv(): Record | undefined { /** * Build an augmented environment for spawning subprocesses. - * Sources ~/.zshenv at call time to get fresh env vars, then augments - * PATH with common bin directories. Falls back to process.env if - * sourcing fails. + * Starts with process.env, overlays ~/.zshenv at call time when available, + * then augments PATH with common bin directories. */ export function buildSpawnEnv( extraEnv?: Record, ): Record { const base: Record = {}; - const source = sourceZshEnv() || (process.env as Record); + const zshEnv = sourceZshEnv(); + const source = { + ...(process.env as Record), + ...(zshEnv ?? {}), + }; - // Copy source env + // Copy merged env for (const [k, v] of Object.entries(source)) { if (v !== undefined) base[k] = v; }