diff --git a/src/main/codex/codex-structured-real-cli-command-env.test.ts b/src/main/codex/codex-structured-real-cli-command-env.test.ts new file mode 100644 index 00000000000..401684247e2 --- /dev/null +++ b/src/main/codex/codex-structured-real-cli-command-env.test.ts @@ -0,0 +1,84 @@ +// The installed Codex under a user policy that keeps only core env vars: a user shell command +// (`thread/shellCommand`) must still see the chat's identity, on a new thread and on a cold resume. +// The model's exec tool reads the same `shell_environment_policy.set` in Codex's source. Opt-in +// (ORCA_REAL_CODEX_CLI_TEST=1); no account is used, nothing reaches a model. +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { resolveCodexCommand } from '../codex-cli/command' +import { openCodexAppServerConnection } from './codex-app-server-connection' +import { openCodexThread } from './codex-structured-thread-open' +import { codexStructuredPermissionPolicy } from './codex-structured-permission-policy' + +// The probe is a POSIX shell command. +const enabled = process.env.ORCA_REAL_CODEX_CLI_TEST === '1' && process.platform !== 'win32' +const SESSION_ID = 'codex_real_command_env' +const PROBE = 'printf "probe[%s]" "$ORCA_AGENT_SESSION_ID"' + +async function probeIdentity( + base: string, + thread: { resumeThreadId: string | null; commandEnv?: Record } +): Promise<{ threadId: string; seen: string }> { + let output = '' + let turnCompleted = false + const cwd = join(base, 'workspace') + const connection = await openCodexAppServerConnection( + { + command: resolveCodexCommand(), + args: ['app-server'], + cwd, + env: { HOME: base, CODEX_HOME: join(base, 'codex-home'), ORCA_AGENT_SESSION_ID: SESSION_ID } + }, + { + onNotification: (method, params) => { + output += JSON.stringify(params) + turnCompleted ||= method === 'turn/completed' + } + } + ) + try { + const opened = await openCodexThread( + connection, + { cwd, permissionPolicy: codexStructuredPermissionPolicy('ask'), ...thread }, + 20_000 + ) + await connection.request( + 'thread/shellCommand', + { threadId: opened.threadId, command: PROBE }, + { timeoutMs: 10_000 } + ) + // Until the turn completes Codex may not have saved the rollout the cold resume reads. + const deadline = Date.now() + 15_000 + while (!turnCompleted && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 100)) + } + const seen = output.match(/probe\[([^\]%]*)\]/) + return { threadId: opened.threadId, seen: seen?.[1] ?? '' } + } finally { + await connection.close() + } +} + +describe.skipIf(!enabled)('Codex real app-server command env', () => { + it('keeps the chat identity in commands under inherit = "core", on start and resume', async () => { + const base = mkdtempSync(join(tmpdir(), 'orca-real-codex-env-')) + try { + mkdirSync(join(base, 'workspace'), { recursive: true }) + mkdirSync(join(base, 'codex-home'), { recursive: true }) + writeFileSync( + join(base, 'codex-home', 'config.toml'), + '[shell_environment_policy]\ninherit = "core"\n' + ) + const commandEnv = { ORCA_AGENT_SESSION_ID: SESSION_ID } + // The policy really strips it: the app-server holds the id, its command does not. + expect((await probeIdentity(base, { resumeThreadId: null })).seen).toBe('') + const started = await probeIdentity(base, { resumeThreadId: null, commandEnv }) + expect(started.seen).toBe(SESSION_ID) + const resumed = await probeIdentity(base, { resumeThreadId: started.threadId, commandEnv }) + expect(resumed).toEqual({ threadId: started.threadId, seen: SESSION_ID }) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }, 120_000) +}) diff --git a/src/main/codex/codex-structured-session-acquire.ts b/src/main/codex/codex-structured-session-acquire.ts index 7d400dab145..88e30f4d162 100644 --- a/src/main/codex/codex-structured-session-acquire.ts +++ b/src/main/codex/codex-structured-session-acquire.ts @@ -24,6 +24,7 @@ import { } from './codex-structured-owner-identity' import { openCodexThread } from './codex-structured-thread-open' import { withCodexVisualsThreadConfig } from './codex-structured-visuals' +import { NATIVE_CHAT_SCRATCH_DIR_ENV } from '../native-chat/native-chat-visuals-delivery' import { closeCodexPublishedSession, handleCodexSessionExit @@ -193,7 +194,15 @@ export async function acquireCodexStructuredSession(input: { ...(deps.logger ? { logger: deps.logger } : {}) }) acquisitions.assertCurrent(sessionId, attempt) - const opened = await openCodexThread(connection, threadLaunch, deps.requestTimeoutMs) + const commandEnv = { + ...launch.commandEnv, + ...(launch.visuals ? { [NATIVE_CHAT_SCRATCH_DIR_ENV]: launch.visuals.folder } : {}) + } + const opened = await openCodexThread( + connection, + { ...threadLaunch, commandEnv }, + deps.requestTimeoutMs + ) acquisitions.assertCurrent(sessionId, attempt) primaryThreadId = opened.threadId const restoreAdmission = translator?.restoreThread(opened.threadId, opened.thread ?? {}) diff --git a/src/main/codex/codex-structured-session-adapter.test.ts b/src/main/codex/codex-structured-session-adapter.test.ts index 6ed9144c6e9..9ed195bfd24 100644 --- a/src/main/codex/codex-structured-session-adapter.test.ts +++ b/src/main/codex/codex-structured-session-adapter.test.ts @@ -171,7 +171,19 @@ describe('CodexStructuredSessionAdapter.acquire', () => { cwd: '/work/repo', approvalPolicy: 'on-request', sandbox: 'workspace-write', - approvalsReviewer: 'user' + approvalsReviewer: 'user', + // A user's `inherit = "core"` policy would strip these from every command the chat runs. + config: { + 'shell_environment_policy.set.ORCA_AGENT_SESSION_ID': 'session-1', + 'shell_environment_policy.set.ORCA_STRUCTURED_SESSION': '1', + 'shell_environment_policy.set.ORCA_CLI_COMMAND': expect.stringMatching( + /^[^:;]*[\\/]cli[\\/]bin[\\/]orca-dev$/ + ), + 'shell_environment_policy.set.ORCA_USER_DATA_PATH': expect.any(String), + 'shell_environment_policy.set.ORCA_ENVIRONMENT': '', + 'shell_environment_policy.set.ORCA_PAIRING_CODE': '', + 'shell_environment_policy.set.ORCA_REMOTE_PAIRING': '' + } } }) expect(acquisition.process).toEqual({ diff --git a/src/main/codex/codex-structured-session-state.ts b/src/main/codex/codex-structured-session-state.ts index 7a253003bd9..ae04e846c00 100644 --- a/src/main/codex/codex-structured-session-state.ts +++ b/src/main/codex/codex-structured-session-state.ts @@ -48,6 +48,8 @@ export type CodexStructuredLaunch = { /** The child's sealed environment, and the inherited keys it must not keep. */ env?: Record envToDelete?: readonly string[] + /** The session values each command must see, whatever the user's shell environment policy. */ + commandEnv?: Record /** This chat's scratch folder and visuals skill; absent when the chat has no visuals. */ visuals?: NativeChatVisualsLaunch } diff --git a/src/main/codex/codex-structured-thread-open.ts b/src/main/codex/codex-structured-thread-open.ts index c6e686869b7..ce53af7fed9 100644 --- a/src/main/codex/codex-structured-thread-open.ts +++ b/src/main/codex/codex-structured-thread-open.ts @@ -45,6 +45,17 @@ function isCodexNoRolloutError(error: unknown, threadId: string): boolean { ) } +/** + * Codex rebuilds each command's env from `shell_environment_policy`, so `inherit = "core"` drops + * every Orca key the app-server holds. Codex adds `set` keys after that filter and merges these + * per key, so the user's own policy stays as written. + */ +function shellEnvironmentSetConfig(env: Record | undefined): Record { + return Object.fromEntries( + Object.entries(env ?? {}).map(([key, value]) => [`shell_environment_policy.set.${key}`, value]) + ) +} + export async function openCodexThread( connection: Pick, launch: { @@ -58,14 +69,17 @@ export async function openCodexThread( model?: string /** Config overrides for this thread, on start and resume alike. */ threadConfig?: Record + /** Env every command the agent runs must see, whatever the user's shell environment policy. */ + commandEnv?: Record }, timeoutMs: number | undefined ): Promise { const resumeThreadId = launch.resumeThreadId + const config = { ...launch.threadConfig, ...shellEnvironmentSetConfig(launch.commandEnv) } const threadSettings = { cwd: launch.cwd, ...launch.permissionPolicy, - ...(launch.threadConfig ? { config: launch.threadConfig } : {}) + ...(Object.keys(config).length > 0 ? { config } : {}) } const startThread = (): Promise => connection.request( diff --git a/src/main/codex/codex-structured-visuals.test.ts b/src/main/codex/codex-structured-visuals.test.ts index 46b381c4ac7..b4be2830211 100644 --- a/src/main/codex/codex-structured-visuals.test.ts +++ b/src/main/codex/codex-structured-visuals.test.ts @@ -207,7 +207,10 @@ describe('a Codex chat acquisition with visuals', () => { expect(methods.indexOf('config/read')).toBeLessThan(methods.indexOf(open)) expect(connection.calls.find((call) => call.method === open)?.params).toMatchObject({ ...MANUAL, - config: { 'sandbox_workspace_write.writable_roots': ['/home/me/scratch', VISUALS.folder] } + config: { + 'sandbox_workspace_write.writable_roots': ['/home/me/scratch', VISUALS.folder], + [`shell_environment_policy.set.${NATIVE_CHAT_SCRATCH_DIR_ENV}`]: VISUALS.folder + } }) expect(connection.launch.env?.[NATIVE_CHAT_SCRATCH_DIR_ENV]).toBe(VISUALS.folder) } @@ -220,7 +223,11 @@ describe('a Codex chat acquisition with visuals', () => { const methods = connection.calls.map((call) => call.method) expect(methods[0]).toBe('thread/start') expect(methods).not.toContain('skills/extraRoots/set') - expect(connection.calls[0]?.params).not.toHaveProperty('config') + // Only the chat's identity overrides: no visuals root, and no inherited folder handed on. + const config = connection.calls[0]?.params?.config + const scratchDirOverride = `shell_environment_policy.set.${NATIVE_CHAT_SCRATCH_DIR_ENV}` + expect(config).not.toHaveProperty(['sandbox_workspace_write.writable_roots']) + expect(config).not.toHaveProperty([scratchDirOverride]) expect(connection.launch.env).not.toHaveProperty(NATIVE_CHAT_SCRATCH_DIR_ENV) }) }) diff --git a/src/main/native-chat/native-chat-inline-visuals-setting.test.ts b/src/main/native-chat/native-chat-inline-visuals-setting.test.ts index 38962a1602f..1a7f5e75416 100644 --- a/src/main/native-chat/native-chat-inline-visuals-setting.test.ts +++ b/src/main/native-chat/native-chat-inline-visuals-setting.test.ts @@ -129,9 +129,12 @@ describe('inline visuals launch preference', () => { expect(disabled.calls.slice(0, openIndex).map(({ method }) => method)).not.toContain( 'config/read' ) - expect( - disabled.calls.find(({ method }) => method === 'thread/start')?.params - ).not.toHaveProperty('config') + const disabledOpen = disabled.calls.find(({ method }) => method === 'thread/start') + const scratchDirOverride = `shell_environment_policy.set.${NATIVE_CHAT_SCRATCH_DIR_ENV}` + expect(disabledOpen?.params?.config).not.toHaveProperty([ + 'sandbox_workspace_write.writable_roots' + ]) + expect(disabledOpen?.params?.config).not.toHaveProperty([scratchDirOverride]) expect(existsSync(nativeChatScratchFolderFor(stateDirectory, 'disabled'))).toBe(false) expect(first.closed).toBe(false) expect(first.calls).toEqual(firstCalls) diff --git a/src/main/runtime/structured-agent-launch-composition.ts b/src/main/runtime/structured-agent-launch-composition.ts index e868f0f1033..cf69fe4eba0 100644 --- a/src/main/runtime/structured-agent-launch-composition.ts +++ b/src/main/runtime/structured-agent-launch-composition.ts @@ -65,8 +65,11 @@ export type StructuredLaunchRequest = { permissionMode?: AgentChatPermissionMode } +/** What the composition sets on a launch; an agent's part never does. */ +type ComposedLaunchFields = 'cwd' | 'env' | 'envToDelete' | 'commandEnv' + export type StructuredAgentLaunchPart = { - launch: Omit + launch: Omit /** The agent's environment, before the seal names the child. */ env: Record /** Inherited keys this agent's child must not keep; required, so an agent states even none. */ @@ -81,10 +84,12 @@ export type StructuredAgentLaunchPartResolver = ( identity: AgentSessionJournalIdentity ) => Promise> -export type ComposedStructuredLaunch = Omit & { +export type ComposedStructuredLaunch = Omit & { cwd: string env: Record envToDelete: string[] + /** See `SealedStructuredSessionChild`; absent from a read, which seals no child. */ + commandEnv?: Record } /** The child to spawn, sealed with the reservation's token. */ @@ -189,7 +194,13 @@ export function composeStructuredLaunch( inheritedEnvToDelete, ...(cliRuntimeCommand ? { cliRuntimeCommand } : {}) }) - return { ...launch, cwd, env: child.env, envToDelete: child.envToDelete } + return { + ...launch, + cwd, + env: child.env, + envToDelete: child.envToDelete, + commandEnv: child.commandEnv + } } } diff --git a/src/main/runtime/structured-session-child-env.ts b/src/main/runtime/structured-session-child-env.ts index f565a47886f..7ba6ebd5a71 100644 --- a/src/main/runtime/structured-session-child-env.ts +++ b/src/main/runtime/structured-session-child-env.ts @@ -34,7 +34,7 @@ * remote selectors a shell profile may export are set empty, which the CLI reads as unset: empty * rather than deleted, because the child is spawned over Orca's own env, which may carry them. * Explicit `--environment` / `--pairing-code` flags still work, and a person's terminal keeps - * its selectors. A login shell the agent itself starts can export them again. + * its selectors. A login shell the agent starts can export them again, unless its agent sets them. * * Deliberately NOT `ORCA_PANE_KEY`. Claude structured sessions run hooks, and a pane key in their * environment starts flowing into hook-emitted agent-status payloads and the hook-attestation, @@ -84,6 +84,13 @@ export type SealedStructuredSessionChild = { env: Record /** Removed from what the child inherits, after `env` is laid over it. */ envToDelete: string[] + /** + * The session values the seal wrote, for an agent that rebuilds each command's env from its own + * policy (Codex `shell_environment_policy`), which drops them otherwise. Never PATH: Codex keeps + * it under `inherit = "core"`, and a set PATH would freeze it over a login shell's PATH restore. + * Never the spawn token, which is the child's alone. + */ + commandEnv: Record } /** @@ -100,34 +107,29 @@ export function sealStructuredSessionChild(input: { cliRuntimeCommand?: string }): SealedStructuredSessionChild { const identity = childIdentity(input.sessionId, input.env) - const named = new Set([ - ORCA_AGENT_SESSION_ID_ENV, - ORCA_STRUCTURED_SESSION_ENV, - ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV, - ...(identity.workerHandle ? ['ORCA_TERMINAL_HANDLE'] : []) - ]) + const named = new Set([...Object.keys(identity.commandEnv), ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV]) const env = input.cliRuntimeCommand ? withCliRuntimeOnPath(input.cliRuntimeCommand, identity.env, { platform: process.platform }) : identity.env return { env: { ...env, [ORCA_AGENT_SESSION_SPAWN_TOKEN_ENV]: input.spawnToken }, - envToDelete: input.inheritedEnvToDelete.filter((key) => !named.has(key)) + envToDelete: input.inheritedEnvToDelete.filter((key) => !named.has(key)), + commandEnv: identity.commandEnv } } function childIdentity( sessionId: string, childEnv: Record -): { env: Record; workerHandle: string | null } { +): { env: Record; commandEnv: Record } { const identity = resolveStructuredWorkerIdentityForSession(sessionId, null) - const env: Record = { - ...childEnv, + const named: Record = { ...(identity ? { ORCA_TERMINAL_HANDLE: identity.handle } : {}), [ORCA_AGENT_SESSION_ID_ENV]: sessionId, [ORCA_STRUCTURED_SESSION_ENV]: '1' } - applyThisAppCli(env) - return { env, workerHandle: identity?.handle ?? null } + const env = { ...childEnv, ...named } + return { env, commandEnv: { ...named, ...applyThisAppCli(env) } } } const REMOTE_RUNTIME_SELECTOR_ENV_KEYS = [ @@ -137,31 +139,35 @@ const REMOTE_RUNTIME_SELECTOR_ENV_KEYS = [ ] as const /** - * A host with no app environment installed — a plain-Node fork, or a unit test — has no userData - * root to resolve, and inventing one would write a shim into the wrong directory. + * Returns the values it set, beside the PATH it prepends. A host with no app environment + * installed — a plain-Node fork, or a unit test — has no userData root to resolve, and inventing + * one would write a shim into the wrong directory. */ -function applyThisAppCli(env: Record): void { +function applyThisAppCli(env: Record): Record { delete env.ORCA_CLI_COMMAND if (!hasAppEnvironment()) { - return + return {} } const app = getAppEnvironment() const userDataPath = app.getPath('userData') const isPackaged = app.isPackaged() - env.ORCA_USER_DATA_PATH = userDataPath + const set: Record = { ORCA_USER_DATA_PATH: userDataPath } for (const key of REMOTE_RUNTIME_SELECTOR_ENV_KEYS) { - env[key] = '' + set[key] = '' } + Object.assign(env, set) const launcher = prependOrcaCliDirToChildPath(env, { isPackaged, userDataPath, resourcesPath: process.resourcesPath ?? null }) if (launcher) { + set.ORCA_CLI_COMMAND = launcher env.ORCA_CLI_COMMAND = launcher } else { console.warn( "[structured-session] This app's CLI launcher did not resolve; the session's child has no ORCA_CLI_COMMAND." ) } + return set }