Modular pass 4/5: worktree placement and cleanup #253

Activity

1. tannerpolley

   added a parent issue on Oct 29, 2026

   • Modular fork: move remaining fork code out of upstream files #24
2. tannerpolley commented on Oct 30, 2026

   

   tannerpolley

   on Oct 30, 2026

   Last edited by tannerpolley

   OwnerAuthor

   More actions

   T3 Code status: Done

   Issue: #253 Modular pass 4/5: worktree placement and cleanup

   Thread: thread:delegated-task:command%3Amcp%3Aa38b2400-afaf-4c9f-b945-754031b73fff%3Adelegate-task%3Aissue24-pass4-283-fix1-20261102

   Model: claude-opus-5-5

   Branch: issue-253-modular-pass-4-5-worktree-placement-and-3

   Updated: 2026-11-02T18:45:52.453Z
3. tannerpolley commented on Oct 30, 2026

   

   tannerpolley

   on Oct 30, 2026

   OwnerAuthor

   More actions

   T3 Code closeout: done

   Issue: #253 Modular pass 4/5: worktree placement and cleanup
   Thread: thread:delegated-task:command%3Amcp%3Aaef8c35e-cd1d-478e-acf8-8e4b8b075d41%3Adelegate-task%3Aissue24-pass4-253-implement-20261030
   Model: gpt-6.1-sol
   Branch: issue-253-modular-pass-4-5-worktree-placement-and
   Completed: 2026-10-30T06:53:54.638Z

   Implemented #253 locally and committed. The worktree is clean and ready for the parent’s Checker review.

   • Start: b539f4b7c33036acbd2407e49ac1c9f49efba4e6
   • Final: 1208787004a2ef5407a53780ceef41d150397be3
   • Branch: issue-253-modular-pass-4-5-worktree-placement-and
   • Local commits: a656444dce and 1208787004

   The extraction preserves the existing public APIs and callers:

   Area	Fork-owned implementation	Retained hook
   Worktree cleanup	forkWorktreeCleanup.ts	Construction, sweep, explicit removal, and terminal subscription
   Placement and collision handling	projectLocalWorktrees.ts	Git-driver calls for location preparation and atomic folder claiming
   .worktrees layout and exclusions	forkWorktreePaths.ts	Directory resolution, managed roots, and workspace scanners
   Issue/agent folder naming	forkAgentWorktreeName.ts	Existing shared/git.agentWorktreeName entry point

   Cleanup ownership, canonical paths, leases, dirty/unique-work protection, branch associations, remote handling, terminal/session protection, retry limits, rollback authority, and error behavior remain intact. The moved cleanup engine and subscription bodies match the originals after whitespace and export relocation are normalized.

   Validation passed:

   • vp run typecheck inside server and shared.
   • Main affected test run: 13 files, 446 tests.
   • Additional focused run: 5 files, 66 tests, including two naming tests also present in the main run.
   • Final Git-driver rerun after extracting folder claiming: 182 tests.
   • vp lint across all 14 changed files.
   • Explicit vp fmt --check: “All matched files use the correct format.”
   • git diff --check.

   Both normal commit attempts passed formatting and lint, then failed only on #256’s existing 74 unused exports. No introduced export appeared in those findings. Both commits used the authorized --no-verify bypass; the complete hook logs are retained.

   Against the same latest nightly, v0.1.6-nightly.20261030.3904 at 36dac688a17c6de1c6398e62d01546fb787c185b:

   Area	Before paths / blocks	After paths / blocks
   Repository total	17 / 23	17 / 21
   Cleanup: storageCleanup.ts	1 / 2	1 / 1
   Placement: GitVcsDriverCore.ts	1 / 2	1 / 1

   The method counts every conflicted path from git merge-tree --write-tree --name-only --no-messages, then counts <<<<<<< lines in each result-tree blob. Full per-file counts, exact commands, inventory, equivalence evidence, raw conflict blobs, and logs are in the handoff report and measurement data.

   No push, PR, merge, issue mutation, subagent, dev server, UI launch, or live-state access occurred. Parent-owned Checker review and delivery refresh remain open; no unresolved implementation defect was found.
4. tannerpolley commented on Nov 2, 2026

   

   tannerpolley

   on Nov 2, 2026

   OwnerAuthor

   More actions

   T3 Code closeout: done

   Issue: #253 Modular pass 4/5: worktree placement and cleanup
   Thread: thread:delegated-task:command%3Amcp%3Aa38b2400-afaf-4c9f-b945-754031b73fff%3Adelegate-task%3Aissue24-pass4-253-land-20261102
   Model: claude-opus-5-5
   Branch: issue-253-modular-pass-4-5-worktree-placement-and-2
   Completed: 2026-11-02T18:29:48.178Z

   Tests beside the changed files are still running in the background (task b15ehauk2), so this isn't the final result yet. When they finish I'll rebase onto the new main, re-measure, run vp run check:changed, and open the PR.

   Done so far:

   • Branch reset and narrowed, as you chose. The upstream cleanup engine stays in storageCleanup.ts, and that file is untouched.

   • Moved into fork modules:

     • agentWorktreeName → packages/shared/src/forkAgentWorktreeName.ts. git.ts and git.test.ts now match upstream exactly.
     • resolveWorktreePath and isProjectLocalWorktreePath → apps/server/src/forkWorktreePaths.ts.
     • Project-local placement and collision-safe folder claiming → projectLocalWorktrees.placeWorktree.

   • Typecheck passes for server, shared and web. Lint reports two errors in delegateTaskWorkspace.ts, at lines this change doesn't touch.

   • Conflict trial against nightly v0.1.7-nightly.20261102.4080:

     	Files	Blocks	Conflict lines
     main	31	54	1,298
     This branch	31	54	1,316

     Blocks don't drop, and conflict lines go up by 18. All 18 are in GitVcsDriverCore.ts, where git lines up the same checkout conflict differently. Fork lines inside upstream files do drop: GitVcsDriverCore.ts +190 → +111, worktreesDirectory.ts +32 → +4, and git.ts/git.test.ts +33 → 0.
5. tannerpolley commented on Nov 2, 2026

   

   tannerpolley

   on Nov 2, 2026

   OwnerAuthor

   More actions

   T3 Code closeout: waiting_on_sub_issues

   Issue: #253 Modular pass 4/5: worktree placement and cleanup
   Thread: thread:delegated-task:command%3Amcp%3Aa38b2400-afaf-4c9f-b945-754031b73fff%3Adelegate-task%3Aissue24-pass4-253-land-20261102
   Model: claude-opus-5-5
   Branch: issue-253-modular-pass-4-5-worktree-placement-and-2
   Completed: 2026-11-02T18:30:17.776Z

   check:changed is still running (task baavbl69w). The PR goes up after it finishes.
6. tannerpolley commented on Nov 2, 2026

   

   tannerpolley

   on Nov 2, 2026

   OwnerAuthor

   More actions

   T3 Code closeout: done

   Issue: #253 Modular pass 4/5: worktree placement and cleanup
   Thread: thread:delegated-task:command%3Amcp%3Aa38b2400-afaf-4c9f-b945-754031b73fff%3Adelegate-task%3Aissue24-pass4-253-land-20261102
   Model: claude-opus-5-5
   Branch: issue-253-modular-pass-4-5-worktree-placement-and-2
   Completed: 2026-11-02T18:35:12.327Z

   I opened PR #283 for #253, linked to this thread and not merged. It moves the fork's worktree naming and placement code into fork modules, but it doesn't reduce conflicts: the trial merge against the newest nightly has the same 54 conflict blocks before and after.

   Conflicts against v0.1.7-nightly.20261102.4080 (newest nightly, checked against upstream). "Before" is main at 3ab0ef55f6:

   	Files	Blocks	Conflict lines
   Before (main)	31	54	1,298
   After (this PR)	31	54	1,316

   GitVcsDriverCore.ts is the only touched file that conflicts, and it went from 3 blocks / 95 lines to 3 / 113. The three conflicts are upstream's new checkout code against the fork's addWorktree wrapper, which I didn't move. The extra 18 lines are git aligning that same conflict differently.

   What did shrink is fork code inside upstream files (git diff --numstat -w, measured against the tag):

   • GitVcsDriverCore.ts: +219/−99 → +140/−99.
   • worktreesDirectory.ts: +32 → +4.
   • git.ts and git.test.ts: +33 → 0, so they now match upstream exactly.
   • packages/shared/package.json: +4, for the new export entry.

   What moved:

   • agentWorktreeName went from git.ts to packages/shared/src/forkAgentWorktreeName.ts, a new shared export.
   • resolveWorktreePath and isProjectLocalWorktreePath went from worktreesDirectory.ts to apps/server/src/forkWorktreePaths.ts.
   • The .worktrees placement and collision-safe folder claiming went from GitVcsDriverCore.ts into placeWorktree in apps/server/src/projectLocalWorktrees.ts. The upstream file keeps a single call to it.

   I copied each moved body from current main. Compared without whitespace, the only differences are renamed parameters.

   Dropped as moot, per your answer:

   • The storageCleanup.ts engine move. 969 of the 1,171 lines in its target file were upstream's own cleanup code. In the trial it left blocks at 54 and grew that file's conflict from 240 to 883 lines, so every upstream cleanup fix would become a hand port. storageCleanup.ts is unchanged.
   • removeWorktreeNow stays in storageCleanup.ts. Moving it would need six internal functions passed out, a hook upstream wouldn't accept.
   • The original commits a656444dce and 1208787004. One new commit replaces them.

   Checks:

   • Tests beside the changed files: server 17 files / 494 tests passed, shared 2 files / 32 tests passed.
   • Typecheck: server, shared and web are clean.
   • vp run check:changed: format, unused patches and tests (498) pass. It still fails on three things this change doesn't cause:
     • three apps/mobile typecheck errors;
     • knip's 72 unused exports (Commit hook fails on every TypeScript change: 74 unused exports on main #256), none in the touched files;
     • two lint errors in delegateTaskWorkspace.ts that main's copy also has (I only changed its import line).
   • Commit: made with --no-verify because of those failures; the PR body says so.

   No CI checks or reviews have been reported on the PR yet.

   PR #283, head 044ddc5, checks and reviews: none reported yet.
7. tannerpolley commented on Nov 2, 2026

   

   tannerpolley

   on Nov 2, 2026

   OwnerAuthor

   More actions

   T3 Code closeout: done

   Issue: #253 Modular pass 4/5: worktree placement and cleanup
   Thread: thread:delegated-task:command%3Amcp%3Aa38b2400-afaf-4c9f-b945-754031b73fff%3Adelegate-task%3Aissue24-pass4-283-fix1-20261102
   Model: claude-opus-5-5
   Branch: issue-253-modular-pass-4-5-worktree-placement-and-3
   Completed: 2026-11-02T18:44:47.134Z

   Waiting on background task bfef8481j (tests beside changed files, then typecheck of server and shared). When it finishes I'll commit, push with --force-with-lease, re-measure against v0.1.7-nightly.20261102.4080, and update the PR body.
8. tannerpolley

   added a commit that references this issue on Nov 2, 2026

   refactor(worktrees): move fork placement and naming into fork modules

   ...

   9ac2938
9. tannerpolley commented on Nov 2, 2026

   

   tannerpolley

   on Nov 2, 2026

   OwnerAuthor

   More actions

   T3 Code closeout: done

   Issue: #253 Modular pass 4/5: worktree placement and cleanup
   Thread: thread:delegated-task:command%3Amcp%3Aa38b2400-afaf-4c9f-b945-754031b73fff%3Adelegate-task%3Aissue24-pass4-283-fix1-20261102
   Model: claude-opus-5-5
   Branch: issue-253-modular-pass-4-5-worktree-placement-and-3
   Completed: 2026-11-02T18:45:53.346Z

   All three findings are fixed. The branch is rebased onto current main and force-pushed with --force-with-lease. The new head is 9ac293820fc5ddc4acca53d103a01b4556324f04.

   What changed per finding

   1. Upstream's location check is back in GitVcsDriverCore.ts. The "must be an absolute folder… not a drive root" GitCommandError in createWorktree is again written exactly as in upstream nightly. I split the fork hook in two:

      • worktreeLocation (in projectLocalWorktrees.ts) works out the folder path, including the project-local case.
      • createWorktree checks that path for null and raises the error.
      • placeWorktree now receives the checked path plus the resolver it uses for collision suffixes. It no longer builds any error for an invalid location.

   2. Rebase: the branch is rebased onto origin/main at 912112ce1c with no conflicts. The push was guarded by a lease on the old head, 044ddc5c6f. I amended the commit message to describe the new split.

   3. PR body wording: it now says two pieces were restructured as well as having parameters renamed, and that both still behave the same:

      • forkWorktreePaths.ts resolves the project-local folder in its own explicit branch before falling back to upstream's resolver.
      • The collision loop returns the claimed path and ref instead of setting the caller's added, worktreePath and newRefName.

      The body also explains where the location check now lives.

   Conflicts against v0.1.7-nightly.20261102.4080 (I fetched tags first; this is still the newest nightly). Counts come from git merge-tree; the line count includes the conflict marker lines, as before.

   	Files	Blocks	Lines
   Before (main at 912112ce1c)	31	54	1,298
   After (9ac293820f)	31	54	1,290

   • GitVcsDriverCore.ts: conflict blocks stay at 3, and conflict lines drop from 95 to 87.
   • Fork lines in GitVcsDriverCore.ts (diff against the tag, ignoring whitespace): +219/−99 on main, +156/−99 here, so 63 fewer fork lines.
   • worktreesDirectory.ts: 32 fork lines on main, 4 here. git.ts and git.test.ts now match upstream.

   Checks on 9ac293820f (run with the vite-plus Node and GIT_CONFIG_GLOBAL=/dev/null, through agent-heavy)

   • Tests beside changed files: server 17 files, 643 tests passed; shared 3 files, 54 tests passed.
   • vp run typecheck for server and shared: clean, apart from style suggestions in files this PR doesn't touch.
   • vp fmt and vp lint on the two edited files: clean.
   • I did not re-run vp run check:changed this round. The commit uses --no-verify for the known failures from last round: the mobile typecheck errors, unused exports (Commit hook fails on every TypeScript change: 74 unused exports on main #256), and the two delegateTaskWorkspace.ts lint errors that main also has.

   Not done: GitHub shows no checks or reviews on the new head yet. I haven't merged. The unused -3 worktree I was started in still exists.

   PR #283, head 9ac2938, checks and reviews: none reported yet (mergeable, no review decision)
10. tannerpolley commented on Nov 2, 2026

    

    tannerpolley

    on Nov 2, 2026

    OwnerAuthor

    More actions

    Landed in #283 (9ac293820f). Fork worktree placement and naming moved into fork modules: 63 lines out of GitVcsDriverCore.ts and 28 out of worktreesDirectory.ts; git.ts and its test now match upstream. Upstream's storageCleanup.ts stays untouched. Against nightly v0.1.7-nightly.20261102.4080, the conflicts stay 31 files and 54 blocks (1,298 → 1,290 lines).
11. tannerpolley

    closed this as completedon Nov 2, 2026