docs(dist): make the Microsoft Store the recommended Windows route - #285
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe documentation recommends Microsoft Store installation on Windows, describes standalone installer limitations and unsigned artifacts, and records the prerequisites and disabled state of WinGet publishing. ChangesWindows distribution documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 98: Remove the Microsoft Store link from the README’s OpenScreen
installation instructions until the final 1.9.0 AppX package is live; retain the
terminal installation option and restore the link only once the Store listing
contains that package.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 08af2f78-9425-49d8-8a1a-761860f8f729
📒 Files selected for processing (3)
README.mdtechnical-documentation/engineering/build-and-packaging.mdtechnical-documentation/engineering/release-and-secrets.md
Both Windows artifacts leave CI unsigned — verified with Get-AuthenticodeSignature on the 1.8.0 build, the .exe and the .appx alike. Microsoft signs Store submissions during certification and that signed copy lives only in the Store, so it cannot be redistributed. That makes the Store the only Windows route that installs without a SmartScreen interstitial, and it costs nothing beyond the developer account already paid for. The README now recommends it first, with winget --source msstore as the terminal equivalent, and keeps the .exe as an explicitly documented fallback for people who cannot reach the Store. The fallback carries an explanation rather than an apology: reputation attaches to the file hash while an installer is unsigned, so every release starts untrusted however popular the last one was, and there is no signature for a cautious user to check. Saying so is more useful than letting people guess whether the download is safe. Also records two things that were true but written down nowhere: that no Windows signing exists in the repo at all, and that publish-winget.yml has reported "skipped" on every release because WINGET_IDENTIFIER was never set. Turning it on would publish the unsigned .exe to the community source alongside the signed Store package, which is a decision rather than a variable, so the note says as much instead of just listing the secret. Buying a certificate stays open and is not foreclosed here; it now smooths a secondary path rather than the only one.
#329 moves the configuration test out of the job-level if, so "gates on vars.WINGET_IDENTIFIER" stops being true the moment both land. Describe what has to be true instead of the mechanism that checks it, which also lets the paragraph name the two prerequisites nobody could guess: the PAT must be classic, and the action refuses to author a package that does not already exist in winget-pkgs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@technical-documentation/engineering/release-and-secrets.md`:
- Line 121: Update the WinGet publishing description to state that the workflow
runs for stable releases when WINGET_IDENTIFIER is configured, but publishing
does not complete until the required secret, fork, and existing package version
prerequisites are met. Remove claims that publishing is switched off or has
never run, while preserving the manual first-submission guidance.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bbc68e1-0e0a-4bf9-9dae-df33f4c94910
📒 Files selected for processing (1)
technical-documentation/engineering/release-and-secrets.md
913d923 to
6148d13
Compare
CodeRabbit, correctly, on my own paragraph. After #329 the job is not off: it starts on every stable release and either publishes or annotates the run. What is unfinished is the publishing, not the workflow, and describing it as "never run" would be false the moment both land. Keeps the history, which is the part worth remembering: the silent skip is why #148 sat open through eight green releases.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
technical-documentation/engineering/build-and-packaging.md (2)
108-108: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
afterwardinstead ofafterwards.This matches the repository's American English style.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@technical-documentation/engineering/build-and-packaging.md` at line 108, Update the wording near the -KeepRegistered explanation to use “afterward” instead of “afterwards,” preserving the existing meaning and surrounding documentation.Source: Linters/SAST tools
49-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd languages to both output fences.
The fenced output blocks start at Lines 49 and 57 without a language. Use
textfor both fences so Markdown validation passes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@technical-documentation/engineering/build-and-packaging.md` around lines 49 - 59, Add the text language identifier to both fenced output blocks in the documentation, including the fences containing “require BEFORE PATH” and “require AFTER PATH” results, without changing their contents.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@technical-documentation/engineering/build-and-packaging.md`:
- Line 108: Update the wording near the -KeepRegistered explanation to use
“afterward” instead of “afterwards,” preserving the existing meaning and
surrounding documentation.
- Around line 49-59: Add the text language identifier to both fenced output
blocks in the documentation, including the fences containing “require BEFORE
PATH” and “require AFTER PATH” results, without changing their contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d42abf85-e666-40ab-8fa2-a22ae88b6403
📒 Files selected for processing (3)
README.mdtechnical-documentation/engineering/build-and-packaging.mdtechnical-documentation/engineering/release-and-secrets.md
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
1 participant
Replaces #283, which tried to buy the problem away by signing the
.exe.What I verified first
Both Windows artifacts leave CI unsigned. Checked with
Get-AuthenticodeSignatureagainst the actual 1.8.0 build artifacts:The AppX being unsigned is not a defect: Microsoft signs Store submissions during certification, and that signed copy exists only in the Store. It is never handed back, so it cannot be redistributed.
Two consequences that decided the approach:
.appxinstead of the.exewould be worse, not better. Windows runs an unsigned.exeafter a SmartScreen prompt; it refuses outright to install an unsigned MSIX/AppX, because sideloading needs a signature the machine already trusts. We would trade a file that warns for a file that will not install.What changes
The README recommends the Store first, with
winget install --source msstore OpenScreenas the terminal equivalent, and keeps the.exeas an explicitly documented fallback for people who cannot reach the Store (LTSC, managed machines, offline installs, older versions).The fallback gets an explanation rather than an apology. SmartScreen attaches reputation to the file hash while an installer is unsigned, so every release starts untrusted no matter how popular the previous one was, and there is no signature for a cautious user to verify. Users hitting that dialog deserve to know which of those it is; guessing is worse for them than being told.
What I did not build
publish-winget.ymlalready exists and does exactly what a winget CI job should. It has simply never run: it gates onvars.WINGET_IDENTIFIER != '', and neither that variable norWINGET_ACC_TOKENis set, so every release has reportedskipped— stable ones included.I left it dormant on purpose rather than switching it on in passing.
winget-releasersubmits the NSIS.exeto the community repository, and that installer is unsigned — so turning it on adds a second, unsigned route next to the signed Store one. That is a distribution decision, not a variable to flip, so the docs now say so.Docs
build-and-packaging.mdrecords that no Windows signing exists in the repo at all, with the signature table above and the AppX-is-not-a-substitute reasoning, so the next person to have this idea finds the answer instead of re-deriving it.release-and-secrets.mdrecords the dormant winget state and what turning it on would actually publish.Sequencing — do not merge before the Store listing is live
The README will link to
apps.microsoft.com/detail/9MXQ1HQJL5G5, and the submission is still a draft, waiting on the 1.9.0 final AppX. Merging earlier advertises a dead link and awingetcommand that resolves to nothing. Worth confirming the URL against Partner Center's product-identity page at that point too.Buying a certificate is not foreclosed by any of this — it now smooths a secondary path rather than the only one, which makes it a comfort call once install traffic shows how much the fallback is actually used.
Summary by CodeRabbit
wingetinstallation instructions, including automatic updates and signing details.