Remove mature feature flags that no longer need toggles#15075
Remove mature feature flags that no longer need toggles#15075mitchdenny merged 3 commits intorelease/13.2from
Conversation
Remove four feature flags that have been stable and always-on (default: true) for months, making their escape-hatch toggle unnecessary: - minimumSdkCheckEnabled: SDK version check always runs (stable 7+ months) - orphanDetectionWithTimestampEnabled: timestamp always set (zero-cost robustness) - runningInstanceDetectionEnabled: instance detection always active (stable since Dec 2025) - packageSearchDiskCachingEnabled: disk caching always used (purely beneficial) The gated behaviors are now unconditional. IFeatures parameters removed from DotNetSdkInstaller, DotNetCliExecutionFactory, and AppHostLauncher where they were only used for the removed flags. Fixes #14966 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15075Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15075" |
|
/deployment-test |
|
🚀 Deployment tests starting on PR #15075... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
There was a problem hiding this comment.
Pull request overview
Removes four mature, default-on CLI feature flags (minimum SDK check, timestamp-based orphan detection, running-instance detection, and package-search disk caching) by making their behaviors unconditional and pruning related plumbing across the CLI, VS Code schemas, and tests.
Changes:
- Removed the four flags from
KnownFeaturesmetadata and the VS Code settings schemas. - Simplified several CLI components by removing
IFeaturesgating and making the previously-flagged behaviors always-on. - Updated CLI unit tests to reflect the removed flags and new constructor signatures.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/DotNetSdkInstallerTests.cs | Updates tests for DotNetSdkInstaller ctor change and removes tests covering the deleted minimum SDK check flag behavior. |
| tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs | Removes a test that asserted behavior tied to the deleted running-instance detection flag. |
| src/Aspire.Cli/KnownFeatures.cs | Deletes the four feature flag definitions and their metadata entries. |
| src/Aspire.Cli/DotNet/DotNetSdkInstaller.cs | Removes IFeatures gating; SDK minimum check is now always performed. |
| src/Aspire.Cli/DotNet/DotNetCliRunner.cs | Removes flag gating so package-search disk caching follows useCache only. |
| src/Aspire.Cli/DotNet/DotNetCliExecutionFactory.cs | Always sets CliProcessStarted env var for orphan detection and removes IFeatures dependency. |
| src/Aspire.Cli/Commands/RunCommand.cs | Always performs running-instance detection/stop logic (previously gated). |
| src/Aspire.Cli/Commands/AppHostLauncher.cs | Always stops existing instances in detached launch mode; removes IFeatures dependency. |
| src/Aspire.Cli/Commands/AddCommand.cs | Always stops running instances before modifying project files (previously gated). |
| extension/schemas/aspire-settings.schema.json | Removes the four settings from the workspace settings schema. |
| extension/schemas/aspire-global-settings.schema.json | Removes the four settings from the global settings schema. |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22877378575 |
| var runningInstanceDetectionEnabled = _features.IsFeatureEnabled(KnownFeatures.RunningInstanceDetectionEnabled, defaultValue: true); | ||
| // Force option kept for backward compatibility but no longer used since prompt was removed | ||
| // var force = runningInstanceDetectionEnabled && parseResult.GetValue<bool>("--force"); | ||
| // var force = parseResult.GetValue<bool>("--force"); |
There was a problem hiding this comment.
Can this whole line be removed?
| bool cacheEnabled = useCache; | ||
| if (cacheEnabled) |
There was a problem hiding this comment.
(nit)
| bool cacheEnabled = useCache; | |
| if (cacheEnabled) | |
| if (useCache) |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 30-second timeout must cover both dotnet new project creation and the agent-init prompt appearing. In CI under load (e.g., KinD cluster running), dotnet new alone can exceed 30 seconds, causing a WaitUntilTimeoutException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove mature feature flags that no longer need toggles Remove four feature flags that have been stable and always-on (default: true) for months, making their escape-hatch toggle unnecessary: - minimumSdkCheckEnabled: SDK version check always runs (stable 7+ months) - orphanDetectionWithTimestampEnabled: timestamp always set (zero-cost robustness) - runningInstanceDetectionEnabled: instance detection always active (stable since Dec 2025) - packageSearchDiskCachingEnabled: disk caching always used (purely beneficial) The gated behaviors are now unconditional. IFeatures parameters removed from DotNetSdkInstaller, DotNetCliExecutionFactory, and AppHostLauncher where they were only used for the removed flags. Fixes #14966 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback: remove dead comment, simplify cache check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Mitch Denny <mitch@mitchdeny.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Audits CLI feature flags per #14966 and removes four mature flags that no longer need user-facing toggles. All four were default-true and served only as emergency escape hatches for stable functionality:
minimumSdkCheckEnabled— SDK version check now always runs (stable 7+ months)orphanDetectionWithTimestampEnabled— Process timestamp always set for orphan detection (zero-cost robustness)runningInstanceDetectionEnabled— Running instance detection always active (stable since Dec 2025)packageSearchDiskCachingEnabled— Disk caching for package search always used (purely beneficial)The gated behaviors are now unconditional.
IFeaturesparameters were removed fromDotNetSdkInstaller,DotNetCliExecutionFactory, andAppHostLauncherwhere they were only used for the removed flags.The remaining 10 feature flags are kept as they still serve active purposes (user preferences, unreleased features, experimental polyglot languages).
Follows the precedent of
dotnetSdkInstallationEnabledremoval in #14811.Fixes #14966
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: