Remove microphone permission check and simplify editability logic - #87
Merged
Conversation
Four removals, each a symbol or branch that no code path can reach: - `BlurtError.microphonePermissionDenied` — never constructed anywhere. Microphone state is owned by `PermissionsChecker` and the wizard, and a capture failure surfaces as `.audioCaptureFailed`, so this case only ever appeared in its own `errorDescription`/`==` switches (which is why periphery could not see it as unused). Drops with its two switch arms and the `BLURTENGINE.md` error-table row. - `FocusCapture.isEditableTarget`'s `hasFocusedElement` parameter and its `guard` — the sole production caller, `hasEditableFocusedElement()`, already returns early when no element resolves, so it only ever passed `true`. The dead branch existed solely to be asserted by one test. - `AppDelegate.surfaceMainWindow()`'s `NSWindow?` return — all three call sites discard it, hence the `@discardableResult`. - `NSAppleEventsUsageDescription` — Blurt sends no Apple Events: the paste is a synthesized `CGEvent` (Accessibility) and focus reads go through the AX client APIs, so the string could never be shown. Deliberately kept: the availability fallbacks (`glassButtonStyleCompat`, `Observations`) that the macOS 15 deployment target requires, and the documented degradation paths in `SystemClipboard`, `KeyInjector`, `MemoizedKeyStore`, and `FocusCapture` — each encodes a real past bug and is covered by tests. Verification: `scripts/check.sh --portable` is green; the Swift build, tests, and lint gates need a Mac, so CI on macos-26 is the authority here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MyARxdyuq4QTjk5WcwTyXu
There was a problem hiding this comment.
Pull request overview
This PR simplifies Blurt’s permission/error surface and focus editability logic in the engine, while removing an unused Info.plist usage-description key in the app shell. It aligns tests and docs with the updated error cases and the clarified “editability” contract.
Changes:
- Removed the
BlurtError.microphonePermissionDeniedcase and updated equality/error-description tests accordingly. - Simplified
FocusCapture.isEditableTargetby removing thehasFocusedElementparameter; callers now decide “is anything focused?” viahasEditableFocusedElement. - Removed
NSAppleEventsUsageDescriptionfrom the app’s Info.plist configuration.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/BlurtEngineTests/EditableTargetTests.swift | Updates unit tests to match the new isEditableTarget signature/contract. |
| Tests/BlurtEngineTests/BlurtErrorTests.swift | Removes the mic-permission error case from description/equality coverage. |
| Sources/BlurtEngine/Pipeline/PipelinePhase.swift | Updates BlurtError’s Equatable implementation to drop the removed case. |
| Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift | Refactors isEditableTarget signature and updates doc/usage in hasEditableFocusedElement. |
| Sources/BlurtEngine/BlurtError.swift | Removes microphonePermissionDenied and its localized description string. |
| BLURTENGINE.md | Updates engine documentation to reflect the removed error case. |
| App/Blurt/project.yml | Removes NSAppleEventsUsageDescription from generated Info.plist properties. |
| App/Blurt/Blurt/Info.plist | Removes NSAppleEventsUsageDescription from the committed Info.plist. |
| App/Blurt/Blurt/AppDelegate.swift | Minor API cleanup: surfaceMainWindow() no longer returns a window result. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 27, 2026
The merge-queue run failed to compile even though both sides were green on their own: main gained an exhaustive `switch` over `BlurtError` in `isSetupBlocker` (#83) while this branch removed the never-constructed `.microphonePermissionDenied` case. The two edits touch different lines, so git merged them without a conflict and the compiler caught it instead — exactly the failure the exhaustive switch is there to produce. Merges origin/main and removes the stale case from that switch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MyARxdyuq4QTjk5WcwTyXu
Removing `.microphonePermissionDenied` left the engine guide's error table with no mention of Microphone at all, so an integrator reading it had no way to know which case a denied grant arrives as. `MicCapture.start()` throws `.audioCaptureFailed` when `record()` refuses, so say that on the row — and point at `PermissionsChecker` as the up-front check that avoids it. Raised by Copilot's review on #87. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MyARxdyuq4QTjk5WcwTyXu
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
BLURTENGINE.md:104
- This row recommends checking
PermissionsCheckerup front to catch missing Microphone permission, but the PR description states that explicit mic-permission checking is no longer needed and that denial will naturally surface as.audioCaptureFailed. To avoid mixed guidance for integrators, consider rewording to describe the behavior without prescribing an up-front permission check (or make it explicitly optional).
| `.audioCaptureFailed(underlying:)` | The mic couldn't start, or captured audio couldn't be processed. There is no separate microphone-permission case: a denied or revoked Microphone grant surfaces here, so check `PermissionsChecker` up front to catch it before the user speaks. |
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.
What & why
This PR removes the
microphonePermissionDeniederror case and simplifies theisEditableTargetfunction by removing itshasFocusedElementparameter.Microphone permission removal: The microphone permission check is no longer needed since the system will naturally fail audio capture if the permission is missing (surfaced as
.audioCaptureFailed). This eliminates redundant permission checking and simplifies the error handling path.Editability simplification: The
isEditableTargetfunction previously required ahasFocusedElementparameter that always returnedfalseif no element was focused. This check is now the caller's responsibility (viahasEditableFocusedElement), making the function's contract clearer: it evaluates whether a given focused element accepts pasted text, rather than checking if one exists. This reduces parameter noise and makes the decision logic more focused.Also removes the unused
NSAppleEventsUsageDescriptionfrom the app's Info.plist, as it's not required for keyboard event injection on modern macOS.How it was tested
scripts/check.shpasses (or CI will, if I'm not on a Mac)Existing unit tests in
EditableTargetTestsandBlurtErrorTestshave been updated to reflect the API changes. The removal of the microphone permission case is covered by the error description tests.https://claude.ai/code/session_01MyARxdyuq4QTjk5WcwTyXu