From 64345aa061677601004d8f73e5cbd58239b607cd Mon Sep 17 00:00:00 2001 From: Ecko95 <8972676+Ecko95@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:33:08 +0200 Subject: [PATCH 1/2] feat(provider): propagate direnv env to agent sessions Resolve the project's direnv environment (`direnv export json`) and merge it into each provider session's child env (Claude/Codex/Cursor/OpenCode), below the session-injected vars so GITS_PORT (#124) and the git-shim confinement vars always win. The resolver is async and failure-safe (direnv missing, un-allowed .envrc, timeout -> no-op), so session start never breaks or blocks. Fixes the git-shim PATH clobber: allocate() now prepends the shim dir onto the caller's direnv-merged PATH, so the shim stays first (confinement intact) while .envrc PATH_add / nix / mise toolchain dirs survive into the child. Integrated terminal left as-is (its PTY already sources rc files; the login-only /etc/profile gap is a documented follow-up). Closes #134 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/provider/GitShimManager.test.ts | 22 +++ apps/server/src/provider/GitShimManager.ts | 14 +- .../src/provider/Layers/ClaudeAdapter.ts | 23 +++- .../src/provider/Layers/CodexAdapter.test.ts | 101 ++++++++++++++ .../src/provider/Layers/CodexAdapter.ts | 37 +++-- .../src/provider/Layers/CursorAdapter.ts | 37 +++-- .../src/provider/Layers/OpenCodeAdapter.ts | 19 ++- apps/server/src/provider/direnvSessionEnv.ts | 23 ++++ apps/server/src/terminal/Layers/Manager.ts | 7 + packages/shared/src/shell.test.ts | 127 +++++++++++++++++- packages/shared/src/shell.ts | 104 +++++++++++++- 11 files changed, 479 insertions(+), 35 deletions(-) create mode 100644 apps/server/src/provider/direnvSessionEnv.ts diff --git a/apps/server/src/provider/GitShimManager.test.ts b/apps/server/src/provider/GitShimManager.test.ts index e0e418d77de..9b6b1d1546d 100644 --- a/apps/server/src/provider/GitShimManager.test.ts +++ b/apps/server/src/provider/GitShimManager.test.ts @@ -100,6 +100,28 @@ it.layer(testLayer)("GitShimManager", (it) => { }), ); + it.effect( + "prepends the shim dir to a caller-supplied basePath (#134 direnv PATH survival)", + () => + Effect.gen(function* () { + const mgr = yield* GitShimManager; + const config = yield* ServerConfig; + const path = yield* Path.Path; + + const sessionId = "test-alloc-basepath"; + const allowedRoot = config.baseDir; + const basePath = "/scratch/fake-toolchain:/usr/bin:/bin"; + const result = yield* mgr.allocate(sessionId, allowedRoot, basePath); + + const shimDir = path.join(config.baseDir, "gits-shims", sessionId); + if (result.vars["PATH"] !== `${shimDir}:${basePath}`) { + throw new Error(`Expected PATH '${shimDir}:${basePath}', got '${result.vars["PATH"]}'`); + } + + yield* mgr.release(sessionId); + }), + ); + it.effect("fails closed when the shim cannot be written", () => Effect.gen(function* () { const mgr = yield* GitShimManager; diff --git a/apps/server/src/provider/GitShimManager.ts b/apps/server/src/provider/GitShimManager.ts index f65b21d29b9..2d63009e6de 100644 --- a/apps/server/src/provider/GitShimManager.ts +++ b/apps/server/src/provider/GitShimManager.ts @@ -67,6 +67,11 @@ export interface GitShimManagerShape { * Create a shim dir for `sessionId`, write the `git` script that enforces * `allowedRoot` containment, and return the env vars to inject. * + * `basePath` is the PATH to prepend the shim dir onto — pass the caller's + * already direnv-merged PATH here so `.envrc` toolchain additions survive + * (#134); omitted callers fall back to the server's own `process.env.PATH` + * (unchanged prior behavior). + * * Callers are responsible for calling `release(sessionId)` when the session * ends. Codex/Cursor/OpenCode adapters do this via a Scope finalizer; * ClaudeAdapter calls release explicitly in stopSessionInternal. @@ -74,6 +79,7 @@ export interface GitShimManagerShape { readonly allocate: ( sessionId: string, allowedRoot: string, + basePath?: string, ) => Effect.Effect; /** @@ -143,6 +149,7 @@ const makeGitShimManager: Effect.Effect< const allocate = ( sessionId: string, allowedRoot: string, + basePath?: string, ): Effect.Effect => Effect.gen(function* () { const shimDir = shimDirFor(sessionId); @@ -154,8 +161,11 @@ const makeGitShimManager: Effect.Effect< yield* fs.chmod(shimPath, 0o755); const vars: Record = { - // Prepend shim dir to PATH so bare `git` resolves to our script. - PATH: `${shimDir}:${process.env["PATH"] ?? "/usr/local/bin:/usr/bin:/bin"}`, + // Prepend shim dir to `basePath` (the caller's already direnv-merged + // PATH, when given) so bare `git` resolves to our script FIRST while + // `.envrc` toolchain additions survive (#134). Falls back to the + // server's own PATH when no basePath is supplied. + PATH: `${shimDir}:${basePath ?? process.env["PATH"] ?? "/usr/local/bin:/usr/bin:/bin"}`, GITS_REAL_GIT: realGit, GITS_ALLOWED_ROOT: allowedRoot, GITS_PROTECTED_BRANCHES: PROTECTED_BRANCHES, diff --git a/apps/server/src/provider/Layers/ClaudeAdapter.ts b/apps/server/src/provider/Layers/ClaudeAdapter.ts index 74bb11b833c..db9157668f4 100644 --- a/apps/server/src/provider/Layers/ClaudeAdapter.ts +++ b/apps/server/src/provider/Layers/ClaudeAdapter.ts @@ -94,6 +94,7 @@ import { import { type ClaudeAdapterShape } from "../Services/ClaudeAdapter.ts"; import { type EventNdjsonLogger, makeEventNdjsonLogger } from "./EventNdjsonLogger.ts"; import { type GitShimManagerShape } from "../GitShimManager.ts"; +import { resolveSessionEnvWithDirenv } from "../direnvSessionEnv.ts"; import { sessionPortEnv } from "../sessionPort.ts"; const encodeUnknownJsonStringExit = Schema.encodeUnknownExit(Schema.UnknownFromJsonString); const decodeUnknownJsonStringExit = Schema.decodeUnknownExit(Schema.UnknownFromJsonString); @@ -3006,12 +3007,25 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( // Cleanup happens in stopSessionInternal via gitShimManager.release(). // ponytail: shim denied/warn events log via Codex adapter's stderr pipeline; // Claude SDK stderr is not captured here — add stderr capture when needed. + // direnv (.envrc) delta for this session's cwd, layered under sessionChildEnv + // below so GITS_PORT / shim vars always win (#124 invariant). Failure-safe: + // any direnv error (not installed, un-allowed .envrc, timeout) collapses to {}. + // Resolved before git-shim allocation so the shim's PATH can prepend to the + // direnv-merged PATH (#134) instead of silently discarding it. + const claudeEnvironmentWithDirenv = yield* resolveSessionEnvWithDirenv( + input.cwd ?? process.cwd(), + claudeEnvironment, + ); const childEnv = sessionPortEnv(threadId); const sessionChildEnv = gitShimManager ? { ...childEnv, ...(yield* gitShimManager - .allocate(threadId, input.cwd ?? "") + .allocate( + threadId, + input.cwd ?? "", + claudeEnvironmentWithDirenv.PATH ?? process.env["PATH"], + ) .pipe( Effect.mapError((cause) => toProcessError(cause, "Failed to allocate git confinement shim.", threadId), @@ -3049,9 +3063,10 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* ( canUseTool, ...(visualPlanMcpServers ? { mcpServers: visualPlanMcpServers } : {}), // Merge per-session env vars (GITS_PORT, GITS_ALLOWED_ROOT, PATH prepend, etc.) - // into the claude env. claudeEnvironment is the instance-level base; child vars - // add the per-session policy. Neither object is mutated. - env: Object.assign({}, claudeEnvironment, sessionChildEnv), + // into the claude env. claudeEnvironmentWithDirenv is the instance-level base + // plus the project's direnv delta; child vars add the per-session policy and + // always win. Neither input object is mutated. + env: Object.assign({}, claudeEnvironmentWithDirenv, sessionChildEnv), ...(input.cwd ? { additionalDirectories: [input.cwd] } : {}), ...(Object.keys(extraArgs).length > 0 ? { extraArgs } : {}), }; diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index 89c759e87d9..fcf28f7fb72 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -37,6 +37,7 @@ import * as CodexErrors from "effect-codex-app-server/errors"; import { ServerConfig } from "../../config.ts"; import { ServerSettingsService } from "../../serverSettings.ts"; import { ProviderAdapterValidationError } from "../Errors.ts"; +import { GitShimManager, GitShimManagerLive } from "../GitShimManager.ts"; import type { CodexAdapterShape } from "../Services/CodexAdapter.ts"; import { ProviderSessionDirectory } from "../Services/ProviderSessionDirectory.ts"; import { sessionPortEnv } from "../sessionPort.ts"; @@ -1246,3 +1247,103 @@ it.effect("flushes managed native logs when the adapter layer shuts down", () => } }), ); + +it.effect( + "merges the direnv delta below GITS_PORT, and shim-first PATH survives direnv PATH_add (#134/#124)", + () => + Effect.gen(function* () { + // Real (trivial) executable named "direnv" on a scratch PATH, standing in for + // the real binary so `resolveDirenvEnvironment`'s production code path — real + // PATH lookup, real execFile — runs unmodified end to end. Its fake .envrc + // output deliberately also sets GITS_PORT, to prove the session value still wins, + // and a PATH pointing at a scratch toolchain dir, to prove a real `.envrc` + // `PATH_add` (nix/mise/asdf shim) survives the git-shim allocation (#134). + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-codex-adapter-direnv-")); + const binPath = path.join(tempDir, "direnv"); + const fakeToolchainDir = path.join(tempDir, "fake-toolchain"); + fs.mkdirSync(fakeToolchainDir); + fs.writeFileSync( + binPath, + "#!/bin/sh\n" + + "printf '%s' " + + `'{"DIRENV_TEST_VAR":"present","GITS_PORT":"6666-from-direnv","PATH":"${fakeToolchainDir}:/usr/bin:/bin"}'\n`, + ); + fs.chmodSync(binPath, 0o755); + const runtimeFactory = makeRuntimeFactory(); + const scope = yield* Scope.make("sequential"); + let scopeClosed = false; + + try { + // Real GitShimManager on its own scratch baseDir (independent of the + // adapter's own ServerConfig below) — exercises the actual allocate() + // basePath-threading fix, not a stand-in. + const gitShimLayer = GitShimManagerLive.pipe( + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-codex-direnv-shim-" }), + ), + Layer.provideMerge(NodeServices.layer), + ); + const gitShimContext = yield* Layer.buildWithScope(gitShimLayer, scope); + const gitShimManager = yield* Effect.service(GitShimManager).pipe( + Effect.provide(gitShimContext), + ); + const gitShimConfig = yield* Effect.service(ServerConfig).pipe( + Effect.provide(gitShimContext), + ); + + const layer = Layer.effect( + CodexAdapter, + Effect.gen(function* () { + const codexConfig = decodeCodexSettings({}); + return yield* makeCodexAdapter(codexConfig, { + makeRuntime: runtimeFactory.factory, + environment: { PATH: `${tempDir}:${process.env.PATH ?? ""}` }, + gitShimManager, + }); + }), + ).pipe( + Layer.provideMerge(ServerConfig.layerTest(process.cwd(), process.cwd())), + Layer.provideMerge(ServerSettingsService.layerTest()), + Layer.provideMerge(providerSessionDirectoryTestLayer), + Layer.provideMerge(NodeServices.layer), + ); + const context = yield* Layer.buildWithScope(layer, scope); + const adapter = yield* Effect.service(CodexAdapter).pipe(Effect.provide(context)); + + const threadId = asThreadId("sess-direnv-codex"); + yield* adapter.startSession({ + provider: ProviderDriverKind.make("codex"), + threadId, + runtimeMode: "full-access", + }); + + const runtime = runtimeFactory.lastRuntime; + assert.ok(runtime); + assert.equal(runtime.options.environment?.DIRENV_TEST_VAR, "present"); + assert.equal(runtime.options.environment?.GITS_PORT, sessionPortEnv(threadId).GITS_PORT); + + // PATH survival (#134): shim dir FIRST (confinement preserved), direnv's + // toolchain dir PRESENT (PATH_add / nix/mise/asdf shims not discarded). + const finalPath = runtime.options.environment?.PATH ?? ""; + const expectedShimDir = path.join(gitShimConfig.baseDir, "gits-shims", threadId); + const pathEntries = finalPath.split(":"); + assert.equal( + pathEntries[0], + expectedShimDir, + `expected shim dir '${expectedShimDir}' first in PATH, got: ${finalPath}`, + ); + assert.ok( + pathEntries.includes(fakeToolchainDir), + `expected direnv toolchain dir '${fakeToolchainDir}' in PATH, got: ${finalPath}`, + ); + + yield* Scope.close(scope, Exit.void); + scopeClosed = true; + } finally { + if (!scopeClosed) { + yield* Scope.close(scope, Exit.void); + } + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }), +); diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 735741836f0..5634391d044 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -67,6 +67,7 @@ import { } from "./CodexSessionRuntime.ts"; import { type EventNdjsonLogger, makeEventNdjsonLogger } from "./EventNdjsonLogger.ts"; import { type GitShimManagerShape } from "../GitShimManager.ts"; +import { resolveSessionEnvWithDirenv } from "../direnvSessionEnv.ts"; import { sessionPortEnv } from "../sessionPort.ts"; const isCodexAppServerProcessExitedError = Schema.is(CodexErrors.CodexAppServerProcessExitedError); const isCodexAppServerTransportError = Schema.is(CodexErrors.CodexAppServerTransportError); @@ -1421,29 +1422,41 @@ export const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( sessionScopeTransferred ? Effect.void : Scope.close(sessionScope, Exit.void), ); + // direnv (.envrc) delta for this session's cwd, layered under GITS_PORT/shimEnv + // below so they always win (#124 invariant). Failure-safe: any direnv error + // (not installed, un-allowed .envrc, timeout) collapses to {}. Resolved before + // git-shim allocation so the shim's PATH can prepend to the direnv-merged PATH + // (#134) instead of silently discarding it. + const baseEnvWithDirenv = yield* resolveSessionEnvWithDirenv( + sessionCwd, + options?.environment ?? {}, + options?.environment ?? process.env, + ); // Inject git confinement shim for this session. // CONFINEMENT LINE: only sessionEnv is modified; options.environment (the // instance-level env) is never mutated, and server process.env is untouched. // The shim dir is released when sessionScope closes (session teardown). const shimEnv = options?.gitShimManager - ? (yield* options.gitShimManager.allocate(input.threadId, sessionCwd).pipe( - Effect.mapError( - (cause) => - new ProviderAdapterProcessError({ - provider: PROVIDER, - threadId: input.threadId, - detail: cause.message, - cause, - }), - ), - )).vars + ? (yield* options.gitShimManager + .allocate(input.threadId, sessionCwd, baseEnvWithDirenv.PATH ?? process.env["PATH"]) + .pipe( + Effect.mapError( + (cause) => + new ProviderAdapterProcessError({ + provider: PROVIDER, + threadId: input.threadId, + detail: cause.message, + cause, + }), + ), + )).vars : {}; if (options?.gitShimManager && Object.keys(shimEnv).length > 0) { yield* Scope.addFinalizer(sessionScope, options.gitShimManager.release(input.threadId)); } const sessionEnv: NodeJS.ProcessEnv = Object.assign( {}, - options?.environment, + baseEnvWithDirenv, sessionPortEnv(input.threadId), shimEnv, ); diff --git a/apps/server/src/provider/Layers/CursorAdapter.ts b/apps/server/src/provider/Layers/CursorAdapter.ts index 05c81db688d..7b963cbb7ab 100644 --- a/apps/server/src/provider/Layers/CursorAdapter.ts +++ b/apps/server/src/provider/Layers/CursorAdapter.ts @@ -81,6 +81,7 @@ import { type CursorAdapterShape } from "../Services/CursorAdapter.ts"; import { resolveCursorAcpBaseModelId } from "./CursorProvider.ts"; import { type EventNdjsonLogger, makeEventNdjsonLogger } from "./EventNdjsonLogger.ts"; import { type GitShimManagerShape } from "../GitShimManager.ts"; +import { resolveSessionEnvWithDirenv } from "../direnvSessionEnv.ts"; import { sessionPortEnv } from "../sessionPort.ts"; const encodeUnknownJsonStringExit = Schema.encodeUnknownExit(Schema.UnknownFromJsonString); @@ -541,28 +542,40 @@ export function makeCursorAdapter( const visualPlanMcpToken = visualPlanMcpSvc ? yield* visualPlanMcpSvc.issueToken(input.threadId) : undefined; + // direnv (.envrc) delta for this session's cwd, layered under GITS_PORT/shim + // vars below so they always win (#124 invariant). Failure-safe: any direnv + // error (not installed, un-allowed .envrc, timeout) collapses to {}. Resolved + // before git-shim allocation so the shim's PATH can prepend to the + // direnv-merged PATH (#134) instead of silently discarding it. + const cursorBaseEnvWithDirenv = yield* resolveSessionEnvWithDirenv( + cwd, + options?.environment ?? {}, + options?.environment ?? process.env, + ); // Inject git confinement shim for this session. // CONFINEMENT LINE: only sessionEnv is modified; options.environment (the // instance-level env) is never mutated, and server process.env is untouched. const cursorShimEnv = options?.gitShimManager - ? (yield* options.gitShimManager.allocate(input.threadId, cwd).pipe( - Effect.mapError( - (cause) => - new ProviderAdapterProcessError({ - provider: PROVIDER, - threadId: input.threadId, - detail: cause.message, - cause, - }), - ), - )).vars + ? (yield* options.gitShimManager + .allocate(input.threadId, cwd, cursorBaseEnvWithDirenv.PATH ?? process.env["PATH"]) + .pipe( + Effect.mapError( + (cause) => + new ProviderAdapterProcessError({ + provider: PROVIDER, + threadId: input.threadId, + detail: cause.message, + cause, + }), + ), + )).vars : {}; if (options?.gitShimManager && Object.keys(cursorShimEnv).length > 0) { yield* Scope.addFinalizer(sessionScope, options.gitShimManager.release(input.threadId)); } const cursorSessionEnv: NodeJS.ProcessEnv = Object.assign( {}, - options?.environment, + cursorBaseEnvWithDirenv, sessionPortEnv(input.threadId), cursorShimEnv, ); diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 024a5e58d66..fd94e4e309c 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -28,6 +28,7 @@ import { resolveAttachmentPath } from "../../attachmentStore.ts"; import { ServerConfig } from "../../config.ts"; import { type EventNdjsonLogger, makeEventNdjsonLogger } from "./EventNdjsonLogger.ts"; import { type GitShimManagerShape } from "../GitShimManager.ts"; +import { resolveSessionEnvWithDirenv } from "../direnvSessionEnv.ts"; import { sessionPortEnv } from "../sessionPort.ts"; import { ProviderAdapterProcessError, @@ -1045,17 +1046,31 @@ export function makeOpenCodeAdapter( sessions.delete(input.threadId); } + // direnv (.envrc) delta for this session's cwd, layered under GITS_PORT/shim + // vars below so they always win (#124 invariant). Failure-safe: any direnv + // error (not installed, un-allowed .envrc, timeout) collapses to {}. Resolved + // before git-shim allocation so the shim's PATH can prepend to the + // direnv-merged PATH (#134) instead of silently discarding it. + const openCodeBaseEnvWithDirenv = yield* resolveSessionEnvWithDirenv( + directory, + options?.environment ?? {}, + options?.environment ?? process.env, + ); // Inject git confinement shim for this session. // CONFINEMENT LINE: only openCodeSessionEnv is modified; options.environment (the // instance-level env) is never mutated, and server process.env is untouched. const openCodeShimEnv = options?.gitShimManager ? (yield* options.gitShimManager - .allocate(input.threadId, directory) + .allocate( + input.threadId, + directory, + openCodeBaseEnvWithDirenv.PATH ?? process.env["PATH"], + ) .pipe(Effect.mapError((cause) => toProcessError(input.threadId, cause)))).vars : {}; const openCodeSessionEnv: NodeJS.ProcessEnv = Object.assign( {}, - options?.environment, + openCodeBaseEnvWithDirenv, sessionPortEnv(input.threadId), openCodeShimEnv, ); diff --git a/apps/server/src/provider/direnvSessionEnv.ts b/apps/server/src/provider/direnvSessionEnv.ts new file mode 100644 index 00000000000..e6f9a8e145d --- /dev/null +++ b/apps/server/src/provider/direnvSessionEnv.ts @@ -0,0 +1,23 @@ +import { applyDirenvDelta, resolveDirenvEnvironment } from "@t3tools/shared/shell"; +import * as Effect from "effect/Effect"; + +/** + * Resolves this session's `.envrc` delta (if any) via `resolveDirenvEnvironment` + * and layers it under `baseEnv` via `applyDirenvDelta`. Shared by the four + * provider adapters (Claude/Codex/Cursor/OpenCode) — identical logic at each + * call site modulo which env resolves the `direnv` binary itself (`execEnv`, + * defaults to `baseEnv`). + * + * `resolveDirenvEnvironment` is async (#134) so a slow/hung direnv invocation + * never blocks the event loop; `Effect.promise` is correct here because it + * never rejects (failure-safe by design — see shell.ts). + */ +export function resolveSessionEnvWithDirenv( + cwd: string, + baseEnv: NodeJS.ProcessEnv, + execEnv: NodeJS.ProcessEnv = baseEnv, +): Effect.Effect { + return Effect.promise(() => resolveDirenvEnvironment(cwd, execEnv)).pipe( + Effect.map((delta) => applyDirenvDelta(baseEnv, delta)), + ); +} diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index 53df280ef00..0ae89f67f4f 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -369,6 +369,13 @@ function shellCandidateFromCommand( if (platform !== "win32" && shellName === "zsh") { return { shell: command, args: ["-o", "nopromptsp"] }; } + // ponytail: #134 considered adding `-l` (login shell) here so /etc/profile-based + // managers (nix) also load. Deferred: direnv already works in this PTY via the + // interactive rc-file cd-hook (confirmed in the #134 investigation), and forcing + // every terminal session into login-shell semantics is a startup-behavior change + // for ALL sessions, not just the nix subset that would benefit — too risky as a + // one-liner. Upgrade path: scope it behind a platform/settings check if a real + // nix /etc/profile gap is reported, not a blanket flag here. return { shell: command }; } diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index 214c03a7e53..ab6df7d30d5 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -1,6 +1,11 @@ -import { describe, expect, it, vi } from "vitest"; +// @effect-diagnostics nodeBuiltinImport:off +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { + applyDirenvDelta, extractPathFromShellOutput, isCommandAvailable, listLoginShellCandidates, @@ -11,6 +16,7 @@ import { readPathFromLaunchctl, readPathFromLoginShell, resolveCommandPath, + resolveDirenvEnvironment, resolveKnownWindowsCliDirs, resolveWindowsEnvironment, } from "./shell.ts"; @@ -468,3 +474,122 @@ describe("resolveWindowsEnvironment", () => { expect(commandAvailable).toHaveBeenCalledTimes(1); }); }); + +// Creates a real (empty) executable file named "direnv" on a scratch PATH so +// `isCommandAvailable`'s real fs-based resolution finds it deterministically, +// without depending on direnv actually being installed on the host. The +// injected `execFile` stands in for what running it would print. +function withFakeDirenvOnPath(): { readonly env: NodeJS.ProcessEnv; readonly cleanup: () => void } { + const dir = mkdtempSync(join(tmpdir(), "gits-direnv-test-")); + const binPath = join(dir, "direnv"); + writeFileSync(binPath, "#!/bin/sh\nexit 0\n"); + chmodSync(binPath, 0o755); + return { env: { PATH: dir }, cleanup: () => rmSync(dir, { recursive: true, force: true }) }; +} + +describe("resolveDirenvEnvironment", () => { + const cleanups: Array<() => void> = []; + afterEach(() => { + while (cleanups.length > 0) { + cleanups.pop()?.(); + } + }); + + it("is a no-op on win32 regardless of direnv availability", async () => { + expect( + await resolveDirenvEnvironment("/tmp/project", { PATH: "/usr/bin" }, { platform: "win32" }), + ).toEqual({}); + }); + + it("returns {} when direnv is not on PATH", async () => { + expect( + await resolveDirenvEnvironment("/tmp/project", { PATH: "" }, { platform: "linux" }), + ).toEqual({}); + }); + + it("parses the direnv export json delta when direnv is available", async () => { + const fake = withFakeDirenvOnPath(); + cleanups.push(fake.cleanup); + const execFile = vi.fn(() => JSON.stringify({ FOO: "bar", BAZ: null })); + + expect( + await resolveDirenvEnvironment("/tmp/project", fake.env, { platform: "linux", execFile }), + ).toEqual({ FOO: "bar", BAZ: null }); + expect(execFile).toHaveBeenCalledWith("direnv", ["export", "json"], { + cwd: "/tmp/project", + env: fake.env, + encoding: "utf8", + timeout: 5000, + }); + }); + + it("returns {} when execFile throws (e.g. un-allowed .envrc or timeout)", async () => { + const fake = withFakeDirenvOnPath(); + cleanups.push(fake.cleanup); + const execFile = vi.fn(() => { + throw new Error("direnv: error .envrc is blocked"); + }); + + expect( + await resolveDirenvEnvironment("/tmp/project", fake.env, { platform: "linux", execFile }), + ).toEqual({}); + }); + + it("returns {} when execFile rejects (async timeout/error path)", async () => { + const fake = withFakeDirenvOnPath(); + cleanups.push(fake.cleanup); + const execFile = vi.fn(() => Promise.reject(new Error("direnv: error .envrc is blocked"))); + + expect( + await resolveDirenvEnvironment("/tmp/project", fake.env, { platform: "linux", execFile }), + ).toEqual({}); + }); + + it("returns {} on malformed JSON", async () => { + const fake = withFakeDirenvOnPath(); + cleanups.push(fake.cleanup); + const execFile = vi.fn(() => "not json"); + + expect( + await resolveDirenvEnvironment("/tmp/project", fake.env, { platform: "linux", execFile }), + ).toEqual({}); + }); +}); + +describe("applyDirenvDelta", () => { + it("sets string values and deletes null (unset) values", () => { + expect( + applyDirenvDelta({ HOME: "/home/x", STALE: "old" }, { FOO: "bar", STALE: null }), + ).toEqual({ HOME: "/home/x", FOO: "bar" }); + }); + + it("never mutates the base object", () => { + const base = { HOME: "/home/x" }; + applyDirenvDelta(base, { FOO: "bar" }); + expect(base).toEqual({ HOME: "/home/x" }); + }); + + it("carries a PATH value from the delta through to the merged env (#134)", () => { + expect( + applyDirenvDelta( + { PATH: "/usr/local/bin:/usr/bin" }, + { PATH: "/tmp/fake-toolchain:/usr/local/bin:/usr/bin" }, + ), + ).toEqual({ PATH: "/tmp/fake-toolchain:/usr/local/bin:/usr/bin" }); + }); + + it("session-injected vars applied after the delta always win (GITS_PORT survival, #124)", () => { + const instanceEnv = { HOME: "/home/x" }; + // A hostile/coincidental .envrc that tries to set GITS_PORT itself. + const direnvDelta = { GITS_PORT: "6666", PROJECT_VAR: "from-envrc" }; + const sessionVars = { GITS_PORT: "5173" }; + + const merged = Object.assign({}, applyDirenvDelta(instanceEnv, direnvDelta), sessionVars); + + expect(merged).toEqual({ + HOME: "/home/x", + PROJECT_VAR: "from-envrc", + GITS_PORT: "5173", + }); + }); +}); diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index c88ccc10d2a..1d061254aa4 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -1,8 +1,11 @@ // @effect-diagnostics nodeBuiltinImport:off import * as NodeOS from "node:os"; -import { execFileSync } from "node:child_process"; +import { execFile, execFileSync } from "node:child_process"; import { accessSync, constants, statSync } from "node:fs"; import { extname, join } from "node:path"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); const PATH_CAPTURE_START = "__T3CODE_PATH_START__"; const PATH_CAPTURE_END = "__T3CODE_PATH_END__"; @@ -14,7 +17,7 @@ const WINDOWS_SHELL_CANDIDATES = ["pwsh.exe", "powershell.exe"] as const; type ExecFileSyncLike = ( file: string, args: ReadonlyArray, - options: { encoding: "utf8"; timeout: number }, + options: { encoding: "utf8"; timeout: number; cwd?: string; env?: NodeJS.ProcessEnv }, ) => string; export interface CommandAvailabilityOptions { @@ -504,3 +507,100 @@ export function resolveWindowsEnvironment( ? { ...baselinePatch, ...profiledPatch } : baselinePatch; } + +export type ExecFileAsyncLike = ( + file: string, + args: ReadonlyArray, + options: { encoding: "utf8"; timeout: number; cwd?: string; env?: NodeJS.ProcessEnv }, +) => Promise | string; + +export interface DirenvEnvironmentOptions { + readonly platform?: NodeJS.Platform; + readonly execFile?: ExecFileAsyncLike; +} + +async function defaultDirenvExecFile( + file: string, + args: ReadonlyArray, + options: { encoding: "utf8"; timeout: number; cwd?: string; env?: NodeJS.ProcessEnv }, +): Promise { + const { stdout } = await execFileAsync(file, [...args], options); + return stdout; +} + +/** + * Runs `direnv export json` in `cwd` and returns the env delta `.envrc` + * would apply: a string value means "set", `null` means "unset". + * + * direnv's normal hook fires on the shell's PROMPT_COMMAND — it does NOT + * fire for a one-shot `bash -lc`/`-ilc` spawn (verified empirically), so a + * provider session spawned directly (no shell in the tree) never picks up + * `.envrc` on its own. Asking direnv for its diff directly sidesteps that. + * + * Async so a slow/hung direnv invocation (5s timeout) never blocks the + * Node event loop — this runs once per provider session start on a + * concurrent multi-session server (#134). + * + * Failure-safe by design: an agent session must never fail to start because + * of direnv. direnv not installed, a non-zero exit (e.g. an un-allowed + * `.envrc`), a timeout, or a malformed reply all collapse to `{}`. This + * function never rejects. + */ +export async function resolveDirenvEnvironment( + cwd: string, + baseEnv: NodeJS.ProcessEnv, + opts: DirenvEnvironmentOptions = {}, +): Promise> { + const platform = opts.platform ?? process.platform; + if (platform === "win32") { + return {}; + } + if (!isCommandAvailable("direnv", { env: baseEnv, platform })) { + return {}; + } + + const execFile: ExecFileAsyncLike = opts.execFile ?? defaultDirenvExecFile; + try { + // direnv writes status/diagnostics to stderr; only stdout carries the JSON. + const stdout = await execFile("direnv", ["export", "json"], { + cwd, + env: baseEnv, + encoding: "utf8", + timeout: 5000, + }); + const parsed: unknown = JSON.parse(stdout); + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + return {}; + } + const delta: Record = {}; + for (const [name, value] of Object.entries(parsed as Record)) { + if (typeof value === "string" || value === null) { + delta[name] = value; + } + } + return delta; + } catch { + return {}; + } +} + +/** + * Applies a `resolveDirenvEnvironment` delta on top of `base`: string values + * are set, `null` values are unset (deleted). Returns a new object — `base` + * is never mutated, so callers can layer session-injected vars on top of the + * result and know they'll win regardless of what the delta contains. + */ +export function applyDirenvDelta( + base: NodeJS.ProcessEnv, + delta: Record, +): NodeJS.ProcessEnv { + const next: NodeJS.ProcessEnv = { ...base }; + for (const [name, value] of Object.entries(delta)) { + if (value === null) { + delete next[name]; + } else { + next[name] = value; + } + } + return next; +} From ec2f564bc79421492cd2ac8ff9df471cf19e8a63 Mon Sep 17 00:00:00 2001 From: Codex Peer Date: Fri, 10 Jul 2026 01:38:08 +0200 Subject: [PATCH 2/2] Apply Codex peer 233e7f27 changes --- .../src/terminal/Layers/Manager.test.ts | 32 ++++++++++++++++++- apps/server/src/terminal/Layers/Manager.ts | 14 ++++---- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/apps/server/src/terminal/Layers/Manager.test.ts b/apps/server/src/terminal/Layers/Manager.test.ts index 4b3ae5bb0f4..63c21ea5809 100644 --- a/apps/server/src/terminal/Layers/Manager.test.ts +++ b/apps/server/src/terminal/Layers/Manager.test.ts @@ -1258,7 +1258,37 @@ it.layer( expect(spawnInput).toBeDefined(); if (!spawnInput) return; - expect(spawnInput.args).toEqual(["-o", "nopromptsp"]); + expect(spawnInput.args).toEqual(["-l", "-o", "nopromptsp"]); + }), + ); + + it.effect("starts bash as a login shell on POSIX and without login args on Windows", () => + Effect.gen(function* () { + const posixManager = yield* createManager(5, { + platform: "linux", + shellResolver: () => "/bin/bash", + }); + const windowsManager = yield* createManager(5, { + platform: "win32", + shellResolver: () => "bash", + env: { + ComSpec: "C:\\Windows\\System32\\cmd.exe", + PATH: "C:\\Windows\\System32", + SystemRoot: "C:\\Windows", + }, + }); + yield* posixManager.manager.open(openInput()); + yield* windowsManager.manager.open(openInput()); + + const posixSpawnInput = posixManager.ptyAdapter.spawnInputs[0]; + expect(posixSpawnInput).toBeDefined(); + if (!posixSpawnInput) return; + expect(posixSpawnInput.args).toEqual(["-l"]); + + const windowsSpawnInput = windowsManager.ptyAdapter.spawnInputs[0]; + expect(windowsSpawnInput).toBeDefined(); + if (!windowsSpawnInput) return; + expect(windowsSpawnInput.args).toBeUndefined(); }), ); diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index 0ae89f67f4f..86d38591fa4 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -367,15 +367,13 @@ function shellCandidateFromCommand( return { shell: command, args: ["-NoLogo"] }; } if (platform !== "win32" && shellName === "zsh") { - return { shell: command, args: ["-o", "nopromptsp"] }; + return { shell: command, args: ["-l", "-o", "nopromptsp"] }; } - // ponytail: #134 considered adding `-l` (login shell) here so /etc/profile-based - // managers (nix) also load. Deferred: direnv already works in this PTY via the - // interactive rc-file cd-hook (confirmed in the #134 investigation), and forcing - // every terminal session into login-shell semantics is a startup-behavior change - // for ALL sessions, not just the nix subset that would benefit — too risky as a - // one-liner. Upgrade path: scope it behind a platform/settings check if a real - // nix /etc/profile gap is reported, not a blanket flag here. + if (platform !== "win32" && shellName === "bash") { + return { shell: command, args: ["-l"] }; + } + // ponytail: #134 - keep bash/zsh login loading explicit and POSIX-only. + // ponytail: explicit mise/nix shellenv target detection still deferred. return { shell: command }; }