Don't count AppHost builds against CLI startup timeout - #19478
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4577bfec-d13e-49b2-a81e-abaa6ef4303d
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19478Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19478" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Separates AppHost build time from CLI startup timeout accounting and reuses safely prebuilt single-file output.
Changes:
- Starts startup timeout accounting after build preparation.
- Defers detached-launch timeout until backchannel connection.
- Adds unit and CLI E2E regression coverage.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Cli/Commands/RunCommand.cs |
Moves startup timer after build signaling. |
src/Aspire.Cli/Commands/AppHostLauncher.cs |
Delays detached readiness timeout. |
src/Aspire.Cli/Projects/DotNetAppHostProject.cs |
Tracks CLI-built output for reuse. |
src/Aspire.Cli/DotNet/DotNetCliRunner.cs |
Suppresses run hooks during builds. |
tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs |
Tests post-build timeout accounting. |
tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs |
Tests detached long-build behavior. |
tests/Aspire.Cli.Tests/Projects/DotNetAppHostProjectTests.cs |
Updates single-file rebuild expectations. |
tests/Aspire.Cli.Tests/DotNet/DotNetCliRunnerTests.cs |
Tests suppression and --no-build. |
tests/Aspire.Cli.EndToEnd.Tests/StartStopTests.cs |
Adds slow single-file build regression coverage. |
Suppressed comments (1)
src/Aspire.Cli/Commands/RunCommand.cs:378
- This timestamp still precedes builds performed inside
_runner.RunAsync.DotNetAppHostProjectsignals build completion at line 1525, but file-based--no-build/extension-owned paths then forcenoBuild = false, and watch mode also runs with builds enabled. Those builds can therefore still exhaustASPIRE_CLI_START_TIMEOUT, contrary to the new post-build contract. Complete any required build before signalingBuildCompletionSource, or introduce a separate signal emitted when the actual AppHost process starts.
var startupStartTimestamp = _timeProvider.GetTimestamp();
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4577bfec-d13e-49b2-a81e-abaa6ef4303d
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4577bfec-d13e-49b2-a81e-abaa6ef4303d
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Aspire.Cli/Projects/DotNetAppHostProject.cs:1582
BuildCompletionSourceis signaled before this fallback reachesdotnet run, so a file-basedaspire run --no-buildstill startsASPIRE_CLI_START_TIMEOUTbefore the mandatory safety rebuild (noBuild = false). If that rebuild exceeds the configured budget, the command reports the same misleading startup timeout this PR is intended to remove. Move this fallback build into the CLI preparation phase, or add a runner signal that marks its build complete before starting the startup budget.
if (isSingleFileAppHost && !builtByCli)
{
// File-based RunCommand metadata is only safe to reuse when the CLI's pre-build
// generated it with run-hook suppression. Preserve the existing rebuild behavior
// for --no-build and extension-owned builds rather than trusting ambient output.
noBuild = false;
noRestore = false;
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4577bfec-d13e-49b2-a81e-abaa6ef4303d
This comment has been minimized.
This comment has been minimized.
Ensure explicit publish --no-build creates safe current run metadata before local or VS Code extension delegation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4577bfec-d13e-49b2-a81e-abaa6ef4303d
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Exercise fresh --no-build publish and verify edited source produces updated Radius artifacts instead of stale output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4577bfec-d13e-49b2-a81e-abaa6ef4303d
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95635e33-3339-44d7-8dc9-2d8d80e9a4a5
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95635e33-3339-44d7-8dc9-2d8d80e9a4a5
Tests selector2 / 99 PR test projects · 3 PR jobs · 1 advisory-only target, from 22 changed files. Selected PR test projects (2 / 99)
Selected PR jobs (3)
Advisory workflow impact (1)
How these were chosen — grouped by what changed📦 affected project 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Damian Edwards (DamianEdwards)
left a comment
There was a problem hiding this comment.
Reviewed the timeout lifecycle changes and regression coverage; no actionable issues found.
|
Pull request created: #1619
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1619 targeting Updated Note This draft PR needs human review before merging. |
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
Description
Long restores and builds could exhaust
ASPIRE_CLI_START_TIMEOUTbefore the AppHost process started. This causedaspire runand detachedaspire startto report a misleading startup timeout for cold or large solutions.This change:
dotnet run --no-build --file;--no-buildbefore the startup budget begins;--no-build;User-facing usage
The existing configuration now applies only to AppHost startup after build completion:
A required safety build may take longer than fifteen seconds; once it completes, the AppHost receives the full fifteen-second startup budget.
Validation
mainand verified headad625f529ecdd07eb693fe0ecc5ac4f88359951d.13.6.0-pr.19478.gad625f52.aspire runsucceeds when a 20-second file build exceeds a 15-second startup budget;describeandstopsucceeded.aspire start --no-buildsucceeds after a 20-second mandatory safety build with a 15-second budget;describeandstopsucceeded.ASPIRE_CLI_START_TIMEOUTguidance, exits non-zero, and leaves no running AppHost.--no-buildperforms the safe fallback build and executes current source.Fixes #19255
Checklist
<remarks />and<code />elements on your triple slash comments?