Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 0 additions & 79 deletions apps/desktop/src/lib/trpc/routers/system.ts

This file was deleted.

145 changes: 145 additions & 0 deletions apps/desktop/src/lib/trpc/routers/system/detect-git.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
import { describe, expect, test } from "bun:test";
import { type DetectGitDeps, detectGit } from "./detect-git";

type Responses = Record<string, string | Error>;

function makeDeps(
platform: NodeJS.Platform,
responses: Responses,
existingFiles: string[] = [],
) {
const calls: string[] = [];
const deps: DetectGitDeps = {
platform,
run: async (command, args) => {
const key = [command, ...args].join(" ");
calls.push(key);
const response = responses[key];
if (response === undefined) throw new Error(`unexpected: ${key}`);
if (response instanceof Error) throw response;
return response;
},
fileExists: (path) => existingFiles.includes(path),
};
return { deps, calls };
}

function commandError(stderr: string, code: string | number = 1) {
return Object.assign(new Error("Command failed"), { stderr, code });
}

const VERSION = "git version 2.50.1 (Apple Git-155)\n";

describe("detectGit on macOS", () => {
test("does not run the Apple stub when no developer tools are installed", async () => {
const { deps, calls } = makeDeps("darwin", {
"/usr/bin/which git": "/usr/bin/git\n",
"/usr/bin/xcode-select -p": commandError(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The /usr/bin/pgrep call is not mocked in this test, so the mock's "unexpected" guard is silently absorbed by runOrNull. Add an explicit pgrep no-match response (commandError("", 1)) so the installerRunning: false expectation reflects the intended exit-1 path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/desktop/src/lib/trpc/routers/system/detect-git.test.ts, line 37:

<comment>The `/usr/bin/pgrep` call is not mocked in this test, so the mock's "unexpected" guard is silently absorbed by `runOrNull`. Add an explicit pgrep no-match response (`commandError("", 1)`) so the `installerRunning: false` expectation reflects the intended exit-1 path.</comment>

<file context>
@@ -0,0 +1,145 @@
+	test("does not run the Apple stub when no developer tools are installed", async () => {
+		const { deps, calls } = makeDeps("darwin", {
+			"/usr/bin/which git": "/usr/bin/git\n",
+			"/usr/bin/xcode-select -p": commandError(
+				"xcode-select: error: unable to get active developer directory",
+				2,
</file context>

"xcode-select: error: unable to get active developer directory",
2,
),
});
expect(await detectGit(deps)).toEqual({
installed: false,
version: null,
needsCommandLineTools: true,
installerRunning: false,
error: null,
});
expect(calls).not.toContain("git --version");
});

test("reports when Apple's installer window is open", async () => {
const { deps } = makeDeps("darwin", {
"/usr/bin/which git": "/usr/bin/git\n",
"/usr/bin/xcode-select -p": commandError("unable to get", 2),
"/usr/bin/pgrep -f /System/Library/CoreServices/Install Command Line Developer Tools.app/":
"812\n",
});
const result = await detectGit(deps);
expect(result.needsCommandLineTools).toBe(true);
expect(result.installerRunning).toBe(true);
});

test("treats a developer directory without git as missing tools", async () => {
const { deps, calls } = makeDeps("darwin", {
"/usr/bin/which git": "/usr/bin/git\n",
"/usr/bin/xcode-select -p": "/Library/Developer/CommandLineTools\n",
});
expect((await detectGit(deps)).needsCommandLineTools).toBe(true);
expect(calls).not.toContain("git --version");
});

test("reports git from the Command Line Tools", async () => {
const { deps } = makeDeps(
"darwin",
{
"/usr/bin/which git": "/usr/bin/git\n",
"/usr/bin/xcode-select -p": "/Library/Developer/CommandLineTools\n",
"git --version": VERSION,
},
["/Library/Developer/CommandLineTools/usr/bin/git"],
);
expect(await detectGit(deps)).toEqual({
installed: true,
version: "2.50.1",
needsCommandLineTools: false,
installerRunning: false,
error: null,
});
});

test("uses a git outside the stub without asking xcode-select", async () => {
const { deps, calls } = makeDeps("darwin", {
"/usr/bin/which git": "/usr/local/bin/git\n",
"git --version": "git version 2.51.0\n",
});
expect((await detectGit(deps)).installed).toBe(true);
expect(calls).not.toContain("/usr/bin/xcode-select -p");
});

test("passes on the reason when the tools exist but git refuses to run", async () => {
const license =
"You have not agreed to the Xcode license agreements. Please run 'sudo xcodebuild -license'.";
const { deps } = makeDeps(
"darwin",
{
"/usr/bin/which git": "/usr/bin/git\n",
"/usr/bin/xcode-select -p":
"/Applications/Xcode.app/Contents/Developer\n",
"git --version": commandError(`${license}\n`),
},
["/Applications/Xcode.app/Contents/Developer/usr/bin/git"],
);
expect(await detectGit(deps)).toEqual({
installed: false,
version: null,
needsCommandLineTools: false,
installerRunning: false,
error: license,
});
});
});

describe("detectGit on other platforms", () => {
test("never asks xcode-select", async () => {
const { deps, calls } = makeDeps("linux", { "git --version": VERSION });
expect((await detectGit(deps)).installed).toBe(true);
expect(calls).toEqual(["git --version"]);
});

test("reports git missing without a raw spawn error", async () => {
const { deps } = makeDeps("win32", {
"git --version": Object.assign(new Error("spawn git ENOENT"), {
code: "ENOENT",
}),
});
expect(await detectGit(deps)).toEqual({
installed: false,
version: null,
needsCommandLineTools: false,
installerRunning: false,
error: null,
});
});
});
101 changes: 101 additions & 0 deletions apps/desktop/src/lib/trpc/routers/system/detect-git.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import { join } from "node:path";

export interface GitDetectResult {
installed: boolean;
version: string | null;
needsCommandLineTools: boolean;
/** macOS: Apple's "Install Command Line Developer Tools" window is open. */
installerRunning: boolean;
error: string | null;
}

export interface DetectGitDeps {
platform: NodeJS.Platform;
/** Resolves with stdout; rejects when the command fails or is missing. */
run: (command: string, args: string[]) => Promise<string>;
fileExists: (path: string) => boolean;
}

const APPLE_GIT_STUB_PATH = "/usr/bin/git";

// macOS truncates the process name to 15 characters ("Install Command"),
// so match the executable path instead.
export const COMMAND_LINE_TOOLS_INSTALLER_PATH =
"/System/Library/CoreServices/Install Command Line Developer Tools.app/";

async function runOrNull(
run: DetectGitDeps["run"],
command: string,
args: string[],
): Promise<string | null> {
try {
return (await run(command, args)).trim() || null;
} catch {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: runOrNull returns null for every failure, including spawn errors that have nothing to do with git. A transient spawn failure in the darwin branch then reports needsCommandLineTools: true, which shows the wrong onboarding guidance. Only treat non-zero exit codes as "not found", and let other errors surface or log a warning.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/desktop/src/lib/trpc/routers/system/detect-git.ts, line 33:

<comment>`runOrNull` returns null for every failure, including spawn errors that have nothing to do with git. A transient spawn failure in the darwin branch then reports `needsCommandLineTools: true`, which shows the wrong onboarding guidance. Only treat non-zero exit codes as "not found", and let other errors surface or log a warning.</comment>

<file context>
@@ -0,0 +1,101 @@
+): Promise<string | null> {
+	try {
+		return (await run(command, args)).trim() || null;
+	} catch {
+		return null;
+	}
</file context>

return null;
}
}

function failureReason(error: unknown): string | null {
if (error && typeof error === "object" && "code" in error) {
if (error.code === "ENOENT") return null;
}
const stderr =
error && typeof error === "object" && "stderr" in error
? String(error.stderr)
: "";
const text = stderr.trim() || (error instanceof Error ? error.message : "");
return text.split("\n")[0]?.trim() || null;
}

export async function detectGit({
platform,
run,
fileExists,
}: DetectGitDeps): Promise<GitDetectResult> {
if (platform === "darwin") {
// Running Apple's /usr/bin/git stub without developer tools opens the
// "install command line developer tools" prompt, so check them first.
const gitPath = await runOrNull(run, "/usr/bin/which", ["git"]);
if (gitPath === null || gitPath === APPLE_GIT_STUB_PATH) {
const developerDir = await runOrNull(run, "/usr/bin/xcode-select", [
"-p",
]);
if (
developerDir === null ||
!fileExists(join(developerDir, "usr", "bin", "git"))
) {
// pgrep exits 1 when nothing matches, which runOrNull turns into null.
const installerPid = await runOrNull(run, "/usr/bin/pgrep", [
"-f",
COMMAND_LINE_TOOLS_INSTALLER_PATH,
]);
return {
installed: false,
version: null,
needsCommandLineTools: true,
installerRunning: installerPid !== null,
error: null,
};
}
}
}

try {
const stdout = await run("git", ["--version"]);
return {
installed: true,
version: stdout.match(/git version (\S+)/)?.[1] ?? null,
needsCommandLineTools: false,
installerRunning: false,
error: null,
};
} catch (error) {
return {
installed: false,
version: null,
needsCommandLineTools: false,
installerRunning: false,
error: failureReason(error),
};
}
}
Loading
Loading