Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8ef8bb5a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| env.CODEX_API_KEY = args.apiKey; | ||
| } | ||
| if (this.pathDirs.length > 0) { | ||
| env.PATH = prependPathDirs(env.PATH, this.pathDirs); |
There was a problem hiding this comment.
When pathDirs is present this unconditionally writes env.PATH, but on Windows the inherited key is often Path (different casing). That creates both PATH and Path in the env object; Node's child_process passes only the first case-insensitive match, so the newly written PATH can win and drop the original system search path. In that case child Codex processes run with a truncated PATH (often just codex-path), which can break command/tool discovery at runtime on Windows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the latest push. prependPathDirs() now preserves the existing Windows Path key and removes duplicate case-insensitive PATH keys before prepending codex-path; covered by a new TypeScript test.
| existing_path = env.get("PATH", "") | ||
| path_dir_values = [str(path_dir) for path_dir in path_dirs] | ||
| existing_entries = [ | ||
| entry for entry in existing_path.split(path_sep) if entry and entry not in path_dir_values | ||
| ] | ||
| env["PATH"] = path_sep.join([*path_dir_values, *existing_entries]) |
There was a problem hiding this comment.
This helper always reads/writes the uppercase PATH key, which can create duplicate case-insensitive path variables (Path + PATH) in the subprocess environment on Windows when callers provide config.env with Path. Python's Windows subprocess environment handling is sensitive to duplicate keys, so the child may receive an unexpected path value and lose entries the caller intended to preserve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the latest push. _prepend_path_dirs() now preserves an existing Windows Path key, removes duplicate case-insensitive PATH keys, and prepends codex-path to that preserved key; covered by a new Python test.
## Why The Python and TypeScript SDKs launch the native Codex runtime directly, so they need to consume the same package artifact shape that release jobs now produce. The runtime wheel should be built from the canonical Codex package archive rather than reconstructing a parallel layout from loose binaries. ## What Changed - Stage `openai-codex-cli-bin` by extracting `codex-package-<target>.tar.gz` into `src/codex_cli_bin` and validating the expected package layout. - Update release workflows to pass the generated package archive into `stage-runtime` instead of the temporary package directory. - Update Python runtime setup to download `codex-package-*.tar.gz` release assets directly. - Expose Python runtime helpers for the bundled package directory and `codex-path`, and prepend that path when `openai_codex` launches the installed runtime without duplicating Windows `Path`/`PATH` keys. - Teach the TypeScript SDK to resolve package-layout optional dependencies while keeping the existing npm fallback layout, and preserve the existing Windows path variable casing when prepending `codex-path`. ## Test Plan - `python3 -m py_compile sdk/python/scripts/update_sdk_artifacts.py sdk/python/_runtime_setup.py sdk/python/src/openai_codex/client.py sdk/python-runtime/src/codex_cli_bin/__init__.py` - `uv run --frozen --project sdk/python --extra dev ruff check sdk/python/scripts/update_sdk_artifacts.py sdk/python/_runtime_setup.py sdk/python/src/openai_codex/client.py sdk/python/tests/test_artifact_workflow_and_binaries.py sdk/python-runtime/src/codex_cli_bin/__init__.py` - `uv run --frozen --project sdk/python --extra dev pytest sdk/python/tests/test_artifact_workflow_and_binaries.py` - `pnpm eslint src/exec.ts tests/exec.test.ts` - `pnpm test --runInBand tests/exec.test.ts`
2 participants
Why
The Python and TypeScript SDKs launch the native Codex runtime directly, so they need to consume the same package artifact shape that release jobs now produce. The runtime wheel should be built from the canonical Codex package archive rather than reconstructing a parallel layout from loose binaries.
What Changed
openai-codex-cli-binby extractingcodex-package-<target>.tar.gzintosrc/codex_cli_binand validating the expected package layout.stage-runtimeinstead of the temporary package directory.codex-package-*.tar.gzrelease assets directly.codex-path, and prepend that path whenopenai_codexlaunches the installed runtime without duplicating WindowsPath/PATHkeys.codex-path.Test Plan
python3 -m py_compile sdk/python/scripts/update_sdk_artifacts.py sdk/python/_runtime_setup.py sdk/python/src/openai_codex/client.py sdk/python-runtime/src/codex_cli_bin/__init__.pyuv run --frozen --project sdk/python --extra dev ruff check sdk/python/scripts/update_sdk_artifacts.py sdk/python/_runtime_setup.py sdk/python/src/openai_codex/client.py sdk/python/tests/test_artifact_workflow_and_binaries.py sdk/python-runtime/src/codex_cli_bin/__init__.pyuv run --frozen --project sdk/python --extra dev pytest sdk/python/tests/test_artifact_workflow_and_binaries.pypnpm eslint src/exec.ts tests/exec.test.tspnpm test --runInBand tests/exec.test.ts