Skip to content

[codex] structure VCS process errors - #3408

Merged
juliusmarminge merged 2 commits into
mainfrom
codex/structure-vcs-process-errors
Jun 20, 2026
Merged

[codex] structure VCS process errors#3408
juliusmarminge merged 2 commits into
mainfrom
codex/structure-vcs-process-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

What changed

  • keep VCS command arguments and stderr contents out of process errors
  • add argument count, stderr length, and truncation metadata for correlation
  • retain exact spawn causes while keeping public messages stable
  • remove the command-label helper that joined raw arguments into diagnostics

Why

VCS arguments and stderr can contain credentials, repository URLs, and arbitrary remote output. The process boundary should expose useful structure without copying those values into errors or messages.

Validation

  • vp test apps/server/src/vcs/VcsProcess.test.ts (8 tests)
  • vp check
  • vp run typecheck

Note

Medium Risk
Changes error shape and messaging for all VCS subprocess failures; callers matching on full command or stderr in detail may need updates, but behavior is intentional for security.

Overview
VCS process errors no longer leak credentials or remote output in command or message. Diagnostics use the bare executable name plus optional argumentCount, and exit failures add failureKind, stderrLength, and stderrTruncated instead of embedding raw stderr in detail.

Non-zero exits are classified via classifyNonZeroExit into authentication, not-found (gh/glab/az heuristics), or command-failed, with stable user-facing detail strings from VcsProcessExitError.fromProcessExit. Spawn errors still keep the underlying cause while omitting args from the public message.

Breaking for callers that relied on joined command labels or stderr text in detail/message; tests assert secrets never appear in messages.

Reviewed by Cursor Bugbot for commit c7aa6a8. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Structure VCS process errors with failure classification and secret redaction

  • VcsProcessExitError now carries a failureKind (authentication, not-found, command-failed), stderrLength, stderrTruncated, and argumentCount, with a standardized detail message; stderr contents are no longer stored in the error instance.
  • Adds classifyNonZeroExit in VcsProcess.ts to detect auth and not-found failures from stderr for gh, glab, and az CLIs.
  • VcsProcessSpawnError, VcsProcessTimeoutError, and VcsOutputDecodeError all gain an optional argumentCount field via their factory methods.
  • Behavioral Change: process arguments are no longer included in error messages or command labels, preventing accidental secret leakage in logs or error reporting.

Macroscope summarized c7aa6a8.

Co-authored-by: codex <codex@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d7a6b330-3e0d-4b3f-ac5b-da33d9c03d06

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/structure-vcs-process-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jun 20, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: CLI error mapping breaks
    • Restored stderr content in VcsProcessExitError's detail field (falling back to the generic message when stderr is empty) so downstream CLI normalizers can again scan detail/message for auth, not-found, and similar patterns.

Create PR

Or push these changes by commenting:

@cursor push 4afa35e674
Preview (4afa35e674)
diff --git a/apps/server/src/vcs/VcsProcess.test.ts b/apps/server/src/vcs/VcsProcess.test.ts
--- a/apps/server/src/vcs/VcsProcess.test.ts
+++ b/apps/server/src/vcs/VcsProcess.test.ts
@@ -85,12 +85,11 @@
         command: "node",
         argumentCount: 4,
         exitCode: 2,
-        detail: "Process exited with a non-zero status.",
+        detail: secretStderr,
         stderrLength: secretStderr.length,
         stderrTruncated: false,
       });
       expect(error.message).not.toContain(secretArgument);
-      expect(error.message).not.toContain(secretStderr);
     }).pipe(provideLive),
   );
 

diff --git a/apps/server/src/vcs/VcsProcess.ts b/apps/server/src/vcs/VcsProcess.ts
--- a/apps/server/src/vcs/VcsProcess.ts
+++ b/apps/server/src/vcs/VcsProcess.ts
@@ -96,7 +96,7 @@
       return yield* new VcsProcessExitError({
         ...baseError,
         exitCode: result.code,
-        detail: "Process exited with a non-zero status.",
+        detail: result.stderr.trim() || "Process exited with a non-zero status.",
         stderrLength: result.stderr.length,
         stderrTruncated: result.stderrTruncated,
       });

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 6b365df. Configure here.

Comment thread apps/server/src/vcs/VcsProcess.ts
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Security-focused refactor that restructures VCS process errors to prevent sensitive data (tokens, credentials) from leaking into error messages. Changes are well-scoped to error handling with explicit test coverage verifying secrets are not exposed.

You can customize Macroscope's approvability policy. Learn more.

Co-authored-by: codex <codex@users.noreply.github.com>
@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jun 20, 2026
@juliusmarminge
juliusmarminge merged commit 731b1a6 into main Jun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/structure-vcs-process-errors branch June 20, 2026 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant