Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 12, 2025

Description

Introduces factory pattern for AppHost execution to prepare for multi-language AppHost support. Extracts ~380 lines of execution logic from RunCommand.ExecuteAsync into dedicated runner classes, reducing method complexity from 280 to 40 lines.

Changes

New abstractions:

  • IAppHostRunner - Execution contract with RunAsync(ParseResult, CancellationToken)
  • IAppHostRunnerFactory - Factory interface with CreateRunner(AppHostRunnerContext)
  • AppHostRunnerContext - Context containing AppHost file and settings file for extensibility
  • LegacyAppHostRunner - Current .NET AppHost execution logic (extracted from RunCommand)
  • AppHostRunnerFactory - Factory implementation, injects all required dependencies

Simplified RunCommand.ExecuteAsync:

// Before: 280 lines handling project location, SDK checks, building, compatibility, 
// backchannel setup, dashboard URLs, logging, resource state, etc.

// After:
var effectiveAppHostFile = await _projectLocator.UseOrFindAppHostProjectFileAsync(...);
var settingsFilePath = ConfigurationHelper.BuildPathToSettingsJsonFile(ExecutionContext.WorkingDirectory.FullName);
var settingsFile = File.Exists(settingsFilePath) ? new FileInfo(settingsFilePath) : null;
var context = new AppHostRunnerContext(effectiveAppHostFile, settingsFile);
var runner = _appHostRunnerFactory.CreateRunner(context);
return await runner.RunAsync(parseResult, cancellationToken);

Behavior preserved:

  • All exception handling, extension host detection, SDK validation unchanged
  • Factory currently creates only LegacyAppHostRunner; structure enables future Python/Node.js runners
  • Context pattern allows for easy expansion with additional properties (environment variables, configuration, etc.) without breaking the factory interface

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • Existing 527 RunCommand tests pass without modification
  • Did you add public API?
    • Yes
    • No
    • All new types are internal
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
    • No
    • Internal refactoring only
Original prompt

I want to do some refactoring of the way that the AppHost is launched from commands like aspire run, and aspire deploy. At the moment the commands like RunCommand have quite intricate code in ExecuteAsync which figures out what kind of apphost is being launched and modifies various configuration elements.

I want to move towards a model where we have a factory pattern where once we determine the path to the apphost, we create an IAppHostRunner - then as we move towards supporting various different languages for the app host itself we can replace the startup logic as necessary.

This will be a multi-stage effort. But the first step is trying to simplify the structure of RunCommand.ExecuteAsync(...).

Introduce an IAppHostRunnerFactory which has a CreateRunner(...) method which takes a FileInfo which represents the app host path.

Create a new interface called IAppHostRunner and a concrete type called LegacyAppHostRunner and lift the existing code in RunCommand.ExecuteAsync into it. The idea is that the ExecuteAsync method in RunCommand will simply call the project locator. Create the runner, and then call IAppHostRunner.RunAsync(...).

Initially the various interfaces that are injected into RunCommand.ExecuteAsync(...) will also need to be injected into AppHostRunnerFactory to maintain behavior but over time the goal is to reduce these direct dependencies.

This is a potentially large refactoring but try to avoid sprawling changes outside of RunCommand.ExecuteAsync and the new types that are being added.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Refactor app host launch logic for command execution Refactor RunCommand to use factory pattern for AppHost runners Nov 12, 2025
Copilot AI requested a review from mitchdenny November 12, 2025 03:43
Copilot finished work on behalf of mitchdenny November 12, 2025 03:43
Address feedback from @davidfowl to pass context instead of just file info.
The context now contains both the AppHost file and the settings file (if it exists).
This allows for future expansion to include additional context information.

Co-authored-by: davidfowl <[email protected]>
Copilot AI changed the title Refactor RunCommand to use factory pattern for AppHost runners Refactor RunCommand to use factory pattern with context for AppHost runners Nov 12, 2025
Copilot AI requested a review from davidfowl November 12, 2025 06:03
Copilot finished work on behalf of davidfowl November 12, 2025 06:03
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12921

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12921"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants