From e083089cb322844edc001e7f11465f71c7107080 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 16:14:23 -0600 Subject: [PATCH 1/3] fix(app): a saved token is read when it is used, not once when a workspace opens An account workspace took its token from the vault in its constructor and kept it. After the default token was replaced or removed in Settings, open workspaces went on using the old one until the app was restarted; after a default token was saved for the first time, Sync all still skipped the accounts that use it; Quick Sync, which Settings pre-fills, kept the old value too. - A workspace reads its account's token from its source again when it is shown, before it loads, before it syncs, and when Sync all reaches it. A token that is gone by then stops the operation with a message that names the source, instead of running with the old one. - The shell tells the open workspaces when Settings changed the default token. Quick Sync follows only while its box still holds what Settings put there; a token somebody typed stays. - The account wizard shows the choice between the default token and a token of the account's own also for an account that uses the default token while none is saved, which was a dead end at step 2. - The "no token" message names the credential entry the way the vault in use names it (entry names are scoped to the data folder). Closes #157 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 2 +- gclo.Cli/SyncCommand.cs | 5 +- .../AccountTokenResolverTests.cs | 35 +++- .../AccountWizardViewModelTests.cs | 42 +++++ .../AccountsStoreConsistencyTests.cs | 4 + gclo.Engine.Tests/SyncAllCoordinatorTests.cs | 33 ++++ gclo.Engine.Tests/WorkspaceViewModelTests.cs | 157 ++++++++++++++++++ gclo.ViewModels/AccountTokenResolver.cs | 9 +- gclo.ViewModels/AccountWizardViewModel.cs | 12 ++ gclo.ViewModels/CredentialManagerVault.cs | 7 + gclo.ViewModels/ITokenVault.cs | 8 + gclo.ViewModels/InMemoryVault.cs | 5 + gclo.ViewModels/SyncAllCoordinator.cs | 2 + gclo.ViewModels/WorkspaceViewModel.cs | 103 +++++++++++- gclo/AccountWizardDialog.xaml | 2 +- gclo/MainWindow.xaml.cs | 7 + 16 files changed, 417 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index eea832e..1a4d0cd 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ gclo authenticates to GitHub with a personal access token — it is used for bot | Classic PAT | `repo` scope (read access to the org's repositories); optionally `read:org` so the organization dropdown / `gclo orgs` can list your orgs | | Fine-grained PAT | Repository read access for the organization | -**Token storage.** In **Quick Sync**, the token is held in memory for the session only and never written anywhere. A **saved account's** token and the optional **default token** (Settings) are kept in the **Windows Credential Manager** (per-user, DPAPI-protected) — never in `settings.json`, `accounts.json`, or any log. See [SECURITY.md](SECURITY.md). +**Token storage.** In **Quick Sync**, the token is held in memory for the session only and never written anywhere. A **saved account's** token and the optional **default token** (Settings) are kept in the **Windows Credential Manager** (per-user, DPAPI-protected) — never in `settings.json`, `accounts.json`, or any log. See [SECURITY.md](SECURITY.md). A saved token is read at the moment it is used: replacing or removing the default token in Settings, or an account's token in its wizard, takes effect for the workspaces that are already open, without a restart. Quick Sync follows a changed default token only while its token box still holds the one Settings put there. ## Using the GUI diff --git a/gclo.Cli/SyncCommand.cs b/gclo.Cli/SyncCommand.cs index c1fd604..14f7129 100644 --- a/gclo.Cli/SyncCommand.cs +++ b/gclo.Cli/SyncCommand.cs @@ -515,10 +515,11 @@ internal static class SyncCommand } // vault is non-null whenever account is: both come from AccountsCommand.Open. + var resolver = new AccountTokenResolver(vault!); string? token; try { - token = new AccountTokenResolver(vault!).TryResolve(account); + token = resolver.TryResolve(account); } catch(Exception ex) when(CliErrorException.IsStorageFailure(ex)) { @@ -535,7 +536,7 @@ internal static class SyncCommand : "Re-enter the token with 'gclo accounts edit --name --token-stdin' or in the desktop app's " + "account wizard, restore the credential entry manually,"; throw new CliErrorException( - $"Account '{account.Name}' {AccountTokenResolver.MissingDescription(account)}. {fix} " + $"Account '{account.Name}' {resolver.MissingDescription(account)}. {fix} " + "or pass a token with --token-env, --token-file, or --token-stdin."); } return token; diff --git a/gclo.Engine.Tests/AccountTokenResolverTests.cs b/gclo.Engine.Tests/AccountTokenResolverTests.cs index 91ec697..948ee47 100644 --- a/gclo.Engine.Tests/AccountTokenResolverTests.cs +++ b/gclo.Engine.Tests/AccountTokenResolverTests.cs @@ -43,7 +43,7 @@ public sealed class AccountTokenResolverTests var resolver = new AccountTokenResolver(_vault); Assert.Throws(() => resolver.TryResolve(null!)); - Assert.Throws(() => AccountTokenResolver.MissingDescription(null!)); + Assert.Throws(() => resolver.MissingDescription(null!)); } @@ -129,11 +129,38 @@ public sealed class AccountTokenResolverTests Account own = MakeAccount(TokenSource.Own); Account byDefault = MakeAccount(TokenSource.Default); - string ownText = AccountTokenResolver.MissingDescription(own); - string defaultText = AccountTokenResolver.MissingDescription(byDefault); + var resolver = new AccountTokenResolver(_vault); + string ownText = resolver.MissingDescription(own); + string defaultText = resolver.MissingDescription(byDefault); - Assert.Contains($"gclo:account:{own.Id:N}", ownText); + Assert.Contains($"{own.Id:N}", ownText); Assert.Contains("Settings", defaultText); Assert.DoesNotContain("ghp", ownText + defaultText); } + + + + [Fact] + public void MissingDescription_NamesTheEntry_TheWayTheVaultInUseNamesIt() + { + if(!OperatingSystem.IsWindows()) + { + return; // Credential Manager is a Windows store + } + // The data folder scopes the entry names in Credential Manager (#157): the + // message must name the entry a person will actually find there. + Account own = MakeAccount(TokenSource.Own); + var unscoped = new AccountTokenResolver(new CredentialManagerVault(isWindows: true)); + var scoped = new AccountTokenResolver(new CredentialManagerVault(isWindows: true, scope: "0123456789abcdef")); + + Assert.Equal( + $"has no token in Windows Credential Manager (entry 'gclo:account:{own.Id:N}')", + unscoped.MissingDescription(own)); + Assert.Equal( + $"has no token in Windows Credential Manager (entry 'gclo:0123456789abcdef:account:{own.Id:N}')", + scoped.MissingDescription(own)); + Assert.Equal( + $"has no token in this session's memory (account {own.Id:N})", + new AccountTokenResolver(new InMemoryVault()).MissingDescription(own)); + } } diff --git a/gclo.Engine.Tests/AccountWizardViewModelTests.cs b/gclo.Engine.Tests/AccountWizardViewModelTests.cs index a335793..1578b6e 100644 --- a/gclo.Engine.Tests/AccountWizardViewModelTests.cs +++ b/gclo.Engine.Tests/AccountWizardViewModelTests.cs @@ -743,4 +743,46 @@ public sealed class AccountWizardViewModelTests : IDisposable Assert.Equal(account.Id, Assert.Single(_store.GetAll()).Id); Assert.Equal("ghp_rotated", _vault.TryRetrieve(account.Id)); } + + + + // ---------------------------------------------------------------- an account on the default token while none is saved (#157) + + [Fact] + public async Task Edit_OfAnAccountOnTheDefaultToken_WhileNoneIsSaved_OffersTheChoice_SoItCanGetATokenOfItsOwn() + { + Account account = MakeAccount("Shared") with { TokenSource = TokenSource.Default }; + _store.Save(account, token: null); + AccountWizardViewModel wizard = NewWizardWithDefault(defaultToken: null, existing: account); + + Assert.False(wizard.HasDefaultToken); + Assert.True(wizard.UseDefaultToken); + Assert.True(wizard.OffersTokenChoice); + Assert.StartsWith("No default token is saved right now.", wizard.DefaultTokenCaption); + + // Step 2 as it is: nothing to validate with, and the message says what to do... + await AdvanceToStepAsync(wizard, 2); + Assert.False(await wizard.TryAdvanceAsync()); + Assert.Contains("use a different token for this account", wizard.TokenError); + + // ...which is now possible. + wizard.TokenChoiceIndex = 1; + wizard.Token = "ghp_a_token_of_its_own"; + Assert.True(await wizard.TryAdvanceAsync()); + Assert.Equal(3, wizard.Step); + } + + + + [Fact] + public void TheTokenChoice_IsOffered_WhenADefaultTokenIsSaved_AndNotForAnAccountThatNeverUsedOne() + { + Account own = MakeAccount("Own"); + _store.Save(own, "ghp_own"); + + Assert.True(NewWizardWithDefault("ghp_default").OffersTokenChoice); + Assert.True(NewWizardWithDefault("ghp_default", existing: own, existingToken: "ghp_own").OffersTokenChoice); + Assert.False(NewWizardWithDefault(defaultToken: null).OffersTokenChoice); + Assert.False(NewWizardWithDefault(defaultToken: null, existing: own, existingToken: "ghp_own").OffersTokenChoice); + } } diff --git a/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs b/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs index 6ddf8a1..c0e43d1 100644 --- a/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs +++ b/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs @@ -491,6 +491,10 @@ public sealed class AccountsStoreConsistencyTests : IDisposable + public string DescribeEntry(Guid accountId) => _inner.DescribeEntry(accountId); + + + public void Delete(Guid accountId) { DeleteCalls++; diff --git a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs index 57c6dd3..0bb26c5 100644 --- a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs +++ b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs @@ -462,4 +462,37 @@ public sealed class SyncAllCoordinatorTests : IDisposable }, states); } + + + + // ---------------------------------------------------------------- a token saved after the workspace opened (#157) + + [Fact] + public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasSavedAfterItOpened_Runs() + { + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + Assert.Equal("", shared.Token); + _vault.Store(AppSettings.DefaultTokenVaultId, "tok-saved-later"); // saved in Settings meanwhile + + SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); + + Assert.Equal(1, result.Ran); + Assert.Equal(0, result.Skipped); + Assert.Equal("tok-saved-later", shared.Token); + } + + + + [Fact] + public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasRemovedAfterItOpened_IsSkipped() + { + 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.Empty(_lister.Organizations); // GitHub was never asked with the old token + } } diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index a713423..258e7db 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -2392,4 +2392,161 @@ public sealed class WorkspaceViewModelTests : IDisposable Assert.DoesNotContain(vm.Repos, r => string.Equals(r.Name, "ghost-repo", StringComparison.Ordinal)); } + + + + // ---------------------------------------------------------------- a saved token is read when it is used (#157) + + private static Account MakeDefaultTokenAccount(string targetRoot) => MakeAccount(targetRoot) with { TokenSource = TokenSource.Default }; + + + + /// A folder of this test's own; the view model's target folder is removed when the test ends. + private static string NewTargetRoot() => Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + + + + [Fact] + public async Task AccountWorkspace_UsesTheDefaultTokenAsItIsNow_NotAsItWasWhenTheWorkspaceOpened() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + _lister.Repositories = [Repo("alpha")]; + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + await vm.LoadReposCommand.ExecuteAsync(null); + + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); // replaced in Settings + await vm.LoadReposCommand.ExecuteAsync(null); + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Equal(["default-token-old-123", "default-token-new-456"], _lister.Calls.Select(c => c.Token), StringComparer.Ordinal); + Assert.Equal("default-token-new-456", Assert.Single(_git.CloneCalls).Token); + Assert.Equal("default-token-new-456", vm.Token); + } + + + + [Fact] + public async Task AccountWorkspace_WhoseDefaultTokenWasSavedAfterItOpened_LoadsWhenShown() + { + var vault = new InMemoryVault(); + _lister.Repositories = [Repo("alpha")]; + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + Assert.False(vm.LoadRepositoriesIfSaved()); // no token yet: nothing to load with + + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-saved-789"); // first saved in Settings + bool started = vm.LoadRepositoriesIfSaved(); + await WaitUntilAsync(() => vm.HasLoadedRepos, "the load to finish"); + + Assert.True(started); + Assert.Equal("default-token-saved-789", Assert.Single(_lister.Calls).Token); + } + + + + [Fact] + public async Task AccountWorkspace_WhoseDefaultTokenWasRemoved_DoesNotSyncWithTheOldOne_AndSaysWhereToLook() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + _lister.Repositories = [Repo("alpha")]; + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + await vm.LoadReposCommand.ExecuteAsync(null); + + vault.Delete(AppSettings.DefaultTokenVaultId); // removed in Settings + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Empty(_git.CloneCalls); + Assert.Equal(RunResultKind.Error, vm.ResultKind); + Assert.Equal("Account 'Work' uses the default token, but no default token is saved (desktop app: Settings).", vm.ResultMessage); + Assert.Equal("", vm.Token); + } + + + + [Fact] + public async Task AccountWorkspace_WhoseOwnTokenIsGone_FailsTheLoad_NamingTheEntry() + { + var vault = new InMemoryVault(); + Account account = MakeAccount(NewTargetRoot()); + vault.Store(account.Id, "own-token-1234567890"); + WorkspaceViewModel vm = CreateViewModel(account: account, vault: vault); + vault.Delete(account.Id); + + await vm.LoadReposCommand.ExecuteAsync(null); + + Assert.True(vm.LoadErrorOpen); + Assert.Equal($"Account 'Work' has no token in this session's memory (account {account.Id:N}).", vm.LoadErrorMessage); + Assert.Empty(_lister.Calls); + } + + + + [Fact] + public void QuickSync_FollowsTheDefaultToken_OnlyWhileItsBoxHoldsWhatSettingsPutThere() + { + WorkspaceViewModel prefilled = CreateViewModel(); + prefilled.Token = "default-token-old-123"; // what the shell pre-fills from Settings + WorkspaceViewModel typed = CreateViewModel(); + typed.Token = "typed-by-hand-0000000"; + WorkspaceViewModel empty = CreateViewModel(); + + foreach(WorkspaceViewModel vm in new[] { prefilled, typed, empty }) + { + vm.OnDefaultTokenChanged(previous: "default-token-old-123", current: "default-token-new-456"); + } + + Assert.Equal("default-token-new-456", prefilled.Token); + Assert.Equal("typed-by-hand-0000000", typed.Token); // somebody's own token stays + Assert.Equal("", empty.Token); // the box was cleared by hand: it stays cleared + + prefilled.OnDefaultTokenChanged(previous: "default-token-new-456", current: null); + empty.OnDefaultTokenChanged(previous: null, current: "default-token-first-1"); // first saved: nothing was there before + prefilled.RefreshAccountToken(); // Quick Sync has no source to read from: nothing happens + + Assert.Equal("", prefilled.Token); + Assert.Equal("default-token-first-1", empty.Token); + } + + + + [Fact] + public void AccountWorkspace_IsToldWhenTheDefaultTokenChanged_AndTakesItFromTheVault() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + + // What the caller passes is not what an account uses: its source is the vault. + vm.OnDefaultTokenChanged(previous: "something else entirely", current: "also something else"); + + Assert.Equal("default-token-new-456", vm.Token); + } + + + + [Fact] + public void ATokenReadFromItsSource_IsLoggedAsSuch_NotAsTyped_AndNeverByValue() + { + var log = new RecordingActivityLog(); + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + WorkspaceViewModel vm = Track(new WorkspaceViewModel( + _lister, _git, _orgs, handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log, + MakeDefaultTokenAccount(NewTargetRoot()), vault, null, _wsl)); + + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + vm.RefreshAccountToken(); + vm.RefreshAccountToken(); // unchanged: nothing to say + vault.Delete(AppSettings.DefaultTokenVaultId); + vm.RefreshAccountToken(); + + Assert.Equal( + ["Workspace 'Work': its saved token changed (21 characters).", "Workspace 'Work': its token is no longer saved."], + log.Messages.Where(m => m.StartsWith("Workspace 'Work'", StringComparison.Ordinal)), + StringComparer.Ordinal); + Assert.DoesNotContain(log.Messages, m => m.Contains("Token entered", StringComparison.Ordinal)); + Assert.DoesNotContain(log.Messages, m => m.Contains("default-token", StringComparison.Ordinal)); + } } diff --git a/gclo.ViewModels/AccountTokenResolver.cs b/gclo.ViewModels/AccountTokenResolver.cs index 6deaa70..23b56b3 100644 --- a/gclo.ViewModels/AccountTokenResolver.cs +++ b/gclo.ViewModels/AccountTokenResolver.cs @@ -50,14 +50,15 @@ public sealed class AccountTokenResolver /// /// Why returned null for , worded - /// for the user and naming the source, so every "no token" message says what to fix. - /// Never contains a token. + /// for the user and naming the source, so every "no token" message says what to fix: + /// for a token of the account's own, the entry as this vault names it. Never + /// contains a token. /// - public static string MissingDescription(Account account) + public string MissingDescription(Account account) { ArgumentNullException.ThrowIfNull(account); return account.TokenSource == TokenSource.Default ? "uses the default token, but no default token is saved (desktop app: Settings)" - : $"has no token in Windows Credential Manager (entry 'gclo:account:{account.Id:N}')"; + : $"has no token in {_vault.DescribeEntry(account.Id)}"; } } diff --git a/gclo.ViewModels/AccountWizardViewModel.cs b/gclo.ViewModels/AccountWizardViewModel.cs index 8dff12e..719802d 100644 --- a/gclo.ViewModels/AccountWizardViewModel.cs +++ b/gclo.ViewModels/AccountWizardViewModel.cs @@ -210,6 +210,13 @@ public sealed partial class AccountWizardViewModel : ObservableObject /// The token box is enabled only for a token of the account's own. public bool IsOwnTokenSelected => !UseDefaultToken; + /// + /// Whether the choice between the default token and a token of the account's own + /// is shown: when a default token is saved, and also for an account that uses the + /// default token while none is saved, which otherwise could not be given another. + /// + public bool OffersTokenChoice => HasDefaultToken || _existing?.UsesDefaultToken == true; + /// /// as the selected index of a two-item radio group: /// 0 = the default token, 1 = a token of this account's own. @@ -228,6 +235,11 @@ public sealed partial class AccountWizardViewModel : ObservableObject { get { + if(!HasDefaultToken) + { + return "No default token is saved right now. Save one in Settings, or give this account a token of its own."; + } + int others = _store.CountUsingDefaultToken() - (_existing?.UsesDefaultToken == true ? 1 : 0); string shared = others switch { diff --git a/gclo.ViewModels/CredentialManagerVault.cs b/gclo.ViewModels/CredentialManagerVault.cs index f35850c..9b42d86 100644 --- a/gclo.ViewModels/CredentialManagerVault.cs +++ b/gclo.ViewModels/CredentialManagerVault.cs @@ -88,6 +88,13 @@ public sealed class CredentialManagerVault : ITokenVault + /// + /// Names the entry as this process's data root scopes it, which is what Credential Manager lists. + public string DescribeEntry(Guid accountId) + => $"Windows Credential Manager (entry '{TargetName(accountId, _scope)}')"; + + + /// The Credential Manager target name for an account under a scope. internal static string TargetName(Guid accountId, string? scope) => scope is null ? $"gclo:account:{accountId:N}" : $"gclo:{scope}:account:{accountId:N}"; diff --git a/gclo.ViewModels/ITokenVault.cs b/gclo.ViewModels/ITokenVault.cs index 3c54616..5c618b1 100644 --- a/gclo.ViewModels/ITokenVault.cs +++ b/gclo.ViewModels/ITokenVault.cs @@ -32,4 +32,12 @@ public interface ITokenVault /// Removes the account's token. Deleting an absent entry is a no-op. void Delete(Guid accountId); + + + + /// + /// Where this vault keeps the account's token, in words a person can follow to + /// look for it ("Windows Credential Manager (entry '...')"). Never a token. + /// + string DescribeEntry(Guid accountId); } diff --git a/gclo.ViewModels/InMemoryVault.cs b/gclo.ViewModels/InMemoryVault.cs index b343196..0934796 100644 --- a/gclo.ViewModels/InMemoryVault.cs +++ b/gclo.ViewModels/InMemoryVault.cs @@ -37,4 +37,9 @@ public sealed class InMemoryVault : ITokenVault /// public void Delete(Guid accountId) => _tokens.TryRemove(accountId, out _); + + + + /// + public string DescribeEntry(Guid accountId) => $"this session's memory (account {accountId:N})"; } diff --git a/gclo.ViewModels/SyncAllCoordinator.cs b/gclo.ViewModels/SyncAllCoordinator.cs index 6808152..4e95be2 100644 --- a/gclo.ViewModels/SyncAllCoordinator.cs +++ b/gclo.ViewModels/SyncAllCoordinator.cs @@ -78,6 +78,8 @@ public sealed class SyncAllCoordinator(IActivityLog log) continue; } + // A token saved, replaced, or removed since the workspace was opened counts. + workspace.RefreshAccountToken(); if(workspace.UsesDefaultToken && string.IsNullOrEmpty(workspace.Token)) { // The account resolves to the default token and none is saved: a diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index e3838a4..dbfe79b 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -42,6 +42,12 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable private readonly Account? _account; + /// Where an account workspace's token comes from; null for Quick Sync. + private readonly AccountTokenResolver? _tokenResolver; + + /// True while the token is being taken from its source again, which is not the user typing one. + private bool _readingTokenFromItsSource; + private readonly AccountsStore? _accountsStore; private readonly Dictionary _itemsByName = new(StringComparer.OrdinalIgnoreCase); @@ -99,6 +105,10 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable /// organization, target folder, subfolder preference, and concurrency — and pulls the /// token from (the Token setter's org lookup fires /// naturally); finished syncs are then recorded on . + /// The token is read from the vault again whenever it is about to be used + /// (), so one that was replaced or removed since + /// (the default token in Settings, the account's own in the wizard) takes effect + /// without a restart. /// public WorkspaceViewModel( IRepositoryLister? lister = null, @@ -142,7 +152,8 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable MaxConcurrency = account.MaxConcurrency; // The Token setter's existing org lookup fires naturally with the resolved // token: the account's own entry, or the default token from Settings. - Token = tokenVault is null ? "" : new AccountTokenResolver(tokenVault).TryResolve(account) ?? ""; + _tokenResolver = tokenVault is null ? null : new AccountTokenResolver(tokenVault); + Token = _tokenResolver?.TryResolve(account) ?? ""; } _constructed = true; } @@ -175,6 +186,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable /// public bool LoadRepositoriesIfSaved() { + RefreshAccountToken(); if(_account is null || HasLoadedRepos || !LoadReposCommand.CanExecute(null)) { return false; @@ -513,7 +525,13 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // The value itself never reaches the log; its length is enough to tell a // paste from a cleared box when reading back a session (#40). Construction // seeds the property (empty, or the vault token) and is not a user action. - if(_constructed) + if(_readingTokenFromItsSource) + { + _log.Info(value.Length == 0 + ? $"Workspace '{DisplayName}': its token is no longer saved." + : $"Workspace '{DisplayName}': its saved token changed ({value.Length} characters)."); + } + else if(_constructed) { _log.Info(value.Length == 0 ? $"Token cleared in workspace '{DisplayName}'." @@ -524,6 +542,81 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable + /// + /// Takes an account workspace's token from its source again (its own vault entry, + /// or the default token from Settings) and uses it from here on. Called before the + /// token is used and whenever the shell knows a saved token changed. Nothing + /// happens for Quick Sync, whose token is whatever is in its box, or when the + /// source still holds the same token. + /// + public void RefreshAccountToken() + { + if(_account is null || _tokenResolver is null) + { + return; + } + + SetTokenFromItsSource(_tokenResolver.TryResolve(_account) ?? ""); + } + + + + /// + /// Tells the workspace that the default token in Settings was saved, replaced, or + /// removed. An account workspace takes its token from its source again. Quick + /// Sync follows only when its box still holds what Settings put there + /// (, or nothing): a token somebody typed stays. + /// + /// The default token before the change; null when there was none. + /// The default token now; null when there is none. + public void OnDefaultTokenChanged(string? previous, string? current) + { + if(_account is not null) + { + RefreshAccountToken(); + } + else if(string.Equals(Token, previous ?? "", StringComparison.Ordinal)) + { + SetTokenFromItsSource(current ?? ""); + } + } + + + + private void SetTokenFromItsSource(string token) + { + if(string.Equals(token, Token, StringComparison.Ordinal)) + { + return; + } + + _readingTokenFromItsSource = true; + try + { + Token = token; + } + finally + { + _readingTokenFromItsSource = false; + } + } + + + + /// + /// The token to use right now: for an account, read from its source this moment, + /// and a failure that names the source when it holds none. + /// + private string TokenToUseNow() + { + RefreshAccountToken(); + return _account is not null && _tokenResolver is not null && Token.Length == 0 + ? throw new InvalidOperationException($"Account '{_account.Name}' {_tokenResolver.MissingDescription(_account)}.") + : Token.Trim(); + } + + + /// Records that the user opened the target folder from the results bar (#40). public void NoteFolderOpened() => _log.Info($"Opened folder '{EffectiveTargetRoot}'."); @@ -764,11 +857,12 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable try { string organization = Organization.Trim(); + string token = TokenToUseNow(); StatusText = "Loading repositories..."; _log.Info($"Loading repositories for organization '{organization}'."); var descriptors = (await _lister - .ListOrganizationRepositoriesAsync(organization, Token.Trim())) + .ListOrganizationRepositoriesAsync(organization, token)) // Names key the row dictionary; a duplicate (possible from pagination // shifts) must not produce two rows fighting over one folder. .DistinctBy(r => r.Name, StringComparer.OrdinalIgnoreCase) @@ -860,6 +954,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RebuildFilteredRepos(); // every selected row just went back to Queued string targetRoot = EffectiveTargetRoot; + string token = TokenToUseNow(); StatusText = $"Syncing {selected.Count} repositories..."; _log.Info($"Sync started: {selected.Count} repositories into '{targetRoot}'."); @@ -870,7 +965,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable var progress = new FailureLoggingProgress(_progressFactory(HandleProgress), _log); var request = new SyncRequest( - Organization.Trim(), Token.Trim(), targetRoot, MaxConcurrency); + Organization.Trim(), token, targetRoot, MaxConcurrency); IReadOnlyList descriptors = selected.Select(r => r.Descriptor).ToList(); Func, IProgress, CancellationToken, Task> run = diff --git a/gclo/AccountWizardDialog.xaml b/gclo/AccountWizardDialog.xaml index 9319f20..b924a4a 100644 --- a/gclo/AccountWizardDialog.xaml +++ b/gclo/AccountWizardDialog.xaml @@ -65,7 +65,7 @@ AutomationProperties.AutomationId="WizardTokenChoice" Header="Which token should this account use?" SelectedIndex="{x:Bind ViewModel.TokenChoiceIndex, Mode=TwoWay}" - Visibility="{x:Bind local:StatusFormat.VisibleIf(ViewModel.HasDefaultToken), Mode=OneWay}"> + Visibility="{x:Bind local:StatusFormat.VisibleIf(ViewModel.OffersTokenChoice), Mode=OneWay}"> diff --git a/gclo/MainWindow.xaml.cs b/gclo/MainWindow.xaml.cs index 21d58ef..54451ba 100644 --- a/gclo/MainWindow.xaml.cs +++ b/gclo/MainWindow.xaml.cs @@ -1105,8 +1105,15 @@ public sealed partial class MainWindow : Window int concurrencyBefore = _settings.DefaultMaxConcurrency; if(await DialogGuard.ShowAsync(dialog) == ContentDialogResult.Primary) { + // Held only to tell the open workspaces what changed; never logged or stored. + string? defaultTokenBefore = _tokenVault.TryRetrieve(AppSettings.DefaultTokenVaultId); dialog.ApplyAndSave(); // logs what changed ApplySettings(seedQuickSyncConcurrency: _settings.DefaultMaxConcurrency != concurrencyBefore); + string? defaultTokenNow = _tokenVault.TryRetrieve(AppSettings.DefaultTokenVaultId); + foreach(WorkspaceEntry workspace in _workspaces.Values) + { + workspace.ViewModel.OnDefaultTokenChanged(defaultTokenBefore, defaultTokenNow); + } } else { -- 2.54.0 From 20d2f3e3f15f4bbd82a6198cbe1cbd7d238d1cc3 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 16:30:15 -0600 Subject: [PATCH 2/3] fix(app): address the independent review of reading tokens at use - Sync all skips any account whose token is gone, whichever source it comes from, and reports a sync that could not start as failed, not as succeeded. - A sync reads its token before it touches a row: one that cannot start for want of a token leaves failures, their paths and the retry as they were. The WSL clone reads the token at use too. - The organization survives its token going away (the same capture and restore the successful lookup has). - A lookup started by reading the token again says nothing: it does not write the status line or close the error of the load that triggered it. - The token's source is taken from the account as the store has it now. - A credential store that does not answer keeps the token read earlier and is logged; it no longer escapes into Sync all or the shell. - A saved token of nothing but spaces is no token; Quick Sync with an empty box fails in its own words. Refs #157 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/CLI.md | 3 +- gclo.Engine.Tests/SyncAllCoordinatorTests.cs | 40 ++++ gclo.Engine.Tests/WorkspaceViewModelTests.cs | 228 +++++++++++++++++++ gclo.ViewModels/SyncAllCoordinator.cs | 20 +- gclo.ViewModels/WorkspaceViewModel.cs | 88 ++++++- 5 files changed, 362 insertions(+), 17 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index c32d146..01c67c1 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -263,7 +263,8 @@ throttle is reported as such (exit code 4) rather than shrinking the list. An *account* is a saved sync profile: a name, an organization, a target root folder, a parallelism setting, an org-subfolder preference, an optional description, and a token stored in **Windows Credential Manager** (under -`gclo:account:`; the metadata file, +`gclo:account:`, or `gclo::account:` when `GCLO_DATA_DIR` points +the data elsewhere; the metadata file, `%LOCALAPPDATA%\KofTwentyTwo\gclo\accounts.json`, never contains the token). Accounts are shared between the CLI and the desktop app: create them with `gclo accounts add` on a headless mirror server, or in the app's account diff --git a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs index 0bb26c5..bd3f2d7 100644 --- a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs +++ b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs @@ -495,4 +495,44 @@ public sealed class SyncAllCoordinatorTests : IDisposable Assert.Equal(1, result.Skipped); Assert.Empty(_lister.Organizations); // GitHub was never asked with the old token } + + + + // ---------------------------------------------------------------- review of #157: no success for a sync that never ran + + [Fact] + public async Task RunAsync_AnAccountWhoseOwnTokenIsGone_IsSkipped_NotReportedAsSucceeded() + { + WorkspaceViewModel workspace = CreateWorkspace("own"); + await workspace.LoadReposCommand.ExecuteAsync(null); // a loaded table: the sync command can run without a token + _vault.Delete(workspace.AccountId!.Value); + var states = new List(); + _coordinator.AccountStateChanged = (_, state) => states.Add(state); + + 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); + } + + + + [Fact] + public async Task RunAsync_ASyncThatCouldNotStart_IsFailed_NotSucceeded_AndIsNotCountedAsRun() + { + // The target folder cannot be used: a file sits where it should be. + WorkspaceViewModel workspace = CreateWorkspace("blocked"); + Directory.CreateDirectory(_root); + File.WriteAllText(Path.Combine(_root, "blocked"), "a file, not a folder"); + var states = new List(); + _coordinator.AccountStateChanged = (_, state) => states.Add(state); + + SyncAllResult result = await _coordinator.RunAsync([workspace], CancellationToken.None); + + Assert.Equal(RunResultKind.Error, workspace.ResultKind); + Assert.Equal(0, result.Ran); + Assert.Equal(1, result.Skipped); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Running, SyncAllAccountState.Failed], states); + } } diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index 258e7db..c1b0710 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -2549,4 +2549,232 @@ public sealed class WorkspaceViewModelTests : IDisposable Assert.DoesNotContain(log.Messages, m => m.Contains("Token entered", StringComparison.Ordinal)); Assert.DoesNotContain(log.Messages, m => m.Contains("default-token", StringComparison.Ordinal)); } + + + + // ---------------------------------------------------------------- review of #157: what a token that went away must not break + + /// An account workspace with its own token in , loaded with the given repositories. + private async Task<(WorkspaceViewModel Vm, Account Account)> CreateLoadedAccountWorkspaceAsync( + InMemoryVault vault, params RepoDescriptor[] repos) + { + Account account = MakeAccount(NewTargetRoot()); + vault.Store(account.Id, "own-token-1234567890"); + _lister.Repositories = repos; + WorkspaceViewModel vm = CreateViewModel(account: account, vault: vault); + await vm.LoadReposCommand.ExecuteAsync(null); + return (vm, account); + } + + + + [Fact] + public async Task ASyncThatCannotStartForWantOfAToken_LeavesTheRowsAsTheyWere() + { + var vault = new InMemoryVault(); + MakeAlphaFailWithInvalidPaths(); + (WorkspaceViewModel vm, Account account) = await CreateLoadedAccountWorkspaceAsync(vault, Repo("alpha"), Repo("bravo")); + await vm.SyncCommand.ExecuteAsync(null); // alpha fails with its paths, bravo is done + RepoItemViewModel alpha = vm.Repos.Single(r => string.Equals(r.Name, "alpha", StringComparison.Ordinal)); + Assert.True(vm.ResolvePathsCommand.CanExecute(alpha)); + + vault.Delete(account.Id); + await vm.RetryFailedCommand.ExecuteAsync(null); + + Assert.Equal(RunResultKind.Error, vm.ResultKind); + Assert.StartsWith("Account 'Work' has no token in", vm.ResultMessage); + // The failure, its paths, and the way to act on them are all still there. + Assert.Equal(SyncStatus.Failed, alpha.Status); + Assert.NotNull(alpha.Error); + Assert.NotNull(alpha.InvalidPaths); + Assert.True(vm.HasFailedRepos); + Assert.True(vm.ResolvePathsCommand.CanExecute(alpha)); + Assert.True(vm.RetryFailedCommand.CanExecute(null)); + Assert.Single(_git.CloneCalls, c => string.Equals(Path.GetFileName(c.LocalPath), "alpha", StringComparison.Ordinal)); + } + + + + [Fact] + public async Task TheOrganization_SurvivesItsTokenGoingAway_AlsoWhenTheBoxDropsItsTextWithTheList() + { + var vault = new InMemoryVault(); + (WorkspaceViewModel vm, Account account) = await CreateLoadedAccountWorkspaceAsync(vault, Repo("alpha")); + await WaitUntilAsync(() => vm.Organizations.Count > 0 || !vm.IsLoadingOrgs, "the first lookup to settle"); + // What the editable box does: when its list changes it loses its text, and + // the two-way binding writes the empty text back. + vm.Organizations.CollectionChanged += (_, _) => vm.Organization = ""; + + vault.Delete(account.Id); + vm.RefreshAccountToken(); + + Assert.Equal("acme", vm.Organization); + Assert.False(vm.IsTableStale); + } + + + + [Fact] + public async Task TheLookupStartedByReadingTheTokenAgain_SaysNothing_AndLeavesTheLoadsErrorOpen() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + var lookup = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + await WaitUntilAsync(() => vm.StatusText.Length > 0, "the first lookup to settle"); + + // The token is replaced; the load that reads it fails; the lookup it started lands afterwards. + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + _orgs.Handler = (_, _) => lookup.Task; + _lister.ExceptionToThrow = new InvalidOperationException("organization not found"); + await vm.LoadReposCommand.ExecuteAsync(null); + Assert.True(vm.LoadErrorOpen); + string statusAfterTheLoad = vm.StatusText; + lookup.SetResult(["me", "acme"]); + await WaitUntilAsync(() => vm.Organizations.Count == 2, "the lookup to land"); + + Assert.True(vm.LoadErrorOpen, "the error of the load was closed by a lookup nobody asked for"); + Assert.Equal("organization not found", vm.LoadErrorMessage); + Assert.Equal(statusAfterTheLoad, vm.StatusText); + } + + + + [Fact] + public async Task ALookupThatFails_AfterTheTokenWasReadAgain_IsLogged_NotShown() + { + var log = new RecordingActivityLog(); + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + WorkspaceViewModel vm = Track(new WorkspaceViewModel( + _lister, _git, _orgs, handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log, + MakeDefaultTokenAccount(NewTargetRoot()), vault, null, _wsl)); + await WaitUntilAsync(() => vm.StatusText.Length > 0, "the first lookup to settle"); + + _orgs.Handler = (_, _) => Task.FromException>(new InvalidOperationException("rate limited")); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + vm.RefreshAccountToken(); + await WaitUntilAsync( + () => log.Messages.Any(m => m.StartsWith("Organization lookup failed", StringComparison.Ordinal)), "the lookup to fail"); + + Assert.False(vm.LoadErrorOpen); + } + + + + [Fact] + public async Task CloneInWsl_UsesTheTokenAsItIsNow() + { + var vault = new InMemoryVault(); + MakeAlphaFailWithInvalidPaths(); + _wsl.Availability = new WslAvailability(true, "git version 2.53.0"); + (WorkspaceViewModel vm, Account account) = await CreateLoadedAccountWorkspaceAsync(vault, Repo("alpha")); + await vm.SyncCommand.ExecuteAsync(null); + vm.RecoveryInteraction = _ => Task.FromResult(new PathRecoveryDecision.CloneInWsl()); + + vault.Store(account.Id, "own-token-replaced-99"); // replaced from another gclo process + await vm.ResolvePathsCommand.ExecuteAsync(vm.Repos[0]); + + Assert.Equal("own-token-replaced-99", Assert.Single(_wsl.CloneCalls).Token); + } + + + + [Fact] + public void AnAccountSwitchedToATokenOfItsOwn_IsReadFromItsNewSource() + { + string root = NewTargetRoot(); + try + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + var store = new AccountsStore(vault, Path.Combine(root, "store"), new NullActivityLog()); + Account account = MakeDefaultTokenAccount(Path.Combine(root, "repos")); + store.Save(account, token: null); + WorkspaceViewModel vm = CreateViewModel(account: account, vault: vault, store: store); + Assert.Equal("default-token-old-123", vm.Token); + + // The source changes in the store (an edit made elsewhere); the workspace + // still holds the record it was opened with. + store.Save(account with { TokenSource = TokenSource.Own }, "own-token-1234567890"); + vm.RefreshAccountToken(); + + Assert.Equal("own-token-1234567890", vm.Token); + } + finally + { + TryDeleteDirectory(root); + } + } + + + + /// A vault whose reads fail, as Credential Manager's do when it does not answer. + private sealed class UnreadableVault(InMemoryVault inner) : ITokenVault + { + public bool Fails { get; set; } + + public void Store(Guid accountId, string token) => inner.Store(accountId, token); + + public string? TryRetrieve(Guid accountId) + => Fails ? throw new System.ComponentModel.Win32Exception(1312, "A specified logon session does not exist.") : inner.TryRetrieve(accountId); + + public void Delete(Guid accountId) => inner.Delete(accountId); + + public string DescribeEntry(Guid accountId) => inner.DescribeEntry(accountId); + } + + + + [Fact] + public void ACredentialStoreThatDoesNotAnswer_KeepsTheTokenReadEarlier_AndSaysSoInTheLog() + { + var log = new RecordingActivityLog(); + var inner = new InMemoryVault(); + inner.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + var vault = new UnreadableVault(inner); + WorkspaceViewModel vm = Track(new WorkspaceViewModel( + _lister, _git, _orgs, handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log, + MakeDefaultTokenAccount(NewTargetRoot()), vault, null, _wsl)); + + vault.Fails = true; + vm.RefreshAccountToken(); // must not throw: the shell and Sync all call this + vm.OnDefaultTokenChanged(previous: null, current: null); + + Assert.Equal("default-token-old-123", vm.Token); + Assert.Contains(log.Messages, m => m.Contains("its saved token could not be read", StringComparison.Ordinal)); + Assert.DoesNotContain(log.Messages, m => m.Contains("default-token", StringComparison.Ordinal)); + } + + + + [Fact] + public async Task ASavedTokenOfNothingButSpaces_IsNoToken() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, " "); + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + vm.Token = "typed-over-it-000000"; // so the command can be run at all + + await vm.LoadReposCommand.ExecuteAsync(null); + + Assert.True(vm.LoadErrorOpen); + Assert.Equal("Account 'Work' uses the default token, but no default token is saved (desktop app: Settings).", vm.LoadErrorMessage); + Assert.Empty(_lister.Calls); + } + + + + [Fact] + public async Task QuickSync_WhoseTokenBoxWasEmptied_FailsTheSyncInItsOwnWords() + { + WorkspaceViewModel vm = await CreateLoadedViewModelAsync(Repo("alpha")); + vm.OnDefaultTokenChanged(previous: "token-1234567890", current: null); // the pre-filled default was removed + + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Equal(RunResultKind.Error, vm.ResultKind); + Assert.Equal("There is no token: enter one in the token box.", vm.ResultMessage); + Assert.Empty(_git.CloneCalls); + } } diff --git a/gclo.ViewModels/SyncAllCoordinator.cs b/gclo.ViewModels/SyncAllCoordinator.cs index 4e95be2..4851797 100644 --- a/gclo.ViewModels/SyncAllCoordinator.cs +++ b/gclo.ViewModels/SyncAllCoordinator.cs @@ -80,13 +80,14 @@ public sealed class SyncAllCoordinator(IActivityLog log) // A token saved, replaced, or removed since the workspace was opened counts. workspace.RefreshAccountToken(); - if(workspace.UsesDefaultToken && string.IsNullOrEmpty(workspace.Token)) + if(workspace.MissingTokenDescription is { } missingToken) { - // The account resolves to the default token and none is saved: a - // generic auth failure would send the user to the wrong place. + // Whichever source the account's token comes from, it holds none: say + // 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}' — no default token is saved (Settings)."); + _log.Info($"Sync all: skipped '{workspace.DisplayName}' — {missingToken}"); continue; } @@ -116,6 +117,17 @@ public sealed class SyncAllCoordinator(IActivityLog log) _log.Info($"Sync all: syncing '{workspace.DisplayName}'."); await workspace.SyncCommand.ExecuteAsync(null); + if(workspace.ResultKind == RunResultKind.Error) + { + // The sync never ran its repositories (its target folder could not be + // used, its token went away at that moment): that is not a success with + // nothing to report. + skipped++; + Announce(workspace, SyncAllAccountState.Failed); + _log.Info($"Sync all: '{workspace.DisplayName}' could not be synced — {workspace.ResultMessage}"); + continue; + } + ran++; Announce( workspace, diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index dbfe79b..94d46ef 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -556,11 +556,52 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable return; } - SetTokenFromItsSource(_tokenResolver.TryResolve(_account) ?? ""); + string token; + try + { + token = _tokenResolver.TryResolve(AccountAsItIsNow()) ?? ""; + } + catch(Exception ex) when(ex is System.ComponentModel.Win32Exception or IOException or InvalidOperationException) + { + /////////////////////////////////////////////////////////////////////////// + // The credential store did not answer. That says nothing about the // + // token: the one read earlier stays in use, and whatever needs it next // + // succeeds or fails on its own terms. // + /////////////////////////////////////////////////////////////////////////// + _log.Error($"Workspace '{DisplayName}': its saved token could not be read ({ex.Message}); the one read earlier stays in use.", ex); + return; + } + + SetTokenFromItsSource(token); } + /// + /// The account as the store has it now: where its token comes from can have been + /// changed (to the default token, or to a token of its own) since this workspace + /// was opened. The record the workspace was opened with when the store no longer + /// has the account, or when there is no store. + /// + private Account AccountAsItIsNow() + => _accountsStore?.GetAll().FirstOrDefault(a => a.Id == _account!.Id) ?? _account!; + + + + /// + /// Why this workspace has no token to work with, in words that name where to look; + /// null when it has one. For callers that decide before starting anything (Sync + /// all) and want to say why they did not. + /// + public string? MissingTokenDescription + => Token.Trim().Length > 0 + ? null + : _account is not null && _tokenResolver is not null + ? $"Account '{_account.Name}' {_tokenResolver.MissingDescription(AccountAsItIsNow())}." + : "There is no token: enter one in the token box."; + + + /// /// Tells the workspace that the default token in Settings was saved, replaced, or /// removed. An account workspace takes its token from its source again. Quick @@ -610,8 +651,8 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable private string TokenToUseNow() { RefreshAccountToken(); - return _account is not null && _tokenResolver is not null && Token.Length == 0 - ? throw new InvalidOperationException($"Account '{_account.Name}' {_tokenResolver.MissingDescription(_account)}.") + return MissingTokenDescription is { } missing + ? throw new InvalidOperationException(missing) : Token.Trim(); } @@ -667,10 +708,24 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // Captured before any successor can dispose cts; used everywhere below. CancellationToken lookupToken = cts.Token; + /////////////////////////////////////////////////////////////////////////////// + // A lookup started because the saved token was read again is nobody's // + // action: it refreshes the list of organizations and says nothing. The load // + // or sync that read the token owns the status line and the error bar, and a // + // lookup that lands in the middle of it must not write over either. // + /////////////////////////////////////////////////////////////////////////////// + bool quiet = _readingTokenFromItsSource; + string token = Token.Trim(); if(token.Length < 10) { + // The same capture and restore as below: the editable box loses its text + // when its list changes, and an account's organization must survive its + // token going away (it is what the account syncs). + string organizationBeforeClear = Organization; Organizations.Clear(); + Organization = organizationBeforeClear; + OnPropertyChanged(nameof(Organization)); // The newest invocation owns the flag: a canceled predecessor deliberately // leaves it alone, so an early return must clear any spinner it left behind. IsLoadingOrgs = false; @@ -699,12 +754,15 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable } Organization = organizationBeforeRefresh; OnPropertyChanged(nameof(Organization)); - LoadErrorOpen = false; // a lookup that works clears a stale lookup failure - // The production lister always lists the token's own account first, so a - // single entry means no organizations were visible. - StatusText = orgs.Count <= 1 - ? "Only your personal account is visible — add read:org (classic) for organizations, or type an org name manually." - : $"Found {orgs.Count} organizations and accounts."; + if(!quiet) + { + LoadErrorOpen = false; // a lookup that works clears a stale lookup failure + // The production lister always lists the token's own account first, so a + // single entry means no organizations were visible. + StatusText = orgs.Count <= 1 + ? "Only your personal account is visible — add read:org (classic) for organizations, or type an org name manually." + : $"Found {orgs.Count} organizations and accounts."; + } } catch(OperationCanceledException) { @@ -715,7 +773,10 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable if(_orgLoadCts == cts) { _log.Error($"Organization lookup failed: {ex.Message}", ex); - ShowLoadError("Could not list organizations", ex.Message); + if(!quiet) + { + ShowLoadError("Could not list organizations", ex.Message); + } } } finally @@ -938,6 +999,10 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable ActiveRepos.Clear(); try { + // First, before any row is touched: a sync that cannot start for want of a + // token leaves the table (failures, their paths, the retry) as it was. + string token = TokenToUseNow(); + var selected = Repos.Where(r => r.IsSelected).ToList(); foreach(RepoItemViewModel item in selected) { @@ -954,7 +1019,6 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RebuildFilteredRepos(); // every selected row just went back to Queued string targetRoot = EffectiveTargetRoot; - string token = TokenToUseNow(); StatusText = $"Syncing {selected.Count} repositories..."; _log.Info($"Sync started: {selected.Count} repositories into '{targetRoot}'."); @@ -1200,7 +1264,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // The organization the table was loaded for, not whatever the box says now: // the row belongs to that organization, and its folder under ~/gclo should too. WslCloneResult result = await _wsl.CloneAsync( - item.Descriptor.CloneUrl, LoadedOrganization ?? Organization.Trim(), item.Name, Token.Trim(), _lifetimeCts.Token); + item.Descriptor.CloneUrl, LoadedOrganization ?? Organization.Trim(), item.Name, TokenToUseNow(), _lifetimeCts.Token); item.Status = SyncStatus.Done; item.Error = null; -- 2.54.0 From 02bd751357d05febdb98955b652672b54d838b53 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 16:59:20 -0600 Subject: [PATCH 3/3] test(cli): the vault doubles added on main describe their entries Refs #157 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- gclo.Cli.Tests/AccountsCommandTests.cs | 2 ++ gclo.Cli.Tests/SyncCommandTests.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/gclo.Cli.Tests/AccountsCommandTests.cs b/gclo.Cli.Tests/AccountsCommandTests.cs index 23adf18..35f01c8 100644 --- a/gclo.Cli.Tests/AccountsCommandTests.cs +++ b/gclo.Cli.Tests/AccountsCommandTests.cs @@ -764,6 +764,8 @@ public sealed class AccountsCommandTests : IDisposable public void Delete(Guid accountId) { } + + public string DescribeEntry(Guid accountId) => $"a broken vault (account {accountId:N})"; } diff --git a/gclo.Cli.Tests/SyncCommandTests.cs b/gclo.Cli.Tests/SyncCommandTests.cs index e1c8415..8f76845 100644 --- a/gclo.Cli.Tests/SyncCommandTests.cs +++ b/gclo.Cli.Tests/SyncCommandTests.cs @@ -818,6 +818,8 @@ public sealed class SyncCommandTests : IDisposable public void Delete(Guid accountId) { } + + public string DescribeEntry(Guid accountId) => $"an unreadable vault (account {accountId:N})"; } -- 2.54.0