[codex] Structure process diagnostics failures#3389
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 structured diagnostic fields to error classes for better error reporting. The changes are additive, don't alter control flow, include tests, and are authored by the file's primary maintainer. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 44734fa
Summary\n- attach bounded command metadata, working directory, timeout, exit status, and output byte/truncation counts to diagnostics failures\n- preserve exact spawn causes without deriving messages from them or retaining raw process streams\n- include both target and server PIDs when rejecting non-descendant processes
Tests
vp test apps/server/src/diagnostics/ProcessDiagnostics.test.tsvp checkvp run typecheckNote
Low Risk
Changes are confined to diagnostics error shaping and messages; behavior of successful reads/signals is unchanged, though alert rules matching old error strings may need updates.
Overview
Process diagnostics failures now carry structured fields instead of relying on stderr text for messages.
Query errors (
ProcessDiagnosticsQueryTimeoutError,ProcessDiagnosticsQueryFailedError) gainargCount,cwd, and (for timeouts)timeoutMillis; failed queries also recordexitCodeplus bounded stdout/stderr byte counts and truncation flags. User-facing messages mention cwd, timeout, and exit code—not raw stderr.runProcesscaptures cwd and stream collection metadata once and threads it into both POSIX (ps) and Windows (powershell.exe) failure paths.ProcessDiagnosticsNotDescendantErrornow includesserverPidalongside the targetpid.A new test asserts failed
psexits produce the expected structured error and message format.Reviewed by Cursor Bugbot for commit 44734fa. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure process diagnostics errors with bounded command metadata
ProcessDiagnosticsQueryFailedErroris extended withargCount,cwd,exitCode,stdoutBytes,stderrBytes,stdoutTruncated, andstderrTruncatedfields; the raw stderr string is removed from the error.ProcessDiagnosticsQueryTimeoutErrorgainsargCount,cwd, andtimeoutMillisfields with an updated message format.ProcessDiagnosticsNotDescendantErrorgains aserverPidfield.ProcessOutputinterface andrunProcessfunction in ProcessDiagnostics.ts now capturecwdand output byte counts/truncation flags on every execution.stderrfield ofProcessDiagnosticsQueryFailedErrorwill need to usestderrBytesinstead.Macroscope summarized 44734fa.