ext-keybindings: hot-reload unbox.toml (live config, no restart)

Editing the config now re-applies keybindings live, via the kernel's watch_file
service. In activate() we watch the effective config path (the create() arg, else
~/.config/unbox/unbox.toml) — even if it doesn't exist yet, so creating it later is
picked up — holding the FileWatch as a member. On change, reload_config() re-reads
+ re-parses (the existing pure toml core) and SWAPs the live binding table the
key_filter link matches against (matcher_), so new bindings apply with no
re-subscribe. A malformed / unreadable / mid-edit-broken file KEEPS the current
bindings and logs one warning — the session never loses working keys, never throws.

Real-seat verified: editing the command (fuzzel->foot) logged "config reloaded
(5 binding(s))" live; a deliberately broken file logged "reload failed; keeping
current bindings" with the session staying ALIVE; restoring it recovered. Added a
pure reload-semantics doctest (A->B swap; malformed keeps prior). ext-keybindings
2/2 green on build + build-asan. Edits confined to packages/ext-keybindings/.

4 files changed, 210 insertions, 5 deletions

diff --git a/packages/ext-keybindings/src/config.cpp b/packages/ext-keybindings/src/config.cpp
index 4d1045f..726b24b 100644
--- a/packages/ext-keybindings/src/config.cpp
+++ b/packages/ext-keybindings/src/config.cpp
@@ -101,4 +101,25 @@ auto load_from_string(std::string_view toml_text) -> LoadResult {
     return result;
 }
 
+auto reload_bindings(const std::vector<policy::Binding>& current, std::string_view toml_text)
+    -> ReloadDecision {
+    ReloadDecision decision;
+    LoadResult loaded = load_from_string(toml_text);
+    decision.warnings = std::move(loaded.warnings);
+
+    // KEEP-OLD on a syntax error or on a parse that produced no usable bindings
+    // (an empty file, a [[keybind]]-less doc, or one where every entry was
+    // skipped). Either way the live table must remain the user's working keys.
+    if (loaded.parse_error || loaded.bindings.empty()) {
+        decision.bindings = current; // unchanged copy
+        decision.swapped = false;
+        return decision;
+    }
+
+    // SUCCESS: the new table replaces the live one (the swap).
+    decision.bindings = std::move(loaded.bindings);
+    decision.swapped = true;
+    return decision;
+}
+
 } // namespace unbox::ext_keybindings::config
diff --git a/packages/ext-keybindings/src/config.hpp b/packages/ext-keybindings/src/config.hpp
index d15babd..967a997 100644
--- a/packages/ext-keybindings/src/config.hpp
+++ b/packages/ext-keybindings/src/config.hpp
@@ -40,4 +40,25 @@ struct LoadResult {
 // ONE entry (with a warning) and never abort the rest.
 [[nodiscard]] auto load_from_string(std::string_view toml_text) -> LoadResult;
 
+// The PURE reload decision (the swap-or-keep-old policy of hot-reload, with no
+// file I/O and no event loop). Re-parse `toml_text` via load_from_string and
+// decide which binding table stays LIVE:
+//   * SUCCESS  -> the file parsed AND yielded at least one usable binding:
+//                 `bindings` is the NEW table (the swap), `swapped` is true.
+//   * KEEP-OLD -> a toml parse error, OR zero usable bindings: `bindings` is a
+//                 COPY of `current` unchanged (`swapped` false). A half-saved or
+//                 broken edit must never drop the user's working keys.
+// `warnings` carries every per-entry / parse warning from the underlying load so
+// the glue can log exactly one summarizing line. NEVER throws (the toml parse
+// error is caught inside load_from_string). The glue applies the SAME keep-old
+// rule when the file is merely unreadable (it then does not call this at all).
+struct ReloadDecision {
+    std::vector<policy::Binding> bindings; // the table to install (new or kept)
+    std::vector<std::string> warnings;     // diagnostics from the parse
+    bool swapped = false;                  // true iff a new table replaced current
+};
+
+[[nodiscard]] auto reload_bindings(const std::vector<policy::Binding>& current,
+                                   std::string_view toml_text) -> ReloadDecision;
+
 } // namespace unbox::ext_keybindings::config
diff --git a/packages/ext-keybindings/src/extension.cpp b/packages/ext-keybindings/src/extension.cpp
index b582a31..273fd47 100644
--- a/packages/ext-keybindings/src/extension.cpp
+++ b/packages/ext-keybindings/src/extension.cpp
@@ -147,6 +147,7 @@ class KeybindingsExt final : public kernel::Extension {
 public:
     explicit KeybindingsExt(std::optional<std::string> config_path)
         : config_path_(std::move(config_path)),
+          effective_path_(discover_config_path(config_path_)),
           matcher_(load_bindings(config_path_)) {}
 
     auto manifest() const -> const kernel::Manifest& override { return manifest_; }
@@ -202,9 +203,64 @@ public:
                 // continues from wherever focus ACTUALLY is.
                 ring_.note_focused(e.toplevel);
             });
+
+        // Config hot-reload: watch the EFFECTIVE config path so editing
+        // unbox.toml re-applies keybindings live, with no restart. We watch even
+        // if the file does not exist yet — the kernel fires on_change when a
+        // not-yet-existing file is CREATED, so a user who later writes the config
+        // gets it picked up. The FileWatch is a member (config_watch_) so the
+        // watch lives exactly as long as this extension; on_change is coalesced
+        // and error-isolated by the kernel. No path -> no watch (defaults stand).
+        if (effective_path_) {
+            config_watch_ = host.watch_file(*effective_path_, [this] { reload_config(); });
+        }
     }
 
 private:
+    // Re-read + re-parse the EFFECTIVE config file and SWAP the live binding
+    // table the key_filter link matches against. Runs on the event-loop thread
+    // from the FileWatch callback. ROBUST by construction: an unreadable file, a
+    // toml parse error, or a parse that yields zero usable bindings KEEPS the
+    // currently-active bindings and logs exactly ONE warning — a half-saved /
+    // broken-mid-edit file never drops the user's working keys, and nothing
+    // throws out of this callback (toml::parse_error is caught inside the pure
+    // config loader). On success the new table replaces matcher_ in place; the
+    // key_filter lambda reads matcher_ through `this`, so the swap takes effect
+    // immediately with NO re-subscribe.
+    void reload_config() {
+        if (!effective_path_) {
+            return; // defensive: only registered when a path exists
+        }
+
+        std::string text;
+        if (!read_file(*effective_path_, text)) {
+            // File vanished or became unreadable mid-edit (e.g. an editor's
+            // unlink+rename window). Keep the working keys; the watch stays
+            // armed for the next write.
+            wlr_log(WLR_ERROR,
+                    "ext-keybindings: config '%s' not readable on reload; keeping current bindings",
+                    effective_path_->c_str());
+            return;
+        }
+
+        // Pure swap-or-keep-old decision over the CURRENTLY-LIVE table.
+        config::ReloadDecision decision =
+            config::reload_bindings(matcher_.bindings(), text);
+        if (!decision.swapped) {
+            // Parse error or zero usable bindings: keep current, ONE warning.
+            wlr_log(WLR_ERROR,
+                    "ext-keybindings: reload of '%s' failed (%zu issue(s)); keeping current bindings",
+                    effective_path_->c_str(), decision.warnings.size());
+            return;
+        }
+
+        // SUCCESS: swap the live matcher (and thus its binding table) in place.
+        const std::size_t n = decision.bindings.size();
+        matcher_ = policy::Matcher(std::move(decision.bindings));
+        wlr_log(WLR_INFO, "ext-keybindings: config reloaded (%zu binding(s)) from '%s'",
+                n, effective_path_->c_str());
+    }
+
     void run_action(const policy::Binding& b) {
         switch (b.action) {
         case policy::Action::spawn:
@@ -256,23 +312,32 @@ private:
     };
 
     std::optional<std::string> config_path_;
+    // The EFFECTIVE config path (explicit --config, else the discovered XDG /
+    // ~/.config path), resolved ONCE in the ctor. Drives both the initial load
+    // and the hot-reload watch + re-read. nullopt only when no HOME/XDG exists.
+    // Declared before matcher_ so it is initialized first (ctor init order).
+    std::optional<std::string> effective_path_;
 
     // Decision cores (constructed before any wiring; matcher_ owns the parsed
-    // bindings). Declared before the subscriptions so they outlive callbacks
-    // that capture `this` (members tear down in reverse declaration order:
-    // subscriptions drop first, then the cores).
+    // bindings). matcher_ is the LIVE binding table the key_filter link reads
+    // through `this`; reload_config() swaps it in place. Declared before the
+    // subscriptions so they outlive callbacks that capture `this` (members tear
+    // down in reverse declaration order: subscriptions drop first, then cores).
     policy::Matcher matcher_;
     policy::FocusRing<Toplevel*> ring_;
 
     Host* host_ = nullptr;
     ext_xdg_shell::Service* shell_ = nullptr; // borrow; fetched in activate()
 
-    // RAII subscriptions — destruction unsubscribes (listener-lifetime). Last
-    // members so they release FIRST at teardown, before the cores they touch.
+    // RAII subscriptions + the file watch — destruction unsubscribes / stops the
+    // watch (listener-lifetime). Last members so they release FIRST at teardown,
+    // before the cores their callbacks touch (matcher_, ring_). config_watch_'s
+    // on_change reads matcher_ + effective_path_, so it must die before them.
     kernel::Subscription key_filter_;
     kernel::Subscription mapped_;
     kernel::Subscription unmapped_;
     kernel::Subscription focused_;
+    kernel::FileWatch config_watch_;
 };
 
 } // namespace
diff --git a/packages/ext-keybindings/tests/test_policy.cpp b/packages/ext-keybindings/tests/test_policy.cpp
index 0774418..8b6b91b 100644
--- a/packages/ext-keybindings/tests/test_policy.cpp
+++ b/packages/ext-keybindings/tests/test_policy.cpp
@@ -181,6 +181,104 @@ TEST_CASE("compiled defaults match the documented out-of-the-box set") {
 }
 
 // ============================================================================
+// config hot-reload semantics (the swap-or-keep-old reload decision)
+// ============================================================================
+// Pure proof of the live-reload logic WITHOUT inotify or the event loop: the
+// kernel already tests the watcher; here we test only what reload_config() does
+// with the file's TEXT once it has been read. config A -> bindings A; reload
+// with config B -> bindings B (the swap); reload with MALFORMED text -> bindings
+// unchanged (still B) and NO throw. This is exactly the keep-old-on-bad +
+// swap-on-good contract the glue's reload_config() relies on.
+
+TEST_CASE("reload: config A -> A, reload B -> B (swap), reload malformed -> unchanged") {
+    // --- Initial load: config A (one binding: Alt+Tab -> focus-next). ---
+    const std::string config_a = R"(
+[[keybind]]
+keys   = "Alt+Tab"
+action = "focus-next"
+)";
+    auto a = cfg::load_from_string(config_a);
+    REQUIRE_FALSE(a.parse_error);
+    REQUIRE(a.bindings.size() == 1);
+    std::vector<Binding> live = a.bindings; // the live table the glue holds
+
+    // --- Reload with config B (two bindings: Super tap + Ctrl+Alt+BackSpace). ---
+    const std::string config_b = R"(
+[[keybind]]
+keys    = "Super"
+action  = "spawn"
+command = "wofi"
+
+[[keybind]]
+keys   = "Ctrl+Alt+BackSpace"
+action = "quit"
+)";
+    auto dec_b = cfg::reload_bindings(live, config_b);
+    REQUIRE(dec_b.swapped);                 // a usable parse -> SWAP
+    REQUIRE(dec_b.bindings.size() == 2);
+    CHECK(dec_b.bindings[0].combo.is_tap);
+    CHECK(dec_b.bindings[0].action == Action::spawn);
+    CHECK(dec_b.bindings[0].command == "wofi"); // command change rides the swap
+    CHECK(dec_b.bindings[1].action == Action::quit);
+    live = dec_b.bindings; // glue installs the new table
+
+    // --- Reload with MALFORMED text: keep-old, swapped == false, no throw. ---
+    cfg::ReloadDecision dec_bad; // (default-constructed; assigned below)
+    CHECK_NOTHROW(dec_bad = cfg::reload_bindings(live, "this = = not valid toml [[["));
+    CHECK_FALSE(dec_bad.swapped);                // parse error -> KEEP OLD
+    CHECK_FALSE(dec_bad.warnings.empty());       // and a diagnostic is surfaced
+    REQUIRE(dec_bad.bindings.size() == 2);       // STILL config B, untouched
+    CHECK(dec_bad.bindings == live);             // byte-for-byte the working keys
+}
+
+TEST_CASE("reload: a valid-but-empty doc keeps the old table (never drops working keys)") {
+    // A file that parses cleanly but yields ZERO usable bindings (e.g. saved
+    // mid-edit with the [[keybind]] block deleted, or every entry malformed)
+    // must NOT swap — the user's live keys survive.
+    std::vector<Binding> live = pol::default_bindings();
+    REQUIRE(live.size() == 5);
+
+    auto empty = cfg::reload_bindings(live, "title = \"unrelated\"\n");
+    CHECK_FALSE(empty.swapped);
+    REQUIRE(empty.bindings.size() == 5);
+    CHECK(empty.bindings == live); // kept
+
+    // Same for a doc where every entry is individually skipped (all malformed).
+    const std::string all_bad = R"(
+[[keybind]]
+keys   = "Alt+Bogus"
+action = "focus-next"
+
+[[keybind]]
+keys   = "Super"
+action = "spawn"
+)";
+    auto bad = cfg::reload_bindings(live, all_bad); // bad combo + spawn w/o command
+    CHECK_FALSE(bad.swapped);
+    CHECK(bad.bindings == live); // STILL the defaults, no throw
+}
+
+TEST_CASE("reload: a partial save with at least ONE usable binding swaps to it") {
+    // The reload contract is "swap on >=1 usable binding": even if some entries
+    // are skipped, as long as one survives, that becomes the new live table.
+    std::vector<Binding> live = pol::default_bindings();
+    const std::string partial = R"(
+[[keybind]]
+keys   = "Alt+Bogus"
+action = "focus-next"
+
+[[keybind]]
+keys   = "Alt+Tab"
+action = "focus-next"
+)";
+    auto dec = cfg::reload_bindings(live, partial);
+    REQUIRE(dec.swapped);
+    REQUIRE(dec.bindings.size() == 1); // the one usable binding
+    CHECK(dec.bindings[0].action == Action::focus_next);
+    CHECK_FALSE(dec.warnings.empty()); // the skipped entry was reported
+}
+
+// ============================================================================
 // matcher + tap state machine
 // ============================================================================