[codex] Enrich Git VCS driver errors - #3253
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 refactors Git VCS driver error handling to provide more structured error metadata while removing raw command arguments and stderr from error messages (a security improvement). The changes don't affect git operation logic, only error reporting, and include comprehensive test updates. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate bb03e7a
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate f9485b2
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate c4d0cd4
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Non-git cwd status regression
- Added stderr substring checks for 'not a git repository' in both readStatusDetailsLocal and readStatusDetailsRemote before the generic GitCommandError throw, returning the non-repository sentinel values so GitManager correctly identifies non-git directories.
Or push these changes by commenting:
@cursor push 51cf41d948
Preview (51cf41d948)
diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts
--- a/apps/server/src/vcs/GitVcsDriverCore.ts
+++ b/apps/server/src/vcs/GitVcsDriverCore.ts
@@ -1196,6 +1196,9 @@
return NON_REPOSITORY_REMOTE_STATUS_DETAILS;
}
if (branchResult.exitCode !== 0) {
+ if (branchResult.stderr.toLowerCase().includes("not a git repository")) {
+ return NON_REPOSITORY_REMOTE_STATUS_DETAILS;
+ }
return yield* new GitCommandError({
...gitCommandContext({
operation: "GitVcsDriver.statusDetailsRemote.branch",
@@ -1316,6 +1319,9 @@
}
if (statusResult.exitCode !== 0) {
+ if (statusResult.stderr.toLowerCase().includes("not a git repository")) {
+ return NON_REPOSITORY_STATUS_DETAILS;
+ }
return yield* new GitCommandError({
...gitCommandContext({
operation: "GitVcsDriver.statusDetails.status",You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit c4d0cd4. Configure here.
Co-authored-by: codex <codex@users.noreply.github.com>
Addresses review: drop `stderrTail` and the message change entirely, and log the output at the failure site instead. The first attempt put git's stderr on `GitCommandError` as a bounded, redacted attribute. That was wrong on three counts, all correctly flagged: it reintroduces raw command output into a value that crosses RPC, UI and persistence boundaries; it makes `message` unbounded rather than derived from stable structural attributes; and the redaction only covered `//user:pass@` remotes, missing single-token URLs, query-string tokens, and anything git echoes back from an argument or hook. It also regressed the existing test asserting a secret passed as a git argument never reaches `error.message`, and effectively reverted pingdotgg#3253, which removed stderr from this error for exactly these reasons. The error's public shape is now untouched. Both non-zero-exit sites call `Effect.logDebug` with the command context and its stdout/stderr, so the diagnostic is reachable when someone goes looking without becoming part of the serialized error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>


Summary
Validation
vp test apps/server/src/vcs/GitVcsDriverCore.test.ts apps/server/src/git/GitManager.test.ts(82 tests)vp checkvp run typecheck