Skip to content

Preserve CLI status messages without interactive spinners - #19272

Merged
James Newton-King (JamesNK) merged 2 commits into
microsoft:mainfrom
JamesNK:jamesnk/fix-console-status-review-feedback
Aug 12, 2026
Merged

James Newton-King (JamesNK) merged 2 commits into
microsoft:mainfrom
JamesNK:jamesnk/fix-console-status-review-feedback

Conversation

@JamesNK

Copy link
Copy Markdown
Member

Description

Console logging and debug mode should not use Spectre.Console's interactive status renderer because spinner frames can interleave with diagnostic logs. They should still show status updates as stable, subtle messages so users retain progress context.

This follow-up to #19214 and the review feedback on #19256:

  • Centralizes interactive status eligibility in ShouldShowInteractiveStatus, including the atomic nested-status guard.
  • Restores debug-mode spinner suppression across synchronous, asynchronous, and dynamic status methods.
  • Keeps status messages visible when console logging is enabled while suppressing spinner rendering.
  • Makes the doctor regression recording resilient to stale files and validates debug logging explicitly.

User-facing usage

Running doctor with console debug logging keeps readable status messages without interactive spinner frames:

aspire doctor --log-level debug

Screenshots / Recordings

This PR includes UI changes. Please add screenshots or screen recordings so reviewers can evaluate the visual changes without running locally.

  • For before/after comparisons, place them side-by-side or label them clearly.
  • For interactive changes (animations, transitions, new flows), prefer a short screen recording (GIF or video).
  • If you cannot capture visuals now, note what scenario to test and mark this section as TODO.

TODO: Run aspire doctor --log-level debug in an interactive terminal and confirm status messages remain visible without spinner glyphs. Local archive creation was blocked by unrelated existing Dashboard package path-length errors (NU5123).

Validation

  • ConsoleInteractionServiceTests and SpectreConsoleLoggerProviderTests: 119 passed.
  • Aspire.Cli.EndToEnd.Tests project build passed.
  • git diff --check passed.

Fixes # (issue)

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
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

@github-actions

Copy link
Copy Markdown
Contributor

🚀 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/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19272

Or

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Preserves CLI status messages while suppressing interactive spinners during debug or console logging.

Changes:

  • Centralizes spinner eligibility and nested-status guarding.
  • Displays stable status updates in fallback modes.
  • Expands unit and doctor end-to-end regression coverage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/Aspire.Cli/Interaction/ConsoleInteractionService.cs Centralizes interactive-status selection.
tests/Aspire.Cli.Tests/Interaction/ConsoleInteractionServiceTests.cs Tests fallback status behavior.
tests/Aspire.Cli.Tests/Interaction/SpectreConsoleLoggerProviderTests.cs Verifies trace filtering.
tests/Aspire.Cli.EndToEnd.Tests/DoctorCommandTests.cs Validates debug output without spinner glyphs.

Comment thread src/Aspire.Cli/Interaction/ConsoleInteractionService.cs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blocking output regression found: console logging now duplicates status messages because the fallback both logs and renders them directly. Focused CLI unit tests pass, but the current combined-output assertions do not detect the duplication.

Comment thread src/Aspire.Cli/Interaction/ConsoleInteractionService.cs
Copilot AI review requested due to automatic review settings August 12, 2026 03:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tests/Aspire.Cli.Tests/Interaction/SpectreConsoleLoggerProviderTests.cs:36

  • This expectation conflicts with the CLI contract: RootCommandStrings.DebugLevelArgumentDescription advertises Trace as a console log level, and Program.CreateLoggerFactory configures the Aspire.Cli filter to the requested level. Because SpectreConsoleLogger.IsEnabled currently rejects Trace, --log-level trace silently omits all trace messages; asserting false locks in that bug. Permit trace for Aspire CLI categories and expect true here.
        Assert.False(aspireLogger.IsEnabled(LogLevel.Trace));

Copilot AI review requested due to automatic review settings August 12, 2026 04:17
@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/fix-console-status-review-feedback branch from 7848bbc to b5005c3 Compare August 12, 2026 04:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@mitchdenny Mitch Denny (mitchdenny) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this direction. We'll want to fix the double printing of status messages when console logging is enabled, where the status can be emitted through both the logger and the fallback display path.

@JamesNK
James Newton-King (JamesNK) merged commit fa20d28 into microsoft:main Aug 12, 2026
1073 of 1080 checks passed
@github-actions github-actions Bot added this to the 13.6 milestone Aug 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 CI Failure Analysis: Transient Infrastructure Failure

The CI build failed due to transient infrastructure issues.

Failed jobs:

  • Tests / Hosting.Oracle / Hosting.Oracle (ubuntu-latest) — The 'Checkout code' step failed with git exit code 128 due to 'server certificate verification failed' when accessing github.com. This is a runner-side SSL/network issue unrelated to any PR change; the checkout step runs before any PR code is compiled or tested. (transient-infra)

If a rerun was not already requested automatically, visit the workflow run page to rerun the failed jobs manually.

Jose Perez Rodriguez (joperezr) pushed a commit that referenced this pull request Aug 12, 2026
…19256)

* Suppress CLI progress when console logging is enabled

* Address console logging review feedback

* Preserve CLI status messages without interactive spinners (#19272)

---------

Co-authored-by: James Newton-King <james@newtonking.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants