Repository navigation
Conversation
Move the Notification Center registration, Apple Events probe, and mDNS local-network probe out of boot. The renderer triggers them once onboardedAt is set.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Notification-center registration apps/desktop/src/main/lib/notification-center-registration.ts, apps/desktop/src/main/index.ts |
The registration helper moves to its own module. Startup no longer calls notification-center registration, Apple Events access, or local network access. |
Permission request mutation apps/desktop/src/lib/trpc/routers/permissions.ts |
The permissions router adds a mutation that invokes the three access helpers. |
Authenticated-layout trigger apps/desktop/src/renderer/routes/_authenticated/components/PostOnboardingPermissions/*, apps/desktop/src/renderer/routes/_authenticated/layout.tsx |
The authenticated layout passes the user's onboarding state to a component. The component invokes the mutation when the user is onboarded and renders nothing. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant AuthenticatedLayout
participant PostOnboardingPermissions
participant PermissionsRouter
participant NotificationCenterRegistration
participant AccessHelpers
AuthenticatedLayout->>PostOnboardingPermissions: pass onboarded state
PostOnboardingPermissions->>PermissionsRouter: invoke requestPostOnboardingAccess
PermissionsRouter->>NotificationCenterRegistration: register with macOS notification center
PermissionsRouter->>AccessHelpers: request Apple Events and local network access
Suggested reviewers: saddlepaddle
Merge Risk: 
b71a6
-
Permission requests are deferred until onboarding; no merge-blocking regression was established.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: delaying first-launch permission prompts until onboarding is complete. It also follows the repository's conventional commit format. Description check The description explains the problem, the implementation, the linked issue, and the verification performed. It does not include the template's explicit Checklist section, but the description is otherw… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the problem, the implementation, the linked issue, and the verification performed. It does not include the template's explicit Checklist section, but the description is otherwise mostly complete and relevant.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 6 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
 [Medium impact] The change affects macOS permission prompts in the desktop onboarding flow. The PR appears safe to merge, with a non-blocking improvement to avoid repeating permission work when windows open or reload. Findings
Fix with agent prompt### Issue 1
apps/desktop/src/lib/trpc/routers/permissions.ts:36-39
`requestPostOnboardingAccess` now runs once per authenticated window mount, rather than once per app launch. Opening another window or reloading one creates another throwaway notification, starts another `osascript` process, and sends another network probe. Add a main-process once-per-launch guard so window changes do not repeat this app-wide work.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryMoves notification registration, Apple Events access, and local-network access out of desktop startup.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
Boot[Desktop startup] --> SignIn[Sign in]
SignIn --> Gate{onboardedAt set?}
Gate -->|No| Onboarding[Complete onboarding]
Onboarding --> Refresh[Refresh session]
Refresh --> Gate
Gate -->|Yes| Effect[PostOnboardingPermissions]
Effect --> Mutation[requestPostOnboardingAccess]
Mutation --> Notifications[Register notifications]
Mutation --> AppleEvents[Request Apple Events access]
Mutation --> Network[Request local-network access]
Reviews (1) · Last reviewed commit: "fix(desktop): keep first launch quiet un..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| requestPostOnboardingAccess: publicProcedure.mutation(() => { | ||
| registerWithMacOSNotificationCenter(); | ||
| requestAppleEventsAccess(); | ||
| requestLocalNetworkAccess(); |
There was a problem hiding this comment.
requestPostOnboardingAccess now runs once per authenticated window mount, rather than once per app launch. Opening another window or reloading one creates another throwaway notification, starts another osascript process, and sends another network probe. Add a main-process once-per-launch guard so window changes do not repeat this app-wide work.
Knowledge Base Used: Desktop application
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/lib/trpc/routers/permissions.ts
Line: 36-39
Comment:
**Window changes repeat permission work**
`requestPostOnboardingAccess` now runs once per authenticated window mount, rather than once per app launch. Opening another window or reloading one creates another throwaway notification, starts another `osascript` process, and sends another network probe. Add a main-process once-per-launch guard so window changes do not repeat this app-wide work.
**Knowledge Base Used:** [Desktop application](https://app.greptile.com/-/switch-tenant?tenant=personal-org-6810&next=%2Fcustom-context%2Fknowledge-base%2Fparadevai%2Fparadev%2F-%2Fdocs%2Fdesktop-application.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/permissions.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/permissions.ts:36">
P2: `requestPostOnboardingAccess` has no process-wide once guard, so opening another authenticated window or remounting the layout repeats all three native requests and can duplicate/race permission prompts while the first request is pending. Make this mutation idempotent in the main process before launching the side effects.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/components/PostOnboardingPermissions/PostOnboardingPermissions.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/components/PostOnboardingPermissions/PostOnboardingPermissions.tsx:12">
P2: This fire-and-forget mutation discards IPC failures, so onboarding can finish without requesting the deferred permissions and without a useful renderer diagnostic. Add an `onError` handler that logs the failure (or otherwise provides a retry path).</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| requestPostOnboardingAccess: publicProcedure.mutation(() => { | ||
| registerWithMacOSNotificationCenter(); | ||
| requestAppleEventsAccess(); | ||
| requestLocalNetworkAccess(); | ||
| }), |
There was a problem hiding this comment.
P2: requestPostOnboardingAccess has no process-wide once guard, so opening another authenticated window or remounting the layout repeats all three native requests and can duplicate/race permission prompts while the first request is pending. Make this mutation idempotent in the main process before launching the side effects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/desktop/src/lib/trpc/routers/permissions.ts, line 36:
<comment>`requestPostOnboardingAccess` has no process-wide once guard, so opening another authenticated window or remounting the layout repeats all three native requests and can duplicate/race permission prompts while the first request is pending. Make this mutation idempotent in the main process before launching the side effects.</comment>
<file context>
@@ -30,6 +33,12 @@ export const createPermissionsRouter = () => {
await requestAppleEvents();
}),
+ requestPostOnboardingAccess: publicProcedure.mutation(() => {
+ registerWithMacOSNotificationCenter();
+ requestAppleEventsAccess();
</file context>
| requestPostOnboardingAccess: publicProcedure.mutation(() => { | |
| registerWithMacOSNotificationCenter(); | |
| requestAppleEventsAccess(); | |
| requestLocalNetworkAccess(); | |
| }), | |
| requestPostOnboardingAccess: (() => { | |
| let requested = false; | |
| return publicProcedure.mutation(() => { | |
| if (requested) return; | |
| requested = true; | |
| registerWithMacOSNotificationCenter(); | |
| requestAppleEventsAccess(); | |
| requestLocalNetworkAccess(); | |
| }); | |
| })(), |
| const { mutate } = | ||
| electronTrpc.permissions.requestPostOnboardingAccess.useMutation(); | ||
| useEffect(() => { | ||
| if (onboarded) mutate(); |
There was a problem hiding this comment.
P2: This fire-and-forget mutation discards IPC failures, so onboarding can finish without requesting the deferred permissions and without a useful renderer diagnostic. Add an onError handler that logs the failure (or otherwise provides a retry path).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/desktop/src/renderer/routes/_authenticated/components/PostOnboardingPermissions/PostOnboardingPermissions.tsx, line 12:
<comment>This fire-and-forget mutation discards IPC failures, so onboarding can finish without requesting the deferred permissions and without a useful renderer diagnostic. Add an `onError` handler that logs the failure (or otherwise provides a retry path).</comment>
<file context>
@@ -0,0 +1,15 @@
+ const { mutate } =
+ electronTrpc.permissions.requestPostOnboardingAccess.useMutation();
+ useEffect(() => {
+ if (onboarded) mutate();
+ }, [onboarded, mutate]);
+ return null;
</file context>
| if (onboarded) mutate(); | |
| if (onboarded) { | |
| mutate(undefined, { | |
| onError: (error) => { | |
| console.warn( | |
| "[permissions] Post-onboarding access request failed", | |
| error, | |
| ); | |
| }, | |
| }); | |
| } |
Fixes SUPER-3088.
Problem
On first launch macOS showed the Local Network and notification prompts before sign-in, with no context.
Change
permissions.requestPostOnboardingAccessmutation runs all three.PostOnboardingPermissionscalls it from the authenticated layout onceonboardedAtis set.Verification
tschas pre-existing route-tree errors from a stale generated file; none in touched code.Summary by cubic
Fixes macOS showing Local Network, notification, and Apple Events permission prompts at first launch before sign-in. These prompts now appear only after the user completes onboarding.
requestPostOnboardingAccessmutation runs all three, triggered from the authenticated layout onceonboardedAtis set.Written for commit b71a664. Summary will update on new commits.
Summary by CodeRabbit