fix(coding-agent): queue file mutations across edit and write

closes #2327

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,

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);
+								}
+							}
+						})();
+					}),
+			);
 		},
 	};
 }

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<string, Promise<void>>();
+
+async function getMutationQueueKey(filePath: string): Promise<string> {
+	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<T>(filePath: string, fn: () => Promise<T>): Promise<T> {
+	const key = await getMutationQueueKey(filePath);
+	const currentQueue = fileMutationQueues.get(key) ?? Promise.resolve();
+
+	let releaseNext!: () => void;
+	const nextQueue = new Promise<void>((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);
+		}
+	}
+}

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,

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);
+									}
+								}
+							})();
+						},
+					),
 			);
 		},
 	};

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,