Merge claudedo/c52ba287db6e4a7c9bac38c30bcc21ec
This commit is contained in:
@@ -122,6 +122,15 @@ the host wires handlers and then calls `Start()`. So the tile appears **immediat
|
||||
spinner while the worker is still preparing the worktree. A failed launch keeps the tile with an
|
||||
inline error banner instead of the tile never appearing.
|
||||
|
||||
`SubmitForReviewCommand.CanExecute` also gates on `Terminal.IsStarting` / `StartError` /
|
||||
`HasExited` (not just `IsTaskBased`) — a starting or dead pane can't offer a review it would only
|
||||
have the worker reject, and `MissionControlViewModel.OnPaneSubmitForReview` sets the pane's
|
||||
`IsSubmitPending` flag for the duration of the round trip so a rapid double-click can't race two
|
||||
`SubmitTaskForReviewAsync` calls. A failed launch also offers `RetryCommand` (visible whenever
|
||||
`HasExited && StartError != null`) — it swaps in a fresh `InteractiveTerminalViewModel` and calls
|
||||
`Start()` again on the **same** pane/`TaskId` dedupe slot, since `PtyTerminalSession` throws on a
|
||||
second `StartAsync` call and can't be restarted in place.
|
||||
|
||||
### ⚠️ Gotcha: the terminal library kills its child on visual-tree detach
|
||||
|
||||
`Iciclecreek.Avalonia.Terminal`'s `TerminalView.OnDetachedFromLogicalTree` calls
|
||||
|
||||
@@ -285,6 +285,8 @@
|
||||
"submitForReviewFailed": "Einreichen zum Review fehlgeschlagen: {0}",
|
||||
"submitForReview": "Zum Review einreichen",
|
||||
"submitForReviewTip": "Diesen Worktree committen und den Task ins Review bringen, damit er gemergt werden kann",
|
||||
"retry": "Erneut versuchen",
|
||||
"retryTip": "Diese Sitzung erneut starten",
|
||||
"planningTitleSuffix": " (Planung)",
|
||||
"question": {
|
||||
"title": "Claude fragt nach",
|
||||
|
||||
@@ -285,6 +285,8 @@
|
||||
"submitForReviewFailed": "Couldn't submit for review: {0}",
|
||||
"submitForReview": "Submit for review",
|
||||
"submitForReviewTip": "Commit this worktree and move the task to review so it can be merged",
|
||||
"retry": "Retry",
|
||||
"retryTip": "Try launching this session again",
|
||||
"planningTitleSuffix": " (Planning)",
|
||||
"question": {
|
||||
"title": "Claude is asking",
|
||||
|
||||
@@ -473,7 +473,7 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable
|
||||
|
||||
private void SyncInteractiveSessionChips()
|
||||
{
|
||||
if (MissionControl is null) return;
|
||||
if (MissionControl is null || Tasks is null) return;
|
||||
Tasks.SyncInteractiveSessions(
|
||||
MissionControl.ConPtySessions
|
||||
.Select(s => s.TaskId)
|
||||
|
||||
@@ -20,7 +20,11 @@ public sealed partial class ConPtyPaneViewModel : ViewModelBase, IMissionControl
|
||||
|
||||
[ObservableProperty] private string _displayTitle;
|
||||
|
||||
public InteractiveTerminalViewModel Terminal { get; } = new();
|
||||
[ObservableProperty] private InteractiveTerminalViewModel _terminal = new();
|
||||
|
||||
/// <summary>Set by the host (Mission Control) while a submit-for-review round trip is in
|
||||
/// flight, so a rapid double-click can't race two submissions for the same task.</summary>
|
||||
[ObservableProperty] private bool _isSubmitPending;
|
||||
|
||||
/// <summary>Raised when the terminal failed to start — the host surfaces this via the footer error strip.</summary>
|
||||
public event Action<string>? ErrorReported;
|
||||
@@ -76,12 +80,21 @@ public sealed partial class ConPtyPaneViewModel : ViewModelBase, IMissionControl
|
||||
{
|
||||
if (e.PropertyName == nameof(InteractiveTerminalViewModel.StartError) && Terminal.StartError is { Length: > 0 } error)
|
||||
ErrorReported?.Invoke(error);
|
||||
|
||||
if (e.PropertyName is nameof(InteractiveTerminalViewModel.IsStarting)
|
||||
or nameof(InteractiveTerminalViewModel.StartError)
|
||||
or nameof(InteractiveTerminalViewModel.HasExited))
|
||||
{
|
||||
SubmitForReviewCommand.NotifyCanExecuteChanged();
|
||||
RetryCommand.NotifyCanExecuteChanged();
|
||||
}
|
||||
}
|
||||
|
||||
[RelayCommand]
|
||||
private void Close() => CloseRequested?.Invoke(this);
|
||||
|
||||
private bool CanSubmitForReview() => IsTaskBased;
|
||||
private bool CanSubmitForReview() =>
|
||||
IsTaskBased && !IsSubmitPending && !Terminal.IsStarting && Terminal.StartError is null && !Terminal.HasExited;
|
||||
|
||||
[RelayCommand(CanExecute = nameof(CanSubmitForReview))]
|
||||
private void SubmitForReview()
|
||||
@@ -89,6 +102,22 @@ public sealed partial class ConPtyPaneViewModel : ViewModelBase, IMissionControl
|
||||
if (TaskId is { } id) SubmitForReviewRequested?.Invoke(id);
|
||||
}
|
||||
|
||||
partial void OnIsSubmitPendingChanged(bool value) => SubmitForReviewCommand.NotifyCanExecuteChanged();
|
||||
|
||||
// A launch failure permanently occupies the TaskId dedupe slot unless the user can retry —
|
||||
// re-opening the same task would otherwise just re-focus a dead tile.
|
||||
private bool CanRetry() => Terminal.HasExited && Terminal.StartError is not null;
|
||||
|
||||
[RelayCommand(CanExecute = nameof(CanRetry))]
|
||||
private void Retry()
|
||||
{
|
||||
Terminal.PropertyChanged -= OnTerminalPropertyChanged;
|
||||
Terminal.Dispose();
|
||||
Terminal = new InteractiveTerminalViewModel();
|
||||
Terminal.PropertyChanged += OnTerminalPropertyChanged;
|
||||
Start();
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
Terminal.PropertyChanged -= OnTerminalPropertyChanged;
|
||||
|
||||
@@ -330,16 +330,22 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
|
||||
|
||||
// Submit a task's hand-driven ConPTY work for review, then close the pane (the interactive
|
||||
// session is finished). The worker commits the worktree and moves the task to WaitingForReview.
|
||||
// Guarded by the pane's IsSubmitPending flag — a rapid double-click would otherwise race two
|
||||
// SubmitTaskForReviewAsync calls, with the loser flashing a spurious footer error.
|
||||
private async void OnPaneSubmitForReview(string taskId)
|
||||
{
|
||||
if (ConPtySessions.FirstOrDefault(s => s.TaskId == taskId) is not { } pane || pane.IsSubmitPending)
|
||||
return;
|
||||
|
||||
pane.IsSubmitPending = true;
|
||||
try
|
||||
{
|
||||
await _worker.SubmitTaskForReviewAsync(taskId);
|
||||
if (ConPtySessions.FirstOrDefault(s => s.TaskId == taskId) is { } pane)
|
||||
CloseConPtySession(pane);
|
||||
CloseConPtySession(pane);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
pane.IsSubmitPending = false;
|
||||
ErrorReported?.Invoke(Loc.T("missionControl.submitForReviewFailed", ex.Message));
|
||||
}
|
||||
}
|
||||
@@ -349,6 +355,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
|
||||
if (!ConPtySessions.Contains(pane)) return;
|
||||
pane.ErrorReported -= OnConPtyPaneError;
|
||||
pane.CloseRequested -= CloseConPtySession;
|
||||
pane.SubmitForReviewRequested -= OnPaneSubmitForReview;
|
||||
ConPtySessions.Remove(pane);
|
||||
pane.Dispose();
|
||||
}
|
||||
@@ -398,6 +405,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
|
||||
{
|
||||
c.ErrorReported -= OnConPtyPaneError;
|
||||
c.CloseRequested -= CloseConPtySession;
|
||||
c.SubmitForReviewRequested -= OnPaneSubmitForReview;
|
||||
c.Dispose();
|
||||
}
|
||||
ConPtySessions.Clear();
|
||||
|
||||
@@ -39,9 +39,15 @@
|
||||
Background="{DynamicResource ErrorTintBrush}"
|
||||
BorderBrush="{DynamicResource BloodBrush}"
|
||||
BorderThickness="0,0,0,1" Padding="12,6">
|
||||
<TextBlock Classes="meta" Text="{Binding Terminal.StartError}"
|
||||
Foreground="{DynamicResource BloodBrush}"
|
||||
TextWrapping="Wrap" />
|
||||
<Grid ColumnDefinitions="*,Auto">
|
||||
<TextBlock Grid.Column="0" Classes="meta" Text="{Binding Terminal.StartError}"
|
||||
Foreground="{DynamicResource BloodBrush}"
|
||||
TextWrapping="Wrap" VerticalAlignment="Center" />
|
||||
<Button Grid.Column="1" Classes="btn" Margin="12,0,0,0"
|
||||
Content="{loc:Tr missionControl.retry}"
|
||||
ToolTip.Tip="{loc:Tr missionControl.retryTip}"
|
||||
Command="{Binding RetryCommand}" />
|
||||
</Grid>
|
||||
</Border>
|
||||
|
||||
<!-- Embedded ConPTY terminal, with a starting overlay until the session is spawned -->
|
||||
|
||||
@@ -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;
|
||||
@@ -606,6 +607,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()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user