Repository navigation
fix(codex): pair structured launches with the CLI runtime - #32061
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title is concise, specific, and accurately describes the primary change: pairing structured Codex launches with the CLI runtime. |
| Docstring Coverage | |
Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 8 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The description is detailed and covers the change, motivation, linked issue, testing across platforms, visual proof, risks, and known limitations. It omits some template headings, including Why, AI Di… |
Full details: Description check
-
Explanation
The description is detailed and covers the change, motivation, linked issue, testing across platforms, visual proof, risks, and known limitations. It omits some template headings, including Why, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist, but the required technical information is largely present.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
nwparker
force-pushed
the
nwparker/codex-structured-cli-runtime
branch
from
October 23, 2026 03:38
2229d76 to
3306b2c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
c969f4b4-6c66-43cc-ba00-167e1a587147
📥 Commits
Reviewing files that changed from the base of the PR and between b7d5ad6 and 3306b2c88a07e42d398e328ec537e69261075cb1.
⛔ Files ignored due to path filters: 1
src/main/runtime/__snapshots__/structured-agent-launch-parity.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing: 7
src/main/claude/claude-structured-child-env.tssrc/main/claude/claude-structured-launch-resolution.test.tssrc/main/codex-cli/command.test.tssrc/main/codex/codex-structured-child-environment.test.tssrc/main/runtime/structured-session-child-env.tssrc/shared/cli-runtime-pairing-inputs.tssrc/shared/node-cli-command-resolution.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
Comment on lines
+37
to
+39
| for (const key of Object.keys(env)) { | ||
| if (key.toLowerCase() === 'path' && env[key] !== undefined) { | ||
| return key |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,110p' src/shared/cli-runtime-pairing-inputs.ts
sed -n '325,385p' src/shared/node-cli-command-resolution.ts
rg -n 'duplicate|Path:|PATH:|firstWindowsPathEnvKey|withCliRuntimeOnPath|sealStructuredSessionChild' src/main/codex-cli/command.test.ts src/main/runtime/structured-session-child-env.ts src/sharedRepository: stablyai/orca
Length of output: 41239
🌐 Web query:
Node.js 24 child_process spawn env Windows case insensitive duplicate PATH keys lexicographic sorted first only
💡 Result:
<source_evidence>
<source kind="find_in_page">
<title>node/lib/child_process.js at v24.0.0 · nodejs/node · GitHub</title>
<location>https://github.com/nodejs/node/blob/v24.0.0/lib/child_process.js</location>
<excerpt>citeturn2view0 [wordlim: 200] Crawled: today; Content type: text/html; Source: find({"ref_id":"turn1view1","pattern":"envPairs"}); Total lines: 3868
L3259: file = '/system/bin/sh';
L3260:
L3261: else
L3262:
L3263: file = '/bin/sh';
L3264:
L3265: args = ['-c', command];
L3266:
L3267: }
L3268:
L3269: }
L3270:
L3271: if (typeof options.argv0 === 'string') {
L3272:
L3273: ArrayPrototypeUnshift(args, options.argv0);
L3274:
L3275: } else {
L3276:
L3277: ArrayPrototypeUnshift(args, file);
L3278:
L3279: }
L3280:
L3281: const env = options.env || process.env;
L3282:
L3283: const envPairs = [];
L3284:
L3285: // process.env.NODE_V8_COVERAGE always propagates, making it possible to
L3286:
L3287: // collect coverage for programs that spawn with white-listed environment.
L3288:
L3289: copyProcessEnvToEnv(env, 'NODE_V8_COVERAGE', options.env);
L3290:
L3291: if (isZOS) {
L3292: // The following environment variables must always propagate if set.
L3293:
L3294: copyProcessEnvToEnv(env, '_BPXK_AUTOCVT', options.env);
L3295:
L3296: copyProcessEnvToEnv(env, '_CEE_RUNOPTS', options.env);
L3297:
L3298: copyProcessEnvToEnv(env, '_TAG_REDIR_ERR', options.env);
L3299:
L3300: copyProcessEnvToEnv(env, '_TAG_REDIR_IN', options.env);
L3301:
L3302: copyProcessEnvToEnv(env, '_TAG_REDIR_OUT', options.env);
L3303:
L3304: copyProcessEnvToEnv(env, 'STEPLIB', options.env);
L3305:
L3306: copyProcessEnvToEnv(env, 'LIBPATH', options.env);
L3307: copyProcessEnvToEnv(env, '_EDC_SIG_DFLT', options.env);
L3308:
L3309: copyProcessEnvToEnv(env, '_EDC_SUSV3', options.env);
L3310:
L3311: }
L3312:
L3313: let envKeys = [];
L3314:
L3315: // Prototype values are intentionally included.
L3316:
L3317: for (const key in env) {
L3318:
L3319: ArrayPrototypePush(envKeys, key);
L3320:
L3321: }
L3322:
L3323: if (process.platform === 'win32') {
L3324:
L3325: // On Windows env keys are case insensitive. Filter out duplicates,
L3326:
L3327: // keeping only the first one (in lexicographic order)
L3328:
L3329: const sawKey = new SafeSet();
L3330:
L3331: envKeys = ArrayPrototypeFilter(
L3332:
L3333: ArrayPrototypeSort(envKeys),
L3334: (key) => {
L3335:
L3336: const uppercaseKey = StringPrototypeToUpperCase(key);
L3337:
L3338: if (sawKey.has(uppercaseKey)) {
L3339:
L3340: return false;
L3341:
L3342: }
L3343:
L3344: sawKey.add(uppercaseKey);
L3345:
L3346: return true;
L3347:
L3348: },
L3349:
L3350: );
L3351:
L3352: }
L3353:
L3354: for (const key of envKeys) {
L3355:
L3356: const value = env[key];
L3357:
L3358: if (value !== undefined) {
L3359:
L3360: validateArgumentNullCheck(key, `options.env['${key}']`);
L3361:
L3362: validateArgumentNullCheck(value, `options.env['${key}']`);
L3363:
L3364: ArrayPrototypePush(envPairs, `${key}=${value}`);
L3365:
L3366: }
L3367:
L3368: }
L3369:
L3370: return {
L3371:
L3372: // Make a shallow copy so we don't clobber the user's options object.
L3373: __proto__: null,
L3374:
L3375: ...options,
L3376:
L3377: args,
L3378:
L3379: cwd,
L3380:
L3381: detached: !!options.detached,
L3382:
L3383: envPairs,
L3384:
L3385: file,
L3386:
L3387: windowsHide: !!options.windowsHide,
L3388:
L3389: windowsVerbatimArguments: !!windowsVerbatimArguments,
L3390:
L3391: };
L3392:
L3393: }
L3394:
L3395: function abortChildProcess(child, killSignal, reason) {
L3396:
L3397: if (!child)
L3398:
L3399: return;
L3400:
L3401: try {
L3402:
L3403: if (child.kill(killSignal)) {
L3404:
L3405: child.emit('error', new AbortError(undefined, { cause: reason }));
L3406:
L3407: }
L3408:
L3409: } catch (err) {
L3410:
L3411: child.emit('error', err);
L3412:
L3413: }
L3414:
L3415: }
L3416:
L3417: /**
L3418:
L3419: * Spawns a new process using the given `file`.
L3420:
L3421: * @param {string} file
L3422: * @param {…[truncated]</excerpt>
</source>
<source kind="find_in_page">
<title>node/deps/uv/src/win/process.c at v24.0.0 · nodejs/node · GitHub</title>
<location>https://github.com/nodejs/node/blob/v24.0.0/deps/uv/src/win/process.c</location>
<excerpt>citeturn2view2 [wordlim: 200] Crawled: today; Content type: text/html; Source: find({"ref_id":"turn1view2","pattern":"env_block"}); Total lines: 3838
L3213: * The way windows takes environment variables is different than what C does;
L3214:
L3215: * Windows wants a contiguous block of null-terminated strings, terminated
L3216:
L3217: * with an additional null.
L3218:
L3219: *
L3220:
L3221: * Windows has a few "essential" environment variables. winsock will fail
L3222:
L3223: * to initialize if SYSTEMROOT is not defined; some APIs make reference to
L3224:
L3225: * TEMP. SYSTEMDRIVE is probably also important. We therefore ensure that
L3226:
L3227: * these get defined if the input environment block does not contain any
L3228:
L3229: * values for them.
L3230:
L3231: *
L3232: * Also add variables known to Cygwin to be required for correct
L3233:
L3234: * subprocess operation in many cases:
L3235:
L3236: * https://github.com/Alexpux/Cygwin/blob/b266b04fbbd3a595f02ea149e4306d3ab9b1fe3d/winsup/cygwin/environ.cc#L955
L3237:
L3238: *
L3239:
L3240: */
L3241:
L3242: int make_program_env(char* env_block[], WCHAR** dst_ptr) {
L3243:
L3244: WCHAR* dst;
L3245:
L3246: WCHAR* ptr;
L3247:
L3248: char** env;
L3249:
L3250: size_t env_len = 0;
L3251:
L3252: size_t len;
L3253:
L3254: size_t i;
L3255:
L3256: size_t var_size;
L3257:
L3258: size_t env_block_count = 1; /* 1 for null-terminator */
L3259:
L3260: WCHAR* dst_copy;
L3261:
L3262: WCHAR** ptr_copy;
L3263:
L3264: WCHAR** env_copy;
L3265:
L3266: char* p;
L3267: size_t required_vars_value_len[ARRAY_SIZE(required_vars)];
L3268:
L3269: /* first pass: determine size in UTF-16 */
L3270:
L3271: for (env = env_block; *env; env++) {
L3272:
L3273: ssize_t len;
L3274:
L3275: if (strchr(*env, '=')) {
L3276:
L3277: len = uv_wtf8_length_as_utf16(*env);
L3278:
L3279: if (len < 0)
L3280:
L3281: return len;
L3282:
L3283: env_len += len;
L3284:
L3285: env_block_count++;
L3286:
L3287: }
L3288:
L3289: }
L3290:
L3291: /* second pass: copy to UTF-16 environment block */
L3292:
L3293: len = env_block_count * sizeof(WCHAR*);
L3294:
L3295: p = uv__malloc(len + env_len * sizeof(WCHAR));
L3296:
L3297: if (p == NULL) {
L3298:
L3299: return UV_ENOMEM;
L3300:
L3301: }
L3302:
L3303: env_copy = (void*) &p[0];
L3304: dst_copy = (void*) &p[len];
L3305:
L3306: ptr = dst_copy;
L3307:
L3308: ptr_copy = env_copy;
L3309:
L3310: for (env = env_block; *env; env++) {
L3311:
L3312: ssize_t len;
L3313:
L3314: if (strchr(*env, '=')) {
L3315:
L3316: len = uv_wtf8_length_as_utf16(*env);
L3317:
L3318: assert(len > 0);
L3319:
L3320: assert((size_t) len <= env_len - (ptr - dst_copy));
L3321:
L3322: uv_wtf8_to_utf16(*env, ptr, len);
L3323:
L3324: *ptr_copy++ = ptr;
L3325:
L3326: ptr += len;
L3327:
L3328: }
L3329:
L3330: }
L3331:
L3332: *ptr_copy = NULL;
L3333:
L3334: assert(env_len == 0 || env_len == (size_t) (ptr - dst_copy));
L3335:
L3336: /* sort our (UTF-16) copy */
L3337:
L3338: qsort(env_copy, env_block_count-1, sizeof(wchar_t*), qsort_wcscmp);
L3339: /* third pass: check for required variables */
L3340:
L3341: for (ptr_copy = env_copy, i = 0; i < ARRAY_SIZE(required_vars); ) {
L3342:
L3343: int cmp;
L3344:
L3345: if (!*ptr_copy) {
L3346:
L3347: cmp = -1;
L3348:
L3349: } else {
L3350:
L3351: cmp = env_strncmp(required_vars[i].wide_eq,
L3352:
L3353: required_vars[i].len,
L3354:
L3355: *ptr_copy);
L3356:
L3357: }
L3358:
L3359: if (cmp < 0) {
L3360:
L3361: /* missing required var */
L3362:
L3363: var_size = GetEnvironmentVariableW(required_vars[i].wide, NULL, 0);
L3364:
L3365: required_vars_value_len[i] = var_size;
L3366:
L3367: if (var_size != 0) {
L3368:
L3369: env_len += required_vars[i].len;
L3370:
L3371: env_len += var_size;
L3372:
L3373: }
L3374:
L3375: i++;
L3376:
L3377: } else {
L3378: ptr_copy++;
L3379:
L3380: if (cmp == 0)
L3381:
L3382: i++;
L3383:…[truncated]</excerpt>
</source>
<source kind="open_page">
<title>node/lib/child_process.js at v24.0.0 · nodejs/node · GitHub</title>
<location>https://github.com/nodejs/node/blob/v24.0.0/lib/child_process.js</location>
<excerpt>citeturn1view1 [wordlim: 200] Crawled: today; Content type: text/html; Source: open({"ref_id":"https://github.com/nodejs/node/blob/v24.0.0/lib/child_process.js","lineno":null}); Total lines: 3868
L0: 【0†Skip to content】
L1: ## Navigation Menu
L2:
L3: 【1†Sign in】
L4:
L5: Appearance settings
L6:
L7: * Platform
L8:
L9: * AI CODE CREATION
L10: * 【2†GitHub CopilotWrite better code with AI】
L11: * 【3†GitHub Copilot appDirect agents from issue to merge】
L12: * 【4†MCP RegistryIntegrate external tools】
L13:
L14: * DEVELOPER WORKFLOWS
L15: * 【5†ActionsAutomate any workflow】
L16: * 【6†CodespacesInstant dev environments】
L17: * 【7†IssuesPlan and track work】
L18: * 【8†Code ReviewManage code changes】
L19: * 【9†Code QualityEnforce quality at merge】
L20: * APPLICATION SECURITY
L21: * 【10†GitHub Advanced SecurityFind and fix vulnerabilities】
L22: * 【11†Code securitySecure your code as you build】
L23: * 【12†Secret protectionStop leaks before they start】
L24:
L25: * EXPLORE
L26: * 【13†Why GitHub】
L27: * 【14†Documentation†docs.github.com】
L28: * 【15†Blog†github.blog】
L29: * 【16†Changelog†github.blog】
L30: * 【17†Marketplace】
L31:
L32: 【18†View all features】
L33:
L34: * Solutions
L35: * BY COMPANY SIZE
L36: * 【19†Enterprises】
L37: * 【20†Small and medium teams】
L38: * 【21†Startups】
L39: * 【22†Nonprofits】
L40:
L41: * BY USE CASE
L42: * 【23†App Modernization】
L43: * 【24†DevSecOps】
L44: * 【25†DevOps】
L45: * 【26†CI/CD】
L46: * 【27†View all use cases】
L47:
L48: * BY INDUSTRY
L49: * 【28†Healthcare】
L50: * 【29†Financial services】
L51: * 【30†Manufacturing】
L52: * 【31†Government】
L53: * 【32†View all industries】
L54:
L55: 【33†View all solutions】
L56:
L57: * Resources
L58: * EXPLORE BY TOPIC
L59: * 【34†AI】
L60: * 【35†Software Development】
L61: * 【36†DevOps】
L62: * 【37†Security】
L63: * 【38†View all topics】
L64:
L65: * EXPLORE BY TYPE
L66: * 【39†Customer stories】
L67: * 【40†Events & webinars】
L68: * 【41†Ebooks & reports】
L69: * 【42†Business insights】
L70: * 【43†GitHub Skills†skills.github.com】
L71: * SUPPORT & SERVICES
L72: * 【14†Documentation†docs.github.com】
L73: * 【44†Customer support†support.github.com】
L74: * 【45†Community forum】
L75: * 【46†Trust center】
L76: * 【47†Partners】
L77:
L78: 【48†View all resources】
L79:
L80: * Open Source
L81:
L82: * COMMUNITY
L83: * 【49†GitHub SponsorsFund open source developers】
L84: * PROGRAMS
L85: * 【50†Security Lab†securitylab.github.com】
L86: * 【51†Maintainer Community†maintainers.github.com】
L87: * 【52†GitHub Stars†stars.github.com】
L88: * 【53†Archive Program†archiveprogram.github.com】
L89:
L90: * REPOSITORIES
L91: * 【54†Topics】
L92: * 【55†Trending】
L93: * 【56†Collections】
L94:
L95: * Enterprise
L96:
L97: * ENTERPRISE SOLUTIONS
L98: * 【19†Enterprise platformAI-powered developer platform】
L99: * AVAILABLE ADD-ONS
L100: * 【10†GitHub Advanced SecurityEnterprise-grade security features】
L101: * 【57†Copilot for BusinessEnterprise-grade AI features】
L102: * 【58†Premium SupportEnterprise-grade 24/7 support】
L103:
L104: * 【59†Pricing】
L105:
L106: Search`/`
L107:
L108: 【1†Sign in】
L109:
L110: 【60†Sign up】
L111:
L112: Appearance settings
L113: You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session. You switched accounts on another tab or window. Reload to refresh your session. Dismiss alert
L114:
L115: 1. 【61†nodejs】
L116: 2. 【62†node】
L117: ## Repository navigation
L118:
L119: * * 【62†Code】
L120: * 【63†Issues626 (626)】
L121: * 【64†Pull requests6…[truncated]</excerpt>
</source>
<source kind="open_page">
<title>node/deps/uv/src/win/process.c at v24.0.0 · nodejs/node · GitHub</title>
<location>https://github.com/nodejs/node/blob/v24.0.0/deps/uv/src/win/process.c</location>
<excerpt>citeturn1view2 [wordlim: 200] Crawled: last week; Content type: text/html; Source: open({"ref_id":"https://github.com/nodejs/node/blob/v24.0.0/deps/uv/src/win/process.c","lineno":null}); Total lines: 3838
L0: 【0†Skip to content】
L1: ## Navigation Menu
L2:
L3: 【1†Sign in】
L4:
L5: Appearance settings
L6:
L7: * Platform
L8:
L9: * AI CODE CREATION
L10: * 【2†GitHub CopilotWrite better code with AI】
L11: * 【3†GitHub Copilot appDirect agents from issue to merge】
L12: * 【4†MCP RegistryIntegrate external tools】
L13:
L14: * DEVELOPER WORKFLOWS
L15: * 【5†ActionsAutomate any workflow】
L16: * 【6†CodespacesInstant dev environments】
L17: * 【7†IssuesPlan and track work】
L18: * 【8†Code ReviewManage code changes】
L19: * 【9†Code QualityEnforce quality at merge】
L20: * APPLICATION SECURITY
L21: * 【10†GitHub Advanced SecurityFind and fix vulnerabilities】
L22: * 【11†Code securitySecure your code as you build】
L23: * 【12†Secret protectionStop leaks before they start】
L24:
L25: * EXPLORE
L26: * 【13†Why GitHub】
L27: * 【14†Documentation†docs.github.com】
L28: * 【15†Blog†github.blog】
L29: * 【16†Changelog†github.blog】
L30: * 【17†Marketplace】
L31:
L32: 【18†View all features】
L33:
L34: * Solutions
L35: * BY COMPANY SIZE
L36: * 【19†Enterprises】
L37: * 【20†Small and medium teams】
L38: * 【21†Startups】
L39: * 【22†Nonprofits】
L40:
L41: * BY USE CASE
L42: * 【23†App Modernization】
L43: * 【24†DevSecOps】
L44: * 【25†DevOps】
L45: * 【26†CI/CD】
L46: * 【27†View all use cases】
L47:
L48: * BY INDUSTRY
L49: * 【28†Healthcare】
L50: * 【29†Financial services】
L51: * 【30†Manufacturing】
L52: * 【31†Government】
L53: * 【32†View all industries】
L54:
L55: 【33†View all solutions】
L56:
L57: * Resources
L58: * EXPLORE BY TOPIC
L59: * 【34†AI】
L60: * 【35†Software Development】
L61: * 【36†DevOps】
L62: * 【37†Security】
L63: * 【38†View all topics】
L64:
L65: * EXPLORE BY TYPE
L66: * 【39†Customer stories】
L67: * 【40†Events & webinars】
L68: * 【41†Ebooks & reports】
L69: * 【42†Business insights】
L70: * 【43†GitHub Skills†skills.github.com】
L71: * SUPPORT & SERVICES
L72: * 【14†Documentation†docs.github.com】
L73: * 【44†Customer support†support.github.com】
L74: * 【45†Community forum】
L75: * 【46†Trust center】
L76: * 【47†Partners】
L77:
L78: 【48†View all resources】
L79:
L80: * Open Source
L81:
L82: * COMMUNITY
L83: * 【49†GitHub SponsorsFund open source developers】
L84: * PROGRAMS
L85: * 【50†Security Lab†securitylab.github.com】
L86: * 【51†Maintainer Community†maintainers.github.com】
L87: * 【52†GitHub Stars†stars.github.com】
L88: * 【53†Archive Program†archiveprogram.github.com】
L89:
L90: * REPOSITORIES
L91: * 【54†Topics】
L92: * 【55†Trending】
L93: * 【56†Collections】
L94:
L95: * Enterprise
L96:
L97: * ENTERPRISE SOLUTIONS
L98: * 【19†Enterprise platformAI-powered developer platform】
L99: * AVAILABLE ADD-ONS
L100: * 【10†GitHub Advanced SecurityEnterprise-grade security features】
L101: * 【57†Copilot for BusinessEnterprise-grade AI features】
L102: * 【58†Premium SupportEnterprise-grade 24/7 support】
L103:
L104: * 【59†Pricing】
L105:
L106: Search`/`
L107:
L108: 【1†Sign in】
L109:
L110: 【60†Sign up】
L111:
L112: Appearance settings
L113: You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session. You switched accounts on another tab or window. Reload to refresh your session. Dismiss alert
L114:
L115: 1. 【61†nodejs】
L116: 2. 【62†node】
L117: ## Repository navigation
L118:
L119: * * 【62†Code】
L120: * 【63†Issues609 (609)】
L121: * 【64†Pull …[truncated]</excerpt>
</source>
<source kind="search">
<title>Child process | Node.js v26.12.1 Documentation</title>
<location>https://nodejs.org/api/child_process.html</location>
<excerpt>citeturn0search0 [wordlim: 200] Crawled: yesterday; On Windows, environment variables are case-insensitive.Node.js lexicographically sorts the `env` keys and uses the first one that case-insensitively matches.Only first (in lexicographic order) entry will be passed to the subprocess. ... Node.js overwrites `argv[0]` with `process.execPath` on startup, so `process.argv[0]` in a Node.js child process will not match the `argv0` parameter passed to `spawn` from the parent.
On Windows, environment variables are case-insensitive. Node.js lexicographically sorts the `env` keys and uses the first one that case-insensitively matches. Only first (in lexicographic order) entry will be passed to the subprocess. This might lead to issues on Windows when passing objects to the `env` option that have multiple variants of the same key, such as `PATH` and `Path`.
The `child_process.spawn()` method spawns the child process asynchronously, without blocking the Node.js event loop. The `child_process.spawnSync()` function provides equivalent functionality in a synchronous manner that blocks the event loop until the spawned process either exits or is terminated.
For convenience, the `node:child_process` module provides a handful of synchronous and asynchronous alternatives to `child_process.spawn()` and `child_process.spawnSync()`. Each of these alternatives are implemented on top of `child_process.spawn()` or `child_process.spawnSync()`.
</excerpt>
</source>
<omitted_sources count="36">Additional source records were omitted from this bounded result.</omitted_sources>
</source_evidence>
Citations:
- 1: https://github.com/nodejs/node/blob/v24.0.0/lib/child_process.js
- 2: https://github.com/nodejs/node/blob/v24.0.0/lib/child_process.js
🏁 Script executed:
nl -ba src/main/codex-cli/command.test.ts | sed -n '324,435p'
printf '\n--- structured child environment ---\n'
nl -ba src/main/runtime/structured-session-child-env.ts | sed -n '90,125p'
printf '\n--- all pairing call sites ---\n'
rg -n -F -- 'withCliRuntimeOnPath(' srcRepository: stablyai/orca
Length of output: 10420
🏁 Script executed:
rg -n -F -- 'sealStructuredSessionChild(' src
rg -n -F -- 'cliRuntimeCommand' src/main src/sharedRepository: stablyai/orca
Length of output: 1655
Select the Windows PATH key using Node’s spawn ordering.
When both keys exist with different values, firstWindowsPathEnvKey selects Path by insertion order, but Node 24 selects PATH lexicographically. The pairing routine then modifies Path and deletes PATH, so the structured child can lose directories from the effective PATH and fail to find commands. This is limited to duplicate case variants with different values.
Suggested fix
--- "a/src/shared/cli-runtime-pairing-inputs.ts"
+++ "b/src/shared/cli-runtime-pairing-inputs.ts"
@@ -34,7 +34,7 @@
// Mirrors resolvePathEnvKey in src/main/pty/windows-path-segment-merge.ts, which
// src/shared must not import.
export function firstWindowsPathEnvKey(env: NodeJS.ProcessEnv): string {
- for (const key of Object.keys(env)) {
+ for (const key of Object.keys(env).sort()) {
if (key.toLowerCase() === 'path' && env[key] !== undefined) {
return key
}Update the duplicate-key test to expect only PATH and to preserve the original PATH value, prefixed with the runtime directory.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const key of Object.keys(env)) { | |
| if (key.toLowerCase() === 'path' && env[key] !== undefined) { | |
| return key | |
| for (const key of Object.keys(env).sort()) { | |
| if (key.toLowerCase() === 'path' && env[key] !== undefined) { | |
| return key |
nwparker
force-pushed
the
nwparker/codex-structured-cli-runtime
branch
from
October 23, 2026 03:58
51837a8 to
35246f5
Compare
Pairing ran after the Orca CLI prepend, so a runtime dir that also holds an `orca` (e.g. /usr/local/bin with node+codex+orca) shadowed this app's launcher for the child. Pair first; the CLI dir stays first on PATH. Drops a PATH-shape test the parity snapshots already cover. Co-Authored-By: Claude <noreply@anthropic.com>
…ring a sibling node A native codex/claude beside a node (Homebrew, /usr/bin) never runs on it, so pairing only reordered PATH. An env naming no PATH now pairs over the PATH the child inherits instead of replacing it with the CLI dir alone. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
nwparker
force-pushed
the
nwparker/codex-structured-cli-runtime
branch
from
October 23, 2026 04:03
35246f5 to
63d6b63
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Codex installed with npm is a small JavaScript file, so it needs
nodeto run. When Orca starts a Codex chat, it may find Codex inside one NVM Node version folder (for example~/.nvm/versions/node/v26.7.0/bin/codex), but the chat process's PATH may have nonodeat all, or a different one. Before this PR, the chat then failed withenv: node: No such file or directoryand the user saw "Unable to start chat". Now Orca puts thenodethat sits next to the Codex it found on the chat's PATH. Claude chats and one-shot Codex runs already did this.This PR also makes sure Orca's own
orcacommand stays first on the chat's PATH, ahead of that Node folder. Without that, a folder holdingnode,codexand an installedorca(for example/usr/local/binon Intel Macs, or/usr/binon Linux) could make the chat call a different Orca build'sorca. Claude chats had this ordering bug too; it is fixed in the same shared function.A Codex that is a compiled program (Homebrew, the standalone installer, a Linux package) never runs on
node, so Orca now leaves PATH alone for it even when anodehappens to sit in the same folder. And if the env Orca builds names no PATH at all, Orca keeps the PATH the chat would inherit instead of replacing it with just the Codex folder.What Changed
codex-structured-launch-resolution.ts: pass the resolved Codex command ascliRuntimeCommand, as Claude already does.structured-session-child-env.ts:sealStructuredSessionChildpairs the runtime before it adds this app's CLI dir, so PATH becomes<orca-cli>/bin : <codex's node dir> : <inherited PATH>. Before, the runtime dir went ahead of the Orca CLI dir. The doc comment already said "after the identity's", and the existingDEV_CLI_BIN_FIRSTassertions require the CLI dir to be first.Snapshots: the Codex parity snapshots now match
mainagain. The five default Claude cases change back to Orca CLI first.Tests: one real-process POSIX test starts a shebang CLI that would fail under the wrong
node. It checks the PATH order is CLI dir, then runtime dir, then the inherited PATH. One redundant PATH-shape test was removed because the parity snapshots already cover the sibling and no-sibling cases.withCliRuntimeOnPath(shared by every Node-CLI pairing caller): skips a command whose first bytes are ELF, Mach-O or PE (newsrc/shared/cli-runtime-pairing-inputs.ts, which also takesfirstWindowsPathEnvKeyout of the over-limit resolver file). When the env has no PATH key it now pairs overprocess.env's PATH, the one the child inherits, matchingreadInheritedPath. Before, it wrotePATH=<cli dir>and the inherited PATH was lost. Two unit tests insrc/main/codex-cli/command.test.tscover both cases.Linked Issue
Related to #27502. This fixes the pairing gap in the persistent launch. The issue's other cause, a stale or broken
codexshim earlier on PATH that blocks the fallback to a working NVM install, is not fixed here, so the issue stays open.Verification
codex app-serverhandshake (initializeonly: no model requests, scratchCODEX_HOME) against the real npm install~/.nvm/versions/node/v26.7.0/bin/codex(codex-cli 0.150.0)./usr/bin:/binand no node: without the pairing line it fails withenv: node: No such file or directory. With the fix the handshake succeeds.nodethat exits 79 earlier on PATH: without the fix it fails withWRONG-NODE. With the fix the handshake succeeds.<orca-dev cli>/bin:~/.nvm/versions/node/v26.7.0/bin:..., so the Orca CLI stays first.pnpm test src/main/codex src/main/claude/claude-structured-launch-resolution.test.ts src/main/runtime/structured-agent-launch-parity.test.ts: 293 files, 2988 tests pass. The shebang test fails if the pairing line is removed.pnpm teston the Codex child-environment, Claude launch-resolution and parity suites: 92/92 pass. This includes the POSIX shebang test.C:\Program Files\nodejs, codex from npm at%APPDATA%\npm\codex.cmd):pnpm teston the Codex child-environment, Claude launch-resolution and parity suites: the same 4 tests fail on this branch and onmain(they expectorca-devbut Windows getsorca-dev.cmd). This PR adds no new failures. The shebang test is skipped on Windows, as intended.codex.cmd, which has nonode.exenext to it, so nothing is added.Pathstill starts with the Orca CLI dir.node.exe,codex.cmdand a junction to the real npmnode_modules. A fakenode.cmdthat exits 79 comes first onPath, and a stalePATHkey sits next toPath. On this branch, the composed env has a singlePathkey, ordered<orca cli>\bin;<codex folder>;<competing>;.... The realcodex.cmd app-serverinitializehandshake succeeds. Onmain(with the two source files swapped in), the folder is not added and bothPathandPATHkeys are left in the env.withCliRuntimeOnPathwith esbuild into a throwaway probe (not checked in) and ran it on real installs:~/.local/bin/codex, symlinked beside anode, leaves PATH alone. The npm~/.nvm/versions/node/v26.7.0/bin/codexstill pairs. With an empty env, the result is<codex dir>:<inherited PATH>, not just<codex dir>.nodeleaves PATH alone. A#!/usr/bin/env nodescript beside mise's node pairs, runs on that node, and keeps the inherited PATH.%APPDATA%\npm\codex.cmdhas no siblingnode.exe, so nothing changes. Acodex.cmdbeside a copiednode.exepairs (<dir>;C:\Windows). A PE.exebesidenode.exeleaves PATH alone.main, read only, not run): tool commands get their env fromshell_environment_policy. Bothinherit = "all"and"core"keepPATH(UNIX_CORE_ENV_VARS/WINDOWS_CORE_ENV_VARSinprotocol/src/shell_environment.rs). Theshell_snapshotfeature is stable and on by default. It captures PATH from an interactive login shell started with Codex's env, so the user's rc files (nvm use,brew shellenv, macOSpath_helper) run on top of our PATH and usually put their own entries in front again. In other words, a tool command sees PATH the way a new terminal started from that env would.main28c7268, which includes fix(runtime): main lints and typechecks again after #32018 #32168 (head 63d6b63, no conflicts):pnpm testoversrc/main/codex,src/main/claude,src/main/codex-cli,src/main/rate-limits,src/main/text-generation,src/main/agent-session, structured-session-child-env and the node-cli resolver: 567 files, 5901 tests pass. The parity suite passes too (56/56). oxlint and oxfmt are clean on the changed files.withCliRuntimeOnPathwith tsx: the npm@openai/codex(bin/codex.js,#!/usr/bin/env node) symlinked beside anodepairs, giving<dir>:/usr/bin:/bin. A Mach-O copied beside the samenodeleaves PATH alone.codexandnodeshims share a folder, and each shim is an ELF (/usr/bin/mise), so this PR no longer pairs that folder. I checked this is harmless.env -i PATH=/usr/bin:/bin ~/.local/share/mise/shims/codex --versionprintscodex-cli 0.160.0. Inside a shim, mise puts its own node install on PATH (mise exec -- command -v nodegives.../mise/installs/node/26.8.1/bin/node). The codex mise runs is itself an ELF.%APPDATA%\npm\codex.cmdstarts@ECH, so it is not native and behaves as before. There is nonode.exebeside it. The realclaude.exeandnode.exestart4d 5a(MZ), so they are detected as native.mainitself (headless-terminal-tab-close.tswas over max-lines after fix(runtime): reject unsupported renderer-owned tab properties #32018). fix(runtime): main lints and typechecks again after #32018 #32168 fixed that, and this branch is now rebased on top of it.pnpm tcis clean (node, cli and web), and oxlint is clean on the changed files plus that file.Risks / Tradeoffs
node; compiled binaries are skipped), just as a terminal has when that NVM version is active. Commands Codex runs undershell_environment_policy inherit = "core"see that PATH unless their login shell rebuilds it. Homebrew, standalone and distro-package Codex builds are compiled binaries, so they are no longer affected. Codex's default shell snapshot re-runs the user's rc files over this PATH, so nvm/brew setups usually reorder it again for tool commands. Bare commands and installs without that siblingnodebehave exactly as before. A native shim beside anodeshim (mise) is no longer paired; mise supplies its own node (checked on omarchy above)..cmdshims already prefer anode.exenext to them, so on Windows this mostly changesPathorder (checked above). Not exercised: WSL, and SSH/remote hosts, which compose their launches on the host. Mobile and the wire protocol are unaffected: this only changes the local child environment.Visual Proof
N/A: this only changes the process environment. Nothing in the UI changes.
Related contributor PRs
None found. #10932 (closed) covered the general NVM CLI/runtime pairing.