Add skill file update detection to aspire update command#14348
Add skill file update detection to aspire update command#14348mitchdenny wants to merge 8 commits intomainfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14348Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14348" |
There was a problem hiding this comment.
Pull request overview
This PR enhances the aspire update command to detect and prompt users to update outdated SKILL.md files that were previously created by the aspire agent init command. The feature checks for skill files after updating project packages and offers to update them if they differ from the current expected content.
Changes:
- Added skill file detection logic to UpdateCommand that checks for outdated SKILL.md files after package updates
- Added new resource strings for user prompts and success messages with localization support across 13 languages
- Created comprehensive unit and E2E test coverage with 5 new unit tests and 1 E2E test
- Enhanced test infrastructure with TestGitRepository mock and DisplaySuccessCallback in TestConsoleInteractionService
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Commands/UpdateCommand.cs | Added CheckAndUpdateSkillFilesAsync method, IGitRepository dependency, and skill file path constant to detect and update outdated skill files |
| src/Aspire.Cli/Resources/UpdateCommandStrings.Designer.cs | Added properties for new resource strings (SkillFileOutdatedPrompt, SkillFileUpdatedMessage) |
| src/Aspire.Cli/Resources/UpdateCommandStrings.resx | Defined base English resource strings for skill file update prompts and messages |
| src/Aspire.Cli/Resources/xlf/*.xlf | Added localization entries for 13 languages (marked as "new" for translation) |
| tests/Aspire.Cli.Tests/TestServices/TestGitRepository.cs | Created mock implementation of IGitRepository for unit testing |
| tests/Aspire.Cli.Tests/TestServices/TestConsoleInteractionService.cs | Added DisplaySuccessCallback property to test service for success message verification |
| tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.cs | Added UpdateCommandSkillFileTests class with 5 comprehensive unit tests covering various scenarios |
| tests/Aspire.Cli.EndToEnd.Tests/UpdateSkillFileTests.cs | Added end-to-end test validating skill file detection, update prompt, and file content update |
Files not reviewed (1)
- src/Aspire.Cli/Resources/UpdateCommandStrings.Designer.cs: Language not supported
| // Try to discover the git repository root | ||
| var gitRoot = await _gitRepository.GetRootAsync(cancellationToken); | ||
| if (gitRoot is null) | ||
| { | ||
| _logger.LogDebug("Not in a git repository, skipping skill file update check"); | ||
| return; | ||
| } | ||
|
|
||
| var skillFilePath = Path.Combine(gitRoot.FullName, s_skillFileRelativePath); | ||
|
|
||
| if (!File.Exists(skillFilePath)) | ||
| { | ||
| _logger.LogDebug("No skill file found at {SkillFilePath}, skipping update check", skillFilePath); | ||
| return; | ||
| } | ||
|
|
||
| // Read existing content and compare with current expected content | ||
| var existingContent = await File.ReadAllTextAsync(skillFilePath, cancellationToken); | ||
| var normalizedExisting = NormalizeLineEndings(existingContent); | ||
| var normalizedExpected = NormalizeLineEndings(CommonAgentApplicators.SkillFileContent); | ||
|
|
||
| if (string.Equals(normalizedExisting, normalizedExpected, StringComparison.Ordinal)) | ||
| { | ||
| _logger.LogDebug("Skill file at {SkillFilePath} is up to date", skillFilePath); | ||
| return; | ||
| } | ||
|
|
||
| // Skill file is outdated, prompt for update | ||
| var promptMessage = string.Format( | ||
| CultureInfo.CurrentCulture, | ||
| UpdateCommandStrings.SkillFileOutdatedPrompt, | ||
| s_skillFileRelativePath); | ||
|
|
||
| var shouldUpdate = await InteractionService.ConfirmAsync( | ||
| promptMessage, | ||
| defaultValue: true, | ||
| cancellationToken); | ||
|
|
||
| if (shouldUpdate) | ||
| { | ||
| await File.WriteAllTextAsync(skillFilePath, CommonAgentApplicators.SkillFileContent, cancellationToken); | ||
| InteractionService.DisplaySuccess(UpdateCommandStrings.SkillFileUpdatedMessage); | ||
| _logger.LogInformation("Updated skill file at {SkillFilePath}", skillFilePath); | ||
| } |
There was a problem hiding this comment.
The CheckAndUpdateSkillFilesAsync method lacks error handling. If an exception occurs during file I/O operations (reading at line 599 or writing at line 622), it will propagate to the outer try-catch block and cause the entire update command to fail. Since skill file checking is an optional enhancement feature, consider wrapping the method body in a try-catch block that logs errors and continues gracefully. This ensures that failures in skill file detection don't prevent the main update operation from completing successfully.
| // Try to discover the git repository root | |
| var gitRoot = await _gitRepository.GetRootAsync(cancellationToken); | |
| if (gitRoot is null) | |
| { | |
| _logger.LogDebug("Not in a git repository, skipping skill file update check"); | |
| return; | |
| } | |
| var skillFilePath = Path.Combine(gitRoot.FullName, s_skillFileRelativePath); | |
| if (!File.Exists(skillFilePath)) | |
| { | |
| _logger.LogDebug("No skill file found at {SkillFilePath}, skipping update check", skillFilePath); | |
| return; | |
| } | |
| // Read existing content and compare with current expected content | |
| var existingContent = await File.ReadAllTextAsync(skillFilePath, cancellationToken); | |
| var normalizedExisting = NormalizeLineEndings(existingContent); | |
| var normalizedExpected = NormalizeLineEndings(CommonAgentApplicators.SkillFileContent); | |
| if (string.Equals(normalizedExisting, normalizedExpected, StringComparison.Ordinal)) | |
| { | |
| _logger.LogDebug("Skill file at {SkillFilePath} is up to date", skillFilePath); | |
| return; | |
| } | |
| // Skill file is outdated, prompt for update | |
| var promptMessage = string.Format( | |
| CultureInfo.CurrentCulture, | |
| UpdateCommandStrings.SkillFileOutdatedPrompt, | |
| s_skillFileRelativePath); | |
| var shouldUpdate = await InteractionService.ConfirmAsync( | |
| promptMessage, | |
| defaultValue: true, | |
| cancellationToken); | |
| if (shouldUpdate) | |
| { | |
| await File.WriteAllTextAsync(skillFilePath, CommonAgentApplicators.SkillFileContent, cancellationToken); | |
| InteractionService.DisplaySuccess(UpdateCommandStrings.SkillFileUpdatedMessage); | |
| _logger.LogInformation("Updated skill file at {SkillFilePath}", skillFilePath); | |
| } | |
| try | |
| { | |
| // Try to discover the git repository root | |
| var gitRoot = await _gitRepository.GetRootAsync(cancellationToken); | |
| if (gitRoot is null) | |
| { | |
| _logger.LogDebug("Not in a git repository, skipping skill file update check"); | |
| return; | |
| } | |
| var skillFilePath = Path.Combine(gitRoot.FullName, s_skillFileRelativePath); | |
| if (!File.Exists(skillFilePath)) | |
| { | |
| _logger.LogDebug("No skill file found at {SkillFilePath}, skipping update check", skillFilePath); | |
| return; | |
| } | |
| // Read existing content and compare with current expected content | |
| var existingContent = await File.ReadAllTextAsync(skillFilePath, cancellationToken); | |
| var normalizedExisting = NormalizeLineEndings(existingContent); | |
| var normalizedExpected = NormalizeLineEndings(CommonAgentApplicators.SkillFileContent); | |
| if (string.Equals(normalizedExisting, normalizedExpected, StringComparison.Ordinal)) | |
| { | |
| _logger.LogDebug("Skill file at {SkillFilePath} is up to date", skillFilePath); | |
| return; | |
| } | |
| // Skill file is outdated, prompt for update | |
| var promptMessage = string.Format( | |
| CultureInfo.CurrentCulture, | |
| UpdateCommandStrings.SkillFileOutdatedPrompt, | |
| s_skillFileRelativePath); | |
| var shouldUpdate = await InteractionService.ConfirmAsync( | |
| promptMessage, | |
| defaultValue: true, | |
| cancellationToken); | |
| if (shouldUpdate) | |
| { | |
| await File.WriteAllTextAsync(skillFilePath, CommonAgentApplicators.SkillFileContent, cancellationToken); | |
| InteractionService.DisplaySuccess(UpdateCommandStrings.SkillFileUpdatedMessage); | |
| _logger.LogInformation("Updated skill file at {SkillFilePath}", skillFilePath); | |
| } | |
| } | |
| catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) | |
| { | |
| throw; | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Failed to check or update Aspire skill files. Continuing without updating skill files."); | |
| } |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #21893791897 |
|
Super slick. |
af7982f to
5fabcee
Compare
- Modified UpdateCommand to detect outdated SKILL.md files dropped by 'aspire agent init' - After updating project packages, checks if a skill file exists in the repository - Compares existing skill file content with the current expected content - Prompts user to update the skill file if it's out of date - Added IGitRepository dependency to find git repository root Tests: - Added 5 unit tests for skill file detection and update functionality - Added E2E CLI test that creates an outdated SKILL.md and verifies prompt appears Test infrastructure: - Added TestGitRepository for mocking git repository operations in tests - Added DisplaySuccessCallback to TestConsoleInteractionService
This reverts commit 102ae86.
- Check all three skill file locations: .github, .opencode, .claude - Prompt for each outdated file separately - Add unit tests for each location and multi-file scenario - Update E2E test to verify all locations are detected
Add support for the agent skills convention (.agents/skills/) which is a universal location recognized by multiple coding agents. Changes: - Add UniversalSkillFileScanner that always offers to create the skill file at .agents/skills/aspire/SKILL.md regardless of detected agents - Update UpdateCommand to check the new skill file path - Add unit tests for the new scanner - Add E2E test to verify agent init offers the universal skill file - Add VerifyFileExists helper method for E2E tests
Add .NotRequired() to MultiSelectionPrompt to allow users to skip agent environment selection and proceed to skill file options. This enables pressing Enter without selecting any items.
00de1af to
bfb4279
Compare
Summary
This PR improves the
aspire updatecommand to detect the presence of SKILL.md files dropped byaspire agent initand prompt the user to update them if they are out of date.Changes
Core Feature
UpdateCommand.csto detect outdated SKILL.md files after updating project packagesIGitRepositorydependency to find the git repository rootCheckAndUpdateSkillFilesAsyncmethod that:.github/skills/aspire/SKILL.mdResource Strings
SkillFileOutdatedPrompt- "The Aspire skill file ({0}) is out of date. Would you like to update it?"SkillFileUpdatedMessage- "Aspire skill file updated successfully."Test Infrastructure
TestGitRepository.cs- Mock implementation ofIGitRepositoryfor unit testsDisplaySuccessCallbacktoTestConsoleInteractionServiceUnit Tests (5 new tests in
UpdateCommandSkillFileTests)UpdateCommand_DetectsOutdatedSkillFile_PromptsForUpdate- Verifies prompt appears for outdated skill fileUpdateCommand_WithOutdatedSkillFile_UpdatesWhenConfirmed- Verifies file is updated when user confirmsUpdateCommand_WithUpToDateSkillFile_DoesNotPrompt- Verifies no prompt when file is currentUpdateCommand_WithNoSkillFile_DoesNotPrompt- Verifies no prompt when no skill file existsUpdateCommand_WhenNotInGitRepo_DoesNotCheckSkillFile- Verifies no check when not in git repoE2E Test
UpdateSkillFileTests.cswith testUpdateCommand_DetectsOutdatedSkillFile_PromptsForUpdateaspire updateTesting