diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index dd1a5bdde..b9aa77227 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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)). diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 3f350ea70..2999c127f 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -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, diff --git a/packages/coding-agent/src/edit/file-snapshot-store.ts b/packages/coding-agent/src/edit/file-snapshot-store.ts index 80c3c6952..ec2f410a0 100644 --- a/packages/coding-agent/src/edit/file-snapshot-store.ts +++ b/packages/coding-agent/src/edit/file-snapshot-store.ts @@ -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, ): 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)); } diff --git a/packages/coding-agent/src/edit/hashline/execute.ts b/packages/coding-agent/src/edit/hashline/execute.ts index d2236e1a3..fc3d4fc78 100644 --- a/packages/coding-agent/src/edit/hashline/execute.ts +++ b/packages/coding-agent/src/edit/hashline/execute.ts @@ -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); 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 75167ae16..bc317dcd6 100644 --- a/packages/coding-agent/test/edit/seen-line-guard.test.ts +++ b/packages/coding-agent/test/edit/seen-line-guard.test.ts @@ -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 `…` 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 `…` 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"); + }); +}); diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 151d36fad..56cfe9c98 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -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 diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index b49156135..3b49e63a9 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -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); }