diff --git a/docs/AGENTS.md b/docs/AGENTS.md index a2b084c1d..62184dbcd 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -481,6 +481,8 @@ The IPC layer is the boundary between backend and frontend. Follow these rules t Notice when you've made the same change many times, refactor to create a shared function or component, update all the duplicated code, and then continue on with the original work. When repeating string literals (especially in error messages, UI text, or system instructions), extract them to named constants in a relevant constants/utils file - never define the same string literal multiple times across files. +Before defining a constant, search the codebase to see if it already exists and import it instead. +If a constant exists in a layer-specific location (`services/`, `utils/main/`) but is needed across layers, move it to a shared location (`src/constants/`, `src/types/`) rather than duplicating it. **Avoid unnecessary callback indirection**: If a hook detects a condition and has access to all data needed to handle it, let it handle the action directly rather than passing callbacks up to parent components. Keep hooks self-contained when possible. diff --git a/src/components/tools/FileEditToolCall.tsx b/src/components/tools/FileEditToolCall.tsx index fd505c088..75011a767 100644 --- a/src/components/tools/FileEditToolCall.tsx +++ b/src/components/tools/FileEditToolCall.tsx @@ -22,6 +22,7 @@ import { useToolExpansion, getStatusDisplay, type ToolStatus } from "./shared/to import { TooltipWrapper, Tooltip } from "../Tooltip"; import { DiffContainer, DiffRenderer, SelectableDiffRenderer } from "../shared/DiffRenderer"; import { KebabMenu, type KebabMenuItem } from "../KebabMenu"; +import { WRITE_DENIED_PREFIX } from "@/types/tools"; type FileEditOperationArgs = | FileEditReplaceStringToolArgs @@ -97,7 +98,11 @@ export const FileEditToolCall: React.FC = ({ status = "pending", onReviewNote, }) => { - const { expanded, toggleExpanded } = useToolExpansion(true); + // Collapse WRITE DENIED errors by default since they're common and expected + const isWriteDenied = result && !result.success && result.error?.startsWith(WRITE_DENIED_PREFIX); + const initialExpanded = !isWriteDenied; + + const { expanded, toggleExpanded } = useToolExpansion(initialExpanded); const [showRaw, setShowRaw] = React.useState(false); const [copied, setCopied] = React.useState(false); diff --git a/src/services/tools/fileCommon.ts b/src/services/tools/fileCommon.ts index 0d451ac1c..c6726ddd3 100644 --- a/src/services/tools/fileCommon.ts +++ b/src/services/tools/fileCommon.ts @@ -2,11 +2,7 @@ import type * as fs from "fs"; import * as path from "path"; import { createPatch } from "diff"; -/** - * Prefix for all file write error messages. - * This consistent prefix helps models detect when writes fail and need to retry. - */ -export const WRITE_DENIED_PREFIX = "WRITE DENIED, FILE UNMODIFIED:"; +// WRITE_DENIED_PREFIX moved to @/types/tools for frontend/backend sharing /** * Maximum file size for file operations (1MB) diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 525f77f17..9637619ac 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -4,7 +4,8 @@ import * as path from "path"; import type { FileEditInsertToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { validatePathInCwd, WRITE_DENIED_PREFIX } from "./fileCommon"; +import { validatePathInCwd } from "./fileCommon"; +import { WRITE_DENIED_PREFIX } from "@/types/tools"; import { executeFileEditOperation } from "./file_edit_operation"; /** diff --git a/src/services/tools/file_edit_operation.test.ts b/src/services/tools/file_edit_operation.test.ts index e32ecae86..67bb5ab74 100644 --- a/src/services/tools/file_edit_operation.test.ts +++ b/src/services/tools/file_edit_operation.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "bun:test"; import { executeFileEditOperation } from "./file_edit_operation"; -import { WRITE_DENIED_PREFIX } from "./fileCommon"; +import { WRITE_DENIED_PREFIX } from "@/types/tools"; const TEST_CWD = "/tmp"; diff --git a/src/services/tools/file_edit_operation.ts b/src/services/tools/file_edit_operation.ts index 0a87c922e..97a8e9872 100644 --- a/src/services/tools/file_edit_operation.ts +++ b/src/services/tools/file_edit_operation.ts @@ -2,13 +2,9 @@ import * as fs from "fs/promises"; import * as path from "path"; import writeFileAtomic from "write-file-atomic"; import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/types/tools"; +import { WRITE_DENIED_PREFIX } from "@/types/tools"; import type { ToolConfiguration } from "@/utils/tools/tools"; -import { - generateDiff, - validateFileSize, - validatePathInCwd, - WRITE_DENIED_PREFIX, -} from "./fileCommon"; +import { generateDiff, validateFileSize, validatePathInCwd } from "./fileCommon"; type FileEditOperationResult = | { diff --git a/src/types/tools.ts b/src/types/tools.ts index d7be2038f..03264e7e1 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -117,6 +117,12 @@ export interface FileEditInsertToolArgs { export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorResult; +/** + * Prefix for file write denial error messages. + * This consistent prefix helps both the UI and models detect when writes fail. + */ +export const WRITE_DENIED_PREFIX = "WRITE DENIED, FILE UNMODIFIED:"; + export type FileEditToolArgs = | FileEditReplaceStringToolArgs | FileEditReplaceLinesToolArgs