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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
27c8cfbd4b
commit
4b12ebd5c0
@@ -0,0 +1,153 @@
|
|||||||
|
commit 07b077f1a28e7126f9d2b48be03bca8486173aac
|
||||||
|
Author: Mohamed Boudra <boudra.moha@gmail.com>
|
||||||
|
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 <noreply@anthropic.com>
|
||||||
|
|
||||||
|
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<typeof vi.fn> }> = [];
|
||||||
|
+ 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<typeof import("node:fs/promises")>("node:fs/promises");
|
||||||
|
+ return {
|
||||||
|
+ ...actual,
|
||||||
|
+ readdir: readdirMock,
|
||||||
|
+ };
|
||||||
|
+});
|
||||||
|
+
|
||||||
|
+vi.mock("node:fs", async () => {
|
||||||
|
+ const actual = await vi.importActual<typeof import("node:fs")>("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();
|
||||||
|
+ });
|
||||||
|
+});
|
||||||
@@ -1,4 +1,6 @@
|
|||||||
import { watch, type FSWatcher } from "node:fs";
|
import { watch, type FSWatcher } from "node:fs";
|
||||||
|
import { readdir } from "node:fs/promises";
|
||||||
|
import { join } from "node:path";
|
||||||
import { exec } from "child_process";
|
import { exec } from "child_process";
|
||||||
import { promisify } from "util";
|
import { promisify } from "util";
|
||||||
import type pino from "pino";
|
import type pino from "pino";
|
||||||
@@ -39,6 +41,10 @@ type CheckoutDiffWatchTarget = {
|
|||||||
refreshQueued: boolean;
|
refreshQueued: boolean;
|
||||||
latestPayload: CheckoutDiffSnapshotPayload | null;
|
latestPayload: CheckoutDiffSnapshotPayload | null;
|
||||||
latestFingerprint: string | null;
|
latestFingerprint: string | null;
|
||||||
|
watchedPaths: Set<string>;
|
||||||
|
repoWatchPath: string | null;
|
||||||
|
linuxTreeRefreshPromise: Promise<void> | null;
|
||||||
|
linuxTreeRefreshQueued: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
export class CheckoutDiffManager {
|
export class CheckoutDiffManager {
|
||||||
@@ -145,6 +151,7 @@ export class CheckoutDiffManager {
|
|||||||
watcher.close();
|
watcher.close();
|
||||||
}
|
}
|
||||||
target.watchers = [];
|
target.watchers = [];
|
||||||
|
target.watchedPaths.clear();
|
||||||
target.listeners.clear();
|
target.listeners.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -276,9 +283,14 @@ export class CheckoutDiffManager {
|
|||||||
refreshQueued: false,
|
refreshQueued: false,
|
||||||
latestPayload: null,
|
latestPayload: null,
|
||||||
latestFingerprint: null,
|
latestFingerprint: null,
|
||||||
|
watchedPaths: new Set<string>(),
|
||||||
|
repoWatchPath: null,
|
||||||
|
linuxTreeRefreshPromise: null,
|
||||||
|
linuxTreeRefreshQueued: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
const repoWatchPath = watchRoot ?? cwd;
|
const repoWatchPath = watchRoot ?? cwd;
|
||||||
|
target.repoWatchPath = repoWatchPath;
|
||||||
const watchPaths = new Set<string>([repoWatchPath]);
|
const watchPaths = new Set<string>([repoWatchPath]);
|
||||||
const gitDir = await resolveCheckoutGitDir(cwd);
|
const gitDir = await resolveCheckoutGitDir(cwd);
|
||||||
if (gitDir) {
|
if (gitDir) {
|
||||||
@@ -287,52 +299,15 @@ export class CheckoutDiffManager {
|
|||||||
|
|
||||||
let hasRecursiveRepoCoverage = false;
|
let hasRecursiveRepoCoverage = false;
|
||||||
const allowRecursiveRepoWatch = process.platform !== "linux";
|
const allowRecursiveRepoWatch = process.platform !== "linux";
|
||||||
|
if (process.platform === "linux") {
|
||||||
|
hasRecursiveRepoCoverage = await this.ensureLinuxRepoTreeWatchers(target, repoWatchPath);
|
||||||
|
}
|
||||||
for (const watchPath of watchPaths) {
|
for (const watchPath of watchPaths) {
|
||||||
const shouldTryRecursive = watchPath === repoWatchPath && allowRecursiveRepoWatch;
|
if (process.platform === "linux" && watchPath === repoWatchPath) {
|
||||||
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) {
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
const shouldTryRecursive = watchPath === repoWatchPath && allowRecursiveRepoWatch;
|
||||||
watcher.on("error", (error) => {
|
const watcherIsRecursive = this.addWatcher(target, watchPath, shouldTryRecursive);
|
||||||
this.logger.warn({ err: error, watchPath, cwd, compare }, "Checkout diff watcher error");
|
|
||||||
});
|
|
||||||
target.watchers.push(watcher);
|
|
||||||
if (watchPath === repoWatchPath && watcherIsRecursive) {
|
if (watchPath === repoWatchPath && watcherIsRecursive) {
|
||||||
hasRecursiveRepoCoverage = true;
|
hasRecursiveRepoCoverage = true;
|
||||||
}
|
}
|
||||||
@@ -358,4 +333,148 @@ export class CheckoutDiffManager {
|
|||||||
this.targets.set(targetKey, target);
|
this.targets.set(targetKey, target);
|
||||||
return 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<boolean> {
|
||||||
|
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<void> {
|
||||||
|
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<string[]> {
|
||||||
|
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;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user