From ad5b8b793198d41b44bb0ff0e6a32dba3d44eff9 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Tue, 6 Oct 2026 23:35:26 -0400 Subject: [PATCH 1/2] fix(server): a failed commit says why instead of a generic git error Signed-off-by: Yordis Prieto --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 39 ++++++++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 36 +++++++++++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 82d070f87cd0..cb36cb0a3b4c 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -3415,6 +3415,45 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("says when a hook rejected the commit without echoing its output", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; + const pathService = yield* Path.Path; + + const hookOutput = "hook-output-secret"; + yield* writeTextFile( + cwd, + ".git/hooks/pre-commit", + `#!/bin/sh\necho "${hookOutput}" >&2\nexit 1\n`, + ); + yield* fileSystem.chmod(pathService.join(cwd, ".git/hooks/pre-commit"), 0o755); + yield* writeTextFile(cwd, "a.txt", "a\n"); + + const error = yield* driver + .commit(cwd, "Add a", "", { stage: { filePaths: ["a.txt"] } }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitCommandError"); + assert.include(error.message, "A Git hook rejected the commit."); + assert.notInclude(error.message, hookOutput); + }), + ); + + it.effect("says when there is nothing to commit", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + + const error = yield* driver.commit(cwd, "Empty", "").pipe(Effect.flip); + + assert.include(error.message, "There are no staged changes to commit."); + }), + ); + it.effect.each([false, true])( "preserves partial staging and split-index files (split: %s)", (split) => diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 92f7478fa34b..5b558570e8af 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -491,6 +491,28 @@ function classifyGitFailure(stderr: string): GitCommandFailureReason | null { return null; } +// Names why `git commit` failed without echoing its stderr. Git prints its own +// `fatal:` line for failures it owns; a rejecting hook exits with only the +// hook's output, so a failure without one is the hook's. +function describeCommitFailure({ + stdout, + stderr, +}: Pick): string { + if (/nothing to commit|no changes added to commit/i.test(stdout)) { + return "There are no staged changes to commit."; + } + if (/Author identity unknown|unable to auto-detect email address/i.test(stderr)) { + return "Git does not know who you are. Set user.name and user.email, then try again."; + } + if (/failed to write commit object|failed to sign the data/i.test(stderr)) { + return "Git could not sign the commit. Check that your signing key or agent is available."; + } + if (!/^fatal:/m.test(stderr)) { + return "A Git hook rejected the commit. Run the commit in a terminal to see the hook's output."; + } + return "Git command exited with a non-zero status."; +} + function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null { const trimmed = value.trim(); const prefix = `refs/remotes/${remoteName}/`; @@ -2250,10 +2272,20 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* onStderrLine: (line: string) => options.progress?.onOutputLine?.({ stream: "stderr", text: line }) ?? Effect.void, }; - yield* executeGit("GitVcsDriver.commit.commit", cwd, args, { + const commitResult = yield* executeGit("GitVcsDriver.commit.commit", cwd, args, { ...(options?.timeoutMs !== undefined ? { timeoutMs: options.timeoutMs } : {}), ...(progress ? { progress } : {}), - }).pipe(Effect.asVoid); + allowNonZeroExit: true, + }); + if (commitResult.exitCode !== 0) { + return yield* new GitCommandError({ + ...gitCommandContext({ operation: "GitVcsDriver.commit.commit", cwd, args }), + detail: describeCommitFailure(commitResult), + ...(commitResult.exitCode === null ? {} : { exitCode: commitResult.exitCode }), + stdoutLength: commitResult.stdout.length, + stderrLength: commitResult.stderr.length, + }); + } const commitSha = yield* runGitStdout("GitVcsDriver.commit.revParseHead", cwd, [ "rev-parse", "HEAD", From cd23d53fd18c73fb8a93a0a86445c4fc8876587e Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Tue, 6 Oct 2026 23:53:24 -0400 Subject: [PATCH 2/2] fix(server): show git's own output when a commit fails Signed-off-by: Yordis Prieto --- apps/server/src/git/GitManager.test.ts | 2 +- apps/server/src/vcs/GitVcsDriverCore.test.ts | 9 +++---- apps/server/src/vcs/GitVcsDriverCore.ts | 26 ++++++-------------- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 43eaa08de53a..9421e7af27fb 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -6041,7 +6041,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { ); expect(errorMessage).toContain("Git command failed in GitVcsDriver.commit.commit"); - expect(errorMessage).not.toContain("hook: fail"); + expect(errorMessage).toContain("hook: fail"); expect(events).toEqual( expect.arrayContaining([ expect.objectContaining({ diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index cb36cb0a3b4c..99c3209b8169 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -3415,7 +3415,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); - it.effect("says when a hook rejected the commit without echoing its output", () => + it.effect("surfaces the output of a hook that rejected the commit", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); yield* initRepoWithCommit(cwd); @@ -3423,7 +3423,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { const fileSystem = yield* FileSystem.FileSystem; const pathService = yield* Path.Path; - const hookOutput = "hook-output-secret"; + const hookOutput = "commit message is missing Signed-off-by"; yield* writeTextFile( cwd, ".git/hooks/pre-commit", @@ -3437,8 +3437,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { .pipe(Effect.flip); assert.equal(error._tag, "GitCommandError"); - assert.include(error.message, "A Git hook rejected the commit."); - assert.notInclude(error.message, hookOutput); + assert.include(error.message, hookOutput); }), ); @@ -3450,7 +3449,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { const error = yield* driver.commit(cwd, "Empty", "").pipe(Effect.flip); - assert.include(error.message, "There are no staged changes to commit."); + assert.include(error.message, "nothing to commit"); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 5b558570e8af..4a40f913e482 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -491,26 +491,16 @@ function classifyGitFailure(stderr: string): GitCommandFailureReason | null { return null; } -// Names why `git commit` failed without echoing its stderr. Git prints its own -// `fatal:` line for failures it owns; a rejecting hook exits with only the -// hook's output, so a failure without one is the hook's. -function describeCommitFailure({ +const COMMIT_FAILURE_OUTPUT_MAX_CHARS = 1_000; + +// `git commit` is local, so its output carries no remote URLs; it is surfaced +// because a rejecting hook's own words are the only explanation of why. +function commitFailureOutput({ stdout, stderr, }: Pick): string { - if (/nothing to commit|no changes added to commit/i.test(stdout)) { - return "There are no staged changes to commit."; - } - if (/Author identity unknown|unable to auto-detect email address/i.test(stderr)) { - return "Git does not know who you are. Set user.name and user.email, then try again."; - } - if (/failed to write commit object|failed to sign the data/i.test(stderr)) { - return "Git could not sign the commit. Check that your signing key or agent is available."; - } - if (!/^fatal:/m.test(stderr)) { - return "A Git hook rejected the commit. Run the commit in a terminal to see the hook's output."; - } - return "Git command exited with a non-zero status."; + const output = (stderr.trim() || stdout.trim()).slice(-COMMIT_FAILURE_OUTPUT_MAX_CHARS); + return output.length > 0 ? output : "Git command exited with a non-zero status."; } function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null { @@ -2280,7 +2270,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* if (commitResult.exitCode !== 0) { return yield* new GitCommandError({ ...gitCommandContext({ operation: "GitVcsDriver.commit.commit", cwd, args }), - detail: describeCommitFailure(commitResult), + detail: commitFailureOutput(commitResult), ...(commitResult.exitCode === null ? {} : { exitCode: commitResult.exitCode }), stdoutLength: commitResult.stdout.length, stderrLength: commitResult.stderr.length,