From dadaceaa6fe0c67ddd5944b5ca377959c6c08faa Mon Sep 17 00:00:00 2001 From: Kigbnajd Date: Thu, 13 Aug 2026 20:15:18 +0200 Subject: [PATCH] fix(edit): add compact seen-line retries --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/edit/hashline/execute.ts | 83 +++++++++++++++++-- .../test/edit/seen-line-guard.test.ts | 73 ++++++++++++++++ packages/hashline/CHANGELOG.md | 4 + packages/hashline/src/grammar.lark | 5 +- packages/hashline/src/patcher.ts | 19 ++++- packages/hashline/src/prompt.md | 4 + packages/hashline/test/patcher.test.ts | 11 ++- 8 files changed, 189 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d0b7ec695..787732603 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed seen-line guard retries forcing agents to resend entire unchanged patches. Complete inline reveals now issue one-shot `RETRY ` continuations that rerun validation against live files, while numbered lines in successful edit output join the returned snapshot's seen-line provenance. + ## [17.3.1] - 2026-08-13 ### Fixed diff --git a/packages/coding-agent/src/edit/hashline/execute.ts b/packages/coding-agent/src/edit/hashline/execute.ts index 2e0025b20..f4dd10977 100644 --- a/packages/coding-agent/src/edit/hashline/execute.ts +++ b/packages/coding-agent/src/edit/hashline/execute.ts @@ -10,6 +10,7 @@ * batch's `flush` flag to true only for the final write so diagnostics * round-trip once. */ +import { randomUUID } from "node:crypto"; import { type BlockResolution, buildCompactDiffPreview, @@ -22,6 +23,7 @@ import { type PatchSectionResult, type PreparedSection, startClipboardBatch, + UnseenLinesError, } from "@oh-my-pi/hashline"; import type { AgentToolResult } from "@oh-my-pi/pi-agent-core"; import type { FileDiagnosticsResult, WritethroughCallback, WritethroughDeferredHandle } from "../../lsp"; @@ -30,7 +32,7 @@ import { outputMeta } from "../../tools/output-meta"; import { ToolError } from "../../tools/tool-errors"; import { generateDiffString } from "../diff"; import { getEditClipboard } from "../edit-clipboard"; -import { getFileSnapshotStore } from "../file-snapshot-store"; +import { canonicalSnapshotKey, getFileSnapshotStore, recordSeenLinesFromBody } from "../file-snapshot-store"; import type { EditToolDetails, EditToolPerFileResult, LspBatchRequest } from "../renderer"; import { pruneOversizedEditSnapshots } from "../snapshot-details"; import { nativeBlockResolver } from "./block-resolver"; @@ -47,6 +49,53 @@ export interface ExecuteHashlineSingleOptions { beginDeferredDiagnosticsForPath: (path: string) => WritethroughDeferredHandle; } +interface PendingSeenLineRetry { + token: string; + input: string; +} + +const pendingSeenLineRetries = new WeakMap(); +const SEEN_LINE_RETRY_INPUT = /^RETRY ([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\n?$/i; + +function resolveHashlineInput(session: ToolSession, input: string): string { + const match = SEEN_LINE_RETRY_INPUT.exec(input); + if (!match) { + pendingSeenLineRetries.delete(session); + return input; + } + const pending = pendingSeenLineRetries.get(session); + if (!pending || pending.token !== match[1]) { + throw new ToolError( + "Unknown or expired edit retry token. Use the token from the latest seen-line rejection, or submit a full patch.", + ); + } + pendingSeenLineRetries.delete(session); + return pending.input; +} + +async function prepareWithSeenLineRetry( + patcher: Patcher, + section: Parameters[0], + clipboard: Clipboard, + session: ToolSession, + input: string, +): Promise { + try { + return await patcher.prepare(section, clipboard); + } catch (error) { + if (!(error instanceof UnseenLinesError) || !error.retryable) throw error; + const token = randomUUID(); + pendingSeenLineRetries.set(session, { token, input }); + throw new ToolError( + `${error.message}\n\n` + + "The original patch is stored for a one-shot retry. After verifying the revealed lines match your intent, " + + "call edit with only:\n" + + `RETRY ${token}\n` + + "If your intent changed, submit a revised full patch instead. The retry revalidates the live files.", + ); + } +} + function noChangeDiagnostic(path: string): string { // The patch parsed and applied cleanly but produced no change ā€” the // `+TEXT` body rows matched the file content at the targeted lines @@ -128,6 +177,7 @@ function renderSection( result: PatchSectionResult, diagnostics: FileDiagnosticsResult | undefined, sourcePath: string, + session: ToolSession, ): RenderedSection { if (result.op === "delete") { const toolResult: AgentToolResult = { @@ -176,12 +226,14 @@ function renderSection( : ""; const moveBlock = result.moveDest ? `\nMoved to ${result.moveDest}` : ""; const firstChangedLine = result.firstChangedLine ?? diff.firstChangedLine; + const text = `${result.header}${blockBlock}${moveBlock}${previewBlock}${warningsBlock}`; + recordSeenLinesFromBody(session, canonicalSnapshotKey(result.canonicalPath), result.fileHash, text); return { toolResult: { content: [ { type: "text", - text: `${result.header}${blockBlock}${moveBlock}${previewBlock}${warningsBlock}`, + text, }, ], details: pruneOversizedEditSnapshots({ @@ -214,7 +266,8 @@ function renderSection( export async function executeHashlineSingle( options: ExecuteHashlineSingleOptions, ): Promise> { - const patch = Patch.parse(options.input, { cwd: options.session.cwd }); + const input = resolveHashlineInput(options.session, options.input); + const patch = Patch.parse(input, { cwd: options.session.cwd }); if (patch.sections.length === 0) { throw new Error("No hashline sections found in input."); } @@ -237,10 +290,10 @@ export async function executeHashlineSingle( const clipboard = startClipboardBatch(sessionClipboard); // Single-section fast path: prepare, commit, render. - const inputHash = hashPatchInput(options.input); + const inputHash = hashPatchInput(input); if (patch.sections.length === 1) { fs.setBatchRequest(narrowBatchRequest(options.batchRequest, true)); - const prepared = await patcher.prepare(patch.sections[0], clipboard); + const prepared = await prepareWithSeenLineRetry(patcher, patch.sections[0], clipboard, options.session, input); const sectionResult = await patcher.commit(prepared); commitClipboard(clipboard, sessionClipboard); if (sectionResult.op === "noop") { @@ -248,10 +301,15 @@ export async function executeHashlineSingle( if (escalate) { throw new ToolError(noChangeLoopDiagnostic(sectionResult.path, count)); } - return renderSection(sectionResult, undefined, prepared.section.path).toolResult; + return renderSection(sectionResult, undefined, prepared.section.path, options.session).toolResult; } resetNoopEdit(options.session, sectionResult.canonicalPath); - return renderSection(sectionResult, fs.consumeDiagnostics(sectionResult.path), prepared.section.path).toolResult; + return renderSection( + sectionResult, + fs.consumeDiagnostics(sectionResult.path), + prepared.section.path, + options.session, + ).toolResult; } // Multi-section: prepare every section up front so we fail fast before @@ -264,7 +322,7 @@ export async function executeHashlineSingle( // deleted would otherwise be lost. const sectionStates: Clipboard[] = []; for (const section of patch.sections) { - prepared.push(await patcher.prepare(section, clipboard)); + prepared.push(await prepareWithSeenLineRetry(patcher, section, clipboard, options.session, input)); sectionStates.push(forkClipboard(clipboard)); } assertUniqueCanonicalPaths(prepared); @@ -292,7 +350,14 @@ export async function executeHashlineSingle( : new ToolError(noChangeDiagnostic(sectionResult.path)); } resetNoopEdit(options.session, sectionResult.canonicalPath); - rendered.push(renderSection(sectionResult, fs.consumeDiagnostics(sectionResult.path), prepared[i].section.path)); + rendered.push( + renderSection( + sectionResult, + fs.consumeDiagnostics(sectionResult.path), + prepared[i].section.path, + options.session, + ), + ); } return { content: [ diff --git a/packages/coding-agent/test/edit/seen-line-guard.test.ts b/packages/coding-agent/test/edit/seen-line-guard.test.ts index d5a85cea5..4409a3f7c 100644 --- a/packages/coding-agent/test/edit/seen-line-guard.test.ts +++ b/packages/coding-agent/test/edit/seen-line-guard.test.ts @@ -302,6 +302,78 @@ describe("read ā†’ edit seen-line guard", () => { expect(after).not.toContain("line 10"); }); + it("retries an unchanged patch through a one-shot token without resending its body", async () => { + const file = path.join(tmpDir, "notes.txt"); + await Bun.write(file, CONTENT); + const session = createSession(tmpDir); + + const read = await new ReadTool(session).execute("r1", { path: `${file}:1-3` }); + const tag = tagFromOutput(resultText(read)); + const input = `[notes.txt#${tag}]\nPUT 10.=12:\n+X10\n+X11\n+X12`; + + let message: string | undefined; + try { + await executeHashlineSingle(execOptions(input, session)); + } catch (err) { + message = (err as Error).message; + } + const retry = message?.match(/(?:^|\n)(RETRY [0-9a-f-]{36})(?:\n|$)/)?.[1]; + expect(retry).toBeDefined(); + expect(await Bun.file(file).text()).toBe(CONTENT); + + await executeHashlineSingle(execOptions(retry as string, session)); + const after = await Bun.file(file).text(); + expect(after).toContain("X10\nX11\nX12"); + expect(after).not.toContain("line 10"); + + await expect(executeHashlineSingle(execOptions(retry as string, session))).rejects.toThrow( + /Unknown or expired edit retry token/, + ); + }); + + it("revalidates live content before consuming a retry token", async () => { + const file = path.join(tmpDir, "notes.txt"); + await Bun.write(file, CONTENT); + const session = createSession(tmpDir); + + const read = await new ReadTool(session).execute("r1", { path: `${file}:1-3` }); + const tag = tagFromOutput(resultText(read)); + const input = `[notes.txt#${tag}]\nPUT 10.=12:\n+X10\n+X11\n+X12`; + + let message: string | undefined; + try { + await executeHashlineSingle(execOptions(input, session)); + } catch (err) { + message = (err as Error).message; + } + const retry = message?.match(/(?:^|\n)(RETRY [0-9a-f-]{36})(?:\n|$)/)?.[1]; + expect(retry).toBeDefined(); + + const drifted = CONTENT.replace("line 10", "EXTERNAL"); + await Bun.write(file, drifted); + await expect(executeHashlineSingle(execOptions(retry as string, session))).rejects.toThrow(); + expect(await Bun.file(file).text()).toBe(drifted); + }); + + it("records numbered edit output under the returned tag and keeps undisplayed lines guarded", async () => { + const file = path.join(tmpDir, "notes.txt"); + await Bun.write(file, CONTENT); + const session = createSession(tmpDir); + const store = getFileSnapshotStore(session); + + const read = await new ReadTool(session).execute("r1", { path: `${file}:1-3` }); + const tag = tagFromOutput(resultText(read)); + const edit = await executeHashlineSingle(execOptions(`[notes.txt#${tag}]\nPUT 2.=2:\n+EDITED`, session)); + const nextTag = tagFromOutput(resultText(edit)); + const seen = store.byHash(canonicalSnapshotKey(file), nextTag)?.seenLines; + + expect(seen?.has(2)).toBe(true); + expect(seen?.has(12)).toBe(false); + await expect( + executeHashlineSingle(execOptions(`[notes.txt#${nextTag}]\nPUT 12.=12:\n+UNSEEN`, session)), + ).rejects.toThrow(/never displayed \(it showed/); + }); + it("keeps the re-read fallback when the anchor set exceeds the inline reveal cap", async () => { const file = path.join(tmpDir, "long.txt"); const lines = Array.from({ length: 200 }, (_, i) => `line ${i + 1}`); @@ -328,6 +400,7 @@ describe("read ā†’ edit seen-line guard", () => { expect(message).not.toContain("140:line 140"); // Guidance directs at a range re-read of the FULL anchor range. expect(message).toMatch(/long\.txt:100-159/); + expect(message).not.toMatch(/(?:^|\n)RETRY [0-9a-f-]{36}(?:\n|$)/); expect(await Bun.file(file).text()).toBe(`${lines.join("\n")}\n`); }); diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 77504aff8..af22e8b4f 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added structured `UnseenLinesError.retryable` metadata so hosts can offer compact retry continuations only when every unseen anchor was revealed in full. + ## [17.3.0] - 2026-08-13 ### Fixed diff --git a/packages/hashline/src/grammar.lark b/packages/hashline/src/grammar.lark index 1b4ed17e4..6cfdfee94 100644 --- a/packages/hashline/src/grammar.lark +++ b/packages/hashline/src/grammar.lark @@ -1,6 +1,9 @@ -start: begin_patch file_patch+ end_patch +start: retry_patch | begin_patch file_patch+ end_patch begin_patch: "*** Begin Patch" LF end_patch: "*** End Patch" LF? +retry_patch: "RETRY " UUID LF? +UUID: /[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}/ + file_patch: file_header hunk+ file_header: "[" filename "#" file_hash "]" LF diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index f93c18ef7..530cfb925 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -65,6 +65,20 @@ const SEEN_LINE_REVEAL_CAP = 40; */ const SEEN_LINE_REVEAL_MAX_COLUMNS = 512; +/** + * Seen-line rejection metadata for hosts that can offer an explicit retry + * continuation after presenting the revealed source to the caller. + */ +export class UnseenLinesError extends Error { + constructor( + message: string, + readonly retryable: boolean, + ) { + super(message); + this.name = "UnseenLinesError"; + } +} + export interface PatcherOptions { /** Storage backend used for all reads and writes. */ fs: Filesystem; @@ -650,7 +664,10 @@ export class Patcher { if (!truncated) { for (const { line } of revealed) seen.add(line); } - throw new Error(unseenLinesMessage(section.path, unseen, expected, { lines: revealed, truncated })); + throw new UnseenLinesError( + unseenLinesMessage(section.path, unseen, expected, { lines: revealed, truncated }), + !truncated, + ); } #mismatchError( section: PatchSection, diff --git a/packages/hashline/src/prompt.md b/packages/hashline/src/prompt.md index 0811096e1..fe8e20773 100644 --- a/packages/hashline/src/prompt.md +++ b/packages/hashline/src/prompt.md @@ -4,6 +4,10 @@ Line-anchored patch language: name original lines/gaps to replace, insert, cut, Section: `[PATH#TAG]`; `TAG`: 4-hex snapshot from latest `read`/`search`, REQUIRED each section. New files: `write`; hashline edits existing files only. + +Only after a seen-line rejection supplies a retry token: verify its revealed lines, then use exactly `RETRY TOKEN` to apply the stored unchanged patch without resending it. A retry is one-shot and revalidates live files. If intent changes, submit a revised full patch instead. NEVER invent or reuse tokens. + + `PUT N.=M:`: replace original inclusive lines Nā€“M with body. `PUT N*:`: replace syntactic block beginning N; closing line resolved. diff --git a/packages/hashline/test/patcher.test.ts b/packages/hashline/test/patcher.test.ts index 452d91c60..bca10887b 100644 --- a/packages/hashline/test/patcher.test.ts +++ b/packages/hashline/test/patcher.test.ts @@ -12,6 +12,7 @@ import { NodeFilesystem, Patch, Patcher, + UnseenLinesError, type WriteResult, } from "@oh-my-pi/hashline"; @@ -272,9 +273,13 @@ describe("Patcher seen-line provenance", () => { const tag = snapshots.record(PATH, CONTENT, [1, 2]); const patcher = new Patcher({ fs, snapshots }); - await expect(patcher.apply(Patch.parse(`[${PATH}#${tag}]\nPUT 4-4:\n+L4`))).rejects.toThrow( - /never displayed \(it showed/, - ); + const error = await patcher + .apply(Patch.parse(`[${PATH}#${tag}]\nPUT 4-4:\n+L4`)) + .then(() => undefined) + .catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(UnseenLinesError); + expect((error as UnseenLinesError).retryable).toBe(true); + expect((error as Error).message).toMatch(/never displayed \(it showed/); expect(fs.get(PATH)).toBe(CONTENT); });