fix(server): a failed worktree hook shows its output and retry recovers

.macroscope/check-run-agents/effect-service-conventions.md

@@ -69,7 +69,7 @@ Review changed TypeScript for the conventions below. They apply when a pull requ
 - Define service failures with `Schema.TaggedError` and structured attributes: operation or stage, resource path or entity identifier, normalized category or status. Derive `message` from those attributes only. Never derive it from `cause`, `cause.message`, or a stringified defect, and do not add a `detail` field that copies `cause.message`.
 - When wrapping a real failure, keep the immediate underlying error as `cause` so the chain and stack survive. Make `cause` required if every construction wraps a failure. Pure validation or domain errors created without an underlying failure need no cause.
 - Keep attributes and log annotations safe and bounded: no raw wire payloads, command arguments or output, signed URLs, credentials, query strings, or arbitrary defect text. Preserve the exact value only as `cause`; expose normalized categories, lengths, counts, and safe URL protocol or hostname where useful.
-  The documented exception is `DeviceBootError.diagnostics`: device hub launch commands and failure text are intentionally exposed to authorized device users and agents, capped at 2,000 characters and included in the error message. Do not apply this exception to other service errors.
+  The documented exceptions are `DeviceBootError.diagnostics`, where device hub launch commands and failure text are intentionally exposed to authorized device users and agents, capped at 2,000 characters and included in the error message, and `GitCommandError.detail` for a failed worktree checkout, which carries hook or filter output through `withHookOutput` (Git's own diagnostics dropped, URL credentials and query strings redacted, capped at 1,000 characters). Do not apply these exceptions to other service errors.
 - At a translation boundary, pass through an already structured domain error when it is part of the target error channel; wrap only unknown or lower-level failures. Map failures where the context is known instead of wrapping a whole multi-step pipeline in one generic error.
 - Do not encode the same distinction twice with both a specific error tag and a single-value `operation`, `reason`, `kind`, or `phase` literal. Split into separate error classes when a discriminator drives caller control flow or the user-facing message; a discriminator used only for diagnostics may stay a field. Caller-visible messages exposed through HTTP, RPC, persisted state, or UI are behavior and must survive a structural refactor.
 - Do not add a helper whose only behavior is `(...args) => new SomeError({ ...args })`. Construct the error at the failure boundary. Keep a mapper only when it performs real normalization, passes through domain errors, or adds reusable context; when such a mapper belongs to the target error type, prefer a static factory on that class.

apps/server/src/vcs/GitVcsDriverCore.test.ts

@@ -562,7 +562,10 @@ it.effect.each([
         const metadataQueries = spawnedArgs.filter((args) => args.includes("--git-common-dir"));
         assert.deepStrictEqual(
           metadataQueries,
-          Array.from({ length: 2 }, () => ["rev-parse", "--git-common-dir"]),
+          Array.from({ length: hookExitCode === 0 ? 2 : 3 }, () => [
+            "rev-parse",
+            "--git-common-dir",
+          ]),
         );
         assert.isFalse(spawnedArgs.some((args) => args.includes("symbolic-ref")));
         invalidationFailure = "none";
@@ -573,11 +576,15 @@ it.effect.each([
         assert.isTrue(spawnedArgs.some((args) => args.includes("--git-common-dir")));
         const siblingRefs = yield* driver.listRefs({ cwd: siblingPath });
         const createdRef = siblingRefs.refs.find((ref) => ref.name === "feature/cache-created");
-        assert.isDefined(createdRef?.worktreePath);
-        assert.equal(
-          NodeFS.realpathSync.native(createdRef!.worktreePath!),
-          NodeFS.realpathSync.native(worktreePath),
-        );
+        if (hookExitCode === 0 || commonDirFailure !== "none") {
+          assert.isDefined(createdRef?.worktreePath);
+          assert.equal(
+            NodeFS.realpathSync.native(createdRef!.worktreePath!),
+            NodeFS.realpathSync.native(worktreePath),
+          );
+        } else {
+          assert.isUndefined(createdRef);
+        }
         assert.isTrue(siblingRefs.hasPrimaryRemote);
         assert.isTrue(siblingRefs.refs.find((ref) => ref.name === "cache-sibling")?.current);
         const statusAfter = yield* driver.statusDetailsLocal(siblingPath);
@@ -6343,6 +6350,90 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
       }),
     );
 
+    it.effect("reports a failed post-checkout hook's output and rolls back for a retry", () =>
+      Effect.gen(function* () {
+        const cwd = yield* makeTmpDir();
+        const { initialBranch } = yield* initRepoWithCommit(cwd);
+        const fs = yield* FileSystem.FileSystem;
+        const path = yield* Path.Path;
+        const driver = yield* GitVcsDriver.GitVcsDriver;
+        const hook = path.join(cwd, ".git/hooks/post-checkout");
+        yield* fs.writeFileString(
+          hook,
+          "#!/bin/sh\n" +
+            "echo \"This repository is configured for Git LFS but 'git-lfs' was not found on your path.\" >&2\n" +
+            "echo 'see https://user:hun@ter2@example.com/lfs?token=sig' >&2\n" +
+            `echo '{"first":"https://u:p@a.example","second":"https://v:s3cret@b.example"}' >&2\n` +
+            `echo '${"a".repeat(33)}://u:long-secret@example.com/lfs?key=long-token' >&2\n` +
+            "exit 2\n",
+        );
+        yield* fs.chmod(hook, 0o755);
+        const worktreePath = path.join(yield* makeTmpDir("git-worktrees-"), "retry");
+        // Relative to the repository, as Git resolves it.
+        const input = {
+          cwd,
+          path: path.relative(cwd, worktreePath),
+          refName: initialBranch,
+          newRefName: "t3/retry",
+        };
+
+        const error = yield* driver.createWorktree(input).pipe(Effect.flip);
+
+        assert.include(error.message, "'git-lfs' was not found on your path.");
+        assert.include(error.message, "https://<redacted>@example.com/lfs?<redacted>");
+        assert.notInclude(error.message, "long-secret");
+        assert.notInclude(error.message, "long-token");
+        assert.notInclude(error.message, "ter2");
+        assert.notInclude(error.message, "token=");
+        assert.notInclude(error.message, "s3cret");
+        assert.notInclude(error.message, "Preparing worktree");
+        assert.isFalse(yield* fs.exists(worktreePath));
+        assert.notInclude(yield* git(cwd, ["branch", "--list", "t3/retry"]), "t3/retry");
+
+        yield* fs.writeFileString(hook, "#!/bin/sh\nexit 0\n");
+        yield* driver.createWorktree(input);
+        assert.equal(yield* git(worktreePath, ["branch", "--show-current"]), "t3/retry");
+      }),
+    );
+
+    it.effect("keeps a worktree it did not create when a failed checkout rolls back", () =>
+      Effect.gen(function* () {
+        const cwd = yield* makeTmpDir();
+        const { initialBranch } = yield* initRepoWithCommit(cwd);
+        const fs = yield* FileSystem.FileSystem;
+        const path = yield* Path.Path;
+        const existing = path.join(yield* makeTmpDir("git-worktrees-"), "existing");
+        yield* git(cwd, ["worktree", "add", "-q", "-b", "other", existing, initialBranch]);
+        yield* writeTextFile(existing, "notes.txt", "uncommitted\n");
+        const real = yield* GitVcsDriver.GitVcsDriver;
+        const delegate = yield* ChildProcessSpawner.ChildProcessSpawner;
+        // Another process checks the new branch out at that path before the checkout runs.
+        const spawner = ChildProcessSpawner.make((command) =>
+          ChildProcess.isStandardCommand(command) &&
+          command.args.includes("worktree") &&
+          command.args.includes("add")
+            ? real
+                .execute({
+                  operation: "GitVcsDriver.test.race",
+                  cwd: existing,
+                  args: ["checkout", "-q", "t3/race"],
+                })
+                .pipe(Effect.orDie, Effect.andThen(delegate.spawn(command)))
+            : delegate.spawn(command),
+        );
+        const driver = yield* makeGitVcsDriverCore().pipe(
+          Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
+          Effect.provide(layerServerConfig),
+        );
+
+        yield* driver
+          .createWorktree({ cwd, path: existing, refName: initialBranch, newRefName: "t3/race" })
+          .pipe(Effect.flip);
+
+        assert.equal(yield* fs.readFileString(path.join(existing, "notes.txt")), "uncommitted\n");
+      }),
+    );
+
     // regression: #27749 hooks inherited the driver's forced LC_ALL=C
     it.effect("runs post-checkout hooks under the server's own locale", () =>
       Effect.gen(function* () {

apps/server/src/vcs/GitVcsDriverCore.ts

@@ -1,5 +1,4 @@
 import * as Cache from "effect/Cache";
-import * as Cause from "effect/Cause";
 import * as Clock from "effect/Clock";
 import * as Data from "effect/Data";
 import * as Deferred from "effect/Deferred";
@@ -52,6 +51,7 @@ import {
   gitConfigEnv,
   unsetInheritedRepositoryEnv,
 } from "./VcsProcess.ts";
+import { withHookOutput } from "./gitHookOutput.ts";
 import { makeWorktreeCheckoutCache } from "./worktreeCheckoutCache.ts";
 import { removeLinkedWorktree } from "./worktreeRemoval.ts";
 import { resolveWorktreesDirectory } from "../worktreesDirectory.ts";
@@ -217,6 +217,7 @@ interface ExecuteGitOptions {
   timeoutMs?: number | null | undefined;
   allowNonZeroExit?: boolean | undefined;
   fallbackErrorDetail?: string | undefined;
+  hookOutputInDetail?: boolean | undefined;
   env?: NodeJS.ProcessEnv | undefined;
   maxOutputBytes?: number | undefined;
   appendTruncationMarker?: boolean | undefined;
@@ -1266,11 +1267,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
           return Effect.succeed(result);
         }
         const reason = classifyGitFailure(result.stderr);
+        const detail = options.fallbackErrorDetail ?? gitFailureDetail(reason);
         return Effect.fail(
           new GitCommandError({
             ...gitCommandContext({ operation, cwd, args }),
             ...(reason === null ? {} : { reason }),
-            detail: options.fallbackErrorDetail ?? gitFailureDetail(reason),
+            detail: options.hookOutputInDetail ? withHookOutput(detail, result.stderr) : detail,
             ...(result.exitCode === null ? {} : { exitCode: result.exitCode }),
             stdoutLength: result.stdout.length,
             stderrLength: result.stderr.length,
@@ -5100,6 +5102,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
       ],
       {
         fallbackErrorDetail: "git worktree add failed",
+        hookOutputInDetail: true,
         // Large checkouts and post-checkout hooks can legitimately run for many
         // minutes; a fixed deadline kills them and leaves an orphaned worktree.
         timeoutMs: null,
@@ -5153,13 +5156,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
             )
           : Effect.void,
       ),
-      Effect.onError((cause) =>
+      Effect.onError(() =>
         newRefName && !claimed
-          ? // A failed hook keeps its worktree, as Git does; only a cancel abandons it.
-            (Cause.hasInterrupts(cause)
-              ? lockReason
-                ? releaseWorktreeRegistration(input.cwd, lockReason, true)
-                : Effect.void
+          ? (lockReason
+              ? releaseWorktreeRegistration(input.cwd, lockReason, true)
               : Effect.void
             ).pipe(
               Effect.andThen(

apps/server/src/vcs/gitHookOutput.ts

@@ -0,0 +1,27 @@
+// Git's own lines stay out: they are classified into a reason and can name private refs or paths.
+const GIT_OWN_LINE = /^(?:fatal|error|warning|hint):|^Preparing worktree |^Updating files:/;
+// Userinfo runs through the authority's last `@`; query strings can carry tokens or signatures.
+// Quotes and brackets end a URL, so adjacent ones redact separately. Only start a scheme at its boundary so scanning stays linear.
+const URL_PATTERN =
+  /(?<![a-z0-9+.-])([a-z][a-z0-9+.-]*:\/\/)([^\s/?#"'<>]*@)?([^\s?#"'<>]*)(\?[^\s#"'<>]*)?/gi;
+const MAX_LINES = 20;
+const MAX_CHARS = 1_000;
+
+const redactUrls = (text: string) =>
+  text.replace(
+    URL_PATTERN,
+    (_, scheme: string, userinfo?: string, rest?: string, query?: string) =>
+      `${scheme}${userinfo ? "<redacted>@" : ""}${rest ?? ""}${query ? "?<redacted>" : ""}`,
+  );
+
+/** Appends what a hook or filter printed, which often names the fix for a failed checkout. */
+export function withHookOutput(detail: string, stderr: string): string {
+  const lines = stderr
+    .split(/\r\n|\r|\n/)
+    .map((line) => line.trim())
+    .filter((line) => line.length > 0 && !GIT_OWN_LINE.test(line))
+    .slice(-MAX_LINES);
+  if (lines.length === 0) return detail;
+  const output = redactUrls(lines.join("\n"));
+  return `${detail}: ${output.length > MAX_CHARS ? `…${output.slice(-MAX_CHARS)}` : output}`;
+}

apps/server/src/vcs/worktreeCheckoutCache.ts

@@ -15,6 +15,7 @@ import * as Semaphore from "effect/Semaphore";
 
 import * as CheckoutCloner from "./CheckoutCloner.ts";
 import type * as GitVcsDriver from "./GitVcsDriver.ts";
+import { withHookOutput } from "./gitHookOutput.ts";
 import { indexEntryStat, parseIndex, restatIndex, type IndexEntryStat } from "./gitIndexStat.ts";
 
 const KEEP_PER_REPOSITORY = 3;
@@ -678,7 +679,10 @@ export const makeWorktreeCheckoutCache = Effect.fn("WorktreeCheckoutCache.make")
         operation: "GitVcsDriver.createWorktree",
         command: "git worktree add",
         cwd: input.cwd,
-        detail: "git worktree add failed: the post-checkout hook failed",
+        detail: withHookOutput(
+          "git worktree add failed: the post-checkout hook failed",
+          hook.stderr,
+        ),
         exitCode: hook.exitCode,
         stderrLength: hook.stderr.length,
       });

docs/internals/effect-services.md

@@ -135,7 +135,11 @@ optional-service layer. Don't route around the layer with an imperative runtime.
   lives only in `cause`. Expose a category, length, count, or a URL's protocol and host instead.
   `DeviceBootError.diagnostics` is a deliberate exception: authorized device users and agents
   need the device hub's launch command and failure text to diagnose host setup. It is capped at
-  2,000 characters and included in the error message; other service errors keep the rule above.
+  2,000 characters and included in the error message. `GitCommandError.detail` for a failed
+  worktree checkout is the other: a hook or filter's output often names the server setup fix (a
+  missing `git-lfs`). [gitHookOutput.ts](../../apps/server/src/vcs/gitHookOutput.ts) drops Git's
+  own diagnostics, redacts URL credentials and query strings, and caps it at 1,000 characters.
+  Other service errors keep the rule above.
 - **Translation.** Construct the error where the failure happens, and map it to a transport error
   only in the transport. A translation boundary passes through domain errors already in the target
   channel and wraps only unknown or lower-level failures. Map each failure where its context is

packages/contracts/src/git.ts

@@ -355,7 +355,7 @@ export const VcsPullResult = Schema.Struct({
 export type VcsPullResult = typeof VcsPullResult.Type;
 
 // Git stderr can echo credentials from argv and remote URLs. Only a fixed
-// reason reaches the error and its diagnostic suffix; newer, unknown reasons
+// reason reaches this field and the diagnostic suffix; newer, unknown reasons
 // decode as absent through GitCommandError.
 export const GitCommandFailureReason = Schema.Literals([
   "authentication_failed",