From fd26b89b581eb25c86ace5d0c9c8fcf40c752603 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 21:25:47 -0400 Subject: [PATCH 01/16] fix(watch): anchor reload signatures to source context --- benchmarks/highlight-prefetch.ts | 1 + benchmarks/large-stream-fixture.ts | 2 + src/core/loaders.test.ts | 59 +++++++++++++++ src/core/loaders.ts | 32 ++++++-- src/core/startup.test.ts | 1 + src/core/types.ts | 7 ++ src/core/vcs/git.test.ts | 51 ++++++------- src/core/vcs/git.ts | 27 ++++--- src/core/vcs/jj.test.ts | 6 ++ src/core/vcs/jj.ts | 8 +- src/core/vcs/sl.test.ts | 8 ++ src/core/watch.test.ts | 75 ++++++++++++------- src/core/watch.ts | 30 ++++++-- src/hunk-session/sessionFileBounds.test.ts | 1 + src/hunk-session/sessionFileBounds.ts | 2 +- src/hunk-session/sessionRegistration.test.ts | 1 + src/ui/App.tsx | 6 +- src/ui/AppHost.interactions.test.tsx | 6 ++ src/ui/AppHost.tsx | 4 +- .../scrollbar/VerticalScrollbar.test.tsx | 6 ++ test/helpers/app-bootstrap.ts | 1 + 21 files changed, 243 insertions(+), 91 deletions(-) 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/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 3921ef26..fdb09e16 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -377,8 +377,15 @@ export type ParsedCliInput = | MarkupRenderCommandInput | MarkupGuideCommandInput; +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..8e4400dd 100644 --- a/src/core/vcs/git.test.ts +++ b/src/core/vcs/git.test.ts @@ -158,35 +158,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..ef23e6cc 100644 --- a/src/core/vcs/git.ts +++ b/src/core/vcs/git.ts @@ -306,12 +306,19 @@ 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))}`, - ); + watchSignature(input, { cwd, gitExecutable = "git" }) { + const trackedPatch = runGitText({ + input, + args: buildGitDiffArgs(input), + cwd, + gitExecutable, + }); + const repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable }); + const untrackedSignatures = listGitUntrackedFiles(input, { + cwd, + repoRoot, + gitExecutable, + }).map((filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`); return [trackedPatch, ...untrackedSignatures].join("\n---\n"); }, }, @@ -341,8 +348,8 @@ export const GitVcsAdapter: VcsAdapter = { ), }; }, - watchSignature(input) { - return runGitText({ input, args: buildGitShowArgs(input) }); + watchSignature(input, { cwd, gitExecutable = "git" }) { + return runGitText({ input, args: buildGitShowArgs(input), cwd, gitExecutable }); }, }, "stash-show": { @@ -371,8 +378,8 @@ export const GitVcsAdapter: VcsAdapter = { ), }; }, - watchSignature(input) { - return runGitText({ input, args: buildGitStashShowArgs(input) }); + watchSignature(input, { cwd, gitExecutable = "git" }) { + return runGitText({ input, args: buildGitStashShowArgs(input), cwd, gitExecutable }); }, }, }, 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/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/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 b4536a3b..af2b8879 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -679,7 +679,9 @@ export function App({ let lastSignature: string; try { - lastSignature = computeWatchSignature(bootstrap.input); + lastSignature = + bootstrap.reloadContext.initialWatchSignature ?? + computeWatchSignature(bootstrap.input, bootstrap.reloadContext); } catch (error) { console.error("Failed to initialize watch mode.", error); return; @@ -693,7 +695,7 @@ export function App({ polling = true; try { - const nextSignature = computeWatchSignature(bootstrap.input); + const nextSignature = computeWatchSignature(bootstrap.input, bootstrap.reloadContext); if (nextSignature !== lastSignature) { lastSignature = nextSignature; refreshing = true; diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index ac4bc961..89cd03f5 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( { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2270,6 +2272,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2328,6 +2331,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2401,6 +2405,7 @@ describe("App interactions", () => { ).join("\n") + "\n"; const bootstrap: AppBootstrap = { + reloadContext: { cwd: process.cwd() }, input: { kind: "vcs", staged: false, @@ -2462,6 +2467,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 8abb26c6..e6d381ca 100644 --- a/src/ui/AppHost.tsx +++ b/src/ui/AppHost.tsx @@ -31,9 +31,7 @@ export function AppHost({ 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, 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/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, From f1653c96619b03db8535bebd362f946e0065dce3 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 21:30:02 -0400 Subject: [PATCH 02/16] feat(watch): define backend-neutral input plans --- src/core/watchPlan.test.ts | 236 +++++++++++++++++++++++++++++++++++++ src/core/watchPlan.ts | 134 +++++++++++++++++++++ 2 files changed, 370 insertions(+) create mode 100644 src/core/watchPlan.test.ts create mode 100644 src/core/watchPlan.ts diff --git a/src/core/watchPlan.test.ts b/src/core/watchPlan.test.ts new file mode 100644 index 00000000..e20fc4c6 --- /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 adapter placeholders for VCS inputs and adds file sidecars", () => { + const vcsInput = { kind: "vcs", staged: false, options: {} } satisfies CliInput; + const showInput = { + kind: "show", + options: { 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..4a48970f --- /dev/null +++ b/src/core/watchPlan.ts @@ -0,0 +1,134 @@ +import { posix, win32 } from "node:path"; +import { normalizePathForOS } from "../lib/osPath"; +import type { CliInput } from "./types"; + +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; +} + +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"; + + 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": + // Adapter-specific directory trees will replace this polling placeholder. + coverage = "poll-only"; + break; + } + + if (input.options.agentContext) { + fileTargets.push({ path: input.options.agentContext, source: "sidecar" }); + coverage = "hybrid"; + } + + return { + coverage, + targets: groupFileTargets(fileTargets, context), + }; +} From d809c856a19e223944e8ece32482860860d2b8e6 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 21:37:37 -0400 Subject: [PATCH 03/16] feat(watch): derive operation-sensitive Git plans --- src/core/git.test.ts | 25 +++++++++++- src/core/git.ts | 55 ++++++++++++++++++++++--- src/core/vcs/git.test.ts | 84 +++++++++++++++++++++++++++++++++++++- src/core/vcs/git.ts | 81 ++++++++++++++++++++++++++++++++++-- src/core/vcs/index.test.ts | 45 ++++++++++++++++++++ src/core/vcs/index.ts | 14 +++++++ src/core/vcs/types.ts | 2 + src/core/watchPlan.test.ts | 6 +-- src/core/watchPlan.ts | 17 ++++++-- 9 files changed, 311 insertions(+), 18 deletions(-) diff --git a/src/core/git.test.ts b/src/core/git.test.ts index 06f6a364..2602d25c 100644 --- a/src/core/git.test.ts +++ b/src/core/git.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -7,6 +7,7 @@ import { buildGitStashShowArgs, buildGitStatusArgs, resolveGitDiffEndpoints, + resolveGitMetadata, runGitText, } from "./git"; import type { VcsDiffCommandInput } from "./types"; @@ -112,6 +113,28 @@ describe("git command helpers", () => { }); }); +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..0a170c3e 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 { @@ -363,6 +364,7 @@ function runGitCommand({ args, cwd = process.cwd(), gitExecutable = "git", + preventOptionalLocks = false, acceptedExitCodes = [0], }: RunGitCommandOptions): RunGitCommandResult { let proc: ReturnType; @@ -373,6 +375,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 +484,10 @@ function isWorkingTreeGitDiffInput( cwd = process.cwd(), gitExecutable = "git", repoRoot, - }: Pick & { repoRoot?: string } = {}, + preventOptionalLocks = false, + }: Pick & { + repoRoot?: string; + } = {}, ) { if (input.staged) { return false; @@ -502,6 +508,7 @@ function isWorkingTreeGitDiffInput( args: ["rev-parse", "--revs-only", input.range], cwd, gitExecutable, + preventOptionalLocks, }) .split("\n") .map((line) => line.trim()) @@ -518,7 +525,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); } @@ -569,9 +578,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 +590,7 @@ export function listGitUntrackedFiles( args: buildGitStatusArgs(input), cwd, gitExecutable, + preventOptionalLocks, }); const untrackedFiles = parseUntrackedFilePaths(statusText); @@ -587,7 +598,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 +651,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/vcs/git.test.ts b/src/core/vcs/git.test.ts index 8e4400dd..b255e80c 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,88 @@ 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"); + git(repo, "add", "file.txt"); + git(repo, "commit", "-m", "initial"); + + 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 : []).toContain( + join(repo, ".git"), + ); + 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"); diff --git a/src/core/vcs/git.ts b/src/core/vcs/git.ts index ef23e6cc..4ffb47fd 100644 --- a/src/core/vcs/git.ts +++ b/src/core/vcs/git.ts @@ -1,5 +1,5 @@ import fs from "node:fs"; -import { dirname, join, resolve } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve } from "node:path"; import { buildGitDiffArgs, buildGitDiffNumstatArgs, @@ -10,6 +10,7 @@ import { resolveGitColorMovedOptions, resolveGitCommitRef, resolveGitDiffEndpoints, + resolveGitMetadata, resolveGitRepoRoot, runGitText, runGitUntrackedFileDiffText, @@ -28,6 +29,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 +250,50 @@ 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: [join(metadata.repoRoot, ".git")], + 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,18 +352,27 @@ export const GitVcsAdapter: VcsAdapter = { ], }; }, + 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 repoRoot = resolveGitRepoRoot(input, { cwd, gitExecutable }); const untrackedSignatures = listGitUntrackedFiles(input, { cwd, repoRoot, gitExecutable, + preventOptionalLocks: true, }).map((filePath) => `untracked:${statSignature(join(repoRoot, filePath))}`); return [trackedPatch, ...untrackedSignatures].join("\n---\n"); }, @@ -348,8 +403,17 @@ export const GitVcsAdapter: VcsAdapter = { ), }; }, + 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 }); + return runGitText({ + input, + args: buildGitShowArgs(input), + cwd, + gitExecutable, + preventOptionalLocks: true, + }); }, }, "stash-show": { @@ -378,8 +442,17 @@ export const GitVcsAdapter: VcsAdapter = { ), }; }, + 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 }); + 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/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/watchPlan.test.ts b/src/core/watchPlan.test.ts index e20fc4c6..d30626ac 100644 --- a/src/core/watchPlan.test.ts +++ b/src/core/watchPlan.test.ts @@ -192,11 +192,11 @@ describe("resolveWatchPlan", () => { expect(resolveWatchPlan(input, { cwd, platform: "linux" })).toBeNull(); }); - test("uses poll-only adapter placeholders for VCS inputs and adds file sidecars", () => { - const vcsInput = { kind: "vcs", staged: false, options: {} } satisfies CliInput; + 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: { agentContext: "agent.json" }, + options: { vcs: "jj", agentContext: "agent.json" }, } satisfies CliInput; expect(resolveWatchPlan(vcsInput, { cwd, platform: "linux" })).toEqual({ diff --git a/src/core/watchPlan.ts b/src/core/watchPlan.ts index 4a48970f..fae84a48 100644 --- a/src/core/watchPlan.ts +++ b/src/core/watchPlan.ts @@ -1,6 +1,7 @@ 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"; @@ -28,6 +29,7 @@ export interface WatchPlan { export interface WatchPlanContext { cwd: string; platform?: NodeJS.Platform; + gitExecutable?: string; } interface FileTarget { @@ -99,6 +101,7 @@ export function resolveWatchPlan(input: CliInput, context: WatchPlanContext): Wa const fileTargets: FileTarget[] = []; let coverage: WatchPlan["coverage"] = "hybrid"; + let adapterTargets: WatchTarget[] = []; switch (input.kind) { case "diff": @@ -116,10 +119,16 @@ export function resolveWatchPlan(input: CliInput, context: WatchPlanContext): Wa break; case "vcs": case "show": - case "stash-show": - // Adapter-specific directory trees will replace this polling placeholder. - coverage = "poll-only"; + 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) { @@ -129,6 +138,6 @@ export function resolveWatchPlan(input: CliInput, context: WatchPlanContext): Wa return { coverage, - targets: groupFileTargets(fileTargets, context), + targets: [...adapterTargets, ...groupFileTargets(fileTargets, context)], }; } From 9315af9f0bc861fb8e020b4897407643c55a60aa Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 21:42:04 -0400 Subject: [PATCH 04/16] feat(watch): coordinate hybrid refresh checks --- src/core/watchController.test.ts | 479 +++++++++++++++++++++++++++++++ src/core/watchController.ts | 262 +++++++++++++++++ 2 files changed, 741 insertions(+) create mode 100644 src/core/watchController.test.ts create mode 100644 src/core/watchController.ts diff --git a/src/core/watchController.test.ts b/src/core/watchController.test.ts new file mode 100644 index 00000000..ab1ba439 --- /dev/null +++ b/src/core/watchController.test.ts @@ -0,0 +1,479 @@ +import { describe, expect, test } from "bun:test"; + +import { + createWatchController, + 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); + }, + 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("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(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(); + let oldChecks = 0; + let newChecks = 0; + const oldController = createWatchController({ + initialSignature: "old", + clock, + createEventSource: oldSource.create, + getSignature: () => (++oldChecks, "old"), + refresh: () => {}, + }); + oldController.close(); + createWatchController({ + initialSignature: "new", + clock, + createEventSource: newSource.create, + getSignature: () => (++newChecks, "new"), + refresh: () => {}, + }); + + oldSource.event(); + newSource.event(); + clock.advance(200); + await settle(); + expect(oldChecks).toBe(0); + expect(newChecks).toBe(1); + }); +}); diff --git a/src/core/watchController.ts b/src/core/watchController.ts new file mode 100644 index 00000000..08de7ccb --- /dev/null +++ b/src/core/watchController.ts @@ -0,0 +1,262 @@ +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; +} + +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; +} + +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); +} + +/** 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 state: WatchControllerState = { + phase: "starting", + dirty: false, + degraded: Boolean(options.pollOnly), + appliedSignature: options.initialSignature, + }; + let eventSource: WatchEventSource | undefined; + let timer: unknown; + let timerDeadline: 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 = [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"; + + /** 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(); + }; + + /** Consume a due debounce, maximum-delay, or safety deadline as one check. */ + function onTimer() { + timer = undefined; + timerDeadline = undefined; + if (state.phase === "closed") return; + const now = clock.now(); + 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") 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(); + }; + + /** Degrade only for watcher resource exhaustion; other source errors stay nonfatal. */ + const onSourceError = (error: unknown) => { + if (state.phase === "closed") return; + const code = getErrorCode(error); + if (code === "ENOSPC" || code === "EMFILE") { + state.degraded = true; + 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) { + try { + eventSource = options.createEventSource({ onEvent, onError: onSourceError }); + } catch (error) { + 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(); + eventSource?.close(); + eventSource = undefined; + }, + /** Expose a snapshot for diagnostics without allowing state mutation. */ + getState() { + return { ...state }; + }, + }; +} From 8868824bc5c8a7cfb79d85809270c2f33e33f5e6 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 21:47:08 -0400 Subject: [PATCH 05/16] feat(watch): add Chokidar runtime observer --- bun.lock | 5 + package.json | 1 + src/core/watchController.test.ts | 20 +++ src/core/watchController.ts | 17 ++- src/core/watchObserver.fs.test.ts | 236 ++++++++++++++++++++++++++++++ src/core/watchObserver.ts | 129 ++++++++++++++++ 6 files changed, 407 insertions(+), 1 deletion(-) create mode 100644 src/core/watchObserver.fs.test.ts create mode 100644 src/core/watchObserver.ts diff --git a/bun.lock b/bun.lock index 35733868..89cb794c 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/package.json b/package.json index 74fb62cd..2023fe6c 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,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/watchController.test.ts b/src/core/watchController.test.ts index ab1ba439..41f40870 100644 --- a/src/core/watchController.test.ts +++ b/src/core/watchController.test.ts @@ -83,6 +83,9 @@ function fakeSource() { error(error: unknown) { callbacks.onError(error); }, + ready() { + callbacks.onReady?.(); + }, get closes() { return closes; }, @@ -306,6 +309,23 @@ describe("createWatchController", () => { 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("runs a healthy safety check without an event", async () => { const clock = new FakeWatchClock(); let checks = 0; diff --git a/src/core/watchController.ts b/src/core/watchController.ts index 08de7ccb..2d8c8f47 100644 --- a/src/core/watchController.ts +++ b/src/core/watchController.ts @@ -19,6 +19,7 @@ export interface WatchEventSource { export interface WatchEventSourceCallbacks { onEvent(): void; onError(error: unknown): void; + onReady?(): void; } export interface WatchControllerOptions { @@ -213,6 +214,16 @@ export function createWatchController(options: WatchControllerOptions): WatchCon schedule(); }; + /** Close the startup scan race with an immediate signature check after watcher readiness. */ + const onSourceReady = () => { + if (state.phase === "closed") return; + 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") return; @@ -232,7 +243,11 @@ export function createWatchController(options: WatchControllerOptions): WatchCon safetyDeadline = clock.now() + safetyInterval(); if (options.createEventSource && !options.pollOnly) { try { - eventSource = options.createEventSource({ onEvent, onError: onSourceError }); + eventSource = options.createEventSource({ + onEvent, + onError: onSourceError, + onReady: onSourceReady, + }); } catch (error) { state.degraded = true; safetyDeadline = clock.now() + degradedCheckMs; diff --git a/src/core/watchObserver.fs.test.ts b/src/core/watchObserver.fs.test.ts new file mode 100644 index 00000000..08346441 --- /dev/null +++ b/src/core/watchObserver.fs.test.ts @@ -0,0 +1,236 @@ +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 pendingEvents = 0; + const waiters: Array<() => void> = []; + const observer = createWatchObserver(plan, { + onEvent() { + 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, + 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("Chokidar 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("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"], + }, + ], + }; + + await expectNoRefresh( + plan, + () => readFileSync(worktreeFile, "utf8"), + () => writeFile(metadata, "after"), + ); + }); + + 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.ts b/src/core/watchObserver.ts new file mode 100644 index 00000000..02a156df --- /dev/null +++ b/src/core/watchObserver.ts @@ -0,0 +1,129 @@ +import { resolve } from "node:path"; +import { watch, type FSWatcher } from "chokidar"; + +import type { WatchEventSource, WatchEventSourceCallbacks } from "./watchController"; +import type { WatchPlan, WatchTarget } from "./watchPlan"; + +export interface WatchObserver extends WatchEventSource { + ready: Promise; + closed: Promise; +} + +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; +} + +/** Return whether a path is one ignored root or one of its descendants. */ +function isWithinRoot(path: string, root: string) { + if (path === root) return true; + const separator = process.platform === "win32" ? "\\" : "/"; + return path.startsWith(`${root}${separator}`); +} + +/** Create the Chokidar watcher for one neutral target and its event filter. */ +function watchTarget(target: WatchTarget, onEvent: () => void) { + if (target.kind === "directory-entries") { + const entries = new Set(target.entries.map(normalizedPath)); + const watcher = watch(target.directory, { ...WATCH_OPTIONS, depth: 0 }); + watcher.on("all", (_event, path) => { + if (entries.has(normalizedPath(path))) onEvent(); + }); + return watcher; + } + + const ignoredRoots = target.ignoredRoots.map(normalizedPath); + const watcher = watch(target.directory, { + ...WATCH_OPTIONS, + ignored: (path) => { + const normalized = normalizedPath(path); + return ignoredRoots.some((root) => isWithinRoot(normalized, root)); + }, + }); + watcher.on("all", () => onEvent()); + return watcher; +} + +/** Observe a hybrid watch plan with Chokidar and expose bounded lifecycle promises for callers. */ +export function createWatchObserver( + plan: WatchPlan, + callbacks: WatchEventSourceCallbacks, +): WatchObserver { + let isClosed = false; + const watchers: FSWatcher[] = []; + 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 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 watcher = watchTarget(target, onEvent); + watchers.push(watcher); + watcher.once("ready", markReady); + watcher.on("error", (error) => { + if (!isClosed) callbacks.onError(error); + }); + } + } catch (error) { + isClosed = true; + resolveReady(); + void Promise.all(watchers.map((watcher) => watcher.close())).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 Promise.all(watchers.map((watcher) => watcher.close())).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); +} From e00e77bc19da46648cfe0783751cd71d3a83db59 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 21:54:50 -0400 Subject: [PATCH 06/16] feat(watch): wire observer lifecycle into UI --- src/ui/App.tsx | 64 ++------- src/ui/AppHost.interactions.test.tsx | 100 -------------- src/ui/AppHost.tsx | 4 + src/ui/AppHost.watch.test.tsx | 192 +++++++++++++++++++++++++++ src/ui/hooks/useWatchedInput.ts | 71 ++++++++++ test/helpers/watchTest.ts | 88 ++++++++++++ 6 files changed, 366 insertions(+), 153 deletions(-) create mode 100644 src/ui/AppHost.watch.test.tsx create mode 100644 src/ui/hooks/useWatchedInput.ts create mode 100644 test/helpers/watchTest.ts diff --git a/src/ui/App.tsx b/src/ui/App.tsx index af2b8879..8571e377 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -17,7 +17,7 @@ import type { PersistedViewPreferences, 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 { ConfirmDialog, confirmDialogHeight } from "./components/chrome/ConfirmDialog"; @@ -35,6 +35,7 @@ import { useAppKeyboardShortcuts } from "./hooks/useAppKeyboardShortcuts"; import { useHunkSessionBridge } from "./hooks/useHunkSessionBridge"; import { useMenuController } from "./hooks/useMenuController"; import { useReviewController, type AgentNoteGeometrySnapshot } from "./hooks/useReviewController"; +import { useWatchedInput, type WatchedInputRuntime } from "./hooks/useWatchedInput"; import { agentNoteMarkupWidth } from "./lib/agentNoteGeometry"; import { buildAppMenus } from "./lib/appMenus"; import { fileRowId } from "./lib/ids"; @@ -106,6 +107,7 @@ export function App({ noticeText, onQuit = () => process.exit(0), onReloadSession, + watchRuntime, }: { bootstrap: AppBootstrap; hostClient?: HunkSessionBrokerClient; @@ -115,6 +117,7 @@ export function App({ nextInput: CliInput, options?: { resetApp?: boolean; sourcePath?: string }, ) => Promise; + watchRuntime?: WatchedInputRuntime; }) { const SIDEBAR_MIN_WIDTH = 22; const DIFF_MIN_WIDTH = 48; @@ -668,58 +671,13 @@ export function App({ triggerRefreshCurrentInput, ]); - useEffect(() => { - if (!watchEnabled) { - return; - } - - let cancelled = false; - let polling = false; - let refreshing = false; - let lastSignature: string; - - try { - lastSignature = - bootstrap.reloadContext.initialWatchSignature ?? - computeWatchSignature(bootstrap.input, bootstrap.reloadContext); - } 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, bootstrap.reloadContext); - 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, + }); /** Save current view preferences to user config and then leave the app. */ const saveViewPreferencesAndQuit = useCallback(() => { diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 89cd03f5..2e7554c0 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -1674,106 +1674,6 @@ describe("App interactions", () => { } }); - 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"; diff --git a/src/ui/AppHost.tsx b/src/ui/AppHost.tsx index e6d381ca..4ed58a9e 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,11 +23,13 @@ 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); @@ -97,6 +100,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/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/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(); + }, + }; +} From a69ede4467bab3e6abd6a9f9d1762c8bfb83e312 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 22:05:43 -0400 Subject: [PATCH 07/16] test(watch): verify passive PTY refreshes --- .github/workflows/ci.yml | 5 +++ .github/workflows/pr-ci.yml | 5 +++ test/pty/harness.ts | 50 ++++++++++++++++++++++++-- test/pty/watch.test.ts | 72 +++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 test/pty/watch.test.ts 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/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(); + } + }); +}); From 81b3477bf523d0b17c60a3eb25626fff0c498ac9 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 22:08:12 -0400 Subject: [PATCH 08/16] docs(watch): clarify evented refresh behavior --- .changeset/calm-files-watch.md | 5 +++++ README.md | 2 ++ nix/bun.lock.nix | 8 ++++++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/calm-files-watch.md 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/README.md b/README.md index 4a72fdc1..04251bdb 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/nix/bun.lock.nix b/nix/bun.lock.nix index 41d6dcc8..8abeff84 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=="; From 1f93941953ea6f30eb6c9af41e0c282ea818b4a9 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Sun, 12 Jul 2026 22:21:28 -0400 Subject: [PATCH 09/16] fix(watch): release observers after resource exhaustion --- src/core/watchController.test.ts | 1 + src/core/watchController.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/core/watchController.test.ts b/src/core/watchController.test.ts index 41f40870..ac7fc11c 100644 --- a/src/core/watchController.test.ts +++ b/src/core/watchController.test.ts @@ -401,6 +401,7 @@ describe("createWatchController", () => { 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); } }); diff --git a/src/core/watchController.ts b/src/core/watchController.ts index 2d8c8f47..63d376a2 100644 --- a/src/core/watchController.ts +++ b/src/core/watchController.ts @@ -230,6 +230,8 @@ export function createWatchController(options: WatchControllerOptions): WatchCon const code = getErrorCode(error); if (code === "ENOSPC" || code === "EMFILE") { state.degraded = true; + eventSource?.close(); + eventSource = undefined; safetyDeadline = Math.min( safetyDeadline ?? Number.POSITIVE_INFINITY, clock.now() + degradedCheckMs, From d27a9a5af06733d02aad1fba2f9f3f0501ef0f41 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Mon, 13 Jul 2026 15:10:04 -0400 Subject: [PATCH 10/16] fix(watch): prune Git-ignored worktree roots --- src/core/git.test.ts | 109 ++++++++++++++++++++++++++++++++++++++- src/core/git.ts | 50 ++++++++++++++++++ src/core/vcs/git.test.ts | 19 ++++++- src/core/vcs/git.ts | 12 ++++- 4 files changed, 185 insertions(+), 5 deletions(-) diff --git a/src/core/git.test.ts b/src/core/git.test.ts index 2602d25c..555abfd4 100644 --- a/src/core/git.test.ts +++ b/src/core/git.test.ts @@ -1,11 +1,14 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, realpathSync, 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, @@ -96,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({ @@ -113,6 +139,85 @@ 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-"); diff --git a/src/core/git.ts b/src/core/git.ts index 0a170c3e..09e991b5 100644 --- a/src/core/git.ts +++ b/src/core/git.ts @@ -151,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.) @@ -540,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); diff --git a/src/core/vcs/git.test.ts b/src/core/vcs/git.test.ts index b255e80c..ea8aeb7b 100644 --- a/src/core/vcs/git.test.ts +++ b/src/core/vcs/git.test.ts @@ -153,8 +153,11 @@ describe("GitVcsAdapter", () => { 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"); - git(repo, "add", "file.txt"); + 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!( @@ -165,9 +168,21 @@ describe("GitVcsAdapter", () => { const worktreeTarget = unstaged.targets.find( (target) => target.kind === "directory-tree" && target.directory === repo, ); - expect(worktreeTarget?.kind === "directory-tree" ? worktreeTarget.ignoredRoots : []).toContain( + 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", diff --git a/src/core/vcs/git.ts b/src/core/vcs/git.ts index 4ffb47fd..aca9dc0e 100644 --- a/src/core/vcs/git.ts +++ b/src/core/vcs/git.ts @@ -5,6 +5,7 @@ import { buildGitDiffNumstatArgs, buildGitShowArgs, buildGitStashShowArgs, + listGitIgnoredDirectoryRoots, listGitUntrackedFiles, normalizeUntrackedPatchHeaders, resolveGitColorMovedOptions, @@ -286,7 +287,16 @@ function buildGitWatchPlan( targets.push({ kind: "directory-tree", directory: metadata.repoRoot, - ignoredRoots: [join(metadata.repoRoot, ".git")], + ignoredRoots: [ + ...new Set([ + join(metadata.repoRoot, ".git"), + ...listGitIgnoredDirectoryRoots(operation.input, { + cwd: metadata.repoRoot, + repoRoot: metadata.repoRoot, + gitExecutable, + }), + ]), + ], sources: ["worktree"], }); } From 38b4201384d61a4c060df433dc4e890eb9cf1671 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Mon, 13 Jul 2026 15:22:45 -0400 Subject: [PATCH 11/16] fix(watch): bound recursive watchers on macOS and Windows --- .changeset/native-recursive-watch.md | 5 + src/core/watchObserver.fs.test.ts | 54 +++++++- src/core/watchObserver.test.ts | 175 ++++++++++++++++++++++++ src/core/watchObserver.ts | 194 ++++++++++++++++++++++----- 4 files changed, 385 insertions(+), 43 deletions(-) create mode 100644 .changeset/native-recursive-watch.md create mode 100644 src/core/watchObserver.test.ts 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/src/core/watchObserver.fs.test.ts b/src/core/watchObserver.fs.test.ts index 08346441..05ceacd1 100644 --- a/src/core/watchObserver.fs.test.ts +++ b/src/core/watchObserver.fs.test.ts @@ -48,10 +48,12 @@ function entriesPlan(directory: string, entries: string[]): WatchPlan { /** 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++; @@ -68,6 +70,9 @@ async function startObserver(plan: WatchPlan) { return { observer, + get eventCount() { + return eventCount; + }, nextEvent() { if (pendingEvents > 0) { pendingEvents--; @@ -126,7 +131,7 @@ async function expectNoRefresh( expect(refreshes).toBe(0); } -describe("Chokidar watch observer", () => { +describe("filesystem watch observer", () => { test("does not create an event-source factory for poll-only plans", () => { expect(createWatchEventSource({ coverage: "poll-only", targets: [] })).toBeUndefined(); }); @@ -195,6 +200,45 @@ describe("Chokidar watch observer", () => { 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"); @@ -215,11 +259,9 @@ describe("Chokidar watch observer", () => { ], }; - await expectNoRefresh( - plan, - () => readFileSync(worktreeFile, "utf8"), - () => writeFile(metadata, "after"), - ); + const source = await startObserver(plan); + await writeFile(metadata, "after"); + await expectNoEvent(source.nextEvent()); }); test("close releases handles and suppresses later events", async () => { diff --git a/src/core/watchObserver.test.ts b/src/core/watchObserver.test.ts new file mode 100644 index 00000000..836eeaf0 --- /dev/null +++ b/src/core/watchObserver.test.ts @@ -0,0 +1,175 @@ +import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; + +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.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 index 02a156df..480a164c 100644 --- a/src/core/watchObserver.ts +++ b/src/core/watchObserver.ts @@ -1,14 +1,44 @@ -import { resolve } from "node:path"; -import { watch, type FSWatcher } from "chokidar"; +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 { WatchPlan, WatchTarget } from "./watchPlan"; +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, @@ -23,43 +53,134 @@ function normalizedPath(path: string) { return process.platform === "win32" ? absolute.toLowerCase() : absolute; } -/** Return whether a path is one ignored root or one of its descendants. */ -function isWithinRoot(path: string, root: string) { - if (path === root) return true; - const separator = process.platform === "win32" ? "\\" : "/"; - return path.startsWith(`${root}${separator}`); +/** 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), + }; } -/** Create the Chokidar watcher for one neutral target and its event filter. */ -function watchTarget(target: WatchTarget, onEvent: () => void) { - if (target.kind === "directory-entries") { - const entries = new Set(target.entries.map(normalizedPath)); - const watcher = watch(target.directory, { ...WATCH_OPTIONS, depth: 0 }); - watcher.on("all", (_event, path) => { - if (entries.has(normalizedPath(path))) onEvent(); - }); - return watcher; - } +/** 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), + }; +}; - const ignoredRoots = target.ignoredRoots.map(normalizedPath); - const watcher = watch(target.directory, { +/** 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) => { - const normalized = normalizedPath(path); - return ignoredRoots.some((root) => isWithinRoot(normalized, root)); - }, + ignored: (path) => isIgnored(path), }); - watcher.on("all", () => onEvent()); - return watcher; + 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 with Chokidar and expose bounded lifecycle promises for callers. */ +/** 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 watchers: FSWatcher[] = []; + const registrations: WatchRegistration[] = []; let resolveReady!: () => void; let resolveClosed!: () => void; const ready = new Promise((resolvePromise) => { @@ -69,6 +190,8 @@ export function createWatchObserver( 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(); @@ -84,17 +207,17 @@ export function createWatchObserver( try { for (const target of plan.targets) { - const watcher = watchTarget(target, onEvent); - watchers.push(watcher); - watcher.once("ready", markReady); - watcher.on("error", (error) => { + 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 Promise.all(watchers.map((watcher) => watcher.close())).then(resolveClosed, resolveClosed); + void closeRegistrations(registrations).then(resolveClosed, resolveClosed); throw error; } @@ -114,10 +237,7 @@ export function createWatchObserver( if (isClosed) return; isClosed = true; resolveReady(); - void Promise.all(watchers.map((watcher) => watcher.close())).then( - resolveClosed, - resolveClosed, - ); + void closeRegistrations(registrations).then(resolveClosed, resolveClosed); }, }; } From 53c2a86e2d75c6ba5d7da40864050ab8ab9e01a8 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Mon, 13 Jul 2026 15:31:09 -0400 Subject: [PATCH 12/16] fix(watch): bound observer startup readiness --- src/core/watchController.test.ts | 126 ++++++++++++++++++++++++++++++- src/core/watchController.ts | 80 +++++++++++++++++--- src/core/watchObserver.test.ts | 41 ++++++++++ 3 files changed, 235 insertions(+), 12 deletions(-) diff --git a/src/core/watchController.test.ts b/src/core/watchController.test.ts index ac7fc11c..6f2066c8 100644 --- a/src/core/watchController.test.ts +++ b/src/core/watchController.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import { createWatchController, + WATCH_EVENT_SOURCE_STARTUP_TIMEOUT_CODE, type WatchControllerClock, type WatchEventSourceCallbacks, } from "./watchController"; @@ -326,6 +327,118 @@ describe("createWatchController", () => { 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; @@ -472,6 +585,7 @@ describe("createWatchController", () => { const clock = new FakeWatchClock(); const oldSource = fakeSource(); const newSource = fakeSource(); + const errors: unknown[] = []; let oldChecks = 0; let newChecks = 0; const oldController = createWatchController({ @@ -480,6 +594,8 @@ describe("createWatchController", () => { createEventSource: oldSource.create, getSignature: () => (++oldChecks, "old"), refresh: () => {}, + reportError: (error) => errors.push(error), + startupTimeoutMs: 500, }); oldController.close(); createWatchController({ @@ -488,13 +604,19 @@ describe("createWatchController", () => { createEventSource: newSource.create, getSignature: () => (++newChecks, "new"), refresh: () => {}, + startupTimeoutMs: 500, }); + oldSource.ready(); oldSource.event(); - newSource.event(); - clock.advance(200); + 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 index 63d376a2..cabc929f 100644 --- a/src/core/watchController.ts +++ b/src/core/watchController.ts @@ -22,6 +22,9 @@ export interface WatchEventSourceCallbacks { 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; @@ -35,6 +38,7 @@ export interface WatchControllerOptions { healthyCheckMs?: number; degradedCheckMs?: number; duplicateErrorIntervalMs?: number; + startupTimeoutMs?: number; } export interface WatchControllerState { @@ -69,6 +73,17 @@ function getErrorKey(error: unknown) { 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; @@ -77,6 +92,8 @@ export function createWatchController(options: WatchControllerOptions): WatchCon 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", @@ -85,8 +102,10 @@ export function createWatchController(options: WatchControllerOptions): WatchCon 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; @@ -116,7 +135,7 @@ export function createWatchController(options: WatchControllerOptions): WatchCon if (state.phase === "closed" || state.phase === "checking" || state.phase === "refreshing") { return; } - const deadlines = [quietDeadline, maximumDeadline, safetyDeadline].filter( + const deadlines = [startupDeadline, quietDeadline, maximumDeadline, safetyDeadline].filter( (deadline): deadline is number => deadline !== undefined, ); if (deadlines.length === 0) return; @@ -130,6 +149,19 @@ export function createWatchController(options: WatchControllerOptions): WatchCon /** 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; @@ -183,12 +215,29 @@ export function createWatchController(options: WatchControllerOptions): WatchCon finishCheck(); }; - /** Consume a due debounce, maximum-delay, or safety deadline as one check. */ + /** 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); @@ -202,7 +251,9 @@ export function createWatchController(options: WatchControllerOptions): WatchCon /** Treat an event as a hint and retain the first event's maximum deadline. */ const onEvent = () => { - if (state.phase === "closed") return; + if (state.phase === "closed" || (sourceStatus !== "starting" && sourceStatus !== "ready")) { + return; + } if (state.phase === "checking" || state.phase === "refreshing") { state.dirty = true; return; @@ -216,7 +267,9 @@ export function createWatchController(options: WatchControllerOptions): WatchCon /** Close the startup scan race with an immediate signature check after watcher readiness. */ const onSourceReady = () => { - if (state.phase === "closed") return; + if (state.phase === "closed" || sourceStatus !== "starting") return; + sourceStatus = "ready"; + startupDeadline = undefined; if (state.phase === "checking" || state.phase === "refreshing") { state.dirty = true; return; @@ -226,12 +279,11 @@ export function createWatchController(options: WatchControllerOptions): WatchCon /** Degrade only for watcher resource exhaustion; other source errors stay nonfatal. */ const onSourceError = (error: unknown) => { - if (state.phase === "closed") return; + if (state.phase === "closed" || sourceStatus === "closed") return; const code = getErrorCode(error); if (code === "ENOSPC" || code === "EMFILE") { state.degraded = true; - eventSource?.close(); - eventSource = undefined; + closeEventSource(); safetyDeadline = Math.min( safetyDeadline ?? Number.POSITIVE_INFINITY, clock.now() + degradedCheckMs, @@ -244,13 +296,22 @@ export function createWatchController(options: WatchControllerOptions): WatchCon 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 { - eventSource = options.createEventSource({ + 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); @@ -268,8 +329,7 @@ export function createWatchController(options: WatchControllerOptions): WatchCon maximumDeadline = undefined; safetyDeadline = undefined; clearTimer(); - eventSource?.close(); - eventSource = undefined; + closeEventSource(); }, /** Expose a snapshot for diagnostics without allowing state mutation. */ getState() { diff --git a/src/core/watchObserver.test.ts b/src/core/watchObserver.test.ts index 836eeaf0..82ab46c7 100644 --- a/src/core/watchObserver.test.ts +++ b/src/core/watchObserver.test.ts @@ -1,6 +1,8 @@ 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, @@ -57,6 +59,45 @@ async function selectedBackend(platform: NodeJS.Platform) { } 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"]); }); From af957398055d789acd7db4ab134ce058de4cf546 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Tue, 14 Jul 2026 14:34:47 -0400 Subject: [PATCH 13/16] test(watch): normalize cross-platform path assertions --- src/core/git.test.ts | 42 +++++++++++++-------- src/core/vcs/git.test.ts | 61 +++++++++++++++++++++---------- src/core/watchObserver.fs.test.ts | 12 ++++-- 3 files changed, 77 insertions(+), 38 deletions(-) diff --git a/src/core/git.test.ts b/src/core/git.test.ts index 555abfd4..3fdabfd8 100644 --- a/src/core/git.test.ts +++ b/src/core/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 { tmpdir } from "node:os"; -import { join, resolve, sep } from "node:path"; +import { join, resolve } from "node:path"; import { buildGitDiffArgs, buildGitIgnoredDirectoryArgs, @@ -43,6 +43,11 @@ function createTempRepo(prefix: string) { return dir; } +/** Normalize symlinked and Windows short/long temp path spellings before path comparisons. */ +function normalizeComparablePath(path: string) { + return realpathSync.native(path).replace(/\\/g, "/"); +} + function makeGitInput(overrides: Partial = {}): VcsDiffCommandInput { return { kind: "vcs", @@ -154,8 +159,12 @@ describe("listGitIgnoredDirectoryRoots", () => { const roots = listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: join(repoRoot, "src") }); - expect(roots).toEqual([join(realpathSync(repoRoot), "node_modules")]); - expect(roots).not.toContain(join(realpathSync(repoRoot), "src")); + expect(roots.map(normalizeComparablePath)).toEqual([ + normalizeComparablePath(join(repoRoot, "node_modules")), + ]); + expect(roots.map(normalizeComparablePath)).not.toContain( + normalizeComparablePath(join(repoRoot, "src")), + ); }); test("honors nested ignore negation instead of pruning its visible ancestor", () => { @@ -174,9 +183,9 @@ describe("listGitIgnoredDirectoryRoots", () => { 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"), - ]); + expect( + listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: repoRoot }).map(normalizeComparablePath), + ).toEqual([normalizeComparablePath(join(repoRoot, "generated", "discard"))]); }); test("honors repository-local excludes", () => { @@ -185,9 +194,9 @@ describe("listGitIgnoredDirectoryRoots", () => { 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"), - ]); + expect( + listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: repoRoot }).map(normalizeComparablePath), + ).toEqual([normalizeComparablePath(join(repoRoot, "local-cache"))]); }); test("does not prune an ignored parent containing a forced tracked file", () => { @@ -201,10 +210,13 @@ describe("listGitIgnoredDirectoryRoots", () => { git(repoRoot, "commit", "-m", "track forced file"); const roots = listGitIgnoredDirectoryRoots(makeGitInput(), { cwd: repoRoot }); - const trackedPath = join(realpathSync(repoRoot), "vendor", "tracked.txt"); + const comparableRoots = roots.map(normalizeComparablePath); + const trackedPath = normalizeComparablePath(join(repoRoot, "vendor", "tracked.txt")); - expect(roots).toEqual([join(realpathSync(repoRoot), "vendor", "generated")]); - expect(roots.some((root) => trackedPath.startsWith(`${root}${sep}`))).toBe(false); + expect(comparableRoots).toEqual([ + normalizeComparablePath(join(repoRoot, "vendor", "generated")), + ]); + expect(comparableRoots.some((root) => trackedPath.startsWith(`${root}/`))).toBe(false); }); test("falls back to no pruning when best-effort discovery fails", () => { @@ -226,16 +238,16 @@ describe("resolveGitMetadata", () => { git(repoRoot, "commit", "-m", "initial"); const normal = resolveGitMetadata(makeGitInput(), { cwd: repoRoot }); - expect(normal.repoRoot).toBe(realpathSync(repoRoot)); + expect(normalizeComparablePath(normal.repoRoot)).toBe(normalizeComparablePath(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(normalizeComparablePath(linked.repoRoot)).toBe(normalizeComparablePath(linkedRoot)); expect(linked.gitDir).not.toBe(linked.commonDir); - expect(linked.gitDir).toContain(join("worktrees", "hunk-git-linked-")); + expect(normalizeComparablePath(linked.gitDir)).toContain("/worktrees/hunk-git-linked-"); expect(linked.commonDir).toBe(normal.commonDir); }); }); diff --git a/src/core/vcs/git.test.ts b/src/core/vcs/git.test.ts index ea8aeb7b..bef5ec5c 100644 --- a/src/core/vcs/git.test.ts +++ b/src/core/vcs/git.test.ts @@ -164,25 +164,36 @@ describe("GitVcsAdapter", () => { { kind: "vcs", staged: false, options: { vcs: "git" } }, { cwd: repo }, ); - expect(unstaged.targets.some((target) => target.directory === repo)).toBe(true); + expect( + unstaged.targets.some( + (target) => normalizeComparablePath(target.directory) === normalizeComparablePath(repo), + ), + ).toBe(true); const worktreeTarget = unstaged.targets.find( - (target) => target.kind === "directory-tree" && target.directory === repo, + (target) => + target.kind === "directory-tree" && + normalizeComparablePath(target.directory) === normalizeComparablePath(repo), ); - expect(worktreeTarget?.kind === "directory-tree" ? worktreeTarget.ignoredRoots : []).toEqual([ - join(repo, ".git"), - join(repo, "generated"), + expect( + worktreeTarget?.kind === "directory-tree" + ? worktreeTarget.ignoredRoots.map(normalizeComparablePath) + : [], + ).toEqual([ + normalizeComparablePath(join(repo, ".git")), + normalizeComparablePath(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"], - }, - ]); + expect(metadataTargets).toHaveLength(1); + expect(normalizeComparablePath(metadataTargets[0]!.directory)).toBe( + normalizeComparablePath(join(repo, ".git")), + ); + expect( + metadataTargets[0]!.kind === "directory-tree" + ? metadataTargets[0]!.ignoredRoots.map(normalizeComparablePath) + : [], + ).toEqual([normalizeComparablePath(join(repo, ".git", "objects"))]); const refPlan = operation.watchPlan!( { kind: "vcs", @@ -193,14 +204,22 @@ describe("GitVcsAdapter", () => { }, { cwd: repo }, ); - expect(refPlan.targets.some((target) => target.directory === repo)).toBe(true); + expect( + refPlan.targets.some( + (target) => normalizeComparablePath(target.directory) === normalizeComparablePath(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) => normalizeComparablePath(target.directory) === normalizeComparablePath(repo), + ), + ).toBe(false); expect(plan.targets.some((target) => target.sources.includes("vcs-metadata"))).toBe(true); } }); @@ -212,10 +231,14 @@ describe("GitVcsAdapter", () => { { 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"), - ]); + expect(normalizeComparablePath(metadataTarget!.directory)).toBe( + normalizeComparablePath(join(repo, ".git")), + ); + expect( + metadataTarget?.kind === "directory-tree" + ? metadataTarget.ignoredRoots.map(normalizeComparablePath) + : [], + ).toEqual([normalizeComparablePath(join(repo, ".git", "objects"))]); }); test("deduplicates common metadata while covering linked-worktree state", () => { diff --git a/src/core/watchObserver.fs.test.ts b/src/core/watchObserver.fs.test.ts index 05ceacd1..a92eaa12 100644 --- a/src/core/watchObserver.fs.test.ts +++ b/src/core/watchObserver.fs.test.ts @@ -239,7 +239,7 @@ describe("filesystem watch observer", () => { ); }); - test("excludes a worktree .git subtree when metadata is observed separately", async () => { + test("does not refresh for excluded worktree metadata churn", async () => { const directory = await temporaryDirectory(); const metadataDirectory = join(directory, ".git"); await mkdir(metadataDirectory); @@ -259,9 +259,13 @@ describe("filesystem watch observer", () => { ], }; - const source = await startObserver(plan); - await writeFile(metadata, "after"); - await expectNoEvent(source.nextEvent()); + // Windows may report only the basename from a recursive callback. That ambiguous hint is + // intentionally conservative, so assert the user-visible signature/refresh policy instead. + await expectNoRefresh( + plan, + () => readFileSync(worktreeFile, "utf8"), + () => writeFile(metadata, "after"), + ); }); test("close releases handles and suppresses later events", async () => { From 45ef7e32a8ad2a17a552a9b5b4d635390b635912 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Tue, 14 Jul 2026 14:41:20 -0400 Subject: [PATCH 14/16] test(watch): await sidecar reload rendering --- src/ui/AppHost.watch.test.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/ui/AppHost.watch.test.tsx b/src/ui/AppHost.watch.test.tsx index cb8441b2..144ac671 100644 --- a/src/ui/AppHost.watch.test.tsx +++ b/src/ui/AppHost.watch.test.tsx @@ -18,6 +18,14 @@ async function flush(setup: Awaited>) { }); } +/** Yield across render and filesystem turns until an asynchronous view update is visible. */ +async function flushUntil(setup: Awaited>, predicate: () => boolean) { + for (let attempt = 0; attempt < 50 && !predicate(); attempt++) { + await flush(setup); + await new Promise((resolve) => setTimeout(resolve, 0)); + } +} + /** Advance the injected watch debounce and settle its asynchronous soft reload. */ async function advanceWatch( setup: Awaited>, @@ -135,6 +143,7 @@ describe("watched input lifecycle", () => { watch.setSignature("signature:sidecar"); watch.emit(); await advanceWatch(setup, watch, 200); + await flushUntil(setup, () => setup.captureCharFrame().includes("Watch rationale updated")); expect(setup.captureCharFrame()).toContain("Watch rationale updated"); } finally { await act(async () => setup.renderer.destroy()); From 5224c3819fc913a3ae495675f164436693b6af96 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Tue, 14 Jul 2026 16:58:31 -0400 Subject: [PATCH 15/16] docs(watch): publish final benchmark evidence --- docs/watch-benchmark-final.md | 309 ++++++++++++++++++++++++++++++++++ 1 file changed, 309 insertions(+) create mode 100644 docs/watch-benchmark-final.md diff --git a/docs/watch-benchmark-final.md b/docs/watch-benchmark-final.md new file mode 100644 index 00000000..69854a12 --- /dev/null +++ b/docs/watch-benchmark-final.md @@ -0,0 +1,309 @@ +# Final watch-mode benchmark report + +> **Reviewed final report.** This report supersedes the headline claims in the [preliminary watch benchmark](watch-benchmark.md), which remains available as preliminary, non-comparable evidence. + +Campaign: `watch-20260714T185115Z-b04ed2b0b-h4f35a0ef` (`watch-v1`) + +Frozen: 2026-07-14 18:51:15 UTC + +## 1. Executive summary + +PR #531 replaces the legacy 250 ms Git-backed watch loop with event hints plus an authoritative Git signature and a 10-second safety check. On the two hosts that could run the complete frozen TUI protocol, the candidate cut one session's cumulative 120-second idle main-process CPU by **1.8–6.4×** and Git invocations by **35–36×**. Mean launch-to-`File View` remained close to the base: **+21 to +61 ms** across the four host/fixture cells. Every one of the 120 refresh trials was correct; the candidate's deliberate debounce made refreshes slower than the polling base, but candidate p95 remained below 610 ms in these cells. + +Headline values are means over five startup trials or two uninstrumented 120-second idle trials, plus one separate 120-second Git cohort. Each row identifies the actual fixture `totalSubdirectoryCount`. + +| Platform | Fixture | Actual subdirectories | Startup mean, base → candidate | Idle CPU at 120 s, base → candidate | Reduction | Git calls at 120 s, base → candidate | Reduction | +| ------------- | ----------- | --------------------: | -----------------------------: | ----------------------------------: | --------: | -----------------------------------: | --------: | +| macOS arm64 | little repo | 1,260 | 528 → 554 ms | 5,025 → 780 ms | 6.4× | 1,416 → 40 | 35× | +| macOS arm64 | big repo | 25,223 | 560 → 621 ms | 3,235 → 740 ms | 4.4× | 1,425 → 40 | 36× | +| Linux x64 | little repo | 1,260 | 648 → 669 ms | 3,615 → 675 ms | 5.4× | 1,434 → 40 | 36× | +| Linux x64 | big repo | 25,223 | 726 → 750 ms | 3,205 → 1,765 ms | 1.8× | 1,434 → 40 | 36× | +| Windows ARM64 | little repo | 1,260 | N/A — TUI deferred | N/A | N/A | N/A | N/A | +| Windows ARM64 | big repo | 25,223 | N/A — TUI deferred | N/A | N/A | N/A | N/A | + +The evidence supports the candidate's backend policy: + +- **macOS:** native recursive observation reached ready in 151 ms on little repo and 236 ms on big repo without per-directory registration. +- **Linux:** Git-pruned Chokidar reached ready in 259 ms and 858 ms while limiting traversal to relevant roots. This avoids the roughly 24,000 inotify registrations per session observed in the supplemental native-recursive probe, which approached the host's 61,504-watch limit after only a few sessions. +- **Windows:** native recursion remains the intended policy because correctness probes reached ready and it avoids Chokidar's per-directory resource model. Windows performance is **not** inferred from macOS: full Windows TUI measurements are deferred because of Bun ARM64 FFI and headless ConPTY lifecycle blockers. + +## 2. Frozen provenance and campaign manifest + +| Input | Frozen SHA / checksum | +| ------------------------------ | ------------------------------------------------------------------ | +| Base source | `04ed2b0bd51aae633ab94b3a6d157d5d5e568dd0` | +| Candidate source | `4f35a0ef67f9d8094b4b3ff5bd74c3b74940dd57` | +| Harness source | `bcfb232b4e83e3c8a07834c5e114e7f0f364e8d5` | +| Big-repo source | `85caaceeff750eeceb2e26a7336c8aef1d8372f5` | +| Campaign manifest SHA256 | `e4bf360c9a31ecf84ca70f5075b5adb89e0694bac395ece5094d68d19511b4e1` | +| Input index SHA256 | `b0703c2896034363200edb0087650d1ef44284c36fdc4fb1bb53634ce1f6a4b7` | +| Complete artifact index SHA256 | `6f5ed0e16c0363f100dab38547f808375d5ba357074ed0ce9d043f7d18892712` | + +The candidate is a clean descendant of the base. The campaign was built from `inputs/hunk.bundle`; no result used an npm package or a PATH-resolved Hunk binary. Each host built both exact revisions with Bun 1.3.14 and `bun install --frozen-lockfile`, then invoked the absolute compiled binary recorded in provenance. The provenance includes source SHA, executable SHA256, file/process architecture, exact Bun path, build command, logs, and a successful absolute-path `--help` smoke check. + +| Host | Revision | Binary SHA256 | Format / architecture | +| ------------ | --------- | ------------------------------------------------------------------ | --------------------- | +| aarmstrong | base | `cd7b70e70bcbc6c7665cd7e6811ebeae9e78e7b286ed25e1b9b7eaef2bc311e6` | Mach-O arm64 | +| aarmstrong | candidate | `9c9f511d5a8440e00310962594c2e9c2fd2004bb521aeb4d0042fd02c2a316e3` | Mach-O arm64 | +| sentry-agent | base | `89b20bfe7af596e3f0ff3fb344917dbc765c2fa9baf6e4886fd219830c94d42c` | ELF x86-64 | +| sentry-agent | candidate | `20e33edcd67268c85f5857c58f087abe557bd6fa386f16bfa2f99932c80d4b2f` | ELF x86-64 | + +### Raw artifact index + +The checksum-verified campaign archive is retained outside the Git worktree at: + +```text +/Users/justin/DEV/hunk-watch-campaigns/campaigns/ + watch-20260714T185115Z-b04ed2b0b-h4f35a0ef/ +``` + +`campaign-artifacts.sha256` inventories 771 files (the index excludes itself): canonical raw JSON and terminal bytes under `raw/`, host provenance and failure evidence under `hosts/`, frozen bundles/manifests under `inputs/`, generated summaries, and recovery evidence. The archive occupies approximately 482 MiB; canonical raw results are approximately 5.6 MiB. `shasum -a 256 -c campaign-artifacts.sha256` and the nested input index both passed before publication. The canonical raw set contains 104/104 valid records for each measured host. + +## 3. Host and environment matrix + +| Role | Host | Environment | Filesystem / resource notes | Status | +| -------------------- | ---------------------------- | -------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------ | ------------------------------------------------------- | +| Primary macOS | `macos-arm64-aarmstrong` | macOS 15.7.4, Apple M1 Max, 10 cores, 64 GiB, Bun 1.3.14, Git 2.53.0 | APFS SSD; AC power, battery 100%, power mode 0; no thermal/performance warning | Complete, 104/104 | +| Primary Linux | `linux-x64-sentry-agent` | Ubuntu 24.04.4, Linux 6.8.0-134, KVM, 4 vCPU Intel i7-6770HQ, 8.3 GB, Bun 1.3.14, Git 2.43.0 | ext4; inotify watches 61,504, instances 128, queue 16,384 | Complete after disclosed recovery, 104/104 | +| Primary Windows | `windows-arm64-hunk-windows` | Windows ARM64 VM, native Bun 1.3.14 | ARM64 OpenTUI FFI cannot initialize (`TinyCC is disabled`) | Correctness/probe only; TUI metrics deferred | +| Supplemental Windows | `windows-x64-gha` | GitHub-hosted `windows-latest` x64 | Headless ConPTY child survived forced process-tree teardown | Correctness/build/provenance only; TUI metrics deferred | +| Supplemental macOS | `macos-arm64-currie` | Not staged for this campaign | Canonical archive/aggregation host only | Excluded from measurements | + +The report does not combine architectures or average hosts. Windows ARM64 and x64 are separately disclosed rather than averaged. + +## 4. Fixture definitions and exact directory counts + +Both portable fixtures are sanitized Git bundles plus deterministic ignored-tree manifests. Reconstruction sets `core.autocrlf=false` and `core.symlinks=false`; source symlinks are materialized as plain files. Before each run group, the harness restores one standardized dirty tracked file and one existing untracked file. + +| Fixture | Source / manifest SHA256 | Actual total subdirectories | Ignored | Relevant | Tracked files | Initial untracked | Maximum depth | +| ----------- | ------------------------------------------------------------------------------------- | --------------------------: | ------: | -------: | ------------: | ----------------: | ------------: | +| little repo | candidate source / `c0e646dc219f12ba378030fb01a5f1028e722177d4a8be3fccfcb12e3077e06f` | 1,260 | 1,176 | 84 | 444 | 1 | 11 | +| big repo | modem fixture / `697436aff9ba09f6211f8915f15b001faf50106ce48284132be3476017c7ee29` | 25,223 | 24,095 | 1,128 | 6,346 | 1 | 15 | + +Manifest hashes, reconstructed counts, and the counts repeated in every raw record agree on both measured operating systems. + +## Methodology + +The fixed terminal was 120×30. Base and candidate never ran concurrently. Each fixture/revision cell contains one supplemental `cold-ish` first launch, one excluded warmup, five warm launches in a deterministic balanced ABBA-style sequence, one observer record, two uninstrumented 120-second idle runs sampled at t=0,10,…,120 seconds, one separate 120-second Trace2 Git cohort, and five refresh trials for each of three scenarios. The base observer record is an explicit N/A because the legacy implementation polls. + +Median and p95 below use the five warm trials; with n=5, nearest-rank p95 is the maximum observed trial. Idle values are means of two cumulative per-process deltas, not sampled CPU percentages. Git counts come from the separate instrumentation cohort so Trace2 does not contaminate headline CPU/RSS. No run was labeled truly cold, and the campaign defines no pass/fail performance threshold. + +## 5. Startup: launch to `File View` + +| Platform | Fixture | Actual subdirectories | Revision | Trials | Mean | Median | p95 | Supplemental cold-ish | +| -------- | ----------- | --------------------: | --------- | -----: | --------: | --------: | --------: | --------------------: | +| macOS | little repo | 1,260 | base | 5 | 527.60 ms | 531.37 ms | 538.06 ms | 577.84 ms | +| macOS | little repo | 1,260 | candidate | 5 | 554.47 ms | 557.47 ms | 561.53 ms | 604.44 ms | +| macOS | big repo | 25,223 | base | 5 | 560.27 ms | 567.16 ms | 572.37 ms | 753.26 ms | +| macOS | big repo | 25,223 | candidate | 5 | 621.10 ms | 617.75 ms | 631.26 ms | 718.04 ms | +| Linux | little repo | 1,260 | base | 5 | 647.53 ms | 643.08 ms | 660.70 ms | 690.49 ms | +| Linux | little repo | 1,260 | candidate | 5 | 669.02 ms | 669.14 ms | 684.70 ms | 732.39 ms | +| Linux | big repo | 25,223 | base | 5 | 726.35 ms | 714.31 ms | 761.60 ms | 720.24 ms | +| Linux | big repo | 25,223 | candidate | 5 | 750.16 ms | 751.35 ms | 754.64 ms | 752.63 ms | + +The candidate adds 24–61 ms to the means, while the balanced warm trials remain in the same sub-second range. “Startup parity” here means no order-of-magnitude regression, not statistical equivalence; five trials are too few for such a claim. + +## 6. Candidate observer-ready metric + +Observer ready is measured from probe-process launch, not as an isolated CPU measurement. `plan` is Git-aware plan derivation; `construction` is backend construction to ready. The remaining launch time includes process/bootstrap work. + +| Platform | Fixture | Actual subdirectories | Backend | Launch → ready | Plan | Construction → ready | Base | +| ------------- | ----------- | --------------------: | ------------------- | -------------------------------------------: | -------: | -------------------: | -------------------- | +| macOS | little repo | 1,260 | native recursive | 150.86 ms | 27.09 ms | 26.02 ms | N/A — legacy polling | +| macOS | big repo | 25,223 | native recursive | 236.47 ms | 44.35 ms | 62.52 ms | N/A — legacy polling | +| Linux | little repo | 1,260 | Git-pruned Chokidar | 259.22 ms | 11.11 ms | 107.54 ms | N/A — legacy polling | +| Linux | big repo | 25,223 | Git-pruned Chokidar | 857.70 ms | 31.87 ms | 656.88 ms | N/A — legacy polling | +| Windows ARM64 | little repo | 1,260 | native intended | Ready in correctness probe; final timing N/A | N/A | N/A | N/A — legacy polling | +| Windows ARM64 | big repo | 25,223 | native intended | TUI/probe matrix deferred | N/A | N/A | N/A — legacy polling | + +No measured candidate cell selected the degraded 2-second fallback. + +## 7. Idle CPU and RSS + +Each CPU cell is the mean cumulative delta from the t=0 sample. The 60-second column uses the exact t=60 sample; the 120-second column uses t=120. RSS at 60 seconds is the mean exact slice, and peak is the maximum observed sample across both runs. + +| Platform | Fixture | Actual subdirectories | Revision | Runs | CPU at 60 s | CPU at 120 s | RSS at 60 s | Peak RSS | +| -------- | ----------- | --------------------: | --------- | ---: | ----------: | -----------: | ----------: | ---------: | +| macOS | little repo | 1,260 | base | 2 | 2,565 ms | 5,025 ms | 189.91 MiB | 190.66 MiB | +| macOS | little repo | 1,260 | candidate | 2 | 435 ms | 780 ms | 190.72 MiB | 192.33 MiB | +| macOS | big repo | 25,223 | base | 2 | 1,665 ms | 3,235 ms | 190.43 MiB | 192.59 MiB | +| macOS | big repo | 25,223 | candidate | 2 | 420 ms | 740 ms | 189.71 MiB | 190.13 MiB | +| Linux | little repo | 1,260 | base | 2 | 1,900 ms | 3,615 ms | 110.88 MiB | 143.61 MiB | +| Linux | little repo | 1,260 | candidate | 2 | 475 ms | 675 ms | 119.43 MiB | 144.52 MiB | +| Linux | big repo | 25,223 | base | 2 | 1,685 ms | 3,205 ms | 110.98 MiB | 143.84 MiB | +| Linux | big repo | 25,223 | candidate | 2 | 1,570 ms | 1,765 ms | 191.54 MiB | 206.09 MiB | + +Linux big repo shows the tradeoff most clearly: pruned Chokidar concentrated setup/memory work early, yet the candidate still used 1.8× less main-process CPU over 120 seconds. macOS native recursion did not show a comparable RSS increase. + +## 8. Git invocations and families + +These are measured totals from one separate 120-second Trace2 cohort per cell. Child Git CPU was intentionally **not available** in this low-distortion instrumentation mode and is not estimated. + +| Platform | Fixture | Actual subdirectories | Revision | Total | Command families | +| -------- | ----------- | --------------------: | --------- | ----: | ---------------------------------------------------- | +| macOS | little repo | 1,260 | base | 1,416 | `diff` 472; `rev-parse` 472; `status` 472 | +| macOS | little repo | 1,260 | candidate | 40 | `rev-parse` 15; `ls-files` 1; `diff` 12; `status` 12 | +| macOS | big repo | 25,223 | base | 1,425 | `diff` 475; `rev-parse` 475; `status` 475 | +| macOS | big repo | 25,223 | candidate | 40 | `rev-parse` 15; `ls-files` 1; `diff` 12; `status` 12 | +| Linux | little repo | 1,260 | base | 1,434 | `diff` 478; `rev-parse` 478; `status` 478 | +| Linux | little repo | 1,260 | candidate | 40 | `rev-parse` 15; `ls-files` 1; `diff` 12; `status` 12 | +| Linux | big repo | 25,223 | base | 1,434 | `diff` 478; `rev-parse` 478; `status` 478 | +| Linux | big repo | 25,223 | candidate | 40 | `rev-parse` 15; `ls-files` 1; `diff` 12; `status` 12 | + +The candidate performs one ignored-root planning query and then roughly one authoritative signature family per 10-second safety interval. The base continuously performs the three-command signature at the legacy polling cadence. + +## 9. Refresh latency and correctness + +All rows contain 5/5 correct visible refreshes. Values are median / nearest-rank p95. + +| Platform | Fixture | Actual subdirectories | Revision | Tracked write | Atomic rename | Relevant untracked creation | +| -------- | ----------- | --------------------: | --------- | -----------------: | -----------------: | --------------------------: | +| macOS | little repo | 1,260 | base | 371.37 / 378.92 ms | 368.60 / 389.76 ms | 78.17 / 82.61 ms | +| macOS | little repo | 1,260 | candidate | 409.69 / 413.91 ms | 407.01 / 409.59 ms | 364.08 / 367.94 ms | +| macOS | big repo | 25,223 | base | 191.38 / 203.68 ms | 196.91 / 199.21 ms | 159.65 / 161.61 ms | +| macOS | big repo | 25,223 | candidate | 528.17 / 542.71 ms | 538.68 / 543.68 ms | 444.55 / 453.19 ms | +| Linux | little repo | 1,260 | base | 76.84 / 80.15 ms | 76.82 / 79.66 ms | 69.81 / 71.80 ms | +| Linux | little repo | 1,260 | candidate | 273.63 / 277.30 ms | 278.68 / 284.70 ms | 262.50 / 267.64 ms | +| Linux | big repo | 25,223 | base | 192.34 / 200.22 ms | 191.42 / 202.97 ms | 150.13 / 168.78 ms | +| Linux | big repo | 25,223 | candidate | 411.02 / 591.75 ms | 439.15 / 503.86 ms | 477.66 / 602.37 ms | + +The base can notice an edit on its next 250 ms poll. The candidate intentionally waits for a 200 ms quiet debounce before checking the authoritative signature, trading some latency for much lower continuous work. Correctness covered ordinary writes, atomic replacement, and untracked creation without user input. + +## 10. Setup / steady-state break-even + +The harness did not isolate observer CPU, so this analysis does **not** mislabel observer-ready wall time as setup CPU. Instead it uses the measured median end-to-end startup difference as a conservative one-time cost proxy and divides it by the measured one-session 120-second main-process CPU saving rate. + +| Platform | Fixture | Candidate median startup difference | Idle CPU saved per wall second | Approximate break-even | +| -------- | ----------- | ----------------------------------: | -----------------------------: | ---------------------: | +| macOS | little repo | +26.10 ms | 35.38 ms/s | 0.74 s | +| macOS | big repo | +50.60 ms | 20.79 ms/s | 2.43 s | +| Linux | little repo | +26.06 ms | 24.50 ms/s | 1.06 s | +| Linux | big repo | +37.05 ms | 12.00 ms/s | 3.09 s | + +This is descriptive amortization, not a user-interactivity threshold. Concentrated setup can still affect the first interaction even when its CPU-equivalent cost is recovered within seconds. Linux big repo's 858 ms observer-ready wall time and 206 MiB peak RSS are therefore important alongside its three-second CPU break-even. + +## 11. Measured one-session values and projections + +The one-session 120-second CPU/Git values are measured in sections 7–8. The values below are renderer-style **projections** from those rates; no 3- or 5-session process matrix was run. CPU is projected main-process CPU milliseconds per wall minute; Git is projected invocations per wall minute. + +| Platform | Fixture | Revision | 1 session projected CPU / Git | 3 sessions projected CPU / Git | 5 sessions projected CPU / Git | +| -------- | ----------- | --------- | ----------------------------: | -----------------------------: | -----------------------------: | +| macOS | little repo | base | 2,512.5 ms / 708 | 7,537.5 ms / 2,124 | 12,562.5 ms / 3,540 | +| macOS | little repo | candidate | 390 ms / 20 | 1,170 ms / 60 | 1,950 ms / 100 | +| macOS | big repo | base | 1,617.5 ms / 712.5 | 4,852.5 ms / 2,137.5 | 8,087.5 ms / 3,562.5 | +| macOS | big repo | candidate | 370 ms / 20 | 1,110 ms / 60 | 1,850 ms / 100 | +| Linux | little repo | base | 1,807.5 ms / 717 | 5,422.5 ms / 2,151 | 9,037.5 ms / 3,585 | +| Linux | little repo | candidate | 337.5 ms / 20 | 1,012.5 ms / 60 | 1,687.5 ms / 100 | +| Linux | big repo | base | 1,602.5 ms / 717 | 4,807.5 ms / 2,151 | 8,012.5 ms / 3,585 | +| Linux | big repo | candidate | 882.5 ms / 20 | 2,647.5 ms / 60 | 4,412.5 ms / 100 | + +Linear projections do not model cache sharing, scheduler contention, filesystem contention, or watch-limit exhaustion. + +## 12. Backend strategy rationale + +### macOS: native recursive + +The frozen candidate selected `native-recursive` in every macOS record. It registered the worktree as one recursive native target and filtered event paths against Git-derived ignored roots. Ready time grew from 151 to 236 ms across a 20× directory-count increase, peak RSS remained near 190–192 MiB, and 120-second CPU fell 4.4–6.4×. This is direct evidence for retaining native recursion on macOS. + +### Linux: Git-pruned Chokidar + +The frozen candidate selected `chokidar-portable`, never degraded, and traversed Git-relevant roots rather than all 25,223 directories. It reached ready in 259 ms on little repo and 858 ms on big repo. Frozen-campaign steady-state CPU and Git work still fell substantially, although big-repo setup reached 206 MiB peak RSS. + +A separate preliminary backend probe on this same Linux class measured unpruned Chokidar at 2.65 s / 24,554 directories / about 286 MiB, Git-pruned Chokidar at 0.47 s / 1,129 directories / about 122 MiB, and native recursive `fs.watch` at 0.33 s / about 46 MiB but roughly 24,000 inotify registrations per process. With `max_user_watches=61,504`, native recursion exhausted the shared limit at roughly three sessions. Those supplemental measurements are not mixed into the frozen headline table, but they explain why the somewhat slower pruned backend is the safer multi-session Linux policy. + +### Windows: native recursive, performance pending + +On the ARM64 VM, 103 focused watch tests passed and forced native and Chokidar observer probes both reached ready. The native policy avoids per-directory Chokidar registrations and matches Windows' recursive `fs.watch` capability. However, Bun 1.3.14 ARM64 could not load OpenTUI's FFI library; x64 emulation also failed to load the Ghostty addon. On GitHub's x64 runner, focused tests/build/provenance completed, but the headless ConPTY child survived forced teardown and correctly tripped leak detection. Therefore the backend choice is supported by correctness probes and measured resource behavior on the other backend implementations, but this report makes **no Windows startup, idle, memory, or latency claim**. Full Windows TUI measurements remain follow-up work. + +## 13. Failures, retries, and degraded states + +- **macOS:** 104/104 canonical records valid; no failed attempts or retries. Transfer-created AppleDouble metadata was removed from the campaign destination without changing measured artifacts. +- **Linux attempt 1:** reached 103/104 valid cells, then Bun/OpenTUI extraction left 201 temporary `.so` files totaling 2,707,790,272 bytes in `/tmp`, exhausting disk before the final big-repo candidate untracked-creation record could be written. Evidence and the zero-byte record are retained under `failed-attempt-enospc-20260714T2011Z/`. +- **Linux containment retry:** moving temporary extraction to campaign-owned `/dev/shm` caused OpenTUI initialization to fail. Both failures are retained under `failed-attempt-tmpdir-20260714T2012Z/`. +- **Approved Linux recovery:** all 103 valid original records were retained. Only `big-repo-candidate-refresh-15-attempt-1` was recovered using the frozen candidate binary and committed fixture, PTY, mutation, schema, and report modules. Its 412.76 ms correct refresh is included in the five-trial scenario. Final canonical validation is 104/104. +- **Windows:** failures are platform-blocker evidence, not excluded benchmark outliers. No Windows TUI record appears in headline conclusions. +- **Degraded fallback:** no canonical candidate measurement entered degraded mode. Base observer status is explicitly `not-applicable-legacy-polling`. + +No performance outlier was removed from a completed trial set. + +## 14. Preliminary report comparison + +The [preliminary report](watch-benchmark.md) compared npm `hunkdiff@0.17.0` against a mutable PR source build on a dirty production modem checkout. It reported 799 → 21 Git calls over 60 seconds, about 1.50 → 0.13 seconds of idle CPU, and 1,679 → 1,699 ms mean startup. It used a PATH wrapper, Herdr pane capture, a different fixture, and an incompletely frozen host/source environment. + +Those values are useful for discovering the problem and agree directionally with the frozen campaign. They are **not numerically comparable** with this report: npm versus source differs from exact base versus candidate source SHAs; the production checkout differs from both portable fixtures; its 60-second instrumentation differs from the separated 120-second CPU and Trace2 cohorts; and its cache/provenance controls differ. Final claims in PR #531 should cite this document, not transplant preliminary ratios. + +## 15. Limitations + +- Two complete performance hosts are not a population; hardware, filesystem, background work, and Git versions differ. +- Five startup and five scenario trials make p95 equal to the maximum; the label is descriptive, not a stable tail estimate. +- `cold-ish` means first launch after fixture reconstruction, not a dropped OS page cache or true cold-cache run. +- Main-process CPU excludes child Git CPU. Child CPU was unavailable in the separate low-distortion Trace2 cohort. +- Idle CPU is cumulative process CPU sampled at 10-second resolution; short bursts inside an interval are not localized. +- Observer-ready is wall time and does not isolate setup CPU. +- RSS sampling does not identify allocator ownership or shared-memory accounting. +- Refresh timing includes debounce, signature verification, reload, render, and terminal marker detection; it is user-visible pipeline latency, not raw filesystem event latency. +- The session projections are linear calculations, not measured concurrent runs. +- Windows provides correctness/probe evidence only, so no cross-architecture Windows average or performance conclusion is presented. +- The frozen fixtures preserve geometry, not private source contents or every behavior of the original repositories. + +## Infrastructure and change accounting + +### Candidate branch (`04ed2b0` → `4f35a0e`) + +Fourteen commits implement the production change: + +- `34df732` anchor reload signatures to source context +- `f0b517d`, `f640c7d`, `80de321` add backend-neutral plans, Git-derived plans, and the deterministic hybrid controller +- `dc37a0d`, `0336d76` add Chokidar observation and wire lifecycle into the UI +- `8ae9c69`, `e22e333` add passive PTY coverage and continuous-watch documentation +- `c152100`, `0659a30`, `d36c33f`, `fd00dd6` add resource fallback, Git ignored-root pruning, native recursive selection, and a bounded readiness deadline +- `f6165bd`, `4f35a0e` stabilize cross-platform and sidecar watch coverage + +Production implementation/seams are in `src/core/watch.ts`, `watchPlan.ts`, `watchController.ts`, `watchObserver.ts`, `src/core/vcs/*`, `src/core/git.ts`, loader/type plumbing, `src/ui/hooks/useWatchedInput.ts`, and the App/AppHost integration. Matching unit, filesystem, AppHost, helper, and PTY coverage lives beside those files and in `test/helpers/watchTest.ts` and `test/pty/watch.test.ts`. CI and PR CI add a compiled-binary watch PTY check. README documents continuous observation with fallback. + +The only new runtime dependency is `chokidar@^4.0.3` (and transitive `readdirp@4.1.2`), recorded in `package.json`, `bun.lock`, and `nix/bun.lock.nix`. The two user-visible patch changesets are `.changeset/calm-files-watch.md` and `.changeset/native-recursive-watch.md`. Existing benchmark fixture imports received small type/data updates, but no benchmark runner is a production runtime dependency. + +**Recommendation:** keep the production watch implementation, runtime dependency, packaging lock updates, documentation, compiled-binary CI checks, tests, and the two release changesets in PR #531. + +### Harness-only branch (`4f35a0e` → `bcfb232`) + +Twenty commits created and hardened campaign infrastructure: `d60a77f`, `2d9ad55`, `74eef02`, `22bf81c`, `c88a6ff`, `7011eeb`, `7d27741`, `799de5e`, `716c4a2`, `45a79e5`, `2b1ea62`, `8903535`, `eb23e5e`, `f63ce7d`, `8457e8e`, `8fcb9df`, `362e71a`, `e3818cf`, `e76197c`, and `bcfb232`. + +Complete harness file inventory: + +- **Protocol/fixture/schema/runner/report:** `benchmarks/watch/README.md`, `fixture.ts`, `campaign.ts`, `schema.ts`, `run.ts`, `report.ts`, and their `fixture.test.ts`, `campaign.test.ts`, `schema.test.ts`, and `run.test.ts`. +- **Terminal, ConPTY, sampling, and Git instrumentation:** `terminal.ts`, `sampler.ts`, `git-log.ts`, `observer-probe.ts`, `artifacts.ts`, with `terminal.test.ts`, `sampler.test.ts`, `git-log.test.ts`, and `observer-probe.test.ts`. +- **Freeze, provenance, transfer, build, and preflight:** `freeze.ts`, `stage.ts`, `build-host.ts`, `host-identity.ts`, `host-preflight.ts`, `prepare-preflight.ts`, with `stage.test.ts`, `build-host.test.ts`, `host-identity.test.ts`, `host-preflight.test.ts`, and shared `test-helpers.ts`. +- **Package/build integration:** seven `bench:watch*` scripts in `package.json`; dev-only `@xterm/headless@5.5.0` and `ghostty-opentui@1.4.10` plus `bun.lock`; `scripts/build-bin.ts` reuses `process.execPath` to prevent ambient-Bun builds. +- **Benchmark-only production seams:** `src/core/watchObserver.ts` and `.test.ts` accept an injected `auto|native|chokidar` backend for forced probes; production remains `auto`. +- **Workflow:** `.github/workflows/benchmarks.yml` contains the Windows adapter. +- **Maintenance changesets:** `.changeset/watch-benchmark-harness.md` and `.changeset/witty-schools-sit.md` are empty/non-release changesets. + +No harness-only commit is part of frozen PR #531 candidate `4f35a0e`; the campaign bundle records the separate harness SHA. The 7,070-line harness delta must not be presented as product code in the PR. + +### GitHub Actions workflow accounting + +The existing `Benchmarks` workflow still runs its normal Ubuntu benchmark job on non-documentation pushes to `main`. The harness branch adds an opt-in `windows-watch` job gated by `workflow_dispatch && run-windows-watch`. Dispatch inputs choose provisional refs or a frozen artifact, preflight/final mode, and forced observer probes. Provisional refs are preflight-only; final mode requires frozen campaign inputs. + +The Windows job pins Bun 1.3.14 and Node 22, disables CRLF conversion, validates all refs/artifact inputs, installs frozen dependencies, runs focused tests, captures host/build/binary provenance, executes the common bounded campaign command, runs forced backend probes, and uploads evidence even on failure. Artifacts retain raw JSON, terminal bytes, probes, build logs, and provenance for 30 days. Actions are SHA-pinned. The workflow declares no elevated `permissions:` block and adds no tunnel, remote desktop, service, or release permission; it uses the repository token only for checkout/artifact download/upload. It is informational and manual, not a required check or performance gate. + +Multiple failed Windows workflow runs are themselves retained diagnostic evidence; the final adapter reached tests/build/provenance but not trustworthy TUI teardown. + +**Recommendation:** do not merge the large workflow adapter through PR #531. If Windows benchmarking will recur, move a simplified manual job plus the minimal harness into a dedicated follow-up after ConPTY teardown is solved. Otherwise remove the harness-branch workflow changes; never enable the large matrix on every push. + +### Windows VM and runner assumptions + +The ARM64 VM uses native ARM64 Bun 1.3.14 and builds/checksums a native PE. Because the compiled Bun runtime cannot load OpenTUI FFI, the preflight adapter can run the frozen source through the same absolute native Bun for correctness/probe plumbing only; such source-adapter values must never enter compiled performance rows. The x64 path assumes GitHub's headless `windows-latest` ConPTY, PowerShell 7 checksum handling, `core.autocrlf=false`, Windows-safe fixture names, and explicit process-tree cleanup/leak detection. No VM image mutation or sysctl-like resource change is part of the campaign. + +**Recommendation:** retain the blocker notes and correctness tests. Remove source-adapter/performance scaffolding when Bun ARM64 FFI is fixed, then rerun compiled ARM64 and x64 TUI cells separately. Keep strict leak detection rather than accepting a surviving ConPTY child as success. + +### Staging, cleanup, and maintenance ownership + +- Canonical aggregation/archive: `/Users/justin/DEV/hunk-watch-campaigns/campaigns/` on currie. +- macOS staging: `/Users/justin/hunk-watch-campaigns/campaigns/` on aarmstrong; Bun is isolated at `/Users/justin/.hunk-watch-tools/bun-1.3.14/bin/bun`. +- Linux staging: `/home/justin/hunk-watch-campaigns/campaigns/` on sentry-agent; existing `/home/justin/perf` was inspection-only and untouched. +- Windows GHA staging: runner-local `campaigns/` plus uploaded 30-day artifacts. + +Campaign-owned worktrees, binaries, fixture reconstructions, terminal logs, and temporary libraries are cleanup-owned by the campaign operator. Linux recovery inventoried and removed only campaign-created extraction files; unrelated paths were not touched. Remote copies may be deleted after archive/checksum acceptance, but the canonical frozen bundle, raw JSON, terminal evidence, failure records, manifests, provenance, summaries, and checksum index should be retained together. + +The reusable fixture/freeze/schema/report pieces have value for a future benchmark, but maintaining the full harness means owning ConPTY adapters, two terminal parsers, OS samplers, workflow behavior, and cross-platform tests. **Recommended disposition:** retain the canonical campaign archive and this report; keep PR #531 free of harness-only code; preserve or extract the reusable core in a focused benchmark follow-up only if another campaign is planned; otherwise delete the temporary harness branch, manual workflow additions, dev-only terminal dependencies, empty changesets, forced-backend seam, and recovery script after the archive is secured. The exact-Bun `scripts/build-bin.ts` fix is independently useful but should be proposed separately if retained. From e19943fb6dd62adbe6fafac8038a92309d574228 Mon Sep 17 00:00:00 2001 From: Justin Giancola Date: Tue, 14 Jul 2026 16:58:53 -0400 Subject: [PATCH 16/16] docs(watch): preserve preliminary benchmark context --- docs/watch-benchmark.md | 80 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 docs/watch-benchmark.md diff --git a/docs/watch-benchmark.md b/docs/watch-benchmark.md new file mode 100644 index 00000000..5fbfb0f8 --- /dev/null +++ b/docs/watch-benchmark.md @@ -0,0 +1,80 @@ +# Watch Mode Benchmark: Evented vs Polling + +Benchmark comparing the old 250ms polling watch implementation (npm `hunkdiff@0.17.0`) against the evented Chokidar-based hybrid observer (`elucid/file-watch` PR #531). + +## Setup + +- **Repo:** `modem/modem` (large production monorepo) +- **Branch state:** dirty working tree with untracked files +- **OLD binary:** `/Users/justin/.npm-global/bin/hunk` (npm release 0.17.0, 250ms `setInterval` polling) +- **NEW binary:** `/Users/justin/.local/bin/hunk` (PR build from `elucid/file-watch`, Chokidar + 10s safety poll) +- **Command:** `hunk diff --watch` + +## Method + +### Git subprocess counting + +A wrapper script was placed ahead of real `git` on PATH. It logs every invocation with a timestamp to a per-version log file, then `exec`s the real git binary: + +```bash +#!/bin/bash +printf '%s %s\n' "$(date +%s.%N)" "$*" >> "${HUNK_BENCH_GIT_LOG}" +exec /nix/store/.../git "$@" +``` + +Both versions were launched in separate herdr panes with `HUNK_BENCH_GIT_LOG` and `PATH` set via `--env` on `herdr pane split`. After both rendered their initial diff, the log files were zeroed and CPU times recorded. The benchmark then sampled every 10 seconds for 60 seconds using `ps -p $PID -o cputime=` and `wc -l` on the log files. + +### Startup time + +Both versions were launched sequentially in herdr panes on the same repo. A polling loop read each pane's visible content via `herdr pane read` at ~20ms intervals, looking for hunk's menu bar (`File View`). The elapsed time from `herdr pane run` to first detection was recorded. Each version was quit (`q`) and relaunched between trials. Five trials were run per version. + +## Results + +### Idle overhead (60 seconds, no file changes) + +Sampled at 10-second intervals: + +``` +t=10s OLD cpu=0:02.68 git= 199 | NEW cpu=0:01.61 git= 6 +t=20s OLD cpu=0:02.93 git= 319 | NEW cpu=0:01.63 git= 9 +t=30s OLD cpu=0:03.14 git= 439 | NEW cpu=0:01.65 git= 12 +t=40s OLD cpu=0:03.35 git= 559 | NEW cpu=0:01.67 git= 15 +t=50s OLD cpu=0:03.55 git= 679 | NEW cpu=0:01.69 git= 18 +t=60s OLD cpu=0:03.76 git= 799 | NEW cpu=0:01.71 git= 21 +``` + +| Metric | OLD (polling) | NEW (evented) | Improvement | +| -------------------------------- | ------------- | ------------- | ------------- | +| Git invocations in 60s | 799 | 21 | **38× fewer** | +| Git calls/second | ~13.3/s | ~0.35/s | — | +| CPU time consumed (idle portion) | ~1.50s | ~0.13s | **~12× less** | + +The old version fires multiple git commands per 250ms poll cycle (~13/s). The new version fires a small batch every ~10 seconds (safety poll only). + +### Startup time (5 trials) + +| Trial | OLD | NEW | +| -------- | ---------- | ---------- | +| 1 | 1708ms | 1674ms | +| 2 | 1669ms | 1703ms | +| 3 | 1671ms | 1670ms | +| 4 | 1662ms | 1756ms | +| 5 | 1684ms | 1691ms | +| **Mean** | **1679ms** | **1699ms** | + +Startup time is identical within noise (~1.7s). Both versions are dominated by the initial `git diff` load cost on this repo. The PR's improvement is entirely in the idle steady-state after the diff is rendered. + +### Refresh latency (evented PR, separate E2E test repo) + +| Scenario | Latency | +| --------------------------- | ------- | +| Simple tracked-file write | ~340ms | +| Atomic temp-file + rename | ~418ms | +| Large 241-line diff update | ~406ms | +| New untracked file creation | ~682ms | + +All refreshes were passive — no keyboard or mouse input was sent to hunk. + +## Conclusion + +The evented observer eliminates virtually all idle CPU and subprocess overhead while maintaining identical startup performance and sub-second passive refresh latency. The improvement scales with the number of open `--watch` sessions: each idle tab drops from ~13 git calls/second to ~0.35.