fix(companion): harden task path parsing and availability probes - #574
fix(companion): harden task path parsing and availability probes#574SomSamantray wants to merge 4 commits into
Conversation
Fix missing-cwd misreporting as CLI not installed, prevent prompt fragments from being parsed as --model on task, and fail results when turn capture records an app-server error. Adds stopAtFirstPositional for handleTask, -- before prompt in rescue docs, and node --test coverage.
![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: 0edc4e5b8e
ℹ️ 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".
|
@codex review |
Skip storing error notifications with willRetry and clear state.error on a real completed turn so successful retries are not reported as failures.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c087f1ba1
ℹ️ 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".
Those flags are Claude-side execution control only. Listing them among task flags before -- would queue a second detached job and return the launch message instead of Codex output.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce4ac23bf2
ℹ️ 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 (!token.startsWith("-") || token === "-") { | ||
| positionals.push(token); | ||
| if (config.stopAtFirstPositional) { | ||
| stopOptions = true; | ||
| } |
There was a problem hiding this comment.
When a raw task prompt starts with an unrecognized dash-prefixed token, such as --coverage run -m pytest, that token is added to positionals without enabling stopOptions because this new block only handles non-dash tokens. The later -m pytest is consequently still consumed as the model, so the prompt-parsing failure this option is intended to prevent remains for option-looking prompt prefixes. Set stopOptions whenever an unrecognized option is treated as positional as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 790d49c: when \stopAtFirstPositional\ is set, unrecognized long/short options that fall through as positionals now arm \stopOptions, so later tokens like -m\ stay in the prompt instead of being parsed as --model. Covered in \ ests/args.test.mjs.
| emitProgress( | ||
| state.onProgress, | ||
| `Codex error (retrying): ${message.params.error?.message ?? "unknown error"}`, | ||
| "failed" | ||
| ); |
There was a problem hiding this comment.
When the app server emits an error with willRetry: true, this progress event is persisted by createJobProgressUpdater as the running job's phase, and /codex:status renders that phase directly. The job can therefore appear as running with phase failed while Codex is still retrying, potentially causing users or callers monitoring a background task to treat an active run as failed. Report a retrying/running phase here and reserve failed for terminal errors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 790d49c: retriable app-server errors (\willRetry: true) now emit progress phase
etrying\ via
esolveErrorProgressPhase; terminal errors still use \ ailed. Covered in \ ests/codex-turn-status.test.mjs.
Arm stopOptions when unrecognized dash tokens become positional so -m in prompts is not consumed as --model. Emit progress phase retrying (not failed) for willRetry app-server errors.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Operators on the
codex:codex-rescue→taskpath could see false “CLI not installed” errors after a worktree cleanup, have prompts like-m pytestturned into invalid--modelvalues (surfacing Codex 400s as task output), or get a clean exit when the app-server had already emitted an error.RC1 — availability:
binaryAvailable()drops a non-existentcwdfor version/help probes only, so a missing worktree no longer masquerades as a missing Codex install.RC2 — prompt parsing:
handleTaskenablesstopAtFirstPositionalinparseArgs, and rescue/runtime docs require routing flags before--and the prompt after--. Prompt fragments after the first positional are no longer parsed as CLI flags.RC3 — result status:
buildResultStatus()returns failure whenturnState.erroris set, even iffinalTurnwas inferred ascompleted.node --test tests/process.test.mjs tests/args.test.mjs tests/codex-turn-status.test.mjs tests/commands.test.mjs— 19/19 pass. RC2 integration:node --test --test-name-pattern "does not treat prompt" tests/runtime.test.mjs— pass.Fixes #393