Skip to content

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

Open
tobi wants to merge 1 commit into
omacom:quattrofrom
tobi:fix/menu-keybindings-stub-scan
Open

Make the keybindings menu's Lua bind scan safe against load-time config reads#7564
tobi wants to merge 1 commit into
omacom:quattrofrom
tobi:fix/menu-keybindings-stub-scan

Conversation

@tobi

@tobi tobi commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

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-keybindingslua, 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.

…ig reads

omarchy-menu-keybindings learns about Lua-only binds by running the user's
hyprland.lua under a stub `hl` whose every lookup answers a truthy noop
table. That stub breaks as soon as a config reads compositor state at load
time, which real configs do:

- `for _, m in ipairs(hl.get_monitors())` never terminates, because noop[1],
  noop[2], ... are all non-nil. The menu exits, the `lua` child keeps
  spinning at 100% CPU under systemd --user until the next reboot. One
  such orphan ran for 27 hours on a laptop after a single SUPER+K.
- `monitor.scale <= 0` (default/hypr/qconsole.lua:64) raises "attempt to
  compare table with number", which aborts the pcall'd scan, so every bind
  declared after that file is silently missing from the menu.

The stub now answers nil for numeric indexes (so ipairs/`#` terminate),
reports length 0, and degrades comparisons, arithmetic, and concatenation
to harmless values instead of raising. The scan is also wrapped in
`timeout 10` so a config the stub still cannot satisfy can never outlive
the menu.

Adds a shell test that scans a fixture config doing exactly those
load-time reads; without the fix it hangs for the full timeout.
Copilot AI balanced review requested due to automatic review settings August 20, 2026 01:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

agush22 commented Aug 30, 2026

Copy link
Copy Markdown

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants