Repository navigation
fix(desktop,host-service): classify environmental git failures as typed non-500s - #6260
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughGit environment failures are classified into typed TRPC errors. Desktop Git commands and operations, the Git worker, host-service status handling, staging, and repository initialization use shared error handling. ChangesGit environment error handling
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to This change reclassifies routine environmental Git failures into typed non-500 responses while preserving unexpected failures as server errors. No actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant GitOperation
participant runGitOperation
participant rethrowEnvironmentalGitError
participant TRPCError
GitOperation->>runGitOperation: Execute Git operation
runGitOperation->>rethrowEnvironmentalGitError: Handle caught Git failure
rethrowEnvironmentalGitError->>TRPCError: Map recognized environmental failure
TRPCError-->>GitOperation: Return typed non-500 error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/host-service/src/trpc/router/git/git.ts (1)
197-204: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider extending the wrapper to the other in-process git procedures.
Only
getStatusclassifies environmental failures. Sibling procedures in this router run simple-git in-process against the same worktree and can produce the same messages. Examples:discardChanges(Line 380),discardAllStaged(Line 405),stageAll(Line 457),getDiff, andrenameBranch. Those paths still report a deleted or unreadable worktree as a 500. A small shared helper around the procedure bodies would close the gap without changing behavior for unrelated errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/host-service/src/trpc/router/git/git.ts` around lines 197 - 204, Extend the existing rethrowEnvironmentalGitError handling from getStatus to the sibling in-process git procedures discardChanges, discardAllStaged, stageAll, getDiff, and renameBranch. Wrap each procedure’s simple-git operation so environmental worktree failures are classified by the shared helper, while unrelated errors continue to propagate unchanged.apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree desktop modules define the same environmental-error wrapper. Each module repeats
try { … } catch (error) { rethrowEnvironmentalGitError(error); throw error; }because no shared wrapper is exported next to the classifier. Export one helper fromapps/desktop/src/lib/trpc/routers/workspaces/utils/git-errors.tsand reuse it, so a future change to the boundary behavior applies everywhere at once.
apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts#L27-L36: delete the localrunGitdefinition and import the shared helper under the same local name.apps/desktop/src/lib/trpc/routers/changes/git-operations.ts#L34-L43: delete the localrunGitOperationdefinition and import the shared helper under the same local name.apps/desktop/src/lib/trpc/routers/changes/staging.ts#L25-L34: keepgetStatusOrRethrowfor theassertRegisteredWorktreeand client setup, but replace its inline try/catch with a call to the shared helper aroundgit.status().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts` around lines 27 - 36, Export a shared generic git-command wrapper from apps/desktop/src/lib/trpc/routers/workspaces/utils/git-errors.ts that applies rethrowEnvironmentalGitError and preserves the original error, then reuse it across all sites. In apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts#L27-L36, remove runGit and import the shared helper under that name; in apps/desktop/src/lib/trpc/routers/changes/git-operations.ts#L34-L43, remove runGitOperation and import the helper under that name; in apps/desktop/src/lib/trpc/routers/changes/staging.ts#L25-L34, retain getStatusOrRethrow for setup operations but wrap git.status() with the shared helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts`:
- Around line 119-131: Update the push error handling around runGitOperation and
rethrowEnvironmentalGitError so non-TRPCError failures are classified before any
push-specific wrapping. Preserve existing TRPCError propagation, but let
environmental errors such as missing repositories or inaccessible working
directories produce their classified BAD_REQUEST response with the appropriate
cause.kind.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts`:
- Around line 27-36: Export a shared generic git-command wrapper from
apps/desktop/src/lib/trpc/routers/workspaces/utils/git-errors.ts that applies
rethrowEnvironmentalGitError and preserves the original error, then reuse it
across all sites. In
apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts#L27-L36,
remove runGit and import the shared helper under that name; in
apps/desktop/src/lib/trpc/routers/changes/git-operations.ts#L34-L43, remove
runGitOperation and import the helper under that name; in
apps/desktop/src/lib/trpc/routers/changes/staging.ts#L25-L34, retain
getStatusOrRethrow for setup operations but wrap git.status() with the shared
helper.
In `@packages/host-service/src/trpc/router/git/git.ts`:
- Around line 197-204: Extend the existing rethrowEnvironmentalGitError handling
from getStatus to the sibling in-process git procedures discardChanges,
discardAllStaged, stageAll, getDiff, and renameBranch. Wrap each procedure’s
simple-git operation so environmental worktree failures are classified by the
shared helper, while unrelated errors continue to propagate unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b250b031-4733-4cbf-a7ef-593d6eaac3ff
📒 Files selected for processing (11)
apps/desktop/src/lib/trpc/routers/changes/git-operations.tsapps/desktop/src/lib/trpc/routers/changes/security/git-commands.tsapps/desktop/src/lib/trpc/routers/changes/staging.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git-errors.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git-errors.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/main/git-task-worker.tspackages/host-service/src/trpc/router/git/git.tspackages/host-service/src/trpc/router/git/utils/classify-git-error.test.tspackages/host-service/src/trpc/router/git/utils/classify-git-error.ts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/trpc/router/git/utils/classify-git-error.ts">
<violation number="1" location="packages/host-service/src/trpc/router/git/utils/classify-git-error.ts:26">
P1: A worktree removed during the existing-check-to-spawn race still surfaces as a 500 instead of `NOT_FOUND/{kind: "WORKTREE_MISSING"}` because this classifier only recognizes Git’s stderr wording, not the `spawn git ENOENT` failure produced for a missing cwd. The classification needs worktree context or an existence check to distinguish this case from Git itself being unavailable, then map the missing-worktree case to the typed error.</violation>
</file>
<file name="packages/host-service/src/trpc/router/git/git.ts">
<violation number="1" location="packages/host-service/src/trpc/router/git/git.ts:202">
P2: A worktree deleted during status loses its path in the typed `NOT_FOUND` error, making failures across multiple workspaces harder to diagnose. Preserving `worktreePath` in the classifier context/cause would match the existing `resolveWorktreePath` error.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts:85">
P2: Git-client initialization failures can still surface as 500s because `getGitWithShellPath` runs outside `runGit`; the new classification only covers the subsequent command. Including git-client creation in the same classification boundary would keep these environmental failures as typed non-500 errors.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Desktop Client / Host Client
participant TRPC as tRPC Router
participant GitOps as Git Operations (in-process)
participant GitCmd as security/git-commands.ts
participant Staging as staging.ts
participant GitErrors as git-errors.ts / classify-git-error.ts
participant Worker as Worker Thread
participant SimpleGit as simple-git
participant Sentry as Sentry Error Reporter
Note over Client,Sentry: Desktop: In-Process Git Operations
Client->>TRPC: commit/push/pull/sync/fetch/createPR
TRPC->>GitOps: git-operations.ts
GitOps->>GitCmd: gitSwitchBranch/gitCheckoutFiles/gitStageFiles/etc.
GitCmd->>SimpleGit: raw git command
alt Environmental failure (cwd gone, permission denied, not a repo)
SimpleGit-->>GitCmd: GitError
GitCmd->>GitCmd: runGit() catches error
GitCmd->>GitErrors: rethrowEnvironmentalGitError(error)
alt NotGitRepoError by name or message
GitErrors-->>GitCmd: throw TRPCError(BAD_REQUEST, kind:NOT_GIT_REPO)
else GitEnvironmentError by name or message
GitErrors-->>GitCmd: throw TRPCError(PRECONDITION_FAILED, kind:GIT_ENVIRONMENT)
end
GitCmd-->>TRPC: typed non-500 TRPCError
TRPC-->>Client: error response (non-500)
else Genuine git failure
SimpleGit-->>GitCmd: GitError
GitCmd->>GitErrors: rethrowEnvironmentalGitError(error) → no-op
GitCmd-->>TRPC: original error
TRPC->>Sentry: INTERNAL_SERVER_ERROR reported
TRPC-->>Client: 500 error
else Success
SimpleGit-->>GitCmd: result
GitCmd-->>GitOps: success
GitOps-->>TRPC: success response
TRPC-->>Client: 200 OK
end
Note over Client,Sentry: Desktop: Staging Operations
Client->>TRPC: status/discard/stage operations
TRPC->>Staging: getUntrackedFilePaths/getStagedNewFilePaths
Staging->>SimpleGit: git.status()
alt Environmental failure
SimpleGit-->>Staging: GitError
Staging->>GitErrors: rethrowEnvironmentalGitError(error)
GitErrors-->>Staging: throw typed TRPCError
Staging-->>TRPC: typed non-500
else Success
SimpleGit-->>Staging: status result
Staging-->>TRPC: file paths
end
Note over Client,Sentry: Desktop: Worker Path (branches/status)
Client->>TRPC: getBranches/getStatus
TRPC->>Worker: executeGitTask(task)
Worker->>SimpleGit: git operation
alt Environmental failure
SimpleGit-->>Worker: GitError
Worker->>GitErrors: classifyEnvironmentalGitError(error)
alt NotGitRepoError
GitErrors-->>Worker: NotGitRepoError
else GitEnvironmentError
GitErrors-->>Worker: GitEnvironmentError
end
Worker-->>TRPC: serialized domain error
TRPC->>TRPC: existing branch.ts/status.ts catches
alt NotGitRepoError
TRPC-->>Client: TRPCError(BAD_REQUEST, kind:NOT_GIT_REPO)
else GitEnvironmentError
TRPC-->>Client: TRPCError(PRECONDITION_FAILED, kind:GIT_ENVIRONMENT)
end
end
Note over Client,Sentry: Desktop: initGitAndOpen
Client->>TRPC: initGitAndOpen(path)
TRPC->>GitOps: initGitRepo(path)
GitOps->>SimpleGit: git init
alt Environmental failure
SimpleGit-->>GitOps: error
GitOps->>GitErrors: rethrowEnvironmentalGitError(error)
GitErrors-->>GitOps: throw typed TRPCError
GitOps-->>TRPC: typed non-500
end
Note over Client,Sentry: Host Service: git.getStatus
Client->>TRPC: git.getStatus(workspaceId)
TRPC->>TRPC: resolveWorktreePath(ctx, workspaceId)
TRPC->>Worker: gitStatusSnapshotTask(worktreePath)
Worker->>SimpleGit: git status
alt Environmental failure (worktree vanished between check and spawn)
SimpleGit-->>Worker: GitError
Worker-->>TRPC: serialized error
TRPC->>GitErrors: rethrowEnvironmentalGitError(error)
alt CWD gone (No such file or directory)
GitErrors-->>TRPC: throw TRPCError(NOT_FOUND, kind:WORKTREE_MISSING)
else Permission denied (Operation not permitted)
GitErrors-->>TRPC: throw TRPCError(PRECONDITION_FAILED, kind:GIT_ENVIRONMENT)
else Not a git repository
GitErrors-->>TRPC: throw TRPCError(BAD_REQUEST, kind:NOT_GIT_REPO)
end
TRPC-->>Client: typed non-500 error
else Success
Worker-->>TRPC: status snapshot
TRPC-->>Client: 200 OK
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| cause: { kind: "NOT_GIT_REPO" }, | ||
| }); | ||
| } | ||
| if (CWD_GONE_PATTERN.test(error.message)) { |
Contributor
There was a problem hiding this comment.
P1: A worktree removed during the existing-check-to-spawn race still surfaces as a 500 instead of NOT_FOUND/{kind: "WORKTREE_MISSING"} because this classifier only recognizes Git’s stderr wording, not the spawn git ENOENT failure produced for a missing cwd. The classification needs worktree context or an existence check to distinguish this case from Git itself being unavailable, then map the missing-worktree case to the typed error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/git/utils/classify-git-error.ts, line 26:
<comment>A worktree removed during the existing-check-to-spawn race still surfaces as a 500 instead of `NOT_FOUND/{kind: "WORKTREE_MISSING"}` because this classifier only recognizes Git’s stderr wording, not the `spawn git ENOENT` failure produced for a missing cwd. The classification needs worktree context or an existence check to distinguish this case from Git itself being unavailable, then map the missing-worktree case to the typed error.</comment>
<file context>
@@ -0,0 +1,40 @@
+ cause: { kind: "NOT_GIT_REPO" },
+ });
+ }
+ if (CWD_GONE_PATTERN.test(error.message)) {
+ throw new TRPCError({
+ code: "NOT_FOUND",
</file context>
| // arrives as a plain error — classify it by message here. The | ||
| // worktree can vanish between resolveWorktreePath's existsSync | ||
| // check and the git spawn. | ||
| rethrowEnvironmentalGitError(error); |
Contributor
There was a problem hiding this comment.
P2: A worktree deleted during status loses its path in the typed NOT_FOUND error, making failures across multiple workspaces harder to diagnose. Preserving worktreePath in the classifier context/cause would match the existing resolveWorktreePath error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/git/git.ts, line 202:
<comment>A worktree deleted during status loses its path in the typed `NOT_FOUND` error, making failures across multiple workspaces harder to diagnose. Preserving `worktreePath` in the classifier context/cause would match the existing `resolveWorktreePath` error.</comment>
<file context>
@@ -157,41 +158,50 @@ export const gitRouter = router({
+ // arrives as a plain error — classify it by message here. The
+ // worktree can vanish between resolveWorktreePath's existsSync
+ // check and the git spawn.
+ rethrowEnvironmentalGitError(error);
+ throw error;
+ }
</file context>
| await git.checkout(branch); | ||
| } else { | ||
| throw switchError; | ||
| await runGit(() => |
Contributor
There was a problem hiding this comment.
P2: Git-client initialization failures can still surface as 500s because getGitWithShellPath runs outside runGit; the new classification only covers the subsequent command. Including git-client creation in the same classification boundary would keep these environmental failures as typed non-500 errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts, line 85:
<comment>Git-client initialization failures can still surface as 500s because `getGitWithShellPath` runs outside `runGit`; the new classification only covers the subsequent command. Including git-client creation in the same classification boundary would keep these environmental failures as typed non-500 errors.</comment>
<file context>
@@ -70,28 +82,30 @@ export async function gitSwitchBranch(
- await git.checkout(branch);
- } else {
- throw switchError;
+ await runGit(() =>
+ runWithPostCheckoutHookTolerance({
+ context: `Switched branch to "${branch}" in ${worktreePath}`,
</file context>
This was referenced Aug 10, 2026
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
# Conflicts: # apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
…fore the push-specific mapping
This was referenced Aug 13, 2026
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Environmental git failures — the worktree directory deleted out from under a running git process, macOS folder permissions denying reads (
fatal: Unable to read current working directory: Operation not permitted/No such file or directory), or a directory that is not a git repository — propagate out of simple-git as plainGitErrors. The tRPC Sentry middleware reports everyINTERNAL_SERVER_ERROR, so these routine lifecycle states report as bugs. DESKTOP-1A alone is ~5,300 events/day and the largest quota driver on the desktop project.Root cause
The typed classification from #6164/#6199 only covers
existsSyncpre-checks and the exec-basedgetStatusNoLock. simple-git's ownGitErrorfor these environmental failures maps to nothing:changes.getBranches→ DESKTOP-HQ): the worker funneltranslateRawGitErroringit-task-worker.tsonly classified "not a git repository"; the cwd-unreadable variants crossed the boundary as rawGitErrorand missed the domain catches inbranches.ts/status.ts.changes.commit/push/pull/sync/fetch/createPR,changes.stageFiles/discardChanges/discardFiles/stashviasecurity/git-commands.ts,projects.initGitAndOpen): simple-git runs in the main process with no message classification at all.git.getStatus→ HOST-SERVICE-X/Y):resolve-worktree.tsonly has theexistsSyncpre-check; the worktree can vanish between that check and the git spawn, and permission walls pass the check entirely.Fix
Classification at throw/capture sites only — no Sentry-side filtering. Existing error messages are preserved verbatim; genuine unexpected git failures keep reporting as 500s.
desktop (
workspaces/utils/git-errors.ts, one shared helper pair):classifyEnvironmentalGitErrormessage-checks a caught git error ("Unable to read current working directory" either variant →GitEnvironmentError; "not a git repository" →NotGitRepoError). The worker funnel now uses it, so every worker task crosses the boundary in the domain vocabulary and the existing catches inbranches.ts/status.tstranslate to the existing typed non-500 TRPCErrors (PRECONDITION_FAILED/{kind:"GIT_ENVIRONMENT"},BAD_REQUEST/{kind:"NOT_GIT_REPO"}).rethrowEnvironmentalGitErrorapplies the same classification at the in-process boundaries (git-operations.tsprocedures,security/git-commands.tscommand helpers,staging.tsstatus helpers,projects.initGitAndOpen), rethrowing the same typed TRPCErrors directly. Domain errors are matched by name (neverinstanceof) since worker serialization strips prototypes.NotGitRepoErrormoved fromgit.tsintogit-errors.ts(re-exported fromgit.ts, so all existing importers andinstanceofchecks are unchanged) to avoid an import cycle.host-service (
trpc/router/git/utils/classify-git-error.ts, matching the #6199 pattern):rethrowEnvironmentalGitErrorapplied at thegit.getStatusboundary: cwd gone →NOT_FOUND/{kind:"WORKTREE_MISSING"}, permission wall →PRECONDITION_FAILED/{kind:"GIT_ENVIRONMENT"}, not a repo →BAD_REQUEST/{kind:"NOT_GIT_REPO"}.Unit tests cover both helpers (all three message variants, name-checked domain errors, and the no-op path for genuine failures); verified they fail under mutation of the classification.
Verification
bunx biome check <11 changed files>:cd apps/desktop && bun run typecheck:cd packages/host-service && bun run typecheck:bun test(desktop: newgit-errors.test.ts+ existinggit-task-handlers.test.ts; host-service: newclassify-git-error.test.ts+ existingget-check-job-logs.test.ts):Refs DESKTOP-HQ
Refs DESKTOP-1A
Refs HOST-SERVICE-X
Refs HOST-SERVICE-Y
https://claude.ai/code/session_9a505eb2-10a1-4f40-910d-5ee0d7f2e0b4
Summary by cubic
Classifies environmental
simple-gitfailures as typed non-500@trpc/servererrors in Desktop and Host Service. Previously these routine states returned 500s and were reported to Sentry; now they return BAD_REQUEST, PRECONDITION_FAILED, or NOT_FOUND with cause kinds, while unexpected failures stay 500s. Messages are preserved verbatim.Desktop
classifyEnvironmentalGitErrorandrethrowEnvironmentalGitError; applies via newrunGitOperation/runGitwrappers across commit/push/pull/sync/fetch/createPR, staging status reads, andprojects.initGitAndOpen.cause.kind.classifyEnvironmentalGitError; movesNotGitRepoErrortogit-errors.tsand re-exports. Tests added ingit-errors.test.ts; tightensno-main-process-blocking.test.tsafter consolidation.Host Service
git.getStatuswithrethrowEnvironmentalGitError: CWD gone → NOT_FOUND/{kind:"WORKTREE_MISSING"}; permission wall → PRECONDITION_FAILED/{kind:"GIT_ENVIRONMENT"}; not-a-repo → BAD_REQUEST/{kind:"NOT_GIT_REPO"}. Tests added inclassify-git-error.test.ts.No client migration required; codes and
cause.kindare now consistent across worker and in-process paths.Written for commit 387e021. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests