Skip to content

Commit e10ba47

Browse files
committed
Refine sidebar worktrees implementation with type safety
- Add showSidebarWorktrees to settings rehydration test - Clarify padding calculation comment in SidebarSection - Use AdoptableWorktree named type instead of inline object type
1 parent 9103574 commit e10ba47

4 files changed

Lines changed: 217 additions & 1 deletion

File tree

packages/ui/src/features/settings/settingsStore.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ describe("feature settingsStore cloud selections", () => {
219219
["debugLogsCloudRuns", false, true],
220220
["slotMachineMode", false, true],
221221
["dismissibleUpdateBanners", false, true],
222+
["showSidebarWorktrees", false, true],
222223
] as const)("rehydrates %s", async (field, initial, persisted) => {
223224
getItem.mockResolvedValue(
224225
JSON.stringify({ state: { [field]: persisted }, version: 0 }),

packages/ui/src/features/sidebar/components/SidebarSection.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export function SidebarSection({
4747
className="flex w-full items-center justify-between pl-2 not-hover:aria-expanded:bg-transparent"
4848
style={{
4949
marginTop: addSpacingBefore ? "12px" : undefined,
50+
// + 8 matches the base pl-2 padding set via className below.
5051
paddingLeft: depth ? `${depth * INDENT_SIZE + 8}px` : undefined,
5152
}}
5253
onContextMenu={onContextMenu}
Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
import type { Task } from "@posthog/shared/domain-types";
2+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
3+
import { act, renderHook, waitFor } from "@testing-library/react";
4+
import type { ReactNode } from "react";
5+
import { beforeEach, describe, expect, it, vi } from "vitest";
6+
7+
const createTaskMock = vi.hoisted(() => vi.fn());
8+
const invalidateTasksMock = vi.hoisted(() => vi.fn());
9+
const toastErrorMock = vi.hoisted(() => vi.fn());
10+
const setFailedMock = vi.hoisted(() => vi.fn());
11+
const openTaskMock = vi.hoisted(() => vi.fn());
12+
const trackMock = vi.hoisted(() => vi.fn());
13+
const queryFilterMock = vi.hoisted(() =>
14+
vi.fn((input: { mainRepoPath: string }) => ({
15+
queryKey: ["workspace", "listAdoptableWorktrees", input],
16+
})),
17+
);
18+
19+
vi.mock("@posthog/di/react", () => ({
20+
useService: () => ({ createTask: createTaskMock }),
21+
}));
22+
vi.mock("@posthog/host-router/react", () => ({
23+
useHostTRPC: () => ({
24+
workspace: { listAdoptableWorktrees: { queryFilter: queryFilterMock } },
25+
}),
26+
}));
27+
vi.mock("@posthog/ui/features/tasks/useTaskCrudMutations", () => ({
28+
useCreateTask: () => ({ invalidateTasks: invalidateTasksMock }),
29+
}));
30+
vi.mock("@posthog/ui/features/notifications/errorDetails", () => ({
31+
toastError: toastErrorMock,
32+
}));
33+
vi.mock("@posthog/ui/features/provisioning/store", () => ({
34+
useProvisioningStore: { getState: () => ({ setFailed: setFailedMock }) },
35+
}));
36+
vi.mock("@posthog/ui/router/useOpenTask", () => ({
37+
openTask: openTaskMock,
38+
}));
39+
vi.mock("@posthog/ui/shell/analytics", () => ({
40+
track: trackMock,
41+
}));
42+
43+
import { useStartTaskFromWorktree } from "./useStartTaskFromWorktree";
44+
45+
const MAIN_REPO_PATH = "/repo";
46+
const BRANCH = "feature/orphan";
47+
48+
function fakeTask(): Task {
49+
return {
50+
id: "task-123",
51+
task_number: 1,
52+
slug: "task-123",
53+
title: BRANCH,
54+
description: BRANCH,
55+
created_at: "2026-06-15T00:00:00.000Z",
56+
updated_at: "2026-06-15T00:00:00.000Z",
57+
origin_product: "user_created",
58+
};
59+
}
60+
61+
function wrapper({ children }: { children: ReactNode }) {
62+
const queryClient = new QueryClient({
63+
defaultOptions: { queries: { retry: false } },
64+
});
65+
return (
66+
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
67+
);
68+
}
69+
70+
describe("useStartTaskFromWorktree", () => {
71+
beforeEach(() => {
72+
vi.clearAllMocks();
73+
});
74+
75+
it("creates the task, opens it, tracks the event, and clears the in-flight branch", async () => {
76+
const task = fakeTask();
77+
createTaskMock.mockImplementationOnce(async (_input, onTaskReady) => {
78+
onTaskReady?.({ task, workspace: null });
79+
return { success: true, data: { task, workspace: null } };
80+
});
81+
82+
const { result } = renderHook(
83+
() => useStartTaskFromWorktree(MAIN_REPO_PATH),
84+
{
85+
wrapper,
86+
},
87+
);
88+
89+
await act(async () => {
90+
await result.current.startTask(BRANCH);
91+
});
92+
93+
expect(createTaskMock).toHaveBeenCalledWith(
94+
expect.objectContaining({
95+
taskDescription: BRANCH,
96+
repoPath: MAIN_REPO_PATH,
97+
workspaceMode: "worktree",
98+
branch: BRANCH,
99+
reuseExistingWorktree: true,
100+
}),
101+
expect.any(Function),
102+
);
103+
expect(invalidateTasksMock).toHaveBeenCalledWith(task);
104+
expect(openTaskMock).toHaveBeenCalledWith(task);
105+
expect(trackMock).toHaveBeenCalledWith(
106+
"Task created",
107+
expect.objectContaining({
108+
created_from: "sidebar-worktree",
109+
workspace_mode: "worktree",
110+
}),
111+
);
112+
expect(queryFilterMock).toHaveBeenCalledWith({
113+
mainRepoPath: MAIN_REPO_PATH,
114+
});
115+
expect(toastErrorMock).not.toHaveBeenCalled();
116+
expect(setFailedMock).not.toHaveBeenCalled();
117+
await waitFor(() =>
118+
expect(result.current.startingBranches.has(BRANCH)).toBe(false),
119+
);
120+
});
121+
122+
it("toasts and does not track when task creation fails validation", async () => {
123+
createTaskMock.mockResolvedValueOnce({
124+
success: false,
125+
error: "Task description cannot be empty",
126+
failedStep: "validation",
127+
});
128+
129+
const { result } = renderHook(
130+
() => useStartTaskFromWorktree(MAIN_REPO_PATH),
131+
{
132+
wrapper,
133+
},
134+
);
135+
136+
await act(async () => {
137+
await result.current.startTask(BRANCH);
138+
});
139+
140+
expect(toastErrorMock).toHaveBeenCalledWith(
141+
"Task creation failed",
142+
"Task description cannot be empty",
143+
);
144+
expect(trackMock).not.toHaveBeenCalled();
145+
expect(invalidateTasksMock).not.toHaveBeenCalled();
146+
expect(queryFilterMock).not.toHaveBeenCalled();
147+
await waitFor(() =>
148+
expect(result.current.startingBranches.has(BRANCH)).toBe(false),
149+
);
150+
});
151+
152+
it("marks the task failed and toasts on a provisioning error, but still tracks and invalidates", async () => {
153+
const task = fakeTask();
154+
createTaskMock.mockImplementationOnce(async (_input, onTaskReady) => {
155+
const output = {
156+
task,
157+
workspace: null,
158+
provisioningError: "git clone failed",
159+
};
160+
onTaskReady?.(output);
161+
return { success: true, data: output };
162+
});
163+
164+
const { result } = renderHook(
165+
() => useStartTaskFromWorktree(MAIN_REPO_PATH),
166+
{
167+
wrapper,
168+
},
169+
);
170+
171+
await act(async () => {
172+
await result.current.startTask(BRANCH);
173+
});
174+
175+
expect(setFailedMock).toHaveBeenCalledWith(task.id, "git clone failed");
176+
expect(toastErrorMock).toHaveBeenCalledWith(
177+
"Failed to create workspace",
178+
"git clone failed",
179+
);
180+
// Provisioning failure is still a successful task creation: the create
181+
// event still fires and the adoptable-worktrees list still refetches.
182+
expect(trackMock).toHaveBeenCalled();
183+
expect(queryFilterMock).toHaveBeenCalledWith({
184+
mainRepoPath: MAIN_REPO_PATH,
185+
});
186+
await waitFor(() =>
187+
expect(result.current.startingBranches.has(BRANCH)).toBe(false),
188+
);
189+
});
190+
191+
it("toasts and clears the in-flight branch when createTask throws", async () => {
192+
createTaskMock.mockRejectedValueOnce(new Error("network down"));
193+
194+
const { result } = renderHook(
195+
() => useStartTaskFromWorktree(MAIN_REPO_PATH),
196+
{
197+
wrapper,
198+
},
199+
);
200+
201+
await act(async () => {
202+
await result.current.startTask(BRANCH);
203+
});
204+
205+
expect(toastErrorMock).toHaveBeenCalledWith(
206+
"Failed to start task from worktree",
207+
expect.any(Error),
208+
);
209+
await waitFor(() =>
210+
expect(result.current.startingBranches.has(BRANCH)).toBe(false),
211+
);
212+
});
213+
});

packages/workspace-server/src/services/workspace/workspace.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ import type {
6565
WorkspaceProvisioning,
6666
} from "./ports";
6767
import type {
68+
AdoptableWorktree,
6869
BranchChangedPayload,
6970
CheckWorktreeBranchInput,
7071
CheckWorktreeBranchOutput,
@@ -1481,7 +1482,7 @@ export class WorkspaceService extends TypedEventEmitter<WorkspaceServiceEvents>
14811482
*/
14821483
async listAdoptableWorktrees(
14831484
mainRepoPath: string,
1484-
): Promise<Array<{ worktreePath: string; branch: string }>> {
1485+
): Promise<AdoptableWorktree[]> {
14851486
const [linkedWorktrees, localStashPath] = await Promise.all([
14861487
listLinkedWorktrees(mainRepoPath),
14871488
this.getLocalWorktreePathIfExists(mainRepoPath),

0 commit comments

Comments
 (0)