Repository navigation
fix(host-service): classify a missing project directory as NOT_FOUND in workspaces.create - #6664
Conversation
…in workspaces.create 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
|
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 (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughWorkspace creation now validates and resolves the project repository path before filesystem and Git operations. Integration tests cover missing directories, permission errors, and prevented background creation after repository removal. ChangesWorkspace repository validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Workspace creation now returns an actionable NOT_FOUND error for missing project directories and avoids enqueueing guaranteed failures; the change is localized and verified by the supplied checks, with no actionable merge-blocking risk remaining. 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Problem
Creating a workspace for a project whose repository folder no longer exists at its registered path (moved or deleted outside the app) fails with simple-git's construct error ("Cannot use simple-git on a directory that does not exist") thrown from the create mutation's
ctx.git(localProject.repoPath)call. It escapes as INTERNAL_SERVER_ERROR, so the user sees an opaque internal error instead of an actionable "project directory is gone" message, and every retry is a fresh 500 — the latest events show one user retrying repeatedly within minutes against a condition that is deterministic and permanent until they re-point or restore the folder. 13 events in the last 24h, 233 lifetime, regressed on 1.23.0.User-visible harm: a user whose project folder was moved/deleted outside Superset gets a raw internal error on every workspace-create attempt, with no hint that the project path is the problem.
Reach: the create path is unchanged on main; this ships in the next host-service/desktop release and reaches every user creating workspaces on it. The condition is a routine lifecycle state, so it will keep firing until classified.
Why code (vs. archiving / filtering): this issue already regressed once — #6495 classified the same construct error on the git.getStatus path, and the issue came back through the uncovered create path. Archiving would just repeat that cycle, and Sentry-side suppression is banned by the repo's classification contract (#6164). The classification helper for exactly this state already exists (
requireProjectRepoPath, added in #6316); the create mutation was simply never given it.Root cause
PR #6316 added
requireProjectRepoPath— an ENOENT-safestatSync(..., { throwIfNoEntry: false })?.isDirectory()precheck that throwsTRPCError NOT_FOUNDwithcause: { kind: "PROJECT_DIR_MISSING" }— and applied it to search-branches, list-project-worktrees and adopt, but not toworkspaces.createorcreateEnqueued. The reporting middleware reports on INTERNAL_SERVER_ERROR alone, andcreateEnqueueddrives the realcreatethrough a server-side caller whose middleware chain reports the escaping construct error even though the enqueue wrapper catches it afterwards.Fix
Structural precheck only — no message matcher, matching the shape #6316 and #6634 established:
workspaces.create, resolve the path throughrequireProjectRepoPath(localProject)once, immediately afterrequireLocalProject, and use the validated path for everything downstream that consumedlocalProject.repoPath(ensureMainWorkspace,ctx.git, the worker base-ref fetcher, fetchPrMetadata, both listBranchNames calls, applyGeneratedWorkspaceNames).createEnqueued's cheap-validation layer, the same one-line guard, so a missing directory rejects synchronously with NOT_FOUND instead of enqueueing a create that is guaranteed to fail and settling with an error event. That layer already throws synchronous non-500s today (BAD_REQUEST for a missing id, PRECONDITION_FAILED for an unregistered project), so this adds no new client contract.Typed cause: reuses the helper's existing
PROJECT_DIR_MISSINGas-is; nothing new is invented and no new reader is added.What still reports, deliberately:
throwIfNoEntry: falseswallows only ENOENT, and the new negative test pins that a permission error is NOT converted to NOT_FOUND (the over-match fix(host-service): let destroy complete when the project repo is gone from disk #6634's merge gate had to fix).Adjacent sites left alone:
workspaces.generateBranchName(andaiRename) also touchproject.repoPathoutside the create mutation; they have not appeared in this issue and are out of scope.Verification
New tests written first and confirmed failing before the fix: the two NOT_FOUND tests failed (both paths returned 500s), and the EACCES negative pin passed pre-fix (stays INTERNAL_SERVER_ERROR before and after).
bunx biome checkon the three changed files:cd packages/host-service && bun run typecheck:(no errors)
Full
bun test testin packages/host-service:Co-located router suites (
bun test src/trpc/router/workspaces src/trpc/router/workspace-creation):Refs HOST-SERVICE-W
https://claude.ai/code/session_01PWSqv5bU8VzRDHP8Z4iWqi
Summary by cubic
Classifies a missing project repo directory as NOT_FOUND during workspace creation to replace opaque 500s. Previously,
workspaces.createandworkspaces.createEnqueuedthrew INTERNAL_SERVER_ERROR fromsimple-gitwhen the repo path was gone; now both precheck and return NOT_FOUND with cause PROJECT_DIR_MISSING, and the enqueued path rejects before scheduling.requireProjectRepoPathafterrequireLocalProjectinworkspaces.create, then uses the validated path for all downstream calls (main workspace ensure, git, base-ref fetcher, PR metadata, branch listing, name application).createEnqueuedso a missing directory rejects synchronously and is not enqueued.Written for commit f67582a. Summary will update on new commits.
Summary by CodeRabbit