fix(ci): add generate:icons step to desktop build#2164
Conversation
📝 WalkthroughWalkthroughAdds a pre-compile "Generate file icons" step to the desktop build workflow and increments the desktop app package version from 1.0.6 to 1.1.0; no other behavioral changes detected. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The renderer imports resources/public/file-icons/manifest.json which is generated by the generate:icons script. CI was calling compile:app directly without generating icons first, causing a Rollup resolve error.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-desktop.yml:
- Around line 84-86: The workflow step that runs "bun run generate:icons" allows
the script to succeed even when no SVGs are produced; update CI to fail if icon
generation produced no assets by either (A) changing
apps/desktop/scripts/generate-file-icons.ts to detect missing referenced SVGs
and call process.exit(1) (or throw) when zero icons are found or any required
SVGs are missing, or (B) adding a follow-up job/step after the existing Generate
file icons step that inspects apps/desktop/public/manifest.json (or the script’s
output location) and exits non-zero if the manifest has no entries or expected
asset keys are missing; reference the generate-file-icons.ts script, the
generate:icons npm script, and manifest.json to locate the relevant
code/outputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c2ea5d0-0ce1-4f97-a4e1-d42f2b1471d0
📒 Files selected for processing (1)
.github/workflows/build-desktop.yml
| - name: Generate file icons | ||
| working-directory: apps/desktop | ||
| run: bun run generate:icons |
There was a problem hiding this comment.
Fail the build if icon generation produced no SVG assets.
This step only checks that the script ran. apps/desktop/scripts/generate-file-icons.ts still exits 0 when referenced SVGs are missing, because it skips missing files and writes manifest.json anyway. That means CI can now get past compile:app but still publish a desktop build with broken file icons at runtime.
Suggested workflow hardening
- - name: Generate file icons
- working-directory: apps/desktop
- run: bun run generate:icons
+ - name: Generate file icons
+ working-directory: apps/desktop
+ run: |
+ bun run generate:icons
+ test -f src/resources/public/file-icons/manifest.json || {
+ echo "::error::file icon manifest was not generated"
+ exit 1
+ }
+ test -n "$(find src/resources/public/file-icons -maxdepth 1 -name '*.svg' -print -quit)" || {
+ echo "::error::no file icon SVGs were generated"
+ exit 1
+ }Also applies to: 195-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-desktop.yml around lines 84 - 86, The workflow step
that runs "bun run generate:icons" allows the script to succeed even when no
SVGs are produced; update CI to fail if icon generation produced no assets by
either (A) changing apps/desktop/scripts/generate-file-icons.ts to detect
missing referenced SVGs and call process.exit(1) (or throw) when zero icons are
found or any required SVGs are missing, or (B) adding a follow-up job/step after
the existing Generate file icons step that inspects
apps/desktop/public/manifest.json (or the script’s output location) and exits
non-zero if the manifest has no entries or expected asset keys are missing;
reference the generate-file-icons.ts script, the generate:icons npm script, and
manifest.json to locate the relevant code/outputs.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/package.json">
<violation number="1">
P2: This change rolls the desktop app version backward from `1.1.0` to `1.0.6`, which is unrelated to the CI fix and can break release/update version ordering.
(Based on your team's feedback about keeping PRs focused and avoiding unrelated changes.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Summary
generate:iconsstep beforecompile:appin the desktop CI build workflow (both macOS and Linux jobs)resources/public/file-icons/manifest.jsonwhich is generated bybun run generate:icons. CI was callingcompile:appdirectly, skipping this step and causing a Rollup resolve error.Test plan