feat(hashline): secured boundary repairs with parse checks and added warnings

- Gate closer-spare boundary repairs on tree-sitter parse validation to prevent incorrect rewrites on unrecognized languages or pathless edits.
- Add a warning when a `+` body row matches a valid hunk header format to flag accidental literal text insertion.
This commit is contained in:
can1357
2026-08-08 19:02:11 +02:00
parent 731c051733
commit abf80e4f74
8 changed files with 149 additions and 51 deletions
+4 -3
View File
@@ -7,9 +7,10 @@
### Added
- `applyEdits` now takes a `path` and uses the native tree-sitter parser as a veto over every boundary repair that depends on delimiter *semantics*: the edits are materialized as authored first, and if that result parses, no repair or advisory may touch it. A `}` inside a regex literal, a string, or Markdown prose is therefore never mistaken for a block closer. Wired through the patcher, recovery, section apply, and the edit tool's preview.
- Auto-repair for replacement ranges that start one line early on a structural closer (the `}` of the construct above): the closer is spared and the payload lands after it. Fires only when the authored edit does not parse, so it fixes the off-by-one that leaves an unclosed delimiter without touching prose.
- Warning for balanced payloads over ranges that end mid-block (deleting opener(s) whose closer(s) survive below), pointing at the block-op remedy (`PUT N*:`). Also gated behind the parser veto, so it stays silent whenever the authored edit is sound.
- `applyEdits` now takes a `path` and uses the native tree-sitter parser to decide every boundary repair that depends on delimiter *semantics*. The authored edits are materialized first: if that result parses, it is returned untouched, so a `}` inside a regex literal, a string, or Markdown prose is never mistaken for a block closer. A closer-spare repair lands only when the repaired result is *shown* to parse — never on delimiter arithmetic alone — so an unrecognized language or an unprovable candidate leaves the edit exactly as authored. Wired through the patcher, recovery, section apply, and the edit tool's preview.
- Auto-repair for replacement ranges that start one line early on a structural closer (the `}` of the construct above): the closer is spared and the payload lands after it, gated on the same parse proof.
- Warning for balanced payloads over ranges that end mid-block (deleting opener(s) whose closer(s) survive below), pointing at the block-op remedy (`PUT N*:`). Raised only when the baseline parsed and the authored result does not, so it cannot fire on prose or an unknown language.
- Warning when a `+` body row is itself a valid hunk header (`+CUT 5.=9`). Such a row is literal content by definition and is inserted into the file as text; naming it at the moment it happens turns a silent source-file corruption into an actionable diagnostic.
### Fixed
+42 -12
View File
@@ -123,6 +123,19 @@ function bucketAnchorEditsByLine(edits: IndexedEdit[]): Map<number, IndexedEdit[
}
return byLine;
}
/**
* A closer-spare repair could not tell which side of a spared delimiter the
* payload belongs on. Distinct from the evidence-complete textual rejections
* (a one-sided boundary echo) so {@link applyEdits} can withhold *only* this
* delimiter-semantics verdict on a file the parser cannot vouch for, while
* every other rejection propagates unconditionally.
*/
class CloserSpareAmbiguityError extends Error {
constructor(message: string) {
super(message);
this.name = "CloserSpareAmbiguityError";
}
}
// ═══════════════════════════════════════════════════════════════════════════
// Replacement-boundary repair
@@ -1157,7 +1170,7 @@ function repairReplacementBoundaries(
balanceNegate(droppedClosers.balance),
);
if (!payloadOpens && !(payloadIndent !== undefined && isIndentDeeper(payloadIndent, keptIndent))) {
throw new Error(
throw new CloserSpareAmbiguityError(
ambiguousCloserSpareMessage(
slot.group.startLine,
slot.group.endLine,
@@ -1213,7 +1226,7 @@ function repairReplacementBoundaries(
const payloadIndent = bodyTargetIndent(slot.group.payload);
if (payloadIndent !== undefined) {
if (!closerIndent.startsWith(payloadIndent)) {
throw new Error(
throw new CloserSpareAmbiguityError(
ambiguousLeadingCloserSpareMessage(slot.group.startLine, slot.group.endLine, droppedPrefix.count),
);
}
@@ -1628,17 +1641,34 @@ export function applyEdits(text: string, edits: readonly Edit[], options: ApplyE
};
const authoredWarnings = [...leading, ...authored.warnings];
if (!authored.suspicious) return finish(materializeEdits(fileLines, authored.edits), authoredWarnings);
const authoredResult = materializeEdits(fileLines, authored.edits);
// The parser's veto: the edit as authored is syntactically sound, so no
// delimiter heuristic may second-guess its boundaries and no advisory is
// warranted. This is what keeps a `}` in prose, in a string, or in a regex
// literal from ever being mistaken for a block closer.
// The authored edit keeps the file parsing, so no delimiter heuristic may
// second-guess its boundaries. This is what keeps a `}` in prose, in a
// string, or in a regex literal from ever being mistaken for a block closer.
if (parsesCleanly(options.path, authoredResult.text)) return finish(authoredResult, authoredWarnings);
if (!authored.sparesProposed) return finish(authoredResult, [...authoredWarnings, ...authored.advisories]);
// No veto: the authored result does not parse (or the language is not one
// the parser knows), and a swallowed block closer explains the damage.
const spared = repairReplacementBoundaries(targetEdits, fileLines, true);
return finish(materializeEdits(fileLines, spared.edits), [...leading, ...spared.warnings, ...spared.advisories]);
// The authored result does not parse — or the parser does not know this
// language, in which case nothing below can be proven and nothing is
// rewritten. A repair lands only when it is *shown* to restore a parsing
// file, never on delimiter arithmetic alone.
const baselineParses = parsesCleanly(options.path, text);
if (authored.sparesProposed) {
try {
const spared = repairReplacementBoundaries(targetEdits, fileLines, true);
const sparedResult = materializeEdits(fileLines, spared.edits);
if (parsesCleanly(options.path, sparedResult.text)) {
return finish(sparedResult, [...leading, ...spared.warnings, ...spared.advisories]);
}
} catch (error) {
// Only the closer-spare verdict is the parser's business, and only on
// a file it can vouch for. Every other rejection — notably the
// evidence-complete one-sided boundary echo, which is proven by exact
// line equality and would otherwise delete range lines the body never
// restates — propagates regardless of what the parser knows.
if (baselineParses || !(error instanceof CloserSpareAmbiguityError)) throw error;
}
}
// Nothing proven: leave the authored edit exactly as written. Report the
// damage only when the baseline parsed, so this edit demonstrably caused it.
return finish(authoredResult, baselineParses ? [...authoredWarnings, ...authored.advisories] : authoredWarnings);
}
+21 -1
View File
@@ -1,6 +1,13 @@
/** Centralized error/warning text for the hashline parser, applier, and patcher. */
import { formatNumberedLine, HL_FILE_HASH_SEP, HL_FILE_PREFIX, HL_FILE_SUFFIX, HL_RANGE_SEP } from "./format";
import {
formatNumberedLine,
HL_FILE_HASH_SEP,
HL_FILE_PREFIX,
HL_FILE_SUFFIX,
HL_PAYLOAD_REPLACE,
HL_RANGE_SEP,
} from "./format";
import type { BlockSpan } from "./types";
/** Lines of context shown either side of a hash mismatch. */
@@ -133,6 +140,19 @@ export function repeatedSnapshotRowMessage(line: number): string {
`rows holding their complete final content.`
);
}
/**
* A `+` body row whose text is itself a valid hunk header — the op was written
* with the payload prefix, so it is inserted into the file as literal text
* instead of executing. Warned rather than rejected: a literal `CUT …` line is
* legitimate content in documentation and test fixtures.
*/
export function literalOpRowWarning(line: number, text: string): string {
return (
`line ${line}: body row \`${HL_PAYLOAD_REPLACE}${text}\` is itself a valid hunk header, so it was inserted ` +
`into the file as literal text rather than executed. Ops are never \`${HL_PAYLOAD_REPLACE}\`-prefixed — drop ` +
`the \`${HL_PAYLOAD_REPLACE}\` to run it, and re-issue if this line landed in the file by mistake.`
);
}
/** Bare range header recovered as an implicit replacement hunk. */
export const BARE_RANGE_AUTO_PUT_WARNING = `Recovered a bare \`N${HL_RANGE_SEP}M:\` header as \`PUT N${HL_RANGE_SEP}M:\`. Prefix replacement ranges with \`PUT\`.`;
+6 -1
View File
@@ -17,6 +17,7 @@ import {
EMPTY_INSERT,
EMPTY_PUT_AUTO_CUT_WARNING,
invalidAbsoluteRangeMessage,
literalOpRowWarning,
MINUS_BULLET_AUTO_PIPED_WARNING,
MINUS_ROW_REJECTED,
MOVE_TAKES_NO_BODY,
@@ -28,7 +29,7 @@ import {
SNAPSHOT_ROWS_AUTO_PUT_WARNING,
} from "./messages";
import { isReadMetadataLine, stripOneLeadingHashlinePrefix } from "./prefixes";
import { type BlockTarget, cloneCursor, type ParsedRange, type Token, Tokenizer } from "./tokenizer";
import { type BlockTarget, cloneCursor, isHunkHeaderText, type ParsedRange, type Token, Tokenizer } from "./tokenizer";
import type { Anchor, BlockSpan, Cursor, Edit, FileOp, PasteTarget } from "./types";
/** Bounds parser amplification before the target file's line count is available. */
@@ -461,6 +462,10 @@ export class Executor {
const noBodyOnLiteral = bodylessTargetMessage(pending.target, pending.hadColon);
if (noBodyOnLiteral !== null) throw new Error(`line ${lineNum}: ${noBodyOnLiteral}`);
this.#commitDeferredBlanks(pending);
// An op written with the payload prefix is inserted as literal text. That
// is the correct reading of `+TEXT`, but it silently plants a `CUT …` line
// in the file, so name it at the moment it happens.
if (isHunkHeaderText(text)) this.#warnings.push(literalOpRowWarning(lineNum, text));
pending.payloads.push({ kind: "literal", text, lineNum });
}
+4 -2
View File
@@ -20,11 +20,13 @@ const PARSE_CACHE_MAX = 256;
* `true` when `text` parses without a syntax error under the language inferred
* from `path`. `false` covers "does not parse" and "cannot tell" alike — no
* path, an unrecognized language, or a native failure — because both mean the
* probe has no veto to cast.
* probe has nothing to prove with. Callers must therefore never treat `false`
* as evidence *about the edit*: it only withholds permission to rewrite.
*
* Uses `enclosingBlockBoundaries` over a whole-file window: no node can cross
* that window, so the boundary walk is trivial and the tree-sitter parse is the
* only real cost. It returns `null` precisely when the source does not parse.
* only real cost. It returns `null` for an unrecognized language and for a
* source that fails to parse, which this predicate deliberately conflates.
*/
export function parsesCleanly(path: string | undefined, text: string): boolean {
if (path === undefined) return false;
+15
View File
@@ -428,6 +428,21 @@ function tryParseHunkHeader(line: string): ParsedHunkHeader | null {
if (scan.nextIndex !== end) return null;
return { target: scan.target, hadColon: scan.hadColon };
}
/**
* Whether `text` would parse as a hunk header on its own (`PUT …`, `CUT …`,
* `REM`, `MV …`). Used to catch an op row mistakenly written as a `+` body row,
* which the applier would otherwise insert into the file as literal text.
*/
export function isHunkHeaderText(text: string): boolean {
const end = trimEndIndex(text);
const lead = skipWhitespace(text, 0, end);
const isHunkLead =
text.startsWith(HL_PUT_KEYWORD, lead) ||
text.startsWith(HL_CUT_KEYWORD, lead) ||
text.startsWith(HL_REM_KEYWORD, lead) ||
text.startsWith(HL_MOVE_KEYWORD, lead);
return isHunkLead && tryParseHunkHeader(text) !== null;
}
function tryParseHeader(line: string): { path: string; fileHash?: string } | null {
if (!line.startsWith(HL_FILE_PREFIX)) return null;
+48 -32
View File
@@ -408,13 +408,13 @@ describe("boundary-balance repair", () => {
" auto* handle = payloadFor<PyThreadHandle>(self);",
" if (!handle)",
' return threadError(globalObject, "thread not started");',
" handle->setDone();",
" handle.setDone();",
"}",
].join("\n");
const diff = [
"PUT 3-4:",
"+ auto* handle = payloadFor<PyThreadHandle>(self);",
"+ if (!handle || !handle->isStarted())",
"+ if (!handle || !handle.isStarted())",
].join("\n");
expect(() => apply(file, diff)).toThrow(/rejected: the body opens by restating/);
});
@@ -435,10 +435,10 @@ describe("boundary-balance repair", () => {
it("rejects sparing a deleted closer when the payload claims no position inside the block", () => {
const file = [
" if (!global) {",
" handle->setDone();",
" handle.setDone();",
" return;",
" }",
" handle->setIdent(currentIdent());",
" handle.setIdent(currentIdent());",
].join("\n");
const diff = ["PUT 4-4:", "+ after();"].join("\n");
expect(() => apply(file, diff)).toThrow(/before or after the closer is ambiguous/);
@@ -480,9 +480,9 @@ describe("boundary-balance repair", () => {
// the net deleted-prefix balance is zero, so the closer is correctly kept.
it("keeps the closer when the matching opener is replaced rather than removed", () => {
const file = ["if (a) {", "\told();", "}"].join("\n");
const diff = ["PUT 1-1:", "+if (b) {", "PUT 2-3:", "+\tnew();"].join("\n");
const diff = ["PUT 1-1:", "+if (b) {", "PUT 2-3:", "+\tfresh();"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["if (b) {", "\tnew();", "}"].join("\n"));
expect(text).toBe(["if (b) {", "\tfresh();", "}"].join("\n"));
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1);
});
@@ -526,9 +526,9 @@ describe("boundary-balance repair", () => {
it("ignores non-contiguously deleted openers when choosing which closer to keep", () => {
const file = ["if (a) {", "\told();", "\tmore();", "}", "const obj = {", "\ta: 1,", "};"].join("\n");
const diff = ["CUT 1", "PUT 3-4:", "+\tnew();", "PUT 7-7:", "+\tb: 2,"].join("\n");
const diff = ["CUT 1", "PUT 3-4:", "+\tfresh();", "PUT 7-7:", "+\tb: 2,"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["\told();", "\tnew();", "const obj = {", "\ta: 1,", "\tb: 2,", "};"].join("\n"));
expect(text).toBe(["\told();", "\tfresh();", "const obj = {", "\ta: 1,", "\tb: 2,", "};"].join("\n"));
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1);
});
@@ -543,7 +543,7 @@ describe("boundary-balance repair", () => {
].join("\n");
const diff = [
"PUT 2-3:",
"+\tnew();",
"+\tfresh();",
"PUT 4-6:",
"+function supportsDevinThinking(config: ClientModelConfig): boolean {",
"+\treturn config.supportsThinking === true;",
@@ -553,7 +553,7 @@ describe("boundary-balance repair", () => {
expect(text).toBe(
[
"if (a) {",
"\tnew();",
"\tfresh();",
"}",
"function supportsDevinThinking(config: ClientModelConfig): boolean {",
"\treturn config.supportsThinking === true;",
@@ -565,51 +565,51 @@ describe("boundary-balance repair", () => {
it("does not let an earlier kept closer cover a later orphan closer", () => {
const file = ["if (a) {", "\told();", "}", "}"].join("\n");
const diff = ["PUT 2-3:", "+\tnew();", "PUT 4-4:", "+after();"].join("\n");
const diff = ["PUT 2-3:", "+\tfresh();", "PUT 4-4:", "+after();"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["if (a) {", "\tnew();", "}", "after();"].join("\n"));
expect(text).toBe(["if (a) {", "\tfresh();", "}", "after();"].join("\n"));
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1);
});
it("does not keep a deleted outer closer when one survives below the range", () => {
const file = ["class C {", "\tmethod() {", "\t\told();", "\t}", "}", "}"].join("\n");
const diff = ["PUT 2-5:", "+\tmethod() {", "+\t\tnew();", "+\t}"].join("\n");
const diff = ["PUT 2-5:", "+\tmethod() {", "+\t\tfresh();", "+\t}"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["class C {", "\tmethod() {", "\t\tnew();", "\t}", "}"].join("\n"));
expect(text).toBe(["class C {", "\tmethod() {", "\t\tfresh();", "\t}", "}"].join("\n"));
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(0);
});
it("keeps an omitted inner closer when the outer closer survives below", () => {
const file = ["class C {", "\tmethod() {", "\t\told();", "\t}", "}", "}"].join("\n");
const diff = ["PUT 2-5:", "+\tmethod() {", "+\t\tnew();"].join("\n");
const diff = ["PUT 2-5:", "+\tmethod() {", "+\t\tfresh();"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["class C {", "\tmethod() {", "\t\tnew();", "\t}", "}"].join("\n"));
expect(text).toBe(["class C {", "\tmethod() {", "\t\tfresh();", "\t}", "}"].join("\n"));
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1);
});
it("counts head insertions before replacement payloads in original coordinates", () => {
const file = ["\told();", "}"].join("\n");
const diff = ["PUT <1:", "+if (a) {", "PUT 1-2:", "+\tnew();"].join("\n");
const diff = ["PUT <1:", "+if (a) {", "PUT 1-2:", "+\tfresh();"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["if (a) {", "\tnew();", "}"].join("\n"));
expect(text).toBe(["if (a) {", "\tfresh();", "}"].join("\n"));
expect(warnings.some(warning => /kept 1 structural closing line/.test(warning))).toBe(true);
});
it("counts a separately inserted closer immediately below the range", () => {
const file = ["class C {", "\told();", "}", "after();", "const obj = {", "\ta: 1,", "};"].join("\n");
const diff = ["PUT 2-3:", "+\tnew();", "PUT <4:", "+}", "PUT 7-7:", "+\tb: 2,"].join("\n");
const diff = ["PUT 2-3:", "+\tfresh();", "PUT <4:", "+}", "PUT 7-7:", "+\tb: 2,"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(
["class C {", "\tnew();", "}", "after();", "const obj = {", "\ta: 1,", "\tb: 2,", "};"].join("\n"),
["class C {", "\tfresh();", "}", "after();", "const obj = {", "\ta: 1,", "\tb: 2,", "};"].join("\n"),
);
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1);
});
it("keeps an omitted outer closer even when the payload restates an inner closer", () => {
const file = ["if (a) {", "\tif (b) {", "\t\told();", "\t}", "}", "after();"].join("\n");
const diff = ["PUT 1-5:", "+if (a) {", "+\tif (c) {", "+\t\tnew();", "+\t}"].join("\n");
const diff = ["PUT 1-5:", "+if (a) {", "+\tif (c) {", "+\t\tfresh();", "+\t}"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["if (a) {", "\tif (c) {", "\t\tnew();", "\t}", "}", "after();"].join("\n"));
expect(text).toBe(["if (a) {", "\tif (c) {", "\t\tfresh();", "\t}", "}", "after();"].join("\n"));
expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1);
});
@@ -813,22 +813,38 @@ describe("boundary-balance repair", () => {
expect(text).toBe(["fn f() {", "\tif a {", "\t\treturn;", "\t}", "\tlet lead = new1();", "}"].join("\n"));
expect(warnings.filter(warning => /leading structural closing line/.test(warning))).toHaveLength(1);
});
// The veto needs a language to reason about: with no path the probe abstains
// and the delimiter heuristics remain the only available evidence.
it("falls back to delimiter heuristics when no path is supplied", () => {
// No proof, no mutation. Without a path the probe cannot judge anything, so
// the closer-spare must not fire: the edit lands exactly as authored, even
// though the delimiter heuristics alone would have "repaired" it.
it("applies as authored when no path is supplied, since no repair can be proven", () => {
const file = ["fn f() {", "\tif a {", "\t\treturn;", "\t}", "\tlet lead = old1();", "}"].join("\n");
const { text } = applyEdits(file, parsePatch(["PUT 4-5:", "+\tlet lead = new1();"].join("\n")).edits);
expect(text).toBe(["fn f() {", "\tif a {", "\t\treturn;", "\t}", "\tlet lead = new1();", "}"].join("\n"));
const { text } = applyEdits(file, parsePatch(["PUT 4-5:", "+\tlet lead = fresh1();"].join("\n")).edits);
expect(text).toBe(["fn f() {", "\tif a {", "\t\treturn;", "\tlet lead = fresh1();", "}"].join("\n"));
});
// An unparseable *authored* result is what unlocks the repair, so a file in
// a language tree-sitter does not know behaves like the pathless case.
it("falls back to delimiter heuristics for a language the parser does not know", () => {
// Same for a language tree-sitter does not know: nothing can be proven, so
// nothing is rewritten and no advisory is invented.
it("applies as authored for a language the parser does not know", () => {
const file = ["fn f() {", "\tif a {", "\t\treturn;", "\t}", "\tlet lead = old1();", "}"].join("\n");
const result = applyEdits(file, parsePatch(["PUT 4-5:", "+\tlet lead = new1();"].join("\n")).edits, {
const result = applyEdits(file, parsePatch(["PUT 4-5:", "+\tlet lead = fresh1();"].join("\n")).edits, {
path: "fixture.unknownlang",
});
expect(result.text).toBe(["fn f() {", "\tif a {", "\t\treturn;", "\t}", "\tlet lead = new1();", "}"].join("\n"));
expect(result.text).toBe(["fn f() {", "\tif a {", "\t\treturn;", "\tlet lead = fresh1();", "}"].join("\n"));
expect(result.warnings ?? []).toHaveLength(0);
});
// The one-sided boundary echo is proven by exact line equality, not by
// delimiter semantics, so the parser has no say over it. It must reject even
// on a language the probe cannot read — otherwise suppressing the
// closer-spare verdict would also let this unsafe edit through, deleting
// range lines the body never restates.
it("still rejects a too-short one-sided echo on a language the parser cannot read", () => {
const file = ["alpha", "beta", "gamma", "delta", "eps"].join("\n");
const diff = ["PUT 2-4:", "+alpha", "+fresh1"].join("\n");
for (const path of [undefined, "fixture.unknownlang", "fixture.ts"]) {
expect(() => applyEdits(file, parsePatch(diff).edits, path === undefined ? {} : { path })).toThrow(
/too short to be the full final content/,
);
}
});
});
+9
View File
@@ -97,6 +97,15 @@ describe("hashline core — verb header forms", () => {
expect(() => parsePatch("2:B\n4:first\n4:second")).toThrow(/name line 4/);
expect(() => parsePatch("2:B\n4:first\n4:second")).toThrow(/keep only the last row/);
});
// The xutf `native.rs` incident: `+CUT 1266.=1277` inside a `PUT` body is a
// literal row by spec, so it was inserted into the Rust file as text. That
// reading is correct, but it must be named — the agent that hit this filed a
// bug against the tool instead of repairing the line it had just planted.
it("warns when a body row is itself a hunk header written with the payload prefix", () => {
const result = parsePatch("PUT >1:\n+inserted();\n+CUT 1266.=1277");
expect(applyEdits(FILE, result.edits).text).toBe("a\ninserted();\nCUT 1266.=1277\nb\nc\nd\ne");
expect(result.warnings.some(w => /is itself a valid hunk header/.test(w))).toBe(true);
});
it("recovers a bare range header as an implicit PUT", () => {
const result = parsePatch("2.=3:\n+X");