Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Commit 6b73ec2

Browse files
authored
refactor(branch-selector): address qa-swarm review nits
Multi-perspective review (paul + xp + security-audit) surfaced low/nit items; this addresses the actionable ones: - Extract the duplicated "Loading branches…" literal into a shared LOADING_BRANCHES_LABEL const so the two render sites can't drift. - Clarify the TaskInput comment: the cached default is best-effort — a default branch renamed since it was cached stays seeded until the user picks another (the auto-select only fires while nothing is selected). - Lock the empty-state gating with a test asserting "No branches found." keeps the `hidden` class when the seeded trunk row makes the list non-empty, so it never flashes above the seeded row. security-audit: 0 findings (API-derived branch name → auto-escaped React text + the pre-existing selection path; no new sink). Generated-By: PostHog Code Task-Id: 562fb90f-d823-425d-b53c-5f482b55a8b2
1 parent 593d744 commit 6b73ec2

3 files changed

Lines changed: 17 additions & 3 deletions

File tree

‎packages/ui/src/features/git-interaction/components/BranchSelector.test.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ describe("BranchSelector cloud mode", () => {
9696

9797
expect(await screen.findByRole("option", { name: "main" })).toBeVisible();
9898
expect(screen.getByText("Loading branches…")).toBeVisible();
99+
// The seeded row makes the list non-empty, so the empty-state stays gated
100+
// off (Base UI only reveals it when the content group is data-empty) — it
101+
// keeps the `hidden` class rather than flashing "No branches found." above
102+
// the trunk row.
103+
expect(screen.getByText("No branches found.")).toHaveClass("hidden");
99104
});
100105

101106
it("does not seed the default branch once the user is searching", async () => {

‎packages/ui/src/features/git-interaction/components/BranchSelector.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ import { getSuggestedBranchName } from "../utils/getSuggestedBranchName";
4040

4141
const COMBOBOX_LIMIT = 50;
4242

43+
// Shared so the two "still loading branches" render sites (the empty-list
44+
// spinner and the seeded-default row) can never drift out of sync on a copy edit.
45+
const LOADING_BRANCHES_LABEL = "Loading branches…";
46+
4347
// Sentinel value for the "Create new branch" action. Rendered as a real
4448
// ComboboxItem in the list footer so it's reachable by keyboard, not a
4549
// plain button the combobox's roving focus skips over.
@@ -472,7 +476,7 @@ export function BranchSelector({
472476
) : null}
473477

474478
{branchListLoading && branches.length === 0 ? (
475-
<LoadingRow label="Loading branches…" />
479+
<LoadingRow label={LOADING_BRANCHES_LABEL} />
476480
) : (
477481
<ComboboxEmpty>No branches found.</ComboboxEmpty>
478482
)}
@@ -540,7 +544,9 @@ export function BranchSelector({
540544
item while the remote list loads. A loading row directly below it
541545
makes clear the rest of the branches are still on the way.
542546
*/}
543-
{seededDefaultBranch ? <LoadingRow label="Loading branches…" /> : null}
547+
{seededDefaultBranch ? (
548+
<LoadingRow label={LOADING_BRANCHES_LABEL} />
549+
) : null}
544550

545551
{isCloudMode && cloudBranchesHasMore ? (
546552
<ComboboxListFooter>

‎packages/ui/src/features/task-detail/components/TaskInput.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,10 @@ export function TaskInput({
435435
const liveCloudDefaultBranch = cloudBranchData?.defaultBranch ?? null;
436436
// Serve the persisted default branch until the live list resolves, so the
437437
// majority "start on trunk" case pre-selects trunk with zero wait on a cold
438-
// start. Falls through to the fresh value the moment it arrives.
438+
// start. The cached value is best-effort: `cloudDefaultBranch` switches to the
439+
// live value the moment it arrives, but the picker's auto-select only fires
440+
// while nothing is selected — so a default branch renamed since it was cached
441+
// (rare) would leave the seeded name selected until the user picks another.
439442
const cloudDefaultBranch =
440443
liveCloudDefaultBranch ??
441444
(selectedCloudRepository

0 commit comments

Comments
 (0)