diff --git a/src/DodoSSH.Client.Shell/ViewModels/TransfersViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/TransfersViewModel.cs index c63bfd8..4af48e5 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/TransfersViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/TransfersViewModel.cs @@ -620,6 +620,19 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo [ObservableProperty] private string? connectedCipher; + /// + /// Whether there is a live SFTP connection this session would lose by dying — the phone's foreground- + /// service question, not the desktop's. + /// + /// + /// is already the fact that tells a host apart from a bucket, because only + /// a host set it — a bucket is HTTP, per-request, and closes nothing a dying process would have kept + /// open, so it answers false here even while is true. Android reads this to + /// decide whether an idle Files screen with no transfer moving still needs the process kept alive; the + /// desktop has no such question because nothing stops its process for having gone quiet. + /// + internal bool HasLiveFileSession => IsConnected && ConnectedCipher is not null; + /// The accepted host key's algorithm, e.g. ssh-ed25519. See . [ObservableProperty] private string? connectedHostKeyAlgorithm; @@ -745,13 +758,18 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo internal ObservableCollection Transfers { get; } = []; - /// Raised on the UI thread whenever a transfer appears or changes state. + /// + /// Raised on the UI thread whenever a transfer appears or changes state, or a host or bucket connects or + /// disconnects. + /// /// /// For a head that has to tell the operating system what this process is doing — Android's foreground - /// service, which must be up for as long as bytes are moving and down afterwards. An event rather than - /// letting that head watch itself: the collection announces rows arriving and - /// leaving, and the transition that matters most is neither of those but a row going from RUNNING to - /// DONE without moving. + /// service, which must be up for as long as bytes are moving, or a host session sits open, and down + /// afterwards. An event rather than letting that head watch itself: the + /// collection announces rows arriving and leaving, and the transition that matters most is neither of + /// those but a row going from RUNNING to DONE without moving. Connecting and disconnecting are the other + /// two transitions the service cares about — see — and neither touches + /// at all, so they need this same announcement made by hand. /// internal event EventHandler? ActivityChanged; @@ -1075,6 +1093,13 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo // far as the remote's own auth.log is concerned, so a log of ours that omitted it would disagree with // the host's — and anybody comparing the two would be right to believe the host. connected = (ConnectedTo, row.Label, row.EntityId, TimeProvider.System.GetUtcNow()); + + // Raised here rather than from OnIsConnectedChanged, on purpose: IsConnected is set first, above, + // and ConnectedCipher second — a partial method firing off the first assignment would read + // HasLiveFileSession against a ConnectedCipher still holding whatever the previous session left + // there. Only at the end of this method are both facts actually true together. + OnPropertyChanged(nameof(HasLiveFileSession)); + ActivityChanged?.Invoke(this, EventArgs.Empty); } /// @@ -1856,6 +1881,11 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo RemoteTrail.Clear(); SelectedRemoteEntry = null; + // Same ordering reason as the raise at the end of MarkHostConnected: both properties this reads are + // already null above, so the raise belongs after them rather than in OnIsConnectedChanged. This also + // covers OpenBucketAsync, which calls this method first and never itself turns HasLiveFileSession on. + OnPropertyChanged(nameof(HasLiveFileSession)); + ActivityChanged?.Invoke(this, EventArgs.Empty); } /// diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs index 6f24e3a..9cb60be 100644 --- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs @@ -8114,6 +8114,57 @@ public sealed class ShellFlowTests : IAsyncLifetime shell.Transfers.HasConnectedPins.ShouldBeFalse(); } + /// + /// The phone's foreground-service question, proven at the view model rather than through Android: a + /// connect that opens an SFTP session is exactly the transition SessionKeepAlive needs to hear + /// about even when no transfer ever moves — see 's own + /// remark for why the queue's own raise, in OnTransferChanged, cannot cover a connect that never + /// touches Transfers at all. + /// + [Fact] + public async Task ConnectingATransfersHost_RaisesActivityChangedAndTurnsOnHasLiveFileSession() + { + var vault = await ReadyToConnectAsync(); + + shell.Transfers.Attach(vault, knownHosts); + shell.Transfers.SelectedHost = shell.Transfers.Hosts[0]; + + var raised = 0; + shell.Transfers.ActivityChanged += (_, _) => raised++; + + await shell.Transfers.ConnectCommand.ExecuteAsync(null); + + shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status); + shell.Transfers.HasLiveFileSession.ShouldBeTrue(); + raised.ShouldBeGreaterThan(0); + } + + /// + /// The other half: a disconnect is as much a transition the service must hear about as a connect is, + /// because it is the moment the connection promised + /// was open stops being true — and the foreground service would otherwise keep the process alive over a + /// session that has already closed. + /// + [Fact] + public async Task DisconnectingTheTransfersScreen_RaisesActivityChangedAndTurnsOffHasLiveFileSession() + { + var vault = await ReadyToConnectAsync(); + + shell.Transfers.Attach(vault, knownHosts); + shell.Transfers.SelectedHost = shell.Transfers.Hosts[0]; + + await shell.Transfers.ConnectCommand.ExecuteAsync(null); + shell.Transfers.HasLiveFileSession.ShouldBeTrue(); + + var raised = 0; + shell.Transfers.ActivityChanged += (_, _) => raised++; + + await shell.Transfers.DisconnectCommand.ExecuteAsync(null); + + shell.Transfers.HasLiveFileSession.ShouldBeFalse(); + raised.ShouldBeGreaterThan(0); + } + /// /// A bucket is an IRemoteFileStore with no HostSecret underneath it, so there is no /// PinnedPaths to read at all — see 's own remark. @@ -8137,15 +8188,56 @@ public sealed class ShellFlowTests : IAsyncLifetime vault.BucketEditorRegion = "eu-west-1"; await vault.SaveObjectStoreCommand.ExecuteAsync(null); + // Remote is what ConnectAsync branches on, and Attach's RefreshHosts has already auto-selected the + // host ReadyToConnectAsync left in the picker — without this line the command below dialled that + // host, and every assertion here passed only because that host happens to have no pins either. The + // ConnectedTo check is the proof the bucket path was actually taken. + shell.Transfers.Remote = RemoteKind.Bucket; shell.Transfers.SelectedBucket = shell.Transfers.Buckets[0]; await shell.Transfers.ConnectCommand.ExecuteAsync(null); + shell.Transfers.ConnectedTo.ShouldBe("s3://backups"); shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status); shell.Transfers.ConnectedPinnedPaths.ShouldBeEmpty(); shell.Transfers.HasConnectedPins.ShouldBeFalse(); } + /// + /// A bucket is HTTP, per-request, with nothing open that a dying process would lose — see + /// 's own remark. IsConnected alone would have + /// answered this wrongly, which is exactly why the flag reads ConnectedCipher as well: nothing + /// underneath a bucket ever sets it. + /// + [Fact] + public async Task ConnectingABucket_LeavesHasLiveFileSessionOff() + { + var vault = await ReadyToConnectAsync(); + + shell.Transfers.Attach(vault, knownHosts, buckets: new FakeObjectStoreFactory()); + + vault.NewObjectStoreCommand.Execute(null); + vault.BucketEditorLabel = "Backups"; + vault.BucketEditorBucket = "backups"; + vault.BucketEditorAccessKeyId = "AKIAEXAMPLE"; + vault.BucketEditorSecretAccessKey = "a-secret-access-key"; + vault.BucketEditorRegion = "eu-west-1"; + await vault.SaveObjectStoreCommand.ExecuteAsync(null); + + // ReadyToConnectAsync already left a host in the picker, and Attach's own RefreshHosts auto-selects + // it — so without this the CONNECT command below would dial that host rather than open the bucket, + // and a host with no pins would make ConnectedPinnedPathsEmpty-style assertions pass for the wrong + // reason. Remote is what ConnectAsync actually branches on. + shell.Transfers.Remote = RemoteKind.Bucket; + shell.Transfers.SelectedBucket = shell.Transfers.Buckets[0]; + + await shell.Transfers.ConnectCommand.ExecuteAsync(null); + + shell.Transfers.ConnectedTo.ShouldBe("s3://backups", "proof this opened the bucket rather than the host"); + shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status); + shell.Transfers.HasLiveFileSession.ShouldBeFalse(); + } + /// A bucket that opens and lists as empty, so a bucket connect can be proven with no network. private sealed class FakeObjectStoreFactory : IObjectStoreFactory {