Emit unified exec end on startup failure#22743
Conversation
85c0cc1 to
5f091bb
Compare
a422183 to
db00244
Compare
5f091bb to
bbe638a
Compare
21c7e73 to
ca1d3de
Compare
bbe638a to
3d80311
Compare
77a8012 to
c2a9217
Compare
6078c03 to
cabeb8c
Compare
8c0690f to
8d80010
Compare
cabeb8c to
84af755
Compare
8d80010 to
5b422b9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b422b9e11
ℹ️ 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".
| ToolError::Rejected(message) if message == "rejected by user" => { | ||
| UnifiedExecError::rejected(message) | ||
| } | ||
| ToolError::Rejected(message) => UnifiedExecError::create_process(message), |
There was a problem hiding this comment.
Classify guardian denials as declined
In sessions where approval is routed through Guardian or strict auto-review, reject_if_not_approved returns ToolError::Rejected with a guardian-specific denial or timeout message rather than exactly "rejected by user", so this branch falls through to CreateProcess and the startup failure event is emitted as Failed. That means a unified exec that was declined before any process was started appears to app-server clients as a command failure instead of a declined command, unlike the shell ToolEmitter::finish path that treats ToolError::Rejected as Declined; preserve rejection semantics for these approval-denial messages instead of only the literal local-user string.
Useful? React with 👍 / 👎.
374d141 to
55bdd2c
Compare
912fd90 to
85d0d78
Compare
5967bef to
9d4cfa0
Compare
9d4cfa0 to
a96805c
Compare
Why
Remote
rust-ci-fullexposed unified exec startup failures that happen before a process record exists. In that path, clients could receive the startup error without the matching exec end lifecycle event, leaving the tool call half-open from the app-server/client perspective. Approval rejections in the same startup path also need to stay declined rather than being reported as generic execution failures.Failure evidence: remote Ubuntu test job with uploaded nextest JUnit artifact
What changed
Validation
cargo test -p codex-core startup_ -- --nocapturecargo test -p codex-core --test all unified_exec_interrupt_preserves_long_running_session -- --nocapture