Skip to content

fix: create task from PR failing when branch already checked out in external worktree#2094

Merged
jschwxrz merged 3 commits into
mainfrom
jona/eng-1297-opening-an-existing-conductor-worktree-in-emdash-fails
May 18, 2026
Merged

fix: create task from PR failing when branch already checked out in external worktree#2094
jschwxrz merged 3 commits into
mainfrom
jona/eng-1297-opening-an-existing-conductor-worktree-in-emdash-fails

Conversation

@jschwxrz
Copy link
Copy Markdown
Collaborator

  • reuse an existing linked worktree when creating a task from a PR branch
  • avoid fetching the PR branch when git already has that branch checked out elsewhere
  • resolve inactive task settings targets via branch discovery so external worktrees are included

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR fixes task creation from a PR when the PR's head branch is already checked out in an external git worktree. Previously the code tried to fetch/update a branch that git refuses to modify while it is checked out, causing the operation to fail.

  • createTask.ts: guards against re-fetching a branch by calling findBranchAnywhere before fetchPrForReview; if the branch is already in any registered worktree the fetch is skipped and provisioning proceeds using the existing path.
  • project-settings-target-resolver.ts: upgrades the inactive-task lookup from the pool-scoped getWorktree to findBranchAnywhere so external worktrees are included when resolving settings targets.
  • New createTask.test.ts covers the skip-fetch path; share-project-settings-to-config.test.ts updated to match the renamed method.

Confidence Score: 4/5

Safe to merge — the core logic change is small and correct, and the provisioning layer independently re-validates the worktree path via findBranchAnywhere.

The two production-code changes are straightforward and well-motivated. The new createTask.test.ts only exercises the skip-fetch branch; the normal path where the branch is absent and fetchPrForReview must run has no test coverage, leaving a gap on the primary PR-creation flow.

createTask.test.ts would benefit from a second test covering the fetch path.

Important Files Changed

Filename Overview
src/main/core/tasks/operations/createTask.ts Replaces project.getWorktreeForBranch with project.worktreeService.findBranchAnywhere to detect PR branches checked out in external worktrees before attempting a git fetch. Logic is correct and consistent with how the WorktreeService already handles this internally.
src/main/core/projects/settings/sharing/project-settings-target-resolver.ts Swaps getWorktree (pool-scoped lookup) for findBranchAnywhere (git-wide lookup) so inactive tasks whose branches live in external worktrees are still resolved as settings targets. The existing repoPath guard prevents the main worktree from leaking through.
src/main/core/projects/settings/sharing/share-project-settings-to-config.test.ts Tests updated mechanically to reflect the method rename; one test also uses an external path to validate the broader lookup. Coverage is adequate for the settings-sharing path.
src/main/core/tasks/operations/createTask.test.ts New test file covers only the skip-fetch branch; the normal fetch path (findBranchAnywhere returns undefined, fetchPrForReview is called) has no test, leaving a coverage gap on the happy-path PR creation flow.

Sequence Diagram

sequenceDiagram
    participant UI
    participant createTask
    participant worktreeService
    participant repository
    participant taskManager

    UI->>createTask: "createTask({ strategy: from-pull-request })"
    createTask->>worktreeService: findBranchAnywhere(headBranch)
    alt branch already checked out in any worktree
        worktreeService-->>createTask: existingPath (skip fetch)
    else branch not found anywhere
        worktreeService-->>createTask: undefined
        createTask->>repository: fetchPrForReview(prNumber, headBranch, ...)
        repository-->>createTask: ok / err
    end
    createTask->>taskManager: provisionTask(project, task, ...)
    taskManager-->>createTask: success
    createTask-->>UI: "ok({ task })"
Loading
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
src/main/core/tasks/operations/createTask.test.ts:118-164
**Missing test for the normal fetch path**

The `beforeEach` always resolves `findBranchAnywhere` to a path, so the only test in the suite covers the skip-fetch branch. The complementary case — where `findBranchAnywhere` returns `undefined` and `fetchPrForReview` is actually called — has no coverage. A regression that accidentally skips the fetch unconditionally (e.g. `findBranchAnywhere` always returning a truthy value) would pass this suite unnoticed.

Reviews (1): Last reviewed commit: "fix(settings): resolve external task wor..." | Re-trigger Greptile

Comment thread src/main/core/tasks/operations/createTask.test.ts
@jschwxrz jschwxrz merged commit a84854b into main May 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant