diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2efdd3a7c..90c59d6fb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,10 @@ # Changelog ## [Unreleased] + ### Changed +- Improved auto-generated file detection to gracefully handle ENOENT errors when peeking file content, preventing unnecessary abort failures - Optimized context emission by skipping message cloning when no extensions have context handlers - Improved message cloning resilience by falling back to shallow array clone when structured cloning fails due to non-cloneable objects - Made `assertEditableFileContent` synchronous instead of async for improved performance in streaming edit checks diff --git a/packages/coding-agent/src/tools/auto-generated-guard.ts b/packages/coding-agent/src/tools/auto-generated-guard.ts index d917e1ea4..d6aadd693 100644 --- a/packages/coding-agent/src/tools/auto-generated-guard.ts +++ b/packages/coding-agent/src/tools/auto-generated-guard.ts @@ -5,6 +5,7 @@ * by code generation tools (protoc, sqlc, buf, swagger, etc.). */ import * as path from "node:path"; +import { isEnoent, peekFile } from "@oh-my-pi/pi-utils"; import { LRUCache } from "lru-cache/raw"; import { settings } from "../config/settings"; import { ToolError } from "./tool-errors"; @@ -238,8 +239,6 @@ function buildAutoGeneratedError(displayPath: string, detected: string): ToolErr ); } -import { peekFile } from "@oh-my-pi/pi-utils"; - const decoder = new TextDecoder("utf-8"); const autoGeneratedMap = new LRUCache({ max: 10 }); @@ -248,11 +247,21 @@ async function getAutoGeneratedMarker(filePath: string): Promise decoder.decode(header)); - const marker = detectAutoGeneratedMarker(content, filePath); + let marker: string | undefined; + try { + const content = await peekFile(filePath, CHECK_BYTE_COUNT, header => decoder.decode(header)); + marker = detectAutoGeneratedMarker(content, filePath); + } catch (err) { + if (isEnoent(err)) { + return undefined; + } + throw err; + } + autoGeneratedMap.set(filePath, { marker }); return marker; } diff --git a/packages/coding-agent/src/utils/file-mentions.ts b/packages/coding-agent/src/utils/file-mentions.ts index d9d3e0674..9f7056d61 100644 --- a/packages/coding-agent/src/utils/file-mentions.ts +++ b/packages/coding-agent/src/utils/file-mentions.ts @@ -8,6 +8,7 @@ import * as fs from "node:fs/promises"; import path from "node:path"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { glob } from "@oh-my-pi/pi-natives"; import { formatAge, formatBytes, readImageMetadata } from "@oh-my-pi/pi-utils"; import { formatHashLines } from "../edit/line-hash"; @@ -321,7 +322,7 @@ export async function generateFileMentionMessages( } const base64Content = buffer.toBase64(); - let image = { type: "image" as const, mimeType, data: base64Content }; + let image: ImageContent = { type: "image", mimeType, data: base64Content }; let dimensionNote: string | undefined; if (autoResizeImages) { @@ -329,12 +330,12 @@ export async function generateFileMentionMessages( const resized = await resizeImage({ type: "image", data: base64Content, mimeType }); dimensionNote = formatDimensionNote(resized); image = { - type: "image" as const, + type: "image", mimeType: resized.mimeType, data: resized.data, }; } catch { - image = { type: "image" as const, mimeType, data: base64Content }; + image = { type: "image", mimeType, data: base64Content }; } } diff --git a/packages/coding-agent/test/core/chunk-tree.test.ts b/packages/coding-agent/test/core/chunk-tree.test.ts index 43530df9c..7146efd14 100644 --- a/packages/coding-agent/test/core/chunk-tree.test.ts +++ b/packages/coding-agent/test/core/chunk-tree.test.ts @@ -187,25 +187,6 @@ describe("applyChunkEdits", () => { expect(result.diffSourceAfter).toContain("constructor"); }); - test("replace preserves attached doc comments when replacement starts at the declaration", () => { - const source = `class Worker {\n\t/** restart note */\n\trestart(): void {\n\t\tboot();\n\t}\n}\n`; - const checksum = getChecksum(source, "class_Worker.fn_restart"); - const result = edit( - [ - { - op: "replace", - sel: targetWithChecksum("class_Worker.fn_restart", checksum), - content: `\trestart(): void {\n\t\tshutdown();\n\t}`, - }, - ], - source, - ); - - expect(result.diffSourceAfter).toContain("\t/** restart note */\n\trestart(): void {"); - expect(result.diffSourceAfter).toContain("\t\tshutdown();"); - expect(result.diffSourceAfter).not.toContain("\t\tboot();"); - }); - test("replace does not duplicate attached doc comments when replacement includes a new one", () => { const source = `class Worker {\n\t/** restart note */\n\trestart(): void {\n\t\tboot();\n\t}\n}\n`; const checksum = getChecksum(source, "class_Worker.fn_restart"); diff --git a/packages/coding-agent/test/streaming-edit-abort.test.ts b/packages/coding-agent/test/streaming-edit-abort.test.ts index 8a5d44a01..9a72a93e1 100644 --- a/packages/coding-agent/test/streaming-edit-abort.test.ts +++ b/packages/coding-agent/test/streaming-edit-abort.test.ts @@ -275,7 +275,7 @@ describe("streaming edit abort", () => { it("does not abort when auto-generated peek fails with ENOENT (non-ToolError)", async () => { const checkSpy = vi - .spyOn(autoGeneratedGuard, "checkAutoGeneratedFile") + .spyOn(autoGeneratedGuard, "assertEditableFile") .mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })); await Bun.write(path.join(tempDir, "sample.txt"), "alpha\nbeta\ngamma\n"); @@ -305,7 +305,7 @@ describe("streaming edit abort", () => { it("aborts when auto-generated check rejects with ToolError", async () => { const checkSpy = vi - .spyOn(autoGeneratedGuard, "checkAutoGeneratedFile") + .spyOn(autoGeneratedGuard, "assertEditableFile") .mockRejectedValue(new ToolError("Cannot modify auto-generated file")); await Bun.write(path.join(tempDir, "sample.txt"), "alpha\nbeta\ngamma\n"); diff --git a/packages/coding-agent/test/tools/auto-generated-guard.test.ts b/packages/coding-agent/test/tools/auto-generated-guard.test.ts index 8c3acbf85..771e55d04 100644 --- a/packages/coding-agent/test/tools/auto-generated-guard.test.ts +++ b/packages/coding-agent/test/tools/auto-generated-guard.test.ts @@ -15,67 +15,67 @@ beforeAll(async () => { }); describe("assertEditableFileContent", () => { - it("detects canonical TypeScript generated header", async () => { + it("detects canonical TypeScript generated header", () => { const content = "// Code generated by sqlc. DO NOT EDIT.\n\nexport const foo = 1;"; - await expect(assertEditableFileContent(content, "test.ts")).rejects.toBeInstanceOf(ToolError); + expect(() => assertEditableFileContent(content, "test.ts")).toThrow(ToolError); }); - it("detects @generated marker", async () => { + it("detects @generated marker", () => { const content = "// @generated\n\nexport const foo = 1;"; - await expect(assertEditableFileContent(content, "test.ts")).rejects.toBeInstanceOf(ToolError); + expect(() => assertEditableFileContent(content, "test.ts")).toThrow(ToolError); }); - it("detects generated-by marker for Python files", async () => { + it("detects generated-by marker for Python files", () => { const content = "# Generated by buf\n\nvalue = 1"; - await expect(assertEditableFileContent(content, "test.py")).rejects.toBeInstanceOf(ToolError); + expect(() => assertEditableFileContent(content, "test.py")).toThrow(ToolError); }); - it("detects generated-by marker for SQL files", async () => { + it("detects generated-by marker for SQL files", () => { const content = "-- generated by sqlc\n\nselect 1;"; - await expect(assertEditableFileContent(content, "query.sql")).rejects.toBeInstanceOf(ToolError); + expect(() => assertEditableFileContent(content, "query.sql")).toThrow(ToolError); }); - it("detects generated markers in leading block comments", async () => { + it("detects generated markers in leading block comments", () => { const content = "/*\n * Code generated by mockery. DO NOT EDIT.\n */\nexport const foo = 1;"; - await expect(assertEditableFileContent(content, "test.ts")).rejects.toBeInstanceOf(ToolError); + expect(() => assertEditableFileContent(content, "test.ts")).toThrow(ToolError); }); - it("detects kysely-codegen generated header", async () => { + it("detects kysely-codegen generated header", () => { const content = "/**\n * This file was generated by kysely-codegen.\n * Please do not edit it manually.\n */\n\nexport interface Database {}"; - await expect(assertEditableFileContent(content, "db.ts")).rejects.toBeInstanceOf(ToolError); + expect(() => assertEditableFileContent(content, "db.ts")).toThrow(ToolError); }); - it("does not block broad prose comment markers", async () => { + it("does not block broad prose comment markers", () => { const content = "// auto generated dont edit bla bla\n// this is a hand-written file note\nexport const foo = 1;"; - await expect(assertEditableFileContent(content, "test.ts")).resolves.toBeUndefined(); + expect(() => assertEditableFileContent(content, "test.ts")).not.toThrow(); }); - it("does not match generated markers after code starts", async () => { + it("does not match generated markers after code starts", () => { const content = "export const foo = 1;\n\n// Code generated by sqlc. DO NOT EDIT."; - await expect(assertEditableFileContent(content, "test.ts")).resolves.toBeUndefined(); + expect(() => assertEditableFileContent(content, "test.ts")).not.toThrow(); }); - it("uses language-specific comment styles", async () => { + it("uses language-specific comment styles", () => { const tsContent = "# Code generated by sqlc. DO NOT EDIT.\nexport const foo = 1;"; - await expect(assertEditableFileContent(tsContent, "test.ts")).resolves.toBeUndefined(); + expect(() => assertEditableFileContent(tsContent, "test.ts")).not.toThrow(); const pyContent = "// Code generated by sqlc. DO NOT EDIT.\nvalue = 1"; - await expect(assertEditableFileContent(pyContent, "test.py")).resolves.toBeUndefined(); + expect(() => assertEditableFileContent(pyContent, "test.py")).not.toThrow(); }); it("does not block editing the guard file itself", async () => { const guardPath = path.join(import.meta.dir, "../../src/tools/auto-generated-guard.ts"); const content = await Bun.file(guardPath).text(); - await expect( + expect(() => assertEditableFileContent(content, "packages/coding-agent/src/tools/auto-generated-guard.ts"), - ).resolves.toBeUndefined(); + ).not.toThrow(); }); - it("checks only first 1024 bytes of content", async () => { + it("checks only first 1024 bytes of content", () => { const prefix = "A".repeat(1024); const content = `${prefix}\n// Code generated by sqlc. DO NOT EDIT.`; - await expect(assertEditableFileContent(content, "test.ts")).resolves.toBeUndefined(); + expect(() => assertEditableFileContent(content, "test.ts")).not.toThrow(); }); }); diff --git a/packages/utils/test/peek-file.test.ts b/packages/utils/test/peek-file.test.ts index 5aa271659..7d6da88b4 100644 --- a/packages/utils/test/peek-file.test.ts +++ b/packages/utils/test/peek-file.test.ts @@ -8,6 +8,10 @@ function rangeBuffer(length: number): Buffer { return Buffer.from(Array.from({ length }, (_, index) => index % 256)); } +function bytesOf(input: Uint8Array): number[] { + return Array.from(input); +} + describe("peekFile", () => { let tempDir: string; @@ -24,8 +28,8 @@ describe("peekFile", () => { const content = rangeBuffer(1024); fs.writeFileSync(filePath, content); - const header = await peekFile(filePath, 37, bytes => Buffer.from(bytes)); - expect(header).toEqual(content.subarray(0, 37)); + const header = await peekFile(filePath, 37, bytes => bytes.slice()); + expect(bytesOf(header)).toEqual(bytesOf(content.subarray(0, 37))); }); it("reads an exact header slice synchronously", () => { @@ -33,8 +37,8 @@ describe("peekFile", () => { const content = rangeBuffer(2048); fs.writeFileSync(filePath, content); - const header = peekFileSync(filePath, 777, bytes => Buffer.from(bytes)); - expect(header).toEqual(content.subarray(0, 777)); + const header = peekFileSync(filePath, 777, bytes => bytes.slice()); + expect(bytesOf(header)).toEqual(bytesOf(content.subarray(0, 777))); }); it("serves concurrent async peeks without corrupting buffers", async () => { @@ -43,10 +47,10 @@ describe("peekFile", () => { fs.writeFileSync(filePath, content); const lengths = [17, 33, 64, 128, 257, 511, 512, 513, 777, 1024, 1536, 2048]; - const headers = await Promise.all(lengths.map(length => peekFile(filePath, length, bytes => Buffer.from(bytes)))); + const headers = await Promise.all(lengths.map(length => peekFile(filePath, length, bytes => bytes.slice()))); expect(headers).toHaveLength(lengths.length); for (const [index, header] of headers.entries()) { - expect(header).toEqual(content.subarray(0, lengths[index])); + expect(bytesOf(header)).toEqual(bytesOf(content.subarray(0, lengths[index]))); } }); });