From 36bdf631acafcd33b1a3010802a8d76bd811ff2d Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 2 Jun 2026 00:19:24 -0700 Subject: [PATCH 1/2] feat(import): initialize git for non-git folders on v2 folder import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Importing a folder that wasn't a git repo yet dead-ended on "Not a git repository". v2 now detects the case and offers to `git init` the folder (with user confirmation), then imports it as a local-only project — no remote required. Likely fixes #5033. Server (host-service): - resolve-repo: add tryRevParseGitRoot (non-throwing) and initLocalRepoInPlace (git init in place + empty initial commit; idempotent; nested dirs resolve to parent root). Export validateDirectoryPath; revParseGitRoot now wraps the non-throwing variant. - project.findByPath returns an additive optional `needsGitInit` instead of throwing on a non-git folder. - create importLocal gains `initIfNeeded`; createFromImportLocal inits in place only when set. Cloud path unchanged (local-only => no clone URL). Desktop: - New imperative git-init-confirm store + GitInitConfirmDialog, mounted once via AddRepositoryModals. useFolderFirstImport branches on needsGitInit, confirms, then creates with initIfNeeded. All 5 hook consumers untouched. Tests: 6 new resolve-repo cases; 2 new hook cases (confirm/cancel). --- .../AddRepositoryModals.tsx | 26 +- .../GitInitConfirmDialog.tsx | 52 ++++ .../components/GitInitConfirmDialog/index.ts | 1 + .../useFolderFirstImport.test.ts | 68 ++++- .../useFolderFirstImport.ts | 17 ++ .../src/renderer/stores/git-init-confirm.ts | 47 ++++ .../src/trpc/router/project/handlers.ts | 23 +- .../src/trpc/router/project/project.ts | 24 +- .../router/project/utils/resolve-repo.test.ts | 120 +++++++- .../trpc/router/project/utils/resolve-repo.ts | 54 +++- ...60601-v2-import-git-init-non-git-folder.md | 262 ++++++++++++++++++ 11 files changed, 672 insertions(+), 22 deletions(-) create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/index.ts create mode 100644 apps/desktop/src/renderer/stores/git-init-confirm.ts create mode 100644 plans/20260601-v2-import-git-init-non-git-folder.md diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx index d0a58650ff6..3002126df1f 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx @@ -4,6 +4,7 @@ import { useCloseAddRepositoryModal, useResolveNewProjectModal, } from "renderer/stores/add-repository-modal"; +import { GitInitConfirmDialog } from "./components/GitInitConfirmDialog"; import { NewProjectModal } from "./components/NewProjectModal"; export function AddRepositoryModals() { @@ -12,16 +13,19 @@ export function AddRepositoryModals() { const resolveNewProject = useResolveNewProjectModal(); return ( - { - if (!open) close(); - }} - onSuccess={(result) => { - toast.success("Project created."); - resolveNewProject({ projectId: result.projectId }); - }} - onError={(message) => toast.error(`Create failed: ${message}`)} - /> + <> + { + if (!open) close(); + }} + onSuccess={(result) => { + toast.success("Project created."); + resolveNewProject({ projectId: result.projectId }); + }} + onError={(message) => toast.error(`Create failed: ${message}`)} + /> + + ); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx new file mode 100644 index 00000000000..16b0c868232 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx @@ -0,0 +1,52 @@ +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@superset/ui/alert-dialog"; +import { Button } from "@superset/ui/button"; +import { useGitInitConfirmStore } from "renderer/stores/git-init-confirm"; + +/** + * Confirms initializing git in a folder the user picked to import that isn't a + * git repo yet. Driven imperatively by `useGitInitConfirmStore.request()` from + * the folder-first import flow; mounted once via AddRepositoryModals. + */ +export function GitInitConfirmDialog() { + const isOpen = useGitInitConfirmStore((s) => s.isOpen); + const repoPath = useGitInitConfirmStore((s) => s.repoPath); + const resolve = useGitInitConfirmStore((s) => s.resolve); + + const folderName = repoPath?.split("/").pop() ?? repoPath ?? "this folder"; + + return ( + { + if (!open) resolve(false); + }} + > + + + Initialize git repository? + +

+ + {folderName} + {" "} + isn't a git repository yet. Initialize git here and import it? +

+ + + + + + + + + ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/index.ts new file mode 100644 index 00000000000..ce4c6ae35c7 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/index.ts @@ -0,0 +1 @@ +export { GitInitConfirmDialog } from "./GitInitConfirmDialog"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.test.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.test.ts index c8150c176ad..19682b902fb 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.test.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.test.ts @@ -15,10 +15,16 @@ const selectDirectoryMock = mock(async () => ({ canceled: false, path: repoPath, })); -const findByPathMock = mock(async () => ({ - candidates: [] as { id: string; name: string }[], - cloudErrors: [] as (typeof cloudError)[], -})); +const findByPathMock = mock( + async (): Promise<{ + candidates: { id: string; name: string }[]; + cloudErrors: (typeof cloudError)[]; + needsGitInit?: boolean; + }> => ({ + candidates: [], + cloudErrors: [], + }), +); const setupMock = mock(async () => setupResult); const createMock = mock(async () => ({ projectId: "created-project", @@ -26,6 +32,7 @@ const createMock = mock(async () => ({ mainWorkspaceId: "workspace-created", })); const finalizeSetupMock = mock(() => undefined); +const requestGitInitMock = mock(async () => false); mock.module("react", () => ({ useCallback: unknown>(callback: T) => @@ -63,6 +70,10 @@ mock.module( }), ); +mock.module("renderer/stores/git-init-confirm", () => ({ + useRequestGitInitConfirm: () => requestGitInitMock, +})); + const { useFolderFirstImport } = await import("./useFolderFirstImport"); describe("useFolderFirstImport", () => { @@ -73,10 +84,12 @@ describe("useFolderFirstImport", () => { setupMock, createMock, finalizeSetupMock, + requestGitInitMock, ]) { fn.mockClear(); } findByPathMock.mockResolvedValue({ candidates: [], cloudErrors: [] }); + requestGitInitMock.mockResolvedValue(false); }); it("reports cloud lookup errors instead of creating a duplicate local import when no candidates exist", async () => { @@ -97,4 +110,51 @@ describe("useFolderFirstImport", () => { expect(setupMock).not.toHaveBeenCalled(); expect(finalizeSetupMock).not.toHaveBeenCalled(); }); + + it("imports with init after the user confirms a non-git folder", async () => { + findByPathMock.mockResolvedValue({ + candidates: [], + cloudErrors: [], + needsGitInit: true, + }); + requestGitInitMock.mockResolvedValue(true); + const onError = mock(() => undefined); + + const result = await useFolderFirstImport({ onError }).start(); + + expect(requestGitInitMock).toHaveBeenCalledWith(repoPath); + expect(createMock).toHaveBeenCalledWith({ + name: "octocat", + mode: { kind: "importLocal", repoPath, initIfNeeded: true }, + }); + expect(finalizeSetupMock).toHaveBeenCalledWith(hostUrl, { + projectId: "created-project", + repoPath, + mainWorkspaceId: "workspace-created", + }); + expect(result).toEqual({ + projectId: "created-project", + repoPath, + mainWorkspaceId: "workspace-created", + }); + expect(onError).not.toHaveBeenCalled(); + }); + + it("does nothing when the user cancels the git-init confirmation", async () => { + findByPathMock.mockResolvedValue({ + candidates: [], + cloudErrors: [], + needsGitInit: true, + }); + requestGitInitMock.mockResolvedValue(false); + const onError = mock(() => undefined); + + const result = await useFolderFirstImport({ onError }).start(); + + expect(result).toBeNull(); + expect(requestGitInitMock).toHaveBeenCalledWith(repoPath); + expect(createMock).not.toHaveBeenCalled(); + expect(finalizeSetupMock).not.toHaveBeenCalled(); + expect(onError).not.toHaveBeenCalled(); + }); }); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts index 61d73e5b319..764d63818e5 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts @@ -8,6 +8,7 @@ import { useFinalizeProjectSetup, } from "renderer/react-query/projects"; import { useLocalHostService } from "renderer/routes/_authenticated/providers/LocalHostServiceProvider"; +import { useRequestGitInitConfirm } from "renderer/stores/git-init-confirm"; export interface UseFolderFirstImportResult { start: () => Promise; @@ -26,6 +27,7 @@ export function useFolderFirstImport(options?: { const { activeHostUrl } = hostService; const finalizeSetup = useFinalizeProjectSetup(); const selectDirectory = electronTrpc.window.selectDirectory.useMutation(); + const requestGitInit = useRequestGitInitConfirm(); const { onError, onMultipleProjects } = options ?? {}; const start = useCallback(async (): Promise => { @@ -54,6 +56,20 @@ export function useFolderFirstImport(options?: { let candidates: MatchingProject[]; try { const response = await client.project.findByPath.query({ repoPath }); + + // Folder isn't a git repo yet: offer to `git init` it, then import + // via the create path with init enabled. + if ("needsGitInit" in response && response.needsGitInit) { + const confirmed = await requestGitInit(repoPath); + if (!confirmed) return null; + const result = await client.project.create.mutate({ + name: getBaseName(repoPath), + mode: { kind: "importLocal", repoPath, initIfNeeded: true }, + }); + finalizeSetup(activeHostUrl, result); + return result; + } + candidates = response.candidates; if (candidates.length === 0 && response.cloudErrors.length > 0) { const first = response.cloudErrors[0]; @@ -107,6 +123,7 @@ export function useFolderFirstImport(options?: { hostService, onError, onMultipleProjects, + requestGitInit, selectDirectory, ]); diff --git a/apps/desktop/src/renderer/stores/git-init-confirm.ts b/apps/desktop/src/renderer/stores/git-init-confirm.ts new file mode 100644 index 00000000000..c6bba89019d --- /dev/null +++ b/apps/desktop/src/renderer/stores/git-init-confirm.ts @@ -0,0 +1,47 @@ +import { create } from "zustand"; +import { devtools } from "zustand/middleware"; + +interface GitInitConfirmState { + isOpen: boolean; + repoPath: string | null; + /** + * Opens the confirm dialog and resolves `true` if the user agrees to + * `git init` the folder, `false` if they cancel/dismiss. Only one request + * can be in flight at a time — a second call resolves the prior request to + * `false` before opening fresh. Safe today because there is a single global + * dialog instance (rendered by AddRepositoryModals). + */ + request: (repoPath: string) => Promise; + resolve: (confirmed: boolean) => void; +} + +// Module-level resolver so the pending promise isn't stored in zustand state. +// The store drives the dialog's open/close UI; the resolver bridges the +// imperative request() call back to its caller. +let pendingResolve: ((confirmed: boolean) => void) | null = null; + +export const useGitInitConfirmStore = create()( + devtools( + (set) => ({ + isOpen: false, + repoPath: null, + request: (repoPath) => { + pendingResolve?.(false); + return new Promise((resolve) => { + pendingResolve = resolve; + set({ isOpen: true, repoPath }); + }); + }, + resolve: (confirmed) => { + const resolve = pendingResolve; + pendingResolve = null; + set({ isOpen: false, repoPath: null }); + resolve?.(confirmed); + }, + }), + { name: "git-init-confirm" }, + ), +); + +export const useRequestGitInitConfirm = () => + useGitInitConfirmStore((state) => state.request); diff --git a/packages/host-service/src/trpc/router/project/handlers.ts b/packages/host-service/src/trpc/router/project/handlers.ts index 6e94b7b92c2..3a13e285690 100644 --- a/packages/host-service/src/trpc/router/project/handlers.ts +++ b/packages/host-service/src/trpc/router/project/handlers.ts @@ -10,8 +10,10 @@ import { cloneRepoInto, cloneTemplateInto, initEmptyRepo, + initLocalRepoInPlace, type ResolvedRepo, resolveLocalRepo, + tryRevParseGitRoot, } from "./utils/resolve-repo"; import { templateUrlFor } from "./utils/templates"; @@ -196,11 +198,28 @@ export async function createFromClone( }); } +/** + * Resolve an existing repo, or — when `initIfNeeded` and the folder isn't a git + * repo yet — `git init` it in place first. The init branch only runs after the + * UI has confirmed intent with the user. + */ +async function resolveOrInitLocalRepo( + repoPath: string, + initIfNeeded: boolean, +): Promise { + if (!initIfNeeded) return resolveLocalRepo(repoPath); + const root = await tryRevParseGitRoot(repoPath); + return root ? resolveLocalRepo(root) : initLocalRepoInPlace(repoPath); +} + export async function createFromImportLocal( ctx: HostServiceContext, - args: { name: string; repoPath: string }, + args: { name: string; repoPath: string; initIfNeeded?: boolean }, ): Promise { - const resolved = await resolveLocalRepo(args.repoPath); + const resolved = await resolveOrInitLocalRepo( + args.repoPath, + args.initIfNeeded ?? false, + ); return persistFromResolved(ctx, { name: args.name, resolved, diff --git a/packages/host-service/src/trpc/router/project/project.ts b/packages/host-service/src/trpc/router/project/project.ts index 668e75e32cd..cba5f75fa6c 100644 --- a/packages/host-service/src/trpc/router/project/project.ts +++ b/packages/host-service/src/trpc/router/project/project.ts @@ -25,6 +25,8 @@ import { type ResolvedRepo, resolveLocalRepo, resolveMatchingSlug, + tryRevParseGitRoot, + validateDirectoryPath, } from "./utils/resolve-repo"; export const projectRouter = router({ @@ -162,7 +164,22 @@ export const projectRouter = router({ }), ) .query(async ({ ctx, input }) => { - const resolved = await resolveLocalRepo(input.repoPath); + // Detect "folder isn't a git repo yet" without throwing, so the import + // UI can offer to `git init` it (create importLocal + initIfNeeded) + // instead of dead-ending on a BAD_REQUEST. Additive optional field — + // repo paths never carry needsGitInit, so existing callers are + // unaffected. + const root = await tryRevParseGitRoot(input.repoPath); + if (root === null) { + validateDirectoryPath(input.repoPath, "Path"); // 400 on missing / not-a-dir + return { + candidates: [], + cloudErrors: [] as { url: string; message: string }[], + needsGitInit: true as const, + }; + } + + const resolved = await resolveLocalRepo(root); const gitRoot = resolved.repoPath; const expectedParsed = @@ -391,6 +408,10 @@ export const projectRouter = router({ z.object({ kind: z.literal("importLocal"), repoPath: z.string().min(1), + // When set, `git init` a non-git folder in place before + // importing. The UI sets this only after confirming intent + // with the user (see findByPath's needsGitInit). + initIfNeeded: z.boolean().optional().default(false), }), z.object({ kind: z.literal("template"), @@ -423,6 +444,7 @@ export const projectRouter = router({ return createFromImportLocal(ctx, { name: input.name, repoPath: input.mode.repoPath, + initIfNeeded: input.mode.initIfNeeded, }); } }), diff --git a/packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts b/packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts index e18feec44a7..508395aae58 100644 --- a/packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts +++ b/packages/host-service/src/trpc/router/project/utils/resolve-repo.test.ts @@ -1,4 +1,12 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, +} from "bun:test"; import { existsSync, mkdirSync, @@ -10,7 +18,11 @@ import { import { tmpdir } from "node:os"; import { join } from "node:path"; import simpleGit, { type SimpleGit } from "simple-git"; -import { cloneRepoInto, resolveLocalRepo } from "./resolve-repo"; +import { + cloneRepoInto, + initLocalRepoInPlace, + resolveLocalRepo, +} from "./resolve-repo"; /** * Integration tests against real on-disk git repositories. The point is @@ -190,6 +202,110 @@ describe("resolveLocalRepo", () => { }); }); +// ── initLocalRepoInPlace ────────────────────────────────────────── + +describe("initLocalRepoInPlace", () => { + // The in-place initial commit happens inside the call, so there's no + // window to set local git identity. Provide one via env (git honors these + // without config) so the commit succeeds under CI's identity-less env. + const savedEnv: Record = {}; + const identity = { + GIT_AUTHOR_NAME: "test", + GIT_AUTHOR_EMAIL: "test@example.com", + GIT_COMMITTER_NAME: "test", + GIT_COMMITTER_EMAIL: "test@example.com", + }; + + beforeAll(() => { + for (const [k, v] of Object.entries(identity)) { + savedEnv[k] = process.env[k]; + process.env[k] = v; + } + }); + + afterAll(() => { + for (const k of Object.keys(identity)) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + }); + + async function commitCount(path: string): Promise { + const out = await simpleGit(path).raw(["rev-list", "--count", "HEAD"]); + return Number(out.trim()); + } + + test("initializes a plain folder as a local-only repo on main with one commit", async () => { + const dir = join(workRoot, "plain"); + mkdirSync(dir); + + const resolved = await initLocalRepoInPlace(dir); + + expect(eqRealpath(resolved.repoPath, dir)).toBe(true); + expect(resolved.remoteName).toBeNull(); + expect(resolved.parsed).toBeNull(); + + const branch = ( + await simpleGit(dir).raw(["rev-parse", "--abbrev-ref", "HEAD"]) + ).trim(); + expect(branch).toBe("main"); + expect(await commitCount(dir)).toBe(1); + }); + + test("adopts a non-empty folder without erroring and preserves its files", async () => { + const dir = join(workRoot, "with-files"); + mkdirSync(dir); + writeFileSync(join(dir, "README.md"), "hello"); + + const resolved = await initLocalRepoInPlace(dir); + + expect(eqRealpath(resolved.repoPath, dir)).toBe(true); + expect(existsSync(join(dir, "README.md"))).toBe(true); + }); + + test("is idempotent on an already-initialized repo (no second commit)", async () => { + const repo = join(workRoot, "already"); + const git = await initRepoAt(repo); + await seedCommit(git); + expect(await commitCount(repo)).toBe(1); + + const resolved = await initLocalRepoInPlace(repo); + + expect(eqRealpath(resolved.repoPath, repo)).toBe(true); + // Resolved the existing repo rather than re-initializing / re-committing. + expect(await commitCount(repo)).toBe(1); + }); + + test("resolves to the parent toplevel for a subdir of an existing repo (no nested init)", async () => { + const repo = join(workRoot, "outer"); + const git = await initRepoAt(repo); + await seedCommit(git); + const inner = join(repo, "packages", "child"); + mkdirSync(inner, { recursive: true }); + + const resolved = await initLocalRepoInPlace(inner); + + expect(eqRealpath(resolved.repoPath, repo)).toBe(true); + // No standalone repo created inside the subdir. + expect(existsSync(join(inner, ".git"))).toBe(false); + }); + + test("rejects a path that does not exist", async () => { + await expect( + initLocalRepoInPlace(join(workRoot, "missing")), + ).rejects.toThrow(/Path does not exist/); + }); + + test("rejects a path that points at a file", async () => { + const file = join(workRoot, "file.txt"); + writeFileSync(file, "x"); + + await expect(initLocalRepoInPlace(file)).rejects.toThrow( + /Path is not a directory/, + ); + }); +}); + // ── cloneRepoInto ───────────────────────────────────────────────── describe("cloneRepoInto", () => { diff --git a/packages/host-service/src/trpc/router/project/utils/resolve-repo.ts b/packages/host-service/src/trpc/router/project/utils/resolve-repo.ts index a46a04d393c..878a1cb5df1 100644 --- a/packages/host-service/src/trpc/router/project/utils/resolve-repo.ts +++ b/packages/host-service/src/trpc/router/project/utils/resolve-repo.ts @@ -20,7 +20,7 @@ export interface ResolvedGitHubRepo extends ResolvedRepo { parsed: ParsedGitHubRemote; } -function validateDirectoryPath(path: string, label: string): void { +export function validateDirectoryPath(path: string, label: string): void { if (!existsSync(path)) { throw new TRPCError({ code: "BAD_REQUEST", @@ -95,17 +95,30 @@ async function gitInitMainBranch(targetPath: string): Promise { } } -async function revParseGitRoot(path: string): Promise { +/** + * Returns the canonical git root for `path`, or `null` when `path` is not + * inside a git work tree. Non-throwing variant of `revParseGitRoot` — callers + * that want to branch on "is this a git repo?" use this instead of catching. + */ +export async function tryRevParseGitRoot(path: string): Promise { try { return ( await createUserSimpleGit(path).revparse(["--show-toplevel"]) ).trim(); } catch { + return null; + } +} + +async function revParseGitRoot(path: string): Promise { + const root = await tryRevParseGitRoot(path); + if (root === null) { throw new TRPCError({ code: "BAD_REQUEST", message: `Not a git repository: ${path}`, }); } + return root; } /** @@ -129,6 +142,43 @@ export async function resolveLocalRepo( return { repoPath: gitRoot, remoteName: firstName, parsed: firstParsed }; } +/** + * Initialize git in an EXISTING, populated folder (in place) and resolve it as + * a local-only project. Unlike `initEmptyRepo`, this neither mkdirs nor fails on + * a non-empty directory — it adopts the user's folder. Use for "import a folder + * that isn't a git repo yet"; the caller must have confirmed intent with the + * user first, since `git init` writes into their directory. + * + * Idempotent: if the path is already inside a git work tree (e.g. it was + * initialized between detection and this call, or it's nested under a parent + * repo) we skip init and just resolve the existing root. + * + * Like `initEmptyRepo`, creates an `--allow-empty` initial commit so + * `ensureMainWorkspaceStrict` has a real branch/HEAD to point at; a bare + * `git init` leaves an unborn branch. + */ +export async function initLocalRepoInPlace( + repoPath: string, +): Promise { + validateDirectoryPath(repoPath, "Path"); + + const existingRoot = await tryRevParseGitRoot(repoPath); + if (existingRoot) return resolveLocalRepo(existingRoot); + + await gitInitMainBranch(repoPath); + try { + await createUserSimpleGit(repoPath).raw([ + "commit", + "--allow-empty", + "-m", + "Initial commit", + ]); + } catch (err) { + throw asInitialCommitTrpcError(err); + } + return resolveLocalRepo(repoPath); +} + /** * Validates that a path is a git working tree and returns the canonical git * root plus the GitHub remote whose `owner/name` matches `expectedSlug`. diff --git a/plans/20260601-v2-import-git-init-non-git-folder.md b/plans/20260601-v2-import-git-init-non-git-folder.md new file mode 100644 index 00000000000..229f37fd58e --- /dev/null +++ b/plans/20260601-v2-import-git-init-non-git-folder.md @@ -0,0 +1,262 @@ +# v2 "Import folder" — initialize git for a non-git folder + +## Problem + +When a user imports a folder that is **not yet a git repository**, v2 hard-errors with +`Not a git repository: ` instead of offering to initialize one. This is the likely +root cause of issue [#5033](https://github.com/superset-sh/superset/issues/5033) ("why +cannot import local git folder without remote?") — the report is really about a *non-git* +folder, not a remote-less one (remote-less repos already import fine). + +### Where it fails today + +- Import UI: `apps/desktop/.../AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts` + - `start()` picks a directory → calls `client.project.findByPath.query({ repoPath })` (line 56). +- `project.findByPath` (`packages/host-service/src/trpc/router/project/project.ts:165`) calls + `resolveLocalRepo(input.repoPath)` → +- `resolveLocalRepo` (`utils/resolve-repo.ts:116`) → `revParseGitRoot` (`:98`) runs + `git rev-parse --show-toplevel`; on a non-git folder it **throws** + `TRPCError BAD_REQUEST: "Not a git repository: "`. +- The UI catches it (`useFolderFirstImport.ts:63-66`) and surfaces it via `onError` as a toast. + +So the throw happens at **findByPath**, *before* `project.create importLocal` is ever +reached. The detection/branch point must live at or before findByPath, not only in create. + +### Existing pieces we can reuse + +`utils/resolve-repo.ts` already has the low-level building blocks: +- `gitInitMainBranch` (`:88`) — `git init --initial-branch=main` with a bare `git init` fallback. +- `asInitialCommitTrpcError` (`:69`) — maps git "empty ident"/`user.email`/`user.name` + failures to a `PRECONDITION_FAILED` with setup instructions. +- `initEmptyRepo` (`:177`) — the "empty project" mode does mkdir + init + empty commit. + +We are NOT reusing `initEmptyRepo` (it *creates* a new dir and fails on `EEXIST`). We need +to initialize git **in place** in the user's existing, populated folder. + +## Design + +Flow: **detect → confirm → init + import**. Never silently init an arbitrary folder the +user pointed at — `git init` writing into their directory is a side effect that deserves +explicit consent. + +Three layers: + +1. **Server — in-place init helper** (`utils/resolve-repo.ts`) +2. **Server — detection + opt-in init on the import path** (`project.ts` + `handlers.ts`) +3. **Desktop UI — confirm dialog + create-with-init** (`useFolderFirstImport` + a small dialog) + +### 1. Server: `initLocalRepoInPlace` + +Add to `utils/resolve-repo.ts`: + +```ts +/** + * Initialize git in an EXISTING, populated folder (in place) and resolve it as a + * local-only project. Unlike initEmptyRepo, this does not mkdir and does not fail + * if the directory is non-empty — it adopts the user's folder. + * + * Guards: + * - path must exist and be a directory (validateDirectoryPath) + * - path must NOT already be inside a git work tree — re-checked here to close the + * TOCTOU window after the UI's detection call (git init is idempotent, but we want + * to avoid re-initializing a nested repo's parent by surprise) + */ +export async function initLocalRepoInPlace(repoPath: string): Promise { + validateDirectoryPath(repoPath, "Path"); + + // Re-check: if it became a git work tree since detection, just resolve it. + const existingRoot = await tryRevParseGitRoot(repoPath); // returns null instead of throwing + if (existingRoot) return resolveLocalRepo(existingRoot); + + await gitInitMainBranch(repoPath); // reuse existing helper + try { + await createUserSimpleGit(repoPath).raw([ + "commit", "--allow-empty", "-m", "Initial commit", + ]); + } catch (err) { + throw asInitialCommitTrpcError(err); // reuse existing PRECONDITION_FAILED mapping + } + return resolveLocalRepo(repoPath); // resolves to { remoteName: null, parsed: null } +} +``` + +Supporting change: extract a non-throwing variant of the existing `revParseGitRoot`: + +```ts +async function tryRevParseGitRoot(path: string): Promise { + try { + return (await createUserSimpleGit(path).revparse(["--show-toplevel"])).trim(); + } catch { + return null; + } +} +// revParseGitRoot stays as the throwing wrapper around tryRevParseGitRoot. +``` + +**Initial commit is required**, not cosmetic: `ensureMainWorkspaceStrict` needs a real +branch/HEAD. A bare `git init` leaves an unborn branch; the `--allow-empty` initial commit +(same as `initEmptyRepo`) gives the main workspace something to point at. + +**Edge — folder nested inside a parent git repo:** `git rev-parse --show-toplevel` succeeds +and resolves to the *parent* root, so detection reports "already a git repo" and we never +offer init — we import the parent root, which is the current behavior. Leave it unchanged. + +### 2. Server: detection via `findByPath` + opt-in init on create + +**a) Detection — fold into `findByPath`, no separate query.** + +`findByPath` is already the single host-service call the import UI makes before create +(`useFolderFirstImport.ts:56`), and it already runs `resolveLocalRepo` (the exact line that +throws on a non-git folder, `project.ts:165`). The idiomatic pattern here is "server returns +a discriminated result, client branches" — exactly how `findByPath` returns `candidates` +today and how the UI's multiple-projects branch (`onMultipleProjects`) works. So make +`findByPath` catch the non-git case and return an additive, optional `needsGitInit` flag +rather than throwing: + +```ts +// project.ts findByPath — replace the unconditional resolveLocalRepo(input.repoPath) +const gitRoot = await tryRevParseGitRoot(input.repoPath); +if (gitRoot === null) { + validateDirectoryPath(input.repoPath, "Path"); // still 400 on missing / not-a-dir + return { candidates: [], cloudErrors: [], needsGitInit: true as const }; +} +const resolved = await resolveLocalRepo(gitRoot); // existing path, now repo-confirmed +``` + +`needsGitInit` is an optional field defaulting to absent/false — additive to the wire +contract, so existing `walkAllRemotes` callers are unaffected. One round-trip, no new +procedure, and the throw becomes a typed branch. + +**b) Opt-in init on create.** Extend the `importLocal` create mode so init only happens +after explicit user consent: + +```ts +// project.ts — create input, importLocal variant +z.object({ + kind: z.literal("importLocal"), + repoPath: z.string().min(1), + initIfNeeded: z.boolean().optional().default(false), +}), +``` + +> **Design tension (flag vs. separate mode).** The create modes are a discriminatedUnion +> where each `kind` has fixed init semantics (`empty`/`template` always init, +> `clone`/`importLocal` never), so a behavioral boolean sits slightly against the grain. +> Counter-argument: a separate `initLocal` mode would have an **identical input shape** +> (`{ repoPath }`) to `importLocal`, and discriminated unions are meant to distinguish by +> *shape*, not behavior — two same-shape variants is its own smell. Net: a genuine judgment +> call. **Recommendation: keep the `initIfNeeded` flag** (identical shape ⇒ same mode), but +> this is the one open API-design decision worth a maintainer's sign-off before coding. + +```ts +// handlers.ts +export async function createFromImportLocal( + ctx: HostServiceContext, + args: { name: string; repoPath: string; initIfNeeded?: boolean }, +): Promise { + const resolved = args.initIfNeeded + ? await resolveOrInitLocalRepo(args.repoPath) + : await resolveLocalRepo(args.repoPath); + return persistFromResolved(ctx, { + name: args.name, + resolved, + cleanupRepoPathOnFailure: false, // user's folder — never rm it (unchanged) + repoCloneUrlForCloud: resolved.parsed?.url, + }); +} + +// resolveOrInitLocalRepo: resolve if already a repo, else init in place. +async function resolveOrInitLocalRepo(repoPath: string): Promise { + const root = await tryRevParseGitRoot(repoPath); + return root ? resolveLocalRepo(root) : initLocalRepoInPlace(repoPath); +} +``` + +Cloud side needs **no change**: a freshly-init'd repo has `parsed: null`, so +`repoCloneUrlForCloud` is `undefined` — the cloud `v2Project.create` schema already marks +`repoCloneUrl` optional for "local-only imports have no remote yet" (the path `empty` and +`template` modes already exercise). + +**Rollback note:** `persistFromResolved` keeps `cleanupRepoPathOnFailure: false`, so we never +delete the user's folder. We *do* leave behind the `.git` we created if the cloud/workspace +saga later fails — acceptable, and re-running import simply resolves the now-existing repo. +Do not add `.git` teardown (risky). + +### 3. Desktop UI: confirm dialog + create-with-init + +Branch on the `needsGitInit` flag already returned by `findByPath` (no extra call): + +```ts +const response = await client.project.findByPath.query({ repoPath }); +if (response.needsGitInit) { + const confirmed = await options?.onConfirmGitInit?.({ repoPath }); // modal owns the dialog + if (!confirmed) return null; + const result = await client.project.create.mutate({ + name: getBaseName(repoPath), + mode: { kind: "importLocal", repoPath, initIfNeeded: true }, + }); + finalizeSetup(activeHostUrl, result); + return result; +} +// else: existing candidates → setup/create flow, unchanged +``` + +**Implementation note (as built):** `useFolderFirstImport` has **5 consumers**, not one host +modal, so threading an `onConfirmGitInit` callback through all of them is the wrong shape. +Instead the confirm is encapsulated entirely in the hook via a small v2-owned imperative +zustand store (`renderer/stores/git-init-confirm.ts`, mirroring the `add-repository-modal` +promise-resolve pattern): the hook calls `await requestGitInit(repoPath)`. The dialog +(`GitInitConfirmDialog`) is rendered once via `AddRepositoryModals` (already mounted once in +the dashboard layout). All 5 call sites are untouched. + +- Render a **dedicated confirm dialog** (shared `ui` alert-dialog) so the import flow owns its + own UI. Do not wire into the existing global git-init dialog store — it drives a different + (v1) project-creation path. Init must go through host-service `project.create` so the result + is a v2 project. + > " isn't a git repository yet. Initialize git here and import it?" + > [Cancel] [Initialize & import] + Per `apps/desktop/AGENTS.md`, rendered error text needs `select-text cursor-text` + (sonner toasts are exempt). +- Surface the `PRECONDITION_FAILED` "Git user is not configured…" message verbatim if the + initial commit fails — it's actionable. + +## Files to touch + +| File | Change | +| --- | --- | +| `packages/host-service/src/trpc/router/project/utils/resolve-repo.ts` | Add `tryRevParseGitRoot`, `initLocalRepoInPlace`; export `validateDirectoryPath`; refactor `revParseGitRoot` to wrap the non-throwing variant | +| `packages/host-service/src/trpc/router/project/handlers.ts` | `createFromImportLocal` accepts `initIfNeeded`; add `resolveOrInitLocalRepo` | +| `packages/host-service/src/trpc/router/project/project.ts` | `findByPath` returns optional `needsGitInit` instead of throwing on non-git; add `initIfNeeded` to `importLocal` create input; thread into handler call | +| `apps/desktop/.../AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts` | Branch on `needsGitInit`; `await requestGitInit(repoPath)`; create with `initIfNeeded: true` | +| `apps/desktop/src/renderer/stores/git-init-confirm.ts` (new) | Imperative confirm store (`request`/`resolve`) | +| `apps/desktop/.../AddRepositoryModals/components/GitInitConfirmDialog/` (new) | Confirm dialog, store-driven | +| `apps/desktop/.../AddRepositoryModals/AddRepositoryModals.tsx` | Mount `` alongside `NewProjectModal` | + +## Tests + +Extend `utils/resolve-repo.test.ts` (already covers "no remotes at all" / "gitlab origin"): +- `initLocalRepoInPlace` initializes a non-git temp dir → returns `{ remoteName: null, parsed: null }`, HEAD on `main`, exactly one commit. +- Adopts a folder with existing files (does not error on non-empty, unlike `initEmptyRepo`). +- Idempotent: pointed at an already-initialized repo → resolves it, no second init/commit. +- Nested-folder case: a subdir inside an existing repo resolves to the parent root (no init). +- Missing path / file (not dir) → `BAD_REQUEST`. +- Initial-commit failure with unset `user.email`/`user.name` → `PRECONDITION_FAILED` with setup text. + +Handler/router: +- `findByPath` on a non-git dir returns `{ candidates: [], needsGitInit: true }` (no throw); on a repo it behaves exactly as today (no `needsGitInit`). +- `findByPath` still 400s on a missing path / non-directory. +- `createFromImportLocal({ initIfNeeded: true })` on a non-git dir creates a local-only project + main workspace; with `initIfNeeded: false` (default) it still throws `Not a git repository` (back-compat). + +## Out of scope / explicitly unchanged + +- Remote-less *git* repos already import fine — untouched. +- `resolveGithubRepo` (the "no GitHub remote" throw) stays GitHub-feature-only (PRs/Issues). +- No auto-init without explicit user confirmation. + +## Open questions + +1. **`initIfNeeded` flag vs. separate `initLocal` mode** — the one API-shape decision worth a + maintainer's sign-off (see "Design tension" above). Plan recommends the flag because the + input shape is identical to `importLocal`. +2. **`.git` left behind on saga rollback** — accept (recommended) vs. tear down the `.git` we + created when the cloud/workspace step fails. From 70b131e7b8a5392f8adbdb4024e0175b7663f710 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 2 Jun 2026 00:53:30 -0700 Subject: [PATCH 2/2] fix(import): use cross-platform getBaseName for git-init dialog folder label split("/") mishandled Windows backslash paths and trailing slashes (empty label). getBaseName handles both separators and drops empty segments. --- .../components/GitInitConfirmDialog/GitInitConfirmDialog.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx index 16b0c868232..4f9fdd8f4ef 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/GitInitConfirmDialog/GitInitConfirmDialog.tsx @@ -7,6 +7,7 @@ import { AlertDialogTitle, } from "@superset/ui/alert-dialog"; import { Button } from "@superset/ui/button"; +import { getBaseName } from "renderer/lib/pathBasename"; import { useGitInitConfirmStore } from "renderer/stores/git-init-confirm"; /** @@ -19,7 +20,7 @@ export function GitInitConfirmDialog() { const repoPath = useGitInitConfirmStore((s) => s.repoPath); const resolve = useGitInitConfirmStore((s) => s.resolve); - const folderName = repoPath?.split("/").pop() ?? repoPath ?? "this folder"; + const folderName = repoPath ? getBaseName(repoPath) : "this folder"; return (