feat(coding-agent): made hashline seen-line guard opt-in via edit.enforceSeenLines
- Added the `enforceSeenLines` option to hashline `PatcherOptions` (defaults `true`); the seen-line guard in `Patcher` now runs only when enabled. - Added the `edit.enforceSeenLines` coding-agent setting (default off) and wired it through `edit/hashline/execute.ts` into the `Patcher`. - Stopped `file-snapshot-store` excluding column-clipped (>512-char) lines from a snapshot's seen set, so single-line edits on long lines apply without a full-width re-read. - Updated `seen-line-guard` tests and the hashline/coding-agent changelogs.
This commit is contained in:
@@ -11,11 +11,13 @@
|
||||
- Changed the default `astGrep.enabled` setting to `false`
|
||||
- Batched todo operations with real tool calls to prevent solo todo turns and extra round trips
|
||||
|
||||
- Added the `edit.enforceSeenLines` setting (default off) to gate the hashline seen-line guard. When off, hashline tags validate on content hash alone and any anchor into the tagged content applies; when on, edits anchored on lines a prior `read`/`grep` never displayed are rejected.
|
||||
### Fixed
|
||||
|
||||
- Fixed Bash internal URLs remaining unresolved when used as unquoted arguments inside command substitutions ([#5535](https://github.com/can1357/oh-my-pi/issues/5535)).
|
||||
- Fixed the built-in `fd` printing `fd: Broken pipe (os error 32)` when a downstream pipeline reader exited early (e.g. `fd … | head`); it now exits silently with 141 (128+SIGPIPE), matching real fd.
|
||||
- Fixed prewalk repeatedly continuing after a bash-only task such as `commit` had already completed ([#5551](https://github.com/can1357/oh-my-pi/issues/5551)).
|
||||
- Made the hashline seen-line guard opt-in and off by default (see `edit.enforceSeenLines`), and stopped excluding column-clipped (>512-char) lines from a snapshot's seen set: a displayed line now counts as seen even when its display was column-truncated, so single-line edits on long lines found via `read`/`grep` apply without a separate full-width re-read.
|
||||
- Fixed the Bash tool hanging when in-process commands read process substitution operands such as `<(cmd)` ([#5557](https://github.com/can1357/oh-my-pi/issues/5557)).
|
||||
- Fixed `/share` and `/export` web views rendering inline Markdown inside list items as literal text ([#5567](https://github.com/can1357/oh-my-pi/issues/5567)).
|
||||
|
||||
|
||||
@@ -3023,6 +3023,17 @@ export const SETTINGS_SCHEMA = {
|
||||
},
|
||||
},
|
||||
|
||||
"edit.enforceSeenLines": {
|
||||
type: "boolean",
|
||||
default: false,
|
||||
ui: {
|
||||
tab: "files",
|
||||
group: "Editing",
|
||||
label: "Enforce Seen-Line Guard",
|
||||
description: "Reject edits anchored on lines a prior read/search never displayed in full",
|
||||
},
|
||||
},
|
||||
|
||||
readLineNumbers: {
|
||||
type: "boolean",
|
||||
default: false,
|
||||
|
||||
@@ -131,27 +131,18 @@ export function recordSeenLines(
|
||||
}
|
||||
|
||||
/**
|
||||
* Attach the lines a read displayed to the snapshot it minted, so the patcher
|
||||
* can reject edits anchored on lines the model never saw. Best-effort: a no-op
|
||||
* when the body has no numbered rows or the snapshot already aged out. `tag`
|
||||
* must be the tag returned when this exact content was recorded.
|
||||
*
|
||||
* `excludedLines` prunes 1-indexed line numbers whose displayed text was
|
||||
* column-truncated (or otherwise not shown in full). A column-clipped row
|
||||
* still carries a `NN:` prefix — the parser sees the number and would
|
||||
* otherwise mark the line "seen" even though only its prefix ever reached
|
||||
* the model. Producers that apply per-line column truncation MUST supply
|
||||
* the clipped line set so the patcher's seen-line guard keeps rejecting
|
||||
* edits against those lines until a full-width read of them occurs.
|
||||
* Attach the lines a read displayed to the snapshot it minted, so the patcher's
|
||||
* (opt-in) seen-line guard can reject edits anchored on lines the model never
|
||||
* saw. Best-effort: a no-op when the body has no numbered rows or the snapshot
|
||||
* already aged out. `tag` must be the tag returned when this exact content was
|
||||
* recorded. Every displayed `NN:` row counts as seen, including column-clipped
|
||||
* rows — the guard no longer distinguishes full-width from truncated display.
|
||||
*/
|
||||
export function recordSeenLinesFromBody(
|
||||
session: FileSnapshotStoreOwner,
|
||||
absolutePath: string,
|
||||
tag: string,
|
||||
body: string,
|
||||
excludedLines?: ReadonlySet<number>,
|
||||
): void {
|
||||
const parsed = parseSeenLinesFromHashlineBody(body);
|
||||
const filtered = excludedLines && excludedLines.size > 0 ? parsed.filter(line => !excludedLines.has(line)) : parsed;
|
||||
recordSeenLines(session, absolutePath, tag, filtered);
|
||||
recordSeenLines(session, absolutePath, tag, parseSeenLinesFromHashlineBody(body));
|
||||
}
|
||||
|
||||
@@ -210,7 +210,8 @@ export async function executeHashlineSingle(
|
||||
batchRequest: options.batchRequest,
|
||||
});
|
||||
const snapshots = getFileSnapshotStore(options.session);
|
||||
const patcher = new Patcher({ fs, snapshots, blockResolver: nativeBlockResolver });
|
||||
const enforceSeenLines = options.session.settings.get("edit.enforceSeenLines");
|
||||
const patcher = new Patcher({ fs, snapshots, blockResolver: nativeBlockResolver, enforceSeenLines });
|
||||
|
||||
// Single-section fast path: prepare, commit, render.
|
||||
const inputHash = hashPatchInput(options.input);
|
||||
|
||||
@@ -19,7 +19,7 @@ function createSession(cwd: string): ToolSession {
|
||||
getSessionSpawns: () => "*",
|
||||
getArtifactsDir: () => path.join(cwd, "artifacts"),
|
||||
allocateOutputArtifact: async () => ({ id: "artifact-1", path: path.join(cwd, "artifact-1.log") }),
|
||||
settings: Settings.isolated(),
|
||||
settings: Settings.isolated({ "edit.enforceSeenLines": true }),
|
||||
enableLsp: false,
|
||||
} as ToolSession;
|
||||
}
|
||||
@@ -331,12 +331,11 @@ describe("read → edit seen-line guard", () => {
|
||||
expect(await Bun.file(file).text()).toBe(`${lines.join("\n")}\n`);
|
||||
});
|
||||
|
||||
it("does not mark column-clipped read lines as seen", async () => {
|
||||
it("marks column-clipped read lines as seen (clipped-line check removed)", async () => {
|
||||
// A 4KB single line — the read tool's column cap (default 512 chars)
|
||||
// clips this into `<prefix>…` in the numbered output. The clipped line
|
||||
// number MUST stay out of the tag's seenLines, or a subsequent edit
|
||||
// anchored there would slip past the seen-line guard having seen only
|
||||
// the first 512 chars.
|
||||
// clips this into `<prefix>…` in the numbered output. The clipped-line
|
||||
// exclusion was removed, so the displayed line counts as seen and a
|
||||
// follow-up edit anchored there applies even with the guard enabled.
|
||||
const file = path.join(tmpDir, "wide.txt");
|
||||
const wide = "a".repeat(4096);
|
||||
const content = `head\n${wide}\nfoot\n`;
|
||||
@@ -347,14 +346,10 @@ describe("read → edit seen-line guard", () => {
|
||||
const tag = tagFromOutput(resultText(read));
|
||||
|
||||
const seen = getFileSnapshotStore(session).byHash(canonicalSnapshotKey(file), tag)?.seenLines;
|
||||
expect(seen?.has(2)).toBe(false);
|
||||
expect(seen?.has(2)).toBe(true);
|
||||
|
||||
// A straight edit anchored at the clipped line 2 is still rejected —
|
||||
// the seen-line guard fires because the model only saw the prefix.
|
||||
await expect(
|
||||
executeHashlineSingle(execOptions(`[wide.txt#${tag}]\nSWAP 2.=2:\n+REPLACED`, session)),
|
||||
).rejects.toThrow(/never displayed \(it showed/);
|
||||
expect(await Bun.file(file).text()).toBe(content);
|
||||
await executeHashlineSingle(execOptions(`[wide.txt#${tag}]\nSWAP 2.=2:\n+REPLACED`, session));
|
||||
expect(await Bun.file(file).text()).toBe("head\nREPLACED\nfoot\n");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -381,7 +376,11 @@ describe("search → edit seen-line guard", () => {
|
||||
getArtifactsDir: () => path.join(cwd, "artifacts"),
|
||||
allocateOutputArtifact: async () => ({ id: "artifact-1", path: path.join(cwd, "artifact-1.log") }),
|
||||
// Zero context so the seen set is exactly the matched lines.
|
||||
settings: Settings.isolated({ "grep.contextBefore": 0, "grep.contextAfter": 0 }),
|
||||
settings: Settings.isolated({
|
||||
"grep.contextBefore": 0,
|
||||
"grep.contextAfter": 0,
|
||||
"edit.enforceSeenLines": true,
|
||||
}),
|
||||
enableLsp: false,
|
||||
} as ToolSession;
|
||||
}
|
||||
@@ -419,3 +418,31 @@ describe("search → edit seen-line guard", () => {
|
||||
expect(await Bun.file(file).text()).toBe(`${lines.join("\n")}\n`);
|
||||
});
|
||||
});
|
||||
|
||||
describe("seen-line guard disabled by default", () => {
|
||||
let tmpDir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
await Settings.init({ inMemory: true });
|
||||
});
|
||||
beforeEach(async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "seen-line-off-"));
|
||||
});
|
||||
afterEach(async () => {
|
||||
await removeWithRetries(tmpDir);
|
||||
});
|
||||
|
||||
it("applies an edit on an unseen line when edit.enforceSeenLines is off (default)", async () => {
|
||||
const file = path.join(tmpDir, "notes.txt");
|
||||
await Bun.write(file, CONTENT);
|
||||
// createSession enables the guard; a default session leaves it off.
|
||||
const session = { ...createSession(tmpDir), settings: Settings.isolated() } as ToolSession;
|
||||
|
||||
const read = await new ReadTool(session).execute("r1", { path: `${file}:1-3` });
|
||||
const tag = tagFromOutput(resultText(read));
|
||||
|
||||
// Line 12 was never displayed, but the guard is disabled, so it applies.
|
||||
await executeHashlineSingle(execOptions(`[notes.txt#${tag}]\nSWAP 12.=12:\n+EDITED`, session));
|
||||
expect(await Bun.file(file).text()).toContain("EDITED");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,14 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added `enforceSeenLines` option to `PatcherOptions` to control whether seen-line validation is enforced on anchored edits; defaults to `true` for safety
|
||||
|
||||
### Changed
|
||||
|
||||
- Seen-line guard now respects `enforceSeenLines` setting; when `false`, tags validate on content hash alone without requiring lines to have been seen during reading
|
||||
|
||||
## [16.5.0] - 2026-07-13
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -73,6 +73,12 @@ export interface PatcherOptions {
|
||||
* host did not wire a resolver). Plain line-range ops never need it.
|
||||
*/
|
||||
blockResolver?: BlockResolver;
|
||||
/**
|
||||
* Enforce the seen-line guard: reject anchored edits on lines the read/search
|
||||
* that minted the tag never displayed. Defaults to `true`. When `false`, tags
|
||||
* validate on content hash alone and any anchor into the tagged content applies.
|
||||
*/
|
||||
enforceSeenLines?: boolean;
|
||||
}
|
||||
|
||||
/** Per-section result returned by {@link Patcher.apply} / {@link Patcher.commit}. */
|
||||
@@ -197,6 +203,7 @@ export class Patcher {
|
||||
readonly snapshots: SnapshotStore;
|
||||
readonly recovery: Recovery;
|
||||
readonly blockResolver: BlockResolver | undefined;
|
||||
readonly #enforceSeenLines: boolean;
|
||||
|
||||
constructor(options: PatcherOptions) {
|
||||
if (!options.snapshots) {
|
||||
@@ -206,6 +213,7 @@ export class Patcher {
|
||||
this.snapshots = options.snapshots;
|
||||
this.recovery = new Recovery(options.snapshots);
|
||||
this.blockResolver = options.blockResolver;
|
||||
this.#enforceSeenLines = options.enforceSeenLines ?? true;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -628,7 +636,9 @@ export class Patcher {
|
||||
// The line numbers in `edits` index the exact content the tag names.
|
||||
// Reject any anchor the read never displayed: editing lines the model
|
||||
// has not seen is the off-by-memory mistake that mangles files.
|
||||
if (expected !== undefined) this.#assertSeenLines(section, expected, matchedSnapshot);
|
||||
if (expected !== undefined && this.#enforceSeenLines) {
|
||||
this.#assertSeenLines(section, expected, matchedSnapshot);
|
||||
}
|
||||
const result = applyEdits(normalized, resolved);
|
||||
return withResolveWarnings(blockResolutions.length > 0 ? { ...result, blockResolutions } : result);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user