pipe: a filter that fails says so, and `| head -1` stops failing

Two things made the selection pipe feel like it had never worked. It runs —
test/snapshots/pipe.snap drives the real binary through a pty and filters
`alpha beta` to `ALPHA BETA` — but it had no way to tell you when it did not,
and one of its failure conditions was not a failure at all.

IT NOW SAYS WHY. `runOne` read the command's stderr into memory and freed it
two lines later, unread; every caller answered a failed filter with a bare
`return`; `pipeResponse` had eight more silent exits under that. So `| trr`
(a typo), `| grep nomatch` (exit 1), `| jq .` on bad JSON — all did nothing,
said nothing, and left the text alone with no way to find out why. The runner
carries a `Failure` home instead: which selection, what became of the command,
and its own stderr. The core turns that into an `+Errors` buffer — acme's name
for output that came from the program rather than from a word anybody clicked:

    | trr

    exit status 127
    sh: line 1: trr: command not found

An output buffer rather than the message row because the useful half of a
shell failure is the text the shell wrote, and a 256-byte row would keep the
label and throw away the reason. Focus stays with the file: `openRead` moves
`p.active` to what it opens, which is right for a Grep you asked to read and
wrong for a report you did not — you want to fix the command and press `|`
again. A host with no `pull_pipe` at all (the detached daemon, the browser,
the board) now says that too, instead of answering failure into the void.

`| head -1` NOW WORKS. `writer_context.ok` was part of the success condition,
so a command that stopped reading its stdin failed the filter even though it
had done exactly its job: `head` takes the line it wants and closes the pipe,
the write gets EPIPE, and a selection bigger than the 64 KiB pipe buffer was
enough to trigger it. helix joins its input task and ignores the result for
this reason; the exit status is the whole verdict. Also reported rather than
swallowed: the ten-second timeout, the output ceilings, and a file edited
while the filter ran — one keystroke during a slow command used to discard the
result in a way indistinguishable from the filter doing nothing.

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

6 files changed, 266 insertions, 31 deletions

diff --git a/src/gui/gui.zig b/src/gui/gui.zig
index a87eebeb..b31fb5f6 100644
--- a/src/gui/gui.zig
+++ b/src/gui/gui.zig
@@ -3624,6 +3624,7 @@ const Shell = struct {
                     .id = response.id,
                     .success = response.success,
                     .outputs = response.outputs,
+                    .failure = response.failure,
                 } });
                 response.deinit(s.gpa);
                 s.saw_event = true;
diff --git a/src/macos.zig b/src/macos.zig
index 9d4f6039..c50d64cf 100644
--- a/src/macos.zig
+++ b/src/macos.zig
@@ -1418,6 +1418,7 @@ fn drainInbox(st: *State) bool {
                     .id = value.id,
                     .success = value.success,
                     .outputs = value.outputs,
+                    .failure = value.failure,
                 } });
                 st.pipe_tasks.finish(st.io, value.id);
             },
diff --git a/src/output_pane.zig b/src/output_pane.zig
index 1850fa92..288c3e43 100644
--- a/src/output_pane.zig
+++ b/src/output_pane.zig
@@ -572,6 +572,14 @@ pub fn openEffectCode(p: *Pardes, id: usize, argument: []const u8) !void {
 ///
 /// `content` is gpa-owned: adopted by the buffer, or freed here when there is
 /// nowhere to put it.
+/// Put a report in this directory's `+Errors` buffer — acme's own name for
+/// output that came from the PROGRAM rather than from a word somebody clicked.
+/// Refills the one already open rather than stacking a twin, which is what a
+/// second failed filter wants: the newest reason is the one being read.
+pub fn openErrors(p: *Pardes, id: usize, content: []u8) !void {
+    return openRead(p, id, .errors, "", content);
+}
+
 fn openRead(p: *Pardes, id: usize, from: Origin, arg: []const u8, content: []u8) !void {
     errdefer p.gpa.free(content);
     const pane = p.panes[id] orelse return error.MissingPane;
diff --git a/src/pardes.zig b/src/pardes.zig
index 00226f3c..edff0ec7 100644
--- a/src/pardes.zig
+++ b/src/pardes.zig
@@ -784,6 +784,52 @@ test "selection pipe failure and stale completion never mutate the file" {
     try std.testing.expectEqual(@as(usize, 0), pane.file.?.undo_len);
 }
 
+test "a failed filter opens an errors buffer carrying the command's own words" {
+    const gpa = std.testing.allocator;
+    const p = try Pardes.init(gpa, .{ .tty_only = true, .cols = 100, .rows = 30 });
+    defer p.deinit();
+    while (p.nextEffect()) |_| {}
+    const pane = try p.hxOpenFileContent("abc\n");
+    pane.cur_col = 2;
+    pane.vsel = .{ .active = true, .row = 0, .col = 0, .explicit = true };
+
+    p.update(.{ .key = .{ .cp = '|' } });
+    for ("trr") |c| p.update(.{ .key = .{ .cp = c, .text = &.{c} } });
+    p.update(.{ .key = .{ .cp = Key.enter } });
+    const id = nextPipeEffect(p) orelse return error.MissingPipeEffect;
+
+    // The shape a real runner brings home for `| trr`: nonzero exit, and the
+    // shell's own sentence about it.
+    var stderr = "sh: line 1: trr: command not found\n".*;
+    p.update(.{ .pipe_resp = .{
+        .id = id,
+        .success = false,
+        .outputs = &.{},
+        .failure = .{ .kind = .exit, .code = 127, .stderr = &stderr },
+    } });
+
+    // The text is untouched — a failed filter is not an edit...
+    try std.testing.expectEqualSlices(u8, "abc\n", pane.file.?.content);
+    try std.testing.expectEqual(@as(usize, 0), pane.file.?.undo_len);
+    // ...and the cursor did not go anywhere, so `|` again edits the same file.
+    try std.testing.expectEqual(@as(usize, 0), p.active);
+
+    // ...but the reason is now READABLE, in an +Errors buffer: the command as
+    // typed, what became of it, and what the shell said. Every one of those
+    // three used to be dropped on the floor.
+    var found: ?[]const u8 = null;
+    for (p.panes) |slot| {
+        const q = slot orelse continue;
+        const qf = q.file orelse continue;
+        const o = qf.output orelse continue;
+        if (std.meta.activeTag(o.from) == .errors) found = qf.content;
+    }
+    const report = found orelse return error.NoErrorsBuffer;
+    try std.testing.expect(std.mem.indexOf(u8, report, "| trr") != null);
+    try std.testing.expect(std.mem.indexOf(u8, report, "exit status 127") != null);
+    try std.testing.expect(std.mem.indexOf(u8, report, "command not found") != null);
+}
+
 test "selection pipe rejects a reused pane slot and a superseded request" {
     const gpa = std.testing.allocator;
     const p = try Pardes.init(gpa, .{ .tty_only = true });
@@ -4046,7 +4092,15 @@ pub const Event = union(enum) {
     /// A selection-pipe worker finished. Every output is borrowed for this
     /// update only; success is atomic, so a failed/nonzero invocation carries
     /// no usable outputs and changes nothing.
-    pipe_resp: struct { id: u32, success: bool, outputs: []const []const u8 },
+    /// `failure` is borrowed for this update like `outputs`, and is what the
+    /// core turns into an `+Errors` pane. Null with `success = false` means
+    /// nobody ever ran it — a host with no `pull_pipe` at all.
+    pipe_resp: struct {
+        id: u32,
+        success: bool,
+        outputs: []const []const u8,
+        failure: ?selection_pipe.Failure = null,
+    },
     /// a file the shell was asked to watch changed on disk; `bytes` are the
     /// exact snapshot the host hashed, borrowed for this call like `output`.
     /// Text panes adopt a copy; PDF panes reopen the path so MuPDF owns its
@@ -7525,7 +7579,7 @@ pub const Pardes = struct {
             },
             .eof => |e| p.removePane(e.pane),
             .lsp_resp => |r| p.lspResponse(r.id, r.rows),
-            .pipe_resp => |r| p.pipeResponse(r.id, r.success, r.outputs),
+            .pipe_resp => |r| p.pipeResponse(r.id, r.success, r.outputs, r.failure),
             .file_changed => |fc| _ = p.applyWatchedFileChanged(fc.pane, fc.bytes),
             .key => |key| p.handleKey(key),
             .mouse => |m| {
@@ -10365,17 +10419,80 @@ pub const Pardes = struct {
         return null;
     }
 
-    fn pipeResponse(p: *Pardes, id: u32, success: bool, outputs: []const []const u8) void {
+    /// Put a failed filter where it can be READ: the command, which selection
+    /// it was, what became of it, and the command's own stderr underneath.
+    ///
+    /// An `+Errors` pane rather than the message row, because a message row is
+    /// 256 bytes and one line, and the useful half of a shell failure is the
+    /// text the shell wrote — `sh: line 1: trr: command not found`, a compiler
+    /// diagnostic, a `jq` parse error with a column in it. Truncating that to
+    /// fit a row would throw away the reason and keep the label.
+    fn pipeFailed(p: *Pardes, wait: *const PendingPipe, failure: ?selection_pipe.Failure) void {
+        var out: std.Io.Writer.Allocating = .init(p.gpa);
+        defer out.deinit();
+        const w = &out.writer;
+        w.print("| {s}\n\n", .{wait.command}) catch return;
+        if (failure) |fail| {
+            if (wait.inputs.len > 1)
+                w.print("selection {d} of {d}: ", .{ fail.index + 1, wait.inputs.len }) catch return;
+            switch (fail.kind) {
+                .exit => w.print("exit status {d}\n", .{fail.code}) catch return,
+                .signal => w.writeAll("killed by a signal\n") catch return,
+                .timeout => w.print(
+                    "still running after {d} seconds, and stopped\n",
+                    .{selection_pipe.command_timeout_seconds},
+                ) catch return,
+                .too_large => w.writeAll("produced more output than a filter may return\n") catch return,
+                .spawn => w.writeAll("could not be started\n") catch return,
+                .io => w.writeAll("could not be read\n") catch return,
+            }
+            if (fail.stderr.len > 0) w.print("\n{s}", .{fail.stderr}) catch return;
+        } else {
+            // No diagnosis at all: nobody ran it. The detached daemon, the
+            // browser and the board all leave `pull_pipe` null on purpose.
+            w.writeAll("this session cannot run filters\n") catch return;
+        }
+        const content = out.toOwnedSlice() catch return;
+        // FOCUS STAYS WITH THE TEXT. `openRead` moves `p.active` to the buffer
+        // it opens, which is right for a `Grep` you asked to read and wrong for
+        // a report you did not: a failed filter should put the reason on screen
+        // and leave the cursor in the file you were filtering, ready to fix the
+        // command and press `|` again.
+        const was = p.active;
+        output_pane.openErrors(p, wait.pane, content) catch |err| {
+            p.gpa.free(content);
+            // Nowhere to put the report is itself worth one line.
+            p.reportError(wait.pane, "pipe", err);
+            return;
+        };
+        if (p.panes[was] != null) p.active = was;
+    }
+
+    fn pipeResponse(
+        p: *Pardes,
+        id: u32,
+        success: bool,
+        outputs: []const []const u8,
+        failure: ?selection_pipe.Failure,
+    ) void {
+        // A SUPERSEDED OR UNKNOWN id is the one silence worth keeping: it is
+        // the answer to a question nobody is still asking.
         if (p.pipe_wait == null or p.pipe_wait.?.id != id) return;
         var wait = p.pipe_wait.?;
         p.pipe_wait = null;
         defer wait.deinit(p.gpa);
-        if (!success or outputs.len != wait.inputs.len) return;
+
+        if (!success or outputs.len != wait.inputs.len) return p.pipeFailed(&wait, failure);
 
         const pane = p.panes[wait.pane] orelse return;
         if (pane.serial != wait.serial) return;
         const f = if (pane.file) |*file| file else return;
-        if (!output_pane.fileTraits(f.output).saves or f.revision != wait.revision) return;
+        if (!output_pane.fileTraits(f.output).saves) return;
+        // THE FILE MOVED UNDER THE FILTER. One keystroke during a ten-second
+        // command was enough to discard the whole result in silence, which is
+        // indistinguishable from the filter having done nothing at all.
+        if (f.revision != wait.revision)
+            return p.reportError(wait.pane, "pipe", error.FileChangedWhileFiltering);
 
         var total_output: usize = 0;
         var removed: usize = 0;
diff --git a/src/selection_pipe.zig b/src/selection_pipe.zig
index c1104010..8e721751 100644
--- a/src/selection_pipe.zig
+++ b/src/selection_pipe.zig
@@ -65,19 +65,60 @@ pub const Job = struct {
     }
 };
 
-/// One worker answer. `outputs` owns each slice; a failure owns an empty list.
+/// WHY a filter produced nothing, carried home so somebody can be told.
+///
+/// Until this existed the runner read the command's stderr into memory and
+/// then FREED IT UNREAD — the one artifact that explains a failure, discarded
+/// two lines after it arrived — and every caller answered a failed filter with
+/// a bare `return`. `| trr a-z A-Z` (a typo), `| grep nomatch` (exit 1),
+/// `| jq .` on bad JSON: all of them did nothing, said nothing, and left the
+/// text alone with no way to find out why.
+pub const Failure = struct {
+    pub const Kind = enum {
+        /// The command never started: no `/bin/sh`, a cwd that is gone, a NUL
+        /// in the command, a fork that failed.
+        spawn,
+        /// Still running at `command_timeout_seconds`.
+        timeout,
+        /// Past `max_stdout_bytes` / `max_stderr_bytes` / the job total.
+        too_large,
+        /// Ran, and exited nonzero. `code` says which.
+        exit,
+        /// Killed by a signal.
+        signal,
+        /// A read, a write or an allocation failed under us.
+        io,
+    };
+
+    kind: Kind = .io,
+    /// Which selection this was, so a report over several cursors can say.
+    index: u32 = 0,
+    /// The exit status, when `kind` is `.exit`.
+    code: u8 = 0,
+    /// stderr exactly as the command wrote it, OWNED by the response. Empty
+    /// when the command said nothing, which is why `kind` and `code` exist.
+    stderr: []u8 = &.{},
+};
+
+/// One worker answer. `outputs` owns each slice; a failure owns an empty list
+/// and, usually, the command's own account of itself in `failure`.
 pub const Response = struct {
     id: u32,
     success: bool,
     outputs: [][]u8,
+    failure: ?Failure = null,
 
     pub fn deinit(response: *Response, gpa: std.mem.Allocator) void {
         for (response.outputs) |output| gpa.free(output);
         if (response.outputs.len > 0) gpa.free(response.outputs);
+        if (response.failure) |f| if (f.stderr.len > 0) gpa.free(f.stderr);
         response.* = undefined;
     }
 };
 
+/// What one invocation came to: the bytes, or the reason there are none.
+pub const Outcome = union(enum) { ok: []u8, failed: Failure };
+
 /// The in-flight set a host keeps while pipes run off its loop.
 ///
 /// tty.zig and gui.zig each had this verbatim — same `finish` walk, same
@@ -151,9 +192,14 @@ pub fn runOne(
     command: []const u8,
     cwd: []const u8,
     input: []const u8,
-) ?[]u8 {
+) Outcome {
+    const fail = struct {
+        fn k(kind: Failure.Kind) Outcome {
+            return .{ .failed = .{ .kind = kind } };
+        }
+    };
     if (std.mem.indexOfScalar(u8, command, 0) != null or
-        std.mem.indexOfScalar(u8, cwd, 0) != null) return null;
+        std.mem.indexOfScalar(u8, cwd, 0) != null) return fail.k(.spawn);
 
     var child = std.process.spawn(io, .{
         .argv = &.{ "/bin/sh", "-c", command },
@@ -161,7 +207,7 @@ pub fn runOne(
         .stdin = .pipe,
         .stdout = .pipe,
         .stderr = .pipe,
-    }) catch return null;
+    }) catch return fail.k(.spawn);
 
     var writer_context: WriterContext = .{
         .io = io,
@@ -172,7 +218,7 @@ pub fn runOne(
     var writer: ?std.Thread = std.Thread.spawn(.{}, writeInput, .{&writer_context}) catch {
         writer_context.file.close(io);
         child.kill(io);
-        return null;
+        return fail.k(.spawn);
     };
     // On every early return kill first, unblocking a command which never read
     // stdin, then join the short-lived writer before its borrowed input dies.
@@ -192,33 +238,49 @@ pub fn runOne(
     } }).toDeadline(io);
     while (multi_reader.fill(64, deadline)) |_| {
         if (stdout_reader.buffered().len > max_stdout_bytes or
-            stderr_reader.buffered().len > max_stderr_bytes) return null;
+            stderr_reader.buffered().len > max_stderr_bytes) return fail.k(.too_large);
     } else |err| switch (err) {
         error.EndOfStream => {},
-        else => return null,
+        // The deadline is the only one of these a human is likely to cause,
+        // and it is the one they are least able to guess at: ten seconds of
+        // nothing used to be followed by nothing.
+        error.Timeout => return fail.k(.timeout),
+        else => return fail.k(.io),
     }
     if (stdout_reader.buffered().len > max_stdout_bytes or
-        stderr_reader.buffered().len > max_stderr_bytes) return null;
-    multi_reader.checkAnyError() catch return null;
-    const term = child.wait(io) catch return null;
+        stderr_reader.buffered().len > max_stderr_bytes) return fail.k(.too_large);
+    multi_reader.checkAnyError() catch return fail.k(.io);
+    const term = child.wait(io) catch return fail.k(.io);
     writer.?.join();
     writer = null;
 
-    const stdout = multi_reader.toOwnedSlice(0) catch return null;
+    const stdout = multi_reader.toOwnedSlice(0) catch return fail.k(.io);
     const stderr = multi_reader.toOwnedSlice(1) catch {
         gpa.free(stdout);
-        return null;
-    };
-    defer gpa.free(stderr);
-    const exited_zero = switch (term) {
-        .exited => |code| code == 0,
-        else => false,
+        return fail.k(.io);
     };
-    if (!writer_context.ok or !exited_zero) {
-        gpa.free(stdout);
-        return null;
+    // stderr is NOT freed here any more. It is the command's own account of
+    // what went wrong, and it goes home with the failure.
+    errdefer gpa.free(stderr);
+
+    // A STDIN WRITE THAT ENDED EARLY IS NOT A FAILURE. `writer_context.ok` was
+    // part of this condition, so `| head -1` over a selection bigger than the
+    // pipe buffer failed — the command closed stdin after the line it wanted,
+    // the write got EPIPE, and a filter that had done exactly its job reported
+    // nothing. helix joins its input task and ignores the result for this
+    // reason; the exit status is the whole verdict.
+    switch (term) {
+        .exited => |code| if (code != 0) {
+            gpa.free(stdout);
+            return .{ .failed = .{ .kind = .exit, .code = code, .stderr = stderr } };
+        },
+        else => {
+            gpa.free(stdout);
+            return .{ .failed = .{ .kind = .signal, .stderr = stderr } };
+        },
     }
-    return stdout;
+    gpa.free(stderr);
+    return .{ .ok = stdout };
 }
 
 /// Invoke the command independently for every selection. The response is all
@@ -230,7 +292,17 @@ pub fn runJob(gpa: std.mem.Allocator, io: std.Io, job: *const Job) Response {
     var made: usize = 0;
     var total: usize = 0;
     for (job.inputs, 0..) |input, i| {
-        const output = runOne(gpa, io, job.command, job.cwd, input) orelse break;
+        const output = switch (runOne(gpa, io, job.command, job.cwd, input)) {
+            .ok => |bytes| bytes,
+            .failed => |f| {
+                // WHICH selection, because with several cursors "it failed" is
+                // not enough to go looking with.
+                var owned = f;
+                owned.index = @intCast(i);
+                response.failure = owned;
+                break;
+            },
+        };
         if (std.math.add(usize, total, output.len) catch null) |next_total| {
             if (next_total <= max_total_stdout_bytes) {
                 outputs[i] = output;
@@ -240,6 +312,7 @@ pub fn runJob(gpa: std.mem.Allocator, io: std.Io, job: *const Job) Response {
             }
         }
         gpa.free(output);
+        response.failure = .{ .kind = .too_large, .index = @intCast(i) };
         break;
     }
     if (made != job.inputs.len) {
@@ -252,12 +325,46 @@ pub fn runJob(gpa: std.mem.Allocator, io: std.Io, job: *const Job) Response {
     return response;
 }
 
-test "native pipe runner preserves stdin/stdout bytes and rejects failure" {
+test "native pipe runner preserves stdin/stdout bytes and reports how it failed" {
     const gpa = std.testing.allocator;
     const io = std.testing.io;
-    const output = runOne(gpa, io, "printf 'prefix:'; cat; printf '\\n'", "/tmp", "a\x00b\n") orelse
-        return error.PipeCommandFailed;
+    const output = switch (runOne(gpa, io, "printf 'prefix:'; cat; printf '\\n'", "/tmp", "a\x00b\n")) {
+        .ok => |bytes| bytes,
+        .failed => return error.PipeCommandFailed,
+    };
     defer gpa.free(output);
     try std.testing.expectEqualSlices(u8, "prefix:a\x00b\n\n", output);
-    try std.testing.expect(runOne(gpa, io, "printf ignored; exit 7", "/tmp", "") == null);
+
+    // A NONZERO EXIT COMES HOME WITH ITS REASON. The status and the command's
+    // own stderr are the whole of what a human needs to fix a typo'd filter,
+    // and both used to be freed on the floor.
+    switch (runOne(gpa, io, "echo trouble >&2; exit 7", "/tmp", "")) {
+        .ok => |bytes| {
+            gpa.free(bytes);
+            return error.PipeShouldHaveFailed;
+        },
+        .failed => |f| {
+            defer gpa.free(f.stderr);
+            try std.testing.expectEqual(Failure.Kind.exit, f.kind);
+            try std.testing.expectEqual(@as(u8, 7), f.code);
+            try std.testing.expectEqualSlices(u8, "trouble\n", f.stderr);
+        },
+    }
+
+    // ...and a command that stops reading its stdin SUCCEEDS. `| head -1` over
+    // a selection bigger than the pipe buffer takes the line it wanted and
+    // closes the pipe; the write gets EPIPE, and that used to fail the filter
+    // even though it had done exactly its job.
+    const big = try gpa.alloc(u8, 512 * 1024);
+    defer gpa.free(big);
+    @memset(big, 'x');
+    big[0] = 'a';
+    big[1] = '\n';
+    switch (runOne(gpa, io, "head -1", "/tmp", big)) {
+        .ok => |bytes| {
+            defer gpa.free(bytes);
+            try std.testing.expectEqualSlices(u8, "a\n", bytes);
+        },
+        .failed => return error.EarlyStdinCloseShouldNotFail,
+    }
 }
diff --git a/src/tty/tty.zig b/src/tty/tty.zig
index de24f289..ff86ef88 100644
--- a/src/tty/tty.zig
+++ b/src/tty/tty.zig
@@ -1160,6 +1160,7 @@ const Shell = struct {
                     .id = response.id,
                     .success = response.success,
                     .outputs = response.outputs,
+                    .failure = response.failure,
                 } });
                 response.deinit(s.gpa);
                 s.pipe_tasks.finish(s.io, response_value.id);