[codex] Structure terminal PTY operation failures#3364
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 restructures terminal PTY error handling by splitting a generic error type into specific variants (NotFound, NotDirectory, StatError) and adding structured error handling for write/resize operations. The changes improve error observability without altering core functionality, and include comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 7e5a4d6
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 4219331
Co-authored-by: codex <codex@users.noreply.github.com>
Summary
Verification
vp test apps/server/src/terminal/Manager.test.ts(46 passed)vp check(passes with 20 pre-existing warnings)vp run typecheckNote
Medium Risk
Changes the terminal error contract and PTY failure paths (write/resize/open); callers matching old
TerminalCwdError.reasonmust use new tags, but behavior is more predictable and input is not leaked on write failures.Overview
Replaces the monolithic
TerminalCwdError(reason+ optional cause) withTerminalCwdNotFoundError,TerminalCwdNotDirectoryError, andTerminalCwdStatError(stat failures keep thePlatformErrorascause).TerminalCwdErrorremains a union type forTerminalError.Adds
TerminalWriteErrorandTerminalResizeErrorfor synchronous PTYwrite/resizefailures, withthreadId,terminalId,terminalPid, and resize dimensions; write errors do not include inputdata.TerminalManagerwraps PTY I/O inEffect.tryviaresizePtyProcess, applies resize on open/attach/resize only after success, and logs fullcauseobjects instead oferror.messagestrings. Tests cover cwd variants and PTY I/O error shape.Reviewed by Cursor Bugbot for commit 30a32cf. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure terminal PTY operation failures into typed errors for cwd, write, and resize
TerminalCwdErrorclass with a union ofTerminalCwdNotFoundError,TerminalCwdNotDirectoryError, andTerminalCwdStatError, each with distinct tags and appropriate cause fields.TerminalWriteErrorandTerminalResizeErrortagged errors that carry thread/terminal/PID context and the underlying cause without leaking input data.writeandresizecalls inEffect.try(and a newresizePtyProcesshelper) so failures surface as structured errors instead of thrown exceptions.TerminalErrorunion in terminal.ts to include the two new I/O error types.TerminalCwdErroror caught raw exceptions from PTY operations must now handle the new error variants.Macroscope summarized 30a32cf.