Fix permission notifications after auto-allow

[austinywang]

Summary

• Add regression coverage for permission requests resolved before native notification display
• Skip native Feed notifications once the blocking request already has a decision
• Clear pending/delivered notification requests when a Feed reply arrives

Closes #3702

Testing

• Not run locally per repository policy; CI will run the regression.

---

Note

Medium Risk
Touches blocking hook wait/notification flow and asynchronous UNUserNotificationCenter delivery, which can regress user-visible notifications and cleanup timing if waiter state or cancellation is wrong.

Overview
Prevents native Feed notification banners from appearing after a blocking request has already been auto-resolved (or timed out) by gating all notification posting/delivery/fallback effects on a new waiter-backed isAwaitingDecision check and re-checking after policy-hook authorization/evaluation and notification-center callbacks.

Adds explicit cleanup by calling cancelNotification(requestId:) on reply and on ingest completion/timeout, refactors the notification pipeline into postNotificationIfStillAwaiting/deliverFeedNotificationIfStillAwaiting/addNotificationIfStillAwaiting, and adds DEBUG test hooks plus a regression test ensuring auto-allowed permissions do not post notifications.

^{Reviewed by Cursor Bugbot for commit 0841a38. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Prevents stale native Feed banners after auto-allow by gating all notification steps on waiter state and canceling pending/delivered notifications on resolve, reply, or timeout; addresses #3702.

• Bug Fixes

  • Gate post/deliver/add/fallbacks behind waiter-backed isAwaitingDecision; skip when app is active (DEBUG override).
  • Recheck awaiting state after policy hooks, authorization, and add callbacks; use async authorization; cancel pending/delivered off-main on resolve/reply/timeout; keep sound/command fallbacks when desktop is disabled or enqueue fails.

• Refactors

  • Centralize flow into postNotificationIfStillAwaiting, deliverFeedNotificationIfStillAwaiting, addNotificationIfStillAwaiting, runFallbackEffectsIfStillAwaiting, cancelNotification, and isAwaitingDecision.
  • Make desktop notification effect explicit (default on) in policy context; add DEBUG hooks (afterBlockingEventIngested, isAppActiveOverride, notificationPostObserver) and a regression test with teardown reset.

^{Written for commit 0841a38. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

• Bug Fixes

  • Notifications are now shown only while a request is actively awaiting a decision and are reliably cancelled when a request resolves or times out, preventing stale banners.
  • Blocking wait behavior now cleans up notifications on timeout or unresolved outcomes.

• New Features

  • Desktop notification effects enabled for richer native banners.

• Tests

  • Added a test ensuring no notification is posted when permission resolves before display.
  • Reworked async test synchronization for more reliable blocking-ingest verification.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 19, 2026 7:40am
cmux-staging	Building	Preview, Comment	May 19, 2026 7:40am

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

FeedCoordinator posts native permission-request notifications only while the request's waiter remains unresolved; notifications are cancelled on resolution or timeout. A DEBUG-only FeedCoordinatorTestHooks and isAwaitingDecision(requestId:) gate posting. A private notification extension handles authorization, policy evaluation, delivery, and cancellation. Tests updated to verify behavior.

Changes

Notification Eligibility and Conditional Posting

Layer / File(s)	Summary
Test hooks and decision eligibility check Sources/Feed/FeedCoordinator.swift	Adds DEBUG-only FeedCoordinatorTestHooks and isAwaitingDecision(requestId:); deliverReply cancels notifications when resolving a waiter.
Notification posting and management system Sources/Feed/FeedCoordinator.swift	Replaces standalone banner functions with a private extension that gates on app-active state, runs a DEBUG observer, performs authorization and policy evaluation, posts only while awaiting, and cancels pending/delivered notifications by requestId. makeFeedNotificationPolicyContext enables desktop effects.
Blocking ingest and timeout integration Sources/Feed/FeedCoordinator.swift	ingestBlocking invokes the post-ingest DEBUG hook and uses postNotificationIfStillAwaiting; on unresolved completion or timeout it cancels notifications before expiring the item and returning .timedOut.
Test infrastructure and coverage cmuxTests/FeedCoordinatorTests.swift	Replaces semaphore coordination with XCTest expectations, adds testBlockingIngestSkipsNotificationWhenPermissionResolvesBeforeDisplay, resetFeedCoordinatorTestHooks(), and NotificationRequestRecorder using NSLock to record posted request IDs.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant FeedCoordinator
  participant MainActorStore
  participant PIDWatcher
  participant UNUserNotificationCenter
  Client->>FeedCoordinator: trigger PermissionRequest event
  FeedCoordinator->>MainActorStore: ingest(event)
  FeedCoordinator->>PIDWatcher: armWatcher(requestId)
  FeedCoordinator->>UNUserNotificationCenter: postNotificationIfStillAwaiting(requestId) (auth & policy checks)
  alt permission resolves before display
    Client->>FeedCoordinator: deliverReply(requestId, decision)
    FeedCoordinator->>UNUserNotificationCenter: cancelNotification(requestId)
  else still awaiting
    UNUserNotificationCenter-->>Client: show banner
    Client->>FeedCoordinator: deliverReply(requestId, decision) (later)
    FeedCoordinator->>UNUserNotificationCenter: cancelNotification(requestId)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped through queues where banners might spring,
I checked who still waited before I would sing,
If hooks answered quick, no bubble was cast,
I canceled the echoes that lingered from past.
Quiet feed, tidy state — the garden grows fast.

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (2 errors, 1 warning)

Check name	Status	Explanation	Resolution
Cmux Swift Actor Isolation	❌ Error	The cancelNotification method accesses MainThread-only UI APIs without @MainActor isolation but is called from non-isolated contexts that may run on background threads.	Mark cancelNotification as @MainActor or wrap UNUserNotificationCenter calls in DispatchQueue.main.async to ensure proper isolation.
Cmux Swift @Concurrent	❌ Error	File I/O-heavy TerminalNotificationPolicyEngine.evaluate awaited on MainActor without explicit hop. postNotificationIfStillAwaiting contains Task { @MainActor in } that blocks UI.	Add @concurrent to evaluate(), use Task.detached to hop off MainActor, or make postNotificationIfStillAwaiting async.
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix permission notifications after auto-allow' directly addresses the main change: preventing native notifications from displaying when permission requests are auto-allowed.
Linked Issues check	✅ Passed	The PR fully addresses #3702 by gating notifications on waiter state via isAwaitingDecision, canceling pending/delivered notifications on decision/timeout, and adding regression test coverage for auto-allowed requests.
Out of Scope Changes check	✅ Passed	All changes directly support the core objective of #3702: gating notifications on waiter state, canceling stale notifications, adding test hooks, and regression test coverage; no unrelated modifications detected.
Cmux Swift Blocking Runtime	✅ Passed	No blocking constructs introduced. Semaphore and lock counts unchanged from main. isAwaitingDecision() uses existing waiterLock for state checks. Tests improve with async XCTest expectations.
Cmux No Hacky Sleeps	✅ Passed	Check scope is TypeScript/JavaScript/shell; PR contains only Swift files which are excluded from this rule per swift-blocking-runtime.md.
Cmux Swift Concurrency	✅ Passed	Tasks only at required API boundaries with proper lifecycle guards via isAwaitingDecision checks. No Combine, background queues, or problematic completion handlers.
Cmux Swift File And Package Boundaries	✅ Passed	File grew from 631 to 878 lines (+247). Under 250-line threshold for already-oversized files. Additions are cohesive notification-gating logic for bug fix #3702.
Cmux Swift Logging	✅ Passed	All logging changes comply with swift-logging.md. FeedCoordinatorTestHooks are DEBUG-only, properly isolated with #if DEBUG blocks, and no forbidden logging statements found in production code.
Cmux Swiftui State Layout	✅ Passed	No SwiftUI changes detected in this PR. Files modify AppKit notification coordination logic with no SwiftUI imports, state patterns, or view components. Check is not applicable.
Cmux Architecture Rethink	✅ Passed	Fixes notification race via isAwaitingDecision() on existing waiter state. No new semaphores/locks. Clearly names invariant. DEBUG-only test hooks. Consolidates notification flow.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	Check not applicable. PR modifies notification handling in FeedCoordinator (non-UI coordinator), not auxiliary windows. No NSWindow, NSPanel, NSWindowController, or SwiftUI Window/WindowGroup added.
Description check	✅ Passed	The pull request description includes a summary of changes, rationale, and testing approach, but lacks some sections from the repository template.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-3702-permission-notifications-after-autoallow

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes stale native Feed notification banners appearing after a blocking permission request has already been auto-resolved, by gating every step of the notification pipeline on a waiter-backed isAwaitingDecision check and canceling pending/delivered notifications on reply, resolve, and timeout.

• Notification pipeline refactor: Replaces postFeedNotification (a free function with no waiter check) with postNotificationIfStillAwaiting, deliverFeedNotificationIfStillAwaiting, addNotificationIfStillAwaiting, runFallbackEffectsIfStillAwaiting, and cancelNotification — all gated on isAwaitingDecision so resolved requests cannot enqueue or keep stale banners.
• Timeout and reply cleanup: Adds cancelNotification(requestId:) to both ingestBlocking timeout branches and to deliverReply, so delivered and pending notifications are torn down off-main when the request concludes through any path.
• Regression test: Adds testBlockingIngestSkipsNotificationWhenPermissionResolvesBeforeDisplay using FeedCoordinatorTestHooks (new DEBUG-only hooks) to exercise the auto-allow race and assert no notification is posted; existing timeout test is reworked from DispatchSemaphore.wait to XCTestExpectation/fulfillment.

Confidence Score: 5/5

Safe to merge — all notification paths are now correctly gated on waiter state and the race between auto-allow and banner display is closed.

The waiter-backed isAwaitingDecision checks are placed correctly at every async suspension point in the notification pipeline, cancelNotification is called on all resolve and timeout exit paths, and the new @MainActor isolation eliminates the off-main callback issues flagged in earlier rounds. The regression test exercises the auto-allow race through the full ingestBlocking path and correctly drains the main actor before asserting.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/Feed/FeedCoordinator.swift	Refactors the notification pipeline into waiter-gated methods; adds isAwaitingDecision, cancelNotification, and FeedCoordinatorTestHooks; correctly threads all new @MainActor callbacks; cancelNotification is called on all resolve/timeout paths.
cmuxTests/FeedCoordinatorTests.swift	Adds regression test for auto-allow skipping notification; replaces semaphore-based synchronization with XCTestExpectation; teardown resets hooks via MainActor.assumeIsolated; NotificationRequestRecorder is correctly lock-protected.

_{Reviews (23): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 414-521: The DEBUG test-observer is currently invoked before the
NSApp.isActive early return, causing tests to observe notifications that
production would suppress; move the MainActor.assumeIsolated block that reads
FeedCoordinatorTestHooks.notificationPostObserver so it runs after the
NSApp.isActive check (inside postNotificationIfStillAwaiting) and still returns
early when the observer handles the event, or alternatively add a clear comment
on FeedCoordinatorTestHooks.notificationPostObserver documenting that observers
are invoked regardless of app focus. Ensure you update the code path around
postNotificationIfStillAwaiting, the MainActor.assumeIsolated call, and the
FeedCoordinatorTestHooks.notificationPostObserver usage so test behavior matches
production gating by NSApp.isActive.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cbbce97c-f618-439e-a143-ba8bc43b099c

📥 Commits

Reviewing files that changed from the base of the PR and between 04f421c and 00a95cf.

📒 Files selected for processing (1)

• Sources/Feed/FeedCoordinator.swift

[austinywang]

@coderabbitai review\n\nThe prior actionable feedback about FeedCoordinatorTestHooks.notificationPostObserver running before the NSApp.isActive gate is addressed in 975182e: the observer now runs after the app-focus early return in postNotificationIfStillAwaiting.

[coderabbitai]

@austinywang Thanks for the heads-up on the fix in 975182ed5! Moving the FeedCoordinatorTestHooks.notificationPostObserver call to after the NSApp.isActive early return in postNotificationIfStillAwaiting makes sense — the observer now accurately reflects only the code paths where a notification would actually be posted to the system. I'll kick off a fresh review now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit c351823. Configure here.}

[austinywang]

@coderabbitai review

Latest review follow-up is in 1864d62: add failures now use the same waiter-gated fallback path as other fallback cases, preserving sound and command effects while avoiding stale resolved-request effects.

[coderabbitai]

@austinywang Thanks for the update! Unifying add-failure fallback under the same waiter-gated path is a clean approach — it keeps sound and command effects intact for legitimately-pending requests while ensuring no effects fire for already-resolved ones. Kicking off the review now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

Dismissed as stale: current head moves the DEBUG notificationPostObserver after the app-active gate, and the corresponding inline thread is resolved/confirmed addressed.