fix(codex): normalize native command exit status by shbernal · Pull Request #88336 · openclaw/openclaw

shbernal · 2026-05-30T13:17:36Z

Summary

Normalize only the reported Codex-native grep/rg no-match shape: direct grep/rg commands and grep/rg pipeline segments with exitCode: 1 are completed tool results instead of lastToolError warnings.
Keep generic exitCode: 1 native command failures visible, including chained grep/rg commands and shell-wrapped search commands, until maintainers define a broader Codex-native failure taxonomy.
Add focused event-projector coverage for direct grep/rg no-match, grep pipeline no-match, chained grep/rg failures, generic exit-1 failures, shell-wrapped search staying failed, and command-not-found 127.

Scope note

This could later be enlarged to Codex parsed command actions with type: "search" or a broader search-command taxonomy, but this PR intentionally stays conservative and only handles direct grep/rg commands plus grep/rg pipeline segments. Semicolon and logical-command chains stay visible as failures.

Real behavior proof

Behavior addressed: Codex-native grep/rg no-match command status projection no longer turns expected search misses into channel-visible OpenClaw failed-tool warnings.

Real environment tested: local source checkout only; no live Telegram or Discord run.

Exact steps or command run after this patch: git diff --check; OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs extensions/codex/src/app-server/event-projector.test.ts -- --reporter=verbose -t 'grep no-match|chained grep|generic exit 1|shell-wrapped search|streamed command output'

Evidence after fix: git diff --check passed; the focused projector test selection passed with 6 tests run and 73 skipped.

Observed result after fix: Direct grep/rg commands and grep/rg pipeline segments with exitCode: 1 are completed tool results without lastToolError; generic exit-1 failures, chained grep/rg commands, and shell-wrapped search commands still surface as failed native tool errors; exit code 127 remains a failed native tool error.

What was not tested: Per maintainer instruction for this machine, I did not run the full OpenClaw test suite or live channel E2E because broad test runs can freeze this computer.

Verification

git diff --check
OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs extensions/codex/src/app-server/event-projector.test.ts -- --reporter=verbose -t 'grep no-match|chained grep|generic exit 1|shell-wrapped search|streamed command output'

Not run:

Full OpenClaw test suite, per maintainer instruction.

clawsweeper · 2026-05-30T13:19:21Z

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 4:40 PM ET / 20:40 UTC.

Summary
The branch changes the Codex app-server event projector so failed commandExecution items with numeric exit codes other than 126/127 are reported as completed tool results, and adjusts focused projector tests.

PR surface: Source +15, Tests +36. Total +51 across 2 files.

Reproducibility: yes. at source level: current main maps failed Codex commandExecution items into mutating lastToolError, and the payload path can surface that as a visible warning. I did not establish a fresh live Telegram or Discord reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

Native status predicate: 1 completion predicate added. The new predicate controls whether native Codex command failures feed diagnostics and channel-visible warning payloads.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #88332
Summary: This PR is the open candidate fix for the canonical Codex-native status to visible warning issue, but the current branch is too broad and still needs taxonomy review and real behavior proof.

Members:

canonical: [Bug]: Codex native tool statuses surface expected checks as channel-visible OpenClaw failures #88332 - The PR body uses closing syntax for the open canonical issue, which contains the broader Codex-native status warning report and related channel evidence.
same_root_cause: [Bug]: Telegram shows internal "⚠️ 🛠️ ... failed" for normal grep exit(1) after upgrade to 2026.5.20 #87610 - The closed Telegram grep no-match report is the direct visible warning case preserved as a reproduction anchor for the canonical issue.
partial_overlap: [Bug]: 2026.5.19 Discord shows failed file reads in chat #85310 - The closed Discord report shares the visible failed-tool warning symptom but also involved a separate heartbeat delivery-policy gap.
partial_overlap: fix(googlechat): sanitize internal tool-trace lines from outbound text (#90684) #95084 - The merged Google Chat sanitizer PR suppresses one downstream channel leak but explicitly leaves the Codex-native warning taxonomy out of scope.
adjacent_distinct: Tool-error warning (⚠️ … failed) not suppressed for Codex app-server exec_command (EXEC_LIKE_TOOL_NAMES misses it despite NATIVE_HOOK_TOOL_NAME_ALIASES) #93482 - That open issue tracks raw Codex exec_command warning normalization in shared helpers, which is related warning-policy work but not the same commandExecution status predicate changed here.
1 more in the report.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Narrow the completion predicate to expected no-match commands or a maintainer-approved Codex-native failure taxonomy.
[P1] Add redacted live output, logs, terminal proof, or a Telegram/Discord recording that shows the warning behavior after the patch.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body reports only local focused projector tests and explicitly says no live Telegram or Discord run was performed, so after-fix real behavior proof is still missing. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A native Telegram proof would show whether grep/rg no-match still produces a visible failed-tool banner after the projector change. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify that a Codex grep/rg no-match command no longer posts a visible failed-tool warning in Telegram.

Risk before merge

[P1] Merging the current predicate can suppress lastToolError and channel-visible warnings for real Codex-native command failures with numeric exit codes other than 126/127.
[P2] Sandbox-denied, timeout, network-denied, or other security-relevant native execution failures could be hidden if Codex reports them as failed command output with a numeric exit code.
[P1] The PR body describes a narrower grep/rg allowlist, but the live branch still implements the broad numeric-exit rule.
[P1] The contributor proof is local focused tests only; no redacted live Telegram, Discord, terminal, log, or copied runtime output shows the changed warning behavior after the patch.

Maintainer options:

Narrow The Status Predicate (recommended)
Only classify explicit expected-negative grep/rg no-match outcomes as completed and keep generic numeric command failures recorded as lastToolError.
Adopt Broad Numeric Completion Deliberately
Maintainers can accept the broad exec contract only with an explicit taxonomy decision and proof for timeout, denial, sandbox, and command-failure cases.
Pause For The Canonical Taxonomy Issue
If the native failure boundary is not settled, pause this PR and resolve the policy in the linked canonical issue first.

Next step before merge

[P1] Human review remains the right lane because contributor live proof and maintainer agreement on the Codex-native failure taxonomy are still merge gates.

Security
Needs attention: The diff has no supply-chain changes, but broad status normalization can hide security-relevant native command failures from warning paths.

Review findings

[P1] Narrow completed exits to expected no-match commands — extensions/codex/src/app-server/event-projector.ts:1950-1953

Review details

Best possible solution:

Narrow completion to explicit expected-negative search outcomes or a maintainer-approved Codex-native taxonomy, keep generic failures visible, and require redacted live channel or runtime-log proof before merge.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main maps failed Codex commandExecution items into mutating lastToolError, and the payload path can surface that as a visible warning. I did not establish a fresh live Telegram or Discord reproduction in this read-only review.

Is this the best way to solve the issue?

No: the live diff normalizes all numeric failed command exits except 126/127, while the safer shape is the PR body's narrower expected no-match search policy or a maintainer-approved taxonomy.

Full review comments:

[P1] Narrow completed exits to expected no-match commands — extensions/codex/src/app-server/event-projector.ts:1950-1953
isCompletedCommandExit() treats every failed Codex commandExecution with a numeric exit code other than 126/127 as completed. Codex exposes only status and exitCode here, and sibling Codex tests show sandbox-denied paths can also use exit code 1, so this can clear lastToolError for real failures; narrow it to explicit grep/rg no-match outcomes or a maintainer-approved taxonomy.
Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 15de9d881a84.

Label changes

Label justifications:

P2: The PR targets a normal-priority user-visible Codex/channel warning regression with limited blast radius but nontrivial merge risk.
merge-risk: 🚨 message-delivery: The event projector feeds channel warning payloads, and changing failed command statuses can suppress messages for real failed native commands.
merge-risk: 🚨 security-boundary: Codex can report sandbox or network-denied execution failures through command failure paths, and the broad numeric-exit rewrite can hide those warnings.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports only local focused projector tests and explicitly says no live Telegram or Discord run was performed, so after-fix real behavior proof is still missing. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The linked bug is visible as Telegram channel warning output, so a short Telegram proof lane can demonstrate the user-visible behavior.

Evidence reviewed

PR surface:

Source +15, Tests +36. Total +51 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	18	3	+15
Tests	1	39	3	+36
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	57	6	+51

Security concerns:

[medium] Broad status normalization can hide native failures — extensions/codex/src/app-server/event-projector.ts:1953
Failed Codex commandExecution items with numeric exit codes other than 126/127 become completed, which can suppress lastToolError for sandbox-denied, timeout, network-denied, or other security-relevant native command failures.
Confidence: 0.84

What I checked:

PR diff broadens command completion: The PR head adds isCompletedCommandExit(), which treats any commandExecution with a numeric exitCode except 126/127 as completed; it does not inspect grep/rg, commandActions, pipeline shape, or expected no-match semantics. (extensions/codex/src/app-server/event-projector.ts:1950, 37c31a27778f)
PR tests preserve only command-not-found as failed: The PR adds one grep exit-1 success case and changes the existing streamed failed-command coverage from generic exitCode 1 to 127, leaving ordinary real exit-1 failures unprotected on the branch. (extensions/codex/src/app-server/event-projector.test.ts:1698, 37c31a27778f)
Current main records failed native items as lastToolError candidates: Current main computes result status from itemStatus(), records non-success native tool items through recordNativeToolError(), and emits tool result isError from the same status. (extensions/codex/src/app-server/event-projector.ts:1178, 15de9d881a84)
Current main treats Codex native commands as mutating: Current main intentionally returns true for every commandExecution because Codex commandActions are presentation rather than safety, so clearing failed status changes the stricter warning path. (extensions/codex/src/app-server/event-projector.ts:2302, 15de9d881a84)
Channel warning consumer still surfaces mutating tool errors: buildEmbeddedRunPayloads uses lastToolError and shows warnings for mutating tool errors unless a user-facing failure acknowledgement already exists. (src/agents/embedded-agent-runner/run/payloads.ts:529, 15de9d881a84)
Current main preserves generic exit-1 failure coverage: The current projector test asserts a failed commandExecution with exitCode 1 records lastToolError from streamed output, which the PR weakens by moving that case to 127. (extensions/codex/src/app-server/event-projector.test.ts:2243, 15de9d881a84)

Likely related people:

steipete: GitHub path history shows repeated Codex app-server, embedded-runner, terminal outcome, and warning-policy work across the affected projector and payload surfaces. (role: feature-history owner; confidence: high; commits: a09f6b1b2787, 0314819f918a, 3bac0bcbfb20; files: extensions/codex/src/app-server/event-projector.ts, src/agents/embedded-agent-runner/run/payloads.ts)
vincentkoc: Recent GitHub path history and duplicate-triage discussion show adjacent work on Codex projector/payload warning behavior and related agent delivery surfaces. (role: recent area contributor; confidence: high; commits: 756e9d16a0a6, 2b75806197ab, a1af47e5da18; files: extensions/codex/src/app-server/event-projector.ts, src/agents/embedded-agent-runner/run/payloads.ts)
LiuwqGit: Recent commits specifically touched degraded/orphan tool delivery and lastToolError behavior adjacent to this native status projection path. (role: recent Codex projector contributor; confidence: medium; commits: a1af47e5da18, ed3a0241f39f; files: extensions/codex/src/app-server/event-projector.ts)
fuller-stack-dev: Recent empty post-tool final-turn recovery work is adjacent because the canonical issue includes final-reply delivery and visible warning interactions after native tool failures. (role: adjacent embedded-runner contributor; confidence: medium; commits: 77a682c5def2, 0314819f918a; files: extensions/codex/src/app-server/event-projector.ts, src/agents/embedded-agent-runner/run/payloads.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (1 earlier review cycle)

reviewed 2026-06-21T07:39:14.499Z sha 37c31a2 :: needs real behavior proof before merge. :: [P1] Narrow completed exits to expected no-match commands

byungskers · 2026-05-30T17:02:17Z

I like the direction for the grep/no-match case, but the current predicate feels too broad. isCompletedCommandExit() reclassifies every failed commandExecution with a numeric exit code except 126/127 as completed, which seems likely to sweep in timeouts / sandbox-denied / other real execution failures if Codex surfaces them as numeric exits. Could this be narrowed to an explicit allowlist (or richer failure taxonomy) for "expected negative result" commands instead of a blanket numeric-exit rule?

shbernal · 2026-05-30T20:02:44Z

@byungskers thanks for calling this out. The PR is treating too much as completed, because it reclassified every failed Codex native commandExecution with a numeric exit code except 126/127. That fixes the grep no-match report, but it can also hide real failures with exit code 1, which is the part I want to avoid.

I've tried narrower shapes but none of them seem really convincing:

Blanket numeric exit completion (current PR) -> hides exit code 1 for actual failures.
exitCode: 1 plus grep/rg: closer to the reported case, but still too broad for compound commands like grep missing; pnpm test, where the later command could be the meaningful failure.
Direct grep/rg commands and grep/rg pipeline segments only, while rejecting shell chains containing ;, &&, or ||: conservative and easy to test, but still command-text based rather than a real native failure taxonomy.

The last option is the current I am considering.

It matches the documented grep/rg no-match contract (1 is no match, real grep/rg errors are a different exit code) and keeps generic exitCode: 1 failures, shell-wrapped searches, and chained commands visible as lastToolError.
But it intentionally has false negatives: for example, bash -lc 'rg missing docs' would still be surfaced as failed even if Codex also parsed it as a search action.

openclaw-barnacle · 2026-07-20T04:37:59Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

fix(codex): normalize native command exit status

37c31a2

openclaw-barnacle Bot added extensions: codex size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026

clawsweeper Bot had a problem deploying to qa-live-shared May 30, 2026 13:27 Failure

openclaw-barnacle Bot added triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026

clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 10, 2026

clawsweeper Bot mentioned this pull request Jun 15, 2026

[Bug]: Codex native tool statuses surface expected checks as channel-visible OpenClaw failures #88332

Open

clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 15, 2026

clawsweeper Bot mentioned this pull request Jun 20, 2026

fix(googlechat): sanitize internal tool-trace lines from outbound text (#90684) #95084

Merged

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): normalize native command exit status#88336

fix(codex): normalize native command exit status#88336
shbernal wants to merge 1 commit into
openclaw:mainfrom
shbernal:fix/codex-native-tool-status-warnings

shbernal commented May 30, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 30, 2026 •

edited

Loading

Uh oh!

byungskers commented May 30, 2026

Uh oh!

shbernal commented May 30, 2026

Uh oh!

openclaw-barnacle Bot commented Jul 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

shbernal commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope note

Real behavior proof

Verification

Uh oh!

clawsweeper Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

byungskers commented May 30, 2026

Uh oh!

shbernal commented May 30, 2026

Uh oh!

openclaw-barnacle Bot commented Jul 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shbernal commented May 30, 2026 •

edited

Loading

clawsweeper Bot commented May 30, 2026 •

edited

Loading