diff --git a/.changeset/calm-files-watch.md b/.changeset/calm-files-watch.md new file mode 100644 index 00000000..34dbc919 --- /dev/null +++ b/.changeset/calm-files-watch.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Reduce Git polling and CPU use in watch mode while preserving continuous refreshes with a polling fallback. diff --git a/.changeset/native-recursive-watch.md b/.changeset/native-recursive-watch.md new file mode 100644 index 00000000..ade76f67 --- /dev/null +++ b/.changeset/native-recursive-watch.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Reduce watch-mode startup cost on macOS and Windows by using bounded native recursive filesystem observation. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe9c8dfc..c37e209b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -207,6 +207,11 @@ jobs: - name: Build binary run: bun run build:bin + - name: Verify compiled binary watch mode + run: bun test ./test/pty/watch.test.ts + env: + HUNK_TEST_EXECUTABLE: ${{ github.workspace }}/dist/hunk + - name: Upload binary artifact uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index b813403c..0adb8fae 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -180,6 +180,11 @@ jobs: - name: Stage prebuilt npm packages run: bun run build:prebuilt:npm + - name: Verify compiled binary watch mode + run: bun test ./test/pty/watch.test.ts + env: + HUNK_TEST_EXECUTABLE: ${{ github.workspace }}/dist/hunk + - name: Verify staged prebuilt packs run: bun run check:prebuilt-pack diff --git a/README.md b/README.md index a6189e45..2e6b7841 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,8 @@ hunk diff before.ts after.ts --watch # auto-reload when either file chang git diff --no-color | hunk patch - # review a patch from stdin ``` +Watch mode remains continuous. Direct-file and Git-backed reviews normally use filesystem observation to refresh promptly, with periodic polling retained as a fallback for missed events or unavailable watchers. Jujutsu and Sapling reviews currently use polling rather than filesystem observation. + ### Working with agents 1. Open Hunk in another terminal with `hunk diff` or `hunk show`. diff --git a/benchmarks/highlight-prefetch.ts b/benchmarks/highlight-prefetch.ts index 4b036874..e5af1623 100644 --- a/benchmarks/highlight-prefetch.ts +++ b/benchmarks/highlight-prefetch.ts @@ -52,6 +52,7 @@ function createDiffFile(index: number, marker: string): DiffFile { function createBootstrap(): AppBootstrap { return { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, diff --git a/benchmarks/large-stream-fixture.ts b/benchmarks/large-stream-fixture.ts index 969de1e1..82d70a66 100644 --- a/benchmarks/large-stream-fixture.ts +++ b/benchmarks/large-stream-fixture.ts @@ -130,6 +130,7 @@ export function createLargeSplitStreamBootstrap({ contentVariant = "ascii", }: LargeSplitStreamFixtureOptions = {}): AppBootstrap { return { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -230,6 +231,7 @@ export function createHugeStreamBootstrap(): AppBootstrap { files.push(createGiantSingleDiffFile(HUGE_FILE_COUNT + 1)); return { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, diff --git a/bun.lock b/bun.lock index 65f85103..433d7008 100644 --- a/bun.lock +++ b/bun.lock @@ -7,6 +7,7 @@ "dependencies": { "@pierre/diffs": "1.2.2", "bun": "^1.3.14", + "chokidar": "^4.0.3", "commander": "^14.0.3", "diff": "^8.0.3", "shell-quote": "1.8.4", @@ -280,6 +281,8 @@ "character-entities-legacy": ["character-entities-legacy@3.0.0", "", {}, "sha512-RpPp0asT/6ufRm//AJVwpViZbGM/MkjQFxJccQRHmISF/22NBtsHqAWmL+/pmkPWoIUJdWyeVleTl1wydHATVQ=="], + "chokidar": ["chokidar@4.0.3", "", { "dependencies": { "readdirp": "^4.0.1" } }, "sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA=="], + "cli-cursor": ["cli-cursor@5.0.0", "", { "dependencies": { "restore-cursor": "^5.0.0" } }, "sha512-aCj4O5wKyszjMmDT4tZj93kxyydN/K5zPWSCe6/0AV/AA1pqe5ZBIw0a2ZfPQV7lL5/yb5HsUreJ6UFAF1tEQw=="], "cli-truncate": ["cli-truncate@5.2.0", "", { "dependencies": { "slice-ansi": "^8.0.0", "string-width": "^8.2.0" } }, "sha512-xRwvIOMGrfOAnM1JYtqQImuaNtDEv9v6oIYAs4LIHwTiKee8uwvIi363igssOC0O5U04i4AlENs79LQLu9tEMw=="], @@ -404,6 +407,8 @@ "react-reconciler": ["react-reconciler@0.33.0", "", { "dependencies": { "scheduler": "^0.27.0" }, "peerDependencies": { "react": "^19.2.0" } }, "sha512-KetWRytFv1epdpJc3J4G75I4WrplZE5jOL7Yq0p34+OVOKF4Se7WrdIdVC45XsSSmUTlht2FM/fM1FZb1mfQeA=="], + "readdirp": ["readdirp@4.1.2", "", {}, "sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg=="], + "regex": ["regex@6.1.0", "", { "dependencies": { "regex-utilities": "^2.3.0" } }, "sha512-6VwtthbV4o/7+OaAF9I5L5V3llLEsoPyq9P1JVXkedTP33c7MfCG0/5NOPcSJn0TzXcG9YUrR0gQSWioew3LDg=="], "regex-recursion": ["regex-recursion@6.0.2", "", { "dependencies": { "regex-utilities": "^2.3.0" } }, "sha512-0YCaSCq2VRIebiaUviZNs0cBz1kg5kVS2UKUfNIx8YVs1cN3AV7NTctO5FOKBA+UT2BPJIWZauYHPqJODG50cg=="], diff --git a/nix/bun.lock.nix b/nix/bun.lock.nix index c775bba2..35bc895a 100644 --- a/nix/bun.lock.nix +++ b/nix/bun.lock.nix @@ -429,6 +429,10 @@ url = "https://registry.npmjs.org/character-entities-legacy/-/character-entities-legacy-3.0.0.tgz"; hash = "sha512-RpPp0asT/6ufRm//AJVwpViZbGM/MkjQFxJccQRHmISF/22NBtsHqAWmL+/pmkPWoIUJdWyeVleTl1wydHATVQ=="; }; + "chokidar@4.0.3" = fetchurl { + url = "https://registry.npmjs.org/chokidar/-/chokidar-4.0.3.tgz"; + hash = "sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA=="; + }; "cli-cursor@5.0.0" = fetchurl { url = "https://registry.npmjs.org/cli-cursor/-/cli-cursor-5.0.0.tgz"; hash = "sha512-aCj4O5wKyszjMmDT4tZj93kxyydN/K5zPWSCe6/0AV/AA1pqe5ZBIw0a2ZfPQV7lL5/yb5HsUreJ6UFAF1tEQw=="; @@ -681,6 +685,10 @@ url = "https://registry.npmjs.org/react/-/react-19.2.4.tgz"; hash = "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ=="; }; + "readdirp@4.1.2" = fetchurl { + url = "https://registry.npmjs.org/readdirp/-/readdirp-4.1.2.tgz"; + hash = "sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg=="; + }; "regex-recursion@6.0.2" = fetchurl { url = "https://registry.npmjs.org/regex-recursion/-/regex-recursion-6.0.2.tgz"; hash = "sha512-0YCaSCq2VRIebiaUviZNs0cBz1kg5kVS2UKUfNIx8YVs1cN3AV7NTctO5FOKBA+UT2BPJIWZauYHPqJODG50cg=="; diff --git a/package.json b/package.json index 126d9a92..90188de6 100644 --- a/package.json +++ b/package.json @@ -95,6 +95,7 @@ "dependencies": { "@pierre/diffs": "1.2.2", "bun": "^1.3.14", + "chokidar": "^4.0.3", "commander": "^14.0.3", "diff": "^8.0.3", "shell-quote": "1.8.4", diff --git a/src/core/git.test.ts b/src/core/git.test.ts index 06f6a364..555abfd4 100644 --- a/src/core/git.test.ts +++ b/src/core/git.test.ts @@ -1,12 +1,16 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve, sep } from "node:path"; import { buildGitDiffArgs, + buildGitIgnoredDirectoryArgs, buildGitStashShowArgs, buildGitStatusArgs, + listGitIgnoredDirectoryRoots, + parseGitIgnoredDirectoryRoots, resolveGitDiffEndpoints, + resolveGitMetadata, runGitText, } from "./git"; import type { VcsDiffCommandInput } from "./types"; @@ -95,6 +99,29 @@ describe("git command helpers", () => { ]); }); + test("builds the collapsed ignored-directory query", () => { + expect(buildGitIgnoredDirectoryArgs()).toEqual([ + "ls-files", + "--full-name", + "--others", + "--ignored", + "--exclude-standard", + "--directory", + "-z", + ]); + }); + + test("parses only NUL-delimited collapsed directories into unique absolute roots", () => { + const repoRoot = resolve(tmpdir(), "hunk-ignored-parser"); + + expect( + parseGitIgnoredDirectoryRoots( + ["dependencies/", "ignored.log", "build/nested/", "dependencies/", ""].join("\0"), + repoRoot, + ), + ).toEqual([resolve(repoRoot, "dependencies"), resolve(repoRoot, "build/nested")]); + }); + test("reports a friendly error when git is not installed or not on PATH", () => { expect(() => runGitText({ @@ -112,6 +139,107 @@ describe("git command helpers", () => { }); }); +describe("listGitIgnoredDirectoryRoots", () => { + test("collapses a dependency-heavy ignored tree without pruning nonignored paths", () => { + const repoRoot = createTempRepo("hunk-git-ignored-dependencies-"); + writeFileSync(join(repoRoot, ".gitignore"), "node_modules/\nignored.log\n"); + for (let index = 0; index < 25; index += 1) { + const packageRoot = join(repoRoot, "node_modules", `package-${index}`, "cache"); + mkdirSync(packageRoot, { recursive: true }); + writeFileSync(join(packageRoot, "index.js"), `${index}\n`); + } + writeFileSync(join(repoRoot, "ignored.log"), "ignored file\n"); + mkdirSync(join(repoRoot, "src"), { recursive: true }); + writeFileSync(join(repoRoot, "src", "untracked.ts"), "export {};\n"); + + const roots = listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: join(repoRoot, "src") }); + + expect(roots).toEqual([join(realpathSync(repoRoot), "node_modules")]); + expect(roots).not.toContain(join(realpathSync(repoRoot), "src")); + }); + + test("honors nested ignore negation instead of pruning its visible ancestor", () => { + const repoRoot = createTempRepo("hunk-git-ignored-negation-"); + writeFileSync( + join(repoRoot, ".gitignore"), + ["generated/*", "!generated/.gitignore", "!generated/keep/", ""].join("\n"), + ); + mkdirSync(join(repoRoot, "generated", "discard"), { recursive: true }); + mkdirSync(join(repoRoot, "generated", "keep"), { recursive: true }); + writeFileSync( + join(repoRoot, "generated", ".gitignore"), + ["keep/*", "!keep/visible.txt", ""].join("\n"), + ); + writeFileSync(join(repoRoot, "generated", "discard", "output.js"), "ignored\n"); + writeFileSync(join(repoRoot, "generated", "keep", "hidden.txt"), "ignored file\n"); + writeFileSync(join(repoRoot, "generated", "keep", "visible.txt"), "visible\n"); + + expect(listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: repoRoot })).toEqual([ + join(realpathSync(repoRoot), "generated", "discard"), + ]); + }); + + test("honors repository-local excludes", () => { + const repoRoot = createTempRepo("hunk-git-ignored-info-exclude-"); + writeFileSync(join(repoRoot, ".git", "info", "exclude"), "local-cache/\n"); + mkdirSync(join(repoRoot, "local-cache", "nested"), { recursive: true }); + writeFileSync(join(repoRoot, "local-cache", "nested", "data"), "ignored\n"); + + expect(listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: repoRoot })).toEqual([ + join(realpathSync(repoRoot), "local-cache"), + ]); + }); + + test("does not prune an ignored parent containing a forced tracked file", () => { + const repoRoot = createTempRepo("hunk-git-ignored-tracked-"); + writeFileSync(join(repoRoot, ".gitignore"), "vendor/\n"); + mkdirSync(join(repoRoot, "vendor", "generated"), { recursive: true }); + writeFileSync(join(repoRoot, "vendor", "tracked.txt"), "tracked\n"); + writeFileSync(join(repoRoot, "vendor", "generated", "output.txt"), "ignored\n"); + git(repoRoot, "add", ".gitignore"); + git(repoRoot, "add", "-f", "vendor/tracked.txt"); + git(repoRoot, "commit", "-m", "track forced file"); + + const roots = listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: repoRoot }); + const trackedPath = join(realpathSync(repoRoot), "vendor", "tracked.txt"); + + expect(roots).toEqual([join(realpathSync(repoRoot), "vendor", "generated")]); + expect(roots.some((root) => trackedPath.startsWith(`${root}${sep}`))).toBe(false); + }); + + test("falls back to no pruning when best-effort discovery fails", () => { + expect( + listGitIgnoredDirectoryRoots(makeGitInput(), { + cwd: tmpdir(), + repoRoot: tmpdir(), + gitExecutable: "definitely-not-a-real-git-binary", + }), + ).toEqual([]); + }); +}); + +describe("resolveGitMetadata", () => { + test("resolves normal and linked-worktree metadata directories", () => { + const repoRoot = createTempRepo("hunk-git-metadata-"); + writeFileSync(join(repoRoot, "x.txt"), "x\n"); + git(repoRoot, "add", "x.txt"); + git(repoRoot, "commit", "-m", "initial"); + + const normal = resolveGitMetadata(makeGitInput(), { cwd: repoRoot }); + expect(normal.repoRoot).toBe(realpathSync(repoRoot)); + expect(normal.gitDir).toBe(normal.commonDir); + + const linkedRoot = mkdtempSync(join(tmpdir(), "hunk-git-linked-")); + tempDirs.push(linkedRoot); + git(repoRoot, "worktree", "add", linkedRoot, "-b", "linked-test"); + const linked = resolveGitMetadata(makeGitInput(), { cwd: linkedRoot }); + expect(linked.repoRoot).toBe(realpathSync(linkedRoot)); + expect(linked.gitDir).not.toBe(linked.commonDir); + expect(linked.gitDir).toContain(join("worktrees", "hunk-git-linked-")); + expect(linked.commonDir).toBe(normal.commonDir); + }); +}); + describe("resolveGitDiffEndpoints", () => { test("staged diffs compare HEAD against the index", () => { const repoRoot = createTempRepo("hunk-endpoints-staged-"); diff --git a/src/core/git.ts b/src/core/git.ts index 3c52b362..09e991b5 100644 --- a/src/core/git.ts +++ b/src/core/git.ts @@ -1,5 +1,5 @@ import fs from "node:fs"; -import { join } from "node:path"; +import { isAbsolute, join, resolve } from "node:path"; import { HunkUserError } from "./errors"; import { escapeUntrackedPatchPath } from "./patch/normalize"; import type { VcsDiffCommandInput, VcsShowCommandInput, VcsStashShowCommandInput } from "./types"; @@ -12,6 +12,7 @@ export interface RunGitTextOptions { args: string[]; cwd?: string; gitExecutable?: string; + preventOptionalLocks?: boolean; } interface RunGitCommandResult { @@ -150,6 +151,19 @@ export function buildGitStatusArgs(input: VcsDiffCommandInput) { return args; } +/** Build the read-only query that asks Git for collapsed ignored directory entries. */ +export function buildGitIgnoredDirectoryArgs() { + return [ + "ls-files", + "--full-name", + "--others", + "--ignored", + "--exclude-standard", + "--directory", + "-z", + ]; +} + /** Build the synthetic patch used to render one untracked file as a new-file diff. */ function buildGitNewFileDiffArgs(filePath: string) { // `--no-ext-diff` keeps user-configured `diff.external` tools (difftastic, delta, etc.) @@ -363,6 +377,7 @@ function runGitCommand({ args, cwd = process.cwd(), gitExecutable = "git", + preventOptionalLocks = false, acceptedExitCodes = [0], }: RunGitCommandOptions): RunGitCommandResult { let proc: ReturnType; @@ -373,6 +388,7 @@ function runGitCommand({ stdin: "ignore", stdout: "pipe", stderr: "pipe", + env: preventOptionalLocks ? { ...process.env, GIT_OPTIONAL_LOCKS: "0" } : undefined, }); } catch (error) { throw translateGitSpawnFailure(input, error, gitExecutable); @@ -481,7 +497,10 @@ function isWorkingTreeGitDiffInput( cwd = process.cwd(), gitExecutable = "git", repoRoot, - }: Pick & { repoRoot?: string } = {}, + preventOptionalLocks = false, + }: Pick & { + repoRoot?: string; + } = {}, ) { if (input.staged) { return false; @@ -502,6 +521,7 @@ function isWorkingTreeGitDiffInput( args: ["rev-parse", "--revs-only", input.range], cwd, gitExecutable, + preventOptionalLocks, }) .split("\n") .map((line) => line.trim()) @@ -518,7 +538,9 @@ function isWorkingTreeGitDiffInput( /** Return whether working-tree review should synthesize untracked files into the patch stream. */ function shouldIncludeUntrackedFiles( input: VcsDiffCommandInput, - options: Pick & { repoRoot?: string } = {}, + options: Pick & { + repoRoot?: string; + } = {}, ) { return input.options.excludeUntracked !== true && isWorkingTreeGitDiffInput(input, options); } @@ -531,6 +553,43 @@ function parseUntrackedFilePaths(statusText: string) { .flatMap((entry) => (entry.startsWith("?? ") ? [entry.slice(3)] : [])); } +/** Parse Git's NUL output into absolute roots only for collapsed directory entries. */ +export function parseGitIgnoredDirectoryRoots(output: string, repoRoot: string) { + const roots = output + .split("\0") + .filter((entry) => entry.endsWith("/")) + .map((entry) => normalizePathForOS(resolve(repoRoot, entry.slice(0, -1)))); + return [...new Set(roots)]; +} + +/** Discover Git-ignored directory roots, falling back to no pruning when optimization fails. */ +export function listGitIgnoredDirectoryRoots( + input: GitBackedInput, + { + cwd = process.cwd(), + repoRoot, + gitExecutable = "git", + }: Omit & { + repoRoot?: string; + } = {}, +) { + try { + const normalizedRepoRoot = + repoRoot ?? resolveGitRepoRoot(input, { cwd, gitExecutable, preventOptionalLocks: true }); + const output = runGitText({ + input, + args: buildGitIgnoredDirectoryArgs(), + cwd: normalizedRepoRoot, + gitExecutable, + preventOptionalLocks: true, + }); + return parseGitIgnoredDirectoryRoots(output, normalizedRepoRoot); + } catch { + // Ignore discovery only reduces observer work; it must never make watch mode unavailable. + return []; + } +} + /** Return whether one untracked path can be synthesized into a file diff. */ function isReviewableUntrackedPath(repoRoot: string, filePath: string) { const absolutePath = join(repoRoot, filePath); @@ -569,9 +628,10 @@ export function listGitUntrackedFiles( cwd = process.cwd(), repoRoot, gitExecutable = "git", + preventOptionalLocks = false, }: Omit & { repoRoot?: string } = {}, ) { - if (!shouldIncludeUntrackedFiles(input, { cwd, gitExecutable })) { + if (!shouldIncludeUntrackedFiles(input, { cwd, gitExecutable, preventOptionalLocks })) { return []; } @@ -580,6 +640,7 @@ export function listGitUntrackedFiles( args: buildGitStatusArgs(input), cwd, gitExecutable, + preventOptionalLocks, }); const untrackedFiles = parseUntrackedFilePaths(statusText); @@ -587,7 +648,8 @@ export function listGitUntrackedFiles( return []; } - const normalizedRepoRoot = repoRoot ?? resolveGitRepoRoot(input, { cwd, gitExecutable }); + const normalizedRepoRoot = + repoRoot ?? resolveGitRepoRoot(input, { cwd, gitExecutable, preventOptionalLocks }); return untrackedFiles.filter((filePath) => isReviewableUntrackedPath(normalizedRepoRoot, filePath), ); @@ -639,6 +701,39 @@ export function runGitUntrackedFileDiffText( }).stdout; } +export interface GitMetadata { + repoRoot: string; + gitDir: string; + commonDir: string; +} + +/** Resolve repository, per-worktree, and shared Git metadata directories. */ +export function resolveGitMetadata( + input: GitBackedInput, + options: Omit = {}, +): GitMetadata { + const cwd = options.cwd ?? process.cwd(); + const repoRoot = resolveGitRepoRoot(input, options); + const gitDir = normalizePathForOS( + runGitText({ input, args: ["rev-parse", "--absolute-git-dir"], ...options }).trim(), + ); + const absoluteCommon = runGitCommand({ + input, + args: ["rev-parse", "--path-format=absolute", "--git-common-dir"], + ...options, + acceptedExitCodes: [0, 128, 129], + }); + const commonOutput = + absoluteCommon.exitCode === 0 + ? absoluteCommon.stdout.trim() + : runGitText({ input, args: ["rev-parse", "--git-common-dir"], ...options }).trim(); + const commonDir = normalizePathForOS( + isAbsolute(commonOutput) ? commonOutput : resolve(cwd, commonOutput), + ); + + return { repoRoot, gitDir, commonDir }; +} + export function resolveGitRepoRoot( input: GitBackedInput, options: Omit = {}, diff --git a/src/core/loaders.test.ts b/src/core/loaders.test.ts index da114d4d..7fddf9de 100644 --- a/src/core/loaders.test.ts +++ b/src/core/loaders.test.ts @@ -5,6 +5,7 @@ import { join } from "node:path"; import { SourceTextTooLargeError } from "./fileSource"; import { loadAppBootstrap } from "./loaders"; import type { CliInput } from "./types"; +import { computeWatchSignature } from "./watch"; const tempDirs: string[] = []; @@ -173,6 +174,54 @@ afterEach(() => { }); describe("loadAppBootstrap", () => { + test("captures a watched signature before content loading", async () => { + const dir = createTempDir("hunk-watch-bootstrap-"); + const left = join(dir, "before.ts"); + const right = join(dir, "after.ts"); + writeFileSync(left, "export const value = 1;\n"); + writeFileSync(right, "export const value = 2;\n"); + + const originalBunFile = Bun.file; + Bun.file = ((path: string | URL | number, options?: BlobPropertyBag) => { + const file = + typeof path === "number" ? originalBunFile(path, options) : originalBunFile(path, options); + if (String(path) === right) { + file.text = async () => { + writeFileSync(right, "export const value = 3;\n"); + return "export const value = 3;\n"; + }; + } + return file; + }) as typeof Bun.file; + + try { + const bootstrap = await loadAppBootstrap( + { kind: "diff", left: "before.ts", right: "after.ts", options: { watch: true } }, + { cwd: dir }, + ); + + expect(bootstrap.reloadContext.cwd).toBe(dir); + expect(bootstrap.reloadContext.initialWatchSignature).toBeDefined(); + expect(computeWatchSignature(bootstrap.input, bootstrap.reloadContext)).not.toBe( + bootstrap.reloadContext.initialWatchSignature, + ); + } finally { + Bun.file = originalBunFile; + } + }); + + test("does not fail a valid initial load when best-effort watch signing fails", async () => { + const bootstrap = await loadAppBootstrap({ + kind: "patch", + file: "-", + text: "diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-one\n+two\n", + options: { watch: true }, + }); + + expect(bootstrap.changeset.files).toHaveLength(1); + expect(bootstrap.reloadContext.initialWatchSignature).toBeUndefined(); + }); + test("loads file-pair diffs and agent context", async () => { const dir = mkdtempSync(join(tmpdir(), "hunk-diff-")); tempDirs.push(dir); @@ -242,6 +291,7 @@ describe("loadAppBootstrap", () => { options: { mode: "auto", agentContext: "agent.json", + watch: true, }, }, { cwd: nested }, @@ -251,8 +301,17 @@ describe("loadAppBootstrap", () => { expect(normalizeComparablePath(bootstrap.changeset.sourceLabel)).toBe( normalizeComparablePath(dir), ); + expect(normalizeComparablePath(bootstrap.reloadContext.cwd)).toBe( + normalizeComparablePath(nested), + ); + expect(normalizeComparablePath(bootstrap.reloadContext.repoRoot!)).toBe( + normalizeComparablePath(dir), + ); expect(bootstrap.changeset.files[0]?.path).toBe("example.ts"); expect(bootstrap.changeset.files[0]?.agent?.annotations).toHaveLength(1); + expect(computeWatchSignature(bootstrap.input, bootstrap.reloadContext)).toBe( + bootstrap.reloadContext.initialWatchSignature!, + ); }); test("skips binary file-pair diffs instead of reading their contents", async () => { diff --git a/src/core/loaders.ts b/src/core/loaders.ts index 9c7e0a0e..ff6eb9ab 100644 --- a/src/core/loaders.ts +++ b/src/core/loaders.ts @@ -13,6 +13,7 @@ import { createFileSourceFetcher, type FileSourceSpec } from "./fileSource"; import { splitPatchIntoFileChunks, findPatchChunk } from "./patch/chunks"; import { normalizePatchText, stripTerminalControl } from "./patch/normalize"; import { getConfiguredVcsAdapter, loadVcsReview, operationFromInput } from "./vcs"; +import { computeWatchSignature } from "./watch"; import type { AppBootstrap, AgentContext, @@ -390,9 +391,12 @@ async function loadVcsChangeset( agent: findAgentFileContext(agentContext, file.path, file.previousPath), })); return { - ...parsedChangeset, - files: [...parsedChangeset.files, ...adapterFiles], - } satisfies Changeset; + changeset: { + ...parsedChangeset, + files: [...parsedChangeset.files, ...adapterFiles], + } satisfies Changeset, + repoRoot: result.repoRoot, + }; } /** Build a changeset from patch text supplied by file or stdin. */ @@ -421,17 +425,30 @@ export async function loadAppBootstrap( input: CliInput, { cwd = process.cwd(), customTheme, gitExecutable = "git" }: LoadAppBootstrapOptions = {}, ): Promise { - const agentContext = await loadAgentContext(input.options.agentContext, { - cwd, - }); + // Capture before loading content so watch mode can detect mutations that race initial loading. + let initialWatchSignature: string | undefined; + if (input.options.watch) { + try { + initialWatchSignature = computeWatchSignature(input, { cwd, gitExecutable }); + } catch { + // A transient signature failure must not prevent an otherwise valid initial review. + } + } + + const agentContext = await loadAgentContext(input.options.agentContext, { cwd }); let changeset: Changeset; + let repoRoot: string | undefined; switch (input.kind) { case "vcs": case "show": case "stash-show": - changeset = await loadVcsChangeset(input, agentContext, cwd, gitExecutable); + { + const result = await loadVcsChangeset(input, agentContext, cwd, gitExecutable); + changeset = result.changeset; + repoRoot = result.repoRoot; + } break; case "diff": changeset = await loadFileDiffChangeset(input, agentContext, cwd); @@ -451,6 +468,7 @@ export async function loadAppBootstrap( return { input, + reloadContext: { cwd, repoRoot, initialWatchSignature }, changeset, initialMode: input.options.mode ?? "auto", initialTheme: input.options.theme, diff --git a/src/core/startup.test.ts b/src/core/startup.test.ts index 1ab4d06e..20fccafc 100644 --- a/src/core/startup.test.ts +++ b/src/core/startup.test.ts @@ -6,6 +6,7 @@ import type { AppBootstrap, CliInput, ParsedCliInput } from "./types"; function createBootstrap(input: CliInput): AppBootstrap { return { input, + reloadContext: { cwd: process.cwd() }, changeset: { id: "changeset:startup", sourceLabel: "repo", diff --git a/src/core/types.ts b/src/core/types.ts index 4c6c5c95..6ea6fa05 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -356,8 +356,15 @@ export type ParsedCliInput = | DaemonServeCommandInput | SessionCommandInput; +export interface ReloadContext { + cwd: string; + repoRoot?: string; + initialWatchSignature?: string; +} + export interface AppBootstrap { input: CliInput; + reloadContext: ReloadContext; changeset: Changeset; initialMode: LayoutMode; initialTheme?: string; diff --git a/src/core/vcs/git.test.ts b/src/core/vcs/git.test.ts index 3a218efa..ea8aeb7b 100644 --- a/src/core/vcs/git.test.ts +++ b/src/core/vcs/git.test.ts @@ -1,7 +1,7 @@ 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 { isAbsolute, join, resolve } from "node:path"; import { GitVcsAdapter, gitEndpointSourceSpec, @@ -150,6 +150,103 @@ describe("GitVcsAdapter", () => { expect(GitVcsAdapter.detect(createTempDir("hunk-git-adapter-none-"))).toBeNull(); }); + test("builds operation-sensitive watch plans for working tree and metadata reviews", () => { + const repo = createTempRepo("hunk-git-adapter-plan-"); + writeFileSync(join(repo, "file.txt"), "one\n"); + writeFileSync(join(repo, ".gitignore"), "generated/\n"); + git(repo, "add", "file.txt", ".gitignore"); + git(repo, "commit", "-m", "initial"); + mkdirSync(join(repo, "generated", "nested"), { recursive: true }); + writeFileSync(join(repo, "generated", "nested", "output.js"), "ignored\n"); + + const operation = GitVcsAdapter.operations["working-tree-diff"]!; + const unstaged = operation.watchPlan!( + { kind: "vcs", staged: false, options: { vcs: "git" } }, + { cwd: repo }, + ); + expect(unstaged.targets.some((target) => target.directory === repo)).toBe(true); + const worktreeTarget = unstaged.targets.find( + (target) => target.kind === "directory-tree" && target.directory === repo, + ); + expect(worktreeTarget?.kind === "directory-tree" ? worktreeTarget.ignoredRoots : []).toEqual([ + join(repo, ".git"), + join(repo, "generated"), + ]); + const metadataTargets = unstaged.targets.filter((target) => + target.sources.includes("vcs-metadata"), + ); + expect(metadataTargets).toEqual([ + { + kind: "directory-tree", + directory: join(repo, ".git"), + ignoredRoots: [join(repo, ".git", "objects")], + sources: ["vcs-metadata"], + }, + ]); + const refPlan = operation.watchPlan!( + { + kind: "vcs", + staged: false, + range: "HEAD", + pathspecs: ["file.txt"], + options: { vcs: "git" }, + }, + { cwd: repo }, + ); + expect(refPlan.targets.some((target) => target.directory === repo)).toBe(true); + + for (const input of [ + { kind: "vcs", staged: true, options: { vcs: "git" } }, + { kind: "vcs", staged: false, range: "HEAD^..HEAD", options: { vcs: "git" } }, + ] satisfies VcsDiffCommandInput[]) { + const plan = operation.watchPlan!(input, { cwd: repo }); + expect(plan.targets.some((target) => target.directory === repo)).toBe(false); + expect(plan.targets.some((target) => target.sources.includes("vcs-metadata"))).toBe(true); + } + }); + + test("keeps stash reflogs observable for ordinal selectors", () => { + const repo = createTempRepo("hunk-git-adapter-stash-plan-"); + const plan = GitVcsAdapter.operations["stash-show"]!.watchPlan!( + { kind: "stash-show", ref: "stash@{1}", options: { vcs: "git" } }, + { cwd: repo }, + ); + const metadataTarget = plan.targets.find((target) => target.sources.includes("vcs-metadata")); + expect(metadataTarget?.directory).toBe(join(repo, ".git")); + expect(metadataTarget?.kind === "directory-tree" ? metadataTarget.ignoredRoots : []).toEqual([ + join(repo, ".git", "objects"), + ]); + }); + + test("deduplicates common metadata while covering linked-worktree state", () => { + const repo = createTempRepo("hunk-git-adapter-linked-plan-"); + writeFileSync(join(repo, "file.txt"), "one\n"); + git(repo, "add", "file.txt"); + git(repo, "commit", "-m", "initial"); + const linked = createTempDir("hunk-git-adapter-worktree-"); + git(repo, "worktree", "add", linked, "-b", "linked-plan"); + + const plan = GitVcsAdapter.operations["revision-show"]!.watchPlan!( + { kind: "show", ref: "HEAD", options: { vcs: "git" } }, + { cwd: linked }, + ); + const metadataTargets = plan.targets.filter( + (target) => target.kind === "directory-tree" && target.sources.includes("vcs-metadata"), + ); + expect(metadataTargets).toHaveLength(1); + const commonDirOutput = git(linked, "rev-parse", "--git-common-dir").trim(); + const commonDir = isAbsolute(commonDirOutput) + ? commonDirOutput + : resolve(linked, commonDirOutput); + expect(normalizeComparablePath(metadataTargets[0]!.directory)).toBe( + normalizeComparablePath(commonDir), + ); + const metadataTarget = metadataTargets[0]!; + expect(metadataTarget.kind === "directory-tree" ? metadataTarget.ignoredRoots : []).toEqual([ + join(metadataTarget.directory, "objects"), + ]); + }); + test("computes watch signatures for each review operation", () => { const repo = createTempRepo("hunk-git-adapter-watch-"); writeFileSync(join(repo, "file.txt"), "one\n"); @@ -158,35 +255,28 @@ describe("GitVcsAdapter", () => { writeFileSync(join(repo, "file.txt"), "two\n"); writeFileSync(join(repo, "untracked.txt"), "fresh\n"); - // watchSignature reads process.cwd(), so run it from inside the temp repo. - const previousCwd = process.cwd(); - process.chdir(repo); - 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 = 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 = 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 = GitVcsAdapter.operations["stash-show"]!.watchSignature!( - { kind: "stash-show", options: { vcs: "git" } }, - { cwd: repo }, - ); - expect(stashSignature).toContain("diff --git"); - } finally { - process.chdir(previousCwd); - } + // 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 = 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 = 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 = 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 85e17f11..aca9dc0e 100644 --- a/src/core/vcs/git.ts +++ b/src/core/vcs/git.ts @@ -1,15 +1,17 @@ import fs from "node:fs"; -import { dirname, join, resolve } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve } from "node:path"; import { buildGitDiffArgs, buildGitDiffNumstatArgs, buildGitShowArgs, buildGitStashShowArgs, + listGitIgnoredDirectoryRoots, listGitUntrackedFiles, normalizeUntrackedPatchHeaders, resolveGitColorMovedOptions, resolveGitCommitRef, resolveGitDiffEndpoints, + resolveGitMetadata, resolveGitRepoRoot, runGitText, runGitUntrackedFileDiffText, @@ -28,6 +30,7 @@ import { type GitFileSourceSpec, } from "./gitSource"; import type { DiffFile, VcsDiffCommandInput } from "../types"; +import type { DirectoryTreeWatchTarget, WatchPlan } from "../watchPlan"; import type { VcsAdapter, VcsReviewOperation } from "./types"; import { buildSkippedLargeUntrackedDiffFile, @@ -248,6 +251,59 @@ function buildGitReviewSourceFetcherBuilder( } } +/** Return whether a recursive directory target safely covers another directory. */ +function directoryContains(parent: string, child: string) { + const path = relative(parent, child); + return path === "" || (!path.startsWith("..") && !isAbsolute(path)); +} + +/** Build metadata targets, collapsing linked-worktree state under its common Git directory. */ +function buildGitMetadataTargets(gitDir: string, commonDir: string): DirectoryTreeWatchTarget[] { + const directories = directoryContains(commonDir, gitDir) ? [commonDir] : [gitDir, commonDir]; + return directories.map((directory) => ({ + kind: "directory-tree", + directory, + ignoredRoots: [join(directory, "objects")], + sources: ["vcs-metadata"], + })); +} + +/** Build a Git watch plan whose worktree recursion matches the reviewed operation. */ +function buildGitWatchPlan( + operation: VcsReviewOperation, + cwd: string, + gitExecutable = "git", +): WatchPlan { + const metadata = resolveGitMetadata(operation.input, { cwd, gitExecutable }); + const targets: DirectoryTreeWatchTarget[] = []; + if ( + operation.kind === "working-tree-diff" && + resolveGitDiffEndpoints(operation.input, { + cwd, + repoRoot: metadata.repoRoot, + gitExecutable, + })?.new.kind === "worktree" + ) { + targets.push({ + kind: "directory-tree", + directory: metadata.repoRoot, + ignoredRoots: [ + ...new Set([ + join(metadata.repoRoot, ".git"), + ...listGitIgnoredDirectoryRoots(operation.input, { + cwd: metadata.repoRoot, + repoRoot: metadata.repoRoot, + gitExecutable, + }), + ]), + ], + sources: ["worktree"], + }); + } + targets.push(...buildGitMetadataTargets(metadata.gitDir, metadata.commonDir)); + return { coverage: "hybrid", targets }; +} + /** VCS adapter translating neutral review operations to Git commands. */ export const GitVcsAdapter: VcsAdapter = { id: "git", @@ -306,12 +362,28 @@ export const GitVcsAdapter: VcsAdapter = { ], }; }, - 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))}`, - ); + watchPlan(input, { cwd, gitExecutable = "git" }) { + return buildGitWatchPlan({ kind: "working-tree-diff", input }, cwd, gitExecutable); + }, + watchSignature(input, { cwd, gitExecutable = "git" }) { + const trackedPatch = runGitText({ + input, + args: buildGitDiffArgs(input), + cwd, + gitExecutable, + preventOptionalLocks: true, + }); + const repoRoot = resolveGitRepoRoot(input, { + cwd, + gitExecutable, + preventOptionalLocks: true, + }); + const untrackedSignatures = listGitUntrackedFiles(input, { + cwd, + repoRoot, + gitExecutable, + preventOptionalLocks: true, + }).map((filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`); return [trackedPatch, ...untrackedSignatures].join("\n---\n"); }, }, @@ -341,8 +413,17 @@ export const GitVcsAdapter: VcsAdapter = { ), }; }, - watchSignature(input) { - return runGitText({ input, args: buildGitShowArgs(input) }); + watchPlan(input, { cwd, gitExecutable = "git" }) { + return buildGitWatchPlan({ kind: "revision-show", input }, cwd, gitExecutable); + }, + watchSignature(input, { cwd, gitExecutable = "git" }) { + return runGitText({ + input, + args: buildGitShowArgs(input), + cwd, + gitExecutable, + preventOptionalLocks: true, + }); }, }, "stash-show": { @@ -371,8 +452,17 @@ export const GitVcsAdapter: VcsAdapter = { ), }; }, - watchSignature(input) { - return runGitText({ input, args: buildGitStashShowArgs(input) }); + watchPlan(input, { cwd, gitExecutable = "git" }) { + return buildGitWatchPlan({ kind: "stash-show", input }, cwd, gitExecutable); + }, + watchSignature(input, { cwd, gitExecutable = "git" }) { + return runGitText({ + input, + args: buildGitStashShowArgs(input), + cwd, + gitExecutable, + preventOptionalLocks: true, + }); }, }, }, diff --git a/src/core/vcs/index.test.ts b/src/core/vcs/index.test.ts index f59fba25..0e37801c 100644 --- a/src/core/vcs/index.test.ts +++ b/src/core/vcs/index.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { createUnsupportedVcsOperationError, + createVcsWatchPlan, detectVcs, findVcsRepoRootCandidate, getVcsAdapter, @@ -138,6 +139,50 @@ describe("VCS adapter registry", () => { ).rejects.toThrow("`hunk stash show` requires Git VCS mode."); }); + test("dispatches watch plans and leaves adapters without one poll-only", () => { + const input = { + kind: "vcs", + staged: false, + options: { vcs: "custom" }, + } satisfies VcsDiffCommandInput; + const target = { + kind: "directory-tree" as const, + directory: "/repo", + ignoredRoots: [], + sources: ["worktree" as const], + }; + const adapter = { + id: "custom", + name: "Custom VCS", + detect: () => null, + operations: { + "working-tree-diff": { + load: async () => ({ + repoRoot: "/repo", + sourceLabel: "/repo", + title: "x", + patchText: "", + }), + watchPlan: () => ({ coverage: "hybrid" as const, targets: [target] }), + }, + }, + } satisfies VcsAdapter; + + expect(createVcsWatchPlan(adapter, operationFromInput(input), { cwd: "/repo" })).toEqual({ + coverage: "hybrid", + targets: [target], + }); + expect( + createVcsWatchPlan( + getVcsAdapter("jj"), + operationFromInput({ ...input, options: { vcs: "jj" } }), + { + cwd: "/repo", + }, + ), + ).toEqual({ coverage: "poll-only", targets: [] }); + }); + test("names the adapter and operation for non-stash unsupported operations", () => { const adapter = { id: "custom", diff --git a/src/core/vcs/index.ts b/src/core/vcs/index.ts index adbd731e..2f35952a 100644 --- a/src/core/vcs/index.ts +++ b/src/core/vcs/index.ts @@ -113,6 +113,20 @@ export async function loadVcsReview( return await handler.load(operation.input, context); } +/** Build an adapter-backed event plan, falling back to signature polling when unsupported. */ +export function createVcsWatchPlan( + adapter: VcsAdapter, + operation: VcsReviewOperation, + context: VcsLoadContext, +) { + const handler = getVcsOperation(adapter, operation); + if (!handler) { + throw createUnsupportedVcsOperationError(adapter, operation.kind); + } + + return handler.watchPlan?.(operation.input, context) ?? { coverage: "poll-only", targets: [] }; +} + /** Build an adapter-backed watch signature when the selected operation supports it. */ export function createVcsWatchSignature( adapter: VcsAdapter, diff --git a/src/core/vcs/jj.test.ts b/src/core/vcs/jj.test.ts index 9cf70dad..52d73f22 100644 --- a/src/core/vcs/jj.test.ts +++ b/src/core/vcs/jj.test.ts @@ -113,6 +113,12 @@ describe("JjVcsAdapter", () => { expect(showResult.title).toContain("show @"); expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); + expect( + JjVcsAdapter.operations["working-tree-diff"]!.watchSignature!(diffInput, { cwd: repo }), + ).toContain("+two"); + expect( + JjVcsAdapter.operations["revision-show"]!.watchSignature!(showInput, { cwd: repo }), + ).toContain("diff --git"); }, JjAdapterIntegrationTestTimeoutMs, ); diff --git a/src/core/vcs/jj.ts b/src/core/vcs/jj.ts index 87c9ebcd..04443830 100644 --- a/src/core/vcs/jj.ts +++ b/src/core/vcs/jj.ts @@ -49,8 +49,8 @@ export const JjVcsAdapter: VcsAdapter = { patchText: runJjText({ input, args: buildJjDiffArgs(input), cwd }), }; }, - watchSignature(input) { - return runJjText({ input, args: buildJjDiffArgs(input) }); + watchSignature(input, { cwd }) { + return runJjText({ input, args: buildJjDiffArgs(input), cwd }); }, }, "revision-show": { @@ -65,8 +65,8 @@ export const JjVcsAdapter: VcsAdapter = { patchText: runJjText({ input, args: buildJjShowArgs(input), cwd }), }; }, - watchSignature(input) { - return runJjText({ input, args: buildJjShowArgs(input) }); + watchSignature(input, { cwd }) { + return runJjText({ input, args: buildJjShowArgs(input), cwd }); }, }, }, diff --git a/src/core/vcs/sl.test.ts b/src/core/vcs/sl.test.ts index f1b02441..4f3bcbb1 100644 --- a/src/core/vcs/sl.test.ts +++ b/src/core/vcs/sl.test.ts @@ -122,6 +122,14 @@ describe("SaplingVcsAdapter", () => { expect(showResult.title).toContain("show ."); expect(showResult.patchText).toContain("diff --git a/file.txt b/file.txt"); + expect( + SaplingVcsAdapter.operations["working-tree-diff"]!.watchSignature!(diffInput, { + cwd: repo, + }), + ).toContain("+two"); + expect( + SaplingVcsAdapter.operations["revision-show"]!.watchSignature!(showInput, { cwd: repo }), + ).toContain("diff --git"); }, SlAdapterIntegrationTestTimeoutMs, ); diff --git a/src/core/vcs/types.ts b/src/core/vcs/types.ts index ea9588b3..4aca0c68 100644 --- a/src/core/vcs/types.ts +++ b/src/core/vcs/types.ts @@ -1,4 +1,5 @@ import type { BuildDiffFileOptions } from "../diffFile"; +import type { WatchPlan } from "../watchPlan"; import type { DiffFile, VcsShowCommandInput, @@ -30,6 +31,7 @@ export type VcsReviewOperationKind = VcsReviewOperation["kind"]; export interface VcsOperation { load(input: Input, context: VcsLoadContext): Promise; watchSignature?: (input: Input, context: VcsLoadContext) => string; + watchPlan?: (input: Input, context: VcsLoadContext) => WatchPlan; } export interface VcsOperations { diff --git a/src/core/watch.test.ts b/src/core/watch.test.ts index 8f794683..8c4b38f2 100644 --- a/src/core/watch.test.ts +++ b/src/core/watch.test.ts @@ -44,17 +44,6 @@ function createTempRepo(prefix: string) { return dir; } -function withCwd(cwd: string, callback: () => T) { - const previousCwd = process.cwd(); - process.chdir(cwd); - - try { - return callback(); - } finally { - process.chdir(previousCwd); - } -} - function createGitInput({ options, ...overrides @@ -77,6 +66,33 @@ afterEach(() => { }); describe("computeWatchSignature", () => { + test("resolves direct files, patch files, and agent sidecars against the supplied cwd", () => { + const dir = createTempRepo("hunk-watch-files-cwd-"); + writeFileSync(join(dir, "left.ts"), "one\n"); + writeFileSync(join(dir, "right.ts"), "two\n"); + writeFileSync(join(dir, "review.patch"), "patch\n"); + writeFileSync(join(dir, "agent.json"), "{}\n"); + + const direct = computeWatchSignature( + { + kind: "diff", + left: "left.ts", + right: "right.ts", + options: { agentContext: "agent.json" }, + }, + { cwd: dir }, + ); + const patch = computeWatchSignature( + { kind: "patch", file: "review.patch", options: {} }, + { cwd: dir }, + ); + + expect(direct).toContain(join(dir, "left.ts")); + expect(direct).toContain(join(dir, "right.ts")); + expect(direct).toContain(join(dir, "agent.json")); + expect(patch).toContain(join(dir, "review.patch")); + }); + test("does not embed full untracked file contents in git watch signatures", () => { const dir = createTempRepo("hunk-watch-untracked-"); @@ -88,9 +104,9 @@ describe("computeWatchSignature", () => { const untrackedPath = join(dir, "large-untracked.txt"); writeFileSync(untrackedPath, largeMarker); - const initialSignature = withCwd(dir, () => computeWatchSignature(createGitInput())); + const initialSignature = computeWatchSignature(createGitInput(), { cwd: dir }); writeFileSync(untrackedPath, `${largeMarker}changed`); - const changedSignature = withCwd(dir, () => computeWatchSignature(createGitInput())); + const changedSignature = computeWatchSignature(createGitInput(), { cwd: dir }); expect(initialSignature).not.toContain(largeMarker); expect(changedSignature).not.toContain(largeMarker); @@ -107,12 +123,14 @@ describe("computeWatchSignature", () => { const untrackedPath = join(dir, "note.txt"); writeFileSync(untrackedPath, "first\n"); - const initialSignature = withCwd(dir, () => - computeWatchSignature(createGitInput({ options: { excludeUntracked: true } })), + const initialSignature = computeWatchSignature( + createGitInput({ options: { excludeUntracked: true } }), + { cwd: dir }, ); writeFileSync(untrackedPath, "second\n"); - const changedSignature = withCwd(dir, () => - computeWatchSignature(createGitInput({ options: { excludeUntracked: true } })), + const changedSignature = computeWatchSignature( + createGitInput({ options: { excludeUntracked: true } }), + { cwd: dir }, ); expect(changedSignature).toEqual(initialSignature); @@ -120,10 +138,13 @@ describe("computeWatchSignature", () => { test("rejects unsupported watch operations before invoking adapter signatures", () => { expect(() => - computeWatchSignature({ - kind: "stash-show", - options: { mode: "auto", vcs: "jj" }, - }), + computeWatchSignature( + { + kind: "stash-show", + options: { mode: "auto", vcs: "jj" }, + }, + { cwd: process.cwd() }, + ), ).toThrow("`hunk stash show` requires Git VCS mode."); }); @@ -142,13 +163,13 @@ describe("computeWatchSignature", () => { const untrackedPath = join(dir, "note.txt"); writeFileSync(untrackedPath, "first\n"); - const initialSignature = withCwd(dir, () => - computeWatchSignature(createGitInput({ range: "main" })), - ); + const initialSignature = computeWatchSignature(createGitInput({ range: "main" }), { + cwd: dir, + }); writeFileSync(untrackedPath, "second\n"); - const changedSignature = withCwd(dir, () => - computeWatchSignature(createGitInput({ range: "main" })), - ); + const changedSignature = computeWatchSignature(createGitInput({ range: "main" }), { + cwd: dir, + }); expect(changedSignature).not.toEqual(initialSignature); }); diff --git a/src/core/watch.ts b/src/core/watch.ts index 04de1dfc..3752a641 100644 --- a/src/core/watch.ts +++ b/src/core/watch.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import { resolve } from "node:path"; import { createVcsWatchSignature, getConfiguredVcsAdapter, operationFromInput } from "./vcs"; import type { CliInput } from "./types"; @@ -22,35 +23,48 @@ function statSignature(path: string) { } /** Build one exact patch signature for adapter-backed review inputs. */ -function vcsPatchSignature(input: Extract) { +function vcsPatchSignature( + input: Extract, + context: WatchSignatureContext, +) { const adapter = getConfiguredVcsAdapter(input.options.vcs); const operation = operationFromInput(input); - return createVcsWatchSignature(adapter, operation, { cwd: process.cwd() }); + return createVcsWatchSignature(adapter, operation, context); } -/** Compute a change-detection signature for one watchable input. */ -export function computeWatchSignature(input: CliInput) { + +export interface WatchSignatureContext { + cwd: string; + gitExecutable?: string; +} + +/** Compute a change-detection signature relative to the source's stable load context. */ +export function computeWatchSignature(input: CliInput, context: WatchSignatureContext) { const parts: string[] = [input.kind]; + const resolveInputPath = (path: string) => resolve(context.cwd, path); switch (input.kind) { case "vcs": case "show": case "stash-show": - parts.push(vcsPatchSignature(input)); + parts.push(vcsPatchSignature(input, context)); break; case "diff": case "difftool": - parts.push(statSignature(input.left), statSignature(input.right)); + parts.push( + statSignature(resolveInputPath(input.left)), + statSignature(resolveInputPath(input.right)), + ); break; case "patch": if (!input.file || input.file === "-") { throw new Error("Watch mode requires a patch file path instead of stdin."); } - parts.push(statSignature(input.file)); + parts.push(statSignature(resolveInputPath(input.file))); break; } if (input.options.agentContext && input.options.agentContext !== "-") { - parts.push(`agent:${statSignature(input.options.agentContext)}`); + parts.push(`agent:${statSignature(resolveInputPath(input.options.agentContext))}`); } return parts.join("\n---\n"); diff --git a/src/core/watchController.test.ts b/src/core/watchController.test.ts new file mode 100644 index 00000000..6f2066c8 --- /dev/null +++ b/src/core/watchController.test.ts @@ -0,0 +1,622 @@ +import { describe, expect, test } from "bun:test"; + +import { + createWatchController, + WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE, + type WatchControllerClock, + type WatchEventSourceCallbacks, +} from "./watchController"; + +class FakeWatchClock implements WatchControllerClock { + nowMs = 0; + nextId = 1; + timers = new Map void }>(); + scheduledDelays: number[] = []; + + /** Return deterministic virtual time. */ + now() { + return this.nowMs; + } + + /** Record one virtual timeout. */ + setTimeout(callback: () => void, delayMs: number) { + const id = this.nextId++; + this.scheduledDelays.push(delayMs); + this.timers.set(id, { at: this.nowMs + delayMs, callback }); + return id; + } + + /** Cancel one virtual timeout. */ + clearTimeout(handle: unknown) { + this.timers.delete(handle as number); + } + + /** Advance through every timeout due in the requested interval. */ + advance(ms: number) { + const target = this.nowMs + ms; + while (true) { + const due = [...this.timers.entries()] + .filter(([, timer]) => timer.at <= target) + .sort((left, right) => left[1].at - right[1].at || left[0] - right[0])[0]; + if (!due) break; + this.nowMs = due[1].at; + this.timers.delete(due[0]); + due[1].callback(); + } + this.nowMs = target; + } +} + +interface Deferred { + promise: Promise; + resolve(value: T): void; + reject(error: unknown): void; +} + +/** Create an externally controlled promise for backpressure tests. */ +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +/** Drain the bounded promise chain used by one controller transition. */ +async function settle() { + for (let index = 0; index < 8; index += 1) await Promise.resolve(); +} + +/** Capture a backend-neutral event source's callbacks. */ +function fakeSource() { + let callbacks!: WatchEventSourceCallbacks; + let closes = 0; + return { + create(nextCallbacks: WatchEventSourceCallbacks) { + callbacks = nextCallbacks; + return { close: () => closes++ }; + }, + event() { + callbacks.onEvent(); + }, + error(error: unknown) { + callbacks.onError(error); + }, + ready() { + callbacks.onReady?.(); + }, + get closes() { + return closes; + }, + }; +} + +describe("createWatchController", () => { + test("debounces a hint without installing a 250 ms recurring timer", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => { + checks++; + return "same"; + }, + refresh: () => {}, + }); + + source.event(); + clock.advance(199); + expect(checks).toBe(0); + clock.advance(1); + await settle(); + expect(checks).toBe(1); + expect(clock.scheduledDelays).not.toContain(250); + }); + + test("uses signatures to distinguish changed and unchanged hints", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const signatures = ["same", "changed"]; + let refreshes = 0; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => signatures.shift()!, + refresh: () => { + refreshes++; + }, + }); + + source.event(); + clock.advance(200); + await settle(); + source.event(); + clock.advance(200); + await settle(); + expect(refreshes).toBe(1); + expect(controller.getState().appliedSignature).toBe("changed"); + }); + + test("coalesces an event burst behind the quiet debounce", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + }); + + source.event(); + clock.advance(100); + source.event(); + clock.advance(100); + source.event(); + clock.advance(199); + expect(checks).toBe(0); + clock.advance(1); + await settle(); + expect(checks).toBe(1); + }); + + test("forces continuous event noise to progress at the maximum delay", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + }); + + source.event(); + for (let elapsed = 100; elapsed < 1_000; elapsed += 100) { + clock.advance(100); + source.event(); + } + clock.advance(100); + await settle(); + expect(checks).toBe(1); + }); + + test("serializes refreshes and performs one changed trailing check", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const firstRefresh = deferred(); + const signatures = ["one", "two"]; + let activeRefreshes = 0; + let maximumRefreshes = 0; + let refreshes = 0; + createWatchController({ + initialSignature: "zero", + clock, + createEventSource: source.create, + getSignature: () => signatures.shift()!, + refresh: async () => { + refreshes++; + activeRefreshes++; + maximumRefreshes = Math.max(maximumRefreshes, activeRefreshes); + if (refreshes === 1) await firstRefresh.promise; + activeRefreshes--; + }, + }); + + source.event(); + clock.advance(200); + await settle(); + source.event(); + source.event(); + firstRefresh.resolve(); + await settle(); + expect(refreshes).toBe(2); + expect(maximumRefreshes).toBe(1); + }); + + test("coalesces multiple refresh-time events into one unchanged trailing check", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const pending = deferred(); + const signatures = ["changed", "changed"]; + let checks = 0; + let refreshes = 0; + createWatchController({ + initialSignature: "old", + clock, + createEventSource: source.create, + getSignature: () => (++checks, signatures.shift()!), + refresh: () => { + refreshes++; + return pending.promise; + }, + }); + + source.event(); + clock.advance(200); + await settle(); + source.event(); + source.event(); + source.event(); + pending.resolve(); + await settle(); + expect(checks).toBe(2); + expect(refreshes).toBe(1); + }); + + test("retains the old baseline after refresh rejection and retries", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const errors: unknown[] = []; + let attempts = 0; + const controller = createWatchController({ + initialSignature: "old", + clock, + createEventSource: source.create, + getSignature: () => "new", + refresh: () => { + attempts++; + if (attempts === 1) throw new Error("reload failed"); + }, + reportError: (error) => errors.push(error), + }); + + source.event(); + clock.advance(200); + await settle(); + expect(controller.getState().appliedSignature).toBe("old"); + source.event(); + clock.advance(200); + await settle(); + expect(attempts).toBe(2); + expect(controller.getState().appliedSignature).toBe("new"); + expect(errors).toHaveLength(1); + }); + + test("keeps a signature exception retryable", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + let refreshes = 0; + createWatchController({ + initialSignature: "old", + clock, + createEventSource: source.create, + getSignature: () => { + checks++; + if (checks === 1) throw new Error("stat failed"); + return "new"; + }, + refresh: () => { + refreshes++; + }, + }); + + source.event(); + clock.advance(200); + await settle(); + source.event(); + clock.advance(200); + await settle(); + expect(checks).toBe(2); + expect(refreshes).toBe(1); + }); + + test("checks the bootstrap signature immediately after source readiness", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + }); + + source.ready(); + await settle(); + expect(checks).toBe(1); + }); + + test("degrades a stalled source after the default startup deadline", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const errors: unknown[] = []; + let checks = 0; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + reportError: (error) => errors.push(error), + }); + + clock.advance(1_999); + expect(controller.getState().degraded).toBe(false); + expect(source.closes).toBe(0); + clock.advance(1); + expect(controller.getState().degraded).toBe(true); + expect(source.closes).toBe(1); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatchObject({ code: WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE }); + + clock.advance(1_999); + expect(checks).toBe(0); + clock.advance(1); + await settle(); + expect(checks).toBe(1); + clock.advance(2_000); + await settle(); + expect(checks).toBe(2); + }); + + test("accepts readiness just before an injected startup deadline", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + startupTimeoutMs: 500, + }); + + clock.advance(499); + source.ready(); + await settle(); + clock.advance(1); + expect(checks).toBe(1); + expect(source.closes).toBe(0); + expect(controller.getState().degraded).toBe(false); + }); + + test("ignores readiness, events, and duplicate errors after startup degradation", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const errors: unknown[] = []; + let checks = 0; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + reportError: (error) => errors.push(error), + startupTimeoutMs: 500, + }); + + clock.advance(500); + source.ready(); + source.event(); + source.error(Object.assign(new Error("late resource error"), { code: "ENOSPC" })); + source.error(Object.assign(new Error("duplicate late error"), { code: "ENOSPC" })); + clock.advance(1_999); + await settle(); + expect(checks).toBe(0); + expect(source.closes).toBe(1); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatchObject({ code: WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE }); + expect(controller.getState().degraded).toBe(true); + + clock.advance(1); + await settle(); + expect(checks).toBe(1); + }); + + test("closing during startup cancels the deadline and closes the source once", () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const errors: unknown[] = []; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => "same", + refresh: () => {}, + reportError: (error) => errors.push(error), + startupTimeoutMs: 500, + }); + + controller.close(); + source.ready(); + source.event(); + source.error(new Error("late")); + clock.advance(10_000); + expect(source.closes).toBe(1); + expect(errors).toEqual([]); + expect(clock.timers.size).toBe(0); + }); + + test("runs a healthy safety check without an event", async () => { + const clock = new FakeWatchClock(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + getSignature: () => (++checks, "same"), + refresh: () => {}, + }); + + clock.advance(9_999); + expect(checks).toBe(0); + clock.advance(1); + await settle(); + expect(checks).toBe(1); + }); + + test("coalesces an event and safety deadline collision", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => (++checks, "same"), + refresh: () => {}, + }); + + clock.advance(9_800); + source.event(); + clock.advance(200); + await settle(); + expect(checks).toBe(1); + }); + + test("falls back to degraded polling when watcher startup fails", async () => { + const clock = new FakeWatchClock(); + const failure = new Error("watcher unavailable"); + const errors: unknown[] = []; + let checks = 0; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: () => { + throw failure; + }, + getSignature: () => (++checks, "same"), + refresh: () => {}, + reportError: (error) => errors.push(error), + }); + + expect(controller.getState().degraded).toBe(true); + clock.advance(2_000); + await settle(); + expect(checks).toBe(1); + expect(errors).toEqual([failure]); + }); + + test("classifies ENOSPC and EMFILE by code and rate-limits duplicate reports", () => { + for (const code of ["ENOSPC", "EMFILE"]) { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const errors: unknown[] = []; + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => "same", + refresh: () => {}, + reportError: (error) => errors.push(error), + }); + source.error(Object.assign(new Error("resource limit"), { code })); + source.error(Object.assign(new Error("resource limit again"), { code })); + expect(controller.getState().degraded).toBe(true); + expect(source.closes).toBe(1); + expect(errors).toHaveLength(1); + } + }); + + test("reports unknown source errors without degrading", () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const errors: unknown[] = []; + const failure = Object.assign(new Error("temporary"), { code: "EACCES" }); + const controller = createWatchController({ + initialSignature: "same", + clock, + createEventSource: source.create, + getSignature: () => "same", + refresh: () => {}, + reportError: (error) => errors.push(error), + }); + + source.error(failure); + expect(controller.getState().degraded).toBe(false); + expect(errors).toEqual([failure]); + }); + + test("supports poll-only plans on the degraded interval", async () => { + const clock = new FakeWatchClock(); + let checks = 0; + createWatchController({ + initialSignature: "same", + clock, + pollOnly: true, + getSignature: () => (++checks, "same"), + refresh: () => {}, + }); + clock.advance(2_000); + await settle(); + expect(checks).toBe(1); + }); + + test("close is idempotent, cancels timers, and ignores late completion", async () => { + const clock = new FakeWatchClock(); + const source = fakeSource(); + const signature = deferred(); + let refreshes = 0; + const controller = createWatchController({ + initialSignature: "old", + clock, + createEventSource: source.create, + getSignature: () => signature.promise, + refresh: () => { + refreshes++; + }, + }); + + source.event(); + clock.advance(200); + controller.close(); + controller.close(); + signature.resolve("new"); + await settle(); + clock.advance(20_000); + expect(controller.getState().phase).toBe("closed"); + expect(refreshes).toBe(0); + expect(source.closes).toBe(1); + expect(clock.timers.size).toBe(0); + }); + + test("replacing a controller isolates the old input from the new one", async () => { + const clock = new FakeWatchClock(); + const oldSource = fakeSource(); + const newSource = fakeSource(); + const errors: unknown[] = []; + let oldChecks = 0; + let newChecks = 0; + const oldController = createWatchController({ + initialSignature: "old", + clock, + createEventSource: oldSource.create, + getSignature: () => (++oldChecks, "old"), + refresh: () => {}, + reportError: (error) => errors.push(error), + startupTimeoutMs: 500, + }); + oldController.close(); + createWatchController({ + initialSignature: "new", + clock, + createEventSource: newSource.create, + getSignature: () => (++newChecks, "new"), + refresh: () => {}, + startupTimeoutMs: 500, + }); + + oldSource.ready(); + oldSource.event(); + newSource.ready(); + await settle(); + clock.advance(500); + await settle(); + expect(oldChecks).toBe(0); + expect(newChecks).toBe(1); + expect(oldSource.closes).toBe(1); + expect(newSource.closes).toBe(0); + expect(errors).toEqual([]); + }); +}); diff --git a/src/core/watchController.ts b/src/core/watchController.ts new file mode 100644 index 00000000..cabc929f --- /dev/null +++ b/src/core/watchController.ts @@ -0,0 +1,339 @@ +export type WatchControllerPhase = + | "starting" + | "idle" + | "debouncing" + | "checking" + | "refreshing" + | "closed"; + +export interface WatchControllerClock { + now(): number; + setTimeout(callback: () => void, delayMs: number): unknown; + clearTimeout(handle: unknown): void; +} + +export interface WatchEventSource { + close(): void; +} + +export interface WatchEventSourceCallbacks { + onEvent(): void; + onError(error: unknown): void; + onReady?(): void; +} + +export const DEFAULT_WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_MS = 2_000; +export const WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE = "HUNK_WATCH_EVENT_SOURCE_STARTUP_TIMEOUT"; + +export interface WatchControllerOptions { + initialSignature: string; + getSignature: () => string | Promise; + refresh: () => void | Promise; + clock?: WatchControllerClock; + createEventSource?: (callbacks: WatchEventSourceCallbacks) => WatchEventSource; + pollOnly?: boolean; + reportError?: (error: unknown) => void; + quietDelayMs?: number; + maximumDelayMs?: number; + healthyCheckMs?: number; + degradedCheckMs?: number; + duplicateErrorIntervalMs?: number; + startupTimeoutMs?: number; +} + +export interface WatchControllerState { + phase: WatchControllerPhase; + dirty: boolean; + degraded: boolean; + appliedSignature: string; +} + +export interface WatchController { + close(): void; + getState(): Readonly; +} + +const defaultClock: WatchControllerClock = { + now: () => Date.now(), + setTimeout: (callback, delayMs) => setTimeout(callback, delayMs), + clearTimeout: (handle) => clearTimeout(handle as ReturnType), +}; + +/** Read an error code without relying on a particular watcher error class. */ +function getErrorCode(error: unknown) { + if (typeof error !== "object" || error === null || !("code" in error)) return undefined; + return String(error.code); +} + +/** Produce a stable key used to suppress repeated reports from a noisy event source. */ +function getErrorKey(error: unknown) { + const code = getErrorCode(error); + if (code) return `code:${code}`; + if (error instanceof Error) return `${error.name}:${error.message}`; + return String(error); +} + +/** Build the stable diagnostic reported when an event source cannot establish readiness. */ +function createEventSourceStartupTimeoutError(timeoutMs: number) { + return Object.assign( + new Error(`The watch event source did not become ready within ${timeoutMs} ms.`), + { + code: WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE, + name: "WatchEventSourceStartupTimeoutError", + }, + ); +} + +/** Coordinate event hints and periodic checks without coupling to a watcher backend. */ +export function createWatchController(options: WatchControllerOptions): WatchController { + const clock = options.clock ?? defaultClock; + const quietDelayMs = options.quietDelayMs ?? 200; + const maximumDelayMs = options.maximumDelayMs ?? 1_000; + const healthyCheckMs = options.healthyCheckMs ?? 10_000; + const degradedCheckMs = options.degradedCheckMs ?? 2_000; + const duplicateErrorIntervalMs = options.duplicateErrorIntervalMs ?? 10_000; + const startupTimeoutMs = + options.startupTimeoutMs ?? DEFAULT_WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_MS; + + const state: WatchControllerState = { + phase: "starting", + dirty: false, + degraded: Boolean(options.pollOnly), + appliedSignature: options.initialSignature, + }; + let eventSource: WatchEventSource | undefined; + let sourceStatus: "none" | "starting" | "ready" | "closed" = "none"; + let timer: unknown; + let timerDeadline: number | undefined; + let startupDeadline: number | undefined; + let quietDeadline: number | undefined; + let maximumDeadline: number | undefined; + let safetyDeadline: number | undefined; + const reportedAt = new Map(); + + /** Report an error at most once per configured interval for the same error key. */ + const reportError = (error: unknown) => { + const key = getErrorKey(error); + const now = clock.now(); + const previous = reportedAt.get(key); + if (previous !== undefined && now - previous < duplicateErrorIntervalMs) return; + reportedAt.set(key, now); + options.reportError?.(error); + }; + + const safetyInterval = () => (state.degraded ? degradedCheckMs : healthyCheckMs); + + /** Clear the one active chained timeout. */ + const clearTimer = () => { + if (timer !== undefined) clock.clearTimeout(timer); + timer = undefined; + timerDeadline = undefined; + }; + + /** Schedule only the earliest outstanding deadline. */ + const schedule = () => { + if (state.phase === "closed" || state.phase === "checking" || state.phase === "refreshing") { + return; + } + const deadlines = [startupDeadline, quietDeadline, maximumDeadline, safetyDeadline].filter( + (deadline): deadline is number => deadline !== undefined, + ); + if (deadlines.length === 0) return; + const deadline = Math.min(...deadlines); + if (timerDeadline === deadline) return; + clearTimer(); + timerDeadline = deadline; + timer = clock.setTimeout(onTimer, Math.max(0, deadline - clock.now())); + }; + + /** Test closure across async boundaries without relying on narrowed phase state. */ + const isClosed = () => state.phase === "closed"; + + /** Test source closure after construction callbacks that TypeScript cannot observe. */ + const isSourceClosed = () => sourceStatus === "closed"; + + /** Close the current source once and invalidate all of its later callbacks. */ + const closeEventSource = () => { + if (sourceStatus === "none" || sourceStatus === "closed") return; + sourceStatus = "closed"; + startupDeadline = undefined; + const source = eventSource; + eventSource = undefined; + source?.close(); + }; + + /** Finish work and honor all in-flight hints as one trailing check. */ + const finishCheck = () => { + if (isClosed()) return; + safetyDeadline = clock.now() + safetyInterval(); + state.phase = "idle"; + if (state.dirty) { + state.dirty = false; + void beginCheck(); + return; + } + schedule(); + }; + + /** Run one serialized signature check and refresh only when it changed. */ + const beginCheck = async () => { + if (state.phase === "closed" || state.phase === "checking" || state.phase === "refreshing") { + return; + } + clearTimer(); + quietDeadline = undefined; + maximumDeadline = undefined; + safetyDeadline = undefined; + state.phase = "checking"; + + let signature: string; + try { + signature = await options.getSignature(); + } catch (error) { + if (isClosed()) return; + reportError(error); + finishCheck(); + return; + } + if (isClosed()) return; + if (signature === state.appliedSignature) { + finishCheck(); + return; + } + + state.phase = "refreshing"; + try { + await options.refresh(); + } catch (error) { + if (isClosed()) return; + reportError(error); + finishCheck(); + return; + } + if (isClosed()) return; + state.appliedSignature = signature; + finishCheck(); + }; + + /** Degrade one source that failed to establish readiness before its deadline. */ + const degradeStalledSource = () => { + if (sourceStatus !== "starting") return; + closeEventSource(); + state.degraded = true; + state.phase = "idle"; + quietDeadline = undefined; + maximumDeadline = undefined; + safetyDeadline = clock.now() + degradedCheckMs; + reportError(createEventSourceStartupTimeoutError(startupTimeoutMs)); + schedule(); + }; + + /** Consume a due startup, debounce, maximum-delay, or safety deadline as one check. */ + function onTimer() { + timer = undefined; + timerDeadline = undefined; + if (state.phase === "closed") return; + const now = clock.now(); + if (startupDeadline !== undefined && startupDeadline <= now) { + degradeStalledSource(); + return; + } + const eventDue = + (quietDeadline !== undefined && quietDeadline <= now) || + (maximumDeadline !== undefined && maximumDeadline <= now); + const safetyDue = safetyDeadline !== undefined && safetyDeadline <= now; + if (eventDue || safetyDue) { + void beginCheck(); + } else { + schedule(); + } + } + + /** Treat an event as a hint and retain the first event's maximum deadline. */ + const onEvent = () => { + if (state.phase === "closed" || (sourceStatus !== "starting" && sourceStatus !== "ready")) { + return; + } + if (state.phase === "checking" || state.phase === "refreshing") { + state.dirty = true; + return; + } + const now = clock.now(); + quietDeadline = now + quietDelayMs; + maximumDeadline ??= now + maximumDelayMs; + state.phase = "debouncing"; + schedule(); + }; + + /** Close the startup scan race with an immediate signature check after watcher readiness. */ + const onSourceReady = () => { + if (state.phase === "closed" || sourceStatus !== "starting") return; + sourceStatus = "ready"; + startupDeadline = undefined; + if (state.phase === "checking" || state.phase === "refreshing") { + state.dirty = true; + return; + } + void beginCheck(); + }; + + /** Degrade only for watcher resource exhaustion; other source errors stay nonfatal. */ + const onSourceError = (error: unknown) => { + if (state.phase === "closed" || sourceStatus === "closed") return; + const code = getErrorCode(error); + if (code === "ENOSPC" || code === "EMFILE") { + state.degraded = true; + closeEventSource(); + safetyDeadline = Math.min( + safetyDeadline ?? Number.POSITIVE_INFINITY, + clock.now() + degradedCheckMs, + ); + schedule(); + } + reportError(error); + }; + + state.phase = "idle"; + safetyDeadline = clock.now() + safetyInterval(); + if (options.createEventSource && !options.pollOnly) { + sourceStatus = "starting"; + startupDeadline = clock.now() + startupTimeoutMs; + // This JS timer is a secondary guard: FSEvents lock contention can also delay timers, so + // bounded native registration remains the primary macOS protection. + schedule(); + try { + const createdSource = options.createEventSource({ + onEvent, + onError: onSourceError, + onReady: onSourceReady, + }); + if (isSourceClosed()) createdSource.close(); + else eventSource = createdSource; + } catch (error) { + sourceStatus = "closed"; + startupDeadline = undefined; + state.degraded = true; + safetyDeadline = clock.now() + degradedCheckMs; + reportError(error); + } + } + schedule(); + + return { + /** Stop observation and ignore any later asynchronous completion. */ + close() { + if (state.phase === "closed") return; + state.phase = "closed"; + state.dirty = false; + quietDeadline = undefined; + maximumDeadline = undefined; + safetyDeadline = undefined; + clearTimer(); + closeEventSource(); + }, + /** Expose a snapshot for diagnostics without allowing state mutation. */ + getState() { + return { ...state }; + }, + }; +} diff --git a/src/core/watchObserver.fs.test.ts b/src/core/watchObserver.fs.test.ts new file mode 100644 index 00000000..05ceacd1 --- /dev/null +++ b/src/core/watchObserver.fs.test.ts @@ -0,0 +1,278 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { mkdtemp, mkdir, rename, rm, unlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { createWatchController } from "./watchController"; +import { createWatchEventSource, createWatchObserver, type WatchObserver } from "./watchObserver"; +import type { WatchPlan } from "./watchPlan"; + +const WAIT_MS = 3_000; +const ABSENCE_MS = 250; +const cleanups: Array<() => Promise> = []; + +afterEach(async () => { + await Promise.all(cleanups.splice(0).map((cleanup) => cleanup())); +}); + +/** Reject when a lifecycle or filesystem event exceeds its explicit test bound. */ +async function bounded(promise: Promise, timeoutMs = WAIT_MS): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`Timed out after ${timeoutMs} ms`)), timeoutMs); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} + +/** Create a disposable real directory for one watcher test. */ +async function temporaryDirectory() { + const directory = await mkdtemp(join(tmpdir(), "hunk-watch-")); + cleanups.push(() => rm(directory, { recursive: true, force: true })); + return directory; +} + +/** Build an exact-entry plan for one or more files in a shared parent directory. */ +function entriesPlan(directory: string, entries: string[]): WatchPlan { + return { + coverage: "hybrid", + targets: [{ kind: "directory-entries", directory, entries, sources: ["content"] }], + }; +} + +/** Start an observer and expose queued events so mutations cannot race test listeners. */ +async function startObserver(plan: WatchPlan) { + let eventCount = 0; + let pendingEvents = 0; + const waiters: Array<() => void> = []; + const observer = createWatchObserver(plan, { + onEvent() { + eventCount++; + const waiter = waiters.shift(); + if (waiter) waiter(); + else pendingEvents++; + }, + onError(error) { + throw error; + }, + }); + cleanups.push(async () => { + observer.close(); + await bounded(observer.closed); + }); + await bounded(observer.ready); + + return { + observer, + get eventCount() { + return eventCount; + }, + nextEvent() { + if (pendingEvents > 0) { + pendingEvents--; + return Promise.resolve(); + } + return new Promise((resolve) => waiters.push(resolve)); + }, + }; +} + +/** Assert that no observer event arrives during a short bounded interval. */ +async function expectNoEvent(nextEvent: Promise) { + const outcome = await Promise.race([ + nextEvent.then(() => "event" as const), + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), ABSENCE_MS)), + ]); + expect(outcome).toBe("timeout"); +} + +/** Verify an irrelevant mutation cannot refresh even when a backend emits a conservative hint. */ +async function expectNoRefresh( + plan: WatchPlan, + getSignature: () => string, + mutate: () => Promise, +) { + let observer!: WatchObserver; + let checks = 0; + let refreshes = 0; + const controller = createWatchController({ + initialSignature: getSignature(), + createEventSource: (callbacks) => { + observer = createWatchObserver(plan, callbacks); + return observer; + }, + getSignature: () => { + checks++; + return getSignature(); + }, + refresh: () => { + refreshes++; + }, + quietDelayMs: 10, + healthyCheckMs: 50, + }); + cleanups.push(async () => { + controller.close(); + await bounded(observer.closed); + }); + await bounded(observer.ready); + await mutate(); + await bounded( + (async () => { + while (checks < 2) await new Promise((resolve) => setTimeout(resolve, 5)); + })(), + ); + expect(refreshes).toBe(0); +} + +describe("filesystem watch observer", () => { + test("does not create an event-source factory for poll-only plans", () => { + expect(createWatchEventSource({ coverage: "poll-only", targets: [] })).toBeUndefined(); + }); + + test("observes an ordinary file write after readiness", async () => { + const directory = await temporaryDirectory(); + const file = join(directory, "input.patch"); + await writeFile(file, "before"); + const source = await startObserver(entriesPlan(directory, [file])); + + await writeFile(file, "after"); + await bounded(source.nextEvent()); + }); + + test("observes temp-file atomic replacement", async () => { + const directory = await temporaryDirectory(); + const file = join(directory, "input.patch"); + const temporary = join(directory, ".input.patch.tmp"); + await writeFile(file, "before"); + const source = await startObserver(entriesPlan(directory, [file])); + + await writeFile(temporary, "after"); + await rename(temporary, file); + await bounded(source.nextEvent()); + }); + + test("observes deletion and recreation", async () => { + const directory = await temporaryDirectory(); + const file = join(directory, "input.patch"); + await writeFile(file, "before"); + const source = await startObserver(entriesPlan(directory, [file])); + + await unlink(file); + await bounded(source.nextEvent()); + await writeFile(file, "after"); + await bounded(source.nextEvent()); + }); + + test("ignores sibling files for an exact-entry target", async () => { + const directory = await temporaryDirectory(); + const target = join(directory, "target.patch"); + const sibling = join(directory, "sibling.patch"); + await writeFile(target, "target"); + await writeFile(sibling, "before"); + const plan = entriesPlan(directory, [target]); + + await expectNoRefresh( + plan, + () => readFileSync(target, "utf8"), + () => writeFile(sibling, "after"), + ); + }); + + test("observes recursive worktree events", async () => { + const directory = await temporaryDirectory(); + const nestedDirectory = join(directory, "src", "nested"); + await mkdir(nestedDirectory, { recursive: true }); + const file = join(nestedDirectory, "file.ts"); + await writeFile(file, "before"); + const source = await startObserver({ + coverage: "hybrid", + targets: [{ kind: "directory-tree", directory, ignoredRoots: [], sources: ["worktree"] }], + }); + + await writeFile(file, "after"); + await bounded(source.nextEvent()); + }); + + test("observes atomic replacement in a recursive tree", async () => { + const directory = await temporaryDirectory(); + const nestedDirectory = join(directory, "src", "nested"); + await mkdir(nestedDirectory, { recursive: true }); + const file = join(nestedDirectory, "file.ts"); + const temporary = join(nestedDirectory, ".file.ts.tmp"); + await writeFile(file, "before"); + const source = await startObserver({ + coverage: "hybrid", + targets: [{ kind: "directory-tree", directory, ignoredRoots: [], sources: ["worktree"] }], + }); + + await writeFile(temporary, "after"); + await rename(temporary, file); + await bounded(source.nextEvent()); + }); + + test("observes files written into a newly created directory", async () => { + const directory = await temporaryDirectory(); + const source = await startObserver({ + coverage: "hybrid", + targets: [{ kind: "directory-tree", directory, ignoredRoots: [], sources: ["worktree"] }], + }); + const nestedDirectory = join(directory, "new", "nested"); + + await mkdir(nestedDirectory, { recursive: true }); + await bounded(source.nextEvent()); + await new Promise((resolve) => setTimeout(resolve, 50)); + const eventsAfterCreation = source.eventCount; + await writeFile(join(nestedDirectory, "file.ts"), "content"); + await bounded( + (async () => { + while (source.eventCount === eventsAfterCreation) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + })(), + ); + }); + + test("excludes a worktree .git subtree when metadata is observed separately", async () => { + const directory = await temporaryDirectory(); + const metadataDirectory = join(directory, ".git"); + await mkdir(metadataDirectory); + const metadata = join(metadataDirectory, "index"); + const worktreeFile = join(directory, "file.ts"); + await writeFile(metadata, "before"); + await writeFile(worktreeFile, "before"); + const plan: WatchPlan = { + coverage: "hybrid", + targets: [ + { + kind: "directory-tree", + directory, + ignoredRoots: [metadataDirectory], + sources: ["worktree"], + }, + ], + }; + + const source = await startObserver(plan); + await writeFile(metadata, "after"); + await expectNoEvent(source.nextEvent()); + }); + + test("close releases handles and suppresses later events", async () => { + const directory = await temporaryDirectory(); + const file = join(directory, "input.patch"); + await writeFile(file, "before"); + const source = await startObserver(entriesPlan(directory, [file])); + + source.observer.close(); + await bounded(source.observer.closed); + await writeFile(file, "after"); + await expectNoEvent(source.nextEvent()); + }); +}); diff --git a/src/core/watchObserver.test.ts b/src/core/watchObserver.test.ts new file mode 100644 index 00000000..82ab46c7 --- /dev/null +++ b/src/core/watchObserver.test.ts @@ -0,0 +1,216 @@ +import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; + +import { createWatchTestClock } from "../../test/helpers/watchTest"; +import { createWatchController, WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE } from "./watchController"; +import { + createNativeTreeWatcher, + createWatchObserver, + type NativeRecursiveWatchFactory, + type WatchTreeBackend, +} from "./watchObserver"; +import type { DirectoryTreeWatchTarget, WatchPlan } from "./watchPlan"; + +/** Build one neutral recursive target for backend and event-filter tests. */ +function treeTarget(directory = "/repo"): DirectoryTreeWatchTarget { + return { + kind: "directory-tree", + directory, + ignoredRoots: [join(directory, ".git"), join(directory, "node_modules")], + sources: ["worktree"], + }; +} + +/** Build a controllable tree backend that records construction and closes. */ +function fakeTreeBackend(calls: string[], name: string): WatchTreeBackend { + return () => { + calls.push(name); + return { + close() { + calls.push(`${name}:close`); + }, + onError() {}, + whenReady(callback) { + queueMicrotask(callback); + }, + }; + }; +} + +/** Start a tree observer with synthetic platform and injected backend choices. */ +async function selectedBackend(platform: NodeJS.Platform) { + const calls: string[] = []; + const plan: WatchPlan = { coverage: "hybrid", targets: [treeTarget()] }; + const observer = createWatchObserver( + plan, + { onEvent() {}, onError() {} }, + { + platform, + treeBackends: { + native: fakeTreeBackend(calls, "native"), + portable: fakeTreeBackend(calls, "portable"), + }, + }, + ); + await observer.ready; + observer.close(); + await observer.closed; + return calls; +} + +describe("watch tree backend selection", () => { + test("degrades and closes an observer whose injected backend stalls during startup", async () => { + const testClock = createWatchTestClock(); + const plan: WatchPlan = { coverage: "hybrid", targets: [treeTarget()] }; + const errors: unknown[] = []; + let closes = 0; + let observer!: ReturnType; + const stalledBackend: WatchTreeBackend = () => ({ + close() { + closes++; + }, + onError() {}, + whenReady() {}, + }); + const controller = createWatchController({ + initialSignature: "same", + clock: testClock.clock, + createEventSource(callbacks) { + observer = createWatchObserver(plan, callbacks, { + platform: "linux", + treeBackends: { native: stalledBackend, portable: stalledBackend }, + }); + return observer; + }, + getSignature: () => "same", + refresh: () => {}, + reportError: (error) => errors.push(error), + startupTimeoutMs: 25, + }); + + testClock.advanceBy(25); + await observer.closed; + expect(controller.getState().degraded).toBe(true); + expect(closes).toBe(1); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatchObject({ code: WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE }); + controller.close(); + expect(closes).toBe(1); + }); + + test.each(["darwin", "win32"] as const)("uses native recursion on %s", async (platform) => { + expect(await selectedBackend(platform)).toEqual(["native", "native:close"]); + }); + + test.each(["linux", "android", "freebsd"] as const)( + "uses portable pruned recursion on %s", + async (platform) => { + expect(await selectedBackend(platform)).toEqual(["portable", "portable:close"]); + }, + ); + + test("does not fall back to portable recursion when native construction fails", () => { + const calls: string[] = []; + const plan: WatchPlan = { coverage: "hybrid", targets: [treeTarget()] }; + + expect(() => + createWatchObserver( + plan, + { onEvent() {}, onError() {} }, + { + platform: "darwin", + treeBackends: { + native() { + calls.push("native"); + throw new Error("native construction failed"); + }, + portable: fakeTreeBackend(calls, "portable"), + }, + }, + ), + ).toThrow("native construction failed"); + expect(calls).toEqual(["native"]); + }); +}); + +describe("native recursive tree watcher", () => { + test("signals readiness on a microtask after construction", async () => { + const order: string[] = []; + const watcher = createNativeTreeWatcher( + treeTarget(), + () => {}, + () => { + order.push("constructed"); + return { close() {}, onError() {} }; + }, + ); + + watcher.whenReady(() => order.push("ready")); + order.push("synchronous"); + expect(order).toEqual(["constructed", "synchronous"]); + await Promise.resolve(); + expect(order).toEqual(["constructed", "synchronous", "ready"]); + }); + + test("suppresses trustworthy paths inside ignored roots", () => { + let emit!: (filename: string | Buffer | null) => void; + let events = 0; + const factory: NativeRecursiveWatchFactory = (_directory, onChange) => { + emit = onChange; + return { close() {}, onError() {} }; + }; + createNativeTreeWatcher(treeTarget(), () => events++, factory); + + emit(join("node_modules", "package", "index.js")); + emit(join(".git", "objects", "pack", "data")); + emit(join("src", "index.ts")); + + expect(events).toBe(1); + }); + + test("conservatively emits for missing and ambiguous filenames", () => { + let emit!: (filename: string | Buffer | null) => void; + let events = 0; + createNativeTreeWatcher( + treeTarget(), + () => events++, + (_directory, onChange) => { + emit = onChange; + return { close() {}, onError() {} }; + }, + ); + + emit(null); + emit(""); + emit("index.js"); + emit(Buffer.from("index.js")); + + expect(events).toBe(4); + }); + + test("forwards errors and releases its native handle", () => { + let reportError!: (error: unknown) => void; + let closes = 0; + const watcher = createNativeTreeWatcher( + treeTarget(), + () => {}, + () => ({ + close() { + closes++; + }, + onError(callback) { + reportError = callback; + }, + }), + ); + const errors: unknown[] = []; + watcher.onError((error) => errors.push(error)); + + const error = new Error("native watcher failed"); + reportError(error); + watcher.close(); + + expect(errors).toEqual([error]); + expect(closes).toBe(1); + }); +}); diff --git a/src/core/watchObserver.ts b/src/core/watchObserver.ts new file mode 100644 index 00000000..480a164c --- /dev/null +++ b/src/core/watchObserver.ts @@ -0,0 +1,249 @@ +import { watch as nativeFsWatch } from "node:fs"; +import { dirname, isAbsolute, resolve } from "node:path"; +import { watch as chokidarWatch, type ChokidarOptions, type FSWatcher } from "chokidar"; + +import type { WatchEventSource, WatchEventSourceCallbacks } from "./watchController"; +import type { DirectoryTreeWatchTarget, WatchPlan, WatchTarget } from "./watchPlan"; + +export interface WatchObserver extends WatchEventSource { + ready: Promise; + closed: Promise; +} + +export interface WatchRegistration { + close(): void | Promise; + onError(callback: (error: unknown) => void): void; + whenReady(callback: () => void): void; +} + +export type WatchTreeBackend = ( + target: DirectoryTreeWatchTarget, + onEvent: () => void, +) => WatchRegistration; + +export interface NativeRecursiveWatchHandle { + close(): void; + onError(callback: (error: unknown) => void): void; +} + +export type NativeRecursiveWatchFactory = ( + directory: string, + onChange: (filename: string | Buffer | null) => void, +) => NativeRecursiveWatchHandle; + +export interface WatchObserverOptions { + platform?: NodeJS.Platform; + treeBackends?: { + native: WatchTreeBackend; + portable: WatchTreeBackend; + }; +} + +const WATCH_OPTIONS = { + ignoreInitial: true, + persistent: true, + followSymlinks: false, + usePolling: false, + awaitWriteFinish: false, +} as const; + +/** Normalize an absolute runtime path for exact event and ignored-root comparisons. */ +function normalizedPath(path: string) { + const absolute = resolve(path); + return process.platform === "win32" ? absolute.toLowerCase() : absolute; +} + +/** Build an ancestor-set matcher whose cost depends on path depth, not ignored-root count. */ +function createIgnoredRootMatcher(ignoredRoots: string[]) { + const roots = new Set(ignoredRoots.map(normalizedPath)); + return (path: string) => { + let current = normalizedPath(path); + for (;;) { + if (roots.has(current)) return true; + const parent = dirname(current); + if (parent === current) return false; + current = parent; + } + }; +} + +/** Adapt one Chokidar handle to the observer's backend-neutral lifecycle. */ +function chokidarRegistration(watcher: FSWatcher): WatchRegistration { + return { + close: () => watcher.close(), + onError: (callback) => watcher.on("error", callback), + whenReady: (callback) => watcher.once("ready", callback), + }; +} + +/** Construct one Bun/Node native recursive handle behind a small test seam. */ +const defaultNativeRecursiveWatch: NativeRecursiveWatchFactory = (directory, onChange) => { + const watcher = nativeFsWatch(directory, { recursive: true }, (_event, filename) => { + onChange(filename); + }); + return { + close: () => watcher.close(), + onError: (callback) => watcher.on("error", callback), + }; +}; + +/** Return whether a native callback supplied enough path context for ignored-root filtering. */ +function isTrustworthyNativeFilename(filename: string) { + return filename.length > 0 && (isAbsolute(filename) || /[\\/]/.test(filename)); +} + +/** Observe one tree with Bun's bounded native recursion and in-process ignore filtering. */ +export function createNativeTreeWatcher( + target: DirectoryTreeWatchTarget, + onEvent: () => void, + watchNative: NativeRecursiveWatchFactory = defaultNativeRecursiveWatch, +): WatchRegistration { + const isIgnored = createIgnoredRootMatcher(target.ignoredRoots); + const watcher = watchNative(target.directory, (rawFilename) => { + const filename = Buffer.isBuffer(rawFilename) ? rawFilename.toString() : rawFilename; + if (!filename || !isTrustworthyNativeFilename(filename)) { + onEvent(); + return; + } + + if (!isIgnored(resolve(target.directory, filename))) onEvent(); + }); + + return { + close: () => watcher.close(), + onError: (callback) => watcher.onError(callback), + // Native registration has no scan-ready event; construction itself establishes the watch. + whenReady: (callback) => queueMicrotask(callback), + }; +} + +/** Observe one tree with Git-pruned Chokidar recursion on portable platforms. */ +function createPortableTreeWatcher( + target: DirectoryTreeWatchTarget, + onEvent: () => void, +): WatchRegistration { + const isIgnored = createIgnoredRootMatcher(target.ignoredRoots); + const watcher = chokidarWatch(target.directory, { + ...WATCH_OPTIONS, + ignored: (path) => isIgnored(path), + }); + watcher.on("all", onEvent); + return chokidarRegistration(watcher); +} + +const DEFAULT_TREE_BACKENDS = { + native: createNativeTreeWatcher, + portable: createPortableTreeWatcher, +} satisfies WatchObserverOptions["treeBackends"]; + +/** Create a depth-zero Chokidar watcher for one exact parent-directory target. */ +function createDirectoryEntriesWatcher( + target: Extract, + onEvent: () => void, +) { + const entries = new Set(target.entries.map(normalizedPath)); + const options: ChokidarOptions = { ...WATCH_OPTIONS, depth: 0 }; + const watcher = chokidarWatch(target.directory, options); + watcher.on("all", (_event, path) => { + if (entries.has(normalizedPath(path))) onEvent(); + }); + return chokidarRegistration(watcher); +} + +/** Select and construct the watcher for one neutral target. */ +function watchTarget( + target: WatchTarget, + onEvent: () => void, + platform: NodeJS.Platform, + treeBackends: NonNullable, +) { + if (target.kind === "directory-entries") { + return createDirectoryEntriesWatcher(target, onEvent); + } + + return platform === "darwin" || platform === "win32" + ? treeBackends.native(target, onEvent) + : treeBackends.portable(target, onEvent); +} + +/** Close every constructed watcher, including handles whose close method throws. */ +function closeRegistrations(registrations: WatchRegistration[]) { + return Promise.all( + registrations.map((registration) => Promise.resolve().then(() => registration.close())), + ); +} + +/** Observe a hybrid watch plan and expose bounded lifecycle promises for callers. */ +export function createWatchObserver( + plan: WatchPlan, + callbacks: WatchEventSourceCallbacks, + options: WatchObserverOptions = {}, +): WatchObserver { + let isClosed = false; + const registrations: WatchRegistration[] = []; + let resolveReady!: () => void; + let resolveClosed!: () => void; + const ready = new Promise((resolvePromise) => { + resolveReady = resolvePromise; + }); + const closed = new Promise((resolvePromise) => { + resolveClosed = resolvePromise; + }); + let remainingReady = plan.targets.length; + const platform = options.platform ?? process.platform; + const treeBackends = options.treeBackends ?? DEFAULT_TREE_BACKENDS; + + const onEvent = () => { + if (!isClosed) callbacks.onEvent(); + }; + const markReady = () => { + if (isClosed) return; + remainingReady--; + if (remainingReady === 0) { + resolveReady(); + callbacks.onReady?.(); + } + }; + + try { + for (const target of plan.targets) { + const registration = watchTarget(target, onEvent, platform, treeBackends); + registrations.push(registration); + registration.onError((error) => { + if (!isClosed) callbacks.onError(error); + }); + registration.whenReady(markReady); + } + } catch (error) { + isClosed = true; + resolveReady(); + void closeRegistrations(registrations).then(resolveClosed, resolveClosed); + throw error; + } + + if (remainingReady === 0) { + queueMicrotask(() => { + if (isClosed) return; + resolveReady(); + callbacks.onReady?.(); + }); + } + + return { + ready, + closed, + /** Mark closed immediately, then release every watcher handle exactly once. */ + close() { + if (isClosed) return; + isClosed = true; + resolveReady(); + void closeRegistrations(registrations).then(resolveClosed, resolveClosed); + }, + }; +} + +/** Adapt a hybrid plan to the controller's injected event-source factory. */ +export function createWatchEventSource(plan: WatchPlan) { + if (plan.coverage === "poll-only") return undefined; + return (callbacks: WatchEventSourceCallbacks) => createWatchObserver(plan, callbacks); +} diff --git a/src/core/watchPlan.test.ts b/src/core/watchPlan.test.ts new file mode 100644 index 00000000..d30626ac --- /dev/null +++ b/src/core/watchPlan.test.ts @@ -0,0 +1,236 @@ +import { describe, expect, test } from "bun:test"; +import { posix, win32 } from "node:path"; +import { resolveWatchPlan } from "./watchPlan"; +import type { CliInput } from "./types"; + +const cwd = posix.join("/", "workspace", "review"); + +/** Build one expected exact-entry target for a plan assertion. */ +function entries( + directory: string, + paths: readonly string[], + sources: ReadonlyArray<"content" | "sidecar">, +) { + return { + kind: "directory-entries" as const, + directory, + entries: [...paths], + sources: [...sources], + }; +} + +describe("resolveWatchPlan", () => { + test.each([ + { + name: "diff", + input: { + kind: "diff", + left: "before/file.ts", + right: "after/file.ts", + options: {}, + } satisfies CliInput, + targets: [ + entries(posix.join(cwd, "after"), [posix.join(cwd, "after", "file.ts")], ["content"]), + entries(posix.join(cwd, "before"), [posix.join(cwd, "before", "file.ts")], ["content"]), + ], + }, + { + name: "difftool without its display-only path", + input: { + kind: "difftool", + left: "tmp/old.ts", + right: "tmp/new.ts", + path: "src/display-only.ts", + options: {}, + } satisfies CliInput, + targets: [ + entries( + posix.join(cwd, "tmp"), + [posix.join(cwd, "tmp", "new.ts"), posix.join(cwd, "tmp", "old.ts")], + ["content"], + ), + ], + }, + { + name: "patch file", + input: { + kind: "patch", + file: "incoming/review.patch", + options: {}, + } satisfies CliInput, + targets: [ + entries( + posix.join(cwd, "incoming"), + [posix.join(cwd, "incoming", "review.patch")], + ["content"], + ), + ], + }, + ])("plans directory entry targets for $name", ({ input, targets }) => { + expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toEqual({ + coverage: "hybrid", + targets: [...targets], + }); + }); + + test("plans missing inputs from their paths without consulting the filesystem", () => { + const input = { + kind: "patch", + file: "not-created-yet/review.patch", + options: {}, + } satisfies CliInput; + + expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toEqual({ + coverage: "hybrid", + targets: [ + entries( + posix.join(cwd, "not-created-yet"), + [posix.join(cwd, "not-created-yet", "review.patch")], + ["content"], + ), + ], + }); + }); + + test.each([ + { + name: "diff", + input: { + kind: "diff", + left: "left.ts", + right: "right.ts", + options: { agentContext: "notes/agent.json" }, + } satisfies CliInput, + contentPaths: [posix.join(cwd, "left.ts"), posix.join(cwd, "right.ts")], + }, + { + name: "difftool", + input: { + kind: "difftool", + left: "left.ts", + right: "right.ts", + path: "display.ts", + options: { agentContext: "notes/agent.json" }, + } satisfies CliInput, + contentPaths: [posix.join(cwd, "left.ts"), posix.join(cwd, "right.ts")], + }, + { + name: "patch file", + input: { + kind: "patch", + file: "review.patch", + options: { agentContext: "notes/agent.json" }, + } satisfies CliInput, + contentPaths: [posix.join(cwd, "review.patch")], + }, + ])("adds an agent sidecar target to $name", ({ input, contentPaths }) => { + expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toEqual({ + coverage: "hybrid", + targets: [ + entries(cwd, contentPaths, ["content"]), + entries(posix.join(cwd, "notes"), [posix.join(cwd, "notes", "agent.json")], ["sidecar"]), + ], + }); + }); + + test("deduplicates same-parent and duplicate paths while retaining every exact entry", () => { + const input = { + kind: "diff", + left: "src/./file.ts", + right: "src/other/../file.ts", + options: { agentContext: "src/notes.json" }, + } satisfies CliInput; + + expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toEqual({ + coverage: "hybrid", + targets: [ + entries( + posix.join(cwd, "src"), + [posix.join(cwd, "src", "file.ts"), posix.join(cwd, "src", "notes.json")], + ["content", "sidecar"], + ), + ], + }); + }); + + test("preserves absolute paths instead of resolving them against cwd", () => { + const input = { + kind: "diff", + left: "/snapshots/before.ts", + right: "after.ts", + options: {}, + } satisfies CliInput; + + expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toEqual({ + coverage: "hybrid", + targets: [ + entries("/snapshots", ["/snapshots/before.ts"], ["content"]), + entries(cwd, [posix.join(cwd, "after.ts")], ["content"]), + ], + }); + }); + + test.each([ + { + name: "stdin patch", + input: { kind: "patch", file: "-", options: {} } satisfies CliInput, + }, + { + name: "implicit stdin patch", + input: { kind: "patch", text: "diff --git a/a b/a", options: {} } satisfies CliInput, + }, + { + name: "stdin agent context", + input: { + kind: "diff", + left: "left.ts", + right: "right.ts", + options: { agentContext: "-" }, + } satisfies CliInput, + }, + ])("leaves $name unwatchable", ({ input }) => { + expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toBeNull(); + }); + + test("uses poll-only plans for adapters without event targets and adds file sidecars", () => { + const vcsInput = { kind: "vcs", staged: false, options: { vcs: "jj" } } satisfies CliInput; + const showInput = { + kind: "show", + options: { vcs: "jj", agentContext: "agent.json" }, + } satisfies CliInput; + + expect(resolveWatchPlan(vcsInput, { cwd, platform: "linux" })).toEqual({ + coverage: "poll-only", + targets: [], + }); + expect(resolveWatchPlan(showInput, { cwd, platform: "linux" })).toEqual({ + coverage: "hybrid", + targets: [entries(cwd, [posix.join(cwd, "agent.json")], ["sidecar"])], + }); + }); + + test("normalizes and deduplicates synthetic Windows paths with win32 helpers", () => { + const windowsCwd = "C:\\work\\review"; + const input = { + kind: "diff", + left: "src\\before.ts", + right: "C:/work/review/src/after.ts", + options: { agentContext: "/c/work/review/src/agent.json" }, + } satisfies CliInput; + + expect(resolveWatchPlan(input, { cwd: windowsCwd, platform: "win32" })).toEqual({ + coverage: "hybrid", + targets: [ + entries( + win32.join(windowsCwd, "src"), + [ + win32.join(windowsCwd, "src", "after.ts"), + win32.join(windowsCwd, "src", "agent.json"), + win32.join(windowsCwd, "src", "before.ts"), + ], + ["content", "sidecar"], + ), + ], + }); + }); +}); diff --git a/src/core/watchPlan.ts b/src/core/watchPlan.ts new file mode 100644 index 00000000..fae84a48 --- /dev/null +++ b/src/core/watchPlan.ts @@ -0,0 +1,143 @@ +import { posix, win32 } from "node:path"; +import { normalizePathForOS } from "../lib/osPath"; +import type { CliInput } from "./types"; +import { createVcsWatchPlan, getConfiguredVcsAdapter, operationFromInput } from "./vcs"; + +export type WatchTargetSource = "content" | "sidecar" | "worktree" | "vcs-metadata"; + +export interface DirectoryEntriesWatchTarget { + kind: "directory-entries"; + directory: string; + entries: string[]; + sources: WatchTargetSource[]; +} + +export interface DirectoryTreeWatchTarget { + kind: "directory-tree"; + directory: string; + ignoredRoots: string[]; + sources: WatchTargetSource[]; +} + +export type WatchTarget = DirectoryEntriesWatchTarget | DirectoryTreeWatchTarget; + +export interface WatchPlan { + coverage: "hybrid" | "poll-only"; + targets: WatchTarget[]; +} + +export interface WatchPlanContext { + cwd: string; + platform?: NodeJS.Platform; + gitExecutable?: string; +} + +interface FileTarget { + path: string; + source: WatchTargetSource; +} + +const SOURCE_ORDER: WatchTargetSource[] = ["content", "sidecar", "worktree", "vcs-metadata"]; + +/** Resolve one source path with path semantics for the source platform. */ +function resolveSourcePath(path: string, context: WatchPlanContext) { + const platform = context.platform ?? process.platform; + const pathApi = platform === "win32" ? win32 : posix; + const cwd = normalizePathForOS(context.cwd, platform); + const inputPath = normalizePathForOS(path, platform); + return pathApi.resolve(cwd, inputPath); +} + +/** Normalize exact file targets into deterministic parent-directory groups. */ +function groupFileTargets(fileTargets: FileTarget[], context: WatchPlanContext) { + const platform = context.platform ?? process.platform; + const pathApi = platform === "win32" ? win32 : posix; + const comparisonKey = (path: string) => (platform === "win32" ? path.toLowerCase() : path); + const comparePaths = (left: string, right: string) => { + const leftKey = comparisonKey(left); + const rightKey = comparisonKey(right); + return leftKey < rightKey ? -1 : leftKey > rightKey ? 1 : 0; + }; + const groups = new Map< + string, + { directory: string; entries: Map; sources: Set } + >(); + + for (const fileTarget of fileTargets) { + const path = resolveSourcePath(fileTarget.path, context); + const directory = pathApi.dirname(path); + const directoryKey = comparisonKey(directory); + let group = groups.get(directoryKey); + + if (!group) { + group = { directory, entries: new Map(), sources: new Set() }; + groups.set(directoryKey, group); + } + + const entryKey = comparisonKey(path); + if (!group.entries.has(entryKey)) { + group.entries.set(entryKey, path); + } + group.sources.add(fileTarget.source); + } + + return [...groups.values()] + .sort((left, right) => comparePaths(left.directory, right.directory)) + .map( + (group): DirectoryEntriesWatchTarget => ({ + kind: "directory-entries", + directory: group.directory, + entries: [...group.entries.values()].sort(comparePaths), + sources: SOURCE_ORDER.filter((source) => group.sources.has(source)), + }), + ); +} + +/** Build a backend-neutral plan for observing one reloadable review input. */ +export function resolveWatchPlan(input: CliInput, context: WatchPlanContext): WatchPlan | null { + if (input.options.agentContext === "-") { + return null; + } + + const fileTargets: FileTarget[] = []; + let coverage: WatchPlan["coverage"] = "hybrid"; + let adapterTargets: WatchTarget[] = []; + + switch (input.kind) { + case "diff": + case "difftool": + fileTargets.push( + { path: input.left, source: "content" }, + { path: input.right, source: "content" }, + ); + break; + case "patch": + if (!input.file || input.file === "-") { + return null; + } + fileTargets.push({ path: input.file, source: "content" }); + break; + case "vcs": + case "show": + case "stash-show": { + const adapter = getConfiguredVcsAdapter(input.options.vcs); + const adapterPlan = createVcsWatchPlan(adapter, operationFromInput(input), { + cwd: context.cwd, + gitExecutable: context.gitExecutable, + }); + coverage = adapterPlan.coverage; + adapterTargets = adapterPlan.targets; + break; + } + } + + if (input.options.agentContext) { + fileTargets.push({ path: input.options.agentContext, source: "sidecar" }); + coverage = "hybrid"; + } + + return { + coverage, + targets: [...adapterTargets, ...groupFileTargets(fileTargets, context)], + }; +} diff --git a/src/hunk-session/sessionFileBounds.test.ts b/src/hunk-session/sessionFileBounds.test.ts index 187f1566..52a1fd70 100644 --- a/src/hunk-session/sessionFileBounds.test.ts +++ b/src/hunk-session/sessionFileBounds.test.ts @@ -13,6 +13,7 @@ function realPath(path: string) { function bootstrapFor(input: CliInput, sourceLabel: string): AppBootstrap { return { input, + reloadContext: { cwd: sourceLabel }, changeset: { id: "changeset:test", sourceLabel, diff --git a/src/hunk-session/sessionFileBounds.ts b/src/hunk-session/sessionFileBounds.ts index 899d0ce9..66f0348f 100644 --- a/src/hunk-session/sessionFileBounds.ts +++ b/src/hunk-session/sessionFileBounds.ts @@ -108,7 +108,7 @@ export function createSessionReloadBounds( case "vcs": case "show": case "stash-show": - roots = [bootstrap.changeset.sourceLabel || initialCwd]; + roots = [bootstrap.reloadContext.repoRoot ?? bootstrap.reloadContext.cwd]; break; case "diff": case "difftool": diff --git a/src/hunk-session/sessionRegistration.test.ts b/src/hunk-session/sessionRegistration.test.ts index 2b80dad9..f3f0ef78 100644 --- a/src/hunk-session/sessionRegistration.test.ts +++ b/src/hunk-session/sessionRegistration.test.ts @@ -33,6 +33,7 @@ function createBootstrap(overrides: Partial = {}): AppBootstrap { initialMode: "split", initialShowAgentNotes: true, ...overrides, + reloadContext: overrides.reloadContext ?? { cwd: "/repo" }, }; } diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 63f8225a..9bb4a66e 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -6,7 +6,7 @@ import { import { useRenderer, useTerminalDimensions } from "@opentui/react"; import { Suspense, lazy, useCallback, useEffect, useMemo, useState, useRef } from "react"; import type { AppBootstrap, CliInput, LayoutMode, UserNoteLineTarget } from "../core/types"; -import { canReloadInput, computeWatchSignature } from "../core/watch"; +import { canReloadInput } from "../core/watch"; import type { HunkSessionBrokerClient, ReloadedSessionResult } from "../hunk-session/types"; import { MenuBar } from "./components/chrome/MenuBar"; import { StatusBar } from "./components/chrome/StatusBar"; @@ -23,6 +23,7 @@ import { useAppKeyboardShortcuts } from "./hooks/useAppKeyboardShortcuts"; import { useHunkSessionBridge } from "./hooks/useHunkSessionBridge"; import { useMenuController } from "./hooks/useMenuController"; import { useReviewController } from "./hooks/useReviewController"; +import { useWatchedInput, type WatchedInputRuntime } from "./hooks/useWatchedInput"; import { buildAppMenus } from "./lib/appMenus"; import { fileRowId } from "./lib/ids"; import { openSelectedFileInEditor } from "./lib/openInEditor"; @@ -93,6 +94,7 @@ export function App({ noticeText, onQuit = () => process.exit(0), onReloadSession, + watchRuntime, }: { bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; @@ -102,6 +104,7 @@ export function App({ nextInput: CliInput, options?: { resetApp?: boolean; sourcePath?: string }, ) => Promise; + watchRuntime?: WatchedInputRuntime; }) { const SIDEBAR_MIN_WIDTH = 22; const DIFF_MIN_WIDTH = 48; @@ -593,56 +596,13 @@ export function App({ triggerRefreshCurrentInput, ]); - useEffect(() => { - if (!watchEnabled) { - return; - } - - let cancelled = false; - let polling = false; - let refreshing = false; - let lastSignature: string; - - try { - lastSignature = computeWatchSignature(bootstrap.input); - } catch (error) { - console.error("Failed to initialize watch mode.", error); - return; - } - - const pollForChanges = () => { - if (cancelled || polling || refreshing) { - return; - } - - polling = true; - - try { - const nextSignature = computeWatchSignature(bootstrap.input); - if (nextSignature !== lastSignature) { - lastSignature = nextSignature; - refreshing = true; - void refreshCurrentInput() - .catch((error) => { - console.error("Failed to auto-reload the current diff.", error); - }) - .finally(() => { - refreshing = false; - }); - } - } catch (error) { - console.error("Failed to poll watch mode input.", error); - } finally { - polling = false; - } - }; - - const interval = setInterval(pollForChanges, 250); - return () => { - cancelled = true; - clearInterval(interval); - }; - }, [bootstrap.input, refreshCurrentInput, watchEnabled]); + useWatchedInput({ + enabled: watchEnabled, + input: bootstrap.input, + refresh: refreshCurrentInput, + reloadContext: bootstrap.reloadContext, + runtime: watchRuntime, + }); /** Leave the app through the shared shutdown path. */ const requestQuit = useCallback(() => { diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 94daad81..5d3e26cf 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -1379,6 +1379,7 @@ describe("App interactions", () => { const setup = await testRender( { } }); - test("watch mode reloads the current file diff from disk", async () => { - const dir = mkdtempSync(join(process.cwd(), ".hunk-watch-")); - const left = join(dir, "before.ts"); - const right = join(dir, "after.ts"); - - writeFileSync(left, "export const answer = 41;\n"); - writeFileSync(right, "export const answer = 42;\n"); - - const bootstrap = await loadAppBootstrap({ - kind: "diff", - left, - right, - options: { - mode: "split", - watch: true, - }, - }); - - const setup = await testRender(, { - width: 220, - height: 20, - }); - - try { - await flush(setup); - - let frame = setup.captureCharFrame(); - expect(frame).not.toContain("export const added = true;"); - - writeFileSync(right, "export const answer = 42;\nexport const added = true;\n"); - - let refreshed = false; - for (let attempt = 0; attempt < 40; attempt += 1) { - await flush(setup); - frame = setup.captureCharFrame(); - if (frame.includes("export const added = true;")) { - refreshed = true; - break; - } - await Bun.sleep(25); - } - - expect(refreshed).toBe(true); - } finally { - await act(async () => { - setup.renderer.destroy(); - }); - rmSync(dir, { force: true, recursive: true }); - } - }); - - test("watch mode preserves the resolved auto theme after refreshing the file diff", async () => { - const dir = mkdtempSync(join(process.cwd(), ".hunk-watch-theme-")); - const left = join(dir, "before.ts"); - const right = join(dir, "after.ts"); - - writeFileSync(left, "export const answer = 41;\n"); - writeFileSync(right, "export const answer = 42;\n"); - - const bootstrap = await loadAppBootstrap({ - kind: "diff", - left, - right, - options: { - mode: "split", - theme: "auto", - watch: true, - }, - }); - // loadAppBootstrap does not do startup-time terminal theme detection in tests. - bootstrap.initialThemeMode = "light"; - - const setup = await testRender(, { - width: 220, - height: 20, - }); - - try { - await flush(setup); - - writeFileSync(right, "export const answer = 42;\nexport const added = true;\n"); - - const refreshedFrame = await waitForFrame( - setup, - (currentFrame) => currentFrame.includes("export const added = true;"), - 40, - ); - expect(refreshedFrame).toContain("export const added = true;"); - - const modalFrame = await openThemesModalFromViewMenu(setup); - expect(modalFrame).toContain("› github-light-default"); - expect(modalFrame).toContain("active"); - } finally { - await act(async () => { - setup.renderer.destroy(); - }); - rmSync(dir, { force: true, recursive: true }); - } - }); - test("custom theme stays active in the theme selector when bootstrap provides a custom palette", async () => { const bootstrap = createBootstrap(); bootstrap.initialTheme = "custom"; @@ -2217,6 +2118,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2270,6 +2172,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2328,6 +2231,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2401,6 +2305,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2462,6 +2367,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index c415a0ba..f64bcf62 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -15,6 +15,7 @@ import { import type { HunkSessionBrokerClient } from "../hunk-session/types"; import { App } from "./App"; import { useStartupUpdateNotice } from "./hooks/useStartupUpdateNotice"; +import type { WatchedInputRuntime } from "./hooks/useWatchedInput"; /** Keep one live Hunk app mounted while allowing daemon-driven session reloads. */ export function AppHost({ @@ -22,18 +23,18 @@ export function AppHost({ hostClient, onQuit = () => process.exit(0), startupNoticeResolver, + watchRuntime, }: { bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; onQuit?: () => void; startupNoticeResolver?: () => Promise; + watchRuntime?: WatchedInputRuntime; }) { const [activeBootstrap, setActiveBootstrap] = useState(bootstrap); const [appVersion, setAppVersion] = useState(0); const [sessionFileBounds] = useState(() => - createSessionReloadBounds(bootstrap, { - cwd: hostClient?.getRegistration().cwd, - }), + createSessionReloadBounds(bootstrap, { cwd: bootstrap.reloadContext.cwd }), ); const startupNoticeText = useStartupUpdateNotice({ enabled: !bootstrap.input.options.pager, @@ -98,6 +99,7 @@ export function AppHost({ noticeText={startupNoticeText} onQuit={onQuit} onReloadSession={reloadSession} + watchRuntime={watchRuntime} /> ); } diff --git a/src/ui/AppHost.watch.test.tsx b/src/ui/AppHost.watch.test.tsx new file mode 100644 index 00000000..cb8441b2 --- /dev/null +++ b/src/ui/AppHost.watch.test.tsx @@ -0,0 +1,192 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, test } from "bun:test"; +import { testRender } from "@opentui/react/test-utils"; +import { act } from "react"; +import { capturedTestColorToHex } from "../../test/helpers/test-color-helpers"; +import { createWatchTestRuntime } from "../../test/helpers/watchTest"; +import { loadAppBootstrap } from "../core/loaders"; +import { AppHost } from "./AppHost"; +import { resolveTheme } from "./themes"; + +async function flush(setup: Awaited>) { + await act(async () => { + await Promise.resolve(); + await setup.renderOnce(); + await Promise.resolve(); + await setup.renderOnce(); + }); +} + +/** Advance the injected watch debounce and settle its asynchronous soft reload. */ +async function advanceWatch( + setup: Awaited>, + watch: ReturnType, + milliseconds: number, +) { + await act(async () => { + watch.advanceBy(milliseconds); + await Promise.resolve(); + await Promise.resolve(); + }); + for (let attempt = 0; attempt < 12; attempt++) { + await flush(setup); + } +} + +describe("watched input lifecycle", () => { + test("an observer event reloads after the controlled debounce and preserves the resolved theme", async () => { + const dir = mkdtempSync(join(process.cwd(), ".hunk-watch-ui-")); + const left = join(dir, "before.ts"); + const right = join(dir, "after.ts"); + writeFileSync(left, "export const answer = 41;\n"); + writeFileSync(right, "export const answer = 42;\n"); + + const bootstrap = await loadAppBootstrap({ + kind: "diff", + left, + right, + options: { mode: "split", theme: "auto", watch: true }, + }); + bootstrap.initialThemeMode = "light"; + const watch = createWatchTestRuntime(); + const setup = await testRender(, { + width: 220, + height: 20, + }); + + try { + await flush(setup); + expect(watch.sources).toHaveLength(1); + await act(async () => { + await setup.mockInput.pressTab(); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("after"); + }); + await flush(setup); + writeFileSync(right, "export const answer = 42;\nexport const observed = true;\n"); + watch.setSignature("signature:1"); + watch.emit(); + + await advanceWatch(setup, watch, 199); + expect(setup.captureCharFrame()).not.toContain("observed"); + await advanceWatch(setup, watch, 1); + expect(setup.captureCharFrame()).toContain("observed"); + expect(setup.captureCharFrame()).toContain("filter:"); + expect(setup.captureCharFrame()).toContain("after"); + expect(watch.sources).toHaveLength(2); + expect(watch.sources[0]?.closeCount).toBe(1); + + const lightTheme = resolveTheme("auto", "light"); + const renderedBackgrounds = setup + .captureSpans() + .lines.flatMap((line) => line.spans) + .map((span) => capturedTestColorToHex(span.bg)?.toLowerCase()); + expect(renderedBackgrounds).toContain(lightTheme.panel.toLowerCase()); + } finally { + await act(async () => setup.renderer.destroy()); + rmSync(dir, { force: true, recursive: true }); + } + }); + + test("a sidecar event refreshes notes through the canonical reload pipeline", async () => { + const dir = mkdtempSync(join(process.cwd(), ".hunk-watch-sidecar-ui-")); + const left = join(dir, "before.ts"); + const right = join(dir, "after.ts"); + const sidecar = join(dir, "agent.json"); + writeFileSync(left, "export const answer = 41;\n"); + writeFileSync(right, "export const answer = 42;\n"); + writeFileSync(sidecar, JSON.stringify({ version: 1, files: [] })); + + const bootstrap = await loadAppBootstrap({ + kind: "diff", + left, + right, + options: { + agentContext: sidecar, + agentNotes: true, + mode: "stack", + watch: true, + }, + }); + const watch = createWatchTestRuntime(); + const setup = await testRender(, { + width: 140, + height: 24, + }); + + try { + await flush(setup); + expect(setup.captureCharFrame()).not.toContain("Watch rationale updated"); + writeFileSync( + sidecar, + JSON.stringify({ + version: 1, + files: [ + { + path: "after.ts", + annotations: [{ newRange: [1, 1], summary: "Watch rationale updated" }], + }, + ], + }), + ); + watch.setSignature("signature:sidecar"); + watch.emit(); + await advanceWatch(setup, watch, 200); + expect(setup.captureCharFrame()).toContain("Watch rationale updated"); + } finally { + await act(async () => setup.renderer.destroy()); + rmSync(dir, { force: true, recursive: true }); + } + }); + + test("replacement and unmount dispose observers once while late events remain inert", async () => { + const dir = mkdtempSync(join(process.cwd(), ".hunk-watch-dispose-ui-")); + const left = join(dir, "before.ts"); + const right = join(dir, "after.ts"); + writeFileSync(left, "before\n"); + writeFileSync(right, "first\n"); + const bootstrap = await loadAppBootstrap({ + kind: "diff", + left, + right, + options: { mode: "stack", watch: true }, + }); + const watch = createWatchTestRuntime(); + const setup = await testRender(, { + width: 120, + height: 20, + }); + + try { + await flush(setup); + const oldSource = watch.sources[0]!; + writeFileSync(right, "second\n"); + watch.setSignature("signature:replacement"); + watch.emit(0); + await advanceWatch(setup, watch, 200); + expect(watch.sources).toHaveLength(2); + expect(oldSource.closeCount).toBe(1); + + writeFileSync(right, "late\n"); + watch.setSignature("signature:late"); + oldSource.callbacks.onEvent(); + await advanceWatch(setup, watch, 200); + expect(setup.captureCharFrame()).not.toContain("late"); + expect(watch.sources).toHaveLength(2); + + await act(async () => setup.renderer.destroy()); + expect(oldSource.closeCount).toBe(1); + expect(watch.sources[1]?.closeCount).toBe(1); + oldSource.callbacks.onEvent(); + watch.sources[1]?.callbacks.onEvent(); + watch.advanceBy(10_000); + expect(watch.sources[1]?.closeCount).toBe(1); + } finally { + setup.renderer.destroy(); + rmSync(dir, { force: true, recursive: true }); + } + }); +}); diff --git a/src/ui/components/scrollbar/VerticalScrollbar.test.tsx b/src/ui/components/scrollbar/VerticalScrollbar.test.tsx index 4c026c1c..5c9b8f83 100644 --- a/src/ui/components/scrollbar/VerticalScrollbar.test.tsx +++ b/src/ui/components/scrollbar/VerticalScrollbar.test.tsx @@ -56,6 +56,7 @@ function createScrollBootstrapWithManyFiles(fileCount: number): AppBootstrap { } return { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -197,6 +198,7 @@ describe("Vertical scrollbar", () => { const after = before.replace("line15 = 15", "line15 = 115 // modified"); const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -261,6 +263,7 @@ describe("Vertical scrollbar", () => { const before = "export const a = 1;\n"; const after = "export const a = 2;\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -306,6 +309,7 @@ describe("Vertical scrollbar", () => { const after = before.replace("line50", "line50modified"); const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -372,6 +376,7 @@ describe("Vertical scrollbar", () => { const after = before.replace("line040", "line040modified"); const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -451,6 +456,7 @@ describe("Vertical scrollbar", () => { const after = before.replace("line08 = 8;", "line08 = 999; // modified"); const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, diff --git a/src/ui/hooks/useWatchedInput.ts b/src/ui/hooks/useWatchedInput.ts new file mode 100644 index 00000000..10b52102 --- /dev/null +++ b/src/ui/hooks/useWatchedInput.ts @@ -0,0 +1,71 @@ +import { useEffect, useRef } from "react"; +import { + createWatchController, + type WatchControllerClock, + type WatchEventSourceCallbacks, +} from "../../core/watchController"; +import { createWatchEventSource } from "../../core/watchObserver"; +import { computeWatchSignature } from "../../core/watch"; +import { resolveWatchPlan, type WatchPlan } from "../../core/watchPlan"; +import type { CliInput, ReloadContext } from "../../core/types"; + +export interface WatchedInputRuntime { + clock?: WatchControllerClock; + getSignature?: (input: CliInput, context: ReloadContext) => string; + resolvePlan?: (input: CliInput, context: ReloadContext) => WatchPlan | null; + createEventSource?: (plan: WatchPlan, callbacks: WatchEventSourceCallbacks) => { close(): void }; +} + +const defaultRuntime: WatchedInputRuntime = {}; + +/** Own the observer and controller lifecycle for one reloadable input. */ +export function useWatchedInput({ + enabled, + input, + reloadContext, + refresh, + runtime = defaultRuntime, +}: { + enabled: boolean; + input: CliInput; + reloadContext: ReloadContext; + refresh: () => void | Promise; + runtime?: WatchedInputRuntime; +}) { + const refreshRef = useRef(refresh); + refreshRef.current = refresh; + + useEffect(() => { + if (!enabled) return; + + const getSignature = runtime.getSignature ?? computeWatchSignature; + let plan: WatchPlan | null; + let initialSignature: string; + try { + plan = (runtime.resolvePlan ?? resolveWatchPlan)(input, reloadContext); + if (!plan) return; + initialSignature = + runtime.getSignature === undefined && reloadContext.initialWatchSignature !== undefined + ? reloadContext.initialWatchSignature + : getSignature(input, reloadContext); + } catch (error) { + console.error("Failed to initialize watch mode.", error); + return; + } + + const eventSourceFactory = runtime.createEventSource + ? (callbacks: WatchEventSourceCallbacks) => runtime.createEventSource!(plan, callbacks) + : createWatchEventSource(plan); + const controller = createWatchController({ + clock: runtime.clock, + createEventSource: eventSourceFactory, + getSignature: () => getSignature(input, reloadContext), + initialSignature, + pollOnly: plan.coverage === "poll-only", + refresh: () => refreshRef.current(), + reportError: (error) => console.error("Failed to auto-reload the current diff.", error), + }); + + return () => controller.close(); + }, [enabled, input, reloadContext, runtime]); +} diff --git a/test/helpers/app-bootstrap.ts b/test/helpers/app-bootstrap.ts index 44ccfec0..acd58d79 100644 --- a/test/helpers/app-bootstrap.ts +++ b/test/helpers/app-bootstrap.ts @@ -36,6 +36,7 @@ export function createTestVcsAppBootstrap({ title?: string; }): AppBootstrap { return { + reloadContext: { cwd: sourceLabel }, input: { kind: "vcs", staged: false, diff --git a/test/helpers/watchTest.ts b/test/helpers/watchTest.ts new file mode 100644 index 00000000..a9def420 --- /dev/null +++ b/test/helpers/watchTest.ts @@ -0,0 +1,88 @@ +import type { + WatchControllerClock, + WatchEventSourceCallbacks, +} from "../../src/core/watchController"; +import type { WatchedInputRuntime } from "../../src/ui/hooks/useWatchedInput"; + +interface ScheduledWatchTestTimer { + callback: () => void; + deadline: number; + id: number; +} + +/** Create a deterministic clock that advances chained watch-controller timers in deadline order. */ +export function createWatchTestClock() { + let now = 0; + let nextId = 1; + const timers = new Map(); + + const clock: WatchControllerClock = { + now: () => now, + setTimeout(callback, delayMs) { + const timer = { callback, deadline: now + delayMs, id: nextId++ }; + timers.set(timer.id, timer); + return timer.id; + }, + clearTimeout(handle) { + timers.delete(handle as number); + }, + }; + + return { + clock, + /** Advance through every timer due by the requested instant. */ + advanceBy(delayMs: number) { + const target = now + delayMs; + while (true) { + const due = [...timers.values()] + .filter((timer) => timer.deadline <= target) + .sort((left, right) => left.deadline - right.deadline || left.id - right.id)[0]; + if (!due) break; + timers.delete(due.id); + now = due.deadline; + due.callback(); + } + now = target; + }, + }; +} + +/** Build an injected watch runtime whose events and signatures are controlled by a test. */ +export function createWatchTestRuntime(initialSignature = "signature:0") { + const testClock = createWatchTestClock(); + let signature = initialSignature; + const sources: Array<{ + callbacks: WatchEventSourceCallbacks; + closeCount: number; + }> = []; + + const runtime: WatchedInputRuntime = { + clock: testClock.clock, + getSignature: () => signature, + resolvePlan: () => ({ + coverage: "hybrid", + targets: [], + }), + createEventSource: (_plan, callbacks) => { + const source = { callbacks, closeCount: 0 }; + sources.push(source); + return { + close() { + source.closeCount++; + }, + }; + }, + }; + + return { + runtime, + sources, + advanceBy: testClock.advanceBy, + setSignature(nextSignature: string) { + signature = nextSignature; + }, + emit(index = sources.length - 1) { + sources[index]?.callbacks.onEvent(); + }, + }; +} diff --git a/test/pty/harness.ts b/test/pty/harness.ts index a19e317f..acf85f19 100644 --- a/test/pty/harness.ts +++ b/test/pty/harness.ts @@ -38,6 +38,9 @@ function resolveBunExecutable() { } const bunExecutable = resolveBunExecutable(); +const explicitHunkExecutable = process.env.HUNK_TEST_EXECUTABLE + ? resolve(repoRoot, process.env.HUNK_TEST_EXECUTABLE) + : undefined; async function loadTuistory() { if (!process.versions.bun) { @@ -410,6 +413,39 @@ export function createPtyHarness() { return { dir, before, after }; } + /** Build direct files whose watched side can be replaced atomically during a PTY test. */ + function createWatchFilePair() { + const dir = makeTempDir("hunk-tuistory-watch-files-"); + const before = join(dir, "before.ts"); + const after = join(dir, "after.ts"); + + runGit(["init"], dir); + writeText(before, "export const watchedValue = 'before';\n"); + writeText(after, "export const watchedValue = 'initial change';\n"); + + return { dir, before, after }; + } + + /** Build a linked worktree with an existing tracked change ready for watch-mode startup. */ + function createLinkedWorktreeWatchFixture() { + const mainDir = makeTempDir("hunk-tuistory-watch-main-"); + const worktreeParent = makeTempDir("hunk-tuistory-watch-linked-"); + const worktreeDir = join(worktreeParent, "worktree"); + const relativeFile = "watched.ts"; + + runGit(["init"], mainDir); + runGit(["config", "user.name", "Pi"], mainDir); + runGit(["config", "user.email", "pi@example.com"], mainDir); + writeText(join(mainDir, relativeFile), "export const linkedValue = 'committed';\n"); + runGit(["add", relativeFile], mainDir); + runGit(["commit", "-m", "initial"], mainDir); + runGit(["worktree", "add", "--detach", worktreeDir, "HEAD"], mainDir); + + const trackedFile = join(worktreeDir, relativeFile); + writeText(trackedFile, "export const linkedValue = 'initial change';\n"); + return { mainDir, worktreeDir, trackedFile }; + } + function createGitRepoFixture(files: ChangedFileSpec[]) { const dir = makeTempDir("hunk-tuistory-repo-"); @@ -611,8 +647,12 @@ export function createPtyHarness() { return { dir, patchFile }; } - /** Build the source-run Hunk command so PTY tests can reuse it inside shell pipelines. */ + /** Build the configured Hunk command so PTY tests can reuse it inside shell pipelines. */ function buildHunkCommand(args: string[]) { + if (explicitHunkExecutable) { + return [shellQuote(explicitHunkExecutable), ...args.map(shellQuote)].join(" "); + } + return [ shellQuote(bunExecutable), "run", @@ -632,8 +672,10 @@ export function createPtyHarness() { const { launchTerminal } = await loadTuistory(); return launchTerminal({ - command: bunExecutable, - args: ["run", sourceEntrypoint, "--", ...options.args], + command: explicitHunkExecutable ?? bunExecutable, + args: explicitHunkExecutable + ? options.args + : ["run", sourceEntrypoint, "--", ...options.args], cwd: options.cwd ?? repoRoot, cols: options.cols ?? 140, rows: options.rows ?? 24, @@ -732,6 +774,7 @@ export function createPtyHarness() { createExpandableContextFilePair, createCrossFileHunkNavigationRepoFixture, createDeletionOnlyFilePair, + createLinkedWorktreeWatchFixture, createLongWrapFilePair, createMultiHunkFilePair, createPagerPatchFixture, @@ -739,6 +782,7 @@ export function createPtyHarness() { createScrollableFilePair, createSidebarJumpRepoFixture, createTwoFileRepoFixture, + createWatchFilePair, createWideCharacterFilePair, launchHunk, launchHunkWithFileBackedStdin, diff --git a/test/pty/watch.test.ts b/test/pty/watch.test.ts new file mode 100644 index 00000000..e46c10c7 --- /dev/null +++ b/test/pty/watch.test.ts @@ -0,0 +1,72 @@ +import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test"; +import { renameSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { createPtyHarness } from "./harness"; + +const harness = createPtyHarness(); + +/** Give source and compiled PTY startup enough headroom while keeping refresh waits event-bounded. */ +setDefaultTimeout(30_000); + +afterEach(() => { + harness.cleanup(); +}); + +describe("PTY watch mode", () => { + test("passively refreshes direct files after an atomic save", async () => { + const fixture = harness.createWatchFilePair(); + const session = await harness.launchHunk({ + args: ["diff", fixture.before, fixture.after, "--watch", "--mode", "stack"], + cwd: fixture.dir, + cols: 120, + rows: 16, + }); + + try { + const initial = await session.waitForText(/watchedValue = 'initial change'/, { + timeout: 15_000, + }); + expect(initial).not.toContain("atomic replacement"); + + const replacement = join(dirname(fixture.after), "after-replacement.ts"); + writeFileSync(replacement, "export const watchedValue = 'atomic replacement';\n"); + renameSync(replacement, fixture.after); + + const refreshed = await session.waitForText(/watchedValue = 'atomic replacement'/, { + timeout: 5_000, + }); + expect(refreshed).not.toContain("watchedValue = 'initial change'"); + } finally { + session.close(); + } + }); + + test("passively refreshes a tracked file in a linked Git worktree", async () => { + const fixture = harness.createLinkedWorktreeWatchFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--watch", "--mode", "stack"], + cwd: fixture.worktreeDir, + cols: 120, + rows: 16, + }); + + try { + const initial = await session.waitForText(/linkedValue = 'initial change'/, { + timeout: 15_000, + }); + expect(initial).not.toContain("passive worktree refresh"); + + writeFileSync( + fixture.trackedFile, + "export const linkedValue = 'passive worktree refresh';\n", + ); + + const refreshed = await session.waitForText(/linkedValue = 'passive worktree refresh'/, { + timeout: 5_000, + }); + expect(refreshed).not.toContain("linkedValue = 'initial change'"); + } finally { + session.close(); + } + }); +});