[codex] fix: surface Codex app-server exit diagnostics#3504
Conversation
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
…exit-diagnostics [codex] fix: surface Codex app-server exit diagnostics
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit add72dd. Configure here.
ApprovabilityVerdict: Needs human review Unable to check for correctness in add72dd. This PR adds substantial new functionality: stderr capture with byte buffering, sensitive data redaction via multiple regex patterns, and new error schema fields. While well-tested and security-conscious, the complexity of the new diagnostic capability and changes to error handling paths warrant human review. You can customize Macroscope's approvability policy. Learn more. |

Summary
codex app-serverchild stderr while continuing to drain stderr for process health.CodexAppServerProcessExitedError, provider status text, or persisted provider status snapshots.Root cause
T3 Code drained child process stderr to avoid blocking protocol responses, but the exit diagnostic path reused that raw stderr tail directly in the process-exited error message. It also snapshotted stderr before awaiting the process exit status, so late stderr chunks could be missed, and the tail helper treated an exact-limit first chunk as truncated even when no bytes were omitted.
Impact
Windows Codex startup failures can still show actionable recent stderr, but secret-shaped values are redacted before the message can flow through provider status or the provider status cache. Shutdown diagnostics now include stderr captured through the process exit path more reliably, and exact-limit stderr tails no longer show a misleading truncation marker.
Validation
PATH="$HOME/.vite-plus/bin:$PATH" vp test packages/effect-codex-app-server/src/client.test.ts packages/effect-codex-app-server/src/_internal/stdio.test.tsPATH="$HOME/.vite-plus/bin:$PATH" vp checkPATH="$HOME/.vite-plus/bin:$PATH" vp run typecheckvp checkreported the repository's existing 20 unrelated warnings and no errors.Refs #2486
Note
Medium Risk
Changes process lifecycle and error messaging on Codex startup failures; redaction reduces secret leakage risk but pattern-based scrubbing could miss or over-redact edge cases.
Overview
Improves Codex app-server child process exit diagnostics by keeping a bounded 4 KiB stderr tail while stderr is still fully drained so large output cannot block the protocol.
makeStderrTailCapturerolls up stderr bytes, redacts secret-shaped content (API keys, Bearer/JWT, URL credentials, etc.), and exposesstderrTail/stderrTruncatedonCodexAppServerProcessExitedError, appended to the error message when present. Exact-limit first chunks are no longer labeled truncated.makeTerminationErrornow reads exit status first, optionally waits up to 50ms for the stderr drain fiber, then snapshots diagnostics. The child-process client wires capture + drain into termination handling instead of only draining stderr.Regression tests cover unit behavior, initialize-before-response exits, and multi-chunk delayed stderr via mock peer env hooks.
Reviewed by Cursor Bugbot for commit add72dd. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Surface stderr tail in Codex app-server child process exit errors
makeStderrTailCaptureinstdio.tsto capture a bounded (4096-byte) tail of stderr, redacting secret-shaped key/value tokens before surfacing them.makeTerminationErrorto accept stderr diagnostics, waiting up to 50ms for stderr to drain before composing the error.CodexAppServerProcessExitedErrorwith optionalstderrTailandstderrTruncatedfields; the error message appends a labeled 'recent stderr' section when content is present.makeChildProcessClientto fork a scoped stderr drain fiber and pass its snapshot intomakeTerminationError.Macroscope summarized add72dd.