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); diff --git a/src/tests.zig b/src/tests.zig index 5b911da..9c06893 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -9443,3 +9443,125 @@ 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()); + } +} + +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]); + } + } +} + +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); +}