diff --git a/apps/backend/internal/worktree/config.go b/apps/backend/internal/worktree/config.go index a5cf8b305c..7a8a5154f7 100644 --- a/apps/backend/internal/worktree/config.go +++ b/apps/backend/internal/worktree/config.go @@ -116,8 +116,7 @@ func SanitizeForBranch(title string, maxLen int) string { result = sb.String() // Remove consecutive hyphens - re := regexp.MustCompile(`-+`) - result = re.ReplaceAllString(result, "-") + result = repoDirHyphenRun.ReplaceAllString(result, "-") // Remove leading and trailing hyphens result = strings.Trim(result, "-") @@ -179,6 +178,43 @@ func SmallSuffix(maxLen int) string { return string(buf) } +// SanitizeRepoDirName converts a repository display name into a single, +// filesystem-safe path segment. Path separators and other unsafe characters +// are replaced with hyphens, runs of hyphens are collapsed, and surrounding +// hyphens/dots are trimmed. Returns an empty string when the input has no +// usable characters. +// +// This guards against names like "owner/repo" producing nested subdirectories +// when used as the worktree directory under a multi-repo task root — the +// extra path level breaks sibling-repo detection in agentctl. +// +// Limitation: distinct names that differ only in unsafe characters (e.g. +// "acme/widget-config" and "acme-widget-config") collapse to the same +// segment. Two such repos in one task would collide on `git worktree add`. +// Acceptable in practice — provider repos (the common case) are uniquely +// identified by owner/name and won't collide with each other. +func SanitizeRepoDirName(name string) string { + if name == "" { + return "" + } + var sb strings.Builder + sb.Grow(len(name)) + for _, r := range name { + switch { + case unicode.IsLetter(r), unicode.IsDigit(r): + sb.WriteRune(r) + case r == '_', r == '.', r == '-': + sb.WriteRune(r) + default: + sb.WriteRune('-') + } + } + result := repoDirHyphenRun.ReplaceAllString(sb.String(), "-") + return strings.Trim(result, "-.") +} + +var repoDirHyphenRun = regexp.MustCompile(`-+`) + // SemanticWorktreeName generates a semantic worktree directory name from a task title. // Format: {sanitizedTitle}_{suffix} e.g. fix-login-bug_ab12cd34 // The title is truncated to 20 characters before adding the suffix. diff --git a/apps/backend/internal/worktree/config_test.go b/apps/backend/internal/worktree/config_test.go new file mode 100644 index 0000000000..f5d06068cc --- /dev/null +++ b/apps/backend/internal/worktree/config_test.go @@ -0,0 +1,29 @@ +package worktree + +import "testing" + +func TestSanitizeRepoDirName(t *testing.T) { + cases := []struct { + in, want string + }{ + {"widget-config", "widget-config"}, + {"acme/widget-config", "acme-widget-config"}, + {"acme/widget", "acme-widget"}, + {"owner\\repo", "owner-repo"}, + {"weird:name space", "weird-name-space"}, + {"with..dots", "with..dots"}, + {"trailing/", "trailing"}, + {"/leading", "leading"}, + {"a//b", "a-b"}, + {"-a-b-", "a-b"}, + {".hidden", "hidden"}, + {"!@#$%", ""}, + {"", ""}, + {"under_score.dot-dash", "under_score.dot-dash"}, + } + for _, c := range cases { + if got := SanitizeRepoDirName(c.in); got != c.want { + t.Errorf("SanitizeRepoDirName(%q) = %q, want %q", c.in, got, c.want) + } + } +} diff --git a/apps/backend/internal/worktree/errors.go b/apps/backend/internal/worktree/errors.go index 92741edbc2..4001085b5a 100644 --- a/apps/backend/internal/worktree/errors.go +++ b/apps/backend/internal/worktree/errors.go @@ -52,6 +52,11 @@ var ( // directory (~/.kandev/tasks/{taskDir}/{repo}/), so callers must populate // both fields. ErrTaskDirRequired = errors.New("worktree create requires TaskDirName and RepoName") + + // ErrInvalidRepoName is returned when RepoName is set but contains no + // characters that survive sanitization (e.g. "!@#$%"), so it cannot be + // used as a directory segment. + ErrInvalidRepoName = errors.New("repo name has no usable characters after sanitization") ) // containsAuthFailure checks if git output indicates an authentication failure. diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index b31778eb40..327fb81643 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -290,8 +290,17 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err // createInTaskDir creates a worktree inside the task directory structure: // ~/.kandev/tasks/{taskDirName}/{repoName}/ +// +// RepoName is sanitized to a single path segment so display names like +// "owner/repo" don't produce a nested subdirectory — that would push the +// worktree one level below the task root and break agentctl's sibling-based +// multi-repo detection. func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRef string) (*Worktree, error) { - worktreePath, err := m.config.TaskWorktreePath(req.TaskDirName, req.RepoName) + repoDir := SanitizeRepoDirName(req.RepoName) + if repoDir == "" { + return nil, ErrInvalidRepoName + } + worktreePath, err := m.config.TaskWorktreePath(req.TaskDirName, repoDir) if err != nil { return nil, fmt.Errorf("failed to get task worktree path: %w", err) } diff --git a/apps/backend/internal/worktree/manager_taskdir_checkout_branch_test.go b/apps/backend/internal/worktree/manager_taskdir_checkout_branch_test.go index c7c942a0e8..55415fc4fc 100644 --- a/apps/backend/internal/worktree/manager_taskdir_checkout_branch_test.go +++ b/apps/backend/internal/worktree/manager_taskdir_checkout_branch_test.go @@ -3,6 +3,7 @@ package worktree import ( "context" "os/exec" + "path/filepath" "strings" "testing" ) @@ -109,3 +110,53 @@ func TestCreateInTaskDir_RemoteBaseRefDoesNotSetUpstream(t *testing.T) { t.Fatalf("expected no upstream for new task branch, but got %q", strings.TrimSpace(string(out))) } } + +// Org-prefixed RepoName ("owner/repo") used to nest the worktree one level +// below the task root, breaking agentctl's sibling-based multi-repo detection. +// The worktree directory must sit directly under the task root so two repos +// (clean + org-prefixed) end up as siblings. +func TestCreateInTaskDir_OrgPrefixedRepoNameLandsAsSibling(t *testing.T) { + cfg := newTestConfig(t) + store := newMockStore() + mgr, err := NewManager(cfg, store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager: %v", err) + } + + repoA := initGitRepoWithRemote(t) + repoB := initGitRepoWithRemote(t) + const taskDir = "test-task" + + wtA, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "t-1", SessionID: "s-1", TaskTitle: "T", + RepositoryID: "repo-a", + RepositoryPath: repoA, + BaseBranch: "main", + TaskDirName: taskDir, + RepoName: "arthur", + }) + if err != nil { + t.Fatalf("Create A: %v", err) + } + + wtB, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "t-1", SessionID: "s-1", TaskTitle: "T", + RepositoryID: "repo-b", + RepositoryPath: repoB, + BaseBranch: "main", + TaskDirName: taskDir, + RepoName: "acme/widget-config", + }) + if err != nil { + t.Fatalf("Create B: %v", err) + } + + parentA := filepath.Dir(wtA.Path) + parentB := filepath.Dir(wtB.Path) + if parentA != parentB { + t.Fatalf("worktrees not siblings: A parent %q vs B parent %q", parentA, parentB) + } + if base := filepath.Base(wtB.Path); base != "acme-widget-config" { + t.Errorf("repo B dir = %q, want %q", base, "acme-widget-config") + } +} diff --git a/apps/web/components/task-create-dialog-options.tsx b/apps/web/components/task-create-dialog-options.tsx index 429d878efe..be83b08389 100644 --- a/apps/web/components/task-create-dialog-options.tsx +++ b/apps/web/components/task-create-dialog-options.tsx @@ -48,13 +48,15 @@ export function useRepositoryOptions( renderLabel: () => ( {repo.name} - - {truncateRepoPath(repo.local_path, 24)} - + {repo.local_path ? ( + + {truncateRepoPath(repo.local_path, 24)} + + ) : null} ), })), diff --git a/apps/web/components/task-create-dialog-pill.tsx b/apps/web/components/task-create-dialog-pill.tsx index 4e4036264c..c4054d7a81 100644 --- a/apps/web/components/task-create-dialog-pill.tsx +++ b/apps/web/components/task-create-dialog-pill.tsx @@ -206,7 +206,7 @@ export function Pill({ {tooltip ? {triggerButton} : triggerButton} - +
diff --git a/apps/web/components/task-create-dialog-repo-chips.test.tsx b/apps/web/components/task-create-dialog-repo-chips.test.tsx index 9adb0b9c01..b03df5064b 100644 --- a/apps/web/components/task-create-dialog-repo-chips.test.tsx +++ b/apps/web/components/task-create-dialog-repo-chips.test.tsx @@ -273,7 +273,7 @@ describe("RepoChipsRow", () => { ); fireEvent.click(screen.getByTestId("repo-chip-trigger")); expect(screen.getByText("frontend")).toBeTruthy(); - expect(screen.getByText("projects/local-project")).toBeTruthy(); + expect(screen.getByText("~/projects/local-project")).toBeTruthy(); expect(screen.queryByText("frontend-dup")).toBeNull(); }); @@ -299,7 +299,7 @@ describe("RepoChipsRow", () => { />, ); fireEvent.click(screen.getByTestId("repo-chip-trigger")); - fireEvent.click(screen.getByText("projects/local-project")); + fireEvent.click(screen.getByText("~/projects/local-project")); expect(onRowRepositoryChange).toHaveBeenCalledWith("r0", "/home/me/projects/local-project"); }); diff --git a/apps/web/components/task-create-dialog-repo-chips.tsx b/apps/web/components/task-create-dialog-repo-chips.tsx index e72c7d05e5..060b454511 100644 --- a/apps/web/components/task-create-dialog-repo-chips.tsx +++ b/apps/web/components/task-create-dialog-repo-chips.tsx @@ -3,12 +3,14 @@ import { useEffect, useMemo } from "react"; import { IconPlus, IconX, IconCode, IconGitBranch, IconGitFork } from "@tabler/icons-react"; import { cn, formatUserHomePath } from "@/lib/utils"; +import { Badge } from "@kandev/ui/badge"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { useBranches, type BranchSource } from "@/hooks/domains/workspace/use-repository-branches"; import type { LocalRepository, Repository } from "@/lib/types/http"; import type { DialogFormState, TaskRepoRow } from "@/components/task-create-dialog-types"; import { autoSelectBranch } from "@/components/task-create-dialog-helpers"; import { scoreBranch } from "@/lib/utils/branch-filter"; +import { scoreRepo } from "@/lib/utils/repo-filter"; import { Pill, sortBranches, @@ -461,11 +463,19 @@ function useRepoChipData({ const repoOptions: PillOption[] = useMemo( () => [ - ...filteredRepos.map((r) => ({ value: r.id, label: r.name })), + ...filteredRepos.map((r) => ({ + value: r.id, + label: r.name, + keywords: [r.name, r.local_path, formatUserHomePath(r.local_path)].filter( + (s): s is string => !!s, + ), + renderLabel: () => renderWorkspaceRepoOption(r), + })), ...filteredDiscovered.map((r) => ({ value: r.path, - label: shortRepoPath(r.path), - keywords: [r.path], + label: leafSegment(r.path), + keywords: [r.path, formatUserHomePath(r.path)], + renderLabel: () => renderDiscoveredRepoOption(r.path), })), ], [filteredRepos, filteredDiscovered], @@ -546,6 +556,7 @@ function RepoChip({ emptyMessage="No repositories" testId="repo-chip-trigger" tooltip={repoTooltip} + filter={scoreRepo} flat /> + {repo.name} + {display ? ( + {display} + ) : null} + + ); +} + +function renderDiscoveredRepoOption(path: string) { + const display = formatUserHomePath(path); + return ( + + + {leafSegment(path)} + {display} + + + on disk + + + ); +} + +function leafSegment(path: string): string { + const cleaned = path.replace(/\\/g, "/").replace(/\/+$/g, ""); + const idx = cleaned.lastIndexOf("/"); + return idx >= 0 ? cleaned.slice(idx + 1) : cleaned; } diff --git a/apps/web/lib/utils/repo-filter.test.ts b/apps/web/lib/utils/repo-filter.test.ts new file mode 100644 index 0000000000..8714407f73 --- /dev/null +++ b/apps/web/lib/utils/repo-filter.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from "vitest"; +import { scoreRepo } from "./repo-filter"; + +describe("scoreRepo", () => { + it("returns a positive score for an exact keyword match", () => { + expect(scoreRepo("uuid-123", "arthur", ["arthur"])).toBeGreaterThan(0); + }); + + it("returns a positive score for a substring/segment hit on the value", () => { + expect(scoreRepo("acme/widget-config", "widget")).toBeGreaterThan(0); + }); + + it("floors weak subsequence-only matches that scoreBranch would surface", () => { + // a-r-t-h-u-r appears in order across the path but no segment contains + // "arthur" — exactly the false positive scoreRepo is designed to kill. + expect(scoreRepo("playground/fun/thm/gameserver/lxd-alpine-builder", "arthur")).toBe(0); + }); + + it("returns a positive score when the search prefixes the value's leaf", () => { + expect(scoreRepo("playground/sky/arthur", "arth")).toBeGreaterThan(0); + }); + + it("returns 0 when nothing in value or keywords resembles the search", () => { + expect(scoreRepo("playground/foo", "completely-unrelated", ["foo"])).toBe(0); + }); +}); diff --git a/apps/web/lib/utils/repo-filter.ts b/apps/web/lib/utils/repo-filter.ts new file mode 100644 index 0000000000..84c55e0191 --- /dev/null +++ b/apps/web/lib/utils/repo-filter.ts @@ -0,0 +1,16 @@ +import { scoreBranch } from "./branch-filter"; + +// scoreRepo wraps scoreBranch but drops weak subsequence-only matches. +// +// scoreBranch's fuzzy fallback returns small non-zero scores when search +// characters appear in order across an arbitrary haystack — useful for +// branch typos, but noisy for repo paths. A query like "arthur" was +// matching long paths like "playground/fun/thm/.../lxd-alpine-builder" +// because a-r-t-h-u-r appears in order across path segments. +// +// scoreBranch awards >= 0.45 for any substring/prefix/word-boundary hit, +// so a 0.4 floor preserves real matches while filtering coincidences. +export function scoreRepo(value: string, search: string, keywords?: string[]): number { + const score = scoreBranch(value, search, keywords); + return score >= 0.4 ? score : 0; +}