fix(codex): a Codex chat keeps its Orca identity when the user's Codex config strips command environment variables (#30406)

* fix(codex): a Codex chat's commands keep its Orca identity under a restricted shell environment policy

Codex rebuilds each command's environment from the user's shell_environment_policy, so
inherit = "core" dropped the chat id, marker, launcher and instance pin Orca gives the
app-server. orca orchestration then could not tell which chat was calling.

Every thread start and resume now sends those values as shell_environment_policy.set
overrides, which Codex applies after its filter and merges per key.

* fix(codex): accept the launch's read-only delete list

* test(codex): a chat without visuals gets no visuals config, identity overrides aside

* refactor(codex): a chat's command env is what the seal wrote, not a second key list

The seal now returns the session values it set (identity, a registered worker's own handle, the CLI launcher, the instance pin and the blanked remote selectors) as commandEnv, and the composed launch carries it. The Codex acquire sends that plus the chat's scratch folder from its own visuals. This deletes the hand-kept key list, the handle prefix test and the delete-list re-check, so a key later added to the seal reaches each command too, and an inherited worker handle the seal never named no longer does.

* test(codex): wait for the shell command's turn to complete before the cold resume
This commit is contained in:
Brennan Benson
2026-10-19 00:31:11 -07:00
committed by GitHub
parent 9ea5434371
commit add1eccae3
9 changed files with 178 additions and 30 deletions
@@ -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<string, string> }
): 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] ?? '<no output>' }
} 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)
})
@@ -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 ?? {})
@@ -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({
@@ -48,6 +48,8 @@ export type CodexStructuredLaunch = {
/** The child's sealed environment, and the inherited keys it must not keep. */
env?: Record<string, string>
envToDelete?: readonly string[]
/** The session values each command must see, whatever the user's shell environment policy. */
commandEnv?: Record<string, string>
/** This chat's scratch folder and visuals skill; absent when the chat has no visuals. */
visuals?: NativeChatVisualsLaunch
}
+15 -1
View File
@@ -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<string, string> | undefined): Record<string, string> {
return Object.fromEntries(
Object.entries(env ?? {}).map(([key, value]) => [`shell_environment_policy.set.${key}`, value])
)
}
export async function openCodexThread(
connection: Pick<CodexAppServerConnection, 'request'>,
launch: {
@@ -58,14 +69,17 @@ export async function openCodexThread(
model?: string
/** Config overrides for this thread, on start and resume alike. */
threadConfig?: Record<string, unknown>
/** Env every command the agent runs must see, whatever the user's shell environment policy. */
commandEnv?: Record<string, string>
},
timeoutMs: number | undefined
): Promise<CodexOpenedThread> {
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<unknown> =>
connection.request(
@@ -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)
})
})
@@ -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)
@@ -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<L> = {
launch: Omit<L, 'cwd' | 'env' | 'envToDelete'>
launch: Omit<L, ComposedLaunchFields>
/** The agent's environment, before the seal names the child. */
env: Record<string, string>
/** Inherited keys this agent's child must not keep; required, so an agent states even none. */
@@ -81,10 +84,12 @@ export type StructuredAgentLaunchPartResolver<L> = (
identity: AgentSessionJournalIdentity
) => Promise<StructuredAgentLaunchPart<L>>
export type ComposedStructuredLaunch<L> = Omit<L, 'cwd' | 'env' | 'envToDelete'> & {
export type ComposedStructuredLaunch<L> = Omit<L, ComposedLaunchFields> & {
cwd: string
env: Record<string, string>
envToDelete: string[]
/** See `SealedStructuredSessionChild`; absent from a read, which seals no child. */
commandEnv?: Record<string, string>
}
/** The child to spawn, sealed with the reservation's token. */
@@ -189,7 +194,13 @@ export function composeStructuredLaunch<L>(
inheritedEnvToDelete,
...(cliRuntimeCommand ? { cliRuntimeCommand } : {})
})
return { ...launch, cwd, env: child.env, envToDelete: child.envToDelete }
return {
...launch,
cwd,
env: child.env,
envToDelete: child.envToDelete,
commandEnv: child.commandEnv
}
}
}
@@ -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<string, string>
/** 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<string, string>
}
/**
@@ -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<string, string>
): { env: Record<string, string>; workerHandle: string | null } {
): { env: Record<string, string>; commandEnv: Record<string, string> } {
const identity = resolveStructuredWorkerIdentityForSession(sessionId, null)
const env: Record<string, string> = {
...childEnv,
const named: Record<string, string> = {
...(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<string, string>): void {
function applyThisAppCli(env: Record<string, string>): Record<string, string> {
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<string, string> = { 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
}