Repository navigation
fix(host-service): classify missing project directory as NOT_FOUND in workspace-creation git procedures - #6316
Conversation
… workspace-creation git procedures
📝 WalkthroughWalkthroughThe change adds ChangesRepository path validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts (1)
33-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the typed error contract.
Cover an existing directory, a missing path, and a path occupied by a regular file. Assert the
NOT_FOUNDcode and bothcausefields. This helper is shared by all three procedures.🤖 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-creation/shared/local-project.ts` around lines 33 - 41, Add tests for requireProjectRepoPath covering an existing directory, a missing path, and a regular file at the path; verify successful paths return repoPath, while invalid paths throw TRPCError with code NOT_FOUND and cause.kind PROJECT_DIR_MISSING plus the expected repoPath.
🤖 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-creation/shared/local-project.ts`:
- Around line 33-40: Update requireProjectRepoPath to validate that
localProject.repoPath is an actual directory, not merely an existing filesystem
entry. Use a directory stat check, map ENOENT and ENOTDIR failures to the
existing TRPCError with kind PROJECT_DIR_MISSING, and allow other filesystem
errors to propagate.
---
Nitpick comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts`:
- Around line 33-41: Add tests for requireProjectRepoPath covering an existing
directory, a missing path, and a regular file at the path; verify successful
paths return repoPath, while invalid paths throw TRPCError with code NOT_FOUND
and cause.kind PROJECT_DIR_MISSING plus the expected repoPath.
🪄 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: 0211e970-fee6-41e1-bd61-f148ce61b92d
📒 Files selected for processing (4)
packages/host-service/src/trpc/router/workspace-creation/procedures/adopt.tspackages/host-service/src/trpc/router/workspace-creation/procedures/list-project-worktrees.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-branches.tspackages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
| export function requireProjectRepoPath(localProject: LocalProject): string { | ||
| if (!existsSync(localProject.repoPath)) { | ||
| throw new TRPCError({ | ||
| code: "NOT_FOUND", | ||
| message: "Project directory no longer exists on disk", | ||
| cause: { kind: "PROJECT_DIR_MISSING", repoPath: localProject.repoPath }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
node <<'NODE'
const { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } = require("node:fs");
const { join } = require("node:path");
const { tmpdir } = require("node:os");
const root = mkdtempSync(join(tmpdir(), "repo-path-check-"));
try {
const filePath = join(root, "repo");
writeFileSync(filePath, "");
if (!existsSync(filePath) || statSync(filePath).isDirectory()) {
throw new Error("Expected a regular file to exist without being a directory");
}
} finally {
rmSync(root, { recursive: true, force: true });
}
NODERepository: superset-sh/superset
Length of output: 158
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate the reviewed file and inspect the relevant implementation and nearby type/export context.
if [ -f "packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts" ]; then
echo "== file exists =="
stat -c '%n %s lines' packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
echo
echo "== imports and exported functions outline =="
ast-grep outline packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts || true
echo
echo "== relevant file contents =="
sed -n '1,120p' packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
else
echo "Reviewed file not found"
fd -a 'local-project.ts' .
fiRepository: superset-sh/superset
Length of output: 218
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== node availability =="
node --version || true
echo "== file availability =="
if [ -f "packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts" ]; then
echo "packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts"
wc -l packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts
else
fd -a 'local-project.ts' . | sed -n '1,50p'
fi
echo "== relevant file contents =="
sed -n '1,140p' packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts 2>/dev/null || true
echo "== related paths in workspace creation =="
rg -n "requireProjectRepoPath|localProject\.repoPath|repoPath" packages/host-service/src/trpc/router/workspace-creation packages/host-service/src 2>/dev/null | sed -n '1,120p' || true
echo "== fs behavior probe =="
node <<'NODE'
const fs = require("node:fs");
const { mkdtempSync, rmSync, writeFileSync, mkdirSync, unlinkSync } = require("node:fs");
const path = require("node:path");
const { tmpdir } = require("node:os");
const root = mkdtempSync(path.join(tmpdir(), "repo-path-check-"));
try {
const filePath = path.join(root, "repo");
const dirPath = path.join(root, "repo-dir");
writeFileSync(filePath, "");
mkdirSync(dirPath);
console.log(JSON.stringify({
existsSync_file: fs.existsSync(filePath),
isDirectory_file: fs.statSync(filePath).isDirectory(),
existsSync_dir: fs.existsSync(dirPath),
isDirectory_dir: fs.statSync(dirPath).isDirectory(),
existsSync_missing: fs.existsSync(path.join(root, "missing")),
}));
try {
writeFileSync(path.join(root, "repo"), "x");
} catch (error) {
console.log(JSON.stringify({ write_same_file_error_code: error.code }));
}
try {
fs.mkdirSync(filePath, { recursive: true });
} catch (error) {
console.log(JSON.stringify({ mkdir_over_existing_file_error_code: error.code }));
}
} finally {
rmSync(root, { recursive: true, force: true });
}
NODERepository: superset-sh/superset
Length of output: 17250
Validate that repoPath is a directory.
existsSync returns true for regular files, so /tmp/repo passes this guard when it is a file. Since callers pass repoPath into Git and workspace setup paths, use statSync(...).isDirectory() or an equivalent directory check. Treat ENOENT and ENOTDIR as PROJECT_DIR_MISSING; let other filesystem errors propagate.
🤖 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-creation/shared/local-project.ts`
around lines 33 - 40, Update requireProjectRepoPath to validate that
localProject.repoPath is an actual directory, not merely an existing filesystem
entry. Use a directory stat check, map ENOENT and ENOTDIR failures to the
existing TRPCError with kind PROJECT_DIR_MISSING, and allow other filesystem
errors to propagate.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
1 issue found across 4 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/workspace-creation/shared/local-project.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/shared/local-project.ts:33">
P2: `existsSync` only checks that the path exists, not that it is a directory — it returns `true` for regular files too. Since `simple-git` itself validates the path with `statSync(...).isDirectory()` before throwing `GitConstructError`, a `repoPath` that resolves to a file (rather than a missing path) will pass this guard but still blow up with the original 500 downstream, defeating the purpose of this fix. Use `statSync` and check `isDirectory()`, treating `ENOENT`/`ENOTDIR` as `PROJECT_DIR_MISSING` and letting other filesystem errors propagate.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client (tRPC)
participant Router as Workspace Creation Router
participant LP as requireLocalProject()
participant RPP as requireProjectRepoPath() (NEW)
participant GitCtx as ctx.git()
participant FS as File System
participant DB as Database
Note over Client,DB: Workspace Creation Git Procedures Flow
Client->>Router: tRPC call (searchBranches / listProjectWorktrees / adopt)
alt searchBranches
Router->>LP: findLocalProject(projectId)
LP->>DB: SELECT projects WHERE id = projectId
DB-->>LP: Project record
LP-->>Router: LocalProject (with repoPath)
Router->>RPP: requireProjectRepoPath(localProject)
RPP->>FS: existsSync(repoPath)
alt Directory exists
FS-->>RPP: true
RPP-->>Router: repoPath (string)
Router->>GitCtx: ctx.git(repoPath)
GitCtx-->>Router: simple-git instance
Router->>Router: Execute git operations (fetch, branch, worktree list)
Router-->>Client: Branch rows / Worktree list
else Directory missing
FS-->>RPP: false
RPP-->>Router: THROW TRPCError (NOT_FOUND)
Router-->>Client: 404 NOT_FOUND error
end
else listProjectWorktrees
Router->>LP: requireLocalProject(projectId)
LP->>DB: SELECT projects WHERE id = projectId
DB-->>LP: Project record
LP-->>Router: LocalProject (with repoPath)
Router->>RPP: requireProjectRepoPath(localProject)
RPP->>FS: existsSync(repoPath)
alt Directory exists
FS-->>RPP: true
RPP-->>Router: repoPath (string)
Router->>GitCtx: ctx.git(repoPath)
GitCtx-->>Router: simple-git instance
Router->>Router: listGitWorktrees(git)
Router->>DB: SELECT workspaces WHERE projectId
DB-->>Router: Workspace records
Router-->>Client: Worktree paths and workspace data
else Directory missing
FS-->>RPP: false
RPP-->>Router: THROW TRPCError (NOT_FOUND)
Router-->>Client: 404 NOT_FOUND error
end
else adopt
Router->>LP: requireLocalProject(projectId)
LP->>DB: SELECT projects WHERE id = projectId
DB-->>LP: Project record
LP-->>Router: LocalProject (with repoPath)
Router->>RPP: requireProjectRepoPath(localProject)
RPP->>FS: existsSync(repoPath)
alt Directory exists
FS-->>RPP: true
RPP-->>Router: repoPath (string)
Router->>Router: ensureMainWorkspace(ctx, projectId, repoPath)
Router->>DB: INSERT/UPDATE workspace row
DB-->>Router: Workspace record
Router->>GitCtx: ctx.git(repoPath)
GitCtx-->>Router: simple-git instance
Router->>Router: Create worktree / checkout branch
Router-->>Client: Adoption success
else Directory missing
FS-->>RPP: false
RPP-->>Router: THROW TRPCError (NOT_FOUND)
Router-->>Client: 404 NOT_FOUND error (no workspace row created)
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // A project directory deleted or moved outside the app is a routine | ||
| // lifecycle state, not a bug — classify it here so simple-git's construct | ||
| // error can't escape a workspace-creation procedure as a reportable 500. | ||
| export function requireProjectRepoPath(localProject: LocalProject): string { |
There was a problem hiding this comment.
P2: existsSync only checks that the path exists, not that it is a directory — it returns true for regular files too. Since simple-git itself validates the path with statSync(...).isDirectory() before throwing GitConstructError, a repoPath that resolves to a file (rather than a missing path) will pass this guard but still blow up with the original 500 downstream, defeating the purpose of this fix. Use statSync and check isDirectory(), treating ENOENT/ENOTDIR as PROJECT_DIR_MISSING and letting other filesystem errors propagate.
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/workspace-creation/shared/local-project.ts, line 33:
<comment>`existsSync` only checks that the path exists, not that it is a directory — it returns `true` for regular files too. Since `simple-git` itself validates the path with `statSync(...).isDirectory()` before throwing `GitConstructError`, a `repoPath` that resolves to a file (rather than a missing path) will pass this guard but still blow up with the original 500 downstream, defeating the purpose of this fix. Use `statSync` and check `isDirectory()`, treating `ENOENT`/`ENOTDIR` as `PROJECT_DIR_MISSING` and letting other filesystem errors propagate.</comment>
<file context>
@@ -24,3 +26,17 @@ export function requireLocalProject(
+// A project directory deleted or moved outside the app is a routine
+// lifecycle state, not a bug — classify it here so simple-git's construct
+// error can't escape a workspace-creation procedure as a reportable 500.
+export function requireProjectRepoPath(localProject: LocalProject): string {
+ if (!existsSync(localProject.repoPath)) {
+ throw new TRPCError({
</file context>
…y fix wave (#6437) * fix(desktop,host-service): address unreviewed findings from the sentry fix wave Four defects that automated reviewers flagged on PRs #6191, #6258, #6316 and #6318 and that were merged without anyone acting on them. - host-service-coordinator: crash reports redacted the child's own auth secret but not the cloud credential the coordinator also injects into its env, so a logged Authorization header could reach Sentry. Replace the single replacement with a per-instance redaction list. - workspaces/create: getCurrentBranch() returns null for a detached HEAD and for any git failure alike, so a missing repo or a permission wall was reported to the user as a detached HEAD and stopped being visible in Sentry. Confirm HEAD is genuinely detached first. - mesh-gradient: the WebGL probe leaked a context per mount, which can exhaust the browser's context budget and pin the real gradient to the static fallback. Release the probe and cache the result. - workspace-creation: existsSync() is true for a regular file, so a repo path pointing at a file passed the directory guard. * refactor(desktop): read HEAD once via a discriminated result instead of a second git client
…in workspaces.create (#6664) Creating a workspace for a project whose repository folder was moved or deleted outside the app threw simple-git's construct error from ctx.git(repoPath) in the create mutation, escaping as a reportable 500 on every retry. PR #6316 added requireProjectRepoPath for exactly this state but only applied it to search-branches, list-project-worktrees and adopt; the create mutation (and createEnqueued's cheap validation) never got it, which is why HOST-SERVICE-W regressed on the current release. Resolve the path through requireProjectRepoPath once after requireLocalProject and use the validated path for everything downstream; give createEnqueued the same one-line guard so it rejects synchronously instead of enqueueing a create that is guaranteed to fail. A directory deleted between check and use still reports (genuine race), and a permission-walled path (EACCES/EPERM) still reports — the guard converts only a true ENOENT, pinned by a negative test. Refs HOST-SERVICE-W Claude-Session: https://claude.ai/code/session_01PWSqv5bU8VzRDHP8Z4iWqi
Problem
Sentry HOST-SERVICE-W (40 events on 1.20.1, regressed): when a project's main repo directory has been deleted or moved outside the app,
workspaceCreation.searchBranches(and siblings) construct a simple-git instance onlocalProject.repoPathwithout checking the directory exists. simple-git's constructor throws itsGitConstructError("cannot use simple-git on a directory that does not exist"), which escapes as an INTERNAL_SERVER_ERROR 500 on every branch-search batch.Root cause
PR #6199 classified this family for the worktree-scoped
git.*procedures via the sharedresolve-worktree.tsboundary, but the workspace-creation procedures construct git directly on the projectrepoPathand were left out — so the issue regressed on the 1.20.1 release that ships #6199.Fix
Mirror #6199 at the workspace-creation layer: one shared guard,
requireProjectRepoPath()inshared/local-project.ts, that checksexistsSync(repoPath)before git construction and throws a typedNOT_FOUNDTRPCError with cause{ kind: "PROJECT_DIR_MISSING", repoPath }. Applied at the three procedures that construct git on the project repo path:searchBranches(the failing frame)listProjectWorktreesadopt(guarded beforeensureMainWorkspaceso no workspace row is created for a missing directory)Behavior is unchanged — the operation still fails when the directory is gone — only the classification changes from an unexpected 500 to a routine typed NOT_FOUND. Genuine unexpected git failures on these paths still surface as 500s.
Verification
bunx biome checkon the four changed files:cd packages/host-service && bun run typecheck:(exited clean, no errors)
No existing tests cover the touched files, so no test run applied.
Refs HOST-SERVICE-W
https://claude.ai/code/session_fb644a69-16aa-40f3-974b-fe2757d6255c
Summary by cubic
Classifies missing project repo directories as NOT_FOUND in workspace-creation git procedures to stop 500s from
simple-gitconstructor errors. Aligns workspace-creation paths with the worktree handlers and reduces HOST-SERVICE-W noise. (Linear: HOST-SERVICE-W)requireProjectRepoPath()to checkexistsSync(repoPath)and throw a typedTRPCErrorwith cause{ kind: "PROJECT_DIR_MISSING", repoPath }.searchBranches,listProjectWorktrees, andadopt(beforeensureMainWorkspace).Written for commit 297bda5. Summary will update on new commits.
Summary by CodeRabbit