fix(host-service): adopt existing worktree at any path on workspace.create#4096
Conversation
…reate When the user types a branch that already has a worktree registered outside the canonical `~/.superset/worktrees/<projectId>/<branch>` path, `workspaces.create` would call `git worktree add` and crash with `fatal: '<branch>' is already used by worktree at ...`, blocking the user from re-entering their own work. Look up the branch in `listWorktreeBranches` and adopt whatever path git already knows about.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughWorkspaces creation now prunes stale git worktrees, uses map-based worktree discovery ( ChangesWorktree adoption and creation flow
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Git as GitClient
participant HostAPI as Host/Cloud API
participant DB
Client->>Router: workspaces.create(...)
Router->>Git: git worktree prune
Router->>Git: listWorktreeBranches()
alt existing worktree path found
Router->>Router: call adoptExistingWorktree(args, hostPromise)
Router->>HostAPI: host.ensure (hostPromise resolves)
adoptExistingWorktree->>HostAPI: v2Workspace.getFromHost / create
adoptExistingWorktree->>DB: delete local conflicts / upsert local workspace
adoptExistingWorktree->>Git: git config write (record baseBranch)
adoptExistingWorktree-->>Router: return workspace (alreadyExists?)
else no existing path
Router->>Git: git worktree add (create fresh)
Router->>HostAPI: create cloud workspace
Router->>DB: register local workspace
Router-->>Client: created workspace
end
Router-->>Client: workspace result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThe PR fixes a crash in Confidence Score: 4/5Safe to merge; the core fix is correct and well-tested; one P2 edge case around prunable worktrees is worth a follow-up. Only P2 findings — the stale/prunable worktree edge case is a narrow regression that requires a previously-deleted-but-not-pruned worktree to exist for the target branch. No P0/P1 issues found. packages/host-service/src/trpc/router/workspaces/workspaces.ts — specifically the prunable-worktree gap in the adoption map lookup.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/workspaces/workspaces.ts | Adoption logic updated to look up the existing worktree path via listWorktreeBranches for any registered location instead of only the canonical path; stale/prunable entries are not excluded from the map. |
| packages/host-service/test/integration/workspace-create-delete.integration.test.ts | New integration test pre-creates a worktree at a non-canonical path and verifies that workspaces.create adopts that exact path instead of failing with git worktree add. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/host-service/src/trpc/router/workspaces/workspaces.ts:719-727
**Stale/prunable worktrees can be adopted as `worktreePath`**
`listWorktreeBranches` iterates all worktrees — including ones marked `prunable` (directory deleted without `git worktree remove`). When such a stale entry exists for `resolvedBranch`, `existingWorktreePath` is set to the now-missing path, `adopted = true` skips `git worktree add`, and the subsequent `enablePushAutoSetupRemote(git, worktreePath, …)` runs against a directory that no longer exists, likely throwing. Before this PR the same risk existed only at the canonical path; now it applies to any registered path. Filtering `wt.prunable != null` entries in `listWorktreeBranches` (or inline here) would prevent this.
Reviews (1): Last reviewed commit: "fix(host-service): adopt existing worktr..." | Re-trigger Greptile
| const existingWorktreePath = typedBranch | ||
| ? (await listWorktreeBranches(git)).worktreeMap.get( | ||
| resolvedBranch, | ||
| ) | ||
| : undefined; | ||
| const adopted = !!existingWorktreePath; | ||
| worktreePath = | ||
| existingWorktreePath ?? | ||
| safeResolveWorktreePath(localProject.id, resolvedBranch); |
There was a problem hiding this comment.
Stale/prunable worktrees can be adopted as
worktreePath
listWorktreeBranches iterates all worktrees — including ones marked prunable (directory deleted without git worktree remove). When such a stale entry exists for resolvedBranch, existingWorktreePath is set to the now-missing path, adopted = true skips git worktree add, and the subsequent enablePushAutoSetupRemote(git, worktreePath, …) runs against a directory that no longer exists, likely throwing. Before this PR the same risk existed only at the canonical path; now it applies to any registered path. Filtering wt.prunable != null entries in listWorktreeBranches (or inline here) would prevent this.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspaces/workspaces.ts
Line: 719-727
Comment:
**Stale/prunable worktrees can be adopted as `worktreePath`**
`listWorktreeBranches` iterates all worktrees — including ones marked `prunable` (directory deleted without `git worktree remove`). When such a stale entry exists for `resolvedBranch`, `existingWorktreePath` is set to the now-missing path, `adopted = true` skips `git worktree add`, and the subsequent `enablePushAutoSetupRemote(git, worktreePath, …)` runs against a directory that no longer exists, likely throwing. Before this PR the same risk existed only at the canonical path; now it applies to any registered path. Filtering `wt.prunable != null` entries in `listWorktreeBranches` (or inline here) would prevent this.
How can I resolve this? If you propose a fix, please make it concise.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
…lper `workspaces.create` and `workspaceCreation.adopt` had diverged copies of the post-worktreePath registration flow — the former missing stale-row hygiene, the latter only reachable from v1→v2 migration. Extract the shared invariants (relink by branch, relink-on-rename by path, stale-row cleanup, fresh cloud+local registration) into `adoptExistingWorktree` and call it from both procedures. Also fixes the PR-creation path: when a PR's local branch is already in a worktree at any path, `git worktree add` would refuse. Look the branch up in the worktree map first; on OID match adopt via the helper, on OID mismatch surface a CONFLICT that points at `git worktree remove` before `git branch -D`.
`git worktree list` keeps reporting an entry after its directory is deleted (marked `prunable`), and git still claims the branch until prune runs. The previous fix would happily set `worktreePath` to the missing dir; the original canonical-path check had the same bug, just narrower. Two changes: - `listWorktreeBranches` filters prunable entries so adopt callers never target a stale path. - `workspaces.create` runs `git worktree prune` up front so the fresh `git worktree add` at the canonical path can succeed without `branch is already used by worktree at <missing-path>`.
Compress JSDoc and rationale comments around the new adoption helper and `workspaces.create` prune step. Same intent, fewer lines.
Changes since v0.2.8: - `superset agents list` (and demoted "presets" to a UI-only concept). CLI now reads canonical agents from host-service. (#4097) - Host-service: refresh stale OAuth access tokens on remote workspace ops instead of failing the request. (#4106) - Host-service: workspace.create now adopts an existing worktree at any path, not just the canonical one. (#4096) - Host-service: AI workspace naming times out after 5s and falls back to a deterministic name. (#4102) Push cli-v0.2.9 after this lands to fire the release pipeline.
…reate (#4096) * fix(host-service): adopt existing worktree at any path on workspace.create When the user types a branch that already has a worktree registered outside the canonical `~/.superset/worktrees/<projectId>/<branch>` path, `workspaces.create` would call `git worktree add` and crash with `fatal: '<branch>' is already used by worktree at ...`, blocking the user from re-entering their own work. Look up the branch in `listWorktreeBranches` and adopt whatever path git already knows about. * refactor(host-service): unify workspace adoption behind one shared helper `workspaces.create` and `workspaceCreation.adopt` had diverged copies of the post-worktreePath registration flow — the former missing stale-row hygiene, the latter only reachable from v1→v2 migration. Extract the shared invariants (relink by branch, relink-on-rename by path, stale-row cleanup, fresh cloud+local registration) into `adoptExistingWorktree` and call it from both procedures. Also fixes the PR-creation path: when a PR's local branch is already in a worktree at any path, `git worktree add` would refuse. Look the branch up in the worktree map first; on OID match adopt via the helper, on OID mismatch surface a CONFLICT that points at `git worktree remove` before `git branch -D`. * fix(host-service): handle prunable worktrees in workspaces.create `git worktree list` keeps reporting an entry after its directory is deleted (marked `prunable`), and git still claims the branch until prune runs. The previous fix would happily set `worktreePath` to the missing dir; the original canonical-path check had the same bug, just narrower. Two changes: - `listWorktreeBranches` filters prunable entries so adopt callers never target a stale path. - `workspaces.create` runs `git worktree prune` up front so the fresh `git worktree add` at the canonical path can succeed without `branch is already used by worktree at <missing-path>`. * chore(host-service): tighten comments in worktree-adoption code Compress JSDoc and rationale comments around the new adoption helper and `workspaces.create` prune step. Same intent, fewer lines.
Changes since v0.2.8: - `superset agents list` (and demoted "presets" to a UI-only concept). CLI now reads canonical agents from host-service. (#4097) - Host-service: refresh stale OAuth access tokens on remote workspace ops instead of failing the request. (#4106) - Host-service: workspace.create now adopts an existing worktree at any path, not just the canonical one. (#4096) - Host-service: AI workspace naming times out after 5s and falls back to a deterministic name. (#4102) Push cli-v0.2.9 after this lands to fire the release pipeline.
Summary
listWorktreeBranchesand adopt whatever path git already has registered, instead of only matching the canonical~/.superset/worktrees/<projectId>/<branch>location.workspaces.createcalledgit worktree addand hitfatal: '<branch>' is already used by worktree at ..., locking the user out of re-entering their own work when the worktree lived elsewhere (prior session, created outside Superset, or moved in place).create()adopts it (persistedworktreePathmatches the existing location).Test plan
bun test packages/host-service/test/integration/workspace-create-delete.integration.test.tsfoooutside~/.superset/worktrees, then create a workspace targetingfoofrom the desktop app — should adopt without error.Summary by cubic
Adopts existing Git worktrees for a branch at any path during workspace creation and PR checkout, avoiding conflicts and letting users re-enter existing work. Also handles stale (prunable) worktrees so new checkouts succeed.
Bug Fixes
workspaces.createand PR checkout, look up the branch vialistWorktreeBranchesand adopt the registered path; if the local branch OID differs from the PR, return a clear CONFLICT with cleanup hints.git worktree prunebefore adds; added integration tests for non-canonical adoption and stale-entry pruning.Refactors
adoptExistingWorktree, used by bothworkspaces.createandworkspaceCreation.adopt, with relink-by-branch/path and stale-row cleanup.Written for commit af2cd5a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests