Repository navigation
Conversation
|
Verified: At For a synthetic config containing
String-property chaining and calls also retained the expected binding output. Source inspection confirms the new regression checks later binding resolution, not just whether the command exits. Compatibility limit: Empty offline lists do not emulate populated API results. In a synthetic config, unguarded Review informationTest scope: Pinned-source review and isolated execution of the unchanged embedded Lua scanners with synthetic configs, including positive controls and a bounded baseline timeout. This does not establish full-suite success, behavior with the reporter's actual config, real plugin state, or interactive menu dispatch. Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review. Automated AI review: Astra Medium performed initial inspection and synthesis; Opus 5.5 High and GPT 6 Sol Xhigh completed independent technical assessments. Public claims were checked against targeted source and runtime evidence. |
Both branches appended a test to keybindings-menu-test.sh. The terminal-bind test from quattro runs first because it needs the stock hyprland.lua, which the plugin-list test overwrites. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Reviewed by Claude Opus 5.5 and run on a worker built from the Omarchy ISO. The fix is correct, and the bug reproduces. No second opinion ran on this one: omabot's daily review budget was spent, so Codex never saw it. Conflict resolved. This branch conflicted with What was checked, at 7a9643a:
One unrelated thing for the maintainer: on current This is labelled |
|
| if type(key) == "number" then | ||
| return nil |
There was a problem hiding this comment.
Direct indexing aborts scanning
If a user's Lua config accesses a field on a known-present API result, such as hl.get_workspace_windows(...)[1].title, the scanner now returns nil for [1]. The field access then errors, ending the config scan before later bindings are collected. The menu silently omits those bindings unless DEBUG=1.
Knowledge Base Used: Hyprland configuration and display control
|
For triage: this is the generic fix for the "keybindings menu hangs when a config iterates an The direct-index case greptile flagged ( |
Both sides appended a test to test/shell.d/keybindings-menu-test.sh; keep both. Quattro's translation test runs first, since this branch's test overwrites hyprland.lua. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This branch conflicted with What was checked, at 1eb6ae7, on a worker built from the Omarchy ISO:
On this worker, quattro's new translation test needs the checkout's Second opinion: Codex Medium. It agreed the hang is real and that the fix is proportionate. That agreement came after reading my notes, so its independence is not guaranteed. It also flagged the trade-off already discussed above: Competing fix: #14128 fixes the same hang for This PR now closes #7025, #12124, #14243 and #14302. I did not link #12852: its report leaves the cause of the runaway unidentified, so I could not show this change fixes it. This is labelled |
The keybinding menu can spin indefinitely when a user's Lua config iterates an API query such as
ipairs(hl.get_loaded_plugins()). The scanner's no-op object's__indexreturns itself for every numeric index, so the iterator never seesnil. Repeated menu launches can leave multiple Lua processes consuming one CPU core each.Return
nilfor numeric lookups while preserving chained attribute access and calls. This lets query results act as empty lists during the offline scan.The regression test loads a plugin through the stub, iterates the plugin query, then checks that subsequent Lua bindings are still resolved. A five-second timeout bounds the test if the bug returns.
Validation:
bash test/shell.d/keybindings-menu-test.shpasses.git diff --checkpass../test/allcompleted: CLI passed; 236 of 245 shell test files passed. Eight failing files also fail on the untouched base commit (0d5232ed) in this environment:config,factory-reset-accounts,launch-about,network-qr,omarchy-kernel-migration,snapper,unowned-system-paths, andupdate-pacman. The packaging checks require an additionalomarchy-pkgscheckout. The ninth failure,locate, tries to decode a generatedbin/__pycache__file as UTF-8; it passes after moving that test-generated cache out of the checkout, and passes on the clean base checkout.Fixes #7025
Fixes #12124
Fixes #14243
Fixes #14302