fix(desktop): confirm the repo root when the picked folder isn't one

apps/desktop/src/lib/trpc/routers/projects/projects.ts

@@ -56,7 +56,12 @@ type OpenNewError = { canceled: false; error: string };
 type OpenNewResult =
 	| OpenNewCanceled
 	| { canceled: false; project: Project }
-	| { canceled: false; needsGitInit: true; selectedPath: string }
+	| {
+			canceled: false;
+			needsGitInit: true;
+			selectedPath: string;
+			enclosingRepoPath?: string;
+	  }
 	| OpenNewError;
 
 /**
@@ -99,7 +104,7 @@ function parsePullRequests(raw: unknown) {
 
 type FolderOutcome =
 	| { status: "success"; project: Project }
-	| { status: "needsGitInit"; selectedPath: string }
+	| { status: "needsGitInit"; selectedPath: string; enclosingRepoPath?: string }
 	| { status: "error"; selectedPath: string; error: string };
 
 type OpenNewMultiResult =
@@ -1095,7 +1100,19 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
 
 			for (const selectedPath of result.filePaths) {
 				try {
-					const mainRepoPath = await getGitRoot(selectedPath);
+					const { root: mainRepoPath, isRoot } = await getGitRoot(selectedPath);
+					// The selected folder sits inside a repo but isn't its root, so
+					// opening it means opening the enclosing repo. Let the user
+					// confirm that (or initialize the folder they actually picked)
+					// before anything is written.
+					if (!isRoot) {
+						outcomes.push({
+							status: "needsGitInit",
+							selectedPath,
+							enclosingRepoPath: mainRepoPath,
+						});
+						continue;
+					}
 					const defaultBranch = await getDefaultBranch(mainRepoPath);
 
 					const project = upsertProject(mainRepoPath, defaultBranch);
@@ -1156,7 +1173,18 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
 
 				let mainRepoPath: string;
 				try {
-					mainRepoPath = await getGitRoot(selectedPath);
+					const gitRoot = await getGitRoot(selectedPath);
+					// See openNew: a folder nested inside a repo resolves to that
+					// repo, so ask before opening something the user didn't pick.
+					if (!gitRoot.isRoot) {
+						return {
+							canceled: false,
+							needsGitInit: true as const,
+							selectedPath,
+							enclosingRepoPath: gitRoot.root,
+						};
+					}
+					mainRepoPath = gitRoot.root;
 				} catch (error) {
 					if (error instanceof NotGitRepoError) {
 						return {

apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts

@@ -7,6 +7,7 @@ import {
 	realpathSync,
 	rmSync,
 	statSync,
+	symlinkSync,
 	writeFileSync,
 } from "node:fs";
 import { tmpdir } from "node:os";
@@ -15,9 +16,11 @@ import {
 	branchExistsOnRemote,
 	createWorktree,
 	getCurrentBranch,
+	getGitRoot,
 	getWorktreeCreatedAt,
 	hasUnpushedCommits,
 	isUnbornHeadError,
+	NotGitRepoError,
 	parsePorcelainStatusV2,
 	parsePrUrl,
 } from "./git";
@@ -1134,3 +1137,159 @@ describe("parsePrUrl", () => {
 		).toBe(null);
 	});
 });
+
+describe("getGitRoot", () => {
+	beforeEach(() => {
+		mkdirSync(TEST_DIR, { recursive: true });
+	});
+
+	afterEach(() => {
+		if (existsSync(TEST_DIR)) {
+			rmSync(TEST_DIR, { recursive: true, force: true });
+		}
+	});
+
+	test("reports the repo root itself as the root", async () => {
+		const repoPath = createTestRepo("root");
+		seedCommit(repoPath);
+
+		const result = await getGitRoot(repoPath);
+
+		expect(result.root).toBe(repoPath);
+		expect(result.isRoot).toBe(true);
+	});
+
+	// Guards the untrimmed read of `--show-toplevel`: a directory name may end
+	// in whitespace, and trimming it hands callers a path that doesn't exist.
+	test("preserves a root whose directory name ends in whitespace", async () => {
+		const repoPath = createTestRepo("trailing space ");
+		seedCommit(repoPath);
+
+		const result = await getGitRoot(repoPath);
+
+		expect(result.root).toBe(repoPath);
+		expect(result.root.endsWith(" ")).toBe(true);
+		expect(existsSync(result.root)).toBe(true);
+		expect(result.isRoot).toBe(true);
+	});
+
+	// Git terminates the line with LF and round-trips a trailing CR that is part
+	// of the name, so the terminator strip must not consume it.
+	test("preserves a root whose directory name ends in a carriage return", async () => {
+		const repoPath = createTestRepo("trailing-cr\r");
+		seedCommit(repoPath);
+
+		const result = await getGitRoot(repoPath);
+
+		expect(result.root).toBe(repoPath);
+		expect(result.root.endsWith("\r")).toBe(true);
+		expect(existsSync(result.root)).toBe(true);
+		expect(result.isRoot).toBe(true);
+	});
+
+	// The walk-up is deliberate and stays: a subdirectory still resolves to the
+	// repo it belongs to. Only isRoot tells the caller the user picked something
+	// other than that root.
+	test("walks up from a subdirectory and reports it is not the root", async () => {
+		const repoPath = createTestRepo("subdir");
+		seedCommit(repoPath);
+		const nested = join(repoPath, "packages", "app");
+		mkdirSync(nested, { recursive: true });
+
+		const result = await getGitRoot(nested);
+
+		expect(result.root).toBe(repoPath);
+		expect(result.isRoot).toBe(false);
+	});
+
+	test("reports an untracked folder inside a repo as not the root", async () => {
+		const repoPath = createTestRepo("untracked-child");
+		seedCommit(repoPath);
+		const scratch = join(repoPath, "scratch");
+		mkdirSync(scratch, { recursive: true });
+
+		const result = await getGitRoot(scratch);
+
+		expect(result.root).toBe(repoPath);
+		expect(result.isRoot).toBe(false);
+	});
+
+	test("throws NotGitRepoError when no ancestor is a repo", async () => {
+		const plainPath = join(TEST_DIR, "plain");
+		mkdirSync(plainPath, { recursive: true });
+
+		await expect(getGitRoot(plainPath)).rejects.toBeInstanceOf(NotGitRepoError);
+	});
+
+	// A worktree root keeps `.git` as a FILE, not a directory, so a filesystem
+	// check would misclassify it and offer to initialize a repo Superset made.
+	test("treats a linked worktree root as a root", async () => {
+		const repoPath = createTestRepo("worktree-main");
+		seedCommit(repoPath);
+		const worktreePath = join(TEST_DIR, "worktree-linked");
+		execSync(`git worktree add -b wt-branch "${worktreePath}"`, {
+			cwd: repoPath,
+			stdio: "ignore",
+		});
+
+		expect(statSync(join(worktreePath, ".git")).isFile()).toBe(true);
+
+		const result = await getGitRoot(worktreePath);
+
+		expect(result.root).toBe(worktreePath);
+		expect(result.isRoot).toBe(true);
+	});
+
+	test("resolves a subdirectory of a worktree to the worktree", async () => {
+		const repoPath = createTestRepo("worktree-sub-main");
+		seedCommit(repoPath);
+		const worktreePath = join(TEST_DIR, "worktree-sub");
+		execSync(`git worktree add -b wt-sub-branch "${worktreePath}"`, {
+			cwd: repoPath,
+			stdio: "ignore",
+		});
+		const nested = join(worktreePath, "src");
+		mkdirSync(nested, { recursive: true });
+
+		const result = await getGitRoot(nested);
+
+		expect(result.root).toBe(worktreePath);
+		expect(result.isRoot).toBe(false);
+	});
+
+	// Git reports the real path, so a symlinked repo root would fail any
+	// comparison of the requested path against the reported root.
+	test("treats a symlink to a repo root as a root", async () => {
+		const repoPath = createTestRepo("symlink-target");
+		seedCommit(repoPath);
+		const linkPath = join(TEST_DIR, "symlink-to-repo");
+		symlinkSync(repoPath, linkPath);
+
+		const result = await getGitRoot(linkPath);
+
+		expect(result.root).toBe(repoPath);
+		expect(result.isRoot).toBe(true);
+	});
+
+	// Bare repos and `.git` directories fail with "must be run in a work tree",
+	// not "not a git repository". They have no `.git` child, so classifying them
+	// as NotGitRepoError would offer to initialize a repo inside a repo.
+	test("does not classify a bare repo as NotGitRepoError", async () => {
+		const barePath = join(TEST_DIR, "bare.git");
+		mkdirSync(barePath, { recursive: true });
+		execSync("git init --bare", { cwd: barePath, stdio: "ignore" });
+
+		await expect(getGitRoot(barePath)).rejects.not.toBeInstanceOf(
+			NotGitRepoError,
+		);
+	});
+
+	test("does not classify a .git directory as NotGitRepoError", async () => {
+		const repoPath = createTestRepo("dotgit");
+		seedCommit(repoPath);
+
+		await expect(getGitRoot(join(repoPath, ".git"))).rejects.not.toBeInstanceOf(
+			NotGitRepoError,
+		);
+	});
+});

apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts

@@ -872,11 +872,38 @@ export async function removeWorktree(
 	}
 }
 
-export async function getGitRoot(path: string): Promise<string> {
+export interface GitRootInfo {
+	/** Absolute path of the work tree root. Git walks up, so this can be an
+	 *  ancestor of the path that was passed in. */
+	root: string;
+	/** True when the requested path is itself that root: a normal clone root,
+	 *  a `git worktree` root, or a submodule root. */
+	isRoot: boolean;
+}
+
+export async function getGitRoot(path: string): Promise<GitRootInfo> {
 	try {
 		const git = await getSimpleGitWithShellPath(path);
-		const root = await git.revparse(["--show-toplevel"]);
-		return root.trim();
+		// Two calls, not one: a directory name may contain a newline, so a single
+		// `rev-parse --show-toplevel --show-prefix` cannot be split back into
+		// its two values unambiguously. `raw` also leaves stdout untrimmed, which
+		// keeps a root whose final directory name ends in whitespace intact —
+		// trimming it would hand callers a path that doesn't exist.
+		const rootOutput = await git.raw(["rev-parse", "--show-toplevel"]);
+		// Strip exactly the LF git terminates the line with. Not `\r?\n`: a
+		// directory name may itself end in a carriage return, and git round-trips
+		// that as `<name>\r\n`, so consuming the CR would yield a path that
+		// doesn't exist.
+		const root = rootOutput.replace(/\n$/, "");
+		// `--show-prefix` is empty exactly when `path` is the work tree root, so
+		// git answers the question in its own terms. Comparing `root` against
+		// `path` would be wrong on macOS three ways: `--show-toplevel`
+		// canonicalizes /tmp to /private/tmp, resolves symlinks to their real
+		// path, and returns the canonical casing on a case-insensitive volume.
+		// Emptiness only — `core.quotePath` can C-quote non-ASCII path
+		// components, so the prefix value itself is not safe to parse.
+		const prefix = await git.revparse(["--show-prefix"]);
+		return { root, isRoot: prefix === "" };
 	} catch (error) {
 		const message = error instanceof Error ? error.message : String(error);
 		if (message.toLowerCase().includes("not a git repository")) {