From 9f1e3729675755f969282a0b242576589823565e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Fri, 1 May 2026 13:24:12 +0100 Subject: [PATCH] feat: refresh + smarter filter for new-task branch selector (#750) --- apps/backend/internal/task/dto/dto.go | 7 + .../task/handlers/repository_handlers.go | 51 +++++- .../internal/task/service/repository_fetch.go | 165 ++++++++++++++++++ .../task/service/repository_fetch_test.go | 140 +++++++++++++++ apps/backend/internal/task/service/service.go | 2 + apps/web/components/branch-refresh-button.tsx | 66 +++++++ apps/web/components/combobox.tsx | 33 +++- .../task-create-dialog-form-body.tsx | 16 ++ .../components/task-create-dialog-options.tsx | 18 ++ .../task-create-dialog-selectors.tsx | 25 +++ .../components/task-create-dialog-state.ts | 14 +- apps/web/components/task-create-dialog.tsx | 28 ++- .../task/create-task-branch-selector.spec.ts | 152 +++++++++++++++- .../workspace/use-repository-branches.ts | 86 ++++++--- apps/web/lib/api/domains/workspace-api.ts | 9 +- apps/web/lib/state/slices/workspace/types.ts | 11 +- .../slices/workspace/workspace-slice.test.ts | 78 +++++++++ .../state/slices/workspace/workspace-slice.ts | 14 +- apps/web/lib/state/store.ts | 7 +- apps/web/lib/types/http.ts | 6 + apps/web/lib/utils/branch-filter.test.ts | 93 ++++++++++ apps/web/lib/utils/branch-filter.ts | 125 +++++++++++++ 22 files changed, 1095 insertions(+), 51 deletions(-) create mode 100644 apps/backend/internal/task/service/repository_fetch.go create mode 100644 apps/backend/internal/task/service/repository_fetch_test.go create mode 100644 apps/web/components/branch-refresh-button.tsx create mode 100644 apps/web/lib/state/slices/workspace/workspace-slice.test.ts create mode 100644 apps/web/lib/utils/branch-filter.test.ts create mode 100644 apps/web/lib/utils/branch-filter.ts diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index 6af7d29386..51d0f56d68 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -309,6 +309,13 @@ type RepositoryBranchesResponse struct { Branches []BranchDTO `json:"branches"` Total int `json:"total"` CurrentBranch string `json:"current_branch,omitempty"` + // FetchedAt is the timestamp of the most recent `git fetch` for this + // repository (RFC3339). Empty when no refresh has been requested in the + // process lifetime. + FetchedAt string `json:"fetched_at,omitempty"` + // FetchError is the human-readable error from the last fetch attempt for + // this request, if one was attempted and failed. Empty otherwise. + FetchError string `json:"fetch_error,omitempty"` } // LocalRepositoryStatusResponse reports current branch + dirty paths for a diff --git a/apps/backend/internal/task/handlers/repository_handlers.go b/apps/backend/internal/task/handlers/repository_handlers.go index aa4fdf6030..3971517169 100644 --- a/apps/backend/internal/task/handlers/repository_handlers.go +++ b/apps/backend/internal/task/handlers/repository_handlers.go @@ -4,6 +4,7 @@ import ( "context" "errors" "net/http" + "time" "github.com/gin-gonic/gin" @@ -274,7 +275,20 @@ func (h *RepositoryHandlers) httpGetRepository(c *gin.Context) { func (h *RepositoryHandlers) httpListRepositoryBranches(c *gin.Context) { repoID := c.Param("id") - branches, err := h.service.ListRepositoryBranches(c.Request.Context(), repoID) + ctx := c.Request.Context() + + repo, err := h.service.GetRepository(ctx, repoID) + if err != nil { + handleNotFound(c, h.logger, err, "repository not found") + return + } + + var fetchedAt, fetchError string + if c.Query("refresh") == queryValueTrue { + fetchedAt, fetchError = h.refreshBranchesAtPath(ctx, repoID, repo.LocalPath) + } + + branches, err := h.service.ListLocalRepositoryBranches(ctx, repo.LocalPath) if err != nil { if errors.Is(err, service.ErrPathNotAllowed) { c.JSON(http.StatusBadRequest, gin.H{"error": "repository path is not allowed"}) @@ -284,7 +298,7 @@ func (h *RepositoryHandlers) httpListRepositoryBranches(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list repository branches"}) return } - current := h.currentBranchForWorkspaceRepo(c.Request.Context(), repoID) + current := h.currentBranchAtPath(ctx, repo.LocalPath) dtoBranches := make([]dto.BranchDTO, len(branches)) for i, branch := range branches { dtoBranches[i] = dto.FromBranch(branch) @@ -293,18 +307,37 @@ func (h *RepositoryHandlers) httpListRepositoryBranches(c *gin.Context) { Branches: dtoBranches, Total: len(dtoBranches), CurrentBranch: current, + FetchedAt: fetchedAt, + FetchError: fetchError, }) } -// currentBranchForWorkspaceRepo is best-effort; failures (repo not found, -// detached HEAD, IO error) return an empty string so the branches response -// still ships. -func (h *RepositoryHandlers) currentBranchForWorkspaceRepo(ctx context.Context, repoID string) string { - repo, err := h.service.GetRepository(ctx, repoID) - if err != nil || repo == nil || repo.LocalPath == "" { +// refreshBranchesAtPath runs `git fetch` for the already-resolved repository +// path and returns the timestamp + error string suitable for the response +// body. Failures are best-effort: the branches are still returned so a +// transient network error doesn't blank the dropdown. +func (h *RepositoryHandlers) refreshBranchesAtPath(ctx context.Context, repoID, localPath string) (fetchedAt, fetchError string) { + res, err := h.service.RefreshBranchesAtPath(ctx, localPath) + if err != nil { + h.logger.Warn("branch refresh failed", zap.String("repo_id", repoID), zap.Error(err)) + return "", err.Error() + } + if !res.FetchedAt.IsZero() { + fetchedAt = res.FetchedAt.UTC().Format(time.RFC3339) + } + if res.Err != nil { + fetchError = res.Err.Error() + } + return fetchedAt, fetchError +} + +// currentBranchAtPath is best-effort; failures (empty path, detached HEAD, +// IO error) return an empty string so the branches response still ships. +func (h *RepositoryHandlers) currentBranchAtPath(ctx context.Context, localPath string) string { + if localPath == "" { return "" } - branch, err := h.service.LocalRepositoryCurrentBranch(ctx, repo.LocalPath) + branch, err := h.service.LocalRepositoryCurrentBranch(ctx, localPath) if err != nil { return "" } diff --git a/apps/backend/internal/task/service/repository_fetch.go b/apps/backend/internal/task/service/repository_fetch.go new file mode 100644 index 0000000000..55763acdba --- /dev/null +++ b/apps/backend/internal/task/service/repository_fetch.go @@ -0,0 +1,165 @@ +package service + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "sync" + "time" + + "go.uber.org/zap" + "golang.org/x/sync/singleflight" +) + +// branchFetchCooldown is the minimum interval between successive `git fetch` +// invocations for the same repository when triggered via the branches API. +// The branches dropdown auto-refreshes on every dialog open, so the cooldown +// keeps a noisy UI from spamming the network and the local git index. +const branchFetchCooldown = 30 * time.Second + +// branchFetchTimeout bounds a single fetch invocation. Long-lived auth prompts +// or unreachable remotes must not stall the dropdown beyond this window. +const branchFetchTimeout = 30 * time.Second + +// BranchRefreshResult reports the outcome of a `git fetch` issued by the +// branches API. Skipped is true when the cooldown short-circuited the fetch; +// in that case FetchedAt and Err are copied from the previous attempt so +// callers see the same outcome until the cooldown expires. +type BranchRefreshResult struct { + FetchedAt time.Time + Skipped bool + Err error +} + +// branchFetcher serializes `git fetch` calls for a repository path with a +// per-path cooldown and single-flight deduplication. Concurrent callers for +// the same path observe the same result; callers within the cooldown window +// receive the cached result without re-fetching. +type branchFetcher struct { + group singleflight.Group + mu sync.Mutex + lastByKey map[string]BranchRefreshResult + logger *zap.Logger +} + +func newBranchFetcher(log *zap.Logger) *branchFetcher { + return &branchFetcher{ + lastByKey: make(map[string]BranchRefreshResult), + logger: log, + } +} + +// Fetch runs `git fetch --all --prune --no-tags` for repoPath unless the +// cooldown is still active, in which case the previous result is returned +// with Skipped=true. Concurrent calls for the same path coalesce. +// +// The singleflight closure deliberately uses a context detached from the +// caller's: if the first caller's HTTP request is cancelled mid-flight (e.g. +// browser tab closed), waiting callers must still get a real result and the +// cooldown cache must not be poisoned with context.Canceled. +func (f *branchFetcher) Fetch(ctx context.Context, repoPath string) BranchRefreshResult { + if cached, ok := f.checkCooldown(repoPath); ok { + return cached + } + v, _, _ := f.group.Do(repoPath, func() (any, error) { + // Re-check inside the singleflight to avoid running back-to-back fetches + // when many callers arrived during the previous flight. + if cached, ok := f.checkCooldown(repoPath); ok { + return cached, nil + } + res := f.runFetch(context.WithoutCancel(ctx), repoPath) + f.mu.Lock() + f.lastByKey[repoPath] = res + f.mu.Unlock() + return res, nil + }) + return v.(BranchRefreshResult) +} + +func (f *branchFetcher) checkCooldown(repoPath string) (BranchRefreshResult, bool) { + f.mu.Lock() + defer f.mu.Unlock() + prev, ok := f.lastByKey[repoPath] + if !ok { + return BranchRefreshResult{}, false + } + if time.Since(prev.FetchedAt) >= branchFetchCooldown { + return BranchRefreshResult{}, false + } + return BranchRefreshResult{FetchedAt: prev.FetchedAt, Skipped: true, Err: prev.Err}, true +} + +func (f *branchFetcher) runFetch(ctx context.Context, repoPath string) BranchRefreshResult { + fetchCtx, cancel := context.WithTimeout(ctx, branchFetchTimeout) + defer cancel() + + cmd := newNonInteractiveGitFetchCmd(fetchCtx, repoPath) + output, err := cmd.CombinedOutput() + res := BranchRefreshResult{FetchedAt: time.Now()} + if err != nil { + res.Err = fmt.Errorf("git fetch: %w", err) + if f.logger != nil { + // Avoid logging raw git output: it can contain remote URLs with + // embedded credentials or other sensitive data. Surface only the + // length and a sanitized summary instead. + f.logger.Warn("branch refresh fetch failed", + zap.String("path", repoPath), + zap.Int("output_bytes", len(output)), + zap.Error(err)) + } + } + return res +} + +// newNonInteractiveGitFetchCmd builds a `git fetch --all --prune --no-tags` +// command with the same non-interactive environment used by the worktree +// manager. Auth prompts are disabled so missing credentials surface as fast +// failures rather than hanging the dropdown. +func newNonInteractiveGitFetchCmd(ctx context.Context, repoPath string) *exec.Cmd { + cmd := exec.CommandContext(ctx, "git", "-C", repoPath, "fetch", "--all", "--prune", "--no-tags") + cmd.Env = append(os.Environ(), + "GIT_TERMINAL_PROMPT=0", + "GCM_INTERACTIVE=Never", + "GIT_ASKPASS=echo", + "SSH_ASKPASS=/bin/false", + "GIT_SSH_COMMAND=ssh -oBatchMode=yes", + ) + cmd.WaitDelay = 500 * time.Millisecond + return cmd +} + +// RefreshRepositoryBranches resolves the repository's local path and runs +// `git fetch` for it, subject to a per-repository cooldown and single-flight +// deduplication. Returns ErrPathNotAllowed when the repository's path is not +// inside an allowed discovery root. +func (s *Service) RefreshRepositoryBranches(ctx context.Context, repoID string) (BranchRefreshResult, error) { + repo, err := s.repoEntities.GetRepository(ctx, repoID) + if err != nil { + return BranchRefreshResult{}, err + } + return s.RefreshBranchesAtPath(ctx, repo.LocalPath) +} + +// RefreshBranchesAtPath validates an already-resolved repository path and runs +// `git fetch` for it, subject to the same cooldown and single-flight as +// RefreshRepositoryBranches. Use this when the caller already has the path in +// hand to avoid a redundant repository lookup. +func (s *Service) RefreshBranchesAtPath(ctx context.Context, localPath string) (BranchRefreshResult, error) { + if localPath == "" { + return BranchRefreshResult{}, errors.New("repository local path is empty") + } + absPath, err := filepath.Abs(localPath) + if err != nil { + return BranchRefreshResult{}, fmt.Errorf("invalid repository path: %w", err) + } + if !isPathAllowed(absPath, s.discoveryRoots()) { + return BranchRefreshResult{}, ErrPathNotAllowed + } + if info, statErr := os.Stat(absPath); statErr != nil || !info.IsDir() { + return BranchRefreshResult{}, errors.New("repository path is not a directory") + } + return s.branchFetcher.Fetch(ctx, absPath), nil +} diff --git a/apps/backend/internal/task/service/repository_fetch_test.go b/apps/backend/internal/task/service/repository_fetch_test.go new file mode 100644 index 0000000000..9f9943ae93 --- /dev/null +++ b/apps/backend/internal/task/service/repository_fetch_test.go @@ -0,0 +1,140 @@ +package service + +import ( + "context" + "errors" + "os/exec" + "path/filepath" + "sync" + "testing" +) + +// TestBranchFetcher_CooldownSkipsRepeatedFetches verifies that a second call +// inside the cooldown window returns the cached result instead of re-running +// `git fetch`. +func TestBranchFetcher_CooldownSkipsRepeatedFetches(t *testing.T) { + root := t.TempDir() + repoPath := makeRepoWithCommit(t, filepath.Join(root, "repo")) + + f := newBranchFetcher(nil) + first := f.Fetch(context.Background(), repoPath) + if first.Skipped { + t.Fatalf("first fetch should not be skipped") + } + if first.FetchedAt.IsZero() { + t.Fatalf("first fetch should set FetchedAt") + } + + second := f.Fetch(context.Background(), repoPath) + if !second.Skipped { + t.Fatalf("second fetch within cooldown should be skipped") + } + if !second.FetchedAt.Equal(first.FetchedAt) { + t.Fatalf("skipped fetch should report the prior FetchedAt: got %v, want %v", + second.FetchedAt, first.FetchedAt) + } +} + +// TestBranchFetcher_SingleflightCoalesces verifies that concurrent callers +// share a single fetch invocation. +func TestBranchFetcher_SingleflightCoalesces(t *testing.T) { + root := t.TempDir() + repoPath := makeRepoWithCommit(t, filepath.Join(root, "repo")) + + f := newBranchFetcher(nil) + const N = 8 + var wg sync.WaitGroup + results := make([]BranchRefreshResult, N) + start := make(chan struct{}) + for i := 0; i < N; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + <-start + results[idx] = f.Fetch(context.Background(), repoPath) + }(i) + } + close(start) + wg.Wait() + + // All results should share the same FetchedAt — if singleflight is working, + // only one runFetch ran and its timestamp is broadcast to every caller. + first := results[0].FetchedAt + for i := 1; i < N; i++ { + if !results[i].FetchedAt.Equal(first) { + t.Fatalf("result %d FetchedAt mismatch: got %v, want %v", i, results[i].FetchedAt, first) + } + } +} + +// TestBranchFetcher_RecordsErrorOnInvalidRepo verifies that fetch failures +// are reported in the result without panicking. +func TestBranchFetcher_RecordsErrorOnInvalidRepo(t *testing.T) { + root := t.TempDir() // no .git inside + + f := newBranchFetcher(nil) + res := f.Fetch(context.Background(), root) + if res.Err == nil { + t.Fatalf("expected error fetching non-git directory") + } + if res.FetchedAt.IsZero() { + t.Fatalf("FetchedAt should be set even on error so the cooldown applies") + } +} + +// TestBranchFetcher_BothCooldownAndError verifies that callers within the +// cooldown window after a failed fetch get the cached error instead of +// triggering a new fetch attempt. +func TestBranchFetcher_BothCooldownAndError(t *testing.T) { + root := t.TempDir() + f := newBranchFetcher(nil) + first := f.Fetch(context.Background(), root) + if first.Err == nil { + t.Fatalf("first fetch should have errored") + } + second := f.Fetch(context.Background(), root) + if !second.Skipped { + t.Fatalf("second call within cooldown should be skipped") + } + if second.Err == nil { + t.Fatalf("skipped result should preserve the prior error") + } +} + +// TestBranchFetcher_FirstCallerCancelDoesNotPoisonCache verifies that a +// caller cancelling its context after triggering the singleflight does not +// cause the cached result to record a context.Canceled error. The fetch must +// run to completion using a detached context so subsequent waiters see a real +// outcome. +func TestBranchFetcher_FirstCallerCancelDoesNotPoisonCache(t *testing.T) { + root := t.TempDir() + repoPath := makeRepoWithCommit(t, filepath.Join(root, "repo")) + + f := newBranchFetcher(nil) + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel before Fetch even runs + + res := f.Fetch(ctx, repoPath) + if errors.Is(res.Err, context.Canceled) { + t.Fatalf("cached result must not record context.Canceled from caller's ctx; got %v", res.Err) + } +} + +// makeRepoWithCommit initializes a real git repo with a single commit so that +// `git fetch --all` succeeds (it has nothing to do but exits cleanly). +func makeRepoWithCommit(t *testing.T, path string) string { + t.Helper() + if err := exec.Command("git", "init", "-q", path).Run(); err != nil { + t.Skipf("git unavailable: %v", err) + } + for _, args := range [][]string{ + {"-C", path, "config", "user.email", "t@t.test"}, + {"-C", path, "config", "user.name", "t"}, + {"-C", path, "commit", "--allow-empty", "-q", "-m", "init"}, + } { + if err := exec.Command("git", args...).Run(); err != nil { + t.Fatalf("git %v: %v", args, err) + } + } + return path +} diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 740c4e1197..322e6f815a 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -135,6 +135,7 @@ type Service struct { workflowStepGetter WorkflowStepGetter startStepResolver StartStepResolver quickChatDir string // Directory for quick-chat workspaces (e.g., ~/.kandev/quick-chat) + branchFetcher *branchFetcher } // NewService creates a new task service @@ -156,6 +157,7 @@ func NewService(repos Repos, eventBus bus.EventBus, log *logger.Logger, discover eventBus: eventBus, logger: log, discoveryConfig: discoveryConfig, + branchFetcher: newBranchFetcher(log.Zap()), } } diff --git a/apps/web/components/branch-refresh-button.tsx b/apps/web/components/branch-refresh-button.tsx new file mode 100644 index 0000000000..22e8dee547 --- /dev/null +++ b/apps/web/components/branch-refresh-button.tsx @@ -0,0 +1,66 @@ +"use client"; + +import { useState } from "react"; +import { IconRefresh } from "@tabler/icons-react"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; + +type BranchRefreshButtonProps = { + onRefresh: () => void; + refreshing?: boolean; + fetchedAt?: string; + fetchError?: string; +}; + +export function BranchRefreshButton({ + onRefresh, + refreshing, + fetchedAt, + fetchError, +}: BranchRefreshButtonProps) { + // Controlled open so the tooltip only reacts to hover, not focus. + // Radix Popover auto-focuses the first focusable child when it opens, which + // would otherwise trigger this tooltip the moment the dropdown is opened. + const [open, setOpen] = useState(false); + const hasError = Boolean(fetchError); + const tooltip = formatRefreshTooltip(fetchedAt, refreshing, fetchError); + return ( + + + + + {tooltip} + + ); +} + +function formatRefreshTooltip( + fetchedAt: string | undefined, + refreshing: boolean | undefined, + fetchError: string | undefined, +) { + if (refreshing) return "Refreshing branches..."; + if (fetchError) return `Last refresh failed: ${fetchError}`; + if (!fetchedAt) return "Refresh branches (git fetch)"; + const date = new Date(fetchedAt); + if (Number.isNaN(date.getTime())) return "Refresh branches (git fetch)"; + return `Refresh branches (last fetched ${date.toLocaleTimeString()})`; +} diff --git a/apps/web/components/combobox.tsx b/apps/web/components/combobox.tsx index c570a7a08b..0c70c7cc1e 100644 --- a/apps/web/components/combobox.tsx +++ b/apps/web/components/combobox.tsx @@ -1,7 +1,7 @@ "use client"; import { memo, useState } from "react"; -import { IconCheck, IconChevronDown } from "@tabler/icons-react"; +import { IconCheck, IconChevronDown, IconLoader2 } from "@tabler/icons-react"; import { cn } from "@/lib/utils"; import { Button } from "@kandev/ui/button"; @@ -19,9 +19,14 @@ export type ComboboxOption = { value: string; label: string; description?: string; + keywords?: string[]; renderLabel?: () => React.ReactNode; }; +// Custom filter compatible with cmdk's `` prop. +// Returns a number in [0, 1]; >0 means the option is included, sorted desc. +export type ComboboxFilter = (value: string, search: string, keywords?: string[]) => number; + interface ComboboxProps { options: ComboboxOption[]; value: string; @@ -39,6 +44,12 @@ interface ComboboxProps { popoverAlign?: "start" | "center" | "end"; /** When true, the trigger always renders the plain label text instead of renderLabel. */ plainTrigger?: boolean; + /** Optional custom filter; defaults to cmdk's built-in command-score. */ + filter?: ComboboxFilter; + /** Optional node rendered to the right of the dropdown label (e.g. refresh button). */ + headerAction?: React.ReactNode; + /** When true, swap the trigger chevron for a spinner to indicate loading. */ + loading?: boolean; } function TriggerLabel({ @@ -71,7 +82,7 @@ function OptionsList({ onSelect(option.value)} className="relative pr-7" > @@ -106,6 +117,9 @@ export const Combobox = memo(function Combobox({ popoverSide, popoverAlign = "start", plainTrigger = false, + filter, + headerAction, + loading = false, }: ComboboxProps) { const [open, setOpen] = useState(false); // Track the highlighted item. Defaults to the selected value so the current @@ -138,7 +152,11 @@ export const Combobox = memo(function Combobox({ placeholder={placeholder} /> - + {loading ? ( + + ) : ( + + )} - - {dropdownLabel ? ( -
- {dropdownLabel} + + {dropdownLabel || headerAction ? ( +
+ {dropdownLabel} + {headerAction}
) : null} {showSearch && } diff --git a/apps/web/components/task-create-dialog-form-body.tsx b/apps/web/components/task-create-dialog-form-body.tsx index 92b80b883a..6b4aee819e 100644 --- a/apps/web/components/task-create-dialog-form-body.tsx +++ b/apps/web/components/task-create-dialog-form-body.tsx @@ -29,6 +29,9 @@ type CreateEditSelectorsProps = { branch: string; onBranchChange: (value: string) => void; branchesLoading: boolean; + onRefreshBranches?: () => void; + branchesFetchedAt?: string; + branchesFetchError?: string; localBranchesLoading: boolean; agentProfiles: AgentProfileOption[]; agentProfilesLoading: boolean; @@ -52,6 +55,11 @@ type CreateEditSelectorsProps = { placeholder: string; searchPlaceholder: string; emptyMessage: string; + onRefresh?: () => void; + refreshing?: boolean; + fetchedAt?: string; + fetchError?: string; + loading?: boolean; }>; AgentSelectorComponent: React.ComponentType<{ options: SelectorOption[]; @@ -137,6 +145,9 @@ export const CreateEditSelectors = memo(function CreateEditSelectors( branch, onBranchChange, branchesLoading, + onRefreshBranches, + branchesFetchedAt, + branchesFetchError, localBranchesLoading, executorProfileOptions, executorProfileId, @@ -182,6 +193,11 @@ export const CreateEditSelectors = memo(function CreateEditSelectors( searchPlaceholder="Search branches..." emptyMessage="No branch found." disabled={branchDisabled} + onRefresh={!isLocalWithoutGitHubUrl ? onRefreshBranches : undefined} + refreshing={branchesLoading} + fetchedAt={branchesFetchedAt} + fetchError={branchesFetchError} + loading={branchesLoading || localBranchesLoading} />
{isLocalWithoutGitHubUrl && ( diff --git a/apps/web/components/task-create-dialog-options.tsx b/apps/web/components/task-create-dialog-options.tsx index da5b4376e0..40713c1e45 100644 --- a/apps/web/components/task-create-dialog-options.tsx +++ b/apps/web/components/task-create-dialog-options.tsx @@ -87,9 +87,13 @@ export function useBranchOptions(branchOptionsRaw: Branch[]) { branchObj.type === "remote" && branchObj.remote ? `${branchObj.remote}/${branchObj.name}` : branchObj.name; + // Keywords give the scorer extra surfaces to match against: the leaf + // branch name, every path segment, and (for remotes) the remote name. + const keywords = buildBranchKeywords(branchObj.name, branchObj.remote); return { value: displayName, label: displayName, + keywords, renderLabel: () => ( @@ -108,6 +112,20 @@ export function useBranchOptions(branchOptionsRaw: Branch[]) { }, [branchOptionsRaw]); } +const BRANCH_SEGMENT_RE = /[/_.\-\s]+/; + +function buildBranchKeywords(name: string, remote?: string): string[] { + const out = new Set(); + out.add(name); + const leafIdx = name.lastIndexOf("/"); + if (leafIdx >= 0) out.add(name.slice(leafIdx + 1)); + for (const seg of name.split(BRANCH_SEGMENT_RE)) { + if (seg) out.add(seg); + } + if (remote) out.add(remote); + return Array.from(out); +} + export function useAgentProfileOptions(agentProfiles: AgentProfileOption[]): OptionItem[] { return useMemo(() => { return agentProfiles.map((profile: AgentProfileOption) => { diff --git a/apps/web/components/task-create-dialog-selectors.tsx b/apps/web/components/task-create-dialog-selectors.tsx index 09c26341bf..3099092e8f 100644 --- a/apps/web/components/task-create-dialog-selectors.tsx +++ b/apps/web/components/task-create-dialog-selectors.tsx @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- groups all create-dialog selector subcomponents; splitting per-selector files is a separate refactor. */ "use client"; import { useEffect, useRef, useState, memo, useCallback, useMemo } from "react"; @@ -5,6 +6,8 @@ import { Textarea } from "@kandev/ui/textarea"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { IconPaperclip } from "@tabler/icons-react"; import { Combobox } from "./combobox"; +import { scoreBranch } from "@/lib/utils/branch-filter"; +import { BranchRefreshButton } from "./branch-refresh-button"; import { processFile, formatBytes, @@ -70,6 +73,7 @@ export const RepositorySelector = memo(function RepositorySelector({ type BranchOption = { value: string; label: string; + keywords?: string[]; renderLabel: () => React.ReactNode; }; @@ -81,6 +85,11 @@ type BranchSelectorProps = { placeholder: string; searchPlaceholder: string; emptyMessage: string; + onRefresh?: () => void; + refreshing?: boolean; + fetchedAt?: string; + fetchError?: string; + loading?: boolean; }; export const BranchSelector = memo(function BranchSelector({ @@ -91,7 +100,20 @@ export const BranchSelector = memo(function BranchSelector({ placeholder, searchPlaceholder, emptyMessage, + onRefresh, + refreshing, + fetchedAt, + fetchError, + loading, }: BranchSelectorProps) { + const headerAction = onRefresh ? ( + + ) : undefined; return ( ); }); diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index 0efb0006b1..2982dc96b6 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -532,10 +532,13 @@ export function useTaskCreateDialogData( useSettingsData(open); const { repositories, isLoading: repositoriesLoading } = useRepositories(workspaceId, open); - const { branches, isLoading: branchesLoading } = useRepositoryBranches( - fs.repositoryId || null, - Boolean(open && fs.repositoryId), - ); + const { + branches, + isLoading: branchesLoading, + refresh: refreshBranches, + fetchedAt: branchesFetchedAt, + fetchError: branchesFetchError, + } = useRepositoryBranches(fs.repositoryId || null, Boolean(open && fs.repositoryId)); const computed = useDialogComputed({ fs, open, @@ -560,6 +563,9 @@ export function useTaskCreateDialogData( repositoriesLoading, branches, branchesLoading, + refreshBranches, + branchesFetchedAt, + branchesFetchError, computed, }; } diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index 8fc240bd67..cb63ab9b9c 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -84,6 +84,9 @@ type DialogFormBodyProps = { onLinearImport?: (issue: LinearIssue) => void; branchOptions: ReturnType; branchesLoading: boolean; + onRefreshBranches: () => void; + branchesFetchedAt?: string; + branchesFetchError?: string; agentProfileOptions: ReturnType; executorProfileOptions: Array<{ value: string; @@ -110,6 +113,7 @@ type DialogFormBodyProps = { onToggleFreshBranch: (enabled: boolean) => void; }; +// eslint-disable-next-line max-lines-per-function -- thin pass-through; each section already factored into its own component function DialogFormBody({ isSessionMode, isCreateMode, @@ -122,6 +126,9 @@ function DialogFormBody({ onLinearImport, branchOptions, branchesLoading, + onRefreshBranches, + branchesFetchedAt, + branchesFetchError, agentProfileOptions, executorProfileOptions, agentProfiles, @@ -164,6 +171,9 @@ function DialogFormBody({ hasRepositorySelection, branchOptions, branchesLoading, + onRefreshBranches, + branchesFetchedAt, + branchesFetchError, agentProfiles, agentProfilesLoading, agentProfileOptions, @@ -212,6 +222,9 @@ type CreateEditSelectorsRenderArgs = Pick< | "hasRepositorySelection" | "branchOptions" | "branchesLoading" + | "onRefreshBranches" + | "branchesFetchedAt" + | "branchesFetchError" | "agentProfiles" | "agentProfilesLoading" | "agentProfileOptions" @@ -237,6 +250,9 @@ function renderCreateEditSelectors(args: CreateEditSelectorsRenderArgs) { branch={fs.branch} onBranchChange={args.onBranchChange} branchesLoading={args.branchesLoading} + onRefreshBranches={args.onRefreshBranches} + branchesFetchedAt={args.branchesFetchedAt} + branchesFetchError={args.branchesFetchError} localBranchesLoading={fs.localBranchesLoading} agentProfiles={args.agentProfiles} agentProfilesLoading={args.agentProfilesLoading} @@ -392,6 +408,9 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) { repositoriesLoading, branches, branchesLoading, + refreshBranches, + branchesFetchedAt, + branchesFetchError, computed, } = useTaskCreateDialogData(open, workspaceId, workflowId, defaultStepId, fs); const repositoryLocalPath = (() => { @@ -439,6 +458,9 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) { snapshots, repositoriesLoading, branchesLoading, + refreshBranches, + branchesFetchedAt, + branchesFetchError, computed, handlers, submitHandlers, @@ -457,7 +479,8 @@ type DialogFormProps = { function DialogForm({ setup, workspaceId }: DialogFormProps) { const { fs, isSessionMode, isCreateMode, isTaskStarted, workflows, agentProfiles } = setup; const { snapshots, branchesLoading, computed, handlers, handleKeyDown } = setup; - const { handleJiraImport, handleLinearImport } = setup; + const { handleJiraImport, handleLinearImport, refreshBranches, branchesFetchedAt } = setup; + const { branchesFetchError } = setup; const { handleSubmit, handleUpdateWithoutAgent, handleCreateWithoutAgent } = setup.submitHandlers; const { handleCreateWithPlanMode, handleCancel } = setup.submitHandlers; return ( @@ -474,6 +497,9 @@ function DialogForm({ setup, workspaceId }: DialogFormProps) { onLinearImport={handleLinearImport} branchOptions={computed.branchOptions} branchesLoading={branchesLoading || (fs.useGitHubUrl && fs.githubBranchesLoading)} + onRefreshBranches={refreshBranches} + branchesFetchedAt={branchesFetchedAt} + branchesFetchError={branchesFetchError} agentProfileOptions={computed.agentProfileOptions} executorProfileOptions={computed.executorProfileOptions} agentProfiles={agentProfiles} diff --git a/apps/web/e2e/tests/task/create-task-branch-selector.spec.ts b/apps/web/e2e/tests/task/create-task-branch-selector.spec.ts index eb34e55e0b..97a1650de0 100644 --- a/apps/web/e2e/tests/task/create-task-branch-selector.spec.ts +++ b/apps/web/e2e/tests/task/create-task-branch-selector.spec.ts @@ -5,6 +5,10 @@ import { test, expect } from "../../fixtures/test-base"; import { KanbanPage } from "../../pages/kanban-page"; import { makeGitEnv } from "../../helpers/git-helper"; +function escapeRe(s: string) { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + test.describe("Branch selector behavior with executor types", () => { test.describe.configure({ retries: 1 }); @@ -236,10 +240,6 @@ test.describe("Fresh-branch flow", () => { .click(); } - function escapeRe(s: string) { - return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - } - test("toggle off (default) — branch selector disabled, placeholder shows current branch", async ({ testPage, apiClient, @@ -518,3 +518,147 @@ test.describe("Fresh-branch flow", () => { } }); }); + +test.describe("Branch refresh + filter", () => { + test.describe.configure({ retries: 1 }); + + test("refresh button in branch dropdown triggers ?refresh=true request", async ({ + testPage, + apiClient, + seedData, + }) => { + if (!seedData.worktreeExecutorProfileId) { + test.skip(true, "No worktree executor profile in seed data"); + return; + } + const { executors } = await apiClient.listExecutors(); + const worktreeProfileName = executors + .flatMap((e) => e.profiles ?? []) + .find((p) => p.id === seedData.worktreeExecutorProfileId)?.name; + if (!worktreeProfileName) { + test.skip(true, "Could not resolve worktree profile name"); + return; + } + + // Resolve the seeded repo's name via the workspace API so we can select + // it explicitly — earlier specs in this worker may have registered extra + // repos in the same workspace, and we need the asserted ?refresh URL to + // be unambiguous. + const repoListRes = await apiClient.rawRequest( + "GET", + `/api/v1/workspaces/${seedData.workspaceId}/repositories`, + ); + const repoList = (await repoListRes.json()) as { + repositories: Array<{ id: string; name: string }>; + }; + const seededRepoName = repoList.repositories.find((r) => r.id === seedData.repositoryId)?.name; + if (!seededRepoName) { + test.skip(true, "Could not resolve seeded repository name"); + return; + } + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.createTaskButton.first().click(); + await expect(testPage.getByTestId("create-task-dialog")).toBeVisible(); + await testPage.getByTestId("task-title-input").fill("Refresh button test"); + await testPage.getByTestId("task-description-input").fill("triggers git fetch"); + await testPage.getByTestId("repository-selector").click(); + await testPage + .getByRole("option", { name: new RegExp(`^${escapeRe(seededRepoName)}\\b`, "i") }) + .first() + .click(); + // Worktree executor → branch selector enabled and refresh button visible. + await testPage.getByTestId("executor-profile-selector").click(); + await testPage.getByRole("option", { name: worktreeProfileName }).click(); + + const branchSelector = testPage.getByTestId("branch-selector"); + await expect(branchSelector).toBeEnabled({ timeout: 5_000 }); + await branchSelector.click(); + + const refreshButton = testPage.getByTestId("branch-refresh-button"); + await expect(refreshButton).toBeVisible({ timeout: 5_000 }); + + // Wait until the cold-load refresh request finishes so the button isn't + // disabled when we click it. + await expect(refreshButton).toBeEnabled({ timeout: 10_000 }); + + const refreshRequest = testPage.waitForRequest( + (req) => + req.url().includes(`/repositories/${seedData.repositoryId}/branches`) && + req.url().includes("refresh=true") && + req.method() === "GET", + ); + await refreshButton.click(); + await refreshRequest; + }); + + test("branch filter ranks exact match above substring matches", async ({ + testPage, + apiClient, + backend, + seedData, + }) => { + if (!seedData.worktreeExecutorProfileId) { + test.skip(true, "No worktree executor profile in seed data"); + return; + } + const { executors } = await apiClient.listExecutors(); + const worktreeProfileName = executors + .flatMap((e) => e.profiles ?? []) + .find((p) => p.id === seedData.worktreeExecutorProfileId)?.name; + if (!worktreeProfileName) { + test.skip(true, "Could not resolve worktree profile name"); + return; + } + + // Seed an isolated repo with branches that contain overlapping substrings + // so the filter has something interesting to rank. + const repoName = "E2E Filter Repo"; + const repoDir = path.join(backend.tmpDir, "repos", "e2e-filter-branches"); + fs.mkdirSync(repoDir, { recursive: true }); + const env = makeGitEnv(backend.tmpDir); + execSync("git init -b main", { cwd: repoDir, env }); + execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env }); + // -B creates or resets the branch so the test is idempotent across retries. + for (const name of ["develop", "feature/develop-helpers", "feature/auth-develop-flow"]) { + execSync(`git checkout -B ${name}`, { cwd: repoDir, env }); + execSync(`git commit --allow-empty -m "${name}"`, { cwd: repoDir, env }); + execSync("git checkout main", { cwd: repoDir, env }); + } + await apiClient.createRepository(seedData.workspaceId, repoDir, "main", { name: repoName }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.createTaskButton.first().click(); + await expect(testPage.getByTestId("create-task-dialog")).toBeVisible(); + await testPage.getByTestId("task-title-input").fill("Filter ranking test"); + await testPage.getByTestId("task-description-input").fill("exact match wins"); + await testPage.getByTestId("repository-selector").click(); + await testPage + .getByRole("option", { name: new RegExp(`^${repoName}\\b`, "i") }) + .first() + .click(); + await testPage.getByTestId("executor-profile-selector").click(); + await testPage.getByRole("option", { name: worktreeProfileName }).click(); + + const branchSelector = testPage.getByTestId("branch-selector"); + await expect(branchSelector).toBeEnabled({ timeout: 5_000 }); + await branchSelector.click(); + + // Wait for the cold-load fetch to populate the dropdown with our branches. + await expect(testPage.getByRole("option", { name: /feature\/develop-helpers/ })).toBeVisible({ + timeout: 10_000, + }); + + await testPage.getByPlaceholder("Search branches...").fill("develop"); + + // After filtering, the exact match "develop" must rank first. cmdk renders + // matched options in descending score order, so the first [role="option"] + // in the DOM is the top-ranked branch. Options carry the branch name in + // data-value; the visible text also contains a "local" badge so we assert + // on the attribute rather than text content. + const firstOption = testPage.getByRole("option").first(); + await expect(firstOption).toHaveAttribute("data-value", "develop", { timeout: 5_000 }); + }); +}); diff --git a/apps/web/hooks/domains/workspace/use-repository-branches.ts b/apps/web/hooks/domains/workspace/use-repository-branches.ts index 306d1d2664..05bf41f5c0 100644 --- a/apps/web/hooks/domains/workspace/use-repository-branches.ts +++ b/apps/web/hooks/domains/workspace/use-repository-branches.ts @@ -1,10 +1,15 @@ -import { useEffect, useRef } from "react"; +import { useCallback, useEffect, useRef } from "react"; import { useAppStore } from "@/components/state-provider"; import { listRepositoryBranches } from "@/lib/api"; import type { Branch } from "@/lib/types/http"; const EMPTY_BRANCHES: Branch[] = []; +// Stale-while-revalidate: when the cache is older than this we fire a +// background refresh on hook mount/repo change. The backend additionally +// enforces a per-repo cooldown so this is the soft, client-side check. +const CLIENT_STALE_MS = 60_000; + export function useRepositoryBranches(repositoryId: string | null, enabled = true) { const branches = useAppStore((state) => repositoryId @@ -17,30 +22,69 @@ export function useRepositoryBranches(repositoryId: string | null, enabled = tru const isLoading = useAppStore((state) => repositoryId ? (state.repositoryBranches.loadingByRepositoryId[repositoryId] ?? false) : false, ); + const fetchedAt = useAppStore((state) => + repositoryId ? state.repositoryBranches.fetchedAtByRepositoryId[repositoryId] : undefined, + ); + const fetchError = useAppStore((state) => + repositoryId ? state.repositoryBranches.fetchErrorByRepositoryId[repositoryId] : undefined, + ); const setRepositoryBranches = useAppStore((state) => state.setRepositoryBranches); const setRepositoryBranchesLoading = useAppStore((state) => state.setRepositoryBranchesLoading); - const inFlightRef = useRef(false); + const setRepositoryBranchesFetchError = useAppStore( + (state) => state.setRepositoryBranchesFetchError, + ); + const inFlightRef = useRef(null); + + // Stable fetcher closed over the store actions; the repo id is a parameter + // so the same identity works for both mount-time fetches and refresh(). + const runFetch = useCallback( + (repoId: string, refresh: boolean) => { + if (inFlightRef.current === repoId) return; + inFlightRef.current = repoId; + setRepositoryBranchesLoading(repoId, true); + listRepositoryBranches(repoId, { refresh }, { cache: "no-store" }) + .then((response) => { + setRepositoryBranches(repoId, response.branches, { + fetchedAt: response.fetched_at, + fetchError: response.fetch_error, + }); + }) + .catch((err: unknown) => { + // Stale-while-revalidate: keep any previously-cached branches in + // place on transport failure so the dropdown stays usable. Only the + // fetchError is updated so callers can surface the failure. + const message = err instanceof Error ? err.message : "request failed"; + setRepositoryBranchesFetchError(repoId, message); + }) + .finally(() => { + if (inFlightRef.current === repoId) inFlightRef.current = null; + setRepositoryBranchesLoading(repoId, false); + }); + }, + [setRepositoryBranches, setRepositoryBranchesLoading, setRepositoryBranchesFetchError], + ); useEffect(() => { if (!enabled || !repositoryId) return; - if (isLoaded || inFlightRef.current) return; - inFlightRef.current = true; - setRepositoryBranchesLoading(repositoryId, true); - // Capture the repositoryId at effect start for the closure - const fetchRepoId = repositoryId; - listRepositoryBranches(fetchRepoId, { cache: "no-store" }) - .then((response) => { - // Safe to always store: data is keyed by repositoryId in the store - setRepositoryBranches(fetchRepoId, response.branches); - }) - .catch(() => { - setRepositoryBranches(fetchRepoId, []); - }) - .finally(() => { - inFlightRef.current = false; - setRepositoryBranchesLoading(fetchRepoId, false); - }); - }, [enabled, isLoaded, repositoryId, setRepositoryBranches, setRepositoryBranchesLoading]); + // Cold load: nothing in the cache, do a foreground fetch with refresh so + // the user sees fresh remote branches the first time the dialog opens. + if (!isLoaded) { + runFetch(repositoryId, true); + return; + } + // Warm cache, possibly stale: revalidate in the background. The backend + // cooldown ensures this doesn't hammer git when the dialog is reopened + // in quick succession. + const ageMs = fetchedAt ? Date.now() - new Date(fetchedAt).getTime() : Infinity; + if (ageMs >= CLIENT_STALE_MS) { + runFetch(repositoryId, true); + } + }, [enabled, isLoaded, repositoryId, fetchedAt, runFetch]); - return { branches, isLoading }; + const refresh = useCallback(() => { + if (!repositoryId) return; + runFetch(repositoryId, true); + }, [repositoryId, runFetch]); + + return { branches, isLoading, fetchedAt, fetchError, refresh }; } diff --git a/apps/web/lib/api/domains/workspace-api.ts b/apps/web/lib/api/domains/workspace-api.ts index f95558307f..1259073b32 100644 --- a/apps/web/lib/api/domains/workspace-api.ts +++ b/apps/web/lib/api/domains/workspace-api.ts @@ -37,9 +37,14 @@ export async function listRepositories( return fetchJson(url, options); } -export async function listRepositoryBranches(repositoryId: string, options?: ApiRequestOptions) { +export async function listRepositoryBranches( + repositoryId: string, + params?: { refresh?: boolean }, + options?: ApiRequestOptions, +) { + const qs = params?.refresh ? "?refresh=true" : ""; return fetchJson( - `/api/v1/repositories/${repositoryId}/branches`, + `/api/v1/repositories/${repositoryId}/branches${qs}`, options, ); } diff --git a/apps/web/lib/state/slices/workspace/types.ts b/apps/web/lib/state/slices/workspace/types.ts index 942e350fe9..b66297e5f0 100644 --- a/apps/web/lib/state/slices/workspace/types.ts +++ b/apps/web/lib/state/slices/workspace/types.ts @@ -26,6 +26,10 @@ export type RepositoryBranchesState = { itemsByRepositoryId: Record; loadingByRepositoryId: Record; loadedByRepositoryId: Record; + // RFC3339 timestamp of the most recent successful refresh from the backend. + fetchedAtByRepositoryId: Record; + // Last fetch error for each repository, when a refresh attempt failed. + fetchErrorByRepositoryId: Record; }; export type RepositoryScriptsState = { @@ -46,8 +50,13 @@ export type WorkspaceSliceActions = { setWorkspaces: (workspaces: WorkspaceState["items"]) => void; setRepositories: (workspaceId: string, repositories: Repository[]) => void; setRepositoriesLoading: (workspaceId: string, loading: boolean) => void; - setRepositoryBranches: (repositoryId: string, branches: Branch[]) => void; + setRepositoryBranches: ( + repositoryId: string, + branches: Branch[], + meta?: { fetchedAt?: string; fetchError?: string }, + ) => void; setRepositoryBranchesLoading: (repositoryId: string, loading: boolean) => void; + setRepositoryBranchesFetchError: (repositoryId: string, error: string | undefined) => void; setRepositoryScripts: (repositoryId: string, scripts: RepositoryScript[]) => void; setRepositoryScriptsLoading: (repositoryId: string, loading: boolean) => void; clearRepositoryScripts: (repositoryId: string) => void; diff --git a/apps/web/lib/state/slices/workspace/workspace-slice.test.ts b/apps/web/lib/state/slices/workspace/workspace-slice.test.ts new file mode 100644 index 0000000000..80733114a1 --- /dev/null +++ b/apps/web/lib/state/slices/workspace/workspace-slice.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "vitest"; +import { create } from "zustand"; +import { immer } from "zustand/middleware/immer"; +import { createWorkspaceSlice } from "./workspace-slice"; +import type { WorkspaceSlice } from "./types"; +import type { Branch } from "@/lib/types/http"; + +function makeStore() { + return create()( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + immer((...a) => ({ ...(createWorkspaceSlice as any)(...a) })), + ); +} + +const REPO = "repo-1"; +const BRANCHES: Branch[] = [{ name: "main", type: "local" }]; +const FETCHED_AT = "2026-04-30T10:00:00Z"; + +describe("setRepositoryBranches", () => { + it("stores branches and marks loaded without meta", () => { + const s = makeStore(); + s.getState().setRepositoryBranches(REPO, BRANCHES); + const state = s.getState().repositoryBranches; + expect(state.itemsByRepositoryId[REPO]).toEqual(BRANCHES); + expect(state.loadedByRepositoryId[REPO]).toBe(true); + expect(state.loadingByRepositoryId[REPO]).toBe(false); + expect(state.fetchedAtByRepositoryId[REPO]).toBeUndefined(); + expect(state.fetchErrorByRepositoryId[REPO]).toBeUndefined(); + }); + + it("records fetchedAt + clears prior fetchError on success", () => { + const s = makeStore(); + s.getState().setRepositoryBranches(REPO, BRANCHES, { fetchError: "boom" }); + expect(s.getState().repositoryBranches.fetchErrorByRepositoryId[REPO]).toBe("boom"); + s.getState().setRepositoryBranches(REPO, BRANCHES, { fetchedAt: FETCHED_AT }); + const state = s.getState().repositoryBranches; + expect(state.fetchedAtByRepositoryId[REPO]).toBe(FETCHED_AT); + expect(state.fetchErrorByRepositoryId[REPO]).toBeUndefined(); + }); + + it("preserves the prior fetchedAt when meta omits it", () => { + const s = makeStore(); + s.getState().setRepositoryBranches(REPO, BRANCHES, { fetchedAt: FETCHED_AT }); + s.getState().setRepositoryBranches(REPO, BRANCHES); + expect(s.getState().repositoryBranches.fetchedAtByRepositoryId[REPO]).toBe(FETCHED_AT); + }); +}); + +describe("setRepositoryBranchesLoading", () => { + it("toggles only the loading flag", () => { + const s = makeStore(); + s.getState().setRepositoryBranchesLoading(REPO, true); + expect(s.getState().repositoryBranches.loadingByRepositoryId[REPO]).toBe(true); + s.getState().setRepositoryBranchesLoading(REPO, false); + expect(s.getState().repositoryBranches.loadingByRepositoryId[REPO]).toBe(false); + }); +}); + +describe("setRepositoryBranchesFetchError", () => { + it("records the error without touching cached branches", () => { + const s = makeStore(); + s.getState().setRepositoryBranches(REPO, BRANCHES, { fetchedAt: FETCHED_AT }); + s.getState().setRepositoryBranchesFetchError(REPO, "network down"); + const state = s.getState().repositoryBranches; + expect(state.fetchErrorByRepositoryId[REPO]).toBe("network down"); + // Cached branches and fetchedAt are preserved so the dropdown stays usable + // (stale-while-revalidate semantics). + expect(state.itemsByRepositoryId[REPO]).toEqual(BRANCHES); + expect(state.fetchedAtByRepositoryId[REPO]).toBe(FETCHED_AT); + }); + + it("clears the error when called with undefined", () => { + const s = makeStore(); + s.getState().setRepositoryBranchesFetchError(REPO, "boom"); + s.getState().setRepositoryBranchesFetchError(REPO, undefined); + expect(s.getState().repositoryBranches.fetchErrorByRepositoryId[REPO]).toBeUndefined(); + }); +}); diff --git a/apps/web/lib/state/slices/workspace/workspace-slice.ts b/apps/web/lib/state/slices/workspace/workspace-slice.ts index c6a1b7b67a..f5b5ab3d7f 100644 --- a/apps/web/lib/state/slices/workspace/workspace-slice.ts +++ b/apps/web/lib/state/slices/workspace/workspace-slice.ts @@ -8,6 +8,8 @@ export const defaultWorkspaceState: WorkspaceSliceState = { itemsByRepositoryId: {}, loadingByRepositoryId: {}, loadedByRepositoryId: {}, + fetchedAtByRepositoryId: {}, + fetchErrorByRepositoryId: {}, }, repositoryScripts: { itemsByRepositoryId: {}, @@ -48,16 +50,26 @@ export const createWorkspaceSlice: StateCreator< set((draft) => { draft.repositories.loadingByWorkspaceId[workspaceId] = loading; }), - setRepositoryBranches: (repositoryId, branches) => + setRepositoryBranches: (repositoryId, branches, meta) => set((draft) => { draft.repositoryBranches.itemsByRepositoryId[repositoryId] = branches; draft.repositoryBranches.loadingByRepositoryId[repositoryId] = false; draft.repositoryBranches.loadedByRepositoryId[repositoryId] = true; + if (meta?.fetchedAt !== undefined) { + draft.repositoryBranches.fetchedAtByRepositoryId[repositoryId] = meta.fetchedAt; + } + // fetchError is replaced on every successful response (empty string clears it). + draft.repositoryBranches.fetchErrorByRepositoryId[repositoryId] = + meta?.fetchError ?? undefined; }), setRepositoryBranchesLoading: (repositoryId, loading) => set((draft) => { draft.repositoryBranches.loadingByRepositoryId[repositoryId] = loading; }), + setRepositoryBranchesFetchError: (repositoryId, error) => + set((draft) => { + draft.repositoryBranches.fetchErrorByRepositoryId[repositoryId] = error; + }), setRepositoryScripts: (repositoryId, scripts) => set((draft) => { draft.repositoryScripts.itemsByRepositoryId[repositoryId] = scripts; diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index c082946a6d..393874c5bd 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -291,8 +291,13 @@ export type AppState = { setAgentProfiles: (profiles: AgentProfilesState["items"]) => void; setRepositories: (workspaceId: string, repositories: Repository[]) => void; setRepositoriesLoading: (workspaceId: string, loading: boolean) => void; - setRepositoryBranches: (repositoryId: string, branches: Branch[]) => void; + setRepositoryBranches: ( + repositoryId: string, + branches: Branch[], + meta?: { fetchedAt?: string; fetchError?: string }, + ) => void; setRepositoryBranchesLoading: (repositoryId: string, loading: boolean) => void; + setRepositoryBranchesFetchError: (repositoryId: string, error: string | undefined) => void; setRepositoryScripts: (repositoryId: string, scripts: RepositoryScript[]) => void; setRepositoryScriptsLoading: (repositoryId: string, loading: boolean) => void; clearRepositoryScripts: (repositoryId: string) => void; diff --git a/apps/web/lib/types/http.ts b/apps/web/lib/types/http.ts index 68f9bf6083..56913885c7 100644 --- a/apps/web/lib/types/http.ts +++ b/apps/web/lib/types/http.ts @@ -495,6 +495,12 @@ export type RepositoryBranchesResponse = { branches: Branch[]; total: number; current_branch?: string; + // RFC3339 timestamp of the most recent `git fetch` for this repository, + // when refresh was requested. Empty if no refresh has been performed. + fetched_at?: string; + // Human-readable error from the last fetch attempt for this request, if + // one was attempted and failed. Empty otherwise. + fetch_error?: string; }; export type LocalRepositoryStatusResponse = { diff --git a/apps/web/lib/utils/branch-filter.test.ts b/apps/web/lib/utils/branch-filter.test.ts new file mode 100644 index 0000000000..756b152e7b --- /dev/null +++ b/apps/web/lib/utils/branch-filter.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect } from "vitest"; +import { scoreBranch } from "./branch-filter"; + +function rank(values: string[], q: string, keywords?: Record): string[] { + return values + .map((v) => ({ v, s: scoreBranch(v, q, keywords?.[v]) })) + .filter((x) => x.s > 0) + .sort((a, b) => b.s - a.s) + .map((x) => x.v); +} + +describe("scoreBranch", () => { + it("returns a positive score for empty query so all options stay visible", () => { + expect(scoreBranch("origin/main", "")).toBeGreaterThan(0); + expect(scoreBranch("anything", " ")).toBeGreaterThan(0); + }); + + it("scores exact full match highest", () => { + const exact = scoreBranch("main", "main"); + const leafExact = scoreBranch("origin/main", "main"); + const prefix = scoreBranch("mainline", "main"); + expect(exact).toBeGreaterThan(leafExact); + expect(leafExact).toBeGreaterThan(prefix); + }); + + it("ranks the leaf-name match above a mid-string substring", () => { + const ranked = rank(["origin/feature/foo-bar", "barometer/main"], "bar"); + expect(ranked[0]).toBe("barometer/main"); // full prefix wins over leaf substring + // But leaf-segment match still scores + expect(scoreBranch("origin/feature/foo-bar", "bar")).toBeGreaterThan(0); + }); + + it("ranks segment-boundary matches above mid-word matches", () => { + const segment = scoreBranch("feature/auth/login", "auth"); + const midWord = scoreBranch("zauthority/login", "auth"); + expect(segment).toBeGreaterThan(midWord); + }); + + it("treats /, -, _, . as segment separators", () => { + expect(scoreBranch("feature_auth-login", "auth")).toBeGreaterThan(0); + expect(scoreBranch("feature.auth.login", "auth")).toBeGreaterThan(0); + }); + + it("falls back to fuzzy subsequence match when no substring matches", () => { + const score = scoreBranch("feature/foobar", "ffb"); + expect(score).toBeGreaterThan(0); + // ...but a substring match outranks the fuzzy fallback + const sub = scoreBranch("feature/foobar", "foo"); + expect(sub).toBeGreaterThan(score); + }); + + it("returns 0 when no signal is present at all", () => { + expect(scoreBranch("main", "xyz")).toBe(0); + }); + + it("is case-insensitive", () => { + expect(scoreBranch("Origin/Feature/Foo", "FEATURE")).toBeGreaterThan(0); + expect(scoreBranch("MAIN", "main")).toBe(scoreBranch("main", "MAIN")); + }); + + it("uses keywords as a secondary signal when value doesn't match", () => { + const noKeywords = scoreBranch("origin/release-1.2.3", "auth"); + const withKeywords = scoreBranch("origin/release-1.2.3", "auth", ["auth-feature"]); + expect(noKeywords).toBe(0); + expect(withKeywords).toBeGreaterThan(0); + }); + + it("prefers shorter haystacks among substring matches (length penalty)", () => { + const shortHit = scoreBranch("foo-bar", "bar"); + const longHit = scoreBranch("a".repeat(900) + "bar", "bar"); + expect(shortHit).toBeGreaterThan(longHit); + }); + + it("ranks long branch list realistically", () => { + const list = [ + "main", + "origin/main", + "origin/feature/auth/login", + "origin/feature/auth/logout", + "origin/feature/billing/checkout", + "origin/release/2026-04-15", + "origin/some-very-long-branch-name-that-mentions-auth-somewhere", + ]; + const ranked = rank(list, "auth"); + // Segment-boundary matches come before the mid-name occurrence. + expect(ranked.indexOf("origin/feature/auth/login")).toBeLessThan( + ranked.indexOf("origin/some-very-long-branch-name-that-mentions-auth-somewhere"), + ); + expect(ranked.indexOf("origin/feature/auth/logout")).toBeLessThan( + ranked.indexOf("origin/some-very-long-branch-name-that-mentions-auth-somewhere"), + ); + }); +}); diff --git a/apps/web/lib/utils/branch-filter.ts b/apps/web/lib/utils/branch-filter.ts new file mode 100644 index 0000000000..1d251bf476 --- /dev/null +++ b/apps/web/lib/utils/branch-filter.ts @@ -0,0 +1,125 @@ +// Custom scorer for the branch combobox in the new-task dialog. +// +// cmdk's default `command-score` filter is biased toward short strings and +// doesn't understand path-like separators (`/`, `-`, `_`, `.`), so long +// branches such as `origin/feature/foo-bar-baz` rank poorly when the user +// types a fragment like `bar`. This scorer is path-aware, ranks word-boundary +// matches above mid-word matches, and falls back to a plain subsequence +// (fuzzy) match so misspellings still surface results. +// +// The signature matches the cmdk `` prop: returns a number +// in [0, 1]; cmdk treats >0 as "include" and sorts items in descending order. + +const SEGMENT_RE = /[/_.\-\s]+/g; + +const SCORE_EXACT = 1.0; +const SCORE_LEAF_EXACT = 0.95; +const SCORE_FULL_PREFIX = 0.9; +const SCORE_LEAF_PREFIX = 0.85; +const SCORE_SEGMENT_PREFIX = 0.75; +const SCORE_FULL_SUBSTRING = 0.65; +const SCORE_SEGMENT_SUBSTRING = 0.5; +const SCORE_KEYWORD_PREFIX = 0.7; +const SCORE_KEYWORD_SUBSTRING = 0.45; +const SCORE_SUBSEQUENCE_BASE = 0.3; + +function leafSegment(value: string): string { + const idx = value.lastIndexOf("/"); + return idx >= 0 ? value.slice(idx + 1) : value; +} + +function segments(value: string): string[] { + return value.split(SEGMENT_RE).filter((s) => s.length > 0); +} + +function isSubsequence(needle: string, haystack: string): boolean { + if (needle.length === 0) return true; + let i = 0; + for (let j = 0; j < haystack.length && i < needle.length; j++) { + if (haystack[j] === needle[i]) i++; + } + return i === needle.length; +} + +// Subsequence density: how tightly the matched chars cluster in the haystack. +// 1.0 means contiguous, lower means more spread out. +function subsequenceDensity(needle: string, haystack: string): number { + if (needle.length === 0) return 1; + let first = -1; + let last = -1; + let i = 0; + for (let j = 0; j < haystack.length && i < needle.length; j++) { + if (haystack[j] === needle[i]) { + if (first < 0) first = j; + last = j; + i++; + } + } + if (i < needle.length || first < 0) return 0; + const span = last - first + 1; + return needle.length / span; +} + +function scoreSegments(segs: string[], q: string, prefixScore: number, subScore: number): number { + let best = 0; + for (const seg of segs) { + if (seg === q) return prefixScore + 0.05; + if (seg.startsWith(q)) best = Math.max(best, prefixScore); + else if (seg.includes(q)) best = Math.max(best, subScore); + } + return best; +} + +function scoreKeywords(keywords: string[] | undefined, q: string): number { + if (!keywords || keywords.length === 0) return 0; + let best = 0; + for (const raw of keywords) { + if (!raw) continue; + const k = raw.toLowerCase(); + if (k === q) best = Math.max(best, SCORE_KEYWORD_PREFIX + 0.05); + else if (k.startsWith(q)) best = Math.max(best, SCORE_KEYWORD_PREFIX); + else if (k.includes(q)) best = Math.max(best, SCORE_KEYWORD_SUBSTRING); + } + return best; +} + +// scoreBranch returns a numeric match score between value+keywords and the +// search query. Returns 0 when no signal at all is found. +export function scoreBranch(value: string, search: string, keywords?: string[]): number { + const q = search.trim().toLowerCase(); + if (q.length === 0) return SCORE_SEGMENT_SUBSTRING; // keep current order; show all + const v = value.toLowerCase(); + const leaf = leafSegment(v); + + if (v === q) return SCORE_EXACT; + if (leaf === q) return SCORE_LEAF_EXACT; + if (v.startsWith(q)) return SCORE_FULL_PREFIX; + if (leaf.startsWith(q)) return SCORE_LEAF_PREFIX; + + const segScore = scoreSegments(segments(v), q, SCORE_SEGMENT_PREFIX, SCORE_SEGMENT_SUBSTRING); + if (segScore > 0) return segScore; + + // Note: a leaf-substring case is intentionally absent — `leaf` is a suffix + // of `v`, so any substring match in the leaf is also a match in `v` and is + // already caught by the SCORE_FULL_SUBSTRING branch above. + if (v.includes(q)) return SCORE_FULL_SUBSTRING - lengthPenalty(v); + + const kwScore = scoreKeywords(keywords, q); + if (kwScore > 0) return kwScore; + + // Last resort: subsequence (fuzzy) match. Density tightens the score so + // a clustered match outranks a wildly spread one. + if (isSubsequence(q, v)) { + const density = subsequenceDensity(q, v); + return SCORE_SUBSEQUENCE_BASE * density; + } + return 0; +} + +// lengthPenalty subtracts a tiny amount so that, between two substring +// matches, the shorter haystack ranks higher. Capped to avoid pushing a +// genuine match below the subsequence tier. +function lengthPenalty(s: string): number { + const p = Math.min(s.length / 1000, 0.05); + return p; +}