fix(review): retain safe diagnostics for Codex failures

[vincentkoc]

What Problem This Solves

Fixes an issue where an exact review could catch a Codex process failure, keep the runner alive long enough to write a local failure report, and then finish without durable diagnostic evidence for the underlying startup or execution cause.

Why This Change Was Made

Caught exact-review failures now produce a publication-isolated failure-diagnostics/ bundle containing a typed manifest plus bounded, sanitized error, structured stdout error, and stderr-tail files. The writer stores manifest.json last as the readiness marker, caps the complete bundle at 24 KiB, and fails closed per file when content remains unsafe.

The security boundary is deliberately narrow:

• exact environment, prompt-line, and model values are redacted before generic high-confidence token, assignment, private-path, and internal-host rules
• unstructured stdout and prompt events are omitted; only existing Codex error / turn.failed JSONL detail is retained from stdout
• ambiguous multiline secrets, private keys, control bytes, or residual opaque credentials replace the whole file with an omission sentinel
• no raw report, prompt, stdout, session log, action ledger, provider metadata, or publication input is uploaded
• diagnostic write and upload failures remain secondary; the original review failure remains primary

The workflow uploads this bundle only for a non-cancelled failed exact-review job, with a unique run/attempt name and 14-day retention. It does not feed queue publication or exact-review bundle construction.

User Impact

Maintainers can inspect useful, bounded evidence for caught Codex execution failures after the runner has exited. Cancellation and runner loss remain outside this guarantee.

OpenClaw Bay Impact

Bay, queue, dashboard, publication, ledger, and persistent schemas are unchanged. The new artifact is failure-only and has no dependency edge into durable exact-review publication.

Documentation Impact

Updated the active scheduler runbook to describe the failure-only artifact, its retained file set, sanitization boundary, and 14-day retention. The workflow is the source of truth; this documentation should be updated when the diagnostic file set, upload condition, or retention policy changes. No changelog entry is included because this is an internal operational diagnostic repair within the independently reviewed seven-file scope.

Evidence

• Signed head: fa9b1e2e938bcdc77e79ae115e5b0b76d8fe1022
• Focused owner tests: 13/13 passed
• Workflow structural test: 1/1 passed
• Diagnostics coverage: 97.99% lines, 84.85% branches, 100% functions
• All three TypeScript builds, full lint, static checks, formatting, and git diff --check: passed
• Codex local preflight with gpt-5.6-sol: passed
• Exact branch autoreview with Sol/high: clean, correctness 0.96, no accepted P0 finding

pnpm run check itself was not used in the linked worktree because pnpm attempted to reconcile the required shared node_modules symlink. Its static, build, lint, format, and changed-owner coverage surfaces were run directly. A prior repo-wide coverage attempt reached 4,252 tests and passed coverage thresholds, with unrelated host-specific failures and one unrelated live-proof timeout.

Real Behavior Proof

• Claim: the compiled review CLI retains distinguishable, repository-reader-safe diagnostics for caught generic Codex failures only in exact mode.
• Surface: node dist/clawsweeper.js review --local-range.
• Scenario: a synthetic committed range, trusted fake scanner, and fake Codex process produced two different status-17 codex_execution failures containing adversarial prompt, model, prefixed assignment, credential, private-path, internal-host, and unstructured stdout values. A third run omitted exact mode.
• Environment: direct AWS Crabbox, Linux, Node 24, compiled from the signed head.
• Command: Crabbox script invoking the compiled CLI three times with --local-range.
• Observed result: two exact bundles remained distinguishable through structured stdout and sanitized stderr; the non-exact run wrote no bundle; typed status/classification/source facts matched; known leak count was zero; each aggregate stayed within 24 KiB.
• Trace: run run_c4ff948163e5, lease cbx_48bbe54562d8, result succeeded, leaseStopped=true.
• Limits: fake scanner and Codex were used to avoid manufacturing a production outage. Cancellation and runner loss are not covered; the first production canary is the next natural caught failure.

Codex contract inspection covered codex-rs/cli/src/main.rs:220-245,2840-2870, codex-rs/exec/src/event_processor_with_jsonl_output.rs:103-115,531-550, and codex-rs/exec/src/exec_events.rs:1-65.

Scope And LOC

Changed paths are limited to the approved workflow, scheduler docs, command wiring, private diagnostics writer, and three focused tests.

• production TypeScript: +211 net
• workflow: +15 net
• docs: +7 net
• production/workflow/docs total: +233 net, within the reviewed +240 hard ceiling
• tests: +189 net, within the reviewed +190 hard ceiling

The +190-line private writer owns the new bounded sanitization and atomic readiness capability. The remaining production growth is +21 lines of failure-only command wiring; workflow and docs add the upload contract and operator description.

Overlap

• chore: refresh development dependencies and workflow tools #1318 remains open; its overlapping files contain distant dependency-version updates only.
• fix(review): restrict hosted reviews to public targets #1292 remains open; it owns hosted-target admission and does not change this diagnostics writer or upload dependency.
• fix(repair): require a trusted status comment author #1073 and fix(review): keep trailing markers within their HTML comments #1070 are merged and were already present in the starting main revision.
• Current origin/main advanced through b445c64591bcf9bf6d68122f28f6c95c4fa1dbf4; those intervening commits do not touch the seven retained paths.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 1, 2026, 7:47 AM ET / 11:47 UTC.

ClawSweeper review

What this changes

The PR adds a bounded, sanitized failure-diagnostics artifact for caught Codex failures in exact reviews and documents its workflow upload policy.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep this member-authored PR open for normal maintainer review. The introduced path keeps failure diagnostics separate from review publication, bounds and sanitizes the three retained diagnostic inputs, and uploads only after a caught exact-review failure.

Priority: P2
Reviewed head: fa9b1e2e938bcdc77e79ae115e5b0b76d8fe1022

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	The patch is focused and has concrete source-backed safeguards and coverage, with the remaining consideration being normal review of its new artifact boundary.
Proof confidence	🌊 off-meta tidepool	Not applicable: The PR body records a compiled CLI run that exercised the production review workflow with controlled Codex failures and observed distinct bounded diagnostics; the ordinary contributor proof gate does not apply to this MEMBER-authored PR.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The PR body records a compiled CLI run that exercised the production review workflow with controlled Codex failures and observed distinct bounded diagnostics; the ordinary contributor proof gate does not apply to this MEMBER-authored PR.
Evidence reviewed	5 items	Bounded, isolated diagnostic writer: The introduced writer accepts only the error message, structured JSONL error detail, and stderr; it sanitizes each file, enforces per-file and aggregate limits, and stages the directory before publishing its readiness manifest. Exact-review upload gate: The workflow exposes the upload flag only after a nonzero review exit with a diagnostics manifest, then uploads the fixed directory with 14-day retention and continues to preserve the original review failure. Focused sanitization coverage: Tests cover prompt, model, token, assignment, path, host, unsafe-control, private-key, and opaque-value handling as well as the size limits and manifest omission list.
Findings	None	None.
Security	None	None.

How this fits together

Exact reviews run Codex over queued repository items and normally produce durable review records. This change creates a separate failure-only artifact for maintainers when Codex fails, without making that artifact an input to publication or queue state.

flowchart LR
A[Exact review request] --> B[Codex review process]
B --> C{Caught failure?}
C -->|Yes| D[Sanitize bounded diagnostics]
D --> E[Failure artifact upload]
C -->|No| F[Normal review publication]
E --> G[Maintainer inspection]

Loading

Before merge

• Resolve merge risk (P1) - The new artifact is readable by repository readers, so future additions to its inputs or sanitizer rules require the same security-boundary review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Implementation and coverage delta	production/workflow/docs +233; tests +189	The new sanitizer and upload path have focused coverage roughly proportional to the added operational behavior.
Artifact policy	1 new upload step; 3 diagnostic files; 14-day retention	These are the material operator-facing limits for the newly retained failure data.

Merge-risk options

Maintainer options:

1. Retain the bounded diagnostic boundary (recommended)
   Accept the new artifact only with its three-file allowlist, fail-closed sanitization, and 14-day retention kept intact.

Technical review

Best possible solution:

Keep the failure artifact limited to the three sanitized diagnostic fields, with no queue, publication, or durable-record dependency.

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

Not applicable as issue reproduction: this is a PR review. The PR body supplies a concrete compiled-CLI trace exercising caught synthetic Codex failures through the introduced exact-review path.

Is this the best way to solve the issue?

Yes. A failure-only artifact, isolated from publication and restricted to sanitized diagnostic fields, is a narrow maintainable way to retain useful operational evidence.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add P2: This is a contained operational reliability improvement to the exact-review workflow.
• add merge-risk: 🚨 security-boundary: The PR changes which Codex failure data becomes available as a GitHub artifact, making the sanitizer and allowlist merge-critical.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR body records a compiled CLI run that exercised the production review workflow with controlled Codex failures and observed distinct bounded diagnostics; the ordinary contributor proof gate does not apply to this MEMBER-authored PR.

Label justifications:

• P2: This is a contained operational reliability improvement to the exact-review workflow.
• merge-risk: 🚨 security-boundary: The PR changes which Codex failure data becomes available as a GitHub artifact, making the sanitizer and allowlist merge-critical.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR body records a compiled CLI run that exercised the production review workflow with controlled Codex failures and observed distinct bounded diagnostics; the ordinary contributor proof gate does not apply to this MEMBER-authored PR.

Evidence

What I checked:

• Bounded, isolated diagnostic writer: The introduced writer accepts only the error message, structured JSONL error detail, and stderr; it sanitizes each file, enforces per-file and aggregate limits, and stages the directory before publishing its readiness manifest. (src/clawsweeper-review-failure-diagnostics.ts:43, fa9b1e2e938b)
• Exact-review upload gate: The workflow exposes the upload flag only after a nonzero review exit with a diagnostics manifest, then uploads the fixed directory with 14-day retention and continues to preserve the original review failure. (.github/workflows/sweep.yml:1310, fa9b1e2e938b)
• Focused sanitization coverage: Tests cover prompt, model, token, assignment, path, host, unsafe-control, private-key, and opaque-value handling as well as the size limits and manifest omission list. (test/exact-review-failure-diagnostics.test.ts:37, fa9b1e2e938b)
• Configured target scope: The current Worker configuration names only explicit public OpenClaw repositories as review targets, so the manifest’s repository and source revision fields do not introduce a private-target disclosure path in the configured deployment. (dashboard/wrangler.toml:41, b445c64591bc)
• Introduced-change provenance: The reviewed commit has the pinned main revision as its sole parent and introduces the diagnostics writer, workflow gate, documentation, and focused tests. (src/clawsweeper-review-failure-diagnostics.ts:1, fa9b1e2e938b)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.