feat(review): unify review actions into the Git-tab cockpit
Grow the detail-pane Git tab into the review+merge cockpit: target, pre-flight mergeability, inspect actions, then the four review verbs (Approve & Merge / Send back / Park / Cancel) plus a demoted Reset (discard branch). The decision block is gated independently of the merge controls so sandbox (no-worktree) review tasks still get the buttons. - Add ParkReviewCommand (-> RejectReviewToIdleAsync) - Send back (reject-to-queue) disabled until feedback is entered - Remove the mislabeled [Continue]/[Reset] line from the Output tab - Accent dot on the Git tab while awaiting review
This commit is contained in:
@@ -337,7 +337,12 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
||||
public Func<string, System.Threading.Tasks.Task>? ShowErrorAsync { get; set; }
|
||||
|
||||
// ── Review ──────────────────────────────────────────────────────────────
|
||||
[ObservableProperty] private string _reviewFeedback = "";
|
||||
[ObservableProperty]
|
||||
[NotifyPropertyChangedFor(nameof(HasReviewFeedback))]
|
||||
[NotifyCanExecuteChangedFor(nameof(RejectReviewCommand))]
|
||||
private string _reviewFeedback = "";
|
||||
|
||||
public bool HasReviewFeedback => !string.IsNullOrWhiteSpace(ReviewFeedback);
|
||||
|
||||
// Kept for backwards-compat surface — delegates to Merge.RequestConflictResolution
|
||||
public Func<string, string, System.Threading.Tasks.Task>? RequestConflictResolution
|
||||
@@ -1169,7 +1174,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
||||
}
|
||||
}
|
||||
|
||||
[RelayCommand]
|
||||
[RelayCommand(CanExecute = nameof(HasReviewFeedback))]
|
||||
private async System.Threading.Tasks.Task RejectReviewAsync()
|
||||
{
|
||||
if (Task is null || !_worker.IsConnected) return;
|
||||
@@ -1180,6 +1185,15 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
||||
ReviewFeedback = "";
|
||||
}
|
||||
|
||||
// Park: set the task aside (back to Idle), keeping its worktree intact.
|
||||
[RelayCommand]
|
||||
private async System.Threading.Tasks.Task ParkReviewAsync()
|
||||
{
|
||||
if (Task is null || !_worker.IsConnected) return;
|
||||
try { await _worker.RejectReviewToIdleAsync(Task.Id); }
|
||||
catch { /* stale review action; broadcast reconciles */ }
|
||||
}
|
||||
|
||||
[RelayCommand]
|
||||
private async System.Threading.Tasks.Task ResetReviewAsync()
|
||||
{
|
||||
|
||||
@@ -167,9 +167,16 @@
|
||||
CommandParameter="output" />
|
||||
<Button Classes="tab-btn"
|
||||
Classes.active="{Binding IsGitTab}"
|
||||
Content="Git"
|
||||
Command="{Binding SelectTabCommand}"
|
||||
CommandParameter="git" />
|
||||
CommandParameter="git">
|
||||
<StackPanel Orientation="Horizontal" Spacing="6">
|
||||
<TextBlock Text="Git" VerticalAlignment="Center" />
|
||||
<!-- Review-pending dot: where to act when a task awaits review -->
|
||||
<Ellipse Width="6" Height="6" VerticalAlignment="Center"
|
||||
Fill="{DynamicResource AccentBrush}"
|
||||
IsVisible="{Binding IsWaitingForReview}" />
|
||||
</StackPanel>
|
||||
</Button>
|
||||
<Button Classes="tab-btn"
|
||||
Classes.active="{Binding IsSessionTab}"
|
||||
Content="Session"
|
||||
@@ -205,42 +212,6 @@
|
||||
</StackPanel>
|
||||
</Border>
|
||||
|
||||
<!-- Review prompt — sits directly on the terminal, like a shell input line;
|
||||
only while awaiting review. No border/fill so it reads as part of the log. -->
|
||||
<Grid DockPanel.Dock="Bottom"
|
||||
IsVisible="{Binding IsWaitingForReview}"
|
||||
ColumnDefinitions="Auto,*,Auto"
|
||||
Margin="12,2,12,8">
|
||||
<TextBlock Grid.Column="0" Text="❯"
|
||||
FontFamily="{StaticResource MonoFont}"
|
||||
FontSize="{StaticResource FontSizeMono}"
|
||||
Foreground="{DynamicResource AccentBrush}"
|
||||
VerticalAlignment="Top" Margin="0,2,8,0" />
|
||||
<TextBox Grid.Column="1"
|
||||
Name="ReviewInput"
|
||||
KeyDown="OnReviewInputKeyDown"
|
||||
Text="{Binding ReviewFeedback, Mode=TwoWay}"
|
||||
AcceptsReturn="True"
|
||||
TextWrapping="Wrap"
|
||||
MaxHeight="160"
|
||||
PlaceholderText="Feedback for the next run…"
|
||||
Background="Transparent"
|
||||
BorderThickness="0"
|
||||
Padding="0"
|
||||
VerticalContentAlignment="Center"
|
||||
FontFamily="{StaticResource MonoFont}"
|
||||
FontSize="{StaticResource FontSizeMono}" />
|
||||
<StackPanel Grid.Column="2" Orientation="Horizontal" Spacing="10"
|
||||
VerticalAlignment="Top" Margin="12,2,0,0">
|
||||
<Button Classes="prompt-action accent" Content="[Continue]"
|
||||
ToolTip.Tip="{loc:Tr session.reviewContinueTip}"
|
||||
Command="{Binding RejectReviewCommand}" />
|
||||
<Button Classes="prompt-action" Content="[Reset]"
|
||||
ToolTip.Tip="{loc:Tr session.reviewResetTip}"
|
||||
Command="{Binding ResetReviewCommand}" />
|
||||
</StackPanel>
|
||||
</Grid>
|
||||
|
||||
<ScrollViewer Name="LogScroll"
|
||||
VerticalScrollBarVisibility="Visible"
|
||||
AllowAutoHide="False"
|
||||
@@ -264,48 +235,95 @@
|
||||
|
||||
</DockPanel>
|
||||
|
||||
<!-- Git: one Approve + merge cockpit -->
|
||||
<!-- Git: the review + merge cockpit -->
|
||||
<ScrollViewer IsVisible="{Binding IsGitTab}" Padding="14,10">
|
||||
<StackPanel Spacing="12" IsVisible="{Binding Merge.ShowMergeSection}">
|
||||
<TextBlock Classes="section-label" Text="MERGE" />
|
||||
<StackPanel Spacing="14">
|
||||
|
||||
<StackPanel Spacing="4">
|
||||
<TextBlock Classes="field-label" Text="Target branch" />
|
||||
<ComboBox ItemsSource="{Binding Merge.MergeTargetBranches}"
|
||||
SelectedItem="{Binding Merge.SelectedMergeTarget, Mode=TwoWay}"
|
||||
HorizontalAlignment="Stretch" />
|
||||
<!-- Merge controls — shown whenever there's a worktree / unit to merge.
|
||||
Header reads REVIEW while a decision is pending, otherwise MERGE. -->
|
||||
<StackPanel Spacing="14" IsVisible="{Binding Merge.ShowMergeSection}">
|
||||
<TextBlock Classes="section-label" Text="REVIEW"
|
||||
IsVisible="{Binding IsWaitingForReview}" />
|
||||
<TextBlock Classes="section-label" Text="MERGE"
|
||||
IsVisible="{Binding !IsWaitingForReview}" />
|
||||
|
||||
<!-- Change summary (review only) -->
|
||||
<StackPanel Orientation="Horizontal" Spacing="6"
|
||||
IsVisible="{Binding IsWaitingForReview}">
|
||||
<TextBlock Classes="diff-add" Text="{Binding DiffAddText}" />
|
||||
<TextBlock Classes="diff-del" Text="{Binding DiffDelText}" />
|
||||
</StackPanel>
|
||||
|
||||
<!-- Target branch + pre-flight mergeability -->
|
||||
<StackPanel Spacing="4">
|
||||
<TextBlock Classes="field-label" Text="Target branch" />
|
||||
<ComboBox ItemsSource="{Binding Merge.MergeTargetBranches}"
|
||||
SelectedItem="{Binding Merge.SelectedMergeTarget, Mode=TwoWay}"
|
||||
HorizontalAlignment="Stretch" />
|
||||
</StackPanel>
|
||||
|
||||
<StackPanel Spacing="0">
|
||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
||||
Foreground="{DynamicResource MossBrush}"
|
||||
IsVisible="{Binding Merge.MergeIsClean}" />
|
||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
||||
Foreground="{DynamicResource BloodBrush}"
|
||||
IsVisible="{Binding Merge.MergeIsConflict}" />
|
||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
||||
Foreground="{DynamicResource TextMuteBrush}"
|
||||
IsVisible="{Binding Merge.ShowMergePreviewMuted}" />
|
||||
</StackPanel>
|
||||
|
||||
<!-- Inspect: diff / worktree / combined diff -->
|
||||
<WrapPanel Orientation="Horizontal">
|
||||
<Button Classes="btn" Content="Open Diff" Margin="0,0,8,8"
|
||||
Command="{Binding Merge.OpenDiffCommand}" />
|
||||
<Button Classes="btn" Margin="0,0,8,8"
|
||||
ToolTip.Tip="{loc:Tr agent.openWorktreeTip}"
|
||||
Command="{Binding Merge.OpenWorktreeCommand}">
|
||||
<StackPanel Orientation="Horizontal" Spacing="5">
|
||||
<TextBlock Text="Worktree" />
|
||||
<PathIcon Data="{StaticResource Icon.ArrowOut}" Width="11" Height="11" />
|
||||
</StackPanel>
|
||||
</Button>
|
||||
<Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8"
|
||||
Command="{Binding Merge.ReviewCombinedDiffCommand}" />
|
||||
</WrapPanel>
|
||||
</StackPanel>
|
||||
|
||||
<StackPanel Spacing="0">
|
||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
||||
Foreground="{DynamicResource MossBrush}"
|
||||
IsVisible="{Binding Merge.MergeIsClean}" />
|
||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
||||
Foreground="{DynamicResource BloodBrush}"
|
||||
IsVisible="{Binding Merge.MergeIsConflict}" />
|
||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
||||
Foreground="{DynamicResource TextMuteBrush}"
|
||||
IsVisible="{Binding Merge.ShowMergePreviewMuted}" />
|
||||
</StackPanel>
|
||||
<!-- Review decision — feedback + the four review verbs. Always present while
|
||||
awaiting review, even for sandbox runs with no worktree to merge. -->
|
||||
<StackPanel Spacing="10" IsVisible="{Binding IsWaitingForReview}">
|
||||
<Border Height="1" Background="{DynamicResource LineBrush}"
|
||||
IsVisible="{Binding Merge.ShowMergeSection}" />
|
||||
|
||||
<!-- Primary action: Approve flows straight into the merge. -->
|
||||
<WrapPanel Orientation="Horizontal">
|
||||
<Button Classes="btn accent" Content="Approve & Merge" Margin="0,0,8,8"
|
||||
Command="{Binding ApproveReviewCommand}"
|
||||
IsVisible="{Binding IsWaitingForReview}" />
|
||||
<Button Classes="btn" Content="Open Diff" Margin="0,0,8,8"
|
||||
Command="{Binding Merge.OpenDiffCommand}" />
|
||||
<Button Classes="btn" Margin="0,0,8,8"
|
||||
ToolTip.Tip="{loc:Tr agent.openWorktreeTip}"
|
||||
Command="{Binding Merge.OpenWorktreeCommand}">
|
||||
<StackPanel Orientation="Horizontal" Spacing="5">
|
||||
<TextBlock Text="Worktree" />
|
||||
<PathIcon Data="{StaticResource Icon.ArrowOut}" Width="11" Height="11" />
|
||||
</StackPanel>
|
||||
</Button>
|
||||
<Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8"
|
||||
Command="{Binding Merge.ReviewCombinedDiffCommand}" />
|
||||
</WrapPanel>
|
||||
<TextBox Name="ReviewInput"
|
||||
KeyDown="OnReviewInputKeyDown"
|
||||
Text="{Binding ReviewFeedback, Mode=TwoWay}"
|
||||
AcceptsReturn="True"
|
||||
TextWrapping="Wrap"
|
||||
MaxHeight="120"
|
||||
PlaceholderText="Feedback for a re-run…"
|
||||
FontFamily="{StaticResource MonoFont}"
|
||||
FontSize="{StaticResource FontSizeMono}" />
|
||||
|
||||
<WrapPanel Orientation="Horizontal">
|
||||
<Button Classes="btn accent" Content="Approve & Merge" Margin="0,0,8,8"
|
||||
Command="{Binding ApproveReviewCommand}" />
|
||||
<Button Classes="btn" Content="Send back" Margin="0,0,8,8"
|
||||
ToolTip.Tip="{loc:Tr session.reviewContinueTip}"
|
||||
Command="{Binding RejectReviewCommand}" />
|
||||
<Button Classes="btn" Content="Park" Margin="0,0,8,8"
|
||||
ToolTip.Tip="Set aside — back to Idle, keeps the worktree"
|
||||
Command="{Binding ParkReviewCommand}" />
|
||||
<Button Classes="btn" Content="Cancel" Margin="0,0,8,8"
|
||||
Command="{Binding CancelReviewCommand}" />
|
||||
</WrapPanel>
|
||||
|
||||
<Button Classes="prompt-action" Content="Reset (discard branch)…"
|
||||
ToolTip.Tip="{loc:Tr session.reviewResetTip}"
|
||||
Command="{Binding ResetReviewCommand}" />
|
||||
</StackPanel>
|
||||
</StackPanel>
|
||||
</ScrollViewer>
|
||||
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Ui.Services;
|
||||
using ClaudeDo.Ui.ViewModels.Islands;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
namespace ClaudeDo.Ui.Tests.ViewModels;
|
||||
|
||||
public class DetailsIslandReviewActionsTests : IDisposable
|
||||
{
|
||||
private readonly string _dbPath;
|
||||
|
||||
public DetailsIslandReviewActionsTests()
|
||||
{
|
||||
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_review_actions_test_{Guid.NewGuid():N}.db");
|
||||
using var ctx = NewContext();
|
||||
ctx.Database.EnsureCreated();
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
try { File.Delete(_dbPath); } catch { }
|
||||
try { File.Delete(_dbPath + "-wal"); } catch { }
|
||||
try { File.Delete(_dbPath + "-shm"); } catch { }
|
||||
}
|
||||
|
||||
private ClaudeDoDbContext NewContext()
|
||||
{
|
||||
var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
|
||||
.UseSqlite($"Data Source={_dbPath}")
|
||||
.Options;
|
||||
return new ClaudeDoDbContext(opts);
|
||||
}
|
||||
|
||||
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
|
||||
{
|
||||
private readonly Func<ClaudeDoDbContext> _create;
|
||||
public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
|
||||
public ClaudeDoDbContext CreateDbContext() => _create();
|
||||
}
|
||||
|
||||
private sealed class NullServiceProvider : IServiceProvider
|
||||
{
|
||||
public object? GetService(Type serviceType) => null;
|
||||
}
|
||||
|
||||
private sealed class StubNotesApi : ClaudeDo.Ui.Services.Interfaces.INotesApi
|
||||
{
|
||||
public Task<List<DailyNoteDto>> ListAsync(DateOnly day) =>
|
||||
Task.FromResult(new List<DailyNoteDto>());
|
||||
public Task<DailyNoteDto?> AddAsync(DateOnly day, string text) =>
|
||||
Task.FromResult<DailyNoteDto?>(null);
|
||||
public Task UpdateAsync(string id, string text) => Task.CompletedTask;
|
||||
public Task DeleteAsync(string id) => Task.CompletedTask;
|
||||
}
|
||||
|
||||
private sealed class RecordingWorkerClient : StubWorkerClient
|
||||
{
|
||||
public override bool IsConnected => true;
|
||||
public string? ParkedTaskId;
|
||||
public override Task RejectReviewToIdleAsync(string taskId)
|
||||
{
|
||||
ParkedTaskId = taskId;
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
}
|
||||
|
||||
private DetailsIslandViewModel BuildVm(StubWorkerClient worker)
|
||||
{
|
||||
var factory = new TestDbFactory(NewContext);
|
||||
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ParkReview_CallsRejectReviewToIdle_ForTheBoundTask()
|
||||
{
|
||||
var worker = new RecordingWorkerClient();
|
||||
var vm = BuildVm(worker);
|
||||
vm.Bind(new TaskRowViewModel { Id = "task-park-1", Status = TaskStatus.WaitingForReview });
|
||||
|
||||
await vm.ParkReviewCommand.ExecuteAsync(null);
|
||||
|
||||
Assert.Equal("task-park-1", worker.ParkedTaskId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SendBack_IsDisabledUntilFeedbackIsEntered()
|
||||
{
|
||||
var vm = BuildVm(new RecordingWorkerClient());
|
||||
vm.Bind(new TaskRowViewModel { Id = "task-fb-1", Status = TaskStatus.WaitingForReview });
|
||||
|
||||
Assert.False(vm.RejectReviewCommand.CanExecute(null));
|
||||
|
||||
vm.ReviewFeedback = "tighten the error handling";
|
||||
Assert.True(vm.RejectReviewCommand.CanExecute(null));
|
||||
|
||||
vm.ReviewFeedback = " ";
|
||||
Assert.False(vm.RejectReviewCommand.CanExecute(null));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user