Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddf9672335
ℹ️ 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".
| if ( | ||
| parsed.diagnostics.some((diagnostic) => diagnostic.severity === 'error') | ||
| ) { |
There was a problem hiding this comment.
This branch only checks diagnostic.severity, but the previous IR shape for grep diagnostics (changed in this commit) did not include severity; those payloads still rely on usageError. Because execute.ts already documents that step.args can be legacy until compiler artifacts are rebuilt, this can silently skip the usage-error path and run grep with invalid arguments (e.g., unknown/missing options) instead of returning deterministic error status 2 with diagnostics. Keep a parsed.usageError fallback here to avoid mixed-version regressions.
Useful? React with 👍 / 👎.
|
@codex review pls |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb00e6bd69
ℹ️ 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".
| if ( | ||
| parsed.diagnostics.some((diagnostic) => diagnostic.severity === 'error') | ||
| ) { |
There was a problem hiding this comment.
runGrepCommandInner now gates compile-time failures on diagnostic.severity === 'error', but this path drops legacy GrepArgsIR.diagnostics objects that only have {code,message,token,tokenIndex} (the compatibility case already documented in execute.ts about stale compiler outputs). In that case, invalid invocations like unknown options can bypass the early error return, proceed through normal matching, and return success/nomatch statuses without emitting stderr diagnostics. Please keep a fallback to parsed.usageError (or normalize legacy diagnostics) so pre-rebuild/legacy parse results still fail deterministically.
Useful? React with 👍 / 👎.
1 participant
No description provided.