Detect lua special dispatchers by reference

hyprtester/clients/keyboard-modifiers.cpp

@@ -8,6 +8,7 @@
 #include <string>
 #include <fstream>
 #include <utility>
+#include <vector>
 
 #include <wayland-client.h>
 #include <wayland.hpp>
@@ -42,6 +43,8 @@ struct SWlState {
 
     CSharedPointer<CCWlKeyboard>   keyboard;
     uint32_t                       lastLocked = 0;
+
+    std::vector<std::pair<uint32_t, uint32_t>> keyEvents;
 };
 
 static std::ofstream logfile;
@@ -211,15 +214,27 @@ static bool setupSeat(SWlState& state) {
         state.lastLocked = locked;
     });
 
-    state.keyboard->setKey([&](CCWlKeyboard* p, uint32_t serial, uint32_t time, uint32_t key, uint32_t state) { debugLog("Got key event: key={} state={}", key, state); });
+    state.keyboard->setKey([&](CCWlKeyboard* p, uint32_t serial, uint32_t time, uint32_t key, uint32_t keyState) {
+        state.keyEvents.emplace_back(key, keyState);
+        debugLog("Got key event: key={} state={}", key, keyState);
+    });
 
     return true;
 }
 
 static void parseRequest(SWlState& state, std::string req) {
     if (req.starts_with("get"))
         clientLog("{}", state.lastLocked);
-    else if (req.starts_with("exit"))
+    else if (req.starts_with("keys")) {
+        uint32_t pressed = 0, released = 0;
+        for (const auto& [code, keyState] : state.keyEvents) {
+            if (keyState == 1)
+                ++pressed;
+            else if (keyState == 0)
+                ++released;
+        }
+        clientLog("{} {}", pressed, released);
+    } else if (req.starts_with("exit"))
         shouldExit = true;
 }
 

hyprtester/src/tests/clients/keyboard-modifiers.cpp

@@ -9,7 +9,9 @@
 #include <optional>
 #include <sys/poll.h>
 #include <csignal>
+#include <sstream>
 #include <thread>
+#include <utility>
 
 using namespace Hyprutils::OS;
 using namespace Hyprutils::Memory;
@@ -28,7 +30,9 @@
         CClient();
         ~CClient();
         uint32_t getLockedMods();
-        pid_t    pid();
+        // total (pressCount, releaseCount) of wl_keyboard.key events this window has received
+        std::pair<uint32_t, uint32_t> getReceivedKeyCounts();
+        pid_t                         pid();
     };
 }
 
@@ -129,10 +133,37 @@
     } catch (...) { return 0; }
 }
 
+std::pair<uint32_t, uint32_t> CClient::getReceivedKeyCounts() {
+    std::string cmd = "keys\n";
+    if ((size_t)write(this->writeFd.get(), cmd.c_str(), cmd.length()) != cmd.length())
+        return {0, 0};
+
+    if (poll(&this->fds, 1, 1500) != 1 || !(this->fds.revents & POLLIN))
+        return {0, 0};
+    ssize_t bytesRead = read(this->fds.fd, this->readBuf.data(), 1023);
+    if (bytesRead == -1)
+        return {0, 0};
+
+    this->readBuf[bytesRead] = 0;
+    std::istringstream iss{std::string{this->readBuf.data()}};
+    uint32_t           pressed = 0, released = 0;
+    iss >> pressed >> released;
+    return {pressed, released};
+}
+
 pid_t CClient::pid() {
     return this->proc->pid();
 }
 
+// modifier index understood by hl.plugin.test.keybind (see eKeyboardModifierIndex in tests/main/keybinds.cpp)
+static constexpr uint32_t MOD_CTRL = 3;
+
+// inject a synthetic key press/release through the test plugin's virtual keyboard. `modifier` is the
+// modifier index reported as held while the event is processed (0 = none); `key` is the xkb keycode.
+static std::string keyCmd(bool pressed, uint32_t modifier, uint32_t key) {
+    return "/eval hl.plugin.test.keybind(" + std::to_string(pressed ? 1 : 0) + ", " + std::to_string(modifier) + ", " + std::to_string(key) + ")";
+}
+
 TEST_CASE(keyboardModifiersMergedOnFocus) {
     NLog::log("{}Testing keyboard modifiers merged on focus", Colors::GREEN);
 
@@ -160,3 +191,124 @@
     NLog::log("{}Client reports locked mods: {}", Colors::BLUE, locked);
     EXPECT(locked, 18u);
 }
+
+// Regression: a Lua `pass` bound to a *modifier* keycode must forward BOTH the press and the release
+// to the target window. Unless the bind is recognized as a special dispatcher (tracked in
+// m_pressedSpecialBinds), the modmask guard in CKeybindManager skips a modifier key on release, so the
+// press was forwarded but the release was dropped - leaving a key bound to a modifier (e.g. a
+// push-to-talk key) latched "on" forever.
+TEST_CASE(luaPassForwardsModifierRelease) {
+    NLog::log("{}Testing Lua pass forwards a held modifier's release", Colors::GREEN);
+
+    std::optional<CClient> client;
+    try {
+        client.emplace();
+    } catch (...) { FAIL_TEST("Couldn't start the client"); }
+
+    OK(getFromSocket("/eval hl.bind('code:105', hl.dsp.pass({ window = 'class:keyboard-modifiers' }))"));
+
+    // drop keyboard focus so a dropped release cannot reach the recorder via normal event delivery -
+    // the recorder must only ever observe events that `pass` explicitly forwards to it.
+    OK(getFromSocket("/eval hl.plugin.test.nullfocus()"));
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    OK(getFromSocket(keyCmd(true, 0, 105)));         // press (modifier's own bit not yet applied)
+    OK(getFromSocket(keyCmd(false, MOD_CTRL, 105))); // release (Ctrl still applied)
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    const auto [pressed, released] = client->getReceivedKeyCounts();
+    EXPECT(pressed, 1u);
+    EXPECT(released, 1u); // 0 before the fix: the release is never forwarded
+
+    OK(getFromSocket("/eval hl.unbind('code:105')"));
+}
+
+// Complement to A1: a Lua `pass` on a *non-modifier* key must forward both edges too. A1 fails on release
+// via the modmask guard; this case fails via the non-special release suppression - both are only avoided
+// once the bind is recognized as a special dispatcher, so this confirms the fix covers non-modifier keys.
+TEST_CASE(luaPassForwardsNonModifierRelease) {
+    NLog::log("{}Testing Lua pass still forwards a non-modifier key's release", Colors::GREEN);
+
+    std::optional<CClient> client;
+    try {
+        client.emplace();
+    } catch (...) { FAIL_TEST("Couldn't start the client"); }
+
+    // keycode 31 is a normal (non-modifier) key; no modifier is held for either edge.
+    OK(getFromSocket("/eval hl.bind('code:31', hl.dsp.pass({ window = 'class:keyboard-modifiers' }))"));
+
+    OK(getFromSocket("/eval hl.plugin.test.nullfocus()"));
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    OK(getFromSocket(keyCmd(true, 0, 31)));
+    OK(getFromSocket(keyCmd(false, 0, 31)));
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    const auto [pressed, released] = client->getReceivedKeyCounts();
+    EXPECT(pressed, 1u);
+    EXPECT(released, 1u);
+
+    OK(getFromSocket("/eval hl.unbind('code:31')"));
+}
+
+// The fix is not pass-specific: every Lua dispatcher recognized as special (pass / global /
+// send_shortcut / mouse drag+resize) gets the same release handling. Exercise that through
+// `send_shortcut`, which forwards a configured key ('a') to the target. Bound to a modifier keycode it
+// has the identical dropped-release bug without the fix.
+TEST_CASE(luaSendShortcutForwardsModifierRelease) {
+    NLog::log("{}Testing Lua send_shortcut forwards its release when bound to a modifier", Colors::GREEN);
+
+    std::optional<CClient> client;
+    try {
+        client.emplace();
+    } catch (...) { FAIL_TEST("Couldn't start the client"); }
+
+    OK(getFromSocket("/eval hl.bind('code:105', hl.dsp.send_shortcut({ mods = '', key = 'a', window = 'class:keyboard-modifiers' }))"));
+
+    OK(getFromSocket("/eval hl.plugin.test.nullfocus()"));
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    OK(getFromSocket(keyCmd(true, 0, 105)));
+    OK(getFromSocket(keyCmd(false, MOD_CTRL, 105)));
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    const auto [pressed, released] = client->getReceivedKeyCounts();
+    EXPECT(pressed, 1u);
+    EXPECT(released, 1u); // 0 before the fix: the shortcut key's release is never sent and it sticks down
+
+    OK(getFromSocket("/eval hl.unbind('code:105')"));
+}
+
+// Held-key path: a modifier `pass` bind must forward exactly one press and one release per cycle even
+// while another, unrelated key stays held down for the whole test (historically this kept internal
+// release-tracking state from resetting between cycles). Black-box check: each press/release cycle
+// increments the counts by exactly one - never zero (dropped) and never two (double-forwarded).
+TEST_CASE(luaPassModifierReleaseWithHeldKey) {
+    NLog::log("{}Testing Lua pass modifier release with an unrelated key held down", Colors::GREEN);
+
+    std::optional<CClient> client;
+    try {
+        client.emplace();
+    } catch (...) { FAIL_TEST("Couldn't start the client"); }
+
+    OK(getFromSocket("/eval hl.bind('code:105', hl.dsp.pass({ window = 'class:keyboard-modifiers' }))"));
+
+    OK(getFromSocket("/eval hl.plugin.test.nullfocus()"));
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // hold an unrelated, unbound key (keycode 40) down for the whole test so m_pressedKeys never empties
+    OK(getFromSocket(keyCmd(true, 0, 40)));
+
+    for (uint32_t i = 1; i <= 3; ++i) {
+        OK(getFromSocket(keyCmd(true, 0, 105)));
+        OK(getFromSocket(keyCmd(false, MOD_CTRL, 105)));
+        std::this_thread::sleep_for(std::chrono::milliseconds(40));
+
+        const auto [pressed, released] = client->getReceivedKeyCounts();
+        EXPECT(pressed, i);
+        EXPECT(released, i);
+    }
+
+    OK(getFromSocket(keyCmd(false, 0, 40))); // release the held key
+    OK(getFromSocket("/eval hl.unbind('code:105')"));
+}

src/config/lua/bindings/LuaBindingsDispatchers.cpp

@@ -181,9 +181,6 @@ static int dsp_pass(lua_State* L) {
     if (!PWINDOW)
         return Internal::dispatcherError(L, "hl.pass: window not found", WARN, C_NOTFOUND);
 
-    if (g_pKeybindManager->m_currentKeybind)
-        g_pKeybindManager->m_currentKeybind->releasePending = true;
-
     return Internal::checkResult(L, CA::pass(PWINDOW));
 }
 
@@ -209,9 +206,6 @@ static int dsp_event(lua_State* L) {
 }
 
 static int dsp_global(lua_State* L) {
-    if (g_pKeybindManager->m_currentKeybind)
-        g_pKeybindManager->m_currentKeybind->releasePending = true;
-
     return Internal::checkResult(L, CA::global(lua_tostring(L, lua_upvalueindex(1))));
 }
 
@@ -398,9 +392,6 @@ static int dsp_sendShortcut(lua_State* L) {
             return Internal::dispatcherError(L, "send_shortcut: window not found", WARN, C_NOTFOUND);
     }
 
-    if (g_pKeybindManager->m_currentKeybind)
-        g_pKeybindManager->m_currentKeybind->releasePending = true;
-
     return Internal::checkResult(L, CA::pass(modMask, *keycodeResult, window));
 }
 
@@ -660,16 +651,10 @@ static int dsp_denyFromGroup(lua_State* L) {
 }
 
 static int dsp_mouseDrag(lua_State* L) {
-    if (g_pKeybindManager->m_currentKeybind)
-        g_pKeybindManager->m_currentKeybind->releasePending = true;
-
     return Internal::checkResult(L, CA::mouse("movewindow"));
 }
 
 static int dsp_mouseResize(lua_State* L) {
-    if (g_pKeybindManager->m_currentKeybind)
-        g_pKeybindManager->m_currentKeybind->releasePending = true;
-
     auto keepAspectRatio = Check::string(L, lua_upvalueindex(1));
     if (!keepAspectRatio)
         return Internal::configError(L, std::format("resize: bad argument 1: {}", keepAspectRatio.error()));
@@ -1277,6 +1262,15 @@ static int hlWorkspaceSwapMonitors(lua_State* L) {
     return 1;
 }
 
+bool Internal::dispatcherFunctionIsSpecial(lua_State* L, int idx) {
+    // These forward input to (or release it from) a target, so they must be released the instant the key
+    // goes up regardless of modifier state - the lua equivalents of the native pass/global/sendshortcut/mouse
+    // special handlers. lua_tocfunction sees through the hl.dsp.* closure to the underlying C dispatcher;
+    // a plain lua function (or any non-C-function value) returns nullptr and is never special.
+    const lua_CFunction fn = lua_tocfunction(L, idx);
+    return fn == dsp_pass || fn == dsp_global || fn == dsp_sendShortcut || fn == dsp_mouseDrag || fn == dsp_mouseResize;
+}
+
 void Internal::registerDispatcherBindings(lua_State* L) {
     lua_newtable(L);
     Internal::markDispatcherTable(L);

src/config/lua/bindings/LuaBindingsInternal.hpp

@@ -186,6 +186,10 @@ namespace Config::Lua::Bindings::Internal {
     void markDispatcherTable(lua_State* L);
     int  wrapDispatcher(lua_State* L);
     bool pushDispatcherFunction(lua_State* L, int idx);
+    // true if the function at `idx` is one of the native-special dispatchers (pass / global / send_shortcut /
+    // mouse drag+resize). Used to mark a lua bind's SKeybind::wrapsSpecialDispatcher at creation so it is
+    // tracked and released like a native special handler. A plain lua function is never special.
+    bool dispatcherFunctionIsSpecial(lua_State* L, int idx);
 
     template <typename T>
     SParseError parseTableField(lua_State* L, int tableIdx, const char* field, T& parser) {

src/config/lua/bindings/LuaBindingsToplevel.cpp

@@ -144,6 +144,10 @@ static int hlBind(lua_State* L) {
     if (kb.catchAll && mgr->m_currentSubmap.empty())
         return Internal::configError(L, "hl.bind: catchall keybinds are only allowed in submaps.");
 
+    // detect a wrapped native-special dispatcher now, while the dispatcher function is still on the stack
+    // (luaL_ref below pops it). Lets the bind be tracked/released like a native pass/global/send_shortcut/mouse.
+    kb.wrapsSpecialDispatcher = Internal::dispatcherFunctionIsSpecial(L, -1);
+
     int ref       = luaL_ref(L, LUA_REGISTRYINDEX);
     kb.handler    = "__lua";
     kb.arg        = std::to_string(ref);

src/managers/KeybindManager.cpp

@@ -432,12 +432,6 @@ bool CKeybindManager::onKeyEvent(std::any event, SP<IKeyboard> pKeyboard) {
         shadowKeybinds();
     }
 
-    if (m_pressedKeys.empty()) {
-        for (const auto& k : m_keybinds) {
-            k->releasePending = false; // reset this flag once we don't press any keys to avoid stale
-        }
-    }
-
     return !suppressEvent && !mouseBindWasActive;
 }
 
@@ -630,7 +624,7 @@ SDispatchResult CKeybindManager::handleKeybinds(const uint32_t modmask, const SP
     std::vector<SP<SKeybind>> bindsHit;
 
     for (auto& k : m_keybinds) {
-        const bool SPECIALDISPATCHER = k->handler == "global" || k->handler == "pass" || k->handler == "sendshortcut" || k->handler == "mouse" || k->releasePending;
+        const bool SPECIALDISPATCHER = k->handler == "global" || k->handler == "pass" || k->handler == "sendshortcut" || k->handler == "mouse" || k->wrapsSpecialDispatcher;
         const bool SPECIALTRIGGERED  = std::ranges::find_if(m_pressedSpecialBinds, [&](const auto& other) { return other == k; }) != m_pressedSpecialBinds.end();
         const bool IGNORECONDITIONS =
             SPECIALDISPATCHER && !pressed && SPECIALTRIGGERED; // ignore mods. Pass, global dispatchers should be released immediately once the key is released.
@@ -774,13 +768,12 @@ SDispatchResult CKeybindManager::handleKeybinds(const uint32_t modmask, const SP
     }
 
     for (const auto& k : bindsHit) {
-        const bool SPECIALDISPATCHER = k->handler == "global" || k->handler == "pass" || k->handler == "sendshortcut" || k->handler == "mouse" || k->releasePending;
+        const bool SPECIALDISPATCHER = k->handler == "global" || k->handler == "pass" || k->handler == "sendshortcut" || k->handler == "mouse" || k->wrapsSpecialDispatcher;
         const bool SPECIALTRIGGERED  = std::ranges::find_if(m_pressedSpecialBinds, [&](const auto& other) { return other == k; }) != m_pressedSpecialBinds.end();
 
         const auto DISPATCHER = m_dispatchers.find(k->mouse ? "mouse" : k->handler);
 
-        k->releasePending = false; // reset this flag if it's set
-        m_currentKeybind  = k;
+        m_currentKeybind = k;
 
         Hyprutils::Utils::CScopeGuard x([this] { m_currentKeybind.reset(); });
 

src/managers/KeybindManager.hpp

@@ -59,9 +59,15 @@ struct SKeybind {
 
     std::string                     displayKey = "";
 
+    // Set once at bind creation for a lua bind whose dispatcher wraps a native-special handler
+    // (pass/global/send_shortcut/mouse). Folded into SPECIALDISPATCHER so these binds are tracked on press
+    // and released on key-up exactly like the native handlers. Legacy/native binds are matched by their
+    // handler string instead and leave this false - keep it after `devices` so the aggregate init in the
+    // legacy config path (which fills fields only up to `devices`) is unaffected.
+    bool wrapsSpecialDispatcher = false;
+
     // DO NOT INITIALIZE
-    bool shadowed       = false;
-    bool releasePending = false;
+    bool shadowed = false;
 };
 
 enum eFocusWindowMode : uint8_t {