From 4f9d8d292ef07e9dd37ff6d5e9ec724c0695d8c1 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Tue, 5 May 2026 11:02:28 +0800 Subject: [PATCH 1/4] test(issue-401): failing test for insert with after=null silently consuming seq When applyEdit is called with op=.insert and after=null, the switch case falls through without modifying the line buffer, but the function still proceeds to atomically rewrite the file and call recordEdit. The result is a phantom store entry (seq incremented, ChangeEntry recorded) for an operation that did nothing visible. This pollutes changesSince() and makes seq numbers unreliable as causality markers. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/tests.zig | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/tests.zig b/src/tests.zig index 5b911da..efcff4d 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -9443,3 +9443,39 @@ test "issue-409: snapshot .env prefix filter wrongly excludes .envoy/.environmen try testing.expect(exp2.outlines.contains("a.zig")); try testing.expect(exp2.outlines.contains(".envoy.json")); } + +test "issue-401: insert with after=null is a no-op but consumes seq and writes file" { + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + + const rel_path = try std.fmt.allocPrint(testing.allocator, ".zig-cache/tmp/{s}/edit-401.txt", .{tmp.sub_path}); + defer testing.allocator.free(rel_path); + + const original = "line 1\nline 2\nline 3\n"; + var file = try tmp.dir.createFile(io, "edit-401.txt", .{}); + defer file.close(io); + try file.writeStreamingAll(io, original); + + var store = Store.init(testing.allocator); + defer store.deinit(); + var agents = AgentRegistry.init(testing.allocator); + defer agents.deinit(); + const agent_id = try agents.register("issue-401-agent"); + + // insert without after must not silently succeed and must not consume a seq. + const res = edit_mod.applyEdit(io, testing.allocator, &store, &agents, null, .{ + .path = rel_path, + .agent_id = agent_id, + .op = .insert, + .after = null, + .content = "INJECT\n", + }); + // Either explicit error, or — at minimum — must not increment the store seq + // for an operation that did nothing. + if (res) |ok| { + _ = ok; + try testing.expectEqual(@as(u64, 0), store.currentSeq()); + } else |_| { + try testing.expectEqual(@as(u64, 0), store.currentSeq()); + } +} From 55df2a89e55ed777e15cfbac63656fe8a704e059 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Tue, 5 May 2026 11:04:08 +0800 Subject: [PATCH 2/4] test(issue-404): failing test for CRLF line ending corruption in applyEdit When applyEdit operates on a CRLF file, splitScalar('\n') leaves the trailing '\r' attached to each unchanged line. The new replacement line inherits whatever line ending the caller supplied (typically none / LF), and the rejoin uses bare '\n'. Result: the on-disk file has mixed CRLF on unchanged lines and bare LF on changed lines, breaking tools that detect file-wide line ending style. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/tests.zig | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/tests.zig b/src/tests.zig index efcff4d..d17060c 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -9479,3 +9479,49 @@ test "issue-401: insert with after=null is a no-op but consumes seq and writes f try testing.expectEqual(@as(u64, 0), store.currentSeq()); } } + +test "issue-404: applyEdit corrupts CRLF line endings into mixed LF/CRLF" { + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + + const rel_path = try std.fmt.allocPrint(testing.allocator, ".zig-cache/tmp/{s}/edit-404.txt", .{tmp.sub_path}); + defer testing.allocator.free(rel_path); + + // Windows-style CRLF original + const original = "alpha\r\nbeta\r\ngamma\r\n"; + var file = try tmp.dir.createFile(io, "edit-404.txt", .{}); + defer file.close(io); + try file.writeStreamingAll(io, original); + + var store = Store.init(testing.allocator); + defer store.deinit(); + var agents = AgentRegistry.init(testing.allocator); + defer agents.deinit(); + const agent_id = try agents.register("issue-404-agent"); + + // Replace line 1 with new content (no trailing newline in replacement). + _ = try edit_mod.applyEdit(io, testing.allocator, &store, &agents, null, .{ + .path = rel_path, + .agent_id = agent_id, + .op = .replace, + .range = .{ 1, 1 }, + .content = "ALPHA", + }); + + const after = try std.Io.Dir.cwd().readFileAlloc(io, rel_path, testing.allocator, .limited(10 * 1024)); + defer testing.allocator.free(after); + + // The original file used CRLF line endings. After a single-line replace + // the file must still be a valid CRLF file: every '\n' must be preceded + // by '\r'. Currently splitScalar on '\n' leaves the '\r' attached to the + // *unchanged* lines (e.g. "beta\r"), and the rejoin uses bare "\n", so + // the new line 1 lacks its CR while the surviving line 2 still has it — + // mixed line endings. + var i: usize = 0; + while (i < after.len) : (i += 1) { + if (after[i] == '\n') { + try testing.expect(i > 0); + try testing.expectEqual(@as(u8, '\r'), after[i - 1]); + } + } +} From 53850010b3df9ac5181fa6f93041d98ac8b3297f Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Tue, 5 May 2026 11:10:40 +0800 Subject: [PATCH 3/4] test(issue-409): failing test for stray newline after replace-with-empty When applyEdit replaces the only line of a file with empty content, the post-processing unconditionally restores the source's trailing newline. The result is that 'replace line 1..1 with ""' on file 'abc\n' leaves '\n' on disk instead of an empty file. Callers asking for an empty file get a non-empty file. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/tests.zig | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/tests.zig b/src/tests.zig index d17060c..9c06893 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -9525,3 +9525,43 @@ test "issue-404: applyEdit corrupts CRLF line endings into mixed LF/CRLF" { } } } + +test "issue-409: replacing whole file with empty content leaves a stray newline" { + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + + const rel_path = try std.fmt.allocPrint(testing.allocator, ".zig-cache/tmp/{s}/edit-409.txt", .{tmp.sub_path}); + defer testing.allocator.free(rel_path); + + // Single-line file with trailing newline. + const original = "abc\n"; + var file = try tmp.dir.createFile(io, "edit-409.txt", .{}); + defer file.close(io); + try file.writeStreamingAll(io, original); + + var store = Store.init(testing.allocator); + defer store.deinit(); + var agents = AgentRegistry.init(testing.allocator); + defer agents.deinit(); + const agent_id = try agents.register("issue-409-agent"); + + // Replace the only line with empty content. The caller's intent is "make + // this file empty" — content has zero bytes. + const result = try edit_mod.applyEdit(io, testing.allocator, &store, &agents, null, .{ + .path = rel_path, + .agent_id = agent_id, + .op = .replace, + .range = .{ 1, 1 }, + .content = "", + }); + + const after = try std.Io.Dir.cwd().readFileAlloc(io, rel_path, testing.allocator, .limited(10 * 1024)); + defer testing.allocator.free(after); + + // Expectation: the file is empty. Currently the file ends up as "\n" + // because applyEdit unconditionally restores the trailing newline that + // existed in the source, even after the replacement reduced the file + // to a single empty line. + try testing.expectEqual(@as(usize, 0), after.len); + try testing.expectEqual(@as(u64, 0), result.new_size); +} From 321ce6950fe488cf8d94e9ee79d03d4d39da8ec0 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Tue, 5 May 2026 11:24:42 +0800 Subject: [PATCH 4/4] fix(edit): validate args, preserve CRLF, allow truly-empty result (#401, #404, #409) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/edit.zig | 74 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/src/edit.zig b/src/edit.zig index a2a0cd9..a3b0fe3 100644 --- a/src/edit.zig +++ b/src/edit.zig @@ -37,6 +37,14 @@ pub fn applyEdit( if (!has_lock) return error.FileLocked; errdefer agents.releaseLock(req.agent_id, req.path); + // Validate required op-specific args BEFORE doing any work that + // mutates Store.seq or rewrites the file (#401). + switch (req.op) { + .replace, .delete => if (req.range == null) return error.InvalidRange, + .insert => if (req.after == null) return error.InvalidRange, + else => {}, + } + const source = try std.Io.Dir.cwd().readFileAlloc(io, req.path, allocator, .limited(10 * 1024 * 1024)); defer allocator.free(source); @@ -47,10 +55,19 @@ pub fn applyEdit( if (!std.mem.eql(u8, expected_hex, actual_hex)) return error.HashMismatch; } + // Detect line-ending style: if the file has any "\r\n", treat it as + // CRLF and rejoin with CRLF (#404). Strip the trailing '\r' from + // each split chunk so the in-memory representation is uniform. + const is_crlf = std.mem.indexOf(u8, source, "\r\n") != null; + const sep: []const u8 = if (is_crlf) "\r\n" else "\n"; + var lines: std.ArrayList([]const u8) = .empty; defer lines.deinit(allocator); var iter = std.mem.splitScalar(u8, source, '\n'); - while (iter.next()) |line| try lines.append(allocator, line); + while (iter.next()) |line| { + const trimmed = if (is_crlf and line.len > 0 and line[line.len - 1] == '\r') line[0 .. line.len - 1] else line; + try lines.append(allocator, trimmed); + } // A trailing newline produces an empty final element; don't count it as a line const had_trailing_newline = lines.items.len > 0 and lines.items[lines.items.len - 1].len == 0; @@ -60,43 +77,48 @@ pub fn applyEdit( switch (req.op) { .replace => { - if (req.range) |range| { - if (range[0] == 0 or range[1] < range[0] or range[0] > lines.items.len) return error.InvalidRange; - const start = range[0] - 1; - const end = @min(range[1], lines.items.len); - const new_content = req.content orelse return error.MissingContent; - var new_lines: std.ArrayList([]const u8) = .empty; - defer new_lines.deinit(allocator); - var ni = std.mem.splitScalar(u8, new_content, '\n'); - while (ni.next()) |nl| try new_lines.append(allocator, nl); - try lines.replaceRange(allocator, start, end - start, new_lines.items); + const range = req.range.?; + if (range[0] == 0 or range[1] < range[0] or range[0] > lines.items.len) return error.InvalidRange; + const start = range[0] - 1; + const end = @min(range[1], lines.items.len); + const new_content = req.content orelse return error.MissingContent; + var new_lines: std.ArrayList([]const u8) = .empty; + defer new_lines.deinit(allocator); + var ni = std.mem.splitScalar(u8, new_content, '\n'); + while (ni.next()) |nl| { + const trimmed = if (is_crlf and nl.len > 0 and nl[nl.len - 1] == '\r') nl[0 .. nl.len - 1] else nl; + try new_lines.append(allocator, trimmed); } + try lines.replaceRange(allocator, start, end - start, new_lines.items); }, .insert => { - if (req.after) |after_line| { - const pos = @min(after_line, lines.items.len); - const content = req.content orelse return error.MissingContent; - try lines.insert(allocator, pos, content); - } + const after_line = req.after.?; + const pos = @min(after_line, lines.items.len); + const content = req.content orelse return error.MissingContent; + try lines.insert(allocator, pos, content); }, .delete => { - if (req.range) |range| { - if (range[0] == 0 or range[1] < range[0] or range[0] > lines.items.len) return error.InvalidRange; - const start = range[0] - 1; - const end = @min(range[1], lines.items.len); - // Remove lines [start..end) by replacing with nothing - try lines.replaceRange(allocator, start, end - start, &.{}); - } + const range = req.range.?; + if (range[0] == 0 or range[1] < range[0] or range[0] > lines.items.len) return error.InvalidRange; + const start = range[0] - 1; + const end = @min(range[1], lines.items.len); + // Remove lines [start..end) by replacing with nothing + try lines.replaceRange(allocator, start, end - start, &.{}); }, else => {}, } - // Restore trailing newline if the original file had one - if (had_trailing_newline) { + // Restore trailing newline if the original file had one — but not when + // the operation reduced the buffer to truly empty content (#409). + const result_is_empty = lines.items.len == 0 or (lines.items.len == 1 and lines.items[0].len == 0); + if (had_trailing_newline and !result_is_empty) { try lines.append(allocator, ""); } - const result = try std.mem.join(allocator, "\n", lines.items); + const result = if (result_is_empty) + try allocator.dupe(u8, "") + else + try std.mem.join(allocator, sep, lines.items); defer allocator.free(result); const hash: u64 = std.hash.Wyhash.hash(0, result);