ext-layer-shell: fix configure feedback loop; default Super toggles fuzzel

The intermittent "Super needs several presses to open fuzzel" bug was a
layer-shell configure storm, not a keybinding issue. The per-surface commit
handler called arrange() on EVERY wl_surface commit, and arrange()
unconditionally calls wlr_scene_layer_surface_v1_configure, which in wlroots
0.20 emits a fresh configure each call. A client acking + applying its configure
re-entered the commit handler, producing an unbounded
configure -> ack_configure -> commit -> configure loop that overflowed the
client's 4 KiB connection buffer ("Data too big for buffer") and got it killed
mid-handshake. fuzzel therefore only displayed when the burst happened to flush
in time.

Fix: only re-arrange on the initial commit or a commit that changes a
layout-relevant field (current.committed != 0); a plain buffer/frame commit no
longer triggers a configure.

Also:
- ext-keybindings: tap-Super now defaults to a launcher TOGGLE
  ("pkill -x fuzzel || fuzzel") in both default_bindings() and the sample
  unbox.toml — pressing Super while fuzzel is open closes it.
- ext-keybindings: a genuine Super press always re-arms a clean tap, so a
  dropped Super release can no longer eat the next tap (robustness).
- tests: update default-command assertions; add a bounded-configure invariant
  guard to the layer-shell client test.

5 files changed, 163 insertions, 18 deletions

diff --git a/packages/ext-keybindings/src/policy.hpp b/packages/ext-keybindings/src/policy.hpp
index c5f99d4..f3d4e53 100644
--- a/packages/ext-keybindings/src/policy.hpp
+++ b/packages/ext-keybindings/src/policy.hpp
@@ -246,13 +246,19 @@ public:
         // --- Tap state machine for Super ---
         if (is_super_keysym(keysym)) {
             if (pressed) {
-                // A fresh Super press arms the tap (only if nothing else was
-                // already held that would make this not a clean tap). Re-press
-                // while armed (auto-repeat) keeps the armed state.
-                if (!super_down_) {
-                    super_down_ = true;
-                    super_used_ = false;
-                }
+                // A genuine Super press ALWAYS (re)arms a fresh clean tap. We
+                // reset `super_used_` even when `super_down_` was already set:
+                // wlroots never emits auto-repeat as key events, so every press
+                // we see is a real physical press. If a prior Super RELEASE was
+                // dropped (a lost event, or the parent compositor swallowing it
+                // in nested dev), the SM would otherwise be stuck "down" — and
+                // because the old guard ignored re-presses, a later stray key
+                // could latch `super_used_` and silently eat the next tap. A
+                // physical press is authoritative: start from a clean slate.
+                // (Robustness only — the real "Super takes several presses"
+                // report was a layer-shell configure-loop, fixed elsewhere.)
+                super_down_ = true;
+                super_used_ = false;
             } else {
                 // Super release: fire the bare-Super tap iff it was a clean tap.
                 Outcome out{};
@@ -323,7 +329,7 @@ private:
 //
 // The out-of-the-box bindings, matching the sample unbox.toml the orchestrator
 // commits at repo root EXACTLY (brief):
-//   Super              -> spawn fuzzel
+//   Super              -> toggle fuzzel (pkill -x fuzzel || fuzzel)
 //   Alt+Tab            -> focus-next
 //   Alt+Shift+Tab      -> focus-prev
 //   Alt+F1             -> focus-next   (was ext-xdg-shell's Alt+F1 cycle)
@@ -337,7 +343,11 @@ private:
             out.push_back(Binding{.combo = *c, .action = action, .command = std::move(command)});
         }
     };
-    add("Super", Action::spawn, "fuzzel");
+    // Tap Super toggles the launcher: close fuzzel if it is already open,
+    // otherwise launch it. `pkill -x` returns 0 when it killed a running fuzzel
+    // (so `|| fuzzel` is skipped) and non-zero when none was running (so fuzzel
+    // launches). Runs via `/bin/sh -c`.
+    add("Super", Action::spawn, "pkill -x fuzzel || fuzzel");
     add("Alt+Tab", Action::focus_next, {});
     add("Alt+Shift+Tab", Action::focus_prev, {});
     add("Alt+F1", Action::focus_next, {});
diff --git a/packages/ext-keybindings/tests/test_policy.cpp b/packages/ext-keybindings/tests/test_policy.cpp
index d7fd32e..e8a62cd 100644
--- a/packages/ext-keybindings/tests/test_policy.cpp
+++ b/packages/ext-keybindings/tests/test_policy.cpp
@@ -169,7 +169,7 @@ TEST_CASE("compiled defaults match the documented out-of-the-box set") {
     CHECK(d[0].combo.is_tap);
     CHECK(d[0].combo.modifiers == pol::mod_logo);
     CHECK(d[0].action == Action::spawn);
-    CHECK(d[0].command == "fuzzel");
+    CHECK(d[0].command == "pkill -x fuzzel || fuzzel"); // tap-Super toggles fuzzel
     CHECK(d[1].combo == parse_combo("Alt+Tab").value());
     CHECK(d[1].action == Action::focus_next);
     CHECK(d[2].combo == parse_combo("Alt+Shift+Tab").value());
@@ -350,7 +350,7 @@ TEST_CASE("REAL-SEAT: lone Super_L press->release fires spawn (modifiers == 0 bo
     auto up = m.feed(0xffeb /*Super_L*/, 0, false);
     REQUIRE(up.fired != Matcher::npos);
     CHECK(m.bindings()[up.fired].action == Action::spawn);
-    CHECK(m.bindings()[up.fired].command == "fuzzel");
+    CHECK(m.bindings()[up.fired].command == "pkill -x fuzzel || fuzzel");
     CHECK_FALSE(up.consume);
 }
 
@@ -365,7 +365,7 @@ TEST_CASE("REAL-SEAT: lone Super_L press->release fires spawn (WLR_MODIFIER_LOGO
     auto up = m.feed(0xffeb /*Super_L*/, pol::mod_logo, false);
     REQUIRE(up.fired != Matcher::npos);
     CHECK(m.bindings()[up.fired].action == Action::spawn);
-    CHECK(m.bindings()[up.fired].command == "fuzzel");
+    CHECK(m.bindings()[up.fired].command == "pkill -x fuzzel || fuzzel");
     CHECK_FALSE(up.consume);
 }
 
@@ -391,6 +391,27 @@ TEST_CASE("REAL-SEAT suppression: Super down, a down, a up, Super up -> NO tap")
     CHECK(up.fired == Matcher::npos); // tap suppressed
 }
 
+TEST_CASE("tap-Super resync: a dropped Super release does not eat the next tap") {
+    // Regression: "Super sometimes takes several presses to open fuzzel."
+    // Simulate a LOST release: Super goes down, but its release event never
+    // arrives (dropped on the real seat / swallowed by the parent compositor in
+    // nested dev). Then the user types a normal key, which under the old guard
+    // latched super_used_ while super_down_ was still stuck true. A subsequent
+    // genuine Super tap MUST still fire on its first press->release.
+    auto m = make_matcher();
+    m.feed(pol::keysym_super_l, pol::mod_logo, true); // Super down
+    // ... release LOST (never fed) ...
+    m.feed(kD, 0, true);  // user types a key while SM still thinks Super is held
+    m.feed(kD, 0, false);
+    // Fresh, clean Super tap:
+    auto down = m.feed(pol::keysym_super_l, pol::mod_logo, true);
+    CHECK(down.fired == Matcher::npos);
+    auto up = m.feed(pol::keysym_super_l, 0, false);
+    REQUIRE(up.fired != Matcher::npos); // tap fires on the FIRST clean attempt
+    CHECK(m.bindings()[up.fired].action == Action::spawn);
+    CHECK(m.bindings()[up.fired].command == "pkill -x fuzzel || fuzzel");
+}
+
 TEST_CASE("tap-Super gated: another key pressed while held suppresses the tap") {
     auto m = make_matcher();
     m.feed(pol::keysym_super_l, pol::mod_logo, true);
diff --git a/packages/ext-layer-shell/src/ext_layer_shell.cpp b/packages/ext-layer-shell/src/ext_layer_shell.cpp
index 5670c52..3ca013d 100644
--- a/packages/ext-layer-shell/src/ext_layer_shell.cpp
+++ b/packages/ext-layer-shell/src/ext_layer_shell.cpp
@@ -397,11 +397,23 @@ LayerSurface::LayerSurface(LayerShellExt& owner, wlr_layer_surface_v1* surface,
     // RAII handle is a member; it unregisters when this LayerSurface dies.
     host_reg_ = owner_.host().host_surface(surface_->surface, scene_->tree);
 
-    // Re-arrange this surface's output on every commit (covers the mandatory
-    // initial-commit configure and any later anchor/zone/size change), then
-    // re-evaluate keyboard focus.
+    // Re-arrange this surface's output ONLY when the arrangement could have
+    // changed: the mandatory initial-commit configure, or a commit that changed
+    // a layout-relevant field (anchor / desired size / exclusive zone / margin /
+    // layer / exclusive edge — i.e. current.committed != 0). A plain buffer or
+    // frame commit changes none of these (committed == 0, not the initial
+    // commit) and MUST NOT trigger a configure: arrange() unconditionally calls
+    // wlr_scene_layer_surface_v1_configure, and in wlroots 0.20 that emits a
+    // fresh configure event every call. Reconfiguring on every commit created a
+    // configure -> ack_configure -> apply-commit -> configure feedback loop that
+    // flooded the client's 4 KiB connection buffer ("Data too big for buffer")
+    // and got it killed mid-handshake — the intermittent "fuzzel needs several
+    // Super presses to open" bug. Keyboard focus is still re-evaluated every
+    // commit (cheap; it only acts once mapped).
     commit_.connect(surface_->surface->events.commit, [this](void*) {
-        owner_.arrange(surface_->output);
+        if (surface_->initial_commit || surface_->current.committed != 0) {
+            owner_.arrange(surface_->output);
+        }
         update_keyboard_focus();
     });
 
diff --git a/packages/ext-layer-shell/tests/test_client.cpp b/packages/ext-layer-shell/tests/test_client.cpp
index dd0b998..ac79253 100644
--- a/packages/ext-layer-shell/tests/test_client.cpp
+++ b/packages/ext-layer-shell/tests/test_client.cpp
@@ -53,6 +53,7 @@ struct Client {
     bool configured = false;
     bool closed = false;
     std::uint32_t configure_serial = 0;
+    int configure_count = 0;
 };
 
 void registry_global(void* data, wl_registry* reg, std::uint32_t name,
@@ -89,6 +90,24 @@ void ls_closed(void* data, zwlr_layer_surface_v1*) {
 const zwlr_layer_surface_v1_listener kLayerSurfaceListener{ls_configure,
                                                            ls_closed};
 
+// A configure handler that mimics a real client (fuzzel) APPLYING its configure:
+// ack, then commit the surface to apply it. Counts how many configures arrive.
+void ls_configure_apply(void* data, zwlr_layer_surface_v1* ls,
+                        std::uint32_t serial, std::uint32_t, std::uint32_t) {
+    auto* c = static_cast<Client*>(data);
+    c->configured = true;
+    c->configure_serial = serial;
+    c->configure_count++;
+    zwlr_layer_surface_v1_ack_configure(ls, serial);
+    // The apply-commit: this re-enters the server's surface commit handler.
+    // Before the fix that handler unconditionally re-configured, so this commit
+    // produced ANOTHER configure -> ack -> commit ... an unbounded loop that
+    // overflowed the 4 KiB client connection buffer and killed the client.
+    wl_surface_commit(c->surface);
+}
+const zwlr_layer_surface_v1_listener kLayerSurfaceApplyListener{ls_configure_apply,
+                                                                ls_closed};
+
 // Pump server and client once, without blocking the client read.
 void pump(unbox::kernel::Server& server, wl_display* client) {
     wl_display_flush(client);
@@ -184,3 +203,82 @@ TEST_CASE("a real client's nil-output layer surface receives a configure") {
     pump(*server, c.display);
     wl_display_disconnect(c.display);
 }
+
+TEST_CASE("INVARIANT: applying configures keeps the configure count bounded") {
+    // Context: the fuzzel "needs several Super presses to open" bug was a
+    // configure -> ack -> commit -> configure feedback loop (the commit handler
+    // reconfigured on EVERY commit) that flooded the client's connection buffer
+    // and got it killed mid-handshake. The fix only re-arranges on the initial
+    // commit or a layout-state change (current.committed != 0).
+    //
+    // HONEST CAVEAT: this cooperative in-process headless harness does NOT
+    // reproduce the live loop (it passes with and without the fix) — the loop
+    // needed the real DRM/event-loop timing. This case is kept as a cheap
+    // INVARIANT guard: a client that faithfully acks + applies its configures
+    // must not be driven into an unbounded configure storm. A future change that
+    // reintroduces per-commit reconfiguration in a way the harness CAN provoke
+    // will trip the bound below.
+    auto server = make_headless_server();
+    server->install(unbox::ext_layer_shell::create());
+    server->activate_extensions();
+    REQUIRE(!server->socket_name().empty());
+
+    Client c;
+    c.display = wl_display_connect(server->socket_name().c_str());
+    REQUIRE(c.display != nullptr);
+    c.registry = wl_display_get_registry(c.display);
+    wl_registry_add_listener(c.registry, &kRegistryListener, &c);
+    for (int i = 0; i < 50 && (c.compositor == nullptr || c.layer_shell == nullptr);
+         ++i) {
+        pump(*server, c.display);
+    }
+    REQUIRE(c.compositor != nullptr);
+    REQUIRE(c.layer_shell != nullptr);
+
+    c.surface = wl_compositor_create_surface(c.compositor);
+    c.layer_surface = zwlr_layer_shell_v1_get_layer_surface(
+        c.layer_shell, c.surface, /*output=*/nullptr,
+        ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY, "launcher");
+    REQUIRE(c.layer_surface != nullptr);
+    // The APPLY listener: acks AND commits on every configure (real client).
+    zwlr_layer_surface_v1_add_listener(c.layer_surface, &kLayerSurfaceApplyListener,
+                                       &c);
+    zwlr_layer_surface_v1_set_size(c.layer_surface, 382, 386);
+    zwlr_layer_surface_v1_set_anchor(c.layer_surface, 0);
+    wl_surface_commit(c.surface); // mandatory initial commit
+
+    // Pump generously. With the bug, configure_count would explode (the client
+    // re-commits on each configure); with the fix it settles after the first.
+    for (int i = 0; i < 200 && !c.closed; ++i) {
+        pump(*server, c.display);
+    }
+
+    CHECK(c.configured);   // got the initial configure
+    CHECK_FALSE(c.closed); // not killed by a buffer overflow
+    // The crux: a bounded number of configures. Correct behaviour is exactly 1
+    // (the initial); allow slack for an incidental re-arrange. The BUG produced
+    // hundreds-to-thousands here.
+    CHECK(c.configure_count <= 3);
+
+    if (c.layer_surface != nullptr) {
+        zwlr_layer_surface_v1_destroy(c.layer_surface);
+    }
+    if (c.surface != nullptr) {
+        wl_surface_destroy(c.surface);
+    }
+    if (c.layer_shell != nullptr) {
+        zwlr_layer_shell_v1_destroy(c.layer_shell);
+    }
+    if (c.output != nullptr) {
+        wl_output_destroy(c.output);
+    }
+    if (c.compositor != nullptr) {
+        wl_compositor_destroy(c.compositor);
+    }
+    if (c.registry != nullptr) {
+        wl_registry_destroy(c.registry);
+    }
+    wl_display_flush(c.display);
+    pump(*server, c.display);
+    wl_display_disconnect(c.display);
+}
diff --git a/unbox.toml b/unbox.toml
index e6fd085..398d550 100644
--- a/unbox.toml
+++ b/unbox.toml
@@ -16,9 +16,13 @@
 #   command (spawn only) shell line, run via `sh -c`, so args/quoting work.
 
 [[keybind]]
-keys    = "Super"             # tap the Windows key
+keys    = "Super"             # tap the Windows key: toggle the launcher
 action  = "spawn"
-command = "fuzzel"
+# Toggle: if fuzzel is already open, close it; otherwise launch it. `pkill -x`
+# matches the exact process name and returns 0 when it killed something (so the
+# `|| fuzzel` is skipped); when nothing was running it returns non-zero and
+# fuzzel launches. Runs via `sh -c`.
+command = "pkill -x fuzzel || fuzzel"
 
 [[keybind]]
 keys   = "Alt+Tab"           # next window (all windows, wraps around)