Repository navigation
Conversation
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
The fix looks correct, but I couldn't verify it end to end: the folder-access prompt only shows up in a packaged macOS app, and I only had Linux. A maintainer with a Mac should confirm the prompt appears for a ~/Documents workspace after tccutil reset SystemPolicyDocumentsFolder <bundle id>.
What I checked: the keys go into getBase(), which the dev, beta and prod configs all share, and none of them overrides mac. The bundled CLI is launched by the app, so macOS should treat the app as the one asking for access, and the CLI shouldn't need keys of its own. The config test fails on the base (7 pass / 1 fail) and passes with this change (8 pass / 0 fail).
Public API: unchanged; this only adds keys to the macOS app bundle's Info.plist.
One small request: please fold the test into the existing one (see the inline comment).
Related: #50951
| ]) | ||
| }) | ||
|
|
||
| test("declares a macOS usage description for every folder a workspace can live in", () => { |
There was a problem hiding this comment.
This can be three assertions in the existing shared packaging defaults test, which already checks mac.extendInfo (the AutoFill key) on the same config. A separate test with a multi-line comment is more than this config change needs, so it can be dropped in favour of that.
|
Folded — thanks. The three assertions now sit in The assertions still bite after the move: with the config reverted to Agreed on the verification gap — I have no Mac either, so a maintainer confirming the prompt after |
Issue for this PR
Closes #56479
Type of change
What does this PR do
This is the first half of what the report asks for: the bundle's usage descriptions.
packages/desktop/electron-builder.config.tsdeclared no folder usage description for macOS — itsmac.extendInfoheld only the AutoFill key. macOS gates Documents, Desktop and Downloads behind TCC, and the usage description is the only thing that turns an access denial into a permission prompt. Without it the system denies silently: the app never appears under Privacy & Security → Files and Folders, so there is nothing for the user to allow, and every location-scoped API call for a workspace in one of those folders fails withEPERM— which is the empty model selector in the report.The change adds
NSDocumentsFolderUsageDescription,NSDesktopFolderUsageDescriptionandNSDownloadsFolderUsageDescription, one per protected folder. A workspace can be any folder the user picks, so a workspace in any of the three needs its own string; declaring one does not cover the others.The second half — explaining an unreadable workspace in the UI instead of showing an empty model list — is left alone, because #50952 is already open on that direction.
How did you verify your code works?
Three assertions added to
shared packaging defaultsinpackages/desktop/electron-builder.config.test.ts, beside the existingmac.extendInfocheck on the same config: each of the three keys must be a non-empty string. Empty counts as missing here — macOS shows no prompt for an empty description either — so they usetoMatch(/\S/)rather thantoBeString().shared packaging defaultsfails (6 pass / 1 fail,Received value must be a string: undefined); with the change it passes (7 pass / 0 fail).Info.plist: all three keys present with the text above.oxlinton both changed files: 0 warnings, 0 errors.bun run typecheckinpackages/desktopreports 14 errors, all inpackages/app,packages/gui-extensions,packages/session-uiandsrc/main/storage/drafts.ts; the pristine tree at this base reports the same 14, and none is in a file this PR touches.What I could not verify: I have no macOS machine here, so nothing here exercised the TCC prompt, a packaged
.app, or theEPERMpath itself. The claim rests on the missing keys being the documented cause — which the report already measured against the built bundle — and on the config now carrying them. A maintainer with a Mac still needs to confirm the prompt appears.scripts/picker.test.tsalso fails in this environment (2 failures,The service was stopped); it fails identically on the pristine tree.AI-assisted implementation and tests; reviewed before publication.
Screenshots / recordings
Not applicable; packaging configuration only.
Checklist
Added by the opencode bot (AI-generated).
Related: #50951