diff --git a/src/hooks/hashline-read-enhancer/hook.ts b/src/hooks/hashline-read-enhancer/hook.ts index b6f4db30..652c000f 100644 --- a/src/hooks/hashline-read-enhancer/hook.ts +++ b/src/hooks/hashline-read-enhancer/hook.ts @@ -1,6 +1,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import { computeLineHash } from "../../tools/hashline-edit/hash-computation" -import { toHashlineContent } from "../../tools/hashline-edit/diff-utils" + +const WRITE_SUCCESS_MARKER = "File written successfully." interface HashlineReadEnhancerConfig { hashline_edit?: { enabled: boolean } @@ -12,6 +13,7 @@ const CONTENT_OPEN_TAG = "" const CONTENT_CLOSE_TAG = "" const FILE_OPEN_TAG = "" const FILE_CLOSE_TAG = "" +const OPENCODE_LINE_TRUNCATION_SUFFIX = "... (line truncated to 2000 chars)" function isReadTool(toolName: string): boolean { return toolName.toLowerCase() === "read" @@ -55,6 +57,9 @@ function transformLine(line: string): string { if (!parsed) { return line } + if (parsed.content.endsWith(OPENCODE_LINE_TRUNCATION_SUFFIX)) { + return line + } const hash = computeLineHash(parsed.lineNumber, parsed.content) return `${parsed.lineNumber}#${hash}|${parsed.content}` } @@ -137,7 +142,12 @@ function extractFilePath(metadata: unknown): string | undefined { } async function appendWriteHashlineOutput(output: { output: string; metadata: unknown }): Promise { - if (output.output.includes("Updated file (LINE#ID|content):")) { + if (output.output.startsWith(WRITE_SUCCESS_MARKER)) { + return + } + + const outputLower = output.output.toLowerCase() + if (outputLower.startsWith("error") || outputLower.includes("failed")) { return } @@ -152,8 +162,8 @@ async function appendWriteHashlineOutput(output: { output: string; metadata: unk } const content = await file.text() - const hashlined = toHashlineContent(content) - output.output = `${output.output}\n\nUpdated file (LINE#ID|content):\n${hashlined}` + const lineCount = content === "" ? 0 : content.split("\n").length + output.output = `${WRITE_SUCCESS_MARKER} ${lineCount} lines written.` } export function createHashlineReadEnhancerHook( diff --git a/src/hooks/hashline-read-enhancer/index.test.ts b/src/hooks/hashline-read-enhancer/index.test.ts index 0f41874b..dcab65bc 100644 --- a/src/hooks/hashline-read-enhancer/index.test.ts +++ b/src/hooks/hashline-read-enhancer/index.test.ts @@ -84,6 +84,33 @@ describe("hashline-read-enhancer", () => { expect(lines[7]).toBe("") }) + it("keeps OpenCode-truncated lines unhashed while hashifying normal lines", async () => { + //#given + const hook = createHashlineReadEnhancerHook(mockCtx(), { hashline_edit: { enabled: true } }) + const input = { tool: "read", sessionID: "s", callID: "c" } + const truncatedLine = `${"x".repeat(60)}... (line truncated to 2000 chars)` + const output = { + title: "demo.ts", + output: [ + "/tmp/demo.ts", + "file", + "", + `1: ${truncatedLine}`, + "2: normal line", + "", + ].join("\n"), + metadata: {}, + } + + //#when + await hook["tool.execute.after"](input, output) + + //#then + const lines = output.output.split("\n") + expect(lines[3]).toBe(`1: ${truncatedLine}`) + expect(lines[4]).toMatch(/^2#[ZPMQVRWSNKTXJBYH]{2}\|normal line$/) + }) + it("hashifies plain read output without content tags", async () => { //#given const hook = createHashlineReadEnhancerHook(mockCtx(), { hashline_edit: { enabled: true } }) @@ -164,7 +191,7 @@ describe("hashline-read-enhancer", () => { expect(lines[2]).toMatch(/^2#[ZPMQVRWSNKTXJBYH]{2}\|const y = 2$/) }) - it("appends LINE#ID output for write tool using metadata filepath", async () => { + it("appends simple summary for write tool instead of full hashlined content", async () => { //#given const hook = createHashlineReadEnhancerHook(mockCtx(), { hashline_edit: { enabled: true } }) const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "hashline-write-")) @@ -181,9 +208,55 @@ describe("hashline-read-enhancer", () => { await hook["tool.execute.after"](input, output) //#then - expect(output.output).toContain("Updated file (LINE#ID|content):") - expect(output.output).toMatch(/1#[ZPMQVRWSNKTXJBYH]{2}\|const x = 1/) - expect(output.output).toMatch(/2#[ZPMQVRWSNKTXJBYH]{2}\|const y = 2/) + expect(output.output).toContain("File written successfully.") + expect(output.output).toContain("2 lines written.") + expect(output.output).not.toContain("Updated file (LINE#ID|content):") + expect(output.output).not.toContain("const x = 1") + + fs.rmSync(tempDir, { recursive: true, force: true }) + }) + + it("does not re-process write output that already contains the success marker", async () => { + //#given + const hook = createHashlineReadEnhancerHook(mockCtx(), { hashline_edit: { enabled: true } }) + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "hashline-idem-")) + const filePath = path.join(tempDir, "demo.ts") + fs.writeFileSync(filePath, "a\nb\nc\nd\ne") + const input = { tool: "write", sessionID: "s", callID: "c" } + const output = { + title: "write", + output: "File written successfully. 99 lines written.", + metadata: { filepath: filePath }, + } + + //#when + await hook["tool.execute.after"](input, output) + + //#then — guard should prevent re-reading the file and updating the count + expect(output.output).toBe("File written successfully. 99 lines written.") + + fs.rmSync(tempDir, { recursive: true, force: true }) + }) + + it("does not overwrite write tool error output with success message", async () => { + //#given — write tool failed, but stale file exists from previous write + const hook = createHashlineReadEnhancerHook(mockCtx(), { hashline_edit: { enabled: true } }) + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "hashline-err-")) + const filePath = path.join(tempDir, "demo.ts") + fs.writeFileSync(filePath, "const x = 1") + const input = { tool: "write", sessionID: "s", callID: "c" } + const output = { + title: "write", + output: "Error: EACCES: permission denied, open '" + filePath + "'", + metadata: { filepath: filePath }, + } + + //#when + await hook["tool.execute.after"](input, output) + + //#then — error output must be preserved, not overwritten with success message + expect(output.output).toContain("Error: EACCES") + expect(output.output).not.toContain("File written successfully.") fs.rmSync(tempDir, { recursive: true, force: true }) }) diff --git a/src/tools/hashline-edit/edit-operations.test.ts b/src/tools/hashline-edit/edit-operations.test.ts index 1940b9e4..5d8ad08b 100644 --- a/src/tools/hashline-edit/edit-operations.test.ts +++ b/src/tools/hashline-edit/edit-operations.test.ts @@ -92,6 +92,22 @@ describe("hashline edit operations", () => { expect(result).toEqual("line 1\ninserted\nline 2\nmodified") }) + it("applies replace before prepend when both target same line", () => { + //#given + const content = "line 1\nline 2\nline 3" + const lines = content.split("\n") + const edits: HashlineEdit[] = [ + { op: "prepend", pos: anchorFor(lines, 2), lines: "before line 2" }, + { op: "replace", pos: anchorFor(lines, 2), lines: "modified line 2" }, + ] + + //#when + const result = applyHashlineEdits(content, edits) + + //#then + expect(result).toEqual("line 1\nbefore line 2\nmodified line 2\nline 3") + }) + it("deduplicates identical insert edits in one pass", () => { //#given const content = "line 1\nline 2" diff --git a/src/tools/hashline-edit/edit-operations.ts b/src/tools/hashline-edit/edit-operations.ts index fae662d1..caec2307 100644 --- a/src/tools/hashline-edit/edit-operations.ts +++ b/src/tools/hashline-edit/edit-operations.ts @@ -27,7 +27,13 @@ export function applyHashlineEditsWithReport(content: string, edits: HashlineEdi } const dedupeResult = dedupeEdits(edits) - const sortedEdits = [...dedupeResult.edits].sort((a, b) => getEditLineNumber(b) - getEditLineNumber(a)) + const EDIT_PRECEDENCE: Record = { replace: 0, append: 1, prepend: 2 } + const sortedEdits = [...dedupeResult.edits].sort((a, b) => { + const lineA = getEditLineNumber(a) + const lineB = getEditLineNumber(b) + if (lineB !== lineA) return lineB - lineA + return (EDIT_PRECEDENCE[a.op] ?? 3) - (EDIT_PRECEDENCE[b.op] ?? 3) + }) let noopEdits = 0 @@ -87,4 +93,3 @@ export function applyHashlineEditsWithReport(content: string, edits: HashlineEdi export function applyHashlineEdits(content: string, edits: HashlineEdit[]): string { return applyHashlineEditsWithReport(content, edits).content } - diff --git a/src/tools/hashline-edit/hashline-edit-executor.ts b/src/tools/hashline-edit/hashline-edit-executor.ts index bbbba4ac..e20ebbf9 100644 --- a/src/tools/hashline-edit/hashline-edit-executor.ts +++ b/src/tools/hashline-edit/hashline-edit-executor.ts @@ -5,6 +5,7 @@ import { countLineDiffs, generateUnifiedDiff } from "./diff-utils" import { canonicalizeFileText, restoreFileText } from "./file-text-canonicalization" import { normalizeHashlineEdits, type RawHashlineEdit } from "./normalize-edits" import type { HashlineEdit } from "./types" +import { HashlineMismatchError } from "./validation" interface HashlineEditArgs { filePath: string @@ -158,7 +159,7 @@ export async function executeHashlineEditTool(args: HashlineEditArgs, context: T return `Updated ${effectivePath}` } catch (error) { const message = error instanceof Error ? error.message : String(error) - if (message.toLowerCase().includes("hash")) { + if (error instanceof HashlineMismatchError) { return `Error: hash mismatch - ${message}\nTip: reuse LINE#ID entries from the latest read/edit output, or batch related edits in one call.` } return `Error: ${message}` diff --git a/src/tools/hashline-edit/tools.test.ts b/src/tools/hashline-edit/tools.test.ts index 46f663a3..cb76b834 100644 --- a/src/tools/hashline-edit/tools.test.ts +++ b/src/tools/hashline-edit/tools.test.ts @@ -103,6 +103,25 @@ describe("createHashlineEditTool", () => { expect(result).toContain(">>>") }) + it("does not classify invalid pos format as hash mismatch", async () => { + //#given + const filePath = path.join(tempDir, "invalid-format.txt") + fs.writeFileSync(filePath, "line1\nline2") + + //#when + const result = await tool.execute( + { + filePath, + edits: [{ op: "replace", pos: "42", lines: "updated" }], + }, + createMockContext(), + ) + + //#then + expect(result).toContain("Error") + expect(result.toLowerCase()).not.toContain("hash mismatch") + }) + it("preserves literal backslash-n and supports string[] payload", async () => { //#given const filePath = path.join(tempDir, "test.txt") diff --git a/src/tools/hashline-edit/validation.test.ts b/src/tools/hashline-edit/validation.test.ts index cf2457ab..751a90b9 100644 --- a/src/tools/hashline-edit/validation.test.ts +++ b/src/tools/hashline-edit/validation.test.ts @@ -19,7 +19,49 @@ describe("parseLineRef", () => { const ref = "42:VK" //#when / #then - expect(() => parseLineRef(ref)).toThrow("LINE#ID") + expect(() => parseLineRef(ref)).toThrow("{line_number}#{hash_id}") + }) + + it("gives specific hint when literal text is used instead of line number", () => { + //#given — model sends "LINE#HK" instead of "1#HK" + const ref = "LINE#HK" + + //#when / #then — error should mention that LINE is not a valid number + expect(() => parseLineRef(ref)).toThrow(/not a line number/i) + }) + + it("gives specific hint for other non-numeric prefixes like POS#VK", () => { + //#given + const ref = "POS#VK" + + //#when / #then + expect(() => parseLineRef(ref)).toThrow(/not a line number/i) + }) + + it("extracts valid line number from mixed prefix like LINE42 without throwing", () => { + //#given — normalizeLineRef extracts 42#VK from LINE42#VK + const ref = "LINE42#VK" + + //#when / #then — should parse successfully as line 42 + const result = parseLineRef(ref) + expect(result.line).toBe(42) + expect(result.hash).toBe("VK") + }) + + it("gives specific hint when hyphenated prefix like line-ref is used", () => { + //#given + const ref = "line-ref#VK" + + //#when / #then + expect(() => parseLineRef(ref)).toThrow(/not a line number/i) + }) + + it("gives specific hint when prefix contains a period like line.ref", () => { + //#given + const ref = "line.ref#VK" + + //#when / #then + expect(() => parseLineRef(ref)).toThrow(/not a line number/i) }) it("accepts refs copied with markers and trailing content", () => { @@ -32,6 +74,28 @@ describe("parseLineRef", () => { //#then expect(result).toEqual({ line: 42, hash: "VK" }) }) + + it("accepts refs copied with >>> marker only", () => { + //#given + const ref = ">>> 42#VK" + + //#when + const result = parseLineRef(ref) + + //#then + expect(result).toEqual({ line: 42, hash: "VK" }) + }) + + it("accepts refs with spaces around hash separator", () => { + //#given + const ref = "42 # VK" + + //#when + const result = parseLineRef(ref) + + //#then + expect(result).toEqual({ line: 42, hash: "VK" }) + }) }) describe("validateLineRef", () => { @@ -60,4 +124,13 @@ describe("validateLineRef", () => { expect(() => validateLineRefs(lines, ["2#ZZ"])) .toThrow(/>>>\s+2#[ZPMQVRWSNKTXJBYH]{2}\|two/) }) + + it("suggests correct line number when hash matches a file line", () => { + //#given — model sends LINE#XX where XX is the actual hash for line 1 + const lines = ["function hello() {", " return 42", "}"] + const hash = computeLineHash(1, lines[0]) + + //#when / #then — error should suggest the correct reference + expect(() => validateLineRefs(lines, [`LINE#${hash}`])).toThrow(new RegExp(`1#${hash}`)) + }) }) diff --git a/src/tools/hashline-edit/validation.ts b/src/tools/hashline-edit/validation.ts index 72a33286..fc5b395a 100644 --- a/src/tools/hashline-edit/validation.ts +++ b/src/tools/hashline-edit/validation.ts @@ -16,7 +16,13 @@ const MISMATCH_CONTEXT = 2 const LINE_REF_EXTRACT_PATTERN = /([0-9]+#[ZPMQVRWSNKTXJBYH]{2})/ function normalizeLineRef(ref: string): string { - const trimmed = ref.trim() + const originalTrimmed = ref.trim() + let trimmed = originalTrimmed + trimmed = trimmed.replace(/^(?:>>>|[+-])\s*/, "") + trimmed = trimmed.replace(/\s*#\s*/, "#") + trimmed = trimmed.replace(/\|.*$/, "") + trimmed = trimmed.trim() + if (HASHLINE_REF_PATTERN.test(trimmed)) { return trimmed } @@ -26,7 +32,7 @@ function normalizeLineRef(ref: string): string { return extracted[1] } - return trimmed + return originalTrimmed } export function parseLineRef(ref: string): LineRef { @@ -38,13 +44,25 @@ export function parseLineRef(ref: string): LineRef { hash: match[2], } } + // normalized equals ref.trim() in all error paths — extraction only succeeds for valid refs + const hashIdx = normalized.indexOf('#') + if (hashIdx > 0) { + const prefix = normalized.slice(0, hashIdx) + const suffix = normalized.slice(hashIdx + 1) + if (!/^\d+$/.test(prefix) && /^[ZPMQVRWSNKTXJBYH]{2}$/.test(suffix)) { + throw new Error( + `Invalid line reference: "${ref}". "${prefix}" is not a line number. ` + + `Use the actual line number from the read output.` + ) + } + } throw new Error( - `Invalid line reference format: "${ref}". Expected format: "LINE#ID" (e.g., "42#VK")` + `Invalid line reference format: "${ref}". Expected format: "{line_number}#{hash_id}"` ) } export function validateLineRef(lines: string[], ref: string): void { - const { line, hash } = parseLineRef(ref) + const { line, hash } = parseLineRefWithHint(ref, lines) if (line < 1 || line > lines.length) { throw new Error( @@ -92,7 +110,7 @@ export class HashlineMismatchError extends Error { const output: string[] = [] output.push( `${mismatches.length} line${mismatches.length > 1 ? "s have" : " has"} changed since last read. ` + - "Use updated LINE#ID references below (>>> marks changed lines)." + "Use updated {line_number}#{hash_id} references below (>>> marks changed lines)." ) output.push("") @@ -117,11 +135,34 @@ export class HashlineMismatchError extends Error { } } +function suggestLineForHash(ref: string, lines: string[]): string | null { + const hashMatch = ref.trim().match(/#([ZPMQVRWSNKTXJBYH]{2})$/) + if (!hashMatch) return null + const hash = hashMatch[1] + for (let i = 0; i < lines.length; i++) { + if (computeLineHash(i + 1, lines[i]) === hash) { + return `Did you mean "${i + 1}#${hash}"?` + } + } + return null +} +function parseLineRefWithHint(ref: string, lines: string[]): LineRef { + try { + return parseLineRef(ref) + } catch (parseError) { + const hint = suggestLineForHash(ref, lines) + if (hint && parseError instanceof Error) { + throw new Error(`${parseError.message} ${hint}`) + } + throw parseError + } +} + export function validateLineRefs(lines: string[], refs: string[]): void { const mismatches: HashMismatch[] = [] for (const ref of refs) { - const { line, hash } = parseLineRef(ref) + const { line, hash } = parseLineRefWithHint(ref, lines) if (line < 1 || line > lines.length) { throw new Error(`Line number ${line} out of bounds (file has ${lines.length} lines)`)