From 27c8cfbd4b86dd67f4475345406490edd47ee9f1 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 1 Apr 2026 04:44:29 +0000 Subject: [PATCH] Revert "fix: Linux checkout diff watchers and rewind command ordering" This reverts commit 07b077f1a28e7126f9d2b48be03bca8486173aac. --- .gitignore | 1 + .../server/agent/providers/claude-agent.ts | 6 +- ...ude-agent.voice-history-regression.test.ts | 1 - .../src/server/checkout-diff-manager.test.ts | 134 ------------ .../src/server/checkout-diff-manager.ts | 205 ++++-------------- 5 files changed, 45 insertions(+), 302 deletions(-) delete mode 100644 packages/server/src/server/checkout-diff-manager.test.ts diff --git a/.gitignore b/.gitignore index f7520743b..f5c8aac8e 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,4 @@ packages/server/src/server/fixtures/dictation/dictation-debug-largest.transcript /artifacts packages/desktop/.cache/ +packages/desktop/src-tauri/resources/managed-runtime/ diff --git a/packages/server/src/server/agent/providers/claude-agent.ts b/packages/server/src/server/agent/providers/claude-agent.ts index b846c829f..9e2dd9237 100644 --- a/packages/server/src/server/agent/providers/claude-agent.ts +++ b/packages/server/src/server/agent/providers/claude-agent.ts @@ -1589,11 +1589,7 @@ class ClaudeAgentSession implements AgentSession { if (!commandMap.has(REWIND_COMMAND_NAME)) { commandMap.set(REWIND_COMMAND_NAME, REWIND_COMMAND); } - const rewindCommand = commandMap.get(REWIND_COMMAND_NAME); - const remainingCommands = Array.from(commandMap.values()) - .filter((command) => command.name !== REWIND_COMMAND_NAME) - .sort((a, b) => a.name.localeCompare(b.name)); - return rewindCommand ? [rewindCommand, ...remainingCommands] : remainingCommands; + return Array.from(commandMap.values()).sort((a, b) => a.name.localeCompare(b.name)); } private resolveSlashCommandInvocation(prompt: AgentPromptInput): SlashCommandInvocation | null { diff --git a/packages/server/src/server/agent/providers/claude-agent.voice-history-regression.test.ts b/packages/server/src/server/agent/providers/claude-agent.voice-history-regression.test.ts index 2d5139a5d..2d2eb583c 100644 --- a/packages/server/src/server/agent/providers/claude-agent.voice-history-regression.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent.voice-history-regression.test.ts @@ -234,7 +234,6 @@ describe("ClaudeAgentSession history replay regression", () => { try { const commands = await session.listCommands?.(); expect(commands?.some((command) => command.name === "rewind")).toBe(true); - expect(commands?.[0]?.name).toBe("rewind"); } finally { await session.close(); } diff --git a/packages/server/src/server/checkout-diff-manager.test.ts b/packages/server/src/server/checkout-diff-manager.test.ts deleted file mode 100644 index 22f0fda8c..000000000 --- a/packages/server/src/server/checkout-diff-manager.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -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 cd6372133..db7a9096b 100644 --- a/packages/server/src/server/checkout-diff-manager.ts +++ b/packages/server/src/server/checkout-diff-manager.ts @@ -1,6 +1,4 @@ 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"; @@ -41,10 +39,6 @@ type CheckoutDiffWatchTarget = { refreshQueued: boolean; latestPayload: CheckoutDiffSnapshotPayload | null; latestFingerprint: string | null; - watchedPaths: Set; - repoWatchPath: string | null; - linuxTreeRefreshPromise: Promise | null; - linuxTreeRefreshQueued: boolean; }; export class CheckoutDiffManager { @@ -151,7 +145,6 @@ export class CheckoutDiffManager { watcher.close(); } target.watchers = []; - target.watchedPaths.clear(); target.listeners.clear(); } @@ -283,14 +276,9 @@ 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) { @@ -299,15 +287,52 @@ 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) { - if (process.platform === "linux" && watchPath === repoWatchPath) { + 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) { continue; } - const shouldTryRecursive = watchPath === repoWatchPath && allowRecursiveRepoWatch; - const watcherIsRecursive = this.addWatcher(target, watchPath, shouldTryRecursive); + + watcher.on("error", (error) => { + this.logger.warn({ err: error, watchPath, cwd, compare }, "Checkout diff watcher error"); + }); + target.watchers.push(watcher); if (watchPath === repoWatchPath && watcherIsRecursive) { hasRecursiveRepoCoverage = true; } @@ -333,148 +358,4 @@ 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; - } }