Merge pull request #75 from bnema/fix/create-worktree-gitignore
fix(tools): recognize unanchored worktree ignore
2 files · +79 -7
M packages/agent/tools/create_worktree.go +9 -7
| @@ -19,11 +19,12 @@ import ( | |||
| 19 | 19 | ) | |
| 20 | 20 | ||
| 21 | 21 | const ( | |
| 22 | - | worktreesIgnoreEntry = "/.worktrees/" | |
| 23 | - | defaultWorktreeRoot = ".worktrees" | |
| 24 | - | worktreeConfigKey = "zut.worktrees.path" | |
| 25 | - | maxCreateWorktreeGitOutput = 64 * 1024 | |
| 26 | - | maxCreateWorktreeIgnoreBytes = 1 << 20 | |
| 22 | + | worktreesIgnoreEntry = "/.worktrees/" | |
| 23 | + | worktreesUnanchoredIgnoreEntry = ".worktrees/" | |
| 24 | + | defaultWorktreeRoot = ".worktrees" | |
| 25 | + | worktreeConfigKey = "zut.worktrees.path" | |
| 26 | + | maxCreateWorktreeGitOutput = 64 * 1024 | |
| 27 | + | maxCreateWorktreeIgnoreBytes = 1 << 20 | |
| 27 | 28 | ) | |
| 28 | 29 | ||
| 29 | 30 | var createWorktreeOperationMu sync.Mutex | |
| @@ -358,7 +359,7 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||
| 358 | 359 | if err != nil { | |
| 359 | 360 | return createWorktreePlan{}, fmt.Errorf("create_worktree: read .gitignore: %w", err) | |
| 360 | 361 | } | |
| 361 | - | ignoreAction = "already contains " + worktreesIgnoreEntry | |
| 362 | + | ignoreAction = "already ignores " + defaultWorktreeRoot + "/" | |
| 362 | 363 | if ignore.changed { | |
| 363 | 364 | ignoreAction = "add " + worktreesIgnoreEntry | |
| 364 | 365 | } | |
| @@ -818,7 +819,8 @@ func newWorktreesIgnoreUpdate(path string) (worktreesIgnoreUpdate, error) { | |||
| 818 | 819 | return worktreesIgnoreUpdate{}, err | |
| 819 | 820 | } | |
| 820 | 821 | for _, line := range strings.Split(string(contents), "\n") { | |
| 821 | - | if strings.TrimSuffix(line, "\r") == worktreesIgnoreEntry { | |
| 822 | + | switch strings.TrimSuffix(line, "\r") { | |
| 823 | + | case worktreesIgnoreEntry, worktreesUnanchoredIgnoreEntry: | |
| 822 | 824 | return worktreesIgnoreUpdate{path: path, before: contents, existed: existed, mode: mode}, nil | |
| 823 | 825 | } | |
| 824 | 826 | } | |
| @@ -19,11 +19,12 @@ import ( | |||||
| 19 | ) | 19 | ) | ||
| 20 | 20 | ||||
| 21 | const ( | 21 | const ( | ||
| 22 | - | worktreesIgnoreEntry = "/.worktrees/" | 22 | + | worktreesIgnoreEntry = "/.worktrees/" |
| 23 | - | defaultWorktreeRoot = ".worktrees" | 23 | + | worktreesUnanchoredIgnoreEntry = ".worktrees/" |
| 24 | - | worktreeConfigKey = "zut.worktrees.path" | 24 | + | defaultWorktreeRoot = ".worktrees" |
| 25 | - | maxCreateWorktreeGitOutput = 64 * 1024 | 25 | + | worktreeConfigKey = "zut.worktrees.path" |
| 26 | - | maxCreateWorktreeIgnoreBytes = 1 << 20 | 26 | + | maxCreateWorktreeGitOutput = 64 * 1024 |
| 27 | + | maxCreateWorktreeIgnoreBytes = 1 << 20 | |||
| 27 | ) | 28 | ) | ||
| 28 | 29 | ||||
| 29 | var createWorktreeOperationMu sync.Mutex | 30 | var createWorktreeOperationMu sync.Mutex | ||
| @@ -358,7 +359,7 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre | |||||
| 358 | if err != nil { | 359 | if err != nil { | ||
| 359 | return createWorktreePlan{}, fmt.Errorf("create_worktree: read .gitignore: %w", err) | 360 | return createWorktreePlan{}, fmt.Errorf("create_worktree: read .gitignore: %w", err) | ||
| 360 | } | 361 | } | ||
| 361 | - | ignoreAction = "already contains " + worktreesIgnoreEntry | 362 | + | ignoreAction = "already ignores " + defaultWorktreeRoot + "/" |
| 362 | if ignore.changed { | 363 | if ignore.changed { | ||
| 363 | ignoreAction = "add " + worktreesIgnoreEntry | 364 | ignoreAction = "add " + worktreesIgnoreEntry | ||
| 364 | } | 365 | } | ||
| @@ -818,7 +819,8 @@ func newWorktreesIgnoreUpdate(path string) (worktreesIgnoreUpdate, error) { | |||||
| 818 | return worktreesIgnoreUpdate{}, err | 819 | return worktreesIgnoreUpdate{}, err | ||
| 819 | } | 820 | } | ||
| 820 | for _, line := range strings.Split(string(contents), "\n") { | 821 | for _, line := range strings.Split(string(contents), "\n") { | ||
| 821 | - | if strings.TrimSuffix(line, "\r") == worktreesIgnoreEntry { | 822 | + | switch strings.TrimSuffix(line, "\r") { |
| 823 | + | case worktreesIgnoreEntry, worktreesUnanchoredIgnoreEntry: | |||
| 822 | return worktreesIgnoreUpdate{path: path, before: contents, existed: existed, mode: mode}, nil | 824 | return worktreesIgnoreUpdate{path: path, before: contents, existed: existed, mode: mode}, nil | ||
| 823 | } | 825 | } | ||
| 824 | } | 826 | } | ||
M packages/agent/tools/create_worktree_test.go +70 -0
| @@ -431,6 +431,76 @@ func TestExecuteCreateWorktreePlanRollsBackBootstrapConfigOnIgnoreFailure(t *tes | |||
| 431 | 431 | } | |
| 432 | 432 | } | |
| 433 | 433 | ||
| 434 | + | func TestCreateWorktreeAcceptsViableGitignoreEntries(t *testing.T) { | |
| 435 | + | if _, err := exec.LookPath("git"); err != nil { | |
| 436 | + | t.Skip("git is required") | |
| 437 | + | } | |
| 438 | + | for _, test := range []struct { | |
| 439 | + | name string | |
| 440 | + | entry string | |
| 441 | + | }{ | |
| 442 | + | {name: "root-anchored", entry: worktreesIgnoreEntry}, | |
| 443 | + | {name: "unanchored", entry: worktreesUnanchoredIgnoreEntry}, | |
| 444 | + | } { | |
| 445 | + | t.Run(test.name, func(t *testing.T) { | |
| 446 | + | repo := initWorktreeTestRepo(t) | |
| 447 | + | ignorePath := filepath.Join(repo, ".gitignore") | |
| 448 | + | before := []byte(test.entry + "\n") | |
| 449 | + | if err := os.WriteFile(ignorePath, before, 0o644); err != nil { | |
| 450 | + | t.Fatal(err) | |
| 451 | + | } | |
| 452 | + | tool := newCreateWorktreeTestTool(repo) | |
| 453 | + | args := mustJSON(t, map[string]string{ | |
| 454 | + | "branch": "feature/" + test.name + "-ignore", | |
| 455 | + | "bootstrap_root": defaultWorktreeRoot, | |
| 456 | + | }) | |
| 457 | + | ||
| 458 | + | result, err := tool.Execute(context.Background(), args, nil) | |
| 459 | + | if err != nil { | |
| 460 | + | t.Fatal(err) | |
| 461 | + | } | |
| 462 | + | text := result.Content[0].(provider.TextBlock).Text | |
| 463 | + | if !strings.Contains(text, ".gitignore: already ignores .worktrees/") { | |
| 464 | + | t.Fatalf("result did not recognize existing ignore entry:\n%s", text) | |
| 465 | + | } | |
| 466 | + | ignore, err := os.ReadFile(ignorePath) | |
| 467 | + | if err != nil { | |
| 468 | + | t.Fatal(err) | |
| 469 | + | } | |
| 470 | + | if string(ignore) != string(before) { | |
| 471 | + | t.Fatalf(".gitignore changed from %q to %q", before, ignore) | |
| 472 | + | } | |
| 473 | + | }) | |
| 474 | + | } | |
| 475 | + | } | |
| 476 | + | ||
| 477 | + | func TestNewWorktreesIgnoreUpdateRejectsLookalikeEntries(t *testing.T) { | |
| 478 | + | for _, entry := range []string{ | |
| 479 | + | "!.worktrees/\n", | |
| 480 | + | "nested/.worktrees/\n", | |
| 481 | + | "#.worktrees/\n", | |
| 482 | + | ".worktrees-other/\n", | |
| 483 | + | } { | |
| 484 | + | t.Run(strings.TrimSpace(entry), func(t *testing.T) { | |
| 485 | + | ignorePath := filepath.Join(t.TempDir(), ".gitignore") | |
| 486 | + | if err := os.WriteFile(ignorePath, []byte(entry), 0o644); err != nil { | |
| 487 | + | t.Fatal(err) | |
| 488 | + | } | |
| 489 | + | ||
| 490 | + | update, err := newWorktreesIgnoreUpdate(ignorePath) | |
| 491 | + | if err != nil { | |
| 492 | + | t.Fatal(err) | |
| 493 | + | } | |
| 494 | + | if !update.changed { | |
| 495 | + | t.Fatalf("entry %q was treated as ignoring the root .worktrees directory", strings.TrimSpace(entry)) | |
| 496 | + | } | |
| 497 | + | if got, want := string(update.after), entry+worktreesIgnoreEntry+"\n"; got != want { | |
| 498 | + | t.Fatalf("updated .gitignore = %q, want %q", got, want) | |
| 499 | + | } | |
| 500 | + | }) | |
| 501 | + | } | |
| 502 | + | } | |
| 503 | + | ||
| 434 | 504 | func TestCreateWorktreePreservesGitignoreLineEndings(t *testing.T) { | |
| 435 | 505 | if _, err := exec.LookPath("git"); err != nil { | |
| 436 | 506 | t.Skip("git is required") | |
| @@ -431,6 +431,76 @@ func TestExecuteCreateWorktreePlanRollsBackBootstrapConfigOnIgnoreFailure(t *tes | |||||
| 431 | } | 431 | } | ||
| 432 | } | 432 | } | ||
| 433 | 433 | ||||
| 434 | + | func TestCreateWorktreeAcceptsViableGitignoreEntries(t *testing.T) { | |||
| 435 | + | if _, err := exec.LookPath("git"); err != nil { | |||
| 436 | + | t.Skip("git is required") | |||
| 437 | + | } | |||
| 438 | + | for _, test := range []struct { | |||
| 439 | + | name string | |||
| 440 | + | entry string | |||
| 441 | + | }{ | |||
| 442 | + | {name: "root-anchored", entry: worktreesIgnoreEntry}, | |||
| 443 | + | {name: "unanchored", entry: worktreesUnanchoredIgnoreEntry}, | |||
| 444 | + | } { | |||
| 445 | + | t.Run(test.name, func(t *testing.T) { | |||
| 446 | + | repo := initWorktreeTestRepo(t) | |||
| 447 | + | ignorePath := filepath.Join(repo, ".gitignore") | |||
| 448 | + | before := []byte(test.entry + "\n") | |||
| 449 | + | if err := os.WriteFile(ignorePath, before, 0o644); err != nil { | |||
| 450 | + | t.Fatal(err) | |||
| 451 | + | } | |||
| 452 | + | tool := newCreateWorktreeTestTool(repo) | |||
| 453 | + | args := mustJSON(t, map[string]string{ | |||
| 454 | + | "branch": "feature/" + test.name + "-ignore", | |||
| 455 | + | "bootstrap_root": defaultWorktreeRoot, | |||
| 456 | + | }) | |||
| 457 | + | ||||
| 458 | + | result, err := tool.Execute(context.Background(), args, nil) | |||
| 459 | + | if err != nil { | |||
| 460 | + | t.Fatal(err) | |||
| 461 | + | } | |||
| 462 | + | text := result.Content[0].(provider.TextBlock).Text | |||
| 463 | + | if !strings.Contains(text, ".gitignore: already ignores .worktrees/") { | |||
| 464 | + | t.Fatalf("result did not recognize existing ignore entry:\n%s", text) | |||
| 465 | + | } | |||
| 466 | + | ignore, err := os.ReadFile(ignorePath) | |||
| 467 | + | if err != nil { | |||
| 468 | + | t.Fatal(err) | |||
| 469 | + | } | |||
| 470 | + | if string(ignore) != string(before) { | |||
| 471 | + | t.Fatalf(".gitignore changed from %q to %q", before, ignore) | |||
| 472 | + | } | |||
| 473 | + | }) | |||
| 474 | + | } | |||
| 475 | + | } | |||
| 476 | + | ||||
| 477 | + | func TestNewWorktreesIgnoreUpdateRejectsLookalikeEntries(t *testing.T) { | |||
| 478 | + | for _, entry := range []string{ | |||
| 479 | + | "!.worktrees/\n", | |||
| 480 | + | "nested/.worktrees/\n", | |||
| 481 | + | "#.worktrees/\n", | |||
| 482 | + | ".worktrees-other/\n", | |||
| 483 | + | } { | |||
| 484 | + | t.Run(strings.TrimSpace(entry), func(t *testing.T) { | |||
| 485 | + | ignorePath := filepath.Join(t.TempDir(), ".gitignore") | |||
| 486 | + | if err := os.WriteFile(ignorePath, []byte(entry), 0o644); err != nil { | |||
| 487 | + | t.Fatal(err) | |||
| 488 | + | } | |||
| 489 | + | ||||
| 490 | + | update, err := newWorktreesIgnoreUpdate(ignorePath) | |||
| 491 | + | if err != nil { | |||
| 492 | + | t.Fatal(err) | |||
| 493 | + | } | |||
| 494 | + | if !update.changed { | |||
| 495 | + | t.Fatalf("entry %q was treated as ignoring the root .worktrees directory", strings.TrimSpace(entry)) | |||
| 496 | + | } | |||
| 497 | + | if got, want := string(update.after), entry+worktreesIgnoreEntry+"\n"; got != want { | |||
| 498 | + | t.Fatalf("updated .gitignore = %q, want %q", got, want) | |||
| 499 | + | } | |||
| 500 | + | }) | |||
| 501 | + | } | |||
| 502 | + | } | |||
| 503 | + | ||||
| 434 | func TestCreateWorktreePreservesGitignoreLineEndings(t *testing.T) { | 504 | func TestCreateWorktreePreservesGitignoreLineEndings(t *testing.T) { | ||
| 435 | if _, err := exec.LookPath("git"); err != nil { | 505 | if _, err := exec.LookPath("git"); err != nil { | ||
| 436 | t.Skip("git is required") | 506 | t.Skip("git is required") | ||