fix(ui): gate Mission Control submit-for-review, add retry, fix event leak

Submit for Review is now disabled while a ConPTY pane is starting, has
failed to launch, or has already exited, and MissionControlViewModel
guards against a rapid double-click racing two SubmitTaskForReviewAsync
calls. A failed launch no longer permanently occupies its TaskId dedupe
slot -- a Retry button re-fetches the launch spec and restarts the pane
in place. CloseConPtySession/Dispose now also unsubscribe
SubmitForReviewRequested, matching the other pane event handlers. Also
fixes the pre-existing nullable-dereference warning in
IslandsShellViewModel.SyncInteractiveSessionChips.
This commit is contained in:
mika kuns
2026-08-06 13:42:06 +02:00
parent 0d1e3b9a6f
commit 57c61a2043
9 changed files with 299 additions and 8 deletions
@@ -0,0 +1,138 @@
using ClaudeDo.Ui.Services;
using ClaudeDo.Ui.ViewModels.MissionControl;
using Xunit;
namespace ClaudeDo.Ui.Tests.ViewModels.MissionControl;
public class ConPtyPaneViewModelTests
{
private static ConPtyPaneViewModel NewTaskPane(
Func<Task<TerminalLaunchDescriptor>> descriptorFactory,
string taskId = "t1")
=> new(taskId, "Some Task", descriptorFactory);
private static Task<TerminalLaunchDescriptor> NeverCompletes()
=> new TaskCompletionSource<TerminalLaunchDescriptor>().Task;
private static Task<TerminalLaunchDescriptor> Failing(string message = "boom")
=> Task.FromException<TerminalLaunchDescriptor>(new InvalidOperationException(message));
// ── CanSubmitForReview gating (fix: don't offer Submit for Review on a dead/starting pane) ──
[Fact]
public void CanSubmitForReview_False_WhileStarting()
{
using var pane = NewTaskPane(NeverCompletes);
pane.Start();
Assert.True(pane.Terminal.IsStarting);
Assert.False(pane.SubmitForReviewCommand.CanExecute(null));
}
[Fact]
public void CanSubmitForReview_False_AfterLaunchFailure()
{
using var pane = NewTaskPane(() => Failing());
pane.Start();
Assert.NotNull(pane.Terminal.StartError);
Assert.True(pane.Terminal.HasExited);
Assert.False(pane.SubmitForReviewCommand.CanExecute(null));
}
[Fact]
public void CanSubmitForReview_True_WhenRunning()
{
using var pane = NewTaskPane(NeverCompletes);
SetRunning(pane);
Assert.True(pane.SubmitForReviewCommand.CanExecute(null));
}
[Fact]
public void CanSubmitForReview_False_ForAdHocPane_EvenWhileRunning()
{
using var pane = ConPtyPaneViewModel.CreateAdHoc("Ad hoc", NeverCompletes);
SetRunning(pane);
Assert.False(pane.IsTaskBased);
Assert.False(pane.SubmitForReviewCommand.CanExecute(null));
}
[Fact]
public void CanSubmitForReview_False_WhileSubmitPending()
{
using var pane = NewTaskPane(NeverCompletes);
SetRunning(pane);
Assert.True(pane.SubmitForReviewCommand.CanExecute(null));
pane.IsSubmitPending = true;
Assert.False(pane.SubmitForReviewCommand.CanExecute(null));
pane.IsSubmitPending = false;
Assert.True(pane.SubmitForReviewCommand.CanExecute(null));
}
// ── Retry (fix: a failed launch used to permanently occupy the TaskId dedupe slot) ──────────
[Fact]
public void RetryCommand_Disabled_BeforeAndWhileStarting()
{
using var pane = NewTaskPane(NeverCompletes);
Assert.False(pane.RetryCommand.CanExecute(null));
pane.Start();
Assert.False(pane.RetryCommand.CanExecute(null)); // still starting, no failure yet
}
[Fact]
public void RetryCommand_Enabled_AfterLaunchFailure()
{
using var pane = NewTaskPane(() => Failing());
pane.Start();
Assert.True(pane.RetryCommand.CanExecute(null));
}
[Fact]
public void Retry_ReplacesTerminal_AndRefetchesDescriptor()
{
var callCount = 0;
Func<Task<TerminalLaunchDescriptor>> factory = () =>
{
callCount++;
return callCount == 1 ? Failing() : NeverCompletes();
};
using var pane = NewTaskPane(factory);
pane.Start();
Assert.True(pane.RetryCommand.CanExecute(null));
var terminalBeforeRetry = pane.Terminal;
pane.RetryCommand.Execute(null);
Assert.Equal(2, callCount);
Assert.NotSame(terminalBeforeRetry, pane.Terminal);
Assert.Null(pane.Terminal.StartError);
Assert.False(pane.Terminal.HasExited);
Assert.False(pane.RetryCommand.CanExecute(null));
}
[Fact]
public void Retry_FailsAgain_StillOffersRetry()
{
using var pane = NewTaskPane(() => Failing());
pane.Start();
Assert.True(pane.RetryCommand.CanExecute(null));
pane.RetryCommand.Execute(null);
Assert.NotNull(pane.Terminal.StartError);
Assert.True(pane.RetryCommand.CanExecute(null));
}
private static void SetRunning(ConPtyPaneViewModel pane) => pane.Terminal.IsRunning = true;
}
@@ -3,6 +3,7 @@ using ClaudeDo.Data;
using ClaudeDo.Data.Models;
using ClaudeDo.Ui.Services;
using ClaudeDo.Ui.ViewModels;
using ClaudeDo.Ui.ViewModels.MissionControl;
using Microsoft.EntityFrameworkCore;
using Xunit;
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
@@ -473,6 +474,102 @@ public class MissionControlViewModelTests : IDisposable
Assert.NotNull(error);
}
private sealed class BlockingSubmitWorker : StubWorkerClient
{
public int CallCount { get; private set; }
public readonly TaskCompletionSource<object?> Gate = new();
public override Task SubmitTaskForReviewAsync(string taskId, CancellationToken ct = default)
{
CallCount++;
return Gate.Task;
}
}
[Fact]
public async Task SubmitForReview_RapidDoubleClick_OnlyCallsWorkerOnce()
{
var worker = new BlockingSubmitWorker();
using var vm = BuildVm(worker);
await vm.OpenConPtySessionAsync("t1");
var pane = vm.ConPtySessions[0];
pane.Terminal.IsRunning = true; // simulate a live hand-driven session
// Bypass CanExecute entirely -- Execute(null) is what a genuinely simultaneous
// double-click would still reach even if the button briefly disables itself.
pane.SubmitForReviewCommand.Execute(null);
pane.SubmitForReviewCommand.Execute(null);
Assert.Equal(1, worker.CallCount);
Assert.True(pane.IsSubmitPending);
worker.Gate.SetResult(null);
await Task.Delay(20);
Assert.Empty(vm.ConPtySessions);
}
[Fact]
public async Task SubmitForReview_Failure_ClearsIsSubmitPending_AllowingRetry()
{
var worker = new ThrowingSubmitWorker();
using var vm = BuildVm(worker);
await vm.OpenConPtySessionAsync("t1");
var pane = vm.ConPtySessions[0];
pane.Terminal.IsRunning = true;
string? error = null;
vm.ErrorReported += msg => error = msg;
pane.SubmitForReviewCommand.Execute(null);
Assert.NotNull(error);
Assert.False(pane.IsSubmitPending);
Assert.True(pane.SubmitForReviewCommand.CanExecute(null));
}
private sealed class ThrowingSubmitWorker : StubWorkerClient
{
public override Task SubmitTaskForReviewAsync(string taskId, CancellationToken ct = default)
=> throw new InvalidOperationException("worker unreachable");
}
private static int SubscriberCount(ConPtyPaneViewModel pane, string eventFieldName)
{
var field = typeof(ConPtyPaneViewModel).GetField(eventFieldName,
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
var del = (Delegate?)field!.GetValue(pane);
return del?.GetInvocationList().Length ?? 0;
}
[Fact]
public async Task CloseConPtySession_UnsubscribesSubmitForReviewRequested()
{
var worker = new FakeWorker();
using var vm = BuildVm(worker);
await vm.OpenConPtySessionAsync("t1");
var pane = vm.ConPtySessions[0];
Assert.Equal(1, SubscriberCount(pane, "SubmitForReviewRequested"));
pane.CloseCommand.Execute(null);
Assert.Equal(0, SubscriberCount(pane, "SubmitForReviewRequested"));
Assert.Equal(0, SubscriberCount(pane, "ErrorReported"));
}
[Fact]
public async Task Dispose_UnsubscribesSubmitForReviewRequested()
{
var worker = new FakeWorker();
var vm = BuildVm(worker);
await vm.OpenConPtySessionAsync("t1");
var pane = vm.ConPtySessions[0];
vm.Dispose();
Assert.Equal(0, SubscriberCount(pane, "SubmitForReviewRequested"));
}
[Fact]
public void ToggleLayoutCommand_FlipsIsFocusMode()
{