refactor(server): route git config through one env helper and end option parsing before names

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

@@ -1584,7 +1584,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
       yield* runGit(repoDir, ["push", "-u", "upstream", "main"]);
       yield* runGit(repoDir, ["checkout", "-b", "develop"]);
       yield* runGit(repoDir, ["push", "-u", "upstream", "develop"]);
-      yield* runGit(remoteDir, ["symbolic-ref", "HEAD", "refs/heads/develop"]);
+      yield* runGit(remoteDir, ["--git-dir=.", "symbolic-ref", "HEAD", "refs/heads/develop"]);
       yield* runGit(repoDir, ["remote", "set-head", "upstream", "develop"]);
 
       const { manager } = yield* makeManager({
@@ -4198,9 +4198,13 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
         ),
       ).toContain("?? .vercel/");
       expect(
-        yield* runGit(remoteDir, ["log", "-1", "--pretty=%s", "feature/push-dirty"]).pipe(
-          Effect.map((output) => output.stdout.trim()),
-        ),
+        yield* runGit(remoteDir, [
+          "--git-dir=.",
+          "log",
+          "-1",
+          "--pretty=%s",
+          "feature/push-dirty",
+        ]).pipe(Effect.map((output) => output.stdout.trim())),
       ).toBe("Push dirty branch");
     }),
   );
@@ -4249,7 +4253,11 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
       expect(result.pr.number).toBe(303);
       expect((yield* runGit(repoDir, ["status", "--porcelain"])).stdout).toBe(statusBefore);
       expect(
-        (yield* runGit(remoteDir, ["rev-parse", "feature/create-pr-only"])).stdout.trim(),
+        (yield* runGit(remoteDir, [
+          "--git-dir=.",
+          "rev-parse",
+          "feature/create-pr-only",
+        ])).stdout.trim(),
       ).toBe(headBefore);
       expect(
         ghCalls.some((call) =>
@@ -5009,7 +5017,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
       const remoteDir = yield* createBareRemote();
       yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
       yield* runGit(repoDir, ["push", "-u", "origin", "main"]);
-      yield* runGit(remoteDir, ["symbolic-ref", "HEAD", "refs/heads/main"]);
+      yield* runGit(remoteDir, ["--git-dir=.", "symbolic-ref", "HEAD", "refs/heads/main"]);
 
       const peerDir = yield* makeTempDir("t3code-git-peer-");
       yield* runGit(peerDir, ["clone", remoteDir, "."]);

apps/server/src/project/RepositoryIdentityResolver.ts

@@ -11,6 +11,7 @@ import * as Exit from "effect/Exit";
 import * as Layer from "effect/Layer";
 
 import * as ProcessRunner from "../processRunner.ts";
+import { explicitBareRepositoryEnv } from "../vcs/VcsProcess.ts";
 
 const DEFAULT_REPOSITORY_IDENTITY_CACHE_CAPACITY = 512;
 // Background sweeps resolve every project each minute. A long TTL keeps them
@@ -124,6 +125,7 @@ const resolveRepositoryIdentityCacheKey = Effect.fn("RepositoryIdentityResolver.
       .run({
         command: "git",
         args: ["-C", cwd, "rev-parse", "--show-toplevel"],
+        env: explicitBareRepositoryEnv(process.env),
         timeoutBehavior: "timedOutResult",
       })
       .pipe(Effect.option);
@@ -146,6 +148,7 @@ const resolveRepositoryIdentityFromCacheKey = Effect.fn(
     .run({
       command: "git",
       args: ["-C", cacheKey, "remote", "-v"],
+      env: explicitBareRepositoryEnv(process.env),
       timeoutBehavior: "timedOutResult",
     })
     .pipe(Effect.option);

apps/server/src/pullRequest/PullRequestReviewGuideService.ts

@@ -158,7 +158,7 @@ const make = Effect.gen(function* () {
       git.execute({
         operation: "PullRequestReviewGuideService.fetch",
         cwd: workspaceRoot,
-        args: ["fetch", "--quiet", "--no-tags", remote, ...args],
+        args: ["fetch", "--quiet", "--no-tags", "--", remote, ...args],
         timeoutMs: 5 * 60_000,
       });
     // The host names the base branch. A name git rejects, or one that would change the refspec

apps/server/src/pullRequest/PullRequestService.test.ts

@@ -7714,3 +7714,79 @@ it.effect.each([
       assert.deepStrictEqual(calls, [expected]);
     }),
 );
+
+it.effect("expands only files the change request changes, across every slice of its diff", () =>
+  Effect.gen(function* () {
+    const read: Array<string> = [];
+    const service = yield* makeService({
+      projects: [
+        project({ id: "p1", title: "t3code", workspaceRoot: "/a", repository: "pingdotgg/t3code" }),
+      ],
+      providers: [
+        fakeProvider("github", {
+          getDiff: (input) =>
+            Effect.succeed(
+              input.cursor === undefined
+                ? {
+                    patch: [
+                      "diff --git a/src/old.ts b/src/new.ts",
+                      "rename from src/old.ts",
+                      "rename to src/new.ts",
+                      "--- a/src/old.ts",
+                      "+++ b/src/new.ts",
+                      "",
+                    ].join("\n"),
+                    truncated: false,
+                    nextCursor: "2",
+                  }
+                : {
+                    patch: [
+                      "diff --git a/gone.md b/gone.md",
+                      "deleted file mode 100644",
+                      "--- a/gone.md",
+                      "+++ /dev/null",
+                      "",
+                    ].join("\n"),
+                    truncated: false,
+                    nextCursor: null,
+                  },
+            ),
+          getDiffFileContents: (input) =>
+            Effect.sync(() => {
+              read.push(`${input.oldPath}->${input.newPath}`);
+              return { oldContents: "old", newContents: "new" };
+            }),
+        }),
+      ],
+    });
+    const ref = { projectId: "p1" as ProjectId, repository: "pingdotgg/t3code", number: 1 };
+
+    yield* service.diffFileContents({
+      ...ref,
+      changeType: "rename-changed",
+      oldPath: "src/old.ts",
+      newPath: "src/new.ts",
+    });
+    yield* service.diffFileContents({
+      ...ref,
+      changeType: "deleted",
+      oldPath: "gone.md",
+      newPath: "gone.md",
+    });
+    const outside = yield* service
+      .diffFileContents({ ...ref, changeType: "deleted", oldPath: ".env", newPath: ".env" })
+      .pipe(Effect.flip);
+    const halfRename = yield* service
+      .diffFileContents({
+        ...ref,
+        changeType: "change",
+        oldPath: "src/secret.ts",
+        newPath: "src/new.ts",
+      })
+      .pipe(Effect.flip);
+
+    assert.strictEqual(outside._tag, "PullRequestOperationError");
+    assert.strictEqual(halfRename._tag, "PullRequestOperationError");
+    assert.deepStrictEqual(read, ["src/old.ts->src/new.ts", "gone.md->gone.md"]);
+  }),
+);

apps/server/src/pullRequest/PullRequestService.ts

@@ -4,6 +4,7 @@ import {
   sourceControlRepositorySelector,
 } from "@t3tools/shared/sourceControl";
 import { normalizeGitRemoteUrl } from "@t3tools/shared/git";
+import { parseDiffFilePaths } from "@t3tools/source-control-core/server/diffFileRevisions";
 import * as Cache from "effect/Cache";
 import * as Cause from "effect/Cause";
 import * as Clock from "effect/Clock";
@@ -1717,7 +1718,7 @@ export const make = Effect.gen(function* () {
       ),
     );
 
-  const diffFileContents: PullRequestService["Service"]["diffFileContents"] = (input) =>
+  const readDiffFileContents: PullRequestService["Service"]["diffFileContents"] = (input) =>
     requireProject(input).pipe(
       Effect.flatMap((project) => {
         const read = project.api.getDiffFileContents;
@@ -3248,6 +3249,52 @@ export const make = Effect.gen(function* () {
         );
       });
 
+  const cachedDiff = credentialCached(diff);
+  /**
+   * Whether the change request, or the one commit asked for, changes this file under these names.
+   * Walks the same cached slices the reader was shown, so the file being expanded is already held.
+   */
+  const changesFile = (input: PullRequestDiffFileContentsInput) => {
+    const named = (patch: string) =>
+      parseDiffFilePaths(patch).some(
+        (file) =>
+          (file.oldPath ?? file.newPath) === input.oldPath &&
+          (file.newPath ?? file.oldPath) === input.newPath,
+      );
+    const walk = (cursor: string | undefined): Effect.Effect<boolean, PullRequestError> =>
+      cachedDiff({
+        ...input,
+        ...(cursor === undefined ? {} : { cursor }),
+        ...(input.commit === undefined ? {} : { commit: input.commit }),
+      }).pipe(
+        Effect.flatMap((slice) =>
+          named(slice.patch)
+            ? Effect.succeed(true)
+            : slice.nextCursor === null
+              ? Effect.succeed(false)
+              : walk(slice.nextCursor),
+        ),
+      );
+    return walk(undefined);
+  };
+  // Only the files the change request itself changes: these are read with the project's own
+  // credential, and a reader who may look at the change request may not thereby read the rest of
+  // the repository.
+  const diffFileContents: PullRequestService["Service"]["diffFileContents"] = (input) =>
+    changesFile(input).pipe(
+      Effect.flatMap((changed) =>
+        changed
+          ? readDiffFileContents(input)
+          : Effect.fail(
+              new PullRequestOperationError({
+                operation: "diffFileContents",
+                detail: "This file is not part of the change request.",
+                reason: "not-found",
+              }),
+            ),
+      ),
+    );
+
   return PullRequestService.of({
     routing,
     routingIdentity,

apps/server/src/sourceControl/githubStackRebase.test.ts

@@ -64,11 +64,11 @@ const setup = Effect.gen(function* () {
   yield* git(work, "add", "-A");
   yield* git(work, "commit", "--quiet", "-m", "Docs");
   yield* git(work, "push", "--quiet", "origin", "main", "feat/answer-42", "feat/answer-doc");
-  const head = (branch: string) => git(remote, "rev-parse", `refs/heads/${branch}`);
+  const head = (branch: string) => git(remote, "--git-dir=.", "rev-parse", `refs/heads/${branch}`);
   return {
     remote,
     head,
-    git: (...args: string[]) => git(remote, ...args),
+    git: (...args: string[]) => git(remote, "--git-dir=.", ...args),
     layers: [
       { number: 1, headBranch: "feat/answer-42", headSha: yield* head("feat/answer-42") },
       { number: 2, headBranch: "feat/answer-doc", headSha: yield* head("feat/answer-doc") },

apps/server/src/storageCleanup.ts

@@ -835,7 +835,7 @@ const make = Effect.gen(function* () {
           yield* cleanupGit(worktreePath, ["clean", "-ffdx"], "git clean");
         yield* cleanupGit(
           project.workspaceRoot,
-          ["-c", "status.showUntrackedFiles=normal", "worktree", "remove", worktreePath],
+          ["-c", "status.showUntrackedFiles=normal", "worktree", "remove", "--", worktreePath],
           "git worktree remove",
         );
         entries.push({ ...entry, outcome: "removed", reason: sentence(removalReason), bytes });