fix(cli): history show --diff works without git on PATH (#6475)

* fix(cli): history show --diff works without git on PATH

* fix(cli): unifiedDiff passes lint and fallow: blocks, shared direction rule, split test

* refactor(cli): split the diff walk and its backtrack into small steps
This commit is contained in:
Miguel Ángel
2026-10-27 01:23:02 +00:00
committed by GitHub
parent 5a28b4e7eb
commit a0b0bf5f64
4 changed files with 229 additions and 98 deletions
+1 -45
View File
@@ -1,5 +1,5 @@
// @vitest-environment node
import { spawn, type SpawnSyncOptionsWithStringEncoding } from "node:child_process";
import { spawn } from "node:child_process";
import {
chmodSync,
existsSync,
@@ -36,33 +36,6 @@ const clock = { at: 0 };
const advance = (ms: number) => void (clock.at += ms);
const tracked = vi.hoisted(() => [] as Array<{ action: string; via: string }>);
const outputRace = vi.hoisted(() => ({ enabled: false, replacements: 0 }));
vi.mock("node:child_process", async (importOriginal) => {
const original = await importOriginal<typeof import("node:child_process")>();
const { renameSync, writeFileSync } = await import("node:fs");
const { join } = await import("node:path");
return {
...original,
spawnSync(
command: string,
args: readonly string[],
options: SpawnSyncOptionsWithStringEncoding,
) {
const result = original.spawnSync(command, args, options);
if (
outputRace.enabled &&
command === "git" &&
args[0] === "diff" &&
typeof options.cwd === "string"
) {
renameSync(join(options.cwd, "diff"), join(options.cwd, "written-diff"));
writeFileSync(join(options.cwd, "diff"), "replacement-file-content");
outputRace.replacements++;
}
return result;
},
};
});
vi.mock("../telemetry/events.js", () => ({
trackHistoryAction: (props: { action: string; via: string }) => tracked.push(props),
}));
@@ -304,23 +277,6 @@ describe.each(["direct", "preview"])("hyperframes history (%s)", (mode) => {
expect(diff).toContain(`-${before}+${after.trimEnd()}`);
});
it("reads the written diff when its pathname is replaced after Git exits", async () => {
const { hf, turn } = await setup();
const entry = await turn("claude", "Retitle", "index.html", "A2");
outputRace.enabled = true;
outputRace.replacements = 0;
try {
const result = await hf("show", entry.id, "--diff", "--json");
expect(outputRace.replacements).toBe(1);
expect(result.code, result.err).toBe(0);
expect(JSON.parse(result.out).diff).toMatch(/-A\n\\ No newline at end of file\n\+A2/);
expect(result.out).not.toContain("replacement-file-content");
} finally {
outputRace.enabled = false;
}
});
it("peek reads a file as it was without writing, and restore puts every file back", async () => {
const { read, hf, turn } = await setup();
const entry = await turn("claude", "Retitle", "index.html", "A2");
+11 -53
View File
@@ -1,16 +1,3 @@
import { spawnSync } from "node:child_process";
import {
closeSync,
fstatSync,
mkdirSync,
mkdtempSync,
openSync,
readSync,
rmSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { defineCommand, type ArgsDef } from "citty";
import {
HISTORY_START,
@@ -25,6 +12,7 @@ import {
import type { Example } from "./_examples.js";
import { trackHistoryAction } from "../telemetry/events.js";
import { setCommandExitCode } from "../utils/commandResult.js";
import { unifiedDiff } from "../utils/unifiedDiff.js";
import {
Refusal,
endTurn,
@@ -204,48 +192,18 @@ function print(json: boolean, data: object, text: string): void {
}
async function textDiff(reader: Reader, entry: HistoryEntry): Promise<string> {
const work = mkdtempSync(join(tmpdir(), "hf-history-diff-"));
try {
const out: string[] = [];
for (const file of entry.files) {
const sides = await Promise.all(
[file.before, file.after].map((hash) => (hash ? reader.blob(hash) : Buffer.alloc(0))),
);
if (sides.some((bytes) => bytes.subarray(0, 8000).includes(0))) {
out.push(`Binary ${file.path} changed`);
continue;
}
const [a, b] = ["a", "b"].map((side, index) => {
const path = join(side, file.path);
mkdirSync(dirname(join(work, path)), { recursive: true });
writeFileSync(join(work, path), sides[index]!);
return path;
});
const outputPath = join(work, "diff");
const outputFd = openSync(outputPath, "w+", 0o600);
try {
const diff = spawnSync("git", ["diff", "--no-index", "--no-color", "--no-prefix", a!, b!], {
cwd: work,
encoding: "utf-8",
stdio: ["ignore", outputFd, "pipe"],
});
if (diff.error) throw new Refusal("--diff needs git on PATH");
const output = Buffer.alloc(fstatSync(outputFd).size);
let offset = 0;
while (offset < output.length) {
const count = readSync(outputFd, output, offset, output.length - offset, offset);
if (count === 0) break;
offset += count;
}
out.push(output.subarray(0, offset).toString("utf-8").trimEnd());
} finally {
closeSync(outputFd);
}
const out: string[] = [];
for (const file of entry.files) {
const sides = await Promise.all(
[file.before, file.after].map((hash) => (hash ? reader.blob(hash) : Buffer.alloc(0))),
);
if (sides.some((bytes) => bytes.subarray(0, 8000).includes(0))) {
out.push(`Binary ${file.path} changed`);
continue;
}
return out.join("\n");
} finally {
rmSync(work, { recursive: true, force: true });
out.push(unifiedDiff(file.path, sides[0]!.toString("utf-8"), sides[1]!.toString("utf-8")));
}
return out.join("\n");
}
/** `again` is the command to run once more with a choice: `undo <id>`, or `undo --who <name>` for a whole turn. */
+103
View File
@@ -0,0 +1,103 @@
import { spawnSync } from "node:child_process";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { describe, expect, it } from "vitest";
import { unifiedDiff } from "./unifiedDiff.js";
/** Applies a diff's hunks back onto `before` to check it reproduces `after`. */
function apply(before: string, diff: string): string {
const lines = before === "" ? [] : before.replace(/\n$/, "").split("\n");
const out: string[] = [];
let at = 0;
for (const row of diff.split("\n").slice(3)) {
if (row.startsWith("@@")) {
const first = Number(/-(\d+)/.exec(row)![1]);
const start = /-0,0/.test(row) ? 0 : first - 1;
out.push(...lines.slice(at, start));
at = start;
} else if (row.startsWith("\\")) continue;
else if (row.startsWith("+")) out.push(row.slice(1));
else if (row.startsWith("-")) at++;
else {
out.push(row.slice(1));
at++;
}
}
out.push(...lines.slice(at));
return out.join("\n");
}
function randomizer(seed: number) {
return (n: number) => (seed = (seed * 1103515245 + 12345) & 0x7fffffff) % n;
}
/** Two texts, the second the first with lines dropped and inserted, either with or without a final newline. */
function randomPair(rand: (n: number) => number): [string, string] {
const before = Array.from({ length: rand(25) }, () => `w${rand(6)}`);
const after = before.filter(() => rand(4) > 0);
for (let i = rand(5); i > 0; i--) after.splice(rand(after.length + 1), 0, `n${rand(9)}`);
const text = (lines: string[], bare: boolean) =>
lines.length ? lines.join("\n") + (bare ? "" : "\n") : "";
return [text(before, rand(3) === 0), text(after, rand(3) === 0)];
}
const bodyOf = (text: string) => text.slice(text.indexOf("@@")).trimEnd();
/** What git prints for the two texts, or undefined where git is not installed. */
function gitDiffBody(dir: string, a: string, b: string): string | undefined {
writeFileSync(join(dir, "a"), a);
writeFileSync(join(dir, "b"), b);
const git = spawnSync("git", ["diff", "--no-index", "--no-color", "--no-prefix", "a", "b"], {
cwd: dir,
encoding: "utf-8",
});
return git.error ? undefined : bodyOf(git.stdout);
}
describe("unifiedDiff", () => {
it("marks a missing final newline like git", () => {
expect(unifiedDiff("a.html", "A", "A2")).toBe(
"diff --git a/a.html b/a.html\n--- a/a.html\n+++ b/a.html\n@@ -1 +1 @@\n-A\n\\ No newline at end of file\n+A2\n\\ No newline at end of file",
);
});
it("is empty for equal text, and shows a whole added or deleted file", () => {
expect(unifiedDiff("a", "x\n", "x\n")).toBe("");
expect(unifiedDiff("a", "", "x\ny\n")).toContain("@@ -0,0 +1,2 @@\n+x\n+y");
expect(unifiedDiff("a", "x\ny\n", "")).toContain("@@ -1,2 +0,0 @@\n-x\n-y");
});
it("keeps three lines of context and splits distant changes into hunks", () => {
const lines = Array.from({ length: 30 }, (_, i) => `l${i}`);
const after = lines.map((l) => (l === "l2" || l === "l27" ? `${l}!` : l));
const diff = unifiedDiff("a", `${lines.join("\n")}\n`, `${after.join("\n")}\n`);
expect(diff.match(/^@@/gm)).toHaveLength(2);
expect(diff).toContain("@@ -1,6 +1,6 @@");
expect(diff).toContain("@@ -25,6 +25,6 @@");
});
it("matches git's own diff and reproduces the new text on random edits", () => {
const dir = mkdtempSync(join(tmpdir(), "hf-udiff-"));
try {
const rand = randomizer(7);
for (let round = 0; round < 40; round++) {
const [a, b] = randomPair(rand);
const mine = unifiedDiff("f", a, b);
expect(apply(a, mine).replace(/\n?$/, "")).toBe(b.replace(/\n$/, ""));
const git = gitDiffBody(dir, a, b);
if (git !== undefined) expect(bodyOf(mine)).toBe(a === b ? "" : git);
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it("shows a change past the edit cap as one replace of the changed middle", () => {
const lines = (prefix: string) => Array.from({ length: 2500 }, (_, i) => `${prefix}${i}`);
const text = (prefix: string) => `keep\n${lines(prefix).join("\n")}\n`;
const diff = unifiedDiff("a", text("x"), text("y"));
expect(apply(text("x"), diff)).toBe(text("y").trimEnd());
expect(diff).toContain("-x0\n-x1");
});
});
+114
View File
@@ -0,0 +1,114 @@
const CONTEXT = 3;
// Past this many changed lines Myers' trace grows quadratically; the middle is then shown as one replace.
const MAX_EDIT_LINES = 4000;
type Op = { kind: " " | "-" | "+"; text: string };
/** Lines of `text` with a marker on a last line that has no newline, so "A" and "A\n" differ as in git. */
function linesOf(text: string): string[] {
if (text === "") return [];
const lines = text.split("\n");
if (lines[lines.length - 1] === "") lines.pop();
else lines[lines.length - 1] += "\0";
return lines;
}
const display = (line: string): string =>
line.endsWith("\0") ? `${line.slice(0, -1)}\n\\ No newline at end of file` : line;
/** Edit script turning `a` into `b` (Myers), after the shared head and tail are set aside. */
function editScript(a: string[], b: string[]): Op[] {
let head = 0;
while (head < a.length && head < b.length && a[head] === b[head]) head++;
let tail = 0;
while (
tail < a.length - head &&
tail < b.length - head &&
a[a.length - 1 - tail] === b[b.length - 1 - tail]
)
tail++;
const x = a.slice(head, a.length - tail);
const y = b.slice(head, b.length - tail);
const same = (lines: string[]): Op[] => lines.map((text) => ({ kind: " ", text }));
return [...same(a.slice(0, head)), ...middle(x, y), ...same(a.slice(a.length - tail))];
}
const replaceAll = (a: string[], b: string[]): Op[] => [
...a.map((text): Op => ({ kind: "-", text })),
...b.map((text): Op => ({ kind: "+", text })),
];
/** Whether diagonal `k` of round `d` is reached by moving down (an insert) from the one before it. */
const movesDown = (v: Int32Array, offset: number, k: number, d: number) =>
k === -d || (k !== d && v[offset + k - 1]! < v[offset + k + 1]!);
/** How far along diagonal `k` of round `d` the walk gets in `a`, taking every shared line it meets. */
function reach(v: Int32Array, offset: number, k: number, d: number, a: string[], b: string[]) {
let x = movesDown(v, offset, k, d) ? v[offset + k + 1]! : v[offset + k - 1]! + 1;
while (x < a.length && x - k < b.length && a[x] === b[x - k]) x++;
return x;
}
function middle(a: string[], b: string[]): Op[] {
if (a.length + b.length > MAX_EDIT_LINES) return replaceAll(a, b);
const offset = a.length + b.length;
const v = new Int32Array(2 * offset + 2);
const trace: Int32Array[] = [];
for (let d = 0; ; d++) {
trace.push(v.slice());
for (let k = -d; k <= d; k += 2) {
const x = reach(v, offset, k, d, a, b);
v[offset + k] = x;
if (x >= a.length && x - k >= b.length) return backtrack(trace, a, b, offset, d);
}
}
}
/** Pushes the shared lines of `a` from index `x` back down to `to`, and returns `to`. */
function slideBack(ops: Op[], a: string[], x: number, to: number) {
for (; x > to; x--) ops.push({ kind: " ", text: a[x - 1]! });
return to;
}
function backtrack(trace: Int32Array[], a: string[], b: string[], offset: number, last: number) {
const ops: Op[] = [];
let x = a.length;
let y = b.length;
for (let d = last; d > 0; d--) {
const k = x - y;
const down = movesDown(trace[d]!, offset, k, d);
const prevX = trace[d]![offset + (down ? k + 1 : k - 1)]!;
x = slideBack(ops, a, x, down ? prevX : prevX + 1);
y = x - k;
if (down) ops.push({ kind: "+", text: b[--y]! });
else ops.push({ kind: "-", text: a[--x]! });
}
slideBack(ops, a, x, 0);
return ops.reverse();
}
/** A git-style unified diff of one text file, `after` empty for a deleted file and `before` empty for an added one. */
export function unifiedDiff(path: string, before: string, after: string): string {
const ops = editScript(linesOf(before), linesOf(after));
const changed = ops.flatMap((op, index) => (op.kind === " " ? [] : [index]));
if (changed.length === 0) return "";
const out = [`diff --git a/${path} b/${path}`, `--- a/${path}`, `+++ b/${path}`];
let from = 0;
while (from < changed.length) {
let to = from;
while (to + 1 < changed.length && changed[to + 1]! - changed[to]! <= 2 * CONTEXT + 1) to++;
const start = Math.max(0, changed[from]! - CONTEXT);
const end = Math.min(ops.length, changed[to]! + CONTEXT + 1);
const hunk = ops.slice(start, end);
const before0 = ops.slice(0, start).filter((op) => op.kind !== "+").length;
const after0 = ops.slice(0, start).filter((op) => op.kind !== "-").length;
const oldLen = hunk.filter((op) => op.kind !== "+").length;
const newLen = hunk.filter((op) => op.kind !== "-").length;
const range = (first: number, length: number) =>
length === 1 ? `${first + 1}` : `${length === 0 ? first : first + 1},${length}`;
out.push(`@@ -${range(before0, oldLen)} +${range(after0, newLen)} @@`);
for (const op of hunk) out.push(`${op.kind}${display(op.text)}`);
from = to + 1;
}
return out.join("\n");
}