What the PR does. Upstream #7564 (tobi) fixes the keybindings menu's Lua bind scan, which runs the user's hyprland.lua under a stub hl. The stub now answers nil for numeric keys, so ipairs over stub values ends. Comparisons, arithmetic, .., # and tostring give harmless values instead of raising. The scan runs under timeout 10, and the cache version goes v14 to v15. A new test (menu-keybindings-lua-scan-test.sh) pins it. On the mirror that is commit 454ad3cd. The later commits are our proof work: a two-arm reproducer, KI notes and evidence, trace re-reviews, a Verifies: line on the PR's test, and two obligation tags on existing keycode tests.
Risk rationale: The change is small and targeted. The PR's test fails at the base and passes on the merge, and it fixes every reported trigger: qconsole's comparison and ipairs loops over stub values. But coverage of the changed Lua was not measured. The package's compatible result does not exercise the scan. The one observed obligation cell rests on keycode tests that the change does not touch. Two user-config patterns still drop every later bind: a type-checked library call on a stub value, and an unguarded numeric index. Neither is a regression against base as shipped. This assessment is advisory only.
Read the full review (6 more paragraphs)
Is the fix verified? Coverage of the changed file was not measured (coverage_unmeasured): the changed lines are Lua inside a heredoc. The PR's scan test ran in the audit and passed. 69 results were ingested (69 passed, 0 failed). My runs: the scan test fails at base 1e433732 (the scan spins to the 10s limit) and passes 3/3 on the merge. The KI reproducer's shipped-config arm flips; its user-module arm does not.
Compatibility. The package says compatible (4 sources, 0 findings). But the differential harness covers only the menu QML/JS files, and the removed-behaviour inventory is empty, so none of this exercises the scan.
Readiness. ready under the approved policy, with 4 informational disclosures. The next decision is human_judgement (acceptance), by a maintainer. Nothing in this review is an approval.
Questions for a human. (1) A library call that type-checks its argument (string.format("%d", m.width), math.floor) in a user monitors.lua still ends the scan before hypr.bindings. (2) An unguarded hl.get_monitors()[1].name also ends it, because numeric keys now answer nil. On base as shipped the scan already stopped at qconsole, so for such a user the PR leaves the binds missing; it does not fix them. (3) A config the stub cannot finish now makes the menu wait 10s. A per-module pcall around require settles (1) and (2) in my runs.
Traceability. 0W96 and 9DMS match only at file level. Change effects were not analyzed (effects_unavailable), so the empty spec-gap list proves nothing.
Not caused by this PR. No gate failed: 178 passed, unchanged from the baseline. 13 were skipped by configuration. The reproducer file is out of scope.
AI-generated review opinion derived from the evidence package. It is not a verdict, does not change readiness, and must be confirmed by a human reviewer.
12 context notes (info — not a finding)
- AI · confirmedThe stub change serves SW-REQ-260922-0W96: binds declared after load-time config reads reach the cache again
0W96: "Binds Hyprland reports as dispatcher __lua are shown with resolved keys from the source-derived cache and remain dispatchable." The cache is built by build_lua_bind_cache, which runs the user's hyprland.lua under the stub (
pcall(dofile, config)at line 296). Before this PR, one throwing read ended the scan, and every bind declared after it was missing. The new stub (lines 199-254) absorbs the shipped trigger, qconsole'smon.scalecomparison. The match is file-level only: no function-level trace names build_lua_bind_cache. - AI · confirmedSW-REQ-260922-9DMS is touched only through file-level traces; keycode resolution is unchanged
9DMS covers code:N keysym resolution (xkbcli, the fallback table, GRAVE as ~). The PR does not touch parse_keycodes, the xkbcli lookup or the fallback table. The scan still emits code:N keys in the same field. The cache-key bump to v15 (line 598) only makes existing caches rebuild once. The other changed file matched to 9DMS is keybindings-menu-test.sh: two obligation-tag comments added by our proof work. That test passed in the run.
- AI opinion — unverifiedThe purpose is exercised by the PR's own test, but coverage of the changed code was not measured
review.diffCoverage counts 0 changed decisions: bin/omarchy-menu-keybindings is unmeasured, and the changed lines are Lua inside a bash heredoc that the bash coverage does not see (disclosure coverage_unmeasured). The execution evidence is the PR's test. menu-keybindings-lua-scan-test.sh runs the embedded scan over a config that loops over, compares and computes with stub values. It then checks that two binds declared afterwards are reported (lines 47-58). It passed in the audit run. My runs on scratch copies: base 1e433732 fails
Lua bind scan exits cleanly ...after the 10s timeout; the merge passes 3/3 in 0.06s. bin/ and this test are byte-identical between e1ff55c9 and the earlier merge 60750dc8, where I ran them. - AI opinion — unverifiedRemoved-behaviour inventory (my reading; the engine's inventory is empty)
What the old stub did, and what the PR says about it. (a) Every key, numeric ones included, returned the stub. Now a numeric key gives
nil(line 202). The PR states it: "noop.__indexanswersnilfor numeric keys (soipairs/numeric loops end)". Its side effect on direct indexing is finding 6. (b) Comparisons, arithmetic and..on a stub raised and ended the scan. Now they give false, 0 and a string (lines 214-250); PR: "degrade to harmless values instead of raising". (c)tostring(stub)gavetable: 0x...; now it gives an empty string (line 251). (d) The scan had no time limit; now it has 10s (line 90). (e) The cache version was v14; now it is v15 (line 598). (a), (b) and (d) are the PR's stated intent. No test pins (c). None of these is recorded withproof change behavior-diff classify-inventory. - AI opinion — unverifiedKI-MENU-KEYBINDINGS-SCAN-DOFILE-ABORT stays open: arm A flips with this PR, arm B does not
My run of pocs/crs-260930-jvx8-c01.sh (a copy that keeps its scratch dir) gives these results. Base 1e433732: arm A fails at qconsole.lua:123 (
attempt to compare number with table) and arm B atbad argument #2 to 'format'; both lose the SSH bind. The merge: arm A keeps the bind; arm B still loses it; the control passes. The KI's remediation names a per-require pcall boundary. Our proof commits 2300da6c, 0c2ae27b, aeae174b and c6b255fc record the same effect, plus the numeric-index case, in the KI notes and evidence. - AI opinion — unverifiedThe cache version bump makes stale caches rebuild; other open PRs also pick v15 (opinion, outside the package)
keybindings_cache_key now hashes
v15(line 598), so a cache written by the old scan, with binds missing, is not reused. That is intended: the key covers only the version, the keymap andhyprctl binds, so a scanner fix changes nothing else in it. Several other open upstream PRs that edit this script also move to v15. Whichever merges second needs a fresh number, or it reuses caches written by the first. - AI · confirmedpocs/crs-260930-jvx8-c01.sh is our proof-layer reproducer, not part of the upstream PR
Commits 2e26e5df and 8a396d2b rewrite the KI reproducer. make_home copies the whole shipped hypr config: a partial copy would end a fixed scan on a missing module and read as the defect. scan_has_ssh replaces cache_has_ssh and runs one fresh scan per arm. These are unmapped units, and the file is outside the approved verification scope (changed_files_out_of_scope). They realize the KI's reproducer for 0W96, so no product requirement is missing.
- AI · confirmedscan_has_ssh, the reproducer's per-arm probe, is unmapped for the same reason
It clears the three global LUA_BIND_* maps, runs build_lua_bind_cache with HOME pointing at the arm's fixture, and checks for the user's SSH bind. It is proof tooling for the KI, not product code.
- AI opinion — unverifiedThe observed obligation cell rests on keycode tests that this PR does not change
testResults: ingested and current; 69 passed, 0 failed, 0 skipped; 1 tagged test, joined. The cells in scope are as follows. 9DMS error_handling is observed_pass. Its witnesses are our two tags in keybindings-menu-test.sh: the resolved-keycode test is nominal (line 171) and the unresolvable-keycode test negative (line 192), commit 258ca2da. They exercise keycode fallback, not the changed scan, so they add no evidence about this PR's code. 0W96 error_handling is suppressed (a recorded degradation-not-corruption suppression in the base data). 9DMS external_call_timeout_bounded is not_linked: it is deferred to KI-MENU-KEYBINDINGS-NO-TIMEOUT (hyprctl and xkbcli calls), and the new
timeout 10bounds only theluachild. No obligation cell is tagged on the PR's scan test. - AI opinion — unverifiedOur proof work re-reviewed the traces, linked the PR's test, and moved one review citation
Commit 24f959e5 re-reviews the implemented_by traces of 0W96 and 9DMS that the diff made suspect, citing the diff. Commit 0375251e adds
# Verifies: SW-REQ-260922-0W96to the PR's new test. That line is ours: the upstream test file has no proof annotation. Commit 258ca2da adds the two 9DMS tags. It also moves REVIEW-75's citation from keybindings-menu-test.sh:223 to :225, the same assertion after the two inserted lines. - AI opinion — unverifiedChange effects were not analyzed (effects_unavailable); uncodified connections cannot be detected
review.effects is absent for this bash/Lua change, so no new call paths, gained effects or callers were computed. The empty spec-gap list proves nothing about uncodified connections (review.scope approximations). From reading: build_lua_bind_cache is called only from output_binding_records_uncached (line 578), on a cache miss. Its new external call is
timeout, wrappinglua. These facts come from reading, not from the engine. - AI opinion — unverifiedNot caused by this PR: no gate failed; 13 checks skipped by configuration
Gates against baseline run run-f5b0e01e: 178 passed, 28 not applicable and 13 skipped, all with delta unchanged. The checks_skipped disclosure lists the 13 configured skips, e.g. documentation_coverage and security_surface_covered. They produced no verdict either way.