diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index d09ed387..68c9e770 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -1354,6 +1354,36 @@ Use `promptGuidelines` to add tool-specific bullets to the default system prompt Note: Some models are idiots and include the @ prefix in tool path arguments. Built-in tools strip a leading @ before resolving paths. If your custom tool accepts a path, normalize a leading @ as well. +If your custom tool mutates files, use `withFileMutationQueue()` so it participates in the same per-file queue as built-in `edit` and `write`. This matters because tool calls run in parallel by default. Without the queue, two tools can read the same old file contents, compute different updates, and then whichever write lands last overwrites the other. + +Example failure case: your custom tool edits `foo.ts` while built-in `edit` also changes `foo.ts` in the same assistant turn. If your tool does not participate in the queue, both can read the original `foo.ts`, apply separate changes, and one of those changes is lost. + +Pass the real target file path to `withFileMutationQueue()`, not the raw user argument. Resolve it to an absolute path first, relative to `ctx.cwd` or your tool's working directory. For existing files, the helper canonicalizes through `realpath()`, so symlink aliases for the same file share one queue. For new files, it falls back to the resolved absolute path because there is nothing to `realpath()` yet. + +Queue the entire mutation window on that target path. That includes read-modify-write logic, not just the final write. + +```typescript +import { withFileMutationQueue } from "@mariozechner/pi-coding-agent"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { dirname, resolve } from "node:path"; + +async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + const absolutePath = resolve(ctx.cwd, params.path); + + return withFileMutationQueue(absolutePath, async () => { + await mkdir(dirname(absolutePath), { recursive: true }); + const current = await readFile(absolutePath, "utf8"); + const next = current.replace(params.oldText, params.newText); + await writeFile(absolutePath, next, "utf8"); + + return { + content: [{ type: "text", text: `Updated ${params.path}` }], + details: {}, + }; + }); +} +``` + ### Tool Definition ```typescript @@ -1617,7 +1647,7 @@ renderResult(result, { expanded, isPartial }, theme) { #### Keybinding Hints -Use `keyHint()` to display keybinding hints that respect user's keybinding configuration: +Use `keyHint()` to display keybinding hints that respect the active keybinding configuration: ```typescript import { keyHint } from "@mariozechner/pi-coding-agent"; @@ -1625,18 +1655,23 @@ import { keyHint } from "@mariozechner/pi-coding-agent"; renderResult(result, { expanded }, theme) { let text = theme.fg("success", "✓ Done"); if (!expanded) { - text += ` (${keyHint("expandTools", "to expand")})`; + text += ` (${keyHint("app.tools.expand", "to expand")})`; } return new Text(text, 0, 0); } ``` Available functions: -- `keyHint(action, description)` - Editor actions (e.g., `"expandTools"`, `"selectConfirm"`) -- `appKeyHint(keybindings, action, description)` - App actions (requires `KeybindingsManager`) -- `editorKey(action)` - Get raw key string for editor action +- `keyHint(keybinding, description)` - Formats a configured keybinding id such as `"app.tools.expand"` or `"tui.select.confirm"` +- `keyText(keybinding)` - Returns the raw configured key text for a keybinding id - `rawKeyHint(key, description)` - Format a raw key string +Use namespaced keybinding ids: +- Coding-agent ids use the `app.*` namespace, for example `app.tools.expand`, `app.editor.external`, `app.session.rename` +- Shared TUI ids use the `tui.*` namespace, for example `tui.select.confirm`, `tui.select.cancel`, `tui.input.tab` + +Custom editors and `ctx.ui.custom()` components receive `keybindings: KeybindingsManager` as an injected argument. They should use that injected manager directly instead of calling `getKeybindings()` or `setKeybindings()`. + #### Best Practices - Use `Text` with padding `(0, 0)` - the Box handles padding diff --git a/packages/coding-agent/examples/extensions/antigravity-image-gen.ts b/packages/coding-agent/examples/extensions/antigravity-image-gen.ts index 72a48111..bbb9d06a 100644 --- a/packages/coding-agent/examples/extensions/antigravity-image-gen.ts +++ b/packages/coding-agent/examples/extensions/antigravity-image-gen.ts @@ -30,7 +30,7 @@ import { existsSync, readFileSync } from "node:fs"; import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { StringEnum } from "@mariozechner/pi-ai"; -import { type ExtensionAPI, getAgentDir } from "@mariozechner/pi-coding-agent"; +import { type ExtensionAPI, getAgentDir, withFileMutationQueue } from "@mariozechner/pi-coding-agent"; import { type Static, Type } from "@sinclair/typebox"; const PROVIDER = "google-antigravity"; @@ -228,12 +228,14 @@ function imageExtension(mimeType: string): string { } async function saveImage(base64Data: string, mimeType: string, outputDir: string): Promise { - await mkdir(outputDir, { recursive: true }); const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); const ext = imageExtension(mimeType); const filename = `image-${timestamp}-${randomUUID().slice(0, 8)}.${ext}`; const filePath = join(outputDir, filename); - await writeFile(filePath, Buffer.from(base64Data, "base64")); + await withFileMutationQueue(filePath, async () => { + await mkdir(outputDir, { recursive: true }); + await writeFile(filePath, Buffer.from(base64Data, "base64")); + }); return filePath; } diff --git a/packages/coding-agent/examples/extensions/subagent/index.ts b/packages/coding-agent/examples/extensions/subagent/index.ts index 87a967ac..316979c4 100644 --- a/packages/coding-agent/examples/extensions/subagent/index.ts +++ b/packages/coding-agent/examples/extensions/subagent/index.ts @@ -19,7 +19,7 @@ import * as path from "node:path"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import type { Message } from "@mariozechner/pi-ai"; import { StringEnum } from "@mariozechner/pi-ai"; -import { type ExtensionAPI, getMarkdownTheme } from "@mariozechner/pi-coding-agent"; +import { type ExtensionAPI, getMarkdownTheme, withFileMutationQueue } from "@mariozechner/pi-coding-agent"; import { Container, Markdown, Spacer, Text } from "@mariozechner/pi-tui"; import { Type } from "@sinclair/typebox"; import { type AgentConfig, type AgentScope, discoverAgents } from "./agents.js"; @@ -207,11 +207,13 @@ async function mapWithConcurrencyLimit( return results; } -function writePromptToTempFile(agentName: string, prompt: string): { dir: string; filePath: string } { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-subagent-")); +async function writePromptToTempFile(agentName: string, prompt: string): Promise<{ dir: string; filePath: string }> { + const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "pi-subagent-")); const safeName = agentName.replace(/[^\w.-]+/g, "_"); const filePath = path.join(tmpDir, `prompt-${safeName}.md`); - fs.writeFileSync(filePath, prompt, { encoding: "utf-8", mode: 0o600 }); + await withFileMutationQueue(filePath, async () => { + await fs.promises.writeFile(filePath, prompt, { encoding: "utf-8", mode: 0o600 }); + }); return { dir: tmpDir, filePath }; } @@ -274,7 +276,7 @@ async function runSingleAgent( try { if (agent.systemPrompt.trim()) { - const tmp = writePromptToTempFile(agent.name, agent.systemPrompt); + const tmp = await writePromptToTempFile(agent.name, agent.systemPrompt); tmpPromptDir = tmp.dir; tmpPromptPath = tmp.filePath; args.push("--append-system-prompt", tmpPromptPath); diff --git a/packages/coding-agent/examples/extensions/tool-override.ts b/packages/coding-agent/examples/extensions/tool-override.ts index bf9a8028..5ba3444a 100644 --- a/packages/coding-agent/examples/extensions/tool-override.ts +++ b/packages/coding-agent/examples/extensions/tool-override.ts @@ -21,10 +21,10 @@ */ import type { TextContent } from "@mariozechner/pi-ai"; -import { type ExtensionAPI, getAgentDir } from "@mariozechner/pi-coding-agent"; +import { type ExtensionAPI, getAgentDir, withFileMutationQueue } from "@mariozechner/pi-coding-agent"; import { Type } from "@sinclair/typebox"; -import { appendFileSync, constants, readFileSync } from "fs"; -import { access, readFile } from "fs/promises"; +import { constants, readFileSync } from "fs"; +import { access, appendFile, readFile } from "fs/promises"; import { join, resolve } from "path"; const LOG_FILE = join(getAgentDir(), "read-access.log"); @@ -44,14 +44,16 @@ function isBlockedPath(path: string): boolean { return BLOCKED_PATTERNS.some((pattern) => pattern.test(path)); } -function logAccess(path: string, allowed: boolean, reason?: string) { +async function logAccess(path: string, allowed: boolean, reason?: string) { const timestamp = new Date().toISOString(); const status = allowed ? "ALLOWED" : "BLOCKED"; const msg = reason ? ` (${reason})` : ""; const line = `[${timestamp}] ${status}: ${path}${msg}\n`; try { - appendFileSync(LOG_FILE, line); + await withFileMutationQueue(LOG_FILE, async () => { + await appendFile(LOG_FILE, line); + }); } catch { // Ignore logging errors } @@ -77,7 +79,7 @@ export default function (pi: ExtensionAPI) { // Check if path is blocked if (isBlockedPath(absolutePath)) { - logAccess(absolutePath, false, "matches blocked pattern"); + await logAccess(absolutePath, false, "matches blocked pattern"); return { content: [ { @@ -90,7 +92,7 @@ export default function (pi: ExtensionAPI) { } // Log allowed access - logAccess(absolutePath, true); + await logAccess(absolutePath, true); // Perform the actual read (simplified implementation) try { diff --git a/packages/coding-agent/examples/extensions/truncated-tool.ts b/packages/coding-agent/examples/extensions/truncated-tool.ts index 0a0b3892..3a79a550 100644 --- a/packages/coding-agent/examples/extensions/truncated-tool.ts +++ b/packages/coding-agent/examples/extensions/truncated-tool.ts @@ -14,6 +14,7 @@ * built-in `grep` tool in src/core/tools/grep.ts for a more complete implementation. */ +import { mkdtemp, writeFile } from "node:fs/promises"; import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; import { DEFAULT_MAX_BYTES, @@ -21,11 +22,11 @@ import { formatSize, type TruncationResult, truncateHead, + withFileMutationQueue, } from "@mariozechner/pi-coding-agent"; import { Text } from "@mariozechner/pi-tui"; import { Type } from "@sinclair/typebox"; import { execSync } from "child_process"; -import { mkdtempSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; @@ -108,9 +109,11 @@ export default function (pi: ExtensionAPI) { if (truncation.truncated) { // Save full output to a temp file so LLM can access it if needed - const tempDir = mkdtempSync(join(tmpdir(), "pi-rg-")); + const tempDir = await mkdtemp(join(tmpdir(), "pi-rg-")); const tempFile = join(tempDir, "output.txt"); - writeFileSync(tempFile, output); + await withFileMutationQueue(tempFile, async () => { + await writeFile(tempFile, output, "utf8"); + }); details.truncation = truncation; details.fullOutputPath = tempFile; diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 8c572370..7ef0ac7f 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -35,6 +35,7 @@ import { readTool, type Tool, type ToolName, + withFileMutationQueue, writeTool, } from "./tools/index.js"; @@ -109,6 +110,7 @@ export { codingTools, readOnlyTools, allTools as allBuiltInTools, + withFileMutationQueue, // Tool factories (for custom cwd) createCodingTools, createReadOnlyTools, diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 1cfe8e2e..badca2db 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -11,6 +11,7 @@ import { restoreLineEndings, stripBom, } from "./edit-diff.js"; +import { withFileMutationQueue } from "./file-mutation-queue.js"; import { resolveToCwd } from "./path-utils.js"; const editSchema = Type.Object({ @@ -68,157 +69,161 @@ export function createEditTool(cwd: string, options?: EditToolOptions): AgentToo ) => { const absolutePath = resolveToCwd(path, cwd); - return new Promise<{ - content: Array<{ type: "text"; text: string }>; - details: EditToolDetails | undefined; - }>((resolve, reject) => { - // Check if already aborted - if (signal?.aborted) { - reject(new Error("Operation aborted")); - return; - } - - let aborted = false; - - // Set up abort handler - const onAbort = () => { - aborted = true; - reject(new Error("Operation aborted")); - }; - - if (signal) { - signal.addEventListener("abort", onAbort, { once: true }); - } - - // Perform the edit operation - (async () => { - try { - // Check if file exists - try { - await ops.access(absolutePath); - } catch { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject(new Error(`File not found: ${path}`)); + return withFileMutationQueue( + absolutePath, + () => + new Promise<{ + content: Array<{ type: "text"; text: string }>; + details: EditToolDetails | undefined; + }>((resolve, reject) => { + // Check if already aborted + if (signal?.aborted) { + reject(new Error("Operation aborted")); return; } - // Check if aborted before reading - if (aborted) { - return; - } + let aborted = false; - // Read the file - const buffer = await ops.readFile(absolutePath); - const rawContent = buffer.toString("utf-8"); + // Set up abort handler + const onAbort = () => { + aborted = true; + reject(new Error("Operation aborted")); + }; - // Check if aborted after reading - if (aborted) { - return; - } - - // Strip BOM before matching (LLM won't include invisible BOM in oldText) - const { bom, text: content } = stripBom(rawContent); - - const originalEnding = detectLineEnding(content); - const normalizedContent = normalizeToLF(content); - const normalizedOldText = normalizeToLF(oldText); - const normalizedNewText = normalizeToLF(newText); - - // Find the old text using fuzzy matching (tries exact match first, then fuzzy) - const matchResult = fuzzyFindText(normalizedContent, normalizedOldText); - - if (!matchResult.found) { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject( - new Error( - `Could not find the exact text in ${path}. The old text must match exactly including all whitespace and newlines.`, - ), - ); - return; - } - - // Count occurrences using fuzzy-normalized content for consistency - const fuzzyContent = normalizeForFuzzyMatch(normalizedContent); - const fuzzyOldText = normalizeForFuzzyMatch(normalizedOldText); - const occurrences = fuzzyContent.split(fuzzyOldText).length - 1; - - if (occurrences > 1) { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject( - new Error( - `Found ${occurrences} occurrences of the text in ${path}. The text must be unique. Please provide more context to make it unique.`, - ), - ); - return; - } - - // Check if aborted before writing - if (aborted) { - return; - } - - // Perform replacement using the matched text position - // When fuzzy matching was used, contentForReplacement is the normalized version - const baseContent = matchResult.contentForReplacement; - const newContent = - baseContent.substring(0, matchResult.index) + - normalizedNewText + - baseContent.substring(matchResult.index + matchResult.matchLength); - - // Verify the replacement actually changed something - if (baseContent === newContent) { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject( - new Error( - `No changes made to ${path}. The replacement produced identical content. This might indicate an issue with special characters or the text not existing as expected.`, - ), - ); - return; - } - - const finalContent = bom + restoreLineEndings(newContent, originalEnding); - await ops.writeFile(absolutePath, finalContent); - - // Check if aborted after writing - if (aborted) { - return; - } - - // Clean up abort handler if (signal) { - signal.removeEventListener("abort", onAbort); + signal.addEventListener("abort", onAbort, { once: true }); } - const diffResult = generateDiffString(baseContent, newContent); - resolve({ - content: [ - { - type: "text", - text: `Successfully replaced text in ${path}.`, - }, - ], - details: { diff: diffResult.diff, firstChangedLine: diffResult.firstChangedLine }, - }); - } catch (error: any) { - // Clean up abort handler - if (signal) { - signal.removeEventListener("abort", onAbort); - } + // Perform the edit operation + (async () => { + try { + // Check if file exists + try { + await ops.access(absolutePath); + } catch { + if (signal) { + signal.removeEventListener("abort", onAbort); + } + reject(new Error(`File not found: ${path}`)); + return; + } - if (!aborted) { - reject(error); - } - } - })(); - }); + // Check if aborted before reading + if (aborted) { + return; + } + + // Read the file + const buffer = await ops.readFile(absolutePath); + const rawContent = buffer.toString("utf-8"); + + // Check if aborted after reading + if (aborted) { + return; + } + + // Strip BOM before matching (LLM won't include invisible BOM in oldText) + const { bom, text: content } = stripBom(rawContent); + + const originalEnding = detectLineEnding(content); + const normalizedContent = normalizeToLF(content); + const normalizedOldText = normalizeToLF(oldText); + const normalizedNewText = normalizeToLF(newText); + + // Find the old text using fuzzy matching (tries exact match first, then fuzzy) + const matchResult = fuzzyFindText(normalizedContent, normalizedOldText); + + if (!matchResult.found) { + if (signal) { + signal.removeEventListener("abort", onAbort); + } + reject( + new Error( + `Could not find the exact text in ${path}. The old text must match exactly including all whitespace and newlines.`, + ), + ); + return; + } + + // Count occurrences using fuzzy-normalized content for consistency + const fuzzyContent = normalizeForFuzzyMatch(normalizedContent); + const fuzzyOldText = normalizeForFuzzyMatch(normalizedOldText); + const occurrences = fuzzyContent.split(fuzzyOldText).length - 1; + + if (occurrences > 1) { + if (signal) { + signal.removeEventListener("abort", onAbort); + } + reject( + new Error( + `Found ${occurrences} occurrences of the text in ${path}. The text must be unique. Please provide more context to make it unique.`, + ), + ); + return; + } + + // Check if aborted before writing + if (aborted) { + return; + } + + // Perform replacement using the matched text position + // When fuzzy matching was used, contentForReplacement is the normalized version + const baseContent = matchResult.contentForReplacement; + const newContent = + baseContent.substring(0, matchResult.index) + + normalizedNewText + + baseContent.substring(matchResult.index + matchResult.matchLength); + + // Verify the replacement actually changed something + if (baseContent === newContent) { + if (signal) { + signal.removeEventListener("abort", onAbort); + } + reject( + new Error( + `No changes made to ${path}. The replacement produced identical content. This might indicate an issue with special characters or the text not existing as expected.`, + ), + ); + return; + } + + const finalContent = bom + restoreLineEndings(newContent, originalEnding); + await ops.writeFile(absolutePath, finalContent); + + // Check if aborted after writing + if (aborted) { + return; + } + + // Clean up abort handler + if (signal) { + signal.removeEventListener("abort", onAbort); + } + + const diffResult = generateDiffString(baseContent, newContent); + resolve({ + content: [ + { + type: "text", + text: `Successfully replaced text in ${path}.`, + }, + ], + details: { diff: diffResult.diff, firstChangedLine: diffResult.firstChangedLine }, + }); + } catch (error: any) { + // Clean up abort handler + if (signal) { + signal.removeEventListener("abort", onAbort); + } + + if (!aborted) { + reject(error); + } + } + })(); + }), + ); }, }; } diff --git a/packages/coding-agent/src/core/tools/file-mutation-queue.ts b/packages/coding-agent/src/core/tools/file-mutation-queue.ts new file mode 100644 index 00000000..60f468fe --- /dev/null +++ b/packages/coding-agent/src/core/tools/file-mutation-queue.ts @@ -0,0 +1,39 @@ +import { realpath } from "node:fs/promises"; +import { resolve } from "node:path"; + +const fileMutationQueues = new Map>(); + +async function getMutationQueueKey(filePath: string): Promise { + const resolvedPath = resolve(filePath); + try { + return await realpath(resolvedPath); + } catch { + return resolvedPath; + } +} + +/** + * Serialize file mutation operations targeting the same file. + * Operations for different files still run in parallel. + */ +export async function withFileMutationQueue(filePath: string, fn: () => Promise): Promise { + const key = await getMutationQueueKey(filePath); + const currentQueue = fileMutationQueues.get(key) ?? Promise.resolve(); + + let releaseNext!: () => void; + const nextQueue = new Promise((resolveQueue) => { + releaseNext = resolveQueue; + }); + const chainedQueue = currentQueue.then(() => nextQueue); + fileMutationQueues.set(key, chainedQueue); + + await currentQueue; + try { + return await fn(); + } finally { + releaseNext(); + if (fileMutationQueues.get(key) === chainedQueue) { + fileMutationQueues.delete(key); + } + } +} diff --git a/packages/coding-agent/src/core/tools/index.ts b/packages/coding-agent/src/core/tools/index.ts index b5ab0f97..b3f86338 100644 --- a/packages/coding-agent/src/core/tools/index.ts +++ b/packages/coding-agent/src/core/tools/index.ts @@ -17,6 +17,7 @@ export { type EditToolOptions, editTool, } from "./edit.js"; +export { withFileMutationQueue } from "./file-mutation-queue.js"; export { createFindTool, type FindOperations, diff --git a/packages/coding-agent/src/core/tools/write.ts b/packages/coding-agent/src/core/tools/write.ts index ccdae8a3..c44ecb4e 100644 --- a/packages/coding-agent/src/core/tools/write.ts +++ b/packages/coding-agent/src/core/tools/write.ts @@ -2,6 +2,7 @@ import type { AgentTool } from "@mariozechner/pi-agent-core"; import { type Static, Type } from "@sinclair/typebox"; import { mkdir as fsMkdir, writeFile as fsWriteFile } from "fs/promises"; import { dirname } from "path"; +import { withFileMutationQueue } from "./file-mutation-queue.js"; import { resolveToCwd } from "./path-utils.js"; const writeSchema = Type.Object({ @@ -49,66 +50,72 @@ export function createWriteTool(cwd: string, options?: WriteToolOptions): AgentT const absolutePath = resolveToCwd(path, cwd); const dir = dirname(absolutePath); - return new Promise<{ content: Array<{ type: "text"; text: string }>; details: undefined }>( - (resolve, reject) => { - // Check if already aborted - if (signal?.aborted) { - reject(new Error("Operation aborted")); - return; - } - - let aborted = false; - - // Set up abort handler - const onAbort = () => { - aborted = true; - reject(new Error("Operation aborted")); - }; - - if (signal) { - signal.addEventListener("abort", onAbort, { once: true }); - } - - // Perform the write operation - (async () => { - try { - // Create parent directories if needed - await ops.mkdir(dir); - - // Check if aborted before writing - if (aborted) { + return withFileMutationQueue( + absolutePath, + () => + new Promise<{ content: Array<{ type: "text"; text: string }>; details: undefined }>( + (resolve, reject) => { + // Check if already aborted + if (signal?.aborted) { + reject(new Error("Operation aborted")); return; } - // Write the file - await ops.writeFile(absolutePath, content); + let aborted = false; - // Check if aborted after writing - if (aborted) { - return; - } + // Set up abort handler + const onAbort = () => { + aborted = true; + reject(new Error("Operation aborted")); + }; - // Clean up abort handler if (signal) { - signal.removeEventListener("abort", onAbort); + signal.addEventListener("abort", onAbort, { once: true }); } - resolve({ - content: [{ type: "text", text: `Successfully wrote ${content.length} bytes to ${path}` }], - details: undefined, - }); - } catch (error: any) { - // Clean up abort handler - if (signal) { - signal.removeEventListener("abort", onAbort); - } + // Perform the write operation + (async () => { + try { + // Create parent directories if needed + await ops.mkdir(dir); - if (!aborted) { - reject(error); - } - } - })(); - }, + // Check if aborted before writing + if (aborted) { + return; + } + + // Write the file + await ops.writeFile(absolutePath, content); + + // Check if aborted after writing + if (aborted) { + return; + } + + // Clean up abort handler + if (signal) { + signal.removeEventListener("abort", onAbort); + } + + resolve({ + content: [ + { type: "text", text: `Successfully wrote ${content.length} bytes to ${path}` }, + ], + details: undefined, + }); + } catch (error: any) { + // Clean up abort handler + if (signal) { + signal.removeEventListener("abort", onAbort); + } + + if (!aborted) { + reject(error); + } + } + })(); + }, + ), ); }, }; diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index 5e5253eb..9a792399 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -53,7 +53,7 @@ export type { AgentStartEvent, AgentToolResult, AgentToolUpdateCallback, - AppAction, + AppKeybinding, BashToolCallEvent, BeforeAgentStartEvent, BeforeProviderRequestEvent, @@ -261,6 +261,7 @@ export { type WriteOperations, type WriteToolInput, type WriteToolOptions, + withFileMutationQueue, writeTool, } from "./core/tools/index.js"; // Main entry point @@ -277,8 +278,6 @@ export { export { ArminComponent, AssistantMessageComponent, - appKey, - appKeyHint, BashExecutionComponent, BorderedLoader, BranchSummaryMessageComponent, @@ -289,9 +288,9 @@ export { ExtensionEditorComponent, ExtensionInputComponent, ExtensionSelectorComponent, - editorKey, FooterComponent, keyHint, + keyText, LoginDialogComponent, ModelSelectorComponent, OAuthSelectorComponent, diff --git a/packages/coding-agent/test/file-mutation-queue.test.ts b/packages/coding-agent/test/file-mutation-queue.test.ts new file mode 100644 index 00000000..561f2814 --- /dev/null +++ b/packages/coding-agent/test/file-mutation-queue.test.ts @@ -0,0 +1,164 @@ +import { access, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { createEditTool } from "../src/core/tools/edit.js"; +import { withFileMutationQueue } from "../src/core/tools/file-mutation-queue.js"; +import { createWriteTool } from "../src/core/tools/write.js"; + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +const tempDirs: string[] = []; + +async function createTempDir(): Promise { + const dir = await mkdtemp(join(tmpdir(), "pi-file-mutation-queue-")); + tempDirs.push(dir); + return dir; +} + +afterEach(async () => { + await Promise.all(tempDirs.splice(0, tempDirs.length).map((dir) => rm(dir, { recursive: true, force: true }))); +}); + +describe("withFileMutationQueue", () => { + it("serializes operations for the same file", async () => { + const order: string[] = []; + const path = "/tmp/file-mutation-queue-same"; + + const first = withFileMutationQueue(path, async () => { + order.push("first:start"); + await delay(30); + order.push("first:end"); + }); + const second = withFileMutationQueue(path, async () => { + order.push("second:start"); + order.push("second:end"); + }); + + await Promise.all([first, second]); + expect(order).toEqual(["first:start", "first:end", "second:start", "second:end"]); + }); + + it("allows different files to proceed in parallel", async () => { + const order: string[] = []; + + await Promise.all([ + withFileMutationQueue("/tmp/file-mutation-queue-a", async () => { + order.push("a:start"); + await delay(30); + order.push("a:end"); + }), + withFileMutationQueue("/tmp/file-mutation-queue-b", async () => { + order.push("b:start"); + await delay(30); + order.push("b:end"); + }), + ]); + + expect(order.indexOf("a:start")).toBeLessThan(order.indexOf("a:end")); + expect(order.indexOf("b:start")).toBeLessThan(order.indexOf("b:end")); + expect(order.indexOf("b:start")).toBeLessThan(order.indexOf("a:end")); + }); + + it("uses the same queue for symlink aliases", async () => { + const dir = await createTempDir(); + const targetPath = join(dir, "target.txt"); + const symlinkPath = join(dir, "alias.txt"); + await writeFile(targetPath, "hello\n", "utf8"); + await symlink(targetPath, symlinkPath); + + const order: string[] = []; + await Promise.all([ + withFileMutationQueue(targetPath, async () => { + order.push("target:start"); + await delay(30); + order.push("target:end"); + }), + withFileMutationQueue(symlinkPath, async () => { + order.push("alias:start"); + order.push("alias:end"); + }), + ]); + + expect(order).toEqual(["target:start", "target:end", "alias:start", "alias:end"]); + }); +}); + +describe("built-in edit and write tools", () => { + it("preserves both parallel edits on the same file", async () => { + const dir = await createTempDir(); + const filePath = join(dir, "parallel-edit.txt"); + await writeFile(filePath, "alpha\nbeta\ngamma\n", "utf8"); + + const editTool = createEditTool(dir, { + operations: { + access, + readFile: async (path) => { + const buffer = await readFile(path); + await delay(30); + return buffer; + }, + writeFile: async (path, content) => { + await delay(30); + await writeFile(path, content, "utf8"); + }, + }, + }); + + await Promise.all([ + editTool.execute("call-1", { path: filePath, oldText: "alpha", newText: "ALPHA" }), + editTool.execute("call-2", { path: filePath, oldText: "beta", newText: "BETA" }), + ]); + + const content = await readFile(filePath, "utf8"); + expect(content).toBe("ALPHA\nBETA\ngamma\n"); + }); + + it("shares the queue between edit and write", async () => { + const dir = await createTempDir(); + const filePath = join(dir, "mixed.txt"); + await writeFile(filePath, "original\n", "utf8"); + + const editTool = createEditTool(dir, { + operations: { + access, + readFile: async (path) => { + const buffer = await readFile(path); + await delay(30); + return buffer; + }, + writeFile: async (path, content) => { + await delay(30); + await writeFile(path, content, "utf8"); + }, + }, + }); + const writeTool = createWriteTool(dir, { + operations: { + mkdir: async () => {}, + writeFile: async (path, content) => { + await delay(10); + await writeFile(path, content, "utf8"); + }, + }, + }); + + const editPromise = editTool.execute("call-1", { + path: filePath, + oldText: "original", + newText: "edited", + }); + await delay(5); + const writePromise = writeTool.execute("call-2", { + path: filePath, + content: "replacement\n", + }); + + await Promise.all([editPromise, writePromise]); + + const content = await readFile(filePath, "utf8"); + expect(content).toBe("replacement\n"); + }); +});