fix(codex): preserve terminal outcome ordering#93287
Conversation
|
Land-ready proof for
|
openclaw-barnacle
Bot
added
agents
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(codex): preserve terminal outcome ordering This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
![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: e3dc5bb5d3
ℹ️ 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 ( | ||
| observation.toolCallOrdinal !== undefined && | ||
| suppressedDynamicToolOutcomeOrdinals.has(observation.toolCallOrdinal) | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
When a Codex dynamic tool times out/aborts but the underlying tool ignores cancellation and later completes, this ordinal-wide suppression drops every subsequent onToolOutcome, including the normal non-presentationOnly observation emitted by the before-tool-call wrapper. The runner's observer in src/agents/embedded-agent-runner/run.ts:1352 only feeds postCompactionGuard.observe() for non-presentation observations, so these late real outcomes no longer participate in the post-compaction loop guard; only the terminal presentation needed to be suppressed/cleared. Consider forwarding non-presentation observations with terminalPresentation stripped instead of returning here.
Useful? React with 👍 / 👎.
1 participant
Summary
Follow-up to #93228 and its ordering review findings:
Verification
pnpm test extensions/codex/src/app-server/event-projector.test.ts extensions/codex/src/app-server/run-attempt.dynamic-tools.test.ts extensions/codex/src/app-server/dynamic-tool-build.test.ts extensions/codex/src/app-server/dynamic-tool-execution.test.ts src/agents/agent-tools.before-tool-call.integration.e2e.test.tsrun_ec1338fa8af3:pnpm check:changed,pnpm build, and the same focused 168-test setReal behavior proof
Behavior addressed: Codex incomplete-turn fallback now selects the last model-ordered tool outcome instead of notification or completion order.
Real environment tested: Azure Linux
Standard_D32ads_v6through Crabbox.Exact steps or command run after this patch: changed checks, full production build, then focused core/Codex E2E tests.
Evidence after fix: Crabbox run
run_ec1338fa8af3exited 0; 22 core E2E and 146 Codex extension tests passed.Observed result after fix: delayed native command/web-search completions and earlier dynamic failures no longer clear a later dynamic summary; late success after timeout is suppressed.
What was not tested: live provider traffic against a production Codex account.