diff --git a/.qwen/e2e-tests/2026-08-28-webshell-branch-picker-action-hints.md b/.qwen/e2e-tests/2026-08-28-webshell-branch-picker-action-hints.md new file mode 100644 index 00000000000..15606253c7e --- /dev/null +++ b/.qwen/e2e-tests/2026-08-28-webshell-branch-picker-action-hints.md @@ -0,0 +1,58 @@ +# Branch picker action hints + +## Scenario + +Open a trusted git workspace in the Web Shell (`qwen serve` + the sidebar). +Click the branch chip on the workspace folder header (or the composer chip / +Environment panel row) to open the branch picker. Exercise the repo through +these states between opens: + +1. Tracking `origin/main`, in sync, clean tree. +2. `git reset --hard HEAD~3` (behind 3, clean). +3. Same as 2 plus an edited tracked file and one new untracked file. +4. `git checkout -b feat/no-upstream` with one local commit (no upstream). +5. Start a conflicting `git rebase` and leave it in progress. +6. `git checkout --detach`. +7. Start a conflicting `git merge` on a branch with one commit ahead. +8. Push a branch with `-u`, delete it on the remote, `git fetch --prune`. + +## Checks + +- State 1: Update Project shows "Up to date", Push shows "Nothing to push", + Commit shows "No changes"; all three rows are dimmed but still enabled. +- State 2: Update Project shows "↓3 · origin/main" in the neutral tone. +- State 3: Update Project shows "↓3 · uncommitted changes" in the warning tone + and stays enabled; Commit shows "2 changes (1 untracked)" (entries, not + files: a partially staged file counts twice). +- State 4: Update Project shows "No upstream" and is disabled; Push shows + "Sets upstream on push" and is enabled. +- State 5: Update Project and Push both show "Rebasing" in the warning tone and + are disabled (a rebase detaches HEAD); Commit stays enabled. +- State 6: Update Project and Push both show "Detached HEAD" and are disabled. +- State 7: Update Project shows "Merging" and is disabled; Push shows "Merging" + in the warning tone but stays enabled (a push does not consult the index). +- State 8: Update Project shows "Upstream gone" and is disabled; Push shows + "Sets upstream on push". +- After committing through the Commit dialog and reopening the picker from + any of the three entry points (sidebar chip, composer chip, Environment + panel), the Commit hint reflects the new tree without waiting for the poll. +- With the picker open, run `git branch --unset-upstream` in a terminal and + refocus the window: once the chip's status updates, Update Project flips to + disabled "No upstream" without reopening. +- Switching the UI language to 中文 renders the localized copy + ("已是最新", "无上游分支", "↓3 · 有未提交更改", "2 处更改（1 未跟踪）"). + +## Evidence + +Unit coverage lives in +`packages/web-shell/client/components/BranchPickerPopover.test.tsx` +(`deriveActionHints` decision table + rendered disabled/tone assertions) and the +open-time status refresh in +`packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx`. + +```sh +cd packages/web-shell && npx vitest run \ + client/components/BranchPickerPopover.test.tsx \ + client/components/sidebar/WorkspaceSection.test.tsx \ + client/components/panels/EnvironmentPanel.test.tsx +``` diff --git a/packages/core/src/utils/git-branches.test.ts b/packages/core/src/utils/git-branches.test.ts index 9bae4326355..d3290693317 100644 --- a/packages/core/src/utils/git-branches.test.ts +++ b/packages/core/src/utils/git-branches.test.ts @@ -141,6 +141,40 @@ describe('gitEnv (R12 env isolation)', () => { }); }); +describe('fetchGitBranches upstream tracking', () => { + it('marks a branch whose upstream ref was deleted and pruned as gone', async () => { + const dir = makeRepo(); + const remote = makeBareRemote(); + git(dir, 'remote', 'add', 'origin', remote); + git(dir, 'push', '-q', '-u', 'origin', 'master'); + git(dir, 'checkout', '-q', '-b', 'feat'); + git(dir, 'push', '-q', '-u', 'origin', 'feat'); + + const tracked = (await fetchGitBranches(dir)).local.find( + (b) => b.name === 'feat', + ); + expect(tracked?.upstream).toBe('origin/feat'); + expect(tracked?.upstreamGone).toBeUndefined(); + + git(dir, 'push', '-q', 'origin', '--delete', 'feat'); + git(dir, 'fetch', '-q', '--prune', 'origin'); + + const gone = (await fetchGitBranches(dir)).local.find( + (b) => b.name === 'feat', + ); + // The configured upstream is still reported so the UI can name it, but + // the flag says its ref no longer exists. + expect(gone?.upstream).toBe('origin/feat'); + expect(gone?.upstreamGone).toBe(true); + expect(gone?.ahead).toBe(0); + expect(gone?.behind).toBe(0); + const master = (await fetchGitBranches(dir)).local.find( + (b) => b.name === 'master', + ); + expect(master?.upstreamGone).toBeUndefined(); + }); +}); + describe('fetchGitBranches recent branches', () => { it('lists recently checked-out branches from the reflog', async () => { const dir = makeRepo(); diff --git a/packages/core/src/utils/git-branches.ts b/packages/core/src/utils/git-branches.ts index 88218afa8f0..ded50281e8b 100644 --- a/packages/core/src/utils/git-branches.ts +++ b/packages/core/src/utils/git-branches.ts @@ -18,6 +18,12 @@ export interface GitBranchInfo { name: string; isHead: boolean; upstream?: string; + /** + * `true` when the configured upstream ref no longer exists (git's + * `[gone]` tracking state, e.g. after the remote branch was deleted and + * pruned). `upstream` still names the configured ref in that case. + */ + upstreamGone?: boolean; ahead: number; behind: number; /** Unix epoch seconds of the branch tip commit. */ @@ -198,11 +204,15 @@ function parseBranchLines(raw: string): GitBranchInfo[] { const behindMatch = /behind (\d+)/.exec(track); if (aheadMatch) ahead = parseInt(aheadMatch[1], 10); if (behindMatch) behind = parseInt(behindMatch[1], 10); + // `%(upstream:track,nobracket)` prints `gone` when the upstream is + // configured but its ref is missing; ahead/behind are meaningless then. + const upstreamGone = upstream !== undefined && /\bgone\b/.test(track); return { name, isHead, upstream, + ...(upstreamGone ? { upstreamGone } : {}), ahead, behind, commitDate, diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index d910f04ed76..155be6a4a50 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -303,6 +303,8 @@ export interface DaemonGitBranchInfo { name: string; isHead: boolean; upstream?: string; + /** The configured upstream ref no longer exists (git's `[gone]` state). */ + upstreamGone?: boolean; ahead: number; behind: number; /** Unix epoch seconds of the branch tip commit. */ diff --git a/packages/web-shell/client/components/BranchPickerPopover.module.css b/packages/web-shell/client/components/BranchPickerPopover.module.css index 5d5188194db..5d55189c91b 100644 --- a/packages/web-shell/client/components/BranchPickerPopover.module.css +++ b/packages/web-shell/client/components/BranchPickerPopover.module.css @@ -265,3 +265,27 @@ font-size: 12px; color: var(--muted-foreground, #888); } + +.actionItemMuted .actionLabel { + color: var(--muted-foreground, #888); +} + +.actionHint { + flex-shrink: 0; + max-width: 55%; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: 11px; + font-variant-numeric: tabular-nums; + color: var(--muted-foreground, #888); +} + +.actionHint[data-tone='info'] { + color: var(--foreground, #e0e0e0); + opacity: 0.8; +} + +.actionHint[data-tone='warning'] { + color: var(--warning-color, #f59e0b); +} diff --git a/packages/web-shell/client/components/BranchPickerPopover.test.tsx b/packages/web-shell/client/components/BranchPickerPopover.test.tsx index 93d5554135e..850a6a02690 100644 --- a/packages/web-shell/client/components/BranchPickerPopover.test.tsx +++ b/packages/web-shell/client/components/BranchPickerPopover.test.tsx @@ -8,6 +8,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; +import type { + DaemonGitBranchesResult, + DaemonWorkspaceGitStatus, +} from '@qwen-code/sdk/daemon'; // The real popover shell is Radix, whose focus/scroll-lock effects never // settle under `act` in jsdom. Render the trigger and content inline instead @@ -24,27 +28,38 @@ vi.mock('./ui/popover', async () => { }; }); -const { workspaceGitBranches, workspaceGitCreateBranch, workspaceClient } = - vi.hoisted(() => { - const workspaceGitBranches = vi.fn(); - const workspaceGitCreateBranch = vi.fn(); - // A stable client so the popover's memoized workspace handle (and thus its - // fetch effect) stays referentially stable across renders. - const workspaceClient = { - workspaceByCwd: () => ({ - workspaceGitBranches, - workspaceGitCheckout: vi.fn().mockResolvedValue(undefined), - workspaceGitCreateBranch, - workspaceGitPush: vi - .fn() - .mockResolvedValue({ success: true, output: '' }), - workspaceGitPull: vi - .fn() - .mockResolvedValue({ success: true, output: '' }), - }), - }; - return { workspaceGitBranches, workspaceGitCreateBranch, workspaceClient }; - }); +const { + workspaceGitBranches, + workspaceGitCreateBranch, + workspaceGit, + workspaceClient, +} = vi.hoisted(() => { + const workspaceGitBranches = vi.fn(); + const workspaceGitCreateBranch = vi.fn(); + const workspaceGit = vi.fn(); + // A stable client so the popover's memoized workspace handle (and thus its + // fetch effect) stays referentially stable across renders. + const workspaceClient = { + workspaceByCwd: () => ({ + workspaceGitBranches, + workspaceGit, + workspaceGitCheckout: vi.fn().mockResolvedValue(undefined), + workspaceGitCreateBranch, + workspaceGitPush: vi + .fn() + .mockResolvedValue({ success: true, output: '' }), + workspaceGitPull: vi + .fn() + .mockResolvedValue({ success: true, output: '' }), + }), + }; + return { + workspaceGitBranches, + workspaceGitCreateBranch, + workspaceGit, + workspaceClient, + }; +}); vi.mock('@qwen-code/webui/daemon-react-sdk', async (importOriginal) => { const actual = @@ -59,7 +74,8 @@ vi.mock('@qwen-code/webui/daemon-react-sdk', async (importOriginal) => { }); const { I18nProvider } = await import('../i18n'); -const { BranchPickerPopover } = await import('./BranchPickerPopover'); +const { BranchPickerPopover, deriveActionHints, listingContradictsStatus } = + await import('./BranchPickerPopover'); globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -77,6 +93,8 @@ function mount( onOpenDiff: () => void; onOpenCommit: () => void; onOpenChange: (open: boolean) => void; + onStatusRefreshed: (status: DaemonWorkspaceGitStatus) => void; + status: DaemonWorkspaceGitStatus; }> = {}, ): void { act(() => { @@ -86,6 +104,8 @@ function mount( open onOpenChange={overrides.onOpenChange ?? vi.fn()} workspaceCwd="/repo" + status={overrides.status} + onStatusRefreshed={overrides.onStatusRefreshed} onOpenDiff={overrides.onOpenDiff} onOpenCommit={overrides.onOpenCommit} > @@ -110,7 +130,11 @@ afterEach(() => { act(() => root.unmount()); container.remove(); vi.clearAllMocks(); + // Default: the popover's own status fetch yields nothing, so hints derive + // from the caller's `status` prop alone unless a test resolves it. + workspaceGit.mockRejectedValue(new Error('no status')); }); +workspaceGit.mockRejectedValue(new Error('no status')); describe('BranchPickerPopover actions', () => { it('wires "View Changes" to onOpenDiff and closes', async () => { @@ -215,3 +239,557 @@ describe('BranchPickerPopover actions', () => { expect(workspaceGitCreateBranch).not.toHaveBeenCalled(); }); }); + +// Identity translator: hints assert on keys / interpolated vars, not copy. +const tKey = (key: string, vars?: Record) => + vars ? `${key}:${JSON.stringify(vars)}` : key; + +function branches( + head: Partial = {}, + detached = false, +): DaemonGitBranchesResult { + return { + v: 1, + workspaceCwd: '/repo', + available: true, + local: [ + { + name: 'main', + isHead: true, + ahead: 0, + behind: 0, + commitDate: 0, + commitSubject: '', + ...head, + }, + ], + remote: [], + tags: [], + recent: [], + head: 'main', + detached, + }; +} + +function status( + over: Partial = {}, +): DaemonWorkspaceGitStatus { + return { + v: 2, + workspaceCwd: '/repo', + branch: 'main', + computedAt: 1, + staged: 0, + unstaged: 0, + untracked: 0, + conflicted: 0, + ...over, + }; +} + +describe('deriveActionHints', () => { + it('dims pull/push/commit when tracking upstream, in sync, and clean', () => { + const h = deriveActionHints( + tKey, + branches({ upstream: 'origin/main' }), + status(), + ); + expect(h.pull).toEqual({ + text: 'branchPicker.hint.upToDate', + tone: 'muted', + }); + expect(h.pullDisabled).toBe(false); + expect(h.push).toEqual({ + text: 'branchPicker.hint.nothingToPush', + tone: 'muted', + }); + expect(h.pushDisabled).toBe(false); + expect(h.commit).toEqual({ + text: 'branchPicker.hint.noChanges', + tone: 'muted', + }); + }); + + it('shows behind count with upstream for a clean tree', () => { + const h = deriveActionHints( + tKey, + branches({ upstream: 'origin/main', behind: 3 }), + status(), + ); + expect(h.pull).toEqual({ text: '↓3 · origin/main', tone: 'info' }); + expect(h.pullDisabled).toBe(false); + }); + + it('warns on pull when behind with uncommitted changes', () => { + const h = deriveActionHints( + tKey, + branches({ upstream: 'origin/main', behind: 2 }), + status({ unstaged: 1 }), + ); + expect(h.pull).toEqual({ + text: 'branchPicker.hint.behindDirty:{"count":2}', + tone: 'warning', + }); + expect(h.pullDisabled).toBe(false); + }); + + it('disables pull without upstream and says push will set one', () => { + const h = deriveActionHints(tKey, branches({ ahead: 1 }), status()); + expect(h.pull).toEqual({ + text: 'branchPicker.hint.noUpstream', + tone: 'muted', + }); + expect(h.pullDisabled).toBe(true); + expect(h.push).toEqual({ + text: 'branchPicker.hint.setsUpstream', + tone: 'info', + }); + expect(h.pushDisabled).toBe(false); + }); + + it('treats a gone upstream like no upstream, with its own copy on pull', () => { + const h = deriveActionHints( + tKey, + branches({ upstream: 'origin/feat', upstreamGone: true, ahead: 0 }), + status({ hasUpstream: true }), + ); + expect(h.pull).toEqual({ + text: 'branchPicker.hint.upstreamGone', + tone: 'muted', + }); + expect(h.pullDisabled).toBe(true); + expect(h.push).toEqual({ + text: 'branchPicker.hint.setsUpstream', + tone: 'info', + }); + expect(h.pushDisabled).toBe(false); + }); + + it('shows ahead count on push and warns when also behind', () => { + expect( + deriveActionHints( + tKey, + branches({ upstream: 'origin/main', ahead: 2 }), + status(), + ).push, + ).toEqual({ text: '↑2', tone: 'info' }); + expect( + deriveActionHints( + tKey, + branches({ upstream: 'origin/main', ahead: 2, behind: 1 }), + status(), + ).push, + ).toEqual({ + text: 'branchPicker.hint.aheadBehind:{"ahead":2,"behind":1}', + tone: 'warning', + }); + }); + + it('counts changes (entries, not files) for commit and calls out untracked ones', () => { + expect( + deriveActionHints( + tKey, + branches({ upstream: 'origin/main' }), + status({ staged: 1, unstaged: 2 }), + ).commit, + ).toEqual({ + text: 'branchPicker.hint.changes:{"count":3}', + tone: 'info', + }); + expect( + deriveActionHints( + tKey, + branches({ upstream: 'origin/main' }), + status({ staged: 1, unstaged: 2, untracked: 2 }), + ).commit, + ).toEqual({ + text: 'branchPicker.hint.changesUntracked:{"count":5,"untracked":2}', + tone: 'info', + }); + // A partially staged file (porcelain `MM`) is one file but two entries; + // the copy must not call it "2 files". + expect( + deriveActionHints( + tKey, + branches({ upstream: 'origin/main' }), + status({ staged: 1, unstaged: 1 }), + ).commit?.text, + ).toBe('branchPicker.hint.changes:{"count":2}'); + }); + + it('blocks pull during an in-progress operation or conflicts but only warns on push', () => { + // `git pull` refuses both states; `git push` does not consult the index, + // so the push row stays clickable with the same warning. + const op = deriveActionHints( + tKey, + branches({ upstream: 'origin/main', behind: 1 }), + status({ operation: 'merge' }), + ); + expect(op.pull).toEqual({ text: 'git.operation.merge', tone: 'warning' }); + expect(op.pullDisabled).toBe(true); + expect(op.push).toEqual({ text: 'git.operation.merge', tone: 'warning' }); + expect(op.pushDisabled).toBe(false); + + const conflict = deriveActionHints( + tKey, + branches({ upstream: 'origin/main' }), + status({ conflicted: 2 }), + ); + expect(conflict.pull).toEqual({ + text: 'git.conflicted:{"count":2}', + tone: 'warning', + }); + expect(conflict.pullDisabled).toBe(true); + expect(conflict.pushDisabled).toBe(false); + // Conflicted entries still count as uncommitted work for the commit hint. + expect(conflict.commit?.text).toBe('branchPicker.hint.changes:{"count":2}'); + }); + + it('blocks both pull and push on a detached HEAD, naming the operation when there is one', () => { + const detached = deriveActionHints(tKey, branches({}, true), status()); + expect(detached.pull).toEqual({ text: 'git.detached', tone: 'warning' }); + expect(detached.pullDisabled).toBe(true); + expect(detached.push).toEqual({ text: 'git.detached', tone: 'warning' }); + expect(detached.pushDisabled).toBe(true); + + // A rebase detaches HEAD: push is blocked for that reason, but the row + // says "Rebasing" since that is what the user is in the middle of. + const rebase = deriveActionHints( + tKey, + branches({}, true), + status({ operation: 'rebase', detached: true }), + ); + expect(rebase.push).toEqual({ + text: 'git.operation.rebase', + tone: 'warning', + }); + expect(rebase.pushDisabled).toBe(true); + expect(rebase.pullDisabled).toBe(true); + }); + + it('prefers the freshly fetched branch listing over the polled status for ahead/behind', () => { + const h = deriveActionHints( + tKey, + branches({ upstream: 'origin/main', behind: 0 }), + status({ hasUpstream: true, behind: 4 }), + ); + expect(h.pull?.text).toBe('branchPicker.hint.upToDate'); + }); + + it('falls back to status for ahead/behind when the listing has no head entry', () => { + const noHead: DaemonGitBranchesResult = { ...branches(), local: [] }; + const h = deriveActionHints( + tKey, + noHead, + status({ hasUpstream: true, behind: 4 }), + ); + expect(h.pull?.text).toBe('↓4'); + }); + + it('shows no hints at all when neither source is known', () => { + const noHead: DaemonGitBranchesResult = { ...branches(), local: [] }; + const h = deriveActionHints(tKey, noHead, undefined); + expect(h).toEqual({ pullDisabled: false, pushDisabled: false }); + }); + + it('omits the commit hint on a v1 status without a computed tree summary', () => { + const h = deriveActionHints(tKey, branches({ upstream: 'origin/main' }), { + v: 1, + workspaceCwd: '/repo', + branch: 'main', + }); + expect(h.commit).toBeUndefined(); + expect(h.pull?.text).toBe('branchPicker.hint.upToDate'); + }); +}); + +describe('BranchPickerPopover action hints', () => { + function setup(): void { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + } + + it('renders hints beside the actions and disables pull without upstream', async () => { + workspaceGitBranches.mockResolvedValue(branches({ ahead: 1 })); + setup(); + mount({ onOpenCommit: vi.fn(), status: status({ unstaged: 2 }) }); + await flush(); + + const pull = document.body.querySelector( + '[data-testid="branch-picker-pull"]', + ); + expect(pull?.disabled).toBe(true); + expect(pull?.textContent).toContain('No upstream'); + + // The pull row is dimmed, not just disabled: both the class hook the + // stylesheet keys on and the tone attribute must be present. + expect(pull?.className).toMatch(/actionItemMuted/); + expect( + pull + ?.querySelector('[data-testid="branch-picker-action-hint"]') + ?.getAttribute('data-tone'), + ).toBe('muted'); + + const commit = document.body.querySelector( + '[data-testid="branch-picker-commit"]', + ); + expect(commit?.disabled).toBe(false); + expect(commit?.textContent).toContain('2 changes'); + expect(commit?.className).not.toMatch(/actionItemMuted/); + + const push = document.body.querySelector( + '[data-testid="branch-picker-push"]', + ); + expect(push?.disabled).toBe(false); + expect(push?.textContent).toContain('Sets upstream on push'); + expect(push?.className).not.toMatch(/actionItemMuted/); + }); + + it('dims every row on an in-sync clean tree', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main' }), + ); + setup(); + mount({ onOpenCommit: vi.fn(), status: status() }); + await flush(); + + for (const id of [ + 'branch-picker-pull', + 'branch-picker-commit', + 'branch-picker-push', + ]) { + const btn = document.body.querySelector( + `[data-testid="${id}"]`, + ); + expect(btn?.disabled).toBe(false); + expect(btn?.className).toMatch(/actionItemMuted/); + } + }); + + it('words a partially staged file as changes, not files', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main' }), + ); + setup(); + mount({ + onOpenCommit: vi.fn(), + status: status({ staged: 1, unstaged: 1 }), + }); + await flush(); + + const commit = document.body.querySelector( + '[data-testid="branch-picker-commit"]', + ); + expect(commit?.textContent).toContain('2 changes'); + expect(commit?.textContent).not.toContain('files'); + }); + + it('warns on pull when behind with uncommitted changes and keeps it enabled', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main', behind: 3 }), + ); + setup(); + mount({ status: status({ untracked: 1 }) }); + await flush(); + + const pull = document.body.querySelector( + '[data-testid="branch-picker-pull"]', + ); + expect(pull?.disabled).toBe(false); + const hint = pull?.querySelector( + '[data-testid="branch-picker-action-hint"]', + ); + expect(hint?.getAttribute('data-tone')).toBe('warning'); + expect(hint?.textContent).toBe('↓3 · uncommitted changes'); + }); + + it('disables pull and push while a rebase (detached HEAD) is in progress', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main', behind: 1 }, true), + ); + setup(); + mount({ + status: status({ operation: 'rebase', detached: true, conflicted: 1 }), + }); + await flush(); + + for (const id of ['branch-picker-pull', 'branch-picker-push']) { + const btn = document.body.querySelector( + `[data-testid="${id}"]`, + ); + expect(btn?.disabled).toBe(true); + expect(btn?.textContent).toContain('Rebasing'); + } + }); + + it('keeps push clickable during a conflicted merge on a branch', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main', ahead: 1 }), + ); + setup(); + mount({ status: status({ operation: 'merge', conflicted: 1 }) }); + await flush(); + + const pull = document.body.querySelector( + '[data-testid="branch-picker-pull"]', + ); + const push = document.body.querySelector( + '[data-testid="branch-picker-push"]', + ); + expect(pull?.disabled).toBe(true); + expect(push?.disabled).toBe(false); + expect( + push + ?.querySelector('[data-testid="branch-picker-action-hint"]') + ?.getAttribute('data-tone'), + ).toBe('warning'); + expect(push?.textContent).toContain('Merging'); + }); + + it('fetches its own status once on open, reports it, and prefers it over an older prop', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main' }), + ); + // The caller's snapshot says clean; the daemon now says otherwise. + workspaceGit.mockResolvedValue(status({ unstaged: 3, computedAt: 200 })); + setup(); + const onStatusRefreshed = vi.fn(); + const onOpenCommit = vi.fn(); + mount({ + onOpenCommit, + onStatusRefreshed, + status: status({ computedAt: 100 }), + }); + await flush(); + // Re-render with a new callback identity, as a parent whose handler + // calls setState would; the open effect must not re-arm. + mount({ + onOpenCommit, + onStatusRefreshed: (s) => onStatusRefreshed(s), + status: status({ computedAt: 100 }), + }); + await flush(); + + expect(workspaceGit).toHaveBeenCalledTimes(1); + expect(workspaceGit).toHaveBeenCalledWith({ wait: true }); + expect(onStatusRefreshed).toHaveBeenCalledTimes(1); + expect(onStatusRefreshed.mock.calls[0]?.[0]).toMatchObject({ + unstaged: 3, + }); + expect(workspaceGitBranches).toHaveBeenCalledTimes(1); + const commit = document.body.querySelector( + '[data-testid="branch-picker-commit"]', + ); + expect(commit?.textContent).toContain('3 changes'); + }); + + it('reads status through the worktree cwd when one is given', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main' }), + ); + workspaceGit.mockResolvedValue(status()); + setup(); + act(() => { + root.render( + + + + + , + ); + }); + await flush(); + expect(workspaceGit).toHaveBeenCalledWith({ + cwd: '/repo/.qwen/worktrees/wt', + }); + }); + + it('re-fetches the listing when a newer status contradicts it, once per status', async () => { + // Listing on open: tracking origin/main. Then the terminal runs + // `git branch --unset-upstream` and a newer status arrives while the + // popover is still open; the second listing fetch reflects that. + workspaceGitBranches + .mockResolvedValueOnce(branches({ upstream: 'origin/main' })) + .mockResolvedValue(branches({})); + setup(); + mount({ status: status({ hasUpstream: true, computedAt: 1 }) }); + await flush(); + expect(workspaceGitBranches).toHaveBeenCalledTimes(1); + expect( + document.body.querySelector( + '[data-testid="branch-picker-pull"]', + )?.disabled, + ).toBe(false); + + mount({ + status: status({ hasUpstream: false, computedAt: Date.now() + 60_000 }), + }); + await flush(); + expect(workspaceGitBranches).toHaveBeenCalledTimes(2); + const pull = document.body.querySelector( + '[data-testid="branch-picker-pull"]', + ); + expect(pull?.disabled).toBe(true); + expect(pull?.textContent).toContain('No upstream'); + + // The same status arriving again must not fetch again. + mount({ + status: status({ hasUpstream: false, computedAt: Date.now() + 60_000 }), + }); + await flush(); + expect(workspaceGitBranches).toHaveBeenCalledTimes(2); + }); + + it('leaves the listing alone when the newer status agrees with it', async () => { + workspaceGitBranches.mockResolvedValue( + branches({ upstream: 'origin/main', ahead: 2 }), + ); + setup(); + mount({ status: status({ computedAt: 1 }) }); + await flush(); + mount({ + status: status({ + hasUpstream: true, + ahead: 2, + behind: 0, + computedAt: Date.now() + 60_000, + }), + }); + await flush(); + expect(workspaceGitBranches).toHaveBeenCalledTimes(1); + }); +}); + +describe('listingContradictsStatus', () => { + it('flags upstream, detached, and ahead/behind disagreements only', () => { + const listing = branches({ upstream: 'origin/main', ahead: 1 }); + expect(listingContradictsStatus(listing, status())).toBe(false); + expect( + listingContradictsStatus(listing, status({ hasUpstream: false })), + ).toBe(true); + expect(listingContradictsStatus(listing, status({ detached: true }))).toBe( + true, + ); + expect(listingContradictsStatus(listing, status({ ahead: 2 }))).toBe(true); + expect(listingContradictsStatus(listing, status({ behind: 1 }))).toBe(true); + // Tree counters are not the listing's business. + expect( + listingContradictsStatus(listing, status({ unstaged: 5, staged: 2 })), + ).toBe(false); + // The status cannot express a gone upstream (it still reports tracking), + // so a gone listing entry never disagrees on the upstream axis. + const gone = branches({ upstream: 'origin/feat', upstreamGone: true }); + expect(listingContradictsStatus(gone, status({ hasUpstream: true }))).toBe( + false, + ); + expect(listingContradictsStatus(gone, status({ hasUpstream: false }))).toBe( + false, + ); + }); +}); diff --git a/packages/web-shell/client/components/BranchPickerPopover.tsx b/packages/web-shell/client/components/BranchPickerPopover.tsx index 532ca1f7777..59ebf0fda10 100644 --- a/packages/web-shell/client/components/BranchPickerPopover.tsx +++ b/packages/web-shell/client/components/BranchPickerPopover.tsx @@ -9,6 +9,7 @@ import { useWorkspace } from '@qwen-code/webui/daemon-react-sdk'; import type { DaemonGitBranchesResult, DaemonGitBranchInfo, + DaemonWorkspaceGitStatus, } from '@qwen-code/sdk/daemon'; import { ArrowDownToLineIcon, @@ -27,6 +28,7 @@ import { import { useI18n } from '../i18n'; import { Popover, PopoverContent, PopoverTrigger } from './ui/popover'; import { validateBranchName } from './GitModePopover'; +import { deriveStatus, hasComputedTreeSummary } from './GitBranchIndicator'; import styles from './BranchPickerPopover.module.css'; interface BranchPickerPopoverProps { @@ -36,6 +38,18 @@ interface BranchPickerPopoverProps { gitCwd?: string; side?: 'top' | 'right' | 'bottom' | 'left'; onBranchChanged?: () => void; + /** + * Working-tree summary from the trigger chip. Seeds the hints beside the + * Update / Commit / Push actions (dirty counts, in-progress operation) until + * the popover's own on-open fetch lands; whichever of the two carries the + * newer `computedAt` wins. + */ + status?: DaemonWorkspaceGitStatus; + /** + * Receives the status the popover fetches for itself on open, so a caller + * that renders a chip from the same object can update it in step. + */ + onStatusRefreshed?: (status: DaemonWorkspaceGitStatus) => void; onOpenDiff?: () => void; onOpenCommit?: () => void; children: React.ReactNode; @@ -43,6 +57,172 @@ interface BranchPickerPopoverProps { type SectionKey = 'recent' | 'local' | 'remote' | 'tags'; +type HintTone = 'muted' | 'info' | 'warning'; + +interface ActionHint { + text: string; + tone: HintTone; +} + +interface ActionHints { + pull?: ActionHint; + pullDisabled: boolean; + commit?: ActionHint; + push?: ActionHint; + pushDisabled: boolean; +} + +type TranslateFn = ReturnType['t']; + +/** + * Derive the per-action hints shown beside Update / Commit / Push so the user + * can judge before clicking. + * + * Disabling is reserved for what git itself refuses: `git pull` during a + * merge/rebase/cherry-pick, with unmerged entries, on a detached HEAD, or + * without a usable upstream; `git push --set-upstream` only on a detached + * HEAD (a push does not consult the index, so conflicts and in-progress + * operations are shown as warnings on an enabled row). Soft states (up to + * date, nothing to push, clean tree) only dim the row since the action is + * still harmless. + * + * The branch listing (fetched on open) provides ahead/behind/upstream; the + * status provides the tree counters and the in-progress operation. When the + * listing has no head entry the status fills in. Exported for tests. + */ +export function deriveActionHints( + t: TranslateFn, + data: DaemonGitBranchesResult | null, + status: DaemonWorkspaceGitStatus | undefined, +): ActionHints { + const head = data?.local.find((b) => b.isHead); + const s = deriveStatus(status); + const detached = data?.detached ?? s.detached; + const ahead = head?.ahead ?? s.ahead; + const behind = head?.behind ?? s.behind; + const upstream = head?.upstream; + const upstreamGone = head?.upstreamGone === true; + const hasUpstream: boolean | undefined = head + ? Boolean(head.upstream) && !upstreamGone + : status?.hasUpstream; + // Entry-granularity counters (a partially staged file counts twice, an + // untracked directory once), so the copy says "changes", not "files". + const changed = s.staged + s.unstaged + s.untracked + s.conflicted; + + const blocker: ActionHint | undefined = s.operation + ? { text: t(`git.operation.${s.operation}`), tone: 'warning' } + : s.conflicted > 0 + ? { text: t('git.conflicted', { count: s.conflicted }), tone: 'warning' } + : detached + ? { text: t('git.detached'), tone: 'warning' } + : undefined; + + let pull: ActionHint | undefined; + let pullDisabled = false; + if (blocker) { + pull = blocker; + pullDisabled = true; + } else if (hasUpstream === false) { + pull = { + text: t( + upstreamGone + ? 'branchPicker.hint.upstreamGone' + : 'branchPicker.hint.noUpstream', + ), + tone: 'muted', + }; + pullDisabled = true; + } else if (behind > 0) { + pull = + changed > 0 + ? { + text: t('branchPicker.hint.behindDirty', { count: behind }), + tone: 'warning', + } + : { + text: upstream ? `↓${behind} · ${upstream}` : `↓${behind}`, + tone: 'info', + }; + } else if (hasUpstream) { + pull = { text: t('branchPicker.hint.upToDate'), tone: 'muted' }; + } + + let push: ActionHint | undefined; + // Only a detached HEAD makes the daemon's `git push --set-upstream` fail; + // an in-progress operation or conflicts are surfaced but left clickable. + const pushDisabled = detached; + if (blocker) { + push = blocker; + } else if (hasUpstream === false) { + push = { text: t('branchPicker.hint.setsUpstream'), tone: 'info' }; + } else if (ahead > 0 && behind > 0) { + push = { + text: t('branchPicker.hint.aheadBehind', { ahead, behind }), + tone: 'warning', + }; + } else if (ahead > 0) { + push = { text: `↑${ahead}`, tone: 'info' }; + } else if (hasUpstream) { + push = { text: t('branchPicker.hint.nothingToPush'), tone: 'muted' }; + } + + let commit: ActionHint | undefined; + if (hasComputedTreeSummary(status)) { + commit = + changed > 0 + ? { + text: + s.untracked > 0 + ? t('branchPicker.hint.changesUntracked', { + count: changed, + untracked: s.untracked, + }) + : t('branchPicker.hint.changes', { count: changed }), + tone: 'info', + } + : { text: t('branchPicker.hint.noChanges'), tone: 'muted' }; + } + + return { pull, pullDisabled, commit, push, pushDisabled }; +} + +/** Of two statuses, the one the daemon computed later (a missing stamp loses). */ +function newerStatus( + a: DaemonWorkspaceGitStatus | undefined, + b: DaemonWorkspaceGitStatus | undefined, +): DaemonWorkspaceGitStatus | undefined { + if (!a) return b; + if (!b) return a; + return (b.computedAt ?? -1) >= (a.computedAt ?? -1) ? b : a; +} + +/** + * True when a status disagrees with the branch listing on a field the hints + * take from the listing — the signal that the listing is stale and should be + * re-fetched. Exported for tests. + */ +export function listingContradictsStatus( + data: DaemonGitBranchesResult, + status: DaemonWorkspaceGitStatus, +): boolean { + if (status.detached !== undefined && status.detached !== data.detached) { + return true; + } + const head = data.local.find((b) => b.isHead); + if (!head) return false; + // The status cannot express a gone upstream (it reports the configured + // tracking as present), so the listing's `upstreamGone` is not a + // disagreement — only a genuinely set/unset upstream is. + const upstreamComparable = !head.upstreamGone; + return ( + (upstreamComparable && + status.hasUpstream !== undefined && + status.hasUpstream !== Boolean(head.upstream)) || + (status.ahead !== undefined && status.ahead !== head.ahead) || + (status.behind !== undefined && status.behind !== head.behind) + ); +} + export function BranchPickerPopover({ open, onOpenChange, @@ -50,6 +230,8 @@ export function BranchPickerPopover({ gitCwd, side = 'bottom', onBranchChanged, + status, + onStatusRefreshed, onOpenDiff, onOpenCommit, children, @@ -82,6 +264,19 @@ export function BranchPickerPopover({ const searchRef = useRef(null); const contentRef = useRef(null); const requestIdRef = useRef(0); + // Wall-clock time the current listing was received; lets a status the + // daemon computed later trigger a listing re-fetch (see the effect below). + const [listingFetchedAt, setListingFetchedAt] = useState(); + // The popover's own on-open status fetch, so every entry point (sidebar + // chip, composer chip, environment panel) sees fresh counters instead of + // whatever its caller last polled. + const [liveStatus, setLiveStatus] = useState(); + const statusRequestIdRef = useRef(0); + const reconciledAtRef = useRef(undefined); + // Held in a ref so an inline callback from the parent doesn't re-arm the + // open effect on every render (callback → setState → render → refetch…). + const onStatusRefreshedRef = useRef(onStatusRefreshed); + onStatusRefreshedRef.current = onStatusRefreshed; const fetchBranches = useCallback(async () => { const requestId = ++requestIdRef.current; @@ -91,6 +286,7 @@ export function BranchPickerPopover({ const result = await ws.workspaceGitBranches(gitCwd); if (requestId !== requestIdRef.current) return; setData(result); + setListingFetchedAt(Date.now()); } catch (err) { if (requestId !== requestIdRef.current) return; setError(err instanceof Error ? err.message : String(err)); @@ -101,9 +297,37 @@ export function BranchPickerPopover({ } }, [ws, gitCwd]); + const fetchStatus = useCallback(async () => { + const requestId = ++statusRequestIdRef.current; + try { + // Mirrors the app-level poll: a worktree `?cwd=` read always computes + // directly, so `wait` only matters for the workspace root. + const fresh = await ws.workspaceGit( + gitCwd ? { cwd: gitCwd } : { wait: true }, + ); + if (requestId !== statusRequestIdRef.current) return; + setLiveStatus(fresh); + onStatusRefreshedRef.current?.(fresh); + } catch { + // Keep whatever the caller passed; the hints degrade to the listing. + } + }, [ws, gitCwd]); + + // A status fetched for a previous workspace must not seed the next one. + useEffect(() => { + setLiveStatus(undefined); + statusRequestIdRef.current++; + }, [ws, gitCwd]); + + const effectiveStatus = useMemo( + () => newerStatus(status, liveStatus), + [status, liveStatus], + ); + useEffect(() => { if (open) { void fetchBranches(); + void fetchStatus(); setSearch(''); setNewBranchMode(false); setCheckoutRefMode(false); @@ -112,7 +336,23 @@ export function BranchPickerPopover({ setStatusMsg(null); setTimeout(() => searchRef.current?.focus(), 50); } - }, [open, fetchBranches]); + }, [open, fetchBranches, fetchStatus]); + + // The listing is fetched once on open. If a status the daemon computed + // after that disagrees with it (upstream unset, HEAD detached, new commits + // from a terminal), re-fetch the listing so the rows follow the repo rather + // than the snapshot — once per status, so a persistent disagreement can't + // loop. + useEffect(() => { + if (!open || !data || !effectiveStatus || listingFetchedAt === undefined) + return; + const at = effectiveStatus.computedAt; + if (at === undefined || at <= listingFetchedAt) return; + if (reconciledAtRef.current === at) return; + if (!listingContradictsStatus(data, effectiveStatus)) return; + reconciledAtRef.current = at; + void fetchBranches(); + }, [open, data, effectiveStatus, listingFetchedAt, fetchBranches]); const showStatus = useCallback( (msg: string, type: 'info' | 'error' | 'success' = 'info') => { @@ -254,6 +494,11 @@ export function BranchPickerPopover({ return groups; }, [filteredRemote]); + const hints = useMemo( + () => deriveActionHints(t, data, effectiveStatus), + [t, data, effectiveStatus], + ); + const actionsVisible = !q || t('branchPicker.action.pull').toLowerCase().includes(q) || @@ -321,9 +566,10 @@ export function BranchPickerPopover({ <> {onOpenCommit && ( )} {onOpenDiff && (