Repository navigation
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A thread failed to provision with an opaque VCS error. It left
thread.createdwith
worktreePath: nulland then nothing at all — no activity, no message,nothing in the server journal — so there was no way to tell what went wrong from
either the UI or the logs.
The cause is that
createWorktreewithoutnewRefNamechecks out an existingbranch by name, and git allows a branch in only one worktree. Three paths
call it that way, passing whatever branch the thread recorded:
ws.ts:1044)worktree add <path> <existingBranch>WorktreeLifecycle.ts:298,356)worktree add <path> <thread.branch>ws.ts:1108)worktree add -b <new> <path> <commitSha>— safeIf that branch is the project's default, a primary clone normally has it checked
out, so the collision is reachable in ordinary use.
reuseBaseBranchreaches itdeliberately: it checks the selected branch out rather than branching off it, and
for a local ref clears
newRefName(ws.ts:1092-94).What
Detect the collision before running
git worktree addand report which branch,where it is already checked out, and the two ways out.
How
The guard lives in the driver rather than at the three call sites, so it covers
all of them — including any added later — from one place. It reuses
parseWorktreeBranchPaths, which already parsesworktree list --porcelain -zfor
listRefs, and only runs on thenewRefName-less path, so the safe new-threadpath is untouched.
The message follows the shape
preparePullRequestThreadalready uses for exactlythis condition, which until now was the only path that explained it.
Remarks
Deliberately not included: the
t3.jsonpolicy for default-branch threads.The agreed design is
worktrees.defaultBranch: "branch" | "root"—"branch"derives a thread branch off the base,
"root"runs the thread in the primarycheckout instead of creating a worktree. I drafted the schema and then pulled it
back out, because
"root"means two threads can resolve to the same directoryand I did not want to ship a config key whose interesting value is half-wired.
That needs the shared-path guard
CheckpointReactoralready does(
other.worktreePath === thread.worktreePath) and its own verification, so it isa separate PR rather than a rushed addition to this one.
This PR is the part that stands alone: it makes the failure legible no matter
which policy lands later.
Flow doc updated: n/a
E2E coverage: none added — see below
Design angles: assumed goal — thread provisioning that fails legibly instead of
silently; pragmatic (guard the primitive, keep every existing behaviour) vs.
greenfield (a worktree never checks out a shared branch at all: always
-bfrom aresolved SHA, and default-branch work routes to the root checkout). The greenfield
shape is the follow-up above; this PR does not preclude it.