From dadaceaa6fe0c67ddd5944b5ca377959c6c08faa Mon Sep 17 00:00:00 2001 From: Kigbnajd Date: Thu, 13 Aug 2026 20:15:18 +0200 Subject: [PATCH 1/3] 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); }); From 705442dbdefa3f7636d614a553a5c90ccf170a1e Mon Sep 17 00:00:00 2001 From: Kigbnajd Date: Thu, 13 Aug 2026 21:33:22 +0200 Subject: [PATCH 2/3] fix(edit): preserve guard after transformed writes --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/edit/hashline/execute.ts | 6 +++- .../test/edit/seen-line-guard.test.ts | 30 +++++++++++++++++++ packages/hashline/CHANGELOG.md | 4 +++ packages/hashline/src/patcher.ts | 17 ++++++----- packages/hashline/src/snapshots.ts | 3 +- packages/hashline/test/patcher.test.ts | 22 ++++++++++++-- 7 files changed, 70 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 787732603..aad4e1d28 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### 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. +- 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 only when the written content exactly matches that output. ## [17.3.1] - 2026-08-13 diff --git a/packages/coding-agent/src/edit/hashline/execute.ts b/packages/coding-agent/src/edit/hashline/execute.ts index f4dd10977..88e124e8f 100644 --- a/packages/coding-agent/src/edit/hashline/execute.ts +++ b/packages/coding-agent/src/edit/hashline/execute.ts @@ -18,11 +18,13 @@ import { commitClipboard, forkClipboard, MismatchError as HashlineMismatchError, + normalizeToLF, Patch, Patcher, type PatchSectionResult, type PreparedSection, startClipboardBatch, + stripBom, UnseenLinesError, } from "@oh-my-pi/hashline"; import type { AgentToolResult } from "@oh-my-pi/pi-agent-core"; @@ -227,7 +229,9 @@ 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); + if (normalizeToLF(stripBom(result.written).text) === result.after) { + recordSeenLinesFromBody(session, canonicalSnapshotKey(result.canonicalPath), result.fileHash, text); + } return { toolResult: { 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 4409a3f7c..f0d2b5bcd 100644 --- a/packages/coding-agent/test/edit/seen-line-guard.test.ts +++ b/packages/coding-agent/test/edit/seen-line-guard.test.ts @@ -374,6 +374,36 @@ describe("read → edit seen-line guard", () => { ).rejects.toThrow(/never displayed \(it showed/); }); + it("does not trust requested diff lines after an ACP client transforms the write", async () => { + const file = path.join(tmpDir, "notes.txt"); + const content = "ONE\nTWO\nTHREE\nFOUR\nFIVE\n"; + await Bun.write(file, content); + const bridge = { + capabilities: { writeTextFile: true }, + writeTextFile: async ({ path: target, content: requested }: { path: string; content: string }) => { + await Bun.write(target, `CLIENT\n${requested}`); + }, + }; + const session = { + ...createSession(tmpDir), + getClientBridge: () => bridge, + } as ToolSession; + const store = getFileSnapshotStore(session); + const originalTag = store.record(canonicalSnapshotKey(file), content, [2]); + + const first = await executeHashlineSingle( + execOptions(`[notes.txt#${originalTag}]\nPUT 2.=2:\n+TWO EDITED`, session), + ); + const persistedTag = tagFromOutput(resultText(first)); + const drifted = "CLIENT\nONE\nTWO EDITED\nTHREE\nFOUR\nFIVE\n"; + expect(await Bun.file(file).text()).toBe(drifted); + + await expect( + executeHashlineSingle(execOptions(`[notes.txt#${persistedTag}]\nPUT 1.=1:\n+OVERWRITE`, session)), + ).rejects.toThrow(/never displayed/); + expect(await Bun.file(file).text()).toBe(drifted); + }); + 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}`); diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index af22e8b4f..3cacee203 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -6,6 +6,10 @@ - Added structured `UnseenLinesError.retryable` metadata so hosts can offer compact retry continuations only when every unseen anchor was revealed in full. +### Fixed + +- Distinguished absent seen-line provenance from an explicitly observed empty set, so transformed writes can require a fresh read without weakening the guard. + ## [17.3.0] - 2026-08-13 ### Fixed diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index 530cfb925..08ca22a07 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -569,7 +569,7 @@ export class Patcher { // false "drift" purely from BOM/line-ending restoration asymmetry. const recorded = normalizeToLF(stripBom(write.text).text); const driftedOnWrite = recorded !== after; - const fileHash = this.#recordFullSnapshot(canonicalPath, recorded); + const fileHash = this.#recordFullSnapshot(canonicalPath, recorded, driftedOnWrite ? [] : undefined); const allWarnings = driftedOnWrite ? [...warnings, writeDriftWarning(section.path)] : warnings; return { @@ -604,18 +604,19 @@ export class Patcher { } } - #recordFullSnapshot(canonicalPath: string, normalized: string): string { - return this.snapshots.record(canonicalPath, normalized); + #recordFullSnapshot(canonicalPath: string, normalized: string, seenLines?: Iterable): string { + return this.snapshots.record(canonicalPath, normalized, seenLines); } /** * Reject an anchored edit that references a line the read which minted * `expected` never displayed. `matchedSnapshot` is the store version whose * text equals the live normalized content — the exact snapshot the model - * anchored against. Absent means no provenance was recorded (the tag was - * externally minted or aged out), so the edit applies as before. Only runs - * on the no-drift path, where anchor line numbers index the tagged content - * 1:1. + * anchored against. A missing snapshot or undefined `seenLines` means no + * provenance was recorded (the tag was externally minted or aged out), so + * the edit applies as before. An empty set means provenance is active but no + * exact lines were displayed, so every anchor remains guarded. Only runs on + * the no-drift path, where anchor line numbers index the tagged content 1:1. * * The rejection inlines the actual file content at the unseen anchor lines * (from `matchedSnapshot.text`, which by definition equals the live @@ -635,7 +636,7 @@ export class Patcher { */ #assertSeenLines(section: PatchSection, expected: string, matchedSnapshot: Snapshot | null): void { const seen = matchedSnapshot?.seenLines; - if (!seen || seen.size === 0) return; + if (seen === undefined) return; const unseen = section.collectAnchorLines().filter(line => !seen.has(line)); if (unseen.length === 0) return; const sourceLines = matchedSnapshot?.text.split("\n") ?? []; diff --git a/packages/hashline/src/snapshots.ts b/packages/hashline/src/snapshots.ts index 6cae609c4..33935b8d8 100644 --- a/packages/hashline/src/snapshots.ts +++ b/packages/hashline/src/snapshots.ts @@ -41,7 +41,8 @@ export interface Snapshot { * bodies) leaves this sparse; a whole-file read fills every line. Multiple * reads of the same content union into one set. `undefined` means "no * provenance recorded" — the patcher then skips the seen-line check and - * applies as before. Mutated in place as more of the same content is read. + * applies as before. An empty set means provenance is active but no exact + * lines were displayed, so every anchored line remains guarded. */ seenLines?: Set; } diff --git a/packages/hashline/test/patcher.test.ts b/packages/hashline/test/patcher.test.ts index bca10887b..2be912c1f 100644 --- a/packages/hashline/test/patcher.test.ts +++ b/packages/hashline/test/patcher.test.ts @@ -193,9 +193,13 @@ describe("Patcher snapshot tag stays honest across a write-time content transfor // what turned a one-line edit into unexplained whole-file corruption. expect(section.warnings.some(w => w.includes(PATH) && /reformatted it on save/.test(w))).toBe(true); - // A follow-up edit anchored on the returned tag must succeed against - // the real (drifted) file instead of failing a stale-tag mismatch. - await patcher.apply(Patch.parse(`[${PATH}#${section.fileHash}]\nPUT 1-1:\n+function g() {`)); + // The returned tag must still resolve against the real drifted file. + // Because no exact persisted lines were displayed after the transform, + // the seen-line guard first reveals the anchor, then the same-tag retry + // succeeds instead of failing a stale-tag mismatch. + const followUp = `[${PATH}#${section.fileHash}]\nPUT 1-1:\n+function g() {`; + await expect(patcher.apply(Patch.parse(followUp))).rejects.toThrow(/never displayed/); + await patcher.apply(Patch.parse(followUp)); expect(fs.get(PATH)).toBe("function g() {\n\treturn 2;\n}\n"); }); }); @@ -417,6 +421,18 @@ describe("Patcher seen-line provenance", () => { expect(fs.get(PATH)).toBe(wideContent); }); + it("guards every anchor when provenance recorded no displayed lines", async () => { + const fs = new InMemoryFilesystem([[PATH, CONTENT]]); + const snapshots = new InMemorySnapshotStore(); + const tag = snapshots.record(PATH, CONTENT, []); + 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/, + ); + expect(fs.get(PATH)).toBe(CONTENT); + }); + it("skips the check when no seen lines were recorded (absent → allow)", async () => { const fs = new InMemoryFilesystem([[PATH, CONTENT]]); const snapshots = new InMemorySnapshotStore(); From 5e903cbb79abf75a0883c2dd054455e9f1c71a6f Mon Sep 17 00:00:00 2001 From: Kigbnajd Date: Thu, 13 Aug 2026 22:09:50 +0200 Subject: [PATCH 3/3] fix(edit): address seen-line retry review --- .../coding-agent/src/edit/hashline/execute.ts | 65 ++++++++++++------- .../coding-agent/src/tools/output-meta.ts | 5 ++ .../test/edit/seen-line-guard.test.ts | 29 ++++++++- 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/packages/coding-agent/src/edit/hashline/execute.ts b/packages/coding-agent/src/edit/hashline/execute.ts index 88e124e8f..948be4b5b 100644 --- a/packages/coding-agent/src/edit/hashline/execute.ts +++ b/packages/coding-agent/src/edit/hashline/execute.ts @@ -30,7 +30,7 @@ import { import type { AgentToolResult } from "@oh-my-pi/pi-agent-core"; import type { FileDiagnosticsResult, WritethroughCallback, WritethroughDeferredHandle } from "../../lsp"; import type { ToolSession } from "../../tools"; -import { outputMeta } from "../../tools/output-meta"; +import { outputMeta, resolveArtifactSpillThresholdBytes } from "../../tools/output-meta"; import { ToolError } from "../../tools/tool-errors"; import { generateDiffString } from "../diff"; import { getEditClipboard } from "../edit-clipboard"; @@ -66,7 +66,7 @@ function resolveHashlineInput(session: ToolSession, input: string): string { return input; } const pending = pendingSeenLineRetries.get(session); - if (!pending || pending.token !== match[1]) { + if (!pending || pending.token.toLowerCase() !== match[1].toLowerCase()) { throw new ToolError( "Unknown or expired edit retry token. Use the token from the latest seen-line rejection, or submit a full patch.", ); @@ -149,9 +149,33 @@ function narrowBatchRequest(outer: LspBatchRequest | undefined, isLast: boolean) return { id: outer.id, flush: isLast && outer.flush }; } +interface SeenLineProvenance { + absolutePath: string; + tag: string; + body: string; +} + interface RenderedSection { toolResult: AgentToolResult; perFileResult: EditToolPerFileResult; + seenLineProvenance?: SeenLineProvenance; +} + +function recordRenderedSeenLines( + session: ToolSession, + result: AgentToolResult, + rendered: readonly RenderedSection[], +): void { + const fullText = result.content + .filter(part => part.type === "text" && part.text) + .map(part => (part.type === "text" ? part.text : "")) + .join("\n"); + if (Buffer.byteLength(fullText, "utf8") > resolveArtifactSpillThresholdBytes(session.settings)) return; + for (const section of rendered) { + const provenance = section.seenLineProvenance; + if (!provenance) continue; + recordSeenLinesFromBody(session, provenance.absolutePath, provenance.tag, provenance.body); + } } const BLOCK_OP_LABELS: Record = { @@ -179,7 +203,6 @@ function renderSection( result: PatchSectionResult, diagnostics: FileDiagnosticsResult | undefined, sourcePath: string, - session: ToolSession, ): RenderedSection { if (result.op === "delete") { const toolResult: AgentToolResult = { @@ -229,10 +252,16 @@ function renderSection( const moveBlock = result.moveDest ? `\nMoved to ${result.moveDest}` : ""; const firstChangedLine = result.firstChangedLine ?? diff.firstChangedLine; const text = `${result.header}${blockBlock}${moveBlock}${previewBlock}${warningsBlock}`; - if (normalizeToLF(stripBom(result.written).text) === result.after) { - recordSeenLinesFromBody(session, canonicalSnapshotKey(result.canonicalPath), result.fileHash, text); - } + const seenLineProvenance = + normalizeToLF(stripBom(result.written).text) === result.after + ? { + absolutePath: canonicalSnapshotKey(result.canonicalPath), + tag: result.fileHash, + body: text, + } + : undefined; return { + seenLineProvenance, toolResult: { content: [ { @@ -305,15 +334,12 @@ export async function executeHashlineSingle( if (escalate) { throw new ToolError(noChangeLoopDiagnostic(sectionResult.path, count)); } - return renderSection(sectionResult, undefined, prepared.section.path, options.session).toolResult; + return renderSection(sectionResult, undefined, prepared.section.path).toolResult; } resetNoopEdit(options.session, sectionResult.canonicalPath); - return renderSection( - sectionResult, - fs.consumeDiagnostics(sectionResult.path), - prepared.section.path, - options.session, - ).toolResult; + const rendered = renderSection(sectionResult, fs.consumeDiagnostics(sectionResult.path), prepared.section.path); + recordRenderedSeenLines(options.session, rendered.toolResult, [rendered]); + return rendered.toolResult; } // Multi-section: prepare every section up front so we fail fast before @@ -354,16 +380,9 @@ 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, - options.session, - ), - ); + rendered.push(renderSection(sectionResult, fs.consumeDiagnostics(sectionResult.path), prepared[i].section.path)); } - return { + const result: AgentToolResult = { content: [ { type: "text", @@ -377,6 +396,8 @@ export async function executeHashlineSingle( perFileResults: rendered.map(r => r.perFileResult), }), }; + recordRenderedSeenLines(options.session, result, rendered); + return result; } export { HashlineMismatchError, type HashlineParams, hashlineEditParamsSchema }; diff --git a/packages/coding-agent/src/tools/output-meta.ts b/packages/coding-agent/src/tools/output-meta.ts index e2f8e301e..e403983b5 100644 --- a/packages/coding-agent/src/tools/output-meta.ts +++ b/packages/coding-agent/src/tools/output-meta.ts @@ -625,6 +625,11 @@ function getSpillConfig(s: Settings | undefined) { }; } +/** Resolve the byte threshold above which ordinary tool output spills to an artifact. */ +export function resolveArtifactSpillThresholdBytes(s: Settings | undefined): number { + return getSpillConfig(s).threshold; +} + /** * Resolve the OutputSink `headBytes` budget from session settings. * Exposed so streaming executors (bash/python/ssh/eval) can opt into 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 f0d2b5bcd..1ce314be2 100644 --- a/packages/coding-agent/test/edit/seen-line-guard.test.ts +++ b/packages/coding-agent/test/edit/seen-line-guard.test.ts @@ -321,7 +321,7 @@ describe("read → edit seen-line guard", () => { expect(retry).toBeDefined(); expect(await Bun.file(file).text()).toBe(CONTENT); - await executeHashlineSingle(execOptions(retry as string, session)); + await executeHashlineSingle(execOptions((retry as string).toUpperCase(), session)); const after = await Bun.file(file).text(); expect(after).toContain("X10\nX11\nX12"); expect(after).not.toContain("line 10"); @@ -374,6 +374,33 @@ describe("read → edit seen-line guard", () => { ).rejects.toThrow(/never displayed \(it showed/); }); + it("does not record edit-output lines when the result will spill", async () => { + const file = path.join(tmpDir, "notes.txt"); + await Bun.write(file, CONTENT); + const session = { + ...createSession(tmpDir), + settings: Settings.isolated({ + "edit.enforceSeenLines": true, + "tools.artifactSpillThreshold": 0.1, + "tools.artifactHeadBytes": 0.03, + "tools.artifactTailBytes": 0.03, + }), + } as ToolSession; + 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 ".repeat(80)}`, session), + ); + const text = resultText(edit); + const nextTag = tagFromOutput(text); + expect(text).toContain("2:"); + + const seen = store.byHash(canonicalSnapshotKey(file), nextTag)?.seenLines; + expect(seen?.has(2) ?? false).toBe(false); + }); + it("does not trust requested diff lines after an ACP client transforms the write", async () => { const file = path.join(tmpDir, "notes.txt"); const content = "ONE\nTWO\nTHREE\nFOUR\nFIVE\n";