Skip to content

Commit be5e194

Browse files
committed
fix(web): keep diff base selector available after errors
1 parent 6575ce3 commit be5e194

2 files changed

Lines changed: 228 additions & 14 deletions

File tree

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
import { RegistryContext } from "@effect/atom-react";
2+
import { scopedThreadKey } from "@t3tools/client-runtime/environment";
3+
import {
4+
DEFAULT_CLIENT_SETTINGS,
5+
EnvironmentId,
6+
ProjectId,
7+
ThreadId,
8+
type ClientSettings,
9+
type ReviewBranchCommitsResult,
10+
type ReviewDiffPreviewResult,
11+
type ScopedThreadRef,
12+
type VcsListRefsResult,
13+
type VcsStatusResult,
14+
} from "@t3tools/contracts";
15+
import * as DateTime from "effect/DateTime";
16+
import { AsyncResult, Atom, AtomRegistry } from "effect/reactivity";
17+
import { expect, it, vi } from "vite-plus/test";
18+
import { page, userEvent } from "vite-plus/test/browser";
19+
20+
import "~/index.css";
21+
import { render } from "../../test/render";
22+
import { makeThreadFixture } from "~/test-fixtures";
23+
24+
type ResultAtom<A> = Atom.Atom<AsyncResult.AsyncResult<A, Error>>;
25+
26+
const state = vi.hoisted(() => ({
27+
thread: null as ReturnType<typeof makeThreadFixture> | null,
28+
threadRef: null as ScopedThreadRef | null,
29+
previews: new Map<string | null, ResultAtom<ReviewDiffPreviewResult>>(),
30+
config: null as unknown as Atom.Atom<null>,
31+
status: null as unknown as ResultAtom<VcsStatusResult>,
32+
localRefs: null as unknown as ResultAtom<VcsListRefsResult>,
33+
remoteRefs: null as unknown as ResultAtom<VcsListRefsResult>,
34+
commits: null as unknown as ResultAtom<ReviewBranchCommitsResult>,
35+
}));
36+
37+
vi.mock("~/state/server", () => ({
38+
serverEnvironment: { configValueAtom: () => state.config },
39+
}));
40+
vi.mock("~/state/review", () => ({
41+
reviewEnvironment: {
42+
diffPreview: ({ input }: { input: { baseRef?: string } }) =>
43+
state.previews.get(input.baseRef ?? null)!,
44+
branchCommits: () => state.commits,
45+
diffFileContents: {},
46+
},
47+
}));
48+
vi.mock("~/state/vcs", () => ({
49+
vcsEnvironment: {
50+
status: () => state.status,
51+
listRefs: ({ input }: { input: { refKind: "local" | "remote" } }) =>
52+
input.refKind === "local" ? state.localRefs : state.remoteRefs,
53+
},
54+
}));
55+
vi.mock("~/state/entities", () => ({
56+
useQuietThreadShell: () => state.thread,
57+
useProject: () => ({ workspaceRoot: "/project", repositoryIdentity: null }),
58+
useThreadCheckpointSummaries: () => [],
59+
useQuietThreadVisibleTurnItems: () => [],
60+
}));
61+
vi.mock("~/state/filesystem", () => ({
62+
useFilesystemReadAccess: () => ({ canReadFiles: true, isPending: false, error: null }),
63+
}));
64+
vi.mock("~/hooks/useThreadRouteTarget", () => ({ useThreadRouteRef: () => state.threadRef }));
65+
vi.mock("~/hooks/useSettings", () => ({
66+
useClientSettings: (select: (settings: ClientSettings) => unknown) =>
67+
select(DEFAULT_CLIENT_SETTINGS),
68+
updateClientSettings: async () => {},
69+
}));
70+
vi.mock("~/hooks/useTheme", () => ({ useRenderedAppearance: () => "dark" }));
71+
vi.mock("~/state/queries", () => ({
72+
useCheckpointDiff: () => ({ data: null, error: null, isPending: false }),
73+
useTurnItemDetail: () => ({ data: null, error: null, isPending: false }),
74+
}));
75+
vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => async () => {} }));
76+
vi.mock("~/editorPreferences", () => ({ useOpenInPreferredEditor: () => async () => {} }));
77+
vi.mock("~/fileContextMenu", () => ({ useFileContextMenuHandler: () => () => {} }));
78+
vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => async () => {} }));
79+
// The test drives comparison controls, without mounting virtualized diff viewers or their workers.
80+
vi.mock("./diffs/AnnotatableCodeView", () => ({ AnnotatableCodeView: () => null }));
81+
vi.mock("./diffs/DiffFileTree", () => ({ DiffFileTree: () => null }));
82+
vi.mock("./DiffFilePathCopyButton", () => ({ DiffFilePathCopyButton: () => null }));
83+
84+
import DiffPanel from "./DiffPanel";
85+
import { useDiffPanelSessionStore, useDiffPanelStore } from "~/diffPanelStore";
86+
87+
const cwd = "/project-worktree";
88+
const errorMessage = "Could not find a common commit between 'origin/orphan' and HEAD.";
89+
90+
function preview(): ReviewDiffPreviewResult {
91+
return {
92+
cwd,
93+
generatedAt: DateTime.makeUnsafe("2026-10-30T00:00:00Z"),
94+
sources: [
95+
{
96+
id: "branch",
97+
kind: "branch-range",
98+
title: "Changes",
99+
baseRef: "main",
100+
headRef: "dev",
101+
diff: "",
102+
diffHash: "empty",
103+
truncated: false,
104+
},
105+
],
106+
};
107+
}
108+
109+
function setup(failedBaseRef: string | null) {
110+
const reset = () => {
111+
useDiffPanelStore.setState({
112+
byThreadKey: {},
113+
branchBaseRefByThreadKey: {},
114+
keepSelectionOnOpenThreadKey: null,
115+
});
116+
useDiffPanelSessionStore.setState({ collapsedFileKeysByScopeKey: {} });
117+
};
118+
reset();
119+
state.threadRef = {
120+
environmentId: EnvironmentId.make("environment"),
121+
threadId: ThreadId.make("thread"),
122+
};
123+
state.thread = makeThreadFixture({
124+
...state.threadRef,
125+
id: state.threadRef.threadId,
126+
projectId: ProjectId.make("project"),
127+
branch: "dev",
128+
worktreePath: cwd,
129+
});
130+
state.previews.clear();
131+
state.previews.set(null, Atom.make(AsyncResult.success(preview())));
132+
state.previews.set("main", Atom.make(AsyncResult.success(preview())));
133+
state.previews.set(failedBaseRef, Atom.make(AsyncResult.fail(new Error(errorMessage))));
134+
state.config = Atom.make(null);
135+
state.status = Atom.make(
136+
AsyncResult.success({
137+
isRepo: true,
138+
hasPrimaryRemote: true,
139+
isDefaultRef: false,
140+
refName: "dev",
141+
hasWorkingTreeChanges: false,
142+
workingTree: { rootPath: cwd, files: [], insertions: 0, deletions: 0 },
143+
hasUpstream: true,
144+
aheadCount: 0,
145+
behindCount: 0,
146+
pr: null,
147+
}),
148+
);
149+
const refs = (names: string[], isRemote: boolean): VcsListRefsResult => ({
150+
isRepo: true,
151+
hasPrimaryRemote: true,
152+
nextCursor: null,
153+
totalCount: names.length,
154+
refs: names.map((name) => ({
155+
name,
156+
isRemote,
157+
...(isRemote ? { remoteName: "origin" } : {}),
158+
current: name === "dev",
159+
isDefault: name === "main",
160+
worktreePath: null,
161+
})),
162+
});
163+
state.localRefs = Atom.make(AsyncResult.success(refs(["dev", "main"], false)));
164+
state.remoteRefs = Atom.make(AsyncResult.success(refs(["origin/orphan"], true)));
165+
state.commits = Atom.make(AsyncResult.success({ commits: [], truncated: false }));
166+
useDiffPanelStore.getState().selectBranchBaseRef(state.threadRef, failedBaseRef);
167+
const registry = AtomRegistry.make();
168+
return {
169+
registry,
170+
threadRef: state.threadRef,
171+
[Symbol.dispose]() {
172+
registry.dispose();
173+
reset();
174+
},
175+
};
176+
}
177+
178+
it.each([
179+
{
180+
name: "a saved unrelated branch",
181+
failedBaseRef: "origin/orphan",
182+
recovery: "Automatic",
183+
baseRef: null,
184+
},
185+
{
186+
name: "an automatic base with unrelated history",
187+
failedBaseRef: null,
188+
recovery: "main",
189+
baseRef: "main",
190+
},
191+
])(
192+
"recovers from $name without leaving the diff panel",
193+
async ({ failedBaseRef, recovery, baseRef }) => {
194+
using context = setup(failedBaseRef);
195+
await using screen = await render(
196+
<RegistryContext.Provider value={context.registry}>
197+
<DiffPanel composerDraftTarget={context.threadRef} workspaceMutationId={null} visible />
198+
</RegistryContext.Provider>,
199+
);
200+
await expect.element(screen.getByText(errorMessage, { exact: true })).toBeVisible();
201+
const selector = screen.getByRole("combobox", { name: /Change comparison target/ });
202+
await vi.waitFor(() => expect(selector.query()).not.toBeNull());
203+
await userEvent.click(selector);
204+
await expect.element(page.getByRole("option", { name: "main", exact: true })).toBeVisible();
205+
await userEvent.click(page.getByRole("option", { name: recovery, exact: true }));
206+
await expect.element(screen.getByText("No net changes in this selection.")).toBeVisible();
207+
await expect.element(screen.getByText(errorMessage, { exact: true })).not.toBeInTheDocument();
208+
const key = scopedThreadKey(context.threadRef);
209+
expect(useDiffPanelStore.getState().byThreadKey[key]).toEqual({ kind: "branch", baseRef });
210+
expect(useDiffPanelStore.getState().branchBaseRefByThreadKey[key]).toBe(baseRef);
211+
},
212+
);

‎apps/web/src/components/DiffPanel.tsx‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,8 @@ export default function DiffPanel({
403403
? "commit"
404404
: "branch-range"),
405405
);
406+
const comparisonHeadRef = selectedGitSource?.headRef ?? gitStatusQuery.data?.refName ?? "HEAD";
407+
const comparisonBaseRef = selectedGitSource?.baseRef ?? selectedBaseRef ?? "Automatic";
406408
const selectedCommitSubject =
407409
selectedCommitSha !== null && selectedGitSource?.headRef === selectedCommitSha
408410
? selectedGitSource.commitSubject
@@ -518,12 +520,14 @@ export default function DiffPanel({
518520
}
519521
return currentLoadDiffFiles(fileDiff);
520522
});
523+
const canSelectBranchBase = canReadFiles && canRefreshGitDiff && selectedGitScope === "branch";
524+
const branchRefsCwd = branchDiffPreview.data?.cwd ?? activeCwd;
521525
const localBranchRefs = useEnvironmentQuery(
522-
isGitSelection && selectedGitScope === "branch" && activeThread && branchDiffPreview.data?.cwd
526+
canSelectBranchBase && activeThread && branchRefsCwd
523527
? vcsEnvironment.listRefs({
524528
environmentId: activeThread.environmentId,
525529
input: {
526-
cwd: branchDiffPreview.data.cwd,
530+
cwd: branchRefsCwd,
527531
includeMatchingRemoteRefs: true,
528532
refKind: "local",
529533
...(baseRefQuery.trim().length > 0 ? { query: baseRefQuery.trim() } : {}),
@@ -533,11 +537,11 @@ export default function DiffPanel({
533537
: null,
534538
);
535539
const remoteBranchRefs = useEnvironmentQuery(
536-
isGitSelection && selectedGitScope === "branch" && activeThread && branchDiffPreview.data?.cwd
540+
canSelectBranchBase && activeThread && branchRefsCwd
537541
? vcsEnvironment.listRefs({
538542
environmentId: activeThread.environmentId,
539543
input: {
540-
cwd: branchDiffPreview.data.cwd,
544+
cwd: branchRefsCwd,
541545
includeMatchingRemoteRefs: true,
542546
refKind: "remote",
543547
...(baseRefQuery.trim().length > 0 ? { query: baseRefQuery.trim() } : {}),
@@ -547,7 +551,7 @@ export default function DiffPanel({
547551
: null,
548552
);
549553
const baseRefChoices = buildBaseRefChoices(
550-
localBranchRefs.data?.refs.filter((ref) => ref.name !== selectedGitSource?.headRef) ?? [],
554+
localBranchRefs.data?.refs.filter((ref) => ref.name !== comparisonHeadRef) ?? [],
551555
remoteBranchRefs.data?.refs ?? [],
552556
);
553557
const matchingBaseRefChoices = filterBaseRefChoices(baseRefChoices, baseRefQuery);
@@ -949,10 +953,10 @@ export default function DiffPanel({
949953
</DropdownMenuSub>
950954
</DropdownMenuContent>
951955
</DropdownMenu>
952-
{isGitSelection && selectedGitScope === "branch" && selectedGitSource?.baseRef && (
956+
{canSelectBranchBase && (
953957
<div
954958
className="flex min-w-0 max-w-full items-center gap-2 overflow-hidden text-xs text-muted-foreground"
955-
aria-label={`Comparing ${selectedGitSource.headRef ?? "HEAD"} against ${selectedGitSource.baseRef}`}
959+
aria-label={`Comparing ${comparisonHeadRef} against ${comparisonBaseRef}`}
956960
>
957961
<Tooltip>
958962
<TooltipTrigger
@@ -962,13 +966,11 @@ export default function DiffPanel({
962966
<span className="flex min-w-[min(3rem,calc(100%-2.75rem))] items-center gap-2 overflow-hidden @max-[20rem]/diff-refs:hidden" />
963967
}
964968
>
965-
<span className="min-w-0 max-w-48 truncate">
966-
{selectedGitSource.headRef ?? "HEAD"}
967-
</span>
969+
<span className="min-w-0 max-w-48 truncate">{comparisonHeadRef}</span>
968970
<ArrowRightIcon className="size-3.5 shrink-0 opacity-70" />
969971
</TooltipTrigger>
970972
<TooltipPopup side="top">
971-
{`${selectedGitSource.headRef ?? "HEAD"} → ${selectedGitSource.baseRef}`}
973+
{`${comparisonHeadRef} → ${comparisonBaseRef}`}
972974
</TooltipPopup>
973975
</Tooltip>
974976
<Tooltip>
@@ -978,7 +980,7 @@ export default function DiffPanel({
978980
vs
979981
</TooltipTrigger>
980982
<TooltipPopup side="top">
981-
{`${selectedGitSource.headRef ?? "HEAD"} → ${selectedGitSource.baseRef}`}
983+
{`${comparisonHeadRef} → ${comparisonBaseRef}`}
982984
</TooltipPopup>
983985
</Tooltip>
984986
<Combobox
@@ -996,9 +998,9 @@ export default function DiffPanel({
996998
<ComboboxTrigger
997999
render={<Button variant="ghost-muted" size="xs" />}
9981000
className="min-w-9 max-w-48 shrink-0"
999-
aria-label={`Change comparison target. Currently ${selectedGitSource.baseRef}`}
1001+
aria-label={`Change comparison target. Currently ${comparisonBaseRef}`}
10001002
>
1001-
<span className="min-w-0 truncate">{selectedGitSource.baseRef}</span>
1003+
<span className="min-w-0 truncate">{comparisonBaseRef}</span>
10021004
<ChevronDownIcon className="size-3.5 shrink-0 opacity-70" />
10031005
</ComboboxTrigger>
10041006
<ComboboxPopup

0 commit comments

Comments
 (0)