Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions agent_docs/ui-lifecycle-and-resource-bounds.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 30 additions & 7 deletions src/SkillView.Core/Ui/RemoveScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
Expand All @@ -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,
Expand Down Expand Up @@ -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);
}

Expand All @@ -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)
Expand Down
62 changes: 51 additions & 11 deletions src/SkillView.Core/Ui/SkillViewApp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ private Window BuildUi()
DisposeLogSubscription();
DetachApplicationKeyHandler();
};
UpdateContextBar();
RefreshShellChrome();

if (TuiHelpers.IsWarpTerminal)
{
Expand Down Expand Up @@ -841,7 +841,7 @@ private void ActivateTab(SkillViewTab tab)
}
break;
}
UpdateContextBar();
RefreshShellChrome();
}

internal void LoadSearchResultsForTests(IReadOnlyList<SearchResultSkill> results)
Expand Down Expand Up @@ -2583,6 +2583,7 @@ private async Task<bool> InvokeOwnedAsync(
CancellationCallbackReporter.For(_services.Logger, "owned UI dispatch"));
return await AwaitOwnedDispatchAsync(
app.Invoke,
callback => StageForNextIteration(app, callback),
action,
dispatchLifetime.Token)
.ConfigureAwait(false);
Expand Down Expand Up @@ -2643,28 +2644,40 @@ internal static async Task<bool> AwaitDispatchAsync(

internal static async Task<bool> AwaitOwnedDispatchAsync(
Action<Action> dispatch,
Action<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<bool>(
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;
}
});

Expand All @@ -2678,8 +2691,24 @@ internal static async Task<bool> 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)
{
Expand All @@ -2690,6 +2719,17 @@ internal static async Task<bool> 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,
Expand Down Expand Up @@ -2815,7 +2855,7 @@ internal void ForceActiveTabForTests(SkillViewTab tab)
{
RestoreDiscoverFocus();
}
UpdateContextBar();
RefreshShellChrome();
}

internal void ActivateTabForTests(SkillViewTab tab) => ActivateTab(tab);
Expand Down
Loading
Loading