Codex review: needs real behavior proof before merge. Reviewed July 8, 2026, 1:04 AM ET / 05:04 UTC.

Summary
The PR adds a child.stdout error handler to the ACPX MCP proxy and a regression test for target stdout stream failures.

PR surface: Source +1, Tests +49. Total +50 across 2 files.

Reproducibility: yes. from source and seam evidence, but not from a live non-mocked ACPX run: current main pipes child.stdout without an error listener, so a parent-side stdout stream error can bypass the controlled exitWithError path.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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:

• [P1] Add a non-mocked ACPX runtime transcript/log if one can be produced, or ask a maintainer to apply a proof override for this parent-side stream boundary.
• Keep any posted terminal output or logs redacted for private paths, endpoints, tokens, and other sensitive details.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR body includes before/after terminal-style seam output and green proof-context checks, but the exercised failure uses a temporary preload/mock of child_process.spawn; non-mocked redacted logs/terminal output or a maintainer proof override is still needed before merge. 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.

Risk before merge

• [P1] Proof remains based on a mocked/preloaded parent-side stream failure, so a maintainer needs to accept that seam proof or request a non-mocked ACPX runtime transcript before merge.

Maintainer options:

1. Decide the mitigation before merge
   Land the one-line stdout error handler and regression test once a maintainer accepts the seam proof with an override or the PR supplies non-mocked ACPX runtime proof.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• [P1] Human handling is needed only for proof sufficiency; I found no narrow code defect for ClawSweeper to repair.

Maintainer decision needed

• Question: Should this PR's mocked parent-side target stdout stream proof be accepted with a maintainer proof override, or should the PR keep waiting for non-mocked ACPX runtime proof?
• Rationale: The patch is the narrow code fix, but the external-PR proof gate requires real behavior proof unless a maintainer decides the parent-side stream boundary cannot be shown naturally and accepts the seam comparison.
• Likely owner: vincentkoc — Recent ACPX MCP proxy refactor and stream-error hardening history makes this the clearest proof-sufficiency routing candidate.
• Options:
  • Accept seam proof with override (recommended): A maintainer can treat the before/after preload comparison as sufficient for this hard-to-trigger Node stream boundary and clear the proof gate before merge.
  • Require non-mocked ACPX proof: Keep the needs-proof state until the author provides a redacted live run, terminal transcript, or logs from a non-mocked ACPX target exercising the failure.

Security
Cleared: The diff only adds an event handler and a focused test; it does not change dependencies, workflows, secrets, package resolution, or executable supply-chain surfaces.

@clawsweeper re-review

Maintainer follow-up proof for the current head 149cd86b70e5b738945c1905347edcd4c15a2ac1:

• The failing Real behavior proof gate was caused by a leading UTF-8 BOM before the PR body's ## What Problem This Solves heading. I reproduced the checker locally: before body cleanup, evaluatePullRequestContext returned missing: What Problem This Solves.
• I removed the BOM from the PR body and added this maintainer follow-up proof summary. After cleanup, the local policy evaluation returned passed: External PR includes problem context and evidence.
• GitHub Actions proof is now green for the exact PR head: https://github.com/openclaw/openclaw/actions/runs/28890588288, job Real behavior proof, completed successfully at 2026-07-07T18:50:09Z. The job log includes External PR includes problem context and evidence.
• Local light check: git diff --check upstream/main...HEAD passed for the two-file PR diff.
• Local focused Vitest was not run because this clean worktree has no node_modules; I did not run pnpm install locally per repo guidance.

Additional stream-contract probe for the previous non-mocked proof concern:

• A normal child process stdout pipe produces data, stdout end, then close 0; it does not emit a parent-side child.stdout error.
• If the child process itself calls process.stdout.destroy(new Error(...)), Node throws the unhandled stdout error inside the child process; the parent still observes stdout end plus child exit, not a parent-side child.stdout error.
• A parent-side readable stream failure does emit child.stdout error, which is exactly the boundary covered by this PR's regression test and by the added child.stdout.on("error", exitWithError) handler.

Given that boundary, I don't think a stable non-mocked CLI target can naturally produce this exact parent-side readable stream error; the best direct proof is the Node stream contract plus the proxy-boundary regression test already in the PR.

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper re-review

Maintainer follow-up proof was added to the PR body for exact head 149cd86b70e5b738945c1905347edcd4c15a2ac1.

Summary of the new proof:

• Before/after runtime seam comparison now shows base emits Node's unhandled stream error for the parent-side target stdout error, while this PR exits through the controlled proxy error path: base hasUnhandled=true; PR hasUnhandled=false, first stderr line target stdout failed.
• Exact-head Real behavior proof is green: https://github.com/openclaw/openclaw/actions/runs/28890588288/job/85701964100.
• git diff --check upstream/main...149cd86b70e5b738945c1905347edcd4c15a2ac1 -- extensions/acpx/src/runtime-internals/mcp-proxy.mjs extensions/acpx/src/runtime-internals/mcp-proxy.test.ts passed.
• Duplicate search for acpx mcp proxy stdout error found only this PR and no matching issues.

This still intentionally frames the behavior proof as the parent-side Node stream/proxy boundary proof, not as a non-mocked ACPX target transcript.