fix(codex): strip the sequenced resume argv on local spawns too

The `pty:spawn` handler bound `env` to `baseEnv` before the sequenced-env
strip ran. `stripSequencedStartupResumeArgv` returns a new object rather
than mutating, so rebinding `baseEnv` left `env` pointing at the pre-strip
object. Only the `isDaemonHostSpawn` branch re-derived `env` from `baseEnv`,
so every local-provider spawn (LocalPtyProvider, or a provider that routes
fresh spawns there) reached the PTY with ORCA_SEQUENCED_STARTUP_COMMAND
still carrying `resume <id>`.

buildPtyHostEnv prefers that env var over the launch command and the
sequenced wrapper evals it, so the pane resumed under whichever account was
selected — the exact invariant this change exists to protect — while the
renderer showed "started fresh".

Declaring `env` after the strip removes the two-variable asymmetry: there is
no longer a point in the handler where the pre-strip value is reachable. The
runtime-controller path was already correct (it rebinds `env` itself).

The gap survived review because every sequenced assertion ran against the
daemon provider, which re-derives its env. Adds local-provider coverage for
both the stripped and the verified-provenance cases.
This commit is contained in:
Brennan Benson
2026-07-26 17:57:34 -07:00
parent db7e530e3a
commit 3dac5ca445
2 changed files with 39 additions and 1 deletions
+36
View File
@@ -2097,6 +2097,42 @@ describe('registerPtyHandlers', () => {
expect(daemonSpawn.mock.calls.at(-1)![0].env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe(sequenced)
})
posixOnlyIt(
'strips the sequenced startup command on the local-provider spawn path too',
async () => {
// Why: the daemon branch is the only one that re-derives the spawn env from
// baseEnv, so a local spawn is where a strip that lands on the wrong variable
// silently survives — and the wrapper would `eval` the resume anyway.
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
registerWithTrustedHomes([OTHER_HOME], OTHER_HOME)
await spawnCodexResume(ORIGIN_ROLLOUT, {
env: { ORCA_SEQUENCED_STARTUP_COMMAND: `codex 'resume' '${RESUME_SESSION_ID}'` }
})
const env = spawnMock.mock.calls.at(-1)![2].env as Record<string, string>
expect(env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe('codex')
}
)
posixOnlyIt(
'leaves the local-provider sequenced startup command alone when provenance is verified',
async () => {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
registerWithTrustedHomes([ORIGIN_HOME], OTHER_HOME)
const sequenced = `codex 'resume' '${RESUME_SESSION_ID}'`
await spawnCodexResume(ORIGIN_ROLLOUT, {
env: { ORCA_SEQUENCED_STARTUP_COMMAND: sequenced }
})
const env = spawnMock.mock.calls.at(-1)![2].env as Record<string, string>
expect(env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe(sequenced)
}
)
it('omits the notice on a reattach that never ran this launch command', async () => {
setupResumeDaemonProvider({ isReattach: true })
registerWithTrustedHomes([OTHER_HOME], OTHER_HOME)
+3 -1
View File
@@ -4472,7 +4472,6 @@ export function registerPtyHandlers(
// Why: SSH can strip ORCA_PANE_KEY when remote hooks are off; IPC tab/leaf metadata still names the pane.
const reservationPaneKey = metadataPaneKey ?? validatedPaneKey
const validatedLeafId = verifiedLeafId ?? metadataLeafId
let env: Record<string, string> | undefined = baseEnv
const effectiveShellOverride = terminalRuntimeOptions.shellOverride
const nativeWindowsConptySpawn = isNativeWindowsLocalPtySpawn({
connectionId: args.connectionId,
@@ -4498,6 +4497,9 @@ export function registerPtyHandlers(
const codexResumeHome = codexResumeLaunch.codexResumeHome
const launchCommand = codexResumeLaunch.command
baseEnv = stripSequencedStartupResumeArgv(baseEnv, codexResumeLaunch)
// Why: declared after the strip so a local-provider spawn cannot capture the
// pre-strip env — only the daemon branch below re-derives this from baseEnv.
let env: Record<string, string> | undefined = baseEnv
const selectedCodexHomePath = isDaemonHostSpawn
? getCompatibleSelectedCodexHomePath(
codexSelectionTarget,