Allow non-interactive app selection in init via --appId - #159
Allow non-interactive app selection in init via --appId#159Alexandre Zollinger Chohfi (azchohfi) wants to merge 6 commits into
init via --appId#159Conversation
`msstore init` is the only command that produces the artifacts a PWA publish needs (the `*_PWABuilderExtractedBundle` folder and `pwaAppInfo.json`), so it is mandatory for the PWA publishing flow. But `SelectAppAsync` always showed a Spectre selection prompt, which throws "Cannot show selection prompt since the current terminal isn't interactive." on a headless CI runner. There was no way to supply the Store Id up front, so a PWA could not be published to the Microsoft Store from GitHub Actions or Azure DevOps at all. - Add an `--appId`/`-id` option to `init`. When supplied, the app is resolved with `GetApplicationAsync` and the interactive selection is skipped, matching what `publish` already does with its own `--appId` option. - Auto-select the app when the account has exactly one and the CLI is running non-interactively. Interactive runs keep prompting. - Fail fast with an actionable message that points at `--appId` (and `msstore apps list`) when running on CI with more than one app and no `--appId`, instead of letting Spectre throw its generic error. The prompt call is also guarded against `NotSupportedException` so a non-TTY that is not flagged as CI gets the same actionable message. Non-interactive detection uses the existing `IEnvironmentInformationService.IsRunningOnCI` (`CI=true` for GitHub Actions, `TF_BUILD=true` for Azure DevOps), which is already used the same way to skip the category and listing prompts in `FulfillApplicationAsync`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00c29930-8fdc-4c0c-80cc-d45ed0a5d077
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00c29930-8fdc-4c0c-80cc-d45ed0a5d077
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00c29930-8fdc-4c0c-80cc-d45ed0a5d077
The previous commit made CleanResult ANSI-aware, but ProjectConfiguratorParsesPWASuccessfullyOnCIIfAppIdIsProvided never called it, so its 'AppId: <id>' assertion still compared plain text against Spectre-colored output and kept failing on all three CI runners. Reproduced locally by setting GITHUB_ACTIONS=true, which makes Spectre's default profile enrichers enable ANSI the same way CI does. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00c29930-8fdc-4c0c-80cc-d45ed0a5d077
There was a problem hiding this comment.
Pull request overview
Adds non-interactive application selection to msstore init, enabling PWA publishing in CI/CD.
Changes:
- Adds
--appId/-idapplication lookup. - Auto-selects single-app accounts on CI and provides actionable errors otherwise.
- Adds and updates CI/PWA tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
MSStore.CLI/Commands/InitCommand.cs |
Implements non-interactive app selection. |
MSStore.CLI.UnitTests/ProjectConfiguratorTests.cs |
Covers CI PWA initialization paths. |
MSStore.CLI.UnitTests/InitCommandUnitTests.cs |
Covers invalid IDs and multi-app CI failures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The secondary safety net in SelectAppAsync had no coverage: the CI test returns before reaching the prompt, so a change to the prompt exception contract could have silently broken it. Verified the test is meaningful by temporarily removing the catch, which makes it fail. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00c29930-8fdc-4c0c-80cc-d45ed0a5d077
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
MSStore.CLI/Commands/InitCommand.cs:388
GetApplicationAsyncobservesct, so Ctrl+C can throwOperationCanceledExceptionhere. The broad catch converts that cancellation into an “incorrect AppId” failure (and logs it as an API error) instead of allowing command cancellation to propagate. Rethrow caller-requested cancellation before handling retrieval failures.
catch (Exception err)
{
ctx.ErrorStatus(_ansiConsole, "Could not retrieve your application. Please make sure you have the correct AppId.");
_logger.LogError(err, "Could not find application with id '{AppId}'.", appId);
return null;
}
The broad catch turned a Ctrl+C during GetApplicationAsync into 'Please make sure you have the correct AppId', which wrongly tells the user their Store Id is bad and logs cancellation as an API error. The filter is scoped to ct.IsCancellationRequested so an internal timeout surfacing as OperationCanceledException is still treated as a retrieval failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00c29930-8fdc-4c0c-80cc-d45ed0a5d077
| } | ||
|
|
||
| [TestMethod] | ||
| public async Task InitCommandShouldFailOnCIIfNoAppIdIsProvided() |
There was a problem hiding this comment.
It can work on CI with no app id is provided and if there's only one app , right ? can we add a scenario for that as well ?
There was a problem hiding this comment.
Yes, that's right - on CI with no --appId and exactly one registered app, init auto-selects it and proceeds instead of failing.
That scenario is already covered, just in the other test file: ProjectConfiguratorParsesPWASuccessfullyOnCIIfAccountHasASingleApp in MSStore.CLI.UnitTests/ProjectConfiguratorTests.cs. It sets IsRunningOnCI to true, stubs GetApplicationsAsync to return a single app, runs init with no --appId, and asserts:
Using Fake App 1 (9PN3ABCDEFGA), the only application registered in your account.is printed,- the run reaches
Submission commit success!, SelectionPromptAsyncwas never called.
It lives there rather than next to this test because it needs the full PWA pipeline (SetupSuccessfullPWA: PWABuilder, zip extraction, submission mocks) to get all the way to a commit. Duplicating that setup here would be a fair amount of copied scaffolding for the same assertions.
So the CI matrix is: one app -> auto-select, multiple apps -> this test's actionable error, --appId supplied -> direct lookup (ProjectConfiguratorParsesPWASuccessfullyOnCIIfAppIdIsProvided).
Happy to mirror a lighter-weight version here if you'd still prefer both cases side by side in this file - just unresolve and let me know.
There was a problem hiding this comment.
If its not too much can we have a lighter-weight version here as well
Why
msstore initis the only command that produces the artifacts a PWA publish needs: it calls the PWABuilder API to build the MSIX bundle (*_PWABuilderExtractedBundle) and writespwaAppInfo.json. That makes it mandatory for the PWA publishing flow, sincemsstore publish <url> --appId <id>on its own fails while loadingpwaAppInfo.json.But
initalways showed an interactive app-selection prompt, and there was no way to supply the Store Id up front. On a headless runner that prompt throws:Net result: a PWA could not be published to the Microsoft Store from GitHub Actions or Azure DevOps at all, even though the Store Id is already known because it was reserved in Partner Center.
Approach
Three changes, all contained in
InitCommand:New
--appId/-idoption. When supplied, the app is resolved withGetApplicationAsyncandSelectAppAsyncis skipped entirely. Named to match the--appId/-idoptionpublishalready has, rather than the--app-idspelling suggested in the issue, so it stays consistent with the rest of the CLI's camelCase options.Auto-select when the account has exactly one app, but only when running non-interactively. Interactive runs keep prompting so behavior there is unchanged.
Fail fast with an actionable message when running non-interactively with more than one app and no
--appId, instead of letting Spectre throw its generic error:The issue's repro now works end to end:
Notes for reviewers
Non-interactive detection reuses
IEnvironmentInformationService.IsRunningOnCI(CI=truefor GitHub Actions,TF_BUILD=truefor Azure DevOps) rather thanIAnsiConsole.Profile.Capabilities.Interactive. This is deliberate: it is already the established "don't prompt" signal in this codebase, used the same way inFulfillApplicationAsyncto default the category and listings on CI. It is also the only workable option for tests, since every console inBaseCommandLineTestis created withInteractionSupport.No, so keying off the console capability would have silently broken every existing prompt-based test.As a secondary safety net, the prompt call is guarded against
NotSupportedExceptionso a non-TTY that is not flagged as CI gets the same actionable message. Spectre raises that exception for both non-interactive and non-ANSI terminals, andCreateNewAppAsync(which throwsNotImplementedException) sits outside thetry, so nothing unrelated is swallowed.One behavior change worth a look: on CI with more than one app,
initnow fails fast instead of prompting. That is what previously crashed anyway, so it is strictly an improvement, but it does mean an interactive shell withCI=trueset will no longer prompt. This matches the existing contract thatIsRunningOnCImeans "do not prompt".ProjectConfiguratorParsesPWASuccessfullyIfOnCIIfPublishwas updated to pass--appId, reflecting the new requirement that CI runs identify the app.Out of scope
Making
publish <url> --appId <id>work standalone without a priorinitis a much larger change, since PWA packaging lives inPWAProjectConfigurator.ConfigureAsync, which is what calls PWABuilder and writespwaAppInfo.json. The issue's own proposed fix is theinitoption, sopublishremains init-dependent here.--publisherDisplayNameis not a CI blocker and is left alone; it already has non-interactive escape hatches via the flag itself andmsstore settings setpdn.Testing
Five new tests plus one updated:
initwith an unknown--appIdreturns -1 with the "Could not retrieve your application" message, and never falls back to listing apps.--appIdreturns -1 with a message mentioning--appId.-idpublishes successfully with no prompt (the exact scenario from the issue, and it covers the short alias).Release build (warnings as errors): 0 warnings, 0 errors. Targeted tests: 24/24 passing on both
net10.0andnet10.0-windows10.0.17763.0. Two pre-existingPublishCommandUnitTestsarm64 failures were confirmed unrelated by re-running them on a clean tree.Fixes: #146