Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/calm-vcs-foundation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
15 changes: 13 additions & 2 deletions .dependency-cruiser.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,24 @@ module.exports = {
from: { path: "^packages/hunk/src/extension-api/" },
to: { path: "^packages/", pathNot: "^packages/hunk/src/extension-api/" },
},
{
name: "hunk-vcs-stays-provider-neutral",
comment:
"@hunk/vcs owns dependency-bottom implementation helpers; it never imports Hunk, public contracts, or a provider implementation.",
severity: "error",
from: { path: "^packages/hunk-vcs/src/" },
to: { path: "^packages/", pathNot: "^packages/hunk-vcs/src/" },
},
{
name: "lib-is-a-leaf",
comment:
"packages/hunk/src/lib holds dependency-free helpers usable from any tier; it may reach the import-free extension API contract and nothing else.",
"packages/hunk/src/lib holds leaf compatibility exports; it may reach the extension contract and dependency-bottom @hunk/vcs helpers only.",
severity: "error",
from: { path: "^packages/hunk/src/lib/" },
to: { path: "^packages/", pathNot: "^packages/hunk/src/(lib|extension-api)/" },
to: {
path: "^packages/",
pathNot: "^packages/(hunk/src/(lib|extension-api)/|hunk-vcs/src/)",
},
},
{
name: "core-stays-domain",
Expand Down
7 changes: 7 additions & 0 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions docs/module-boundaries.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ Tiers, bottom to top. A tier may import anything strictly below it and nothing a

```text
packages/hunk/src/extension-api published contract; imports nothing
packages/hunk/src/lib dependency-free helpers; may import extension-api only
packages/hunk/src/core domain model (changesets, review, vcs catalog, config)
packages/* standalone publishable units (session broker, term-video);
never import packages/hunk/src/; the per-app broker contract is in
docs/session-broker-sdk.md
packages/hunk-vcs private dependency-bottom VCS implementation helpers
packages/hunk/src/lib compatibility helpers; may import extension-api and hunk-vcs
packages/hunk/src/core domain model (changesets, review, vcs catalog, config)
packages/* isolated workspace units; private provider packages may import
hunkdiff/extension and explicit @hunk/vcs subpaths, while other
packages never import packages/hunk/src/; the per-app broker
contract is in docs/session-broker-sdk.md
packages/hunk/src/extensions extension host + bundled extensions; consume core, never surfaces
packages/hunk/src/session daemon/broker transport + protocol; consumes core and packages
packages/hunk/src/app startup composition: CLI parsing plus the wiring of core,
Expand Down
5 changes: 4 additions & 1 deletion knip.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
"project": ["src/**/*.{ts,tsx}", "bin/**/*.cjs"],
"ignoreExportsUsedInFile": true
},
"packages/hunk-vcs": {
"entry": ["src/**/*.test.ts"],
"project": ["src/**/*.ts"]
},
"packages/session-broker*": {
"entry": ["src/**/*.test.ts"],
"project": ["src/**/*.ts"]
Expand All @@ -55,7 +59,6 @@
"packages/hunk/src/core/process/updateNotice.ts": ["exports", "types"],
"packages/hunk/src/extensions/types.ts": ["exports", "types"],
"packages/hunk/src/extensions/default/ui/sidebar/index.tsx": ["exports", "types"],
"packages/hunk/src/extensions/default/vcs/diffRange.ts": ["exports"],
"packages/hunk/src/session/broker/appContract.ts": ["exports", "types"],
"packages/hunk/src/ui/diff/worker/index.ts": ["exports", "types"]
}
Expand Down
1 change: 1 addition & 0 deletions nix/bun.lock.nix
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"@hunk/session-broker-core" = copyPathToStore ../packages/session-broker-core;
"@hunk/session-broker-node" = copyPathToStore ../packages/session-broker-node;
"@hunk/term-video" = copyPathToStore ../packages/term-video;
"@hunk/vcs" = copyPathToStore ../packages/hunk-vcs;
"@napi-rs/wasm-runtime@1.2.2" = fetchurl {
url = "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.2.2.tgz";
hash = "sha512-JfB4kuJQjaoHuCTseIINHtHWeJnvgEcxjwA5t/Y00ZgaOO1Crz3fjT/p8kT28zA/Caz7oiUMn3d6H2yOVCVwuw==";
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"@hunk/session-broker-bun": "workspace:*",
"@hunk/session-broker-core": "workspace:*",
"@hunk/term-video": "workspace:*",
"@hunk/vcs": "workspace:*",
"@opentui/core": "^0.5.6",
"@opentui/react": "^0.5.6",
"@pierre/diffs": "1.3.5",
Expand Down
38 changes: 38 additions & 0 deletions packages/hunk-vcs/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"name": "@hunk/vcs",
"version": "0.0.0",
"private": true,
"description": "Shared implementation infrastructure for Hunk VCS backends.",
"license": "MIT",
"files": [
"src"
],
"type": "module",
"sideEffects": false,
"exports": {
"./async-process": {
"types": "./src/async-process.ts",
"import": "./src/async-process.ts"
},
"./diff-target": {
"types": "./src/diff-target.ts",
"import": "./src/diff-target.ts"
},
"./large-file": {
"types": "./src/large-file.ts",
"import": "./src/large-file.ts"
},
"./path": {
"types": "./src/path.ts",
"import": "./src/path.ts"
},
"./source": {
"types": "./src/source.ts",
"import": "./src/source.ts"
}
},
"engines": {
"bun": ">=1.3.14",
"node": ">=22"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Node Support Is Misstated

This package declares support for Node 22+, but its exported runtime modules use Bun-only APIs such as Bun.spawn and Bun.file, and the exports point directly to TypeScript source. A Node consumer can therefore meet the stated engine requirement but fail when loading or invoking these helpers. Please remove the Node engine claim or provide and test a Node-compatible implementation.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/hunk-vcs/package.json
Line: 36

Comment:
**Node Support Is Misstated**

This package declares support for Node 22+, but its exported runtime modules use Bun-only APIs such as `Bun.spawn` and `Bun.file`, and the exports point directly to TypeScript source. A Node consumer can therefore meet the stated engine requirement but fail when loading or invoking these helpers. Please remove the Node engine claim or provide and test a Node-compatible implementation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test";
import { runAbortableCommand } from "./asyncProcess";
import { runAbortableCommand } from "./async-process";

describe("abortable bundled VCS subprocesses", () => {
test("does not spawn after cancellation already won", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import type { ExtensionVcsDiffInput } from "hunkdiff/extension";
/** The revision fields needed to describe a provider's diff target. */
interface DiffTargetInput {
readonly range?: string;
readonly rangeEndpoints?: {
readonly from: string;
readonly to: string;
};
}

/**
* Describe a diff's comparison in compact range form for titles and Git arguments.
Expand All @@ -7,18 +14,18 @@ import type { ExtensionVcsDiffInput } from "hunkdiff/extension";
* revset rather than Git's direct two-tree comparison, so they must build process
* arguments from `rangeEndpoints` instead.
*/
export function describeDiffRange(input: ExtensionVcsDiffInput) {
export function describeDiffRange(input: DiffTargetInput) {
const endpoints = input.rangeEndpoints;
return endpoints ? `${endpoints.from}..${endpoints.to}` : input.range;
}

/** Describe the targets using the positional spelling the user supplied. */
export function describeDiffTargets(input: ExtensionVcsDiffInput) {
export function describeDiffTargets(input: DiffTargetInput) {
const endpoints = input.rangeEndpoints;
return endpoints ? `${endpoints.from} ${endpoints.to}` : input.range;
}

/** Return whether a VCS diff has any explicit revision or range target. */
export function hasExplicitDiffTarget(input: ExtensionVcsDiffInput) {
export function hasExplicitDiffTarget(input: DiffTargetInput) {
return describeDiffRange(input) !== undefined;
}
96 changes: 96 additions & 0 deletions packages/hunk-vcs/src/large-file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import fs from "node:fs";
import { join } from "node:path";

/**
* Thresholds past which Hunk lists a file instead of rendering its diff.
*
* Shared by every backend so "too large to review" means one thing across the
* product, and by the host's own untracked-file synthesizer.
*/
export const LARGE_DIFF_FILE_MAX_BYTES = 1_000_000;
export const LARGE_DIFF_FILE_MAX_LINES = 20_000;

/** How much of a file line counting reads before giving up and reporting a truncated count. */
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 LargeFileCheck {
shouldSkip: boolean;
stats?: { additions: number; deletions: number };
/** True when `stats` came from a capped read and undercount the real file. */
statsTruncated?: boolean;
}

/**
* Return whether a whole file on disk is too large to synthesize into a patch.
*
* Used for files that would be rendered from their current contents — untracked
* files, and anything else a backend adds as a full-file addition — where the
* cost is the file itself rather than the size of a change to it.
*/
export function inspectLargeUntrackedFile(repoRoot: string, filePath: string): LargeFileCheck {
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,
};
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test";
import { normalizePathForOS } from "./osPath";
import { normalizePathForOS } from "./path";

describe("normalizePathForOS", () => {
test("normalizes Unix-style Windows paths for native subprocess cwd", () => {
Expand Down
25 changes: 25 additions & 0 deletions packages/hunk-vcs/src/path.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/** Normalize compatibility-layer paths into native paths for the current OS. */
export function normalizePathForOS(path: string, platform = process.platform) {
switch (platform) {
case "win32":
return normalizeWindowsCompatibilityPath(path);
default:
return path;
}
}

/** Convert Unix-style Windows paths to native paths usable as Bun cwd. */
function normalizeWindowsCompatibilityPath(path: string) {
const normalized = path
// Some Windows tools can report slash-prefixed drive paths as `/C:/...`.
.replace(/^\/([a-zA-Z]):(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
// Keep specific compatibility-layer prefixes before the generic `/c/...` form.
// Cygwin commonly reports drive paths as `/cygdrive/c/...`.
.replace(/^\/cygdrive\/([a-zA-Z])(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
// WSL-style paths are commonly reported as `/mnt/c/...`.
.replace(/^\/mnt\/([a-zA-Z])(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
// Git Bash/MSYS2 commonly reports drive paths as `/c/...`.
.replace(/^\/([a-zA-Z])(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`);

return normalized === path ? path : normalized.replaceAll("/", "\\");
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { readFileTextWithLimit, terminateSourceSubprocess } from "./sourceText";
import { readFileTextWithLimit, terminateSourceSubprocess } from "./source";

const tempDirs: string[] = [];

Expand Down
Loading
Loading