authorgravatar for 114993336+walid-baharwal@users.noreply.github.comMuhammad Waleed <114993336+walid-baharwal@users.noreply.github.com> 2026-10-06 02:18:00+05:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-05 14:18:00-07:00
logf973b1d7d5cd7ffb4dd3c1c61cc0caece09fd1a6
tree88436139f89d2f579a7278ccab168323e47770b1
parent73fd3c1ab0d8b2f6add7bbf8c498b9a2a87278ff
signature Signed by PGP key B5690EEEBB952194

fix(server): name the cause of a failed git command (#8645)

Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

3 files changed, 364 insertions(+), 5 deletions(-)

apps/server/src/vcs/GitVcsDriverCore.test.ts+273
...@@ -1007,6 +1007,279 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -1007,6 +1007,279 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
1007 assert.notInclude(error.message, secret);1007 assert.notInclude(error.message, secret);
1008 assert.notProperty(error, "args");1008 assert.notProperty(error, "args");
1009 assert.notProperty(error, "stderr");1009 assert.notProperty(error, "stderr");
1010 assert.notProperty(error, "reason");
1011 }),
1012 );
1013
1014 it.effect("names the branch conflict behind a failed worktree add", () =>
1015 Effect.gen(function* () {
1016 const parent = yield* makeTmpDir();
1017 const pathService = yield* Path.Path;
1018 const cwd = pathService.join(parent, "repo");
1019 const fileSystem = yield* FileSystem.FileSystem;
1020 yield* fileSystem.makeDirectory(cwd);
1021 const driver = yield* GitVcsDriver.GitVcsDriver;
1022 yield* initRepoWithCommit(cwd);
1023 const firstWorktree = pathService.join(parent, "first");
1024 yield* git(cwd, ["worktree", "add", firstWorktree, "-b", "shared-branch"]);
1025
1026 const error = yield* driver
1027 .execute({
1028 operation: "GitVcsDriver.test.worktreeAddConflict",
1029 cwd,
1030 args: ["worktree", "add", pathService.join(parent, "second"), "shared-branch"],
1031 env: { LC_ALL: "C" },
1032 })
1033 .pipe(Effect.flip);
1034
1035 assert.deepInclude(error, {
1036 _tag: "GitCommandError",
1037 operation: "GitVcsDriver.test.worktreeAddConflict",
1038 reason: "branch_checked_out_in_worktree",
1039 });
1040 assert.include(error.message, "(branch_checked_out_in_worktree)");
1041 assert.notInclude(error.message, firstWorktree);
1042 }),
1043 );
1044
1045 it.effect("names a missing repository behind a failed command", () =>
1046 Effect.gen(function* () {
1047 const cwd = yield* makeTmpDir();
1048 const driver = yield* GitVcsDriver.GitVcsDriver;
1049
1050 const error = yield* driver
1051 .execute({
1052 operation: "GitVcsDriver.test.notARepository",
1053 cwd,
1054 args: ["rev-parse", "--abbrev-ref", "HEAD"],
1055 env: { LC_ALL: "C" },
1056 })
1057 .pipe(Effect.flip);
1058
1059 assert.deepInclude(error, {
1060 _tag: "GitCommandError",
1061 operation: "GitVcsDriver.test.notARepository",
1062 reason: "not_a_repository",
1063 });
1064 assert.include(error.message, "(not_a_repository)");
1065 }),
1066 );
1067
1068 it.effect("appends the reason tag to a caller-supplied detail", () =>
1069 Effect.gen(function* () {
1070 const parent = yield* makeTmpDir();
1071 const pathService = yield* Path.Path;
1072 const cwd = pathService.join(parent, "repo");
1073 const fileSystem = yield* FileSystem.FileSystem;
1074 yield* fileSystem.makeDirectory(cwd);
1075 const driver = yield* GitVcsDriver.GitVcsDriver;
1076 yield* initRepoWithCommit(cwd);
1077 yield* git(cwd, [
1078 "worktree",
1079 "add",
1080 pathService.join(parent, "taken"),
1081 "-b",
1082 "taken-branch",
1083 ]);
1084
1085 const error = yield* driver
1086 .createWorktree({
1087 cwd,
1088 refName: "taken-branch",
1089 path: pathService.join(parent, "second"),
1090 })
1091 .pipe(Effect.flip);
1092
1093 assert.equal(error.detail, "git worktree add failed");
1094 assert.include(error.message, "git worktree add failed (branch_checked_out_in_worktree)");
1095 }),
1096 );
1097
1098 it.effect.each([
1099 {
1100 label: "a refused key",
1101 sshMessage: "git@example.invalid: Permission denied (publickey).",
1102 expectedReason: "authentication_failed" as const,
1103 expectedText: "(authentication_failed)",
1104 },
1105 {
1106 label: "a refused key among several methods",
1107 sshMessage: "git@example.invalid: Permission denied (publickey,password).",
1108 expectedReason: "authentication_failed" as const,
1109 expectedText: "(authentication_failed)",
1110 },
1111 {
1112 label: "a refused keyboard-interactive attempt",
1113 sshMessage: "git@example.invalid: Permission denied (keyboard-interactive).",
1114 expectedReason: "authentication_failed" as const,
1115 expectedText: "(authentication_failed)",
1116 },
1117 {
1118 label: "a rejected password",
1119 sshMessage: "Permission denied, please try again.",
1120 expectedReason: "authentication_failed" as const,
1121 expectedText: "(authentication_failed)",
1122 },
1123 {
1124 label: "an untrusted host key",
1125 sshMessage: "Host key verification failed.",
1126 expectedReason: "host_key_unverified" as const,
1127 expectedText: "(host_key_unverified)",
1128 },
1129 ])(
1130 "names $label rather than an unreachable remote",
1131 ({ sshMessage, expectedReason, expectedText }) =>
1132 Effect.gen(function* () {
1133 const parent = yield* makeTmpDir();
1134 const pathService = yield* Path.Path;
1135 const cwd = pathService.join(parent, "repo");
1136 const fileSystem = yield* FileSystem.FileSystem;
1137 const driver = yield* GitVcsDriver.GitVcsDriver;
1138 yield* fileSystem.makeDirectory(cwd);
1139 yield* initRepoWithCommit(cwd);
1140 yield* git(cwd, ["remote", "add", "origin", "git@example.invalid:owner/repo.git"]);
1141 // Stands in for ssh, which prints its refusal unprefixed and leaves
1142 // git to add only a generic "could not read" line after it.
1143 const fakeSsh = pathService.join(parent, "fake-ssh");
1144 yield* writeTextFile(
1145 parent,
1146 "fake-ssh",
1147 `#!/bin/sh\necho "${sshMessage}" >&2\nexit 255\n`,
1148 );
1149 yield* fileSystem.chmod(fakeSsh, 0o755);
1150
1151 const error = yield* driver
1152 .execute({
1153 operation: "GitVcsDriver.test.sshRefusal",
1154 cwd,
1155 args: ["fetch", "origin"],
1156 env: { LC_ALL: "C", GIT_SSH_COMMAND: fakeSsh },
1157 })
1158 .pipe(Effect.flip);
1159
1160 assert.deepInclude(error, { reason: expectedReason });
1161 assert.include(error.message, expectedText);
1162 }),
1163 );
1164
1165 it.effect("does not read a filesystem permission error as an ssh refusal", () =>
1166 Effect.gen(function* () {
1167 const parent = yield* makeTmpDir();
1168 const pathService = yield* Path.Path;
1169 const cwd = pathService.join(parent, "repo");
1170 const fileSystem = yield* FileSystem.FileSystem;
1171 const driver = yield* GitVcsDriver.GitVcsDriver;
1172 yield* fileSystem.makeDirectory(cwd);
1173 yield* initRepoWithCommit(cwd);
1174 yield* git(cwd, ["remote", "add", "origin", "git@example.invalid:owner/repo.git"]);
1175 const fakeSsh = pathService.join(parent, "fake-ssh");
1176 yield* writeTextFile(
1177 parent,
1178 "fake-ssh",
1179 '#!/bin/sh\necho "fatal: cannot open backup file: Permission denied (os error 13)" >&2\nexit 255\n',
1180 );
1181 yield* fileSystem.chmod(fakeSsh, 0o755);
1182
1183 const error = yield* driver
1184 .execute({
1185 operation: "GitVcsDriver.test.osPermission",
1186 cwd,
1187 args: ["fetch", "origin"],
1188 env: { LC_ALL: "C", GIT_SSH_COMMAND: fakeSsh },
1189 })
1190 .pipe(Effect.flip);
1191
1192 assert.notInclude(error.message, "(authentication_failed)");
1193 }),
1194 );
1195
1196 it.effect("does not read remote hook output as a git failure reason", () =>
1197 Effect.gen(function* () {
1198 const parent = yield* makeTmpDir();
1199 const pathService = yield* Path.Path;
1200 const remote = pathService.join(parent, "origin.git");
1201 const cwd = pathService.join(parent, "work");
1202 const fileSystem = yield* FileSystem.FileSystem;
1203 const driver = yield* GitVcsDriver.GitVcsDriver;
1204 yield* fileSystem.makeDirectory(cwd);
1205 yield* initRepoWithCommit(cwd);
1206 yield* git(cwd, ["init", "--bare", remote]);
1207 yield* git(cwd, ["remote", "add", "origin", remote]);
1208 // Git prefixes everything the server says with `remote:`, so a remote
1209 // hook can put arbitrary text in front of the classifier.
1210 const preReceive = pathService.join(remote, "hooks", "pre-receive");
1211 yield* writeTextFile(
1212 remote,
1213 "hooks/pre-receive",
1214 '#!/bin/sh\necho "authentication failed" >&2\necho "Permission denied (publickey)." >&2\necho "Host key verification failed." >&2\nexit 1\n',
1215 );
1216 yield* fileSystem.chmod(preReceive, 0o755);
1217
1218 const error = yield* driver
1219 .execute({
1220 operation: "GitVcsDriver.test.remoteHookOutput",
1221 cwd,
1222 args: ["push", "origin", "HEAD"],
1223 env: { LC_ALL: "C" },
1224 })
1225 .pipe(Effect.flip);
1226
1227 assert.notProperty(error, "reason");
1228 assert.notInclude(error.message, "(authentication_failed)");
1229 }),
1230 );
1231
1232 it.effect("does not read hook output as a git failure reason", () =>
1233 Effect.gen(function* () {
1234 const parent = yield* makeTmpDir();
1235 const pathService = yield* Path.Path;
1236 const remote = pathService.join(parent, "origin.git");
1237 const cwd = pathService.join(parent, "work");
1238 const fileSystem = yield* FileSystem.FileSystem;
1239 const driver = yield* GitVcsDriver.GitVcsDriver;
1240 yield* fileSystem.makeDirectory(cwd);
1241 yield* initRepoWithCommit(cwd);
1242 yield* git(cwd, ["init", "--bare", remote]);
1243 yield* git(cwd, ["remote", "add", "origin", remote]);
1244 yield* writeTextFile(
1245 cwd,
1246 ".git/hooks/pre-push",
1247 '#!/bin/sh\necho "authentication failed" >&2\nexit 1\n',
1248 );
1249 yield* fileSystem.chmod(pathService.join(cwd, ".git/hooks/pre-push"), 0o755);
1250
1251 const error = yield* driver
1252 .execute({
1253 operation: "GitVcsDriver.test.hookOutput",
1254 cwd,
1255 args: ["push", "origin", "HEAD"],
1256 env: { LC_ALL: "C" },
1257 })
1258 .pipe(Effect.flip);
1259
1260 assert.notProperty(error, "reason");
1261 assert.notInclude(error.message, "(authentication_failed)");
1262 }),
1263 );
1264
1265 it.effect("leaves a tag collision unclassified rather than calling it a path", () =>
1266 Effect.gen(function* () {
1267 const cwd = yield* makeTmpDir();
1268 const driver = yield* GitVcsDriver.GitVcsDriver;
1269 yield* initRepoWithCommit(cwd);
1270 yield* git(cwd, ["tag", "v1"]);
1271
1272 const error = yield* driver
1273 .execute({
1274 operation: "GitVcsDriver.test.tagCollision",
1275 cwd,
1276 args: ["tag", "v1"],
1277 env: { LC_ALL: "C" },
1278 })
1279 .pipe(Effect.flip);
1280
1281 assert.notProperty(error, "reason");
1282 assert.notInclude(error.message, "(path_already_exists)");
1010 }),1283 }),
1011 );1284 );
10121285
apps/server/src/vcs/GitVcsDriverCore.ts+71-4
...@@ -20,6 +20,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/process";...@@ -20,6 +20,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/process";
2020
21import {21import {
22 GitCommandError,22 GitCommandError,
23 type GitCommandFailureReason,
23 T3_PROJECT_FILE_NAME,24 T3_PROJECT_FILE_NAME,
24 type ReviewDiffFileContentsInput,25 type ReviewDiffFileContentsInput,
25 type ReviewDiffPreviewInput,26 type ReviewDiffPreviewInput,
...@@ -430,6 +431,65 @@ function gitCommandContext(...@@ -430,6 +431,65 @@ function gitCommandContext(
430 } as const;431 } as const;
431}432}
432433
434// Git states the actual cause on stderr, but stderr never leaves this module:
435// it echoes argv and remote URLs, which can carry credentials. Matching it
436// against fixed patterns yields a tag the caller can act on and a message that
437// quotes none of the matched text. Patterns run in order, specific first.
438const GIT_FAILURE_REASON_PATTERNS: ReadonlyArray<readonly [RegExp, GitCommandFailureReason]> = [
439 [/would clobber existing tag/i, "tag_would_be_clobbered"],
440 [/is already (?:used by worktree at|checked out at)/i, "branch_checked_out_in_worktree"],
441 [/a branch named .+ already exists/i, "branch_already_exists"],
442 // ssh names the methods it tried, so the parenthesized list varies:
443 // `(publickey)`, `(publickey,password)`, `(keyboard-interactive)`.
444 [
445 /(?:authentication failed|could not read Username|could not read Password|permission denied \([a-z-]+(?:,[a-z-]+)*\)|permission denied, please try again)/i,
446 "authentication_failed",
447 ],
448 // Distinct from a credential failure: the remote answered and the key it
449 // presented is untrusted, so pointing the user at credentials would misdirect.
450 [/host key verification failed/i, "host_key_unverified"],
451 [
452 /(?:could not read from remote repository|does not appear to be a git repository|repository .+ not found)/i,
453 "remote_unreachable",
454 ],
455 // Quoted-path forms only: an unquoted `fatal: <thing> already exists` also
456 // covers tag and ref collisions, which are not path collisions.
457 [/fatal: '[^']+' already exists|destination path .+ already exists/i, "path_already_exists"],
458];
459
460// Hooks write to the same stream git does, and nothing distinguishes their
461// text from git's: a pre-push hook echoing "authentication failed" would
462// otherwise be read as a credential failure. Git prefixes its own diagnostics
463// and states a push rejection on a `! [rejected]` line, so only those are
464// classified. `remote:` is deliberately excluded — git prefixes every byte the
465// server sends that way, remote hook output included, so trusting it would
466// reintroduce the same false positive from the other end of the connection.
467const GIT_DIAGNOSTIC_LINE_PATTERN = /^(?:fatal|error):|^!\s|^\s+!\s/;
468// ssh reports the refusal itself, unprefixed, and git only adds a generic
469// "Could not read from remote repository" after it. Dropping ssh's line would
470// leave that generic one to be read as an unreachable remote when the real
471// cause is credentials, so these specific refusals are classified too.
472const SSH_TRANSPORT_REFUSAL_PATTERN =
473 /Permission denied \((?:publickey|password|keyboard-interactive)|Permission denied, please try again|Host key verification failed/i;
474
475function classifyGitFailure(stderr: string): GitCommandFailureReason | null {
476 const diagnostics = stderr
477 .split(/\r?\n/)
478 .filter(
479 (line) =>
480 GIT_DIAGNOSTIC_LINE_PATTERN.test(line) ||
481 // A remote hook can echo ssh's wording; only the local ssh's line counts.
482 (!/^remote:/.test(line) && SSH_TRANSPORT_REFUSAL_PATTERN.test(line)),
483 )
484 .join("\n");
485 if (diagnostics.length === 0) return null;
486 if (isNonRepositoryGitStderr(diagnostics)) return "not_a_repository";
487 for (const [pattern, reason] of GIT_FAILURE_REASON_PATTERNS) {
488 if (pattern.test(diagnostics)) return reason;
489 }
490 return null;
491}
492
433function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null {493function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null {
434 const trimmed = value.trim();494 const trimmed = value.trim();
435 const prefix = `refs/remotes/${remoteName}/`;495 const prefix = `refs/remotes/${remoteName}/`;
...@@ -932,8 +992,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -932,8 +992,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
932 yield* trace2Monitor.flush;992 yield* trace2Monitor.flush;
933993
934 if (!input.allowNonZeroExit && exitCode !== 0) {994 if (!input.allowNonZeroExit && exitCode !== 0) {
995 const reason = classifyGitFailure(stderr.text);
935 return yield* new GitCommandError({996 return yield* new GitCommandError({
936 ...gitCommandContext(commandInput),997 ...gitCommandContext(commandInput),
998 ...(reason === null ? {} : { reason }),
937 detail: "Git command exited with a non-zero status.",999 detail: "Git command exited with a non-zero status.",
938 exitCode,1000 exitCode,
939 stdoutLength: stdout.text.length,1001 stdoutLength: stdout.text.length,
...@@ -1014,17 +1076,22 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -1014,17 +1076,22 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
1014 : {}),1076 : {}),
1015 ...(options.progress ? { progress: options.progress } : {}),1077 ...(options.progress ? { progress: options.progress } : {}),
1016 }).pipe(1078 }).pipe(
1017 Effect.filterOrFail(1079 Effect.flatMap((result) => {
1018 (result) => options.allowNonZeroExit || result.exitCode === 0,1080 if (options.allowNonZeroExit || result.exitCode === 0) {
1019 (result) =>1081 return Effect.succeed(result);
1082 }
1083 const reason = classifyGitFailure(result.stderr);
1084 return Effect.fail(
1020 new GitCommandError({1085 new GitCommandError({
1021 ...gitCommandContext({ operation, cwd, args }),1086 ...gitCommandContext({ operation, cwd, args }),
1087 ...(reason === null ? {} : { reason }),
1022 detail: options.fallbackErrorDetail ?? "Git command exited with a non-zero status.",1088 detail: options.fallbackErrorDetail ?? "Git command exited with a non-zero status.",
1023 ...(result.exitCode === null ? {} : { exitCode: result.exitCode }),1089 ...(result.exitCode === null ? {} : { exitCode: result.exitCode }),
1024 stdoutLength: result.stdout.length,1090 stdoutLength: result.stdout.length,
1025 stderrLength: result.stderr.length,1091 stderrLength: result.stderr.length,
1026 }),1092 }),
1027 ),1093 );
1094 }),
1028 );1095 );
10291096
1030 const executeGitWithStableDiagnostics = (1097 const executeGitWithStableDiagnostics = (
packages/contracts/src/git.ts+20-1
...@@ -356,6 +356,23 @@ export const VcsPullResult = Schema.Struct({...@@ -356,6 +356,23 @@ export const VcsPullResult = Schema.Struct({
356export type VcsPullResult = typeof VcsPullResult.Type;356export type VcsPullResult = typeof VcsPullResult.Type;
357357
358// RPC / domain errors358// RPC / domain errors
359
360// Well-known git failures, recognized from stderr at the driver and carried as
361// a closed set of diagnostic tags. Git's stderr itself stays off the error: it
362// echoes argv and remote URLs, which can hold credentials. The tag names the
363// cause for logs and callers; it does not select a message.
364export const GitCommandFailureReason = Schema.Literals([
365 "authentication_failed",
366 "branch_already_exists",
367 "branch_checked_out_in_worktree",
368 "host_key_unverified",
369 "not_a_repository",
370 "path_already_exists",
371 "remote_unreachable",
372 "tag_would_be_clobbered",
373]);
374export type GitCommandFailureReason = typeof GitCommandFailureReason.Type;
375
359export class GitCommandError extends Schema.TaggedError<GitCommandError>()("GitCommandError", {376export class GitCommandError extends Schema.TaggedError<GitCommandError>()("GitCommandError", {
360 operation: Schema.String,377 operation: Schema.String,
361 command: Schema.String,378 command: Schema.String,
...@@ -365,11 +382,13 @@ export class GitCommandError extends Schema.TaggedError<GitCommandError>()("GitC...@@ -365,11 +382,13 @@ export class GitCommandError extends Schema.TaggedError<GitCommandError>()("GitC
365 stdoutLength: Schema.optional(Schema.Number),382 stdoutLength: Schema.optional(Schema.Number),
366 stderrLength: Schema.optional(Schema.Number),383 stderrLength: Schema.optional(Schema.Number),
367 outputLength: Schema.optional(Schema.Number),384 outputLength: Schema.optional(Schema.Number),
385 reason: Schema.optional(GitCommandFailureReason),
368 detail: Schema.String,386 detail: Schema.String,
369 cause: Schema.optional(Schema.Defect()),387 cause: Schema.optional(Schema.Defect()),
370}) {388}) {
371 override get message(): string {389 override get message(): string {
372 return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}`;390 const reason = this.reason === undefined ? "" : ` (${this.reason})`;
391 return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}${reason}`;
373 }392 }
374}393}
375394