errors: a save that could not happen, and two panics on an ordinary click

A review of what this program does when the environment says no. The finding
that reframes it: there were almost NO panics on ordinary paths — the rule
already held — but there was a great deal of silence, and one case worse than
any panic.

SILENT DATA LOSS ON SAVE. `saveFile` marked the pane saved the moment it
QUEUED the effect, before any host had tried; `host_io.writeFd` returned void,
so a short or failed write was indistinguishable from a complete one; and
`writeFileBytes` returned true regardless. A save to a read-only file, or into
a directory removed under the pane, therefore cleared the tag's ` *` and posted
nothing — and `Del` makes no dirty check, so the next click threw the edits
away with the screen saying they were safe. On a full disk it was worse: the
file is already `O_TRUNC`'d when `write` fails, so the message row said `saved`
over a file that had just been emptied.

Now: `writeFd` reports, `writeFileBytes` returns WHY (`PermissionDenied`,
`NoSpaceLeft`, `ReadOnlyFilesystem`, …) including a failed `close`, which is
where write-back filesystems report at all; the core marks the pane saved
around `perform` rather than at emit, which is also where the bytes are read;
and a host that could not write calls `Pardes.saveFailed`, which puts the
reason on the message row and takes the clean mark back. That is a CALL and
not a return value because host.zig enforces, at comptime, that a `push_`
method reaching every host in a fan-out cannot have one answer — the first
attempt at this changed the signature and the compiler was right to refuse it.

TWO PANICS ON AN ORDINARY KEYSTROKE, in look.zig's number scans. `v = v * 10 +
d` over caller-supplied digits, reached from `parsePathLine` and the `@pN` scan
— which every Look, every right-click and every n/N motion runs on whatever
word is under the pointer. A hash in a log, a CSV column, any output shaped
`foo:99999999999999999999`, and the editor died with "integer overflow". Both
saturate now, the same way acmefs.zig's address parser already did; a saturated
line is refused by `file_pane.open`'s `line <= total` and a saturated pane id
by `focusPaneLine`'s `id < MAX_PANES`, so nothing addressable changes.

A BOOT FILE THAT WILL NOT OPEN joins the missing-name case in the `+Errors`
pane instead of taking the launch down: `pardes /root` resolves as a `.file`,
could not be read, and left `error: PermissionDenied` and a return trace.
`look.readFile` now says which errno it was, so the pane can say "permission
denied" rather than a word from the source code.

The tag-marker test drained no effects and passed anyway, which is exactly the
defect; it drains now.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf

1 files changed, 135 insertions, 9 deletions

diff --git a/src/pardes.zig b/src/pardes.zig
index 55e111e6..4956150a 100644
--- a/src/pardes.zig
+++ b/src/pardes.zig
@@ -1384,6 +1384,12 @@ test "an unsaved file marker sits between its path and builtins until Save" {
     try std.testing.expect(marker_at < save_at);
 
     try std.testing.expect(p.executeBuiltinLine(0, "Save"));
+    // DRAINED FIRST, and the drain is the point rather than ceremony: the
+    // marker now clears when the write LANDS, not when the effect is queued.
+    // This test used to pass without it, which is exactly what was wrong — a
+    // save the host could not do cleared the marker anyway. No frame is
+    // affected, because `pump` drains before it renders.
+    while (p.nextEffect()) |effect| p.perform(effect);
     const saved = try p.tagText(p.scratch.allocator(), pane);
     try std.testing.expect(std.mem.indexOf(u8, saved, "/hxcase.txt *") == null);
 
@@ -1903,6 +1909,43 @@ test "Save takes the path as an argument, relative to the pane's own directory"
     try std.testing.expect(drainForSavePath(p, &buf) == null);
 }
 
+test "a save the host could not do leaves the pane dirty" {
+    const gpa = std.testing.allocator;
+    // A host that refuses every write: a read-only file, a directory that was
+    // removed under the pane, a full disk. The core cannot tell those apart and
+    // does not need to — the host says why on the message row, and this is the
+    // other half, which is that the pane must NOT come clean.
+    const Refusing = struct {
+        fn writeFile(ctx: ?*anyopaque, pane: u8, _: []const u8, _: []const u8) void {
+            const core: *Pardes = @ptrCast(@alignCast(ctx.?));
+            core.saveFailed(pane, "save", error.PermissionDenied);
+        }
+    };
+    const p = try Pardes.init(gpa, .{ .tty_only = true, .cols = 100, .rows = 30 });
+    defer p.deinit();
+    while (p.nextEffect()) |_| {}
+    const pane = try p.hxOpenFileContent("before\n");
+    file_pane.setContent(p, &pane.file.?, try gpa.dupe(u8, "after\n"));
+    try std.testing.expect(pane.file.?.revision != pane.file.?.saved_revision);
+
+    p.host = .{ .ctx = p, .vtable = &.{ .push_write_file = Refusing.writeFile } };
+    try std.testing.expect(p.executeBuiltinLine(0, "Save"));
+    while (p.nextEffect()) |effect| p.perform(effect);
+    // STILL DIRTY. Until this, `saveFile` marked the pane saved the moment it
+    // QUEUED the effect, so the tag's ` *` cleared on a save that never
+    // happened — and `Del` makes no dirty check, so the next click threw the
+    // edits away with the screen saying they were safe.
+    try std.testing.expect(pane.file.?.revision != pane.file.?.saved_revision);
+
+    // ...and the same save against a host that CAN write does come clean, so
+    // the guard above is not simply "never clean".
+    p.host = .{};
+    try std.testing.expect(p.executeBuiltinLine(0, "Save"));
+    while (p.nextEffect()) |effect| p.perform(effect);
+    try std.testing.expectEqual(pane.file.?.revision, pane.file.?.saved_revision);
+    try std.testing.expectEqualStrings("after\n", p.fallback.get("/hxcase.txt").?);
+}
+
 test "Save elsewhere copies a file's bytes and keeps the pane on its own file" {
     const gpa = std.testing.allocator;
     const p = try Pardes.init(gpa, .{ .tty_only = true, .cols = 100, .rows = 30 });
@@ -6328,13 +6371,42 @@ pub const Pardes = struct {
             // is asking to READ it, not to be handed a shell you did not ask
             // for and have to close — and the launch directory is one Newcol
             // away when it is wanted. Doc, PDF and image all boot the same way.
-            _ = initial_doc: {
+            const opened = initial_doc: {
                 if (comptime pdf_enabled) if (look.isPdfPath(path))
-                    break :initial_doc try pdf_pane.openPane(p, 0, path, opts.file_line);
+                    break :initial_doc pdf_pane.openPane(p, 0, path, opts.file_line);
                 if (look.isImagePath(path))
-                    break :initial_doc try image_pane.create(p, 0, path, &.{});
-                break :initial_doc try file_pane.open(p, 0, path, opts.file_line);
+                    break :initial_doc image_pane.create(p, 0, path, &.{});
+                break :initial_doc file_pane.open(p, 0, path, opts.file_line);
             };
+            // ...AND IF IT WILL NOT OPEN, SAY SO IN THE WINDOW. `pardes /root`
+            // is a directory that resolves and cannot be read, so it arrives
+            // here as a `.file` and used to take the whole launch down with
+            // `error: PermissionDenied` and a return trace out of `main` — a
+            // crash, to the human, for asking to read something they are not
+            // allowed to read. Every environment reason lands in the same
+            // `+Errors` pane a missing name does, because they are the same
+            // event to whoever typed it: pardes cannot show you that.
+            //
+            // OUT OF MEMORY IS NOT ONE OF THEM and goes back to the caller.
+            // A core that could not allocate a file cannot allocate the pane
+            // explaining it, and pretending otherwise turns a clean failure
+            // into a second one. Every opener does its fallible IO BEFORE it
+            // claims a pane slot (`look.readFile`, then `newDocPane`), so slot
+            // 0 is still free here — which is what makes this legal.
+            if (opened) |_| {} else |err| {
+                if (err == error.OutOfMemory) return err;
+                const why = switch (err) {
+                    error.PermissionDenied => "permission denied",
+                    error.FileNotFound => "no file of that name",
+                    error.IsDirectory => "that is a directory, and not one that could be read",
+                    error.PathTooLong => "that path is too long",
+                    error.FileTooLarge => "that file is too large to open",
+                    else => @errorName(err),
+                };
+                const content = try std.fmt.allocPrint(gpa, "cannot open\n\n\t{s}\n\n{s}\n", .{ path, why });
+                errdefer gpa.free(content);
+                _ = try output_pane.open(p, 0, std.fs.path.dirname(path) orelse "/", .errors, "", content);
+            }
             p.ncol = 1;
             p.col_n[0] = 1;
             p.col_terms[0][0] = 0;
@@ -7117,7 +7189,22 @@ pub const Pardes = struct {
 
     fn hostWriteFile(p: *Pardes, pane: u8, path: []const u8, bytes: []const u8) void {
         if (p.host.vtable.push_write_file) |f| return f(p.host.ctx, pane, path, bytes);
-        p.fallback.writeFile(path, bytes);
+        // The in-process filesystem reports the same way a real host does, so
+        // an OOM here leaves the pane dirty rather than looking saved.
+        if (!p.fallback.writeFile(path, bytes)) p.saveFailed(pane, "save", error.OutOfMemory);
+    }
+
+    /// A HOST'S ANSWER TO `save_file`, and the only one it needs to give: the
+    /// write did not happen. Puts the reason on the pane's message row and
+    /// takes back the optimistic clean mark `perform` made, so the tag keeps
+    /// its ` *` and the edits keep being edits. See host.zig
+    /// `push_write_file` for why this is a call and not a return value.
+    pub fn saveFailed(p: *Pardes, id: u8, what: []const u8, err: anyerror) void {
+        if (p.panes[id]) |pane| if (pane.file) |*f| {
+            // The `-%` spelling acmefs.zig already uses for "make this dirty".
+            f.saved_revision = f.revision -% 1;
+        };
+        p.reportError(id, what, err);
     }
 
     /// The path a watch is about: a real file's, or a PDF's.
@@ -7171,12 +7258,25 @@ pub const Pardes = struct {
                 p.fallback.setLink(u.slice()),
             .save_file => |sf| {
                 const pane = p.panes[sf.pane] orelse return;
-                const f = pane.file orelse return;
+                const f = if (pane.file) |*file| file else return;
+                // CLEAN HERE rather than where the effect was queued, and
+                // BEFORE the call so the host can take it back. `saveFile` used
+                // to set `saved_revision` at emit time, so a write that could
+                // not happen still cleared the tag's ` *` and left the edits one
+                // `Del` from gone — `Del` makes no dirty check. Here is also
+                // where `f.content` is read, so the revision recorded is the
+                // revision of the bytes that actually went out.
+                f.saved_revision = f.revision;
                 p.hostWriteFile(sf.pane, f.path, f.content);
             },
             .save_text => |st| {
                 const pane = p.panes[st.pane] orelse return;
                 if (pane.serial != st.serial) return; // a recycled slot: not ours
+                // `Save <elsewhere>` is a COPY: it does not clean this pane,
+                // because the file the pane has open is not the file that was
+                // written. The one case that does clean is a scratch buffer
+                // adopting the path, and `saveTo` handles that by emitting
+                // `save_file` for the pane's own path instead.
                 if (pane.file) |f| return p.hostWriteFile(st.pane, st.path.slice(), f.content);
                 if (!pane.isTerminal()) return;
                 const text = term_pane.screenTextAlloc(pane, p.gpa) catch return;
@@ -7191,7 +7291,7 @@ pub const Pardes = struct {
                     // A real host reports where it landed, which is what puts
                     // `Restore <path>` in the topbar; the virtual one owes the
                     // same, or the bytes it holds are unreachable.
-                    p.fallback.writeFile(fallback_dump_path, out);
+                    _ = p.fallback.writeFile(fallback_dump_path, out);
                     p.setLastDump(fallback_dump_path);
                 }
             },
@@ -10306,8 +10406,10 @@ pub const Pardes = struct {
         const pane = p.panes[id] orelse return;
         const f = if (pane.file) |*file| file else return;
         if (f.output != null) return; // nothing behind it yet: saveTo, with a path
+        // NOT marked saved here: the effect has only been QUEUED. `perform`'s
+        // `.save_file` arm cleans the pane if and only if the host says the
+        // bytes landed — see there.
         p.emit(.{ .save_file = .{ .pane = @intCast(id) } });
-        f.saved_revision = f.revision;
     }
 
     /// Commit a path — prompted, typed after the word, or chorded onto it.
@@ -10356,7 +10458,6 @@ pub const Pardes = struct {
             f.path = owned;
             f.output = null; // an ordinary file pane from here on
             pane.cwd = .none; // its directory is now its own path's dirname
-            f.saved_revision = f.revision;
             pane.tag_init = false; // re-derive the tag as a plain file
             pane.tag_tail_len = 0;
             p.emit(.{ .save_file = .{ .pane = @intCast(id) } });
@@ -16460,6 +16561,31 @@ test "Esc alternates between two panes of the SAME kind" {
     }
 }
 
+test "a boot file that will not open boots an errors pane rather than failing the launch" {
+    if (platform == .web) return;
+    const gpa = std.testing.allocator;
+    // A path `look.resolve` would have accepted and `readFile` cannot open.
+    // Spelled as a name that is simply not there rather than by chmod-ing a
+    // fixture to 000, because the second answers differently when the suite
+    // runs as root and this must fail the same way everywhere.
+    const p = try Pardes.init(gpa, .{
+        .cols = 80,
+        .rows = 24,
+        .file = "/definitely/not/here/notes.md",
+    });
+    defer p.deinit();
+    p.update(.{ .resize = .{ .cols = 80, .rows = 24 } });
+
+    const pane = p.panes[0].?;
+    const f = pane.file.?;
+    try std.testing.expectEqual(output_pane.Origin.errors, f.output.?.from);
+    // The reason IN WORDS, not an error name: `PermissionDenied` on a screen
+    // is jargon, and the whole point of this pane is that a human reads it.
+    try std.testing.expect(std.mem.indexOf(u8, f.content, "cannot open") != null);
+    try std.testing.expect(std.mem.indexOf(u8, f.content, "no file of that name") != null);
+    try std.testing.expect(std.mem.indexOf(u8, f.content, "/definitely/not/here/notes.md") != null);
+}
+
 test "argv naming nothing boots an errors pane rather than failing the launch" {
     const gpa = std.testing.allocator;
     const p = try Pardes.init(gpa, .{