diff --git a/docs/superpowers/plans/2026-06-01-waiting-for-review-state.md b/docs/superpowers/plans/2026-06-01-waiting-for-review-state.md new file mode 100644 index 0000000..b60f761 --- /dev/null +++ b/docs/superpowers/plans/2026-06-01-waiting-for-review-state.md @@ -0,0 +1,175 @@ +# Waiting for Review — Task State — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a `WaitingForReview` lifecycle state that standalone tasks enter after a successful run, with approve / reject-rerun / reject-park / cancel exits, exposed via UI and MCP. + +**Architecture:** New enum value + nullable `ReviewFeedback` column. `TaskStateService` gains review transitions. `TaskRunner.HandleSuccess` routes standalone-task success to review. `QueueService.RunInSlotAsync` resumes the Claude session when re-running a rejected task. New MCP `review_task` tool + UI commands. + +**Tech Stack:** .NET 8, EF Core (SQLite, TEXT enum), SignalR, Avalonia MVVM, xUnit. + +**Scope decision (locked):** Only standalone tasks (`ParentTaskId == null`) route to `WaitingForReview`. Planning **child** tasks continue to `Done` on success so the sequential planning chain (which advances on *terminal* states) is unaffected. Flagged for user confirmation. + +--- + +## Task 1: Data layer — enum, converter, column + +**Files:** +- Modify: `src/ClaudeDo.Data/Models/TaskEntity.cs` +- Modify: `src/ClaudeDo.Data/Configuration/TaskEntityConfiguration.cs` +- Create: EF migration via CLI + +- [ ] **Step 1:** Add `WaitingForReview` to `TaskStatus` enum (after `Running`) and add `public string? ReviewFeedback { get; set; }` to `TaskEntity`. +- [ ] **Step 2:** In `TaskEntityConfiguration`, add `TaskStatus.WaitingForReview => "waiting_for_review"` to `StatusToString` and `"waiting_for_review" => TaskStatus.WaitingForReview` to `StatusFromString`; map the column: `builder.Property(t => t.ReviewFeedback).HasColumnName("review_feedback");` +- [ ] **Step 3:** Create migration: `dotnet ef migrations add AddReviewFeedback --project src/ClaudeDo.Data/ClaudeDo.Data.csproj`. Verify it only adds the `review_feedback` TEXT column (nullable). If `dotnet ef` unavailable, hand-write the migration + designer following the latest migration in `Migrations/`. +- [ ] **Step 4:** Build `dotnet build src/ClaudeDo.Data/ClaudeDo.Data.csproj`. Expected: success. +- [ ] **Step 5:** Commit. + +## Task 2: Worker — review transitions in TaskStateService + +**Files:** +- Modify: `src/ClaudeDo.Worker/State/TaskStateService.cs` +- Modify: `src/ClaudeDo.Worker/State/Interfaces/ITaskStateService.cs` (add new method signatures) +- Test: `tests/ClaudeDo.Worker.Tests/...` (state transition tests) + +New methods (all return `TransitionResult`, broadcast `TaskUpdated`): + +- `SubmitForReviewAsync(taskId, finishedAt, result, ct)` — guard `Status == Running`; set `Status=WaitingForReview, FinishedAt, Result`. Does NOT call `OnChildTerminalAsync` (review is non-terminal; only invoked for standalone tasks anyway). +- `ApproveReviewAsync(taskId, ct)` — guard `Status == WaitingForReview`; set `Status=Done`. +- `RejectToQueueAsync(taskId, feedback, ct)` — reject empty/whitespace feedback (`TransitionResult(false, "Feedback is required to reject for re-run.")`); guard `Status == WaitingForReview`; set `Status=Queued, ReviewFeedback=feedback`; `_waker.Wake()`. +- `RejectToIdleAsync(taskId, ct)` — guard `Status == WaitingForReview`; set `Status=Idle, ReviewFeedback=null` (leave `Result` intact). +- `ClearReviewFeedbackAsync(taskId, ct)` — set `ReviewFeedback=null` (no status change, no guard); used by the runner after consuming feedback. +- Extend `CancelAsync` guard: `(Status == Running || Status == Queued || Status == WaitingForReview)`. + +- [ ] **Step 1:** Write failing tests in a new `tests/ClaudeDo.Worker.Tests/State/ReviewTransitionTests.cs` (follow existing TaskStateService test setup). Cover: submit-for-review from Running; approve from WaitingForReview→Done; reject-to-queue stores feedback + status Queued; empty feedback rejected; reject-to-idle clears feedback + keeps Result; cancel from WaitingForReview→Cancelled; invalid (approve from Idle) returns `!Ok`. +- [ ] **Step 2:** Run tests, expect FAIL (methods missing). +- [ ] **Step 3:** Implement the methods + interface signatures + CancelAsync guard. +- [ ] **Step 4:** Run tests, expect PASS. +- [ ] **Step 5:** Commit. + +## Task 3: Worker — route standalone success to review + +**Files:** +- Modify: `src/ClaudeDo.Worker/Runner/TaskRunner.cs` (`HandleSuccess`) + +- [ ] **Step 1:** In `HandleSuccess`, after commit, branch: + ```csharp + var finishedAt = DateTime.UtcNow; + if (task.ParentTaskId is null) + { + await _state.SubmitForReviewAsync(task.Id, finishedAt, result.ResultMarkdown, CancellationToken.None); + await _broadcaster.WorkerLog($"Finished \"{task.Title}\" (waiting for review)", WorkerLogLevel.Success, DateTime.UtcNow); + await _broadcaster.TaskFinished(slot, task.Id, "waiting_for_review", finishedAt); + } + else + { + await _state.CompleteAsync(task.Id, finishedAt, result.ResultMarkdown, CancellationToken.None); + await _broadcaster.WorkerLog($"Finished \"{task.Title}\" (done)", WorkerLogLevel.Success, DateTime.UtcNow); + await _broadcaster.TaskFinished(slot, task.Id, "done", finishedAt); + } + ``` +- [ ] **Step 2:** Build worker. Expected: success. +- [ ] **Step 3:** Commit. + +## Task 4: Worker — resume-aware re-run in QueueService + +**Files:** +- Modify: `src/ClaudeDo.Worker/Queue/QueueService.cs` (`RunInSlotAsync`) +- Test: `tests/ClaudeDo.Worker.Tests/...` + +- [ ] **Step 1:** In `RunInSlotAsync`, after loading `task`: + ```csharp + if (!string.IsNullOrWhiteSpace(task.ReviewFeedback)) + { + var feedback = task.ReviewFeedback!; + string? sessionId; + using (var ctx = _dbFactory.CreateDbContext()) + sessionId = (await new TaskRunRepository(ctx).GetLatestByTaskIdAsync(taskId, ct))?.SessionId; + await _state.ClearReviewFeedbackAsync(taskId, ct); // inject ITaskStateService + if (sessionId is not null) + { + await _runner.ContinueAsync(taskId, feedback, "queue", ct); + return; + } + task.Description = string.IsNullOrWhiteSpace(task.Description) + ? $"Reviewer feedback: {feedback}" + : $"{task.Description}\n\nReviewer feedback: {feedback}"; + } + await _runner.RunAsync(task, "queue", ct); + ``` + Inject `ITaskStateService _state` into `QueueService` (add to ctor + DI already provides it). +- [ ] **Step 2:** Build worker, expect success. +- [ ] **Step 3:** Commit. + +## Task 5: MCP — review_task tool + status reference + +**Files:** +- Modify: `src/ClaudeDo.Worker/External/ExternalMcpService.cs` + +- [ ] **Step 1:** Add `review_task` tool: + ```csharp + [McpServerTool, Description( + "Review a task that is WaitingForReview. decision: 'approve' (→ Done), " + + "'reject_rerun' (→ Queued, resumes the agent session with feedback — feedback required), " + + "'reject_park' (→ Idle for manual editing), 'cancel' (→ Cancelled). ")] + public async Task ReviewTask(string taskId, string decision, string? feedback, CancellationToken cancellationToken) + { + var task = await _tasks.GetByIdAsync(taskId, cancellationToken) + ?? throw new InvalidOperationException($"Task {taskId} not found."); + TransitionResult r = decision.ToLowerInvariant() switch + { + "approve" => await _state.ApproveReviewAsync(taskId, cancellationToken), + "reject_rerun" => await _state.RejectToQueueAsync(taskId, feedback ?? "", cancellationToken), + "reject_park" => await _state.RejectToIdleAsync(taskId, cancellationToken), + "cancel" => await _state.CancelAsync(taskId, DateTime.UtcNow, cancellationToken), + _ => throw new InvalidOperationException($"Unknown decision '{decision}'. Use approve, reject_rerun, reject_park, or cancel."), + }; + if (!r.Ok) throw new InvalidOperationException(r.Reason ?? "Review action failed."); + return ToDto((await _tasks.GetByIdAsync(taskId, cancellationToken))!); + } + ``` +- [ ] **Step 2:** Add `WaitingForReview` to `GetTaskStatusValues` list; update the validation strings in `ListTasks` and the lifecycle text in `GetTask`/`UpdateTaskStatus` to include `WaitingForReview`. +- [ ] **Step 3:** Build worker, expect success. +- [ ] **Step 4:** Commit. + +## Task 6: UI — client + hub methods + +**Files:** +- Modify: `src/ClaudeDo.Worker/Hub/WorkerHub.cs` +- Modify: `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs` +- Modify: `src/ClaudeDo.Ui/Services/WorkerClient.cs` +- Modify: `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs` + +- [ ] **Step 1:** Hub: add `ApproveReview(taskId)`, `RejectReviewToQueue(taskId, feedback)`, `RejectReviewToIdle(taskId)`, `CancelReview(taskId)` — each calls the matching `_state` method via `HubGuard`-style mapping (`if (!result.Ok) throw new HubException(...)`). +- [ ] **Step 2:** `IWorkerClient` + `WorkerClient`: add `ApproveReviewAsync`, `RejectReviewToQueueAsync(taskId, feedback)`, `RejectReviewToIdleAsync`, `CancelReviewAsync` invoking the hub methods. Add no-op/stub impls to `StubWorkerClient`. +- [ ] **Step 3:** Build App + Ui.Tests. Expected: success. +- [ ] **Step 4:** Commit. + +## Task 7: UI — converter, row VM, view buttons + +**Files:** +- Modify: `src/ClaudeDo.Ui/Converters/StatusColorConverter.cs` +- Modify: `src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs` +- Modify: `src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs` (commands) +- Modify: the task row/detail AXAML to surface Approve / Reject / Park / Cancel when `IsWaitingForReview` + +- [ ] **Step 1:** `StatusColorConverter`: add `"waiting_for_review" => Brushes.MediumPurple,` (placeholder — user does visual pass). +- [ ] **Step 2:** `TaskRowViewModel`: add `public bool IsWaitingForReview => Status == TaskStatus.WaitingForReview;`, raise it in `OnStatusChanged`, and add `(TaskStatus.WaitingForReview, _) => "review"` to `StatusChipClass`. +- [ ] **Step 3:** `TasksIslandViewModel`: add relay commands `ApproveReview`, `RejectReviewRerun` (prompts for feedback), `RejectReviewPark`, `CancelReview` operating on the selected/target row, calling the new client methods. +- [ ] **Step 4:** Add buttons to the relevant view bound to those commands, visible when `IsWaitingForReview`. Reject-rerun uses a text-input flyout/dialog for required feedback. +- [ ] **Step 5:** Build App + Ui.Tests. Expected: success. (Visual layout: flagged for user's visual pass — cannot render here.) +- [ ] **Step 6:** Commit. + +## Task 8: Docs + full verification + +**Files:** +- Modify: root `CLAUDE.md`, `src/ClaudeDo.Data/CLAUDE.md`, `src/ClaudeDo.Worker/CLAUDE.md` + +- [ ] **Step 1:** Update status flow lines + worker transition table to include `WaitingForReview` and the new transitions. +- [ ] **Step 2:** Build all projects (csproj individually — `.slnx` needs .NET 9) and run `dotnet test tests/ClaudeDo.Worker.Tests`, `tests/ClaudeDo.Ui.Tests`, `tests/ClaudeDo.Data.Tests`. Expected: all green. +- [ ] **Step 3:** Commit. + +## Self-Review notes + +- Spec coverage: §1 state machine → Tasks 2,3; §2 data → Task 1; §3 transitions → Task 2; §4 resume → Task 4; §5 MCP → Task 5; §6 hub → Task 6; §7 UI → Tasks 6,7; §8 docs → Task 8; testing → Tasks 2,4,8. +- Method names consistent across tasks: `SubmitForReviewAsync`, `ApproveReviewAsync`, `RejectToQueueAsync`, `RejectToIdleAsync`, `ClearReviewFeedbackAsync` (state); `ApproveReview`/`RejectReviewToQueue`/`RejectReviewToIdle`/`CancelReview` (hub); `ApproveReviewAsync`/`RejectReviewToQueueAsync`/`RejectReviewToIdleAsync`/`CancelReviewAsync` (client).