Skip to content

fix(codex): distinguish command exits from wrapper completion - #243

Merged
Abhijeet Prasad (AbhiPrasad) merged 3 commits into
mainfrom
trace-command-outcomes-f3acffea
Oct 28, 2026
Merged

Abhijeet Prasad (AbhiPrasad) merged 3 commits into
mainfrom
trace-command-outcomes-f3acffea

Conversation

@AbhiPrasad

Copy link
Copy Markdown
Member

🤖 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:

wrapper: Script completed
command: exit_code=1
span.error: null

Interpret native command results separately from wrapper completion while retaining the returned output:

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.

resolves SDK-708

> 🤖 **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
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T17:22:55.623022Z 9a542ce New commits
ℹ️ About Codex in GitHub

Your 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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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"] {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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
@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) merged commit aa8e99c into main Oct 28, 2026
23 checks passed
@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) deleted the trace-command-outcomes-f3acffea branch October 28, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants