diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 587a3e4abbde..6f1d9f0743cc 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -712,6 +712,279 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.notInclude(error.message, secret); assert.notProperty(error, "args"); assert.notProperty(error, "stderr"); + assert.notProperty(error, "reason"); + }), + ); + + it.effect("names the branch conflict behind a failed worktree add", () => + Effect.gen(function* () { + const parent = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const cwd = pathService.join(parent, "repo"); + const fileSystem = yield* FileSystem.FileSystem; + yield* fileSystem.makeDirectory(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* initRepoWithCommit(cwd); + const firstWorktree = pathService.join(parent, "first"); + yield* git(cwd, ["worktree", "add", firstWorktree, "-b", "shared-branch"]); + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.worktreeAddConflict", + cwd, + args: ["worktree", "add", pathService.join(parent, "second"), "shared-branch"], + env: { LC_ALL: "C" }, + }) + .pipe(Effect.flip); + + assert.deepInclude(error, { + _tag: "GitCommandError", + operation: "GitVcsDriver.test.worktreeAddConflict", + reason: "branch_checked_out_in_worktree", + }); + assert.include(error.message, "(branch_checked_out_in_worktree)"); + assert.notInclude(error.message, firstWorktree); + }), + ); + + it.effect("names a missing repository behind a failed command", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const driver = yield* GitVcsDriver.GitVcsDriver; + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.notARepository", + cwd, + args: ["rev-parse", "--abbrev-ref", "HEAD"], + env: { LC_ALL: "C" }, + }) + .pipe(Effect.flip); + + assert.deepInclude(error, { + _tag: "GitCommandError", + operation: "GitVcsDriver.test.notARepository", + reason: "not_a_repository", + }); + assert.include(error.message, "(not_a_repository)"); + }), + ); + + it.effect("appends the reason tag to a caller-supplied detail", () => + Effect.gen(function* () { + const parent = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const cwd = pathService.join(parent, "repo"); + const fileSystem = yield* FileSystem.FileSystem; + yield* fileSystem.makeDirectory(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* initRepoWithCommit(cwd); + yield* git(cwd, [ + "worktree", + "add", + pathService.join(parent, "taken"), + "-b", + "taken-branch", + ]); + + const error = yield* driver + .createWorktree({ + cwd, + refName: "taken-branch", + path: pathService.join(parent, "second"), + }) + .pipe(Effect.flip); + + assert.equal(error.detail, "git worktree add failed"); + assert.include(error.message, "git worktree add failed (branch_checked_out_in_worktree)"); + }), + ); + + for (const { label, sshMessage, expectedReason, expectedText } of [ + { + label: "a refused key", + sshMessage: "git@example.invalid: Permission denied (publickey).", + expectedReason: "authentication_failed" as const, + expectedText: "(authentication_failed)", + }, + { + label: "a refused key among several methods", + sshMessage: "git@example.invalid: Permission denied (publickey,password).", + expectedReason: "authentication_failed" as const, + expectedText: "(authentication_failed)", + }, + { + label: "a refused keyboard-interactive attempt", + sshMessage: "git@example.invalid: Permission denied (keyboard-interactive).", + expectedReason: "authentication_failed" as const, + expectedText: "(authentication_failed)", + }, + { + label: "a rejected password", + sshMessage: "Permission denied, please try again.", + expectedReason: "authentication_failed" as const, + expectedText: "(authentication_failed)", + }, + { + label: "an untrusted host key", + sshMessage: "Host key verification failed.", + expectedReason: "host_key_unverified" as const, + expectedText: "(host_key_unverified)", + }, + ]) { + it.effect(`names ${label} rather than an unreachable remote`, () => + Effect.gen(function* () { + const parent = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const cwd = pathService.join(parent, "repo"); + const fileSystem = yield* FileSystem.FileSystem; + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* fileSystem.makeDirectory(cwd); + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["remote", "add", "origin", "git@example.invalid:owner/repo.git"]); + // Stands in for ssh, which prints its refusal unprefixed and leaves + // git to add only a generic "could not read" line after it. + const fakeSsh = pathService.join(parent, "fake-ssh"); + yield* writeTextFile( + parent, + "fake-ssh", + `#!/bin/sh\necho "${sshMessage}" >&2\nexit 255\n`, + ); + yield* fileSystem.chmod(fakeSsh, 0o755); + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.sshRefusal", + cwd, + args: ["fetch", "origin"], + env: { LC_ALL: "C", GIT_SSH_COMMAND: fakeSsh }, + }) + .pipe(Effect.flip); + + assert.deepInclude(error, { reason: expectedReason }); + assert.include(error.message, expectedText); + }), + ); + } + + it.effect("does not read a filesystem permission error as an ssh refusal", () => + Effect.gen(function* () { + const parent = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const cwd = pathService.join(parent, "repo"); + const fileSystem = yield* FileSystem.FileSystem; + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* fileSystem.makeDirectory(cwd); + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["remote", "add", "origin", "git@example.invalid:owner/repo.git"]); + const fakeSsh = pathService.join(parent, "fake-ssh"); + yield* writeTextFile( + parent, + "fake-ssh", + '#!/bin/sh\necho "fatal: cannot open backup file: Permission denied (os error 13)" >&2\nexit 255\n', + ); + yield* fileSystem.chmod(fakeSsh, 0o755); + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.osPermission", + cwd, + args: ["fetch", "origin"], + env: { LC_ALL: "C", GIT_SSH_COMMAND: fakeSsh }, + }) + .pipe(Effect.flip); + + assert.notInclude(error.message, "(authentication_failed)"); + }), + ); + + it.effect("does not read remote hook output as a git failure reason", () => + Effect.gen(function* () { + const parent = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const remote = pathService.join(parent, "origin.git"); + const cwd = pathService.join(parent, "work"); + const fileSystem = yield* FileSystem.FileSystem; + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* fileSystem.makeDirectory(cwd); + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["init", "--bare", remote]); + yield* git(cwd, ["remote", "add", "origin", remote]); + // Git prefixes everything the server says with `remote:`, so a remote + // hook can put arbitrary text in front of the classifier. + const preReceive = pathService.join(remote, "hooks", "pre-receive"); + yield* writeTextFile( + remote, + "hooks/pre-receive", + '#!/bin/sh\necho "authentication failed" >&2\nexit 1\n', + ); + yield* fileSystem.chmod(preReceive, 0o755); + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.remoteHookOutput", + cwd, + args: ["push", "origin", "HEAD"], + env: { LC_ALL: "C" }, + }) + .pipe(Effect.flip); + + assert.notProperty(error, "reason"); + assert.notInclude(error.message, "(authentication_failed)"); + }), + ); + + it.effect("does not read hook output as a git failure reason", () => + Effect.gen(function* () { + const parent = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const remote = pathService.join(parent, "origin.git"); + const cwd = pathService.join(parent, "work"); + const fileSystem = yield* FileSystem.FileSystem; + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* fileSystem.makeDirectory(cwd); + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["init", "--bare", remote]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* writeTextFile( + cwd, + ".git/hooks/pre-push", + '#!/bin/sh\necho "authentication failed" >&2\nexit 1\n', + ); + yield* fileSystem.chmod(pathService.join(cwd, ".git/hooks/pre-push"), 0o755); + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.hookOutput", + cwd, + args: ["push", "origin", "HEAD"], + env: { LC_ALL: "C" }, + }) + .pipe(Effect.flip); + + assert.notProperty(error, "reason"); + assert.notInclude(error.message, "(authentication_failed)"); + }), + ); + + it.effect("leaves a tag collision unclassified rather than calling it a path", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* initRepoWithCommit(cwd); + yield* git(cwd, ["tag", "v1"]); + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.tagCollision", + cwd, + args: ["tag", "v1"], + env: { LC_ALL: "C" }, + }) + .pipe(Effect.flip); + + assert.notProperty(error, "reason"); + assert.notInclude(error.message, "(path_already_exists)"); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 71e478cbaa3d..17efe00070c7 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -21,6 +21,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; import { GitCommandError, + type GitCommandFailureReason, type ReviewDiffFileContentsInput, type ReviewDiffPreviewInput, type ReviewDiffPreviewSource, @@ -391,6 +392,62 @@ function gitCommandContext( } as const; } +// Git states the actual cause on stderr, but stderr never leaves this module: +// it echoes argv and remote URLs, which can carry credentials. Matching it +// against fixed patterns yields a tag the caller can act on and a message that +// quotes none of the matched text. Patterns run in order, specific first. +const GIT_FAILURE_REASON_PATTERNS: ReadonlyArray = [ + [/would clobber existing tag/i, "tag_would_be_clobbered"], + [/is already (?:used by worktree at|checked out at)/i, "branch_checked_out_in_worktree"], + [/a branch named .+ already exists/i, "branch_already_exists"], + // ssh names the methods it tried, so the parenthesized list varies: + // `(publickey)`, `(publickey,password)`, `(keyboard-interactive)`. + [ + /(?:authentication failed|could not read Username|could not read Password|permission denied \([a-z-]+(?:,[a-z-]+)*\)|permission denied, please try again)/i, + "authentication_failed", + ], + // Distinct from a credential failure: the remote answered and the key it + // presented is untrusted, so pointing the user at credentials would misdirect. + [/host key verification failed/i, "host_key_unverified"], + [ + /(?:could not read from remote repository|does not appear to be a git repository|repository .+ not found)/i, + "remote_unreachable", + ], + // Quoted-path forms only: an unquoted `fatal: already exists` also + // covers tag and ref collisions, which are not path collisions. + [/fatal: '[^']+' already exists|destination path .+ already exists/i, "path_already_exists"], +]; + +// Hooks write to the same stream git does, and nothing distinguishes their +// text from git's: a pre-push hook echoing "authentication failed" would +// otherwise be read as a credential failure. Git prefixes its own diagnostics +// and states a push rejection on a `! [rejected]` line, so only those are +// classified. `remote:` is deliberately excluded — git prefixes every byte the +// server sends that way, remote hook output included, so trusting it would +// reintroduce the same false positive from the other end of the connection. +const GIT_DIAGNOSTIC_LINE_PATTERN = /^(?:fatal|error):|^!\s|^\s+!\s/; +// ssh reports the refusal itself, unprefixed, and git only adds a generic +// "Could not read from remote repository" after it. Dropping ssh's line would +// leave that generic one to be read as an unreachable remote when the real +// cause is credentials, so these specific refusals are classified too. +const SSH_TRANSPORT_REFUSAL_PATTERN = + /Permission denied \((?:publickey|password|keyboard-interactive)|Permission denied, please try again|Host key verification failed/i; + +function classifyGitFailure(stderr: string): GitCommandFailureReason | null { + const diagnostics = stderr + .split(/\r?\n/) + .filter( + (line) => GIT_DIAGNOSTIC_LINE_PATTERN.test(line) || SSH_TRANSPORT_REFUSAL_PATTERN.test(line), + ) + .join("\n"); + if (diagnostics.length === 0) return null; + if (isNonRepositoryGitStderr(diagnostics)) return "not_a_repository"; + for (const [pattern, reason] of GIT_FAILURE_REASON_PATTERNS) { + if (pattern.test(diagnostics)) return reason; + } + return null; +} + function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null { const trimmed = value.trim(); const prefix = `refs/remotes/${remoteName}/`; @@ -811,8 +868,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* trace2Monitor.flush; if (!input.allowNonZeroExit && exitCode !== 0) { + const reason = classifyGitFailure(stderr.text); return yield* new GitCommandError({ ...gitCommandContext(commandInput), + ...(reason === null ? {} : { reason }), detail: "Git command exited with a non-zero status.", exitCode, stdoutLength: stdout.text.length, @@ -895,9 +954,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* if (options.allowNonZeroExit || result.exitCode === 0) { return Effect.succeed(result); } + const reason = classifyGitFailure(result.stderr); return Effect.fail( new GitCommandError({ ...gitCommandContext({ operation, cwd, args }), + ...(reason === null ? {} : { reason }), detail: options.fallbackErrorDetail ?? "Git command exited with a non-zero status.", ...(result.exitCode === null ? {} : { exitCode: result.exitCode }), stdoutLength: result.stdout.length, diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 915c3627c9b9..302e56cef400 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -335,6 +335,23 @@ export const VcsPullResult = Schema.Struct({ export type VcsPullResult = typeof VcsPullResult.Type; // RPC / domain errors + +// Well-known git failures, recognized from stderr at the driver and carried as +// a closed set of diagnostic tags. Git's stderr itself stays off the error: it +// echoes argv and remote URLs, which can hold credentials. The tag names the +// cause for logs and callers; it does not select a message. +export const GitCommandFailureReason = Schema.Literals([ + "authentication_failed", + "branch_already_exists", + "branch_checked_out_in_worktree", + "host_key_unverified", + "not_a_repository", + "path_already_exists", + "remote_unreachable", + "tag_would_be_clobbered", +]); +export type GitCommandFailureReason = typeof GitCommandFailureReason.Type; + export class GitCommandError extends Schema.TaggedErrorClass()("GitCommandError", { operation: Schema.String, command: Schema.String, @@ -344,11 +361,13 @@ export class GitCommandError extends Schema.TaggedErrorClass()( stdoutLength: Schema.optional(Schema.Number), stderrLength: Schema.optional(Schema.Number), outputLength: Schema.optional(Schema.Number), + reason: Schema.optional(GitCommandFailureReason), detail: Schema.String, cause: Schema.optional(Schema.Defect()), }) { override get message(): string { - return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}`; + const reason = this.reason === undefined ? "" : ` (${this.reason})`; + return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}${reason}`; } }