From 115cf975817738b4ac36fce03ccc9ca88d337e4d Mon Sep 17 00:00:00 2001 From: Marvin Bitterlich <644950+marvin-bitterlich@users.noreply.github.com> Date: Sat, 14 Mar 2026 21:06:15 +0000 Subject: [PATCH] fix(git): disable external diff tools for app diffs --- .../checkpointing/Layers/CheckpointStore.ts | 9 ++- apps/server/src/git/Layers/GitCore.test.ts | 66 ++++++++++++++++++- apps/server/src/git/Layers/GitCore.ts | 55 +++++++++------- apps/server/src/git/diffArgs.ts | 3 + 4 files changed, 107 insertions(+), 26 deletions(-) create mode 100644 apps/server/src/git/diffArgs.ts diff --git a/apps/server/src/checkpointing/Layers/CheckpointStore.ts b/apps/server/src/checkpointing/Layers/CheckpointStore.ts index fac183ff7a1..e811bf8adc0 100644 --- a/apps/server/src/checkpointing/Layers/CheckpointStore.ts +++ b/apps/server/src/checkpointing/Layers/CheckpointStore.ts @@ -14,6 +14,7 @@ import { randomUUID } from "node:crypto"; import { Effect, Layer, FileSystem, Path } from "effect"; import { CheckpointInvariantError } from "../Errors.ts"; +import { machineReadableGitDiffArgs } from "../../git/diffArgs.ts"; import { GitCommandError } from "../../git/Errors.ts"; import { GitServiceLive } from "../../git/Layers/GitService.ts"; import { GitService } from "../../git/Services/GitService.ts"; @@ -244,7 +245,13 @@ const makeCheckpointStore = Effect.gen(function* () { const result = yield* git.execute({ operation, cwd: input.cwd, - args: ["diff", "--patch", "--minimal", "--no-color", fromCommitOid, toCommitOid], + args: machineReadableGitDiffArgs( + "--patch", + "--minimal", + "--no-color", + fromCommitOid, + toCommitOid, + ), }); return result.stdout; diff --git a/apps/server/src/git/Layers/GitCore.test.ts b/apps/server/src/git/Layers/GitCore.test.ts index 6c98229e8a7..66c2694bdd5 100644 --- a/apps/server/src/git/Layers/GitCore.test.ts +++ b/apps/server/src/git/Layers/GitCore.test.ts @@ -1,4 +1,4 @@ -import { existsSync } from "node:fs"; +import { chmodSync, existsSync } from "node:fs"; import path from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; @@ -223,6 +223,33 @@ function commitWithDate( }); } +function configureExternalDiff( + cwd: string, + scriptDir: string, +): Effect.Effect< + void, + GitCommandError | PlatformError.PlatformError, + GitService | FileSystem.FileSystem +> { + return Effect.gen(function* () { + const scriptPath = path.join( + scriptDir, + process.platform === "win32" ? "external-diff.cmd" : "external-diff.sh", + ); + const scriptContents = + process.platform === "win32" + ? "@echo off\r\necho external-diff %*\r\n" + : '#!/bin/sh\necho external-diff "$@"\n'; + + yield* writeTextFile(scriptPath, scriptContents); + if (process.platform !== "win32") { + chmodSync(scriptPath, 0o755); + } + + yield* git(cwd, ["config", "diff.external", scriptPath]); + }); +} + // ── Tests ── it.layer(TestLayer)("git integration", (it) => { @@ -1734,6 +1761,43 @@ it.layer(TestLayer)("git integration", (it) => { }), ); + it.effect("prepareCommitContext ignores configured external diff tools", () => + Effect.gen(function* () { + const tmp = yield* makeTmpDir(); + const externalDiffDir = yield* makeTmpDir("external-diff-test-"); + yield* initRepoWithCommit(tmp); + const core = yield* GitCore; + + yield* configureExternalDiff(tmp, externalDiffDir); + yield* writeTextFile(path.join(tmp, "README.md"), "new content\n"); + + const context = yield* core.prepareCommitContext(tmp); + expect(context).not.toBeNull(); + expect(context!.stagedPatch).toContain("diff --git a/README.md b/README.md"); + expect(context!.stagedPatch).not.toContain("external-diff"); + }), + ); + + it.effect("readRangeContext ignores configured external diff tools", () => + Effect.gen(function* () { + const tmp = yield* makeTmpDir(); + const externalDiffDir = yield* makeTmpDir("external-diff-test-"); + const { initialBranch } = yield* initRepoWithCommit(tmp); + const core = yield* GitCore; + + yield* git(tmp, ["checkout", "-b", "feature/external-diff"]); + yield* writeTextFile(path.join(tmp, "README.md"), "new content\n"); + yield* git(tmp, ["add", "README.md"]); + yield* git(tmp, ["commit", "-m", "update readme"]); + yield* configureExternalDiff(tmp, externalDiffDir); + + const context = yield* core.readRangeContext(tmp, initialBranch); + expect(context.diffSummary).toContain("README.md"); + expect(context.diffPatch).toContain("diff --git a/README.md b/README.md"); + expect(context.diffPatch).not.toContain("external-diff"); + }), + ); + it.effect("prepareCommitContext stages everything when filePaths is undefined", () => Effect.gen(function* () { const tmp = yield* makeTmpDir(); diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts index f5b9168abbb..3388d04d818 100644 --- a/apps/server/src/git/Layers/GitCore.ts +++ b/apps/server/src/git/Layers/GitCore.ts @@ -1,5 +1,6 @@ import { Cache, Data, Duration, Effect, Exit, FileSystem, Layer, Path } from "effect"; +import { machineReadableGitDiffArgs } from "../diffArgs.ts"; import { GitCommandError } from "../Errors.ts"; import { GitService } from "../Services/GitService.ts"; import { GitCore, type GitCoreShape } from "../Services/GitCore.ts"; @@ -663,12 +664,16 @@ const makeGitCore = Effect.gen(function* () { "--porcelain=2", "--branch", ]), - runGitStdout("GitCore.statusDetails.unstagedNumstat", cwd, ["diff", "--numstat"]), - runGitStdout("GitCore.statusDetails.stagedNumstat", cwd, [ - "diff", - "--cached", - "--numstat", - ]), + runGitStdout( + "GitCore.statusDetails.unstagedNumstat", + cwd, + machineReadableGitDiffArgs("--numstat"), + ), + runGitStdout( + "GitCore.statusDetails.stagedNumstat", + cwd, + machineReadableGitDiffArgs("--cached", "--numstat"), + ), ], { concurrency: "unbounded" }, ); @@ -782,21 +787,20 @@ const makeGitCore = Effect.gen(function* () { yield* runGit("GitCore.prepareCommitContext.addAll", cwd, ["add", "-A"]); } - const stagedSummary = yield* runGitStdout("GitCore.prepareCommitContext.stagedSummary", cwd, [ - "diff", - "--cached", - "--name-status", - ]).pipe(Effect.map((stdout) => stdout.trim())); + const stagedSummary = yield* runGitStdout( + "GitCore.prepareCommitContext.stagedSummary", + cwd, + machineReadableGitDiffArgs("--cached", "--name-status"), + ).pipe(Effect.map((stdout) => stdout.trim())); if (stagedSummary.length === 0) { return null; } - const stagedPatch = yield* runGitStdout("GitCore.prepareCommitContext.stagedPatch", cwd, [ - "diff", - "--cached", - "--patch", - "--minimal", - ]); + const stagedPatch = yield* runGitStdout( + "GitCore.prepareCommitContext.stagedPatch", + cwd, + machineReadableGitDiffArgs("--cached", "--patch", "--minimal"), + ); return { stagedSummary, @@ -970,13 +974,16 @@ const makeGitCore = Effect.gen(function* () { const [commitSummary, diffSummary, diffPatch] = yield* Effect.all( [ runGitStdout("GitCore.readRangeContext.log", cwd, ["log", "--oneline", range]), - runGitStdout("GitCore.readRangeContext.diffStat", cwd, ["diff", "--stat", range]), - runGitStdout("GitCore.readRangeContext.diffPatch", cwd, [ - "diff", - "--patch", - "--minimal", - range, - ]), + runGitStdout( + "GitCore.readRangeContext.diffStat", + cwd, + machineReadableGitDiffArgs("--stat", range), + ), + runGitStdout( + "GitCore.readRangeContext.diffPatch", + cwd, + machineReadableGitDiffArgs("--patch", "--minimal", range), + ), ], { concurrency: "unbounded" }, ); diff --git a/apps/server/src/git/diffArgs.ts b/apps/server/src/git/diffArgs.ts new file mode 100644 index 00000000000..2dd1e732b92 --- /dev/null +++ b/apps/server/src/git/diffArgs.ts @@ -0,0 +1,3 @@ +export function machineReadableGitDiffArgs(...args: ReadonlyArray): ReadonlyArray { + return ["diff", "--no-ext-diff", ...args]; +}