Repository navigation
fix(codex): distinguish command exits from wrapper completion - #243
Conversation
> 🤖 **This text was AI-generated.**
Codex can complete an exec wrapper while a nested command fails, including
when write_stdin returns the final result. Previously, traces could show:
```text
wrapper: Script completed
command: exit_code=1
span.error: null
```
Interpret native command results separately from wrapper completion while
retaining the returned output:
```text
span.error: Command exited with code 1
metadata.wrapper_outcome: completed
metadata.command_outcomes: [{outcome: nonzero_exit, exit_code: 1}]
```
Keep successful calls free of empty errors and record explicit cancellation
without inferring it from missing output. Ignore failure-like text in stdout.
Nonzero exits describe process status; grep can return one for an expected
result. Statuses discarded or truncated by scripts remain unknown.
Refs: SDK-708
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. |
> 🤖 **This text was AI-generated.** The Codex smoke test now checks actual command exits, exposing sandbox startup failures on Linux and policy rejection on Windows: ```text read-only sandbox -> shell blocked -> expected exit never observed ``` Run fixed mock-inference commands without the agent OS sandbox, while keeping live-inference runs read-only: ```text mock inference -> fixed command -> exit 0 or 7 -> trace assertion ``` Also recognize the native Output: header seen in CI, so stdout cannot supply an exit status when the command header omits one. Refs: SDK-708
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0513faf5a7
ℹ️ 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 (@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 (@codex) address that feedback".
| } else { | ||
| // Known container shapes, including Promise.allSettled. | ||
| // Do not recursively scan arbitrary API payload fields. | ||
| for key in ["result", "results", "value", "content"] { |
There was a problem hiding this comment.
Record rejected allSettled command results
When code-mode uses Promise.allSettled and a nested command/tool promise rejects, the native entry has the shape {"status":"rejected","reason":...}. This traversal inspects only result, results, value, and content, while native_error also does not recognize rejected, so the rejection is silently omitted from both the span error and command_outcomes. Inspect reason and classify the rejected status so these explicit failures are retained.
Useful? React with 👍 / 👎.
> 🤖 **This text was AI-generated.**
A completed code-mode wrapper could hide a failed tool promise because
Promise.allSettled reports rejection through status and reason:
```text
{status: rejected, reason: {message: "command launch failed"}}
-> span.error: null
```
Recognize rejected entries, extract the reason with the shared error
helper, and retain the failure alongside other observed command outcomes:
```text
span.error: command launch failed
metadata.command_outcomes: [{outcome: rejected}]
```
Use a clear fallback when the reason has no message. Preserve raw output
and leave the command exit code unknown when it was not reported.
Refs: SDK-708
Codex can complete an exec wrapper while a nested command fails, including when write_stdin returns the final result. Previously, traces could show:
Interpret native command results separately from wrapper completion while retaining the returned output:
Keep successful calls free of empty errors and record explicit cancellation without inferring it from missing output. Ignore failure-like text in stdout. Nonzero exits describe process status; grep can return one for an expected result. Statuses discarded or truncated by scripts remain unknown.
resolves SDK-708