[codex] Structure dev-runner failures - #3283
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 Refactors error handling in dev-runner scripts by replacing a generic error class with specific typed error classes. Changes are confined to development tooling with comprehensive tests and no production behavior impact. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate c1295f1
Summary
Validation
vp test scripts/dev-runner.test.ts(26 tests)vp checkvp run typecheckNote
Medium Risk
Behavior change for anything matching the old
DevRunnerErrortype; dev tooling only, but spawn/exit error handling paths are refactored.Overview
Replaces the single generic
DevRunnerErrormessage bag with five Schema-tagged error types (DevRunnerConfigurationError,DevRunnerInvalidPortOffsetError,DevRunnerPortExhaustedError,DevRunnerProcessError,DevRunnerProcessExitError) plus aDevRunnerErrorunion andisDevRunnerErrorhelper.resolveOffsetnow returns anEffectand fails withDevRunnerInvalidPortOffsetErrorinstead of throwing. Port search failures useDevRunnerPortExhaustedErrorwith structured range and port-requirement fields.runDevRunnerWithInputmaps config read failures, spawn/wait-for-exit platform errors, and non-zero exits to the matching typed errors—preserving underlying causes where appropriate, attaching mode/executable/argument count/shell context, and avoiding argv or cause text in user-facing messages (including sensitiverunArgs).Tests are extended for invalid offsets, port exhaustion, and full
runDevRunnerWithInputerror shapes via mockedNetServiceandChildProcessSpawner.Reviewed by Cursor Bugbot for commit c1295f1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace generic dev-runner errors with structured error classes in
runDevRunnerWithInputDevRunnerConfigurationError,DevRunnerInvalidPortOffsetError,DevRunnerPortExhaustedError,DevRunnerProcessError, andDevRunnerProcessExitError— each carrying structured fields (config keys, port ranges, process context, exit code, etc.).resolveOffsetis refactored from a throwing function into an Effect that fails withDevRunnerInvalidPortOffsetError; callers must handle the Effect.runDevRunnerWithInputnow maps configuration read failures, spawn failures, wait-for-exit failures, and non-zero exits to their respective typed errors instead of collapsing them into a single generic error.resolveOffsetmust now treat it as an Effect; the previous throwing behavior is removed.Macroscope summarized c1295f1.