fix(edit): preserve guard after transformed writes
This commit is contained in:
@@ -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 <token>` 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 <token>` 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
|
||||
|
||||
|
||||
@@ -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: [
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<number>): 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") ?? [];
|
||||
|
||||
@@ -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<number>;
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user