diff --git a/.gitignore b/.gitignore index 113fb2b15..88e6ad49d 100644 --- a/.gitignore +++ b/.gitignore @@ -56,5 +56,5 @@ pi-*.html # Generated files packages/coding-agent/src/internal-urls/docs-index.generated.ts -packages/typescript-edit-benchmark/runs/ +/runs/ python/omp-rpc/src/omp_rpc.egg-info/ diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index cce0e98d2..7364e4e0f 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Changed + +- Changed OpenAI custom Lark grammar payloads to strip comments and blank lines before sending provider requests. ### Fixed diff --git a/packages/ai/src/providers/grammar.ts b/packages/ai/src/providers/grammar.ts new file mode 100644 index 000000000..7ddd169e6 --- /dev/null +++ b/packages/ai/src/providers/grammar.ts @@ -0,0 +1,70 @@ +export function compactGrammarDefinition(syntax: "lark" | "regex", definition: string): string { + if (syntax !== "lark") { + return definition; + } + + return compactLarkGrammarDefinition(definition); +} + +function compactLarkGrammarDefinition(definition: string): string { + const lines: string[] = []; + + for (const line of definition.split(/\r?\n/)) { + const uncommented = stripLarkLineComment(line).trimEnd(); + if (uncommented.trim()) { + lines.push(uncommented); + } + } + + return lines.join("\n"); +} + +function stripLarkLineComment(line: string): string { + let inString: string | undefined; + let inRegex = false; + let escaped = false; + + for (let i = 0; i < line.length; i++) { + const char = line[i]; + const next = line[i + 1]; + + if (escaped) { + escaped = false; + continue; + } + + if (char === "\\") { + escaped = true; + continue; + } + + if (inString) { + if (char === inString) { + inString = undefined; + } + continue; + } + + if (inRegex) { + if (char === "/") { + inRegex = false; + } + continue; + } + + if (char === "/" && next === "/") { + return line.slice(0, i); + } + + if (char === '"' || char === "'") { + inString = char; + continue; + } + + if (char === "/") { + inRegex = true; + } + } + + return line; +} diff --git a/packages/ai/src/providers/openai-codex-responses.ts b/packages/ai/src/providers/openai-codex-responses.ts index de3b03eb0..99c4700e7 100644 --- a/packages/ai/src/providers/openai-codex-responses.ts +++ b/packages/ai/src/providers/openai-codex-responses.ts @@ -42,6 +42,7 @@ import { finalizeErrorMessage, type RawHttpRequestDump } from "../utils/http-ins import { getOpenAIStreamIdleTimeoutMs, iterateWithIdleTimeout } from "../utils/idle-iterator"; import { parseStreamingJson } from "../utils/json-parse"; import { adaptSchemaForStrict, NO_STRICT } from "../utils/schema"; +import { compactGrammarDefinition } from "./grammar"; import { CODEX_BASE_URL, getCodexAccountId, @@ -2393,7 +2394,7 @@ export function convertTools(tools: Tool[], model: Model<"openai-codex-responses format: { type: "grammar", syntax: tool.customFormat.syntax, - definition: tool.customFormat.definition, + definition: compactGrammarDefinition(tool.customFormat.syntax, tool.customFormat.definition), }, }; } diff --git a/packages/ai/src/providers/openai-responses.ts b/packages/ai/src/providers/openai-responses.ts index 4a9efa9f7..84444b716 100644 --- a/packages/ai/src/providers/openai-responses.ts +++ b/packages/ai/src/providers/openai-responses.ts @@ -46,6 +46,7 @@ import { hasCopilotVisionInput, resolveGitHubCopilotBaseUrl, } from "./github-copilot-headers"; +import { compactGrammarDefinition } from "./grammar"; import { appendResponsesToolResultMessages, collectCustomCallIds, @@ -577,7 +578,7 @@ export function convertTools(tools: Tool[], strictMode: boolean, model: Model<"o format: { type: "grammar", syntax: tool.customFormat.syntax, - definition: tool.customFormat.definition, + definition: compactGrammarDefinition(tool.customFormat.syntax, tool.customFormat.definition), }, } as unknown as OpenAITool; } diff --git a/packages/ai/test/apply-patch-freeform.test.ts b/packages/ai/test/apply-patch-freeform.test.ts index dfb51b435..3cca768da 100644 --- a/packages/ai/test/apply-patch-freeform.test.ts +++ b/packages/ai/test/apply-patch-freeform.test.ts @@ -17,7 +17,15 @@ import type { AssistantMessage, Model, Tool, ToolResultMessage } from "@oh-my-pi import { Type } from "@sinclair/typebox"; import type { ResponseStreamEvent } from "openai/resources/responses/responses"; -const GRAMMAR = 'start: "*** Begin Patch" LF'; +const GRAMMAR = [ + "// top-level comment", + "", + 'start: "*** Begin Patch" LF // trailing comment', + "PATH: /https?:\\/\\/[^\\n]+/", + 'LITERAL: "//"', + "", +].join("\n"); +const COMPACT_GRAMMAR = 'start: "*** Begin Patch" LF\nPATH: /https?:\\/\\/[^\\n]+/\nLITERAL: "//"'; function makeModel(overrides: Partial> = {}): Model<"openai-responses"> { return { @@ -96,7 +104,7 @@ describe("convertTools: freeform emission", () => { const [out] = convertTools([editTool], false, freeformModel) as unknown as Array>; expect(out.type).toBe("custom"); expect(out.name).toBe("apply_patch"); // wire name from tool.customWireName - expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: GRAMMAR }); + expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: COMPACT_GRAMMAR }); }); test("regular tools remain function-type alongside a custom one", () => { @@ -316,7 +324,7 @@ describe("codex-backend convertTools (chatgpt.com/backend-api)", () => { expect(out.type).toBe("custom"); expect(out.name).toBe("apply_patch"); if (out.type !== "custom") throw new Error("Expected custom tool payload"); - expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: GRAMMAR }); + expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: COMPACT_GRAMMAR }); }); test("wire shape matches direct-OpenAI convertTools (single serializer contract)", () => { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0d9fd87d5..129984a7a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,12 +1,9 @@ # Changelog ## [Unreleased] - ### Breaking Changes -- Required top-level `path` for `edit` tool `atom`, `hashline`, `patch`, and `replace` calls and removed per-entry `path` overrides, so edits to different files now require separate calls -- Replaced the atom edit `sed` verb with `replace`, requiring `{ find, with, all? }` instead of `{ pat, rep, g? }` -- Removed bracketed atom locators like `(anchor)` and `[anchor]`, so region block rewrites via `splice` are no longer supported and bare anchor `loc` values now target exactly one line +- Changed the `atom` edit mode from JSON `{ path, edits }` calls to the compact file-oriented `input` patch language that was previously exposed as `atomd`; `atomd` is no longer a separate edit variant - Renamed MCP tool identifiers from the `mcp__` format to `mcp___` so custom tool names, active tool lists, and persisted MCP selections must be updated to the new prefix - Renamed the built-in content-search tool from `grep` to `search`, including SDK/tool event names and settings keys (`search.enabled`, `search.contextBefore`, `search.contextAfter`), so integrations using `grep` and `grep.*` references must be updated @@ -14,17 +11,18 @@ - Added internal URL support to the `search` tool, allowing `artifact://`-style paths that resolve to local files to be searched directly - Added IRC relay observation in the main agent UI so every IRC exchange between agents is rendered in the main transcript, even when the main agent is not a direct participant +- Added stateful `href`/`hrefr` prompt helpers that can reuse anchors remembered from prior `hline` helper calls ### Changed +- Changed file-path rendering across search, find, AST, LSP, and related edit outputs to display targets as cwd-relative paths when they resolve inside the working directory and keep absolute paths for files outside the cwd +- Changed system prompt guidance so in-cwd tool paths must be passed as cwd-relative paths and absolute paths only for out-of-cwd targets or `~` expansion - Updated `edit` streaming diff previews for `patch`, `replace`, and `hashline` to produce a single request-level preview for the new single-file `path` mode -- Changed atom inline and file-wide replacements to perform literal substring substitution, with `all: true` replacing all matches on the line -- Changed `loc` parsing so path-qualified atom edits correctly split `path:loc` when the locator suffix contains colons -- Changed `replace.find` to remain single-line; `replace.with` now allows multiline replacements - Bumped default `read.defaultLimit` from 300 to 500 lines, and scaled the read tool's byte budget with the line limit (`max(50KB, lines * 512)`) so the configured line count is no longer truncated by the shared 50KB cap ### Fixed +- Fixed atom edit streaming previews to use atom headers for file names instead of apply_patch parsing errors. - Fixed collapsed search result rendering so summary and truncation rows stay within the collapsed output budget - Updated search path handling to support path lists and internal file paths while preserving previous search behavior diff --git a/packages/coding-agent/src/config/prompt-templates.ts b/packages/coding-agent/src/config/prompt-templates.ts index ee09e9625..9c4007606 100644 --- a/packages/coding-agent/src/config/prompt-templates.ts +++ b/packages/coding-agent/src/config/prompt-templates.ts @@ -43,24 +43,119 @@ function formatHashlineRef(lineNum: unknown, content: unknown): { num: number; t return { num, text, ref }; } +interface HashlineHelperRef { + line: number; + ref: string; +} + +interface HashlineHelperState { + last?: HashlineHelperRef; + byLine: Map; +} + +const HASHLINE_HELPER_STATE = Symbol("hashlineHelperState"); + +interface HashlineHelperStateHolder { + [HASHLINE_HELPER_STATE]?: HashlineHelperState; +} + +function isHelperOptions(value: unknown): value is prompt.HelperOptions { + return typeof value === "object" && value !== null && "hash" in value; +} + +function splitHelperArgs(args: unknown[]): { positional: unknown[]; options?: prompt.HelperOptions } { + const maybeOptions = args.at(-1); + if (!isHelperOptions(maybeOptions)) return { positional: args }; + return { positional: args.slice(0, -1), options: maybeOptions }; +} + +function getHashlineHelperState(context: unknown, options: prompt.HelperOptions | undefined): HashlineHelperState { + const data = options?.data; + const root = data?.root; + const holderTarget = data && typeof data === "object" ? data : root && typeof root === "object" ? root : context; + if (!holderTarget || typeof holderTarget !== "object") { + throw new Error("hashline prompt helpers require an object render context"); + } + + const holder = holderTarget as HashlineHelperStateHolder; + if (!holder[HASHLINE_HELPER_STATE]) { + holder[HASHLINE_HELPER_STATE] = { byLine: new Map() }; + } + return holder[HASHLINE_HELPER_STATE]; +} + +function isLineNumberArg(value: unknown): boolean { + const num = typeof value === "number" ? value : Number.parseInt(String(value), 10); + return Number.isFinite(num); +} + +function rememberHashlineRef(state: HashlineHelperState, line: number, ref: string): void { + const entry = { line, ref }; + state.last = entry; + state.byLine.set(line, entry); +} + +function requireStoredHashlineRef(state: HashlineHelperState, lineArg?: unknown): string { + if (lineArg === undefined) { + if (!state.last) { + throw new Error("{{href}} requires a previous {{hline}} call in the same prompt render"); + } + return state.last.ref; + } + + const line = typeof lineArg === "number" ? lineArg : Number.parseInt(String(lineArg), 10); + const entry = state.byLine.get(line); + if (!entry) { + throw new Error(`{{href ${line}}} requires a previous {{hline ${line} ...}} call in the same prompt render`); + } + return entry.ref; +} + +function wrapHashlineRef(ref: string, args: unknown[]): string { + const preStr = typeof args[0] === "string" ? args[0] : ""; + const postStr = typeof args[1] === "string" ? args[1] : ""; + return `${preStr}${ref}${postStr}`; +} + +function resolveHashlineRef(state: HashlineHelperState, args: unknown[]): string { + if (args.length === 0) return requireStoredHashlineRef(state); + const [first, second, ...rest] = args; + if (isLineNumberArg(first)) { + if (second === undefined) return requireStoredHashlineRef(state, first); + const { ref } = formatHashlineRef(first, second); + return wrapHashlineRef(ref, rest); + } + return wrapHashlineRef(requireStoredHashlineRef(state), args); +} + /** * {{href lineNum "content"}} — compute a real hashline ref for prompt examples. - * {{href lineNum "content" "[" "]"}} — wrap the ref with pre/post chars (still quoted). + * {{href lineNum}} — quote the ref remembered by the earlier {{hline lineNum "..."}} + * {{href}} — quote the ref from the previous {{hline}} call. + * {{href "[" "]"}} — wrap the previous {{hline}} ref with pre/post chars. * Returns `"lineNumBIGRAM"` (e.g., `"42nd"`), or `"[42nd]"` when pre/post are supplied. */ -prompt.registerHelper("href", (lineNum: unknown, content: unknown, pre?: unknown, post?: unknown): string => { - const { ref } = formatHashlineRef(lineNum, content); - const preStr = typeof pre === "string" ? pre : ""; - const postStr = typeof post === "string" ? post : ""; - return JSON.stringify(`${preStr}${ref}${postStr}`); +prompt.registerHelper("href", function (this: unknown, ...args: unknown[]): string { + const { positional, options } = splitHelperArgs(args); + const state = getHashlineHelperState(this, options); + return JSON.stringify(resolveHashlineRef(state, positional)); +}); +prompt.registerHelper("hrefr", function (this: unknown, ...args: unknown[]): string { + const { positional, options } = splitHelperArgs(args); + const state = getHashlineHelperState(this, options); + return resolveHashlineRef(state, positional); }); /** * {{hline lineNum "content"}} — format a full read-style line with prefix. * Returns `"lineNumBIGRAM|content"` (pipe between anchor and content). */ -prompt.registerHelper("hline", (lineNum: unknown, content: unknown): string => { - const { ref, text } = formatHashlineRef(lineNum, content); +prompt.registerHelper("hline", function (this: unknown, ...args: unknown[]): string { + const { positional, options } = splitHelperArgs(args); + const [lineNum, content] = positional; + const { num, ref, text } = formatHashlineRef(lineNum, content); + const state = getHashlineHelperState(this, options); + rememberHashlineRef(state, num, ref); return `${ref}${HASHLINE_CONTENT_SEPARATOR}${text}`; }); diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 13b93a9f8..11c73646c 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -1,5 +1,6 @@ import { THINKING_EFFORTS } from "@oh-my-pi/pi-ai"; import { TASK_SIMPLE_MODES } from "../task/simple-mode"; +import { EDIT_MODES } from "../utils/edit-mode"; /** Unified settings schema - single source of truth for all settings. * Unified settings schema - single source of truth for all settings. @@ -955,12 +956,12 @@ export const SETTINGS_SCHEMA = { // Edit tool "edit.mode": { type: "enum", - values: ["replace", "patch", "hashline", "vim", "apply_patch", "atom"] as const, + values: EDIT_MODES, default: "hashline", ui: { tab: "editing", label: "Edit Mode", - description: "Select the edit tool variant (replace, patch, hashline, vim, or apply_patch)", + description: "Select the edit tool variant (replace, patch, hashline, atom, vim, or apply_patch)", }, }, diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 4e4c39cf9..92236a124 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -326,7 +326,7 @@ export class Settings { /** * Get the edit variant for a specific model. - * Returns "patch", "replace", "hashline", "vim", "apply_patch", or null (use global default). + * Returns "patch", "replace", "hashline", "atom", "vim", "apply_patch", or null (use global default). */ getEditVariantForModel(model: string | undefined): EditMode | null { if (!model) return null; diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index bdb3a3f98..6e6624903 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -19,7 +19,8 @@ import { type EditMode, normalizeEditMode, resolveEditMode } from "../utils/edit import type { VimToolDetails } from "../vim/types"; import { type ApplyPatchParams, applyPatchSchema, expandApplyPatchToEntries } from "./modes/apply-patch"; import applyPatchGrammar from "./modes/apply-patch.lark" with { type: "text" }; -import { type AtomParams, type AtomToolEdit, atomEditParamsSchema, executeAtomSingle } from "./modes/atom"; +import { type AtomParams, atomEditParamsSchema, executeAtomSingle } from "./modes/atom"; +import atomGrammar from "./modes/atom.lark" with { type: "text" }; import { executeHashlineSingle, HashlineMismatchError, @@ -290,8 +291,9 @@ export class EditTool implements AgentTool { * and fall back to emitting a JSON function tool from `parameters`. */ get customFormat(): { syntax: "lark"; definition: string } | undefined { - if (this.mode !== "apply_patch") return undefined; - return { syntax: "lark", definition: applyPatchGrammar }; + if (this.mode === "apply_patch") return { syntax: "lark", definition: applyPatchGrammar }; + if (this.mode === "atom") return { syntax: "lark", definition: atomGrammar }; + return undefined; } /** @@ -410,11 +412,11 @@ export class EditTool implements AgentTool { batchRequest: LspBatchRequest | undefined, _onUpdate?: (partialResult: AgentToolResult) => void, ) => { - const { edits, path } = params as AtomParams; + const { input, path } = params as AtomParams & { path?: string }; return executeAtomSingle({ session: tool.session, + input, path, - edits: edits as AtomToolEdit[], signal, batchRequest, writethrough: tool.#writethrough, diff --git a/packages/coding-agent/src/edit/modes/atom.lark b/packages/coding-agent/src/edit/modes/atom.lark new file mode 100644 index 000000000..cecb8e30a --- /dev/null +++ b/packages/coding-agent/src/edit/modes/atom.lark @@ -0,0 +1,27 @@ +start: file_section+ + +file_section: file_header (line_change | whole_file_change) +file_header: "---" filename LF + +filename: /(.+)/ + +line_change: line* mutation_line line* +line: insert_line | delete_line | set_line | move_line | blank +mutation_line: insert_line | delete_line | set_line + +whole_file_change: blank* whole_file_line blank* +whole_file_line: remove_file | move_file +remove_file: "!rm" LF +move_file: "!mv" WS destination LF +destination: /(?:[^ \t\r\n]+|"[^"\r\n]+"|'[^'\r\n]+')/ + +insert_line: "+" /(.*)/ LF +delete_line: "-" LID LF +set_line: LID "=" /(.*)/ LF +move_line: ("@" LID | "$" | "^") LF + +LID: /[1-9][0-9]*[a-z]{2}/ +WS: /[ \t]+/ +blank: LF + +%import common.LF \ No newline at end of file diff --git a/packages/coding-agent/src/edit/modes/atom.ts b/packages/coding-agent/src/edit/modes/atom.ts index 72aebb905..2ab606068 100644 --- a/packages/coding-agent/src/edit/modes/atom.ts +++ b/packages/coding-agent/src/edit/modes/atom.ts @@ -1,395 +1,492 @@ /** + * Atom edit mode. * - * Flat locator + verb edit mode backed by hashline anchors. Each entry carries - * one shared `loc` selector plus one or more verbs (`pre`, `splice`, `post`, - * `replace`). The runtime resolves those verbs into internal anchor-scoped - * edits and reuses hashline's staleness scheme (`computeLineHash`) verbatim. + * Single-string compact wire format. Each file section starts with `---path`; + * each following line is one statement: * - * External shapes (one entry): - * { path, loc: "5th", splice: ["..."] } // line replace - * { path, loc: "5th", pre: [...], splice: [...], post: [...] } // line verbs combinable - * { path, loc: "5th", replace: { find: "x", with: "y" } } // literal substring on the line - * { path, loc: "$", pre: [...] | post: [...] | replace: {...} } // file-scoped - * - * `splice: []` deletes; `splice: [""]` replaces with a single blank line. - * - * For deleting or moving files, the agent should use bash. + * @Lid move cursor to just after the anchored line + * Lid=TEXT set the anchored line to TEXT and move cursor after it + * -Lid delete the anchored line and move cursor to its slot + * +TEXT insert TEXT at the cursor + * $ move cursor to beginning of file + * ^ move cursor to end of file */ +import * as fs from "node:fs/promises"; +import * as path from "node:path"; import type { AgentToolResult } from "@oh-my-pi/pi-agent-core"; +import { isEnoent } from "@oh-my-pi/pi-utils"; import { type Static, Type } from "@sinclair/typebox"; import type { WritethroughCallback, WritethroughDeferredHandle } from "../../lsp"; import type { ToolSession } from "../../tools"; -import { assertEditableFileContent } from "../../tools/auto-generated-guard"; -import { invalidateFsScanAfterWrite } from "../../tools/fs-cache-invalidation"; +import { assertEditableFile, assertEditableFileContent } from "../../tools/auto-generated-guard"; +import { + invalidateFsScanAfterDelete, + invalidateFsScanAfterRename, + invalidateFsScanAfterWrite, +} from "../../tools/fs-cache-invalidation"; import { outputMeta } from "../../tools/output-meta"; import { enforcePlanModeWrite, resolvePlanPath } from "../../tools/plan-mode-guard"; import { generateDiffString } from "../diff"; -import { computeLineHash, HASHLINE_BIGRAM_RE_SRC, HASHLINE_CONTENT_SEPARATOR } from "../line-hash"; +import { computeLineHash } from "../line-hash"; import { detectLineEnding, normalizeToLF, restoreLineEndings, stripBom } from "../normalize"; import type { EditToolDetails, LspBatchRequest } from "../renderer"; import { ANCHOR_REBASE_WINDOW, type Anchor, buildCompactHashlineDiffPreview, - formatFullAnchorRequirement, HashlineMismatchError, type HashMismatch, - hashlineParseText, - parseTag, tryRebaseAnchor, } from "./hashline"; // ═══════════════════════════════════════════════════════════════════════════ // Schema // ═══════════════════════════════════════════════════════════════════════════ -const textSchema = Type.Array(Type.String()); -/** - * Flat entry shape with shared locator fields and verb-specific payloads. - * The runtime validator (`resolveAtomToolEdit`) enforces legal locator/verb - * combinations. Keeping the schema flat reduces tool-definition size and gives - * weaker models fewer branching shapes to sample from. - */ -export const atomEditSchema = Type.Object( - { - loc: Type.String({ - description: "edit location", - examples: ["1ab", "$"], - }), - splice: Type.Optional(textSchema), - pre: Type.Optional(textSchema), - post: Type.Optional(textSchema), - replace: Type.Optional( - Type.Object( - { - find: Type.String({ description: "literal substring to find" }), - with: Type.String({ description: "literal substring to substitute" }), - all: Type.Optional( - Type.Boolean({ description: "replace every occurrence (default: first only)", default: false }), - ), - }, - { - additionalProperties: false, - }, - ), - ), - }, - { additionalProperties: false }, -); +export const atomEditParamsSchema = Type.Object({ input: Type.String() }); -export const atomEditParamsSchema = Type.Object( - { - path: Type.String({ description: "file path for edits" }), - edits: Type.Array(atomEditSchema, { description: "edit ops" }), - }, - { additionalProperties: false }, -); - -export type AtomToolEdit = Static; export type AtomParams = Static; // ═══════════════════════════════════════════════════════════════════════════ -// Internal resolved op shapes +// Parser // ═══════════════════════════════════════════════════════════════════════════ +// Permissive: any 2 lowercase letters. Invalid hashes flow through to a +// HashlineMismatchError downstream, matching the other hashline-backed modes. +const LID_RE = /^([1-9]\d*)([a-z]{2})/; +const LID_EXACT_RE = /^([1-9]\d*)([a-z]{2})$/; + +interface ParsedAnchor { + line: number; + hash: string; +} + +type ParsedOp = { op: "set"; text: string; allowOldNewRepair: boolean } | { op: "delete" }; + +type AnchorStmt = + | { kind: "bare_anchor"; anchor: ParsedAnchor; lineNum: number } + | { kind: "anchor_op"; anchor: ParsedAnchor; op: ParsedOp; lineNum: number } + | { kind: "bof"; lineNum: number } + | { kind: "eof"; lineNum: number }; + +type InsertStmt = { + kind: "insert"; + text: string; + lineNum: number; +}; + +type DiffishAddStmt = { + kind: "diffish_add"; + anchor: ParsedAnchor; + text: string; + lineNum: number; +}; + +type DeleteWithOldStmt = { + kind: "delete_with_old"; + anchor: ParsedAnchor; + old: string; + lineNum: number; +}; + +type ParsedStmt = AnchorStmt | InsertStmt | DiffishAddStmt | DeleteWithOldStmt; + +type AtomCursor = { kind: "bof" } | { kind: "eof" } | { kind: "anchor"; anchor: Anchor }; + export type AtomEdit = - | { op: "splice"; pos: Anchor; lines: string[] } - | { op: "pre"; pos: Anchor; lines: string[] } - | { op: "post"; pos: Anchor; lines: string[] } - | { op: "del"; pos: Anchor } - | { op: "append_file"; lines: string[] } - | { op: "prepend_file"; lines: string[] } - | { op: "replace"; pos: Anchor; spec: ReplaceSpec; expression: string } - | { op: "replace_file"; spec: ReplaceSpec; expression: string }; + | { kind: "insert"; cursor: AtomCursor; text: string; lineNum: number; index: number } + | { kind: "set"; anchor: Anchor; text: string; lineNum: number; index: number; allowOldNewRepair: boolean } + | { kind: "delete"; anchor: Anchor; lineNum: number; index: number; oldAssertion?: string }; -export interface ReplaceSpec { - find: string; - with: string; - all: boolean; +interface AtomApplyResult { + lines: string; + firstChangedLine?: number; + warnings?: string[]; + noopEdits?: AtomNoopEdit[]; } -// ═══════════════════════════════════════════════════════════════════════════ -// Param guards -// ═══════════════════════════════════════════════════════════════════════════ - -const ATOM_VERB_KEYS = ["splice", "pre", "post", "replace"] as const; -type AtomOptionalKey = "loc" | (typeof ATOM_VERB_KEYS)[number]; -const ATOM_OPTIONAL_KEYS = ["loc", ...ATOM_VERB_KEYS] as const satisfies readonly AtomOptionalKey[]; - -// Matches just the LINE+BIGRAM prefix of an anchor reference. Used to detect -// optional `|content` suffixes (e.g. `82zu| for (...)`) so the suffix can be -// captured as a content hint for anchor disambiguation. -const ANCHOR_PREFIX_RE = new RegExp(`^\\s*[>+-]*\\s*\\d+${HASHLINE_BIGRAM_RE_SRC}`); - -function stripNullAtomFields(edit: AtomToolEdit): AtomToolEdit { - let next: Record | undefined; - const fields = edit as Record; - for (const key of ATOM_OPTIONAL_KEYS) { - if (fields[key] !== null) continue; - next ??= { ...fields }; - delete next[key]; - } - return (next ?? fields) as AtomToolEdit; +interface AtomNoopEdit { + editIndex: number; + loc: string; + reason: string; + current: string; } -type ParsedAtomLoc = { kind: "anchor"; pos: Anchor } | { kind: "file" }; - -// ═══════════════════════════════════════════════════════════════════════════ -// Resolution -// ═══════════════════════════════════════════════════════════════════════════ - -/** - * Parse an anchor reference like `"5th"`. - * - * Tolerant: on a malformed reference we still try to extract a 1-indexed line - * number from the leading digits so the validator can surface the *correct* - * `LINEHASH|content` for the user. The bogus hash is preserved in the returned - * anchor so the validator emits a content-rich mismatch error. - * - * If we cannot recover even a line number, throw a usage-style error with the - * raw reference quoted. - */ -function parseAnchor(raw: string, opName: string): Anchor { - if (typeof raw !== "string" || raw.length === 0) { - throw new Error(`${opName} requires ${formatFullAnchorRequirement()}.`); - } - try { - return parseTag(raw); - } catch { - const lineMatch = /^\s*[>+-]*\s*(\d+)/.exec(raw); - if (lineMatch) { - const line = Number.parseInt(lineMatch[1], 10); - if (line >= 1) { - // Sentinel hash that will never match a real line, forcing the validator - // to report a mismatch with the actual hash + line content. - return { line, hash: "??" }; - } - } - throw new Error( - `${opName} requires ${formatFullAnchorRequirement(raw)} Could not find a line number in the anchor.`, - ); - } +interface IndexedAnchorEdit { + edit: Extract; + idx: number; } -function parseLoc(raw: string, editIndex: number): ParsedAtomLoc { - const trimmed = raw.trim(); - if (trimmed === "$") return { kind: "file" }; - - const pos = parseAnchor(trimmed, "loc"); - // Capture an optional content suffix after the anchor: `82zu| for (...)`. - // The suffix acts as a hint for anchor disambiguation when the model's hash - // is wrong but the content reveals the intended line. - const hint = extractAnchorContentHint(trimmed); - if (hint !== undefined) { - pos.contentHint = hint; - } - - if (pos.line < 1) { - throw new Error(`Edit ${editIndex}: invalid line number in loc "${raw}".`); - } - return { kind: "anchor", pos }; +function cloneCursor(cursor: AtomCursor): AtomCursor { + if (cursor.kind !== "anchor") return cursor; + return { kind: "anchor", anchor: { ...cursor.anchor } }; } -function extractAnchorContentHint(raw: string): string | undefined { - const match = raw.match(ANCHOR_PREFIX_RE); - if (!match) return undefined; - const rest = raw.slice(match[0].length); - // Accept either the canonical `|` (HASHLINE_CONTENT_SEPARATOR) or the legacy - // `:` separator. Models trained on older docs still emit `82zu: for (...)`. - const sep = rest[0]; - if (sep !== HASHLINE_CONTENT_SEPARATOR && sep !== ":") return undefined; - const hint = rest.slice(1); - if (hint.trim().length === 0) return undefined; - return hint; -} +function parseLidStmt(body: string, lineNum: number): AnchorStmt | null { + const m = LID_RE.exec(body); + if (!m) return null; -function parseReplaceSpec(input: unknown, editIndex: number): ReplaceSpec { - if (input === null || typeof input !== "object" || Array.isArray(input)) { - throw new Error(`Edit ${editIndex}: replace must be an object with shape {find, with, all?}.`); + const ln = Number.parseInt(m[1], 10); + const hash = m[2]; + const rest = body.slice(m[0].length); + const anchor = { line: ln, hash }; + if (rest.length === 0) { + return { kind: "bare_anchor", anchor, lineNum }; } - const obj = input as Record; - const find = obj.find; - const withVal = obj.with; - if (typeof find !== "string" || find.length === 0) { - throw new Error(`Edit ${editIndex}: replace.find must be a non-empty string.`); - } - if (find.includes("\n")) { - throw new Error( - `Edit ${editIndex}: replace.find must be a single line; contains a newline. Use \`splice\` to replace multiple lines, anchoring the first changed line and listing replacement lines in the array.`, - ); - } - if (typeof withVal !== "string") { - throw new Error(`Edit ${editIndex}: replace.with must be a string.`); - } - const rawAll = obj.all; - let all = false; - if (rawAll !== undefined) { - if (typeof rawAll !== "boolean") { - throw new Error(`Edit ${editIndex}: replace.all must be a boolean when provided.`); - } - all = rawAll; - } - return { find, with: withVal, all }; -} -function formatReplaceExpression(spec: ReplaceSpec): string { - const obj: { find: string; with: string; all?: boolean } = { find: spec.find, with: spec.with }; - if (spec.all) obj.all = true; - return JSON.stringify(obj); -} - -function applyReplaceToLine(currentLine: string, spec: ReplaceSpec): { result: string; matched: boolean } { - const idx = currentLine.indexOf(spec.find); - if (idx === -1) return { result: currentLine, matched: false }; - if (spec.all) { - return { result: currentLine.split(spec.find).join(spec.with), matched: true }; - } + const replacement = /^[ \t]*([=|])(.*)$/.exec(rest); + if (!replacement) return null; return { - result: currentLine.slice(0, idx) + spec.with + currentLine.slice(idx + spec.find.length), - matched: true, + kind: "anchor_op", + anchor, + op: { op: "set", text: replacement[2], allowOldNewRepair: replacement[1] === "|" }, + lineNum, }; } -function classifyAtomEdit(edit: AtomToolEdit): string { - const entry = stripNullAtomFields(edit); - const verbs = ATOM_VERB_KEYS.filter(k => entry[k] !== undefined); - return verbs.length > 0 ? verbs.join("+") : "unknown"; +function parseDeleteStmt(body: string, lineNum: number): ParsedStmt[] | null { + const trimmedBody = body.trimStart(); + const exact = LID_EXACT_RE.exec(trimmedBody); + if (exact) { + const ln = Number.parseInt(exact[1], 10); + return [{ kind: "anchor_op", anchor: { line: ln, hash: exact[2] }, op: { op: "delete" }, lineNum }]; + } + + const m = LID_RE.exec(trimmedBody); + if (m && (trimmedBody[m[0].length] === "|" || trimmedBody[m[0].length] === "=")) { + const ln = Number.parseInt(m[1], 10); + const old = trimmedBody.slice(m[0].length + 1); + return [{ kind: "delete_with_old", anchor: { line: ln, hash: m[2] }, old, lineNum }]; + } + if (m && trimmedBody[m[0].length] === " ") { + const ln = Number.parseInt(m[1], 10); + const text = trimmedBody.slice(m[0].length + 1); + return [ + { kind: "anchor_op", anchor: { line: ln, hash: m[2] }, op: { op: "delete" }, lineNum }, + { kind: "insert", text, lineNum }, + ]; + } + + return null; } -function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0, _path?: string): AtomEdit[] { - const entry = stripNullAtomFields(edit); - const verbKeysPresent = ATOM_VERB_KEYS.filter(k => entry[k] !== undefined); - if (verbKeysPresent.length === 0) { +function throwMalformedLidDiagnostic(line: string, lineNum: number, raw: string): never { + const text = line.trimStart(); + const withoutLegacyMove = text.startsWith("@@ ") ? text.slice(3).trimStart() : text; + const withoutMove = withoutLegacyMove.startsWith("@") ? withoutLegacyMove.slice(1) : withoutLegacyMove; + const withoutDelete = withoutMove.startsWith("-") ? withoutMove.slice(1).trimStart() : withoutMove; + + const partial = /^([a-z]{2})(?=[ \t]*[=|])/.exec(withoutDelete); + if (partial) { throw new Error( - `Edit ${editIndex}: missing verb. Each entry must include at least one of: ${ATOM_VERB_KEYS.join(", ")}.`, + `Diff line ${lineNum}: \`${partial[1]}\` is not a full Lid. Use the full Lid from read output, e.g. \`119${partial[1]}\`.`, ); } - if (typeof entry.loc !== "string") { - throw new Error(`Edit ${editIndex}: missing loc. Use a selector like "160sr" or "$".`); + + const missing = /^([1-9]\d*)(?=[ \t]*[=|]|$)/.exec(withoutDelete); + if (missing) { + const prefix = text.startsWith("@@ ") ? `@@ ${missing[1]}` : missing[1]; + throw new Error( + `Diff line ${lineNum}: \`${prefix}\` is missing the two-letter Lid suffix. Use the full Lid from read output, e.g. \`${prefix.startsWith("@@ ") ? "@@ " : ""}${missing[1]}ab\`.`, + ); } - const loc = parseLoc(entry.loc, editIndex); - const resolved: AtomEdit[] = []; + throw new Error(`Diff line ${lineNum}: cannot parse "${raw}".`); +} - if (loc.kind === "file") { - if (entry.splice !== undefined) { - throw new Error(`Edit ${editIndex}: loc "$" supports pre, post, and replace (not splice).`); - } - if (entry.pre !== undefined) { - resolved.push({ op: "prepend_file", lines: hashlineParseText(entry.pre) }); - } - if (entry.post !== undefined) { - resolved.push({ op: "append_file", lines: hashlineParseText(entry.post) }); - } - if (entry.replace !== undefined) { - const spec = parseReplaceSpec(entry.replace, editIndex); - resolved.push({ op: "replace_file", spec, expression: formatReplaceExpression(spec) }); - } - return resolved; - } +function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { + // Strip trailing CR (CRLF tolerance). + const line = raw.endsWith("\r") ? raw.slice(0, -1) : raw; + if (line.length === 0) return []; - if (entry.pre !== undefined) { - resolved.push({ op: "pre", pos: loc.pos, lines: hashlineParseText(entry.pre) }); - } - if (entry.splice !== undefined) { - if (Array.isArray(entry.splice) && entry.splice.length === 0) { - // Models often default `splice: []` alongside other verbs (notably `replace`). - // Treating that combination as an explicit `del` produces a confusing - // `Conflicting ops` error. When another mutating verb is present, drop - // the empty `splice` instead of treating it as a deletion. - if (entry.replace === undefined) { - resolved.push({ op: "del", pos: loc.pos }); + // `+TEXT` inserts at the cursor. Everything after `+` is content. A + // `+Lid|TEXT` or `+Lid=TEXT` line is a diff-ish add (unified-diff trap): + // emit a tagged stmt so the normalizer can fuse it with a preceding `-Lid`. + if (line[0] === "+") { + const body = line.slice(1); + const m = LID_RE.exec(body); + if (m) { + const sep = body[m[0].length]; + if (sep === "=" || sep === "|") { + const ln = Number.parseInt(m[1], 10); + const text = body.slice(m[0].length + 1); + return [{ kind: "diffish_add", anchor: { line: ln, hash: m[2] }, text, lineNum }]; } - } else { - resolved.push({ op: "splice", pos: loc.pos, lines: hashlineParseText(entry.splice) }); + } + return [{ kind: "insert", text: body, lineNum }]; + } + + // Canonical file-scope locators. + if (line === "$") return [{ kind: "bof", lineNum }]; + if (line === "^") return [{ kind: "eof", lineNum }]; + + // `-Lid` deletes the anchored line. Leniently accept `- Lid` and the + // historical `-Lid TEXT` delete-then-insert recovery. + if (line[0] === "-") { + const parsed = parseDeleteStmt(line.slice(1), lineNum); + if (parsed) return parsed; + // Lenient: a stray `-` line with no Lid becomes a `+` insert that + // preserves the leading dash so we don't lose data. + return [{ kind: "insert", text: line, lineNum }]; + } + + // Legacy move prefix. Runtime accepts old locators and common slipped edit + // operations, while the grammar/prompt bias models to canonical syntax. + if (line.startsWith("@@ ")) { + const body = line.slice(3); + if (body === "BOF") return [{ kind: "bof", lineNum }]; + if (body === "EOF") return [{ kind: "eof", lineNum }]; + + const deleteStmt = body.startsWith("-") ? parseDeleteStmt(body.slice(1), lineNum) : null; + if (deleteStmt) return deleteStmt; + + const lidStmt = parseLidStmt(body, lineNum); + if (lidStmt) return [lidStmt]; + + throwMalformedLidDiagnostic(line, lineNum, raw); + } + + // Canonical `@Lid` cursor moves. Leniently recover `@Lid=TEXT`, + // `@Lid|TEXT`, `@$`, and `@^`. + if (line[0] === "@") { + const body = line.slice(1); + if (body === "$") return [{ kind: "bof", lineNum }]; + if (body === "^") return [{ kind: "eof", lineNum }]; + const lidStmt = parseLidStmt(body, lineNum); + if (lidStmt) return [lidStmt]; + throwMalformedLidDiagnostic(line, lineNum, raw); + } + + // `Lid=TEXT` sets the anchored line. Legacy `Lid|TEXT` remains accepted. + // A bare `Lid` is a cursor move. + const lidStmt = parseLidStmt(line, lineNum); + if (lidStmt) return [lidStmt]; + + if (/^[a-z]{2}(?=[ \t]*[=|])/.test(line) || /^[1-9]\d*(?=[ \t]*[=|]|$)/.test(line)) { + throwMalformedLidDiagnostic(line, lineNum, raw); + } + + // Lenient catch-all: lines that don't match any recognized prefix become + // inserts at the cursor. + return [{ kind: "insert", text: line, lineNum }]; +} + +function tokenizeDiff(diff: string): ParsedStmt[] { + const out: ParsedStmt[] = []; + const lines = diff.split("\n"); + for (let i = 0; i < lines.length; i++) { + const lineNum = i + 1; + const stmts = parseDiffLine(lines[i], lineNum); + for (const stmt of stmts) { + // Last-set-wins: when the same anchor (line+hash) gets a second `set`, + // drop the earlier one. Models sometimes echo the OLD line and then the + // NEW line as replacements (e.g. `119yh|OLD` / `119yh|NEW`); the last is + // the intended value. + if (stmt.kind === "anchor_op" && stmt.op.op === "set") { + const key = `${stmt.anchor.line}:${stmt.anchor.hash}`; + for (let j = out.length - 1; j >= 0; j--) { + const prior = out[j]; + if ( + prior.kind === "anchor_op" && + prior.op.op === "set" && + `${prior.anchor.line}:${prior.anchor.hash}` === key + ) { + out.splice(j, 1); + break; + } + } + } + out.push(stmt); } } - if (entry.post !== undefined) { - resolved.push({ op: "post", pos: loc.pos, lines: hashlineParseText(entry.post) }); - } - if (entry.replace !== undefined) { - const spliceIsExplicitReplacement = Array.isArray(entry.splice) && entry.splice.length > 0; - // Models often duplicate intent by sending both an explicit `splice` and a - // matching `replace`. The explicit replacement wins; the redundant - // `replace` would otherwise trigger a confusing `Conflicting ops` rejection. - if (!spliceIsExplicitReplacement) { - const spec = parseReplaceSpec(entry.replace, editIndex); - resolved.push({ op: "replace", pos: loc.pos, spec, expression: formatReplaceExpression(spec) }); + return normalizeHunks(out); +} + +// Detect contiguous `[delete | delete_with_old]+ [insert | diffish_add]+` +// hunks and reorder so adds land at the FIRST delete's slot (block +// replacement). Single-line `-Lid` + `+Lid|TEXT` (same Lid) fuses to a +// `set`. Standalone `+Lid|TEXT` and `+Lid|TEXT` referencing a Lid not in +function normalizeHunks(stmts: ParsedStmt[]): ParsedStmt[] { + const isDelete = (s: ParsedStmt): boolean => + (s.kind === "anchor_op" && s.op.op === "delete") || s.kind === "delete_with_old"; + const isAdd = (s: ParsedStmt): boolean => s.kind === "insert" || s.kind === "diffish_add"; + const out: ParsedStmt[] = []; + let i = 0; + while (i < stmts.length) { + const stmt = stmts[i]; + if (!isDelete(stmt)) { + if (stmt.kind === "diffish_add") { + const lid = `${stmt.anchor.line}${stmt.anchor.hash}`; + throw new Error( + `Diff line ${stmt.lineNum}: \`+${lid}|...\` looks like a unified-diff replacement marker. Use \`${lid}=TEXT\` to replace, or precede with \`-${lid}\` to delete-then-replace.`, + ); + } + out.push(stmt); + i++; + continue; + } + const deletes: ParsedStmt[] = []; + while (i < stmts.length && isDelete(stmts[i])) { + deletes.push(stmts[i]); + i++; + } + const adds: ParsedStmt[] = []; + while (i < stmts.length && isAdd(stmts[i])) { + adds.push(stmts[i]); + i++; + } + const deletedLids = new Set( + deletes.map(d => { + const a = (d as { anchor: ParsedAnchor }).anchor; + return `${a.line}${a.hash}`; + }), + ); + for (const add of adds) { + if (add.kind !== "diffish_add") continue; + const lid = `${add.anchor.line}${add.anchor.hash}`; + if (!deletedLids.has(lid)) { + throw new Error( + `Diff line ${add.lineNum}: \`+${lid}|...\` references a Lid that was not deleted in the preceding run. Use \`${lid}=TEXT\` to replace, or precede with \`-${lid}\`.`, + ); + } + } + // Single-line case: 1 delete (with or without OLD) + 1 diffish_add same Lid → fuse to set. + if (deletes.length === 1 && adds.length === 1 && adds[0].kind === "diffish_add") { + const dAnchor = (deletes[0] as { anchor: ParsedAnchor }).anchor; + const a = adds[0]; + if (a.anchor.line === dAnchor.line && a.anchor.hash === dAnchor.hash) { + out.push({ + kind: "anchor_op", + anchor: a.anchor, + op: { op: "set", text: a.text, allowOldNewRepair: false }, + lineNum: a.lineNum, + }); + continue; + } + } + // Block: emit delete[0], then all inserts (which land at delete[0]'s slot + // because the cursor binds to delete[0] before the inserts), then the + // remaining deletes. + out.push(deletes[0]); + for (const add of adds) { + const text = add.kind === "insert" ? add.text : (add as DiffishAddStmt).text; + out.push({ kind: "insert", text, lineNum: add.lineNum }); + } + for (let j = 1; j < deletes.length; j++) { + out.push(deletes[j]); } } - return resolved; + return out; +} + +function makeAnchor(anchor: ParsedAnchor): Anchor { + return { line: anchor.line, hash: anchor.hash }; } // ═══════════════════════════════════════════════════════════════════════════ -// Validation +// Build cursor-program from ParsedStmt[] // ═══════════════════════════════════════════════════════════════════════════ -function* getAtomAnchors(edit: AtomEdit): Iterable { - switch (edit.op) { - case "splice": - case "pre": - case "post": - case "del": - case "replace": - yield edit.pos; - return; - default: - return; +export function parseAtom(diff: string): AtomEdit[] { + const edits: AtomEdit[] = []; + let cursor: AtomCursor = { kind: "eof" }; + let index = 0; + + for (const stmt of tokenizeDiff(diff)) { + if (stmt.kind === "insert") { + edits.push({ kind: "insert", cursor: cloneCursor(cursor), text: stmt.text, lineNum: stmt.lineNum, index }); + index++; + continue; + } + + if (stmt.kind === "bof") { + cursor = { kind: "bof" }; + continue; + } + if (stmt.kind === "eof") { + cursor = { kind: "eof" }; + continue; + } + + if (stmt.kind === "delete_with_old") { + const anchor = makeAnchor(stmt.anchor); + cursor = { kind: "anchor", anchor: { ...anchor } }; + edits.push({ kind: "delete", anchor, lineNum: stmt.lineNum, index, oldAssertion: stmt.old }); + index++; + continue; + } + + if (stmt.kind === "diffish_add") { + throw new Error("Internal atom error: unresolved diff-ish add reached parseAtom."); + } + + const anchor = makeAnchor(stmt.anchor); + cursor = { kind: "anchor", anchor: { ...anchor } }; + if (stmt.kind === "bare_anchor") continue; + + if (stmt.op.op === "set") { + if (stmt.op.text.includes("\r")) { + throw new Error( + `Diff line ${stmt.lineNum}: set value contains a carriage return; use a single-line value.`, + ); + } + edits.push({ + kind: "set", + anchor, + text: stmt.op.text, + lineNum: stmt.lineNum, + index, + allowOldNewRepair: stmt.op.allowOldNewRepair, + }); + index++; + continue; + } + + edits.push({ kind: "delete", anchor, lineNum: stmt.lineNum, index }); + index++; } + + return edits; } -/** - * Search for a line near `anchor.line` whose trimmed content equals the - * anchor's content hint. Returns the closest match (preferring lines below the - * requested anchor on ties) or `null` when no line matches. Strict equality on - * trimmed content keeps this conservative — we only retarget when there is no - * ambiguity about the model's intent. - */ -function findLineByContentHint(anchor: Anchor, fileLines: string[]): number | null { - const hint = anchor.contentHint?.trim(); - if (!hint) return null; - const lo = Math.max(1, anchor.line - ANCHOR_REBASE_WINDOW); - const hi = Math.min(fileLines.length, anchor.line + ANCHOR_REBASE_WINDOW); - let best: { line: number; distance: number } | null = null; - for (let line = lo; line <= hi; line++) { - if (fileLines[line - 1].trim() !== hint) continue; - const distance = Math.abs(line - anchor.line); - if (best === null || distance < best.distance) { - best = { line, distance }; - } - } - return best?.line ?? null; +function formatNoAtomEditDiagnostic(_path: string, diff: string): string { + const body = diff + .split("\n") + .map(line => (line.endsWith("\r") ? line.slice(0, -1) : line)) + .filter(line => line.trim().length > 0) + .slice(0, 3) + .map(line => ` ${line}`) + .join("\n"); + const preview = body.length > 0 ? `\nReceived only locator/context lines:\n${body}` : ""; + return `Cursor moved but no mutation found. Add +TEXT to insert, -Lid to delete, or Lid=TEXT to replace.${preview}`; +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Apply cursor-program +// ═══════════════════════════════════════════════════════════════════════════ + +function getAtomEditAnchors(edit: AtomEdit): Anchor[] { + if (edit.kind === "set" || edit.kind === "delete") return [edit.anchor]; + if (edit.cursor.kind === "anchor") return [edit.cursor.anchor]; + return []; } function validateAtomAnchors(edits: AtomEdit[], fileLines: string[], warnings: string[]): HashMismatch[] { const mismatches: HashMismatch[] = []; + const rebasedAnchors = new Map(); for (const edit of edits) { - for (const anchor of getAtomAnchors(edit)) { + for (const anchor of getAtomEditAnchors(edit)) { if (anchor.line < 1 || anchor.line > fileLines.length) { throw new Error(`Line ${anchor.line} does not exist (file has ${fileLines.length} lines)`); } const actualHash = computeLineHash(anchor.line, fileLines[anchor.line - 1]); if (actualHash === anchor.hash) continue; - // When the model supplied a content hint after the anchor (e.g. - // `82zu| for (...)`), prefer rebasing to the line that actually matches - // that content. This avoids false positives from hash-only rebasing where - // a coincidentally matching hash on a nearby line silently retargets the - // edit to the wrong line. - const hinted = findLineByContentHint(anchor, fileLines); - if (hinted !== null) { - const original = `${anchor.line}${anchor.hash}`; - const hintedHash = computeLineHash(hinted, fileLines[hinted - 1]); - anchor.line = hinted; - anchor.hash = hintedHash; - warnings.push( - `Auto-rebased anchor ${original} → ${hinted}${hintedHash} (matched the content hint provided after the anchor).`, - ); - continue; - } + const rebased = tryRebaseAnchor(anchor, fileLines); if (rebased !== null) { const original = `${anchor.line}${anchor.hash}`; + rebasedAnchors.set(anchor, { line: anchor.line, expected: anchor.hash, actual: actualHash }); anchor.line = rebased; warnings.push( `Auto-rebased anchor ${original} → ${rebased}${anchor.hash} (line shifted within ±${ANCHOR_REBASE_WINDOW}; hash matched).`, @@ -399,49 +496,120 @@ function validateAtomAnchors(edits: AtomEdit[], fileLines: string[], warnings: s mismatches.push({ line: anchor.line, expected: anchor.hash, actual: actualHash }); } } + + // Detect post-rebase conflicts. If any conflicting anchor was rebased, surface + // the original hash mismatch instead — the rebase itself is what created the + // conflict, and the model needs to fix the stale anchor, not deduplicate. + const seenLines = new Map(); + for (const edit of edits) { + if (edit.kind !== "set" && edit.kind !== "delete") continue; + const existing = seenLines.get(edit.anchor.line); + if (existing) { + const rebasedA = rebasedAnchors.get(edit.anchor); + const rebasedB = rebasedAnchors.get(existing); + if (rebasedA) mismatches.push(rebasedA); + else if (rebasedB) mismatches.push(rebasedB); + continue; + } + seenLines.set(edit.anchor.line, edit.anchor); + } return mismatches; } -function validateNoConflictingAnchorOps(edits: AtomEdit[]): void { - // For each anchor line, at most one mutating op (splice/del). Multiple - // `replace` ops on the same line are allowed and applied sequentially. - // `pre`/`post` (insert ops) may coexist with them — they don't mutate the - // anchor line. +function validateNoConflictingAtomMutations(edits: AtomEdit[]): void { const mutatingPerLine = new Map(); for (const edit of edits) { - if (edit.op !== "splice" && edit.op !== "del" && edit.op !== "replace") continue; - const existing = mutatingPerLine.get(edit.pos.line); + if (edit.kind !== "set" && edit.kind !== "delete") continue; + const existing = mutatingPerLine.get(edit.anchor.line); if (existing) { - if (existing === "replace" && edit.op === "replace") continue; throw new Error( - `Conflicting ops on anchor line ${edit.pos.line}: \`${existing}\` and \`${edit.op}\`. ` + - `At most one of splice/del is allowed per anchor.`, + `Conflicting ops on anchor line ${edit.anchor.line}: \`${existing}\` and \`${edit.kind}\`. ` + + "At most one mutating op (set/delete) is allowed per anchor.", ); } - mutatingPerLine.set(edit.pos.line, edit.op); + mutatingPerLine.set(edit.anchor.line, edit.kind); } } -// ═══════════════════════════════════════════════════════════════════════════ -// Apply -// ═══════════════════════════════════════════════════════════════════════════ - -export interface AtomNoopEdit { - editIndex: number; - loc: string; - reason: string; - current: string; +function repairAtomOldNewSetLine(currentLine: string, nextLine: string): string { + const marker = `${currentLine}|`; + if (!nextLine.startsWith(marker)) return nextLine; + const repaired = nextLine.slice(marker.length); + return repaired.length > 0 ? repaired : nextLine; } -export function applyAtomEdits( - text: string, - edits: AtomEdit[], -): { - lines: string; - firstChangedLine: number | undefined; - warnings?: string[]; - noopEdits?: AtomNoopEdit[]; -} { +function insertAtStart(fileLines: string[], lines: string[]): void { + if (lines.length === 0) return; + if (fileLines.length === 1 && fileLines[0] === "") { + fileLines.splice(0, 1, ...lines); + return; + } + fileLines.splice(0, 0, ...lines); +} + +function insertAtEnd(fileLines: string[], lines: string[]): number | undefined { + if (lines.length === 0) return undefined; + if (fileLines.length === 1 && fileLines[0] === "") { + fileLines.splice(0, 1, ...lines); + return 1; + } + const hasTrailingNewline = fileLines.length > 0 && fileLines[fileLines.length - 1] === ""; + const insertIdx = hasTrailingNewline ? fileLines.length - 1 : fileLines.length; + fileLines.splice(insertIdx, 0, ...lines); + return insertIdx + 1; +} + +function isSameFileCursor(a: AtomCursor, b: AtomCursor): boolean { + return a.kind === b.kind && a.kind !== "anchor"; +} + +function collectFileInsertRuns( + fileInserts: Extract[], +): Array<{ cursor: AtomCursor; lines: string[] }> { + const runs: Array<{ cursor: AtomCursor; lines: string[] }> = []; + for (const edit of fileInserts.sort((a, b) => a.index - b.index)) { + const prev = runs[runs.length - 1]; + if (prev && isSameFileCursor(prev.cursor, edit.cursor)) { + prev.lines.push(edit.text); + continue; + } + runs.push({ cursor: edit.cursor, lines: [edit.text] }); + } + return runs; +} +function applyFileCursorInserts( + fileLines: string[], + fileInserts: Extract[], +): number | undefined { + let firstChangedLine: number | undefined; + const trackFirstChanged = (line: number) => { + if (firstChangedLine === undefined || line < firstChangedLine) firstChangedLine = line; + }; + + for (const run of collectFileInsertRuns(fileInserts)) { + if (run.cursor.kind === "bof") { + insertAtStart(fileLines, run.lines); + trackFirstChanged(1); + continue; + } + if (run.cursor.kind === "eof") { + const changedLine = insertAtEnd(fileLines, run.lines); + if (changedLine !== undefined) trackFirstChanged(changedLine); + } + } + + return firstChangedLine; +} + +function getAnchorForAnchorEdit(edit: IndexedAnchorEdit["edit"]): Anchor { + if (edit.kind !== "insert") return edit.anchor; + if (edit.cursor.kind !== "anchor") { + throw new Error("Internal atom error: file-scoped insert reached anchor application."); + } + return edit.cursor.anchor; +} + +export function applyAtomEdits(text: string, edits: AtomEdit[]): AtomApplyResult { if (edits.length === 0) { return { lines: text, firstChangedLine: undefined }; } @@ -455,56 +623,31 @@ export function applyAtomEdits( if (mismatches.length > 0) { throw new HashlineMismatchError(mismatches, fileLines); } - // When a `del` and a `replace`/`splice` target the same anchor (across separate - // edit entries), the `del` is almost always a hallucinated cleanup the model - // added on top of the real replacement. Drop the `del` silently so the - // replacement wins, matching the in-entry handling for `splice: []` paired - // with `replace`. - const replacedLines = new Set(); - for (const e of edits) { - if (e.op === "splice" || e.op === "replace") replacedLines.add(e.pos.line); - } - let effective = edits; - if (replacedLines.size > 0) { - effective = edits.filter(e => !(e.op === "del" && replacedLines.has(e.pos.line))); - } - validateNoConflictingAnchorOps(effective); + validateNoConflictingAtomMutations(edits); const trackFirstChanged = (line: number) => { - if (firstChangedLine === undefined || line < firstChangedLine) { - firstChangedLine = line; - } + if (firstChangedLine === undefined || line < firstChangedLine) firstChangedLine = line; }; - // Partition: anchor-scoped vs file-scoped. Preserve original order via the - // captured idx so multiple pre/post on the same target are emitted in the order - // the model produced them. - type Indexed = { edit: T; idx: number }; - type AnchorEdit = Exclude; - const anchorEdits: Indexed[] = []; - const appendEdits: Indexed>[] = []; - const replaceFileEdits: Indexed>[] = []; - const prependEdits: Indexed>[] = []; - effective.forEach((edit, idx) => { - if (edit.op === "append_file") appendEdits.push({ edit, idx }); - else if (edit.op === "prepend_file") prependEdits.push({ edit, idx }); - else if (edit.op === "replace_file") replaceFileEdits.push({ edit, idx }); - else anchorEdits.push({ edit, idx }); + const anchorEdits: IndexedAnchorEdit[] = []; + const fileInserts: Extract[] = []; + edits.forEach((edit, idx) => { + if (edit.kind === "insert" && edit.cursor.kind !== "anchor") { + fileInserts.push(edit); + return; + } + anchorEdits.push({ edit, idx }); }); - // Group anchor edits by line so all ops on the same line are applied as a - // single splice. This makes the per-anchor outcome independent of index - // shifts caused by sibling ops (e.g. `post` paired with `del` on the same - // anchor, or repeated `pre`/`post` inserts that previously reversed). - const byLine = new Map[]>(); + const byLine = new Map(); for (const entry of anchorEdits) { - const line = entry.edit.pos.line; - let bucket = byLine.get(line); - if (!bucket) { - bucket = []; - byLine.set(line, bucket); + const line = getAnchorForAnchorEdit(entry.edit).line; + const bucket = byLine.get(line); + if (bucket) { + bucket.push(entry); + } else { + byLine.set(line, [entry]); } - bucket.push(entry); } const anchorLines = [...byLine.keys()].sort((a, b) => b - a); @@ -518,219 +661,447 @@ export function applyAtomEdits( let replacement: string[] = [currentLine]; let replacementSet = false; let anchorMutated = false; - let anchorDeleted = false; - const beforeLines: string[] = []; const afterLines: string[] = []; for (const { edit } of bucket) { - switch (edit.op) { - case "pre": - beforeLines.push(...edit.lines); + switch (edit.kind) { + case "insert": + afterLines.push(edit.text); break; - case "post": - afterLines.push(...edit.lines); - break; - case "del": - replacement = []; - replacementSet = true; - anchorDeleted = true; - break; - case "splice": - replacement = edit.lines.length === 0 ? [""] : [...edit.lines]; + case "set": + replacement = [edit.allowOldNewRepair ? repairAtomOldNewSetLine(currentLine, edit.text) : edit.text]; replacementSet = true; anchorMutated = true; break; - case "replace": { - const input = replacementSet ? (replacement[0] ?? "") : currentLine; - const { result, matched } = applyReplaceToLine(input, edit.spec); - if (!matched) { + case "delete": + if (edit.oldAssertion !== undefined && edit.oldAssertion !== currentLine) { throw new Error( - `Edit replace expression ${JSON.stringify(edit.expression)} did not match line ${edit.pos.line}: ${JSON.stringify(input)}`, + `Diff line ${edit.lineNum}: \`-${edit.anchor.line}${edit.anchor.hash}\` asserts the deleted line is ${JSON.stringify(edit.oldAssertion)}, but the file has ${JSON.stringify(currentLine)}. Re-anchor and retry.`, ); } - replacement = [result]; + replacement = []; replacementSet = true; anchorMutated = true; break; - } } } - const noOp = !replacementSet && beforeLines.length === 0 && afterLines.length === 0; - if (noOp) continue; - - const originalLine = fileLines[idx]; const replacementProducesNoChange = - beforeLines.length === 0 && - afterLines.length === 0 && - replacement.length === 1 && - replacement[0] === originalLine; + afterLines.length === 0 && replacement.length === 1 && replacement[0] === currentLine; if (replacementProducesNoChange) { const firstEdit = bucket[0]?.edit; - const loc = firstEdit ? `${firstEdit.pos.line}${firstEdit.pos.hash}` : `${line}`; - const reason = "replacement is identical to the current line content"; + const anchor = firstEdit ? getAnchorForAnchorEdit(firstEdit) : undefined; noopEdits.push({ editIndex: bucket[0]?.idx ?? 0, - loc, - reason, - current: originalLine, + loc: anchor ? `${anchor.line}${anchor.hash}` : `${line}`, + reason: + firstEdit?.kind === "set" + ? "replacement is identical to the current line content; use `Lid=NEW_TEXT` and do not copy an unchanged read line" + : "replacement is identical to the current line content", + current: currentLine, }); continue; } - const combined = [...beforeLines, ...replacement, ...afterLines]; + const combined = [...replacement, ...afterLines]; fileLines.splice(idx, 1, ...combined); - - if (beforeLines.length > 0 || anchorMutated || anchorDeleted) { + if (anchorMutated) { trackFirstChanged(line); } else if (afterLines.length > 0) { trackFirstChanged(line + 1); } + if (!replacementSet && afterLines.length === 0) continue; } - // Apply prepend_file ops in original order so the first one ends up at the - // very top of the file. - prependEdits.sort((a, b) => a.idx - b.idx); - for (const { edit } of prependEdits) { - if (edit.lines.length === 0) continue; - if (fileLines.length === 1 && fileLines[0] === "") { - fileLines.splice(0, 1, ...edit.lines); - } else { - // Insert in reverse cumulative order so later splices push earlier - // content further down, preserving the original op order. - fileLines.splice(0, 0, ...edit.lines); - } - trackFirstChanged(1); - } - - // Apply append_file ops in original order. When the file ends with a - // trailing newline (last split element is the empty sentinel), insert - // before that sentinel so the trailing newline is preserved. - appendEdits.sort((a, b) => a.idx - b.idx); - for (const { edit } of appendEdits) { - if (edit.lines.length === 0) continue; - if (fileLines.length === 1 && fileLines[0] === "") { - fileLines.splice(0, 1, ...edit.lines); - trackFirstChanged(1); - continue; - } - const hasTrailingNewline = fileLines.length > 0 && fileLines[fileLines.length - 1] === ""; - const insertIdx = hasTrailingNewline ? fileLines.length - 1 : fileLines.length; - fileLines.splice(insertIdx, 0, ...edit.lines); - trackFirstChanged(insertIdx + 1); - } - - // Apply replace_file ops last so they observe the post-anchor / post-prepend / - // post-append state of the file. Each op runs across every content line. - replaceFileEdits.sort((a, b) => a.idx - b.idx); - for (const { edit } of replaceFileEdits) { - const hasTrailingNewline = fileLines.length > 1 && fileLines[fileLines.length - 1] === ""; - const upper = hasTrailingNewline ? fileLines.length - 1 : fileLines.length; - let anyMatched = false; - for (let i = 0; i < upper; i++) { - const line = fileLines[i] ?? ""; - const r = applyReplaceToLine(line, edit.spec); - if (!r.matched) continue; - anyMatched = true; - if (r.result !== line) { - fileLines[i] = r.result; - trackFirstChanged(i + 1); - } - } - if (!anyMatched) { - throw new Error( - `Edit replace expression ${JSON.stringify(edit.expression)} did not match any line in the file.`, - ); - } - } + const fileFirstChangedLine = applyFileCursorInserts(fileLines, fileInserts); + if (fileFirstChangedLine !== undefined) trackFirstChanged(fileFirstChangedLine); return { lines: fileLines.join("\n"), firstChangedLine, ...(warnings.length > 0 ? { warnings } : {}), - ...(noopEdits.length > 0 ? { noopEdits } : {}), + ...(noopEdits.length > 0 && firstChangedLine === undefined ? { noopEdits } : {}), }; } // ═══════════════════════════════════════════════════════════════════════════ +// Wire-format split: extract `---` headers from the input string. +// ═══════════════════════════════════════════════════════════════════════════ + +const FILE_HEADER_PREFIX = "---"; +const REMOVE_FILE_OPERATION = "!rm"; +const MOVE_FILE_OPERATION = "!mv"; + +type AtomWholeFileOperation = + | { kind: "delete"; lineNum: number } + | { kind: "move"; destination: string; lineNum: number }; + +interface AtomInputSection { + path: string; + diff: string; + wholeFileOperation?: AtomWholeFileOperation; +} + +export interface SplitAtomOptions { + cwd?: string; + path?: string; +} + +function isBlankHeaderPreamble(line: string): boolean { + return line.replace(/\r$/, "").trim().length === 0; +} + +function unquoteAtomPath(pathText: string): string { + if (pathText.length < 2) return pathText; + const first = pathText[0]; + const last = pathText[pathText.length - 1]; + if ((first === '"' || first === "'") && first === last) { + return pathText.slice(1, -1); + } + return pathText; +} + +function normalizeAtomPath(rawPath: string, cwd?: string): string { + const unquoted = unquoteAtomPath(rawPath.trim()); + if (!cwd || !path.isAbsolute(unquoted)) return unquoted; + + const relative = path.relative(path.resolve(cwd), path.resolve(unquoted)); + const isWithinCwd = relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); + return isWithinCwd ? relative || "." : unquoted; +} + +function parseAtomHeaderLine(line: string, cwd?: string): string | null { + if (!line.startsWith(FILE_HEADER_PREFIX)) return null; + let body = line.slice(FILE_HEADER_PREFIX.length); + if (body.startsWith(" ")) body = body.slice(1); + const parsedPath = normalizeAtomPath(body, cwd); + if (parsedPath.length === 0) { + throw new Error(`atom input header "${FILE_HEADER_PREFIX}" is empty; provide a file path.`); + } + return parsedPath; +} + +function parseSingleAtomPathArgument(rawPath: string, directive: string, lineNum: number, cwd?: string): string { + const trimmed = rawPath.trim(); + if (trimmed.length === 0) { + throw new Error(`Atom line ${lineNum}: ${directive} requires exactly one non-empty destination path.`); + } + + const quote = trimmed[0]; + if (quote === '"' || quote === "'") { + if (trimmed.length < 2 || trimmed[trimmed.length - 1] !== quote) { + throw new Error(`Atom line ${lineNum}: ${directive} requires exactly one destination path.`); + } + } else if (/\s/.test(trimmed)) { + throw new Error(`Atom line ${lineNum}: ${directive} requires exactly one destination path.`); + } + + const destination = normalizeAtomPath(trimmed, cwd); + if (destination.length === 0) { + throw new Error(`Atom line ${lineNum}: ${directive} requires exactly one non-empty destination path.`); + } + return destination; +} + +function parseAtomWholeFileOperationLine( + rawLine: string, + lineNum: number, + cwd?: string, +): AtomWholeFileOperation | null { + const line = rawLine.replace(/\r$/, "").trimEnd(); + if (line === REMOVE_FILE_OPERATION) { + return { kind: "delete", lineNum }; + } + if (line.startsWith(`${REMOVE_FILE_OPERATION} `) || line.startsWith(`${REMOVE_FILE_OPERATION}\t`)) { + throw new Error(`Atom line ${lineNum}: ${REMOVE_FILE_OPERATION} does not take a destination path.`); + } + + if (line === MOVE_FILE_OPERATION) { + throw new Error(`Atom line ${lineNum}: ${MOVE_FILE_OPERATION} requires exactly one non-empty destination path.`); + } + if (line.startsWith(`${MOVE_FILE_OPERATION} `) || line.startsWith(`${MOVE_FILE_OPERATION}\t`)) { + const rawDestination = line.slice(MOVE_FILE_OPERATION.length); + return { + kind: "move", + destination: parseSingleAtomPathArgument(rawDestination, MOVE_FILE_OPERATION, lineNum, cwd), + lineNum, + }; + } + + return null; +} + +function getAtomWholeFileOperation( + sectionPath: string, + lines: string[], + cwd?: string, +): AtomWholeFileOperation | undefined { + let operation: AtomWholeFileOperation | undefined; + let operationToken = ""; + let hasLineEdit = false; + + for (let i = 0; i < lines.length; i++) { + const lineNum = i + 1; + const line = lines[i].replace(/\r$/, ""); + if (line.trim().length === 0) continue; + + const parsed = parseAtomWholeFileOperationLine(line, lineNum, cwd); + if (parsed) { + if (operation) { + throw new Error( + `Atom section ${sectionPath}: use only one ${REMOVE_FILE_OPERATION} or ${MOVE_FILE_OPERATION} operation.`, + ); + } + operation = parsed; + operationToken = parsed.kind === "delete" ? REMOVE_FILE_OPERATION : MOVE_FILE_OPERATION; + continue; + } + + hasLineEdit = true; + } + + if (operation && hasLineEdit) { + throw new Error( + `Atom section ${sectionPath} mixes ${operationToken} with line edits; ${REMOVE_FILE_OPERATION} and ${MOVE_FILE_OPERATION} must be the only operation in their section.`, + ); + } + + return operation; +} + +function hasAtomHeaderLine(input: string): boolean { + const stripped = input.startsWith("\uFEFF") ? input.slice(1) : input; + return stripped.split("\n").some(rawLine => rawLine.replace(/\r$/, "").startsWith(FILE_HEADER_PREFIX)); +} + +function containsRecognizableAtomOperations(input: string): boolean { + for (const rawLine of input.split("\n")) { + const line = rawLine.replace(/\r$/, ""); + if (line.length === 0) continue; + if (line[0] === "+") return true; + if (line === "$" || line === "^") return true; + if (/^- ?[1-9]\d*[a-z]{2}(?: .*)?$/.test(line)) return true; + if (/^@?[1-9]\d*[a-z]{2}(?:[ \t]*[=|].*)?$/.test(line)) return true; + if (/^@@ (?:BOF|EOF|(?:- ?)?[1-9]\d*[a-z]{2}(?:[ \t]*[=|].*)?)$/.test(line)) return true; + } + return false; +} + +function stripLeadingBlankLines(input: string): string { + const stripped = input.startsWith("\uFEFF") ? input.slice(1) : input; + const lines = stripped.split("\n"); + while (lines.length > 0 && isBlankHeaderPreamble(lines[0] ?? "")) { + lines.shift(); + } + return lines.join("\n"); +} + +function normalizeFallbackInput(input: string, options: SplitAtomOptions): string { + if (hasAtomHeaderLine(input) || !options.path || !containsRecognizableAtomOperations(input)) { + return input; + } + const fallbackPath = normalizeAtomPath(options.path, options.cwd); + if (fallbackPath.length === 0) return input; + return `${FILE_HEADER_PREFIX}${fallbackPath}\n${input}`; +} + +function getTextContent(result: AgentToolResult): string { + return result.content.map(part => (part.type === "text" ? part.text : "")).join("\n"); +} + +function getEditDetails(result: AgentToolResult): EditToolDetails { + if (result.details === undefined) { + return { diff: "" }; + } + return result.details; +} + +/** + * Split the wire-format `input` string into `{ path, diff }`. The first + * non-empty line MUST be `---` or `--- `. Tolerates a leading BOM. + */ +export function splitAtomInput(input: string, options: SplitAtomOptions = {}): { path: string; diff: string } { + const [section] = splitAtomInputs(input, options); + return section; +} + +export function splitAtomInputs(input: string, options: SplitAtomOptions = {}): AtomInputSection[] { + const stripped = stripLeadingBlankLines(normalizeFallbackInput(input, options)); + const lines = stripped.split("\n"); + const firstLine = (lines[0] ?? "").replace(/\r$/, ""); + if (!firstLine.startsWith(FILE_HEADER_PREFIX)) { + throw new Error( + `atom input must begin with "${FILE_HEADER_PREFIX}" on the first non-blank line; got: ${JSON.stringify( + firstLine.slice(0, 120), + )}`, + ); + } + + const sections: AtomInputSection[] = []; + let currentPath = ""; + let currentLines: string[] = []; + const flush = () => { + if (currentPath.length === 0) return; + const wholeFileOperation = getAtomWholeFileOperation(currentPath, currentLines, options.cwd); + sections.push({ + path: currentPath, + diff: currentLines.join("\n"), + ...(wholeFileOperation ? { wholeFileOperation } : {}), + }); + currentLines = []; + }; + + for (const rawLine of lines) { + const line = rawLine.replace(/\r$/, ""); + const headerPath = parseAtomHeaderLine(line, options.cwd); + if (headerPath !== null) { + flush(); + currentPath = headerPath; + continue; + } + currentLines.push(rawLine); + } + flush(); + return sections; +} + +// ═════════════════════════════════════════════════════════════════════════════ // Executor // ═══════════════════════════════════════════════════════════════════════════ export interface ExecuteAtomSingleOptions { session: ToolSession; - path: string; - edits: AtomToolEdit[]; + input: string; + path?: string; signal?: AbortSignal; batchRequest?: LspBatchRequest; writethrough: WritethroughCallback; beginDeferredDiagnosticsForPath: (path: string) => WritethroughDeferredHandle; } -export async function executeAtomSingle( - options: ExecuteAtomSingleOptions, -): Promise> { - const { session, path, edits, signal, batchRequest, writethrough, beginDeferredDiagnosticsForPath } = options; +interface ReadAtomFileResult { + exists: boolean; + rawContent: string; +} - const contentEdits = edits.flatMap((edit, i) => resolveAtomToolEdit(edit, i, path)); +async function readAtomFile(absolutePath: string): Promise { + try { + return { exists: true, rawContent: await Bun.file(absolutePath).text() }; + } catch (error) { + if (isEnoent(error)) return { exists: false, rawContent: "" }; + throw error; + } +} + +function hasAnchorScopedEdit(edits: AtomEdit[]): boolean { + return edits.some(edit => edit.kind === "set" || edit.kind === "delete" || edit.cursor.kind === "anchor"); +} + +function formatNoChangeDiagnostic(path: string, result: AtomApplyResult): string { + let diagnostic = `Edits to ${path} resulted in no changes being made.`; + if (result.noopEdits && result.noopEdits.length > 0) { + const details = result.noopEdits + .map(e => { + const preview = + e.current.length > 0 + ? `\n current: ${JSON.stringify(e.current.length > 200 ? `${e.current.slice(0, 200)}…` : e.current)}` + : ""; + return `Edit ${e.editIndex} (${e.loc}): ${e.reason}.${preview}`; + }) + .join("\n"); + diagnostic += `\n${details}`; + } + return diagnostic; +} + +async function executeAtomWholeFileOperation( + options: ExecuteAtomSingleOptions & AtomInputSection & { wholeFileOperation: AtomWholeFileOperation }, +): Promise> { + const { session, path: sectionPath, wholeFileOperation } = options; + const absolutePath = resolvePlanPath(session, sectionPath); + + if (sectionPath.endsWith(".ipynb")) { + throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); + } + + if (wholeFileOperation.kind === "delete") { + enforcePlanModeWrite(session, sectionPath, { op: "delete" }); + await assertEditableFile(absolutePath, sectionPath); + try { + await fs.unlink(absolutePath); + } catch (error) { + if (isEnoent(error)) throw new Error(`File not found: ${sectionPath}`); + throw error; + } + invalidateFsScanAfterDelete(absolutePath); + return { + content: [{ type: "text", text: `Deleted ${sectionPath}` }], + details: { diff: "", op: "delete", meta: outputMeta().get() }, + }; + } + + const destinationPath = wholeFileOperation.destination; + if (destinationPath.endsWith(".ipynb")) { + throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); + } + + enforcePlanModeWrite(session, sectionPath, { op: "update", move: destinationPath }); + const absoluteDestinationPath = resolvePlanPath(session, destinationPath); + if (absoluteDestinationPath === absolutePath) { + throw new Error("rename path is the same as source path"); + } + + await assertEditableFile(absolutePath, sectionPath); + try { + await fs.mkdir(path.dirname(absoluteDestinationPath), { recursive: true }); + await fs.rename(absolutePath, absoluteDestinationPath); + } catch (error) { + if (isEnoent(error)) throw new Error(`File not found: ${sectionPath}`); + throw error; + } + invalidateFsScanAfterRename(absolutePath, absoluteDestinationPath); + + return { + content: [{ type: "text", text: `Moved ${sectionPath} to ${destinationPath}` }], + details: { diff: "", op: "update", move: destinationPath, meta: outputMeta().get() }, + }; +} + +async function executeAtomSection( + options: ExecuteAtomSingleOptions & AtomInputSection, +): Promise> { + const { session, path, diff, signal, batchRequest, writethrough, beginDeferredDiagnosticsForPath } = options; + if (options.wholeFileOperation) { + return executeAtomWholeFileOperation({ ...options, wholeFileOperation: options.wholeFileOperation }); + } + + const edits = parseAtom(diff); + if (edits.length === 0 && diff.trim().length > 0) { + throw new Error(formatNoAtomEditDiagnostic(path, diff)); + } enforcePlanModeWrite(session, path, { op: "update" }); - if (path.endsWith(".ipynb") && contentEdits.length > 0) { + if (path.endsWith(".ipynb") && edits.length > 0) { throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); } const absolutePath = resolvePlanPath(session, path); - - const sourceFile = Bun.file(absolutePath); - const sourceExists = await sourceFile.exists(); - - if (!sourceExists) { - const lines: string[] = []; - for (const edit of contentEdits) { - if (edit.op === "append_file") { - lines.push(...edit.lines); - } else if (edit.op === "prepend_file") { - lines.unshift(...edit.lines); - } else { - throw new Error(`File not found: ${path}`); - } - } - - await Bun.write(absolutePath, lines.join("\n")); - invalidateFsScanAfterWrite(absolutePath); - return { - content: [{ type: "text", text: `Created ${path}` }], - details: { - diff: "", - op: "create", - meta: outputMeta().get(), - }, - }; + const source = await readAtomFile(absolutePath); + if (!source.exists && hasAnchorScopedEdit(edits)) { + throw new Error(`File not found: ${path}`); } - const rawContent = await sourceFile.text(); - assertEditableFileContent(rawContent, path); + if (source.exists) { + assertEditableFileContent(source.rawContent, path); + } - const { bom, text } = stripBom(rawContent); + const { bom, text } = stripBom(source.rawContent); const originalEnding = detectLineEnding(text); const originalNormalized = normalizeToLF(text); - - const result = applyAtomEdits(originalNormalized, contentEdits); + const result = applyAtomEdits(originalNormalized, edits); if (originalNormalized === result.lines) { - let diagnostic = `Edits to ${path} resulted in no changes being made.`; - if (result.noopEdits && result.noopEdits.length > 0) { - const details = result.noopEdits - .map(e => { - const preview = - e.current.length > 0 - ? `\n current: ${JSON.stringify(e.current.length > 200 ? `${e.current.slice(0, 200)}…` : e.current)}` - : ""; - return `Edit ${e.editIndex} (${e.loc}): ${e.reason}.${preview}`; - }) - .join("\n"); - diagnostic += `\n${details}`; - } - throw new Error(diagnostic); + throw new Error(formatNoChangeDiagnostic(path, result)); } const finalContent = bom + restoreLineEndings(result.lines, originalEnding); @@ -748,14 +1119,13 @@ export async function executeAtomSingle( const meta = outputMeta() .diagnostics(diagnostics?.summary ?? "", diagnostics?.messages ?? []) .get(); - - const resultText = `Updated ${path}`; const preview = buildCompactHashlineDiffPreview(diffResult.diff); const summaryLine = `Changes: +${preview.addedLines} -${preview.removedLines}${ preview.preview ? "" : " (no textual diff preview)" }`; const warningsBlock = result.warnings?.length ? `\n\nWarnings:\n${result.warnings.join("\n")}` : ""; const previewBlock = preview.preview ? `\n\nDiff preview:\n${preview.preview}` : ""; + const resultText = source.exists ? `Updated ${path}` : `Created ${path}`; return { content: [ @@ -768,11 +1138,50 @@ export async function executeAtomSingle( diff: diffResult.diff, firstChangedLine: result.firstChangedLine ?? diffResult.firstChangedLine, diagnostics, - op: "update", + op: source.exists ? "update" : "create", meta, }, }; } -// Helpers exposed for tests / external dispatch. -export { classifyAtomEdit, parseAnchor, resolveAtomToolEdit }; +export async function executeAtomSingle( + options: ExecuteAtomSingleOptions, +): Promise> { + const sections = splitAtomInputs(options.input, { cwd: options.session.cwd, path: options.path }); + if (sections.length === 1) { + const [section] = sections; + return executeAtomSection({ ...options, ...section }); + } + + const results = []; + for (const section of sections) { + results.push({ + path: section.path, + result: await executeAtomSection({ ...options, ...section }), + }); + } + + return { + content: [ + { + type: "text", + text: results.map(({ result }) => getTextContent(result)).join("\n\n"), + }, + ], + details: { + diff: results.map(({ result }) => getEditDetails(result).diff).join("\n"), + perFileResults: results.map(({ path, result }) => { + const details = getEditDetails(result); + return { + path, + diff: details.diff, + firstChangedLine: details.firstChangedLine, + diagnostics: details.diagnostics, + op: details.op, + move: details.move, + meta: details.meta, + }; + }), + }, + }; +} diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index 43f54c461..0ebc800d9 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -565,8 +565,8 @@ export class HashlineMismatchError extends Error { const sorted = [...displayLines].sort((a, b) => a - b); const out: string[] = [ - `Edit rejected: ${mismatches.length} line${mismatches.length > 1 ? "s have" : " has"} changed since the last read. The edit was NOT applied.`, - "Realign your edit to the file state shown below. Copy the full anchors exactly as shown (for example `160sr`, not just `sr`).", + `Edit rejected: ${mismatches.length} line${mismatches.length > 1 ? "s have" : " has"} changed since the last read (marked *).`, + "The edit was NOT applied, please use the updated file content shown below, and issue another edit tool-call.", "", ]; @@ -602,8 +602,8 @@ export class HashlineMismatchError extends Error { const lines: string[] = []; lines.push( - `Edit rejected: ${mismatches.length} line${mismatches.length > 1 ? "s have" : " has"} changed since the last read. The edit was NOT applied.`, - "Use the updated anchors shown below (`*` marks changed lines, leading space marks context) and retry the edit.", + `Edit rejected: ${mismatches.length} line${mismatches.length > 1 ? "s have" : " has"} changed since the last read (marked *).`, + "The edit was NOT applied, please use the updated file content shown below, and issue another edit tool-call.", ); lines.push(""); diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index 696c87c2a..f7d372b49 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -106,6 +106,10 @@ type EditRenderEntry = { op?: Operation; }; +interface AtomRenderSummary { + entries: Array<{ path: string }>; +} + interface ApplyPatchRenderSummary { entries: ApplyPatchEntry[]; error?: string; @@ -305,8 +309,54 @@ function getCallPreview( } const MISSING_APPLY_PATCH_END_ERROR = "The last line of the patch must be '*** End Patch'"; +const ATOM_HEADER_PREFIX = "---"; + +function normalizeAtomPreviewPath(rawPath: string): string { + const trimmed = rawPath.trim(); + if (trimmed.length < 2) return trimmed; + const first = trimmed[0]; + const last = trimmed[trimmed.length - 1]; + if ((first === '"' || first === "'") && first === last) { + return trimmed.slice(1, -1); + } + return trimmed; +} + +function parseAtomPreviewHeader(line: string): string | null { + if (!line.startsWith(ATOM_HEADER_PREFIX)) return null; + let body = line.slice(ATOM_HEADER_PREFIX.length); + if (body.startsWith(" ")) body = body.slice(1); + const previewPath = normalizeAtomPreviewPath(body); + return previewPath.length > 0 ? previewPath : null; +} + +function getAtomInputPaths(input: string): string[] { + const stripped = input.startsWith("\uFEFF") ? input.slice(1) : input; + const paths: string[] = []; + for (const rawLine of stripped.split("\n")) { + const line = rawLine.replace(/\r$/, ""); + const path = parseAtomPreviewHeader(line); + if (path) paths.push(path); + } + return paths; +} + +function getAtomRenderSummary(args: EditRenderArgs, editMode: EditMode | undefined): AtomRenderSummary | undefined { + if (editMode !== "atom" || typeof args.input !== "string") { + return undefined; + } + return { entries: getAtomInputPaths(args.input).map(path => ({ path })) }; +} + +function getApplyPatchRenderSummary( + args: EditRenderArgs, + isPartial: boolean, + editMode: EditMode | undefined, +): ApplyPatchRenderSummary | undefined { + if (editMode !== undefined && editMode !== "apply_patch") { + return undefined; + } -function getApplyPatchRenderSummary(args: EditRenderArgs, isPartial: boolean): ApplyPatchRenderSummary | undefined { if (typeof args.input !== "string") { return undefined; } @@ -397,8 +447,10 @@ export const editToolRenderer = { } const editArgs = args as EditRenderArgs; - const applyPatchSummary = getApplyPatchRenderSummary(editArgs, options.isPartial); + const atomSummary = getAtomRenderSummary(editArgs, renderContext?.editMode); + const applyPatchSummary = getApplyPatchRenderSummary(editArgs, options.isPartial, renderContext?.editMode); const firstApplyPatchEntry = applyPatchSummary?.entries[0]; + const firstAtomEntry = atomSummary?.entries[0]; // Extract path from first edit entry when top-level path is absent (new schema) const firstEdit = Array.isArray(editArgs.edits) && editArgs.edits.length > 0 ? editArgs.edits[0] : undefined; const rawPath = @@ -406,6 +458,7 @@ export const editToolRenderer = { editArgs.path || filePathFromEditEntry(firstEdit?.path) || getPartialJsonEditPath(editArgs) || + firstAtomEntry?.path || firstApplyPatchEntry?.path || ""; const rename = editArgs.rename || firstEdit?.rename || firstEdit?.move || firstApplyPatchEntry?.rename; @@ -415,9 +468,10 @@ export const editToolRenderer = { options?.spinnerFrame !== undefined ? formatStatusIcon("running", uiTheme, options.spinnerFrame) : ""; let text = `${formatTitle(getOperationTitle(op), uiTheme)} ${spinner ? `${spinner} ` : ""}${description}`; // Show file count hint for multi-file edits - const fileCount = Array.isArray(editArgs.edits) - ? countEditFiles(editArgs.edits) - : (applyPatchSummary?.entries.length ?? 0); + let fileCount = atomSummary?.entries.length ?? applyPatchSummary?.entries.length ?? 0; + if (Array.isArray(editArgs.edits)) { + fileCount = countEditFiles(editArgs.edits); + } if (fileCount > 1) { text += uiTheme.fg("dim", ` (+${fileCount - 1} more)`); } @@ -465,11 +519,14 @@ function renderSingleFileResult( const details = result.details; const isError = result.isError ?? (details && "isError" in details ? details.isError : false); const firstEdit = args?.edits?.[0]; + const atomSummary = getAtomRenderSummary(args ?? {}, options.renderContext?.editMode); + const firstAtomEntry = atomSummary?.entries[0]; const rawPath = args?.file_path || args?.path || filePathFromEditEntry(firstEdit?.path) || (details && "path" in details ? details.path : "") || + firstAtomEntry?.path || ""; const op = args?.op || firstEdit?.op || details?.op; const rename = args?.rename || firstEdit?.rename || firstEdit?.move || details?.move; diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index 9faf6456f..b7ce3a0f2 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -290,18 +290,17 @@ const vimStrategy: EditStreamingStrategy = { }; interface AtomArgs { - path?: string; - edits?: unknown[]; + input?: string; + __partialJson?: string; } const atomStrategy: EditStreamingStrategy = { - extractCompleteEdits(args, partialJson) { - if (!args.edits) return args; - return { ...args, edits: dropIncompleteLastEdit(args.edits, partialJson, "edits") }; + extractCompleteEdits(args) { + return args; }, async computeDiffPreview() { - // Atom edits are line-anchored and validated against live file hashes; a - // streaming preview without that validation could mislead. Skip for now. + // Atom edits can target file headers plus compact diff statements. + // We intentionally avoid speculative parsing while args are partial. return null; }, renderStreamingFallback() { diff --git a/packages/coding-agent/src/lsp/edits.ts b/packages/coding-agent/src/lsp/edits.ts index c92cd24ab..d5e6dbd38 100644 --- a/packages/coding-agent/src/lsp/edits.ts +++ b/packages/coding-agent/src/lsp/edits.ts @@ -1,5 +1,6 @@ import * as fs from "node:fs/promises"; import path from "node:path"; +import { formatPathRelativeToCwd } from "../tools/path-utils"; import type { CreateFile, DeleteFile, RenameFile, TextDocumentEdit, TextEdit, WorkspaceEdit } from "./types"; import { uriToFile } from "./utils"; @@ -67,7 +68,7 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom for (const [uri, textEdits] of Object.entries(edit.changes)) { const filePath = uriToFile(uri); await applyTextEdits(filePath, textEdits); - applied.push(`Applied ${textEdits.length} edit(s) to ${path.relative(cwd, filePath)}`); + applied.push(`Applied ${textEdits.length} edit(s) to ${formatPathRelativeToCwd(filePath, cwd)}`); } } @@ -80,26 +81,28 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom const filePath = uriToFile(docChange.textDocument.uri); const textEdits = docChange.edits.filter((e): e is TextEdit => "range" in e && "newText" in e); await applyTextEdits(filePath, textEdits); - applied.push(`Applied ${textEdits.length} edit(s) to ${path.relative(cwd, filePath)}`); + applied.push(`Applied ${textEdits.length} edit(s) to ${formatPathRelativeToCwd(filePath, cwd)}`); } else if ("kind" in change && change.kind) { // Resource operations if (change.kind === "create") { const createOp = change as CreateFile; const filePath = uriToFile(createOp.uri); await Bun.write(filePath, ""); - applied.push(`Created ${path.relative(cwd, filePath)}`); + applied.push(`Created ${formatPathRelativeToCwd(filePath, cwd)}`); } else if (change.kind === "rename") { const renameOp = change as RenameFile; const oldPath = uriToFile(renameOp.oldUri); const newPath = uriToFile(renameOp.newUri); await fs.mkdir(path.dirname(newPath), { recursive: true }); await fs.rename(oldPath, newPath); - applied.push(`Renamed ${path.relative(cwd, oldPath)} → ${path.relative(cwd, newPath)}`); + applied.push( + `Renamed ${formatPathRelativeToCwd(oldPath, cwd)} → ${formatPathRelativeToCwd(newPath, cwd)}`, + ); } else if (change.kind === "delete") { const deleteOp = change as DeleteFile; const filePath = uriToFile(deleteOp.uri); await fs.rm(filePath, { recursive: true }); - applied.push(`Deleted ${path.relative(cwd, filePath)}`); + applied.push(`Deleted ${formatPathRelativeToCwd(filePath, cwd)}`); } } } diff --git a/packages/coding-agent/src/lsp/index.ts b/packages/coding-agent/src/lsp/index.ts index e847568b0..24d469bbb 100644 --- a/packages/coding-agent/src/lsp/index.ts +++ b/packages/coding-agent/src/lsp/index.ts @@ -6,7 +6,7 @@ import type { BunFile } from "bun"; import { type Theme, theme } from "../modes/theme/theme"; import lspDescription from "../prompts/tools/lsp.md" with { type: "text" }; import type { ToolSession } from "../tools"; -import { resolveToCwd } from "../tools/path-utils"; +import { formatPathRelativeToCwd, resolveToCwd } from "../tools/path-utils"; import { ToolAbortError, throwIfAborted } from "../tools/tool-errors"; import { clampTimeout } from "../tools/tool-timeouts"; import { @@ -562,7 +562,7 @@ async function getDiagnosticsForFile( } const uri = fileToUri(absolutePath); - const relPath = path.relative(cwd, absolutePath); + const relPath = formatPathRelativeToCwd(absolutePath, cwd); const allDiagnostics: Diagnostic[] = []; const serverNames: string[] = []; @@ -1229,7 +1229,7 @@ export class LspTool implements AgentTool formatDocumentSymbol(s)); output = `Symbols in ${relPath}:\n${lines.join("\n")}`; diff --git a/packages/coding-agent/src/lsp/utils.ts b/packages/coding-agent/src/lsp/utils.ts index 89cf1ea22..a7b6e113e 100644 --- a/packages/coding-agent/src/lsp/utils.ts +++ b/packages/coding-agent/src/lsp/utils.ts @@ -5,7 +5,7 @@ import path from "node:path"; import { isEnoent } from "@oh-my-pi/pi-utils"; import { type Theme, theme } from "../modes/theme/theme"; import { formatGroupedFiles } from "../tools/grouped-file-output"; -import { resolveToCwd } from "../tools/path-utils"; +import { formatPathRelativeToCwd, resolveToCwd } from "../tools/path-utils"; import type { CodeAction, Command, @@ -229,7 +229,7 @@ export function formatDiagnosticsSummary(diagnostics: Diagnostic[]): string { * Format a location as file:line:col relative to cwd. */ export function formatLocation(location: Location, cwd: string): string { - const file = path.relative(cwd, uriToFile(location.uri)); + const file = formatPathRelativeToCwd(uriToFile(location.uri), cwd); const line = location.range.start.line + 1; const col = location.range.start.character + 1; return `${file}:${line}:${col}`; @@ -255,7 +255,7 @@ export function formatWorkspaceEdit(edit: WorkspaceEdit, cwd: string): string[] // Handle changes map (legacy format) if (edit.changes) { for (const [uri, textEdits] of Object.entries(edit.changes)) { - const file = path.relative(cwd, uriToFile(uri)); + const file = formatPathRelativeToCwd(uriToFile(uri), cwd); results.push(`${file}: ${textEdits.length} edit${textEdits.length > 1 ? "s" : ""}`); } } @@ -264,20 +264,20 @@ export function formatWorkspaceEdit(edit: WorkspaceEdit, cwd: string): string[] if (edit.documentChanges) { for (const change of edit.documentChanges) { if ("edits" in change && change.textDocument) { - const file = path.relative(cwd, uriToFile(change.textDocument.uri)); + const file = formatPathRelativeToCwd(uriToFile(change.textDocument.uri), cwd); results.push(`${file}: ${change.edits.length} edit${change.edits.length > 1 ? "s" : ""}`); } else if ("kind" in change) { switch (change.kind) { case "create": - results.push(`CREATE: ${path.relative(cwd, uriToFile(change.uri))}`); + results.push(`CREATE: ${formatPathRelativeToCwd(uriToFile(change.uri), cwd)}`); break; case "rename": results.push( - `RENAME: ${path.relative(cwd, uriToFile(change.oldUri))} ${theme.nav.cursor} ${path.relative(cwd, uriToFile(change.newUri))}`, + `RENAME: ${formatPathRelativeToCwd(uriToFile(change.oldUri), cwd)} ${theme.nav.cursor} ${formatPathRelativeToCwd(uriToFile(change.newUri), cwd)}`, ); break; case "delete": - results.push(`DELETE: ${path.relative(cwd, uriToFile(change.uri))}`); + results.push(`DELETE: ${formatPathRelativeToCwd(uriToFile(change.uri), cwd)}`); break; } } diff --git a/packages/coding-agent/src/prompts/system/system-prompt.md b/packages/coding-agent/src/prompts/system/system-prompt.md index e7bb88203..ffe1b94eb 100644 --- a/packages/coding-agent/src/prompts/system/system-prompt.md +++ b/packages/coding-agent/src/prompts/system/system-prompt.md @@ -198,7 +198,8 @@ You **MUST NOT** use Python or Bash when a specialized tool exists. {{/ifAny}} ### Paths -- For tools that take a `path` (or path-like field), prefer cwd-relative paths for files inside the cwd. Use absolute paths only when targeting files outside the cwd or when expanding `~`. +- For tools that take a `path` or path-like field, you **MUST** use cwd-relative paths for files inside the current working directory. +- You **MUST** use absolute paths only when targeting files outside the current working directory or when expanding `~`. {{#has tools "lsp"}} ### LSP guidance @@ -334,7 +335,7 @@ Some values in tool output are intentionally redacted as `#XXXX#` tokens. Treat {{SECTION_SEPARATOR "Now"}} -The current working directory is '{{cwd}}'. +The current working directory is '{{cwd}}'. Paths inside this directory **MUST** be passed to tools as relative paths. Today is '{{date}}'. Begin now. diff --git a/packages/coding-agent/src/prompts/tools/apply-patch.md b/packages/coding-agent/src/prompts/tools/apply-patch.md index d3838c08c..423a7524b 100644 --- a/packages/coding-agent/src/prompts/tools/apply-patch.md +++ b/packages/coding-agent/src/prompts/tools/apply-patch.md @@ -1,5 +1,3 @@ -## `apply_patch` - Use the `apply_patch` shell command to edit files. Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: diff --git a/packages/coding-agent/src/prompts/tools/atom.md b/packages/coding-agent/src/prompts/tools/atom.md index 15188f4e9..42640906c 100644 --- a/packages/coding-agent/src/prompts/tools/atom.md +++ b/packages/coding-agent/src/prompts/tools/atom.md @@ -1,59 +1,82 @@ -Applies precise file edits using anchors (line+hash). +Your patch language is a compact, file-oriented edit format. + +When emitting a patch, the first non-blank line **MUST** be `---PATH`. +A Lid is the anchor emitted in read/grep etc. (line number + id, e.g. `5th`). -Each call **MUST** have shape `{path:"a.ts",edits:[…]}`. `path` is required and applies to every edit in the call; `loc` is anchor-only and **MUST NOT** include a file prefix. -Each edit **MUST** have exactly one `loc` and **MUST** include one or more verbs. - -# Locators -- `"A"` targets one anchored line (line number + 2-letter suffix, e.g. `160sr`). -- `"$"` targets the whole file: `pre` = BOF, `post` = EOF, `replace` = every line. -# Verbs -- `splice:[…]` replaces the anchored line. `[]` deletes; `[""]` makes a blank line. To replace N lines, anchor the first line and list all replacement lines. -- `pre:[…]` inserts before the anchor, or BOF with `loc:"$"`. -- `post:[…]` inserts after the anchor, or EOF with `loc:"$"`. -- `replace:{find,with,all?}` is a literal substring substitution on the anchored line (or every line with `loc:"$"`). No regex — `find` matches as a literal string. `all:true` replaces every occurrence on the line; default replaces only the first. +---PATH start editing PATH with cursor at EOF +!rm delete PATH +!mv X move file to X +$ move cursor to BOF +^ move cursor to EOF +@Lid move cursor after Lid ++X insert X at the cursor; `+` alone inserts a blank line +Lid=X replace whole line with X; `Lid=` blanks it out +-Lid delete line (repeat for multi) - -Use for tiny inline edits: names, operators, literals. -- `find` is a literal substring; do **NOT** escape regex metacharacters — `(`, `)`, `.`, `?`, `[`, `]`, `*`, `+` all match themselves. -- Keep `find` as short as possible while still being unique on the line; it does **NOT** have to be unique across the file. -- `all:false` by default; set `all:true` to replace every occurrence on the line instead of only the first. - + +- You may have multiple `---PATH` sections to edit multiple files at once. +- Ops starting with `$` / `^` / `@Lid` do not alter lines; you must still issue an op like `+` afterwards. +- Consecutive `+X` ops insert consecutive lines. +- `Lid=X` replaces the whole line. X must be the complete new line, not a fragment. + - -```ts title="a.ts" -{{hline 1 "const FALLBACK = \"guest\";"}} + +{{hline 1 "const DEF = \"guest\";"}} {{hline 2 ""}} {{hline 3 "export function label(name) {"}} -{{hline 4 "\tconst clean = name || FALLBACK;"}} -{{hline 5 "\treturn clean.trim().toLowerCase();"}} +{{hline 4 "\tconst clean = name || DEF;"}} +{{hline 5 "\treturn clean.trim();"}} {{hline 6 "}"}} -``` + -# Single-line replacement: -`{path:"a.ts",edits:[{loc:{{href 1 "const FALLBACK = \"guest\";"}},splice:["const FALLBACK = \"anonymous\";"]}]}` -# Small token edit: prefer `replace`: -`{path:"a.ts",edits:[{loc:{{href 5 "\treturn clean.trim().toLowerCase();"}},replace:{find:"toLowerCase",with:"toUpperCase"}}]}` -# Insert before / after an anchor: -`{path:"a.ts",edits:[{loc:{{href 5 "\treturn clean.trim().toLowerCase();"}},pre:["\tif (!clean) return FALLBACK;"],post:["\t// normalized label"]}]}` -# Delete a line vs make it blank: -`{path:"a.ts",edits:[{loc:{{href 2 ""}},splice:[]}]}` -`{path:"a.ts",edits:[{loc:{{href 2 ""}},splice:[""]}]}` -# File edges: -`{path:"a.ts",edits:[{loc:"$",pre:["// Copyright (c) 2026",""]}]}` -`{path:"a.ts",edits:[{loc:"$",post:["","export { FALLBACK };"]}]}` -# Replace several consecutive lines: anchor the first line and list all replacement lines in `splice`. -`{path:"a.ts",edits:[{loc:{{href 4 "\tconst clean = name || FALLBACK;"}},splice:["\tconst clean = String(name ?? FALLBACK).trim();","\treturn clean.toLowerCase();","}"]}]}` -This anchors line 4 and replaces lines 4-6 of the original function body in one splice. The anchor's hash protects against the file having shifted under you. -# WRONG: bare-anchor `splice` only owns the anchored line. If you list 2 replacement lines, you replace 1 line with 2 — the original line 5 still shifts down. -`{path:"a.ts",edits:[{loc:{{href 4 "\tconst clean = name || FALLBACK;"}},splice:["\tconst clean = String(name ?? FALLBACK).trim();","\treturn clean.toLowerCase();"]}]}` -This produces a function with two `return` statements. To replace lines 4 and 5 together, include the original line 6 (`}`) so the splice covers all the lines you intend to replace. + +# Replace line +---a.ts +{{hrefr 5}}= return clean.trim().toUpperCase(); + +# Append after +---a.ts +@{{hrefr 4}} ++ const suffix = ""; + +# Delete a line +---a.ts +-{{hrefr 2}} + +# Prepend and append +---a.ts +$ ++// Copyright (c) 2026 ++ +^ ++export { DEF }; + +# File ops +---a.ts +!rm +---b.ts +!mv a.ts + +# Wrong: `@Lid=TEXT` is not replacement syntax +---a.ts +@{{hrefr 5}}= return clean.trim().toUpperCase(); + +# Wrong: do not split `Lid=TEXT` across lines +---a.ts +{{hrefr 5}}= + return clean.trim().toUpperCase(); + +# Wrong: do not replace by deleting then adding +---a.ts +-{{hrefr 5}} ++{{hrefr 5}}= return clean.trim().toUpperCase(); -- You **MUST** copy full anchors exactly from a read op (e.g. `160sr`); you **MUST NOT** send only the 2-letter suffix. -- You **MUST** make the minimum exact edit; you **MUST NOT** reformat unrelated code. -- A bare anchor owns exactly one line. To replace N lines, anchor the first one and list all N replacement lines in `splice`. -- You **MUST NOT** include unchanged adjacent lines in `splice`/`pre`/`post`; they shift and duplicate. +- Copy Lids **EXACTLY** from prior tool output. Never guess, shorten, or omit the letters. +- Only emit lines that change. Never repeat unchanged context — anchors imply it. +- This is **NOT** unified diff. Never send `@@`, `-OLD` / `+NEW` pairs, or unchanged context. +- Never split `Lid=TEXT` across two physical lines. diff --git a/packages/coding-agent/src/prompts/tools/hashline.md b/packages/coding-agent/src/prompts/tools/hashline.md index 7f220c5d5..246019d2d 100644 --- a/packages/coding-agent/src/prompts/tools/hashline.md +++ b/packages/coding-agent/src/prompts/tools/hashline.md @@ -43,19 +43,19 @@ All examples below reference the same file: # Replace a block body Replace only the catch body. Do not target the shared boundary line `} catch (err) {`. -`{path:"a.ts",edits:[{loc:{range:{pos:{{href 15 "\t\tconsole.error(err);"}},end:{{href 16 "\t\treturn null;"}}}},content:["\t\tif (isEnoent(err)) return null;","\t\tthrow err;"]}]}` +`{path:"a.ts",edits:[{loc:{range:{pos:{{href 15}},end:{{href 16}}}},content:["\t\tif (isEnoent(err)) return null;","\t\tthrow err;"]}]}` # Replace whole block including closing brace -Replace `alpha`'s entire body including the closing `}`. `end` **MUST** be {{href 7 "}"}} because `content` includes `}`. -`{path:"a.ts",edits:[{loc:{range:{pos:{{href 6 "\tlog();"}},end:{{href 7 "}"}}}},content:["\tvalidate();","\tlog();","}"]}]}` -**Wrong**: `end: {{href 6 "\tlog();"}}` — line 7 (`}`) survives AND content emits `}`, producing two closing braces. +Replace `alpha`'s entire body including the closing `}`. `end` **MUST** be {{href 7}} because `content` includes `}`. +`{path:"a.ts",edits:[{loc:{range:{pos:{{href 6}},end:{{href 7}}}},content:["\tvalidate();","\tlog();","}"]}]}` +**Wrong**: `end: {{href 6}}` — line 7 (`}`) survives AND content emits `}`, producing two closing braces. # Replace one line Single-line replace uses `pos == end`. -`{path:"a.ts",edits:[{loc:{range:{pos:{{href 2 "const timeout = 5000;"}},end:{{href 2 "const timeout = 5000;"}}}},content:["const timeout = 30_000;"]}]}` +`{path:"a.ts",edits:[{loc:{range:{pos:{{href 2}},end:{{href 2}}}},content:["const timeout = 30_000;"]}]}` # Delete a range -`{path:"a.ts",edits:[{loc:{range:{pos:{{href 10 "\t// TODO: remove after migration"}},end:{{href 11 "\tlegacy();"}}}},content:null}]}` +`{path:"a.ts",edits:[{loc:{range:{pos:{{href 10}},end:{{href 11}}}},content:null}]}` # Insert before a sibling When adding a sibling declaration, prefer `prepend` on the next declaration. -`{path:"a.ts",edits:[{loc:{prepend:{{href 9 "function beta() {"}}},content:["function gamma() {","\tvalidate();","}",""]}]}` +`{path:"a.ts",edits:[{loc:{prepend:{{href 9}}},content:["function gamma() {","\tvalidate();","}",""]}]}` diff --git a/packages/coding-agent/src/tools/ast-edit.ts b/packages/coding-agent/src/tools/ast-edit.ts index f3a6f4c44..d6a330b28 100644 --- a/packages/coding-agent/src/tools/ast-edit.ts +++ b/packages/coding-agent/src/tools/ast-edit.ts @@ -1,4 +1,3 @@ -import * as path from "node:path"; import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import { type AstReplaceChange, astEdit } from "@oh-my-pi/pi-natives"; import type { Component } from "@oh-my-pi/pi-tui"; @@ -16,6 +15,7 @@ import { createFileRecorder, formatResultPath } from "./file-recorder"; import { formatGroupedFiles } from "./grouped-file-output"; import type { OutputMeta } from "./output-meta"; import { + formatPathRelativeToCwd, hasGlobPathChars, normalizePathLikeInput, parseSearchPath, @@ -106,10 +106,7 @@ export class AstEditTool implements AgentTool { - const relative = path.relative(this.session.cwd, targetPath).replace(/\\/g, "/"); - return relative.length === 0 ? "." : relative; - }; + const formatScopePath = (targetPath: string): string => formatPathRelativeToCwd(targetPath, this.session.cwd); let searchPath: string | undefined; let scopePath: string | undefined; let globFilter: string | undefined; @@ -164,7 +161,8 @@ export class AstEditTool implements AgentTool formatResultPath(filePath, isDirectory); + const formatPath = (filePath: string): string => + formatResultPath(filePath, isDirectory, resolvedSearchPath, this.session.cwd); const { record: recordFile, list: fileList } = createFileRecorder(); const fileReplacementCounts = new Map(); diff --git a/packages/coding-agent/src/tools/ast-grep.ts b/packages/coding-agent/src/tools/ast-grep.ts index 93381751f..799f9de23 100644 --- a/packages/coding-agent/src/tools/ast-grep.ts +++ b/packages/coding-agent/src/tools/ast-grep.ts @@ -1,4 +1,3 @@ -import * as path from "node:path"; import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import { type AstFindMatch, astGrep } from "@oh-my-pi/pi-natives"; import type { Component } from "@oh-my-pi/pi-tui"; @@ -16,6 +15,7 @@ import { formatGroupedFiles } from "./grouped-file-output"; import { formatMatchLine } from "./match-line-format"; import type { OutputMeta } from "./output-meta"; import { + formatPathRelativeToCwd, hasGlobPathChars, normalizePathLikeInput, parseSearchPath, @@ -87,10 +87,7 @@ export class AstGrepTool implements AgentTool { - const relative = path.relative(this.session.cwd, targetPath).replace(/\\/g, "/"); - return relative.length === 0 ? "." : relative; - }; + const formatScopePath = (targetPath: string): string => formatPathRelativeToCwd(targetPath, this.session.cwd); let searchPath: string | undefined; let scopePath: string | undefined; let globFilter: string | undefined; @@ -147,7 +144,8 @@ export class AstGrepTool implements AgentTool formatResultPath(filePath, isDirectory); + const formatPath = (filePath: string): string => + formatResultPath(filePath, isDirectory, resolvedSearchPath, this.session.cwd); const { record: recordFile, list: fileList } = createFileRecorder(); const fileMatchCounts = new Map(); diff --git a/packages/coding-agent/src/tools/file-recorder.ts b/packages/coding-agent/src/tools/file-recorder.ts index 526c54f16..e3160d153 100644 --- a/packages/coding-agent/src/tools/file-recorder.ts +++ b/packages/coding-agent/src/tools/file-recorder.ts @@ -1,4 +1,5 @@ import * as path from "node:path"; +import { formatPathRelativeToCwd } from "./path-utils"; /** * Creates a deduplicating recorder for relative file paths. @@ -22,14 +23,13 @@ export function createFileRecorder(): { } /** - * Strip a leading slash and, when the search scope is a directory, normalize - * Windows-style separators. For single-file scopes, fall back to the basename - * so tool output does not leak absolute paths. + * Strip native virtual-root prefixes and format file paths relative to cwd when + * they are inside cwd. Paths outside cwd remain absolute. */ -export function formatResultPath(filePath: string, isDirectory: boolean): string { +export function formatResultPath(filePath: string, isDirectory: boolean, basePath: string, cwd: string): string { const cleanPath = filePath.startsWith("/") ? filePath.slice(1) : filePath; if (isDirectory) { - return cleanPath.replace(/\\/g, "/"); + return formatPathRelativeToCwd(path.resolve(basePath, cleanPath), cwd); } - return path.basename(cleanPath); + return formatPathRelativeToCwd(basePath, cwd); } diff --git a/packages/coding-agent/src/tools/find.ts b/packages/coding-agent/src/tools/find.ts index b9717596e..fe02e788a 100644 --- a/packages/coding-agent/src/tools/find.ts +++ b/packages/coding-agent/src/tools/find.ts @@ -23,7 +23,13 @@ import { import type { ToolSession } from "."; import { applyListLimit } from "./list-limit"; import { formatFullOutputReference, type OutputMeta } from "./output-meta"; -import { normalizePathLikeInput, parseFindPattern, resolveMultiFindPattern, resolveToCwd } from "./path-utils"; +import { + formatPathRelativeToCwd, + normalizePathLikeInput, + parseFindPattern, + resolveMultiFindPattern, + resolveToCwd, +} from "./path-utils"; import { formatCount, formatEmptyMessage, formatErrorMessage, PREVIEW_LIMITS } from "./render-utils"; import { ToolAbortError, ToolError, throwIfAborted } from "./tool-errors"; import { toolResult } from "./tool-result"; @@ -101,10 +107,7 @@ export class FindTool implements AgentTool { const { pattern, limit, hidden } = params; return untilAborted(signal, async () => { - const formatScopePath = (targetPath: string): string => { - const relative = path.relative(this.session.cwd, targetPath).replace(/\\/g, "/"); - return relative.length === 0 ? "." : relative; - }; + const formatScopePath = (targetPath: string): string => formatPathRelativeToCwd(targetPath, this.session.cwd); const normalizedPattern = normalizePathLikeInput(pattern).replace(/\\/g, "/"); if (!normalizedPattern) { throw new ToolError("Pattern must not be empty"); @@ -132,14 +135,9 @@ export class FindTool implements AgentTool { const formatMatchPath = (matchPath: string, fileType?: natives.FileType): string => { const hadTrailingSlash = matchPath.endsWith("/") || matchPath.endsWith("\\"); const absolutePath = path.isAbsolute(matchPath) ? matchPath : path.resolve(searchPath, matchPath); - let relativePath = path.relative(this.session.cwd, absolutePath).replace(/\\/g, "/"); - if (relativePath.length === 0) { - relativePath = "."; - } - if ((fileType === natives.FileType.Dir || hadTrailingSlash) && !relativePath.endsWith("/")) { - relativePath += "/"; - } - return relativePath; + return formatPathRelativeToCwd(absolutePath, this.session.cwd, { + trailingSlash: fileType === natives.FileType.Dir || hadTrailingSlash, + }); }; const buildResult = (files: string[]): AgentToolResult => { diff --git a/packages/coding-agent/src/tools/path-utils.ts b/packages/coding-agent/src/tools/path-utils.ts index 191d07f81..e73fe5358 100644 --- a/packages/coding-agent/src/tools/path-utils.ts +++ b/packages/coding-agent/src/tools/path-utils.ts @@ -157,6 +157,27 @@ export function resolveToCwd(filePath: string, cwd: string): string { return path.resolve(cwd, expanded); } +export function formatPathRelativeToCwd( + filePath: string, + cwd: string, + options: { trailingSlash?: boolean } = {}, +): string { + const resolvedCwd = path.resolve(cwd); + const normalized = normalizeLocalScheme(filePath); + if (isInternalUrlPath(normalized)) { + return normalized; + } + const expanded = expandPath(normalized); + const resolvedPath = path.isAbsolute(expanded) ? path.resolve(expanded) : path.resolve(cwd, expanded); + const relative = path.relative(resolvedCwd, resolvedPath); + const isWithinCwd = relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); + let displayPath = normalizePosixPath(isWithinCwd ? relative || "." : resolvedPath); + if (options.trailingSlash && displayPath !== "." && !displayPath.endsWith("/")) { + displayPath += "/"; + } + return displayPath; +} + /** * Strip matching surrounding double quotes from a path string. * Common when users paste quoted paths from Windows Explorer or shell copy-paste. @@ -381,8 +402,14 @@ function findCommonBasePath(paths: string[]): string { return joined || path.parse(path.resolve(paths[0])).root; } -function toScopeDisplay(items: string[]): string { - return items.map(item => normalizePosixPath(item)).join(", "); +function toScopeDisplay(items: string[], cwd: string): string { + return items + .map(item => + formatPathRelativeToCwd(item, cwd, { + trailingSlash: item.endsWith("/") || item.endsWith("\\"), + }), + ) + .join(", "); } function looksLikeDelimitedPathToken(token: string): boolean { @@ -533,7 +560,7 @@ export async function resolveMultiSearchPath( return { basePath: commonBasePath, glob: buildBraceUnion(combinedPatterns), - scopePath: toScopeDisplay(pathItems), + scopePath: toScopeDisplay(pathItems, cwd), exactFilePaths: allExactFiles ? parsedItems.map(item => item.absoluteBasePath) : undefined, }; } @@ -571,7 +598,7 @@ export async function resolveMultiFindPattern( return { basePath: commonBasePath, globPattern: buildBraceUnion(combinedPatterns) ?? "**/*", - scopePath: toScopeDisplay(patternItems), + scopePath: toScopeDisplay(patternItems, cwd), }; } diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 528253cb7..4e49dc36d 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -40,7 +40,7 @@ import { } from "./fetch"; import { applyListLimit } from "./list-limit"; import { formatFullOutputReference, formatStyledTruncationWarning, type OutputMeta } from "./output-meta"; -import { expandPath, resolveReadPath } from "./path-utils"; +import { expandPath, formatPathRelativeToCwd, resolveReadPath } from "./path-utils"; import { formatAge, formatBytes, shortenPath, wrapBrackets } from "./render-utils"; import { executeReadQuery, @@ -1046,7 +1046,10 @@ export class ReadTool implements AgentTool { ? "- Alpha: no" : "- Alpha: unknown", "", - `If you want to analyze the image, call inspect_image with path="${readPath}" and a question describing what to inspect and the desired output format.`, + `If you want to analyze the image, call inspect_image with path="${formatPathRelativeToCwd( + absolutePath, + this.session.cwd, + )}" and a question describing what to inspect and the desired output format.`, ]; content = [{ type: "text", text: metadataLines.join("\n") }]; details = {}; diff --git a/packages/coding-agent/src/tools/search.ts b/packages/coding-agent/src/tools/search.ts index 97c8625a7..2175b2870 100644 --- a/packages/coding-agent/src/tools/search.ts +++ b/packages/coding-agent/src/tools/search.ts @@ -13,11 +13,12 @@ import { DEFAULT_MAX_COLUMN, type TruncationResult, truncateHead } from "../sess import { Ellipsis, Hasher, type RenderCache, renderStatusLine, renderTreeList, truncateToWidth } from "../tui"; import { resolveFileDisplayMode } from "../utils/file-display-mode"; import type { ToolSession } from "."; -import { createFileRecorder } from "./file-recorder"; +import { createFileRecorder, formatResultPath } from "./file-recorder"; import { formatGroupedFiles } from "./grouped-file-output"; import { formatMatchLine } from "./match-line-format"; import { formatFullOutputReference, type OutputMeta } from "./output-meta"; import { + formatPathRelativeToCwd, hasGlobPathChars, normalizePathLikeInput, parseSearchPath, @@ -112,10 +113,7 @@ export class SearchTool implements AgentTool { - const relative = path.relative(this.session.cwd, targetPath).replace(/\\/g, "/"); - return relative.length === 0 ? "." : relative; - }; + const formatScopePath = (targetPath: string): string => formatPathRelativeToCwd(targetPath, this.session.cwd); let searchPath: string; let scopePath: string; let exactFilePaths: string[] | undefined; @@ -225,14 +223,8 @@ export class SearchTool implements AgentTool { - // returns paths starting with / (the virtual root) - const cleanPath = filePath.startsWith("/") ? filePath.slice(1) : filePath; - if (isDirectory) { - return cleanPath.replace(/\\/g, "/"); - } - return path.basename(cleanPath); - }; + const formatPath = (filePath: string): string => + formatResultPath(filePath, isDirectory, searchPath, this.session.cwd); // Build output const roundRobinSelect = (matches: GrepMatch[], limit: number): GrepMatch[] => { diff --git a/packages/coding-agent/src/tools/write.ts b/packages/coding-agent/src/tools/write.ts index 13b7a4b3d..161c96483 100644 --- a/packages/coding-agent/src/tools/write.ts +++ b/packages/coding-agent/src/tools/write.ts @@ -19,6 +19,7 @@ import { parseArchivePathCandidates } from "./archive-reader"; import { assertEditableFile } from "./auto-generated-guard"; import { invalidateFsScanAfterWrite } from "./fs-cache-invalidation"; import { type OutputMeta, outputMeta } from "./output-meta"; +import { formatPathRelativeToCwd } from "./path-utils"; import { enforcePlanModeWrite, resolvePlanPath } from "./plan-mode-guard"; import { formatDiagnostics, @@ -212,7 +213,6 @@ export class WriteTool implements AgentTool> { @@ -278,8 +278,11 @@ export class WriteTool implements AgentTool @@ -468,7 +471,8 @@ export class WriteTool implements AgentTool { + _resetSettingsForTest(); + await Settings.init({ inMemory: true, cwd: process.cwd() }); +}); + +function tag(line: number, content: string): string { + return `${line}${computeLineHash(line, content)}`; } -describe("applyAtomEdits — splice", () => { - it("replaces a single line", () => { - const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [{ op: "splice", pos: tag(2, "bbb"), lines: ["BBB"] }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nBBB\nccc"); - expect(result.firstChangedLine).toBe(2); +// Convenience: parse a diff against a content snapshot and return the resulting text. +function applyDiff(content: string, diff: string): string { + const edits = parseAtom(diff); + return applyAtomEdits(content, edits).lines; +} + +async function withTempDir(fn: (tempDir: string) => Promise): Promise { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "atom-edit-")); + try { + await fn(tempDir); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +} + +function atomExecuteOptions(tempDir: string, input: string): ExecuteAtomSingleOptions { + return { + session: { cwd: tempDir } as ToolSession, + input, + writethrough: async () => { + throw new Error("unexpected write"); + }, + beginDeferredDiagnosticsForPath: () => { + throw new Error("unexpected diagnostics"); + }, + }; +} + +// ─────────────────────────────────────────────────────────────────────────── +// Form coverage +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom parser — basic forms", () => { + const content = "aaa\nbbb\nccc"; + + it("canonical set replaces a single line", () => { + const diff = `${tag(2, "bbb")}=BBB`; + expect(applyDiff(content, diff)).toBe("aaa\nBBB\nccc"); }); - it("expands one line into many", () => { - const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [{ op: "splice", pos: tag(2, "bbb"), lines: ["X", "Y", "Z"] }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nX\nY\nZ\nccc"); + it("prefix delete `-Lid` removes a single line", () => { + const diff = `-${tag(2, "bbb")}`; + expect(applyDiff(content, diff)).toBe("aaa\nccc"); }); - it("rejects on stale hash", () => { + it("`@Lid` moves the cursor after the anchored line", () => { + const diff = `@${tag(2, "bbb")}\n+INSERTED`; + expect(applyDiff(content, diff)).toBe("aaa\nbbb\nINSERTED\nccc"); + }); + + it("$ + + lines prepend to the file", () => { + const diff = `$\n+ZZZ\n+YYY`; + expect(applyDiff(content, diff)).toBe("ZZZ\nYYY\naaa\nbbb\nccc"); + }); + + it("^ + + lines append to the file", () => { + const diff = `^\n+DDD\n+EEE`; + expect(applyDiff(content, diff)).toBe("aaa\nbbb\nccc\nDDD\nEEE"); + }); + + it("+ lines with no cursor move append to the file", () => { + const diff = `+DDD\n+EEE`; + expect(applyDiff(content, diff)).toBe("aaa\nbbb\nccc\nDDD\nEEE"); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Cursor binding rule +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom parser — cursor binding", () => { + const content = "aaa\nbbb\nccc"; + + it("set moves the cursor after the set line", () => { + const diff = `${tag(1, "aaa")}=AAA\n+INSERTED\n@${tag(2, "bbb")}`; + expect(applyDiff(content, diff)).toBe("AAA\nINSERTED\nbbb\nccc"); + }); + + it("set before another set keeps inserts at the previous cursor", () => { + const diff = `${tag(1, "aaa")}=AAA\n+INSERTED\n${tag(2, "bbb")}=BBB`; + expect(applyDiff(content, diff)).toBe("AAA\nINSERTED\nBBB\nccc"); + }); + + it("bare Lid moves the cursor before a following set", () => { + const diff = `@${tag(1, "aaa")}\n+INSERTED\n${tag(2, "bbb")}=BBB`; + expect(applyDiff(content, diff)).toBe("aaa\nINSERTED\nBBB\nccc"); + }); + + it("preserves contiguous + lines at the same cursor", () => { + const diff = `${tag(1, "aaa")}=AAA\n+I1\n+I2\n+I3\n@${tag(2, "bbb")}`; + expect(applyDiff(content, diff)).toBe("AAA\nI1\nI2\nI3\nbbb\nccc"); + }); + + it("+ with only a previous anchor inserts after that anchor", () => { + const diff = `@${tag(2, "bbb")}\n+INSERTED`; + expect(applyDiff(content, diff)).toBe("aaa\nbbb\nINSERTED\nccc"); + }); + + it("+ before any cursor move uses the initial EOF cursor", () => { + const diff = `+INSERTED\n@${tag(2, "bbb")}`; + expect(applyDiff(content, diff)).toBe("aaa\nbbb\nccc\nINSERTED"); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Edge cases +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom parser — edge cases", () => { + const content = "aaa\nbbb\nccc"; + + it("empty set blanks the line", () => { + const diff = `${tag(2, "bbb")}=`; + expect(applyDiff(content, diff)).toBe("aaa\n\nccc"); + }); + + it("set value starting with `!` keeps the leading `!`", () => { + const diff = `${tag(2, "bbb")}=!hello`; + expect(applyDiff(content, diff)).toBe("aaa\n!hello\nccc"); + }); + + it("set value starting with `|` keeps the leading `|`", () => { + const diff = `${tag(2, "bbb")}=|hello`; + expect(applyDiff(content, diff)).toBe("aaa\n|hello\nccc"); + }); + + it("set preserves leading/trailing whitespace exactly", () => { + const diff = `${tag(2, "bbb")}= spaced `; + expect(applyDiff(content, diff)).toBe("aaa\n spaced \nccc"); + }); + + it("empty `+` line inserts a blank line", () => { + const diff = `${tag(1, "aaa")}=AAA\n+\n@${tag(2, "bbb")}`; + expect(applyDiff(content, diff)).toBe("AAA\n\nbbb\nccc"); + }); + + it("`+` block with no cursor emits EOF cursor inserts", () => { + expect(parseAtom(`+lonely`)).toMatchObject([{ kind: "insert", cursor: { kind: "eof" }, text: "lonely" }]); + }); + + it("out-of-order anchors are accepted (sorted internally)", () => { + const diff = `${tag(3, "ccc")}=CCC\n${tag(1, "aaa")}=AAA`; + expect(applyDiff(content, diff)).toBe("AAA\nbbb\nCCC"); + }); + + it("delete + post: insertions take the deleted line's slot", () => { + const diff = `-${tag(2, "bbb")}\n+INSERTED`; + expect(applyDiff(content, diff)).toBe("aaa\nINSERTED\nccc"); + }); + + it("CRLF input is normalized line-by-line", () => { + const diff = `${tag(2, "bbb")}=BBB\r\n${tag(1, "aaa")}=AAA\r`; + expect(applyDiff(content, diff)).toBe("AAA\nBBB\nccc"); + }); + + it("trailing newline at file end is preserved", () => { + const c = "aaa\nbbb\nccc\n"; + const diff = `${tag(2, "bbb")}=BBB`; + expect(applyDiff(c, diff)).toBe("aaa\nBBB\nccc\n"); + }); + + it("empty diff is a no-op", () => { + expect(parseAtom("")).toEqual([]); + expect(parseAtom("\n\n\n")).toEqual([]); + }); + + it("`Lid=TEXT` is the canonical set form", () => { const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [{ op: "splice", pos: { line: 2, hash: "ZZ" }, lines: ["BBB"] }]; + expect(applyDiff(content, `${tag(2, "bbb")}=BBB`)).toBe("aaa\nBBB\nccc"); + }); + + it("legacy set and locator forms remain accepted", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + expect(applyDiff(content, `${t}|BBB`)).toBe("aaa\nBBB\nccc"); + expect(applyDiff(content, `@@ ${t}\n+INSERTED`)).toBe("aaa\nbbb\nINSERTED\nccc"); + }); + + it("recovers common replacement slips with @ and @@ prefixes", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + expect(applyDiff(content, `@${t}=BBB`)).toBe("aaa\nBBB\nccc"); + expect(applyDiff(content, `@${t}|BBB`)).toBe("aaa\nBBB\nccc"); + expect(applyDiff(content, `@@ ${t}=BBB`)).toBe("aaa\nBBB\nccc"); + expect(applyDiff(content, `@@ ${t}|BBB`)).toBe("aaa\nBBB\nccc"); + }); + + it("recovers common delete slips with whitespace and @@ prefixes", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + expect(applyDiff(content, `- ${t}`)).toBe("aaa\nccc"); + expect(applyDiff(content, `@@ -${t}`)).toBe("aaa\nccc"); + expect(applyDiff(content, `@@ - ${t}`)).toBe("aaa\nccc"); + }); + + it("ignores whitespace before replacement separator and preserves whitespace after it", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + expect(applyDiff(content, `${t} = BBB `)).toBe("aaa\n BBB \nccc"); + expect(applyDiff(content, `@${t} | BBB `)).toBe("aaa\n BBB \nccc"); + expect(applyDiff(content, `@@ ${t} | BBB `)).toBe("aaa\n BBB \nccc"); + }); + + it("rejects partial and missing Lids with repairable diagnostics", () => { + expect(() => parseAtom("@@ 98")).toThrow(/`@@ 98` is missing the two-letter Lid suffix/); + expect(() => parseAtom("yh=TEXT")).toThrow(/`yh` is not a full Lid/); + expect(() => parseAtom("123=TEXT")).toThrow(/`123` is missing the two-letter Lid suffix/); + expect(() => parseAtom("123|TEXT")).toThrow(/`123` is missing the two-letter Lid suffix/); + }); + + it("canonical equals replacement does not apply legacy OLD|NEW repair", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + expect(applyDiff(content, `${t}=bbb|BBB`)).toBe("aaa\nbbb|BBB\nccc"); + }); + + it("silently ignores identical replacements when another operation changes the file", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + const result = applyAtomEdits(content, parseAtom(`${t}=bbb\n+DDD`)); + expect(result.lines).toBe("aaa\nbbb\nDDD\nccc"); + expect(result.noopEdits).toBeUndefined(); + expect(result.warnings).toBeUndefined(); + }); + + it("locator-only patches report the cursor diagnostic", async () => { + await expect( + executeAtomSingle({ + session: { cwd: process.cwd() } as ToolSession, + input: "---a.ts\n@123ab", + writethrough: async () => { + throw new Error("unexpected write"); + }, + beginDeferredDiagnosticsForPath: () => { + throw new Error("unexpected diagnostics"); + }, + } as ExecuteAtomSingleOptions), + ).rejects.toThrow( + "Cursor moved but no mutation found. Add +TEXT to insert, -Lid to delete, or Lid=TEXT to replace.", + ); + }); + + it("@Lid move syntax is accepted as canonical", () => { + expect(parseAtom(`@${tag(2, "bbb")}`)).toEqual([]); + }); + + it("anchor with non-pipe trailing characters is leniently treated as an insert", () => { + const content = "aaa\nbbb\nccc"; + // `1aa/foo/bar/` doesn't match `Lid=...`, so it falls through to a + // best-effort insert at EOF rather than throwing. + expect(applyDiff(content, `${tag(1, "aaa")}/foo/bar/`)).toBe(`aaa\nbbb\nccc\n${tag(1, "aaa")}/foo/bar/`); + }); + + it("duplicate sets on the same anchor: last set wins", () => { + const t = tag(2, "bbb"); + const diff = `${t}=OLD\n${t}=NEW`; + expect(applyDiff(content, diff)).toBe("aaa\nNEW\nccc"); + }); + + it("same-line OLD|NEW repairs to the new line when OLD is current content", () => { + const t = tag(2, "bbb"); + const diff = `${t}|bbb|BBB`; + expect(applyDiff(content, diff)).toBe("aaa\nBBB\nccc"); + }); + + it("same-line OLD|NEW repair works through `@` prefix slip", () => { + const t = tag(2, "bbb"); + const diff = `@${t}|bbb|BBB`; + expect(applyDiff(content, diff)).toBe("aaa\nBBB\nccc"); + }); + + it("same-line OLD|NEW repair works through `@@ ` prefix slip", () => { + const t = tag(2, "bbb"); + const diff = `@@ ${t}|bbb|BBB`; + expect(applyDiff(content, diff)).toBe("aaa\nBBB\nccc"); + }); + + it("`-Lid` followed by `+Lid|TEXT` fuses into a single replacement", () => { + const t = tag(2, "bbb"); + const diff = `-${t}\n+${t}|REPLACED`; + expect(applyDiff(content, diff)).toBe("aaa\nREPLACED\nccc"); + }); + + it("`-Lid` followed by `+Lid=TEXT` fuses into a single replacement", () => { + const t = tag(2, "bbb"); + const diff = `-${t}\n+${t}=REPLACED`; + expect(applyDiff(content, diff)).toBe("aaa\nREPLACED\nccc"); + }); + + it("standalone `+Lid|TEXT` is rejected with a diff-ish replacement diagnostic", () => { + const t = tag(2, "bbb"); + expect(() => parseAtom(`+${t}|REPLACED`)).toThrow( + new RegExp(`\`\\+${t}\\|\\.\\.\\.\` looks like a unified-diff replacement marker. Use \`${t}=TEXT\``), + ); + }); + + it("standalone `+Lid=TEXT` is rejected with a diff-ish replacement diagnostic", () => { + const t = tag(2, "bbb"); + expect(() => parseAtom(`+${t}=REPLACED`)).toThrow(/looks like a unified-diff replacement marker/); + }); + + it("`-Lid` followed by `+OtherLid|TEXT` (mismatched) is rejected", () => { + const t1 = tag(1, "aaa"); + const t2 = tag(2, "bbb"); + expect(() => parseAtom(`-${t1}\n+${t2}|REPLACED`)).toThrow( + /references a Lid that was not deleted in the preceding run/, + ); + }); + + it("plain `+TEXT` insertion is unaffected by diff-ish detection", () => { + // `+5xa hello` — no `=` or `|` separator after the Lid-shaped prefix — + // remains a literal insert, including the `5xa hello` text. + const diff = `+5xa hello`; + expect(applyDiff(content, diff)).toBe("aaa\nbbb\nccc\n5xa hello"); + }); + + it("multi-line hunk: deletes followed by inserts becomes a block replacement at the FIRST deleted slot", () => { + const c = "aaa\nbbb\nccc\nddd\neee"; + const t2 = tag(2, "bbb"); + const t3 = tag(3, "ccc"); + const t4 = tag(4, "ddd"); + const diff = `-${t2}\n-${t3}\n-${t4}\n+X1\n+X2\n+X3`; + expect(applyDiff(c, diff)).toBe("aaa\nX1\nX2\nX3\neee"); + }); + + it("multi-line hunk: more inserts than deletes is allowed", () => { + const c = "aaa\nbbb\nccc\nddd"; + const t2 = tag(2, "bbb"); + const diff = `-${t2}\n+X1\n+X2\n+X3`; + expect(applyDiff(c, diff)).toBe("aaa\nX1\nX2\nX3\nccc\nddd"); + }); + + it("multi-line hunk: fewer inserts than deletes is allowed", () => { + const c = "aaa\nbbb\nccc\nddd\neee"; + const t2 = tag(2, "bbb"); + const t3 = tag(3, "ccc"); + const t4 = tag(4, "ddd"); + const diff = `-${t2}\n-${t3}\n-${t4}\n+X`; + expect(applyDiff(c, diff)).toBe("aaa\nX\neee"); + }); + + it("multi-line hunk: `+Lid|TEXT` add lines must reference a deleted Lid", () => { + const c = "aaa\nbbb\nccc\nddd"; + const t2 = tag(2, "bbb"); + const t3 = tag(3, "ccc"); + const t4 = tag(4, "ddd"); + // `+Lid|TEXT` for t2 and t3 (in delete run) is OK; t4 (also deleted) is OK. + const diff = `-${t2}\n-${t3}\n-${t4}\n+${t2}|X1\n+${t3}|X2\n+${t4}|X3`; + expect(applyDiff(c, diff)).toBe("aaa\nX1\nX2\nX3"); + }); + + it("multi-line hunk: `+Lid|TEXT` referencing a Lid not in the delete run is rejected", () => { + const t1 = tag(1, "aaa"); + const t2 = tag(2, "bbb"); + const t3 = tag(3, "ccc"); + expect(() => parseAtom(`-${t1}\n-${t2}\n+${t3}|X`)).toThrow( + /references a Lid that was not deleted in the preceding run/, + ); + }); + + it("`-Lid|OLD` deletes when OLD matches the current line", () => { + const t = tag(2, "bbb"); + const diff = `-${t}|bbb`; + expect(applyDiff(content, diff)).toBe("aaa\nccc"); + }); + + it("`-Lid=OLD` deletes when OLD matches the current line", () => { + const t = tag(2, "bbb"); + const diff = `-${t}=bbb`; + expect(applyDiff(content, diff)).toBe("aaa\nccc"); + }); + + it("`-Lid|OLD` is rejected when OLD does not match the current line", () => { + const t = tag(2, "bbb"); + const diff = `-${t}|XXX`; + expect(() => applyAtomEdits(content, parseAtom(diff))).toThrow( + /asserts the deleted line is "XXX", but the file has "bbb"/, + ); + }); + + it("hunk with `-Lid|OLD` validates each OLD against current line", () => { + const c = "aaa\nbbb\nccc\nddd"; + const t2 = tag(2, "bbb"); + const t3 = tag(3, "ccc"); + // Both OLDs match → accepted. + const ok = `-${t2}|bbb\n-${t3}|ccc\n+X1\n+X2`; + expect(applyDiff(c, ok)).toBe("aaa\nX1\nX2\nddd"); + // Second OLD wrong → rejected. + const bad = `-${t2}|bbb\n-${t3}|wrong\n+X1\n+X2`; + expect(() => applyAtomEdits(c, parseAtom(bad))).toThrow(/asserts the deleted line is "wrong"/); + }); + + it("set with current content reports identical replacement as a no-op", () => { + const t = tag(2, "bbb"); + const result = applyAtomEdits(content, parseAtom(`${t}=bbb`)); + expect(result.lines).toBe(content); + expect(result.noopEdits).toEqual([ + { + editIndex: 0, + loc: t, + reason: + "replacement is identical to the current line content; use `Lid=NEW_TEXT` and do not copy an unchanged read line", + current: "bbb", + }, + ]); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Combined ops on a single anchor +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom — combining set + post on one anchor", () => { + it("set then `+` lines insert after the set line", () => { + const content = "aaa\nbbb\nccc"; + const t = tag(2, "bbb"); + const diff = `${t}=NEW\n+POST`; + expect(applyDiff(content, diff)).toBe("aaa\nNEW\nPOST\nccc"); + }); + + it("a leading `+` starts at EOF, and a trailing `+` uses the current anchor cursor", () => { + const content = "aaa\nbbb\nccc\nddd"; + const t2 = tag(2, "bbb"); + const t3 = tag(3, "ccc"); + const diff = `+BEFORE\n${t2}=BBB\n+AFTER\n@${t3}`; + expect(applyDiff(content, diff)).toBe("aaa\nBBB\nAFTER\nccc\nddd\nBEFORE"); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Hash mismatch flow +// ─────────────────────────────────────────────────────────────────────────── + +describe("atom — hash mismatch", () => { + it("propagates HashlineMismatchError on stale hash", () => { + const content = "aaa\nbbb\nccc"; + const diff = `2zz=BBB`; + const edits = parseAtom(diff); expect(() => applyAtomEdits(content, edits)).toThrow(HashlineMismatchError); }); -}); -describe("applyAtomEdits — del", () => { - it("removes a line", () => { - const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [{ op: "del", pos: tag(2, "bbb") }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nccc"); - }); - - it("multiple deletes apply bottom-up so anchors stay valid", () => { - const content = "aaa\nbbb\nccc\nddd"; - const edits: AtomEdit[] = [ - { op: "del", pos: tag(2, "bbb") }, - { op: "del", pos: tag(3, "ccc") }, - ]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nddd"); + it("does not use replacement text as a rebase content hint", () => { + const content = "aaa\nchanged\nNEW"; + const stale = tag(2, "bbb"); + expect(() => applyAtomEdits(content, parseAtom(`${stale}=NEW`))).toThrow(HashlineMismatchError); }); }); -describe("applyAtomEdits — pre/post", () => { - it("pre inserts above the anchor", () => { - const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [{ op: "pre", pos: tag(2, "bbb"), lines: ["NEW"] }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nNEW\nbbb\nccc"); +// ─────────────────────────────────────────────────────────────────────────── +// Internal AtomEdit shapes +// ─────────────────────────────────────────────────────────────────────────── + +describe("parseAtom — emits internal AtomEdit shapes", () => { + it("emits delete op", () => { + const t = tag(2, "bbb"); + const edits = parseAtom(`-${t}`); + expect(edits).toHaveLength(1); + expect(edits[0]).toMatchObject({ kind: "delete", anchor: { line: 2 } }); }); - it("post inserts below the anchor", () => { - const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [{ op: "post", pos: tag(2, "bbb"), lines: ["NEW"] }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nbbb\nNEW\nccc"); + it("emits EOF cursor insert for ^ + +", () => { + const edits = parseAtom(`^\n+x`); + expect(edits).toMatchObject([{ kind: "insert", cursor: { kind: "eof" }, text: "x" }]); }); - it("pre + post on same anchor coexist with splice", () => { + it("emits EOF cursor insert for bare + +", () => { + const edits = parseAtom(`+x`); + expect(edits).toMatchObject([{ kind: "insert", cursor: { kind: "eof" }, text: "x" }]); + }); + + it("emits BOF cursor insert for $ + +", () => { + const edits = parseAtom(`$\n+x`); + expect(edits).toMatchObject([{ kind: "insert", cursor: { kind: "bof" }, text: "x" }]); + }); + + it("delete + set on same anchor is rejected by validateNoConflictingAnchorOps", () => { const content = "aaa\nbbb\nccc"; - const edits: AtomEdit[] = [ - { op: "pre", pos: tag(2, "bbb"), lines: ["B"] }, - { op: "splice", pos: tag(2, "bbb"), lines: ["BBB"] }, - { op: "post", pos: tag(2, "bbb"), lines: ["A"] }, - ]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nB\nBBB\nA\nccc"); + const t = tag(2, "bbb"); + const diff = `${t}=NEW\n-${t}`; + const edits = parseAtom(diff); + expect(() => applyAtomEdits(content, edits)).toThrow(/Conflicting ops/); }); }); -describe("atom edit schema", () => { - it("rejects sub edits", () => { - expect(Value.Check(atomEditSchema, { loc: "1ab", sub: ["5000", "30_000"] })).toBe(false); +// ─────────────────────────────────────────────────────────────────────────── +// Wire format header +// ─────────────────────────────────────────────────────────────────────────── + +describe("splitAtomInput — wire-format header", () => { + it("extracts path and diff body from `--- path` header", () => { + const input = `---src/foo.ts\n${tag(2, "bbb")}=BBB`; + const { path, diff } = splitAtomInput(input); + expect(path).toBe("src/foo.ts"); + expect(diff).toBe(`${tag(2, "bbb")}=BBB`); }); - it("rejects bracketed loc forms (no longer supported)", () => { - // `(A)` and `[A]` were dropped — they are valid at the schema level - // (loc is a string) but the runtime parser rejects anything that isn't - // a bare anchor or `$`. - expect(() => resolveAtomToolEdit({ loc: "(2ab)", splice: ["X"] })).toThrow(); - expect(() => resolveAtomToolEdit({ loc: "[2ab]", splice: ["X"] })).toThrow(); + it("extracts path and diff body from legacy `--- path` header", () => { + const input = `--- src/foo.ts\n${tag(2, "bbb")}=BBB`; + const { path, diff } = splitAtomInput(input); + expect(path).toBe("src/foo.ts"); + expect(diff).toBe(`${tag(2, "bbb")}=BBB`); }); - it("rejects sed-shaped replace specs", () => { - expect(Value.Check(atomEditSchema, { loc: "1ab", sed: { pat: "x", rep: "y" } })).toBe(false); - }); -}); - -describe("resolveAtomToolEdit — loc syntax", () => { - it('loc:"$" appends at EOF', () => { - const content = "aaa\nbbb"; - const resolved = resolveAtomToolEdit({ loc: "$", post: ["ccc"] }); - expect(resolved).toHaveLength(1); - expect(resolved[0]?.op).toBe("append_file"); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nbbb\nccc"); + it("strips leading blank lines before the first header", () => { + const input = `\n\n---a.ts\n+export const A = 1;`; + expect(splitAtomInput(input)).toEqual({ path: "a.ts", diff: "+export const A = 1;" }); }); - it('loc:"$" + pre prepends to the file', () => { - const content = "aaa\nbbb"; - const resolved = resolveAtomToolEdit({ loc: "$", pre: ["ZZZ"] }); - expect(resolved).toHaveLength(1); - expect(resolved[0]?.op).toBe("prepend_file"); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("ZZZ\naaa\nbbb"); + it("unquotes matching path quotes", () => { + expect(splitAtomInput(`---"foo bar.ts"\n+x`).path).toBe("foo bar.ts"); + expect(splitAtomInput(`---'foo bar.ts'\n+x`).path).toBe("foo bar.ts"); }); - it('loc:"$" + replace substitutes across all lines', () => { - const content = "aaa\nfoo\nbar foo"; - const resolved = resolveAtomToolEdit({ loc: "$", replace: { find: "foo", with: "FOO" } }); - expect(resolved).toHaveLength(1); - expect(resolved[0]?.op).toBe("replace_file"); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nFOO\nbar FOO"); + it("normalizes cwd-prefixed absolute paths to cwd-relative paths", () => { + const cwd = path.join(process.cwd(), "packages", "coding-agent"); + const absolute = path.join(cwd, "src", "foo.ts"); + expect(splitAtomInput(`---${absolute}\n+x`, { cwd }).path).toBe("src/foo.ts"); }); - it('loc:"$" + replace with all:true substitutes every occurrence', () => { - const content = "aaa\nfoo foo\nbar foo"; - const resolved = resolveAtomToolEdit({ loc: "$", replace: { find: "foo", with: "FOO", all: true } }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nFOO FOO\nbar FOO"); + it("preserves absolute paths outside cwd", () => { + const cwd = path.join(process.cwd(), "packages", "coding-agent"); + const outside = path.resolve(process.cwd(), "..", "outside.ts"); + expect(splitAtomInput(`---${outside}\n+x`, { cwd }).path).toBe(outside); }); - it('loc:"$" + replace preserves trailing newline', () => { - const content = "aaa\nbbb\n"; - const resolved = resolveAtomToolEdit({ loc: "$", replace: { find: "bbb", with: "BBB" } }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nBBB\n"); - }); - - it('loc:"$" + replace throws when no line matches', () => { - const content = "aaa\nbbb"; - const resolved = resolveAtomToolEdit({ loc: "$", replace: { find: "zzz", with: "yyy" } }); - expect(() => applyAtomEdits(content, resolved)).toThrow(/did not match any line/); - }); - - it('loc:"$" + pre + post + replace combined', () => { - const content = "aaa\nbbb"; - const resolved = resolveAtomToolEdit({ - loc: "$", - pre: ["PRE"], - replace: { find: "bbb", with: "BBB" }, - post: ["POST"], + it("uses explicit fallback path only when input has operations and no header", () => { + expect(splitAtomInput(`${tag(1, "aaa")}=AAA`, { path: "a.ts" })).toEqual({ + path: "a.ts", + diff: `${tag(1, "aaa")}=AAA`, }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("PRE\naaa\nBBB\nPOST"); + expect(() => splitAtomInput("plain text", { path: "a.ts" })).toThrow(/must begin with/); + expect(() => splitAtomInput("---\n+x", { path: "a.ts" })).toThrow(/empty/); }); - it('loc:"$" rejects splice', () => { - expect(() => resolveAtomToolEdit({ loc: "$", splice: ["X"] })).toThrow(/supports pre, post, and replace/); + it("throws if header is missing", () => { + expect(() => splitAtomInput("124aa=NEW")).toThrow(/must begin with/); }); - it('loc:"^" is no longer supported', () => { - expect(() => resolveAtomToolEdit({ loc: "^", pre: ["ZZZ"] })).toThrow(); + it("throws if header path is empty", () => { + expect(() => splitAtomInput("--- \n124aa=NEW")).toThrow(/empty/); }); - it("expands pre + splice + post from one entry", () => { - const content = "aaa\nbbb\nccc"; - const loc = `2${computeLineHash(2, "bbb")}`; - const resolved = resolveAtomToolEdit({ loc, pre: ["B"], splice: ["BBB"], post: ["A"] }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nB\nBBB\nA\nccc"); + it("tolerates CRLF after header", () => { + const input = `--- a.ts\r\n${tag(1, "alpha")}=ALPHA`; + const { path, diff } = splitAtomInput(input); + expect(path).toBe("a.ts"); + expect(diff).toBe(`${tag(1, "alpha")}=ALPHA`); }); - it("splice: [] deletes the anchor line", () => { - const content = "aaa\nbbb\nccc"; - const loc = `2${computeLineHash(2, "bbb")}`; - const resolved = resolveAtomToolEdit({ loc, splice: [] }); - expect(resolved[0]?.op).toBe("del"); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nccc"); + it("strips BOM from input", () => { + const input = `\uFEFF---a.ts\n${tag(1, "alpha")}=ALPHA`; + const { path } = splitAtomInput(input); + expect(path).toBe("a.ts"); }); - it('splice: [""] preserves a blank line', () => { - const content = "aaa\nbbb\nccc"; - const loc = `2${computeLineHash(2, "bbb")}`; - const resolved = resolveAtomToolEdit({ loc, splice: [""] }); - expect(resolved[0]?.op).toBe("splice"); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\n\nccc"); + it("splits multiple --- path sections", () => { + const input = `---a.ts\n+export const A = 1;\n--- b.ts\n+export const B = 2;`; + expect(splitAtomInputs(input)).toEqual([ + { path: "a.ts", diff: "+export const A = 1;" }, + { path: "b.ts", diff: "+export const B = 2;" }, + ]); }); - it("ignores null optional verb fields", () => { - const content = "aaa\nbbb\nccc"; - const loc = `2${computeLineHash(2, "bbb")}`; - const toolEdit = { loc, pre: null, splice: "BBB", post: null } as unknown as AtomToolEdit; - const resolved = resolveAtomToolEdit(toolEdit); - expect(resolved).toEqual([{ op: "splice", pos: tag(2, "bbb"), lines: ["BBB"] }]); - - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nBBB\nccc"); - }); - - it("supports path override inside loc", () => { - const resolved = resolveAtomEntryPaths([{ loc: "a.ts:1ab", splice: ["X"] }], undefined); - expect(resolved[0]?.path).toBe("a.ts"); - expect(resolved[0]?.loc).toBe("1ab"); - }); - - it("accepts a content-suffix anchor and uses it for hint-based rebase", () => { - // Models sometimes paste line content after the anchor, e.g. - // `loc: "82zu| for (let i = 0; i--; ...) {"`. The bare `--` in the content - // must not break parsing. - const content = "alpha\nbravo\ncharlie"; - const loc = `2${computeLineHash(2, "bravo")}| for (let i = 0; i--; ...) {`; - const resolved = resolveAtomToolEdit({ loc, splice: ["BRAVO"] }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("alpha\nBRAVO\ncharlie"); - }); - - it("resolveAtomEntryPaths peels off path even when the loc content suffix contains colons", () => { - // Mimics a real failure: model wrote `image-input.ts:263ti| " const data: x"`. - // `lastIndexOf(":")` would have picked the colon inside `data:` and broken the split. - const [resolved] = resolveAtomEntryPaths( - [{ loc: 'image-input.ts:263ti| " const data: x"', replace: { find: "x", with: "y" } }], - undefined, - ); - expect(resolved?.path).toBe("image-input.ts"); - expect(resolved?.loc).toBe('263ti| " const data: x"'); + it("rejects the old colon header after the syntax cutover", () => { + expect(() => splitAtomInput(":a.ts\n+export const A = 1;")).toThrow(/must begin with/); }); }); -describe("applyAtomEdits — out of range", () => { - it("rejects line beyond file length", () => { - const content = "aaa\nbbb"; - const edits: AtomEdit[] = [{ op: "splice", pos: { line: 99, hash: "ZZ" }, lines: ["x"] }]; - expect(() => applyAtomEdits(content, edits)).toThrow(/does not exist/); - }); -}); +// ─────────────────────────────────────────────────────────────────────────── +// Whole-file operations +// ─────────────────────────────────────────────────────────────────────────── -describe("parseAnchor (atom tolerant) + applyAtomEdits", () => { - it("surfaces correct anchor + content when the model invents an out-of-alphabet hash", () => { - const content = "alpha\nbravo\ncharlie"; - // `XG` is not in the alphabet; should be rejected with the actual anchor exposed. - const toolEdit = { path: "a.ts", loc: "2XG", splice: ["BRAVO"] }; - const resolved = resolveAtomToolEdit(toolEdit); - expect(() => applyAtomEdits(content, resolved)).toThrow(HashlineMismatchError); - try { - applyAtomEdits(content, resolved); - } catch (err) { - const msg = (err as Error).message; - expect(msg).toMatch(/^\*\d+[a-z]{2}\|/m); - expect(msg).toContain("bravo"); - expect(msg).toContain(`2${computeLineHash(2, "bravo")}`); - } - }); +describe("atom executor — whole-file operations", () => { + it("deletes the section file with !rm", async () => { + await withTempDir(async tempDir => { + const filePath = path.join(tempDir, "file.ts"); + await Bun.write(filePath, "export const x = 1;\n"); - it("surfaces correct anchor + content when the model omits the hash entirely", () => { - const content = "alpha\nbravo\ncharlie"; - const toolEdit = { path: "a.ts", loc: "2", splice: ["BRAVO"] }; - const resolved = resolveAtomToolEdit(toolEdit); - expect(() => applyAtomEdits(content, resolved)).toThrow(HashlineMismatchError); - }); + const result = await executeAtomSingle(atomExecuteOptions(tempDir, "---file.ts\n!rm\n")); - it("surfaces correct anchor when the model uses pipe-separator (LINE|content) form", () => { - const content = "alpha\nbravo\ncharlie"; - const toolEdit = { path: "a.ts", loc: "2|bravo", splice: ["BRAVO"] }; - const resolved = resolveAtomToolEdit(toolEdit); - expect(() => applyAtomEdits(content, resolved)).toThrow(HashlineMismatchError); - }); - - it("throws a usage-style error when no line number can be extracted", () => { - const toolEdit = { path: "a.ts", loc: " if (!x) return;", splice: ["x"] }; - expect(() => resolveAtomToolEdit(toolEdit)).toThrow(/Could not find a line number/); - }); -}); - -describe("applyAtomEdits — replace", () => { - it("applies a literal substring substitution to the anchored line (first occurrence by default)", () => { - const content = "aaa\nfoo bar foo\nccc"; - const loc = `2${computeLineHash(2, "foo bar foo")}`; - const resolved = resolveAtomToolEdit({ loc, replace: { find: "foo", with: "baz" } }); - expect(resolved[0]?.op).toBe("replace"); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nbaz bar foo\nccc"); - }); - - it("`all: true` replaces every occurrence on the line", () => { - const content = "foo foo foo"; - const loc = `1${computeLineHash(1, "foo foo foo")}`; - const first = resolveAtomToolEdit({ loc, replace: { find: "foo", with: "bar" } }); - expect(applyAtomEdits(content, first).lines).toBe("bar foo foo"); - const all = resolveAtomToolEdit({ loc, replace: { find: "foo", with: "bar", all: true } }); - expect(applyAtomEdits(content, all).lines).toBe("bar bar bar"); - }); - - it("treats regex metacharacters as literal", () => { - // `(a, b)` would be a capture group in regex; here it must match the - // literal parens in the source line. - const content = "return wrap(foo(a, b));"; - const loc = `1${computeLineHash(1, content)}`; - const resolved = resolveAtomToolEdit({ loc, replace: { find: "foo(a, b)", with: "foo(b, a)" } }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("return wrap(foo(b, a));"); - }); - - it("treats unbalanced parens as literal characters, not as a regex error", () => { - const content = "x = bar());"; - const loc = `1${computeLineHash(1, content)}`; - const resolved = resolveAtomToolEdit({ loc, replace: { find: "bar())", with: "baz()" } }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("x = baz();"); - }); - - it("throws when the literal substring is not present on the anchor line", () => { - const content = "aaa\nbbb"; - const loc = `2${computeLineHash(2, "bbb")}`; - const resolved = resolveAtomToolEdit({ loc, replace: { find: "zzz", with: "yyy" } }); - expect(() => applyAtomEdits(content, resolved)).toThrow(/did not match line 2/); - }); - - it("combines with pre and post on the same anchor", () => { - const content = "aaa\nfoo\nccc"; - const loc = `2${computeLineHash(2, "foo")}`; - const resolved = resolveAtomToolEdit({ - loc, - pre: ["BEFORE"], - replace: { find: "foo", with: "FOO" }, - post: ["AFTER"], + expect(result.content[0]?.type === "text" ? result.content[0].text : "").toContain("Deleted file.ts"); + expect(result.details?.op).toBe("delete"); + expect(await Bun.file(filePath).exists()).toBe(false); }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nBEFORE\nFOO\nAFTER\nccc"); }); - it("prefers splice when replace is also present on the same anchor", () => { - const content = "aaa\nfoo\nccc"; - const loc = `2${computeLineHash(2, "foo")}`; - const resolved = resolveAtomToolEdit({ loc, splice: ["X"], replace: { find: "foo", with: "Y" } }); - // Models sometimes duplicate intent on the same line; the explicit `splice` - // wins and the redundant `replace` is dropped silently. - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nX\nccc"); + it("renames the section file with !mv", async () => { + await withTempDir(async tempDir => { + const sourcePath = path.join(tempDir, "file.ts"); + const destinationPath = path.join(tempDir, "file2.ts"); + await Bun.write(sourcePath, "export const x = 1;\n"); + + const result = await executeAtomSingle(atomExecuteOptions(tempDir, "---file.ts\n!mv file2.ts\n")); + + expect(result.content[0]?.type === "text" ? result.content[0].text : "").toContain( + "Moved file.ts to file2.ts", + ); + expect(result.details?.op).toBe("update"); + expect(result.details?.move).toBe("file2.ts"); + expect(await Bun.file(sourcePath).exists()).toBe(false); + expect(await Bun.file(destinationPath).text()).toBe("export const x = 1;\n"); + }); }); - it("treats empty `splice: []` as no-op when paired with replace", () => { - const content = "aaa\nfoo\nccc"; - const loc = `2${computeLineHash(2, "foo")}`; - const resolved = resolveAtomToolEdit({ loc, splice: [], replace: { find: "foo", with: "FOO" } }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nFOO\nccc"); + it("rejects sections that mix whole-file operations with line edits", async () => { + await withTempDir(async tempDir => { + await Bun.write(path.join(tempDir, "file.ts"), "export const x = 1;\n"); + + await expect( + executeAtomSingle(atomExecuteOptions(tempDir, "---file.ts\n!rm\n+export const y = 2;\n")), + ).rejects.toThrow(/mixes !rm with line edits/); + await expect( + executeAtomSingle(atomExecuteOptions(tempDir, "---file.ts\n!mv file2.ts\n-1ab\n")), + ).rejects.toThrow(/mixes !mv with line edits/); + }); }); - it("rejects replace.find containing a newline with a splice hint", () => { - const loc = "1ab"; - expect(() => resolveAtomToolEdit({ loc, replace: { find: "a\nb", with: "x" } })).toThrow( - /must be a single line.*splice/, - ); - }); + it("rejects !mv without a destination", async () => { + await withTempDir(async tempDir => { + await Bun.write(path.join(tempDir, "file.ts"), "export const x = 1;\n"); - it("supports replace.with containing a newline", () => { - const content = "aaa\nfoo\nccc"; - const loc = `2${computeLineHash(2, "foo")}`; - const resolved = resolveAtomToolEdit({ loc, replace: { find: "foo", with: "x\ny" } }); - const result = applyAtomEdits(content, resolved); - expect(result.lines).toBe("aaa\nx\ny\nccc"); - }); - - it("drops cross-entry `del` when another edit replaces the same anchor", () => { - // Models sometimes emit a `splice: []` cleanup alongside a `replace`/`splice` that - // already replaces the line. Prefer the replacement and silently drop the del. - const content = "aaa\nfoo\nccc"; - const loc = `2${computeLineHash(2, "foo")}`; - const edits = [ - ...resolveAtomToolEdit({ loc, replace: { find: "foo", with: "FOO" } }), - ...resolveAtomToolEdit({ loc, splice: [] }), - ]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("aaa\nFOO\nccc"); + await expect(executeAtomSingle(atomExecuteOptions(tempDir, "---file.ts\n!mv\n"))).rejects.toThrow( + /!mv requires exactly one non-empty destination path/, + ); + }); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// Schema is permissive for small models that pass extra fields +// ─────────────────────────────────────────────────────────────────────────── + +describe("atomEditParamsSchema — extra-field tolerance", () => { + it("accepts extra `path` field alongside `input`", () => { + const args = { path: "x.ts", input: "---x.ts\n1aa=NEW" }; + expect(Value.Check(atomEditParamsSchema, args)).toBe(true); + }); + + it("accepts extra free-form fields like `_`", () => { + const args = { _: "fixing inverted boolean", input: "---x.ts\n1aa=NEW" }; + expect(Value.Check(atomEditParamsSchema, args)).toBe(true); + }); + + it("still requires `input`", () => { + const args = { path: "x.ts" }; + expect(Value.Check(atomEditParamsSchema, args)).toBe(false); }); }); diff --git a/packages/coding-agent/test/prompt-templates.test.ts b/packages/coding-agent/test/prompt-templates.test.ts index a9d204141..099ba190b 100644 --- a/packages/coding-agent/test/prompt-templates.test.ts +++ b/packages/coding-agent/test/prompt-templates.test.ts @@ -298,6 +298,52 @@ describe("parseCommandArgs + substituteArgs integration", () => { }); }); +// ============================================================================ +// Hashline prompt helpers +// ============================================================================ + +describe("hashline prompt helpers", () => { + function createPromptTemplate(content: string): PromptTemplate { + return { + name: "test-template", + description: "Test template", + content, + source: "test", + }; + } + + function expandPrompt(content: string): string { + return expandPromptTemplate("/test-template", [createPromptTemplate(content)]); + } + + test("href and hrefr should reuse anchors remembered from hline", () => { + const result = expandPrompt( + '{{hline 2 "const timeout = 5000;"}}\nquoted={{href 2}}\nraw={{hrefr 2}}\nlast={{hrefr}}', + ); + const [line, quoted, raw, last] = result.split("\n"); + const ref = line.split("|", 1)[0]; + + expect(line).toBe(`${ref}|const timeout = 5000;`); + expect(quoted).toBe(`quoted="${ref}"`); + expect(raw).toBe(`raw=${ref}`); + expect(last).toBe(`last=${ref}`); + }); + + test("href and hrefr should still support explicit content without hline state", () => { + const result = expandPrompt('quoted={{href 5 "\treturn clean;"}}\nraw={{hrefr 5 "\treturn clean;"}}'); + const [quoted, raw] = result.split("\n"); + const ref = raw.slice("raw=".length); + + expect(quoted).toBe(`quoted="${ref}"`); + expect(ref).toMatch(/^5[a-z]{2}$/); + }); + + test("href should not reuse hline state across prompt renders", () => { + expect(expandPrompt('{{hline 1 "const x = 1;"}}\n{{hrefr}}')).toMatch(/^1[a-z]{2}\|const x = 1;\n1[a-z]{2}$/); + expect(() => expandPrompt("{{hrefr}}")).toThrow("previous {{hline}}"); + }); +}); + // ============================================================================ // expandSlashCommand + expandPromptTemplate fallback behavior // ============================================================================ diff --git a/packages/coding-agent/test/tools/edit-renderer.test.ts b/packages/coding-agent/test/tools/edit-renderer.test.ts index a9fc9d996..f8a3e1d0c 100644 --- a/packages/coding-agent/test/tools/edit-renderer.test.ts +++ b/packages/coding-agent/test/tools/edit-renderer.test.ts @@ -24,4 +24,65 @@ describe("editToolRenderer", () => { const rendered = Bun.stripANSI(component.render(160).join("\n")); expect(rendered).toContain("packages/coding-agent/src/edit/renderer.ts"); }); + + it("uses atom input headers for streaming call path without apply_patch errors", async () => { + const uiTheme = await getUiTheme(); + const component = editToolRenderer.renderCall( + { + input: "---packages/coding-agent/src/edit/renderer.ts\n$\n+// preview", + }, + { expanded: false, isPartial: true, spinnerFrame: 0, renderContext: { editMode: "atom" } }, + uiTheme, + ); + + const rendered = Bun.stripANSI(component.render(160).join("\n")); + expect(rendered).toContain("packages/coding-agent/src/edit/renderer.ts"); + expect(rendered).not.toContain("The first line of the patch must be"); + }); + + it("recognizes compact and quoted atom input headers", async () => { + const uiTheme = await getUiTheme(); + const compactComponent = editToolRenderer.renderCall( + { + input: "---foo bar.ts\n^\n+// preview", + }, + { expanded: true, isPartial: true, spinnerFrame: 0, renderContext: { editMode: "atom" } }, + uiTheme, + ); + + const quotedComponent = editToolRenderer.renderCall( + { + input: "---'baz qux.ts'\n+// preview", + }, + { expanded: false, isPartial: true, spinnerFrame: 0, renderContext: { editMode: "atom" } }, + uiTheme, + ); + + const compactRendered = Bun.stripANSI(compactComponent.render(160).join("\n")); + const quotedRendered = Bun.stripANSI(quotedComponent.render(160).join("\n")); + expect(compactRendered).toContain("foo bar.ts"); + expect(quotedRendered).toContain("baz qux.ts"); + }); + + it("uses atom input headers for completed single-file result path", async () => { + const uiTheme = await getUiTheme(); + const component = editToolRenderer.renderResult( + { + content: [{ type: "text", text: "Updated packages/coding-agent/src/edit/renderer.ts" }], + details: { + diff: "+1|// preview", + op: "update", + }, + }, + { expanded: false, isPartial: false, renderContext: { editMode: "atom" } }, + uiTheme, + { + input: "---packages/coding-agent/src/edit/renderer.ts\n$\n+// preview", + }, + ); + + const rendered = Bun.stripANSI(component.render(160).join("\n")); + expect(rendered).toContain("packages/coding-agent/src/edit/renderer.ts"); + expect(rendered).not.toContain(" …"); + }); }); diff --git a/packages/coding-agent/test/tools/search-path-lists.test.ts b/packages/coding-agent/test/tools/search-path-lists.test.ts index bd2c5385c..02461850f 100644 --- a/packages/coding-agent/test/tools/search-path-lists.test.ts +++ b/packages/coding-agent/test/tools/search-path-lists.test.ts @@ -127,6 +127,45 @@ describe("search tool path lists", () => { expect(details?.scopePath).toBe("packages"); }); + it("search formats absolute in-cwd paths relative to cwd", async () => { + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "search"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("Missing search tool"); + + const absoluteAppsPath = path.join(tempDir, "apps"); + const result = await tool.execute("search-absolute-in-cwd", { + pattern: "shared-needle", + path: absoluteAppsPath, + }); + const text = getText(result); + const details = result.details as { fileCount?: number; scopePath?: string } | undefined; + + expect(text).toContain("# apps"); + expect(text).toContain("## grep.txt"); + expect(text).not.toContain(tempDir); + expect(details?.fileCount).toBe(1); + expect(details?.scopePath).toBe("apps"); + }); + + it("write reports absolute in-cwd targets relative to cwd", async () => { + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "write"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("Missing write tool"); + + const absoluteTarget = path.join(tempDir, "written.txt"); + const result = await tool.execute("write-absolute-in-cwd", { + path: absoluteTarget, + content: "written\n", + }); + const text = getText(result); + + expect(text).toContain("Successfully wrote 8 bytes to written.txt"); + expect(text).not.toContain(tempDir); + expect(await Bun.file(absoluteTarget).text()).toBe("written\n"); + }); + it("ast_grep accepts quoted path and glob filters", async () => { const tools = await createTools(createTestSession(tempDir)); const tool = tools.find(entry => entry.name === "ast_grep"); @@ -253,6 +292,31 @@ describe("search tool path lists", () => { expect(details?.scopePath).toBe("packages"); }); + it("find keeps paths outside cwd absolute", async () => { + const outsideDir = await fs.mkdtemp(path.join(path.dirname(tempDir), "find-outside-")); + try { + await Bun.write(path.join(outsideDir, "outside.txt"), "outside\n"); + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "find"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("Missing find tool"); + + const result = await tool.execute("find-outside-cwd", { + pattern: outsideDir, + }); + const text = getText(result); + const expectedPath = path.join(outsideDir, "outside.txt").replace(/\\/g, "/"); + const details = result.details as { fileCount?: number; scopePath?: string } | undefined; + + expect(text).toContain(expectedPath); + expect(text).not.toContain("../"); + expect(details?.fileCount).toBe(1); + expect(details?.scopePath).toBe(outsideDir.replace(/\\/g, "/")); + } finally { + await fs.rm(outsideDir, { recursive: true, force: true }); + } + }); + it("grep accepts bare space-separated directory names (no trailing slash)", async () => { const tools = await createTools(createTestSession(tempDir)); const tool = tools.find(entry => entry.name === "search"); diff --git a/packages/typescript-edit-benchmark/src/index.ts b/packages/typescript-edit-benchmark/src/index.ts index 7ab279b5a..9917270ad 100755 --- a/packages/typescript-edit-benchmark/src/index.ts +++ b/packages/typescript-edit-benchmark/src/index.ts @@ -14,9 +14,15 @@ import { parseArgs } from "node:util"; import { type ResolvedThinkingLevel, ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import { Effort, THINKING_EFFORTS } from "@oh-my-pi/pi-ai"; import { padding, visibleWidth } from "@oh-my-pi/pi-tui"; -import { TempDir } from "@oh-my-pi/pi-utils"; +import { postmortem, TempDir } from "@oh-my-pi/pi-utils"; import { generateJsonReport, generateReport } from "./report"; -import { type BenchmarkConfig, type ProgressEvent, runBenchmark } from "./runner"; +import { + type BenchmarkConfig, + type BenchmarkResult, + buildBenchmarkResult, + type ProgressEvent, + runBenchmark, +} from "./runner"; import { type EditTask, loadTasksFromDir, validateFixturesFromDir } from "./tasks"; const COLOR_ENABLED = Boolean(process.stdout.isTTY) && !process.env.NO_COLOR; @@ -33,6 +39,10 @@ const ANSI = { cyan: "\x1b[36m", } as const; +const RUNS_DIR = path.resolve(import.meta.dir, "..", "..", "..", "runs"); + +fs.mkdirSync(RUNS_DIR, { recursive: true }); + function paint(code: string, text: string): string { return COLOR_ENABLED ? `${code}${text}${ANSI.reset}` : text; } @@ -59,7 +69,7 @@ function generateReportFilename(config: BenchmarkConfig, format: "markdown" | "j const variant = config.editVariant ?? "replace"; const timestamp = new Date().toISOString().replace(/:/g, "-").replace(/\..+$/, "").replace(/Z$/, "Z"); const ext = format === "json" ? "json" : "md"; - return `runs/${modelName}_${variant}_${timestamp}.${ext}`; + return path.join(RUNS_DIR, `${modelName}_${variant}_${timestamp}.${ext}`); } async function resolveConversationDumpDir(outputPath: string): Promise { @@ -77,6 +87,21 @@ async function resolveConversationDumpDir(outputPath: string): Promise { return path.join(parsed.dir, `${parsed.name}.${timestamp}.dump`); } +async function conversationDumpStatus(dumpDir: string): Promise { + try { + const stat = await fs.promises.stat(dumpDir); + if (stat.isDirectory()) { + return `Conversation dumps written to: ${dumpDir}`; + } + return `Conversation dump path is not a directory: ${dumpDir}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return `No conversation dumps written: ${dumpDir}`; + } + throw error; + } +} + function printUsage(tasks?: EditTask[]): void { const taskList = tasks ? tasks.map(t => ` ${t.id.padEnd(30)} ${t.name}`).join("\n") @@ -436,10 +461,55 @@ async function main(): Promise { console.log(""); const progress = new LiveProgress(tasksToRun.length * config.runsPerTask, config.runsPerTask); - const result = await runBenchmark(tasksToRun, config, event => { - progress.handleEvent(event); + let latestResult = buildBenchmarkResult({ + tasks: tasksToRun, + config, + resultsByTask: new Map(), + startTime: new Date().toISOString(), }); - progress.finish(); + let progressFinished = false; + let reportWritePromise: Promise | undefined; + const finishProgress = () => { + if (progressFinished) return; + progress.finish(); + progressFinished = true; + }; + const writeReport = async (result: BenchmarkResult, interrupted: boolean) => { + if (reportWritePromise) return reportWritePromise; + reportWritePromise = (async () => { + if (interrupted) { + console.log(""); + console.log("Benchmark interrupted; writing partial report..."); + } + const report = formatType === "json" ? generateJsonReport(result) : generateReport(result); + await Bun.write(outputPath, report); + console.log(`Report written to: ${outputPath}`); + if (config.conversationDumpDir) { + console.log(await conversationDumpStatus(config.conversationDumpDir)); + } + })(); + return reportWritePromise; + }; + const unregisterReportCleanup = postmortem.register("typescript-edit-benchmark-report", async reason => { + if (reason === postmortem.Reason.EXIT) return; + finishProgress(); + await writeReport(latestResult, true); + if (cleanup) { + await cleanup(); + } + }); + const result = await runBenchmark( + tasksToRun, + config, + event => { + progress.handleEvent(event); + }, + snapshot => { + latestResult = snapshot; + }, + ); + latestResult = result; + finishProgress(); console.log(""); console.log("Benchmark complete!"); @@ -453,11 +523,8 @@ async function main(): Promise { } console.log(""); - const report = formatType === "json" ? generateJsonReport(result) : generateReport(result); - - await Bun.write(outputPath, report); - console.log(`Report written to: ${outputPath}`); - console.log(`Conversation dumps written to: ${config.conversationDumpDir}`); + await writeReport(result, false); + unregisterReportCleanup(); if (cleanup) { await cleanup(); @@ -571,9 +638,9 @@ class LiveProgress { #printSummary(): void { const n = this.#completed; - if (n === 0) return; + const denom = n || 1; - const successRate = (this.#success / n) * 100; + const successRate = (this.#success / denom) * 100; const editSuccessRate = this.#totalEdits > 0 ? (this.#totalEditSuccesses / this.#totalEdits) * 100 : 100; const avgIndent = this.#indentScores.length > 0 ? this.#indentScores.reduce((a, b) => a + b, 0) / this.#indentScores.length : 0; @@ -590,9 +657,9 @@ class LiveProgress { console.log(` Tool calls: read=${this.#totalReads} edit=${this.#totalEdits} write=${this.#totalWrites}`); console.log(` Tool input chars: ${this.#totalToolInputChars.toLocaleString()}`); console.log( - ` Avg tokens/task: ${Math.round(this.#totalInput / n)} in / ${Math.round(this.#totalOutput / n)} out`, + ` Avg tokens/task: ${Math.round(this.#totalInput / denom)} in / ${Math.round(this.#totalOutput / denom)} out`, ); - console.log(` Avg time/task: ${Math.round(this.#totalDuration / n)}ms`); + console.log(` Avg time/task: ${Math.round(this.#totalDuration / denom)}ms`); } #renderLine(): void { diff --git a/packages/typescript-edit-benchmark/src/report.ts b/packages/typescript-edit-benchmark/src/report.ts index e519c8b09..4b5ef4d21 100644 --- a/packages/typescript-edit-benchmark/src/report.ts +++ b/packages/typescript-edit-benchmark/src/report.ts @@ -31,12 +31,22 @@ function escapeMarkdown(text: string): string { return text.replace(/\|/g, "\\|").replace(/\n/g, " "); } +function getStringField(value: unknown, field: string): string | null { + if (!value || typeof value !== "object") return null; + const fieldValue = (value as Record)[field]; + return typeof fieldValue === "string" ? fieldValue : null; +} + function formatEditArgsBlock(args: unknown): string { if (!args || typeof args !== "object") return "—"; - const diff = (args as { diff?: unknown }).diff; - if (typeof diff === "string") { + const diff = getStringField(args, "diff"); + if (diff !== null) { return diff; } + const input = getStringField(args, "input"); + if (input !== null) { + return input; + } try { return JSON.stringify(args, null, 2); } catch { diff --git a/packages/typescript-edit-benchmark/src/runner.ts b/packages/typescript-edit-benchmark/src/runner.ts index 21f7e642c..67cb00295 100644 --- a/packages/typescript-edit-benchmark/src/runner.ts +++ b/packages/typescript-edit-benchmark/src/runner.ts @@ -9,7 +9,6 @@ import * as fs from "node:fs"; import * as path from "node:path"; import type { AgentMessage, ResolvedThinkingLevel, ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import type { Model } from "@oh-my-pi/pi-ai"; - import { computeLineHash, formatSessionDumpText, RpcClient } from "@oh-my-pi/pi-coding-agent"; import { prompt } from "@oh-my-pi/pi-utils"; import { diffLines } from "diff"; @@ -21,9 +20,16 @@ import benchmarkTaskPrompt from "./prompts/benchmark-task.md" with { type: "text import type { EditTask } from "./tasks"; import { verifyExpectedFileSubset, verifyExpectedFiles } from "./verify"; -const TMP = `/tmp/rb-${Math.random().toString(36).slice(2, 10)}`; +const REPO_ROOT = path.resolve(import.meta.dir, "..", "..", ".."); +const RUNS_DIR = path.join(REPO_ROOT, "runs"); +const TMP = path.join(RUNS_DIR, `rb-${Math.random().toString(36).slice(2, 10)}`); const CLI_PATH = Bun.fileURLToPath(import.meta.resolve("@oh-my-pi/pi-coding-agent/cli")); +function formatLogPath(logFile: string): string { + const relativePath = path.relative(REPO_ROOT, logFile); + return relativePath === "" ? "." : relativePath; +} + /** Subset of session state used for markdown conversation dumps (parity with /dump). */ type ConversationDumpSessionState = { sessionFile?: string; @@ -48,7 +54,7 @@ interface BenchmarkClient { dispose(): Promise; } -fs.mkdirSync(TMP); +fs.mkdirSync(TMP, { recursive: true }); let n = 0; function subtmp(pre: string): string { @@ -191,9 +197,8 @@ function getEditPathFromArgs(args: unknown): string | null { } const HASHLINE_SUBTYPES = ["set", "set_range", "insert"] as const; - -const BENCHMARK_TOOL_NAMES = ["read", "edit", "vim", "write", "apply_patch"] as const; -const EDIT_TOOL_NAMES = ["edit", "vim", "apply_patch"] as const; +const BENCHMARK_TOOL_NAMES = ["read", "edit", "write", "apply_patch"] as const; +const EDIT_TOOL_NAMES = ["edit", "apply_patch"] as const; function isEditTool(toolName: unknown): toolName is (typeof EDIT_TOOL_NAMES)[number] { return toolName === "edit" || toolName === "vim" || toolName === "apply_patch"; @@ -1196,7 +1201,7 @@ async function runSingleTask( ? `Verification failed: ${error}${diff ? `\n\nDiff (expected vs actual):\n\n\`\`\`diff\n${diff}\n\`\`\`` : ""}${mutationIntentSuffix}` : `Previous attempt failed.${mutationIntentSuffix}`; } - if (!useInProcess) { + if (config.conversationDumpDir) { conversationSnapshot = await snapshotConversationDump(client); } } finally { @@ -1225,7 +1230,7 @@ async function runSingleTask( timeoutTelemetry, mutationIntentValidation, }); - console.log(` Log: ${logFile}`); + console.log(` Log: ${formatLogPath(logFile)}`); if (config.conversationDumpDir && conversationSnapshot) { await writeConversationDump({ @@ -1542,7 +1547,7 @@ async function _runRpcBenchmarkRun( timeoutTelemetry, mutationIntentValidation, }); - console.log(` Log: ${logFile}`); + console.log(` Log: ${formatLogPath(logFile)}`); await persistConversationDump({ client, @@ -2013,52 +2018,16 @@ export async function runTask( return summarizeTaskRuns(task, runs); } -export async function runBenchmark( - tasks: EditTask[], - config: BenchmarkConfig, - onProgress?: (event: ProgressEvent) => void, -): Promise { - const startTime = new Date().toISOString(); +export function buildBenchmarkResult(params: { + tasks: EditTask[]; + config: BenchmarkConfig; + resultsByTask: Map; + startTime: string; + endTime?: string; +}): BenchmarkResult { + const taskResults = params.tasks.map(task => summarizeTaskRuns(task, params.resultsByTask.get(task.id) ?? [])); - // Discover shared infrastructure once for in-process mode - const useInProcess = config.inProcess !== false; - const shared = useInProcess - ? await discoverSharedInfra({ - editVariant: config.editVariant, - editFuzzy: config.editFuzzy, - editFuzzyThreshold: config.editFuzzyThreshold, - }) - : undefined; - - const runItems: TaskRunItem[] = tasks.flatMap(task => - Array.from({ length: config.runsPerTask }, (_, runIndex) => ({ task, runIndex })), - ); - - const pending = shuffle(runItems); - const resultsByTask = new Map(); - const concurrency = Math.max(1, Math.floor(config.taskConcurrency)); - const running: Promise[] = []; - - const runNext = async (): Promise => { - const nextItem = pending.shift(); - if (!nextItem) return; - const { task, result } = await runConcurrentBenchmarkRun(nextItem, config, onProgress, shared); - const list = resultsByTask.get(task.id) ?? []; - list.push(result); - resultsByTask.set(task.id, list); - await runNext(); - }; - - const slots = Math.min(concurrency, pending.length); - for (let i = 0; i < slots; i++) { - running.push(runNext()); - } - - await Promise.all(running); - - const taskResults = tasks.map(task => summarizeTaskRuns(task, resultsByTask.get(task.id) ?? [])); - - const endTime = new Date().toISOString(); + const endTime = params.endTime ?? new Date().toISOString(); const allRuns = taskResults.flatMap(t => t.runs); const totalRuns = allRuns.length; @@ -2115,7 +2084,7 @@ export async function runBenchmark( : undefined; const hashlineEditSubtypes: Record | undefined = - config.editVariant === "hashline" + params.config.editVariant === "hashline" ? Object.fromEntries( HASHLINE_SUBTYPES.map(key => [ key, @@ -2126,7 +2095,7 @@ export async function runBenchmark( const denom = effectiveRuns || 1; const summary: BenchmarkSummary = { - totalTasks: tasks.length, + totalTasks: params.tasks.length, totalRuns: effectiveRuns, successfulRuns, overallSuccessRate: successfulRuns / denom, @@ -2168,10 +2137,58 @@ export async function runBenchmark( }; return { - config, + config: params.config, tasks: taskResults, summary, - startTime, + startTime: params.startTime, endTime, }; } + +export async function runBenchmark( + tasks: EditTask[], + config: BenchmarkConfig, + onProgress?: (event: ProgressEvent) => void, + onResultSnapshot?: (result: BenchmarkResult) => void, +): Promise { + const startTime = new Date().toISOString(); + + // Discover shared infrastructure once for in-process mode + const useInProcess = config.inProcess !== false; + const shared = useInProcess + ? await discoverSharedInfra({ + editVariant: config.editVariant, + editFuzzy: config.editFuzzy, + editFuzzyThreshold: config.editFuzzyThreshold, + }) + : undefined; + + const runItems: TaskRunItem[] = tasks.flatMap(task => + Array.from({ length: config.runsPerTask }, (_, runIndex) => ({ task, runIndex })), + ); + + const pending = shuffle(runItems); + const resultsByTask = new Map(); + const concurrency = Math.max(1, Math.floor(config.taskConcurrency)); + const running: Promise[] = []; + + const runNext = async (): Promise => { + const nextItem = pending.shift(); + if (!nextItem) return; + const { task, result } = await runConcurrentBenchmarkRun(nextItem, config, onProgress, shared); + const list = resultsByTask.get(task.id) ?? []; + list.push(result); + resultsByTask.set(task.id, list); + onResultSnapshot?.(buildBenchmarkResult({ tasks, config, resultsByTask, startTime })); + await runNext(); + }; + + const slots = Math.min(concurrency, pending.length); + for (let i = 0; i < slots; i++) { + running.push(runNext()); + } + + await Promise.all(running); + + return buildBenchmarkResult({ tasks, config, resultsByTask, startTime }); +} diff --git a/packages/typescript-edit-benchmark/test/runner.test.ts b/packages/typescript-edit-benchmark/test/runner.test.ts index a86137199..1b23d9b00 100644 --- a/packages/typescript-edit-benchmark/test/runner.test.ts +++ b/packages/typescript-edit-benchmark/test/runner.test.ts @@ -4,7 +4,9 @@ import * as path from "node:path"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import { formatSessionDumpText, SessionManager } from "@oh-my-pi/pi-coding-agent"; import { TempDir } from "@oh-my-pi/pi-utils"; -import { writeConversationDump } from "../src/runner"; +import { generateReport } from "../src/report"; +import { buildBenchmarkResult, type TaskRunResult, writeConversationDump } from "../src/runner"; +import type { EditTask } from "../src/tasks"; const tempDirs: TempDir[] = []; @@ -22,6 +24,126 @@ afterEach(async () => { ); }); +function createTask(id: string): EditTask { + return { + id, + name: id, + prompt: `Fix ${id}`, + files: [`${id}.ts`], + inputDir: "/tmp/input", + expectedDir: "/tmp/expected", + }; +} + +function createRun(runIndex: number, success: boolean): TaskRunResult { + return { + runIndex, + success, + patchApplied: success, + verificationPassed: success, + tokens: { input: 12, output: 8, total: 20 }, + duration: 100, + toolCalls: { + read: 1, + edit: 1, + write: 0, + editSuccesses: success ? 1 : 0, + editFailures: success ? 0 : 1, + editWarnings: 0, + editAutocorrects: 0, + totalInputChars: 50, + }, + editFailures: [], + editWarnings: [], + editAutocorrectCount: 0, + }; +} + +describe("buildBenchmarkResult", () => { + it("summarizes completed runs without requiring every scheduled run to finish", () => { + const completedTask = createTask("completed"); + const pendingTask = createTask("pending"); + const resultsByTask = new Map([[completedTask.id, [createRun(0, true)]]]); + + const result = buildBenchmarkResult({ + tasks: [completedTask, pendingTask], + config: { + provider: "anthropic", + model: "claude", + runsPerTask: 2, + timeout: 1000, + taskConcurrency: 1, + }, + resultsByTask, + startTime: "2026-04-28T00:00:00.000Z", + endTime: "2026-04-28T00:00:01.000Z", + }); + + expect(result.summary.totalTasks).toBe(2); + expect(result.summary.totalRuns).toBe(1); + expect(result.summary.successfulRuns).toBe(1); + expect(result.tasks.find(task => task.id === "pending")?.runs).toEqual([]); + expect(result.startTime).toBe("2026-04-28T00:00:00.000Z"); + expect(result.endTime).toBe("2026-04-28T00:00:01.000Z"); + }); + + it("can generate a report before any run completes", () => { + const result = buildBenchmarkResult({ + tasks: [createTask("pending")], + config: { + provider: "anthropic", + model: "claude", + runsPerTask: 2, + timeout: 1000, + taskConcurrency: 1, + }, + resultsByTask: new Map(), + startTime: "2026-04-28T00:00:00.000Z", + endTime: "2026-04-28T00:00:01.000Z", + }); + + expect(result.summary.totalRuns).toBe(0); + expect(generateReport(result)).toContain("| Total Runs | 0 |"); + }); + + it("renders atom input args directly in edit error patch blocks", () => { + const task = createTask("atom"); + const titleExpression = "$" + "{title}"; + const input = [ + "---orcid.ts", + "276ka= if (works.length > 0) {", + "277fo= for (const title of works) {", + `278hu= md += \`- ${titleExpression}\\n\`;`, + "279he= }", + "280nd= } else {", + "281he= md += 'No works available.\\n';", + "282rd= }", + ].join("\n"); + const failedRun: TaskRunResult = { + ...createRun(0, false), + editFailures: [{ toolCallId: "edit-1", args: { input }, error: "No changes made" }], + }; + const result = buildBenchmarkResult({ + tasks: [task], + config: { + provider: "anthropic", + model: "claude", + runsPerTask: 1, + timeout: 1000, + taskConcurrency: 1, + editVariant: "atom", + }, + resultsByTask: new Map([[task.id, [failedRun]]]), + startTime: "2026-04-28T00:00:00.000Z", + endTime: "2026-04-28T00:00:01.000Z", + }); + + const report = generateReport(result); + expect(report).toContain(`\`\`\`diff\n${input}\n\`\`\``); + expect(report).not.toContain('"input":'); + }); +}); + describe("writeConversationDump", () => { it("writes benchmark conversations as session dumps and copies artifacts", async () => { const sourceRoot = await createTempDir("@typescript-edit-benchmark-source-");