Repository navigation
fix(server): turns recreate a worktree folder that lost its checkout - #34894
juliusmarminge wants to merge 2 commits into
Conversation
…d kept Repro for #24390: turn start skips recreation when the managed worktree folder exists without a checkout, and the storage cleanup sweep fails on the missing .git instead of keeping the folder. AI-assisted; verified with the tests and tools listed above.
A failed removal can leave a thread's managed worktree folder behind with no .git link. Turn start only checked that the folder existed, so the agent ran outside any checkout and git commands reached the repository around it. Turn start now checks for the folder's .git, clears the leftover folder, and recreates the checkout. Storage cleanup keeps such a folder as "not a linked worktree" instead of failing the sweep on the missing .git. Fixes #24390 AI-assisted; verified with the tests and tools listed above.
| readonly branch: string; | ||
| }) { | ||
| const { cwd, worktreePath, branch } = input; | ||
| yield* fileSystem.remove(worktreePath, { recursive: true, force: true }); |
There was a problem hiding this comment.
🔴 Critical orchestration-v2/ProviderTurnStartService.ts:222
When a managed worktree has lost only its .git entry, the next turn recursively deletes the entire worktreePath, permanently removing any remaining modified, untracked, or ignored files. The .git check detects the missing checkout metadata, while isManagedWorktreePath verifies only the path location; preserve the directory by moving it aside before recreation, or refuse recovery when its contents are not safe to discard.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/ProviderTurnStartService.ts around line 222:
When a managed worktree has lost only its `.git` entry, the next turn recursively deletes the entire `worktreePath`, permanently removing any remaining modified, untracked, or ignored files. The `.git` check detects the missing checkout metadata, while `isManagedWorktreePath` verifies only the path location; preserve the directory by moving it aside before recreation, or refuse recovery when its contents are not safe to discard.
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — Recovery now deletes and recreates managed worktree directories when their .git entry is missing. Remaining user files can be permanently lost, so the preservation policy needs resolution before this automatic recovery is safe. Needs human approval because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
ProviderTurnStartService decided whether to rebuild a thread's managed worktree with fileSystem.exists(worktreePath), so a folder left behind without its .git link (e.g. only node_modules after a failed removal) skipped recovery and the provider ran outside any checkout, where git reached the enclosing repository. Turn start now checks exists(/.git), and recreateMissingWorktree first deletes the leftover folder (recursive, force) before pruneWorktrees/createWorktree, because git refuses to add a worktree into a non-empty directory. This stays limited to managed worktree paths (the existing isManagedWorktreePath guard). Storage cleanup's worktreePathReason now treats a NotFound from stat(/.git) as missing (Effect.catchReason PlatformError/NotFound, the pattern used elsewhere in the server), so such folders are kept as 'not a linked worktree' instead of failing every sweep. The only test change: the fake FileSystem in the 'recreates a missing worktree only at the managed location' table now has a no-op remove. No new regex or parser.
Fixes #24390
Reproduction on main
On origin/main 1a1f9ac, ran the dev server on a fresh database (T3CODE_DEV_SEED=0 vp run dev). Created a scratch git repo, added it with
t3 project add, and started a worktree thread witht3 thread start --provider devReplay --model showcase --worktree main --branch feature/leftover --wait. Deleted the worktree's .git file and tracked files, left an empty node_modules/ folder, and rangit worktree prunein the main repo. Sent a second turn witht3 thread send --wait: it completed, no 'provider turn start recreating missing worktree' was logged, the folder still held only node_modules, andgit worktree listshowed no worktree for it. Inside that folder, git commands went up to the repository around it, so the thread's branch was changed to that repository's branch. Control: after deleting the folder entirely, the next turn logged 'recreating missing worktree' and restored a checkout. Two tests fail on main: the real-git recovery test in ProviderTurnStartService.test.ts with a new 'leftover' case (the provider opens in a folder that is not a checkout: not_a_repository), and a new storageCleanup.test.ts case where a thread's worktree has lost its .git (the sweep reports outcome 'failed' with 'NotFound: FileSystem.stat …/.git' instead of 'kept').Evidence
Being captured: before and after in the real app. This section is filled in before the pull request can merge.
Tests
Ran from apps/server:
vp test run src/orchestration-v2/ProviderTurnStartService.test.ts src/storageCleanup.test.ts: 242/242 pass. With only the fix reverted, 2 fail: the repro 'leftover' recovery case and 'keeps a thread's worktree folder that lost its .git link'.vp run typecheck(apps/server) passes.vp run check:changed: lint, knip and tests (3 files, 244) pass; format failed until I ran vp fmt; typecheck failed only on suggestions in scripts/evals, which it checked because the diff against the newer origin/main includes unrelated files. Pre-commit hook fmt/lint/knip passed.Not in this change
AI-assisted; verified with the tests and tools listed above.