Various Publish Profile Fixes Plus Multi-Profile Publish operation - #257
Various Publish Profile Fixes Plus Multi-Profile Publish operation#257aritchie wants to merge 2 commits into
Conversation
Add a batch publish flow for publish profiles, including selection mode in the secrets UI and a new modal that re-resolves and publishes each profile sequentially. The publish profile service now supports progressive refresh/loading, avoids redundant provider reads, and handles cache invalidation during sync without returning an empty list. Also reposition secret provider popovers while open so they stay aligned as content or viewport size changes.
Ensure secrets publishing works from a cold start by loading configured publishers before test/list/publish calls, with coverage for the missing-publisher path. Also avoid rebuilding the native toolbar when only enabled state changes, add the missing macOS toolbar actions, and polish the publish UI so button labels resize correctly and batch publish errors wrap and stand out.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues affect profile loading, refresh behavior, selection, and batch publishing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This pull request improves publish-profile loading, caching, publisher resolution, batch publishing, and related UI behavior.
Changes:
- Adds provider loading, progress reporting, caching, throttling, and key normalization.
- Adds multi-profile selection and sequential batch publishing.
- Improves publisher initialization, popovers, toolbar actions, wizard buttons, and cancellation support.
File summaries
| File | Summary | Final findings |
|---|---|---|
tests/MauiSherpa.Core.Tests/Services/SecretsPublisherServiceTests.cs |
Tests cold publisher resolution. | None. |
tests/MauiSherpa.Core.Tests/Services/PublishProfileServiceTests.cs |
Tests cache invalidation and provider deduplication. | None. |
src/MauiSherpa/wwwroot/js/mauiSherpaPopovers.js |
Adds dynamic popover positioning. | None. |
src/MauiSherpa/Pages/SecretsPublish.razor |
Adds profile selection and progressive loading. | 5 moderate findings: loading-state handling, cancellation disposal race, partial-snapshot selection loss, refresh visibility, and stale progress callbacks. 1 nit: selection checkbox lacks an accessible name. |
src/MauiSherpa/Pages/Modals/BatchPublishPage.cs |
Defines the batch publish modal page. | None. |
src/MauiSherpa/Pages/Modals/BatchPublishModal.razor |
Implements sequential batch publishing. | Moderate (1 vote): preserving provider-specific RepositoryId is required for GitLab and Azure DevOps publishing. |
src/MauiSherpa/Pages/Forms/WizardFormPage.cs |
Fixes primary-button measurement after label changes. | None. |
src/MauiSherpa/Components/SecretProviderPicker.razor |
Repositions dynamic provider popovers. | None. |
src/MauiSherpa.MacOS/BlazorContentPage.cs |
Registers native toolbar actions. | None. |
src/MauiSherpa.Core/Services/SecretsPublisherService.cs |
Resolves publishers asynchronously. | None. |
src/MauiSherpa.Core/Services/SecretItemAdapterHelper.cs |
Adds comparable storage-key normalization. | None. |
src/MauiSherpa.Core/Services/PublishProfileService.cs |
Implements multi-provider loading and caching. | 3 moderate findings: IDs are claimed before successful reads, provider reads remain effectively serialized, and empty partial snapshots can prematurely clear the profile state. |
src/MauiSherpa.Core/Interfaces.cs |
Extends service and cancellation contracts. | None. |
Review details
Suppressed comments (7)
src/MauiSherpa.Core/Services/PublishProfileService.cs:145
- This loop awaits provider listings and then reads each provider in priority order. Starting the listing tasks together does not make the profile reads parallel: a slow first provider blocks every later provider's
GetSecretAsynccalls and progress report, so a fast provider cannot paint until the slow one finishes. Load provider results as they complete and apply the priority rule when merging duplicate profiles.
foreach (var listingTask in listings)
{
var listing = await listingTask;
if (listing.Provider is null)
src/MauiSherpa.Core/Services/PublishProfileService.cs:163
- This reports an empty partial snapshot after a provider contributes no readable profiles.
SecretsPublishtreats every progress callback as the end of initial loading and applies the snapshot, so an empty fast provider can show the “No Publish Profiles” state while a slower provider still has profiles to return. Suppress empty progress snapshots (or add an explicit completion signal) and let the final result clear the list.
progress?.Report(Snapshot(profiles));
src/MauiSherpa/Pages/Modals/BatchPublishModal.razor:506
- This drops the stored
RepositoryIdand always publishes usingRepositoryFullName. That only works for providers whose API uses a path; GitLab parses a numeric project ID and Azure DevOps parses a GUID, while their repository listings explicitly store those values inIdand the display name inFullName. Batch publishing any GitLab or Azure DevOps destination will therefore fail; preserveRepositoryIdinDestinationVMand pass the provider-specific identifier when publishing.
destination.RepositoryFullName,
src/MauiSherpa/Pages/SecretsPublish.razor:253
- The previous CTS is disposed immediately after cancellation, but its already-queued
InvokeAsyncdelegate can still reachdebounce.Tokenat line 259. ReadingTokenfrom a disposedCancellationTokenSourcethrowsObjectDisposedException, which is not caught here, producing an unobserved task failure during a burst of sync events. Capture the token before scheduling so disposal cannot race with token access.
previous?.Dispose();
src/MauiSherpa/Pages/SecretsPublish.razor:276
- An explicit toolbar refresh calls
LoadProfilesAsyncwithbackgroundset to false. When profiles already exist, this branch sets onlyisLoading, but the page displays that indicator only whenprofiles.Count == 0;isRefreshingremains false, so the refresh runs with no visible status while the grid is stale. Treat any reload with existing profiles as a refresh.
if (background && profiles.Count > 0)
isRefreshing = true;
else
isLoading = true;
src/MauiSherpa/Pages/SecretsPublish.razor:76
- The profile-selection checkbox is wrapped in a label with no text and has no accessible name. The label's
titleis not a reliable accessible name for the input, so screen readers will announce an unlabeled checkbox; add anaria-labelthat includes the profile name.
<input type="checkbox"
checked="@selectedProfileIds.Contains(profile.Id)"
@onchange="@(e => ToggleProfileSelection(profile.Id, e.Value is bool on && on))" />
src/MauiSherpa/Pages/SecretsPublish.razor:287
Progress<T>.Reportis asynchronous here, and the handler adds another fire-and-forgetInvokeAsync. The final load can therefore apply the complete list and clearisRefreshingbefore this queued callback runs; because the sequence is unchanged, the stale partial snapshot can overwrite the final list and leave the page showing refresh indefinitely. Synchronize partial updates with finalization or invalidate callbacks once the final result is applied.
var progress = new Progress<IReadOnlyList<PublishProfile>>(partial =>
_ = InvokeAsync(() =>
{
if (sequence != loadSequence)
return;
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var keysToRead = listing.Keys | ||
| .Where(key => !TryGetComparableProfileId(key, out var id) || claimedIds.Add(id)) | ||
| .ToList(); |
| </div> | ||
| } | ||
| else if (isLoading) | ||
| else if (isLoading && profiles.Count == 0) |
| selectedProfileIds.IntersectWith(profiles.Select(profile => profile.Id)); | ||
| if (isSelectionMode && profiles.Count == 0) | ||
| { | ||
| isSelectionMode = false; | ||
| UpdateToolbarItems(); |
This pull request introduces several enhancements and fixes across the publish profiles, secrets publishing, and UI layers. The most significant changes include improving publish profile loading performance and responsiveness, adding batch publishing UI, and addressing various UI and code reliability issues.
Publish Profile Loading and Caching Improvements:
PublishProfileServiceto load profiles from multiple providers in parallel, reporting progress incrementally so the UI can display fast providers' profiles without waiting for slow ones. Added new overloads forGetProfilesAsyncand a newRefreshProfilesAsyncmethod, and improved cache invalidation logic. [1] [2] [3] [4]MaxParallelSecretReads) to prevent resource exhaustion when reading profiles.GetComparableStorageKeyinSecretItemAdapterHelperto reliably compare profile keys across providers with different key formats.Secrets Publisher Reliability:
SecretsPublisherServiceto ensure publisher instances are resolved asynchronously, preventing failures when the cache is uninitialized. This affects methods likeTestConnectionAsync,ListRepositoriesAsync, andPublishSecretsAsync. [1] [2] [3]UI and Modal Enhancements:
BatchPublishPagemodal for publishing multiple profiles at once, with platform-specific sizing for macOS and Linux.Bug Fixes and Minor Improvements:
RequestCancellationmethod to theIRunnableinterface for improved operation cancellation support.These changes collectively improve the responsiveness, reliability, and usability of publish profile management and secrets publishing in the application.