From 441709f1e9c0e71badf0dd431a6c9153d02de838 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 2 Oct 2026 14:08:08 +0200 Subject: [PATCH] Desktop: stop decrypting the whole keychain on a sync pass that changed nothing --- .../ViewModels/VaultViewModel.cs | 42 +++++++++++++++++-- .../ShellFlowTests.cs | 20 +++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs index 84f5757..622ace1 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs @@ -6367,7 +6367,7 @@ internal sealed partial class VaultViewModel( { // Before the sync, so a vault admitted here is one of the vaults that pass then pulls. The // other order would show a newly shared vault as an empty one until the minute after. - await AdmitNewVaultsAsync(server.Account, cancellationToken).ConfigureAwait(true); + var vaultsChanged = await AdmitNewVaultsAsync(server.Account, cancellationToken).ConfigureAwait(true); // Every vault this session can read, not only the one new items are filed into. A team's // vault that never synced would show its hosts exactly once — at the unlock that first @@ -6380,6 +6380,15 @@ internal sealed partial class VaultViewModel( // that had just failed to sync, which is exactly the lie that flag exists to prevent. LastSyncFailed = report.Any(vault => !vault.Succeeded); + if (!vaultsChanged && !MayHaveChangedTheKeychain(report)) + { + // Nothing to redraw, so nothing is decrypted: see MayHaveChangedTheKeychain. The one thing a + // rebuild did that still has to happen is the ago-text on each card, which is a minute older. + RestringLastConnected(); + + return report; + } + await ReloadAsync(cancellationToken).ConfigureAwait(true); // Host key trust arrives with the rest of the vault, and the store the SSH handshake asks holds a @@ -6423,7 +6432,7 @@ internal sealed partial class VaultViewModel( /// this list, and doing that on every quiet pass would rebuild a menu once a minute for nothing. /// /// - private async Task AdmitNewVaultsAsync(IAccountApi api, CancellationToken cancellationToken) + private async Task AdmitNewVaultsAsync(IAccountApi api, CancellationToken cancellationToken) { var before = session.Vaults.Count; @@ -6431,12 +6440,39 @@ internal sealed partial class VaultViewModel( if (admitted == 0 && session.Vaults.Count == before) { - return; + return false; } VaultsChanged?.Invoke(this, EventArgs.Empty); + + return true; } + /// Whether a pass could have left the keychain different from the lists on screen. + /// + /// + /// Every pass used to end by reading the whole keychain back — decrypting every host, key, password, + /// bucket and snippet and rebuilding every list from them — once a minute, whether or not a single item + /// had moved. Almost every pass moves nothing, so almost all of that was spent redrawing what was already + /// drawn. + /// + /// + /// ◆ Conservative on purpose. A failed vault counts as changed, because a pull can apply a page and then + /// fail on the next one, and a report with no counts is no evidence that nothing landed. Log entries do + /// not count — a machine reads back the ones it just wrote, and nothing on these lists is drawn from the + /// log; see . + /// + /// + private static bool MayHaveChangedTheKeychain(IReadOnlyList report) => + report.Any(vault => vault.Report is not { } pass + || pass.PulledItems > 0 + || pass.PushedItems > 0 + || pass.Merged > 0 + || pass.Resurrected > 0 + || pass.DeletesAbandoned > 0 + || pass.Parked > 0 + || pass.ResyncedFromStart); + /// /// Raised when a synchronisation pass found that the vaults this account can reach have changed. /// diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs index bbd04c7..378f9da 100644 --- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs @@ -656,6 +656,26 @@ public sealed partial class ShellFlowTests : IAsyncLifetime vault.IsBusy.ShouldBeFalse("a background pass must never raise the busy flag"); } + /// + /// A pass that moved nothing used to read the whole keychain back anyway — decrypting every item and + /// rebuilding every list, once a minute. A rebuilt list has new row objects in it, so the row still being + /// the same object is the evidence that nothing was read. + /// + [Fact] + public async Task AnAutomaticPassThatMovesNothing_RedrawsNothing() + { + await UnlockedAsync(); + var vault = shell.Vault!; + + await AddHostAsync(vault, "prod-db"); + + var row = vault.Hosts.ShouldHaveSingleItem(); + + await vault.AutoSyncAsync(Token); + + vault.Hosts.ShouldHaveSingleItem().ShouldBeSameAs(row); + } + /// /// The queue has to be arranged by failing the automatic push first. Written the obvious way — add a /// host, then call the pass — this test proved nothing at all: saving pushes, so there was no pending