Repository navigation
feat: single-core Agent + XPC architecture with all automation wrappers - #7
Merged
Merged
Conversation
The lock is a single global state, so locking while already locked is now a no-op success that re-applies the latest settings (re-arming the auto-unlock timer), rather than throwing. Drop the `.alreadyLocked` error and the "owned by another session" semantics it implied. Add `updateSettings(_:)` so a running lock picks up settings changes live. Replace `print` with `os.Logger`.
… to App Move `KeyboardLockerSettingsStore` from Common into Service so no wrapper can `import Common` and spin up its own UserDefaults-backed store — the Agent is the settings source of truth (architecture contract). Move `SystemSettings` (opens the Accessibility pane, pure UI) out of the shared core into the App target and drop its `public`.
Make the single-core contract real: settings now cross XPC and the Agent owns them, and every operation is a stateless one-off call. - Protocol: add `applySettings(Data)` / `currentSettings`; settings travel as JSON `Data` across the `@objc` boundary via new `KeyboardLockerSettings.encodedForXPC()` / `decodedFromXPC(_:)` (corrupt or nil data falls back to `.default`). - AgentService: becomes the settings source of truth — loads persisted settings on init, seeds the engine, and `lockKeyboard` uses stored settings instead of `.default`. - XPCClient: rewrite as async/throwing with one auto-reconnecting connection; a dead Agent throws instead of hanging. Delete `LockSessionController` / `startLockSession` — the forbidden client-owned "session" abstraction. - LockStateSubscriber: add `stateChanges` AsyncStream alongside the token API. - klock: rewrite onto async one-off calls (no more semaphore hang path), print the Agent's real unlock hotkey rather than a local default.
- AgentRegistrar: register the Agent as a launchd-managed login item at launch (idempotent) so it runs on demand for every surface even when the App is closed — the contract's Agent lifecycle requirement. - LockController: thin @mainactor ObservableObject that issues async XPCClient calls and reflects the Agent's broadcast state, never inferring state from whether a call succeeded. Holds no lock/settings logic. - MainView: minimal lock/unlock toggle with live status and error surface, wired through the environment. Settings/About stay stubs.
…ervice Update the XPC flow, component map, and common-task recipes to match the single-core rewrite: async stateless XPCClient (no session), Agent-owned settings via applySettings/currentSettings, relocated store/SystemSettings, SMAppService registration, and idempotent lock. Correct the stale note claiming lock throws `.eventTapCreationFailed` without permission (it throws `.accessibilityPermissionDenied`).
Incidental normalization swept up by `swiftformat .` in files this refactor did not otherwise modify (spacing around `..<`, expanded single-line computed properties). No behavior change.
A distributed broadcast can be missed while a menu-bar app is suspended, leaving the UI showing a stale state until the next change. Fetch the Agent's authoritative state via `LockController.reconcile()` on `ContentView.onAppear`, so opening the menu always shows the truth regardless of what happened (CLI lock, timeout, hotkey) while idle.
…tate Rework LockStateSubscriber so notifications are signals, not truth: on any signal it fetches the Agent's state via `status()` (retried across a transient Agent relaunch, de-duplicated) rather than trusting a payload that can be dropped, reordered, or stale. - Add a Darwin channel observer alongside the existing Distributed one. Darwin can wake an App-Napped / suspended menu-bar app so it learns of a lock started by another surface while it was idle; Distributed remains the reliable path for a running CFRunLoop (and the async CLI). - Document the two wrapper shapes in the architecture contract: one-shot surfaces (CLI/AppleScript/Shortcuts) are synchronized by construction via a direct `status()` call and must NOT add notification handling; only long-lived state-reflecting UI (App/Widgets) subscribes and reconciles.
The State Synchronization section existed but nothing in the "Adding a New Wrapper" checklist or the violations list pointed to it, so someone adding Shortcuts/AppleScript could follow the checklist and never learn a one-shot surface must not add notification handling. Add checklist item 6 (read state per your shape) and two violation entries (one-shot surface subscribing/caching; trusting a payload over status()).
Translate the prose in docs/architecture.md and docs/development.md to Chinese while keeping all code identifiers, file paths, protocol/method names, commands, and the ASCII diagram in English (per the repo's Chinese-context-with-English-code-blocks convention). No content changes.
Prepend a section grounding the contract in the concrete failure modes of a prior implementation (the `main`-branch design): per-process `.shared` singletons that never shared state across App/CLI/AppleScript, callback-based state push that only reached the local process, layering split by concern instead of by process boundary, and per-process UserDefaults drift. Each maps to a rule the contract already states, so newcomers see the "why" before the "what".
A public initializer let any wrapper construct its own XPCClient with a separate cached connection, defeating the single-connection reuse the type guarantees and inviting cross-instance state drift — the in-process cousin of the very divergence the single-core contract forbids.
Subscribers already treat every signal as a content-free hint and fetch authoritative state via status(), so the distributed userInfo was dead data that contradicted the 'payload is never the source of truth' rule. broadcast(isLocked:) becomes a pure broadcast(); docs realigned.
The flagsChanged branch ignored the hotkey's modifier flags and fired on any modifier change while the hotkey's key was physically held — a false unlock risk for the default ⌃⌘L (unlock already flows through keyDown). Its only intended use, modifier-only hotkeys, is unreachable with no settings UI. Dropped the branch; flagsChanged stays in the event mask so modifier keys are still blocked while locked.
lockStartedAt, autoUnlockTargetDate, lockDuration(at:), and remainingAutoUnlockTime(at:) have no cross-module consumers. Dropping public honors the 'minimal public surface' rule — every public in Core is a maintenance commitment — while keeping them available in-module for a future state-display UI.
applySettings, requestAccessibilityPermission, and accessibilityStatus were fully plumbed (protocol + client + agent) but had no caller — no settings UI ever wrote, and no wrapper drove the permission prompt. Per 'minimal public surface' and 'slow is fast', drop the speculative machinery rather than ship dead infrastructure: - protocol keeps only lock/unlock/status/currentSettings (read) - AgentService settings become an immutable init-time value; the mutable store/lock and currentSettingsValue() collapse away - AccessibilityManager keeps hasPermission() (used by LockEngine.lock); requestPermission() and the SettingsStore.save() write path go - delete SystemSettings.swift (dead Accessibility-pane opener) Docs realigned: the architecture contract now states settings are read-only and permission is checked in-engine, with write/request paths marked as unwired — must still route through the Agent when added.
- showsUnlockNotification: no notification code exists anywhere, so the flag toggled nothing — removed until a notification feature lands. - AutoUnlockPolicy.presets / PresetTimeouts: UI-binding helpers with no UI consuming them. - Hotkey.isValid: validation helper with no caller. Shrinks the model to what the engine actually reads (autoUnlockPolicy, unlockHotkey).
CLIInstaller (symlink klock into /usr/local/bin via AppleScript admin prompt) had no call site — the App exposed no install entry point — and its supporting SharedConstants.cliName / cliInstallPath were consumed by nothing else. Delete the file, the constants, and the doc section. The klock binary still builds with the bundle; add an install path back with a real UI when the feature is actually wanted.
Use the running agent signing identity as the Release authorization source of truth. Remove the stale hard-coded Team ID and certificate common-name fallback that rejected valid signed clients.
Mark the embedded Agent and CLI targets as skip-install dependencies so the archive contains only the main app while preserving both embedded products.
Keep all mouse input and system-defined media keys available while locked. Align permission messaging and project guidance with the keyboard-only product contract.
Advertise a versioned service descriptor and gate optional calls on the negotiated connection generation. Protect replacement with an exact-instance prepare/commit drain and recover unknown commit replies through ticket status.
Install same-team, exact-identifier requirements on both XPC endpoints. Embed the namespaced signing identifier in klock and cover requirement construction with unit tests.
Model registration, compatibility, Accessibility, and replacement states explicitly in the wrapper UI. Extract readiness and replacement orchestration behind injectable adapters while keeping LockController as the presentation facade.
Describe connection identity, mutual peer authentication, compatibility negotiation, and fail-safe replacement. Record App-side coordinator ownership, recovery behavior, and the safe test-carrier boundary.
Reconcile lock state only after observers are installed and suppress stale deliveries across cancellation and concurrent signals. Combine notifications with bounded authoritative polling for CLI unlock observation, align auto-unlock deadlines with timer rearming, and remove debug scheme actions that could silently terminate the active agent. Add deterministic regression coverage and align local validation with the active Xcode toolchain.
Keep app-side coordination framework-independent while the UI is redesigned.\n\nReplace the SwiftUI bootstrap with headless Agent registration and update the architecture documentation to match.
… event tap Replace OSAllocatedUnfairLock manual state management with @mainactor isolation and a LockRuntimeState value type that enforces atomic transitions. Introduce EventTapInstallation as a generic transactional factory — any incomplete tap/source installation is rolled back in reverse order, so callers never retain a half-installed input filter. LockEngine changes: - Extract LockRuntimeState: begin/updateSettings/setAutoUnlockTargetDate/ end enforce the legal state machine inline - Extract EventTapInstallation<Tap, Source>: createTap → createSource → attachSource → enableTap → isTapEnabled; defer block rolls back on any failure - Annotate LockEngine @mainactor; remove stateLock and withLock(_:) - Add eventTapGeneration to detect stale re-enable attempts in handleDisabledEvent; if re-enable fails, tear down and transition to unlocked (fail-open rather than reporting a phantom lock) - lock() is now strictly idempotent: duplicate calls return without touching settings, lock-started-at, or the auto-unlock deadline - @preconcurrency import CoreGraphics / ApplicationServices for sendable-unsafe CG types used within @mainactor context
Replace silent .default fallback with explicit error propagation so wrappers never fabricate a second "current settings" snapshot. Common: - Add KeyboardLockerSettingsCodingError (invalidPayload, missingPayload, payloadTooLarge) with LocalizedError descriptions - decodedFromXPC(_:) now throws instead of returning .default on failure; validate size before decoding - encodedForXPC() validates size after encoding (max 16 KB) - Add currentSettingsWithError(reply:) to the @objc protocol as an additive selector alongside the legacy currentSettings(reply:) - Bump ServiceContract to protocol v1.2, require currentSettingsWithError capability Client: - XPCClient.currentSettings() calls currentSettingsWithError and throws on failure - During interruption, invalidate the NSXPCConnection object so it can't transparently reattach to a different Agent process generation - Wrap the connection in XPCConnectionReference for safe sharing between the timeout task and the proxy error handler - Annotate ObserverToken @unchecked Sendable (immutable, tear-down is thread-safe) Tests: - KeyboardLockerSettingsCodingTests: round-trip, missing/invalid/oversized - ServiceCompatibilityTests: currentSettingsWithError is required; descriptor missing it is flagged
Quitting removes the only in-App lock indicator while the Agent-owned lock stays active. The alert names the remaining exits: the configured unlock hotkey, the notification's Unlock Now button, and klock unlock.
An interactive klock lock that acquired the global lock now observes SIGTERM/SIGHUP/SIGINT via GCD signal sources and makes a bounded, best-effort attempt to release the lock it created before exiting with the conventional 128+signal code — closing a terminal or killing the process no longer strands the keyboard. SIGKILL/SIGSTOP remain uncovered by design; the hotkey, notification action, and auto-unlock cover them. --no-wait and already-locked invocations never touch an existing lock.
- Treat the last-known-locked checking state as locked in the quit warning, matching the toggle action's evidence standard - Name the KeyboardLocker agent consistently across errors, menus, and accessibility recovery guidance, and add an Open Accessibility Settings menu entry beside the grant action - Describe the Focus filter's conditional release on deactivation in the intent description - Point CLI service-unavailable failures at klock register-agent and give timedOut a recovery suggestion - Keep locked-with-condition states on the lock icon and rename the steady replacement state to Agent Update in Progress - Fail key-code conversion to nil for unmappable codes so hotkey copy falls back to a verbal phrase instead of a raw "?" - Match the Control gallery preview to the widget's locked sample, and document termination-cleanup generation blindness plus degraded-state limits for Control and silent surfaces
Services handlers only accepted requests synchronously and executed them async, so the AppKit error out-pointer — the one Services channel that can carry a failure reason back to the requesting app — went unused. Handlers now wait, bounded (15s, covering the XPC reply window plus the idempotent retry), on a new ExternalAutomationController.submitAndWait that runs through the same serial chain and returns the failure. When AppKit invokes a handler on the main thread, the wait pumps the run loop instead of hard-blocking, so the main-actor execution chain can proceed. The in-app failure alert is unchanged. Out pointers and pasteboards are still never captured into async tasks.
The menu item shows a direction verb (Lock Keyboard / Unlock Keyboard), so AppCoordinator performs the displayed direction as an explicit idempotent lock/unlock. That is deliberately not the atomic toggle(): the escape-hatch states must only ever unlock, explicit verbs work on legacy agents without the lockToggle capability, and idempotent mutations survive timeout retries. The old toggle() name hid that distinction; rename it at the coordinator, the menu selector, and the test. No behavior change.
Wires the atomic Agent toggle into the CLI: klock toggle flips the global lock inside the Agent serial boundary and prints the resulting authoritative state (Locked. / Unlocked.), the same wire method the Shortcuts Toggle Keyboard Lock action returns. Exit codes stay 0 on success and 1 on failure; legacy agents without the lockToggle capability fail explicitly instead of falling back to a client-side read-then-mutate race.
Gives CLI users the Accessibility-permission path that previously existed only in the app menu. The command short-circuits when the Agent already has access, otherwise triggers the system prompt through the Agent and polls (bounded, 30s by default) for the authoritative grant state — a sent prompt request proves nothing about the grant. Granted exits 0; a pending grant at the end of the window exits 1 with guidance on stdout, kept distinct from stderr error reporting.
Exposes the Agent lockStatusSnapshot through the CLI as a separate JSON contract, leaving the stable single-line --json object byte-identical. The snapshot emits locked, startedAt, autoUnlockTargetDate, and unlockHotkey with alphabetical keys and ISO-8601 GMT dates (absent dates are null) so automation can compare output verbatim. The call stays capability-gated: legacy agents without lockStatusSnapshot fail explicitly instead of falling back to fields fabricated from status.
Install signal handling before the interactive mutation so termination cannot race past lock acquisition. Resolve acquisition, wait completion, and bounded cleanup through one terminal result without unlocking a lock owned by another caller.
Reject incompatible protocol majors before interpreting capability names, and require each optional selector's introduction minor before invocation. Keep capability checks bound to the descriptor's connection generation.
Create the process-lifetime LockEngine and notifier at the Agent composition root, then export the same service to every connection. Remove language-level singletons and keep implementation-only service types internal while preserving test injection seams.
Populate the synchronous Services result channel before scheduling in-app presentation so modal UI cannot delay the caller. Preserve late failure alerts after a timeout while claiming presentation exactly once.
Give AppCoordinator.State one lock-evidence projection for reconciliation and observation. Reuse one NSError description and recovery formatter across automation and AppleScript so wrapper surfaces cannot drift.
Bracket the ad-hoc probe with an authorized control signed from the bundled klock certificate and require one unchanged Agent instance. Classify unavailable, replacement, signing, and compilation uncertainty as indeterminate instead of false security evidence.
Document selector-specific protocol negotiation, process-lifetime service ownership, persisted-settings fallback versus strict wire decoding, and the bracketed evidence boundary for non-team client refusal.
Keep the ten-second fail-safe inside the Agent so the keyboard unlocks even if the App exits. Add first-run guidance, privacy-conscious diagnostics, compatibility negotiation, tests, and architecture documentation.
Convert all Core and app model tests to Testing macros while preserving lifecycle cleanup and deterministic async gates. Serialize suite-local state and isolate keyboard-layout tests on MainActor so Swift Testing concurrency remains safe.
Define the MVP settings contract as read-only and keep Agent-owned writes as a post-MVP extension. Clarify that LockEngine's update hook is internal rather than a missing user capability.
Serialize TIS keyboard-layout access on the main thread to prevent HIToolbox aborts under parallel callers. Make XPC reply transfer, timer scheduling, and wrapper test isolation explicit under complete strict concurrency, with a concurrent display regression.
- Reorder switch cases, drop redundant self/numeric-separator literals, and fix range/guard/multiline-string/closure spacing per SwiftFormat. - Rewrite 'try? await x()' as 'await (try? x())'; drop redundant 'return await'/'return try await' where the call is already the value.
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eab2b7ef7
ℹ️ 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".
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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
Single-core architecture:
KeyboardLockerAgentis the only executor (CGEventTap lock engine, settings source of truth, Accessibility gate). Every other surface is a thin XPC wrapper over the Mach serviceio.lzhlovesjyq.keyboardlocker.agent:lock(interactive with temporary Ctrl+C gesture + termination-signal cleanup, or--no-wait),unlock,status(--json/--snapshot),toggle,register-agent,request-accessXPC contract highlights:
serviceDescriptorhandshake with protocol major/minor + capability gating on the same connection generationDocs included: architecture contract, XPC implementation guide, automation usage, development guide.
Verification
xcodebuild -scheme KeyboardLocker -configuration Debug build— all targets buildxcrun swift test --package-path Core— 172 tests / 23 suites passxcodebuild test -only-testing:KeyboardLockerModelTests— TEST SUCCEEDEDManual-only boundaries (require locally signed build + GUI session): real Quartz event tap interception, TCC prompt, and the end-to-end unsigned-client refusal runbook (
scripts/verify-unsigned-client-refusal.sh).Known follow-ups (post-MVP)