fix: require a modifier key for keyboard shortcut recording

[akshay-soni]

Description

The keyboard shortcut recorder in handleShortcutKeyDownEvent (Sources/Fluid/ContentView.swift) currently accepts a bare key with no modifier (e.g. pressing F alone, keyCode 3) as a valid global hotkey. Once saved, that key is intercepted system-wide by the app's global event monitor, breaking normal typing of that key in every other app, with no UI path to remove the offending shortcut.

The mouse-click recorder already guards against this exact class of bug (isUnmodifiedLeftOrRightClick → "needs a modifier key"). This PR adds the equivalent guard for keyboard shortcuts.

Design note — the .function flag subtlety: a naive relevantModifierFlags.isEmpty check is not enough. macOS auto-sets the .function modifier flag on key events for function-section keys (arrows, F1–F20, Home/End/Page Up/Page Down, Forward Delete) even when the Fn key is not held — and the global event tap maps .maskSecondaryFn back to .function the same way, so a bare arrow key recorded under that check would still be intercepted system-wide. The guard therefore lives on the model as HotkeyShortcut.isUnmodifiedKeyboardKey (the keyboard analogue of isUnmodifiedLeftOrRightClick): it ignores the auto-injected .function flag for function-section key codes, while still honoring a genuine Fn chord on typing keys (e.g. Fn+F) and excluding modifier-only shortcuts (e.g. Right ⌥), which are legitimate.

The cancel shortcut is exempt: the global tap only consumes it while dictation is running or a cancel callback handles it (GlobalHotkeyManager), and it legitimately defaults to a bare Escape.

Type of Change

• 🐞 Bug fix
• ✨ New feature
• 💥 Breaking change
• 🧹 Chore
• 📝 Documentation update

Related Issue or Discussion

Closes #556

Testing

My environment has Xcode Command Line Tools only (no full Xcode), so I could not run xcodebuild or the XCTest suite locally, and swiftlint --strict fails locally for lack of sourcekitd. This PR stays in draft so the repo's Build and Test CI can confirm compilation, lint, and tests before it's marked ready for review.

What was verified locally:

• Sources/Fluid/Models/HotkeyShortcut.swift compiles standalone with swiftc against the macOS SDK, and a 15-assertion harness exercising isUnmodifiedKeyboardKey (bare letters/Space rejected; ⌘F and genuine Fn+F accepted; bare arrows/F5/Home/Forward Delete rejected despite their auto-injected .function flag; ⇧+arrow accepted; modifier-only and mouse shortcuts excluded) passes in full.
• Unit tests covering the same cases were added to the existing Tests/FluidDictationIntegrationTests/HotkeyShortcutTests.swift for CI to run; the file passes swiftc -parse.
• swiftformat --lint reports byte-identical findings on the touched files before and after the change (all pre-existing elsewhere in the files) — no new formatting issues introduced.
• Every symbol the ContentView hunk touches was verified against the current source and mirrors the neighboring mouse-shortcut guard.

Screenshots / Video

• No UI/visual changes; screenshots/video are not applicable.

This changes rejection behavior of an existing recorder, not any visual layout — the same "needs a modifier key" message/style the mouse recorder already shows is reused as-is.

Notes

Open question for maintainers: as implemented, bare F-keys (F1–F20) are rejected along with bare typing keys, arrows, and navigation keys — consistent with the PR title, and matching how the .function flag actually behaves (an earlier revision of this PR believed bare F-keys were blocked while arrows were the open question; the auto-injected .function flag made the opposite true). If you'd prefer to allow bare F-keys as global shortcuts (they collide less with normal typing than letters or arrows), it's a one-line change: drop the F-key codes from HotkeyShortcut.functionSectionKeyCodes so their .function flag counts as a real modifier again. Happy to adjust.

Possible follow-up (out of scope here): users who already saved a modifier-less shortcut before this fix still have it persisted in settings (and it survives backup/restore). They can now recover by re-recording, but a one-time sanitization on settings load that strips modifier-less keyboard shortcuts for non-cancel targets would auto-heal existing installs. Can open a separate issue/PR if wanted.

[github-actions]

The PR Policy check is blocking this PR because required template information is missing.

Please update the PR description with:

• Testing

Visual files detected:

• Sources/Fluid/ContentView.swift

Screenshots or video are required for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes. If this PR has no visual changes, check the no-visual-change box in the template.

If this remains incomplete for 48 hours after opening, the PR may be closed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bfb1a2b5f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve explicit Fn on function-section shortcuts

When the user deliberately holds Globe/Fn and records an F-key, arrow, or navigation key, the flagsChanged path has already put .function into pendingModifierFlags, but this branch strips .function solely because of the key code. As a result Fn+F5 or Fn+Left is treated the same as a bare auto-flagged key and is rejected with “needs a modifier key,” even though the app otherwise treats Fn as a supported modifier (for example Fn+F remains allowed). The guard needs a way to distinguish an explicit Fn press from the auto-injected flag before dropping it.

Useful? React with 👍 / 👎.

[akshay-soni]

Thanks, the record-time half of this is accurate. pendingModifierFlags does know the difference when Fn is physically held (the flagsChanged handler unions .function in), and this guard rejects an explicit Fn+F5 / Fn+Left regardless.

The problem is that match time can't make the same distinction. The global tap derives modifiers purely from CGEventFlags, and macOS sets .maskSecondaryFn on bare presses of every function-section key. So HotkeyShortcut.matches() sees a saved Fn+Left ([.function] + keyCode 123) as identical to a bare Left arrow press. If we preserved the explicit Fn at record time, the saved shortcut would fire on (and consume) every bare Left press system-wide, which is exactly the #556 breakage this PR is closing. For F-keys the collision depends on hardware and settings: Apple keyboards in media-key mode never deliver a bare F5 keyDown, but external keyboards and "use F1, F2, etc. as standard function keys" mode do, so the recorded shortcut would silently behave differently across setups.

Distinguishing a physical Fn hold at match time would mean tracking Fn state in the event tap via flagsChanged (keyCode 63). That gets fragile fast: tap restarts while Fn is held, Globe key remapping, external keyboards that have no Fn key at all. Out of scope for this fix in my opinion.

If F5-style hotkeys are wanted, I think the honest version is the option already flagged in the PR notes: drop the F-key codes from functionSectionKeyCodes so F-keys are recordable again, accepting that they intercept the bare key, rather than recording a modifier the matcher can't honor. Happy to do either depending on maintainer preference.

[altic-dev]

I think it's on purpose and our uses love to add any key to the system. Since it's optional, it's within your control to decide what key to use. More freedom is better than less freedom :)