Repository navigation
fix(host-service): classify missing-worktree git errors as NOT_FOUND instead of 500 - #6199
Merged
Merged
Conversation
…instead of 500
When a workspace's worktree directory is deleted externally (or cleanup
races a poll), every git.* procedure hit simple-git's GitConstructError
("Cannot use simple-git on a directory that does not exist") and escaped
as INTERNAL_SERVER_ERROR into Sentry once per batched procedure per poll.
Guard the shared resolveWorktreePath boundary with an existence check and
throw a typed NOT_FOUND with cause { kind: "WORKTREE_MISSING" }, matching
the desktop v1 changes-router convention. Also route the legacy
workspace.gitStatus surface through the same helper.
Fixes HOST-SERVICE-W
Claude-Session: https://claude.ai/code/session_01JAJWZrVSdXGbsfiGvwCyKW
Contributor
📝 WalkthroughWalkthroughThe worktree resolver now verifies that the path exists on disk. ChangesWorktree validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
Contributor
Greptile SummaryThis PR classifies absent host-service worktrees as
Confidence Score: 4/5The remaining cleanup race should be fixed before merging because a worktree can still disappear after the new guard and produce the same reportable server error. The existence check and the git command are not atomic or synchronized, so concurrent cleanup can still bypass the new Files Needing Attention: packages/host-service/src/trpc/router/git/utils/resolve-worktree.ts
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/git/utils/resolve-worktree.ts | Adds the missing-worktree classification, but the standalone existence check does not cover deletion between resolution and the git operation. |
| packages/host-service/src/trpc/router/workspace/workspace.ts | Routes legacy git status through the shared resolver, inheriting both its intended classification and its remaining race window. |
Prompt To Fix All With AI
### Issue 1
packages/host-service/src/trpc/router/git/utils/resolve-worktree.ts:23
**Worktree check leaves race window**
If workspace cleanup or external deletion occurs after `existsSync` succeeds but before the git command starts, the unprotected path reaches simple-git and the missing directory still escapes as an internal server error, producing the repeated Sentry events this guard is intended to prevent.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(host-service): classify missing-work..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // A worktree deleted outside the app is a routine lifecycle state, not a | ||
| // bug — classify it here so simple-git's construct error can't escape any | ||
| // git.* procedure as a reportable 500. | ||
| if (!existsSync(workspace.worktreePath)) { |
Contributor
There was a problem hiding this comment.
If workspace cleanup or external deletion occurs after existsSync succeeds but before the git command starts, the unprotected path reaches simple-git and the missing directory still escapes as an internal server error, producing the repeated Sentry events this guard is intended to prevent.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/git/utils/resolve-worktree.ts
Line: 23
Comment:
**Worktree check leaves race window**
If workspace cleanup or external deletion occurs after `existsSync` succeeds but before the git command starts, the unprotected path reaches simple-git and the missing directory still escapes as an internal server error, producing the repeated Sentry events this guard is intended to prevent.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![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
🤖 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 `@packages/host-service/src/trpc/router/workspace/workspace.ts`:
- Around line 134-135: Update the workspace flow around resolveWorktreePath and
ctx.git so missing-worktree errors raised by git.status(), including the removal
race after path resolution, are translated to the expected missing-worktree
response instead of INTERNAL_SERVER_ERROR. Handle only this missing-worktree
case, preferably in the shared ctx.git factory if appropriate, and add a
regression test covering removal between resolution and status.
🪄 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: bf28fe0e-75bb-4434-bd61-1f328e83a3f0
📒 Files selected for processing (2)
packages/host-service/src/trpc/router/git/utils/resolve-worktree.tspackages/host-service/src/trpc/router/workspace/workspace.ts
Comment on lines
+134
to
+135
| const worktreePath = resolveWorktreePath(ctx, input.id); | ||
| const git = await ctx.git(worktreePath); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'ctx\.git|simple-git|git\.status\(\)|WORKTREE_MISSING|INTERNAL_SERVER_ERROR' \
packages/host-service/srcRepository: superset-sh/superset
Length of output: 50377
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== workspace gitStatus =="
sed -n '100,170p' packages/host-service/src/trpc/router/workspace/workspace.ts
echo "== candidate git simple-file factories =="
fd -a 'simple-git|git-env|git' packages/host-service/src/runtime packages/host-service/src/db packages/host-service/src/trpc | sed 's#^\./##' | head -80
echo "== locate create git factory definitions/usages =="
rg -n -C 5 'createUserSimpleGit|ctx\.git\(|git\(:Promise<|git:|GitEnvResolver|GitTaskEnv' packages/host-service/src/runtime packages/host-service/src/trpc packages/host-service/src/db --glob '*.ts' | head -300
echo "== error types and constants =="
rg -n -C 5 'NOT_FOUND|PRECONDITION_FAILED|INTERNAL_SERVER_ERROR|ERR_*|ENOENT|ENOTDIR' packages/host-service/src/trpc packages/host-service/src/runtime packages/host-service/src/runtime/git* --glob '*.ts' | head -300Repository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
# Behavioral probe: Node fs.stat existsSync -> stat operation exists while other process deletes target.
# Shows why Line 134 preflight does not eliminate the race before ctx.git(worktreePath) / git.status().
import os
import tempfile
path = tempfile.mkdtemp()
os.rmdir(path)
exists_before = os.path.exists(path)
try:
fd = os.open(path, os.O_RDONLY|os.O_NONBLOCK)
except FileNotFoundError as e:
exists_after = False
error_type = type(e).__name__
else:
os.close(fd)
exists_after = os.path.exists(path)
error_type = "no error in this path"
print({"path": path, "exists_before": exists_before, "exists_after": exists_after, "error_type": error_type})
PYRepository: superset-sh/superset
Length of output: 269
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== git factory and runtime config =="
sed -n '1,90p' packages/host-service/src/runtime/git/git.ts
rg -n -C 4 'createGitFactory|ctx\.git\s*=|gitFactory|HostServiceContext' packages/host-service/src --glob '*.ts'
echo "== workspace path resolver =="
rg -n -C 8 'function resolveWorktreePath|const resolveWorktreePath|resolveWorktreePath' packages/host-service/src --glob '*.ts'
echo "== git test coverage around missing repo =="
sed -n '1,140p' packages/host-service/src/runtime/git/git.test.ts
sed -n '1,130p' packages/host-service/src/runtime/git/simple-git.test.ts
sed -n '1,180p' packages/host-service/src/trpc/router/git/utils/git-status.integration.test.tsRepository: superset-sh/superset
Length of output: 50377
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
# Behavioral probe: simple-git's status operation is spawned after client construction and can
# observe ENOENT if the directory is removed between constructor and status execution.
print("Node ENOENT can occur on filesystem operations scheduled after the path exists")
print("createUserSimpleGit calls simpleGit(baseDir), but git.status() is the later filesystem operation")
PYRepository: superset-sh/superset
Length of output: 334
Translate missing-worktree errors from git.status().
gitStatus can still raise INTERNAL_SERVER_ERROR when another process removes worktreePath after resolveWorktreePath and before or during git.status(). Translate only the missing-worktree case here or in the shared ctx.git factory, and add a race regression test.
🤖 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/workspace/workspace.ts` around lines
134 - 135, Update the workspace flow around resolveWorktreePath and ctx.git so
missing-worktree errors raised by git.status(), including the removal race after
path resolution, are translated to the expected missing-worktree response
instead of INTERNAL_SERVER_ERROR. Handle only this missing-worktree case,
preferably in the shared ctx.git factory if appropriate, and add a regression
test covering removal between resolution and status.
Source: MCP tools
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.
1 issue found across 2 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/resolve-worktree.ts">
<violation number="1" location="packages/host-service/src/trpc/router/git/utils/resolve-worktree.ts:23">
P2: The existsSync guard classifies the missing-worktree case only when the directory is already gone at check time. Because simple-git is constructed afterwards (ctx.git(worktreePath) / createUserSimpleGit), a worktree deleted in the tiny window between this check and the git construct still throws GitConstructError and escapes as a reportable 500 — the exact symptom (HOST-SERVICE-W) this PR is meant to stop. The added comment's claim that the construct error "can't escape any git.* procedure" is stronger than what the check delivers. Consider classifying at the git-construct boundary instead (wrap the simple-git construction so GitConstructError on a missing worktree becomes NOT_FOUND/WORKTREE_MISSING), which removes the race window entirely.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Poller as Git Poll Loop
participant TRPC as tRPC Router (git.*)
participant Resolve as resolveWorktreePath
participant DB as SQLite Database
participant FS as Filesystem
participant SimpleGit as simple-git
participant Sentry as Sentry Middleware
participant Renderer as React Renderer
Note over Poller,Renderer: Happy Path — Worktree exists on disk
Poller->>TRPC: poll git.getStatus / git.listCommits / etc.
TRPC->>Resolve: resolveWorktreePath(ctx, workspaceId)
Resolve->>DB: query workspaces WHERE id = workspaceId
DB-->>Resolve: workspace record (with worktreePath)
Resolve->>FS: existsSync(worktreePath)
FS-->>Resolve: true
Resolve-->>TRPC: worktreePath
TRPC->>SimpleGit: construct simple-git on worktreePath
SimpleGit-->>TRPC: git command result
TRPC-->>Poller: success response
Poller-->>Renderer: workspace data (worktreeExists: true)
Note over Poller,Renderer: NEW: Happy Path — Worktree missing on disk
Poller->>TRPC: poll git.* procedure
TRPC->>Resolve: resolveWorktreePath(ctx, workspaceId)
Resolve->>DB: query workspace record
DB-->>Resolve: workspace record (stale worktreePath)
Resolve->>FS: existsSync(worktreePath)
FS-->>Resolve: false
Resolve->>Resolve: throw TRPCError(NOT_FOUND, { kind: "WORKTREE_MISSING", worktreePath })
Resolve-->>TRPC: exception escapes
alt NOT_FOUND code (non-500)
TRPC->>Sentry: NOT_FOUND → not captured
TRPC-->>Poller: TRPCError with code: "NOT_FOUND"
Poller-->>Renderer: error result (worktreeExists check degraded)
else any other error code (500)
TRPC->>Sentry: captured and reported
TRPC-->>Poller: INTERNAL_SERVER_ERROR
end
Note over Resolve,SimpleGit: Previously: missing worktree would reach simple-git construct → GitConstructError → 500
Note over TRPC,Resolve: workspace.gitStatus procedure also routes through resolveWorktreePath
Poller->>TRPC: workspace.gitStatus({ id })
TRPC->>Resolve: resolveWorktreePath(ctx, id) (CHANGED: was inline db query + ctx.git)
Resolve->>DB: query workspace
DB-->>Resolve: workspace record
alt worktree exists
Resolve->>FS: existsSync → true
Resolve-->>TRPC: worktreePath
TRPC->>SimpleGit: ctx.git(worktreePath)
SimpleGit-->>TRPC: git status result
TRPC-->>Poller: status data
else worktree missing
Resolve->>FS: existsSync → false
Resolve->>Resolve: throw NOT_FOUND
Resolve-->>TRPC: exception
TRPC-->>Poller: NOT_FOUND error
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // A worktree deleted outside the app is a routine lifecycle state, not a | ||
| // bug — classify it here so simple-git's construct error can't escape any | ||
| // git.* procedure as a reportable 500. | ||
| if (!existsSync(workspace.worktreePath)) { |
Contributor
There was a problem hiding this comment.
P2: The existsSync guard classifies the missing-worktree case only when the directory is already gone at check time. Because simple-git is constructed afterwards (ctx.git(worktreePath) / createUserSimpleGit), a worktree deleted in the tiny window between this check and the git construct still throws GitConstructError and escapes as a reportable 500 — the exact symptom (HOST-SERVICE-W) this PR is meant to stop. The added comment's claim that the construct error "can't escape any git.* procedure" is stronger than what the check delivers. Consider classifying at the git-construct boundary instead (wrap the simple-git construction so GitConstructError on a missing worktree becomes NOT_FOUND/WORKTREE_MISSING), which removes the race window entirely.
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/resolve-worktree.ts, line 23:
<comment>The existsSync guard classifies the missing-worktree case only when the directory is already gone at check time. Because simple-git is constructed afterwards (ctx.git(worktreePath) / createUserSimpleGit), a worktree deleted in the tiny window between this check and the git construct still throws GitConstructError and escapes as a reportable 500 — the exact symptom (HOST-SERVICE-W) this PR is meant to stop. The added comment's claim that the construct error "can't escape any git.* procedure" is stronger than what the check delivers. Consider classifying at the git-construct boundary instead (wrap the simple-git construction so GitConstructError on a missing worktree becomes NOT_FOUND/WORKTREE_MISSING), which removes the race window entirely.</comment>
<file context>
@@ -16,5 +17,15 @@ export function resolveWorktreePath(
+ // A worktree deleted outside the app is a routine lifecycle state, not a
+ // bug — classify it here so simple-git's construct error can't escape any
+ // git.* procedure as a reportable 500.
+ if (!existsSync(workspace.worktreePath)) {
+ throw new TRPCError({
+ code: "NOT_FOUND",
</file context>
This was referenced Aug 12, 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
HOST-SERVICE-W (escalating, 16 events/minute on canary
1.18.3-canary.20260805123527): when a workspace's worktree directory no longer exists — deleted externally or a cleanup raced the poll loop — every polled git tRPC endpoint (git.getStatus,git.listCommits,git.listBranches,git.getBranchSyncStatus, ...) constructs simple-git on the missing path, throwsGitConstructError: Cannot use simple-git on a directory that does not exist, and escapes asINTERNAL_SERVER_ERRORinto Sentry — once per batched procedure per poll.This is an expected lifecycle state, not a bug, so per the error-handling contract (#6164) it must be classified at the throw site — the dumb 500=report Sentry middleware stays filter-free.
Fix
resolveWorktreePath— the shared boundary everygit.*procedure resolves the workspace path through — now checks the directory exists and throws a typedNOT_FOUNDwith cause{ kind: "WORKTREE_MISSING", worktreePath }, mirroring the desktop v1 changes-router convention. Non-500 → not captured by the Sentry middleware; the renderer already degrades on error results (workspace.getalready exposesworktreeExists).workspace.gitStatus(legacy CLI/SDK surface) had the same hole via a directctx.git(worktreePath); it now routes through the same helper.Related
DESKTOP-H0 is the same error via the desktop project, but through the desktop v1 changes/workspaces routers. That path is already classified on main (
pathExistsCachedguard →NOT_FOUNDWORKTREE_MISSING, plusGitEnvironmentError→PRECONDITION_FAILED); its remaining events all come from released builds ≤ 1.18.3 whose bundles predate those guards (verified against event stacks), so it drains as users update — no further code change needed.Refs HOST-SERVICE-W
https://claude.ai/code/session_01JAJWZrVSdXGbsfiGvwCyKW
Summary by cubic
Classified missing workspace worktrees as NOT_FOUND to prevent
simple-gitconstruct errors from surfacing as 500s during git polling. This stops Sentry noise (Refs HOST-SERVICE-W) and keeps the UI degrading gracefully.resolveWorktreePathto throwNOT_FOUNDwith{ kind: "WORKTREE_MISSING", worktreePath }, preventingsimple-gitconstruct errors from escaping as 500s across allgit.*procedures.workspace.gitStatusthroughresolveWorktreePathto apply the same guard.Written for commit 9743185. Summary will update on new commits.
Summary by CodeRabbit