From 117e8959ca0eaba00acab20c65aba9274cafbd8d Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 23:57:09 -0600 Subject: [PATCH 1/2] fix(app): an account's page takes no token it would ignore, and Sync all does not call an unsynced account synced Findings of the release reviews of 1.1.1, each reproduced against the view models before it was fixed. - An account's token is read from where it is saved each time it is used (#158), so a token typed into the box on an account's page looked taken and was not used. The box of an account takes no input now; the page says the token is set with Edit account, offers that button, and says why Load cannot run when the account has no token. Quick Sync keeps its box. - Sync all: an account that could not be synced (no token, repositories that could not be loaded) is marked as needing attention and the pass ends as finished with failures, instead of a green "0 accounts synced, 1 skipped". An account whose own sync was canceled is not counted as synced. A canceled pass says how many accounts were not started. - Retry failed gives the selection back when its run is over. Left narrowed, a later Sync or Sync all synced only the rows that had failed and reported the account as synced. - "1 repository", not "1 repositories". Closes #182 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 4 +- gclo.Engine.Tests/SyncAllCoordinatorTests.cs | 185 +++++++++++++++++-- gclo.Engine.Tests/WorkspaceViewModelTests.cs | 6 +- gclo.ViewModels/SyncAllAccountState.cs | 11 +- gclo.ViewModels/SyncAllCoordinator.cs | 38 +++- gclo.ViewModels/SyncAllResult.cs | 13 +- gclo.ViewModels/SyncAllStatus.cs | 14 +- gclo.ViewModels/WorkspaceViewModel.cs | 35 +++- gclo/ConnectionFormControl.xaml.cs | 29 ++- gclo/MainWindow.xaml.cs | 4 +- gclo/WorkspacePage.xaml | 21 +++ gclo/WorkspacePage.xaml.cs | 9 + 12 files changed, 310 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index 6943c5a..fa6233b 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca Finished sync: every row Done, with the summary 58 cloned, 0 updated, 1 failed, 0 canceled of 59

-8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. +8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows, and your selection is afterwards what it was), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. **When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder (otherwise it keeps the branch its HEAD names, and stays empty until origin has that branch); a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). If you have worked in an unfinished clone with git in the meantime, gclo finishes it only when every file there is still the repository's own and nothing you added with `git add` would be removed; otherwise the row fails and names the file. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). This applies to a repository gclo could not check out when it cloned it. A repository that was already checked out normally is never rewritten: when an update brings it such a path, it fails and nothing in it is changed, and **Resolve…** (or `--sanitize-paths`) then says how to get a fresh clone with the renames instead of touching it. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). @@ -124,7 +124,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca ### Saved accounts -Quick Sync is ad-hoc — nothing is saved. For organizations you sync regularly, **Add account** stores a reusable profile: a name, the organization, target folder, and parallelism, with the token kept in the Windows Credential Manager (never in a file). Each account gets its own entry in the navigation pane and its own workspace; **Sync all** runs every saved account in turn. A status bar above the workspace is there for the whole pass, whichever workspace is on screen: it says which account is syncing, which of how many, and how far along it is, takes you to that account, and holds the result when the pass is over. The pane shows each account's live status as well. **Cancel sync all** stops the pass after the account that is syncing. +Quick Sync is ad-hoc — nothing is saved. For organizations you sync regularly, **Add account** stores a reusable profile: a name, the organization, target folder, and parallelism, with the token kept in the Windows Credential Manager (never in a file). Each account gets its own entry in the navigation pane and its own workspace; **Sync all** runs every saved account in turn. A status bar above the workspace is there for the whole pass, whichever workspace is on screen: it says which account is syncing, which of how many, and how far along it is, takes you to that account, and holds the result when the pass is over. The pane shows each account's live status as well. **Cancel sync all** stops the pass after the account that is syncing, and the result says how many accounts were not started. An account that could not be synced (it has no token, or its repositories could not be loaded) is marked in the pane and counted as needing attention, so a pass in which it was left out does not read as a success. An account's token is changed with **Edit account…**, on the account's page or in the pane's menu; the token box on an account's page takes no input. Sync all syncs the repositories each account's page lists: **Refresh** on that page picks up repositories created since it was loaded.

Add account wizard, step 1 of 4: account name and optional description diff --git a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs index 447be17..d84c9cc 100644 --- a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs +++ b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs @@ -178,7 +178,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable [Fact] - public async Task RunAsync_DefaultTokenAccount_WithoutADefaultToken_IsSkipped_NotRun() + public async Task RunAsync_DefaultTokenAccount_WithoutADefaultToken_NeedsAttention_AndIsNotRun() { WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); Assert.True(shared.UsesDefaultToken); @@ -188,10 +188,9 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); - Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Skipped], states); - Assert.Empty(_lister.Organizations); // never asked GitHub: skipped before loading + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 1, WasCanceled: false, Failed: 1), result); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Failed], states); // not a success with one skipped (#182) + Assert.Empty(_lister.Organizations); // never asked GitHub: stopped before loading } @@ -298,7 +297,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable [Fact] - public async Task RunAsync_SkipsAWorkspaceWhoseLoadFails_AndMovesOn() + public async Task RunAsync_AWorkspaceWhoseLoadFails_NeedsAttention_AndThePassMovesOn() { _lister.Handler = org => string.Equals(org, "org-alpha" , StringComparison.Ordinal) ? throw new InvalidOperationException("org not found") @@ -309,7 +308,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync(new[] { alpha, beta }, CancellationToken.None); - Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 1, WasCanceled: false), result); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 1, WasCanceled: false, Failed: 1), result); Assert.False(alpha.HasLoadedRepos); Assert.Equal(new[] { "org-beta-repo" }, git.ClonedRepoNames); } @@ -354,7 +353,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable gate.SetResult(); // ...which must still run to completion SyncAllResult result = await run.WaitAsync(TimeSpan.FromSeconds(30)); - Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true), result); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true, NotStarted: 1), result); Assert.Equal(SyncStatus.Done, alpha.Repos[0].Status); // finished, not canceled Assert.False(alpha.IsRunning); Assert.False(beta.HasLoadedRepos); // the second account never started @@ -374,7 +373,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync(new[] { workspace }, cts.Token); - Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: true), result); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: true, NotStarted: 1), result); Assert.False(workspace.HasLoadedRepos); Assert.Empty(git.CloneCalls); } @@ -497,15 +496,14 @@ public sealed class SyncAllCoordinatorTests : IDisposable [Fact] - public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasRemovedAfterItOpened_IsSkipped() + public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasRemovedAfterItOpened_NeedsAttention() { WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: true); _vault.Delete(AppSettings.DefaultTokenVaultId); // removed in Settings meanwhile SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 1, WasCanceled: false, Failed: 1), result); Assert.Empty(_lister.Organizations); // GitHub was never asked with the old token } @@ -514,7 +512,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable // ---------------------------------------------------------------- review of #157: no success for a sync that never ran [Fact] - public async Task RunAsync_AnAccountWhoseOwnTokenIsGone_IsSkipped_NotReportedAsSucceeded() + public async Task RunAsync_AnAccountWhoseOwnTokenIsGone_NeedsAttention_NotReportedAsSucceeded() { WorkspaceViewModel workspace = CreateWorkspace("own"); await workspace.LoadReposCommand.ExecuteAsync(null); // a loaded table: the sync command can run without a token @@ -524,9 +522,8 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([workspace], CancellationToken.None); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); - Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Skipped], states); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 1, WasCanceled: false, Failed: 1), result); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Failed], states); } @@ -701,7 +698,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable Assert.True(result.WasCanceled); Assert.Equal(SyncAllOutcome.Canceled, status.Outcome); Assert.Equal("Sync all canceled", status.Title); - Assert.Equal("1 account synced, 0 skipped.", status.Message); + Assert.Equal("1 account synced, 0 skipped. 1 account was not started.", status.Message); Assert.False(status.IsRunning); // The next pass starts clean. @@ -752,8 +749,13 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([failing, never], cts.Token); - Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true, Failed: 1), result); - Assert.Equal("1 account synced, 0 skipped. 1 account needs attention; it is marked in the pane.", SyncAllStatus.Describe(result)); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true, Failed: 1, NotStarted: 1), result); + Assert.Equal( + "1 account synced, 0 skipped. 1 account needs attention; it is marked in the pane. 1 account was not started.", + SyncAllStatus.Describe(result)); + Assert.Equal( + "0 accounts synced, 0 skipped. 3 accounts were not started.", + SyncAllStatus.Describe(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: true, NotStarted: 3))); } @@ -833,4 +835,149 @@ public sealed class SyncAllCoordinatorTests : IDisposable Assert.Throws(() => status.Enter(1, null!)); Assert.Throws(() => status.End(null!)); } + + + + // ---------------------------------------------------------------- release reviews of 1.1.1 (#182) + + [Fact] + public async Task AnAccountThatCouldNotBeSynced_DoesNotLeaveThePassLookingLikeASuccess() + { + // Its token is gone: nothing else is wrong in the pass. + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + var status = new SyncAllStatus(); + _coordinator.Status = status; + + await _coordinator.RunAsync([shared], CancellationToken.None); + + Assert.Equal(SyncAllOutcome.FinishedWithFailures, status.Outcome); + Assert.Equal("0 accounts synced, 1 skipped. 1 account needs attention; it is marked in the pane.", status.Message); + } + + + + [Theory] + [InlineData(false)] + [InlineData(true)] // a repository had failed before the cancel: the account needs attention + public async Task AnAccountWhoseOwnSyncWasCanceled_IsNotCountedAsSynced_AndThePassGoesOn(bool afterAFailure) + { + _lister.Handler = _ => [Repo("one"), Repo("two"), Repo("three")]; + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var git = new FakeGitClient + { + CloneHandler = async (_, path, _, _, ct) => + { + if(afterAFailure && path.EndsWith("one", StringComparison.Ordinal)) + { + throw new InvalidOperationException("boom"); // one at a time, in order: this one is over before the next begins + } + entered.TrySetResult(); + await Task.Delay(Timeout.Infinite, ct); + }, + }; + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + alpha.MaxConcurrency = 1; + WorkspaceViewModel beta = CreateWorkspace("beta"); + var states = new List(); + _coordinator.AccountStateChanged = (id, state) => + { + if(id == alpha.AccountId) + { + states.Add(state); + } + }; + var status = new SyncAllStatus(); + _coordinator.Status = status; + + Task run = _coordinator.RunAsync([alpha, beta], CancellationToken.None); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(30)); + alpha.SyncCancelCommand.Execute(null); // Cancel in that account's own workspace + SyncAllResult result = await run.WaitAsync(TimeSpan.FromSeconds(30)); + + Assert.Equal(RunResultKind.Canceled, alpha.ResultKind); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 1, WasCanceled: false, Failed: afterAFailure ? 1 : 0), result); // beta ran + Assert.Equal( + [SyncAllAccountState.Queued, SyncAllAccountState.Running, afterAFailure ? SyncAllAccountState.Failed : SyncAllAccountState.Skipped], + states); + Assert.Equal( + afterAFailure + ? "1 account synced, 1 skipped. 1 account needs attention; it is marked in the pane." + : "1 account synced, 1 skipped.", + status.Message); + } + + + + [Theory] + [InlineData(true)] + [InlineData(false)] // one repository was not selected: it is not selected afterwards either + public async Task RetryFailed_GivesTheSelectionBack_SoTheNextSyncCoversWhatItCoveredBefore(bool allSelected) + { + _lister.Handler = _ => [Repo("one"), Repo("two"), Repo("three")]; + bool failTwo = true; + var git = new FakeGitClient + { + CloneHandler = (_, path, _, _, _) => failTwo && path.EndsWith("two", StringComparison.Ordinal) + ? Task.FromException(new InvalidOperationException("boom")) + : Task.CompletedTask, + }; + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + await alpha.LoadReposCommand.ExecuteAsync(null); + if(!allSelected) + { + alpha.Repos.Single(repo => string.Equals(repo.Name, "three", StringComparison.Ordinal)).IsSelected = false; + } + int selected = allSelected ? 3 : 2; + await alpha.SyncCommand.ExecuteAsync(null); + Assert.True(alpha.HasFailedRepos); + + failTwo = false; + await alpha.RetryFailedCommand.ExecuteAsync(null); + + Assert.False(alpha.HasFailedRepos); + Assert.Equal(selected, alpha.SelectedCount); // the retry ran one repository; the selection is the user's again + Assert.Equal(selected, alpha.Repos.Count(repo => repo.IsSelected)); + Assert.Equal(allSelected, alpha.Repos.Single(repo => string.Equals(repo.Name, "three", StringComparison.Ordinal)).IsSelected); + + int before = git.CloneCalls.Count; + SyncAllResult result = await _coordinator.RunAsync([alpha], CancellationToken.None); + + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: false), result); + Assert.Equal(selected, git.CloneCalls.Count - before); // not only the one that had failed + } + + + + [Fact] + public void AnAccountsPage_IsToldWhenItsTokenIsNoLongerMissing() + { + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + var changed = new List(); + shared.PropertyChanged += (_, e) => changed.Add(e.PropertyName); + Assert.NotNull(shared.MissingTokenDescription); + + _vault.Store(AppSettings.DefaultTokenVaultId, "tok-default"); // saved in Settings meanwhile + shared.RefreshAccountToken(); + + Assert.Null(shared.MissingTokenDescription); + Assert.Contains(changed, name => string.Equals(name, nameof(WorkspaceViewModel.MissingTokenDescription), StringComparison.Ordinal)); + } + + + + [Fact] + public async Task OneRepository_IsCountedInTheSingular() + { + (FakeGitClient git, Task entered, TaskCompletionSource gate) = GatedGit(); + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + + await alpha.LoadReposCommand.ExecuteAsync(null); + Assert.Equal("1 repository loaded.", alpha.StatusText); + + Task sync = alpha.SyncCommand.ExecuteAsync(null); + await entered.WaitAsync(TimeSpan.FromSeconds(30)); + Assert.Equal("Syncing 1 repository...", alpha.StatusText); + gate.SetResult(); + await sync.WaitAsync(TimeSpan.FromSeconds(30)); + } } diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index 4d9b682..3fe9f82 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -641,9 +641,9 @@ public sealed class WorkspaceViewModelTests : IDisposable Assert.Equal(SyncStatus.Done, byName["bravo"].Status); Assert.Null(byName["bravo"].Error); Assert.Equal(SyncStatus.Done, byName["alpha"].Status); // untouched by the retry - Assert.True(byName["bravo"].IsSelected); // retry selected exactly the failures - Assert.False(byName["alpha"].IsSelected); - Assert.False(byName["charlie"].IsSelected); + Assert.True(byName["bravo"].IsSelected); // the retry ran exactly the failure, and the selection is what it was (#182) + Assert.True(byName["alpha"].IsSelected); + Assert.True(byName["charlie"].IsSelected); Assert.Equal(1, vm.TotalCount); // run-scoped (#22): the retry run was just bravo Assert.Equal(1, vm.CompletedCount); Assert.False(vm.RetryFailedCommand.CanExecute(null)); // nothing failed anymore diff --git a/gclo.ViewModels/SyncAllAccountState.cs b/gclo.ViewModels/SyncAllAccountState.cs index e6809d0..33234fe 100644 --- a/gclo.ViewModels/SyncAllAccountState.cs +++ b/gclo.ViewModels/SyncAllAccountState.cs @@ -18,9 +18,16 @@ public enum SyncAllAccountState ///

Finished with no failed repositories. Succeeded, - /// Finished, but at least one repository failed. + /// + /// Needs attention: its sync finished with at least one failed repository, or it + /// could not be synced (no token, repositories that could not be loaded, a sync + /// that could not start). + /// Failed, - /// Not run: busy, unloadable, nothing to sync, or the queue was canceled. + /// + /// Not synced, and nothing is wrong with it: busy, nothing to sync, its own sync + /// was canceled, or the queue was canceled before its turn. + /// Skipped, } diff --git a/gclo.ViewModels/SyncAllCoordinator.cs b/gclo.ViewModels/SyncAllCoordinator.cs index 51ccdf3..26562e7 100644 --- a/gclo.ViewModels/SyncAllCoordinator.cs +++ b/gclo.ViewModels/SyncAllCoordinator.cs @@ -40,9 +40,12 @@ public sealed class SyncAllCoordinator(IActivityLog log) /// /// Syncs each workspace of in list order, /// loading its repositories first when needed. A workspace is skipped (counted, - /// logged, never fatal) when it is already busy, when its repositories cannot be - /// loaded, or when its sync command cannot run. Canceling - /// stops the queue after the in-flight account rather than canceling its sync. + /// logged, never fatal) when it is already busy or has nothing to sync. One that + /// cannot be synced (it has no token, its repositories cannot be loaded, its sync + /// cannot start) is skipped too, and counted and announced as failed: the pass + /// must not read as a success when an account was not synced (#182). Canceling + /// stops the queue after the in-flight account rather than + /// canceling its sync. /// public async Task RunAsync( IReadOnlyList accountWorkspaces, CancellationToken ct) @@ -95,7 +98,7 @@ public sealed class SyncAllCoordinator(IActivityLog log) Announce(accountWorkspaces[rest], SyncAllAccountState.Skipped); } _log.Info("Sync all: canceled — remaining accounts were not started."); - return new SyncAllResult(ran, skipped, WasCanceled: true, failed); + return new SyncAllResult(ran, skipped, WasCanceled: true, failed, NotStarted: accountWorkspaces.Count - i); } if(workspace.IsRunning || workspace.IsLoadingRepos || workspace.IsResolvingPaths) @@ -114,8 +117,9 @@ public sealed class SyncAllCoordinator(IActivityLog log) // which, instead of a sync that cannot start or a generic auth failure // that sends the user to the wrong place. skipped++; - Announce(workspace, SyncAllAccountState.Skipped); - _log.Info($"Sync all: skipped '{workspace.DisplayName}' — {missingToken}"); + failed++; + Announce(workspace, SyncAllAccountState.Failed); + _log.Info($"Sync all: '{workspace.DisplayName}' could not be synced — {missingToken}"); continue; } @@ -129,9 +133,10 @@ public sealed class SyncAllCoordinator(IActivityLog log) if(!workspace.HasLoadedRepos) { skipped++; - Announce(workspace, SyncAllAccountState.Skipped); + failed++; + Announce(workspace, SyncAllAccountState.Failed); _log.Info( - $"Sync all: skipped '{workspace.DisplayName}' — its repositories could not be loaded."); + $"Sync all: '{workspace.DisplayName}' could not be synced — its repositories could not be loaded."); continue; } } @@ -158,6 +163,23 @@ public sealed class SyncAllCoordinator(IActivityLog log) continue; } + if(workspace.ResultKind == RunResultKind.Canceled) + { + /////////////////////////////////////////////////////////////////////////// + // Its own sync was canceled (in its workspace, or because the account // + // was edited meanwhile) and left repositories unsynced: that is not a // + // synced account. The pass itself goes on. // + /////////////////////////////////////////////////////////////////////////// + skipped++; + if(workspace.HasFailedRepos) + { + failed++; + } + Announce(workspace, workspace.HasFailedRepos ? SyncAllAccountState.Failed : SyncAllAccountState.Skipped); + _log.Info($"Sync all: '{workspace.DisplayName}' was canceled before it finished."); + continue; + } + ran++; if(workspace.HasFailedRepos) { diff --git a/gclo.ViewModels/SyncAllResult.cs b/gclo.ViewModels/SyncAllResult.cs index 678bfff..c921de4 100644 --- a/gclo.ViewModels/SyncAllResult.cs +++ b/gclo.ViewModels/SyncAllResult.cs @@ -9,11 +9,16 @@ namespace gclo.ViewModels; /// /// Outcome of one sync-all pass over the account workspaces. /// -/// Accounts whose sync ran to completion (successfully or not). -/// Accounts skipped because they were busy, could not load, or had nothing to sync. +/// Accounts whose sync ran to its end (with or without failed repositories). +/// +/// Accounts whose turn came and whose sync did not run to its end: they were busy, +/// had nothing to sync, could not be synced (no token, repositories that could not +/// be loaded, a sync that could not start), or their own sync was canceled. +/// /// True when cancellation stopped the queue before every account was processed. /// /// Accounts that ended the pass marked as failed: their sync ran and a repository -/// failed, or it could not run at all. The second kind is also in . +/// failed, or they could not be synced. The second kind is also in . /// -public sealed record SyncAllResult(int Ran, int Skipped, bool WasCanceled, int Failed = 0); +/// Accounts whose turn never came, because the pass was canceled before it. +public sealed record SyncAllResult(int Ran, int Skipped, bool WasCanceled, int Failed = 0, int NotStarted = 0); diff --git a/gclo.ViewModels/SyncAllStatus.cs b/gclo.ViewModels/SyncAllStatus.cs index 0245b83..9e47194 100644 --- a/gclo.ViewModels/SyncAllStatus.cs +++ b/gclo.ViewModels/SyncAllStatus.cs @@ -159,11 +159,17 @@ public sealed partial class SyncAllStatus : ObservableObject internal static string Describe(SyncAllResult result) { string summary = $"{Count(result.Ran, "account")} synced, {result.Skipped.ToString(CultureInfo.InvariantCulture)} skipped."; - return result.Failed switch + summary += result.Failed switch { - 0 => summary, - 1 => summary + " 1 account needs attention; it is marked in the pane.", - _ => summary + $" {result.Failed.ToString(CultureInfo.InvariantCulture)} accounts need attention; they are marked in the pane.", + 0 => "", + 1 => " 1 account needs attention; it is marked in the pane.", + _ => $" {result.Failed.ToString(CultureInfo.InvariantCulture)} accounts need attention; they are marked in the pane.", + }; + return summary + result.NotStarted switch + { + 0 => "", + 1 => " 1 account was not started.", + _ => $" {result.NotStarted.ToString(CultureInfo.InvariantCulture)} accounts were not started.", }; } diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index 2481061..b28b506 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -220,6 +220,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable [ObservableProperty] [NotifyCanExecuteChangedFor(nameof(LoadReposCommand))] + [NotifyPropertyChangedFor(nameof(MissingTokenDescription))] public partial string Token { get; set; } [ObservableProperty] @@ -967,7 +968,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RebuildReposIndex(); LoadedOrganization = organization; HasLoadedRepos = true; - StatusText = $"{Repos.Count} repositories loaded."; + StatusText = Repos.Count == 1 ? "1 repository loaded." : $"{Repos.Count} repositories loaded."; _log.Info($"Loaded {Repos.Count} repositories for organization '{organization}'."); } catch(Exception ex) @@ -1032,7 +1033,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RebuildFilteredRepos(); // every selected row just went back to Queued string targetRoot = EffectiveTargetRoot; - StatusText = $"Syncing {selected.Count} repositories..."; + StatusText = selected.Count == 1 ? "Syncing 1 repository..." : $"Syncing {selected.Count} repositories..."; _log.Info($"Sync started: {selected.Count} repositories into '{targetRoot}'."); // Constructed on the UI thread: the delivery captures the WinUI @@ -1129,18 +1130,41 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable - /// Selects exactly the failed repositories and runs the same sync path again. + /// + /// Selects exactly the failed repositories, runs the same sync path again, and + /// gives the selection back as it was. The retry is one run, not a change of what + /// the next sync covers: left narrowed, a later Sync (or Sync all) would sync only + /// the rows that had failed and report the account as synced (#182). + /// [RelayCommand(CanExecute = nameof(CanRetryFailed))] private async Task RetryFailedAsync() { _log.Info($"Retrying {Repos.Count(r => r.Status == SyncStatus.Failed)} failed repositories."); + HashSet selectedBefore = [.. Repos.Where(item => item.IsSelected)]; + SelectOnly(item => item.Status == SyncStatus.Failed); + try + { + // Executing SyncCommand itself keeps SyncCancelCommand working for retry runs. + await SyncCommand.ExecuteAsync(null); + } + finally + { + SelectOnly(selectedBefore.Contains); + } + } + + + + /// Selects the repositories picks and no others, as one change. + private void SelectOnly(Func wanted) + { // One batch: per-row recounts and command re-queries would be O(n^2) (#30). _syncingSelection = true; try { foreach(RepoItemViewModel item in Repos) { - item.IsSelected = item.Status == SyncStatus.Failed; + item.IsSelected = wanted(item); } } finally @@ -1150,9 +1174,6 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RecomputeSelectedCount(); UpdateAllSelectedFromItems(); SyncCommand.NotifyCanExecuteChanged(); - - // Executing SyncCommand itself keeps SyncCancelCommand working for retry runs. - await SyncCommand.ExecuteAsync(null); } diff --git a/gclo/ConnectionFormControl.xaml.cs b/gclo/ConnectionFormControl.xaml.cs index 2f0ca93..007bdac 100644 --- a/gclo/ConnectionFormControl.xaml.cs +++ b/gclo/ConnectionFormControl.xaml.cs @@ -86,15 +86,26 @@ public sealed partial class ConnectionFormControl : UserControl private void OnLoaded(object sender, RoutedEventArgs e) { bool hasToken = ViewModel is not null && ViewModel.Token.Length > 0; + bool savedWithAnAccount = ViewModel?.AccountId is not null; TokenBox.Password = hasToken ? SettingsTokenState.Mask : ""; - TokenBox.PlaceholderText = "ghp_…"; - // The organization placeholder must not ask for a token the box already has. - OrgBox.PlaceholderText = hasToken ? "Choose an organization, or type one" : "Paste a token first, or type an org"; - ToolTipService.SetToolTip(TokenBox, !hasToken - ? "Fine-grained or classic PAT with repository read access; used for the GitHub API and git over HTTPS. Never written to disk or logs." - : ViewModel!.AccountId is null - ? "Using the saved default token from Settings. Type here to use a different one for this session." - : "Using this account's token. Type here to use a different one for this session."); + + /////////////////////////////////////////////////////////////////////////////// + // An account's token is read from where it is saved each time it is used // + // (#158). A token typed here would look taken and then not be used, so the // + // box of an account takes none; the page says where the token is changed // + // (#182). // + /////////////////////////////////////////////////////////////////////////////// + TokenBox.IsEnabled = !savedWithAnAccount; + TokenBox.PlaceholderText = savedWithAnAccount ? "No token is saved for this account" : "ghp_…"; + // The organization placeholder must not ask for a token the box already has, or cannot take. + OrgBox.PlaceholderText = hasToken ? "Choose an organization, or type one" + : savedWithAnAccount ? "Type an organization or account" + : "Paste a token first, or type an org"; + ToolTipService.SetToolTip(TokenBox, savedWithAnAccount + ? "This account's token is set with Edit account…, not typed here." + : !hasToken + ? "Fine-grained or classic PAT with repository read access; used for the GitHub API and git over HTTPS. Never written to disk or logs." + : "Using the saved default token from Settings. Type here to use a different one for this session."); } @@ -105,7 +116,7 @@ public sealed partial class ConnectionFormControl : UserControl private void TokenBox_PasswordChanged(object sender, RoutedEventArgs e) { string typed = ((PasswordBox)sender).Password; - if(ViewModel is not null && !string.Equals(typed, SettingsTokenState.Mask, StringComparison.Ordinal)) + if(ViewModel is not null && ViewModel.AccountId is null && !string.Equals(typed, SettingsTokenState.Mask, StringComparison.Ordinal)) { ViewModel.Token = typed; } diff --git a/gclo/MainWindow.xaml.cs b/gclo/MainWindow.xaml.cs index 8030d31..5cfddba 100644 --- a/gclo/MainWindow.xaml.cs +++ b/gclo/MainWindow.xaml.cs @@ -608,11 +608,13 @@ public sealed partial class MainWindow : Window CreateBadge("InformationalDotInfoBadgeStyle", "SystemFillColorSolidNeutralBrush"), SyncAllAccountState.Succeeded => CreateBadge("SuccessIconInfoBadgeStyle", "SystemFillColorSuccessBrush"), - SyncAllAccountState.Failed => + SyncAllAccountState.Failed when viewModel.HasFailedRepos => CreateBadge( "CriticalValueInfoBadgeStyle", "SystemFillColorCriticalBrush", viewModel.Repos.Count(r => r.Status == SyncStatus.Failed)), + SyncAllAccountState.Failed => // it could not be synced at all: there is no number to show + CreateBadge("CriticalDotInfoBadgeStyle", "SystemFillColorCriticalBrush"), SyncAllAccountState.Skipped => CreateBadge("CautionDotInfoBadgeStyle", "SystemFillColorCautionBrush"), _ => null, // Running is covered by IsRunning above diff --git a/gclo/WorkspacePage.xaml b/gclo/WorkspacePage.xaml index 2e86760..baa4307 100644 --- a/gclo/WorkspacePage.xaml +++ b/gclo/WorkspacePage.xaml @@ -129,6 +129,21 @@ TextWrapping="Wrap" Text="Once this connection works, save it as an account (Edit on the connection chip) so the token is kept for you." Visibility="{x:Bind local:StatusFormat.VisibleIfNot(IsAccountWorkspace), Mode=OneTime}" /> + + + +