Conversation
- Changed PlaywrightCli and DotnetInspect skills to isDefault: false - Added IsDotNetOnly property to SkillDefinition - Marked DotnetInspect as isDotNetOnly: true - Injected ILanguageDiscovery into AgentInitCommand to detect apphost language - Filter out .NET-only skills when workspace is not a .NET apphost - Updated McpInitCommand to pass ILanguageDiscovery through - Updated tests for new default behavior Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/2efe429c-2ad0-4968-8439-d8920c8bcc67 Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/2efe429c-2ad0-4968-8439-d8920c8bcc67 Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
…alues - Replaced bool IsDotNetOnly with IReadOnlyList<string> ApplicableLanguages - Added IsApplicableToLanguage(LanguageId?) method for filtering logic - DotnetInspect uses applicableLanguages: [KnownLanguageId.CSharp] - Empty ApplicableLanguages means the skill is language-agnostic - Updated AgentInitCommand to use IsApplicableToLanguage for filtering - Updated tests to verify new API Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/6bbb18ce-c5f3-4973-8902-f7a31cd8e050 Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
When no language is detected (e.g., standalone `aspire agent init`), skills with ApplicableLanguages restrictions are now excluded instead of being offered. Only language-agnostic skills are shown. Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/e7ba2164-749f-4929-89df-e9cc7e834670 Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
davidfowl
April 2, 2026 04:42
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 -- 15788Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15788" |
davidfowl
changed the title
davidfowl
requested review from
JamesNK,
davidfowl and
mitchdenny
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts aspire agent init skill selection so language-specific skills are only offered when applicable, and removes opt-out defaults for tools that require extra setup.
Changes:
- Introduces language applicability on
SkillDefinitionand filters offered skills based on detected AppHost language. - Updates
aspire mcp initto pass language discovery through to the shared init implementation. - Updates unit tests to validate default-selection and language applicability behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Agents/CommonAgentApplicatorsTests.cs | Updates tests for new default selection behavior and language applicability checks. |
| src/Aspire.Cli/Commands/McpInitCommand.cs | Threads ILanguageDiscovery into the legacy mcp init delegation path. |
| src/Aspire.Cli/Commands/AgentInitCommand.cs | Detects language and filters the skills list before prompting. |
| src/Aspire.Cli/Agents/SkillDefinition.cs | Adds ApplicableLanguages + IsApplicableToLanguage and sets Playwright/dotnet-inspect to opt-in. |
Move pattern matching and directory scanning into shared methods on LanguageInfo so that DefaultLanguageDiscovery, TestLanguageDiscovery, and DotNetSdkCheck all use the same code path: - LanguageInfo.MatchesFile() — checks a filename against detection patterns - LanguageInfo.FindInDirectory() — recursive scan using FileSystemHelper - LanguageInfo.MatchesPattern() — wildcard extension and exact match logic - LanguageInfo.DetectionRecurseLimit — shared depth constant (5 levels) DetectLanguageAsync now uses FindInDirectory (proper glob expansion via Directory.EnumerateFiles) instead of File.Exists which could not match wildcard patterns like *.csproj. DotNetSdkCheck.IsDotNetAppHostAsync fallback now delegates to DetectLanguageAsync instead of duplicating the FileSystemHelper call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DetectLanguageAsync stays flat (immediate directory only) to preserve existing behavior for callers like AgentInitCommand. DetectLanguageRecursiveAsync scans up to DetectionRecurseLimit (5) levels deep, used by DotNetSdkCheck where a broader search is needed. Both use FileSystemHelper.FindFirstFile for proper glob expansion (e.g. *.csproj) instead of File.Exists which can't match wildcards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The agent init command should find apphosts in subdirectories (e.g. src/MyAppHost/MyAppHost.csproj) to correctly determine which skills to offer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
reviewed
Apr 2, 2026
JamesNK
reviewed
Apr 2, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
3 issues found: 1 bug (wrong directory for language detection), 1 latent bug (case-sensitive language ID comparison), 1 E2E test regression (playwright selection toggled the wrong way).
- Use workspaceRoot instead of ExecutionContext.WorkingDirectory for language detection so nested apphosts are found correctly when the user selects a different workspace root in the prompt. - Use case-insensitive comparison in IsApplicableToLanguage to match the rest of the codebase (OrdinalIgnoreCase for language IDs). - Fix E2E tests that navigated Down+Space to deselect playwright — now that playwright is no longer pre-selected, just accept defaults. 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.
|
Contributor
|
🎬 CLI E2E Test Recordings — 55 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23886832539 |
JamesNK
approved these changes
Apr 2, 2026
| // Detect the AppHost language to determine which skills to offer. | ||
| // When no language is detected (e.g., standalone `aspire agent init`), language-restricted skills are excluded. | ||
| var detectedLanguage = await _languageDiscovery.DetectLanguageRecursiveAsync(ExecutionContext.WorkingDirectory, cancellationToken); | ||
| var detectedLanguage = await _languageDiscovery.DetectLanguageRecursiveAsync(workspaceRoot, cancellationToken); |
Member
There was a problem hiding this comment.
nit: add test that detection happens in the right place to prevent regression
IEvangelist
approved these changes
Apr 2, 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.
Description
Two fixes for
aspire agent initskill selection, plus shared language detection infrastructure:1. dotnet-inspect showing for non-.NET apphosts
The dotnet-inspect skill was offered for all AppHost types (including TypeScript). It now has
applicableLanguages: [KnownLanguageId.CSharp]so it's only offered when the workspace contains a .NET AppHost. Language detection usesDetectLanguageRecursiveAsyncagainstworkspaceRoot(not the working directory) to find apphosts in subdirectories.2. playwright selected by default
Both playwright and dotnet-inspect were pre-selected (
isDefault: true), which meant users had to manually deselect them. Changed both toisDefault: falseso users opt-in to these skills.3. Unified language detection helpers
Moved pattern matching and directory scanning into shared
LanguageInfohelpers (MatchesFile,FindInDirectory,MatchesPattern) soDefaultLanguageDiscovery,TestLanguageDiscovery, andDotNetSdkCheckall use the same code path. SplitDetectLanguageAsync(flat, immediate directory) fromDetectLanguageRecursiveAsync(scans 5 levels deep) to preserve existing behavior for callers that don't need recursion.Changes
SkillDefinition: AddedApplicableLanguagescollection andIsApplicableToLanguage(LanguageId?)method with case-insensitive comparison.AgentInitCommand: Detects AppHost language viaILanguageDiscovery.DetectLanguageRecursiveAsyncagainstworkspaceRootand filters skills by applicability.McpInitCommand: PassesILanguageDiscoverythrough toAgentInitCommand.LanguageInfo: New shared helpers —MatchesFile(),FindInDirectory(),MatchesPattern(),DetectionRecurseLimitconstant.DefaultLanguageDiscovery/TestLanguageDiscovery: Refactored to useLanguageInfoshared helpers. AddedDetectLanguageRecursiveAsync.DotNetSdkCheck: Delegates toDetectLanguageRecursiveAsyncinstead of duplicatingFileSystemHelperlogic.LanguageInfoTestscoveringMatchesFile,MatchesPattern,FindInDirectory(including glob, recursion, depth limit).Checklist