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; }
|
public Func<string, System.Threading.Tasks.Task>? ShowErrorAsync { get; set; }
|
||||||
|
|
||||||
// ── Review ──────────────────────────────────────────────────────────────
|
// ── 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
|
// Kept for backwards-compat surface — delegates to Merge.RequestConflictResolution
|
||||||
public Func<string, string, System.Threading.Tasks.Task>? 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()
|
private async System.Threading.Tasks.Task RejectReviewAsync()
|
||||||
{
|
{
|
||||||
if (Task is null || !_worker.IsConnected) return;
|
if (Task is null || !_worker.IsConnected) return;
|
||||||
@@ -1180,6 +1185,15 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
|||||||
ReviewFeedback = "";
|
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]
|
[RelayCommand]
|
||||||
private async System.Threading.Tasks.Task ResetReviewAsync()
|
private async System.Threading.Tasks.Task ResetReviewAsync()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -167,9 +167,16 @@
|
|||||||
CommandParameter="output" />
|
CommandParameter="output" />
|
||||||
<Button Classes="tab-btn"
|
<Button Classes="tab-btn"
|
||||||
Classes.active="{Binding IsGitTab}"
|
Classes.active="{Binding IsGitTab}"
|
||||||
Content="Git"
|
|
||||||
Command="{Binding SelectTabCommand}"
|
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"
|
<Button Classes="tab-btn"
|
||||||
Classes.active="{Binding IsSessionTab}"
|
Classes.active="{Binding IsSessionTab}"
|
||||||
Content="Session"
|
Content="Session"
|
||||||
@@ -205,42 +212,6 @@
|
|||||||
</StackPanel>
|
</StackPanel>
|
||||||
</Border>
|
</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"
|
<ScrollViewer Name="LogScroll"
|
||||||
VerticalScrollBarVisibility="Visible"
|
VerticalScrollBarVisibility="Visible"
|
||||||
AllowAutoHide="False"
|
AllowAutoHide="False"
|
||||||
@@ -264,48 +235,95 @@
|
|||||||
|
|
||||||
</DockPanel>
|
</DockPanel>
|
||||||
|
|
||||||
<!-- Git: one Approve + merge cockpit -->
|
<!-- Git: the review + merge cockpit -->
|
||||||
<ScrollViewer IsVisible="{Binding IsGitTab}" Padding="14,10">
|
<ScrollViewer IsVisible="{Binding IsGitTab}" Padding="14,10">
|
||||||
<StackPanel Spacing="12" IsVisible="{Binding Merge.ShowMergeSection}">
|
<StackPanel Spacing="14">
|
||||||
<TextBlock Classes="section-label" Text="MERGE" />
|
|
||||||
|
|
||||||
<StackPanel Spacing="4">
|
<!-- Merge controls — shown whenever there's a worktree / unit to merge.
|
||||||
<TextBlock Classes="field-label" Text="Target branch" />
|
Header reads REVIEW while a decision is pending, otherwise MERGE. -->
|
||||||
<ComboBox ItemsSource="{Binding Merge.MergeTargetBranches}"
|
<StackPanel Spacing="14" IsVisible="{Binding Merge.ShowMergeSection}">
|
||||||
SelectedItem="{Binding Merge.SelectedMergeTarget, Mode=TwoWay}"
|
<TextBlock Classes="section-label" Text="REVIEW"
|
||||||
HorizontalAlignment="Stretch" />
|
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>
|
||||||
|
|
||||||
<StackPanel Spacing="0">
|
<!-- Review decision — feedback + the four review verbs. Always present while
|
||||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
awaiting review, even for sandbox runs with no worktree to merge. -->
|
||||||
Foreground="{DynamicResource MossBrush}"
|
<StackPanel Spacing="10" IsVisible="{Binding IsWaitingForReview}">
|
||||||
IsVisible="{Binding Merge.MergeIsClean}" />
|
<Border Height="1" Background="{DynamicResource LineBrush}"
|
||||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
IsVisible="{Binding Merge.ShowMergeSection}" />
|
||||||
Foreground="{DynamicResource BloodBrush}"
|
|
||||||
IsVisible="{Binding Merge.MergeIsConflict}" />
|
|
||||||
<TextBlock Classes="meta" Text="{Binding Merge.MergePreviewText}" TextWrapping="Wrap"
|
|
||||||
Foreground="{DynamicResource TextMuteBrush}"
|
|
||||||
IsVisible="{Binding Merge.ShowMergePreviewMuted}" />
|
|
||||||
</StackPanel>
|
|
||||||
|
|
||||||
<!-- Primary action: Approve flows straight into the merge. -->
|
<TextBox Name="ReviewInput"
|
||||||
<WrapPanel Orientation="Horizontal">
|
KeyDown="OnReviewInputKeyDown"
|
||||||
<Button Classes="btn accent" Content="Approve & Merge" Margin="0,0,8,8"
|
Text="{Binding ReviewFeedback, Mode=TwoWay}"
|
||||||
Command="{Binding ApproveReviewCommand}"
|
AcceptsReturn="True"
|
||||||
IsVisible="{Binding IsWaitingForReview}" />
|
TextWrapping="Wrap"
|
||||||
<Button Classes="btn" Content="Open Diff" Margin="0,0,8,8"
|
MaxHeight="120"
|
||||||
Command="{Binding Merge.OpenDiffCommand}" />
|
PlaceholderText="Feedback for a re-run…"
|
||||||
<Button Classes="btn" Margin="0,0,8,8"
|
FontFamily="{StaticResource MonoFont}"
|
||||||
ToolTip.Tip="{loc:Tr agent.openWorktreeTip}"
|
FontSize="{StaticResource FontSizeMono}" />
|
||||||
Command="{Binding Merge.OpenWorktreeCommand}">
|
|
||||||
<StackPanel Orientation="Horizontal" Spacing="5">
|
<WrapPanel Orientation="Horizontal">
|
||||||
<TextBlock Text="Worktree" />
|
<Button Classes="btn accent" Content="Approve & Merge" Margin="0,0,8,8"
|
||||||
<PathIcon Data="{StaticResource Icon.ArrowOut}" Width="11" Height="11" />
|
Command="{Binding ApproveReviewCommand}" />
|
||||||
</StackPanel>
|
<Button Classes="btn" Content="Send back" Margin="0,0,8,8"
|
||||||
</Button>
|
ToolTip.Tip="{loc:Tr session.reviewContinueTip}"
|
||||||
<Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8"
|
Command="{Binding RejectReviewCommand}" />
|
||||||
Command="{Binding Merge.ReviewCombinedDiffCommand}" />
|
<Button Classes="btn" Content="Park" Margin="0,0,8,8"
|
||||||
</WrapPanel>
|
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>
|
</StackPanel>
|
||||||
</ScrollViewer>
|
</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