From 4b12ebd5c00e2e6d20fe062d57d5ea4234626d4f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 1 Apr 2026 04:46:07 +0000 Subject: [PATCH] fix: Linux checkout diff watchers using non-recursive directory watching Use non-recursive directory watchers on Linux since recursive fs.watch is not supported. Dynamically discover and watch subdirectories, refreshing the watcher tree on changes. Co-Authored-By: Claude Opus 4.6 --- .../src/server/checkout-diff-manager.test.ts | 153 +++++++++++++ .../src/server/checkout-diff-manager.ts | 205 ++++++++++++++---- 2 files changed, 315 insertions(+), 43 deletions(-) create mode 100644 packages/server/src/server/checkout-diff-manager.test.ts diff --git a/packages/server/src/server/checkout-diff-manager.test.ts b/packages/server/src/server/checkout-diff-manager.test.ts new file mode 100644 index 000000000..1f5991741 --- /dev/null +++ b/packages/server/src/server/checkout-diff-manager.test.ts @@ -0,0 +1,153 @@ +commit 07b077f1a28e7126f9d2b48be03bca8486173aac +Author: Mohamed Boudra +Date: Wed Apr 1 04:38:45 2026 +0000 + + fix: Linux checkout diff watchers and rewind command ordering + + Use non-recursive directory watchers on Linux since recursive fs.watch + is not supported. Dynamically discover and watch subdirectories, refreshing + the watcher tree on changes. Also pin rewind command first in the slash + command list and remove stale .gitignore entry. + + Co-Authored-By: Claude Opus 4.6 + +diff --git a/packages/server/src/server/checkout-diff-manager.test.ts b/packages/server/src/server/checkout-diff-manager.test.ts +new file mode 100644 +index 00000000..22f0fda8 +--- /dev/null ++++ b/packages/server/src/server/checkout-diff-manager.test.ts +@@ -0,0 +1,134 @@ ++import path from "node:path"; ++import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; ++ ++const { execMock, getCheckoutDiffMock, resolveCheckoutGitDirMock, readdirMock, watchCalls } = ++ vi.hoisted(() => { ++ const hoistedWatchCalls: Array<{ path: string; close: ReturnType }> = []; ++ return { ++ execMock: vi.fn((_command: string, _options: unknown, callback: (error: null, result: { stdout: string; stderr: string }) => void) => { ++ callback(null, { stdout: "/tmp/repo\n", stderr: "" }); ++ }), ++ getCheckoutDiffMock: vi.fn(async () => ({ diff: "", structured: [] })), ++ resolveCheckoutGitDirMock: vi.fn(async () => "/tmp/repo/.git"), ++ readdirMock: vi.fn(async (directory: string) => { ++ if (directory === "/tmp/repo") { ++ return [ ++ { name: "packages", isDirectory: () => true }, ++ { name: ".git", isDirectory: () => true }, ++ { name: "README.md", isDirectory: () => false }, ++ ]; ++ } ++ if (directory === path.join("/tmp/repo", "packages")) { ++ return [ ++ { name: "server", isDirectory: () => true }, ++ { name: "app", isDirectory: () => true }, ++ ]; ++ } ++ if (directory === path.join("/tmp/repo", "packages", "server")) { ++ return [{ name: "src", isDirectory: () => true }]; ++ } ++ if (directory === path.join("/tmp/repo", "packages", "server", "src")) { ++ return [{ name: "server", isDirectory: () => true }]; ++ } ++ return []; ++ }), ++ watchCalls: hoistedWatchCalls, ++ }; ++ }); ++ ++vi.mock("child_process", () => ({ ++ exec: execMock, ++})); ++ ++vi.mock("node:fs/promises", async () => { ++ const actual = await vi.importActual("node:fs/promises"); ++ return { ++ ...actual, ++ readdir: readdirMock, ++ }; ++}); ++ ++vi.mock("node:fs", async () => { ++ const actual = await vi.importActual("node:fs"); ++ return { ++ ...actual, ++ watch: vi.fn((watchPath: string) => { ++ const close = vi.fn(); ++ watchCalls.push({ path: watchPath, close }); ++ return { ++ close, ++ on: vi.fn().mockReturnThis(), ++ } as any; ++ }), ++ }; ++}); ++ ++vi.mock("../utils/checkout-git.js", () => ({ ++ getCheckoutDiff: getCheckoutDiffMock, ++})); ++ ++vi.mock("./checkout-git-utils.js", () => ({ ++ READ_ONLY_GIT_ENV: {}, ++ resolveCheckoutGitDir: resolveCheckoutGitDirMock, ++ toCheckoutError: vi.fn((error: unknown) => ({ ++ message: error instanceof Error ? error.message : String(error), ++ })), ++})); ++ ++import { CheckoutDiffManager } from "./checkout-diff-manager.js"; ++ ++describe("CheckoutDiffManager Linux watchers", () => { ++ const originalPlatform = process.platform; ++ ++ beforeEach(() => { ++ watchCalls.length = 0; ++ execMock.mockClear(); ++ getCheckoutDiffMock.mockClear(); ++ resolveCheckoutGitDirMock.mockClear(); ++ readdirMock.mockClear(); ++ Object.defineProperty(process, "platform", { ++ configurable: true, ++ value: "linux", ++ }); ++ }); ++ ++ afterEach(() => { ++ Object.defineProperty(process, "platform", { ++ configurable: true, ++ value: originalPlatform, ++ }); ++ }); ++ ++ test("watches nested repository directories on Linux", async () => { ++ const logger = { ++ child: () => logger, ++ warn: vi.fn(), ++ }; ++ const manager = new CheckoutDiffManager({ ++ logger: logger as any, ++ paseoHome: "/tmp/paseo-test", ++ }); ++ ++ const subscription = await manager.subscribe( ++ { ++ cwd: path.join("/tmp/repo", "packages", "server"), ++ compare: { mode: "uncommitted" }, ++ }, ++ () => {}, ++ ); ++ ++ expect(subscription.initial.error).toBeNull(); ++ expect(watchCalls.map((entry) => entry.path).sort()).toEqual([ ++ "/tmp/repo", ++ "/tmp/repo/.git", ++ "/tmp/repo/packages", ++ "/tmp/repo/packages/app", ++ "/tmp/repo/packages/server", ++ "/tmp/repo/packages/server/src", ++ "/tmp/repo/packages/server/src/server", ++ ]); ++ ++ subscription.unsubscribe(); ++ manager.dispose(); ++ }); ++}); diff --git a/packages/server/src/server/checkout-diff-manager.ts b/packages/server/src/server/checkout-diff-manager.ts index db7a9096b..cd6372133 100644 --- a/packages/server/src/server/checkout-diff-manager.ts +++ b/packages/server/src/server/checkout-diff-manager.ts @@ -1,4 +1,6 @@ import { watch, type FSWatcher } from "node:fs"; +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; import { exec } from "child_process"; import { promisify } from "util"; import type pino from "pino"; @@ -39,6 +41,10 @@ type CheckoutDiffWatchTarget = { refreshQueued: boolean; latestPayload: CheckoutDiffSnapshotPayload | null; latestFingerprint: string | null; + watchedPaths: Set; + repoWatchPath: string | null; + linuxTreeRefreshPromise: Promise | null; + linuxTreeRefreshQueued: boolean; }; export class CheckoutDiffManager { @@ -145,6 +151,7 @@ export class CheckoutDiffManager { watcher.close(); } target.watchers = []; + target.watchedPaths.clear(); target.listeners.clear(); } @@ -276,9 +283,14 @@ export class CheckoutDiffManager { refreshQueued: false, latestPayload: null, latestFingerprint: null, + watchedPaths: new Set(), + repoWatchPath: null, + linuxTreeRefreshPromise: null, + linuxTreeRefreshQueued: false, }; const repoWatchPath = watchRoot ?? cwd; + target.repoWatchPath = repoWatchPath; const watchPaths = new Set([repoWatchPath]); const gitDir = await resolveCheckoutGitDir(cwd); if (gitDir) { @@ -287,52 +299,15 @@ export class CheckoutDiffManager { let hasRecursiveRepoCoverage = false; const allowRecursiveRepoWatch = process.platform !== "linux"; + if (process.platform === "linux") { + hasRecursiveRepoCoverage = await this.ensureLinuxRepoTreeWatchers(target, repoWatchPath); + } for (const watchPath of watchPaths) { - const shouldTryRecursive = watchPath === repoWatchPath && allowRecursiveRepoWatch; - const createWatcher = (recursive: boolean): FSWatcher => - watch(watchPath, { recursive }, () => { - this.scheduleTargetRefresh(target); - }); - - let watcher: FSWatcher | null = null; - let watcherIsRecursive = false; - try { - if (shouldTryRecursive) { - watcher = createWatcher(true); - watcherIsRecursive = true; - } else { - watcher = createWatcher(false); - } - } catch (error) { - if (shouldTryRecursive) { - try { - watcher = createWatcher(false); - this.logger.warn( - { err: error, watchPath, cwd, compare }, - "Checkout diff recursive watch unavailable; using non-recursive fallback", - ); - } catch (fallbackError) { - this.logger.warn( - { err: fallbackError, watchPath, cwd, compare }, - "Failed to start checkout diff watcher", - ); - } - } else { - this.logger.warn( - { err: error, watchPath, cwd, compare }, - "Failed to start checkout diff watcher", - ); - } - } - - if (!watcher) { + if (process.platform === "linux" && watchPath === repoWatchPath) { continue; } - - watcher.on("error", (error) => { - this.logger.warn({ err: error, watchPath, cwd, compare }, "Checkout diff watcher error"); - }); - target.watchers.push(watcher); + const shouldTryRecursive = watchPath === repoWatchPath && allowRecursiveRepoWatch; + const watcherIsRecursive = this.addWatcher(target, watchPath, shouldTryRecursive); if (watchPath === repoWatchPath && watcherIsRecursive) { hasRecursiveRepoCoverage = true; } @@ -358,4 +333,148 @@ export class CheckoutDiffManager { this.targets.set(targetKey, target); return target; } + + private addWatcher( + target: CheckoutDiffWatchTarget, + watchPath: string, + shouldTryRecursive: boolean, + ): boolean { + if (target.watchedPaths.has(watchPath)) { + return false; + } + + const { cwd, compare } = target; + const onChange = () => { + if (process.platform === "linux" && target.repoWatchPath) { + void this.refreshLinuxRepoTreeWatchers(target); + } + this.scheduleTargetRefresh(target); + }; + const createWatcher = (recursive: boolean): FSWatcher => + watch(watchPath, { recursive }, () => { + onChange(); + }); + + let watcher: FSWatcher | null = null; + let watcherIsRecursive = false; + try { + if (shouldTryRecursive) { + watcher = createWatcher(true); + watcherIsRecursive = true; + } else { + watcher = createWatcher(false); + } + } catch (error) { + if (shouldTryRecursive) { + try { + watcher = createWatcher(false); + this.logger.warn( + { err: error, watchPath, cwd, compare }, + "Checkout diff recursive watch unavailable; using non-recursive fallback", + ); + } catch (fallbackError) { + this.logger.warn( + { err: fallbackError, watchPath, cwd, compare }, + "Failed to start checkout diff watcher", + ); + } + } else { + this.logger.warn( + { err: error, watchPath, cwd, compare }, + "Failed to start checkout diff watcher", + ); + } + } + + if (!watcher) { + return false; + } + + watcher.on("error", (error) => { + this.logger.warn({ err: error, watchPath, cwd, compare }, "Checkout diff watcher error"); + }); + target.watchers.push(watcher); + target.watchedPaths.add(watchPath); + return watcherIsRecursive; + } + + private async ensureLinuxRepoTreeWatchers( + target: CheckoutDiffWatchTarget, + rootPath: string, + ): Promise { + const directories = await this.listLinuxWatchDirectories(rootPath); + let complete = true; + for (const directory of directories) { + const watcherWasRecursive = this.addWatcher(target, directory, false); + if (!watcherWasRecursive && !target.watchedPaths.has(directory)) { + complete = false; + } + } + return complete && target.watchedPaths.has(rootPath); + } + + private async refreshLinuxRepoTreeWatchers(target: CheckoutDiffWatchTarget): Promise { + if (process.platform !== "linux" || !target.repoWatchPath) { + return; + } + const rootPath = target.repoWatchPath; + if (target.linuxTreeRefreshPromise) { + target.linuxTreeRefreshQueued = true; + return; + } + + target.linuxTreeRefreshPromise = (async () => { + do { + target.linuxTreeRefreshQueued = false; + try { + await this.ensureLinuxRepoTreeWatchers(target, rootPath); + } catch (error) { + this.logger.warn( + { + err: error, + cwd: target.cwd, + compare: target.compare, + rootPath, + }, + "Failed to refresh Linux checkout diff tree watchers", + ); + } + } while (target.linuxTreeRefreshQueued); + })(); + + try { + await target.linuxTreeRefreshPromise; + } finally { + target.linuxTreeRefreshPromise = null; + } + } + + private async listLinuxWatchDirectories(rootPath: string): Promise { + const directories: string[] = []; + const pending = [rootPath]; + + while (pending.length > 0) { + const directory = pending.pop(); + if (!directory) { + continue; + } + directories.push(directory); + + let entries; + try { + entries = await readdir(directory, { withFileTypes: true }); + } catch { + continue; + } + + for (const entry of entries) { + if (!entry.isDirectory() || entry.name === ".git") { + continue; + } + pending.push(join(directory, entry.name)); + } + } + + return directories; + } }