diff --git a/AGENTS.md b/AGENTS.md index 39a9e74..9a995ca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -311,6 +311,12 @@ the terminal, with both a full-screen TUI and scriptable CLI commands. native inspection. Installed inventory loading and remove preflight own busy state independently; derive their shared spinner/footer from both owners so either completion cannot hide the other operation's feedback. + While preflight runs, mirror its status prominently in the Installed detail + pane as well as the footer, and do not let selection/filter refreshes + overwrite that owned detail state. When preflight ends, restore the selected + detail even when an inventory error remains in the footer. Seed the advanced + wizard from the preflight evaluation without launching another eager scan; + evaluate alternative removal scopes only after the user selects them. - Use `Logger.SubscribeWithReplay` whenever a consumer needs retained history plus live entries. Do not recreate snapshot-then-subscribe logic. Preserve the logger's message/total-character budgets and the file sink's date+size @@ -356,8 +362,12 @@ the terminal, with both a full-screen TUI and scriptable CLI commands. - Awaiting dispatch of an entire synchronous modal needs the stronger owned dispatch contract: cancellation may reject a callback that is still queued, but once that callback starts the background owner must wait until the nested - modal run returns. Do not use a token-short-circuiting cosmetic dispatcher for - this boundary or shutdown can release ownership while the modal is active. + modal run returns. Stage background-completed modal launches for the next + `IApplication.Iteration`; a worker-thread `IApplication.Invoke` callback runs + inside Terminal.Gui's `TimedEvents` lock, and running the modal there + deadlocks any modal worker that reports progress through `Invoke`. Do not use + a token-short-circuiting cosmetic dispatcher for this boundary or shutdown + can release ownership while the modal is active. - CLI entrypoints translate Ctrl+C/external cancellation to exit code 130 and apply bounded per-command deadlines. Cancellation closes admission before startup side effects, synchronous log cleanup, environment probing, or child diff --git a/agent_docs/ui-lifecycle-and-resource-bounds.md b/agent_docs/ui-lifecycle-and-resource-bounds.md index 1ffe052..2feabae 100644 --- a/agent_docs/ui-lifecycle-and-resource-bounds.md +++ b/agent_docs/ui-lifecycle-and-resource-bounds.md @@ -134,10 +134,20 @@ logging, or subprocess adapters. Render that feedback from both owners (with remove preflight taking display precedence), so a load completion cannot hide an active remove and a remove completion cannot hide an active load. + Mirror remove-preflight feedback in the Installed detail pane so it remains + prominent on tall terminals. The advanced wizard must render the seeded + primary evaluation immediately and evaluate other scopes only when selected; + do not launch an eager second pass over all alternatives when the wizard opens. Dispatching the compact/wizard decision uses a start-aware owned callback: cancellation rejects it while it is queued, but cannot complete its owner after the callback has entered the synchronous nested modal loop. This keeps app shutdown from draining the background task before the modal returns. + Any synchronous modal opened after background work must also be staged for + the next `IApplication.Iteration`; worker-thread `IApplication.Invoke` + callbacks run inside Terminal.Gui's `TimedEvents` callback lock, so starting + a nested modal directly there deadlocks its background progress/completion + callbacks when they call `Invoke`. Keep cleanup, installed removal, and repo + discovery on the shared owned-and-next-iteration dispatch path. Partial cancellation counts still trigger inventory invalidation/rescan. `BatchProgressAdapter` retains the latest aggregate snapshot so the outer cancellation boundary cannot replace mid-target file/directory counts with diff --git a/src/SkillView.Core/Ui/RemoveScreen.cs b/src/SkillView.Core/Ui/RemoveScreen.cs index e598b9a..5183118 100644 --- a/src/SkillView.Core/Ui/RemoveScreen.cs +++ b/src/SkillView.Core/Ui/RemoveScreen.cs @@ -100,8 +100,9 @@ public void Show() // Review/Confirm markdown. Pin a percentage size (matching the // explicit-size pattern in InstallConfirmModal) so the Fill content // gets real space; the 25-col help padding still fits comfortably. - Width = Dim.Percent(80), - Height = Dim.Percent(70), + Width = Dim.Percent(72), + Height = Dim.Func(view => CalculateWizardHeight( + view?.SuperView?.Viewport.Height ?? 42)), }; // Using declarations dispose in reverse order. Keep the operation // owner after the wizard declaration so cancellation and worker drain @@ -124,7 +125,7 @@ public void Show() var choicePicker = new OptionSelector { X = 0, - Y = 1, + Y = 2, Width = Dim.Fill(), Height = Dim.Fill(5), Labels = targets.Select(target => target.Title).ToArray(), @@ -140,16 +141,16 @@ public void Show() var evaluationSpinner = new SpinnerView { X = 0, - Y = Pos.AnchorEnd(1), + Y = 1, Visible = false, AutoSpin = false, }; var evaluationStatus = new Label { X = 2, - Y = Pos.AnchorEnd(1), + Y = 1, Width = Dim.Fill(2), - Text = " checking removal safety…", + Text = string.Empty, }; chooseStep.Add( choiceLabel, @@ -557,7 +558,22 @@ async Task RunRemovalAsync( } wizard.Initialized += (_, _) => - StartEvaluation(selectedIndex, findFirstExecutable: true); + { + // Compact-remove preflight already evaluated the primary target. + // Render that result directly instead of launching a redundant + // worker that walks every alternative looking for an executable + // default. Alternatives are evaluated only when the user selects + // them, keeping expensive package validation demand-driven. + if (!ShouldEvaluateInitialSelection(_initialEvaluation) + && evaluations.TryGetValue(selectedIndex, out var initialEvaluation)) + { + ApplyEvaluation(selectedIndex, initialEvaluation); + SetEvaluationBusy(false); + return; + } + + StartEvaluation(selectedIndex, findFirstExecutable: false); + }; _app.Run(wizard); } @@ -572,6 +588,13 @@ internal string BuildSummary() return RemoveWizardContent.BuildReviewMarkdown(Evaluate(target)); } + internal static int CalculateWizardHeight(int availableHeight) => + Math.Clamp(availableHeight * 58 / 100, 16, 28); + + internal static bool ShouldEvaluateInitialSelection( + RemoveTargetEvaluation? initialEvaluation) => + initialEvaluation is null; + private RemoveTargetEvaluation Evaluate( RemoveTarget target, CancellationToken cancellationToken = default) diff --git a/src/SkillView.Core/Ui/SkillViewApp.cs b/src/SkillView.Core/Ui/SkillViewApp.cs index ecef41f..e20f9fe 100644 --- a/src/SkillView.Core/Ui/SkillViewApp.cs +++ b/src/SkillView.Core/Ui/SkillViewApp.cs @@ -559,7 +559,7 @@ private Window BuildUi() DisposeLogSubscription(); DetachApplicationKeyHandler(); }; - UpdateContextBar(); + RefreshShellChrome(); if (TuiHelpers.IsWarpTerminal) { @@ -841,7 +841,7 @@ private void ActivateTab(SkillViewTab tab) } break; } - UpdateContextBar(); + RefreshShellChrome(); } internal void LoadSearchResultsForTests(IReadOnlyList results) @@ -2583,6 +2583,7 @@ private async Task InvokeOwnedAsync( CancellationCallbackReporter.For(_services.Logger, "owned UI dispatch")); return await AwaitOwnedDispatchAsync( app.Invoke, + callback => StageForNextIteration(app, callback), action, dispatchLifetime.Token) .ConfigureAwait(false); @@ -2643,28 +2644,40 @@ internal static async Task AwaitDispatchAsync( internal static async Task AwaitOwnedDispatchAsync( Action dispatch, + Action stageForNextIteration, Action action, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(dispatch); + ArgumentNullException.ThrowIfNull(stageForNextIteration); ArgumentNullException.ThrowIfNull(action); if (cancellationToken.IsCancellationRequested) { return false; } - // 0 = queued, 1 = callback started, 2 = canceled before start. - // Cancellation may reject a queued callback, but once the callback has - // begun the owner must wait for it to return before releasing state the - // callback still uses (notably a nested modal run loop). + // 0 = queued, 1 = staged for the next iteration, 2 = callback started, + // 3 = canceled before start. IApplication.Invoke runs worker-thread + // callbacks as TimedEvents callbacks. Starting a synchronous modal in + // that callback holds Terminal.Gui's timer lock for the entire nested + // run, deadlocking any modal worker that reports progress via Invoke. + // Stage the modal for the next main-loop iteration so the timer callback + // returns and releases that lock first. var dispatchState = 0; var completion = new TaskCompletionSource( TaskCreationOptions.RunContinuationsAsynchronously); using var registration = cancellationToken.Register(() => { - if (Interlocked.CompareExchange(ref dispatchState, 2, 0) == 0) + var state = Volatile.Read(ref dispatchState); + while (state is 0 or 1) { - completion.TrySetResult(false); + var observed = Interlocked.CompareExchange(ref dispatchState, 3, state); + if (observed == state) + { + completion.TrySetResult(false); + return; + } + state = observed; } }); @@ -2678,8 +2691,24 @@ internal static async Task AwaitOwnedDispatchAsync( try { - action(); - completion.TrySetResult(true); + stageForNextIteration(() => + { + if (Interlocked.CompareExchange(ref dispatchState, 2, 1) != 1) + { + completion.TrySetResult(false); + return; + } + + try + { + action(); + completion.TrySetResult(true); + } + catch (Exception ex) + { + completion.TrySetException(ex); + } + }); } catch (Exception ex) { @@ -2690,6 +2719,17 @@ internal static async Task AwaitOwnedDispatchAsync( return await completion.Task.ConfigureAwait(false); } + private static void StageForNextIteration(IApplication app, Action action) + { + void RunOnce(object? sender, EventArgs args) + { + app.Iteration -= RunOnce; + action(); + } + + app.Iteration += RunOnce; + } + private bool ShouldAutoOpenInstalledOnStartup(InventorySnapshot snapshot) => ShouldAutoOpenInstalledOnStartup( snapshot, @@ -2815,7 +2855,7 @@ internal void ForceActiveTabForTests(SkillViewTab tab) { RestoreDiscoverFocus(); } - UpdateContextBar(); + RefreshShellChrome(); } internal void ActivateTabForTests(SkillViewTab tab) => ActivateTab(tab); diff --git a/src/SkillView.Core/Ui/SkillViewWorkflowCoordinator.cs b/src/SkillView.Core/Ui/SkillViewWorkflowCoordinator.cs index cc5065d..f3ae783 100644 --- a/src/SkillView.Core/Ui/SkillViewWorkflowCoordinator.cs +++ b/src/SkillView.Core/Ui/SkillViewWorkflowCoordinator.cs @@ -192,55 +192,57 @@ public void OpenRepoDiscoveryDialog(InstallRequest request) var listing = await _services.InstallService .ListRepoSkillsAsync(ghPath, request.Repo, version: null, request.AllowHiddenDirs, cancellationToken) .ConfigureAwait(false); - _invoke(() => - { - _clearBusy(); - if (!listing.Succeeded) - { - _setStatusWithLevel( - $"could not list skills in {request.Repo} (exit {listing.ExitCode}) — opening install-all", - TuiHelpers.NotificationLevel.Warn); - OpenInstallAllDialog(request); - return; - } - if (listing.Skills.IsDefaultOrEmpty) + await _invokeAsync( + () => { - _setStatusWithLevel( - $"no skills discovered in {request.Repo}", - TuiHelpers.NotificationLevel.Warn); - return; - } + _clearBusy(); + if (!listing.Succeeded) + { + _setStatusWithLevel( + $"could not list skills in {request.Repo} (exit {listing.ExitCode}) — opening install-all", + TuiHelpers.NotificationLevel.Warn); + OpenInstallAllDialog(request); + return; + } + if (listing.Skills.IsDefaultOrEmpty) + { + _setStatusWithLevel( + $"no skills discovered in {request.Repo}", + TuiHelpers.NotificationLevel.Warn); + return; + } - var picker = new RepoSkillPickerModal( - app, - _services.InstallService, - _services.Logger, - ghPath, - request, - listing.Skills); - var result = picker.Show(); - if (result.Outcome == RepoSkillPickerModal.Outcome.Installed && result.InstalledCount > 0) - { - _services.ListAdapter.Invalidate(); - var suffix = result.FailedCount > 0 ? $" ({result.FailedCount} failed)" : string.Empty; - _setStatusWithLevel( - $"installed {result.InstalledCount} skill(s) from {request.Repo}{suffix} — rescanning…", - result.FailedCount > 0 ? TuiHelpers.NotificationLevel.Warn : TuiHelpers.NotificationLevel.Success); - QueueInventoryRescan(report, successStatus: "installed — inventory now {0} skill(s)"); - } - else if (result.Outcome == RepoSkillPickerModal.Outcome.Failed) - { - _setStatusWithLevel( - $"install failed — {TuiHelpers.ErrorSnippet(result.FirstError)}".TrimEnd(), - TuiHelpers.NotificationLevel.Error); - } - else - { - // Cancelled (or nothing selected): clear the lingering - // "discovering skills in …" busy status the spinner left. - _setStatus($"{request.Repo}: no skills installed"); - } - }); + var picker = new RepoSkillPickerModal( + app, + _services.InstallService, + _services.Logger, + ghPath, + request, + listing.Skills); + var result = picker.Show(); + if (result.Outcome == RepoSkillPickerModal.Outcome.Installed && result.InstalledCount > 0) + { + _services.ListAdapter.Invalidate(); + var suffix = result.FailedCount > 0 ? $" ({result.FailedCount} failed)" : string.Empty; + _setStatusWithLevel( + $"installed {result.InstalledCount} skill(s) from {request.Repo}{suffix} — rescanning…", + result.FailedCount > 0 ? TuiHelpers.NotificationLevel.Warn : TuiHelpers.NotificationLevel.Success); + QueueInventoryRescan(report, successStatus: "installed — inventory now {0} skill(s)"); + } + else if (result.Outcome == RepoSkillPickerModal.Outcome.Failed) + { + _setStatusWithLevel( + $"install failed — {TuiHelpers.ErrorSnippet(result.FirstError)}".TrimEnd(), + TuiHelpers.NotificationLevel.Error); + } + else + { + // Cancelled (or nothing selected): clear the lingering + // "discovering skills in …" busy status the spinner left. + _setStatus($"{request.Repo}: no skills installed"); + } + }, + cancellationToken).ConfigureAwait(false); }, "discover"); } @@ -262,33 +264,35 @@ public void ShowCleanupScreen() snapshot.ScannedRoots, options: null, cancellationToken); - _invoke(() => - { - _clearBusy(); - var screen = new CleanupScreen( - app, - _services.RemoveService, - _services.Logger, - candidates, - snapshot.ScannedRoots, - snapshot.Skills); - screen.Show(); - if (screen.RemovedCount > 0 - || screen.RemovedFileCount > 0 - || screen.RemovedDirectoryCount > 0 - || screen.IgnoredCount > 0) + await _invokeAsync( + () => { - _services.ListAdapter.Invalidate(); - // Cleanup can remove or ignore skills while the user is - // sitting on the Installed/Updates list. The screen runs as - // a modal overlay (not a tab switch), so nothing reloads the - // underlying tab on its own — refresh it here so the list - // reflects what cleanup just changed. - _refreshActiveTab(); - } + _clearBusy(); + var screen = new CleanupScreen( + app, + _services.RemoveService, + _services.Logger, + candidates, + snapshot.ScannedRoots, + snapshot.Skills); + screen.Show(); + if (screen.RemovedCount > 0 + || screen.RemovedFileCount > 0 + || screen.RemovedDirectoryCount > 0 + || screen.IgnoredCount > 0) + { + _services.ListAdapter.Invalidate(); + // Cleanup can remove or ignore skills while the user is + // sitting on the Installed/Updates list. The screen runs as + // a modal overlay (not a tab switch), so nothing reloads the + // underlying tab on its own — refresh it here so the list + // reflects what cleanup just changed. + _refreshActiveTab(); + } - _setStatus($"cleanup: removed {screen.RemovedCount}, ignored {screen.IgnoredCount}"); - }); + _setStatus($"cleanup: removed {screen.RemovedCount}, ignored {screen.IgnoredCount}"); + }, + cancellationToken).ConfigureAwait(false); }, "cleanup"); } diff --git a/src/SkillView.Core/Ui/StatusStripView.cs b/src/SkillView.Core/Ui/StatusStripView.cs index b5615a2..da7108e 100644 --- a/src/SkillView.Core/Ui/StatusStripView.cs +++ b/src/SkillView.Core/Ui/StatusStripView.cs @@ -65,6 +65,8 @@ internal void SetBusy(bool isBusy) internal string LeftBadgesForTests => _leftBadges; + internal IReadOnlyList HintsForTests => _hints; + protected override bool OnDrawingContent(DrawContext? context) { var width = Viewport.Width; diff --git a/src/SkillView.Core/Ui/Tabs/InstalledTabView.cs b/src/SkillView.Core/Ui/Tabs/InstalledTabView.cs index f13fe43..674a4c4 100644 --- a/src/SkillView.Core/Ui/Tabs/InstalledTabView.cs +++ b/src/SkillView.Core/Ui/Tabs/InstalledTabView.cs @@ -189,7 +189,7 @@ internal InstalledTabView( var row = _table.GetSelectedRow(); if (row >= 0 && row < _rows.Count) { - _detail.Text = RenderDetail(_rows[row]); + RefreshSelectedDetail(); _onStateChange?.Invoke(); } }; @@ -555,9 +555,7 @@ private void RefreshAll() RecomputeColumnWidths(); BuildTableSource(); RefreshOperationStatus(); - _detail.Text = _rows.Count == 0 - ? "(no matches)" - : RenderDetail(_rows[Math.Clamp(_table.GetSelectedRow(), 0, _rows.Count - 1)]); + RefreshSelectedDetail(); _onStateChange?.Invoke(); } @@ -749,6 +747,9 @@ private void RefreshOperationStatus() { SetSpinnerActive(true); _footer.Text = " checking removal safety…"; + _detail.Text = "## Checking removal safety…\n\n" + + "SkillView is validating the selected skill and its related installs. " + + "You can leave this tab to cancel."; return; } @@ -760,6 +761,7 @@ private void RefreshOperationStatus() } SetSpinnerActive(false); + RefreshSelectedDetail(); if (_idleStatusMessage is not null) { _footer.Text = _idleStatusMessage; @@ -769,6 +771,18 @@ private void RefreshOperationStatus() RefreshFooter(); } + private void RefreshSelectedDetail() + { + if (_snapshot is null || Volatile.Read(ref _removeActive) != 0) + { + return; + } + + _detail.Text = _rows.Count == 0 + ? "(no matches)" + : RenderDetail(_rows[Math.Clamp(_table.GetSelectedRow(), 0, _rows.Count - 1)]); + } + private void SetSpinnerActive(bool active) { _spinner.Visible = active; @@ -804,6 +818,8 @@ private void SetSpinnerActive(bool active) internal void SendFilterKeyForTests(Key key) => _filterField.NewKeyDownEvent(key); + internal void SetFilterTextForTests(string text) => _filterField.Text = text; + internal bool RemoveActiveForTests => Volatile.Read(ref _removeActive) != 0; internal bool LoadActiveForTests => Volatile.Read(ref _loadActive) != 0; @@ -812,5 +828,7 @@ private void SetSpinnerActive(bool active) internal string FooterTextForTests => _footer.Text.ToString(); + internal string DetailTextForTests => _detail.Text.ToString(); + internal bool TryStartSelectedRemoveForTests() => TryStartSelectedRemove(); } diff --git a/tests/SkillView.Tests/Ui/InstalledTabViewTests.cs b/tests/SkillView.Tests/Ui/InstalledTabViewTests.cs index f157e48..06e61d2 100644 --- a/tests/SkillView.Tests/Ui/InstalledTabViewTests.cs +++ b/tests/SkillView.Tests/Ui/InstalledTabViewTests.cs @@ -48,12 +48,91 @@ public async Task InventoryCompletion_DoesNotClearActiveRemoveFeedback() Assert.False(view.LoadActiveForTests); Assert.True(view.SpinnerVisibleForTests); Assert.Equal(" checking removal safety…", view.FooterTextForTests); + Assert.Contains("## Checking removal safety…", view.DetailTextForTests); view.CancelPendingWork(); releaseRemove.TrySetResult(); await operations["installed.remove"]; } + [Fact] + public async Task SelectionAndFilterChanges_DoNotClearActiveRemoveFeedback() + { + var removeStarted = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var releaseRemove = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var operations = new ConcurrentDictionary(); + var view = new InstalledTabView( + runOnUi: action => + { + action(); + return Task.CompletedTask; + }, + runTask: (operation, name) => operations[name] = operation(), + snapshotLoader: static _ => Task.FromResult(InventorySnapshot.Empty), + onRemove: async (_, _, cancellationToken) => + { + removeStarted.TrySetResult(); + await releaseRemove.Task.WaitAsync(cancellationToken); + }, + onLeaveTab: static () => { }, + onGoToSearch: static () => { }); + view.LoadSeeded(SnapshotWithSkills("frontend-design", "backend-design")); + + Assert.True(view.TryStartSelectedRemoveForTests()); + await removeStarted.Task.WaitAsync(TestContext.Current.CancellationToken); + + view.SetSelectedRowForTests(1); + Assert.Contains("## Checking removal safety…", view.DetailTextForTests); + + view.SetFilterTextForTests("backend"); + Assert.Contains("## Checking removal safety…", view.DetailTextForTests); + + view.CancelPendingWork(); + releaseRemove.TrySetResult(); + await operations["installed.remove"]; + } + + [Fact] + public async Task RemoveCompletion_RestoresDetailWhenInventoryFailureRemainsInFooter() + { + var removeStarted = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var operations = new ConcurrentDictionary(); + var view = new InstalledTabView( + runOnUi: action => + { + action(); + return Task.CompletedTask; + }, + runTask: (operation, name) => operations[name] = operation(), + snapshotLoader: static _ => throw new InvalidOperationException("scan failed"), + onRemove: async (_, _, cancellationToken) => + { + removeStarted.TrySetResult(); + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + }, + onLeaveTab: static () => { }, + onGoToSearch: static () => { }); + view.LoadSeeded(SnapshotWithSkill( + name: "frontend-design", + packageSource: "owner/repo", + agentIds: ["claude"])); + + Assert.True(view.TryStartSelectedRemoveForTests()); + await removeStarted.Task.WaitAsync(TestContext.Current.CancellationToken); + await view.LoadAsync(); + Assert.Contains("## Checking removal safety…", view.DetailTextForTests); + + view.CancelPendingWork(); + await operations["installed.remove"]; + + Assert.Equal(" inventory load failed: scan failed", view.FooterTextForTests); + Assert.Contains("# frontend-design", view.DetailTextForTests); + Assert.DoesNotContain("Checking removal safety", view.DetailTextForTests); + } + [Fact] public async Task RemoveCompletion_DoesNotClearActiveInventoryFeedback() { @@ -402,6 +481,13 @@ public void Constructor_ShowsInstalledFilterGuideAndCompactLegend() onLeaveTab: static () => { }, onGoToSearch: static () => { }); + private static InventorySnapshot SnapshotWithSkills(string firstName, string secondName) + { + var first = SnapshotWithSkill(firstName, "owner/repo", "claude"); + var second = SnapshotWithSkill(secondName, "owner/repo", "claude"); + return first with { Skills = first.Skills.Add(second.Skills[0]) }; + } + private static InventorySnapshot SnapshotWithSkill( string name, string? packageSource, diff --git a/tests/SkillView.Tests/Ui/RemoveScreenTests.cs b/tests/SkillView.Tests/Ui/RemoveScreenTests.cs index 27971f0..3c2adb9 100644 --- a/tests/SkillView.Tests/Ui/RemoveScreenTests.cs +++ b/tests/SkillView.Tests/Ui/RemoveScreenTests.cs @@ -8,6 +8,29 @@ namespace SkillView.Tests.Ui; public sealed class RemoveScreenTests { + [Theory] + [InlineData(24, 16)] + [InlineData(42, 24)] + [InlineData(80, 28)] + public void CalculateWizardHeight_IsCompactButRetainsUsableMinimum( + int availableHeight, + int expected) + { + Assert.Equal(expected, RemoveScreen.CalculateWizardHeight(availableHeight)); + } + + [Fact] + public void SeededPrimaryEvaluation_DoesNotRequestAnotherEagerPass() + { + var path = Path.Combine(Path.GetTempPath(), "skillview-seeded-remove", "demo"); + var evaluation = Evaluation( + RemoveTargetKind.CurrentInstall, + Validation(path)); + + Assert.False(RemoveScreen.ShouldEvaluateInitialSelection(evaluation)); + Assert.True(RemoveScreen.ShouldEvaluateInitialSelection(null)); + } + [Fact] public void HasSameSafetyContract_RejectsChangedDirectoryOrLinkAuthority() { diff --git a/tests/SkillView.Tests/Ui/SkillViewAppTests.cs b/tests/SkillView.Tests/Ui/SkillViewAppTests.cs index a809d92..8d9e7f3 100644 --- a/tests/SkillView.Tests/Ui/SkillViewAppTests.cs +++ b/tests/SkillView.Tests/Ui/SkillViewAppTests.cs @@ -549,6 +549,7 @@ public async Task AwaitOwnedDispatchAsync_CanceledLifetimeRejectsQueuedCallback( var dispatch = SkillViewApp.AwaitOwnedDispatchAsync( callback => queued = callback, + callback => callback(), () => callbackRan = true, lifetime.Token); Assert.NotNull(queued); @@ -570,6 +571,7 @@ public async Task AwaitOwnedDispatchAsync_CancellationWaitsForStartedCallback() Action? queued = null; var dispatch = SkillViewApp.AwaitOwnedDispatchAsync( callback => queued = callback, + callback => callback(), () => { entered.Set(); @@ -600,6 +602,103 @@ public async Task AwaitOwnedDispatchAsync_CancellationWaitsForStartedCallback() Assert.True(await dispatch); } + [Fact] + public async Task AwaitOwnedDispatchAsync_StagesModalAfterDispatchCallbackReturns() + { + Action? queued = null; + Action? nextIteration = null; + var callbackRan = false; + + var dispatch = SkillViewApp.AwaitOwnedDispatchAsync( + callback => queued = callback, + callback => nextIteration = callback, + () => callbackRan = true, + TestContext.Current.CancellationToken); + + Assert.NotNull(queued); + queued(); + + Assert.NotNull(nextIteration); + Assert.False(callbackRan); + Assert.False(dispatch.IsCompleted); + + nextIteration(); + + Assert.True(await dispatch); + Assert.True(callbackRan); + } + + [Fact] + public async Task AwaitOwnedDispatchAsync_ReleasesTimedEventsLockBeforeModalWork() + { + var timedEvents = new TimedEvents(); + Action? nextIteration = null; + var workerQueuedProgress = false; + + var dispatch = SkillViewApp.AwaitOwnedDispatchAsync( + callback => timedEvents.Add( + TimeSpan.Zero, + () => + { + callback(); + return false; + }), + callback => nextIteration = callback, + () => + { + var worker = Task.Factory.StartNew( + () => + { + timedEvents.Add(TimeSpan.Zero, () => false); + workerQueuedProgress = true; + }, + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default); + + Assert.True(worker.Wait( + TimeSpan.FromSeconds(10), + TestContext.Current.CancellationToken)); + }, + TestContext.Current.CancellationToken); + + timedEvents.RunTimers(); + + Assert.NotNull(nextIteration); + Assert.False(dispatch.IsCompleted); + + nextIteration(); + + Assert.True(await dispatch); + Assert.True(workerQueuedProgress); + } + + [Fact] + public async Task AwaitOwnedDispatchAsync_CancellationRejectsStagedModal() + { + using var lifetime = CancellationTokenSource.CreateLinkedTokenSource( + TestContext.Current.CancellationToken); + Action? queued = null; + Action? nextIteration = null; + var callbackRan = false; + + var dispatch = SkillViewApp.AwaitOwnedDispatchAsync( + callback => queued = callback, + callback => nextIteration = callback, + () => callbackRan = true, + lifetime.Token); + + Assert.NotNull(queued); + queued(); + Assert.NotNull(nextIteration); + + lifetime.Cancel(); + + Assert.False(await dispatch); + nextIteration(); + Assert.False(callbackRan); + } + [Fact] public void CancelingPreview_RestoresStillRunningSearchBusyState() { @@ -1100,6 +1199,25 @@ public void ContextBar_UpdatesWhenTabChanges() Assert.Equal(SkillViewTab.Discover, app.ActiveTabForTests); } + [Fact] + public void StatusStrip_UpdatesShortcutsWhenTabChanges() + { + var app = CreateApp(); + using var window = app.BuildUiForTests(); + + app.ForceActiveTabForTests(SkillViewTab.Installed); + var installedHints = app.StatusStripForTests!.HintsForTests; + Assert.Contains(installedHints, hint => hint is { Key: "f", Label: "Filter" }); + Assert.Contains(installedHints, hint => hint is { Key: "x", Label: "Remove" }); + + app.ForceActiveTabForTests(SkillViewTab.Changes); + var changesHints = app.StatusStripForTests.HintsForTests; + Assert.Contains(changesHints, hint => hint is { Key: "Enter", Label: "Open" }); + Assert.Contains(changesHints, hint => hint is { Key: "c", Label: "Cleanup" }); + Assert.Contains(changesHints, hint => hint is { Key: "d", Label: "Doctor" }); + Assert.DoesNotContain(changesHints, hint => hint.Key is "f" or "x"); + } + [Fact] public void InstalledSelectionChange_KeepsShellChromeFocusedOnFiltersInsteadOfRepeatingDetailMetadata() {