Show why a review failed to open instead of spinning forever

src/Stampeded/Documents/StartDocumentViewModel.cs

@@ -160,6 +160,15 @@ public sealed partial class StartState : ObservableObject
 	[ObservableProperty]
 	bool isPreparing;
 
+	/// <summary>Why the review that was opening did not open, or empty. While it is set the
+	/// overlay shows this instead of the checklist and stays until it is dismissed: an overlay
+	/// that simply went away would look like an open that was never asked for.</summary>
+	[ObservableProperty]
+	[NotifyPropertyChangedFor(nameof(PrepareFailed))]
+	string prepareError = "";
+
+	public bool PrepareFailed => PrepareError.Length > 0;
+
 	[ObservableProperty]
 	string recentFilter = "";
 
@@ -182,6 +191,9 @@ public class StartDocumentViewModel : Document
 	/// tooltips that name the host.</summary>
 	public string HostName => workspace.HostName;
 	bool openOverviewWhenReady;
+	// Counts the opens started from here, so one that ends after a later one began can tell
+	// the overlay is no longer its to change.
+	int openAttempt;
 
 	public StartState State { get; } = new();
 	public ObservableCollection<string> Recents { get; } = [];
@@ -505,14 +517,10 @@ async Task RefreshSyncStatesAsync(Dictionary<string, PrSummary> prsByBranch)
 	public void OpenPr(PrSummary pr) => OpenPrNumber(pr.Number);
 
 	public void OpenPrNumber(int number)
-	{
-		BeginPreparation();
-		workspace.OpenPrAsync(number).HandleExceptions();
-	}
+		=> OpenWithPreparation($"PR #{number}", () => workspace.OpenPrAsync(number));
 
 	public void OpenBranch(BranchRow row)
 	{
-		BeginPreparation();
 		// A stash reviews as the range from the commit it was taken on to the stash
 		// commit itself, which is exactly the stashed change.
 		var (baseRef, head) = row.IsStash
@@ -522,7 +530,8 @@ public void OpenBranch(BranchRow row)
 		// along, read against the local commits. Opening the same change from the pull request
 		// list reads the pushed head instead - which is the difference between the two lists
 		// once a branch has moved on from what was pushed.
-		workspace.OpenLocalRangeAsync(baseRef, head, row.PrNumber).HandleExceptions();
+		OpenWithPreparation(row.IsStash ? "the stash" : head,
+			() => workspace.OpenLocalRangeAsync(baseRef, head, row.PrNumber));
 	}
 
 	/// <summary>Gives a stash a durable name by pointing a new branch at its commit. The
@@ -969,10 +978,60 @@ public void OpenBranchPrOnHost(BranchRow row)
 	void BeginPreparation()
 	{
 		openOverviewWhenReady = true;
+		State.PrepareError = "";
 		State.IsPreparing = true;
 		UpdatePreparation();
 	}
 
+	/// <summary>
+	/// Opens a review behind the preparation overlay, and owns what the overlay does when the
+	/// open fails: it says why and waits to be dismissed. The overlay covers the window and
+	/// only a finished open takes it down, so a failure that went to the log alone left the
+	/// reader behind a spinner that would never stop.
+	/// </summary>
+	void OpenWithPreparation(string what, Func<Task> open)
+	{
+		int attempt = ++openAttempt;
+		BeginPreparation();
+		RunAsync().HandleExceptions();
+
+		async Task RunAsync()
+		{
+			try
+			{
+				await open();
+			}
+			catch (OperationCanceledException)
+			{
+				// Cancelled by a later open, whose overlay this now is, or by the workspace
+				// going away. Only in the second case is there an overlay left to take down.
+				if (attempt == openAttempt)
+					State.IsPreparing = false;
+			}
+			catch (Exception ex)
+			{
+				CliLog.Write("error", $"open of {what} failed: {ex.Message}");
+				if (attempt != openAttempt)
+					return;
+				openOverviewWhenReady = false;
+				string reason = ex is ToolFailedException failure ? ExternalTool.Explain(failure) : ex.Message;
+				State.PrepareError = $"Could not open {what}: {reason}";
+				State.Status = State.PrepareError;
+				// The error is drawn inside the preparation overlay, which is visible only while
+				// this is true. Usually it still is; "Continue now" pressed before the failure
+				// arrived has cleared it, and the reason would be set with nothing showing it.
+				State.IsPreparing = true;
+			}
+		}
+	}
+
+	/// <summary>Takes down the overlay a failed open left its reason on.</summary>
+	public void DismissPrepareError()
+	{
+		State.PrepareError = "";
+		State.IsPreparing = false;
+	}
+
 	public void ContinueNow()
 	{
 		openOverviewWhenReady = false;

src/Stampeded/MainWindow.axaml

@@ -229,7 +229,17 @@
         <Border Background="{DynamicResource ThemeBackgroundBrush}" CornerRadius="6" Padding="18,14"
                 MaxWidth="560" HorizontalAlignment="Center" VerticalAlignment="Center"
                 BorderBrush="#3794FF" BorderThickness="1">
-          <StackPanel Spacing="6">
+          <Panel>
+          <!-- What a failed open leaves in place of the checklist, until it is dismissed. -->
+          <StackPanel Spacing="10" IsVisible="{Binding StartPage.State.PrepareFailed, FallbackValue=False}">
+            <TextBlock FontWeight="SemiBold" FontSize="14" Foreground="#F85149"
+                       Text="The review could not be opened" />
+            <SelectableTextBlock Text="{Binding StartPage.State.PrepareError}" TextWrapping="Wrap" FontSize="13" />
+            <TextBlock Text="Nothing was opened. The Log pane has the command that failed."
+                       Opacity="0.7" FontSize="12" TextWrapping="Wrap" />
+            <Button Content="Close" Click="OnDismissPrepareError" HorizontalAlignment="Right" />
+          </StackPanel>
+          <StackPanel Spacing="6" IsVisible="{Binding !StartPage.State.PrepareFailed, FallbackValue=True}">
             <StackPanel Orientation="Horizontal" Spacing="12">
               <TextBlock FontWeight="SemiBold" FontSize="14" Text="Preparing the review workspace" />
               <controls:WaveSpinner SquareSize="7" VerticalAlignment="Center" />
@@ -253,6 +263,7 @@
             <Button Content="Continue now" Click="OnContinueFromPrepare" HorizontalAlignment="Right"
                     ToolTip.Tip="Start assessing without waiting; loading continues in the background" />
           </StackPanel>
+          </Panel>
         </Border>
       </Border>
     </Panel>

src/Stampeded/MainWindow.axaml.cs

@@ -391,6 +391,7 @@ static void UsePassBaseline(PassBaselineKind kind)
 	void OnOpenOverview(object? s, EventArgs e) => App.Workspace?.OpenOverview();
 
 	void OnContinueFromPrepare(object? s, RoutedEventArgs e) => App.Workspace?.StartPage?.ContinueNow();
+	void OnDismissPrepareError(object? s, RoutedEventArgs e) => App.Workspace?.StartPage?.DismissPrepareError();
 
 	void OnOpenOnHost(object? s, EventArgs e)
 	{

src/Stampeded/ReviewWorkspace.cs

@@ -399,14 +399,17 @@ public async Task OpenLocalRangeAsync(string baseRef, string headRef, int? prNum
 		}
 		string headSha = await ResolveAsync(headRef, ct);
 		string baseSha = await Git.GetMergeBaseAsync(await ResolveAsync(baseRef, ct), headSha, ct);
-		DirtyWorktreePath = await FindDirtyCheckoutAsync(headRef, ct);
+		// Nothing of the review on screen is touched until the change has been read, so an open
+		// that fails up to here leaves that review exactly as it was.
+		string? dirty = await FindDirtyCheckoutAsync(headRef, ct);
 		var committed = await Git.DiffAsync(baseSha, headSha, ct);
-		var files = DirtyWorktreePath is { } dirty
+		var files = dirty is not null
 			? await Git.DiffWorkingTreeAsync(dirty, baseSha, ct)
 			: committed;
-		UncommittedFileCount = Math.Max(0, files.Count - committed.Count);
 		ct.ThrowIfCancellationRequested();
 
+		DirtyWorktreePath = dirty;
+		UncommittedFileCount = Math.Max(0, files.Count - committed.Count);
 		Scopes.Reset();
 		Reviewers = null;
 		CurrentPr = detail;
@@ -422,9 +425,17 @@ public async Task OpenLocalRangeAsync(string baseRef, string headRef, int? prNum
 		Files = files;
 		changed = ChangedLines.From(files);
 		ReviewReset?.Invoke();
-		Store.OpenLocal(Path.GetFileName(RepoPath), $"{baseRef}..{headRef}", headSha, baseSha);
-		await ApplyReReviewCarryOverAsync(ct);
-		await PinReviewHeadsAsync(ct);
+		try
+		{
+			Store.OpenLocal(Path.GetFileName(RepoPath), $"{baseRef}..{headRef}", headSha, baseSha);
+			await ApplyReReviewCarryOverAsync(ct);
+			await PinReviewHeadsAsync(ct);
+		}
+		catch (Exception ex) when (ex is not OperationCanceledException)
+		{
+			AbandonHalfOpenedReview();
+			throw;
+		}
 		ComputeChurnAsync().HandleExceptions();
 		history.Clear();
 		CloseDocumentsExceptStart();
@@ -459,9 +470,9 @@ public async Task OpenPrAsync(int number)
 		CliLog.Write("action", $"open PR #{number}");
 		PrDetail detail;
 		string headSha, baseSha;
-		Offline = false;
-		OfflineSince = null;
-		snapshot = null;
+		// Held here until the change has been read: an open that fails before that leaves the
+		// review that was on screen exactly as it was, including what it says about being offline.
+		PrSnapshot? openedFrom = null;
 		try
 		{
 			detail = await Host.GetPrAsync(number, ct);
@@ -480,15 +491,16 @@ public async Task OpenPrAsync(int number)
 			detail = cached.Detail;
 			headSha = cached.HeadSha;
 			baseSha = cached.BaseSha;
-			Offline = true;
-			OfflineSince = cached.TakenAt;
-			snapshot = cached;
+			openedFrom = cached;
 		}
-		DirtyWorktreePath = null;
-		UncommittedFileCount = 0;
 		var files = await Git.DiffAsync(baseSha, headSha, ct);
 		ct.ThrowIfCancellationRequested();
 
+		DirtyWorktreePath = null;
+		UncommittedFileCount = 0;
+		Offline = openedFrom is not null;
+		OfflineSince = openedFrom?.TakenAt;
+		snapshot = openedFrom;
 		Scopes.Reset();
 		Reviewers = null;
 		CurrentPr = detail;
@@ -499,9 +511,17 @@ public async Task OpenPrAsync(int number)
 		Files = files;
 		changed = ChangedLines.From(files);
 		ReviewReset?.Invoke();
-		Store.Open(Path.GetFileName(RepoPath), number, headSha, baseSha);
-		await ApplyReReviewCarryOverAsync(ct);
-		await PinReviewHeadsAsync(ct);
+		try
+		{
+			Store.Open(Path.GetFileName(RepoPath), number, headSha, baseSha);
+			await ApplyReReviewCarryOverAsync(ct);
+			await PinReviewHeadsAsync(ct);
+		}
+		catch (Exception ex) when (ex is not OperationCanceledException)
+		{
+			AbandonHalfOpenedReview();
+			throw;
+		}
 		ComputeChurnAsync().HandleExceptions();
 		history.Clear();
 		CloseDocumentsExceptStart();
@@ -1654,6 +1674,18 @@ public void CloseReview()
 		CliLog.Write("action", "review closed");
 	}
 
+	/// <summary>
+	/// The way out of an open that failed after the review's state was already replaced. The
+	/// review that was there is gone and the new one is not whole - commits named, nothing
+	/// loaded behind them - so neither can be left on screen. Closing is the one state that
+	/// describes what the reader has: no review, and the start page to pick one from.
+	/// </summary>
+	void AbandonHalfOpenedReview()
+	{
+		CliLog.Write("action", "open failed part-way; closing what it left behind");
+		CloseReview();
+	}
+
 	/// <summary>What was open before a scope switch or a reload rebuilt the review, so it can
 	/// be put back.</summary>
 	readonly record struct OpenDocuments(IReadOnlyList<string> Ids, string? Active);