Repository navigation
Conversation
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }), | ||
| ), | ||
| ); | ||
| const sessionDirenv = yield* prepareDirenvEnvironment( |
There was a problem hiding this comment.
🟠 High orchestration-v2/ProviderSessionManager.ts:1720
Concurrent handoff opens for the same thread launch with the wrong workspace environment, causing agents to use incorrect project tools and variables. prepareDirenvEnvironment stores the diff in a process-global per-thread map, but open is locked only by providerSessionId; a second open can overwrite that diff while the first is awaiting prepareMcpSession, so the first adapter.openSession reads the second workspace's environment. Serialize opens per thread or pass the prepared diff through the call chain instead of relying on shared mutable state.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 1720:
Concurrent handoff opens for the same thread launch with the wrong workspace environment, causing agents to use incorrect project tools and variables. `prepareDirenvEnvironment` stores the diff in a process-global per-thread map, but `open` is locked only by `providerSessionId`; a second open can overwrite that diff while the first is awaiting `prepareMcpSession`, so the first `adapter.openSession` reads the second workspace's environment. Serialize opens per thread or pass the prepared diff through the call chain instead of relying on shared mutable state.
| completedAt: now, | ||
| updatedAt: now, | ||
| }; | ||
| yield* eventSink.writeIfRunCurrent({ |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderTurnStartService.ts:591
A pending .envrc failure is consumed even when writeIfRunCurrent returns committed: false, so a run that loses its starting race produces no system_notice and the next run can start without the promised environment warning. Make failure consumption conditional on a committed write, or restore/defer the failure when the conditional write is rejected.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderTurnStartService.ts around line 591:
A pending `.envrc` failure is consumed even when `writeIfRunCurrent` returns `committed: false`, so a run that loses its starting race produces no `system_notice` and the next run can start without the promised environment warning. Make failure consumption conditional on a committed write, or restore/defer the failure when the conditional write is rejected.
| }), | ||
| ), | ||
| ); | ||
| const sessionDirenv = yield* prepareDirenvEnvironment( |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderSessionManager.ts:1720
Successful non-terminal sessions leave each thread's direnv diff in the global diffsByThread map after releaseEntry runs, so ordinary idle/manual releases and plain detaches accumulate potentially large environments for every closed thread until process shutdown. Ensure session release removes the thread's direnv state by calling DirenvEnvironment.setThreadDirenvEnvironment(threadId, undefined) for these paths, not only for terminal detaches and full shutdown.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 1720:
Successful non-terminal sessions leave each thread's direnv diff in the global `diffsByThread` map after `releaseEntry` runs, so ordinary idle/manual releases and plain detaches accumulate potentially large environments for every closed thread until process shutdown. Ensure session release removes the thread's direnv state by calling `DirenvEnvironment.setThreadDirenvEnvironment(threadId, undefined)` for these paths, not only for terminal detaches and full shutdown.
| const { processEnvironment, ...runtimeInput } = input; | ||
| const { processEnvironment, direnvEnvironment, ...runtimeInput } = input; | ||
| const resolved = yield* options.resolver | ||
| .resolve(options.settings, input.cwd, options.environment) |
There was a problem hiding this comment.
🟠 High Adapters/AcpRegistryAdapterV2.ts:152
A bare commandPath provided by the project's .envrc fails with runner_unavailable even when the executable is available on the direnv-provided PATH. resolve receives options.environment at line 152, before direnvEnvironment is applied to resolved.spawn.env, so executable lookup cannot see that PATH; resolve with the direnv-merged environment instead.
| .resolve(options.settings, input.cwd, options.environment) | |
| .resolve( | |
| options.settings, input.cwd, applyDirenvEnvironment(options.environment, direnvEnvironment), | |
| ) |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.ts around line 152:
A bare `commandPath` provided by the project's `.envrc` fails with `runner_unavailable` even when the executable is available on the direnv-provided `PATH`. `resolve` receives `options.environment` at line 152, before `direnvEnvironment` is applied to `resolved.spawn.env`, so executable lookup cannot see that `PATH`; resolve with the direnv-merged environment instead.
| // direnv diffs against the environment it is given, so a staleness | ||
| // check must see exactly what the previous load produced. | ||
| ...(previous | ||
| ? { env: applyDirenvEnvironment(process.env, previous), extendEnv: false } |
There was a problem hiding this comment.
🟡 Medium provider/DirenvEnvironment.ts:152
The staleness check returns None instead of reloading when the previously loaded .envrc unset PATH and direnv is not found in Node's fallback /usr/bin:/bin path. Because extendEnv: false passes that modified PATH to process spawning, the session manager treats the existing diff as gone, repeatedly releases and reopens the idle provider session, and only succeeds again from the server environment. Ensure the direnv executable is resolved using the server's environment while preserving the previous project environment for the check.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/DirenvEnvironment.ts around line 152:
The staleness check returns `None` instead of reloading when the previously loaded `.envrc` unset `PATH` and `direnv` is not found in Node's fallback `/usr/bin:/bin` path. Because `extendEnv: false` passes that modified `PATH` to process spawning, the session manager treats the existing diff as gone, repeatedly releases and reopens the idle provider session, and only succeeds again from the server environment. Ensure the `direnv` executable is resolved using the server's environment while preserving the previous project environment for the check.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a substantial default-on direnv integration that executes approved project environment logic and changes provider launch environments, session restarts, RPC authorization, and web/mobile behavior. Open findings identify concrete concurrency, PATH resolution, lifecycle cleanup, and failure-reporting risks in the new runtime path. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Agents never run through a shell, so projects that provide their toolchain through direnv started without it. Carry pingdotgg/t3code#13755, which loads the approved .envrc for every provider session and offers "Allow .envrc" on a blocked one. #13755 is the orchestration v2 version of the change, which is what the preview builds used here run. It targets the v2 branch, so its diff is applied to the same 0.0.43 preview, keeping the other carried patches unchanged. The only adaptation is in the Codex adapter, where the preview does not yet destructure runtimePolicy from the session input. Cursor runs in-process through its SDK and does not get the environment.
Agents never run through a shell, so projects that provide their toolchain through direnv started without it. Carry pingdotgg/t3code#13755, which loads the approved .envrc for every provider session and offers "Allow .envrc" on a blocked one. #13755 is the orchestration v2 version of the change, which is what the preview builds used here run. It targets the v2 branch, so its diff is applied to the same 0.0.43 preview, keeping the other carried patches unchanged. The only adaptation is in the Codex adapter, where the preview does not yet destructure runtimePolicy from the session input. Cursor runs in-process through its SDK and does not get the environment.
317f019 to
861ed27
Compare
Projects that provide their toolchain through direnv (Nix dev shells, devenv, mise) only worked in integrated terminals: agents never go through a shell, so they ran without the project's tools. Before a provider process starts, ProviderSessionManagerV2 loads the `.envrc` governing the session's cwd, as a shell with the direnv hook would, and the Codex, Claude, OpenCode, Pi, Grok, ACP registry and Antigravity adapters apply it to their launch environment. Trust stays with direnv: only `.envrc` files approved with `direnv allow` are loaded. A blocked or failing one becomes a system notice on the run and the agent starts without it. Cursor runs in-process through its SDK, which takes no environment, so it is left out. Every turn reopens its session, so that is where the shell hook's staleness check runs: direnv compares the files it watches against the environment the process started with, and when the user allowed or edited the `.envrc`, or `flake.lock` changed, an idle session the thread owns alone is released and reopened with the new environment. The native thread resumes as it does after an idle release. A busy or shared session keeps its environment until it idles. A blocked `.envrc` notice offers "Allow .envrc" on web and mobile, backed by a `projectEnvironment.allowDirenv` RPC that resolves the directory from the thread. The behavior is on by default and can be turned off per environment or project with "Load direnv environment".
861ed27 to
4139f88
Compare
|
Note This comment is posted by Julius' dot Closing under prior approval and verification. This adds default-on direnv loading, session restarts when the environment changes, and an Allow .envrc action. Neither this PR nor #13739 supplies maintainer approval of that direction and scope. The V1 timeline image and focused tests are useful, but the description still leaves V2 UI evidence pending. Please obtain scope approval and add before/after evidence for the V2 allow flow and changed settings on the affected clients, then request reconsideration. |
2 participants
What Changed
The v2 orchestration counterpart of #13739: provider sessions load the direnv environment (
.envrc) of their workspace, the way a shell with the direnv hook does. #13739 targetsmain(v1ProviderService); this applies the same behavior toProviderSessionManagerV2and the v2 adapters..envrcfiles approved withdirenv alloware loaded. A blocked, failing or timed-out one becomes asystem_noticeon the run, and the agent starts without it. The same failure is reported once per thread, not on every restart.ProviderSessionManagerV2.openloads the environment beforeadapter.openSession. Codex, Claude, OpenCode, Pi, Grok, the ACP registry and Antigravity apply it to their launch environment (ACP client terminals included). Variables set on the provider instance, such asCODEX_HOME, win over the.envrc.open, so a reused session runs direnv's own staleness check against the environment the process started with. If the user allowed or edited.envrc, or a watched file such asflake.lockchanged, an idle session the thread owns alone is released (environment_changed) and reopened with the new environment, and the native thread resumes as after an idle release. The reopen reuses the environment the check already evaluated. A busy or shared session keeps its environment until it idles.system_noticeturn items gain optionaldetailandactionfields. A blocked-.envrcnotice carries{ type: "direnv.allow" }, rendered as an Allow .envrc button on web and mobile. The button calls a newprojectEnvironment.allowDirenvRPC (orchestration:operate), which resolves the directory from the thread the same wayRuntimePolicyV2does, so a client cannot allow an arbitrary path.@cursor/sdk, whose local agent options take no environment. The user docs say so.Why
Projects that provide their toolchain through direnv (Nix dev shells, devenv, mise) only work in integrated terminals. Agents never go through a shell, so they start without the project's tools, compilers and pinned runtimes.
Checklist
Verification
ProviderSessionManagertests: a session opens with the loaded environment, a current environment keeps the session, a changed one restarts an idle session without a second evaluation, a vanished one restarts without it, and a failing.envrcis reported once.ProjectionStore.test.ts > projects only the latest failed root turn's limit into SQL and memory shellsalso fails on the base branch.v0.0.43-preview.20260925.2240builds as a packaged desktop app.Done with Claude Opus 5.5 in Claude Code.