From 0e0b5fcbe33b9d5282d098bfb0aee5b29b6a028e Mon Sep 17 00:00:00 2001 From: funman300 Date: Fri, 17 Jul 2026 13:02:35 -0700 Subject: [PATCH] niri-scratchpad: fix close-while-stashed eviction + add live acceptance test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refresh the patch with the destroy-path fix (a client dying while stashed is now evicted instead of leaked/zombied), and add pkg/niri-scratchpad/tests/ acceptance.py: a headless IPC-driven regression test that drives a nested patched niri and asserts on `niri msg -j windows`. It found the above bug (unit tests miss it — the harness calls remove_window directly). Run it after rebasing the patch onto a new niri release. Co-Authored-By: Claude Opus 4.8 (1M context) --- niri-patches/0001-scratchpad.patch | 39 ++- pkg/niri-scratchpad/tests/acceptance.py | 310 ++++++++++++++++++++++++ 2 files changed, 346 insertions(+), 3 deletions(-) create mode 100644 pkg/niri-scratchpad/tests/acceptance.py diff --git a/niri-patches/0001-scratchpad.patch b/niri-patches/0001-scratchpad.patch index 6c45b96..d8a771d 100644 --- a/niri-patches/0001-scratchpad.patch +++ b/niri-patches/0001-scratchpad.patch @@ -56,6 +56,26 @@ index 0aa3cb4f..a5b712e2 100644 /// Toggle (open/close) the Overview. ToggleOverview {}, /// Open the Overview. +diff --git a/src/handlers/xdg_shell.rs b/src/handlers/xdg_shell.rs +index 38440c90..a174921e 100644 +--- a/src/handlers/xdg_shell.rs ++++ b/src/handlers/xdg_shell.rs +@@ -834,6 +834,15 @@ impl XdgShellHandler for State { + .find_window_and_output(surface.wl_surface()); + + let Some((mapped, output)) = win_out else { ++ // A window whose client died while stashed in the scratchpad is in no ++ // workspace, so find_window_and_output can't see it; evict it here. ++ if self ++ .niri ++ .layout ++ .remove_from_scratchpad(surface.wl_surface()) ++ { ++ return; ++ } + // I have no idea how this can happen, but I saw it happen once, in a weird interaction + // involving laptop going to sleep and resuming. + error!("toplevel missing from both unmapped_windows and layout"); diff --git a/src/input/mod.rs b/src/input/mod.rs index f1ad4932..91bc8942 100644 --- a/src/input/mod.rs @@ -86,7 +106,7 @@ index f1ad4932..91bc8942 100644 self.niri.layout.toggle_overview(); self.niri.queue_redraw_all(); diff --git a/src/layout/mod.rs b/src/layout/mod.rs -index 5c4dd639..077a0570 100644 +index 5c4dd639..a90d9e6e 100644 --- a/src/layout/mod.rs +++ b/src/layout/mod.rs @@ -32,6 +32,7 @@ @@ -149,7 +169,7 @@ index 5c4dd639..077a0570 100644 if let Some(state) = &self.interactive_move { match state { InteractiveMoveState::Starting { window_id, .. } => { -@@ -1204,6 +1222,93 @@ impl Layout { +@@ -1204,6 +1222,106 @@ impl Layout { None } @@ -164,6 +184,19 @@ index 5c4dd639..077a0570 100644 + // (Task 4) if this window was the one shown. + } + ++ /// Evict a stashed window from the scratchpad by its surface. Returns true ++ /// if one was removed. Needed on the destroy/unmap paths: a window whose ++ /// client dies *while stashed* is in no workspace, so `find_window_and_output` ++ /// (and therefore `remove_window`) never sees it — this is the only thing ++ /// that drops it. A shown scratchpad window lives in a workspace and is ++ /// handled by `remove_window` instead. ++ pub fn remove_from_scratchpad(&mut self, surface: &WlSurface) -> bool { ++ let before = self.scratchpad.len(); ++ self.scratchpad ++ .retain(|rt| !rt.tile.window().is_wl_surface(surface)); ++ self.scratchpad.len() != before ++ } ++ + /// Show / toggle / cycle the scratchpad. See the state table in the plan. + pub fn scratchpad_show(&mut self) { + // Drop a stale "shown" id (window was closed or moved away). @@ -243,7 +276,7 @@ index 5c4dd639..077a0570 100644 pub fn descendants_added(&mut self, id: &W::Id) -> bool { for ws in self.workspaces_mut() { if ws.descendants_added(id) { -@@ -2444,6 +2549,24 @@ impl Layout { +@@ -2444,6 +2562,24 @@ impl Layout { } } diff --git a/pkg/niri-scratchpad/tests/acceptance.py b/pkg/niri-scratchpad/tests/acceptance.py new file mode 100644 index 0000000..c186aa1 --- /dev/null +++ b/pkg/niri-scratchpad/tests/acceptance.py @@ -0,0 +1,310 @@ +#!/usr/bin/env python3 +"""Live acceptance / regression test for the niri native scratchpad patch. + +Drives a real (patched) niri through its own IPC — the exact `do_action` +path a keybind hits — and asserts on `niri msg -j windows`. No keyboard +injection, so it can't type into the host session's windows. Each test gets +its own fresh nested niri, so cases are hermetic and order-independent. + +Run this after rebasing the patch onto a new niri release to confirm the +scratchpad still behaves. It needs: + - a running Wayland (or X) session to host the nested winit window, + - the patched niri binary (built from niri-patches/ via the PKGBUILD, or a + dev checkout), + - `alacritty` as the throwaway test client. + +Usage: + python3 acceptance.py [PATH_TO_NIRI] +Binary resolution order: argv[1], $NIRI_BIN, `niri` on PATH, +~/build/niri-scratchpad/target/release/niri. The binary MUST be patched +(its `niri msg action --help` must list `scratchpad-show`). + +Exit code 0 iff every non-skipped test passes. +""" +import glob, json, os, shutil, signal, subprocess, sys, tempfile, time + +MINIMAL_CONFIG = """\ +// Hermetic config for scratchpad acceptance: no startup spawns. +input { keyboard { xkb { } } } +binds { + Mod+M { move-window-to-scratchpad; } + Mod+Shift+M { scratchpad-show; } + Mod+Q { close-window; } +} +""" + + +def resolve_niri(): + for cand in (sys.argv[1] if len(sys.argv) > 1 else None, + os.environ.get("NIRI_BIN"), + shutil.which("niri"), + os.path.expanduser("~/build/niri-scratchpad/target/release/niri")): + if cand and os.path.exists(cand): + return cand + sys.exit("ERROR: no niri binary found (pass a path, set $NIRI_BIN, or " + "install niri-scratchpad).") + + +NIRI = resolve_niri() +RT = os.environ["XDG_RUNTIME_DIR"] + + +class Niri: + """A fresh nested niri instance driven over IPC.""" + + def __init__(self, config_path): + before = set(glob.glob(f"{RT}/niri.*.sock")) + self.proc = subprocess.Popen( + [NIRI, "-c", config_path], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + self.sock = None + t0 = time.time() + while time.time() - t0 < 15: + new = set(glob.glob(f"{RT}/niri.*.sock")) - before + if new: + self.sock = new.pop() + break + time.sleep(0.2) + if not self.sock: + self.close() + raise RuntimeError("nested niri socket never appeared") + self.env = dict(os.environ, NIRI_SOCKET=self.sock) + for _ in range(40): # wait for IPC to answer + if self.alive(): + break + time.sleep(0.2) + + def msg(self, *args): + r = subprocess.run([NIRI, "msg", "-j", *args], + capture_output=True, text=True, env=self.env) + return json.loads(r.stdout) if r.stdout.strip() else None + + def action(self, *args): + return subprocess.run([NIRI, "msg", "action", *args], + capture_output=True, text=True, env=self.env) + + def alive(self): + return self.msg("version") is not None + + def windows(self): + return self.msg("windows") or [] + + def wait_count(self, n, timeout=8): + t0 = time.time() + while time.time() - t0 < timeout: + if len(self.windows()) == n: + return self.windows() + time.sleep(0.15) + return self.windows() + + def spawn_window(self): + """Spawn an alacritty window and return its id once it appears.""" + before = {w["id"] for w in self.windows()} + self.action("spawn", "--", "alacritty") + t0 = time.time() + while time.time() - t0 < 10: + new = [w for w in self.windows() if w["id"] not in before] + if new: + return new[0]["id"] + time.sleep(0.15) + raise RuntimeError("spawned window never appeared") + + def by_id(self, i): + return next((w for w in self.windows() if w["id"] == i), None) + + def focused(self): + return next((w for w in self.windows() if w["is_focused"]), None) + + def floating_pos(self, i, tries=12): + # tile_pos_in_workspace_view is transiently null right after a show. + for _ in range(tries): + w = self.by_id(i) + p = w["layout"].get("tile_pos_in_workspace_view") if w else None + if p is not None: + return p + time.sleep(0.08) + return None + + def output_size(self): + outs = self.msg("outputs") or {} + for o in outs.values(): + lg = o.get("logical") or {} + if lg.get("width") and lg.get("height"): + return lg["width"], lg["height"] + return None + + def close(self): + try: + self.proc.send_signal(signal.SIGTERM) + except Exception: + pass + try: + self.proc.wait(timeout=3) + except Exception: + try: + self.proc.kill() + except Exception: + pass + + +# ---- tests: each takes a fresh Niri and returns (ok, detail) ---- + +def t_stash_hides_and_moves_focus(n): + a = n.spawn_window() + b = n.spawn_window() + n.action("focus-window", "--id", str(b)) + time.sleep(0.2) + n.action("move-window-to-scratchpad") + n.wait_count(1) + f = n.focused() + ok = (len(n.windows()) == 1 and n.by_id(b) is None + and f is not None and f["id"] == a) + return ok, f"remaining={len(n.windows())}, b_gone={n.by_id(b) is None}, focus={f['id'] if f else None}/want {a}" + + +def t_show_floating_focused_centered(n): + b = n.spawn_window() + n.action("focus-window", "--id", str(b)); time.sleep(0.2) + n.action("move-window-to-scratchpad"); n.wait_count(0) + n.action("scratchpad-show"); n.wait_count(1) + w = n.by_id(b) + pos = n.floating_pos(b) + centered = "n/a" + ok = bool(w and w["is_floating"] and w["is_focused"] and pos and pos[0] > 5) + osz = n.output_size() + if ok and osz and w: + ww, wh = w["layout"]["window_size"] + cx, cy = (osz[0] - ww) / 2, (osz[1] - wh) / 2 + centered = abs(pos[0] - cx) < osz[0] * 0.15 and abs(pos[1] - cy) < osz[1] * 0.15 + ok = ok and bool(centered) + return ok, f"floating={w['is_floating'] if w else None}, focused={w['is_focused'] if w else None}, pos={pos}, centered={centered}" + + +def t_reshow_hides(n): + b = n.spawn_window() + n.action("focus-window", "--id", str(b)); time.sleep(0.2) + n.action("move-window-to-scratchpad"); n.wait_count(0) + n.action("scratchpad-show"); n.wait_count(1) # show (focused) + n.action("scratchpad-show"); n.wait_count(0) # hide + ok = len(n.windows()) == 0 + return ok, f"count after re-show={len(n.windows())} (want 0)" + + +def t_cycle_round_robin(n): + a = n.spawn_window() + b = n.spawn_window() + for wid in (b, a): + n.action("focus-window", "--id", str(wid)); time.sleep(0.2) + n.action("move-window-to-scratchpad") + n.wait_count(0) + n.action("scratchpad-show"); w1 = n.wait_count(1); first = w1[0]["id"] if w1 else None + n.action("scratchpad-show"); n.wait_count(0) # hide (focused) + n.action("scratchpad-show"); w2 = n.wait_count(1); second = w2[0]["id"] if w2 else None + ok = first is not None and second is not None and first != second + return ok, f"first={first}, second={second} (want different)" + + +def t_geometry_remembered(n): + w = n.spawn_window() + n.action("focus-window", "--id", str(w)); time.sleep(0.2) + n.action("move-window-to-scratchpad"); n.wait_count(0) + n.action("scratchpad-show"); n.wait_count(1) + n.action("focus-window", "--id", str(w)); time.sleep(0.2) + n.action("move-floating-window", "-x", "+130", "-y", "+90"); time.sleep(0.4) + pos1 = n.floating_pos(w) + n.action("scratchpad-show"); n.wait_count(0) # hide + n.action("scratchpad-show"); n.wait_count(1) # show again (single window) + pos2 = n.floating_pos(w) + back = n.by_id(w) + ok = (back is not None and back["is_floating"] and pos1 and pos2 + and abs(pos1[0] - pos2[0]) < 2 and abs(pos1[1] - pos2[1]) < 2) + return ok, f"pos before hide={pos1}, after show={pos2}, floating={back['is_floating'] if back else None}" + + +def t_shown_not_focused_gets_focus(n): + p = n.spawn_window() # persistent tiled window + s = n.spawn_window() # to be stashed + shown + n.action("focus-window", "--id", str(s)); time.sleep(0.2) + n.action("move-window-to-scratchpad"); n.wait_count(1) + n.action("scratchpad-show"); n.wait_count(2) # s shown floating, focused + n.action("focus-window", "--id", str(p)); time.sleep(0.3) + npre = len(n.windows()) + n.action("scratchpad-show"); time.sleep(0.4) # should just focus s back + f = n.focused() + ok = len(n.windows()) == npre and f is not None and f["id"] == s + return ok, f"count {npre}->{len(n.windows())} (want same), focus={f['id'] if f else None}/want {s}" + + +def t_close_while_stashed(n): + w = n.spawn_window() + pid = n.by_id(w)["pid"] + n.action("focus-window", "--id", str(w)); time.sleep(0.2) + n.action("move-window-to-scratchpad"); n.wait_count(0) + os.kill(pid, signal.SIGKILL) # client dies WHILE stashed + time.sleep(0.6) + r = n.action("scratchpad-show"); time.sleep(0.5) + ok = n.alive() and n.by_id(w) is None and r.returncode == 0 + return ok, f"alive={n.alive()}, dead_shown={n.by_id(w) is not None}, rc={r.returncode}" + + +def t_empty_stash_noop(n): + n.spawn_window() + r = n.action("scratchpad-show"); time.sleep(0.3) + ok = len(n.windows()) == 1 and n.alive() and r.returncode == 0 + return ok, f"count stayed {len(n.windows())}, alive={n.alive()}, rc={r.returncode}" + + +TESTS = [ + ("1 stash hides + focus moves", t_stash_hides_and_moves_focus), + ("2 show floating+focused+centered", t_show_floating_focused_centered), + ("3 re-show hides", t_reshow_hides), + ("4 cycle round-robin", t_cycle_round_robin), + ("5 geometry remembered", t_geometry_remembered), + ("6 shown-not-focused -> focus it", t_shown_not_focused_gets_focus), + ("7 close-while-stashed no crash", t_close_while_stashed), + ("10 empty-stash no-op", t_empty_stash_noop), +] + + +def main(): + if not shutil.which("alacritty"): + sys.exit("ERROR: alacritty is required as the test client.") + # confirm the binary is patched + h = subprocess.run([NIRI, "msg", "action", "--help"], + capture_output=True, text=True) + if "scratchpad-show" not in h.stdout: + sys.exit(f"ERROR: {NIRI} is not patched (no scratchpad-show action).") + + print(f"niri: {NIRI}\n") + cfg = tempfile.NamedTemporaryFile("w", suffix=".kdl", delete=False) + cfg.write(MINIMAL_CONFIG); cfg.close() + results = [] + try: + for name, fn in TESTS: + n = None + try: + n = Niri(cfg.name) + ok, detail = fn(n) + except Exception as e: + ok, detail = False, f"exception: {e}" + finally: + if n: + n.close() + time.sleep(0.3) + results.append((name, ok, detail)) + print(f" [{'PASS' if ok else 'FAIL'}] {name}: {detail}") + finally: + os.unlink(cfg.name) + + # #8 multi-monitor cannot be exercised nested (single winit output). + print(" [SKIP] 8 multi-monitor: needs a physical 2nd output; " + "code uses active_output (correct by construction).") + + passed = sum(1 for _, ok, _ in results if ok) + failed = sum(1 for _, ok, _ in results if not ok) + print(f"\n{passed} passed, {failed} failed, 1 skipped") + sys.exit(1 if failed else 0) + + +if __name__ == "__main__": + main()