diff --git a/src/browser/components/ProjectCreateModal/ProjectCreateModal.cloneAbort.test.tsx b/src/browser/components/ProjectCreateModal/ProjectCreateModal.cloneAbort.test.tsx index 0a2d8ab8d4..965ec2dce9 100644 --- a/src/browser/components/ProjectCreateModal/ProjectCreateModal.cloneAbort.test.tsx +++ b/src/browser/components/ProjectCreateModal/ProjectCreateModal.cloneAbort.test.tsx @@ -35,6 +35,43 @@ describe("ProjectAddForm", () => { currentClientMock = {}; }); + test("creates a new git project and uses the backend response", async () => { + const createProject = mock(() => + Promise.resolve({ + success: true as const, + data: { + normalizedPath: "/projects/backend-normalized", + projectConfig: { workspaces: [] }, + }, + }) + ); + currentClientMock = { + projects: { + getDefaultProjectDir: () => Promise.resolve("/projects"), + list: () => Promise.resolve([]), + create: createProject, + }, + }; + const onSuccess = mock(() => undefined); + + const { getByText, getByPlaceholderText } = render( + + ); + + fireEvent.click(getByText("New project")); + const projectInput = getByPlaceholderText("my-new-project"); + const user = userEvent.setup({ document: projectInput.ownerDocument }); + await user.type(projectInput, "prototype"); + fireEvent.click(getByText("Create Project")); + + await waitFor(() => + expect(createProject).toHaveBeenCalledWith({ projectPath: "prototype", initGit: true }) + ); + await waitFor(() => + expect(onSuccess).toHaveBeenCalledWith("/projects/backend-normalized", { workspaces: [] }) + ); + }); + test("aborts in-flight clone when unmounted", async () => { let receivedSignal: AbortSignal | null = null; diff --git a/src/browser/components/ProjectCreateModal/ProjectCreateModal.stories.tsx b/src/browser/components/ProjectCreateModal/ProjectCreateModal.stories.tsx index b247388ecd..4220a8431c 100644 --- a/src/browser/components/ProjectCreateModal/ProjectCreateModal.stories.tsx +++ b/src/browser/components/ProjectCreateModal/ProjectCreateModal.stories.tsx @@ -64,6 +64,27 @@ export const LocalFolder: AppStory = { }, }; +/** + * Three labeled modes must fit narrow dialogs without overflowing the right edge; + * the pinned phone viewport guards the wrap layout (dialogs portal to body, so a + * fixed-width decorator cannot constrain them). + */ +export const PhoneViewport: AppStory = { + globals: { + viewport: { value: "mobile1", isRotated: false }, + }, + parameters: { + ...appMeta.parameters, + pixel: { + matrix: { themes: ["dark"], viewports: ["phone"] }, + }, + }, + render: () => , + play: async ({ canvasElement }) => { + await openNewProjectModal(canvasElement); + }, +}; + /** "Clone repo" tab of the Add Project modal. */ export const CloneRepo: AppStory = { // Integration: stories navigate via sidebar → "Add project" button to open the modal portal. diff --git a/src/browser/components/ProjectCreateModal/ProjectCreateModal.tsx b/src/browser/components/ProjectCreateModal/ProjectCreateModal.tsx index 29b75a19e1..d364d5d522 100644 --- a/src/browser/components/ProjectCreateModal/ProjectCreateModal.tsx +++ b/src/browser/components/ProjectCreateModal/ProjectCreateModal.tsx @@ -1,5 +1,5 @@ import React, { useState, useCallback, useEffect, useImperativeHandle, useRef } from "react"; -import { FolderOpen, Github } from "lucide-react"; +import { FolderOpen, FolderPlus, Github } from "lucide-react"; import { Dialog, DialogContent, @@ -95,6 +95,8 @@ interface ProjectCreateFormProps { submitLabel?: string; /** Optional override for the path placeholder. */ placeholder?: string; + /** Create and initialize a new git repository instead of adding an existing folder. */ + createNewGitRepo?: boolean; /** Hide the footer actions (submit/cancel buttons). */ hideFooter?: boolean; onErrorChange?: (hasError: boolean) => void; @@ -114,16 +116,24 @@ export const ProjectCreateForm = React.forwardRef { @@ -255,7 +268,7 @@ export const ProjectCreateForm = React.forwardRef + {createNewGitRepo && ( +

+ Bare names are created in the default projects directory and initialized as git + repositories. +

+ )} + {error &&

{error}

} {!hideFooter && ( @@ -283,7 +303,7 @@ export const ProjectCreateForm = React.forwardRef )} )} @@ -296,8 +316,7 @@ export const ProjectCreateForm = React.forwardRef void; @@ -776,14 +795,21 @@ function ProjectAddFormFooter(props: { onClose?: () => void; }) { const handleSubmit = () => { - if (props.mode === "pick-folder") { - void props.createFormRef.current?.submit(); - } else { + if (props.mode === "clone") { void props.cloneFormRef.current?.submit(); + } else { + void props.createFormRef.current?.submit(); } }; - const actionLabel = props.mode === "pick-folder" ? "Add Project" : "Clone Project"; + const actionLabel = + props.mode === "pick-folder" + ? "Add Project" + : props.mode === "clone" + ? "Clone Project" + : "Create Project"; + const creatingLabel = + props.mode === "pick-folder" ? "Adding…" : props.mode === "clone" ? "Cloning…" : "Creating…"; return ( @@ -793,7 +819,7 @@ function ProjectAddFormFooter(props: { )} ); @@ -888,7 +914,7 @@ export const ProjectAddForm = React.forwardRef { - if (nextMode !== "pick-folder" && nextMode !== "clone") { + if (nextMode !== "pick-folder" && nextMode !== "clone" && nextMode !== "new") { return; } @@ -907,16 +933,16 @@ export const ProjectAddForm = React.forwardRef ({ submit: async () => { - if (mode === "pick-folder") { - return (await projectCreateFormRef.current?.submit()) ?? false; + if (mode === "clone") { + return (await projectCloneFormRef.current?.submit()) ?? false; } - return (await projectCloneFormRef.current?.submit()) ?? false; + return (await projectCreateFormRef.current?.submit()) ?? false; }, getTrimmedInput: () => { - if (mode === "pick-folder") { - return projectCreateFormRef.current?.getTrimmedPath() ?? ""; + if (mode === "clone") { + return projectCloneFormRef.current?.getTrimmedRepoUrl() ?? ""; } - return projectCloneFormRef.current?.getTrimmedRepoUrl() ?? ""; + return projectCreateFormRef.current?.getTrimmedPath() ?? ""; }, getMode: () => mode, }), @@ -929,12 +955,13 @@ export const ProjectAddForm = React.forwardRef + {/* flex-wrap keeps the three labeled modes inside narrow dialogs (~375px). */} @@ -944,21 +971,13 @@ export const ProjectAddForm = React.forwardRef Clone repo + + + New project + - {mode === "pick-folder" ? ( - - ) : ( + {mode === "clone" ? ( + ) : ( + )} @@ -1020,7 +1053,9 @@ export const ProjectCreateModal: React.FC = ({ Add Project - Pick a folder or clone a project repository + + Pick a folder, clone a repository, or create a new project + void }) { body: ( <>

- Projects are the folders or repos you want Xum to work in. Add a local folder or clone - from GitHub, then click Next. + Projects are the folders or repos you want Xum to work in. Add a local folder, clone + from GitHub, or create a new project, then click Next.

{userProjects.size > 0 ? ( diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 26b11778b1..423d2c1533 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -690,7 +690,7 @@ export const mcpOauth = { // Projects export const projects = { create: { - input: z.object({ projectPath: z.string() }), + input: z.object({ projectPath: z.string(), initGit: z.boolean().optional() }), output: ResultSchema( z.object({ projectConfig: ProjectConfigSchema, diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index a79f18782a..d2b4d3f2ce 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -3293,7 +3293,7 @@ export const router = (authToken?: string) => { .input(schemas.projects.create.input) .output(schemas.projects.create.output) .handler(async ({ context, input }) => { - return context.projectService.create(input.projectPath); + return context.projectService.create(input.projectPath, { initGit: input.initGit }); }), getDefaultProjectDir: t .input(schemas.projects.getDefaultProjectDir.input) diff --git a/src/node/services/projectService.test.ts b/src/node/services/projectService.test.ts index aef803e406..5a04b69ce0 100644 --- a/src/node/services/projectService.test.ts +++ b/src/node/services/projectService.test.ts @@ -152,6 +152,363 @@ describe("ProjectService", () => { }); describe("create", () => { + it("creates and registers a git project at a new path", async () => { + const projectPath = path.join(tempDir, "new-git-project"); + + const result = await service.create(projectPath, { initGit: true }); + + expect(result.success).toBe(true); + if (!result.success) throw new Error("Expected success"); + expect(result.data.normalizedPath).toBe(projectPath); + expect((await fs.stat(path.join(projectPath, ".git"))).isDirectory()).toBe(true); + expect( + execSync("git branch --show-current", { cwd: projectPath, encoding: "utf-8" }).trim() + ).toBe("main"); + expect( + execSync("git rev-list --count HEAD", { cwd: projectPath, encoding: "utf-8" }).trim() + ).toBe("1"); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(true); + }); + + it("initializes and registers an existing empty directory", async () => { + const projectPath = path.join(tempDir, "empty-git-project"); + await fs.mkdir(projectPath); + + const result = await service.create(projectPath, { initGit: true }); + + expect(result.success).toBe(true); + expect((await fs.stat(path.join(projectPath, ".git"))).isDirectory()).toBe(true); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(true); + }); + + it("rejects an existing non-empty directory without modifying it", async () => { + const projectPath = path.join(tempDir, "non-empty-project"); + const existingFile = path.join(projectPath, "README.md"); + await fs.mkdir(projectPath); + await fs.writeFile(existingFile, "existing content", "utf-8"); + + const result = await service.create(projectPath, { initGit: true }); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("already exists and is not empty"); + expect(await fs.readFile(existingFile, "utf-8")).toBe("existing content"); + expect(fs.stat(path.join(projectPath, ".git"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(false); + }); + + it("rejects invalid paths without registering a project", async () => { + const filePath = path.join(tempDir, "not-a-directory"); + await fs.writeFile(filePath, "content", "utf-8"); + + const fileResult = await service.create(filePath, { initGit: true }); + const emptyResult = await service.create("", { initGit: true }); + + expect(fileResult.success).toBe(false); + expect(emptyResult.success).toBe(false); + expect(await fs.readFile(filePath, "utf-8")).toBe("content"); + expect(config.loadConfigOrDefault().projects.size).toBe(0); + }); + + it("rejects initGit inside a registered project tree", async () => { + const parentPath = await createLocalGitRepository(tempDir, "parent-repo"); + const parentResult = await service.create(parentPath); + expect(parentResult.success).toBe(true); + + const nestedPath = path.join(parentPath, "nested-new-repo"); + const result = await service.create(nestedPath, { initGit: true }); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain( + "Cannot create a new git repository inside an existing project" + ); + // The rejected nested directory must not linger inside the parent checkout. + expect(fs.stat(nestedPath)).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(nestedPath)).toBe(false); + }); + + it("rejects initGit reached through a symlink into a registered project tree", async () => { + if (process.platform === "win32") return; + + // Canonicalize the temp root (macOS /var is itself a symlink) so the registered + // parent path matches what realpath resolves the alias to. + const realTempDir = await fs.realpath(tempDir); + const parentPath = await createLocalGitRepository(realTempDir, "symlink-parent-repo"); + const parentResult = await service.create(parentPath); + expect(parentResult.success).toBe(true); + + const aliasPath = path.join(realTempDir, "parent-alias"); + await fs.symlink(parentPath, aliasPath); + const nestedAliasPath = path.join(aliasPath, "nested-new-repo"); + + const result = await service.create(nestedAliasPath, { initGit: true }); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain( + "Cannot create a new git repository inside an existing project" + ); + expect(fs.stat(path.join(parentPath, "nested-new-repo"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(nestedAliasPath)).toBe(false); + }); + + it("rejects initGit when the canonical parent registers concurrently via symlink", async () => { + if (process.platform === "win32") return; + + const realTempDir = await fs.realpath(tempDir); + const parentPath = await createLocalGitRepository(realTempDir, "late-parent-repo"); + const aliasPath = path.join(realTempDir, "late-parent-alias"); + await fs.symlink(parentPath, aliasPath); + const nestedAliasPath = path.join(aliasPath, "nested-late-repo"); + + // Deterministic interleaving: queue the parent registration so create()'s + // snapshot read misses it while its transform sees it. The lexical fresh-parent + // check cannot catch this (the alias is not lexically beneath the real path); + // only the canonical re-check in the transform can reject it. + const registerParent = config.editConfig((cfg) => { + cfg.projects.set(parentPath, { workspaces: [] }); + return cfg; + }); + const result = await service.create(nestedAliasPath, { initGit: true }); + await registerParent; + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("changed concurrently"); + // The nested repository must not survive inside the registered checkout. + expect(fs.stat(path.join(parentPath, "nested-late-repo"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(nestedAliasPath)).toBe(false); + }); + + it("fails initGit create without leaving .git when config persistence fails", async () => { + const projectPath = path.join(tempDir, "persist-fail-project"); + const nonPersistingConfig = new Config(tempDir); + // Run the transform (so create() reaches its success path) but drop the save, + // modeling saveConfig's log-and-continue behavior on write failures. + nonPersistingConfig.editConfig = (transform) => { + transform(nonPersistingConfig.loadConfigOrDefault()); + return Promise.resolve(); + }; + const nonPersistingService = new ProjectService(nonPersistingConfig); + + const result = await nonPersistingService.create(projectPath, { initGit: true }); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("save project configuration"); + // Roll back the created directory so a retry is not blocked by leftover .git. + expect(fs.stat(projectPath)).rejects.toThrow(); + }); + + it("rolls back a partial .git when git init itself fails", async () => { + if (process.platform === "win32") return; + + const projectPath = path.join(tempDir, "partial-init-project"); + await fs.mkdir(projectPath); + // Model a broken init template: init creates a partial .git, then fails. + const fakeGit = await installFakeGit( + tempDir, + "create-partial-init-failure", + `#!/bin/sh +prev="" +for arg in "$@"; do + if [ "$arg" = "init" ]; then + mkdir -p "$prev/.git" + printf 'init failed' >&2 + exit 1 + fi + prev="$arg" +done +exit 1 +` + ); + + const result = await withEnv(fakeGit.env, () => + service.create(projectPath, { initGit: true }) + ); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("init failed"); + expect((await fs.stat(projectPath)).isDirectory()).toBe(true); + expect(fs.stat(path.join(projectPath, ".git"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(false); + }); + + it("removes only .git when a plain create wins the same pre-existing directory", async () => { + const projectPath = path.join(tempDir, "plain-vs-initgit"); + await fs.mkdir(projectPath); + + // Deterministic interleaving: queue the plain registration so the initGit + // create's snapshot misses it (git init runs) while its transform hits the + // duplicate re-check; the loser must strip its .git from the winner's project. + const registerPlain = config.editConfig((cfg) => { + cfg.projects.set(projectPath, { workspaces: [] }); + return cfg; + }); + const result = await service.create(projectPath, { initGit: true }); + await registerPlain; + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("already exists"); + expect((await fs.stat(projectPath)).isDirectory()).toBe(true); + expect(fs.stat(path.join(projectPath, ".git"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(true); + }); + + it("initializes the new project even when GIT_DIR/GIT_WORK_TREE point elsewhere", async () => { + const externalRepoPath = await createLocalGitRepository(tempDir, "external-selected-repo"); + const externalCommitsBefore = execSync("git rev-list --count HEAD", { + cwd: externalRepoPath, + encoding: "utf-8", + }).trim(); + const projectPath = path.join(tempDir, "env-selected-project"); + + const result = await withEnv( + { GIT_DIR: path.join(externalRepoPath, ".git"), GIT_WORK_TREE: externalRepoPath }, + () => service.create(projectPath, { initGit: true }) + ); + + expect(result.success).toBe(true); + // The new project owns its own repository; the externally selected one is untouched. + expect((await fs.stat(path.join(projectPath, ".git"))).isDirectory()).toBe(true); + expect( + execSync("git rev-list --count HEAD", { cwd: externalRepoPath, encoding: "utf-8" }).trim() + ).toBe(externalCommitsBefore); + }); + + it("serializes concurrent gitInit calls for the same directory", async () => { + const projectPath = path.join(tempDir, "concurrent-gitinit"); + await fs.mkdir(projectPath); + + const [first, second] = await Promise.all([ + service.gitInit(projectPath), + service.gitInit(projectPath), + ]); + + const outcomes = [first, second]; + expect(outcomes.filter((r) => r.success)).toHaveLength(1); + const loser = outcomes.find((r) => !r.success); + expect(loser && !loser.success ? loser.error : "").toContain("already initializing"); + // The winner's repository survives with exactly its initial commit. + expect( + execSync("git rev-list --count HEAD", { cwd: projectPath, encoding: "utf-8" }).trim() + ).toBe("1"); + }); + + it("removes only .git when a descendant wins registration during an initGit create", async () => { + const parentPath = path.join(tempDir, "late-descendant-parent"); + const descendantPath = path.join(parentPath, "child-project"); + + // Deterministic interleaving: queue the descendant registration so create()'s + // snapshot misses it (git init runs) while its transform sees it and records + // hierarchy-changed-descendant, which must strip our .git but keep the tree. + const registerDescendant = config.editConfig((cfg) => { + cfg.projects.set(descendantPath, { workspaces: [] }); + return cfg; + }); + const result = await service.create(parentPath, { initGit: true }); + await registerDescendant; + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("changed concurrently"); + // The winner's tree survives, but our unregistered outer repository must not. + expect((await fs.stat(parentPath)).isDirectory()).toBe(true); + expect(fs.stat(path.join(parentPath, ".git"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(parentPath)).toBe(false); + expect(config.loadConfigOrDefault().projects.has(descendantPath)).toBe(true); + }); + + it("serializes concurrent initGit creates of the same pre-existing empty directory", async () => { + const projectPath = path.join(tempDir, "shared-empty-project"); + await fs.mkdir(projectPath); + + const [first, second] = await Promise.all([ + service.create(projectPath, { initGit: true }), + service.create(projectPath, { initGit: true }), + ]); + + const outcomes = [first, second]; + expect(outcomes.filter((r) => r.success)).toHaveLength(1); + // The loser must never touch the winner's git state: the repository stays + // intact with its initial commit and stays registered. + expect( + execSync("git rev-list --count HEAD", { cwd: projectPath, encoding: "utf-8" }).trim() + ).toBe("1"); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(true); + }); + + it("rolls back git init in a pre-existing directory when the initial commit fails", async () => { + if (process.platform === "win32") return; + + const projectPath = path.join(tempDir, "existing-empty-project"); + await fs.mkdir(projectPath); + // Real `git init` so the .git rollback has something to remove; the shim fails + // only the commit step to model hook/signing failures after a successful init. + const realGit = execSync("command -v git", { encoding: "utf-8" }).trim(); + const fakeGit = await installFakeGit( + tempDir, + "create-commit-failure", + `#!/bin/sh +for arg in "$@"; do + if [ "$arg" = "commit" ]; then + printf 'commit failed' >&2 + exit 1 + fi +done +exec ${realGit} "$@" +` + ); + + const result = await withEnv(fakeGit.env, () => + service.create(projectPath, { initGit: true }) + ); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("commit failed"); + // The user's directory survives, but the partial .git must be rolled back so a + // retry is not rejected as non-empty. + expect((await fs.stat(projectPath)).isDirectory()).toBe(true); + expect(fs.stat(path.join(projectPath, ".git"))).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(false); + + const retry = await service.create(projectPath, { initGit: true }); + expect(retry.success).toBe(true); + }); + + it("removes a newly created directory when git initialization fails", async () => { + if (process.platform === "win32") return; + + const projectPath = path.join(tempDir, "failed-git-project"); + const fakeGit = await installFakeGit( + tempDir, + "create-init-failure", + "#!/bin/sh\nprintf 'git failed' >&2\nexit 1\n" + ); + + const result = await withEnv(fakeGit.env, () => + service.create(projectPath, { initGit: true }) + ); + + expect(result.success).toBe(false); + expect(!result.success && result.error).toContain("git failed"); + expect(fs.stat(projectPath)).rejects.toThrow(); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(false); + }); + + it("concurrent initGit creates of the same new path leave one intact repository", async () => { + const projectPath = path.join(tempDir, "concurrent-git-project"); + + const [first, second] = await Promise.all([ + service.create(projectPath, { initGit: true }), + service.create(projectPath, { initGit: true }), + ]); + + const outcomes = [first, second]; + expect(outcomes.filter((r) => r.success)).toHaveLength(1); + // The loser must never run git operations in the winner's directory, so the + // winner's repository stays intact and registered. + expect( + execSync("git rev-list --count HEAD", { cwd: projectPath, encoding: "utf-8" }).trim() + ).toBe("1"); + expect(config.loadConfigOrDefault().projects.has(projectPath)).toBe(true); + }); + // Regression (PR #3694 Codex P1): two concurrent create() calls for the same // not-yet-existing path both compute createdDirectory === true before either // registration is serialized. The loser hits the duplicate re-check inside the diff --git a/src/node/services/projectService.ts b/src/node/services/projectService.ts index dcbcba5053..359da41384 100644 --- a/src/node/services/projectService.ts +++ b/src/node/services/projectService.ts @@ -16,6 +16,7 @@ import type { Secret } from "@/common/types/secrets"; import type { Stats } from "fs"; import * as fsPromises from "fs/promises"; import { execFileAsync, killProcessTree } from "@/node/utils/disposableExec"; +import { GIT_SCOPE_ENV_UNSET } from "@/node/services/backup/credentials"; import { buildFileCompletionsIndex, EMPTY_FILE_COMPLETIONS_INDEX, @@ -424,6 +425,8 @@ function hasRegisteredSubProjectAncestor( export class ProjectService { private readonly fileCompletionsCache = new Map(); + /** Canonical paths with git initialization in flight; see create() claim below. */ + private readonly activeGitInits = new Set(); private directoryPicker?: (initialPath?: string | null) => Promise; private readonly sshPromptService: SshPromptService | undefined; private workspaceService?: WorkspaceRemover; @@ -449,8 +452,10 @@ export class ProjectService { } async create( - projectPath: string + projectPath: string, + options?: { initGit?: boolean } ): Promise> { + let gitInitClaimKey: string | null = null; try { // Validate input if (!projectPath || projectPath.trim().length === 0) { @@ -502,31 +507,84 @@ export class ProjectService { return Err("Project path is not a directory"); } + if (options?.initGit && existingStat) { + const entries = await fsPromises.readdir(normalizedPath); + if (entries.length > 0) { + return Err("Directory already exists and is not empty"); + } + } + if (config.projects.has(normalizedPath)) { return Err("Project already exists"); } - const createdDirectory = existingStat == null; + // Create the directory if it doesn't exist. The non-recursive mkdir on the leaf is + // an exclusive ownership claim: only the call that actually created the directory + // may remove it later, so concurrent creates of the same absent path cannot delete + // each other's work. Keep the user-facing path stable in config; Windows realpath + // may expand 8.3 short names and surprise callers. + let createdDirectory = false; + if (existingStat == null) { + try { + await fsPromises.mkdir(path.dirname(normalizedPath), { recursive: true }); + } catch (error) { + const err = error as NodeJS.ErrnoException; + // Recursive mkdir never rejects an existing directory, so EEXIST here means a + // path component exists as a file: report it like ENOTDIR ("not a folder"). + const friendly = friendlyFsError( + err.code === "EEXIST" ? { ...err, code: "ENOTDIR" } : error, + "create folder", + normalizedPath + ); + if (friendly) { + return Err(friendly); + } + throw error; + } + try { + await fsPromises.mkdir(normalizedPath); + createdDirectory = true; + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== "EEXIST") { + const friendly = friendlyFsError(error, "create folder", normalizedPath); + if (friendly) { + return Err(friendly); + } + throw error; + } + // Lost a creation race: the path now belongs to the concurrent winner, so + // re-validate it as a pre-existing entry instead of claiming ownership. + const raceStat = await fsPromises.stat(normalizedPath).catch(() => null); + if (!raceStat?.isDirectory()) { + return Err("Project path is not a directory"); + } + // Never git-init a directory that appeared mid-call: the winner may still be + // initializing it, and interleaved git operations could corrupt or delete its + // work. A retry sees the directory as pre-existing and validates it normally. + if (options?.initGit) { + return Err("Directory already exists"); + } + } + } + + let initializedGitDir = false; const cleanupCreatedDirectory = async () => { - if (!createdDirectory) return; try { - await fsPromises.rm(normalizedPath, { recursive: true, force: true }); + if (createdDirectory) { + await fsPromises.rm(normalizedPath, { recursive: true, force: true }); + } else if (initializedGitDir) { + // We only initialized git inside a pre-existing directory: remove the .git + // we created so a retry does not hit the non-empty-directory rejection. + await fsPromises.rm(path.join(normalizedPath, ".git"), { + recursive: true, + force: true, + }); + } } catch (error) { log.error(`Failed to clean up rejected project directory ${normalizedPath}:`, error); } }; - - // Create the directory if it doesn't exist (like mkdir -p). Keep the user-facing - // path stable in config; Windows realpath may expand 8.3 short names and surprise callers. - try { - await fsPromises.mkdir(normalizedPath, { recursive: true }); - } catch (error) { - const friendly = friendlyFsError(error, "create folder", normalizedPath); - if (friendly) { - return Err(friendly); - } - throw error; - } const canonicalPath = await resolveRealProjectPath(normalizedPath); if (config.projects.has(canonicalPath)) { @@ -541,6 +599,17 @@ export class ProjectService { } const parentProjectPath = findDeepestTopLevelParentProject(normalizedPath, config.projects); + // Initializing a nested repository would flip the new directory's git root after + // the same-repository validation below, persisting a sub-project from a different + // repository (mirrors the clone-into-project-tree rejection). Check the canonical + // path too so a symlinked alias into a registered checkout cannot bypass this. + if ( + options?.initGit && + (parentProjectPath ?? findDeepestTopLevelParentProject(canonicalPath, config.projects)) + ) { + await cleanupCreatedDirectory(); + return Err("Cannot create a new git repository inside an existing project"); + } if (parentProjectPath) { const [parentGitRoot, subProjectGitRoot] = await Promise.all([ readGitTopLevel(parentProjectPath), @@ -568,6 +637,24 @@ export class ProjectService { } } + if (options?.initGit) { + // Exclusive per-canonical-path claim: two initGit creates targeting the same + // pre-existing empty directory would otherwise interleave git init and failure + // rollback, letting one call delete the .git the other just initialized. + if (this.activeGitInits.has(canonicalPath)) { + return Err("Another project creation is already initializing this directory"); + } + this.activeGitInits.add(canonicalPath); + gitInitClaimKey = canonicalPath; + + const gitInitResult = await this.initializeGitRepository(normalizedPath); + if (!gitInitResult.success) { + await cleanupCreatedDirectory(); + return gitInitResult; + } + initializedGitDir = gitInitResult.data.initializedGitDir; + } + // Register the project inside the serialized editConfig transform, re-checking for // duplicates and re-deriving the hierarchy from FRESH config: persisting the pre-read // snapshot would clobber concurrent config edits (lost-update race). The async @@ -615,6 +702,17 @@ export class ProjectService { const hasNewDescendantProject = freshDescendantProjectPaths.some( (candidatePath) => !descendantProjectPaths.includes(candidatePath) ); + // Re-check the canonical parent for initGit: the snapshot-time rejection above + // can miss a parent registered concurrently, and the lexical freshParent check + // below cannot see it when this path reaches the checkout through a symlink. + if ( + options?.initGit && + findDeepestTopLevelParentProject(canonicalPath, freshConfig.projects) + ) { + transformFailure = "hierarchy-changed"; + createResult = Err("Project hierarchy changed concurrently; please retry"); + return freshConfig; + } if (freshParentProjectPath !== parentProjectPath || hasNewDescendantProject) { // A new descendant registered under this path claims the directory tree we // created: recursive cleanup would delete that project's checkout, so treat @@ -642,11 +740,36 @@ export class ProjectService { // depth and parent-only hierarchy rejections leave the directory ours to remove. if (transformFailure === "depth" || transformFailure === "hierarchy-changed") { await cleanupCreatedDirectory(); + } else if ( + (transformFailure === "hierarchy-changed-descendant" || transformFailure === "duplicate") && + initializedGitDir + ) { + // The winning registration owns the files (a duplicate winner registered this + // exact pre-existing directory; a descendant winner lives inside it), but the + // .git this losing request created would silently turn the winner's project + // into a repository it never asked for, or wrap its checkout in an + // unregistered outer repository that changes git discovery. + await fsPromises + .rm(path.join(normalizedPath, ".git"), { recursive: true, force: true }) + .catch((cleanupError: unknown) => { + log.error(`Failed to roll back git init in ${normalizedPath}:`, cleanupError); + }); + } + if (createResult.success && !this.config.loadConfigOrDefault().projects.has(normalizedPath)) { + // Config persistence (editConfig → private saveConfig) logs-and-continues on + // write failures. Without this check a git-initialized project would report + // success, vanish after restart, and block retries on the leftover .git. + await cleanupCreatedDirectory(); + return Err("Failed to save project configuration"); } return createResult; } catch (error) { const message = getErrorMessage(error); return Err(`Failed to create project: ${message}`); + } finally { + if (gitInitClaimKey) { + this.activeGitInits.delete(gitInitClaimKey); + } } } @@ -1382,62 +1505,106 @@ export class ProjectService { } } - /** - * Initialize a git repository in the project directory. - * Runs `git init` and creates an initial commit so branches exist. - * Also handles "unborn" repos (git init already run but no commits yet). - */ - async gitInit(projectPath: string): Promise> { - if (typeof projectPath !== "string" || projectPath.trim().length === 0) { - return Err("Project path is required"); - } + /** Reports whether it created the `.git` directory so callers can roll that back. */ + private async initializeGitRepository( + normalizedPath: string + ): Promise> { + // Set before running `git init`: even a failing init can leave a partial .git + // behind (e.g. a broken init template), which must be rolled back too. + let initializedGitDir = false; try { - const validation = await validateProjectPath(projectPath); - if (!validation.valid) { - return Err(validation.error ?? "Invalid project path"); - } - const normalizedPath = validation.expandedPath!; - const isGitRepo = await isGitRepository(normalizedPath); if (isGitRepo) { - // Check if repo is "unborn" (git init but no commits yet) const branches = await listLocalBranches(normalizedPath); if (branches.length > 0) { return Err("Directory is already a git repository with commits"); } - // Repo exists but is unborn - just create the initial commit } else { - // Initialize git repository with main as default branch - using initProc = execFileAsync("git", ["-C", normalizedPath, "init", "-b", "main"]); + initializedGitDir = true; + using initProc = execFileAsync("git", ["-C", normalizedPath, "init", "-b", "main"], { + // Inherited repository selectors (GIT_DIR/GIT_WORK_TREE) win over -C and + // would redirect init/commit into an unrelated external repository. + env: GIT_SCOPE_ENV_UNSET, + }); await initProc.result; } - // Create an initial empty commit so the branch exists and worktree/SSH can work - // Without a commit, the repo is "unborn" and has no branches - // Use -c flags to set identity only for this commit (don't persist to repo config) - using commitProc = execFileAsync("git", [ - "-C", - normalizedPath, - "-c", - "user.name=mux", - "-c", - "user.email=mux@localhost", - "commit", - "--allow-empty", - "-m", - "Initial commit", - ]); + // A born branch is required by worktree and SSH runtimes. Keep the fallback + // identity scoped to this initial commit instead of changing repository config. + using commitProc = execFileAsync( + "git", + [ + "-C", + normalizedPath, + "-c", + "user.name=mux", + "-c", + "user.email=mux@localhost", + "commit", + "--allow-empty", + "-m", + "Initial commit", + ], + { env: GIT_SCOPE_ENV_UNSET } + ); await commitProc.result; - // Invalidate file completions cache since the repo state changed this.fileCompletionsCache.delete(normalizedPath); + return Ok({ initializedGitDir }); + } catch (error) { + // Roll back a .git we created (or that a failed init left partially behind) so + // the directory returns to its prior state and a retry is not rejected as + // non-empty (e.g. when the initial commit fails). + if (initializedGitDir) { + await fsPromises + .rm(path.join(normalizedPath, ".git"), { recursive: true, force: true }) + .catch((cleanupError: unknown) => { + log.error(`Failed to roll back git init in ${normalizedPath}:`, cleanupError); + }); + } + const message = getErrorMessage(error); + log.error("Failed to initialize git repository:", error); + return Err(`Failed to initialize git repository: ${message}`); + } + } - return Ok(undefined); + /** + * Initialize a git repository in the project directory. + * Runs `git init` and creates an initial commit so branches exist. + * Also handles "unborn" repos (git init already run but no commits yet). + */ + async gitInit(projectPath: string): Promise> { + if (typeof projectPath !== "string" || projectPath.trim().length === 0) { + return Err("Project path is required"); + } + let claimKey: string | null = null; + try { + const validation = await validateProjectPath(projectPath); + if (!validation.valid) { + return Err(validation.error ?? "Invalid project path"); + } + const normalizedPath = validation.expandedPath!; + // Same exclusive claim as create(): concurrent initializations of one directory + // could double-commit or let one call's failure rollback delete the other's .git. + const canonicalPath = await resolveRealProjectPath(normalizedPath).catch( + () => normalizedPath + ); + if (this.activeGitInits.has(canonicalPath)) { + return Err("Another project creation is already initializing this directory"); + } + this.activeGitInits.add(canonicalPath); + claimKey = canonicalPath; + const result = await this.initializeGitRepository(normalizedPath); + return result.success ? Ok(undefined) : result; } catch (error) { const message = getErrorMessage(error); log.error("Failed to initialize git repository:", error); return Err(`Failed to initialize git repository: ${message}`); + } finally { + if (claimKey) { + this.activeGitInits.delete(claimKey); + } } }