A Save whose write fails changes nothing: not the name, not the dirty flag

A scratch took its new name before the host wrote it, so `Save /root/x.txt`
failing with EACCES renamed it anyway; a failed write to another name
marked a clean file dirty. A scratch is now named once its write is done
(promoteSaved), and a failed write puts the saved revision back.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

4 files changed, 101 insertions, 29 deletions

diff --git a/docs/fs.md b/docs/fs.md
index 6b393918..cfcb62a3 100644
--- a/docs/fs.md
+++ b/docs/fs.md
@@ -266,8 +266,10 @@ argument "Save"` (EINVAL), for a builtin that would have asked at a prompt
 lines before a failing one have taken effect and those after it never run,
 which is what acme's ctl loop does (editors/acme/xfid.c:600-790). An error
 that only happens as the editor performs what a line asked for -- a `Save`
-whose disk write fails -- is reported in the editor and /log, not in the
-write's answer. Like any write, a ctl write answers once the editor has
+whose disk write fails -- fails the write too, once the editor has tried
+(`Save /root/x.txt: access denied`, EIO), and changes nothing: a scratch
+keeps its name and stays a scratch, a clean file stays clean, a dirty one
+dirty. Like any write, a ctl write answers once the editor has
 performed what it asked for (a save written, a shell started). A click on
 the same word, or the word written to `exec`, still opens its prompt.
 
diff --git a/src/exec.zig b/src/exec.zig
index 2b0c90cb..0bce67d2 100644
--- a/src/exec.zig
+++ b/src/exec.zig
@@ -301,17 +301,15 @@ pub fn saveTo(p: *Pardes, id: usize, path: []const u8) void {
         if (pane.isTerminal()) askWrite(p, id, pane.serial, full);
         return;
     };
+    // A scratch takes the name once the write is done (promoteSaved): a
+    // Save that fails changes nothing, the name included.
     if (f.output != null and panes.Output.fileTraits(f.output).saves) {
-        const owned = p.gpa.dupe(u8, full) catch return;
-        p.gpa.free(f.path);
-        f.path = owned;
-        f.output = null; // an ordinary file pane from here on
-        f.watch_after_save = true;
-        pane.clearCwd();
-        // re-derive the tag as a plain file's
-        if (pane.tag.own) |own| p.gpa.free(own);
-        pane.tag.own = null;
-        p.emit(.{ .save_file = .{ .pane = @intCast(id) } });
+        p.emit(.{ .save_text = .{
+            .pane = @intCast(id),
+            .serial = pane.serial,
+            .path = Pardes.SavePath.from(full),
+            .promote = true,
+        } });
         return;
     }
     // its own path, spelled out: the in-place write, so the pane comes clean
diff --git a/src/pardes.zig b/src/pardes.zig
index 9ba8a2d3..002ff6f9 100644
--- a/src/pardes.zig
+++ b/src/pardes.zig
@@ -2321,6 +2321,9 @@ test "owned cwd Save promotion releases the former directory" {
     const pane = p.panes[id].?;
     try pane.setOwnedCwd("/old/directory");
     exec.saveTo(p, id, "saved.txt");
+    // Named once the write is done (a failed one changes nothing).
+    try std.testing.expect(pane.file.?.output != null);
+    while (p.nextEffect()) |effect| p.perform(effect);
     try std.testing.expect(pane.cwd == .none);
     try std.testing.expect(pane.file.?.output == null);
     try std.testing.expectEqualStrings("/old/directory/saved.txt", pane.file.?.path);
@@ -2348,16 +2351,16 @@ test "Save on a scratch asks for a path in its inherited dir and makes it a file
     // typing the filename and submitting converts it into an ordinary file
     edit.insertKey(p, &np.input, .{ .cp = 'n', .text = "note.txt" });
     exec.submitSave(p, id);
-    try std.testing.expect(np.file.?.output == null);
-    try std.testing.expectEqualStrings("/tmp/pardes-save-dir/note.txt", np.file.?.path);
+    // The write asked for, the pane is the file once it is done.
     var saved = false;
-    while (p.nextEffect()) |effect| switch (effect) {
-        .save_file => |sf| if (@as(usize, sf.pane) == id) {
-            saved = true;
-        },
-        else => {},
-    };
+    while (p.nextEffect()) |effect| {
+        if (effect == .save_text and @as(usize, effect.save_text.pane) == id) saved = true;
+        p.perform(effect);
+    }
     try std.testing.expect(saved);
+    try std.testing.expect(np.file.?.output == null);
+    try std.testing.expectEqualStrings("/tmp/pardes-save-dir/note.txt", np.file.?.path);
+    try std.testing.expectEqual(np.file.?.revision, np.file.?.saved_revision);
 }
 
 test "Save on a terminal writes its plaintext scrollback and stays a terminal" {
@@ -3640,7 +3643,7 @@ pub const Effect = union(enum) {
     /// write this pane's file content to its path; the shell reads both off
     /// the core (content is unbounded, effects are fixed-size values)
     save_file: struct { pane: u8 },
-    save_text: struct { pane: u8, serial: u32, path: Buf(effect_path_cap) },
+    save_text: struct { pane: u8, serial: u32, path: Buf(effect_path_cap), promote: bool = false },
     write_dump,
     set_clipboard,
     read_clipboard,
@@ -5207,6 +5210,26 @@ pub const Pardes = struct {
     /// A write of `path` the host could not do: the pane stays dirty, the
     /// message row says `Save <path>: <why>`, and the 9P write that asked
     /// for it, waiting on the save, fails with that (`fs.late_failure`).
+    /// A scratch written to `path` is that file from here on: named, an
+    /// ordinary file pane, clean, and watched (exec.saveTo).
+    fn promoteSaved(p: *Pardes, id: u8, path: []const u8) void {
+        const pane = p.panes[id] orelse return;
+        const f = if (pane.file) |*file| file else return;
+        const owned = p.gpa.dupe(u8, path) catch return;
+        p.gpa.free(f.path);
+        f.path = owned;
+        f.output = null;
+        pane.clearCwd();
+        // re-derive the tag as a plain file's
+        if (pane.tag.own) |own| p.gpa.free(own);
+        pane.tag.own = null;
+        f.saved_revision = f.revision;
+        f.disk_gone = false;
+        ctlfs.events.noteLog(p, .save, pane);
+        if (filesystem.localPath(f.path) != null)
+            p.emit(.{ .watch = .{ .pane = id, .on = true } });
+    }
+
     pub fn saveFailed(p: *Pardes, id: u8, path: []const u8, err: anyerror) void {
         if (p.panes[id]) |pane| if (pane.file) |*f| {
             // The `-%` spelling fs.zig already uses for "make this dirty".
@@ -5280,13 +5303,21 @@ pub const Pardes = struct {
                 const pane = p.panes[sf.pane] orelse return;
                 const f = if (pane.file) |*file| file else return;
                 const serial = pane.serial;
+                const was = f.saved_revision;
+                const failures = p.fs.failures;
                 f.saved_revision = f.revision;
                 p.hostWriteFile(sf.pane, f.path, f.content);
                 const saved_pane = p.panes[sf.pane] orelse return;
                 if (saved_pane.serial != serial) return;
                 const saved = if (saved_pane.file) |*file| file else return;
                 // A save the host could not do (saveFailed) is its err
-                // record alone, not a `save`.
+                // record alone, not a `save`, and changes nothing: a clean
+                // file stays clean.
+                if (p.fs.failures != failures) {
+                    saved.saved_revision = was;
+                    return;
+                }
+                // Edited while it was written: what is on disk is not this.
                 if (saved.saved_revision != saved.revision) return;
                 saved.disk_gone = false;
                 ctlfs.events.noteLog(p, .save, saved_pane);
@@ -5298,7 +5329,21 @@ pub const Pardes = struct {
             .save_text => |st| {
                 const pane = p.panes[st.pane] orelse return;
                 if (pane.serial != st.serial) return; // a recycled slot: not ours
-                if (pane.file) |f| return p.hostWriteFile(st.pane, st.path.slice(), f.content);
+                if (pane.file) |*f| {
+                    const was = f.saved_revision;
+                    const failures = p.fs.failures;
+                    p.hostWriteFile(st.pane, st.path.slice(), f.content);
+                    const after = p.panes[st.pane] orelse return;
+                    if (after.serial != st.serial) return;
+                    const g = if (after.file) |*file| file else return;
+                    // Failed, it changes nothing: not the name, not dirty.
+                    if (p.fs.failures != failures) {
+                        g.saved_revision = was;
+                        return;
+                    }
+                    if (st.promote) p.promoteSaved(st.pane, st.path.slice());
+                    return;
+                }
                 if (!pane.isTerminal()) return;
                 const text = panes.Terminal.screenTextAlloc(pane, p.gpa) catch return;
                 defer p.gpa.free(text);
diff --git a/test/panes.zig b/test/panes.zig
index 3d2821aa..1988fe16 100644
--- a/test/panes.zig
+++ b/test/panes.zig
@@ -2500,21 +2500,48 @@ test "Save on a scratch does not watch a path the host could not create" {
     const id = p.active;
     while (p.nextEffect()) |_| {}
     p.host = .{ .ctx = p, .vtable = &.{ .write_file = Refusing.write } };
+    const file = &p.panes[id].?.file.?;
+    const scratch_name = try std.testing.allocator.dupe(u8, file.path);
+    defer std.testing.allocator.free(scratch_name);
+    const was_dirty = file.revision != file.saved_revision;
     try std.testing.expect(p.executeBuiltinLine(id, "Save /new-file.txt"));
     while (p.nextEffect()) |effect| p.perform(effect);
-    const file = &p.panes[id].?.file.?;
-    try std.testing.expect(file.output == null);
-    try std.testing.expect(file.watch_after_save);
-    try std.testing.expect(file.revision != file.saved_revision);
+    // Failed, it changed nothing: still the scratch, its name and dirty
+    // flag as they were, unwatched.
+    try std.testing.expect(file.output != null);
+    try std.testing.expectEqualStrings(scratch_name, file.path);
+    try std.testing.expectEqual(was_dirty, file.revision != file.saved_revision);
     try std.testing.expect(!p.fallback.watched[id]);
     p.host = .{};
-    try std.testing.expect(p.executeBuiltinLine(id, "Save"));
+    try std.testing.expect(p.executeBuiltinLine(id, "Save /new-file.txt"));
     while (p.nextEffect()) |effect| p.perform(effect);
-    try std.testing.expect(!file.watch_after_save);
+    try std.testing.expect(file.output == null);
+    try std.testing.expectEqualStrings("/new-file.txt", file.path);
     try std.testing.expectEqual(file.revision, file.saved_revision);
     try std.testing.expect(p.fallback.watched[id]);
 }
 
+test "a failed Save to another name leaves a clean named file clean" {
+    const Refusing = struct {
+        fn write(ctx: ?*anyopaque, id: u8, _: []const u8, _: []const u8) void {
+            const p: *Pardes = @ptrCast(@alignCast(ctx.?));
+            p.saveFailed(id, "save", error.AccessDenied);
+        }
+    };
+    const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true, .cols = 40, .rows = 12 });
+    defer p.deinit();
+    const pane = try p.setTestFile("clean\n");
+    while (p.nextEffect()) |_| {}
+    const file = &pane.file.?;
+    try std.testing.expectEqual(file.revision, file.saved_revision);
+    p.host = .{ .ctx = p, .vtable = &.{ .write_file = Refusing.write } };
+    try std.testing.expect(p.executeBuiltinLine(0, "Save /root/x.txt"));
+    while (p.nextEffect()) |effect| p.perform(effect);
+    try std.testing.expectEqual(file.revision, file.saved_revision);
+    try std.testing.expectEqualStrings("/test.txt", file.path);
+    p.host = .{};
+}
+
 test "unplaced terminal panes keep their allocated grid until placement" {
     const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true, .cols = 100, .rows = 30 });
     defer p.deinit();