feat: add branch selector dropdown with dirty-state guard

[Oaklight]

Summary

Closes #63 (part of #59)

• Add a clickable branch selector dropdown in the Git sidebar header, replacing the static branch name span
• The dropdown lists local and remote branches, supports switching, creating, and deleting branches
• When the working tree has uncommitted changes, a dirty-state dialog offers "Stash & Switch", "Switch anyway", or "Cancel"
• New branch creation dialog with optional start point
• Full i18n support (English + Chinese) for all new strings
• CSS styles for the dropdown, branch items, section labels, and dialog overlay

Test plan

• Open a git-enabled project, click the branch button in the Git sidebar header
• Verify local and remote branches are listed with correct section labels
• Switch to a different branch when working tree is clean -- verify files refresh
• Modify a file, then switch branch -- verify dirty-state dialog appears with three options
• Test "Stash & Switch": changes are stashed, branch switches, files refresh
• Test "Switch anyway": branch switches without stashing
• Test "Cancel": dialog closes, stays on current branch
• Click "+ New branch" -- verify create dialog appears with name/start-point inputs
• Create a branch -- verify status bar shows success, branch list updates
• Delete a non-current branch via the X button -- verify confirmation prompt and removal
• Test dropdown closes on outside click
• Switch language to Chinese and verify all new strings render correctly
• Verify no JS console errors during all interactions

[milo-oaklight]

Post-Merge Review — PR #70: Branch Selector Dropdown + Dirty-State Guard

Good feature addition — the branch selector replaces the static label cleanly, the dropdown is built with createElement/textContent throughout (no innerHTML-with-user-data), and the dirty-state dialog is a nice touch. API integration with PR #67–#69 backends lines up correctly. A few things worth following up on:

---

1. "Switch anyway" does not actually force-switch (functional gap)

showDirtyDialog offers a "Switch anyway" / "直接切换" button, but its handler just calls doSwitchBranch(branch) — which calls POST .../git/branches/switch, which runs plain git switch <branch>. If dirty changes conflict with the target branch, git will refuse the switch and the user gets an error after clicking what looked like a force option.

For this to work as labeled, the backend would need a force parameter that passes --discard-changes to git switch. Alternatively, rename the button to something like "Try switch" so expectations match behavior.

Files: app.js — showDirtyDialog(), doSwitchBranch()

2. Dialog heading and body are identical

dialog.innerHTML = `
    <h3>${t("dirty_working_tree")}</h3>
    <p>${t("dirty_working_tree")}</p>
    ...
`;

Both <h3> and <p> render the same i18n key ("You have uncommitted changes"). The <p> should be a distinct explanatory message — e.g., "Choose how to handle your changes before switching branches."

File: app.js — showDirtyDialog()

3. No Escape key to close dialogs

Both showDirtyDialog and createBranchDialog support overlay-click-to-close but not Escape key. Standard for modal dialogs. A keydown listener for Escape on the overlay would be a small addition.

4. Outside-click listener accumulation

const closer = (e) => { ... };
setTimeout(() => document.addEventListener("click", closer), 0);

This listener is only removed when an outside click occurs. If the user toggles the dropdown closed via the button, the listener stays attached. Each re-open adds another. Over time this accumulates orphan listeners. Fix: also remove the closer in toggleBranchDropdown when hiding, or track it.

5. Remote branches listed but not actionable

Remote branches appear in the dropdown but have no click handler and no visual hint that they are read-only. Users will likely click expecting to check out a local tracking branch (the standard UI pattern in VS Code, GitKraken, etc.). Options:

• Make them clickable → git switch with a remote ref auto-creates a local tracking branch.
• Add a muted tooltip like "fetch-only" if intentionally non-interactive.

6. Accessibility gaps

• Dropdown button lacks aria-expanded, aria-haspopup="listbox", and aria-controls.
• Dropdown has no role="listbox" or role="menu", and no arrow-key navigation.
• Delete button (✕) is a <span>, not a <button> — not focusable, not keyboard-accessible. Should be <button> with aria-label="Delete branch".
• Dirty-state and create-branch dialogs use plain <div> without role="dialog" or aria-modal="true", and have no focus trapping.

These are not blocking but worth addressing for keyboard-only and screen-reader users.

7. No branch search/filter

If a repo has 30+ branches, the 300px max-height dropdown becomes hard to navigate. A small filter input at the top (like VS Code's branch picker) would improve usability significantly. Low priority but worth noting.

8. Detached HEAD display

If HEAD is detached, git branch --show-current returns "". The API returns current: "", and the UI button would show an empty span (or fall back to whatever refreshGit sets — "???" from the status endpoint). Might be worth explicitly showing "(detached)" or the short SHA.

9. Hardcoded "master" in HTML

<span id="git-branch">master</span>

This is the initial text before refreshGit() runs. If the project's default branch is main (or anything else), there's a brief flash of "master". Consider initializing to "…" or leaving it empty and letting refreshGit populate it.

10. loadBranches() does not use ?fetch=true

PR #67 added a fetch query parameter to GET .../git/branches. The dropdown always calls without it, so remote branch listings can be stale. Consider either auto-fetching on dropdown open, or adding a small refresh button inside the dropdown.

---

What looks good

• XSS-safe rendering: Branch names go through createElement/textContent everywhere in the dropdown. setStatus also uses textContent. No injection vectors found — good lesson learned from PR #71.
• API alignment: Frontend calls match the backend endpoints from PRs #67–#69 exactly.
• Stash integration: The stash-before-switch flow correctly calls the stash API from PR #69 before switching.
• i18n complete: All user-facing strings have both EN and ZH entries.
• CSS: Clean visual hierarchy with section labels, current-branch highlighting, hover-reveal delete. The text-overflow: ellipsis on long branch names is a nice touch.

Overall solid work — the core feature is functional and well-integrated. The main actionable item is #1 (the "Switch anyway" gap). The rest are incremental improvements.

— Milo 👻

[elena-oaklight]

Post-Merge Review — PR #70: Branch Selector Dropdown + Dirty-State Guard

Solid feature addition overall. Backend is well-structured, frontend uses safe DOM APIs. A few items worth following up on:

---

🟡 "Switch anyway" doesn't actually discard changes

The i18n key is dirty_discard_and_switch, implying the intent is to discard uncommitted changes before switching. But the handler just calls doSwitchBranch(targetBranch) directly — no git checkout -- . / git restore . step:

document.getElementById("dirty-discard").onclick = async () => {
    overlay.remove();
    await doSwitchBranch(targetBranch);  // just git switch — no discard
};

Actual behavior: git switch will either carry changes over silently or refuse if they conflict. This could confuse users who expect "switch anyway" to mean a clean switch. Either:

• Rename to something like dirty_switch_anyway / "Try to switch" (if the intent is carry-over), or
• Actually discard via a git restore . && git clean -fd before switching (if the intent is discard)

🟡 No keyboard navigation or accessibility on the dropdown

The branch dropdown is mouse-only:

• No Escape to close
• No arrow keys to navigate items
• No Enter to select
• No ARIA attributes (role="listbox", aria-expanded, aria-activedescendant)
• Focus is not trapped or managed

Not a blocker, but worth a follow-up pass for keyboard-only and screen reader users.

🟢 Event listener accumulation on repeated toggle

Each call to showBranchDropdown() adds a new document.addEventListener("click", closer). The listener is removed on outside-click, but not when the dropdown is closed by clicking the toggle button (which goes through toggleBranchDropdown → dd.style.display = "none" without removing the listener).

Over many open/close cycles, stale listeners accumulate. They're mostly harmless (they'll try to hide an already-hidden dropdown), but could be cleaned up by storing the closer reference:

let _branchCloser = null;

function toggleBranchDropdown() {
  const dd = document.getElementById("git-branch-dropdown");
  if (dd.style.display === "none") {
    showBranchDropdown();
  } else {
    dd.style.display = "none";
    if (_branchCloser) {
      document.removeEventListener("click", _branchCloser);
      _branchCloser = null;
    }
  }
}

🟢 No guard against concurrent branch switches

If the user quickly switches branches (e.g., opens dropdown from status bar, or re-triggers before the first switch completes), two doSwitchBranch calls can race. Consider disabling interactions or tracking an in-flight flag:

let _switching = false;
async function doSwitchBranch(branch) {
  if (_switching) return;
  _switching = true;
  try { /* ... */ } finally { _switching = false; }
}

🟢 Stash & Switch: orphaned stash on switch failure

If "Stash & Switch" succeeds at stashing but then git switch fails, the stash is left in place with no feedback to the user about recovering it. Consider showing a note like "Changes stashed but switch failed — use git stash pop to recover" or auto-popping on failure.

✅ Security

• No command injection: backend uses subprocess.run([...], shell=False) throughout. Branch names are list arguments, never interpolated into shell strings.
• No XSS: branch names rendered via textContent (not innerHTML). The innerHTML usage in dialogs only interpolates t(...) i18n strings which are developer-controlled constants. setStatus() also uses textContent.
• Branch name validation: create_branch() validates via git check-ref-format --branch before use. ✅
• Delete protection: backend refuses to delete the current branch. ✅

✅ Stash integration (PR #69)

The dirty dialog's "Stash & Switch" reuses the same /api/.../git/stash endpoint from PR #69. Works correctly. Minor note: if the stash sidebar is visible, it won't auto-refresh to show the new stash entry — a refreshGit() call after stash might help if it covers the stash list.

---

Verdict: Good work. No security issues, clean backend. The "Switch anyway" semantics mismatch is the most user-facing concern. The rest are polish items for a follow-up.

[clementine-oaklight]

PR #70 — feat: add branch selector dropdown with dirty-state guard → LGTM ✅

前端代码质量很好——branch name 全用 createElement + .textContent，没有 innerHTML XSS（和 #71 对比，这里做得更好）。dirty-state dialog 的 stash-and-switch / switch-anyway / cancel 三选项覆盖了主要场景。

Non-blocking

• showDirtyDialog 的 <h3> 和 <p> 都显示 t("dirty_working_tree")——同一句话在标题和正文重复了，<p> 应该换成更具体的说明（比如 "切换分支前需要处理未提交的更改"）。
• deleteBranch 里 enc(name) 编码 branch name 进 URL path——和 #69 review 里提的一样，feature/foo 类分支名的 %2F 可能被框架 decode 后匹配失败。前端和后端需要一起改。
• Remote branches 不可点击（没有 onclick）——MVP 阶段合理，但可以加个 tooltip 说明 "checkout remote branch" 需要先 create tracking branch。
• createBranchDialog 用 inline style 画 input 框——CSS class 更好维护，不过 dialog 只出现一次，影响小。
• Stash-and-switch 之后没有自动 pop stash，用户切回来时 stash 还在——这个可能是 deliberate（避免冲突），但值得在 UI 上提示 stash 还没 pop。

[Oaklight]

Addressing review feedback

Thanks @milo-oaklight for the post-merge review. Fixing actionable items:

• "Switch anyway" doesn't force-switch: Will add force parameter to backend switch_branch() with --discard-changes ✅
• Dialog heading/body identical: Will add distinct explanatory i18n key for the <p> ✅
• No Escape key for dialogs: Will add keydown listener ✅
• Outside-click listener accumulation: Will track and clean up the closer ✅
• Remote branches not actionable: Will make clickable (git switch auto-creates tracking branch) ✅
• Accessibility gaps: Will address in a follow-up — good callout but bigger scope

All in follow-up PR #72 or subsequent.