fix: fail loud when a delegated Codex task run performs no work - #617
fix: fail loud when a delegated Codex task run performs no work#617petersimmons1972 wants to merge 1 commit into
Conversation
scripts/lib/tracked-jobs.mjs treats execution.exitStatus === 0 as "completed" with no check on whether the delegated turn actually did anything. A Codex app-server turn that reports "completed" status commits to nothing about work performed - a turn where the model described a plan and stopped, with no file touched and no command run, exits/completes exactly like a turn that made the change. This is the same defect class already fixed in the sibling Grok bridge (xai-org/grok-build-plugin-cc#16, "fail loud when a delegated run performs no work"). The codex app-server protocol already gives a better signal than the Grok CLI's JSON envelope hack: scripts/lib/codex.mjs's captureTurn collects fileChanges and commandExecutions thread items for every turn, and runAppServerTurn already returns touchedFiles / commandExecutions on the result. No telemetry-extraction step needs porting - only the gating decision. Adds scripts/lib/work-evidence.mjs with assessWorkEvidence(): - For a --write task run: noWork if the turn touched zero files AND ran zero commands. - For any run: noWork if there is zero output text AND zero tool activity. - Read-only (write: false) runs are not gated on touched-file count - answering a question legitimately touches nothing. - requireWork: false (new --allow-no-work CLI flag) disables the gate for callers that intentionally want a plan-only run. Wires the verdict through codex-companion.mjs::executeTaskRun, buildTaskRequest (so background task-worker runs enforce the same gate as foreground), and runForegroundCommand (foreground exit code 3 on an empty run, mirroring the grok bridge's exit-3 convention). Wires tracked-jobs.mjs::runTrackedJob to demote completionStatus to failed whenever execution.workVerdict.noWork is true, independent of exitStatus, and records workEvidence / emptyRun / errorMessage on the stored job record so /codex:status surfaces the reason.
![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: bca81c6c0b
ℹ️ 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".
| } else if (execution.workVerdict?.noWork) { | ||
| // fleet#264: an empty run must be visible to the shell that invoked codex-companion, not | ||
| // just in the job record. | ||
| process.exitCode = 3; |
There was a problem hiding this comment.
For a foreground empty run, setting exit code 3 makes the companion's Bash invocation fail even though the diagnostic banner was written to stdout. The primary delegated path in plugins/codex/agents/codex-rescue.md:41-42 instructs the rescue subagent to return nothing whenever that Bash call fails, so users receive no explanation in exactly the scenario this change is intended to expose. The wrapper must explicitly forward stdout for this expected exit code, or the companion must signal the verdict without triggering the wrapper's failure-suppression rule.
Useful? React with 👍 / 👎.
| if (evidence.write && evidence.touchedFileCount === 0 && evidence.commandCount === 0) { | ||
| reasons.push( | ||
| "This task was run with --write but the turn touched no files and executed no commands: it described the change instead of making it (fleet#292 shape)." | ||
| ); |
There was a problem hiding this comment.
When a write-capable turn completes work through an mcpToolCall, dynamicToolCall, web search, or collaboration tool without a shell command or app-server fileChange, this condition marks the successful run as empty. These are supported tool types in codex.mjs:241-299, but recordItem only adds commandExecution and fileChange items to the evidence passed here; an MCP tool can even perform an external side effect while both counts remain zero. Track completed activity for all applicable tool types rather than treating these turns as having made no tool calls.
Useful? React with 👍 / 👎.
| const payload = { | ||
| status: result.status, | ||
| threadId: result.threadId, | ||
| rawOutput, |
There was a problem hiding this comment.
For a background empty run whose final message claims success, the new banner exists only in execution.rendered, while this payload also stores that claim as result.rawOutput. /codex:result permits failed jobs but renderStoredJobResult at render.mjs:401-410 returns storedJob.result.rawOutput before consulting storedJob.rendered, so fetching the result drops the failure banner and shows only the misleading Codex claim. Store or render the verdict in the result path before returning raw output.
Useful? React with 👍 / 👎.
| const workVerdict = assessWorkEvidence({ | ||
| write: Boolean(request.write), | ||
| touchedFiles: result.touchedFiles, | ||
| commandExecutions: result.commandExecutions, | ||
| text: rawOutput, | ||
| requireWork: request.requireWork !== false |
There was a problem hiding this comment.
The repository's normal fake app-server task returns a final message without emitting command or file-change items, so enabling this gate by default changes every existing task --write integration run to exit 3. Running node --test --test-name-pattern='write task output focuses' tests/runtime.test.mjs on this commit fails at the expected-zero exit assertion (3 !== 0), which means the checked-in test suite no longer passes. Update the fixture to emit positive work evidence for successful write scenarios, or explicitly disable the gate in tests that are not exercising it.
Useful? React with 👍 / 👎.
1 participant
Problem
scripts/lib/tracked-jobs.mjstreatsexecution.exitStatus === 0ascompletedwith no check on whether the delegated turn actually didanything (
tracked-jobs.mjs:156in the shipped 1.0.6 build). A Codexapp-server turn that reports "completed" status commits to nothing about
work performed — a turn where the model described a plan and stopped, with
no file touched and no command run, exits/completes exactly like a turn
that made the change.
grep-ing the shipped plugin forworkEvidence|emptyRunreturns zero matches: there is no gate at all(fleet#264).
Live specimen
fleet PR #292 (closed) is this failure mode end to end: the delegate's
REPORT claimed the work was done and
run_staterecorded it as delivered,but the actual PR diff was empty. Nothing in the codex bridge would have
caught this —
exitStatuswas 0, so the job was markedcompleted, andevery caller downstream treated
completedas evidence of delivered work.Precedent
This is the same defect class already fixed in the sibling Grok bridge:
xai-org/grok-build-plugin-cc#16, "fail loud when a delegated run performsno work" (fleet#254). That fix added a
work-evidence.mjsmodule and agate in
tracked-jobs.mjsthat demotes an empty run fromcompletedtofailedregardless of process exit code, usingnum_turnsfrom the GrokCLI's structured JSON output as the discriminating signal.
This fix
The codex app-server protocol already gives a better signal than the
Grok CLI's JSON envelope hack:
scripts/lib/codex.mjs'scaptureTurncollects
fileChangesandcommandExecutionsthread items for every turn,and
runAppServerTurnalready returnstouchedFiles/commandExecutionson the result. No telemetry-extraction step needsporting — only the gating decision.
Adds
scripts/lib/work-evidence.mjswithassessWorkEvidence():--writetask run:noWorkif the turn touched zero files ANDran zero commands (the fleet#292 shape).
noWorkif there is zero output text AND zero toolactivity.
write: false) runs are not gated on touched-file count —answering a question legitimately touches nothing.
requireWork: false(new--allow-no-workCLI flag) disables the gatefor callers that intentionally want a plan-only run.
Wires the verdict through
codex-companion.mjs::executeTaskRun,buildTaskRequest(so backgroundtask-workerruns enforce the samegate as foreground), and
runForegroundCommand(foreground exit code 3on an empty run, mirroring the grok bridge's exit-3 convention). Wires
tracked-jobs.mjs::runTrackedJobto demotecompletionStatustofailedwheneverexecution.workVerdict.noWorkis true, independent ofexitStatus, and recordsworkEvidence/emptyRun/errorMessageonthe stored job record so
/codex:statussurfaces the reason.Verification
Unit + integration checks run against a scratch copy of the plugin tree
(not committed to this repo; see fleet#264 lane E1 receipt for paths):
assessWorkEvidencepositive/negative/read-only/disabled-gate cases.runTrackedJobend-to-end: a stubbed execution withexitStatus: 0andworkVerdict.noWork: truelands as job statusfailed(
emptyRun: true); the positive-work control with the sameexitStatus: 0lands ascompleted.git apply --checkandnode --checkagainst a freshly copied,unmodified 1.0.6 tree (not the tree the patch was authored against).