From c78c485b51a4fa3876426eaaa513cd6121ee7a43 Mon Sep 17 00:00:00 2001 From: Marcel Poelker Date: Tue, 16 Jun 2026 15:19:44 +0200 Subject: [PATCH 1/3] fix(task-input): prefill repo from sidebar "+" in both local and cloud modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TASKS sidebar "+" button (and the command menu's "new task in folder") passes a `folderId` to the new-task screen, but `folderId` only ever prefilled the local directory selector — never the cloud repo dropdown. In Cloud mode the prefill was invisible, so clicking "+" on a repo left the cloud dropdown on the last-used (wrong) repo, and tasks could be started against the wrong repo. Make `folderId` drive both selectors. A registered folder already carries both `path` (local) and `remoteUrl` (the resolved `owner/repo` slug), so it is a single source of truth for the local directory and the cloud repo. New `useInitialRepoSelectionFromFolderId` hook + pure `resolveRepoSelectionForFolder` resolver: - always prefill the local directory from the folder path; - prefill the cloud repo when the folder's remote is a real `owner/repo` that is a connected GitHub integration; - keep the user's current workspace mode when it can represent the repo, and only switch (Cloud -> last-used local mode) when it can't (local-only repo while in Cloud). The mode switch uses the non-persisting setter so it does not overwrite the user's mode preference. The cloud/mode decision is gated on the integrations list having loaded, and runs once per `folderId` so it never clobbers a manual change. Replaces the directory-only `useInitialDirectoryFromFolderId` (removed). Call sites are unchanged, so every folder-scoped new-task entry point benefits. Known limits (out of scope): a cloud-only group with no registered local folder falls back to defaults; in Cloud mode, "+" on a repo whose org is not a connected integration intentionally switches to Local. Generated-By: PostHog Code Task-Id: 50f73c6f-d922-4d90-b547-a450102c770c --- .../task-detail/components/TaskInput.tsx | 23 +- .../useInitialDirectoryFromFolderId.test.ts | 89 ------ .../hooks/useInitialDirectoryFromFolderId.ts | 29 -- ...seInitialRepoSelectionFromFolderId.test.ts | 272 ++++++++++++++++++ .../useInitialRepoSelectionFromFolderId.ts | 152 ++++++++++ 5 files changed, 445 insertions(+), 120 deletions(-) delete mode 100644 packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.test.ts delete mode 100644 packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.ts create mode 100644 packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts create mode 100644 packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts diff --git a/packages/ui/src/features/task-detail/components/TaskInput.tsx b/packages/ui/src/features/task-detail/components/TaskInput.tsx index dc8848e288..0a6cd0a4bd 100644 --- a/packages/ui/src/features/task-detail/components/TaskInput.tsx +++ b/packages/ui/src/features/task-detail/components/TaskInput.tsx @@ -51,7 +51,7 @@ import { type AgentAdapter, useSettingsStore, } from "../../settings/settingsStore"; -import { useInitialDirectoryFromFolderId } from "../hooks/useInitialDirectoryFromFolderId"; +import { useInitialRepoSelectionFromFolderId } from "../hooks/useInitialRepoSelectionFromFolderId"; import { usePreviewConfig } from "../hooks/usePreviewConfig"; import { useTaskCreation } from "../hooks/useTaskCreation"; import { CloudGithubMissingNotice } from "./CloudGithubMissingNotice"; @@ -110,6 +110,7 @@ export function TaskInput({ ); const { setLastUsedLocalWorkspaceMode, + lastUsedLocalWorkspaceMode, lastUsedWorkspaceMode, setLastUsedWorkspaceMode, lastUsedAdapter, @@ -466,7 +467,25 @@ export function TaskInput({ setLastUsedCloudRepository, ]); - useInitialDirectoryFromFolderId(view.folderId, folders, setSelectedDirectory); + // Switch mode for a folder-scoped prefill ("+" in the sidebar) without persisting it as + // the user's mode preference. Marks the mode as resolved so the last-used resolver above + // doesn't override the explicit pick. + const switchWorkspaceModeForFolder = useCallback((mode: WorkspaceMode) => { + didResolveWorkspaceModeRef.current = true; + setWorkspaceModeState(mode); + }, []); + + useInitialRepoSelectionFromFolderId({ + folderId: view.folderId, + folders, + repositories, + reposLoaded: !isLoadingRepos && repositories.length > 0, + currentMode: workspaceMode, + lastUsedLocalMode: lastUsedLocalWorkspaceMode, + setSelectedDirectory, + setSelectedRepository, + switchWorkspaceMode: switchWorkspaceModeForFolder, + }); useEffect(() => { setCloudBranchSearchQuery(""); diff --git a/packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.test.ts b/packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.test.ts deleted file mode 100644 index 3f1b781aee..0000000000 --- a/packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { renderHook } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; -import type { RegisteredFolder } from "../../folders/types"; -import { useInitialDirectoryFromFolderId } from "./useInitialDirectoryFromFolderId"; - -const folder = (id: string, path: string): RegisteredFolder => ({ - id, - path, - name: id, - remoteUrl: null, - lastAccessed: "2026-05-21T00:00:00Z", - createdAt: "2026-05-21T00:00:00Z", -}); - -describe("useInitialDirectoryFromFolderId", () => { - it("syncs the directory to the folder matching folderId on first render", () => { - const setSelectedDirectory = vi.fn(); - renderHook(() => - useInitialDirectoryFromFolderId( - "a", - [folder("a", "/repos/a")], - setSelectedDirectory, - ), - ); - expect(setSelectedDirectory).toHaveBeenCalledExactlyOnceWith("/repos/a"); - }); - - it("waits for folders to load before syncing", () => { - const setSelectedDirectory = vi.fn(); - const { rerender } = renderHook( - ({ folders }: { folders: RegisteredFolder[] }) => - useInitialDirectoryFromFolderId("a", folders, setSelectedDirectory), - { initialProps: { folders: [] as RegisteredFolder[] } }, - ); - expect(setSelectedDirectory).not.toHaveBeenCalled(); - - rerender({ folders: [folder("a", "/repos/a")] }); - expect(setSelectedDirectory).toHaveBeenCalledExactlyOnceWith("/repos/a"); - }); - - it("does not re-sync when folders changes but folderId stays the same", () => { - const setSelectedDirectory = vi.fn(); - const { rerender } = renderHook( - ({ folders }: { folders: RegisteredFolder[] }) => - useInitialDirectoryFromFolderId("a", folders, setSelectedDirectory), - { initialProps: { folders: [folder("a", "/repos/a")] } }, - ); - expect(setSelectedDirectory).toHaveBeenCalledExactlyOnceWith("/repos/a"); - - // Simulate adding a folder (e.g. after the user picks one via "Open - // folder..."). The folders list changes but the user's pick must not be - // clobbered by re-syncing from the original folderId. - rerender({ - folders: [folder("a", "/repos/a"), folder("b", "/repos/picked")], - }); - expect(setSelectedDirectory).toHaveBeenCalledTimes(1); - }); - - it("re-syncs when folderId changes", () => { - const setSelectedDirectory = vi.fn(); - const folders = [folder("a", "/repos/a"), folder("b", "/repos/b")]; - const { rerender } = renderHook( - ({ folderId }: { folderId: string }) => - useInitialDirectoryFromFolderId( - folderId, - folders, - setSelectedDirectory, - ), - { initialProps: { folderId: "a" } }, - ); - expect(setSelectedDirectory).toHaveBeenLastCalledWith("/repos/a"); - - rerender({ folderId: "b" }); - expect(setSelectedDirectory).toHaveBeenLastCalledWith("/repos/b"); - expect(setSelectedDirectory).toHaveBeenCalledTimes(2); - }); - - it("does nothing when folderId is undefined", () => { - const setSelectedDirectory = vi.fn(); - renderHook(() => - useInitialDirectoryFromFolderId( - undefined, - [folder("a", "/repos/a")], - setSelectedDirectory, - ), - ); - expect(setSelectedDirectory).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.ts b/packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.ts deleted file mode 100644 index e39dd1574a..0000000000 --- a/packages/ui/src/features/task-detail/hooks/useInitialDirectoryFromFolderId.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { useEffect, useRef } from "react"; -import type { RegisteredFolder } from "../../folders/types"; - -/** - * Syncs `selectedDirectory` to the path of `folders[view.folderId]` once per - * folderId. The dependency on `folders` is required so the sync still fires - * when the folder list hasn't loaded yet on initial mount, but we must not - * re-sync on later `folders` refetches (e.g. after `addFolder`) — that would - * clobber a folder the user just picked via the file dialog. - */ -export function useInitialDirectoryFromFolderId( - folderId: string | undefined, - folders: RegisteredFolder[], - setSelectedDirectory: (path: string) => void, -) { - const lastInitializedRef = useRef(undefined); - useEffect(() => { - if (!folderId) { - lastInitializedRef.current = undefined; - return; - } - if (lastInitializedRef.current === folderId) return; - const folder = folders.find((f) => f.id === folderId); - if (folder) { - setSelectedDirectory(folder.path); - lastInitializedRef.current = folderId; - } - }, [folderId, folders, setSelectedDirectory]); -} diff --git a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts new file mode 100644 index 0000000000..27ba0dddd0 --- /dev/null +++ b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts @@ -0,0 +1,272 @@ +import type { WorkspaceMode } from "@posthog/shared"; +import { renderHook } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { RegisteredFolder } from "../../folders/types"; +import { + resolveRepoSelectionForFolder, + useInitialRepoSelectionFromFolderId, +} from "./useInitialRepoSelectionFromFolderId"; + +const folder = ( + id: string, + path: string, + remoteUrl: string | null = null, +): RegisteredFolder => ({ + id, + path, + name: id, + remoteUrl, + lastAccessed: "2026-05-21T00:00:00Z", + createdAt: "2026-05-21T00:00:00Z", +}); + +describe("resolveRepoSelectionForFolder", () => { + it("prefills both selectors for a cloud-capable folder and keeps cloud mode", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", "posthog/posthog"), + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + lastUsedLocalMode: "local", + }), + ).toEqual({ + directory: "/repos/a", + cloudRepository: "posthog/posthog", + nextMode: undefined, + }); + }); + + it("prefills the cloud repo while keeping local mode (no switch)", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", "posthog/posthog"), + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "local", + lastUsedLocalMode: "local", + }), + ).toEqual({ + directory: "/repos/a", + cloudRepository: "posthog/posthog", + nextMode: undefined, + }); + }); + + it("lower-cases the remote slug before matching", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", "PostHog/PostHog"), + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + lastUsedLocalMode: "local", + }).cloudRepository, + ).toBe("posthog/posthog"); + }); + + it("switches to the last-used local mode for a local-only folder while in cloud", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", null), + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + lastUsedLocalMode: "worktree", + }), + ).toEqual({ + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "worktree", + }); + }); + + it("treats a remote not in the integrations list as not cloud-capable", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", "acme/private"), + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + lastUsedLocalMode: "local", + }), + ).toEqual({ + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "local", + }); + }); + + it("ignores legacy single-segment remote values", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", "posthog"), + repositories: ["posthog"], + reposLoaded: true, + currentMode: "cloud", + lastUsedLocalMode: "local", + }), + ).toEqual({ + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "local", + }); + }); + + it("never switches mode before the integrations list has loaded", () => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", null), + repositories: [], + reposLoaded: false, + currentMode: "cloud", + lastUsedLocalMode: "local", + }), + ).toEqual({ + directory: "/repos/a", + cloudRepository: undefined, + nextMode: undefined, + }); + }); +}); + +type HookArgs = { + folderId: string | undefined; + folders: RegisteredFolder[]; + repositories: string[]; + reposLoaded: boolean; + currentMode: WorkspaceMode; +}; + +function renderRepoSelectionHook(initial: HookArgs) { + const setSelectedDirectory = vi.fn(); + const setSelectedRepository = vi.fn(); + const setWorkspaceMode = vi.fn(); + const utils = renderHook( + (props: HookArgs) => + useInitialRepoSelectionFromFolderId({ + folderId: props.folderId, + folders: props.folders, + repositories: props.repositories, + reposLoaded: props.reposLoaded, + currentMode: props.currentMode, + lastUsedLocalMode: "local", + setSelectedDirectory, + setSelectedRepository, + switchWorkspaceMode: setWorkspaceMode, + }), + { initialProps: initial }, + ); + return { + ...utils, + setSelectedDirectory, + setSelectedRepository, + setWorkspaceMode, + }; +} + +describe("useInitialRepoSelectionFromFolderId", () => { + it("syncs the directory immediately and the cloud repo once repos load", () => { + const { rerender, setSelectedDirectory, setSelectedRepository } = + renderRepoSelectionHook({ + folderId: "a", + folders: [folder("a", "/repos/a", "posthog/posthog")], + repositories: [], + reposLoaded: false, + currentMode: "cloud", + }); + // Directory applies right away, even before the integrations list loads. + expect(setSelectedDirectory).toHaveBeenCalledExactlyOnceWith("/repos/a"); + expect(setSelectedRepository).not.toHaveBeenCalled(); + + rerender({ + folderId: "a", + folders: [folder("a", "/repos/a", "posthog/posthog")], + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + }); + expect(setSelectedRepository).toHaveBeenCalledExactlyOnceWith( + "posthog/posthog", + ); + // Directory is not re-applied (once per folderId). + expect(setSelectedDirectory).toHaveBeenCalledTimes(1); + }); + + it("switches to local mode for a local-only folder once repos load", () => { + const { setWorkspaceMode, setSelectedRepository } = renderRepoSelectionHook( + { + folderId: "a", + folders: [folder("a", "/repos/a", null)], + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + }, + ); + expect(setWorkspaceMode).toHaveBeenCalledExactlyOnceWith("local"); + expect(setSelectedRepository).not.toHaveBeenCalled(); + }); + + it("does not re-sync when folders changes but folderId stays the same", () => { + const { rerender, setSelectedDirectory } = renderRepoSelectionHook({ + folderId: "a", + folders: [folder("a", "/repos/a")], + repositories: [], + reposLoaded: false, + currentMode: "local", + }); + expect(setSelectedDirectory).toHaveBeenCalledExactlyOnceWith("/repos/a"); + + // Simulate the user picking a different folder afterward; the changed list must + // not clobber their pick by re-syncing from the original folderId. + rerender({ + folderId: "a", + folders: [folder("a", "/repos/a"), folder("b", "/repos/picked")], + repositories: [], + reposLoaded: false, + currentMode: "local", + }); + expect(setSelectedDirectory).toHaveBeenCalledTimes(1); + }); + + it("re-syncs when folderId changes", () => { + const folders = [ + folder("a", "/repos/a", "posthog/a"), + folder("b", "/repos/b", "posthog/b"), + ]; + const { rerender, setSelectedDirectory, setSelectedRepository } = + renderRepoSelectionHook({ + folderId: "a", + folders, + repositories: ["posthog/a", "posthog/b"], + reposLoaded: true, + currentMode: "cloud", + }); + expect(setSelectedDirectory).toHaveBeenLastCalledWith("/repos/a"); + expect(setSelectedRepository).toHaveBeenLastCalledWith("posthog/a"); + + rerender({ + folderId: "b", + folders, + repositories: ["posthog/a", "posthog/b"], + reposLoaded: true, + currentMode: "cloud", + }); + expect(setSelectedDirectory).toHaveBeenLastCalledWith("/repos/b"); + expect(setSelectedRepository).toHaveBeenLastCalledWith("posthog/b"); + }); + + it("does nothing when folderId is undefined", () => { + const { setSelectedDirectory, setSelectedRepository, setWorkspaceMode } = + renderRepoSelectionHook({ + folderId: undefined, + folders: [folder("a", "/repos/a", "posthog/posthog")], + repositories: ["posthog/posthog"], + reposLoaded: true, + currentMode: "cloud", + }); + expect(setSelectedDirectory).not.toHaveBeenCalled(); + expect(setSelectedRepository).not.toHaveBeenCalled(); + expect(setWorkspaceMode).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts new file mode 100644 index 0000000000..711494c18b --- /dev/null +++ b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts @@ -0,0 +1,152 @@ +import { parseRepository, type WorkspaceMode } from "@posthog/shared"; +import { useEffect, useRef } from "react"; +import type { RegisteredFolder } from "../../folders/types"; + +export interface RepoSelectionInput { + folder: RegisteredFolder; + /** Lower-cased `owner/repo` slugs the user can use in cloud mode. */ + repositories: string[]; + /** Whether the integrations list has finished loading (gate the mode switch). */ + reposLoaded: boolean; + currentMode: WorkspaceMode; + /** Mode to fall back to when leaving cloud (local or worktree). */ + lastUsedLocalMode: WorkspaceMode; +} + +export interface RepoSelection { + /** Local directory to select (always the folder's path). */ + directory: string; + /** Cloud `owner/repo` slug to select, or undefined to leave the cloud pick as-is. */ + cloudRepository?: string; + /** Workspace mode to switch to, or undefined to keep the current mode. */ + nextMode?: WorkspaceMode; +} + +/** + * Pure resolver: given the folder a user picked (e.g. via the sidebar "+"), decide + * what to select in both the local-directory and cloud-repo pickers, and whether the + * workspace mode must change. + * + * Rules (see plan): prefill both selectors; keep the current mode when it can represent + * the repo; only switch when it can't — i.e. you're in cloud but the repo has no cloud + * counterpart (no remote slug, or the slug isn't a connected integration), in which case + * fall back to the last-used local mode. + */ +export function resolveRepoSelectionForFolder({ + folder, + repositories, + reposLoaded, + currentMode, + lastUsedLocalMode, +}: RepoSelectionInput): RepoSelection { + const slug = folder.remoteUrl?.toLowerCase(); + // A folder is cloud-capable only when its remote is a real `owner/repo` (guards against + // legacy single-segment values) AND that repo is one of the user's connected integrations. + const cloudRepository = + slug && parseRepository(slug) !== null && repositories.includes(slug) + ? slug + : undefined; + + const selection: RepoSelection = { + directory: folder.path, + cloudRepository, + }; + + // Only decide the mode once the integrations list has loaded, so we never switch out + // of cloud while the repo list is still in flight (it would look "not cloud-capable"). + if (reposLoaded && currentMode === "cloud" && !cloudRepository) { + selection.nextMode = lastUsedLocalMode; + } + + return selection; +} + +export interface UseInitialRepoSelectionParams { + folderId: string | undefined; + folders: RegisteredFolder[]; + /** Lower-cased `owner/repo` slugs the user can use in cloud mode. */ + repositories: string[]; + /** Whether the integrations list has finished loading (gate the mode switch). */ + reposLoaded: boolean; + currentMode: WorkspaceMode; + /** Mode to fall back to when leaving cloud (local or worktree). */ + lastUsedLocalMode: WorkspaceMode; + setSelectedDirectory: (path: string) => void; + setSelectedRepository: (repo: string) => void; + /** Switches the workspace mode (without persisting it as the user's preference). */ + switchWorkspaceMode: (mode: WorkspaceMode) => void; +} + +/** + * Applies {@link resolveRepoSelectionForFolder} to the live pickers when a `folderId` + * prefill arrives, syncing both the local directory and the cloud repo and switching + * mode when required. Runs once per `folderId` (guarded by refs) so it never clobbers a + * repo/mode the user changed afterward, and re-runs when `folderId` changes. + * + * The dependency on `folders` / `repositories` lets the sync still fire when those lists + * load after the initial mount. + */ +export function useInitialRepoSelectionFromFolderId({ + folderId, + folders, + repositories, + reposLoaded, + currentMode, + lastUsedLocalMode, + setSelectedDirectory, + setSelectedRepository, + switchWorkspaceMode, +}: UseInitialRepoSelectionParams) { + // Two guards: the local directory syncs immediately (once the folder loads), while the + // cloud repo + mode decision waits for the integrations list — so it isn't marked "done" + // before it can tell whether the repo is cloud-capable. + const dirInitRef = useRef(undefined); + const repoModeInitRef = useRef(undefined); + // Read the current mode through a ref so it doesn't retrigger the effect (which would + // re-run the once-per-folderId logic after we change the mode ourselves). + const currentModeRef = useRef(currentMode); + currentModeRef.current = currentMode; + + useEffect(() => { + if (!folderId) { + dirInitRef.current = undefined; + repoModeInitRef.current = undefined; + return; + } + const folder = folders.find((f) => f.id === folderId); + if (!folder) return; + + const selection = resolveRepoSelectionForFolder({ + folder, + repositories, + reposLoaded, + currentMode: currentModeRef.current, + lastUsedLocalMode, + }); + + if (dirInitRef.current !== folderId) { + setSelectedDirectory(selection.directory); + dirInitRef.current = folderId; + } + + // Defer the cloud/mode decision until the integrations list has loaded. + if (reposLoaded && repoModeInitRef.current !== folderId) { + if (selection.cloudRepository) { + setSelectedRepository(selection.cloudRepository); + } + if (selection.nextMode && selection.nextMode !== currentModeRef.current) { + switchWorkspaceMode(selection.nextMode); + } + repoModeInitRef.current = folderId; + } + }, [ + folderId, + folders, + repositories, + reposLoaded, + lastUsedLocalMode, + setSelectedDirectory, + setSelectedRepository, + switchWorkspaceMode, + ]); +} From 3fccd62c58355423c78d7cf5c5d0493031224383 Mon Sep 17 00:00:00 2001 From: Marcel Poelker Date: Tue, 16 Jun 2026 22:50:50 +0200 Subject: [PATCH 2/3] fix(task-input): distinguish settled-empty from loading repos in prefill guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR review feedback on the sidebar "+" repo prefill. P1 (corrected): the cloud/mode readiness guard previously required `repositories.length > 0`, which never becomes true for users in cloud mode with zero connected GitHub integrations — so clicking "+" on a local-only folder silently did nothing for them. The reviewer's proposed fix (`!isLoadingRepos` alone) reintroduces the transient-empty race the length check guarded against (the window where loading has cleared but per-installation repo queries haven't populated yet), wrongly switching users with integrations to Local on the common path. Instead, extract a pure `areReposReady({ isLoadingRepos, repositoriesCount, hasGithubIntegration })` helper: ready when not loading AND (repos present OR no GitHub integration). Fixes the no-integration cohort while preserving the transient-window guard for the common path. P2: fold the resolver cases into an `it.each` table (per CLAUDE.md test guidance), add the settled-empty `repositories` row, and add a parameterised `areReposReady` table covering the transient-window and no-integration cases. Generated-By: PostHog Code Task-Id: 50f73c6f-d922-4d90-b547-a450102c770c --- .../task-detail/components/TaskInput.tsx | 11 +- ...seInitialRepoSelectionFromFolderId.test.ts | 227 ++++++++++++------ .../useInitialRepoSelectionFromFolderId.ts | 26 ++ 3 files changed, 185 insertions(+), 79 deletions(-) diff --git a/packages/ui/src/features/task-detail/components/TaskInput.tsx b/packages/ui/src/features/task-detail/components/TaskInput.tsx index 0a6cd0a4bd..2abbf366d6 100644 --- a/packages/ui/src/features/task-detail/components/TaskInput.tsx +++ b/packages/ui/src/features/task-detail/components/TaskInput.tsx @@ -51,7 +51,10 @@ import { type AgentAdapter, useSettingsStore, } from "../../settings/settingsStore"; -import { useInitialRepoSelectionFromFolderId } from "../hooks/useInitialRepoSelectionFromFolderId"; +import { + areReposReady, + useInitialRepoSelectionFromFolderId, +} from "../hooks/useInitialRepoSelectionFromFolderId"; import { usePreviewConfig } from "../hooks/usePreviewConfig"; import { useTaskCreation } from "../hooks/useTaskCreation"; import { CloudGithubMissingNotice } from "./CloudGithubMissingNotice"; @@ -479,7 +482,11 @@ export function TaskInput({ folderId: view.folderId, folders, repositories, - reposLoaded: !isLoadingRepos && repositories.length > 0, + reposLoaded: areReposReady({ + isLoadingRepos, + repositoriesCount: repositories.length, + hasGithubIntegration, + }), currentMode: workspaceMode, lastUsedLocalMode: lastUsedLocalWorkspaceMode, setSelectedDirectory, diff --git a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts index 27ba0dddd0..07196c9238 100644 --- a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts +++ b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts @@ -3,7 +3,10 @@ import { renderHook } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import type { RegisteredFolder } from "../../folders/types"; import { + areReposReady, resolveRepoSelectionForFolder, + type RepoSelection, + type RepoSelectionInput, useInitialRepoSelectionFromFolderId, } from "./useInitialRepoSelectionFromFolderId"; @@ -21,115 +24,185 @@ const folder = ( }); describe("resolveRepoSelectionForFolder", () => { - it("prefills both selectors for a cloud-capable folder and keeps cloud mode", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", "posthog/posthog"), + it.each<{ + name: string; + input: Omit & { remoteUrl: string | null }; + expected: RepoSelection; + }>([ + { + name: "cloud-capable folder in cloud mode: prefill cloud repo, keep cloud", + input: { + remoteUrl: "posthog/posthog", repositories: ["posthog/posthog"], reposLoaded: true, currentMode: "cloud", lastUsedLocalMode: "local", - }), - ).toEqual({ - directory: "/repos/a", - cloudRepository: "posthog/posthog", - nextMode: undefined, - }); - }); - - it("prefills the cloud repo while keeping local mode (no switch)", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", "posthog/posthog"), + }, + expected: { + directory: "/repos/a", + cloudRepository: "posthog/posthog", + nextMode: undefined, + }, + }, + { + name: "cloud-capable folder in local mode: seed cloud repo, keep local", + input: { + remoteUrl: "posthog/posthog", repositories: ["posthog/posthog"], reposLoaded: true, currentMode: "local", lastUsedLocalMode: "local", - }), - ).toEqual({ - directory: "/repos/a", - cloudRepository: "posthog/posthog", - nextMode: undefined, - }); - }); - - it("lower-cases the remote slug before matching", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", "PostHog/PostHog"), + }, + expected: { + directory: "/repos/a", + cloudRepository: "posthog/posthog", + nextMode: undefined, + }, + }, + { + name: "lower-cases the remote slug before matching", + input: { + remoteUrl: "PostHog/PostHog", repositories: ["posthog/posthog"], reposLoaded: true, currentMode: "cloud", lastUsedLocalMode: "local", - }).cloudRepository, - ).toBe("posthog/posthog"); - }); - - it("switches to the last-used local mode for a local-only folder while in cloud", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", null), + }, + expected: { + directory: "/repos/a", + cloudRepository: "posthog/posthog", + nextMode: undefined, + }, + }, + { + name: "local-only folder in cloud mode: switch to last-used local mode", + input: { + remoteUrl: null, repositories: ["posthog/posthog"], reposLoaded: true, currentMode: "cloud", lastUsedLocalMode: "worktree", - }), - ).toEqual({ - directory: "/repos/a", - cloudRepository: undefined, - nextMode: "worktree", - }); - }); - - it("treats a remote not in the integrations list as not cloud-capable", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", "acme/private"), + }, + expected: { + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "worktree", + }, + }, + { + name: "remote not in the integrations list: not cloud-capable, switch to local", + input: { + remoteUrl: "acme/private", repositories: ["posthog/posthog"], reposLoaded: true, currentMode: "cloud", lastUsedLocalMode: "local", - }), - ).toEqual({ - directory: "/repos/a", - cloudRepository: undefined, - nextMode: "local", - }); - }); - - it("ignores legacy single-segment remote values", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", "posthog"), + }, + expected: { + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "local", + }, + }, + { + name: "ignores legacy single-segment remote values", + input: { + remoteUrl: "posthog", repositories: ["posthog"], reposLoaded: true, currentMode: "cloud", lastUsedLocalMode: "local", - }), - ).toEqual({ - directory: "/repos/a", - cloudRepository: undefined, - nextMode: "local", - }); - }); - - it("never switches mode before the integrations list has loaded", () => { - expect( - resolveRepoSelectionForFolder({ - folder: folder("a", "/repos/a", null), + }, + expected: { + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "local", + }, + }, + { + name: "loaded with empty repositories (no integration): switch to local in cloud", + input: { + remoteUrl: "posthog/posthog", + repositories: [], + reposLoaded: true, + currentMode: "cloud", + lastUsedLocalMode: "local", + }, + expected: { + directory: "/repos/a", + cloudRepository: undefined, + nextMode: "local", + }, + }, + { + name: "not loaded: never switch mode (await the integrations list)", + input: { + remoteUrl: null, repositories: [], reposLoaded: false, currentMode: "cloud", lastUsedLocalMode: "local", + }, + expected: { + directory: "/repos/a", + cloudRepository: undefined, + nextMode: undefined, + }, + }, + ])("$name", ({ input: { remoteUrl, ...rest }, expected }) => { + expect( + resolveRepoSelectionForFolder({ + folder: folder("a", "/repos/a", remoteUrl), + ...rest, }), - ).toEqual({ - directory: "/repos/a", - cloudRepository: undefined, - nextMode: undefined, - }); + ).toEqual(expected); }); }); +describe("areReposReady", () => { + it.each([ + { + name: "still loading: not ready", + isLoadingRepos: true, + repositoriesCount: 5, + hasGithubIntegration: true, + expected: false, + }, + { + name: "loaded with repos: ready", + isLoadingRepos: false, + repositoriesCount: 5, + hasGithubIntegration: true, + expected: true, + }, + { + name: "loaded, empty, no integration (settled empty): ready", + isLoadingRepos: false, + repositoriesCount: 0, + hasGithubIntegration: false, + expected: true, + }, + { + name: "loaded, empty, but has integration (transient window): not ready", + isLoadingRepos: false, + repositoriesCount: 0, + hasGithubIntegration: true, + expected: false, + }, + ])( + "$name", + ({ isLoadingRepos, repositoriesCount, hasGithubIntegration, expected }) => { + expect( + areReposReady({ + isLoadingRepos, + repositoriesCount, + hasGithubIntegration, + }), + ).toBe(expected); + }, + ); +}); + type HookArgs = { folderId: string | undefined; folders: RegisteredFolder[]; diff --git a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts index 711494c18b..1376843eea 100644 --- a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts +++ b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.ts @@ -2,6 +2,32 @@ import { parseRepository, type WorkspaceMode } from "@posthog/shared"; import { useEffect, useRef } from "react"; import type { RegisteredFolder } from "../../folders/types"; +export interface ReposReadyInput { + /** True while the integrations + per-installation repo queries are in flight. */ + isLoadingRepos: boolean; + /** Number of connectable `owner/repo` slugs currently known. */ + repositoriesCount: number; + /** Whether the user has any connected GitHub integration at all. */ + hasGithubIntegration: boolean; +} + +/** + * Whether the cloud-repo list has *settled* — i.e. it's safe to conclude a folder is or + * isn't cloud-capable. Distinguishes "settled empty because the user has no GitHub + * integration" (ready) from "transiently empty while per-installation repo queries are + * still producing data" (not ready). The latter window is real: `isLoadingRepos` can flip + * false before `repositories` populates (see the validation effect in TaskInput), so + * `!isLoadingRepos` alone would mis-judge a cloud-capable repo during that gap. + */ +export function areReposReady({ + isLoadingRepos, + repositoriesCount, + hasGithubIntegration, +}: ReposReadyInput): boolean { + if (isLoadingRepos) return false; + return repositoriesCount > 0 || !hasGithubIntegration; +} + export interface RepoSelectionInput { folder: RegisteredFolder; /** Lower-cased `owner/repo` slugs the user can use in cloud mode. */ From 2ffebabca18f9b979ae9c4de4d36d8d2349ea533 Mon Sep 17 00:00:00 2001 From: Marcel Poelker Date: Wed, 17 Jun 2026 09:54:50 +0200 Subject: [PATCH 3/3] CI feedback --- ...seInitialRepoSelectionFromFolderId.test.ts | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts index 07196c9238..dc167f4c52 100644 --- a/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts +++ b/packages/ui/src/features/task-detail/hooks/useInitialRepoSelectionFromFolderId.test.ts @@ -4,9 +4,9 @@ import { describe, expect, it, vi } from "vitest"; import type { RegisteredFolder } from "../../folders/types"; import { areReposReady, - resolveRepoSelectionForFolder, type RepoSelection, type RepoSelectionInput, + resolveRepoSelectionForFolder, useInitialRepoSelectionFromFolderId, } from "./useInitialRepoSelectionFromFolderId"; @@ -280,6 +280,28 @@ describe("useInitialRepoSelectionFromFolderId", () => { expect(setSelectedRepository).not.toHaveBeenCalled(); }); + it("waits for folders to load before syncing the directory", () => { + const { rerender, setSelectedDirectory } = renderRepoSelectionHook({ + folderId: "a", + folders: [], + repositories: [], + reposLoaded: false, + currentMode: "local", + }); + // The target folder isn't in the list yet: bail without marking the sync done. + expect(setSelectedDirectory).not.toHaveBeenCalled(); + + // Once folders load, the prefill fires (the guard ref was left unset). + rerender({ + folderId: "a", + folders: [folder("a", "/repos/a")], + repositories: [], + reposLoaded: false, + currentMode: "local", + }); + expect(setSelectedDirectory).toHaveBeenCalledExactlyOnceWith("/repos/a"); + }); + it("does not re-sync when folders changes but folderId stays the same", () => { const { rerender, setSelectedDirectory } = renderRepoSelectionHook({ folderId: "a",