Conversation
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
1 similar comment
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
Pull request overview
Fixes macOS permission prompt spam by preventing the global-home directory scanner from traversing common user folders that trigger TCC privacy prompts.
Changes:
- Expand the macOS ignore list for the “global home” directory discovery logic to skip standard personal folders (Desktop/Documents/Downloads/etc.).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ignore.add("Applications") | ||
| ignore.add("Desktop") | ||
| ignore.add("Documents") | ||
| ignore.add("Downloads") | ||
| ignore.add("Library") | ||
| ignore.add("Movies") | ||
| ignore.add("Music") | ||
| ignore.add("Pictures") | ||
| ignore.add("Public") |
There was a problem hiding this comment.
The macOS ignore list is built with a long sequence of ignore.add(...) calls, which is a bit error-prone to maintain (easy to miss/duplicate entries). Consider defining the list as an array (or constant) and adding via a simple loop so it’s easier to audit and update.
| ignore.add("Applications") | |
| ignore.add("Desktop") | |
| ignore.add("Documents") | |
| ignore.add("Downloads") | |
| ignore.add("Library") | |
| ignore.add("Movies") | |
| ignore.add("Music") | |
| ignore.add("Pictures") | |
| ignore.add("Public") | |
| const macOsIgnore = [ | |
| "Applications", | |
| "Desktop", | |
| "Documents", | |
| "Downloads", | |
| "Library", | |
| "Movies", | |
| "Music", | |
| "Pictures", | |
| "Public", | |
| ] | |
| for (const name of macOsIgnore) { | |
| ignore.add(name) | |
| } |
| if (process.platform === "darwin") { | ||
| ignore.add("Applications") | ||
| ignore.add("Desktop") | ||
| ignore.add("Documents") | ||
| ignore.add("Downloads") | ||
| ignore.add("Library") | ||
| ignore.add("Movies") |
There was a problem hiding this comment.
This change alters the directory discovery behavior for the global home case on macOS (skipping common user folders to avoid permission prompts), but there doesn’t appear to be test coverage for this path. Consider adding a unit test that asserts these folder names are excluded when isGlobalHome on darwin (potentially by factoring the ignore-list construction into a helper that can be tested without having to mutate process.platform).
Issue for this PR
Closes #15090 #14982
Type of change
What does this PR do?
My thesis: after some changing in how the sidecar is spawned it is more tightly coupled with the desktop app
Due to this scanning the personal directories was requesting users for permission to personal folders.
It never went away
If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!
How did you verify your code works?
We need some mac users to verify it
Screenshots / recordings
If this is a UI change, please include a screenshot or recording.
Checklist
not on mac hard to test, help me
If you do not follow this template your PR will be automatically rejected.