A Restore of a dump that cannot be read fails at once, and spends no warning

The dump was read only by the host after the step, where a failure went to
the message row alone: a Restore written to ctl answered success, logged no
err, and a second one past the unsaved-text warning did the same. Restore now
reads the dump first, before the warning, and says 'Restore: <path>: no such
file' (or permission denied, a directory), which fails a ctl write and so
logs err. A bare Restore with no dump yet, or a name too long, says so too.

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

1 files changed, 46 insertions, 6 deletions

diff --git a/src/builtins.zig b/src/builtins.zig
index be0956e0..fbab925c 100644
--- a/src/builtins.zig
+++ b/src/builtins.zig
@@ -236,16 +236,41 @@ test "Exit asks once about unsaved text, and quits when asked again" {
     try std.testing.expect(p.quit);
 }
 
+/// A dump file that exists, for a Restore to get past reading it.
+fn testDump(buf: []u8) ![]const u8 {
+    var tmp = std.testing.tmpDir(.{});
+    try tmp.dir.writeFile(std.testing.io, .{ .sub_path = "d.zon", .data = ".{}" });
+    var dir: [4096]u8 = undefined;
+    const at = dir[0..try tmp.dir.realPath(std.testing.io, &dir)];
+    return std.fmt.bufPrint(buf, "Restore {s}/d.zon", .{at});
+}
+
 test "Restore asks about unsaved text as Exit does, and restores when asked again" {
     const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true });
     defer p.deinit();
     const pane = try p.setTestFile("saved\n");
     pane.file.?.saved_revision = pane.file.?.revision -% 1; // modified
-    try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /tmp/some.dump.zon"));
+    var line_buf: [4200]u8 = undefined;
+    const line = try testDump(&line_buf);
+    try std.testing.expect(p.executeBuiltinLine(p.active, line));
     try std.testing.expect(p.restore_req == null);
     try std.testing.expect(std.mem.endsWith(u8, pane.msg[0..pane.msg_len], ": Modified (Restore again to discard)"));
-    try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /tmp/some.dump.zon"));
-    try std.testing.expectEqualStrings("/tmp/some.dump.zon", p.restore_req.?);
+    try std.testing.expect(p.executeBuiltinLine(p.active, line));
+    try std.testing.expectEqualStrings(line["Restore ".len..], p.restore_req.?);
+}
+
+test "a Restore of a dump that cannot be read says so, before the unsaved-text warning" {
+    const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true });
+    defer p.deinit();
+    const pane = try p.setTestFile("saved\n");
+    pane.file.?.saved_revision = pane.file.?.revision -% 1; // modified
+    p.fs.no_prompt = true;
+    defer p.fs.no_prompt = false;
+    try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /nope/missing.zon"));
+    try std.testing.expect(p.restore_req == null);
+    try std.testing.expectEqualStrings("Restore: /nope/missing.zon: no such file", p.fs.failure[0..p.fs.failure_len]);
+    // No warning spent: the Exit after it still asks.
+    try std.testing.expect(pane.discard_warned == null);
 }
 
 test "a Restore's warning is not an Exit's: each word is warned on its own" {
@@ -253,7 +278,8 @@ test "a Restore's warning is not an Exit's: each word is warned on its own" {
     defer p.deinit();
     const pane = try p.setTestFile("saved\n");
     pane.file.?.saved_revision = pane.file.?.revision -% 1; // modified
-    try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /tmp/some.dump.zon"));
+    var line_buf: [4200]u8 = undefined;
+    try std.testing.expect(p.executeBuiltinLine(p.active, try testDump(&line_buf)));
     try std.testing.expect(p.restore_req == null);
     // The Restore warned; an Exit after it has not been, and asks.
     try std.testing.expect(p.executeBuiltinLine(p.active, "Exit"));
@@ -473,8 +499,22 @@ pub const Restore = struct {
     pub const scope: Scope = .session;
     pub const takes_arg = true;
     pub fn run(c: Ctx) void {
-        const path = c.arg orelse (c.p.last_dump orelse return);
-        if (path.len > c.p.restore_buf.len) return;
+        const path = c.arg orelse (c.p.last_dump orelse return c.p.reportFailure(c.id, "Restore: no dump yet; name one"));
+        if (path.len > c.p.restore_buf.len) return c.p.reportFailure(c.id, "Restore: that name is too long");
+        // A dump that cannot be read is said now, and fails a ctl write: the
+        // host reads it only after this step, where nobody hears. First,
+        // so a Restore that could never happen spends no warning.
+        const bytes = @import("fs.zig").readRestore(c.p.gpa, path, c.p.settings.dump_dir.get()) catch |err| {
+            var buf: [limits.host_path_cap + 64]u8 = undefined;
+            const why = switch (err) {
+                error.FileNotFound => "no such file",
+                error.PermissionDenied => "permission denied",
+                error.IsDirectory => "a directory, not a dump",
+                else => @errorName(err),
+            };
+            return c.p.reportFailure(c.id, std.fmt.bufPrint(&buf, "Restore: {s}: {s}", .{ path, why }) catch "Restore: cannot read the dump");
+        };
+        c.p.gpa.free(bytes);
         // acme's Load adds a dump's windows to the ones there; a Restore
         // replaces them all, so it asks what Exit asks first.
         if (warnModified(c, .Restore)) return;