feat: hotkey sequences (a sequence is a hotkey) - #52
Merged
Merged
Conversation
Sequences were designed as a separate Sequence type owning its action *Hotkey, which split ownership of Hotkey between Mappings.hotkeys and Sequence, forced an ownership-transfer flag through the parser, gave Mode two collections that cross-checked each other, and duplicated the trigger-overlap predicate three ways. It also broke the allocation-free event loop and rescanned every sequence in the mode per key-down. A sequence is a hotkey. Hotkey now carries chords: []const KeyPress (len >= 1), so Sequence.zig, Mappings.sequences, Mode.sequences and the parser flag all disappear and Mappings.hotkeys is the sole owner. Conflict detection collapses to one predicate: two hotkeys conflict iff one's chord list prefixes the other's and their process scopes overlap. That guarantees at most one hotkey matches any (mode, prefix, process), which is what lets a prefix-adapted lookup ignore Robin Hood probe order. Pending state becomes a chord-prefix buffer sized from the config at load, restoring the allocation-free event loop. Behavioral design (300ms interval, no replay, app capture, cancellation) is unchanged. No configuration that parses today changes meaning; the capture-mode fallback gate is preserved explicitly rather than as a side effect of lookup order. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Every existing action form works on a sequence, and two of them need rules stated rather than left to accident. Forwarding a chord the sequence starts with — cmd - q, cmd - q ["Terminal" | cmd - q] — is now the motivating example, since it hands the app a real cmd-q and needs no shell command. It only works because the SKHD_EVENT_MARKER check precedes sequence handling; if it ran after, the forward would re-enter, match its own first chord, go pending and be swallowed. That ordering, and the rule that self-generated events never advance or cancel a pending prefix, become invariants with tests. Passthrough applies to the final chord only. Earlier chords cannot be delivered because completion isn't yet known, so `->` and `~` affect only the chord that completes the binding. passthrough also moves off ModifierFlag onto Hotkey. It is a routing marker, not a modifier — isEmpty already had to zero it out to stop it gating wildcard matching — and once flags means chords[0].flags, a whole-binding property would live inside the first chord's modifier set. Adds a grammar-coverage table and records why multi-mode commas cannot collide with chord commas: key tokens lex as Token_Key, not Token_Identifier, and mode lists require '<'. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
The old plan built the Sequence type the revised design deletes. Six tasks. 1-3 prepare without changing behavior: hoist passthrough off ModifierFlag, fold chords into Hotkey (compiler-driven migration of ~90 sites), add the uniqueness predicate and prefix lookup context. Task 4 is the atomic flip — old and new models cannot coexist — and deletes Sequence.zig. Task 5 pins the marker-ordering and final-chord passthrough invariants that currently hold only by accident. Task 6 documents and verifies against the real config. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Task 2 mandated a TEMPORARY placeholder-chord hack to defer the parser restructure to Task 4. Unnecessary: the Sequence.create block sits below the prologue and works off chords.items plus an already-constructed hotkey, so Task 2 can restructure properly and Task 4 just deletes the block. Removes a plan-mandated defect a reviewer would rightly flag. Task 5 invented a testSkhd helper duplicating the existing createTestSkhdFromConfig, and passed null for CGEventRef where the established tests use a mock pointer. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
passthrough is a routing marker, not a modifier: isEmpty had to zero it out to stop it gating wildcard matching. Folding chords into Hotkey makes flags per-chord, where a whole-binding property cannot live.
Replaces flags/key with an owned chords slice (len >= 1). eql compares whole chord lists and is false on length mismatch, so cmd-q and cmd-q,cmd-q coexist as distinct HotkeyMap entries. Sequence.zig still drives matching; the next tasks retire it.
zig build bench fails identically at the merge-base with main (std.heap.GeneralPurposeAllocator, removed in Zig 0.16). The plan asserted it compiles; that was never verified and is false. It is not a gate. Records the consequence: build.zig wires benchmark.zig into the bench step only, so zig build test never compiles it and changes there are not compiler-verified. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
onePrefixesOther generalizes triggersOverlap over chord lists. PrefixLookupContext answers complete-vs-pending from one getKeyAdapted call; its optional process_name serves both the applicable-hotkey query and the process-blind claim query. WildcardLookupContext gains the same process check and rejects sequences. Not wired into the runtime yet.
829982d claimed to be a docs-only plan correction but also added src/Compiler.zig and src/IR.zig. Those 895 lines are a stashed WIP from 2025-11-19 ("IR route", branch claude/plan-key-alias-support-...) that a git stash pop left staged in the index; `git add <plan.md> && git commit` then committed the whole index, not just the added file. They do not belong to this branch: nothing imports them, and they use hotkey.flags.passthrough plus hotkey.flags/hotkey.key — all removed by Tasks 1 and 2 — so they would not compile if wired in. They also trip Task 6's own grep gate with 7 false positives. The stash has been restored via `git stash store` and is byte-identical to what was committed here; no work is lost. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Parser builds sequence hotkeys through the ordinary add_hotkey path; Mode.hotkey_map holds them; Mappings.hotkeys is the sole owner. Lookup is one prefix-adapted getKeyAdapted whose result chord count decides complete vs pending, and pending state is a chord-prefix buffer sized from the config at load, so the event loop allocates nothing. Deletes Sequence.zig, Mappings.sequences, Mode.sequences, the candidate matcher, and the parser ownership flag. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Two review findings on Task 4 (hotkey sequences): 1. add_hotkey (Mode.zig) let processScopesOverlap gate the whole conflict check, but HotkeyMap keys on Hotkey.eql, which compares chords only. When scopes were disjoint, add_hotkey continued past the check and hotkey_map.put silently kept the first eql-equal entry, dropping the second (e.g. cmd - a bound separately for Terminal and Firefox). Add an identity check ahead of the scope gate so eql-equal hotkeys are always rejected as duplicates, restoring pre-Task-4 detection while keeping the scope gate for the genuinely-overlapping, non-eql sequence-prefix case. 2. reloadConfig (skhd.zig) committed the mappings swap before allocating the new sequence_prefix buffer. Since reloadConfig's error is only logged (the daemon keeps running), a failed alloc after the swap would leave sequence_prefix sized for the old, possibly smaller max_chords — a later commitPrefix could write past the buffer's end. Allocate the new buffer first so the reload is all-or-nothing. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
The spec claimed identical chords with disjoint process lists would now parse, and that uniqueness still held because the two could never both apply. Implementation proved that wrong: HotkeyMap's context keys on Hotkey.eql, which compares chords and ignores scope, so put() keeps the first and silently discards the second. At most one matched only because the other was never stored — a dropped binding, not a resolved one. Identical chords now conflict unconditionally, as they do today. The rule is more permissive in exactly one shape: chords that overlap without being identical (alt - a vs lalt - a) with disjoint scopes, which are distinct map keys and are genuinely both stored. Also switches the process-list examples to the multi-line form — a `:` command lexes to end-of-line, so `[ "app" : cmd ]` inline swallows the `]`. Found by the Task 4 implementer when the plan's own test config failed to parse. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
The marker check must stay above sequence handling or a forward of the sequence's own trigger gets swallowed by its own first chord. Passthrough applies only to the completing chord, since earlier chords cannot be delivered before completion is known.
SYNTAX.md's sequences section was written against the superseded Sequence-type draft. Reconcile it with what shipped: sequences are hotkeys with >1 chord (every action form works, not a separate construct), ->/~ apply to the final chord only, and the uniqueness rule now states the unconditional identical-chords conflict that HotkeyMap's Hotkey.eql keying forces (scope can't rescue an exact duplicate), alongside the pre-existing overlap/disjoint-scope relaxation. Fix every process-list example to the multi-line form, since a `:` command lexes to end-of-line and swallows a same-line closing `]` (confirmed against the real parser; the one-line form errors, the multi-line and `|`-forward forms don't). README's example now leads with the forwarding form from the design doc (cmd - q, cmd - q [ "Protected App" | cmd - q ]), which needs no shell command. Also rename the skhd.zig test misnamed "sequence completes on a forward of its own trigger" — forwardKey no-ops under builtin.is_test, so it never exercises a forward; it actually pins process-scoped completion and non-matching-app fallthrough. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
chordsOverlap asked hotkeyFlagsMatch(x, y) or hotkeyFlagsMatch(y, x) --
whole-set, one direction at a time. But "can both configs match one
physical event" resolves per modifier family, and the required direction
can differ per family:
cmd + lshift - x : echo A
lcmd + shift - x, cmd - y : echo B
cmd needs x-as-config, shift needs y-as-config, so neither whole-set
direction holds and no conflict was detected. Both were accepted, and one
physical lcmd+lshift+x press matched both -- one complete, one pending.
ArrayHashMap probe order decided which fired.
That made the uniqueness rule aspirational, yet Hotkey, Mode and the spec
all assert it as fact, and PrefixLookupContext depends on it to resolve
complete-vs-pending from one getKeyAdapted call.
Test overlap per family instead. familyOverlap intersects the keyboard
states each config accepts, derived from the semantics hotkeyFlagsMatch
already encodes: a general bit accepts general/left/right, a specific bit
accepts exactly that side, an absent family requires the event to lack it.
The rule now delivers the invariant the runtime relies on.
Configs mixing general and specific modifiers across families in two
overlapping rules now error where they previously parsed. They were
already nondeterministic; this turns a silent coin flip into a config
error. The real config (178 hotkeys, 2 modes) has zero such rules.
compareLRMod is untouched -- it stays eql's exact-equality predicate.
Also:
- SYNTAX.md: the `->`/`~` example declared `cmd - k, cmd - c` twice, so a
shipped example failed to parse. Second line is now `cmd - k, cmd - u`.
- skhd.zig: pin the reload buffer swap. The existing test reloaded
1-chord -> 1-chord, so the resize never ran despite its comment naming
the failure mode as a silent out-of-bounds write in ReleaseFast.
- Hotkey.zig: passthrough's doc comment referred to a completed plan task
in the future tense.
Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Mode.add_hotkey relies on onePrefixesOther being order-independent and on eql-equal hotkeys always overlapping (so the identity gate stays reachable). Neither property was tested directly, so a future edit to familyOverlap's branch order could break either silently. Exhaust the 8-state (general, left, right) space for one modifier family and check both properties over all 64 ordered pairs plus the 8 self-pairs. Also reword a stale spec parenthetical that justified eql implies onePrefixesOther via hotkeyFlagsMatch, an implementation onePrefixesOther no longer uses — it goes through familyOverlap now. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
docs/superpowers/ (specs, plans) and .superpowers/ (the subagent-driven development ledger) are working notes for the design/implementation process, not project documentation. They were only ever on this branch, never on main, so untracking them here keeps them out of the merge. Files stay on disk; .gitignore keeps them from coming back. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
jackielii
marked this pull request as ready for review
July 16, 2026 17:20
.sequence_timeout makes the inter-chord budget configurable; the 300ms
default is unchanged. It lives on Mappings, so hot reload picks up a new
value by riding the existing mappings swap. Zero is rejected — it would
expire every prefix instantly while still swallowing the first chord.
parse_duration_ms gains the `s` suffix SYNTAX.md already documented but
never implemented, which benefits .remap tap-hold timeouts too. The
suffix must now sit on the same line as the number: newlines are not
tokens and resolveIdentifierType lexes any single character as a
Token_Key, so a bare `s` opening the next line is otherwise
indistinguishable from a seconds suffix —
.sequence_timeout 500
s : echo hi
would silently mean 500 seconds and orphan the rest of the line.
skhd -k dumped a Zig backtrace on a bad keyspec: synthesizeKey did
`try parser.parse(...)` and discarded parser.error_info. It now prints
the parser's diagnostic, points at -t/--text for prose, and exits 1.
The two "parsed but no hotkey" fallthroughs returned success after
printing an error; they now return InvalidKeySpec too.
skhd -k itself was NOT broken — CGPostKeyboardEvent works, verified by
a closed-loop test (synthesized f19 reached a live tap and fired its
command). It deliberately stays unmarked so configs can use it to
trigger skhd's own hotkeys.
Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
skhd -k "hello world" answered:
1:1: error: Mode 'hello' not found.
Did you forget to declare it with '::hello'?
Nonsense to someone synthesizing a keypress. The cause was structural,
not a bad message: synthesizeKey fabricated a config line
(`<spec> : __dummy__`) and fed it to the config-file parser, which saw a
leading identifier and read it as a mode name. The same leak let -k
silently accept config grammar — `-k 'cmd - q [ "Finder" : echo x ]'`
failed with a process-list diagnostic rather than being rejected outright.
parseKeySpec parses exactly one chord and nothing else, so diagnostics
describe the key the user typed:
skhd: 'hello world' is not a valid key combination.
1:1: error: Expected key, key hex, or literal near 'hello'
-k/--key takes one key combination: 'cmd - q', 'shift - a', 'f19'.
To type literal text, use -t/--text instead.
synthesizeKey no longer builds a Mappings, a mode and a hotkey just to
read one chord out again.
Also guards createAndPostKeyEvent with builtin.is_test — the same guard
forwardKey uses — so a happy-path test cannot fire real keystrokes into
the session running the suite. That makes the valid-keyspec test safe to
have at all. Drops "synthesize key parsing", which lived here but only
ever exercised the config parser.
Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
A binding may now be both complete on its own and the prefix of a longer
sequence. This was AmbiguousSequencePrefix; the error is retired.
lcmd - k : yabai -m window --focus north # everywhere
cmd - k, cmd - k [
"Google Chrome" | cmd - k # Chrome's own Cmd-K
]
In Chrome one Cmd-K runs yabai after the timeout; two send Chrome its own
Cmd-K. Elsewhere Cmd-K fires yabai immediately — the sequence isn't
applicable there, so no longer match exists to wait for. That is how a
global binding and an app's native shortcut share one chord, which
previously needed a second chord (ctrl+cmd-k) as a workaround.
The rule: don't discard the first press if there is something to do with
it. Discarding was only ever right when nothing shorter was declared.
This is Vim's timeoutlen, with two differences: the wait is scoped to
apps where a longer match applies rather than unconditional, and skhd
fires a declared binding rather than replaying raw keys.
The cmd-q safety property survives, because it was never "prefixes are
discarded" — it is "we never invent an action the user didn't declare".
cmd - q, cmd - q ["Protected"] has nothing shorter, so the timeout still
does nothing; macOS's own Cmd-Q is not an skhd binding and can never be
a fallback.
Each chord now asks two questions instead of one: is there an exact match
(the fallback), and is a longer one still reachable (go pending). The
longer query is skipped when max_chords == 1, so configs without
sequences pay nothing. Determinism still rests on at most one exact match
per (mode, prefix, process), which the equal-length rule guarantees.
`~` and `->` cannot be a fallback: both release the original keypress,
but a prefix chord is consumed on arrival, so nothing is left to deliver.
Rejected at parse time, naming the reason.
No config that parses today changes behavior — the configs that gain
fallback are exactly those that error today. Verified: the maintainer's
178-hotkey config parses unchanged, and gains the Chrome binding it
could not express before.
Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Both bugs fall out of the one structural change the design named: the retry loop now executes actions rather than only clearing state. 1. Stale `mode` across `continue`. The mismatch path fires the fallback before retrying, and a `; mode` activation reassigns current_mode — so the retry probed the old mode's map. The same two keys resolved differently depending on whether the timeout beat you to the second chord. Re-read current_mode per iteration. 2. `sequence_deferred = exact` erased a live fallback at a depth with no exact match. With `lcmd - k` and a three-chord sequence but no two-chord binding, stalling at two chords silently dropped yabai — while `k` then an unrelated key still fired it. Only overwrite when this depth has an exact match, which is what "most specific complete match seen" always meant. Also from review: the mismatch path now logs a failed fallback instead of propagating (matching the timer path, so a fork failure can't also swallow the chord); `null` replaces `@ptrFromInt(0)` for the absent CGEvent; tests forward to f19 rather than fork a shell, matching the convention the neighbouring tests already follow. Docs: show rather than explain. The fallback section is a table, the uniqueness rule is four lines plus labelled examples. Fixes a real contradiction — the uniqueness section still documented the retired "prefix + overlapping scope = conflict" rule and marked as an ERROR the exact shape the fallback section introduces as the feature. Every example is verified against the real parser: the OK ones parse, the ERROR ones are rejected. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
Accumulated by throwaway verification tests appended and removed during development. Flagged in review. Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
`skhd --status` printed
warning(agent_grabber): grabber socket not found at
/var/run/skhd/grabber.sock — is skhd-grabber installed and running?
for every user without .remap/.taphold rules — i.e. anyone who needs no
grabber at all — directly above the line that already answers the
question calmly ("skhd-grabber version: not running").
Client.connect logged the failure itself, but four of its five callers
treat "not reachable" as an ordinary answer: two probes return
false/null, --status reports it as a field, and --grabber-status prints
its own [FAIL] line (so the warning was also a duplicate). Whether a
failed connect is worth mentioning is the caller's call, not the
primitive's.
connect now returns the error silently. The one caller that genuinely
wants a warning — the agent, which only dials when the config HAS rules
to forward, so a failure means something the user asked for is broken —
logs it at the call site via connectErrorMessage. That makes "warn only
when the grabber is needed" fall out of the call graph rather than
needing a flag.
--grabber-status still reports [FAIL] IPC socket not reachable;
--grabber-test-rule still propagates.
Claude-Session: https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds comma-separated hotkey sequences, addressing the
cmd - qqrequest in #51:A sequence is a hotkey
The first cut of this branch added a
Sequencetype owning its action*Hotkey. That split ownership ofHotkeybetweenMappings.hotkeysandSequence— one type, two owners, depending on how it was declared — which forced an ownership-transfer flag through the parser, gaveModetwo collections that cross-checked each other, and broke the documented allocation-free event loop by duping the process name on every keystroke.This replaces that:
Hotkey.chords: []const KeyPress(owned,len >= 1) replacesflags/key, and a sequence is simply a hotkey with more than one chord.Gone:
src/Sequence.zig,Mappings.sequences,Mode.sequences, the candidate matcher, the parser's ownership flag, and three near-duplicate overlap predicates.Mappings.hotkeysowns every hotkey, with no exceptions.passthroughalso moves offModifierFlagontoHotkey. It was a routing marker squatting in a modifier bitfield —isEmpty()had to zero it out to stop it gating wildcard matching — and onceflagsmeanschords[0].flags, a whole-binding property has nowhere to live.How lookup works
PrefixLookupContextanswers complete-vs-pending from a singlegetKeyAdaptedcall: the hit's chord count against the prefix length. No candidate list is needed — when two sequences share a prefix, either one proves "pending" and the next chord disambiguates.ArrayHashMapwalks its probe chain througheqlfailures, so an inapplicable hotkey is skipped rather than shadowing a longer sequence behind it. That is what letscmd - q ["Terminal"]coexist withcmd - q, cmd - q ["XYZ"].This is sound only because a config-time rule guarantees at most one hotkey matches any (mode, prefix, process) — probe order is Robin Hood order, not config order. Pending state is a chord-prefix buffer sized from the config at load, so the event loop is allocation-free again.
Behavior
->and~apply to the final chord only. Earlier chords can't be delivered, since completion isn't known when they arrive.fn_layer < cmd - hcases.HotkeyMapkeys on chords alone, soputwould keep one and discard the other.cmd + lshift - xvslcmd + shift - xpreviously parsed while one physical press matched both.Not verified
The 300ms timeout and cross-app
cmd-qpassthrough were not exercised end-to-end — verification ran on a live machine and was aborted rather than injecting a blindcmd-q. Worth a manual pass or a VM before trusting this in anger. The 300ms timer has no test (needs a liveCFRunLoop), and the capture-mode claim gate is correct-but-unpinned (testing it needs a forward-observing seam that doesn't exist).Pre-existing bugs found, not fixed here
--validate/parse-only flag — any config check boots the full daemon and grabs an event tap.main.zig:189writes/tmp/skhd_<user>.pidwith no ownership check, so a dev run clobbers the installed service's pidfile and silently breaksskhd --reload(service.zig:498-508signals whatever PID it reads).zig build benchis broken onmain—src/benchmark.zig:9usesstd.heap.GeneralPurposeAllocator, removed in Zig 0.16.Testing
zig build testandzig buildboth pass. New coverage: chord-listeql, the per-family overlap predicate plus property tests pinning its symmetry (64 pairs) and reflexivity (8), prefix lookup (complete/pending/skip-inapplicable/shared-prefix), the uniqueness rule in both directions, the prefix-buffer state machine, reload buffer resize, re-entrancy (a forwarded chord must not re-match its own sequence), and final-chord passthrough. The pre-existing duplicate-detection tests pass unmodified — they're the canary that the rule generalizes rather than relaxes.https://claude.ai/code/session_01A6HFtHy1SKeSdghZcQ5EHb