Repository navigation
Fix global flag placement and environment precedence - #382
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 2 mixed files remain; 0 existing customizations changed. Compared 2 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 37722744437 --repo openai/openai-cli \
--name castiron-custom-code-37722744437-1 --dir /tmp/castiron-custom-code-37722744437-1
git apply --stat /tmp/castiron-custom-code-37722744437-1/custom-code.patch
cat /tmp/castiron-custom-code-37722744437-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin e6d3f50b92af3ae55cab7ff069d10bcfea7b5ecf 04d93793c75a192bca1448a1f52d5b9208544742
python3 scripts/castiron/custom_code_report.py report \
--base e6d3f50b92af3ae55cab7ff069d10bcfea7b5ecf \
--head 04d93793c75a192bca1448a1f52d5b9208544742 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-04d93793c75a
cat /tmp/castiron-custom-code-04d93793c75a/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![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: c35da86ae3
ℹ️ 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".
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed c35da86ae3ba07ad5ba7ae63539444ede34b6220. One URL-precedence gap remains; see the inline comment.
The root-flag wrapper, project body/header separation, and mTLS explicit-value handling fit the existing architecture. Completion remains local, and the new demos use the isolated capture harness.
Hosted build, lint, and CodeQL checks pass. The test job still fails the two reported groups: their expectations conflict with the new URL precedence and local completion behavior. Update those cases while retaining the sensitive-error and no-file-write assertions.
This was a source-only review, including the pinned CLI/SDK dependencies and failing CI log. I did not execute repository tests or reproduce the URL case at runtime.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
One current-head issue remains; see the inline comment.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
3 participants
This addresses three problems in SDK-1186: global flag placement, environment overrides, and completion blocked by request settings.
What changes
--projectnow work before, between, or after command words. Authentication and organization/project values reach the API request. Webhook settings also inherit.OPENAI_BASE_URLno longer blocks a valid--base-url. Explicit client-certificate and key flags also override their environment defaults, wherever the flags appear.Commands
No new commands or flags. These now send the same project header:
Environment variables still supply defaults. For example, this URL overrides
OPENAI_BASE_URL:An empty
--base-urlkeeps the existing environment fallback.An empty client-certificate option overrides its environment default; incomplete certificate/key pairs and duplicate certificate/key options still fail.
Flags defined by an individual command keep their local meaning.
For example, an invite’s
--projectfield sets membership without replacing the root project header.Completion-script output works even when request settings are invalid:
Code
pkg/custommakes the five root flags available to subcommands and keeps the root project separate from invite fields.It also distinguishes explicit client-certificate options from environment defaults.
internal/autocompletekeeps file completion working through the flag wrapper.main.gouses the explicit URL when the SDK cannot parse the environment URL.It temporarily replaces that environment value for the command and restores it before error handling or exit.
Helper processes inherit the temporary value; the parent shell stays unchanged.
The runner stays in
main.gobecause release builds compile that file directly. Local help and completion bypass request setup.Generated commands and dependencies are unchanged.
Tested
On macOS arm64, 36 focused command and cleanup tests passed without skips. Focused restoration race checks also passed.
These cover malformed URLs, all flag positions, empty fallback, preserved environment defaults, privacy, help, and completion.
Cleanup checks cover success, errors, cancellation, panic, child-process inheritance, and repeated runs.
Independent review passed 92 request, streaming, cancellation, and verified client-certificate cases.
The invalid-percent URL regressions fail before this fix and pass afterward.
The corrected CI tests retain malformed-argument, no-write, and no-request checks.
The single-file entrypoint builds locally. Completion-script and manpage generation also pass.
Native Windows/Linux checks and the full mock-server suite were not rerun locally. CI results for the updated commit are pending.
Demo
Real CLI binaries in macOS Bash terminals, using a local mock API.
The output shows the project header that the mock received. Existing
--transform=id -roptions keep results on one line.Before is
0eae357, the source merged by #379. After is250c875.The recordings show flag placement; automated tests cover the URL and completion fixes.
Before:
After:
Recording recipe