Add quick review button.

README.md

@@ -498,6 +498,13 @@ a terminal leaf). A terminal that has since exited reopens as a fresh shell in
 the recorded directory, as it would without herdr-core. Shells in the core can
 also be reached from anywhere else with `hc ls` and `hc attach`.
 
+Since the shells outlive every restart, so does anything wrong with them — an
+environment variable they started with, say. `mise run fresh-shells` replaces
+them all while keeping the layout: it quits playpen (saving the session), ends
+every terminal playpen started in the core, and relaunches, so each pane comes
+back as a new shell in its last directory. It lists what is running and asks
+first, and it is safe to run from inside one of those panes.
+
 The file is `session.json`, and it lives in the **state** directory rather than
 beside `layouts.json` and `settings.json`:
 

assets/icons/icons.gresource.xml

@@ -7,6 +7,7 @@
 -->
 <gresources>
   <gresource prefix="/dev/greyson/playpen/icons">
+    <file compressed="true">scalable/actions/chat-message-new-symbolic.svg</file>
     <file compressed="true">scalable/actions/document-edit-symbolic.svg</file>
     <file compressed="true">scalable/actions/edit-undo-symbolic.svg</file>
     <file compressed="true">scalable/actions/emblem-system-symbolic.svg</file>

assets/icons/scalable/actions/chat-message-new-symbolic.svg

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg height="16px" viewBox="0 0 16 16" width="16px" xmlns="http://www.w3.org/2000/svg">
+    <path d="m 3 1 c -1.644531 0 -3 1.355469 -3 3 v 6 c 0 1.644531 1.355469 3 3 3 h 1 v 3 l 3 -3 v -1 c 0 -0.550781 -0.449219 -1 -1 -1 h -3 c -0.570312 0 -1 -0.429688 -1 -1 v -6 c 0 -0.554688 0.445312 -1 1 -1 h 10 c 0.554688 0 1 0.445312 1 1 v 4 c 0 0.550781 0.449219 1 1 1 s 1 -0.449219 1 -1 v -4 c 0 -1.644531 -1.355469 -3 -3 -3 z m 8 7 v 3 h -3 v 2 h 3 v 3 h 2 v -3 h 3 v -2 h -3 v -3 z m 0 0" fill="#2e3436"/>
+</svg>

build.zig

@@ -180,17 +180,21 @@ pub fn build(b: *std.Build) void {
     palette_tests.root_module.addImport("ghostty-vt", ghostty.module("ghostty-vt"));
     test_step.dependOn(&b.addRunArtifact(palette_tests).step);
 
-    // herdr-core's frame format is its own root: the bytes of the wire
-    // protocol, testable with no core to talk to. The connection on top of it
+    // herdr-core's client is its own root: the frame format (the bytes of the
+    // wire protocol) and the pure rules about what a terminal's environment
+    // keeps, both testable with no core to talk to. The connection itself
     // wants a socket and a main loop, and is checked by running it.
-    const hc_frame_tests = b.addTest(.{
+    const hc_tests = b.addTest(.{
         .root_module = b.createModule(.{
-            .root_source_file = b.path("src/hc/frame.zig"),
+            .root_source_file = b.path("src/hc.zig"),
             .target = target,
             .optimize = optimize,
+            .link_libc = true,
         }),
     });
-    test_step.dependOn(&b.addRunArtifact(hc_frame_tests).step);
+    hc_tests.root_module.addImport("glib", gobject.module("glib2"));
+    hc_tests.root_module.addImport("glibunix", gobject.module("glibunix2"));
+    test_step.dependOn(&b.addRunArtifact(hc_tests).step);
 
     // And a root for the settings file itself. `Settings.zig` reaches GLib for
     // where the file lives, but reading and writing its *contents* is pure

mise.toml

@@ -184,6 +184,10 @@ done
 rmdir "$backups" 2>/dev/null || true
 '''
 
+[tasks.fresh-shells]
+description = "Replace every herdr-core shell playpen keeps with a fresh one, keeping the layout"
+run = "tools/fresh-shells.sh"
+
 [tasks.install]
 description = "Install playpen into ~/.local with a desktop entry and icon"
 depends = ["build"]

src/Pane.zig

@@ -506,6 +506,13 @@ fn buildHeader(self: *Pane) void {
     _ = gtk.Button.signals.clicked.connect(review_button, *Pane, &onReviewClicked, self, .{});
     header.append(review_button.as(gtk.Widget));
 
+    const agent_review = gtk.Button.newFromIconName("chat-message-new-symbolic");
+    agent_review.as(gtk.Widget).addCssClass("flat");
+    agent_review.as(gtk.Widget).addCssClass("playpen-pane-button");
+    agent_review.as(gtk.Widget).setTooltipText("Have Claude review this tab's changes in the review pane");
+    _ = gtk.Button.signals.clicked.connect(agent_review, *Pane, &onAgentReviewClicked, self, .{});
+    header.append(agent_review.as(gtk.Widget));
+
     // Last before close, so the destructive button stays on the end where it
     // is expected and the zoom toggle sits with the other view controls.
     self.zoom.as(gtk.Widget).addCssClass("flat");
@@ -765,6 +772,10 @@ fn onReviewClicked(_: *gtk.Button, self: *Pane) callconv(.c) void {
     self.view.requestReview();
 }
 
+fn onAgentReviewClicked(_: *gtk.Button, self: *Pane) callconv(.c) void {
+    self.view.startAgentReview(self);
+}
+
 fn onZoomClicked(_: *gtk.Button, self: *Pane) callconv(.c) void {
     self.view.toggleZoom(self);
 }

src/Session.zig

@@ -678,7 +678,8 @@ fn reattach(self: *Session, address: []const u8) !Remote {
 }
 
 /// The environment a herdr-core shell is started with: this process's, as the
-/// local pty would hand on, on top of the core's own.
+/// local pty would hand on — and only that, since `hc.run` clears whatever the
+/// core's own environment would otherwise add.
 ///
 /// Passed whole rather than left to the core, whose environment is whatever
 /// the session that first started it had — not necessarily the one this app

src/View.zig

@@ -355,6 +355,56 @@ pub fn requestReview(self: *View) void {
     self.on_review(self.ctx);
 }
 
+/// What the pane header's agent-review button types into the new shell.
+const agent_review_command = "claude '/pr-review using /leave-review'";
+
+/// Open a Claude Code session beside `from`, in the directory `from` is working
+/// in, and have it review the tab's diff into the review pane.
+///
+/// `/leave-review` posts to this tab's review pane and stops if there isn't
+/// one, so a missing pane is opened first rather than leaving the session to
+/// fail on arrival. `from` is focused before that, so the review is bound to
+/// the same directory the session starts in.
+pub fn startAgentReview(self: *View, from: *Pane) void {
+    var buf: [std.fs.max_path_bytes]u8 = undefined;
+    const cwd = self.paneDirectory(from, &buf);
+
+    if (self.reviewPane() == null) {
+        self.setFocused(from);
+        self.requestReview();
+    }
+
+    self.addPane(.{ .terminal = .{
+        .cwd = cwd,
+        .command = agent_review_command,
+    } }) catch |err| {
+        std.log.err("failed to open the agent review: {s}", .{@errorName(err)});
+    };
+}
+
+/// The directory `pane` is working in, copied into `buf`, or empty to leave it
+/// to the shell's default.
+///
+/// A terminal answers for itself, a review pane with the repository it is bound
+/// to; a web pane has no directory, so it borrows the first terminal's in the
+/// tab.
+fn paneDirectory(self: *View, pane: *Pane, buf: []u8) []const u8 {
+    switch (pane.content) {
+        .terminal => |t| if (t.session.cwd(buf)) |dir| return dir,
+        .review => if (self.review_spec.repo.len > 0) {
+            const n = @min(buf.len, self.review_spec.repo.len);
+            @memcpy(buf[0..n], self.review_spec.repo[0..n]);
+            return buf[0..n];
+        },
+        .web => {},
+    }
+    for (self.panes.items) |other| {
+        const t = other.terminal() orelse continue;
+        if (t.session.cwd(buf)) |dir| return dir;
+    }
+    return "";
+}
+
 pub fn closePane(self: *View, pane: *Pane) void {
     if (self.closing) return;
 

src/hc.zig

@@ -152,7 +152,8 @@ pub const Attached = struct {
 pub const RunOptions = struct {
     argv: []const []const u8,
     cwd: ?[]const u8 = null,
-    /// `NAME=value`, set on top of the core's own environment.
+    /// `NAME=value`: the program's whole environment. Not merely added to the
+    /// core's — see `run`.
     env: []const []const u8 = &.{},
 };
 
@@ -174,6 +175,9 @@ const Core = struct {
 
     next_id: u64 = 1,
 
+    /// The core's process, asked for once per connection — see `coreEnviron`.
+    pid: ?i32 = null,
+
     streams: std.AutoHashMapUnmanaged(u32, Stream) = .empty,
 
     /// Set while `dispatch` runs, which must not nest.
@@ -651,13 +655,40 @@ fn writeAddress(address: []const u8) Error![]u8 {
 /// is forgotten straight away instead of piling up in `hc ls --all`. It has
 /// no parent: a pane is nobody's child, and with none the core types no notes
 /// about it into anything.
+///
+/// The program gets exactly `opts.env`, the way a child of this process would.
+/// The core only *adds* what it is given to its own environment, and its own is
+/// whatever the shell that happened to start it had: start it from a Claude
+/// Code session and every terminal it runs inherits that session's markers
+/// (and so every `claude` in them believes it is a subagent), start it from a
+/// nested compositor and they all inherit its `SWAYSOCK`. So the program is
+/// started through `env -u`, unsetting each variable the core has and `opts`
+/// does not — names only, so no value of the core's ends up in `hc ls`.
 pub fn run(alloc: std.mem.Allocator, opts: RunOptions) Error!Started {
     const self = connection() orelse return error.Unavailable;
 
+    const environ = coreEnviron(self);
+    defer if (environ) |e| glib.free(e.ptr);
+
+    var argv: std.ArrayListUnmanaged([]const u8) = .empty;
+    defer argv.deinit(gpa);
+    if (environ) |e| {
+        var it = std.mem.splitScalar(u8, e, 0);
+        while (it.next()) |entry| {
+            const eq = std.mem.indexOfScalar(u8, entry, '=') orelse continue;
+            const name = entry[0..eq];
+            if (name.len == 0 or coreOwned(name) or setIn(name, opts.env)) continue;
+            try argv.appendSlice(gpa, &.{ "-u", name });
+        }
+        if (argv.items.len > 0) try argv.insert(gpa, 0, envBinary());
+    }
+    try argv.appendSlice(gpa, opts.argv);
+
     var p: Json = undefined;
     p.init();
     defer p.deinit();
-    writeRunParams(&p.j, opts) catch return error.OutOfMemory;
+    writeRunParams(&p.j, .{ .argv = argv.items, .cwd = opts.cwd, .env = opts.env }) catch
+        return error.OutOfMemory;
 
     const reply = try call(self, "terminal.run", p.written());
     defer reply.deinit();
@@ -669,6 +700,55 @@ pub fn run(alloc: std.mem.Allocator, opts: RunOptions) Error!Started {
     };
 }
 
+/// The core's own environment, NUL-separated, to be freed with `glib.free`.
+/// Null when it can't be read, which leaves programs with the core's leftovers
+/// as before rather than failing to start them.
+///
+/// Read out of procfs because the API has no call for it. The core always runs
+/// on this machine here — nothing in this app starts terminals `on` another.
+fn coreEnviron(self: *Core) ?[]u8 {
+    const pid = self.pid orelse blk: {
+        const reply = call(self, "server.status", "{}") catch return null;
+        defer reply.deinit();
+        const pid: i32 = std.math.cast(i32, intField(result(reply), "pid") orelse return null) orelse
+            return null;
+        self.pid = pid;
+        break :blk pid;
+    };
+
+    var path_buf: [64]u8 = undefined;
+    const path = std.fmt.bufPrintSentinel(&path_buf, "/proc/{d}/environ", .{pid}, 0) catch return null;
+    var contents: [*]u8 = undefined;
+    var length: usize = 0;
+    if (glib.fileGetContents(path.ptr, &contents, &length, null) == 0) {
+        std.log.warn("herdr-core: could not read the core's environment; terminals inherit it", .{});
+        return null;
+    }
+    return contents[0..length];
+}
+
+/// Variables the core sets for each terminal itself, which must survive `env -u`
+/// even when its own environment happens to hold them too.
+fn coreOwned(name: []const u8) bool {
+    return std.mem.startsWith(u8, name, "HC_") or
+        std.mem.eql(u8, name, "TERM_PROGRAM") or
+        std.mem.eql(u8, name, "JOURNAL_STREAM");
+}
+
+/// Whether `env` (`NAME=value` entries) sets `name`.
+fn setIn(name: []const u8, env: []const []const u8) bool {
+    for (env) |entry| {
+        if (entry.len > name.len and entry[name.len] == '=' and std.mem.startsWith(u8, entry, name)) return true;
+    }
+    return false;
+}
+
+/// `env` by path where it has one, so finding it doesn't depend on the PATH of
+/// the very environment it is there to correct.
+fn envBinary() []const u8 {
+    return if (c.access("/usr/bin/env", X_OK) == 0) "/usr/bin/env" else "env";
+}
+
 fn writeRunParams(j: *std.json.Stringify, opts: RunOptions) !void {
     try j.beginObject();
     try j.objectField("argv");
@@ -822,6 +902,21 @@ pub fn detach(channel: u32) void {
     sendFrame(self, channel, .close, &.{}) catch {};
 }
 
+test "a variable counts as set only by its exact name" {
+    const env = [_][]const u8{ "PATH=/bin", "CLAUDE=1", "EMPTY=" };
+    try std.testing.expect(setIn("PATH", &env));
+    try std.testing.expect(setIn("EMPTY", &env));
+    try std.testing.expect(!setIn("PAT", &env));
+    try std.testing.expect(!setIn("CLAUDECODE", &env));
+}
+
+test "the core's own per-terminal variables are never unset" {
+    try std.testing.expect(coreOwned("HC_TERMINAL_ID"));
+    try std.testing.expect(coreOwned("TERM_PROGRAM"));
+    try std.testing.expect(!coreOwned("CLAUDE_CODE_CHILD_SESSION"));
+    try std.testing.expect(!coreOwned("SWAYSOCK"));
+}
+
 test {
     _ = frame;
 }

tools/fresh-shells.sh

@@ -0,0 +1,149 @@
+#!/usr/bin/env bash
+# Replace every shell playpen keeps in herdr-core with a fresh one, keeping the
+# layout.
+#
+#   tools/fresh-shells.sh        list what will be ended, ask, then do it
+#   tools/fresh-shells.sh -y     don't ask
+#
+# Acts on the hc session in $HC_SESSION (the default one when unset), and
+# relaunches with $PLAYPEN_LAUNCHER (the installed launcher when unset).
+#
+# A program's environment is fixed when it starts. Shells that came from a core
+# with something wrong in its environment keep it for good, and since playpen
+# reattaches to them on every launch, restarting playpen never clears it. And
+# `hc server restart` cannot either: it is a planned restart that keeps every
+# program running for the next core to adopt.
+#
+# So, in order:
+#   1. Quit playpen with SIGTERM, which saves the session first. This has to
+#      come before the shells go: a pane whose shell exits closes, and closing
+#      the last tab deletes the session.
+#   2. Kill every terminal playpen started in the core (label playpen=1).
+#   3. Start playpen again. It restores the session, finds those terminals gone,
+#      and opens a fresh shell in each pane's last directory.
+#
+# Whatever is running in those shells ends with them — agents included.
+
+set -euo pipefail
+
+unit_prefix="playpen-fresh-shells"
+launcher="${PLAYPEN_LAUNCHER:-$HOME/.local/bin/playpen}"
+
+# By path: the detached half runs under systemd, whose PATH has no ~/.local/bin.
+hc_bin=$(command -v hc || echo "$HOME/.local/bin/hc")
+
+die() { echo "fresh-shells: $*" >&2; exit 1; }
+say() { echo "fresh-shells: $*"; }
+
+# Every hc call runs without this process's Claude Code markers. Any hc command
+# starts a core when none is running, and a core started with them hands them
+# to every terminal it ever runs — which is how this script came to be needed.
+hc() {
+  local unset=()
+  local name
+  for name in $(compgen -e); do
+    case "$name" in CLAUDE*) unset+=(-u "$name") ;; esac
+  done
+  env "${unset[@]}" "$hc_bin" "$@"
+}
+
+[ -x "$hc_bin" ] || die "hc is not installed"
+command -v jq >/dev/null || die "needs jq"
+[ -x "$launcher" ] || die "no playpen launcher at $launcher (run mise run install)"
+
+yes=false
+[ "${1:-}" = "-y" ] && yes=true
+
+terminals() {
+  hc ls --label playpen=1 --json | jq -r '.terminals[].address'
+}
+
+# Whether this script is running in something step 1 or 2 would end: a pane of
+# playpen's own (a local shell, which quitting hangs up), or a terminal in the
+# core that step 2 kills.
+inside_playpen() {
+  if [ -n "${HC_TERMINAL_ID:-}" ] &&
+    [ "$(hc get "$HC_TERMINAL_ID" --json 2>/dev/null | jq -r '.labels.playpen // empty')" = 1 ]; then
+    return 0
+  fi
+  local pid=$$
+  while [ "$pid" -gt 1 ]; do
+    [ "$(cat "/proc/$pid/comm" 2>/dev/null)" = playpen ] && return 0
+    pid=$(awk '{print $4}' "/proc/$pid/stat" 2>/dev/null) || return 1
+  done
+  return 1
+}
+
+if [ -z "${PLAYPEN_FRESH_SHELLS_DETACHED:-}" ]; then
+  mapfile -t addrs < <(terminals)
+  if [ "${#addrs[@]}" -eq 0 ]; then
+    say "playpen has no terminals in herdr-core; nothing to do"
+    exit 0
+  fi
+
+  echo "These ${#addrs[@]} terminals will be ended, and everything running in them:"
+  for a in "${addrs[@]}"; do
+    hc get "$a" --json | jq -r '"  \(.address)  \((.foreground.name // "?"))\t\(.foreground.cwd // .cwd)"'
+  done
+  if ! $yes; then
+    read -r -p "Quit playpen, end them, and reopen with fresh shells? [y/N] " answer
+    case "$answer" in y | Y | yes) ;; *) say "nothing done"; exit 0 ;; esac
+  fi
+
+  # Hand the rest to systemd, which is not inside anything about to end.
+  if inside_playpen; then
+    unit="$unit_prefix-$$"
+    systemd-run --user --collect --quiet --unit="$unit" \
+      --setenv=PLAYPEN_FRESH_SHELLS_DETACHED=1 \
+      ${HC_SESSION:+--setenv=HC_SESSION="$HC_SESSION"} \
+      ${PLAYPEN_LAUNCHER:+--setenv=PLAYPEN_LAUNCHER="$PLAYPEN_LAUNCHER"} \
+      "$(readlink -f "$0")" -y
+    say "this shell is one of them, so the rest runs on its own as $unit."
+    say "playpen will close and come back in a few seconds."
+    say "if it doesn't: journalctl --user -u $unit"
+    exit 0
+  fi
+fi
+
+# 1. Quit playpen, saving the session. Only the ones on this hc session: a
+# playpen talking to another core has nothing to do with these terminals.
+mapfile -t pids < <(
+  for p in $(pgrep -u "$(id -u)" -x playpen); do
+    theirs=$(tr '\0' '\n' <"/proc/$p/environ" 2>/dev/null | sed -n 's/^HC_SESSION=//p')
+    [ "$theirs" = "${HC_SESSION:-}" ] && echo "$p"
+  done
+)
+if [ "${#pids[@]}" -gt 0 ]; then
+  say "quitting playpen (${pids[*]})"
+  kill -TERM "${pids[@]}"
+  for _ in $(seq 1 100); do
+    alive=false
+    for p in "${pids[@]}"; do kill -0 "$p" 2>/dev/null && alive=true; done
+    $alive || break
+    sleep 0.1
+  done
+  # Better to leave the shells alone than to end them under a playpen that is
+  # still up, which would close its panes and lose the layout.
+  $alive && die "playpen did not quit within 10s; nothing else done"
+fi
+
+# 2. End playpen's terminals.
+mapfile -t addrs < <(terminals)
+say "ending ${#addrs[@]} terminals"
+for a in "${addrs[@]}"; do
+  # `rm` terminals are gone the moment they exit, so kill reports them as
+  # "removed" even when it worked.
+  hc kill "$a" >/dev/null 2>&1 || true
+done
+for _ in $(seq 1 100); do
+  [ -z "$(terminals)" ] && break
+  sleep 0.1
+done
+left=$(terminals)
+[ -n "$left" ] && die "still running: $(echo "$left" | tr '\n' ' ')— not relaunching"
+
+# 3. Start playpen again, from systemd rather than from this shell, so it gets
+# the session's environment and not whatever this script was run from.
+systemd-run --user --collect --quiet --unit="app-playpen-$$" \
+  ${HC_SESSION:+--setenv=HC_SESSION="$HC_SESSION"} "$launcher"
+say "playpen relaunched with fresh shells"