From 7f1060631f5b9c2c576f7c1ee262c849c0c43d1e Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Thu, 14 May 2026 15:50:01 -0400 Subject: [PATCH] feat(vcs): add adapter abstraction --- src/core/config.test.ts | 4 +- src/core/config.ts | 35 ++---- src/core/jj.test.ts | 70 ++++++----- src/core/loaders.ts | 247 +++++-------------------------------- src/core/vcs/git.test.ts | 125 +++++++++++++++++++ src/core/vcs/git.ts | 211 +++++++++++++++++++++++++++++++ src/core/vcs/index.test.ts | 116 +++++++++++++++++ src/core/vcs/index.ts | 71 +++++++++++ src/core/vcs/jj.test.ts | 141 +++++++++++++++++++++ src/core/vcs/jj.ts | 99 +++++++++++++++ src/core/vcs/types.ts | 52 ++++++++ src/core/watch.test.ts | 9 ++ src/core/watch.ts | 56 ++------- 13 files changed, 913 insertions(+), 323 deletions(-) create mode 100644 src/core/vcs/git.test.ts create mode 100644 src/core/vcs/git.ts create mode 100644 src/core/vcs/index.test.ts create mode 100644 src/core/vcs/index.ts create mode 100644 src/core/vcs/jj.test.ts create mode 100644 src/core/vcs/jj.ts create mode 100644 src/core/vcs/types.ts diff --git a/src/core/config.test.ts b/src/core/config.test.ts index f1e28080..7ccb1f62 100644 --- a/src/core/config.test.ts +++ b/src/core/config.test.ts @@ -161,7 +161,7 @@ describe("config resolution", () => { expect(fallbackResolved.input.options.excludeUntracked).toBe(false); }); - test("defaults to git VCS mode and accepts jj from config", () => { + test("defaults to git VCS mode and accepts registered VCS modes from config", () => { const home = createTempDir("hunk-config-home-"); mkdirSync(join(home, ".config", "hunk"), { recursive: true }); writeFileSync(join(home, ".config", "hunk", "config.toml"), 'vcs = "jj"\n'); @@ -188,7 +188,7 @@ describe("config resolution", () => { expect(configuredResolved.input.options.vcs).toBe("jj"); }); - test("auto-detects jj checkouts before falling back to git mode", () => { + test("auto-detects registered VCS checkouts before falling back to git mode", () => { const home = createTempDir("hunk-config-home-"); const jjRepo = createTempDir("hunk-config-jj-repo-"); const colocatedRepo = createTempDir("hunk-config-colocated-repo-"); diff --git a/src/core/config.ts b/src/core/config.ts index 21f12fd3..2c6ecd6d 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; -import { dirname, join, resolve } from "node:path"; +import { join } from "node:path"; import { resolveGlobalConfigPath } from "./paths"; +import { detectVcs, findVcsRepoRootCandidate, isVcsId } from "./vcs"; import type { CliInput, CommonOptions, @@ -39,7 +40,7 @@ function normalizeLayoutMode(value: unknown): LayoutMode | undefined { /** Accept only the VCS backends Hunk can load directly. */ function normalizeVcsMode(value: unknown): VcsMode | undefined { - return value === "git" || value === "jj" ? value : undefined; + return isVcsId(value) ? value : undefined; } /** Accept only plain booleans from config files. */ @@ -101,31 +102,9 @@ function resolveConfigLayer(source: Record, input: CliInput): C return resolved; } -/** Return the first parent that looks like a repository root. */ -function findRepoRoot(cwd = process.cwd()) { - let current = resolve(cwd); - - for (;;) { - if (fs.existsSync(join(current, ".git")) || fs.existsSync(join(current, ".jj"))) { - return current; - } - - const parent = dirname(current); - if (parent === current) { - return undefined; - } - - current = parent; - } -} - /** Choose the VCS backend that best matches the discovered checkout. */ -function detectRepoVcsMode(repoRoot?: string): VcsMode { - if (repoRoot && fs.existsSync(join(repoRoot, ".jj"))) { - return "jj"; - } - - return "git"; +function detectRepoVcsMode(cwd: string): VcsMode { + return detectVcs(cwd)?.id ?? "git"; } /** Parse one TOML config file into a plain object. */ @@ -147,13 +126,13 @@ export function resolveConfiguredCliInput( input: CliInput, { cwd = process.cwd(), env = process.env }: ConfigResolutionOptions = {}, ): HunkConfigResolution { - const repoRoot = findRepoRoot(cwd); + const repoRoot = findVcsRepoRootCandidate(cwd); const repoConfigPath = repoRoot ? join(repoRoot, ".hunk", "config.toml") : undefined; const userConfigPath = resolveGlobalConfigPath(env); let resolvedOptions: CommonOptions = { mode: DEFAULT_VIEW_PREFERENCES.mode, - vcs: detectRepoVcsMode(repoRoot), + vcs: detectRepoVcsMode(cwd), // Keep the built-in theme default explicit so stdin-backed startup paths do not depend on // renderer theme-mode detection for their initial palette. theme: "graphite", diff --git a/src/core/jj.test.ts b/src/core/jj.test.ts index aaca8e3f..f551d95a 100644 --- a/src/core/jj.test.ts +++ b/src/core/jj.test.ts @@ -5,6 +5,8 @@ import { join } from "node:path"; import { buildJjDiffArgs, runJjText } from "./jj"; const tempDirs: string[] = []; +// Windows subprocess setup can exceed Bun's default 5s timeout while generating enough jj changes. +const JjAmbiguousPrefixTestTimeoutMs = 20_000; function cleanupTempDirs() { while (tempDirs.length > 0) { @@ -131,36 +133,40 @@ describe("jj command helpers", () => { ).toThrow("`hunk diff missing_revision` could not resolve Jujutsu revset `missing_revision`."); }); - test("reports a friendly error for ambiguous change id prefixes", () => { - const dir = createTempJjRepo("hunk-jj-ambiguous-prefix-"); - let prefix: string | undefined; - - for (let index = 0; index < 32 && !prefix; index += 1) { - writeFileSync(join(dir, `file-${index}.txt`), `${index}\n`); - jj(dir, "commit", "-m", `commit ${index}`); - - prefix = findDuplicatePrefix( - jj(dir, "log", "--no-graph", "-T", 'change_id ++ "\n"').trim().split("\n"), - ); - } - - if (!prefix) { - throw new Error("Expected generated jj changes to include an ambiguous prefix."); - } - - const input = { - kind: "vcs" as const, - range: prefix, - staged: false, - options: { mode: "auto" as const, vcs: "jj" as const }, - }; - - expect(() => - runJjText({ - input, - args: buildJjDiffArgs(input), - cwd: dir, - }), - ).toThrow(`\`hunk diff ${prefix}\` could not resolve Jujutsu revset \`${prefix}\`.`); - }); + test( + "reports a friendly error for ambiguous change id prefixes", + () => { + const dir = createTempJjRepo("hunk-jj-ambiguous-prefix-"); + let prefix: string | undefined; + + for (let index = 0; index < 32 && !prefix; index += 1) { + writeFileSync(join(dir, `file-${index}.txt`), `${index}\n`); + jj(dir, "commit", "-m", `commit ${index}`); + + prefix = findDuplicatePrefix( + jj(dir, "log", "--no-graph", "-T", 'change_id ++ "\n"').trim().split("\n"), + ); + } + + if (!prefix) { + throw new Error("Expected generated jj changes to include an ambiguous prefix."); + } + + const input = { + kind: "vcs" as const, + range: prefix, + staged: false, + options: { mode: "auto" as const, vcs: "jj" as const }, + }; + + expect(() => + runJjText({ + input, + args: buildJjDiffArgs(input), + cwd: dir, + }), + ).toThrow(`\`hunk diff ${prefix}\` could not resolve Jujutsu revset \`${prefix}\`.`); + }, + JjAmbiguousPrefixTestTimeoutMs, + ); }); diff --git a/src/core/loaders.ts b/src/core/loaders.ts index c53e6880..a2131287 100644 --- a/src/core/loaders.ts +++ b/src/core/loaders.ts @@ -11,24 +11,8 @@ import { join, resolve as resolvePath } from "node:path"; import { findAgentFileContext, loadAgentContext } from "./agent"; import { createSkippedBinaryMetadata, isProbablyBinaryFile, patchLooksBinary } from "./binary"; import { normalizeDiffMetadataPaths, normalizeDiffPath } from "./diffPaths"; -import { HunkUserError } from "./errors"; -import { - buildGitDiffArgs, - buildGitDiffNumstatArgs, - buildGitShowArgs, - buildGitStashShowArgs, - listGitUntrackedFiles, - resolveGitRepoRoot, - runGitText, - runGitUntrackedFileDiffText, -} from "./git"; -import { - buildJjDiffArgs, - buildJjShowArgs, - createJjStagedError, - resolveJjRepoRoot, - runJjText, -} from "./jj"; +import { runGitUntrackedFileDiffText } from "./git"; +import { createUnsupportedVcsOperationError, getVcsAdapter, operationFromInput } from "./vcs"; import type { AppBootstrap, AgentContext, @@ -640,69 +624,6 @@ function createSkippedLargeMetadata( }; } -interface GitNumstatFile { - path: string; - additions: number; - deletions: number; -} - -/** Parse `git diff --numstat -z` output for normal path entries. */ -function parseGitNumstat(text: string): GitNumstatFile[] { - return text - .split("\0") - .filter(Boolean) - .flatMap((entry) => { - const [additionsText, deletionsText, path] = entry.split("\t"); - if (!additionsText || !deletionsText || !path) { - return []; - } - - const additions = Number.parseInt(additionsText, 10); - const deletions = Number.parseInt(deletionsText, 10); - if (!Number.isFinite(additions) || !Number.isFinite(deletions)) { - return []; - } - - return [{ path, additions, deletions }]; - }); -} - -/** Return whether tracked diff stats are too large to render by default. */ -function shouldSkipLargeTrackedDiff(file: GitNumstatFile, repoRoot: string) { - if (file.additions + file.deletions > LARGE_DIFF_FILE_MAX_LINES) { - return true; - } - - try { - return fs.statSync(join(repoRoot, file.path)).size > LARGE_DIFF_FILE_MAX_BYTES; - } catch { - return false; - } -} - -/** Build a tracked placeholder for a file whose diff would be too expensive to render. */ -function buildSkippedLargeTrackedDiffFile( - file: GitNumstatFile, - index: number, - sourcePrefix: string, - agentContext: AgentContext | null, -) { - return buildDiffFile( - createSkippedLargeMetadata(file.path, "change"), - "", - index, - sourcePrefix, - agentContext, - { - isTooLarge: true, - stats: { - additions: file.additions, - deletions: file.deletions, - }, - }, - ); -} - /** Parse one synthetic untracked-file patch and reattach the real path after header normalization. */ function parseUntrackedPatchFile(patchText: string, filePath: string) { let parsedPatches: ReturnType; @@ -966,66 +887,50 @@ async function loadFileDiffChangeset( } satisfies Changeset; } -/** Build a changeset from the current repository working tree or a git range. */ -async function loadGitChangeset( - input: VcsCommandInput, +/** Build a changeset from an adapter-backed VCS review operation. */ +async function loadVcsChangeset( + input: VcsCommandInput | ShowCommandInput | StashShowCommandInput, agentContext: AgentContext | null, cwd = process.cwd(), ) { - const repoRoot = resolveGitRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - const title = input.staged - ? `${repoName} staged changes` - : input.range - ? `${repoName} ${input.range}` - : `${repoName} working tree`; - const largeTrackedFiles = parseGitNumstat( - runGitText({ input, args: buildGitDiffNumstatArgs(input), cwd }), - ).filter((file) => shouldSkipLargeTrackedDiff(file, repoRoot)); - const trackedChangeset = normalizePatchChangeset( - runGitText({ - input, - args: buildGitDiffArgs( - input, - largeTrackedFiles.map((file) => file.path), - ), - cwd, - }), - title, - repoRoot, + const adapter = getVcsAdapter(input.options.vcs ?? "git"); + const operation = operationFromInput(input); + if (!adapter.capabilities.reviewOperations.has(operation.kind)) { + throw createUnsupportedVcsOperationError(adapter, operation); + } + + const result = await adapter.loadReview(operation, { cwd }); + const parsedChangeset = normalizePatchChangeset( + result.patchText, + result.title, + result.sourceLabel, agentContext, ); - const trackedFiles = [ - ...trackedChangeset.files, - ...largeTrackedFiles.map((file, index) => - buildSkippedLargeTrackedDiffFile( - file, - trackedChangeset.files.length + index, - repoRoot, - agentContext, - ), - ), - ]; - const untrackedFiles = listGitUntrackedFiles(input, { cwd, repoRoot }); - - if (untrackedFiles.length === 0) { + const adapterFiles = (result.extraFiles ?? []).map((file, index) => ({ + ...file, + id: `${file.id}:extra:${index}`, + agent: findAgentFileContext(agentContext, file.path, file.previousPath), + })); + const trackedFiles = [...parsedChangeset.files, ...adapterFiles]; + + if (operation.kind !== "working-tree-diff" || !result.untrackedFiles?.length) { return { - ...trackedChangeset, + ...parsedChangeset, files: trackedFiles, } satisfies Changeset; } return { - ...trackedChangeset, + ...parsedChangeset, files: [ ...trackedFiles, - ...untrackedFiles.map((filePath, index) => + ...result.untrackedFiles.map((filePath, index) => buildUntrackedDiffFile( - input, + operation.input, filePath, trackedFiles.length + index, - repoRoot, - repoRoot, + result.repoRoot, + result.sourceLabel, agentContext, ), ), @@ -1033,86 +938,6 @@ async function loadGitChangeset( } satisfies Changeset; } -/** Build a changeset from the current Jujutsu working-copy commit or a revset. */ -async function loadJjDiffChangeset( - input: VcsCommandInput, - agentContext: AgentContext | null, - cwd = process.cwd(), -) { - if (input.staged) { - throw createJjStagedError(input); - } - - const repoRoot = resolveJjRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - const title = input.range ? `${repoName} ${input.range}` : `${repoName} working copy`; - - return normalizePatchChangeset( - runJjText({ input, args: buildJjDiffArgs(input), cwd }), - title, - repoRoot, - agentContext, - ); -} - -/** Build a changeset from `git show`, suppressing commit-message chrome so only the patch feeds the UI. */ -async function loadShowChangeset( - input: ShowCommandInput, - agentContext: AgentContext | null, - cwd = process.cwd(), -) { - const repoRoot = resolveGitRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - - return normalizePatchChangeset( - runGitText({ input, args: buildGitShowArgs(input), cwd }), - input.ref ? `${repoName} show ${input.ref}` : `${repoName} show HEAD`, - repoRoot, - agentContext, - ); -} - -/** Build a changeset from one Jujutsu revset using Git-format patch output. */ -async function loadJjShowChangeset( - input: ShowCommandInput, - agentContext: AgentContext | null, - cwd = process.cwd(), -) { - const repoRoot = resolveJjRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - const revset = input.ref ?? "@"; - - return normalizePatchChangeset( - runJjText({ input, args: buildJjShowArgs(input), cwd }), - `${repoName} show ${revset}`, - repoRoot, - agentContext, - ); -} - -/** Build a changeset from `git stash show -p`, which naturally maps to one reviewable patch. */ -async function loadStashShowChangeset( - input: StashShowCommandInput, - agentContext: AgentContext | null, - cwd = process.cwd(), -) { - if (input.options.vcs === "jj") { - throw new HunkUserError("`hunk stash show` requires Git VCS mode.", [ - 'Set `vcs = "git"` in Hunk config, then try again.', - ]); - } - - const repoRoot = resolveGitRepoRoot(input, { cwd }); - const repoName = basename(repoRoot); - - return normalizePatchChangeset( - runGitText({ input, args: buildGitStashShowArgs(input), cwd }), - input.ref ? `${repoName} stash ${input.ref}` : `${repoName} stash`, - repoRoot, - agentContext, - ); -} - /** Build a changeset from patch text supplied by file or stdin. */ async function loadPatchChangeset( input: PatchCommandInput, @@ -1145,19 +970,9 @@ export async function loadAppBootstrap( switch (input.kind) { case "vcs": - changeset = - input.options.vcs === "jj" - ? await loadJjDiffChangeset(input, agentContext, cwd) - : await loadGitChangeset(input, agentContext, cwd); - break; case "show": - changeset = - input.options.vcs === "jj" - ? await loadJjShowChangeset(input, agentContext, cwd) - : await loadShowChangeset(input, agentContext, cwd); - break; case "stash-show": - changeset = await loadStashShowChangeset(input, agentContext, cwd); + changeset = await loadVcsChangeset(input, agentContext, cwd); break; case "diff": changeset = await loadFileDiffChangeset(input, agentContext, cwd); diff --git a/src/core/vcs/git.test.ts b/src/core/vcs/git.test.ts new file mode 100644 index 00000000..32dd396a --- /dev/null +++ b/src/core/vcs/git.test.ts @@ -0,0 +1,125 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { platform, tmpdir } from "node:os"; +import { join } from "node:path"; +import { gitAdapter } from "./git"; +import type { ShowCommandInput, StashShowCommandInput, VcsCommandInput } from "../types"; + +const tempDirs: string[] = []; + +function createTempDir(prefix: string) { + const dir = realpathSync(mkdtempSync(join(tmpdir(), prefix))); + tempDirs.push(dir); + return dir; +} + +/** Normalize Windows short/long temp path spellings before path equality assertions. */ +function normalizeComparablePath(path: string) { + const resolvedPath = platform() === "win32" ? realpathSync.native(path) : path; + return resolvedPath.replace(/\\/g, "/"); +} + +function git(cwd: string, ...cmd: string[]) { + const proc = Bun.spawnSync(["git", ...cmd], { + cwd, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + + if (proc.exitCode !== 0) { + const stderr = Buffer.from(proc.stderr).toString("utf8"); + throw new Error(stderr.trim() || `git ${cmd.join(" ")} failed`); + } + + return Buffer.from(proc.stdout).toString("utf8"); +} + +function createTempRepo(prefix: string) { + const dir = createTempDir(prefix); + git(dir, "init", "--initial-branch", "master"); + git(dir, "config", "user.name", "Test User"); + git(dir, "config", "user.email", "test@example.com"); + git(dir, "config", "commit.gpgsign", "false"); + return dir; +} + +afterEach(() => { + while (tempDirs.length > 0) { + const dir = tempDirs.pop(); + if (dir) { + rmSync(dir, { recursive: true, force: true }); + } + } +}); + +describe("gitAdapter", () => { + test("detects Git repositories from nested directories", () => { + const repo = createTempRepo("hunk-git-adapter-detect-"); + const nested = join(repo, "src", "nested"); + mkdirSync(nested, { recursive: true }); + + expect(gitAdapter.detect(nested)).toEqual({ id: "git", repoRoot: repo }); + }); + + test("loads working-tree diffs with untracked files through the neutral operation", async () => { + const repo = createTempRepo("hunk-git-adapter-diff-"); + writeFileSync(join(repo, "tracked.txt"), "old\n"); + git(repo, "add", "tracked.txt"); + git(repo, "commit", "-m", "initial"); + writeFileSync(join(repo, "tracked.txt"), "new\n"); + writeFileSync(join(repo, "untracked.txt"), "fresh\n"); + + const input = { + kind: "vcs", + staged: false, + options: { vcs: "git" }, + } satisfies VcsCommandInput; + const result = await gitAdapter.loadReview({ kind: "working-tree-diff", input }, { cwd: repo }); + + expect(normalizeComparablePath(result.repoRoot)).toBe(normalizeComparablePath(repo)); + expect(result.title).toContain("working tree"); + expect(result.patchText).toContain("diff --git a/tracked.txt b/tracked.txt"); + expect(result.patchText).toContain("+new"); + expect(result.untrackedFiles).toEqual(["untracked.txt"]); + }); + + test("loads revision and stash patches through adapter operations", async () => { + const repo = createTempRepo("hunk-git-adapter-show-"); + writeFileSync(join(repo, "file.txt"), "one\n"); + git(repo, "add", "file.txt"); + git(repo, "commit", "-m", "initial"); + writeFileSync(join(repo, "file.txt"), "two\n"); + git(repo, "commit", "-am", "change"); + + const showInput = { + kind: "show", + ref: "HEAD", + options: { vcs: "git" }, + } satisfies ShowCommandInput; + const showResult = await gitAdapter.loadReview( + { kind: "revision-show", input: showInput }, + { cwd: repo }, + ); + + expect(showResult.title).toContain("show HEAD"); + expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); + expect(showResult.patchText).toContain("+two"); + + writeFileSync(join(repo, "file.txt"), "three\n"); + git(repo, "stash", "push", "-m", "adapter stash"); + + const stashInput = { + kind: "stash-show", + options: { vcs: "git" }, + } satisfies StashShowCommandInput; + const stashResult = await gitAdapter.loadReview( + { kind: "stash-show", input: stashInput }, + { cwd: repo }, + ); + + expect(stashResult.title).toContain("stash"); + expect(stashResult.patchText).toContain("diff --git a/file.txt b/file.txt"); + expect(stashResult.patchText).toContain("+three"); + }); +}); diff --git a/src/core/vcs/git.ts b/src/core/vcs/git.ts new file mode 100644 index 00000000..3780b907 --- /dev/null +++ b/src/core/vcs/git.ts @@ -0,0 +1,211 @@ +import fs from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import type { FileDiffMetadata } from "@pierre/diffs"; +import { + buildGitDiffArgs, + buildGitDiffNumstatArgs, + buildGitShowArgs, + buildGitStashShowArgs, + listGitUntrackedFiles, + resolveGitRepoRoot, + runGitText, +} from "../git"; +import type { DiffFile } from "../types"; +import type { VcsAdapter } from "./types"; + +const LARGE_DIFF_FILE_MAX_BYTES = 1_000_000; +const LARGE_DIFF_FILE_MAX_LINES = 20_000; + +/** Return the last path segment for review titles. */ +function basename(path: string) { + return path.split(/[\\/]/).filter(Boolean).pop() ?? path; +} + +/** Walk upward to detect a Git worktree marker without spawning Git during config resolution. */ +function detectGitRepo(cwd: string) { + let current = resolve(cwd); + for (;;) { + if (fs.existsSync(join(current, ".git"))) { + return { id: "git" as const, repoRoot: current }; + } + const parent = dirname(current); + if (parent === current) { + return null; + } + current = parent; + } +} + +interface GitNumstatFile { + path: string; + additions: number; + deletions: number; +} + +/** Parse `git diff --numstat -z` output for normal path entries. */ +function parseGitNumstat(text: string): GitNumstatFile[] { + return text + .split("\0") + .filter(Boolean) + .flatMap((entry) => { + const [additionsText, deletionsText, path] = entry.split("\t"); + if (!additionsText || !deletionsText || !path) { + return []; + } + + const additions = Number.parseInt(additionsText, 10); + const deletions = Number.parseInt(deletionsText, 10); + if (!Number.isFinite(additions) || !Number.isFinite(deletions)) { + return []; + } + + return [{ path, additions, deletions }]; + }); +} + +/** Return whether tracked diff stats are too large to render by default. */ +function shouldSkipLargeTrackedDiff(file: GitNumstatFile, repoRoot: string) { + if (file.additions + file.deletions > LARGE_DIFF_FILE_MAX_LINES) { + return true; + } + + try { + return fs.statSync(join(repoRoot, file.path)).size > LARGE_DIFF_FILE_MAX_BYTES; + } catch { + return false; + } +} + +/** Build placeholder metadata for a file whose full diff would be too expensive. */ +function createSkippedLargeMetadata(filePath: string): FileDiffMetadata { + return { + name: filePath, + type: "change", + hunks: [], + splitLineCount: 0, + unifiedLineCount: 0, + isPartial: true, + additionLines: [], + deletionLines: [], + cacheKey: `${filePath}:large-diff-skipped`, + }; +} + +/** Build a tracked placeholder for a file whose diff would be too expensive to render. */ +function buildSkippedLargeTrackedDiffFile( + file: GitNumstatFile, + index: number, + sourcePrefix: string, +): DiffFile { + return { + id: `${sourcePrefix}:${index}:${file.path}`, + path: file.path, + patch: "", + stats: { additions: file.additions, deletions: file.deletions }, + metadata: createSkippedLargeMetadata(file.path), + agent: null, + isTooLarge: true, + }; +} + +/** VCS adapter translating neutral review operations to Git commands. */ +export const gitAdapter: VcsAdapter = { + id: "git", + name: "Git", + capabilities: { + reviewOperations: new Set(["working-tree-diff", "revision-show", "stash-show"]), + stagedDiff: true, + watchSignatures: true, + }, + + detect: detectGitRepo, + + async loadReview(operation, { cwd }) { + switch (operation.kind) { + case "working-tree-diff": { + const input = operation.input; + const repoRoot = resolveGitRepoRoot(input, { cwd }); + const repoName = basename(repoRoot); + const title = input.staged + ? `${repoName} staged changes` + : input.range + ? `${repoName} ${input.range}` + : `${repoName} working tree`; + const largeTrackedFiles = parseGitNumstat( + runGitText({ input, args: buildGitDiffNumstatArgs(input), cwd }), + ).filter((file) => shouldSkipLargeTrackedDiff(file, repoRoot)); + return { + repoRoot, + sourceLabel: repoRoot, + title, + patchText: runGitText({ + input, + args: buildGitDiffArgs( + input, + largeTrackedFiles.map((file) => file.path), + ), + cwd, + }), + untrackedFiles: listGitUntrackedFiles(input, { cwd, repoRoot }), + extraFiles: largeTrackedFiles.map((file, index) => + buildSkippedLargeTrackedDiffFile(file, index, repoRoot), + ), + }; + } + case "revision-show": { + const input = operation.input; + const repoRoot = resolveGitRepoRoot(input, { cwd }); + const repoName = basename(repoRoot); + return { + repoRoot, + sourceLabel: repoRoot, + title: input.ref ? `${repoName} show ${input.ref}` : `${repoName} show HEAD`, + patchText: runGitText({ input, args: buildGitShowArgs(input), cwd }), + }; + } + case "stash-show": { + const input = operation.input; + const repoRoot = resolveGitRepoRoot(input, { cwd }); + const repoName = basename(repoRoot); + return { + repoRoot, + sourceLabel: repoRoot, + title: input.ref ? `${repoName} stash ${input.ref}` : `${repoName} stash`, + patchText: runGitText({ input, args: buildGitStashShowArgs(input), cwd }), + }; + } + } + }, + + watchSignature(operation) { + switch (operation.kind) { + case "working-tree-diff": { + const input = operation.input; + const trackedPatch = runGitText({ input, args: buildGitDiffArgs(input) }); + const repoRoot = resolveGitRepoRoot(input); + const untrackedSignatures = listGitUntrackedFiles(input, { repoRoot }).map( + (filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`, + ); + return [trackedPatch, ...untrackedSignatures].join("\n---\n"); + } + case "revision-show": { + const input = operation.input; + return runGitText({ input, args: buildGitShowArgs(input) }); + } + case "stash-show": { + const input = operation.input; + return runGitText({ input, args: buildGitStashShowArgs(input) }); + } + } + }, +}; + +/** Format one file stat into a stable signature fragment, or mark the path missing. */ +function statSignature(path: string) { + if (!fs.existsSync(path)) { + return `${path}:missing`; + } + + const stat = fs.statSync(path); + return `${path}:${stat.size}:${stat.mtimeMs}:${stat.ino}`; +} diff --git a/src/core/vcs/index.test.ts b/src/core/vcs/index.test.ts new file mode 100644 index 00000000..3517e7f8 --- /dev/null +++ b/src/core/vcs/index.test.ts @@ -0,0 +1,116 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, mkdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + createUnsupportedVcsOperationError, + detectVcs, + findVcsRepoRootCandidate, + getVcsAdapter, + isVcsId, + operationFromInput, + vcsAdapters, +} from "."; +import type { ShowCommandInput, StashShowCommandInput, VcsCommandInput } from "../types"; +import type { VcsAdapter } from "./types"; + +const tempDirs: string[] = []; + +function createTempDir(prefix: string) { + const dir = mkdtempSync(join(tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +afterEach(() => { + while (tempDirs.length > 0) { + const dir = tempDirs.pop(); + if (dir) { + rmSync(dir, { recursive: true, force: true }); + } + } +}); + +describe("VCS adapter registry", () => { + test("registers Git and Jujutsu adapters", () => { + expect(vcsAdapters.map((adapter) => adapter.id)).toEqual(["jj", "git"]); + expect(getVcsAdapter("git").capabilities.reviewOperations.has("stash-show")).toBe(true); + expect(getVcsAdapter("jj").capabilities.reviewOperations.has("stash-show")).toBe(false); + }); + + test("validates VCS ids from the registered adapter list", () => { + expect(isVcsId("git")).toBe(true); + expect(isVcsId("jj")).toBe(true); + expect(isVcsId("sl")).toBe(false); + expect(isVcsId("hg")).toBe(false); + }); + + test("finds repo root candidates through adapter detection instead of id-derived markers", () => { + const repo = createTempDir("hunk-vcs-custom-marker-"); + const nested = join(repo, "src", "nested"); + mkdirSync(join(repo, ".custom"), { recursive: true }); + mkdirSync(nested, { recursive: true }); + + const adapter: VcsAdapter = { + id: "git", + name: "Custom marker test adapter", + capabilities: { reviewOperations: new Set(), watchSignatures: false }, + detect(cwd) { + return cwd === repo ? { id: "git", repoRoot: repo } : null; + }, + async loadReview() { + throw new Error("not used"); + }, + }; + + vcsAdapters.unshift(adapter); + try { + expect(findVcsRepoRootCandidate(nested)).toBe(repo); + } finally { + expect(vcsAdapters.shift()).toBe(adapter); + } + }); + + test("detects repository roots by registered adapter priority", () => { + const repo = createTempDir("hunk-vcs-registry-"); + const nested = join(repo, "src", "nested"); + mkdirSync(nested, { recursive: true }); + mkdirSync(join(repo, ".git")); + + expect(detectVcs(nested)).toEqual({ id: "git", repoRoot: repo }); + expect(findVcsRepoRootCandidate(nested)).toBe(repo); + }); + + test("maps CLI inputs to neutral review operations", () => { + const diffInput = { + kind: "vcs", + staged: false, + options: { vcs: "git" }, + } satisfies VcsCommandInput; + const showInput = { + kind: "show", + ref: "HEAD", + options: { vcs: "git" }, + } satisfies ShowCommandInput; + const stashInput = { + kind: "stash-show", + options: { vcs: "git" }, + } satisfies StashShowCommandInput; + + expect(operationFromInput(diffInput)).toEqual({ kind: "working-tree-diff", input: diffInput }); + expect(operationFromInput(showInput)).toEqual({ kind: "revision-show", input: showInput }); + expect(operationFromInput(stashInput)).toEqual({ kind: "stash-show", input: stashInput }); + }); + + test("creates friendly errors for unsupported adapter operations", () => { + const adapter = getVcsAdapter("jj"); + const input = { + kind: "stash-show", + options: { vcs: "jj" }, + } satisfies StashShowCommandInput; + + expect(createUnsupportedVcsOperationError(adapter, operationFromInput(input)).message).toBe( + "`hunk stash show` requires Git VCS mode.", + ); + }); +}); diff --git a/src/core/vcs/index.ts b/src/core/vcs/index.ts new file mode 100644 index 00000000..ea1cb0fc --- /dev/null +++ b/src/core/vcs/index.ts @@ -0,0 +1,71 @@ +import { dirname, resolve } from "node:path"; +import { HunkUserError } from "../errors"; +import { gitAdapter } from "./git"; +import { jjAdapter } from "./jj"; +import type { VcsAdapter, VcsDetection, VcsId, VcsReviewInput, VcsReviewOperation } from "./types"; + +export const vcsAdapters: VcsAdapter[] = [jjAdapter, gitAdapter]; + +export function getVcsAdapter(id: VcsId): VcsAdapter { + const adapter = vcsAdapters.find((candidate) => candidate.id === id); + if (!adapter) { + throw new Error(`Unsupported VCS: ${id}`); + } + return adapter; +} + +export function isVcsId(value: unknown): value is VcsId { + return vcsAdapters.some((adapter) => adapter.id === value); +} + +export function detectVcs(cwd: string): VcsDetection | null { + for (const adapter of vcsAdapters) { + const detected = adapter.detect(cwd); + if (detected) { + return detected; + } + } + return null; +} + +export function findVcsRepoRootCandidate(cwd = process.cwd()) { + let current = resolve(cwd); + + for (;;) { + if (vcsAdapters.some((adapter) => adapter.detect(current)?.repoRoot === current)) { + return current; + } + + const parent = dirname(current); + if (parent === current) { + return undefined; + } + current = parent; + } +} + +export function operationFromInput(input: VcsReviewInput): VcsReviewOperation { + switch (input.kind) { + case "vcs": + return { kind: "working-tree-diff", input }; + case "show": + return { kind: "revision-show", input }; + case "stash-show": + return { kind: "stash-show", input }; + } +} + +export function createUnsupportedVcsOperationError( + adapter: VcsAdapter, + operation: VcsReviewOperation, +) { + if (operation.kind === "stash-show") { + return new HunkUserError("`hunk stash show` requires Git VCS mode.", [ + 'Set `vcs = "git"` in Hunk config, then try again.', + ]); + } + + return new HunkUserError(`${adapter.name} does not support ${operation.kind}.`, [ + "Use a supported VCS mode or command for this repository.", + ]); +} diff --git a/src/core/vcs/jj.test.ts b/src/core/vcs/jj.test.ts new file mode 100644 index 00000000..ad4b5990 --- /dev/null +++ b/src/core/vcs/jj.test.ts @@ -0,0 +1,141 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { platform, tmpdir } from "node:os"; +import { join } from "node:path"; +import { jjAdapter } from "./jj"; +import type { ShowCommandInput, StashShowCommandInput, VcsCommandInput } from "../types"; + +const tempDirs: string[] = []; +const JjAdapterIntegrationTestTimeoutMs = 20_000; + +function createTempDir(prefix: string) { + const dir = realpathSync(mkdtempSync(join(tmpdir(), prefix))); + tempDirs.push(dir); + return dir; +} + +/** Normalize Windows short/long temp path spellings before path equality assertions. */ +function normalizeComparablePath(path: string) { + const resolvedPath = platform() === "win32" ? realpathSync.native(path) : path; + return resolvedPath.replace(/\\/g, "/"); +} + +function jj(cwd: string, ...cmd: string[]) { + const proc = Bun.spawnSync( + [ + "jj", + "--config", + "signing.behavior=drop", + "--config", + 'user.name="Test User"', + "--config", + "user.email=test@example.com", + ...cmd, + ], + { + cwd, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }, + ); + + if (proc.exitCode !== 0) { + const stderr = Buffer.from(proc.stderr).toString("utf8"); + throw new Error(stderr.trim() || `jj ${cmd.join(" ")} failed`); + } + + return Buffer.from(proc.stdout).toString("utf8"); +} + +function createTempJjRepo(prefix: string) { + const dir = createTempDir(prefix); + jj(tmpdir(), "git", "init", "--colocate", dir); + return dir; +} + +afterEach(() => { + while (tempDirs.length > 0) { + const dir = tempDirs.pop(); + if (dir) { + rmSync(dir, { recursive: true, force: true }); + } + } +}); + +describe("jjAdapter", () => { + test( + "detects Jujutsu repositories from nested directories", + () => { + const repo = createTempJjRepo("hunk-jj-adapter-detect-"); + const nested = join(repo, "src", "nested"); + mkdirSync(nested, { recursive: true }); + + expect(jjAdapter.detect(nested)).toEqual({ id: "jj", repoRoot: repo }); + }, + JjAdapterIntegrationTestTimeoutMs, + ); + + test( + "loads working-copy and revision patches through neutral operations", + async () => { + const repo = createTempJjRepo("hunk-jj-adapter-review-"); + writeFileSync(join(repo, "file.txt"), "one\n"); + jj(repo, "commit", "-m", "initial"); + writeFileSync(join(repo, "file.txt"), "two\n"); + + const diffInput = { + kind: "vcs", + staged: false, + options: { vcs: "jj" }, + } satisfies VcsCommandInput; + const diffResult = await jjAdapter.loadReview( + { kind: "working-tree-diff", input: diffInput }, + { cwd: repo }, + ); + + expect(normalizeComparablePath(diffResult.repoRoot)).toBe(normalizeComparablePath(repo)); + expect(diffResult.title).toContain("working copy"); + expect(diffResult.patchText).toContain("diff --git a/file.txt b/file.txt"); + expect(diffResult.patchText).toContain("+two"); + + const showInput = { + kind: "show", + ref: "@", + options: { vcs: "jj" }, + } satisfies ShowCommandInput; + const showResult = await jjAdapter.loadReview( + { kind: "revision-show", input: showInput }, + { cwd: repo }, + ); + + expect(showResult.title).toContain("show @"); + expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); + }, + JjAdapterIntegrationTestTimeoutMs, + ); + + test( + "rejects staged and stash operations", + async () => { + const repo = createTempJjRepo("hunk-jj-adapter-unsupported-"); + const stagedInput = { + kind: "vcs", + staged: true, + options: { vcs: "jj" }, + } satisfies VcsCommandInput; + const stashInput = { + kind: "stash-show", + options: { vcs: "jj" }, + } satisfies StashShowCommandInput; + + await expect( + jjAdapter.loadReview({ kind: "working-tree-diff", input: stagedInput }, { cwd: repo }), + ).rejects.toThrow("Jujutsu has no staging area"); + await expect( + jjAdapter.loadReview({ kind: "stash-show", input: stashInput }, { cwd: repo }), + ).rejects.toThrow("requires Git VCS mode"); + }, + JjAdapterIntegrationTestTimeoutMs, + ); +}); diff --git a/src/core/vcs/jj.ts b/src/core/vcs/jj.ts new file mode 100644 index 00000000..17e2c128 --- /dev/null +++ b/src/core/vcs/jj.ts @@ -0,0 +1,99 @@ +import fs from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { HunkUserError } from "../errors"; +import { + buildJjDiffArgs, + buildJjShowArgs, + createJjStagedError, + resolveJjRepoRoot, + runJjText, +} from "../jj"; +import type { VcsAdapter } from "./types"; + +/** Return the last path segment for review titles. */ +function basename(path: string) { + return path.split(/[\\/]/).filter(Boolean).pop() ?? path; +} + +/** Walk upward to detect a Jujutsu workspace marker without spawning JJ during config resolution. */ +function detectJjRepo(cwd: string) { + let current = resolve(cwd); + for (;;) { + if (fs.existsSync(join(current, ".jj"))) { + return { id: "jj" as const, repoRoot: current }; + } + const parent = dirname(current); + if (parent === current) { + return null; + } + current = parent; + } +} + +/** Return the user-facing error for Jujutsu operations that only Git supports. */ +function createJjUnsupportedStashShowError() { + return new HunkUserError("`hunk stash show` requires Git VCS mode.", [ + 'Set `vcs = "git"` in Hunk config, then try again.', + ]); +} + +/** VCS adapter translating neutral review operations to Jujutsu commands. */ +export const jjAdapter: VcsAdapter = { + id: "jj", + name: "Jujutsu", + capabilities: { + reviewOperations: new Set(["working-tree-diff", "revision-show"]), + stagedDiff: false, + watchSignatures: true, + }, + + detect: detectJjRepo, + + async loadReview(operation, { cwd }) { + switch (operation.kind) { + case "working-tree-diff": { + const input = operation.input; + if (input.staged) { + throw createJjStagedError(input); + } + const repoRoot = resolveJjRepoRoot(input, { cwd }); + const repoName = basename(repoRoot); + return { + repoRoot, + sourceLabel: repoRoot, + title: input.range ? `${repoName} ${input.range}` : `${repoName} working copy`, + patchText: runJjText({ input, args: buildJjDiffArgs(input), cwd }), + }; + } + case "revision-show": { + const input = operation.input; + const repoRoot = resolveJjRepoRoot(input, { cwd }); + const repoName = basename(repoRoot); + const revset = input.ref ?? "@"; + return { + repoRoot, + sourceLabel: repoRoot, + title: `${repoName} show ${revset}`, + patchText: runJjText({ input, args: buildJjShowArgs(input), cwd }), + }; + } + case "stash-show": + throw createJjUnsupportedStashShowError(); + } + }, + + watchSignature(operation) { + switch (operation.kind) { + case "working-tree-diff": { + const input = operation.input; + return runJjText({ input, args: buildJjDiffArgs(input) }); + } + case "revision-show": { + const input = operation.input; + return runJjText({ input, args: buildJjShowArgs(input) }); + } + case "stash-show": + throw createJjUnsupportedStashShowError(); + } + }, +}; diff --git a/src/core/vcs/types.ts b/src/core/vcs/types.ts new file mode 100644 index 00000000..db95d26e --- /dev/null +++ b/src/core/vcs/types.ts @@ -0,0 +1,52 @@ +import type { + DiffFile, + ShowCommandInput, + StashShowCommandInput, + VcsCommandInput, + VcsMode, +} from "../types"; + +export type VcsId = VcsMode; + +export interface VcsDetection { + id: VcsId; + repoRoot: string; +} + +export interface VcsLoadContext { + cwd: string; +} + +export type VcsReviewInput = VcsCommandInput | ShowCommandInput | StashShowCommandInput; + +export type VcsReviewOperation = + | { kind: "working-tree-diff"; input: VcsCommandInput } + | { kind: "revision-show"; input: ShowCommandInput } + | { kind: "stash-show"; input: StashShowCommandInput }; + +export type VcsReviewOperationKind = VcsReviewOperation["kind"]; + +export interface VcsCapabilities { + reviewOperations: ReadonlySet; + stagedDiff?: boolean; + sourceFetching?: boolean; + watchSignatures?: boolean; +} + +export interface VcsPatchResult { + repoRoot: string; + sourceLabel: string; + title: string; + patchText: string; + untrackedFiles?: string[]; + extraFiles?: DiffFile[]; +} + +export interface VcsAdapter { + id: VcsId; + name: string; + capabilities: VcsCapabilities; + detect(cwd: string): VcsDetection | null; + loadReview(operation: VcsReviewOperation, context: VcsLoadContext): Promise; + watchSignature?: (operation: VcsReviewOperation, context: VcsLoadContext) => string; +} diff --git a/src/core/watch.test.ts b/src/core/watch.test.ts index a19d51ef..8f794683 100644 --- a/src/core/watch.test.ts +++ b/src/core/watch.test.ts @@ -118,6 +118,15 @@ describe("computeWatchSignature", () => { expect(changedSignature).toEqual(initialSignature); }); + test("rejects unsupported watch operations before invoking adapter signatures", () => { + expect(() => + computeWatchSignature({ + kind: "stash-show", + options: { mode: "auto", vcs: "jj" }, + }), + ).toThrow("`hunk stash show` requires Git VCS mode."); + }); + test("tracks untracked file changes when diff compares the working tree against one ref", () => { const dir = createTempRepo("hunk-watch-ref-untracked-"); diff --git a/src/core/watch.ts b/src/core/watch.ts index baa4faaf..8b893c33 100644 --- a/src/core/watch.ts +++ b/src/core/watch.ts @@ -1,14 +1,5 @@ import fs from "node:fs"; -import { join } from "node:path"; -import { - buildGitDiffArgs, - buildGitShowArgs, - buildGitStashShowArgs, - listGitUntrackedFiles, - resolveGitRepoRoot, - runGitText, -} from "./git"; -import { buildJjDiffArgs, buildJjShowArgs, runJjText } from "./jj"; +import { createUnsupportedVcsOperationError, getVcsAdapter, operationFromInput } from "./vcs"; import type { CliInput } from "./types"; /** Return whether the current input can be rebuilt from files or VCS state without rereading stdin. */ @@ -30,52 +21,27 @@ function statSignature(path: string) { return `${path}:${stat.size}:${stat.mtimeMs}:${stat.ino}`; } -/** Build the cheaper watch signature for working-tree git diff inputs without rendering full untracked patches. */ -function gitWorkingTreeWatchSignature(input: Extract) { - const trackedPatch = runGitText({ input, args: buildGitDiffArgs(input) }); - const repoRoot = resolveGitRepoRoot(input); - const untrackedSignatures = listGitUntrackedFiles(input, { repoRoot }).map( - (filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`, - ); - - return [trackedPatch, ...untrackedSignatures].join("\n---\n"); -} - -/** Build one exact patch signature for Git-backed review inputs. */ -function gitPatchSignature(input: Extract) { - switch (input.kind) { - case "vcs": - return gitWorkingTreeWatchSignature(input); - case "show": - return runGitText({ input, args: buildGitShowArgs(input) }); - case "stash-show": - return runGitText({ input, args: buildGitStashShowArgs(input) }); +/** Build one exact patch signature for adapter-backed review inputs. */ +function vcsPatchSignature(input: Extract) { + const adapter = getVcsAdapter(input.options.vcs ?? "git"); + if (!adapter.watchSignature) { + throw new Error(`${adapter.name} does not support watch signatures.`); } -} - -/** Build one exact patch signature for Jujutsu-backed review inputs. */ -function jjPatchSignature(input: Extract) { - switch (input.kind) { - case "vcs": - return runJjText({ input, args: buildJjDiffArgs(input) }); - case "show": - return runJjText({ input, args: buildJjShowArgs(input) }); + const operation = operationFromInput(input); + if (!adapter.capabilities.reviewOperations.has(operation.kind)) { + throw createUnsupportedVcsOperationError(adapter, operation); } + return adapter.watchSignature(operation, { cwd: process.cwd() }); } - /** Compute a change-detection signature for one watchable input. */ export function computeWatchSignature(input: CliInput) { const parts: string[] = [input.kind]; switch (input.kind) { case "vcs": - parts.push(input.options.vcs === "jj" ? jjPatchSignature(input) : gitPatchSignature(input)); - break; case "show": - parts.push(input.options.vcs === "jj" ? jjPatchSignature(input) : gitPatchSignature(input)); - break; case "stash-show": - parts.push(gitPatchSignature(input)); + parts.push(vcsPatchSignature(input)); break; case "diff": case "difftool":