Skip to content

Add pinning to Dashboard run history - #18925

Closed
James Newton-King (JamesNK) wants to merge 1 commit into
jamesnk/dashboard-run-historyfrom
jamesnk/dashboard-run-history-pinning
Closed

Add pinning to Dashboard run history#18925
James Newton-King (JamesNK) wants to merge 1 commit into
jamesnk/dashboard-run-historyfrom
jamesnk/dashboard-run-history-pinning

Conversation

@JamesNK

@JamesNK James Newton-King (JamesNK) commented Jul 29, 2026

Copy link
Copy Markdown
Member

Description

Stacked on #18924.

Dashboard run history can grow difficult to manage when an important run would otherwise age out. This change adds pin controls to the run selector so current and historical runs can be preserved directly from the menu.

The pin action stays hidden until hover unless selected, updates without closing the menu, and persists in run.json. Pinned runs sort ahead of unpinned history and do not count toward the five retained historical runs.

User-facing usage

Open the Dashboard run selector and use the pin action beside a run. Pinned runs remain available across Dashboard restarts and can be unpinned from the same menu.

Screenshots / Recordings

image

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

@adamint Adam Ratzman (adamint) 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.

I don't think this is ready yet. I found a few blocking paths:

  1. PruneRuns decides that a run is unpinned before taking its lock, then never rechecks after the lock is acquired. I reproduced a pin writing IsPinned=true after pruning had started and the same directory was still deleted. The metadata check needs to happen under the run lock, and the metadata write should be atomic so a partial run.json isn't interpreted as unpinned.
  2. A normal pin failure terminates the Blazor circuit. With two Dashboards on the same data root, selecting a historical run in B and pinning it in A throws InvalidOperationException from SetRunPinned, shows #blazor-error-ui, and requires a reload. Could this degrade gracefully, refresh the run list, and surface a nonfatal error instead?
  3. Unpinned actions use visibility: hidden until pointer hover. That removes the pin button from keyboard navigation and leaves no usable action on touch. It needs a focus/touch-accessible presentation, not hover-only visibility.
  4. The live DOM renders pinned aria-pressed as "" and omits it for unpinned items. ARIA needs literal true/false state here.
  5. DashboardRunDescriptor.IsPinned and IsLeased are public setters, so callers can bypass the store/lock/metadata invariants. I think these should be read-only publicly and mutated only by the store.

Also, the parent persistence spec still promises ten retained runs, while this changes the contract to five unpinned historical runs plus pinned runs. If five is intentional, please update the spec in this PR.

@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history branch from d8a767b to d3544e5 Compare July 31, 2026 07:31
Copilot AI balanced review requested due to automatic review settings July 31, 2026 07:31
@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history-pinning branch from 9d2b346 to 4c85cdd Compare July 31, 2026 07:31
@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 -- 18925

Or

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

@github-actions

This comment has been minimized.

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

Adds persistent pinning for Dashboard runs so important history survives restarts and retention pruning.

Changes:

  • Persists pin state in run metadata and excludes pinned runs from retention limits.
  • Adds accessible pin controls and pinned-first sorting to the run selector.
  • Adds persistence, retention, leasing, and component tests.

Reviewed changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/Aspire.Dashboard.Tests/Shared/TestDashboardDataSource.cs Updates the test run store.
tests/Aspire.Dashboard.Tests/Model/DashboardDataSourceTests.cs Tests pin persistence and retention.
tests/Aspire.Dashboard.Tests/Integration/StartupTests.cs Adapts run lookup.
tests/Aspire.Dashboard.Components.Tests/Shared/FluentUISetupHelpers.cs Adds pinning to component fakes.
tests/Aspire.Dashboard.Components.Tests/Layout/MainLayoutTests.cs Tests pin interactions and sorting.
tests/Aspire.Dashboard.Components.Tests/Controls/AspireMenuTests.cs Tests in-place menu refresh.
src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs Implements persisted pinning and retention.
src/Aspire.Dashboard/ServiceClient/DashboardDataSourcePool.cs Adapts dictionary-based run lookup.
src/Aspire.Dashboard/ServiceClient/DashboardDataSource.cs Adapts run selection lookup.
src/Aspire.Dashboard/Resources/xlf/Layout.zh-Hant.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.zh-Hans.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.tr.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.ru.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.pt-BR.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.pl.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.ko.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.ja.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.it.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.fr.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.es.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.de.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/xlf/Layout.cs.xlf Adds pin localization entries.
src/Aspire.Dashboard/Resources/Layout.resx Defines pin and unpin strings.
src/Aspire.Dashboard/Resources/Layout.Designer.cs Exposes generated localization properties.
src/Aspire.Dashboard/Model/MenuButtonItem.cs Adds secondary menu actions.
src/Aspire.Dashboard/Components/Controls/DashboardRunSelect.razor.cs Adds pin commands and sorting.
src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.css Styles secondary actions.
src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs Refreshes menus after secondary actions.
src/Aspire.Dashboard/Components/Controls/AspireMenu.razor Renders secondary action buttons.
Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file

Comment thread src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.css
Comment thread src/Aspire.Dashboard/Components/Controls/DashboardRunSelect.razor.cs Outdated
Comment thread src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs Outdated
Comment thread src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs Outdated
Comment thread src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs
@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.

Copilot AI review requested due to automatic review settings July 31, 2026 08:48
@JamesNK

Copy link
Copy Markdown
Member Author

Resolved the outstanding review issues in 8bfb250: keyboard/touch access to pin actions, pinning failure handling, descriptor mutation ownership, XML parameter documentation, and the menu grid layout. Added regression coverage for pinning failures; 67 focused Dashboard tests pass.

@github-actions

This comment has been minimized.

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 29 out of 30 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file
Suppressed comments (3)

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:362

  • DashboardDataSourcePool.InitializeAsync evaluates GetRuns() through Current before PublishRun() calls this pruning code, so _runs is already cached when these directories are deleted. The cached dictionary therefore continues exposing descriptors for pruned runs; the selector displays them, but selecting or pinning them fails because their directories no longer exist. Remove pruned entries from the cached state or defer run discovery until pruning has completed.
            .Where(directory => !IsPinnedRunDirectory(directory))
            .OrderByDescending(Path.GetFileName, StringComparer.Ordinal)
            .Skip(MaxHistoricalRuns);

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:576

  • IsPinned cannot be initialized outside this assembly: it is absent from the public constructor and its setter is internal. Because both DashboardRunDescriptor and IDashboardRunStore are public, an external store implementation has no supported way to return a pinned descriptor, so it cannot implement the new contract correctly. Make pinned state part of the descriptor's publicly constructible immutable state.
    /// <summary>
    /// Gets a value indicating whether the dashboard run is pinned.
    /// </summary>
    public bool IsPinned { get; internal set; }

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:40

  • This places a focusable button inside a menuitemradio. The Fluent menu's arrow-key navigation focuses the menu item, not the nested button, while Tab leaves the menu, so keyboard users cannot invoke the new pin action. Expose pin/unpin with valid menu semantics and an explicit keyboard interaction, then cover that interaction in a browser test.
                        <FluentButton Class="@($"aspire-menu-secondary-action{(item.IsSecondaryActionSelected ? " selected" : null)}")"
                                      Appearance="Appearance.Stealth"
                                      Title="@item.SecondaryActionAriaLabel"
                                      AriaLabel="@item.SecondaryActionAriaLabel"
                                      aria-pressed="@item.IsSecondaryActionSelected"
                                      @onclick="() => HandleSecondaryActionClicked(item)">

@adamint Adam Ratzman (adamint) 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.

The latest commit fixed the pin-failure circuit, keyboard/touch visibility, and public setters, but I still don't think this is ready:

  • The prune-vs-pin race and non-atomic run.json write are unchanged. The real-path tester reproduced a run being pinned and then deleted by pruning; the pinned decision still happens before the run lock and read failures still fail open as unpinned.
  • aria-pressed is still passed as a bool, so the live DOM renders it valueless for pinned items and omits it for unpinned items instead of emitting literal true/false.
  • In real Chromium, activating the nested pin action still reaches the underlying FAST menuitemradio. Mouse activation can leave two checked runs across the divider, and Enter on the keyboard pin action also switches runs and closes the menu. The current bUnit tests don't execute FAST, so they can't catch this.
  • Run ordering now has two conflicting owners: the store sorts pinned-first into a dictionary while the selector sorts current-first. The persistence spec still describes the old retention/order contract.

32 component tests and 84 store tests pass, but the browser and concurrency paths above remain blocking.

Copilot AI review requested due to automatic review settings August 1, 2026 06:15
@JamesNK

Copy link
Copy Markdown
Member Author

Addressed the follow-up feedback from #18925 (review) in c727d86290 and f005fd121c:

  • Pruning now rechecks the run's pinned state after acquiring the per-run lock and holds that lock through deletion, closing the prune-vs-pin race. I intentionally kept direct run.json writes: this is locally written metadata that is assumed to be reliable during normal usage, while unreadable metadata remains fail-open so incomplete or abandoned directories can still be pruned.
  • aria-pressed now renders literal "true" / "false" values, with component coverage for both states.
  • I couldn't reproduce the reported mouse activation behavior in Chrome; click propagation was already stopped on the secondary-action container. I added keydown propagation handling to that same container so Enter on the pin action doesn't reach the containing menuitemradio.
  • Updated the persistence spec to document the current ordering layers: discovery is pinned-first, while the selector presents current-first, then pinned and unpinned historical runs. It now also documents retention as five unpinned historical runs, excluding current and pinned runs, with locked runs able to temporarily exceed the limit.

Validation: Aspire.Dashboard.Tests passed 1,872 tests and Aspire.Dashboard.Components.Tests passed 223 tests, excluding quarantined and outerloop tests.

@github-actions

This comment has been minimized.

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 30 out of 31 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file
Suppressed comments (3)

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:40

  • A disabled menu item still renders an enabled secondary button, so hovering a disabled item allows its action to run despite IsDisabled. Propagate the disabled state to the nested button as well.
                        <FluentButton Class="@($"aspire-menu-secondary-action{(item.IsSecondaryActionSelected ? " selected" : null)}")"

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:40

  • The core requirement that the secondary action updates without closing/selecting the Fluent menu is only covered by bUnit, which does not execute the Fluent custom element's browser JavaScript or shadow-DOM event handling. Add a Playwright scenario that clicks the real pin button and verifies the menu stays open and the selected run does not change; the existing AspireMenuButtonFocusTests demonstrates this repository's browser-level menu coverage.
                                      @onclick="() => HandleSecondaryActionClicked(item)">

src/Aspire.Dashboard/Model/MenuButtonItem.cs:18

  • These newly added public members are undocumented. Document the secondary action's icon, accessible label, callback, and selected/pressed state so the public model's behavior is clear to consumers.
    public Icon? SecondaryActionIcon { get; set; }
    public string? SecondaryActionAriaLabel { get; set; }
    public Func<Task>? OnSecondaryActionClick { get; set; }
    public bool IsSecondaryActionSelected { get; set; }

Comment thread src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs
@github-actions

github-actions Bot commented Aug 1, 2026

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.

Copilot AI review requested due to automatic review settings August 3, 2026 15:13
@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history-pinning branch from f005fd1 to 65b4a0a Compare August 3, 2026 15:13
@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history-pinning branch from e9935c6 to 6d860d9 Compare August 6, 2026 02:42
Copilot AI review requested due to automatic review settings August 6, 2026 02:42
@github-actions

This comment has been minimized.

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 31 out of 32 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file
Suppressed comments (5)

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:241

  • This overwrites run.json in place even though WriteMetadata below deliberately uses a sibling temp file plus rename to prevent truncated metadata. If the pin write is interrupted or runs out of space, the run can become unreadable on restart and then be treated as unpinned/prunable—the opposite of the preservation guarantee. Refactor the atomic writer to accept the target metadata path and use it here as well.
        File.WriteAllText(metadataPath, JsonSerializer.Serialize(updatedMetadata, s_jsonOptions));

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:627

  • These mutable auto-properties participate in the synthesized equality and hash code of the public record. Pinning or leasing therefore changes an existing descriptor's hash, which can make it unreachable in a consumer's HashSet or keyed collection. Keep mutable runtime state outside record value equality (for example, use immutable replacement descriptors or a reference-identity class).
    public bool IsPinned { get; internal set; }

    internal bool IsLeased { get; set; }

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:625

  • IDashboardRunStore is public, but an implementation outside this assembly cannot construct a pinned descriptor or update one because IsPinned is not a constructor parameter and its setter is internal. That makes the newly added pinning contract unusable for external implementations. Expose pin state through an immutable public construction path and redesign SetRunPinned to return/update state without relying on assembly-internal mutation, or keep this store abstraction internal.

This issue also appears on line 625 of the same file.

    public bool IsPinned { get; internal set; }

src/Aspire.Dashboard/Components/Controls/DashboardRunSelect.razor.cs:80

  • After pinning, this only mutates the existing item's icon and selected state. The open menu's item array is not rebuilt, so a newly pinned historical run stays among the unpinned runs until the menu is closed and reopened, despite the promised pinned-first ordering. Refresh the parent ItemsProvider result (or otherwise reorder the open item collection) after a successful toggle while keeping the menu open.
            menuItem.OnSecondaryActionClick = () =>
            {
                SetRunPinned(run, !run.IsPinned);
                menuItem.SecondaryActionIcon = run.IsPinned ? s_pinnedIcon : s_pinIcon;
                menuItem.SecondaryActionAriaLabel = Loc[run.IsPinned
                    ? nameof(LayoutResources.DashboardRunSelectUnpin)
                    : nameof(LayoutResources.DashboardRunSelectPin)];
                menuItem.IsSecondaryActionSelected = run.IsPinned;

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:48

  • The new action is hidden by CSS and nested inside a Fluent menu item, so keyboard reachability, focus behavior, and activation without selecting/closing the parent are browser-only behaviors. Current coverage only invokes .Click() through bUnit. Add a Playwright test that opens the run menu with the keyboard, reaches and reveals the pin action, toggles it, and verifies the menu/focus remain usable; the existing AspireMenuButtonFocusTests establishes this repository pattern.
                    <span slot="end" @onclick:stopPropagation="true" @onkeydown:stopPropagation="true">
                        <FluentButton Class="@($"aspire-menu-secondary-action{(item.IsSecondaryActionSelected ? " selected" : null)}")"
                                      Appearance="Appearance.Stealth"
                                      Title="@item.SecondaryActionAriaLabel"
                                      AriaLabel="@item.SecondaryActionAriaLabel"
                                      aria-pressed="@(item.IsSecondaryActionSelected ? "true" : "false")"
                                      @onclick="() => HandleSecondaryActionClicked(item)">

Copilot AI review requested due to automatic review settings August 6, 2026 03:07
@github-actions

This comment has been minimized.

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 32 out of 33 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file
Suppressed comments (2)

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:241

  • This overwrites run.json in place, although WriteMetadata below deliberately uses a sibling temporary file plus atomic rename to prevent crashes or power loss from truncating this same metadata. A failure during pin/unpin can make the run undiscoverable after restart and allow retention to prune a run the user intended to preserve. Route pin updates through a path-aware atomic metadata writer as well.
        var updatedMetadata = metadata with { IsPinned = isPinned };
        File.WriteAllText(metadataPath, JsonSerializer.Serialize(updatedMetadata, s_jsonOptions));

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:48

  • The secondary action's actual hover visibility, keyboard focus/activation, event propagation, and FluentMenu refresh behavior are browser-only, but the added coverage is limited to bUnit. Existing AspireMenuButtonFocusTests explicitly uses Playwright because FluentMenu focus behavior cannot be observed in bUnit. Add a Playwright test that reaches this button by keyboard, pins/unpins it, verifies aria-pressed, and confirms the menu remains open.
                    <span slot="end" @onclick:stopPropagation="true" @onkeydown:stopPropagation="true">
                        <FluentButton Class="@($"aspire-menu-secondary-action{(item.IsSecondaryActionSelected ? " selected" : null)}")"
                                      Appearance="Appearance.Stealth"
                                      Title="@item.SecondaryActionAriaLabel"
                                      AriaLabel="@item.SecondaryActionAriaLabel"
                                      aria-pressed="@(item.IsSecondaryActionSelected ? "true" : "false")"
                                      @onclick="() => HandleSecondaryActionClicked(item)">

Comment on lines +625 to +627
public bool IsPinned { get; internal set; }

internal bool IsLeased { get; set; }
@github-actions

github-actions Bot commented Aug 6, 2026

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.

@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history-pinning branch from b595ef3 to 129ef90 Compare August 6, 2026 08:41
Copilot AI review requested due to automatic review settings August 6, 2026 08:41

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 6, 2026 09:37
@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history-pinning branch from 129ef90 to 08c1b05 Compare August 6, 2026 09:37
@github-actions

This comment has been minimized.

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 31 out of 32 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file
Suppressed comments (7)

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:266

  • This overwrites run.json in place, bypassing the crash-safe temp-file-and-rename logic in WriteMetadata below. A process crash or power loss during a pin action can truncate the metadata, making the important pinned run undiscoverable and eligible for deletion. Use the same atomic writer for both current and historical run metadata.
        File.WriteAllText(metadataPath, JsonSerializer.Serialize(updatedMetadata, s_jsonOptions));

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:426

  • The snapshot was originally sorted using the old IsPinned values, but pin state is mutable while pruning runs after startup. If an old pinned run is unpinned before this enumeration, it can remain ahead of newer unpinned runs and cause a newer run to be deleted. Re-sort the filtered historical candidates by StartedAtUtc immediately before applying the retention limit.
        foreach (var run in _runs.Value.Where(run => !run.IsPinned).Skip(MaxRuns))

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:47

  • This exposes a toggle via aria-pressed, but the accessible name changes from “Pin run” to “Unpin run” when pressed. Toggle-button names need to remain stable; otherwise assistive technology announces the confusing state “Unpin run, pressed.” Either keep a stable accessible name and use aria-pressed, or retain the action-specific names and remove aria-pressed.
                                      Title="@item.SecondaryActionAriaLabel"
                                      AriaLabel="@item.SecondaryActionAriaLabel"
                                      aria-pressed="@(item.IsSecondaryActionSelected ? "true" : "false")"

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:426

  • This does not implement the documented five-historical-run limit. _runs includes the current run and MaxRuns is still 10, so this keeps nine unpinned historical runs when the current run is unpinned, and ten when it is pinned. Filter out the current run and apply a limit of five to the remaining unpinned history.

This issue also appears on line 426 of the same file.

        foreach (var run in _runs.Value.Where(run => !run.IsPinned).Skip(MaxRuns))

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:44

  • A disabled MenuButtonItem still renders an enabled secondary button, so its action remains clickable even though the menu item is disabled. Propagate item.IsDisabled to the nested button.

This issue also appears on line 45 of the same file.

                                      Appearance="Appearance.Stealth"

src/Aspire.Dashboard/Model/MenuButtonItem.cs:18

  • These new public API members have no XML documentation. Add summaries describing the icon, accessible label, callback, and selected-state behavior so the API is usable from IntelliSense and meets the repository's public API documentation requirements.
    public Icon? SecondaryActionIcon { get; set; }
    public string? SecondaryActionAriaLabel { get; set; }
    public Func<Task>? OnSecondaryActionClick { get; set; }
    public bool IsSecondaryActionSelected { get; set; }

docs/specs/dashboard-persistence.md:165

  • This retention update leaves the database-size section at line 300 stating that Run retains ten databases. Pinned runs now make the total unbounded and the intended unpinned-history limit is five, so that operational guidance also needs updating.
`Run` mode retains the five newest unpinned historical run directories. The current run and pinned historical runs do not count toward this limit, so the total number of retained runs is not fixed. Pruning happens after a new run writes its metadata. Before deleting a candidate, the pruner acquires its lock and rechecks its pin state. A historical run selected by another Dashboard circuit or a run owned by another Dashboard process is skipped, which can temporarily leave more than five unpinned historical runs. I/O and access failures are logged and do not prevent Dashboard startup.

@github-actions

github-actions Bot commented Aug 6, 2026

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.

Copilot AI review requested due to automatic review settings August 6, 2026 15:21
@JamesNK
James Newton-King (JamesNK) force-pushed the jamesnk/dashboard-run-history-pinning branch from 08c1b05 to 5a91d92 Compare August 6, 2026 15:21
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Tests selector (audit mode)

The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement.

7 / 100 test projects · 4 jobs, from 31 changed files.

Selected test projects (7 / 100)

Aspire.Cli.EndToEnd.Tests, Aspire.Dashboard.Components.Tests, Aspire.Dashboard.Tests, Aspire.Hosting.DotnetTool.Tests, Aspire.Hosting.RemoteHost.Tests, Aspire.Managed.Tests, Aspire.Playground.Tests

Selected jobs (4)

cli-starter, deployment-e2e, extension-e2e, polyglot


How these were chosen — grouped by what changed

🔧 src/Aspire.Dashboard/Components/Controls/AspireMenu.razor (changed source)
4 via the project graph: Aspire.Hosting.DotnetTool.Tests (2 hops), Aspire.Hosting.RemoteHost.Tests (2 hops), Aspire.Managed.Tests (2 hops), Aspire.Playground.Tests (2 hops)

📦 affected project Aspire.Managed
1 test: Aspire.Cli.EndToEnd.Tests

🔧 src/Aspire.Dashboard/wwwroot/css/app.css (changed source)
1 via the project graph: Aspire.Dashboard.Tests

🧪 tests/Aspire.Dashboard.Components.Tests/Controls/AspireMenuTests.cs (changed test)
1 directly: Aspire.Dashboard.Components.Tests

🧪 tests/Aspire.Dashboard.Components.Tests/Layout/MainLayoutTests.cs (changed test)
1 directly: Aspire.Dashboard.Components.Tests

🧪 tests/Aspire.Dashboard.Components.Tests/Shared/FluentUISetupHelpers.cs (changed test)
1 directly: Aspire.Dashboard.Components.Tests

🧪 tests/Aspire.Dashboard.Tests/Model/DashboardDataSourceTests.cs (changed test)
1 directly: Aspire.Dashboard.Tests

🧪 tests/Aspire.Dashboard.Tests/Shared/TestDashboardDataSource.cs (changed test)
1 directly: Aspire.Dashboard.Tests

Job reasons

Job Triggered by
cli-starter • affected project Aspire.Managed
• selected test Aspire.Cli.EndToEnd.Tests
deployment-e2e affected project Aspire.Managed
extension-e2e src/Aspire.Dashboard/Components/Controls/AspireMenu.razor, src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs, src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.css, src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor, src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs, src/Aspire.Dashboard/Components/Controls/DashboardRunSelect.razor.cs, src/Aspire.Dashboard/Model/MenuButtonItem.cs, src/Aspire.Dashboard/Resources/Layout.Designer.cs, src/Aspire.Dashboard/Resources/Layout.resx, src/Aspire.Dashboard/Resources/xlf/Layout.cs.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.de.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.es.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.fr.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.it.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.ja.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.ko.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.pl.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.pt-BR.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.ru.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.tr.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.zh-Hans.xlf, src/Aspire.Dashboard/Resources/xlf/Layout.zh-Hant.xlf, src/Aspire.Dashboard/ServiceClient/DashboardDataSource.cs, src/Aspire.Dashboard/ServiceClient/DashboardDataSourcePool.cs, src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs, src/Aspire.Dashboard/wwwroot/css/app.css
• affected project Aspire.Dashboard
polyglot affected project Aspire.Managed

Selection computed for commit 5a91d92.

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 31 out of 32 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • src/Aspire.Dashboard/Resources/Layout.Designer.cs: Generated file
Suppressed comments (5)

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:266

  • This bypasses WriteMetadata's atomic temp-file-and-rename path. A crash or power loss during pinning can therefore truncate run.json, after which discovery drops the entire run—including the run the user was trying to preserve. Generalize the atomic writer to accept the target metadata path and use it here as well.
        File.WriteAllText(metadataPath, JsonSerializer.Serialize(updatedMetadata, s_jsonOptions));

src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs:426

  • MaxRuns is still 10, and the current run is included before Skip, so this retains nine unpinned historical runs. That contradicts both the PR description and the updated specification, which promise five. Use an explicit historical limit of 5, filter out current and pinned runs before applying it, and update the tests that currently assert MaxRuns - 1.
        foreach (var run in _runs.Value.Where(run => !run.IsPinned).Skip(MaxRuns))

docs/specs/dashboard-persistence.md:163

  • This description does not match LoadRuns, which orders IsCurrent before IsPinned; an unpinned current run is still first. Describe discovery as current-first, followed by pinned and unpinned history.
Run discovery orders pinned runs before unpinned runs, then orders each group by descending start time. The run selector applies its presentation order separately: the current run is first, followed by pinned historical runs and then unpinned historical runs, with each historical group ordered by descending start time. Pin state is stored in `run.json`; both current and historical runs can be pinned or unpinned.

src/Aspire.Dashboard/Components/Controls/AspireMenu.razor:42

  • The new nested action depends on real-browser custom-element event propagation, CSS visibility, and focus behavior while pinning can reorder the open menu. The bUnit tests synthesize clicks but cannot verify that keyboard users can reach the hidden action or retain usable focus with the menu still open afterward. Add focused Playwright coverage; AspireMenuButtonFocusTests.cs already establishes browser testing for this menu focus contract.
                    <span slot="end" @onclick:stopPropagation="true" @onkeydown:stopPropagation="true">

src/Aspire.Dashboard/Model/MenuButtonItem.cs:18

  • These four members are new public API but have no XML documentation. Document the secondary action's icon/accessible label, when the callback is invoked, and how IsSecondaryActionSelected affects its toggle state so the public contract is usable from IntelliSense.
    public Icon? SecondaryActionIcon { get; set; }
    public string? SecondaryActionAriaLabel { get; set; }
    public Func<Task>? OnSecondaryActionClick { get; set; }
    public bool IsSecondaryActionSelected { get; set; }

@JamesNK

Copy link
Copy Markdown
Member Author

Merged into parent.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants