Skip to content

Add back error messages #5528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+3 −5
Closed

Add back error messages #5528

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions codex-rs/core/src/tools/orchestrator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,10 @@ fn build_never_denied_message_from_output(output: &ExecToolCallOutput) -> String

match detail {
Some(tag) => format!("failed in sandbox: {tag}"),
None => "failed in sandbox".to_string(),
None => format!("failed in sandbox: {}", output.aggregated_output.text),
}
}

fn build_denial_reason_from_output(_output: &ExecToolCallOutput) -> String {
// Keep approval reason terse and stable for UX/tests, but accept the
// output so we can evolve heuristics later without touching call sites.
"command failed; retry without sandbox?".to_string()
fn build_denial_reason_from_output(output: &ExecToolCallOutput) -> String {
format!("command failed: {}\nretry without sandbox?", output.aggregated_output.text)
Comment on lines +168 to +169
Copy link
Contributor

Choose a reason for hiding this comment

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

P1 Badge Keep approval reason string stable

The retry prompt now formats command failed: {output}\nretry without sandbox?, which includes the full aggregated command output. The existing approval tests (core/tests/suite/approvals.rs) assert the reason is the constant "command failed; retry without sandbox?", so this change will immediately break those scenarios and reintroduces OS‑specific and potentially multiline text into the approval reason. Either trim/normalize the message or update the tests and callers that expect the concise string.

Useful? React with 👍 / 👎.

}
Loading