Fix TypeScript AppHost package manager detection - #16558
Merged
Sébastien Ros (sebastienros) merged 5 commits intoApr 30, 2026
Merged
Fix TypeScript AppHost package manager detection#16558Sébastien Ros (sebastienros) merged 5 commits into
Sébastien Ros (sebastienros) merged 5 commits into
Conversation
Treat package-lock.json as an npm marker and limit parent directory package manager probing to the AppHost directory's direct parent. Log the marker used to select the TypeScript AppHost package manager. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sébastien Ros (sebastienros)
requested review from
James Newton-King (JamesNK),
David Fowler (davidfowl) and
Mitch Denny (mitchdenny)
as code owners
April 29, 2026 15:23
Copilot started reviewing on behalf of
Sébastien Ros (sebastienros)
April 29, 2026 15:24
View session
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16558Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16558" |
Collaborator
|
Would these tests have caught the problem? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes TypeScript AppHost package-manager detection in the Aspire CLI to avoid accidentally inheriting unrelated workspace markers (e.g., a distant yarn.lock), which could cause npm-based generated projects to run via Yarn. It also adds diagnostic logging to make future resolution behavior easier to understand.
Changes:
- Update TypeScript AppHost toolchain resolution to treat
package-lock.jsonas an npm marker and limit probing to the AppHost directory plus (at most) one eligible parent, skipping root/home. - Add debug logging that records the exact marker/reason used to select the package manager.
- Add/update tests covering npm lockfile detection, limited parent probing behavior, root/home parent skipping, and the debug “reason” log.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Projects/TypeScriptAppHostToolchainResolverTests.cs | Adds/updates unit tests for package manager marker precedence, parent probing limits, and debug logging behavior. |
| src/Aspire.Cli/Utils/EnvironmentChecker/TypeScriptAppHostToolingCheck.cs | Passes a logger into toolchain resolution so the new debug reasoning is emitted during environment checks. |
| src/Aspire.Cli/Scaffolding/ScaffoldingService.cs | Passes a logger into toolchain resolution (and adjusts helper method) so scaffolding/install flows emit the new debug reasoning. |
| src/Aspire.Cli/Projects/TypeScriptAppHostToolchainResolver.cs | Implements the new marker detection rules, adds package-lock.json handling, parent probing constraints, and “reason” logging support. |
| src/Aspire.Cli/Projects/GuestAppHostProject.cs | Passes a logger into toolchain resolution so Guest runtime creation emits the new debug reasoning. |
Use OS-appropriate path comparison when deciding whether to skip root and home parent directories for package manager detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
Yes, the new test |
Keep yarn markers ahead of package-lock.json within the same candidate directory while still allowing a local npm lockfile to beat parent-directory yarn markers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Only use file-based yarn markers for TypeScript AppHost package manager detection and update the resolver test accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Re-running the failed jobs in the CI workflow for this pull request because 2 jobs were identified as retry-safe transient failures in the CI run attempt.
Matched test failure patterns (1 test)
|
David Pine (IEvangelist)
approved these changes
Apr 29, 2026
David Pine (IEvangelist)
left a comment
David Pine (IEvangelist)
left a comment
Member
There was a problem hiding this comment.
Approval
Targeted, well-scoped fix for the cross-workspace package-manager bleed-through. The follow-up commits also addressed the case-sensitivity, Yarn-vs-package-lock.json precedence, and .yarn directory feedback nicely.
A few non-blocking nits inline — including one question about whether limiting parent probing to a single directory is too aggressive for nested-workspace AppHost layouts.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sébastien Ros (sebastienros)
merged commit Apr 30, 2026
7468a03
into
microsoft:main
285 checks passed
Sébastien Ros (sebastienros)
deleted the
sebastienros/sebros-template-yarn
branch
April 30, 2026 13:57
Contributor
Author
|
/backport to release/13.3 |
Contributor
|
Started backporting to |
Nell Shamrell-Harrington (nellshamrell)
pushed a commit
to nellshamrell/aspire
that referenced
this pull request
May 18, 2026
* Fix TypeScript package manager detection Treat package-lock.json as an npm marker and limit parent directory package manager probing to the AppHost directory's direct parent. Log the marker used to select the TypeScript AppHost package manager. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix TypeScript AppHost path comparisons Use OS-appropriate path comparison when deciding whether to skip root and home parent directories for package manager detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve same-directory yarn marker precedence Keep yarn markers ahead of package-lock.json within the same candidate directory while still allowing a local npm lockfile to beat parent-directory yarn markers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove yarn directory package manager hint Only use file-based yarn markers for TypeScript AppHost package manager detection and update the resolver test accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address TypeScript toolchain review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
TypeScript AppHost package-manager detection could accidentally inherit a distant parent
yarn.lock, causing npm-based generated projects to run through Yarn. This is especially visible foraspire newTypeScript starter projects created under a repository that already has a top-level Yarn marker.This change treats
package-lock.jsonas an npm marker, limits package-manager probing to the AppHost directory plus one eligible parent, and skips parent probing when that parent is the filesystem root or user home directory. The resolver now logs the exact marker used for the package-manager decision, which should make future diagnosis easier.Tests added/updated cover npm lockfile detection, parent-only probing, root/home parent skipping, and the debug reason log.
Validation: