diff --git a/.changeset/ninety-friends-count.md b/.changeset/ninety-friends-count.md new file mode 100644 index 00000000..fe9475ba --- /dev/null +++ b/.changeset/ninety-friends-count.md @@ -0,0 +1,4 @@ +--- +--- + +Refactor VCS adapter dispatch to use operation maps without a release note. diff --git a/src/core/config.ts b/src/core/config.ts index e4a76038..f0dced64 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import { join } from "node:path"; import { resolveGlobalConfigPath } from "./paths"; -import { detectVcs, findVcsRepoRootCandidate, isVcsId } from "./vcs"; +import { detectVcs, findVcsRepoRootCandidate, getDefaultVcsAdapter, isVcsId } from "./vcs"; import type { CliInput, CommonOptions, @@ -285,7 +285,7 @@ function resolveConfigLayer(source: Record, input: CliInput): C /** Choose the VCS backend that best matches the discovered checkout. */ function detectRepoVcsMode(cwd: string): VcsMode { - return detectVcs(cwd)?.id ?? "git"; + return detectVcs(cwd)?.id ?? getDefaultVcsAdapter().id; } /** Parse one TOML config file into a plain object. */ @@ -349,7 +349,7 @@ export function resolveConfiguredCliInput( pager: input.options.pager ?? false, watch: input.options.watch ?? resolvedOptions.watch ?? false, excludeUntracked: resolvedOptions.excludeUntracked ?? false, - vcs: resolvedOptions.vcs ?? "git", + vcs: resolvedOptions.vcs ?? getDefaultVcsAdapter().id, mode: resolvedOptions.mode ?? DEFAULT_VIEW_PREFERENCES.mode, lineNumbers: resolvedOptions.lineNumbers ?? DEFAULT_VIEW_PREFERENCES.showLineNumbers, wrapLines: resolvedOptions.wrapLines ?? DEFAULT_VIEW_PREFERENCES.wrapLines, diff --git a/src/core/fileSource.test.ts b/src/core/fileSource.test.ts index 7c2a7fb3..bd0c1c71 100644 --- a/src/core/fileSource.test.ts +++ b/src/core/fileSource.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createFileSourceFetcher, SourceTextTooLargeError } from "./fileSource"; +import { createGitFileSourceFetcher } from "./vcs/gitSource"; const tempDirs: string[] = []; @@ -127,7 +128,7 @@ describe("createFileSourceFetcher", () => { git(repoRoot, "add", "."); git(repoRoot, "commit", "-m", "second"); - const fetcher = createFileSourceFetcher({ + const fetcher = createGitFileSourceFetcher({ old: { kind: "git-blob", repoRoot, ref: "HEAD~1", path: filePath }, new: { kind: "git-blob", repoRoot, ref: "HEAD", path: filePath }, }); @@ -147,7 +148,7 @@ describe("createFileSourceFetcher", () => { git(repoRoot, "add", filePath); writeFileSync(join(repoRoot, filePath), "working tree\n"); - const fetcher = createFileSourceFetcher({ + const fetcher = createGitFileSourceFetcher({ old: { kind: "git-index", repoRoot, path: filePath }, new: { kind: "fs", absolutePath: join(repoRoot, filePath) }, }); @@ -166,7 +167,7 @@ describe("createFileSourceFetcher", () => { writeFileSync(join(repoRoot, filePath), "staged source\n"); git(repoRoot, "add", filePath); - const fetcher = createFileSourceFetcher( + const fetcher = createGitFileSourceFetcher( { old: { kind: "git-blob", repoRoot, ref: "HEAD", path: filePath }, new: { kind: "git-index", repoRoot, path: filePath }, @@ -197,7 +198,7 @@ describe("createFileSourceFetcher", () => { )) as typeof Bun.spawn; try { - const fetcher = createFileSourceFetcher({ + const fetcher = createGitFileSourceFetcher({ old: { kind: "git-blob", repoRoot: process.cwd(), ref: "HEAD", path: "note.txt" }, new: { kind: "none" }, }); @@ -235,7 +236,7 @@ describe("createFileSourceFetcher", () => { }) as typeof Bun.spawn; try { - const fetcher = createFileSourceFetcher( + const fetcher = createGitFileSourceFetcher( { old: { kind: "git-blob", repoRoot: process.cwd(), ref: "HEAD", path: "note.txt" }, new: { kind: "git-index", repoRoot: process.cwd(), path: "note.txt" }, @@ -261,7 +262,7 @@ describe("createFileSourceFetcher", () => { git(repoRoot, "add", "."); git(repoRoot, "commit", "-m", "first"); - const fetcher = createFileSourceFetcher({ + const fetcher = createGitFileSourceFetcher({ old: { kind: "git-blob", repoRoot, ref: "HEAD", path: "missing-from-history.txt" }, new: { kind: "none" }, }); @@ -274,7 +275,7 @@ describe("createFileSourceFetcher", () => { test("logs unexpected git source failures with object context", async () => { const repoRoot = createTempDir("hunk-source-git-not-repo-"); - const fetcher = createFileSourceFetcher({ + const fetcher = createGitFileSourceFetcher({ old: { kind: "git-blob", repoRoot, ref: "HEAD", path: "note.txt" }, new: { kind: "none" }, }); diff --git a/src/core/fileSource.ts b/src/core/fileSource.ts index 7beca90d..a43aaebc 100644 --- a/src/core/fileSource.ts +++ b/src/core/fileSource.ts @@ -1,26 +1,22 @@ /** - * Low-level readers for full file contents used by input and VCS adapters. + * Generic full-file source fetcher primitives used by input loaders and VCS adapters. * * Each `DiffFile` may carry a `FileSourceFetcher` that knows how to read the - * file's "old" and "new" sides without re-running the original diff. VCS - * adapters own VCS-specific source-spec construction; this module only executes - * the concrete reads and returns `null` when source content is unreachable. + * file's "old" and "new" sides without re-running the original diff. Provider- + * specific object reads live beside their VCS adapters; this module only owns + * provider-neutral fetcher contracts and filesystem reads. */ -export type FileSourceSpec = - | { kind: "none" } - | { kind: "fs"; absolutePath: string } - | { kind: "git-blob"; repoRoot: string; ref: string; path: string } - | { kind: "git-index"; repoRoot: string; path: string }; +export type FileSourceSpec = { kind: "none" } | { kind: "fs"; absolutePath: string }; export type FileSourceSide = "old" | "new"; export interface FileSourceFetcher { /** * Returns the file's full source text on the requested side, or `null` when - * the side is not reachable (deleted side, missing path, git error). Built-in - * fetchers resolve `null` instead of rejecting, but UI callers still handle - * custom fetcher rejection defensively. + * the side is not reachable (deleted side, missing path, provider error). + * Built-in fetchers resolve `null` instead of rejecting, but UI callers still + * handle custom fetcher rejection defensively. */ getFullText(side: FileSourceSide): Promise; } @@ -36,7 +32,6 @@ export class SourceTextTooLargeError extends Error { } export interface FileSourceFetcherOptions { - gitExecutable?: string; maxSourceBytes?: number; } @@ -54,7 +49,7 @@ function firstDiagnosticLine(text: string) { } /** Keep source-load diagnostics terse enough to be useful in logs. */ -function logSourceDiagnostic(message: string, detail?: unknown) { +export function logSourceDiagnostic(message: string, detail?: unknown) { if (detail instanceof Error) { console.error(`hunk: ${message}: ${detail.message}`, detail); return; @@ -64,18 +59,6 @@ function logSourceDiagnostic(message: string, detail?: unknown) { console.error(detailText ? `hunk: ${message}: ${detailText}` : `hunk: ${message}`); } -/** Return whether a Git failure is an expected missing source side/path. */ -function isExpectedMissingGitSource(stderr: string) { - const normalized = stderr.toLowerCase(); - return [ - "exists on disk, but not in", - "does not exist in", - "invalid object name", - "needed a single revision", - "unknown revision or path not in the working tree", - ].some((fragment) => normalized.includes(fragment)); -} - async function readFsSpec( spec: Extract, maxSourceBytes: number, @@ -101,28 +84,7 @@ async function readFsSpec( } } -function readGitBlobSpec( - spec: Extract, - gitExecutable = "git", - maxSourceBytes: number, -): Promise { - return readGitObjectSpec( - spec.repoRoot, - `${spec.ref}:${spec.path}`, - gitExecutable, - maxSourceBytes, - ); -} - -function readGitIndexSpec( - spec: Extract, - gitExecutable = "git", - maxSourceBytes: number, -): Promise { - return readGitObjectSpec(spec.repoRoot, `:${spec.path}`, gitExecutable, maxSourceBytes); -} - -async function readStreamTextWithLimit( +export async function readStreamTextWithLimit( stream: ReadableStream | null, maxBytes: number, onTooLarge?: () => void, @@ -163,91 +125,21 @@ async function readStreamTextWithLimit( return new TextDecoder().decode(combined); } -/** Read a blob-like Git object spec such as `HEAD:path` or `:path`. */ -async function readGitObjectSpec( - repoRoot: string, - objectName: string, - gitExecutable = "git", - maxSourceBytes: number, -): Promise { - let proc: Bun.ReadableSubprocess; - - try { - proc = Bun.spawn([gitExecutable, "show", objectName], { - cwd: repoRoot, - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - }); - } catch (error) { - logSourceDiagnostic(`failed to run Git while reading source ${objectName}`, error); - return null; - } - - let output: [number, string, string]; - try { - output = await Promise.all([ - proc.exited, - readStreamTextWithLimit(proc.stdout, maxSourceBytes, () => proc.kill()), - readStreamTextWithLimit( - proc.stderr, - 64 * 1024, - undefined, - (maxBytes) => new Error(`Git source diagnostics exceeded ${maxBytes} bytes.`), - ), - ]); - } catch (error) { - if (error instanceof SourceTextTooLargeError) { - proc.kill(); - await proc.exited.catch(() => undefined); - throw error; - } - - logSourceDiagnostic(`failed to collect Git source ${objectName}`, error); - return null; - } - - const [exitCode, stdout, stderr] = output; - - if (exitCode !== 0) { - if (!isExpectedMissingGitSource(stderr)) { - logSourceDiagnostic(`failed to read Git source ${objectName} in ${repoRoot}`, stderr); - } - return null; - } - - return stdout; -} - async function readSpec( spec: FileSourceSpec, - { - gitExecutable = "git", - maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES, - }: FileSourceFetcherOptions = {}, + { maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES }: FileSourceFetcherOptions = {}, ): Promise { if (spec.kind === "none") { return null; } - if (spec.kind === "fs") { - return readFsSpec(spec, maxSourceBytes); - } - - if (spec.kind === "git-index") { - return readGitIndexSpec(spec, gitExecutable, maxSourceBytes); - } - - return readGitBlobSpec(spec, gitExecutable, maxSourceBytes); + return readFsSpec(spec, maxSourceBytes); } /** Build a per-file source fetcher that caches each side's resolved text. */ export function createFileSourceFetcher( specs: ResolvedSpecs, - { - gitExecutable = "git", - maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES, - }: Readonly = {}, + { maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES }: Readonly = {}, ): FileSourceFetcher { const cache = new Map(); @@ -257,7 +149,7 @@ export function createFileSourceFetcher( return cache.get(side) ?? null; } - const text = await readSpec(specs[side], { gitExecutable, maxSourceBytes }); + const text = await readSpec(specs[side], { maxSourceBytes }); cache.set(side, text); return text; }, diff --git a/src/core/git.test.ts b/src/core/git.test.ts index f5dcf711..06f6a364 100644 --- a/src/core/git.test.ts +++ b/src/core/git.test.ts @@ -9,7 +9,7 @@ import { resolveGitDiffEndpoints, runGitText, } from "./git"; -import type { VcsCommandInput } from "./types"; +import type { VcsDiffCommandInput } from "./types"; const tempDirs: string[] = []; @@ -39,7 +39,7 @@ function createTempRepo(prefix: string) { return dir; } -function makeGitInput(overrides: Partial = {}): VcsCommandInput { +function makeGitInput(overrides: Partial = {}): VcsDiffCommandInput { return { kind: "vcs", staged: false, diff --git a/src/core/git.ts b/src/core/git.ts index 151ee335..3c52b362 100644 --- a/src/core/git.ts +++ b/src/core/git.ts @@ -2,10 +2,10 @@ import fs from "node:fs"; import { join } from "node:path"; import { HunkUserError } from "./errors"; import { escapeUntrackedPatchPath } from "./patch/normalize"; -import type { VcsCommandInput, ShowCommandInput, StashShowCommandInput } from "./types"; +import type { VcsDiffCommandInput, VcsShowCommandInput, VcsStashShowCommandInput } from "./types"; import { normalizePathForOS } from "../lib/osPath"; -export type GitBackedInput = VcsCommandInput | ShowCommandInput | StashShowCommandInput; +export type GitBackedInput = VcsDiffCommandInput | VcsShowCommandInput | VcsStashShowCommandInput; export interface RunGitTextOptions { input: GitBackedInput; @@ -99,7 +99,7 @@ function withGitMovedLineColorConfig(args: string[], colorMoved: GitColorMovedOp /** Build the exact `git diff` arguments used for the shared working-tree and range review path. */ export function buildGitDiffArgs( - input: VcsCommandInput, + input: VcsDiffCommandInput, excludedPathspecs: string[] = [], colorMoved: GitColorMovedOptions | null = null, ) { @@ -127,7 +127,7 @@ export function buildGitDiffArgs( } /** Build the cheap tracked-file stats query used to skip huge file diffs before patch output. */ -export function buildGitDiffNumstatArgs(input: VcsCommandInput) { +export function buildGitDiffNumstatArgs(input: VcsDiffCommandInput) { const args = ["diff", "--no-ext-diff", "--find-renames", "--no-color", "--numstat", "-z"]; if (input.staged) { @@ -143,7 +143,7 @@ export function buildGitDiffNumstatArgs(input: VcsCommandInput) { } /** Build the porcelain status query used to discover untracked files for working-tree review. */ -export function buildGitStatusArgs(input: VcsCommandInput) { +export function buildGitStatusArgs(input: VcsDiffCommandInput) { const args = ["--no-optional-locks", "status", "--porcelain=v1", "-z", "--untracked-files=all"]; appendGitPathspecs(args, input.pathspecs); @@ -167,7 +167,7 @@ function buildGitNewFileDiffArgs(filePath: string) { /** Build the exact `git show` arguments used for commit review. */ export function buildGitShowArgs( - input: ShowCommandInput, + input: VcsShowCommandInput, colorMoved: GitColorMovedOptions | null = null, ) { const args = [ @@ -188,7 +188,7 @@ export function buildGitShowArgs( /** Build the exact `git stash show -p` arguments used for stash review. */ export function buildGitStashShowArgs( - input: StashShowCommandInput, + input: VcsStashShowCommandInput, colorMoved: GitColorMovedOptions | null = null, ) { const args = [ @@ -280,7 +280,7 @@ function createMissingRepoError(input: GitBackedInput) { ); } -function createInvalidRevisionError(input: VcsCommandInput | ShowCommandInput) { +function createInvalidRevisionError(input: VcsDiffCommandInput | VcsShowCommandInput) { if (input.kind === "vcs") { return new HunkUserError( `\`${formatGitCommandLabel(input)}\` could not resolve Git revision or range \`${input.range}\`.`, @@ -295,7 +295,7 @@ function createInvalidRevisionError(input: VcsCommandInput | ShowCommandInput) { ); } -function createMissingStashError(input: StashShowCommandInput) { +function createMissingStashError(input: VcsStashShowCommandInput) { if (input.ref) { return new HunkUserError( `\`${formatGitCommandLabel(input)}\` could not resolve stash entry \`${input.ref}\`.`, @@ -476,7 +476,7 @@ export function resolveGitColorMovedOptions( const workingTreeGitDiffInputCache = new Map(); function isWorkingTreeGitDiffInput( - input: VcsCommandInput, + input: VcsDiffCommandInput, { cwd = process.cwd(), gitExecutable = "git", @@ -517,7 +517,7 @@ function isWorkingTreeGitDiffInput( /** Return whether working-tree review should synthesize untracked files into the patch stream. */ function shouldIncludeUntrackedFiles( - input: VcsCommandInput, + input: VcsDiffCommandInput, options: Pick & { repoRoot?: string } = {}, ) { return input.options.excludeUntracked !== true && isWorkingTreeGitDiffInput(input, options); @@ -564,7 +564,7 @@ function isReviewableUntrackedPath(repoRoot: string, filePath: string) { /** Return the repo-root-relative untracked files for a working-tree review input. */ export function listGitUntrackedFiles( - input: VcsCommandInput, + input: VcsDiffCommandInput, { cwd = process.cwd(), repoRoot, @@ -620,7 +620,7 @@ export function normalizeUntrackedPatchHeaders(patchText: string, filePath: stri /** Return the raw Git patch text for one untracked file using `git diff --no-index`. */ export function runGitUntrackedFileDiffText( - input: VcsCommandInput, + input: VcsDiffCommandInput, filePath: string, { cwd = process.cwd(), @@ -720,7 +720,7 @@ function parseSymmetricDiffRange(range: string): { left: string; right: string } /** Resolve rev-parse output into positive and negative revisions for one diff range. */ function resolveRangeRevisions( - input: VcsCommandInput, + input: VcsDiffCommandInput, range: string, { cwd = process.cwd(), @@ -752,7 +752,7 @@ function resolveRangeRevisions( * source by ref" rather than silently falling back to the working tree. */ export function resolveGitDiffEndpoints( - input: VcsCommandInput, + input: VcsDiffCommandInput, { cwd = process.cwd(), gitExecutable = "git", diff --git a/src/core/jj.ts b/src/core/jj.ts index f500785d..432fca50 100644 --- a/src/core/jj.ts +++ b/src/core/jj.ts @@ -1,8 +1,8 @@ import { HunkUserError } from "./errors"; -import type { VcsCommandInput, ShowCommandInput } from "./types"; +import type { VcsDiffCommandInput, VcsShowCommandInput } from "./types"; import { normalizePathForOS } from "../lib/osPath"; -export type JjBackedInput = VcsCommandInput | ShowCommandInput; +export type JjBackedInput = VcsDiffCommandInput | VcsShowCommandInput; export interface RunJjTextOptions { input: JjBackedInput; @@ -21,7 +21,7 @@ function appendJjFilesets(args: string[], pathspecs?: string[]) { } /** Build the `jj diff --git` arguments for working-copy and revset reviews. */ -export function buildJjDiffArgs(input: VcsCommandInput) { +export function buildJjDiffArgs(input: VcsDiffCommandInput) { const args = ["diff", "--git"]; if (input.range) { @@ -33,7 +33,7 @@ export function buildJjDiffArgs(input: VcsCommandInput) { } /** Build the `jj diff --git -r` arguments used for `hunk show` in Jujutsu mode. */ -export function buildJjShowArgs(input: ShowCommandInput) { +export function buildJjShowArgs(input: VcsShowCommandInput) { const args = ["diff", "--git", "-r", input.ref ?? "@"]; appendJjFilesets(args, input.pathspecs); @@ -96,7 +96,7 @@ function createMissingJjRepoError(input: JjBackedInput) { ); } -export function createJjStagedError(input: VcsCommandInput) { +export function createJjStagedError(input: VcsDiffCommandInput) { return new HunkUserError( `\`${formatJjCommandLabel(input)}\` requires Git VCS mode because Jujutsu has no staging area.`, ['Remove `--staged`, or set `vcs = "git"` in Hunk config.'], diff --git a/src/core/loaders.ts b/src/core/loaders.ts index b889132b..d6340ec5 100644 --- a/src/core/loaders.ts +++ b/src/core/loaders.ts @@ -5,25 +5,14 @@ import { type FileDiffMetadata, } from "@pierre/diffs"; import { createTwoFilesPatch } from "diff"; -import fs from "node:fs"; -import { join, resolve as resolvePath } from "node:path"; +import { resolve as resolvePath } from "node:path"; import { findAgentFileContext, loadAgentContext } from "./agent"; import { createSkippedBinaryMetadata, isProbablyBinaryFile } from "./binary"; -import { - buildDiffFile, - createSkippedLargeMetadata, - type BuildDiffFileOptions, - type DiffFileSourceContext, -} from "./diffFile"; +import { buildDiffFile, type BuildDiffFileOptions, type DiffFileSourceContext } from "./diffFile"; import { createFileSourceFetcher, type FileSourceSpec } from "./fileSource"; -import { normalizeUntrackedPatchHeaders, runGitUntrackedFileDiffText } from "./git"; import { splitPatchIntoFileChunks, findPatchChunk } from "./patch/chunks"; -import { - escapeUntrackedPatchPath, - normalizePatchText, - stripTerminalControl, -} from "./patch/normalize"; -import { createUnsupportedVcsOperationError, getVcsAdapter, operationFromInput } from "./vcs"; +import { normalizePatchText, stripTerminalControl } from "./patch/normalize"; +import { getConfiguredVcsAdapter, loadVcsReview, operationFromInput } from "./vcs"; import type { AppBootstrap, AgentContext, @@ -35,10 +24,10 @@ import type { DiffLineMoveKinds, DiffToolCommandInput, FileCommandInput, - VcsCommandInput, PatchCommandInput, - ShowCommandInput, - StashShowCommandInput, + VcsShowCommandInput, + VcsDiffCommandInput, + VcsStashShowCommandInput, } from "./types"; interface LoadAppBootstrapOptions { @@ -47,10 +36,6 @@ interface LoadAppBootstrapOptions { gitExecutable?: string; } -const LARGE_DIFF_FILE_MAX_BYTES = 1_000_000; -const LARGE_DIFF_FILE_MAX_LINES = 20_000; -const LARGE_DIFF_FILE_SNIFF_BYTES = 256 * 1024; - /** Return the final path segment for display-oriented labels. */ function basename(path: string) { return path.split(/[\\/]/).filter(Boolean).pop() ?? path; @@ -182,192 +167,6 @@ function hasLineMoveKinds(moveKinds: DiffLineMoveKinds | undefined) { return Boolean(moveKinds?.additionLines.some(Boolean) || moveKinds?.deletionLines.some(Boolean)); } -interface CountedLines { - complete: boolean; - lines: number; -} - -/** Count text lines with a byte cap so huge skipped-file stats do not block startup. */ -function countLinesInFile(path: string, maxBytes: number, size: number): CountedLines { - let fd: number | undefined; - - try { - fd = fs.openSync(path, "r"); - const buffer = Buffer.alloc(Math.min(64 * 1024, maxBytes)); - let position = 0; - let lineCount = 0; - let lastByte: number | undefined; - - while (position < maxBytes) { - const bytesToRead = Math.min(buffer.length, maxBytes - position); - const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead, position); - if (bytesRead === 0) { - break; - } - - position += bytesRead; - for (let index = 0; index < bytesRead; index += 1) { - lastByte = buffer[index]; - if (lastByte === 0x0a) { - lineCount += 1; - } - } - } - - return { - complete: position >= size, - lines: lastByte !== undefined && lastByte !== 0x0a ? lineCount + 1 : lineCount, - }; - } catch { - return { complete: true, lines: 0 }; - } finally { - if (fd !== undefined) { - fs.closeSync(fd); - } - } -} - -interface LargeUntrackedFileCheck { - shouldSkip: boolean; - stats?: DiffFile["stats"]; - statsTruncated?: boolean; -} - -/** Return whether an untracked file is too large to synthesize into a full in-memory patch. */ -function inspectLargeUntrackedFile(repoRoot: string, filePath: string): LargeUntrackedFileCheck { - const absolutePath = join(repoRoot, filePath); - - let stat: fs.Stats; - try { - stat = fs.statSync(absolutePath); - } catch { - return { shouldSkip: false }; - } - - const byteLimit = - stat.size > LARGE_DIFF_FILE_MAX_BYTES ? LARGE_DIFF_FILE_MAX_BYTES : LARGE_DIFF_FILE_SNIFF_BYTES; - const counted = countLinesInFile(absolutePath, byteLimit, stat.size); - const shouldSkip = - stat.size > LARGE_DIFF_FILE_MAX_BYTES || counted.lines > LARGE_DIFF_FILE_MAX_LINES; - - return { - shouldSkip, - stats: shouldSkip ? { additions: counted.lines, deletions: 0 } : undefined, - statsTruncated: shouldSkip ? !counted.complete : undefined, - }; -} - -/** Parse one synthetic untracked-file patch and reattach the real path after header normalization. */ -function parseUntrackedPatchFile(patchText: string, filePath: string) { - let parsedPatches: ReturnType; - - try { - parsedPatches = parsePatchFiles(patchText, "patch", true); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to parse untracked file patch for ${JSON.stringify(filePath)}: ${message}`, - ); - } - - const metadataFiles = parsedPatches.flatMap((entry) => entry.files); - if (metadataFiles.length !== 1) { - throw new Error( - `Expected one parsed file for untracked patch ${JSON.stringify(filePath)}, got ${metadataFiles.length}.`, - ); - } - - const metadata = metadataFiles[0]!; - return { - ...metadata, - name: filePath, - prevName: undefined, - } satisfies FileDiffMetadata; -} - -/** Build one reviewable diff file for an untracked working-tree file. */ -function buildUntrackedDiffFile( - input: VcsCommandInput, - filePath: string, - index: number, - repoRoot: string, - sourcePrefix: string, - agentContext: AgentContext | null, - gitExecutable = "git", -) { - const absolutePath = join(repoRoot, filePath); - const largeFileCheck = inspectLargeUntrackedFile(repoRoot, filePath); - if (largeFileCheck.shouldSkip) { - return buildDiffFile( - createSkippedLargeMetadata(filePath, "new"), - "", - index, - sourcePrefix, - agentContext, - { - isTooLarge: true, - isUntracked: true, - stats: largeFileCheck.stats, - statsTruncated: largeFileCheck.statsTruncated, - }, - ); - } - - if (input.options.vcs === "sl") { - if (isProbablyBinaryFile(absolutePath)) { - return buildDiffFile( - createSkippedBinaryMetadata(filePath, "new"), - `Binary file skipped: ${filePath}\n`, - index, - sourcePrefix, - agentContext, - { isBinary: true, isUntracked: true }, - ); - } - - const patch = createTwoFilesPatch( - "/dev/null", - escapeUntrackedPatchPath(filePath), - "", - fs.readFileSync(absolutePath, "utf8"), - "", - "", - { context: 3 }, - ).replaceAll("\r\n", "\n"); - - return buildDiffFile( - parseUntrackedPatchFile(patch, filePath), - patch, - index, - sourcePrefix, - agentContext, - { - isUntracked: true, - }, - ); - } - - const patch = normalizeUntrackedPatchHeaders( - runGitUntrackedFileDiffText(input, filePath, { repoRoot, gitExecutable }), - filePath, - ); - - return buildDiffFile( - parseUntrackedPatchFile(patch, filePath), - patch, - index, - sourcePrefix, - agentContext, - { - isUntracked: true, - sourceFetcherBuilder: createSourceFetcherBuilder(() => ({ - old: { kind: "none" }, - new: { kind: "fs", absolutePath: join(repoRoot, filePath) }, - })), - }, - ); -} - /** Reorder files to follow agent-context narrative order when a sidecar provides one. */ export function orderDiffFiles(files: DiffFile[], agentContext: AgentContext | null) { if (!agentContext || agentContext.files.length === 0) { @@ -570,18 +369,14 @@ async function loadFileDiffChangeset( /** Build a changeset from an adapter-backed VCS review operation. */ async function loadVcsChangeset( - input: VcsCommandInput | ShowCommandInput | StashShowCommandInput, + input: VcsDiffCommandInput | VcsShowCommandInput | VcsStashShowCommandInput, agentContext: AgentContext | null, cwd = process.cwd(), gitExecutable = "git", ) { - const adapter = getVcsAdapter(input.options.vcs ?? "git"); + const adapter = getConfiguredVcsAdapter(input.options.vcs); const operation = operationFromInput(input); - if (!adapter.capabilities.reviewOperations.has(operation.kind)) { - throw createUnsupportedVcsOperationError(adapter, operation); - } - - const result = await adapter.loadReview(operation, { cwd, gitExecutable }); + const result = await loadVcsReview(adapter, operation, { cwd, gitExecutable }); const parsedChangeset = normalizePatchChangeset( result.patchText, result.title, @@ -594,31 +389,9 @@ async function loadVcsChangeset( 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 { - ...parsedChangeset, - files: trackedFiles, - } satisfies Changeset; - } - return { ...parsedChangeset, - files: [ - ...trackedFiles, - ...result.untrackedFiles.map((filePath, index) => - buildUntrackedDiffFile( - operation.input, - filePath, - trackedFiles.length + index, - result.repoRoot, - result.sourceLabel, - agentContext, - gitExecutable, - ), - ), - ], + files: [...parsedChangeset.files, ...adapterFiles], } satisfies Changeset; } diff --git a/src/core/sl.ts b/src/core/sl.ts index 194f6631..8df04966 100644 --- a/src/core/sl.ts +++ b/src/core/sl.ts @@ -1,10 +1,10 @@ import fs from "node:fs"; import { join } from "node:path"; import { HunkUserError } from "./errors"; -import type { VcsCommandInput, ShowCommandInput } from "./types"; +import type { VcsDiffCommandInput, VcsShowCommandInput } from "./types"; import { normalizePathForOS } from "../lib/osPath"; -export type SlBackedInput = VcsCommandInput | ShowCommandInput; +export type SlBackedInput = VcsDiffCommandInput | VcsShowCommandInput; export interface RunSlTextOptions { input: SlBackedInput; @@ -23,7 +23,7 @@ function appendSlPathspecs(args: string[], pathspecs?: string[]) { } /** Build the `sl diff --git` arguments for working-copy and revset reviews. */ -export function buildSlDiffArgs(input: VcsCommandInput) { +export function buildSlDiffArgs(input: VcsDiffCommandInput) { const args = ["diff", "--git"]; if (input.range) { @@ -35,7 +35,7 @@ export function buildSlDiffArgs(input: VcsCommandInput) { } /** Build the `sl diff --git --change` arguments used for `hunk show` in Sapling mode. */ -export function buildSlShowArgs(input: ShowCommandInput) { +export function buildSlShowArgs(input: VcsShowCommandInput) { const args = ["diff", "--git", "--change", input.ref ?? "."]; appendSlPathspecs(args, input.pathspecs); @@ -43,7 +43,7 @@ export function buildSlShowArgs(input: ShowCommandInput) { } /** Build the status query used to discover Sapling unknown files for working-copy review. */ -function buildSlStatusArgs(input: VcsCommandInput) { +function buildSlStatusArgs(input: VcsDiffCommandInput) { const args = ["status", "--unknown", "--print0", "--root-relative"]; appendSlPathspecs(args, input.pathspecs); @@ -108,7 +108,7 @@ function createMissingSlRepoError(input: SlBackedInput) { } /** Return the user-facing error when `--staged` is used with Sapling. */ -export function createSlStagedError(input: VcsCommandInput) { +export function createSlStagedError(input: VcsDiffCommandInput) { return new HunkUserError( `\`${formatSlCommandLabel(input)}\` requires Git VCS mode because Sapling has no staging area.`, ['Remove `--staged`, or set `vcs = "git"` in Hunk config.'], @@ -192,7 +192,7 @@ export function runSlText(options: RunSlTextOptions) { } /** Return whether working-copy review should synthesize unknown Sapling files into the patch stream. */ -function shouldIncludeUntrackedFiles(input: VcsCommandInput) { +function shouldIncludeUntrackedFiles(input: VcsDiffCommandInput) { return !input.staged && input.options.excludeUntracked !== true; } @@ -232,7 +232,7 @@ function isReviewableUntrackedPath(repoRoot: string, filePath: string) { /** Return the repo-root-relative unknown files for a working-copy Sapling review. */ export function listSlUntrackedFiles( - input: VcsCommandInput, + input: VcsDiffCommandInput, { cwd = process.cwd(), repoRoot, diff --git a/src/core/types.ts b/src/core/types.ts index 6ca82835..2c2419c9 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -2,7 +2,7 @@ import type { FileDiffMetadata } from "@pierre/diffs"; import type { FileSourceFetcher } from "./fileSource"; export type LayoutMode = "auto" | "split" | "stack"; -export type VcsMode = "git" | "jj" | "sl"; +export type VcsMode = string; export type TerminalThemeMode = "light" | "dark"; export type ReviewNoteSource = "ai" | "agent" | "user"; @@ -294,7 +294,7 @@ export type SessionCommandInput = | SessionCommentRemoveCommandInput | SessionCommentClearCommandInput; -export interface VcsCommandInput { +export interface VcsDiffCommandInput { kind: "vcs"; range?: string; staged: boolean; @@ -302,14 +302,14 @@ export interface VcsCommandInput { options: CommonOptions; } -export interface ShowCommandInput { +export interface VcsShowCommandInput { kind: "show"; ref?: string; pathspecs?: string[]; options: CommonOptions; } -export interface StashShowCommandInput { +export interface VcsStashShowCommandInput { kind: "stash-show"; ref?: string; options: CommonOptions; @@ -338,9 +338,9 @@ export interface DiffToolCommandInput { } export type CliInput = - | VcsCommandInput - | ShowCommandInput - | StashShowCommandInput + | VcsDiffCommandInput + | VcsShowCommandInput + | VcsStashShowCommandInput | FileCommandInput | PatchCommandInput | DiffToolCommandInput; diff --git a/src/core/vcs/git.test.ts b/src/core/vcs/git.test.ts index f0cbde4e..3a218efa 100644 --- a/src/core/vcs/git.test.ts +++ b/src/core/vcs/git.test.ts @@ -3,13 +3,13 @@ import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "nod import { platform, tmpdir } from "node:os"; import { join } from "node:path"; import { - gitAdapter, + GitVcsAdapter, gitEndpointSourceSpec, parseGitNumstat, shouldSkipLargeTrackedDiff, statSignature, } from "./git"; -import type { ShowCommandInput, StashShowCommandInput, VcsCommandInput } from "../types"; +import type { VcsShowCommandInput, VcsStashShowCommandInput, VcsDiffCommandInput } from "../types"; const tempDirs: string[] = []; @@ -59,13 +59,13 @@ afterEach(() => { } }); -describe("gitAdapter", () => { +describe("GitVcsAdapter", () => { 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 }); + expect(GitVcsAdapter.detect(nested)).toEqual({ id: "git", repoRoot: repo }); }); test("loads working-tree diffs with untracked files through the neutral operation", async () => { @@ -80,14 +80,14 @@ describe("gitAdapter", () => { kind: "vcs", staged: false, options: { vcs: "git" }, - } satisfies VcsCommandInput; - const result = await gitAdapter.loadReview({ kind: "working-tree-diff", input }, { cwd: repo }); + } satisfies VcsDiffCommandInput; + const result = await GitVcsAdapter.operations["working-tree-diff"]!.load(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"]); + expect(result.extraFiles?.map((file) => file.path)).toContain("untracked.txt"); const sourceFetcher = result.sourceFetcherBuilder?.({ path: "tracked.txt", @@ -111,11 +111,10 @@ describe("gitAdapter", () => { kind: "show", ref: "HEAD", options: { vcs: "git" }, - } satisfies ShowCommandInput; - const showResult = await gitAdapter.loadReview( - { kind: "revision-show", input: showInput }, - { cwd: repo }, - ); + } satisfies VcsShowCommandInput; + const showResult = await GitVcsAdapter.operations["revision-show"]!.load(showInput, { + cwd: repo, + }); expect(showResult.title).toContain("show HEAD"); expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); @@ -136,11 +135,10 @@ describe("gitAdapter", () => { const stashInput = { kind: "stash-show", options: { vcs: "git" }, - } satisfies StashShowCommandInput; - const stashResult = await gitAdapter.loadReview( - { kind: "stash-show", input: stashInput }, - { cwd: repo }, - ); + } satisfies VcsStashShowCommandInput; + const stashResult = await GitVcsAdapter.operations["stash-show"]!.load(stashInput, { + cwd: repo, + }); expect(stashResult.title).toContain("stash"); expect(stashResult.patchText).toContain("diff --git a/file.txt b/file.txt"); @@ -149,7 +147,7 @@ describe("gitAdapter", () => { test("returns null when no Git marker exists up to the filesystem root", () => { // A bare temp dir has no .git in any ancestor, exercising the walk-to-root null return. - expect(gitAdapter.detect(createTempDir("hunk-git-adapter-none-"))).toBeNull(); + expect(GitVcsAdapter.detect(createTempDir("hunk-git-adapter-none-"))).toBeNull(); }); test("computes watch signatures for each review operation", () => { @@ -166,32 +164,23 @@ describe("gitAdapter", () => { try { // Measure the working-tree signature while the tree is actually dirty, so the assertion is // meaningful: it must carry the tracked diff and an untracked-file stat signature. - const diffSignature = gitAdapter.watchSignature!( - { - kind: "working-tree-diff", - input: { kind: "vcs", staged: false, options: { vcs: "git" } }, - }, + const diffSignature = GitVcsAdapter.operations["working-tree-diff"]!.watchSignature!( + { kind: "vcs", staged: false, options: { vcs: "git" } }, { cwd: repo }, ); expect(diffSignature).toContain("diff --git a/file.txt b/file.txt"); expect(diffSignature).toContain("untracked:"); - const showSignature = gitAdapter.watchSignature!( - { - kind: "revision-show", - input: { kind: "show", ref: "HEAD", options: { vcs: "git" } }, - }, + const showSignature = GitVcsAdapter.operations["revision-show"]!.watchSignature!( + { kind: "show", ref: "HEAD", options: { vcs: "git" } }, { cwd: repo }, ); expect(showSignature).toContain("diff --git"); // Stash the dirty state so a stash entry exists for the stash-show signature. git(repo, "stash", "push", "--include-untracked", "-m", "watch stash"); - const stashSignature = gitAdapter.watchSignature!( - { - kind: "stash-show", - input: { kind: "stash-show", options: { vcs: "git" } }, - }, + const stashSignature = GitVcsAdapter.operations["stash-show"]!.watchSignature!( + { kind: "stash-show", options: { vcs: "git" } }, { cwd: repo }, ); expect(stashSignature).toContain("diff --git"); diff --git a/src/core/vcs/git.ts b/src/core/vcs/git.ts index 9a5889cd..85e17f11 100644 --- a/src/core/vcs/git.ts +++ b/src/core/vcs/git.ts @@ -6,26 +6,34 @@ import { buildGitShowArgs, buildGitStashShowArgs, listGitUntrackedFiles, + normalizeUntrackedPatchHeaders, resolveGitColorMovedOptions, resolveGitCommitRef, resolveGitDiffEndpoints, resolveGitRepoRoot, runGitText, + runGitUntrackedFileDiffText, type GitDiffEndpoint, type GitDiffEndpoints, } from "../git"; import { + buildDiffFile, createSkippedLargeMetadata, type BuildDiffFileOptions, type DiffFileSourceContext, } from "../diffFile"; import { - createFileSourceFetcher, - type FileSourceFetcherOptions, - type FileSourceSpec, -} from "../fileSource"; -import type { DiffFile } from "../types"; + createGitFileSourceFetcher, + type GitFileSourceFetcherOptions, + type GitFileSourceSpec, +} from "./gitSource"; +import type { DiffFile, VcsDiffCommandInput } from "../types"; import type { VcsAdapter, VcsReviewOperation } from "./types"; +import { + buildSkippedLargeUntrackedDiffFile, + inspectLargeUntrackedFile, + parseUntrackedPatchFile, +} from "./untracked"; const LARGE_DIFF_FILE_MAX_BYTES = 1_000_000; const LARGE_DIFF_FILE_MAX_LINES = 20_000; @@ -107,15 +115,43 @@ function buildSkippedLargeTrackedDiffFile( }; } +/** Build one Git-backed untracked file diff, preserving Git's binary and path quoting behavior. */ +function buildGitUntrackedDiffFile( + input: VcsDiffCommandInput, + filePath: string, + index: number, + repoRoot: string, + sourcePrefix: string, + gitExecutable: string, +) { + const largeFileCheck = inspectLargeUntrackedFile(repoRoot, filePath); + if (largeFileCheck.shouldSkip) { + return buildSkippedLargeUntrackedDiffFile(filePath, index, sourcePrefix, largeFileCheck); + } + + const patch = normalizeUntrackedPatchHeaders( + runGitUntrackedFileDiffText(input, filePath, { repoRoot, gitExecutable }), + filePath, + ); + + return buildDiffFile(parseUntrackedPatchFile(patch, filePath), patch, index, sourcePrefix, null, { + isUntracked: true, + sourceFetcherBuilder: createSourceFetcherBuilder(() => ({ + old: { kind: "none" }, + new: { kind: "fs", absolutePath: join(repoRoot, filePath) }, + })), + }); +} + interface ResolvedFileSourceSpecs { - old: FileSourceSpec; - new: FileSourceSpec; + old: GitFileSourceSpec; + new: GitFileSourceSpec; } /** Build a binary-aware source-fetcher factory from per-file source specs. */ function createSourceFetcherBuilder( resolveSpecs: (file: DiffFileSourceContext) => ResolvedFileSourceSpecs | undefined, - options: FileSourceFetcherOptions = {}, + options: GitFileSourceFetcherOptions = {}, ): NonNullable { return (file) => { if (file.isBinary) { @@ -123,7 +159,7 @@ function createSourceFetcherBuilder( } const specs = resolveSpecs(file); - return specs ? createFileSourceFetcher(specs, options) : undefined; + return specs ? createGitFileSourceFetcher(specs, options) : undefined; }; } @@ -132,7 +168,7 @@ export function gitEndpointSourceSpec( endpoint: GitDiffEndpoint, repoRoot: string, filePath: string, -): FileSourceSpec { +): GitFileSourceSpec { switch (endpoint.kind) { case "none": return { kind: "none" }; @@ -149,7 +185,7 @@ export function gitEndpointSourceSpec( function buildGitEndpointSourceFetcherBuilder( repoRoot: string, endpoints: GitDiffEndpoints, - options: FileSourceFetcherOptions = {}, + options: GitFileSourceFetcherOptions = {}, ): NonNullable { return createSourceFetcherBuilder(({ path, previousPath, type }) => { const oldPath = previousPath ?? path; @@ -169,7 +205,7 @@ function buildRefRangeSourceFetcherBuilder( repoRoot: string, oldRef: string, newRef: string, - options: FileSourceFetcherOptions = {}, + options: GitFileSourceFetcherOptions = {}, ): NonNullable { return buildGitEndpointSourceFetcherBuilder( repoRoot, @@ -213,22 +249,14 @@ function buildGitReviewSourceFetcherBuilder( } /** VCS adapter translating neutral review operations to Git commands. */ -export const gitAdapter: VcsAdapter = { +export const GitVcsAdapter: VcsAdapter = { id: "git", name: "Git", - capabilities: { - reviewOperations: new Set(["working-tree-diff", "revision-show", "stash-show"]), - stagedDiff: true, - sourceFetching: true, - watchSignatures: true, - }, - detect: detectGitRepo, - - async loadReview(operation, { cwd, gitExecutable = "git" }) { - switch (operation.kind) { - case "working-tree-diff": { - const input = operation.input; + operations: { + "working-tree-diff": { + async load(input, { cwd, gitExecutable = "git" }) { + const operation: VcsReviewOperation = { kind: "working-tree-diff", input }; const repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable }); const repoName = basename(repoRoot); const title = input.staged @@ -260,14 +288,36 @@ export const gitAdapter: VcsAdapter = { cwd, gitExecutable, ), - untrackedFiles: listGitUntrackedFiles(input, { cwd, repoRoot, gitExecutable }), - extraFiles: largeTrackedFiles.map((file, index) => - buildSkippedLargeTrackedDiffFile(file, index, repoRoot), - ), + extraFiles: [ + ...largeTrackedFiles.map((file, index) => + buildSkippedLargeTrackedDiffFile(file, index, repoRoot), + ), + ...listGitUntrackedFiles(input, { cwd, repoRoot, gitExecutable }).map( + (filePath, index) => + buildGitUntrackedDiffFile( + input, + filePath, + largeTrackedFiles.length + index, + repoRoot, + repoRoot, + gitExecutable, + ), + ), + ], }; - } - case "revision-show": { - const input = operation.input; + }, + watchSignature(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"); + }, + }, + "revision-show": { + async load(input, { cwd, gitExecutable = "git" }) { + const operation: VcsReviewOperation = { kind: "revision-show", input }; const repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable }); const repoName = basename(repoRoot); return { @@ -290,9 +340,14 @@ export const gitAdapter: VcsAdapter = { gitExecutable, ), }; - } - case "stash-show": { - const input = operation.input; + }, + watchSignature(input) { + return runGitText({ input, args: buildGitShowArgs(input) }); + }, + }, + "stash-show": { + async load(input, { cwd, gitExecutable = "git" }) { + const operation: VcsReviewOperation = { kind: "stash-show", input }; const repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable }); const repoName = basename(repoRoot); return { @@ -315,30 +370,11 @@ export const gitAdapter: VcsAdapter = { gitExecutable, ), }; - } - } - }, - - 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; + }, + watchSignature(input) { return runGitText({ input, args: buildGitStashShowArgs(input) }); - } - } + }, + }, }, }; diff --git a/src/core/vcs/gitSource.ts b/src/core/vcs/gitSource.ts new file mode 100644 index 00000000..fa46e441 --- /dev/null +++ b/src/core/vcs/gitSource.ts @@ -0,0 +1,156 @@ +import { + DEFAULT_SOURCE_TEXT_MAX_BYTES, + SourceTextTooLargeError, + createFileSourceFetcher, + logSourceDiagnostic, + readStreamTextWithLimit, + type FileSourceFetcher, + type FileSourceSide, + type FileSourceSpec, +} from "../fileSource"; + +export type GitFileSourceSpec = + | FileSourceSpec + | { kind: "git-blob"; repoRoot: string; ref: string; path: string } + | { kind: "git-index"; repoRoot: string; path: string }; + +export interface GitFileSourceFetcherOptions { + gitExecutable?: string; + maxSourceBytes?: number; +} + +interface GitResolvedSpecs { + old: GitFileSourceSpec; + new: GitFileSourceSpec; +} + +/** Return whether a Git failure is an expected missing source side/path. */ +function isExpectedMissingGitSource(stderr: string) { + const normalized = stderr.toLowerCase(); + return [ + "exists on disk, but not in", + "does not exist in", + "invalid object name", + "needed a single revision", + "unknown revision or path not in the working tree", + ].some((fragment) => normalized.includes(fragment)); +} + +function readGitBlobSpec( + spec: Extract, + gitExecutable = "git", + maxSourceBytes: number, +): Promise { + return readGitObjectSpec( + spec.repoRoot, + `${spec.ref}:${spec.path}`, + gitExecutable, + maxSourceBytes, + ); +} + +function readGitIndexSpec( + spec: Extract, + gitExecutable = "git", + maxSourceBytes: number, +): Promise { + return readGitObjectSpec(spec.repoRoot, `:${spec.path}`, gitExecutable, maxSourceBytes); +} + +/** Read a blob-like Git object spec such as `HEAD:path` or `:path`. */ +async function readGitObjectSpec( + repoRoot: string, + objectName: string, + gitExecutable = "git", + maxSourceBytes: number, +): Promise { + let proc: Bun.ReadableSubprocess; + + try { + proc = Bun.spawn([gitExecutable, "show", objectName], { + cwd: repoRoot, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + } catch (error) { + logSourceDiagnostic(`failed to run Git while reading source ${objectName}`, error); + return null; + } + + let output: [number, string, string]; + try { + output = await Promise.all([ + proc.exited, + readStreamTextWithLimit(proc.stdout, maxSourceBytes, () => proc.kill()), + readStreamTextWithLimit( + proc.stderr, + 64 * 1024, + undefined, + (maxBytes) => new Error(`Git source diagnostics exceeded ${maxBytes} bytes.`), + ), + ]); + } catch (error) { + if (error instanceof SourceTextTooLargeError) { + proc.kill(); + await proc.exited.catch(() => undefined); + throw error; + } + + logSourceDiagnostic(`failed to collect Git source ${objectName}`, error); + return null; + } + + const [exitCode, stdout, stderr] = output; + + if (exitCode !== 0) { + if (!isExpectedMissingGitSource(stderr)) { + logSourceDiagnostic(`failed to read Git source ${objectName} in ${repoRoot}`, stderr); + } + return null; + } + + return stdout; +} + +async function readGitSpec( + spec: GitFileSourceSpec, + options: GitFileSourceFetcherOptions, +): Promise { + const { gitExecutable = "git", maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES } = options; + if (spec.kind === "git-index") { + return readGitIndexSpec(spec, gitExecutable, maxSourceBytes); + } + + if (spec.kind === "git-blob") { + return readGitBlobSpec(spec, gitExecutable, maxSourceBytes); + } + + return createFileSourceFetcher( + { old: spec, new: { kind: "none" } }, + { maxSourceBytes }, + ).getFullText("old"); +} + +/** Build a Git-aware per-file source fetcher that caches each side's resolved text. */ +export function createGitFileSourceFetcher( + specs: GitResolvedSpecs, + { + gitExecutable = "git", + maxSourceBytes = DEFAULT_SOURCE_TEXT_MAX_BYTES, + }: Readonly = {}, +): FileSourceFetcher { + const cache = new Map(); + + return { + async getFullText(side) { + if (cache.has(side)) { + return cache.get(side) ?? null; + } + + const text = await readGitSpec(specs[side], { gitExecutable, maxSourceBytes }); + cache.set(side, text); + return text; + }, + }; +} diff --git a/src/core/vcs/index.test.ts b/src/core/vcs/index.test.ts index 61ce36a9..f59fba25 100644 --- a/src/core/vcs/index.test.ts +++ b/src/core/vcs/index.test.ts @@ -8,10 +8,11 @@ import { findVcsRepoRootCandidate, getVcsAdapter, isVcsId, + loadVcsReview, operationFromInput, vcsAdapters, } from "."; -import type { ShowCommandInput, StashShowCommandInput, VcsCommandInput } from "../types"; +import type { VcsShowCommandInput, VcsStashShowCommandInput, VcsDiffCommandInput } from "../types"; import type { VcsAdapter } from "./types"; const tempDirs: string[] = []; @@ -32,14 +33,17 @@ afterEach(() => { }); describe("VCS adapter registry", () => { - test("registers Git, Jujutsu, and Sapling adapters", () => { + test("registers Git, Jujutsu, and Sapling operation maps", () => { expect(vcsAdapters.map((adapter) => adapter.id)).toEqual(["jj", "sl", "git"]); - expect(getVcsAdapter("git").capabilities.reviewOperations.has("stash-show")).toBe(true); - expect(getVcsAdapter("git").capabilities.sourceFetching).toBe(true); - expect(getVcsAdapter("jj").capabilities.reviewOperations.has("stash-show")).toBe(false); - expect(getVcsAdapter("jj").capabilities.sourceFetching).toBeUndefined(); - expect(getVcsAdapter("sl").capabilities.reviewOperations.has("stash-show")).toBe(false); - expect(getVcsAdapter("sl").capabilities.sourceFetching).toBeUndefined(); + expect(getVcsAdapter("git").operations["working-tree-diff"]).toBeDefined(); + expect(getVcsAdapter("git").operations["revision-show"]).toBeDefined(); + expect(getVcsAdapter("git").operations["stash-show"]).toBeDefined(); + expect(getVcsAdapter("jj").operations["working-tree-diff"]).toBeDefined(); + expect(getVcsAdapter("jj").operations["revision-show"]).toBeDefined(); + expect(getVcsAdapter("jj").operations["stash-show"]).toBeUndefined(); + expect(getVcsAdapter("sl").operations["working-tree-diff"]).toBeDefined(); + expect(getVcsAdapter("sl").operations["revision-show"]).toBeDefined(); + expect(getVcsAdapter("sl").operations["stash-show"]).toBeUndefined(); }); test("validates VCS ids from the registered adapter list", () => { @@ -62,13 +66,10 @@ describe("VCS adapter registry", () => { 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"); - }, + operations: {}, }; vcsAdapters.unshift(adapter); @@ -106,44 +107,52 @@ describe("VCS adapter registry", () => { kind: "vcs", staged: false, options: { vcs: "git" }, - } satisfies VcsCommandInput; + } satisfies VcsDiffCommandInput; const showInput = { kind: "show", ref: "HEAD", options: { vcs: "git" }, - } satisfies ShowCommandInput; + } satisfies VcsShowCommandInput; const stashInput = { kind: "stash-show", options: { vcs: "git" }, - } satisfies StashShowCommandInput; + } satisfies VcsStashShowCommandInput; 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", () => { + test("creates friendly errors for unsupported adapter operations", async () => { 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.", - ); + } satisfies VcsStashShowCommandInput; + + expect( + createUnsupportedVcsOperationError(adapter, operationFromInput(input).kind).message, + ).toBe("`hunk stash show` requires Git VCS mode."); + await expect( + loadVcsReview(adapter, operationFromInput(input), { cwd: process.cwd() }), + ).rejects.toThrow("`hunk stash show` requires Git VCS mode."); }); test("names the adapter and operation for non-stash unsupported operations", () => { - const adapter = getVcsAdapter("jj"); + const adapter = { + id: "custom", + name: "Custom VCS", + detect: () => null, + operations: {}, + } satisfies VcsAdapter; const input = { kind: "vcs", staged: false, - options: { vcs: "jj" }, - } satisfies VcsCommandInput; + options: { vcs: "custom" }, + } satisfies VcsDiffCommandInput; - expect(createUnsupportedVcsOperationError(adapter, operationFromInput(input)).message).toBe( - "Jujutsu does not support working-tree-diff.", - ); + expect( + createUnsupportedVcsOperationError(adapter, operationFromInput(input).kind).message, + ).toBe("Custom VCS does not support working-tree-diff."); }); }); diff --git a/src/core/vcs/index.ts b/src/core/vcs/index.ts index d3c12ad6..adbd731e 100644 --- a/src/core/vcs/index.ts +++ b/src/core/vcs/index.ts @@ -1,11 +1,32 @@ import { dirname, relative, resolve } from "node:path"; import { HunkUserError } from "../errors"; -import { gitAdapter } from "./git"; -import { jjAdapter } from "./jj"; -import { slAdapter } from "./sl"; -import type { VcsAdapter, VcsDetection, VcsId, VcsReviewInput, VcsReviewOperation } from "./types"; +import { GitVcsAdapter } from "./git"; +import { JjVcsAdapter } from "./jj"; +import { SaplingVcsAdapter } from "./sl"; +import type { + VcsAdapter, + VcsDetection, + VcsId, + VcsLoadContext, + VcsOperation, + VcsPatchResult, + VcsReviewInput, + VcsReviewOperation, + VcsReviewOperationKind, +} from "./types"; -export const vcsAdapters: VcsAdapter[] = [jjAdapter, slAdapter, gitAdapter]; +export const DEFAULT_VCS_ADAPTER = GitVcsAdapter; +export const vcsAdapters: VcsAdapter[] = [JjVcsAdapter, SaplingVcsAdapter, DEFAULT_VCS_ADAPTER]; + +/** Return the fallback adapter used when config has not selected a provider explicitly. */ +export function getDefaultVcsAdapter() { + return DEFAULT_VCS_ADAPTER; +} + +/** Return the configured adapter, or the default adapter when no VCS id was supplied. */ +export function getConfiguredVcsAdapter(id: VcsId | undefined): VcsAdapter { + return id ? getVcsAdapter(id) : getDefaultVcsAdapter(); +} export function getVcsAdapter(id: VcsId): VcsAdapter { const adapter = vcsAdapters.find((candidate) => candidate.id === id); @@ -70,17 +91,57 @@ export function operationFromInput(input: VcsReviewInput): VcsReviewOperation { } } -export function createUnsupportedVcsOperationError( +/** Return the adapter operation handler for one neutral review operation, if supported. */ +export function getVcsOperation( + adapter: VcsAdapter, + operation: VcsReviewOperation, +): VcsOperation | undefined { + return adapter.operations[operation.kind] as VcsOperation | undefined; +} + +/** Load a review through the adapter operation map instead of adapter-local switch dispatch. */ +export async function loadVcsReview( adapter: VcsAdapter, operation: VcsReviewOperation, + context: VcsLoadContext, +): Promise { + const handler = getVcsOperation(adapter, operation); + if (!handler) { + throw createUnsupportedVcsOperationError(adapter, operation.kind); + } + + return await handler.load(operation.input, context); +} + +/** Build an adapter-backed watch signature when the selected operation supports it. */ +export function createVcsWatchSignature( + adapter: VcsAdapter, + operation: VcsReviewOperation, + context: VcsLoadContext, +) { + const handler = getVcsOperation(adapter, operation); + if (!handler) { + throw createUnsupportedVcsOperationError(adapter, operation.kind); + } + if (!handler.watchSignature) { + throw new Error(`${adapter.name} does not support watch signatures for ${operation.kind}.`); + } + + return handler.watchSignature(operation.input, context); +} + +export function createUnsupportedVcsOperationError( + adapter: VcsAdapter, + operationKind: VcsReviewOperationKind, ) { - if (operation.kind === "stash-show") { - return new HunkUserError("`hunk stash show` requires Git VCS mode.", [ - 'Set `vcs = "git"` in Hunk config, then try again.', + const supportingAdapter = vcsAdapters.find((candidate) => candidate.operations[operationKind]); + if (operationKind === "stash-show" && supportingAdapter) { + return new HunkUserError(`\`hunk stash show\` requires ${supportingAdapter.name} VCS mode.`, [ + `Set \`vcs = "${supportingAdapter.id}"\` in Hunk config, then try again.`, ]); } - return new HunkUserError(`${adapter.name} does not support ${operation.kind}.`, [ + return new HunkUserError(`${adapter.name} does not support ${operationKind}.`, [ "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 index 1e285828..9cf70dad 100644 --- a/src/core/vcs/jj.test.ts +++ b/src/core/vcs/jj.test.ts @@ -2,8 +2,8 @@ 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"; +import { JjVcsAdapter } from "./jj"; +import type { VcsShowCommandInput, VcsDiffCommandInput } from "../types"; const tempDirs: string[] = []; const JjAdapterIntegrationTestTimeoutMs = 20_000; @@ -66,7 +66,7 @@ afterEach(() => { // Keep jj-backed adapter coverage opt-in on machines that have the external CLI installed. const jjTest = Bun.which("jj") ? test : test.skip; -describe("jjAdapter", () => { +describe("JjVcsAdapter", () => { jjTest( "detects Jujutsu repositories from nested directories", () => { @@ -74,7 +74,7 @@ describe("jjAdapter", () => { const nested = join(repo, "src", "nested"); mkdirSync(nested, { recursive: true }); - expect(jjAdapter.detect(nested)).toEqual({ id: "jj", repoRoot: repo }); + expect(JjVcsAdapter.detect(nested)).toEqual({ id: "jj", repoRoot: repo }); }, JjAdapterIntegrationTestTimeoutMs, ); @@ -91,11 +91,10 @@ describe("jjAdapter", () => { kind: "vcs", staged: false, options: { vcs: "jj" }, - } satisfies VcsCommandInput; - const diffResult = await jjAdapter.loadReview( - { kind: "working-tree-diff", input: diffInput }, - { cwd: repo }, - ); + } satisfies VcsDiffCommandInput; + const diffResult = await JjVcsAdapter.operations["working-tree-diff"]!.load(diffInput, { + cwd: repo, + }); expect(normalizeComparablePath(diffResult.repoRoot)).toBe(normalizeComparablePath(repo)); expect(diffResult.title).toContain("working copy"); @@ -107,11 +106,10 @@ describe("jjAdapter", () => { kind: "show", ref: "@", options: { vcs: "jj" }, - } satisfies ShowCommandInput; - const showResult = await jjAdapter.loadReview( - { kind: "revision-show", input: showInput }, - { cwd: repo }, - ); + } satisfies VcsShowCommandInput; + const showResult = await JjVcsAdapter.operations["revision-show"]!.load(showInput, { + cwd: repo, + }); expect(showResult.title).toContain("show @"); expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); @@ -127,36 +125,29 @@ describe("jjAdapter", () => { kind: "vcs", staged: true, options: { vcs: "jj" }, - } satisfies VcsCommandInput; - const stashInput = { - kind: "stash-show", - options: { vcs: "jj" }, - } satisfies StashShowCommandInput; - + } satisfies VcsDiffCommandInput; await expect( - jjAdapter.loadReview({ kind: "working-tree-diff", input: stagedInput }, { cwd: repo }), + JjVcsAdapter.operations["working-tree-diff"]!.load(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"); + expect(JjVcsAdapter.operations["stash-show"]).toBeUndefined(); }, JjAdapterIntegrationTestTimeoutMs, ); }); // These branches run before any `jj` invocation, so they need no external binary. -describe("jjAdapter without the jj binary", () => { +describe("JjVcsAdapter without the jj binary", () => { test("detects a .jj workspace marker from a nested directory", () => { const repo = createTempDir("hunk-jj-detect-marker-"); mkdirSync(join(repo, ".jj")); const nested = join(repo, "src", "nested"); mkdirSync(nested, { recursive: true }); - expect(jjAdapter.detect(nested)).toEqual({ id: "jj", repoRoot: repo }); + expect(JjVcsAdapter.detect(nested)).toEqual({ id: "jj", repoRoot: repo }); }); test("returns null when no .jj marker exists up to the filesystem root", () => { - expect(jjAdapter.detect(createTempDir("hunk-jj-detect-none-"))).toBeNull(); + expect(JjVcsAdapter.detect(createTempDir("hunk-jj-detect-none-"))).toBeNull(); }); test("rejects staged working-tree diffs before spawning jj", async () => { @@ -164,22 +155,13 @@ describe("jjAdapter without the jj binary", () => { kind: "vcs", staged: true, options: { vcs: "jj" }, - } satisfies VcsCommandInput; + } satisfies VcsDiffCommandInput; await expect( - jjAdapter.loadReview({ kind: "working-tree-diff", input: stagedInput }, { cwd: tmpdir() }), + JjVcsAdapter.operations["working-tree-diff"]!.load(stagedInput, { cwd: tmpdir() }), ).rejects.toThrow("Jujutsu has no staging area"); }); - test("rejects stash-show in both loadReview and watchSignature", async () => { - const stashInput = { - kind: "stash-show", - options: { vcs: "jj" }, - } satisfies StashShowCommandInput; - await expect( - jjAdapter.loadReview({ kind: "stash-show", input: stashInput }, { cwd: tmpdir() }), - ).rejects.toThrow("requires Git VCS mode"); - expect(() => - jjAdapter.watchSignature!({ kind: "stash-show", input: stashInput }, { cwd: tmpdir() }), - ).toThrow("requires Git VCS mode"); + test("does not expose a stash-show operation", () => { + expect(JjVcsAdapter.operations["stash-show"]).toBeUndefined(); }); }); diff --git a/src/core/vcs/jj.ts b/src/core/vcs/jj.ts index 17e2c128..87c9ebcd 100644 --- a/src/core/vcs/jj.ts +++ b/src/core/vcs/jj.ts @@ -1,6 +1,5 @@ import fs from "node:fs"; import { dirname, join, resolve } from "node:path"; -import { HunkUserError } from "../errors"; import { buildJjDiffArgs, buildJjShowArgs, @@ -30,29 +29,14 @@ function detectJjRepo(cwd: string) { } } -/** 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 = { +export const JjVcsAdapter: 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; + operations: { + "working-tree-diff": { + async load(input, { cwd }) { if (input.staged) { throw createJjStagedError(input); } @@ -64,9 +48,13 @@ export const jjAdapter: VcsAdapter = { title: input.range ? `${repoName} ${input.range}` : `${repoName} working copy`, patchText: runJjText({ input, args: buildJjDiffArgs(input), cwd }), }; - } - case "revision-show": { - const input = operation.input; + }, + watchSignature(input) { + return runJjText({ input, args: buildJjDiffArgs(input) }); + }, + }, + "revision-show": { + async load(input, { cwd }) { const repoRoot = resolveJjRepoRoot(input, { cwd }); const repoName = basename(repoRoot); const revset = input.ref ?? "@"; @@ -76,24 +64,10 @@ export const jjAdapter: VcsAdapter = { 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; + }, + watchSignature(input) { return runJjText({ input, args: buildJjShowArgs(input) }); - } - case "stash-show": - throw createJjUnsupportedStashShowError(); - } + }, + }, }, }; diff --git a/src/core/vcs/sl.test.ts b/src/core/vcs/sl.test.ts index 7b251ccc..f1b02441 100644 --- a/src/core/vcs/sl.test.ts +++ b/src/core/vcs/sl.test.ts @@ -2,8 +2,8 @@ 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 { slAdapter } from "./sl"; -import type { ShowCommandInput, StashShowCommandInput, VcsCommandInput } from "../types"; +import { SaplingVcsAdapter } from "./sl"; +import type { VcsShowCommandInput, VcsDiffCommandInput } from "../types"; const slAvailable = (() => { try { @@ -62,14 +62,14 @@ afterEach(() => { } }); -describe("slAdapter", () => { +describe("SaplingVcsAdapter", () => { test("detects Sapling repositories from nested directories", () => { const repo = createTempDir("hunk-sl-adapter-detect-"); mkdirSync(join(repo, ".sl")); const nested = join(repo, "src", "nested"); mkdirSync(nested, { recursive: true }); - expect(slAdapter.detect(nested)).toEqual({ id: "sl", repoRoot: repo }); + expect(SaplingVcsAdapter.detect(nested)).toEqual({ id: "sl", repoRoot: repo }); }); test("auto-detects .hg directories with treestate as Sapling", () => { @@ -77,7 +77,7 @@ describe("slAdapter", () => { mkdirSync(join(repo, ".hg")); writeFileSync(join(repo, ".hg", "requires"), "revlogv1\nstore\ntreestate\n"); - expect(slAdapter.detect(repo)).toEqual({ id: "sl", repoRoot: repo }); + expect(SaplingVcsAdapter.detect(repo)).toEqual({ id: "sl", repoRoot: repo }); }); test("does not auto-detect .hg directories without treestate", () => { @@ -85,7 +85,7 @@ describe("slAdapter", () => { mkdirSync(join(repo, ".hg")); writeFileSync(join(repo, ".hg", "requires"), "revlogv1\nstore\n"); - expect(slAdapter.detect(repo)).toBeNull(); + expect(SaplingVcsAdapter.detect(repo)).toBeNull(); }); test.skipIf(!slAvailable)( @@ -101,11 +101,10 @@ describe("slAdapter", () => { kind: "vcs", staged: false, options: { vcs: "sl" }, - } satisfies VcsCommandInput; - const diffResult = await slAdapter.loadReview( - { kind: "working-tree-diff", input: diffInput }, - { cwd: repo }, - ); + } satisfies VcsDiffCommandInput; + const diffResult = await SaplingVcsAdapter.operations["working-tree-diff"]!.load(diffInput, { + cwd: repo, + }); expect(normalizeComparablePath(diffResult.repoRoot)).toBe(normalizeComparablePath(repo)); expect(diffResult.title).toContain("working copy"); @@ -116,11 +115,10 @@ describe("slAdapter", () => { kind: "show", ref: ".", options: { vcs: "sl" }, - } satisfies ShowCommandInput; - const showResult = await slAdapter.loadReview( - { kind: "revision-show", input: showInput }, - { cwd: repo }, - ); + } satisfies VcsShowCommandInput; + const showResult = await SaplingVcsAdapter.operations["revision-show"]!.load(showInput, { + cwd: repo, + }); expect(showResult.title).toContain("show ."); expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); @@ -136,34 +134,27 @@ describe("slAdapter", () => { kind: "vcs", staged: true, options: { vcs: "sl" }, - } satisfies VcsCommandInput; - const stashInput = { - kind: "stash-show", - options: { vcs: "sl" }, - } satisfies StashShowCommandInput; - + } satisfies VcsDiffCommandInput; await expect( - slAdapter.loadReview({ kind: "working-tree-diff", input: stagedInput }, { cwd: repo }), + SaplingVcsAdapter.operations["working-tree-diff"]!.load(stagedInput, { cwd: repo }), ).rejects.toThrow("Sapling has no staging area"); - await expect( - slAdapter.loadReview({ kind: "stash-show", input: stashInput }, { cwd: repo }), - ).rejects.toThrow("requires Git VCS mode"); + expect(SaplingVcsAdapter.operations["stash-show"]).toBeUndefined(); }, SlAdapterIntegrationTestTimeoutMs, ); }); // These branches run before any `sl` invocation, so they need no external binary. -describe("slAdapter without the sl binary", () => { +describe("SaplingVcsAdapter without the sl binary", () => { test("treats a .hg directory with no requires file as non-Sapling", () => { const repo = createTempDir("hunk-sl-hg-no-requires-"); mkdirSync(join(repo, ".hg")); // No `.hg/requires` file, so the Sapling check reads a missing file and falls back to false. - expect(slAdapter.detect(repo)).toBeNull(); + expect(SaplingVcsAdapter.detect(repo)).toBeNull(); }); test("returns null when no Sapling marker exists up to the filesystem root", () => { - expect(slAdapter.detect(createTempDir("hunk-sl-detect-none-"))).toBeNull(); + expect(SaplingVcsAdapter.detect(createTempDir("hunk-sl-detect-none-"))).toBeNull(); }); test("rejects staged working-tree diffs before spawning sl", async () => { @@ -171,22 +162,13 @@ describe("slAdapter without the sl binary", () => { kind: "vcs", staged: true, options: { vcs: "sl" }, - } satisfies VcsCommandInput; + } satisfies VcsDiffCommandInput; await expect( - slAdapter.loadReview({ kind: "working-tree-diff", input: stagedInput }, { cwd: tmpdir() }), + SaplingVcsAdapter.operations["working-tree-diff"]!.load(stagedInput, { cwd: tmpdir() }), ).rejects.toThrow("Sapling has no staging area"); }); - test("rejects stash-show in both loadReview and watchSignature", async () => { - const stashInput = { - kind: "stash-show", - options: { vcs: "sl" }, - } satisfies StashShowCommandInput; - await expect( - slAdapter.loadReview({ kind: "stash-show", input: stashInput }, { cwd: tmpdir() }), - ).rejects.toThrow("requires Git VCS mode"); - expect(() => - slAdapter.watchSignature!({ kind: "stash-show", input: stashInput }, { cwd: tmpdir() }), - ).toThrow("requires Git VCS mode"); + test("does not expose a stash-show operation", () => { + expect(SaplingVcsAdapter.operations["stash-show"]).toBeUndefined(); }); }); diff --git a/src/core/vcs/sl.ts b/src/core/vcs/sl.ts index 8147fae9..0e9588a9 100644 --- a/src/core/vcs/sl.ts +++ b/src/core/vcs/sl.ts @@ -1,6 +1,5 @@ import fs from "node:fs"; import { dirname, join, resolve } from "node:path"; -import { HunkUserError } from "../errors"; import { buildSlDiffArgs, buildSlShowArgs, @@ -10,6 +9,7 @@ import { runSlText, } from "../sl"; import type { VcsAdapter } from "./types"; +import { buildFilesystemUntrackedDiffFile } from "./untracked"; /** Return the last path segment for review titles. */ function basename(path: string) { @@ -45,13 +45,6 @@ function detectSlRepo(cwd: string) { } } -/** Return the user-facing error for Sapling operations that only Git supports. */ -function createSlUnsupportedStashShowError() { - return new HunkUserError("`hunk stash show` requires Git VCS mode.", [ - 'Set `vcs = "git"` in Hunk config, then try again.', - ]); -} - /** Format one file stat into a stable signature fragment, or mark the path missing. */ function statSignature(path: string) { if (!fs.existsSync(path)) { @@ -63,21 +56,13 @@ function statSignature(path: string) { } /** VCS adapter translating neutral review operations to Sapling commands. */ -export const slAdapter: VcsAdapter = { +export const SaplingVcsAdapter: VcsAdapter = { id: "sl", name: "Sapling", - capabilities: { - reviewOperations: new Set(["working-tree-diff", "revision-show"]), - stagedDiff: false, - watchSignatures: true, - }, - detect: detectSlRepo, - - async loadReview(operation, { cwd }) { - switch (operation.kind) { - case "working-tree-diff": { - const input = operation.input; + operations: { + "working-tree-diff": { + async load(input, { cwd }) { if (input.staged) { throw createSlStagedError(input); } @@ -88,11 +73,22 @@ export const slAdapter: VcsAdapter = { sourceLabel: repoRoot, title: input.range ? `${repoName} ${input.range}` : `${repoName} working copy`, patchText: runSlText({ input, args: buildSlDiffArgs(input), cwd }), - untrackedFiles: listSlUntrackedFiles(input, { cwd, repoRoot }), + extraFiles: listSlUntrackedFiles(input, { cwd, repoRoot }).map((filePath, index) => + buildFilesystemUntrackedDiffFile(repoRoot, filePath, index, repoRoot), + ), }; - } - case "revision-show": { - const input = operation.input; + }, + watchSignature(input, { cwd }) { + const trackedPatch = runSlText({ input, args: buildSlDiffArgs(input), cwd }); + const repoRoot = resolveSlRepoRoot(input, { cwd }); + const untrackedSignatures = listSlUntrackedFiles(input, { cwd, repoRoot }).map( + (filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`, + ); + return [trackedPatch, ...untrackedSignatures].join("\n---\n"); + }, + }, + "revision-show": { + async load(input, { cwd }) { const repoRoot = resolveSlRepoRoot(input, { cwd }); const repoName = basename(repoRoot); const revset = input.ref ?? "."; @@ -102,29 +98,10 @@ export const slAdapter: VcsAdapter = { title: `${repoName} show ${revset}`, patchText: runSlText({ input, args: buildSlShowArgs(input), cwd }), }; - } - case "stash-show": - throw createSlUnsupportedStashShowError(); - } - }, - - watchSignature(operation, { cwd }) { - switch (operation.kind) { - case "working-tree-diff": { - const input = operation.input; - const trackedPatch = runSlText({ input, args: buildSlDiffArgs(input), cwd }); - const repoRoot = resolveSlRepoRoot(input, { cwd }); - const untrackedSignatures = listSlUntrackedFiles(input, { cwd, repoRoot }).map( - (filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`, - ); - return [trackedPatch, ...untrackedSignatures].join("\n---\n"); - } - case "revision-show": { - const input = operation.input; + }, + watchSignature(input, { cwd }) { return runSlText({ input, args: buildSlShowArgs(input), cwd }); - } - case "stash-show": - throw createSlUnsupportedStashShowError(); - } + }, + }, }, }; diff --git a/src/core/vcs/types.ts b/src/core/vcs/types.ts index d04a8f1f..ea9588b3 100644 --- a/src/core/vcs/types.ts +++ b/src/core/vcs/types.ts @@ -1,13 +1,12 @@ import type { BuildDiffFileOptions } from "../diffFile"; import type { DiffFile, - ShowCommandInput, - StashShowCommandInput, - VcsCommandInput, - VcsMode, + VcsShowCommandInput, + VcsStashShowCommandInput, + VcsDiffCommandInput, } from "../types"; -export type VcsId = VcsMode; +export type VcsId = string; export interface VcsDetection { id: VcsId; @@ -19,20 +18,24 @@ export interface VcsLoadContext { gitExecutable?: string; } -export type VcsReviewInput = VcsCommandInput | ShowCommandInput | StashShowCommandInput; +export type VcsReviewInput = VcsDiffCommandInput | VcsShowCommandInput | VcsStashShowCommandInput; export type VcsReviewOperation = - | { kind: "working-tree-diff"; input: VcsCommandInput } - | { kind: "revision-show"; input: ShowCommandInput } - | { kind: "stash-show"; input: StashShowCommandInput }; + | { kind: "working-tree-diff"; input: VcsDiffCommandInput } + | { kind: "revision-show"; input: VcsShowCommandInput } + | { kind: "stash-show"; input: VcsStashShowCommandInput }; export type VcsReviewOperationKind = VcsReviewOperation["kind"]; -export interface VcsCapabilities { - reviewOperations: ReadonlySet; - stagedDiff?: boolean; - sourceFetching?: boolean; - watchSignatures?: boolean; +export interface VcsOperation { + load(input: Input, context: VcsLoadContext): Promise; + watchSignature?: (input: Input, context: VcsLoadContext) => string; +} + +export interface VcsOperations { + "working-tree-diff"?: VcsOperation; + "revision-show"?: VcsOperation; + "stash-show"?: VcsOperation; } export interface VcsPatchResult { @@ -41,15 +44,12 @@ export interface VcsPatchResult { title: string; patchText: string; sourceFetcherBuilder?: BuildDiffFileOptions["sourceFetcherBuilder"]; - 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; + operations: VcsOperations; } diff --git a/src/core/vcs/untracked.ts b/src/core/vcs/untracked.ts new file mode 100644 index 00000000..d9d7df54 --- /dev/null +++ b/src/core/vcs/untracked.ts @@ -0,0 +1,172 @@ +import { parsePatchFiles, type FileDiffMetadata } from "@pierre/diffs"; +import { createTwoFilesPatch } from "diff"; +import fs from "node:fs"; +import { join } from "node:path"; +import { createSkippedBinaryMetadata, isProbablyBinaryFile } from "../binary"; +import { buildDiffFile, createSkippedLargeMetadata } from "../diffFile"; +import { escapeUntrackedPatchPath } from "../patch/normalize"; +import type { DiffFile } from "../types"; + +const LARGE_DIFF_FILE_MAX_BYTES = 1_000_000; +const LARGE_DIFF_FILE_MAX_LINES = 20_000; +const LARGE_DIFF_FILE_SNIFF_BYTES = 256 * 1024; + +interface CountedLines { + complete: boolean; + lines: number; +} + +/** Count text lines with a byte cap so huge skipped-file stats do not block startup. */ +function countLinesInFile(path: string, maxBytes: number, size: number): CountedLines { + let fd: number | undefined; + + try { + fd = fs.openSync(path, "r"); + const buffer = Buffer.alloc(Math.min(64 * 1024, maxBytes)); + let position = 0; + let lineCount = 0; + let lastByte: number | undefined; + + while (position < maxBytes) { + const bytesToRead = Math.min(buffer.length, maxBytes - position); + const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead, position); + if (bytesRead === 0) { + break; + } + + position += bytesRead; + for (let index = 0; index < bytesRead; index += 1) { + lastByte = buffer[index]; + if (lastByte === 0x0a) { + lineCount += 1; + } + } + } + + return { + complete: position >= size, + lines: lastByte !== undefined && lastByte !== 0x0a ? lineCount + 1 : lineCount, + }; + } catch { + return { complete: true, lines: 0 }; + } finally { + if (fd !== undefined) { + fs.closeSync(fd); + } + } +} + +export interface LargeUntrackedFileCheck { + shouldSkip: boolean; + stats?: DiffFile["stats"]; + statsTruncated?: boolean; +} + +/** Return whether an untracked file is too large to synthesize into a full in-memory patch. */ +export function inspectLargeUntrackedFile( + repoRoot: string, + filePath: string, +): LargeUntrackedFileCheck { + const absolutePath = join(repoRoot, filePath); + + let stat: fs.Stats; + try { + stat = fs.statSync(absolutePath); + } catch { + return { shouldSkip: false }; + } + + const byteLimit = + stat.size > LARGE_DIFF_FILE_MAX_BYTES ? LARGE_DIFF_FILE_MAX_BYTES : LARGE_DIFF_FILE_SNIFF_BYTES; + const counted = countLinesInFile(absolutePath, byteLimit, stat.size); + const shouldSkip = + stat.size > LARGE_DIFF_FILE_MAX_BYTES || counted.lines > LARGE_DIFF_FILE_MAX_LINES; + + return { + shouldSkip, + stats: shouldSkip ? { additions: counted.lines, deletions: 0 } : undefined, + statsTruncated: shouldSkip ? !counted.complete : undefined, + }; +} + +/** Build a skipped placeholder for one untracked file that is too large to render. */ +export function buildSkippedLargeUntrackedDiffFile( + filePath: string, + index: number, + sourcePrefix: string, + largeFileCheck: LargeUntrackedFileCheck, +) { + return buildDiffFile(createSkippedLargeMetadata(filePath, "new"), "", index, sourcePrefix, null, { + isTooLarge: true, + isUntracked: true, + stats: largeFileCheck.stats, + statsTruncated: largeFileCheck.statsTruncated, + }); +} + +/** Parse one synthetic untracked-file patch and reattach the real path after header normalization. */ +export function parseUntrackedPatchFile(patchText: string, filePath: string) { + let parsedPatches: ReturnType; + + try { + parsedPatches = parsePatchFiles(patchText, "patch", true); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Failed to parse untracked file patch for ${JSON.stringify(filePath)}: ${message}`, + ); + } + + const metadataFiles = parsedPatches.flatMap((entry) => entry.files); + if (metadataFiles.length !== 1) { + throw new Error( + `Expected one parsed file for untracked patch ${JSON.stringify(filePath)}, got ${metadataFiles.length}.`, + ); + } + + const metadata = metadataFiles[0]!; + return { + ...metadata, + name: filePath, + prevName: undefined, + } satisfies FileDiffMetadata; +} + +/** Build one filesystem-backed untracked file diff from its current contents. */ +export function buildFilesystemUntrackedDiffFile( + repoRoot: string, + filePath: string, + index: number, + sourcePrefix: string, +) { + const absolutePath = join(repoRoot, filePath); + const largeFileCheck = inspectLargeUntrackedFile(repoRoot, filePath); + if (largeFileCheck.shouldSkip) { + return buildSkippedLargeUntrackedDiffFile(filePath, index, sourcePrefix, largeFileCheck); + } + + if (isProbablyBinaryFile(absolutePath)) { + return buildDiffFile( + createSkippedBinaryMetadata(filePath, "new"), + `Binary file skipped: ${filePath}\n`, + index, + sourcePrefix, + null, + { isBinary: true, isUntracked: true }, + ); + } + + const patch = createTwoFilesPatch( + "/dev/null", + escapeUntrackedPatchPath(filePath), + "", + fs.readFileSync(absolutePath, "utf8"), + "", + "", + { context: 3 }, + ).replaceAll("\r\n", "\n"); + + return buildDiffFile(parseUntrackedPatchFile(patch, filePath), patch, index, sourcePrefix, null, { + isUntracked: true, + }); +} diff --git a/src/core/watch.ts b/src/core/watch.ts index 8b893c33..04de1dfc 100644 --- a/src/core/watch.ts +++ b/src/core/watch.ts @@ -1,5 +1,5 @@ import fs from "node:fs"; -import { createUnsupportedVcsOperationError, getVcsAdapter, operationFromInput } from "./vcs"; +import { createVcsWatchSignature, getConfiguredVcsAdapter, operationFromInput } from "./vcs"; import type { CliInput } from "./types"; /** Return whether the current input can be rebuilt from files or VCS state without rereading stdin. */ @@ -23,15 +23,9 @@ function statSignature(path: string) { /** 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.`); - } + const adapter = getConfiguredVcsAdapter(input.options.vcs); const operation = operationFromInput(input); - if (!adapter.capabilities.reviewOperations.has(operation.kind)) { - throw createUnsupportedVcsOperationError(adapter, operation); - } - return adapter.watchSignature(operation, { cwd: process.cwd() }); + return createVcsWatchSignature(adapter, operation, { cwd: process.cwd() }); } /** Compute a change-detection signature for one watchable input. */ export function computeWatchSignature(input: CliInput) { diff --git a/test/helpers/app-bootstrap.ts b/test/helpers/app-bootstrap.ts index d49cb88f..6b8b664e 100644 --- a/test/helpers/app-bootstrap.ts +++ b/test/helpers/app-bootstrap.ts @@ -1,4 +1,4 @@ -import type { AppBootstrap, DiffFile, VcsCommandInput, LayoutMode } from "../../src/core/types"; +import type { AppBootstrap, DiffFile, VcsDiffCommandInput, LayoutMode } from "../../src/core/types"; export function createTestVcsAppBootstrap({ agentSummary, @@ -21,7 +21,7 @@ export function createTestVcsAppBootstrap({ agentSummary?: string; changesetId?: string; files: DiffFile[]; - vcsOptions?: Partial; + vcsOptions?: Partial; initialMode?: LayoutMode; initialCopyDecorations?: boolean; initialShowAgentNotes?: boolean;