[codex] Structure GitHub CLI failures#3456
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 |
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: Spawn failures misreported unavailable
- Added an isEnoentCause check so only ENOENT spawn failures map to GitHubCliUnavailableError; other spawn failures (EACCES, etc.) now correctly fall through to GitHubCliCommandError.
Or push these changes by commenting:
@cursor push 05040000e0
Preview (05040000e0)
diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts
--- a/apps/server/src/sourceControl/GitHubCli.ts
+++ b/apps/server/src/sourceControl/GitHubCli.ts
@@ -161,6 +161,14 @@
export const isGitHubCliError = Schema.is(GitHubCliError);
+function isEnoentCause(cause: unknown): boolean {
+ if (typeof cause === "object" && cause !== null && "code" in cause) {
+ return (cause as { code: unknown }).code === "ENOENT";
+ }
+ const text = String(cause).toLowerCase();
+ return text.includes("enoent") || text.includes("command not found");
+}
+
export function fromVcsError(
context: {
readonly operation: "execute";
@@ -170,7 +178,10 @@
error: VcsError,
): GitHubCliError {
if (error._tag === "VcsProcessSpawnError") {
- return new GitHubCliUnavailableError({ ...context, cause: error });
+ if (isEnoentCause(error.cause)) {
+ return new GitHubCliUnavailableError({ ...context, cause: error });
+ }
+ return new GitHubCliCommandError({ ...context, cause: error });
}
if (error._tag === "VcsProcessExitError") {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit e04ad5d. Configure here.
There was a problem hiding this comment.
Effect Service Conventions — one finding on the new GitHubCli error model. The split into distinct tagged errors, the required cause, the Schema.Union + Schema.is predicate, and message-from-attributes are all good; the only issue is a redundant single-value operation discriminator carried alongside each distinct tag.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Approved This PR refactors GitHub CLI error handling by replacing a generic error class with specialized typed error classes. The changes are mechanical restructuring of error classification with comprehensive test updates, maintaining the same user-facing error messages. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
60acd3a to
dbba925
Compare
dbba925 to
a9a11db
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
a9a11db to
81dd825
Compare


Summary
Validation
vp test apps/server/src/sourceControl/GitHubCli.test.ts apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts apps/server/src/git/GitManager.test.ts(75 tests)vp test apps/server/src/sourceControl/GitHubCli.test.ts apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts(16 tests)vp check(20 pre-existing warnings)vp run typecheckNote
Medium Risk
Touches GitHub CLI error handling used by git workflows and source-control provider mapping; behavior should stay the same for callers reading
detail, but misclassification of spawn vs auth vs not-found could change surfaced messages.Overview
Replaces the single free-form
GitHubCliErrorwith distinct Schema tagged errors (unavailable, authentication, PR not found, command failure, and JSON decode failures for PR lists, change requests, single PRs, and repositories). User-facing guidance now lives on each class’sdetailgetter; failures carrycommand,cwd, andcauseinstead ofoperation+detailstrings.Classification no longer inspects error message text.
fromVcsErrormapsVcsProcessSpawnError(ChildProcess spawnNotFoundonly) to unavailable, andVcsProcessExitErrorwithfailureKindauthentication/not-foundto the matching types; everything else becomesGitHubCliCommandError. A new unit test asserts a missing cwd (FileSystemNotFound) is not treated as a missingghbinary.Call sites and tests use
GitHubCliErroras a union type,isGitHubCliError, and the specific error classes.GitHubSourceControlProvidermaps any failure toSourceControlProviderErrorviaEffect.mapErrorusingerror.detail, and emitsGitHubChangeRequestListDecodeErrorfor bad non-open list JSON.GitManagerfakes and integration tests were updated to the new shapes (e.g.GitHubCliUnavailableError/GitHubCliAuthenticationError).Reviewed by Cursor Bugbot for commit 81dd825. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace string-matched GitHub CLI errors with structured error classes in
GitHubCliGitHubCli.ts:GitHubCliUnavailableError,GitHubCliAuthenticationError,GitHubPullRequestNotFoundError,GitHubCliCommandError, and decode-specific errors for PR lists, single PRs, repositories, and change requests.fromVcsErrormapper that usesVcsErrordata (spawnNotFound, exitfailureKind) to select the appropriate error class.GitHubCliErrorunion type andisGitHubCliErrortype guard so callers can identify GitHub CLI errors without inspecting raw error shapes.GitHubSourceControlProviderto map any error intoSourceControlProviderErrorvia a singleEffect.mapError, removing the previousEffect.catchTagson the oldGitHubCliError.Macroscope summarized 81dd825.