Skip to content

Trim double Total output lines#4787

Merged
pakrym-oai merged 2 commits intomainfrom
pakrym/trim-double-total-output-lines
Oct 5, 2025
Merged

Trim double Total output lines#4787
pakrym-oai merged 2 commits intomainfrom
pakrym/trim-double-total-output-lines

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

No description provided.

- Add regex patterns for outputs including exit code, wall time, and total output lines
- Allow matching of both new and legacy error output formats
- Fallback to previous error pattern if new patterns fail
- Strengthen test robustness for spawn failure output
Copy link
Copy Markdown
Contributor

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

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.

ℹ️ 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

Comment on lines 516 to 520
Wall time: [0-9]+(?:\.[0-9]+)? seconds
Total output lines: \d+
Output:
Total output lines: \d+

failed in sandbox: .*?(?:Operation not permitted|Permission denied|Read-only file system).*?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Remove stale blank line in denied sandbox regex

The structured output now strips the Total output lines header and its separating blank line, so the output string begins immediately after Output:. The regex here still contains an empty line between Output: and the first expected message, which means shell_sandbox_denied_truncates_error_output will fail once the new formatting is produced. Drop the blank line or make it optional so the pattern matches the updated output.

Useful? React with 👍 / 👎.

Comment on lines 600 to 604
Wall time: [0-9]+(?:\.[0-9]+)? seconds
Total output lines: \d+
Output:
Total output lines: \d+

execution error: .*$"#;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Update spawn failure regex for compact output header

Similar to the previous test, build_structured_output now removes the truncation header from the embedded output, eliminating the blank line that used to follow Output:. The spawn_truncated_pattern still includes an empty line in the regex, so it will no longer match the formatted output and will cause the test to fail. Adjust the pattern to expect Output: followed immediately by the error text (or make the extra newline optional).

Useful? React with 👍 / 👎.

@ghost
Copy link
Copy Markdown

ghost commented Oct 5, 2025

_ No description provided. _

@pakrym-oai pakrym-oai merged commit a90a58f into main Oct 5, 2025
20 checks passed
@pakrym-oai pakrym-oai deleted the pakrym/trim-double-total-output-lines branch October 5, 2025 23:41
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants