Make the keybindings menu's Lua bind scan safe against load-time config reads

[tobi]

Problem

omarchy-menu-keybindings learns about Lua-only binds by dofile-ing the user's hyprland.lua under a stub hl whose every lookup returns a truthy noop table. Real configs read compositor state at load time, and the stub cannot survive that:

1. Runaway lua at 100 % CPU. for _, m in ipairs(hl.get_monitors()) never terminates, because noop[1], noop[2], … are all non-nil. The menu exits, but the lua child keeps spinning under systemd --user until reboot. Found one on my laptop that had run for 27 hours after a single SUPER+K (PIDs omarchy-menu-keybindings → lua, 60 % of a core, dofile(hyprland.lua) never returned).

   local noop = setmetatable({}, { __index = function() return noop end, __call = function() return noop end })
   for _, m in ipairs(noop) do end   -- never returns

2. Silently dropped binds. On current quattro, default/hypr/qconsole.lua:64 does monitor.scale <= 0, which under the stub raises attempt to compare table with number. The scan is pcall'd, so it just stops there — every Lua bind declared after that require is missing from the menu (DEBUG=1 shows lua bind scan failed: …qconsole.lua:64).

Fix

• noop.__index answers nil for numeric keys (so ipairs/numeric loops end), __len is 0, and __lt/__le, arithmetic, __concat, __tostring degrade to harmless values instead of raising. String-key lookups behave as before.
• The scan is wrapped in timeout 10 so a config the stub still cannot satisfy can never outlive the menu.

Test

test/shell.d/menu-keybindings-lua-scan-test.sh extracts the embedded scan and runs it against a fixture hyprland.lua that iterates monitors, compares monitor.scale, does arithmetic/concatenation on stub fields, and then declares two binds. Without the fix it hangs for the full 10 s timeout and fails; with it the scan returns in milliseconds and both binds are reported. Also verified on my own config: the scan now completes with no scan failed and lists the Lua binds that were previously lost after qconsole.lua.

Fixes #7025

Fixes #12052

Fixes #12124

[Copilot]

Pull request overview

Hardens Lua keybinding discovery against hangs and load-time type errors.

Changes:

• Time-boxes Lua scanning to 10 seconds.
• Adds safer proxy metamethods.
• Adds regression coverage for loops, comparisons, and arithmetic.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File	Description
bin/omarchy-menu-keybindings	Hardens and time-boxes Lua bind scanning.
test/shell.d/menu-keybindings-lua-scan-test.sh	Tests load-time reads and timeout configuration.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[agush22]

Ran into this exact issue after upgrading to Quattro 4.0
Pressing SUPER + K pegged a background lua process at 100% CPU because my monitors.lua iterates over hl.get_monitors() with ipairs.

Tested this fix locally; it resolves the infinite loop cleanly and makes the keybindings scan instantaneous. Hope to see this merged soon!

[shwig1]

Confirming on omarchy 4.0.3-1 (Hyprland 0.56.2, Lua 5.5.1): a personal module that
iterated hl.get_windows() at require time hung the scan, and repeated Super+K presses
left eight orphaned lua processes at 100% CPU each for over six hours. The timeout in
this PR is the part that would have contained that; the stub fix removes the trigger.

[alinuxfan]

Confirming another real-world manifestation of this on 4.0.0.r2147.g2fbac0c-1 — this one doesn't just drop a bind from the menu, it makes the menu launch the wrong app.

I have flea set as my default file manager. Its installer (flea --default) correctly unbinds and rewrites SUPER+SHIFT+F in ~/.config/hypr/bindings.lua:

o.bind("SUPER + SHIFT + F", "File manager", { launch = 'flea --gui' })

Pressing SUPER+SHIFT+F directly opens flea correctly (hyprctl binds shows the live dispatcher resolving as expected). But selecting the same "File manager" row from the SUPER+K search menu opens Nautilus instead.

Root cause is this PR's diagnosis exactly: hyprland.lua loads default.hypr.omarchy (which pulls in default/hypr/qconsole.lua) before hypr.bindings (the user's own overrides). The scan crashes inside qconsole.lua — same attempt to compare number with table on a monitor.scale > 0 check, now at line 123 rather than 64, so the line's moved but the bug hasn't:

$ DEBUG=1 omarchy-menu-keybindings
[DEBUG] lua bind scan failed: /usr/share/omarchy/default/hypr/qconsole.lua:123: attempt to compare number with table

Because the crash happens before hypr.bindings is ever reached, the scan's cache keeps the stale default from default/hypr/bindings/applications.lua:

$ lua <extracted scan snippet>
65      File manager    F       exec    omarchy-launch-nautilus
73      File manager (cwd)      F       exec    omarchy-launch-nautilus-cwd

...instead of the user's actual flea --gui override. So it's not just that entries silently go missing — for anyone who has overridden a default Omarchy keybind, the search menu can confidently dispatch the old action while the real key combo does the right thing. Would be good to land this fix; the noop stub degrading harmlessly under comparison (not just under ipairs/length) directly fixes this case too.

[omarchybot]

Reviewed and reproduced on a worker built from the Omarchy ISO, with the stock config plus a user o.rebind("SUPER + SHIFT + F", "File manager", { launch = "flea" }) and, separately, an ipairs(hl.get_monitors()) loop:

• On quattro the scan aborts at default/hypr/qconsole.lua:123 and the menu's File manager row dispatches omarchy-launch-nautilus; the loop hangs the menu until killed.
• On this branch the row dispatches flea and the loop returns at once. With the stub changes reverted, both failures come back. A config stuck in while true do end now ends at the 10 s timeout and leaves no lua behind.
• test/shell.d/menu-keybindings-lua-scan-test.sh passes 3/3 here and fails on quattro; test/shell.d/keybindings-menu-test.sh passes 16/16. ./test/cli stops at "vscode generated theme references current theme file" on this branch and on quattro alike, so that failure is not this change's, but it does mean the rest of that suite did not run.

I pushed two commits to the branch. The first merges quattro, which had moved the cache version to v14. The second bumps the version to v15. The records cache is keyed only on that version, the keymap and hyprctl binds, so without the bump anyone already holding a cache from the broken scan keeps getting the stale default after this lands. I confirmed that on the worker: a v14 cache written by quattro kept dispatching nautilus until the bump.

Six other open pull requests fix parts of the same bug: #8876, #12712, #12916, #13276, #12155 and #11733. This one fixes both the endless loop and the qconsole abort, and it has a test that fails without the fix. #12712, #12916 and #12155 do not get past qconsole. #8876 gets past it by returning nil monitors, which still breaks a config that reads a field off one. #13276 has no test. #11733 is a redesign that would need its own review.

Second opinion from Codex Medium, which agreed with that choice and with the cache bump (its independence from this review is not guaranteed). It also raised two design points that I have not changed, because they are a call about how far the stub should pretend:

• A scan cut off by the timeout is cached like a finished one, so its partial records stay until the binds change. A scan that errors partway is already cached the same way.
• Because comparisons, arithmetic and concatenation on stub values now return made-up results, a config that guards a comparison with pcall, or builds a command from a monitor field, can take a different branch or record a truncated command where the scan used to stop.

This links #7025, #12052 and #12124, which it fixes. The next step is the maintainer's.

[linyiru]

Merged this with #12557 locally and measured it, because the two edit the same scan and are textually compatible while being behaviourally incompatible. git merge-tree reports no conflict, and the merged tree fails one test from each side.

#12557 adds an id column to the scan's output, inserted as field 3, so a row that read modmask description key kind arg reads modmask description id key kind arg. That breaks the two fixed-string assertions in menu-keybindings-lua-scan-test.sh:52-53, which pin the five-field layout:

expected: 64	Keybindings menu	K	exec	omarchy-menu keybindings
actual:   64	Keybindings menu	Keybindings menu	K	exec	omarchy-menu keybindings

In the other direction, this PR rewrites the heredoc to timeout 10 lua <<'LUA', and #12557's hyprland-bind-id-test.sh extracts the scan by matching that line exactly, so on the merge the extraction returns nothing and the five assertions that read scan output go with it.

On the merge: menu-keybindings-lua-scan-test.sh 1 ok and 1 not ok, hyprland-bind-id-test.sh 4 ok and 1 not ok, keybindings-menu-test.sh unaffected at 16/16. Both breakages are a one-line fix in whichever PR lands second, and I will make them on my side if this one lands first. Writing it down because the repo has no CI, so nothing would surface it before a merge.

Separately, as the author of one of the seven PRs touching this scan: this is the only one of them whose test fails on quattro and passes on the branch, which is the property that makes a fix here checkable later. The timeout matters for a reason the other candidates cannot cover either, since a stub that degrades comparisons can still be sent into a loop by a config that iterates on a condition.

[buger]

TL;DR: This fixes the reported scan failure. One optional follow-up for custom configs: a type error in monitors.lua can still prevent bindings.lua from loading. Catching each require separately preserves later binds; the two tests below cover a numeric format and direct monitor indexing. This need not hold up the shipped-config fix.

Remaining aborts and proposed tests, with the shipped-config baseline

I ran the real build_lua_bind_cache from each tree against the shipped config/hypr, plus two user binds in bindings.lua, on Lua 5.5.1 (Linux). This head merges onto 035ce29f cleanly, and its scan code is byte-identical either way.

	quattro 035ce29f	this PR (7b49d0bb) on 035ce29f
Scan of the shipped config	stops at default/hypr/qconsole.lua:123 (line 64 in the description; it has moved)	completes
User o.rebind("SUPER + RETURN", "Terminal", { launch = "alacritty" })	the menu row runs the default omarchy-launch-terminal (the wrong-app symptom @alinuxfan reported)	uwsm-app -- alacritty
User o.bind("SUPER + SHIFT + R", "SSH", …)	missing from the menu cache	uwsm-app -- alacritty -e ssh your-server
the PR's menu-keybindings-lua-scan-test.sh run against each tree's scan	fails (hangs until its 10s limit)	3/3; keybindings-menu-test.sh still passes

This is our known issue KI-MENU-KEYBINDINGS-SCAN-DOFILE-ABORT. Its reproducer has two arms. The shipped-config arm flips with this head: the scan no longer fails, and the SSH bind is cached. The second arm, a user monitors.lua that builds a mode with string.format, still loses the bind on this head. Both arms pass with the change suggested below.

1. Other reads can still abort the scan. The new stub absorbs operators (comparison, arithmetic, .., #). Some library functions, including numeric string.format conversions and math.floor, reject the stub table instead of using its arithmetic metamethods. math.max uses comparison and can invoke comparison metamethods (Lua 5.5 source). The examples below also include a failed numeric conversion, a numeric for limit and a nil get_config() result. hyprland.lua requires hypr.monitors and hypr.input before hypr.bindings. So any one of these lines in a user's monitors.lua still ends the scan on this head. Every bind from bindings.lua is then missing, and every rebind falls back to its default:

local m = hl.get_active_monitor()
string.format("%dx%d@%d", m.width, m.height, m.refresh_rate)   -- bad argument #2 to 'format'
math.floor(m.width)                                            -- bad argument #1 to 'floor'
if tonumber(m.scale) > 1 then end                              -- tonumber(stub) is nil
string.lower(m.name)                                           -- bad argument #1 to 'lower'
for x = 0, m.width, 960 do end                                 -- 'for' limit must be a number
local gaps = hl.get_config("general.gaps_out") * 2             -- the stub's get_config returns nil

On this head the scan survives ipairs/pairs/# loops, comparisons, arithmetic, .., m.name:match(...), math.max, table.sort and hl.get_config(...) or 1. The monitor/window-loop and wrong-app reports covered by the worker run are fixed by this PR. I did not find these additional examples in this PR's thread, and the shipped monitors.lua sets a literal mode ("preferred"), so this only matters for hand-written monitor logic.

2. A direct [1] now ends the scan. The review of #13276 already identified direct [1] access returning nil with this stub design. I reproduced it here and checked that per-module recovery preserves later binds. In the stock load order, the existing qconsole error masks it; custom code loaded before qconsole can regress. The stub answers nil for a numeric key, which is what lets ipairs end. A line that takes the first element directly then indexes nil:

local first = hl.get_monitors()[1]
hl.monitor({ output = first.name })   -- attempt to index a nil value (local 'first')

hl.get_workspaces()[1].id fails the same way. The old stub answered every key with itself, so with qconsole's comparison guarded, quattro's scan keeps the user's SSH bind and Terminal rebind with this line; this head loses both. if first then … end is unaffected.

Suggestion: in the scan, load each module under its own pcall. A module that still can't run against the stub then costs only its own binds:

-- Load each module under its own pcall, so a module that still cannot run
-- against the stub costs only its own binds, not every bind loaded after it
-- (the user's hypr.bindings is required after hypr.monitors and hypr.input).
local load_module = require
require = function(name)
  local ok, result = pcall(load_module, name)
  if ok then
    return result
  end

  if os.getenv("DEBUG") == "1" then
    io.stderr:write("[DEBUG] lua bind scan skipped " .. tostring(name) .. ": " .. tostring(result) .. "\n")
  end

  return noop
end

It goes just before local config = …. With it, all six lines above and both [1] lines keep the user's binds. So do an error() in a user module and a require of a module that doesn't exist. On the shipped config, the records are byte-identical to this head's, and nothing is logged. It has two limits. A throw inside bindings.lua itself still loses the binds after that line in that file. A loop that never ends still needs your 10s limit: a while w and w.parent do w = w.parent end walk runs to the limit on this head with or without the change. The stub fixes keep their value alongside it, because they keep the binds that come after a stub read inside the same module.

Tests for menu-keybindings-lua-scan-test.sh: they fail on this head and pass with the change

# The user's hypr.bindings loads after hypr.monitors and hypr.input, so a
# module the stub still cannot run must cost only its own binds, not every
# bind loaded after it.
mkdir -p "$TMPDIR/later/.config/hypr"
cat >"$TMPDIR/later/.config/hypr/hyprland.lua" <<'LUA'
package.path = os.getenv("HOME") .. "/.config/?.lua;" .. package.path
require("hypr.monitors")
require("hypr.bindings")
LUA
cat >"$TMPDIR/later/.config/hypr/monitors.lua" <<'LUA'
local monitor = hl.get_active_monitor()
hl.monitor({ output = monitor.name, mode = string.format("%dx%d", monitor.width, monitor.height) })
LUA
cat >"$TMPDIR/later/.config/hypr/bindings.lua" <<'LUA'
hl.bind("SUPER + RETURN", hl.dsp.exec_cmd("alacritty"), { description = "Terminal" })
LUA

output=$(HOME="$TMPDIR/later" timeout 10 lua - <<<"$scan" 2>&1)
grep -qF $'64\tTerminal\tRETURN\texec\talacritty' <<<"$output" ||
  fail "a module the stub cannot run does not drop the binds loaded after it (got: $output)"
pass "a module the stub cannot run does not drop the binds loaded after it"

# A stub list answers nil for [1] so ipairs ends; taking the first element
# directly must not cost the binds loaded later either.
cat >"$TMPDIR/later/.config/hypr/monitors.lua" <<'LUA'
local first = hl.get_monitors()[1]
hl.monitor({ output = first.name })
LUA

output=$(HOME="$TMPDIR/later" timeout 10 lua - <<<"$scan" 2>&1)
grep -qF $'64\tTerminal\tRETURN\texec\talacritty' <<<"$output" ||
  fail "indexing the first element of a stub list does not drop the binds loaded after it (got: $output)"
pass "indexing the first element of a stub list does not drop the binds loaded after it"

Tree	Result
this head	exit 1: the first three checks pass, then not ok - a module the stub cannot run does not drop the binds loaded after it; run alone, the [1] check fails the same way
this head + the change	exit 0, 5/5 ok

The time limit and #13276

• When the 10s limit fires, the binds printed before the hang are kept: Lua flushes stdout on every print. A hang therefore costs the same as an abort, not the whole Lua cache. No lua is left running.
• fix(keybindings): keep the Lua bind scan alive past qconsole.lua #13276 (@playGitboy) fixes the same abort with nearly the same stub. @omarchybot's reasons for carrying this PR forward still hold on 035ce29f. fix(keybindings): keep the Lua bind scan alive past qconsole.lua #13276 still writes v14, which is quattro's current cache version, so stale caches are kept. It also has no time limit: the while walk above runs until killed there.
• v15 is also the bump in other open PRs that edit this script (for example Render every modifier Hyprland can bind in the keybindings menu #12042 and List submap bindings under the chord that enters their mode in the keybindings menu #13461), and Dispatch Lua function keybindings from the menu #11851 moves to v19. Whichever lands second needs a fresh number, or caches written by the first are reused (the v14 case @omarchybot found).

Thanks @tobi for the fix and the clear write-up. The worker run by @omarchybot and @linyiru's #12557 note here, and on #13276 @llstrk's review and @KewkLW's merge-order note, already cover the rest, so I haven't repeated them.

---

Independent verification: the evidence for this PR comes from a full proof audit of 7b49d0b merged onto quattro at 035ce29 (evaluated merge e1ff55c): test report 69 passed · 0 failed · 0 skipped. Rechecked on quattro b83d3df (2026-10-08): the scan script is unchanged but the shipped Hyprland Lua configuration changed, and rerunning this head merged onto that tip preserves the first table's results and the remaining custom-monitor aborts. Checked by running the scan from each tree; not a formal proof.