Eagerly migrate legacy .aspire/settings.json to aspire.config.json on CLI startup#17234
Merged
mitchdenny merged 4 commits intoMay 19, 2026
Merged
Conversation
Fixes #15488. When the CLI runs against an existing AppHost workspace that only has the legacy .aspire/settings.json, ConfigurationHelper. RegisterSettingsFiles now migrates it to aspire.config.json at startup via AspireConfigFile.LoadOrCreate, mirroring the existing global-config migration in Program.GetGlobalSettingsPath. Previously, migration only happened when a command actively wrote settings (createSettingsFile: true in ProjectLocator), so read-only commands such as 'aspire doctor', 'aspire ls', or 'aspire --version' left workspaces stuck on the legacy layout indefinitely. A LegacySettingsFileHasMigratableData guard requires the legacy file to carry an appHostPath before migrating, so empty seed files (e.g. the {} marker that TemporaryWorkspace writes to stop directory walks) do not produce spurious aspire.config.json files. Migration failures fall back to registering the legacy path so the CLI still starts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17234Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17234" |
Plumb the startup ILogger from Program.BuildApplicationAsync through ConfigurationHelper.RegisterSettingsFiles so the eager legacy migration emits an Information entry on success and a Warning (with exception) on failure. Mirrors the logging pattern used by GetGlobalSettingsPath for the global-config migration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ensures legacy workspaces that still only have .aspire/settings.json are migrated to aspire.config.json during CLI startup (via ConfigurationHelper.RegisterSettingsFiles), so even read-only commands advance the workspace layout and avoid getting “stuck” on the legacy format.
Changes:
- Eagerly migrates legacy
.aspire/settings.jsontoaspire.config.jsonon startup when the legacy file appears to contain real AppHost data. - Adds best-effort migration with logging + fallback to using the legacy file when migration can’t be performed.
- Adds unit tests covering eager migration, no-clobber behavior when
aspire.config.jsonalready exists, and malformed legacy JSON behavior.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs | Adds tests validating eager migration, preservation of existing aspire.config.json, and malformed-legacy fallback behavior. |
| src/Aspire.Cli/Utils/ConfigurationHelper.cs | Implements eager legacy-to-modern config migration during settings-file registration, gated by an appHostPath presence check and with best-effort error handling/logging. |
| src/Aspire.Cli/Program.cs | Passes the startup logger into RegisterSettingsFiles to enable migration diagnostics. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
JamesNK
reviewed
May 19, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Clean implementation — the guard + try/catch fallback pattern handles all edge cases well. 1 minor comment on a test description.
The existing RegisterSettingsFiles_FallsBackToLegacyWhenMigrationFails test used unparseable JSON, which is rejected by LegacySettingsFileHasMigratableData before migration is attempted, so it never actually entered the try/catch in RegisterSettingsFiles. The test still has value (it proves unparseable legacy files don't crash the CLI), but its comments misdescribed the mechanism. Renamed it to RegisterSettingsFiles_GuardRejectsUnparseableLegacyFile and corrected its comments to describe what it actually exercises: the guard's defense against unparseable JSON. Added RegisterSettingsFiles_FallsBackToLegacyWhenMigrationLoadThrows which uses a parseable legacy file with a valid appHostPath (so the guard returns true) but a string value for the strict Dictionary<string, bool> features field. That makes AspireConfigFile.LoadOrCreate -> AspireJsonConfiguration.Load throw inside the try block, genuinely exercising the catch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
reviewed
May 19, 2026
JamesNK
approved these changes
May 19, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Reviewed the migration logic, guard implementation, error handling, and test coverage. The two-layer defense (guard rejects unparseable files, catch block handles migration failures) ensures the CLI never crashes on startup. Loop ordering correctly prevents re-migration when aspire.config.json already exists. LGTM.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
❓ CLI E2E Tests unknown — 90 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26077806387 |
Contributor
|
See the workflow run for details: https://github.com/microsoft/aspire/actions/runs/26078776570 Added a bullet to the CLI quality-of-life section of
|
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
When a user upgrades to CLI 13.2+ and runs it against an existing AppHost workspace that only has the legacy
.aspire/settings.json, the workspace was never migrated toaspire.config.jsonunless they ran a command that actively wrote settings (run,add,update,pipeline). Read-only commands likeaspire doctor,aspire ls, oraspire --versionleft the workspace stuck on the legacy layout indefinitely, becauseProjectLocatoronly triggers migration when called withcreateSettingsFile: true.This change moves migration up to the single CLI startup chokepoint,
ConfigurationHelper.RegisterSettingsFiles, so it runs unconditionally for any command. The implementation mirrors the existing global-config migration inProgram.GetGlobalSettingsPathand reusesAspireConfigFile.LoadOrCreate, which already contains the legacy-read / new-write logic.A
LegacySettingsFileHasMigratableDataguard prevents spurious migrations: it requires the legacy file to contain anappHostPathbefore migrating. This avoids materializing emptyaspire.config.jsonfiles alongside the{}directory-walk stop markers that test infrastructure (and some user setups) leave behind. The check usesJsonDocumentdirectly rather than the strict-typedAspireJsonConfigurationdeserializer so loosely-typed values in pre-normalized files (e.g. a string"false"inside aDictionary<string, bool>) don't throw and skip migration. Migration failures fall back to registering the legacy path so the CLI still starts.Three new tests cover the behavior: the original repro (legacy-only workspace migrates on startup), no-clobber (existing
aspire.config.jsonis preserved), and graceful fallback on malformed JSON.Fixes: #15488
Checklist
<remarks />and<code />elements on your triple slash comments?