[codex] Keep PTY spawn errors structural#3325
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 Minor change to error message formatting that removes duplication of cause messages while preserving the full error cause chain. The added test verifies the structural error handling remains intact. You can customize Macroscope's approvability policy. Learn more. |
Summary
PtySpawnError.messageexclusively from adapter and shell contextValidation
vp test apps/server/src/terminal/PtyAdapter.test.tsvp check(passes with 20 pre-existing warnings)vp run typecheckNote
Low Risk
Localized change to error string formatting for PTY spawn failures; cause chaining and spawn behavior are unchanged.
Overview
PtySpawnError.messageno longer appends the nestedcause’s error text. User-facing strings are built only fromadapter, optionalshell, and optionalattemptedShells(e.g. terminal-manager retry lists).The
causefield is unchanged and still chains the original spawn failure for stacks and diagnostics—only the duplicated message inmessageis removed.Adds
PtyAdapter.test.tsto lock in those messages and assertmanagerError.cause → adapterError.cause → spawnCause.Reviewed by Cursor Bugbot for commit eeefcaa. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Remove cause message concatenation from
PtySpawnError.messagePtySpawnError.messagepreviously appended the underlying cause's message string, mixing structured error context with raw exception text. The message now includes only the adapter, shell, and attemptedShells fields; the full cause chain remains accessible via thecauseproperty. Tests in PtyAdapter.test.ts verify the new message format and cause chain behavior.Macroscope summarized eeefcaa.