Repository navigation
Offer to set up the GitHub CLI in winapp-pr instead of just demanding it - #700
Merged
Merged
Conversation
A missing gh only surfaced as a one-line hint, and a gh that was installed but not signed in surfaced as a raw 'gh: Bad credentials (HTTP 401)' dump from whichever API call happened to run first. Check both before doing any work. When the console is interactive, offer to install gh via winget and to run gh auth login; otherwise print the exact commands. gh drives its own login prompts so no git protocol or SSH choice is made on the user's behalf. A freshly installed gh is not on this process's PATH, so the path is refreshed from the registry and known install locations are probed, and every call goes through the resolved executable. A token is genuinely required: GitHub serves run and artifact metadata anonymously but returns 401 for the artifact zip, so there is no tokenless path to a build. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db5ddd3d-7017-4d4a-9b45-d7f8dc70deaf
Contributor
There was a problem hiding this comment.
Pull request overview
Improves winapp-pr setup by proactively checking GitHub CLI availability and authentication.
Changes:
- Offers to install
ghusing winget. - Offers interactive authentication and handles non-interactive failures.
- Resolves and consistently uses the installed executable path.
Comments suppressed due to low confidence (1)
scripts/winapp-pr.ps1:244
- This post-login verification again checks all accounts and hosts, so an unrelated stale account can make a successful
github.comlogin appear to have failed. Verify only the active account forgithub.com, matching the credentials subsequent API calls use.
& $script:GhExe auth status *> $null
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| $script:GhExe = $ghPath | ||
|
|
||
| & $script:GhExe auth status *> $null |
| } | ||
|
|
||
| # gh drives its own prompts here, so let it own the console. | ||
| & $script:GhExe auth login |
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Jul 30, 2026
The build metrics comment already links the MSIX artifacts, but actually using one still means downloading a zip, unpacking it, trusting a certificate, and removing the previously installed package. `winapp-pr` does all of that from a PR number, so the comment now offers the command directly. Appended to the comment when the run produced an `msix-packages` artifact: --- ### Try This Build Installs the MSIX for your architecture, replacing any previously installed build. Needs the [GitHub CLI](https://cli.github.com) — the command offers to install it and sign you in if it is missing. ```powershell & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 697 ``` <details><summary>Switching between builds often?</summary> Put the tool on your PATH once: ```powershell & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPath ``` Then this build is just: ```powershell winapp-pr 697 ``` Run `winapp-pr` with no arguments to pick from a list of open PRs. </details> --- The headline command runs the tool straight from the web, so trying a PR needs no prior setup at all — the right shape for a reviewer who wants to look at one build once. Putting it on PATH is a collapsed aside for people switching between builds repeatedly. The GitHub CLI is genuinely required, because GitHub serves run and artifact metadata anonymously but returns 401 for the artifact zip itself. #700 makes `winapp-pr` offer to install and sign in to `gh` rather than just failing, which is what the wording above refers to; that PR is worth merging first so the sentence is accurate. ## Security This workflow treats the `metrics-data` artifact as attacker-controlled and allowlists everything rendered into Markdown. The new block keeps to that: it interpolates only `prNumber` — resolved from the trusted `head_sha` and verified against the PR's current head, then coerced with `Number()` — and `context.repo`. No artifact-derived value reaches it, and no branch or title text is echoed. ## Validation Rendered the comment locally by extracting the real script from the workflow and running it against stubbed APIs and fixture metrics, confirming the section appears with the correct PR number and that it is omitted when no `msix-packages` artifact exists. YAML parses and the embedded JavaScript passes `node --check` under the same async wrapper `actions/github-script` uses. Co-authored-by: Nikola Metulev <711864+nmetulev@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db5ddd3d-7017-4d4a-9b45-d7f8dc70deaf
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Jul 30, 2026
Picking up newer commits on the build you are running meant remembering which PR or branch it came from and naming it again, and re-resolving the same target silently uninstalled and reinstalled an identical package with no sign you were already current. -Update installs the newest build for whatever is installed, using the repo and branch from the install record so a private-fork build stays on that fork. Any resolution landing on the installed run now reports it and stops; -Force still reinstalls. Updating winapp-pr itself moves to -UpdateTool, since updating the build is the far more common intent. The install record is only trusted while it still describes the package that is actually installed, so a package installed by other means is not mistaken for a known build: -Status says the source is unknown, -Update asks for an explicit target, and the picker stops marking it. The record now tracks architecture too, so asking for a different -Arch is never mistaken for a no-op. The picker distinguishes the installed build being current from having a newer build waiting, and only claims a build is installed when it came from the repo being listed, since branch names collide across forks. Adds -Help. PowerShell's built-in -? covers a script on disk, but throws when the script is run from the web as a scriptblock, which is exactly how the documented installer runs it. Also carries two review fixes for #700, which merged before they were written. The gh readiness check is scoped to the active github.com account, because a bare 'gh auth status' fails when any configured account on any host is stale. And when an invalid GH_TOKEN or GITHUB_TOKEN is the cause, say so rather than offering a sign-in that gh will refuse while an env token is set. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db5ddd3d-7017-4d4a-9b45-d7f8dc70deaf
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Jul 30, 2026
Picking up newer commits on the build you are running meant remembering which PR or branch it came from and naming it again, and re-resolving the same target silently uninstalled and reinstalled an identical package with no sign you were already current. -Update installs the newest build for whatever is installed. It resolves by the PR the build came from where there was one, because fork PRs run in the base repo and a branch name alone can match another fork's runs; where only a branch is known it pins to that build's head repository. Only an explicit -Repo overrides the recorded source, since WINAPP_PR_REPO is a default for new installs rather than a redirect for an existing one. Any resolution landing on the installed run now reports it and stops; -Force still reinstalls. Updating winapp-pr itself moves to -UpdateTool, since updating the build is the far more common intent. The install record is only trusted while it still describes the package that is actually installed, so a package installed by other means is not mistaken for a known build: -Status says the source is unknown, -Update asks for an explicit target, and the picker stops marking it. The record now tracks architecture too, so asking for a different -Arch is never mistaken for a no-op. The picker distinguishes the installed build being current from having a newer build waiting, and only claims a build is installed when it came from the repo being listed, since branch names collide across forks. Adds -Help. PowerShell's built-in -? covers a script on disk, but throws when the script is run from the web as a scriptblock, which is exactly how the documented installer runs it. Also carries two review fixes for #700, which merged before they were written. The gh readiness check is scoped to the active github.com account, because a bare 'gh auth status' fails when any configured account on any host is stale. And when an invalid GH_TOKEN or GITHUB_TOKEN is the cause, say so rather than offering a sign-in that gh will refuse while an env token is set. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db5ddd3d-7017-4d4a-9b45-d7f8dc70deaf
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Jul 30, 2026
Picking up newer commits on the build you are running meant remembering which PR or branch it came from and naming it again, and re-resolving the same target silently uninstalled and reinstalled an identical package with no sign you were already current. -Update installs the newest build for whatever is installed. It resolves by the PR the build came from where there was one, because fork PRs run in the base repo and a branch name alone can match another fork's runs; where only a branch is known it pins to that build's head repository. Only an explicit -Repo overrides the recorded source, since WINAPP_PR_REPO is a default for new installs rather than a redirect for an existing one. Any resolution landing on the installed run now reports it and stops; -Force still reinstalls. Updating winapp-pr itself moves to -UpdateTool, since updating the build is the far more common intent. The install record is only trusted while it still describes the package that is actually installed, so a package installed by other means is not mistaken for a known build: -Status says the source is unknown, -Update asks for an explicit target, and the picker stops marking it. The record now tracks architecture too, so asking for a different -Arch is never mistaken for a no-op. The picker distinguishes the installed build being current from having a newer build waiting, and only claims a build is installed when it came from the repo being listed, since branch names collide across forks. Adds -Help. PowerShell's built-in -? covers a script on disk, but throws when the script is run from the web as a scriptblock, which is exactly how the documented installer runs it. Also carries two review fixes for #700, which merged before they were written. The gh readiness check is scoped to the active github.com account, because a bare 'gh auth status' fails when any configured account on any host is stale. And when an invalid GH_TOKEN or GITHUB_TOKEN is the cause, say so rather than offering a sign-in that gh will refuse while an env token is set. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db5ddd3d-7017-4d4a-9b45-d7f8dc70deaf
Nikola Metulev (nmetulev)
added a commit
that referenced
this pull request
Jul 30, 2026
Based directly on `main`. Single commit, one file. Picking up newer commits on the build you are already running was more work than it should be. You had to remember which PR or branch it came from — `-Status` would tell you, but that is an extra step — and name it again. Worse, re-resolving the same target silently uninstalled and reinstalled an identical package with no indication you were already current. ## Changes **`-Update`** installs the newest build for whatever is installed: ```powershell winapp-pr -Update ``` It resolves by the PR the build came from where there was one. Fork PRs run in the *base* repo, so a branch name alone can match another fork's runs — `main` being the obvious case — and picking "the newest run on that branch" could hand you an unrelated contributor's build. Where only a branch is known, resolution is pinned to that build's head repository instead. Only an explicit `-Repo` overrides the recorded source. `WINAPP_PR_REPO` is a default for *new* installs, not a redirect for an existing one; otherwise having it point at the public repo while running a private-fork build would send `-Update` looking for that branch in the wrong place. Updating the tool itself moves from `-Update` to **`-UpdateTool`**. Updating the *build* is by far the more common intent, so it should own the obvious name. **Already-installed builds are no longer reinstalled.** Any resolution that lands on the run you are already on stops early: ``` [OK] Run 30544907744 [success] de22c73 [OK] Already on this build -- nothing to do. Use -Force to reinstall it anyway. ``` This applies to naming a target explicitly too, so `winapp-pr 681` when you are on 681's newest build is a no-op instead of a ~10 second uninstall/reinstall cycle. **The install record is only trusted while it still describes the installed package.** A package installed by other means — double-clicking an MSIX, `setup-winapprun.ps1` — used to leave the record describing a build that was no longer there, which would have let the no-op above skip a genuinely needed install. The record is now validated against the live package first: ``` >> Installed winapp package winapp-dev 0.5.1.40 [CN=runneradmin] [WARN] This package was not installed by winapp-pr; its source is unknown. ``` `-Update` refuses rather than guessing, and points at a concrete alternative: ``` [ERROR] The installed package (winapp-dev 0.5.1.40) was not installed by winapp-pr, so there is no way to tell which build it came from. Name what you want instead, for example: winapp-pr jay/find-ui-port ``` The record now tracks architecture as well, so requesting a different `-Arch` for a run you already have is not mistaken for a no-op. Records written before this change lack the newer fields and fall back to the old behaviour, so nobody has to reinstall to get a working record. **The picker shows build freshness**, distinguishing "you have this" from "there is something newer": ``` * installed ^ newer build available . current branch 1. . #702 Add winapp-pr -Update and show build freshness in the picker nmetulev - 2m 2. ^ #681 feat: add winapp find-ui — WinUI control & sample search Jaylyn-Barbee - 4m 3. #701 ci: make sample-test Pester install resilient to PSGallery nmetulev - 2h ``` Freshness is resolved the same way an install would resolve it — the newest run that actually has an artifact, pinned to the same head repository — so neither an in-progress build nor another fork's run produces a phantom `^`. It costs two extra API calls, only for the one installed branch. **`-Help`** lists everything. PowerShell's built-in `-?` already covers the on-disk case, but it throws when the script is run from the web as a scriptblock: ``` InvalidArgument: Cannot process argument because the value of argument "helpTarget" is null. ``` That is exactly how the documented installer runs it, so `-Help` uses `Get-Help` when there is a file and falls back to the script's own comment-based help block otherwise. ## Also carries the #700 review fixes Those review comments were written after #700 had already merged, so they never landed. They are folded in here rather than left on a dead branch: - The gh readiness check is scoped with `--active --hostname github.com`. A bare `gh auth status` fails when *any* configured account on *any* host is stale, so anyone with a second account would have been pushed into a pointless sign-in. - When an invalid `GH_TOKEN` or `GITHUB_TOKEN` is the cause, say so instead of offering to sign in. `gh auth login` refuses to store credentials while an env token is set, so that prompt could only ever dead-end. ## Validation The fork-collision fix was reproduced directly: two `Build and Package` runs on a branch named `feature` from different head repositories, the unrelated one newer. Unfiltered resolution selects the stranger's run; pinned to the head repository it selects the right one. `-Update` was checked with `WINAPP_PR_REPO` pointing at a different repo, confirming it stays on the recorded source; with a record carrying a PR number, confirming it resolves by PR; and with a legacy record lacking the new fields, confirming the branch path still works. A real install was run end to end to confirm `Pr`, `HeadRepo`, and `Arch` are persisted. Also exercised against real branch builds: `-Update` when current reports it and exits 0; `-Update -Force` performs a full reinstall; `-Update` with no record fails cleanly; `-UpdateTool` still targets the tool. Record validation was covered as a table — matching version and architecture, mismatched architecture, legacy record, and a version mismatch standing in for a manual install — and end to end by pointing the record at a version that is not installed. Picker markers were checked in all three states, and `-Help` both from a file and through a scriptblock. --------- Co-authored-by: Nikola Metulev <711864+nmetulev@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db5ddd3d-7017-4d4a-9b45-d7f8dc70deaf
This was referenced Aug 14, 2026
This was referenced Aug 21, 2026
This was referenced Sep 11, 2026
This was referenced Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
winapp-prneeds the GitHub CLI, but handled its absence poorly. A missingghproduced a one-line hint, and aghthat was installed but not signed in produced a raw dump from whichever API call happened to run first:Both are now checked before any work happens, and on an interactive console the tool offers to fix them:
Answering yes runs
winget install --id GitHub.cli -e; a missing sign-in similarly offers to rungh auth login. Declining, or a non-interactive console, prints the exact commands and exits 1.Notes
gh auth loginruns without flags so it drives its own prompts. Passing--git-protocolwould have set the git protocol for all users on the host, which is not this tool's business.A freshly installed
ghis not on the current process's PATH, which would make the offer useless — you would install it and immediately be told it is missing. So PATH is refreshed from the registry, known install locations are probed, and every call goes through the resolved executable rather than a baregh.A token is genuinely required, so there is no "just skip gh" option. Run and artifact metadata are served anonymously, but the artifact zip is not:
Validation
Verified the happy path is unchanged; that a
ghpresent in Program Files but absent from PATH is found and used, which is exactly the post-winget state; and that a completely unfindableghin a non-interactive console fails cleanly with instructions and exit 1. Unauthenticated failures were reproduced with a bogusGH_TOKEN.The winget install and
gh auth logininvocations themselves are not exercised — doing so would mutate the machine running the tests — but both are single calls whose success is re-verified before continuing.