feat(app): permission health check — detect broken TCC entries#90
Merged
feat(app): permission health check — detect broken TCC entries#90
Conversation
pasrom
force-pushed
the
feat/permission-health-check
branch
2 times, most recently
from
3d719ac to
f9860d0
Compare
Detect broken permissions (TCC says granted but CGWindowList returns no titles, or AVAudioEngine can't get samples). Pure async functions for testability, live wrappers for runtime. Delegates existing Permissions.checkScreenRecording() to avoid duplication. 22 unit tests.
Run async health check, notify user when permissions are broken. Expose permissionHealth for badge/UI. Use cached result for pre-recording checks to avoid live AVAudioEngine probe.
BadgeKind.compute() gains permissionProblem parameter. Broken permissions show error badge, overridden only by active recording.
Abort recording with clear error when permissions are broken. Async injectable permissionChecker closure for testability. New RecorderError.permissionDenied case.
Error badge now shows a red circle with white "!" overlay in the bottom-right of the menu bar icon. Non-template image so macOS renders the red color. Waveform adapts to dark/light mode.
What: Revamp PermissionHealthCheck to cover Microphone, Screen Recording, and Accessibility with distinct denied/broken states each. Reasoning: - Problem 1 (mic false broken): the mic probe waited only 50 ms for the first AVAudioEngine buffer, but the first buffer typically arrives after ~100–130 ms. Result: authorized mic was repeatedly classified as broken and showed a permanent permission badge. - Fix 1: poll up to 500 ms with a thread-safe BufferCounter so the audio thread tap callback can't race the main thread's read, and log the actual elapsed time for diagnostics. - Problem 2 (no Accessibility check): Accessibility is required for ParticipantReader but was never validated — a broken TCC entry went unnoticed until Teams rosters silently stopped working. - Fix 2: add checkAccessibility(trusted:probeSucceeds:) plus a live probe that performs kAXFocusedApplicationAttribute against the system-wide element. AXIsProcessTrustedWithOptions is not re-used because it only mirrors AXIsProcessTrusted(); we need a real API call to detect broken entries. - Problem 3 (SR conflates denied vs broken): the old checkScreenRecording took a raw window list and had no way to tell "TCC denied" apart from "TCC granted but no windows visible". Notifications couldn't give the user a useful hint. - Fix 3: split into checkScreenRecording(systemAllowed:hasForeignWithTitle:) driven by CGPreflightScreenCaptureAccess + hasForeignWindowWithTitle. "denied" vs "broken" now carry distinct, actionable messages. Also: write a /tmp/mt-permission.log from all three live-check paths because os_log is not visible from ad-hoc signed dev bundles.
…ission problem What: When any permission is denied or broken while the app is recording, transcribing, diarizing, or generating a protocol, composite a small red exclamation dot over the existing animated icon instead of hiding the animation behind an .error badge. Reasoning: - Problem: BadgeKind.compute returned .error as the lowest-priority fallback, so any active pipeline state (recording, transcribing, etc.) silently overrode the permission problem. Users running a job never noticed their mic/SR/AX was broken until the pipeline failed. - Considered: (A) leave as-is — user can't act during recording anyway; (B) flip the priority — permission problem wins, hides active animation; (C) compositing overlay — active animation stays visible, problem still signalled. - Rejected A: silently hides real errors during the entire active window. - Rejected B: loses the critical "recording is live" feedback. - Decision: compositing overlay via a new `permissionOverlay` parameter on MenuBarIcon.image(...). The frame cache is bypassed when the overlay is on (18×18 px render, cheap) and the image becomes non-template because the red dot cannot render monochrome.
What: Remember the last notified problem set in AppState so repeated identical results from checkPermissions() don't spam notifications, and re-run the health check on NSApplication.didBecomeActiveNotification so the badge clears as soon as the user returns from System Settings. Reasoning: - Problem 1: previously the startup .task was the only trigger — if the user fixed a permission after the app launched, the red badge lingered until the app was restarted. - Problem 2: adding a second trigger would repeat the notification every time the user tabbed back, because handlePermissionHealth always fired the notifier when !isHealthy. - Decision: track lastNotifiedProblems, notify only when the problem set changes, and clear the memory on recovery so a problem that reappears later still pings. Trigger the re-check on didBecomeActive — cheap enough to run eagerly (no polling loop needed).
pasrom
force-pushed
the
feat/permission-health-check
branch
from
2410dae to
dfd4584
Compare
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
1 participant
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
Detects when macOS permissions are "granted" in TCC but functionally broken (common after macOS updates). Instead of silently failing to record, the app now:
Architecture
PermissionHealthCheckenum with pure testable functions (injected window lists, auth status) + live wrappers. Integrated via:AppState.permissionHealthproperty +checkPermissions()BadgeKind.compute(permissionProblem:)parameterWatchLoop.permissionCheckerinjectable closureRecorderError.permissionDeniedcaseTest plan