[codex] Preserve child process termination context#3469
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
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)
Comment |
ApprovabilityVerdict: Approved This PR adds optional You can customize Macroscope's approvability policy. Learn more. |
Summary
Why
Termination failures previously retained the exit code or read failure but not the process identity, which made concurrent child-process failures harder to correlate. The existing termination helper remains because it performs an effectful status read and selects between distinct error types; it is not a constructor-only wrapper.
Validation
Note
Low Risk
Additive optional fields on error types and clearer diagnostics; no changes to auth, data handling, or core transport behavior beyond enriched error payloads.
Overview
Child process termination errors now carry the process ID in both
effect-acpandeffect-codex-app-server.makeTerminationErrorpasseshandle.pidintoAcpProcessExitedError/CodexAppServerProcessExitedErroron normal exit and into the corresponding transport error when reading exit status fails; the exit-status failure still uses the originalcauseunchanged.The termination helper now accepts a narrowed
ChildProcessTerminationHandle(pid+exitCodeonly). Error schemas gain an optionalpidfield on process-exited and read-exit-status transport types. Newstdio.test.tsfiles assert PID retention, cause identity, and that user-facing messages stay free of nested diagnostic text.Reviewed by Cursor Bugbot for commit f751e75. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Preserve child process PID in termination errors for ACP and Codex App Server
pidfield toAcpProcessExitedError,AcpTransportError,CodexAppServerProcessExitedError, andCodexAppServerTransportErrorschema classes.makeTerminationErrorin both packages to accept aChildProcessTerminationHandle(typed asPick<ChildProcessHandle, 'exitCode' | 'pid'>) and populatepidon all produced errors.Macroscope summarized f751e75.