Repository navigation
fix: Reuse issue branches, isolate tests from the repo - #114
Conversation
Code Review by Qodo
1.
|
PR Summary by QodoReuse issue branches and isolate Git tests from hook environments
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
🟡 Changes recommended
Issue-worktree reuse can fail on path conflicts or a detached HEAD, return deleted directories, and lacks a regression test for inherited Git environment isolation.
4 open issues
The CLI resolves the base branch inhandleCreateActionbefore reaching this reuse check. With a… · New The target-path check above runs before this lookup, so the common case where the issue branch is… · NewGetWorktreesincludes prunable entries whose directories have been deleted. If an issue worktree… · New Please add a regression test for the inherited Git environment this helper clears. Normal test runs… · New
What changed in this PR
This PR adds issue-branch reuse to worktree creation and isolates Git-running tests from repository environment variables inherited through hooks.
Changes:
- Reuses existing issue branches when creating worktrees or switching with
--no-workspace. - Adds shared Git-environment cleanup through package-level
TestMainfunctions. - Adds branch-reuse tests and documents the new behaviour and test helper.
Tests were not run because project test execution is unavailable in this review environment.
| File | Description |
|---|---|
lazyworktree.1 |
Documents existing issue-branch reuse. |
internal/testenv/testenv.go |
Adds shared cleanup of Git repository environment variables. |
internal/git/main_test.go |
Applies environment cleanup before Git tests. |
internal/config/main_test.go |
Applies environment cleanup before configuration tests. |
internal/cli/operations.go |
Adds existing issue-branch and worktree reuse paths. |
internal/cli/operations_flow_test.go |
Tests branch reuse, attached worktrees, and branch switching. |
internal/cli/main_test.go |
Applies environment cleanup before CLI tests. |
internal/bootstrap/main_test.go |
Applies environment cleanup before bootstrap tests. |
internal/app/services/main_test.go |
Applies environment cleanup before application service tests. |
internal/app/main_test.go |
Applies environment cleanup before application tests. |
docs/development/architecture.md |
Describes the test-environment helper’s purpose. |
docs/cli/create.md |
Documents issue-specific branch reuse behaviour. |
Review effort: Balanced
Creating a worktree from an issue on the command line (for example with "lazyworktree create -I") stopped with a "branch already exists" error whenever a branch for that issue was left over from earlier work. This happens when a previous worktree for the issue was removed but its branch was kept. The same problem also hit the "--no-workspace" mode. Lazyworktree now picks up the existing branch and keeps going. If that branch is already open in another worktree, it hands back the path of that worktree, which is what already happens for pull requests. Otherwise it opens a new worktree on the branch with its existing commits. In "--no-workspace" mode it switches to the branch. The command reference and the manual page now describe this.
Running the test suite from a git hook inside a linked worktree wrote into the developer's own repository. Git hands its hooks the location of the repository it is working on, and in a worktree that location is a full path. The tests that build throwaway repositories inherited it, so their commits, branches, remotes and settings landed in the real repository instead. It also turned the repository into a bare one, set a fake author name and email, and switched off commit signing. Every package whose tests run git now clears those inherited repository settings before any test starts, so each test only ever touches its own temporary repository. The architecture notes list the new test helper.
Running the issue create command twice for the same issue used to stop with "path already exists" unless the -U flag was given, even though the worktree it wanted was already there. It now hands back that worktree straight away. With --no-workspace, switching to an issue branch that another worktree already holds failed with a raw git error. The message now says which worktree has the branch, so it is clear where to go instead. The docs explain both cases.
If the worktree holding an issue branch had been deleted outside git, the create command still handed back its old path, which no longer existed. It now says the worktree is gone and suggests running "git worktree prune" first. A new test makes sure tests run from a git hook in a linked worktree cannot change the real repository again, so the earlier fix cannot quietly break.
The Herdr tests that just arrived from main create git repositories, and their package had not been protected yet. Running them from the push hook in a worktree added test commits and branches to the real repository again and marked it as bare. Those tests are now protected too. The push hook also clears git's repository settings before running the tests, so a package added later cannot repeat this, even if it forgets the protection.
c07555f to
10fd49b
Compare


Two fixes found while creating a worktree from an issue.
Creating from an issue fails when its branch already exists
lazyworktree create -I(and--from-issue) stopped withfatal: a branch named 'issue-397-...' already existswhenever a branch for that issue was left over from an earlier worktree.--no-workspacefailed the same way.The existing branch is now reused, matching what
--from-pralready does:flowchart LR A[issue branch exists?] -- no --> B[create branch from base, new worktree] A -- yes --> C{checked out in a worktree?} C -- yes --> D[return that worktree's path] C -- no --> E[new worktree on the existing branch]Running the same command twice now returns the existing worktree, with or without
-U. If the worktree holding the branch was deleted outside git, the command says so and suggestsgit worktree prunerather than returning a path that no longer exists.With
--no-workspace, it switches to the existing branch instead of creating it. If another worktree already holds the branch, the error names that worktree. The CLI docs and man page describe the new behaviour.Tests wrote into the developer's repository from git hooks
The
go test ./...pre-commit/pre-push hook, run inside a linked worktree, corrupted the real repository. Git gives hooks an absoluteGIT_DIRin that case, and the tests that build scratch repos inherited it. In one push it turned the repo bare, setuser.name/user.emailto test values, added anupstreamremote, createdfeature*branches, committed on top ofmainand the current branch, and wrotecommit.gpgsign=false. That last setting had turned off commit signing in this repo since March.Every package whose tests run git now clears git's repository variables (the
git rev-parse --local-env-varslist) inTestMain, through a smallinternal/testenvhelper. The Herdr tests that landed on main while this PR was open did the same damage again, so the hook now clears those variables before runninggo testas well. A package that forgets theTestMainis still covered. A regression test ininternal/testenvpointsGIT_DIRat a scratch linked worktree and checks that the repository stays untouched.To reproduce, create a scratch repo with a linked worktree and run
GIT_DIR=<that worktree's gitdir> go test ./.... Before this change the scratch repo picks up the branches, identity andcore.bare=true. After it, nothing changes.