Repository navigation
fix: propagate branding SVG write failures (#670) - #717
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Walkthrough
ChangesSVG file writing
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~5 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to A failed SVG write now prevents generation from being reported as successful; no concrete merge-blocking risk remains. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: propagating branding SVG write failures. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| 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. |
✨ 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.
Comment @coderabbitai help to get the list of available commands.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Jamie-BitFlight
left a comment
Jamie-BitFlight
left a comment
There was a problem hiding this comment.
Holistic review of PRs #717–#728
Reviewed 5415670a8cc136cc611010d732471ea93286384a against main at edb05b4eeea35e03df75b066b8ddd1d708de9f1c.
Assessment: correct targeted fix; ready for final verification. No blocking defect found.
The existing synchronous branding API now has truthful completion semantics. A native filesystem probe with the SVG destination set to an existing directory makes main return success, update the image cache and continue writing the README; this head propagates EISDIR before the cache/success update and before the README dump. Successful title-only, branding-only and combined generation reused one completed image; missing title/branding markers still skipped image creation. This remains compatible with the marker-gating and first-image cache work already merged in #706/#664.
Current-head Node 24/26 jobs and both rebuilt third-party integration jobs are green (Node 24). The inline suggestion is behavioral regression coverage, not a request to redesign this synchronous boundary. Local source probes used available dependencies, not a canonical local Vite+ build.
## [2.0.5](v2.0.4...v2.0.5) (2026-10-11) ### Bug Fixes * add CLI-only generation error boundary ([#654](#654)) ([#725](#725)) ([6a27eaa](6a27eaa)) * **ci:** honor explicit Node versions before automatic detection ([95a9e68](95a9e68)) * **ci:** honor explicit Node versions in the test matrix ([#735](#735)) ([3f03f0d](3f03f0d)) * complete SVG writes before reporting success ([#717](#717)) ([170e499](170e499)) * consistently interpret GitHub Actions environment flag ([#670](#670)) ([#723](#723)) ([68756ad](68756ad)) * fail generation on formatting and requested save errors ([#654](#654)) ([#718](#718)) ([bb48434](bb48434)) * guard malformed runs metadata and preserve load error causes ([#720](#720)) ([c6caf7a](c6caf7a)) * load native ESM configuration and verify fresh package entry points ([5f92784](5f92784)) * load native ESM configuration and verify fresh package entry points ([#733](#733)) ([55746ed](55746ed)) * pair section log groups and honor Actions mode at output boundaries ([2d45100](2d45100)) * pair section log groups and honor Actions output mode ([#734](#734)) ([ceaa03a](ceaa03a)) * preserve empty metadata descriptions and cover reviewed errors ([3c0a181](3c0a181)) * preserve empty metadata descriptions and verify reviewed error paths ([#732](#732)) ([f8c0f4e](f8c0f4e)) * preserve original file read error as cause ([#722](#722)) ([05553e0](05553e0)) * validate consumed nested metadata descriptions ([#670](#670)) ([#729](#729)) ([330ab9a](330ab9a)) * validate README section configuration at input boundary ([#670](#670)) ([#726](#726)) ([40c9efd](40c9efd)) * verify declared defaults using their rendered projection ([beb2180](beb2180)) * verify empty metadata descriptions through the contract checker ([a5e2d31](a5e2d31))
|
🎉 This PR is included in version 2.0.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
Problem
writeSVGFileuses a callback-based write without checking the callback error. Generation returns and caches branding before the file is written; a failed write can still be reported as successful.Change
Use
writeFileSyncat the existing synchronous boundary. A successful return now means the file has been written, and filesystem errors propagate to the caller. No async API changes to branding/title generation.Verification needed
Related: #670 and #654.
Summary by CodeRabbit