Move branding and package install into cargo-run#4266
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the Node.js-based branding installer with a Rust-based implementation in tools/cargo-run, allowing the removal of the tar dependency from the frontend. The Rust runner now automatically downloads and verifies branding assets for non-CLI tasks. Feedback points out the use of unstable let_chains in the new Rust code, which requires a nightly compiler, and suggests refactoring it with map_or to ensure compatibility with stable Rust.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
!build (Run ID 27920534150) |
|
!build desktop (Run ID 27920540132) |
Wasm: 23.16 MB — JS: 0.44 MB — CSS: 0.09 MB — Fonts: 0.30 MB — Images: 0.09 MB — All Assets: 24.08 MB |
|
|
|
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 3/5
- In
tools/cargo-run/src/branding.rs, branding assets are deleted before the replacement download/extract succeeds, so any fetch or unpack failure can leave the workspace in a broken partial state and disrupt subsequent runs—switch to a staged/atomic replace flow (or rollback on failure) before merging.
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="tools/cargo-run/src/branding.rs">
<violation number="1" location="tools/cargo-run/src/branding.rs:32">
P2: Existing branding assets are deleted before replacement is confirmed. Download/extract failure leaves the workspace in a broken partial state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| eprintln!("Downloading branding assets from <{url}>..."); | ||
|
|
||
| if dir_path.exists() { | ||
| std::fs::remove_dir_all(&dir_path).map_err(|e| Error::Io(e, format!("removing '{}'", dir_path.display())))?; |
There was a problem hiding this comment.
P2: Existing branding assets are deleted before replacement is confirmed. Download/extract failure leaves the workspace in a broken partial state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/cargo-run/src/branding.rs, line 32:
<comment>Existing branding assets are deleted before replacement is confirmed. Download/extract failure leaves the workspace in a broken partial state.</comment>
<file context>
@@ -0,0 +1,40 @@
+ eprintln!("Downloading branding assets from <{url}>...");
+
+ if dir_path.exists() {
+ std::fs::remove_dir_all(&dir_path).map_err(|e| Error::Io(e, format!("removing '{}'", dir_path.display())))?;
+ }
+
</file context>
No description provided.