Preserve commit hook output attribution

Author: tim-smart

apps/server/src/git/GitManager.test.ts

@@ -3639,6 +3639,63 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
     }),
   );
 
+  it.effect("does not attribute ambiguous output to the wrong commit hook", () =>
+    Effect.gen(function* () {
+      const repoDir = yield* makeTempDir("t3code-git-manager-");
+      yield* initRepo(repoDir);
+      NodeFS.writeFileSync(NodePath.join(repoDir, "multi-hook.txt"), "multi hook\n");
+      NodeFS.writeFileSync(
+        NodePath.join(repoDir, ".git", "hooks", "pre-commit"),
+        '#!/bin/sh\necho "output from pre-commit" >&2\n',
+        { mode: 0o755 },
+      );
+      NodeFS.writeFileSync(
+        NodePath.join(repoDir, ".git", "hooks", "commit-msg"),
+        '#!/bin/sh\necho "output from commit-msg" >&2\n',
+        { mode: 0o755 },
+      );
+
+      const { manager } = yield* makeManager();
+      const events: GitActionProgressEvent[] = [];
+
+      const result = yield* runStackedAction(
+        manager,
+        {
+          cwd: repoDir,
+          action: "commit",
+          commitMessage: "test: preserve hook output attribution",
+        },
+        {
+          actionId: "action-multi-hook",
+          progressReporter: {
+            publish: (event) =>
+              Effect.sync(() => {
+                events.push(event);
+              }),
+          },
+        },
+      );
+
+      expect(result.commit.status).toBe("created");
+      const hookOutput = events.filter((event) => event.kind === "hook_output");
+      const preCommitOutput = hookOutput.find((event) =>
+        event.text.includes("output from pre-commit"),
+      );
+      const commitMsgOutput = hookOutput.find((event) =>
+        event.text.includes("output from commit-msg"),
+      );
+      const gitOutput = hookOutput.find((event) =>
+        event.text.includes("test: preserve hook output attribution"),
+      );
+
+      expect(preCommitOutput).toBeDefined();
+      expect([null, "pre-commit"]).toContain(preCommitOutput?.hookName);
+      expect(commitMsgOutput).toBeDefined();
+      expect([null, "commit-msg"]).toContain(commitMsgOutput?.hookName);
+      expect(gitOutput).toMatchObject({ hookName: null });
+    }),
+  );
+
   it.effect("emits action_failed when a commit hook rejects", () =>
     Effect.gen(function* () {
       const repoDir = yield* makeTempDir("t3code-git-manager-");

apps/server/src/git/GitManager.ts

@@ -1382,9 +1382,10 @@ export const make = Effect.gen(function* () {
     });
 
     let currentHookName: string | null = null;
-    let pendingHookOutput: Array<{ stream: "stdout" | "stderr"; text: string }> = [];
+    let sawCommitHook = false;
+    let pendingUnattributedOutput: Array<{ stream: "stdout" | "stderr"; text: string }> = [];
     const emitHookOutput = (
-      hookName: string,
+      hookName: string | null,
       { stream, text }: { stream: "stdout" | "stderr"; text: string },
     ) => {
       const sanitized = sanitizeProgressText(text);
@@ -1398,32 +1399,33 @@ export const make = Effect.gen(function* () {
         text: sanitized,
       });
     };
+    const finalizeUnattributedOutput = (shouldEmit: boolean) =>
+      Effect.suspend(() => {
+        const pending = pendingUnattributedOutput;
+        pendingUnattributedOutput = [];
+        return shouldEmit
+          ? Effect.forEach(pending, (output) => emitHookOutput(null, output), { discard: true })
+          : Effect.void;
+      });
     const commitProgress =
       progressReporter && actionId
         ? {
             onOutputLine: (output: { stream: "stdout" | "stderr"; text: string }) =>
               Effect.suspend(() => {
                 if (currentHookName === null) {
-                  pendingHookOutput.push(output);
+                  pendingUnattributedOutput.push(output);
                   return Effect.void;
                 }
                 return emitHookOutput(currentHookName, output);
               }),
             onHookStarted: (hookName: string) =>
               Effect.suspend(() => {
+                sawCommitHook = true;
                 currentHookName = hookName;
-                const bufferedOutput = pendingHookOutput;
-                pendingHookOutput = [];
                 return emit({
                   kind: "hook_started",
                   hookName,
-                }).pipe(
-                  Effect.flatMap(() =>
-                    Effect.forEach(bufferedOutput, (output) => emitHookOutput(hookName, output), {
-                      discard: true,
-                    }),
-                  ),
-                );
+                });
               }),
             onHookFinished: ({
               hookName,
@@ -1446,11 +1448,19 @@ export const make = Effect.gen(function* () {
             },
           }
         : null;
-    const { commitSha } = yield* gitCore.commit(cwd, suggestion.subject, suggestion.body, {
-      timeoutMs: COMMIT_TIMEOUT_MS,
-      ...(disableSigning ? { disableSigning: true } : {}),
-      ...(commitProgress ? { progress: commitProgress } : {}),
-    });
+    const { commitSha } = yield* gitCore
+      .commit(cwd, suggestion.subject, suggestion.body, {
+        timeoutMs: COMMIT_TIMEOUT_MS,
+        ...(disableSigning ? { disableSigning: true } : {}),
+        ...(commitProgress ? { progress: commitProgress } : {}),
+      })
+      .pipe(
+        Effect.tapError((error) =>
+          finalizeUnattributedOutput(
+            sawCommitHook && error.failureKind !== "commit_signing_failed",
+          ),
+        ),
+      );
     if (currentHookName !== null) {
       yield* emit({
         kind: "hook_finished",
@@ -1460,7 +1470,7 @@ export const make = Effect.gen(function* () {
       });
       currentHookName = null;
     }
-    pendingHookOutput = [];
+    yield* finalizeUnattributedOutput(sawCommitHook);
     return {
       status: "created" as const,
       commitSha,