Repository navigation
feat: editable settings with popover app UI - #8
Merged
Merged
Conversation
Protocol 1.8 introduces the capability-gated `applySettings` selector so the Agent stays the only writer of its own settings source of truth. Settings were readable but immutable, which left the unlock hotkey and auto-unlock timeout fixed at their defaults. Two lock-out guardrails shape the design: `LockEngine.updateSettings` re-arms the auto-unlock timer from the moment it is called, and cancels it outright for a `.disabled` policy. Applying settings during an active lock could therefore extend or remove the fail-safe that recovers a locked keyboard, so a running lock keeps its active configuration, gesture, start time, and deadline; new values seed the next lock. Validation lives in `Common` and is enforced by the Agent rather than by a wrapper, since every future write surface reaches the same entry point. A hotkey needs at least one modifier that survives event-tap normalization and a key code the ASCII-capable layout can display; a timed policy is bounded and finite. `.disabled` stays reachable — the UI carries the warning. The reply returns the values actually stored, not the request, because validation normalizes them. A lost reply re-reads the authoritative settings instead of re-sending, keeping the outcome explicit rather than assumed. Also removes the duplicated capability/minor table from ClientTests, which had already drifted from the implementation by omitting this selector.
The coordinator only carried a lock Boolean, which is not enough to present auto-unlock deadlines or let the user edit the configuration. It now also publishes the authoritative lock snapshot and the persisted settings. The two settings values answer different questions and are kept separate on purpose: `currentSettings()` is the stored configuration the next lock will use and is what the settings UI edits, while `lockStatusSnapshot().settings` is what a running lock is enforcing. They diverge while locked, because a write leaves an active lock alone — `hasSettingsPendingNextLock` reports exactly that state so the UI can say so rather than implying the change is already in effect. A settings read failure becomes explicitly unavailable instead of falling back to `.default`, which would invent a second source of truth and could display an unlock hotkey the Agent is not using. Detail is dropped whenever the Agent stops being trustworthy for it — including when an update is required, since such an agent may not implement the write selector at all — and a detail failure never downgrades an otherwise ready state.
The app had no window at all: every interaction lived in menu items and modal alerts, which cannot express a countdown, a lock start time, or an editable hotkey. This adds two SwiftUI windows hosted in AppKit controllers. The entry point stays an AppKit accessory process. Converting it to a SwiftUI `App` to gain a `Settings` scene would put the activation policy, services provider, and URL handling — three already-tested paths — at risk for no user-visible gain, so `NSHostingController` provides the same windows without touching startup. The status menu keeps every existing item. It remains the surface reachable while the keyboard is locked and only the mouse works, so the window is a parallel entry point rather than a replacement; the window reuses the menu's diagnostics and CLI flows instead of duplicating them. Settings enforcement stays in the Agent. `HotkeyCapture` defers to `Hotkey.validated()`, so the picker cannot offer a combination the write path would reject, and it drops CapsLock, Fn, and numeric-pad flags that the event tap's matcher discards. Escape and Return leave the recorder rather than being recorded, keeping a keyboard-only exit. Turning auto-unlock off stays reachable but requires confirmation naming the remaining unlock routes. While locked, the window and settings both state that stored values apply to the next lock, since a write deliberately leaves an active lock untouched.
Settings were documented as intentionally read-only for the MVP. Protocol 1.8 adds `applySettings`, so the architecture contract, XPC guide, development map, and project overview now describe the write path and its boundaries: validation lives in `Common` and is enforced by the Agent, a locked keyboard keeps its active configuration while new values seed the next lock, and reads that fail stay unavailable rather than falling back to defaults. Also documents the new SwiftUI UI layer, the settings-store save path, the coordinator's snapshot detail, and the test coverage for all of the above, including the on-device checks that remain manual.
Clicking the menu-bar icon opened a plain NSMenu, and status/settings lived in separate NSWindows — text-heavy and visually rough. The icon now toggles one SwiftUI popover that carries the whole UI: a status card, the primary action, and settings behind an in-popover back navigation. Low-frequency actions (diagnostics, CLI, quit) move into a bottom "⋯" menu. This is a presentation-layer change only. The domain layer — AppUIStore, AppCoordinator, the Client, and protocol 1.8 — is untouched, which is exactly what keeping the UI a thin layer bought us. The store gains only pure presentation routing (recovery-action list, safety-check availability) derived from the same snapshot. Constraints preserved: - The popover stays a mouse-reachable unlock surface while locked: a left-click opens it and it carries the Unlock button, needing no keyboard. - Destructive or system-level actions (agent update/restart, quit while locked, auto-unlock off, CLI management) stay modal NSAlerts; the popover closes before each so it never covers the alert. These live in StatusItemController and reach the SwiftUI layer through PopoverActions, keeping the views free of AppKit presentation. - Every AppCoordinator.State/Activity branch is mapped in the status page, so approval-required, accessibility-required, update-required, replacement, and unavailable states keep their explanation and recovery button. StatusMenuController becomes StatusItemController; AppWindowPresenter becomes PopoverPresenter; MainView/SettingsView become StatusPage/SettingsPage under a PopoverRootView that owns the status↔settings navigation.
The component map and app-role notes still described the NSMenu and the separate status/settings windows. Update them to the single NSPopover surface, the StatusItemController/PopoverPresenter split, the StatusPage/SettingsPage navigation, and the PopoverActions seam, and adjust the manual-verification boundary to popover presentation.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Summary
Implements the documented post-MVP "user-editable settings" extension, keeping the architecture contract: the Agent remains the single owner and executor of settings; all UI goes through the XPC boundary.
KeyboardLockerServiceProtocolwrite method implemented byAgentServiceand persisted by the Agent; settings validation covered by newClientTests/ServiceTestssuitesStatusPage+SettingsPage), backed byAppUIStore; lock detail and agent-reported settings are surfaced read-first, edits go through the Agent write pathHotkeyRecorderField+HotkeyCapturefor capturing a new unlock hotkey in Settingsarchitecture.md/development.md/xpc.mdupdated to describe the settings write path and the popover UI layerVerification
xcrun swift test --package-path Core— 200 tests, 24 suites, all passedxcodebuild -scheme KeyboardLocker -configuration Debug -destination 'platform=macOS' test -only-testing:KeyboardLockerModelTests— TEST SUCCEEDEDd31a10d), no conflicts