[codex] Structure terminal adapter startup defects - #3425
Conversation
|
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 |
Co-authored-by: codex <codex@users.noreply.github.com>
e991116 to
5681b5c
Compare
ApprovabilityVerdict: Approved This PR converts unstructured error strings into typed error classes for terminal adapter startup failures. The control flow and error scenarios remain unchanged - this is a structural improvement to error handling that improves testability without affecting runtime behavior. You can customize Macroscope's approvability policy. Learn more. |
…ctured-errors-b [codex] Structure terminal adapter startup defects (pingdotgg#3425)
Summary
Terminal adapter startup failures were not consistently diagnosable. Bun's unsupported Windows path died with a bare string, while a rejected
node-ptynative-module import surfaced as an unstructured defect without host context.This change preserves the existing fatal startup semantics while making both defects structured. The Bun defect records the unsupported platform. The Node defect records platform and architecture and retains the exact module-load rejection as
cause, keeping the original stack chain intact. The external module loader is injectable so the failure boundary can be tested without mocking core logic.Validation
vp test apps/server/src/terminal/BunPtyAdapter.test.ts apps/server/src/terminal/NodePtyAdapter.test.tsvp check(passes with existing repository warnings)vp run typecheckNote
Low Risk
Behavior remains fatal startup defects; changes are limited to error shape, diagnostics, and testability in terminal adapters.
Overview
Terminal adapter startup still fails fatally on unsupported Bun hosts and when
node-ptycannot load, but those paths now die with typedSchema.TaggedErrorClassdefects instead of bare strings or unstructured rejections.Bun:
BunPtyAdapter.makeonwin32dies withBunPtyUnsupportedPlatformError(platform + user-facing message). Node:NodePtyAdapter.makewraps a failed dynamic import inNodePtyModuleLoadError(platform, architecture, originalcause) viaEffect.tryPromise+orDie. An optionalNodePtyModuleLoaderhook lets tests simulate load failure without mocking internals.New effect tests assert the squashed defect shape for both adapters.
Reviewed by Cursor Bugbot for commit 5681b5c. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure terminal adapter startup defects with typed error classes
BunPtyUnsupportedPlatformErrorto BunPtyAdapter.ts: a tagged Schema error with aplatformfield, replacing the plain string die on win32.NodePtyModuleLoadErrorto NodePtyAdapter.ts: a tagged Schema error carryingplatform,architecture, and the underlying cause, thrown whennode-ptyfails to load.NodePtyAdapter.makenow accepts an optional module loader, enabling the new load-failure path to be tested without native modules.Macroscope summarized 5681b5c.