From 956f20a572730158f8728b915b8243d4c67d95fd Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 19 May 2026 15:27:16 +0200 Subject: [PATCH] fix(coding-agent): use async filesystem operations in tools --- packages/coding-agent/src/core/tools/bash.ts | 121 ++++++++++-------- .../src/core/tools/file-mutation-queue.ts | 46 +++++-- packages/coding-agent/src/core/tools/find.ts | 12 +- packages/coding-agent/src/core/tools/grep.ts | 6 +- packages/coding-agent/src/core/tools/ls.ts | 16 ++- .../coding-agent/src/core/tools/path-utils.ts | 44 +++++++ packages/coding-agent/src/core/tools/read.ts | 7 +- 7 files changed, 172 insertions(+), 80 deletions(-) diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index 207285005..7ec7f17da 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -1,4 +1,5 @@ -import { existsSync } from "node:fs"; +import { constants } from "node:fs"; +import { access as fsAccess } from "node:fs/promises"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import { Container, Text, truncateToWidth } from "@earendil-works/pi-tui"; import { spawn } from "child_process"; @@ -66,62 +67,70 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas return { exec: (command, cwd, { onData, signal, timeout, env }) => { return new Promise((resolve, reject) => { - const { shell, args } = getShellConfig(options?.shellPath); - if (!existsSync(cwd)) { - reject(new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`)); - return; - } - const child = spawn(shell, [...args, command], { - cwd, - detached: process.platform !== "win32", - env: env ?? getShellEnv(), - stdio: ["ignore", "pipe", "pipe"], - windowsHide: true, - }); - if (child.pid) trackDetachedChildPid(child.pid); - let timedOut = false; - let timeoutHandle: NodeJS.Timeout | undefined; - // Set timeout if provided. - if (timeout !== undefined && timeout > 0) { - timeoutHandle = setTimeout(() => { - timedOut = true; - if (child.pid) killProcessTree(child.pid); - }, timeout * 1000); - } - // Stream stdout and stderr. - child.stdout?.on("data", onData); - child.stderr?.on("data", onData); - // Handle abort signal by killing the entire process tree. - const onAbort = () => { - if (child.pid) killProcessTree(child.pid); - }; - if (signal) { - if (signal.aborted) onAbort(); - else signal.addEventListener("abort", onAbort, { once: true }); - } - // Handle shell spawn errors and wait for the process to terminate without hanging - // on inherited stdio handles held by detached descendants. - waitForChildProcess(child) - .then((code) => { - if (child.pid) untrackDetachedChildPid(child.pid); - if (timeoutHandle) clearTimeout(timeoutHandle); - if (signal) signal.removeEventListener("abort", onAbort); - if (signal?.aborted) { - reject(new Error("aborted")); - return; - } - if (timedOut) { - reject(new Error(`timeout:${timeout}`)); - return; - } - resolve({ exitCode: code }); - }) - .catch((err) => { - if (child.pid) untrackDetachedChildPid(child.pid); - if (timeoutHandle) clearTimeout(timeoutHandle); - if (signal) signal.removeEventListener("abort", onAbort); - reject(err); + void (async () => { + const { shell, args } = getShellConfig(options?.shellPath); + try { + await fsAccess(cwd, constants.F_OK); + } catch { + reject(new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`)); + return; + } + if (signal?.aborted) { + reject(new Error("aborted")); + return; + } + const child = spawn(shell, [...args, command], { + cwd, + detached: process.platform !== "win32", + env: env ?? getShellEnv(), + stdio: ["ignore", "pipe", "pipe"], + windowsHide: true, }); + if (child.pid) trackDetachedChildPid(child.pid); + let timedOut = false; + let timeoutHandle: NodeJS.Timeout | undefined; + // Set timeout if provided. + if (timeout !== undefined && timeout > 0) { + timeoutHandle = setTimeout(() => { + timedOut = true; + if (child.pid) killProcessTree(child.pid); + }, timeout * 1000); + } + // Stream stdout and stderr. + child.stdout?.on("data", onData); + child.stderr?.on("data", onData); + // Handle abort signal by killing the entire process tree. + const onAbort = () => { + if (child.pid) killProcessTree(child.pid); + }; + if (signal) { + if (signal.aborted) onAbort(); + else signal.addEventListener("abort", onAbort, { once: true }); + } + // Handle shell spawn errors and wait for the process to terminate without hanging + // on inherited stdio handles held by detached descendants. + waitForChildProcess(child) + .then((code) => { + if (child.pid) untrackDetachedChildPid(child.pid); + if (timeoutHandle) clearTimeout(timeoutHandle); + if (signal) signal.removeEventListener("abort", onAbort); + if (signal?.aborted) { + reject(new Error("aborted")); + return; + } + if (timedOut) { + reject(new Error(`timeout:${timeout}`)); + return; + } + resolve({ exitCode: code }); + }) + .catch((err) => { + if (child.pid) untrackDetachedChildPid(child.pid); + if (timeoutHandle) clearTimeout(timeoutHandle); + if (signal) signal.removeEventListener("abort", onAbort); + reject(err); + }); + })().catch((err: unknown) => reject(err instanceof Error ? err : new Error(String(err)))); }); }, }; diff --git a/packages/coding-agent/src/core/tools/file-mutation-queue.ts b/packages/coding-agent/src/core/tools/file-mutation-queue.ts index 220112559..5505a7a27 100644 --- a/packages/coding-agent/src/core/tools/file-mutation-queue.ts +++ b/packages/coding-agent/src/core/tools/file-mutation-queue.ts @@ -1,14 +1,27 @@ -import { realpathSync } from "node:fs"; +import { realpath } from "node:fs/promises"; import { resolve } from "node:path"; const fileMutationQueues = new Map>(); +let registrationQueue = Promise.resolve(); -function getMutationQueueKey(filePath: string): string { +function isMissingPathError(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error.code === "ENOENT" || error.code === "ENOTDIR") + ); +} + +async function getMutationQueueKey(filePath: string): Promise { const resolvedPath = resolve(filePath); try { - return realpathSync.native(resolvedPath); - } catch { - return resolvedPath; + return await realpath(resolvedPath); + } catch (error) { + if (isMissingPathError(error)) { + return resolvedPath; + } + throw error; } } @@ -17,16 +30,25 @@ function getMutationQueueKey(filePath: string): string { * Operations for different files still run in parallel. */ export async function withFileMutationQueue(filePath: string, fn: () => Promise): Promise { - const key = getMutationQueueKey(filePath); - const currentQueue = fileMutationQueues.get(key) ?? Promise.resolve(); + const registration = registrationQueue.then(async () => { + const key = await getMutationQueueKey(filePath); + const currentQueue = fileMutationQueues.get(key) ?? Promise.resolve(); - let releaseNext!: () => void; - const nextQueue = new Promise((resolveQueue) => { - releaseNext = resolveQueue; + let releaseNext!: () => void; + const nextQueue = new Promise((resolveQueue) => { + releaseNext = resolveQueue; + }); + const chainedQueue = currentQueue.then(() => nextQueue); + fileMutationQueues.set(key, chainedQueue); + + return { key, currentQueue, chainedQueue, releaseNext }; }); - const chainedQueue = currentQueue.then(() => nextQueue); - fileMutationQueues.set(key, chainedQueue); + registrationQueue = registration.then( + () => undefined, + () => undefined, + ); + const { key, currentQueue, chainedQueue, releaseNext } = await registration; await currentQueue; try { return await fn(); diff --git a/packages/coding-agent/src/core/tools/find.ts b/packages/coding-agent/src/core/tools/find.ts index e2d552d4f..23af86893 100644 --- a/packages/coding-agent/src/core/tools/find.ts +++ b/packages/coding-agent/src/core/tools/find.ts @@ -1,8 +1,9 @@ +import { constants } from "node:fs"; +import { access as fsAccess } from "node:fs/promises"; import { createInterface } from "node:readline"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import { Text } from "@earendil-works/pi-tui"; import { spawn } from "child_process"; -import { existsSync } from "fs"; import path from "path"; import { type Static, Type } from "typebox"; import { keyHint } from "../../modes/interactive/components/keybinding-hints.js"; @@ -46,7 +47,14 @@ export interface FindOperations { } const defaultFindOperations: FindOperations = { - exists: existsSync, + exists: async (absolutePath) => { + try { + await fsAccess(absolutePath, constants.F_OK); + return true; + } catch { + return false; + } + }, // This is a placeholder. Actual fd execution happens in execute() when no custom glob is provided. glob: () => [], }; diff --git a/packages/coding-agent/src/core/tools/grep.ts b/packages/coding-agent/src/core/tools/grep.ts index 4d441b180..3c6b975db 100644 --- a/packages/coding-agent/src/core/tools/grep.ts +++ b/packages/coding-agent/src/core/tools/grep.ts @@ -1,8 +1,8 @@ +import { readFile as fsReadFile, stat as fsStat } from "node:fs/promises"; import { createInterface } from "node:readline"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import { Text } from "@earendil-works/pi-tui"; import { spawn } from "child_process"; -import { readFileSync, statSync } from "fs"; import path from "path"; import { type Static, Type } from "typebox"; import { keyHint } from "../../modes/interactive/components/keybinding-hints.js"; @@ -55,8 +55,8 @@ export interface GrepOperations { } const defaultGrepOperations: GrepOperations = { - isDirectory: (p) => statSync(p).isDirectory(), - readFile: (p) => readFileSync(p, "utf-8"), + isDirectory: async (p) => (await fsStat(p)).isDirectory(), + readFile: (p) => fsReadFile(p, "utf-8"), }; export interface GrepToolOptions { diff --git a/packages/coding-agent/src/core/tools/ls.ts b/packages/coding-agent/src/core/tools/ls.ts index e6ae22c75..c23927a33 100644 --- a/packages/coding-agent/src/core/tools/ls.ts +++ b/packages/coding-agent/src/core/tools/ls.ts @@ -1,6 +1,7 @@ +import { constants } from "node:fs"; +import { access as fsAccess, readdir as fsReaddir, stat as fsStat } from "node:fs/promises"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import { Text } from "@earendil-works/pi-tui"; -import { existsSync, readdirSync, statSync } from "fs"; import nodePath from "path"; import { type Static, Type } from "typebox"; import { keyHint } from "../../modes/interactive/components/keybinding-hints.js"; @@ -38,9 +39,16 @@ export interface LsOperations { } const defaultLsOperations: LsOperations = { - exists: existsSync, - stat: statSync, - readdir: readdirSync, + exists: async (absolutePath) => { + try { + await fsAccess(absolutePath, constants.F_OK); + return true; + } catch { + return false; + } + }, + stat: fsStat, + readdir: fsReaddir, }; export interface LsToolOptions { diff --git a/packages/coding-agent/src/core/tools/path-utils.ts b/packages/coding-agent/src/core/tools/path-utils.ts index 0c10b52ec..a677d8dcd 100644 --- a/packages/coding-agent/src/core/tools/path-utils.ts +++ b/packages/coding-agent/src/core/tools/path-utils.ts @@ -1,4 +1,5 @@ import { accessSync, constants } from "node:fs"; +import { access } from "node:fs/promises"; import * as os from "node:os"; import { isAbsolute, resolve as resolvePath } from "node:path"; @@ -32,6 +33,15 @@ function fileExists(filePath: string): boolean { } } +async function fileExistsAsync(filePath: string): Promise { + try { + await access(filePath, constants.F_OK); + return true; + } catch { + return false; + } +} + function normalizeAtPrefix(filePath: string): string { return filePath.startsWith("@") ? filePath.slice(1) : filePath; } @@ -92,3 +102,37 @@ export function resolveReadPath(filePath: string, cwd: string): string { return resolved; } + +export async function resolveReadPathAsync(filePath: string, cwd: string): Promise { + const resolved = resolveToCwd(filePath, cwd); + + if (await fileExistsAsync(resolved)) { + return resolved; + } + + // Try macOS AM/PM variant (narrow no-break space before AM/PM) + const amPmVariant = tryMacOSScreenshotPath(resolved); + if (amPmVariant !== resolved && (await fileExistsAsync(amPmVariant))) { + return amPmVariant; + } + + // Try NFD variant (macOS stores filenames in NFD form) + const nfdVariant = tryNFDVariant(resolved); + if (nfdVariant !== resolved && (await fileExistsAsync(nfdVariant))) { + return nfdVariant; + } + + // Try curly quote variant (macOS uses U+2019 in screenshot names) + const curlyVariant = tryCurlyQuoteVariant(resolved); + if (curlyVariant !== resolved && (await fileExistsAsync(curlyVariant))) { + return curlyVariant; + } + + // Try combined NFD + curly quote (for French macOS screenshots like "Capture d'écran") + const nfdCurlyVariant = tryCurlyQuoteVariant(nfdVariant); + if (nfdCurlyVariant !== resolved && (await fileExistsAsync(nfdCurlyVariant))) { + return nfdCurlyVariant; + } + + return resolved; +} diff --git a/packages/coding-agent/src/core/tools/read.ts b/packages/coding-agent/src/core/tools/read.ts index 867d04dcf..0fd210073 100644 --- a/packages/coding-agent/src/core/tools/read.ts +++ b/packages/coding-agent/src/core/tools/read.ts @@ -12,7 +12,7 @@ import { formatDimensionNote, resizeImage } from "../../utils/image-resize.js"; import { detectSupportedImageMimeTypeFromFile } from "../../utils/mime.js"; import { formatPathRelativeToCwdOrAbsolute } from "../../utils/paths.js"; import type { ToolDefinition, ToolRenderResultOptions } from "../extensions/types.js"; -import { resolveReadPath } from "./path-utils.js"; +import { resolveReadPathAsync, resolveToCwd } from "./path-utils.js"; import { getTextOutput, invalidArgText, replaceTabs, shortenPath, str } from "./render-utils.js"; import { wrapToolDefinition } from "./tool-definition-wrapper.js"; import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize, type TruncationResult, truncateHead } from "./truncate.js"; @@ -124,7 +124,7 @@ function getCompactReadClassification( const rawPath = str(args?.file_path ?? args?.path); if (!rawPath) return undefined; - const absolutePath = resolveReadPath(rawPath, cwd); + const absolutePath = resolveToCwd(rawPath, cwd); const fileName = basename(absolutePath); if (fileName === "SKILL.md") { return { kind: "skill", label: basename(dirname(absolutePath)) || fileName }; @@ -223,7 +223,6 @@ export function createReadToolDefinition( _onUpdate?, ctx?, ) { - const absolutePath = resolveReadPath(path, cwd); return new Promise<{ content: (TextContent | ImageContent)[]; details: ReadToolDetails | undefined }>( (resolve, reject) => { if (signal?.aborted) { @@ -239,6 +238,8 @@ export function createReadToolDefinition( (async () => { try { + const absolutePath = await resolveReadPathAsync(path, cwd); + if (aborted) return; // Check if file exists and is readable. await ops.access(absolutePath); if (aborted) return;