Skip to content
Merged
46 changes: 46 additions & 0 deletions .qwen/e2e-tests/2026-09-02-webshell-push-nonff-disable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Push disabled on non-fast-forward states

## Scenario

Open a trusted git workspace in the Web Shell and open the branch picker from
the sidebar git chip with the repo in each state:

1. Tracking upstream, ahead 1 / behind 0 (pushable).
2. Behind 3, clean tree, ahead 0.
3. Ahead 1 / behind 1 (diverged).
4. Conflicted merge in progress on the branch, ahead 1 / behind 1.
5. Update Project blocked by uncommitted changes (409 panel up), ahead 1 /
behind 0; click Push while the panel shows.
Comment thread
wenshao marked this conversation as resolved.
Resolved

## Checks

- State 1: Push shows `↑1` and is enabled.
- State 2: Push shows "Nothing to push" (dimmed) and is now **disabled** —
pushing an older tip is a non-fast-forward the remote rejects
unconditionally.
- State 3: Push shows the warning `↑1 ↓1 · update first` and is now
**disabled** for the same reason.
- State 4: Push shows the warning "Merging" and stays **enabled** — during an
in-progress operation the behind count is in flux (concluding the merge is
what resolves it), so the row only warns.
- State 5: the pull-resolution panel clears and the push outcome shows in the
status line (unchanged from #10390's flow; the scenario's fixture must be
ahead-only now).
- A caller status and the popover's own on-open fetch carrying the same
`computedAt` render the fetched counters (tie breaks toward the fetch).

## Evidence

Follow-ups to the sandboxed verification of #10397 (run 33195566824): F1's
measured rule (`detached || (!operation && hasUpstream && behind > 0)`, its
M6' row, green there on 170 tests) and F2's surviving M5 mutant
(`newerStatus` `>=` → `>`), now pinned. Local mutation A/B: reverting the rule
to `detached` fails 3 tests; `>=` → `>` fails exactly the tie-break test.

```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 \
client/components/ChatEditor.test.tsx
```
74 changes: 70 additions & 4 deletions packages/web-shell/client/components/BranchPickerPopover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ afterEach(() => {
});
workspaceGit.mockRejectedValue(new Error('no status'));

function mountWithBranches(): void {
workspaceGitBranches.mockResolvedValue(BRANCHES);
function mountWithBranches(branchesResult: typeof BRANCHES = BRANCHES): void {
workspaceGitBranches.mockResolvedValue(branchesResult);
container = document.createElement('div');
document.body.appendChild(container);
root = createRoot(container);
Expand Down Expand Up @@ -430,7 +430,12 @@ describe('BranchPickerPopover actions', () => {
success: true,
output: 'pushed to origin',
});
mountWithBranches();
// Ahead-only: the push row must be enabled for this scenario (a branch
// behind its upstream now has push disabled as a non-fast-forward).
mountWithBranches({
...BRANCHES,
local: [{ ...BRANCHES.local[0], ahead: 1, behind: 0 }],
});
await flush();

clickButton('Update Project');
Expand Down Expand Up @@ -766,14 +771,20 @@ describe('deriveActionHints', () => {
});
});

it('shows behind count with upstream for a clean tree', () => {
it('shows behind count with upstream for a clean tree and blocks push', () => {
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);
// Pushing an older tip is a non-fast-forward the remote refuses.
expect(h.push).toEqual({
text: 'branchPicker.hint.nothingToPush',
tone: 'muted',
});
expect(h.pushDisabled).toBe(true);
});

it('warns on pull when behind with uncommitted changes', () => {
Expand Down Expand Up @@ -839,6 +850,21 @@ describe('deriveActionHints', () => {
text: 'branchPicker.hint.aheadBehind:{"ahead":2,"behind":1}',
tone: 'warning',
});
// Diverged tips are a non-fast-forward too: the warning row is disabled.
expect(
deriveActionHints(
tKey,
branches({ upstream: 'origin/main', ahead: 2, behind: 1 }),
status(),
).pushDisabled,
).toBe(true);
expect(
deriveActionHints(
tKey,
branches({ upstream: 'origin/main', ahead: 2 }),
status(),
).pushDisabled,
).toBe(false);
});

it('counts changes (entries, not files) for commit and calls out untracked ones', () => {
Expand Down Expand Up @@ -884,6 +910,8 @@ describe('deriveActionHints', () => {
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' });
// behind > 0, but mid-operation the behind count is in flux (the merge
// being concluded is what resolves it), so the row only warns.
expect(op.pushDisabled).toBe(false);

const conflict = deriveActionHints(
Expand Down Expand Up @@ -1079,6 +1107,44 @@ describe('BranchPickerPopover action hints', () => {
}
});

it('disables push while the branch is behind or diverged from its upstream', async () => {
workspaceGitBranches.mockResolvedValue(
branches({ upstream: 'origin/main', ahead: 1, behind: 2 }),
);
setup();
mount({ status: status() });
await flush();

const push = document.body.querySelector<HTMLButtonElement>(
'[data-testid="branch-picker-push"]',
);
expect(push?.disabled).toBe(true);
expect(
push
?.querySelector('[data-testid="branch-picker-action-hint"]')
?.getAttribute('data-tone'),
).toBe('warning');
expect(push?.textContent).toContain('update first');
});

it("breaks a computedAt tie in favor of the popover's own fetch", async () => {
// The caller's snapshot and the on-open fetch can carry the same stamp
// (the daemon dedupes concurrent computations); the fresher fetch wins.
workspaceGitBranches.mockResolvedValue(
branches({ upstream: 'origin/main' }),
);
workspaceGit.mockResolvedValue(status({ unstaged: 3, computedAt: 100 }));
setup();
mount({ onOpenCommit: vi.fn(), status: status({ computedAt: 100 }) });
await flush();

const commit = document.body.querySelector<HTMLButtonElement>(
'[data-testid="branch-picker-commit"]',
);
expect(commit?.textContent).toContain('3 changes');
expect(commit?.textContent).not.toContain('No changes');
});

it('keeps push clickable during a conflicted merge on a branch', async () => {
workspaceGitBranches.mockResolvedValue(
branches({ upstream: 'origin/main', ahead: 1 }),
Expand Down
22 changes: 14 additions & 8 deletions packages/web-shell/client/components/BranchPickerPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,13 @@ type TranslateFn = ReturnType<typeof useI18n>['t'];
*
* 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.
* without a usable upstream; `git push` on a detached HEAD or from a branch
* that is behind its upstream — pushing an older or diverged tip is a
* non-fast-forward the remote rejects unconditionally. A push does not
Comment thread
wenshao marked this conversation as resolved.
Resolved
* consult the index, and mid-operation the outcome depends on where the
* operation ends, 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
Expand Down Expand Up @@ -191,9 +193,13 @@ export function deriveActionHints(
}

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;
// A detached HEAD makes the daemon's `git push --set-upstream` fail, and a
// branch behind its upstream cannot push: moving the remote ref to an older
// (behind-only) or diverged tip is a non-fast-forward rejection regardless
// of config. Mid-operation the picture can change by the time the operation
// ends, so the row only warns then; conflicts alone don't stop a push.
const pushDisabled =
detached || (!s.operation && hasUpstream === true && behind > 0);
Comment thread
wenshao marked this conversation as resolved.
Resolved
Comment thread
wenshao marked this conversation as resolved.
Resolved
if (blocker) {
push = blocker;
} else if (hasUpstream === false) {
Expand Down
Loading