Skip to content

fix(windows): correct Rustup installation guidance - #2066

Merged
debpalash merged 3 commits into
debpalash:mainfrom
Rukhaam:rukhaam'sbranch
Sep 17, 2026
Merged

debpalash merged 3 commits into
debpalash:mainfrom
Rukhaam:rukhaam'sbranch

Conversation

@Rukhaam

@Rukhaam Rukhaam commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

The Windows Rustup package identifier is corrected to Rustlang.Rustup in launcher guidance and installation documentation. The unit test and changelog reflect this correction. The change affects failure text and documentation only, with low merge risk.

Copilot AI lite review requested due to automatic review settings September 13, 2026 08:09
@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Retrigger

The PR appears safe to merge with no outstanding findings.

Fix All in Claude CodeFindings

  1. P2 Installation guide remains stale
Summary

Corrects the Windows Rustup package identifier in toolchain guidance and keeps the installation documentation, changelog, and unit test synchronized.

Reviews (2) · Last reviewed commit: "fix: integrate current main and finish r..."

"",
" Install the Rust toolchain, then reopen your terminal:",
" Windows: winget install Rust.Rustup",
" Windows: winget install Rustlang.Rustup",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Installation guide remains stale

The corrected Windows Rustup command leaves docs/install/windows.md directing users to the obsolete Rust.Rustup package, so users following the guide can remain blocked from launching the desktop app. The repository's docs-sync directive requires install-flow documentation to be updated in the same PR; change the documented command to winget install Rustlang.Rustup before merging.

Context Used: Review as a panel of senior domain experts (ML inference, audio DSP, desktop systems). Comment ONLY on findings that would change what gets merged: a concrete bug, a violated house rule from CLAUDE.md, a real security/data risk. Per finding: at most ... (source)

Knowledge Base Used: Desktop application runtime

Fix in Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Update the Windows installation guide to use Rustlang.Rustup.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Corrects the Windows Rustup WinGet package ID and updates its unit test.

Changes:

  • Uses Rustlang.Rustup in launcher guidance.
  • Updates the corresponding test assertion.
File summaries
File Description
tests/frontend/desktopScripts.test.mjs Verifies the corrected package ID.
scripts/desktop-toolchain-path.mjs Updates the Windows installation command. A nit remains: the Windows installation guide still uses the old package ID.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

"",
" Install the Rust toolchain, then reopen your terminal:",
" Windows: winget install Rust.Rustup",
" Windows: winget install Rustlang.Rustup",
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 4d0293eb-da68-436b-b618-01ee6ea82239

📥 Commits

Reviewing files that changed from the base of the PR and between 63e647b and fc98d07.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • docs/install/windows.md

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The Windows Cargo installation command now uses Rustlang.Rustup. The test, installation documentation, and changelog use the corrected package identifier.

Changes

Rustup package correction

Layer / File(s) Summary
Update installation guidance and validation
scripts/desktop-toolchain-path.mjs, tests/frontend/desktopScripts.test.mjs, docs/install/windows.md, CHANGELOG.md
The Windows winget command, test regex, installation documentation, and unreleased changelog entry now use Rustlang.Rustup instead of Rust.Rustup.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Suggested reviewers: debpalash

Merge Risk: ⚪ Minimal · up to fc98d

No concrete merge-blocking risk is identified; the Windows installation guidance is consistently updated.

🚥 Pre-merge checks | ✅ 6 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided, so the required Summary, Changes, Type, Testing, Checklist, and Release cadence information is missing. Add the repository template sections. Describe the Rustup package identifier correction, select the applicable type, document testing, complete the checklist, and include the required Release cadence section.
I18n Completeness (21 Locales) ⚠️ Warning The PR adds no frontend t('...') key, so no locale file is missing a new translation key. However, scripts/desktop-toolchain-path.mjs:103 changes a hardcoded English line in cargoMissingMessage;… Move the user-facing cargoMissingMessage text to the i18n resource path, add its key and interpolation to all 21 locale files, and resolve it with an English fallback for the desktop launchers. Update the unit test to verify the localized…
Title check ⚠️ Warning The title uses conventional-commit format with a scope and accurately describes the Windows Rustup guidance fix. The provided title and body context do not include an issue reference. Add the issue reference, such as #2066, to the pull request title or body. For example: "fix(windows): correct Rustup installation guidance (#2066)".
✅ Passed checks (6 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cross-Platform Default Parity ✅ Passed The PR changes one Windows install command string in cargoMissingMessage and the matching Windows documentation/test. The message is generated identically for all platforms; the PR adds no platform-…
Local-First Guarantee ✅ Passed PASS — The authoritative diff changes only a Windows Rustup package identifier, its test expectation, and related documentation/changelog text. The changed script still only probes local executables a…
Backward Compatibility ✅ Passed PASS — The authoritative PR diff changes only the displayed Windows Rustup package identifier, its test expectation, documentation, and changelog. cargoMissingMessage is failure text for missing Rus…
Full details: I18n Completeness (21 Locales)

Explanation

The PR adds no frontend t('...') key, so no locale file is missing a new translation key. However, scripts/desktop-toolchain-path.mjs:103 changes a hardcoded English line in cargoMissingMessage; desktop launchers send this message to users through console.error, and it bypasses the 21-locale i18n resources.

Resolution

Move the user-facing cargoMissingMessage text to the i18n resource path, add its key and interpolation to all 21 locale files, and resolve it with an English fallback for the desktop launchers. Update the unit test to verify the localized message.

  • Fix all pre-merge checks with AI

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@debpalash debpalash added the ready-for-agent Fully specified, ready for an AFK agent label Sep 17, 2026
@debpalash debpalash changed the title feat: add desktop toolchain PATH healing script and unit tests fix(windows): correct Rustup installation guidance Sep 17, 2026
@debpalash
debpalash merged commit 020febe into debpalash:main Sep 17, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-agent Fully specified, ready for an AFK agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants