feat: [ENG-2058] add web UI branch management

[wzlng]

No description provided.

[github-actions]

Claude finished @wzlng's task in 4m 3s —— View job

---

PR Review: feat: [ENG-2058] add web UI branch management

Score: 3.5 / 5

---

Summary

This PR adds branch management to the web UI: a BranchDropdown in the header with create, checkout, delete, fetch, pull, and set-upstream operations. The server-side additions are minimal and correct (threading startPoint through createBranch / checkout, adding the start-point guard on checkout). The webui API layer follows the established transport/react-query patterns well. Test coverage is present for the new utilities and the new handler paths.

---

Issues found

Bugs / correctness

#	File	Severity	Description
1	branch-dropdown.tsx:332	medium	CheckoutRefDialog is rendered but dialog === 'checkout-ref' can never be set — no UI element calls closeAndOpenDialog('checkout-ref'). Dead code or missing trigger.
2	execute-vc-push.ts:30	medium	useVcPush only invalidates the status query. Every other mutation (fetch, pull, checkout, merge) also invalidates getVcBranchesQueryOptions. After pushing a new branch, remote-tracking refs in the dropdown stay stale until the 10-second background poll fires.

TypeScript standards

#	File	Severity	Description
3	with-unborn-current.ts:15	medium	as T assertion violates the project's "Avoid as Type assertions" rule. If T ever carries extra required fields the synthesized object silently breaks. Drop the generic or return VcBranch[] (see inline).

Code quality / consistency

#	File	Severity	Description
4	get-vc-branches.ts:8	low	VcBranch type is defined identically in both get-vc-branches.ts and filter-branches.ts. Consolidate to a single source of truth.
5	get-vc-branches.ts:10-12	low	Only async function + bare throw in the API layer; siblings all use sync + Promise.reject. Minor inconsistency plus a stray trailing semicolon.
6	branch-dropdown.tsx:231	low	Axis3D (a 3D-axes coordinate icon) is used for the Fetch button. It has no semantic connection to "fetch from remote"; RefreshCw or ArrowDownToLine would be immediately recognisable.
7	new-branch-dialog.tsx:67	low	Hardcoded id="input-invalid" – duplicate DOM IDs break accessibility if the dialog is ever mounted more than once. Use React.useId().
8	branch-dropdown.tsx:363	info	trackingBranch badge is only threaded to the current-branch submenu, not to other local branches. Intentional or oversight? A short comment would clarify.

Missing tests

#	File	Description
9	with-unborn-current.ts	No test file for this utility despite being meaningful edge-case logic (empty repo / unborn branch). filter-branches and partition-branches have test files; this one should too.

---

What's good

• Server-side diff is surgical: startPoint is plumbed cleanly through the whole stack with proper NotFoundError → VcError(BRANCH_NOT_FOUND) mapping, and the guard that rejects startPoint without create: true is a nice defensive addition.
• partitionBranches + filterBranches + withUnbornCurrent are well-separated pure utilities with good test coverage (unit tests added for both).
• handleRemoteCheckout correctly falls back to tracking the remote ref and surfaces (but doesn't block on) upstream-set failure.
• The delete dialog includes a helpful warning about unmerged commits.
• InitializeVcButton handles the zero-state (no git repo) cleanly without duplicating status logic.

[github-actions]

Claude finished @wzlng's task in 2m 40s —— View job

---

PR Review: feat: [ENG-2058] address PR review comments and harden branch UX

Score: 4.0 / 5

---

Summary

The second commit cleanly addresses the majority of issues from the first review. The three medium-severity findings (dead code, missing cache invalidation, as T assertion) are all resolved, as is the duplicate VcBranch type, the hardcoded DOM id, and the trackingBranch intent comment. Two low-priority style items and one missing test remain open.

---

Resolved from previous review ✅

#	Issue	Resolution
1	CheckoutRefDialog dead code	Deleted checkout-ref-dialog.tsx, removed import and 'checkout-ref' from DialogKind
2	useVcPush missing getVcBranches cache invalidation	Both getVcStatusQueryOptions and getVcBranchesQueryOptions are now invalidated on push
3	as T assertion in with-unborn-current.ts	Generic dropped; function now takes readonly VcBranch[] and returns VcBranch[]
4	Duplicate VcBranch type	Both filter-branches.ts and get-vc-branches.ts now import from shared/transport/events/vc-events
5	trackingBranch badge intent unclear	JSDoc comment added to LocalBranchSubmenu explaining the per-branch upstream isn't in the status payload
6	Hardcoded id="input-invalid"	new-branch-dialog.tsx now uses React.useId()

---

Remaining issues

Code quality / consistency

#	File	Severity	Description
1	branch-dropdown.tsx:231	low	Axis3D (a 3D coordinate axes icon) is still used for the Fetch button. It has no semantic connection to "fetch from remote". RefreshCw or ArrowDownToLine from lucide-react would be immediately recognisable. Fix this →
2	get-vc-branches.ts:8	low	Function is still declared async with a bare throw. All other API functions in this directory (getVcStatus, executeVcFetch, etc.) are synchronous and return Promise.reject(...) on the guard path — a minor inconsistency. Fix this →

Missing tests

#	File	Description
3	with-unborn-current.ts	Still no test file. filter-branches and partition-branches both have test files; this utility handles a meaningful edge case (empty repo / unborn branch) and should too. The test helper pattern from the sibling test files (a b() factory + VcBranch import) can be reused directly. Fix this →%20returns%20branches%20unchanged%20when%20currentName%20is%20undefined%2C%202)%20returns%20branches%20unchanged%20when%20current%20branch%20already%20exists%20in%20the%20list%2C%203)%20prepends%20a%20synthetic%20entry%20when%20the%20branch%20is%20unborn%20(not%20in%20the%20list)%2C%204)%20does%20not%20mutate%20the%20input%20array.)

---

What's good in this update

• The type consolidation is clean — VcBranch is now canonical in shared/transport/events/vc-events.ts and all three utilities (filter-branches, partition-branches, with-unborn-current) import from there. The interface declaration in shared gives it the right shape for downstream extension.
• The withUnbornCurrent simplification correctly narrows the signature. Dropping the <T extends VcBranch> generic is the right call since none of the callers rely on returning a subtype.
• The checkout-ref-dialog.tsx cleanup is complete — file deleted, import removed, DialogKind trimmed.

[github-actions]

Claude finished @wzlng's task in 2m 35s —— View job

---

PR Review: feat: [ENG-2058] add web UI branch management

Score: 4.5 / 5

---

Summary

Third commit (e6eab30) addresses two of three remaining issues from the previous review: getVcBranches guard style is now consistent (sync + Promise.reject) and with-unborn-current.test.ts is added with seven well-scoped cases. The Axis3D icon is the only carry-over from prior reviews. One new cluster of inconsistencies is surfaced: the fix to getVcBranches exposes that three other new API functions (execute-vc-branch-delete.ts, execute-vc-branch-set-upstream.ts, execute-vc-merge.ts, execute-vc-init.ts) still use bare throw in the guard path.

---

Resolved from previous review ✅

#	Issue	Resolution
1	getVcBranches declared async with bare throw	Converted to sync arrow with Promise.reject(new Error('Not connected')) — matches all siblings
2	No test for withUnbornCurrent	test/unit/webui/features/vc/utils/with-unborn-current.test.ts added; 7 cases covering undefined name, existing match, remote-only name, empty input, immutability

---

Remaining issues

Code quality / consistency

#	File	Severity	Description
1	branch-dropdown.tsx:231	low	Axis3D (3D coordinate axes icon) is still used for the Fetch button. RefreshCw or ArrowDownToLine from lucide-react would be immediately recognisable. Fix this →
2	execute-vc-branch-delete.ts:9 execute-vc-branch-set-upstream.ts:10	low	Both declare async and use a bare throw on the guard path. The same pattern in get-vc-branches.ts was fixed in this commit — the fix should be applied consistently. Convert these to sync arrow functions returning Promise.reject(new Error('Not connected')). Fix this →
3	execute-vc-init.ts:12 execute-vc-merge.ts:16	low	Sync arrow functions declared as (): Promise<T> that bare-throw on the guard path. A synchronous throw from a non-async function escapes as a synchronous exception rather than a rejected promise — subtly inconsistent with sibling files and React Query's expectation. Change guard to return Promise.reject(new Error('Not connected')). Fix this →

---

What's good in this update

• with-unborn-current.test.ts is thorough — the "remote branch shares the name" case (test 4) is a subtle edge the previous reviews didn't explicitly call for, and it's covered.
• getVcBranches guard-style fix is clean; the file is now indistinguishable from its siblings.
• handleBranchSetUpstream in vc-handler.ts correctly parses origin/feature/foo (splits on first /) and validates both the remote and the remote-tracking ref before writing config — defensive and correct.
• Remote branch name format (origin/main) from listBranches aligns with what handleBranchSetUpstream expects, so the handleRemoteCheckout → setUpstream.mutateAsync(branch.name) path is safe.
• Merge safety check in handleBranchDelete (verifies branch is fully merged into current before deleting) is solid and correctly surfaces BRANCH_NOT_MERGED to the delete dialog's error toast.

[github-actions]

Claude finished @wzlng's task in 1m 33s —— View job

---

PR Review: feat: [ENG-2058] add web UI branch management

Score: 4.5 / 5

---

Summary

Commit 413a7ef resolves all three guard-style issues flagged in the previous review, and goes one further by also refactoring execute-vc-commit.ts. Every API function in src/webui/features/vc/api/ now uses the canonical sync-arrow-returning-Promise pattern. The Axis3D icon is the sole surviving issue — it has been flagged in every review cycle.

---

Resolved from previous review ✅

#	Issue	Resolution
1	execute-vc-branch-delete.ts:9 — async + bare throw	Converted to sync arrow with Promise.reject(new Error('Not connected'))
2	execute-vc-branch-set-upstream.ts:10 — async + bare throw	Same fix
3	execute-vc-init.ts:12 — sync arrow but bare throw escaping as sync exception	Changed to return Promise.reject(...)
4	execute-vc-merge.ts:16 — sync arrow but bare throw	Same fix
✨	execute-vc-commit.ts (bonus) — async function + await	Proactively refactored to sync arrow + .then(commit) chaining; behaviour is equivalent and the style now matches all siblings

---

Remaining issues

Code quality / consistency

#	File	Severity	Description
1	branch-dropdown.tsx:231	low	Axis3D (3D coordinate-axes icon) is still used for the Fetch button — flagged in all four review cycles. RefreshCw or ArrowDownToLine from lucide-react would be immediately recognisable. Fix this →

---

What's good in this update

• The execute-vc-commit.ts refactor is clean: add().then(commit) is a faithful, idiomatic translation of await add(); return commit(). No behaviour change.
• All ten API files now share an identical guard-path structure — a new contributor reading any one file will immediately understand the pattern across the whole directory.