Uh oh!
There was an error while loading. Please reload this page.
fix(desktop): use a supported macOS bundle icon size - #3353
Conversation
Refs apache#3352 Refs apache#1919 Refs apache#1920 Generated-by: OpenAI Codex
jackwener
left a comment
There was a problem hiding this comment.
Automated Codex review on exact head 654ef0c506b521c5641656bd2e4145e2b1585dce found no actionable code issue.
The bug still exists on current main: both packaged macOS icon reads request Electron's unsupported large size, matching the native IconLoader::ReadIcon() crash evidence in #3352. Centralizing the supported { size: 'normal' } option in loadNativeBundleIcon fixes both call sites without changing the unpackaged bypass, and the focused test would fail if the old option returned.
Required conclusions:
- Optimal for the actual problem: yes.
- Production code to delete: none identified.
- Tests to delete/replace: none identified.
- Deeper refactor: no; the shared icon-loading helper is the correct owner.
- Ready to merge: no; the PR is Draft, has no hosted required
test, and has no independent human review. - Residual risk/gaps: the package-only native crash has not been smoked in a signed/notarized macOS bundle; a maintainer should decide whether the Electron contract plus focused test is sufficient or require that package evidence before leaving Draft.
This changes user-visible packaged-app behavior and therefore requires independent human review under CONTRIBUTING.md. No security, licensing, governance, or public-contract effect was identified.
Summary
Use Electron's macOS-supported
normalfile-icon size for the packaged permission overlay. Both native bundle-icon reads now share the same option, while unpackaged development continues to skip native icon loading.Add a regression test covering the unpackaged bypass and the packaged
normaloption.Fixes#3352
Refs #1919
Refs #1920
Verification
Passed locally:
corepack npm --workspace @maka/desktop test— 979 passedcorepack npm --workspace @maka/desktop run e2e— 34 passed, 1 skippedcorepack npm run lint— passedcorepack npm run format:check— passedcorepack npm run build— passedcorepack npm run typecheck— passedcorepack npm exec -- knip --workspace apps/desktop— passedcorepack npm exec -- knip --workspace packages/ui— passedcorepack npm run astryx:theme -- --check— passednode scripts/audit-alignment.mjs— passednode --test --test-concurrency=1 scripts/ci-test-plan.test.mjs— 21 passedcorepack npm run windows:inventory— passedgit diff --check— passedcorepack npm testwas also attempted, but was not green on this machine for failures outside this Desktop change:NODE_NO_WARNINGS=1.Not run:
AI use
Select exactly one:
Tool(s) and scope:
OpenAI Codex assisted with crash-log analysis, implementation, the regression test, local verification, and drafting this description. I reviewed the final diff and test results.
Checklist
Does this PR entail a change in behavior?