From 260cb5a89c3c554be20b4aa1ff41f2bc61598e68 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Tue, 11 Aug 2026 19:29:04 +0200 Subject: [PATCH] =?UTF-8?q?docs(plans):=20Vorgaben=20je=20Merge-Helper-Gru?= =?UTF-8?q?ppe=20f=C3=BCr=20Operations-Feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../plans/2026-08-11-operation-feedback.md | 334 ++++++++++++++++++ 1 file changed, 334 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-11-operation-feedback.md diff --git a/docs/superpowers/plans/2026-08-11-operation-feedback.md b/docs/superpowers/plans/2026-08-11-operation-feedback.md new file mode 100644 index 00000000..59515a94 --- /dev/null +++ b/docs/superpowers/plans/2026-08-11-operation-feedback.md @@ -0,0 +1,334 @@ +# Feedback für langlaufende Operationen — Vorgaben für die Umsetzung + +> **Dieses Dokument ist absichtlich kein Task-Skript.** Es enthält die Vorgaben pro Gruppe. +> Die konkreten Tasks schreibt der **Merge-Helper je Gruppe** beim Ausführen — er kennt dann +> den tatsächlichen Codestand. Was hier steht, ist bindend; was hier nicht steht, entscheidet +> er. Checkboxen stehen auf Paketebene, damit der Fortschritt sichtbar bleibt. + +**Design:** `docs/superpowers/specs/2026-08-11-operation-feedback-design.md` — dort stehen die +Belege (Datei:Zeile) und die Begründungen. Dieses Dokument wiederholt sie nicht. + +**Ziel:** Jede Operation, die länger als ~300 ms dauern kann, zeigt an, dass sie läuft, was sie +tut und wie lange sie schon läuft — über **einen** Mechanismus statt pro Fall neu gebaut. + +--- + +## Ausführungsmodell + +``` +P0 (Fundament) ── muss ALLEIN und ZUERST landen + │ + ├── Gruppe A UI-Stille Merge-Helper 1 + ├── Gruppe B UI-Freeze Merge-Helper 2 + └── Gruppe C Worker-Stille Merge-Helper 3 + +Gruppe D MCP-Stille Merge-Helper 4 ── unabhängig von P0, kann sofort starten +``` + +**Ein Merge-Helper pro Gruppe.** Nach P0 laufen A, B, C und D parallel. Jede Gruppe hält sich +strikt an ihr Datei-Eigentum — das ist die einzige Absicherung gegen gegenseitiges +Überschreiben, weil alle im gemeinsamen `main`-Checkout arbeiten. + +### Datei-Eigentum (bindend) + +| Gruppe | Besitzt exklusiv | Darf **nicht** anfassen | +|---|---|---| +| **P0** | `Ui/Services/OperationStatus.cs` (neu), `Ui/Views/Controls/OperationIndicator.axaml(.cs)` (neu), **beide `locales/*.json`**, `Ui/Services/WorkerClient.cs` (nur Timing-Hook) | alles andere | +| **A** | Island-VMs, Modal-VMs, deren AXAML, `Ui/Services/UpdateCheckService.cs` | `locales/*`, `WorkerClient`, `IWorkerClient`, Worker-Projekt | +| **B** | `Ui/ViewModels/Modals/DiffViewerViewModel.cs`, `Ui/Views/Controls/DiffTextView.axaml.cs` | `locales/*`, Island-VMs, Worker-Projekt | +| **C** | `Worker/Hub/HubBroadcaster.cs`, `Worker/Hub/WorkerHub.cs`, `Ui/Services/WorkerClient.cs`, `IWorkerClient.cs`, `StubWorkerClient.cs`, `Worker/Lifecycle/*Recovery.cs`, `Worker/Runner/WorktreeManager.cs`, `Worker/Worktrees/WorktreeMaintenanceService.cs` | `locales/*`, Island-VMs, `Worker/External/*` | +| **D** | `Worker/External/*McpTools.cs`, `Worker/External/ExternalMcpService.cs`, `Worker/Lifecycle/TaskMergeService.cs` (nur die Progress-Schleife) | `locales/*`, alles im Ui-Projekt | + +**Locale-Regel:** Nur P0 schreibt in `en.json`/`de.json`. Braucht eine Gruppe doch einen Key, +den P0 nicht vorgesehen hat: als **letzte** Änderung des Pakets anhängen und im Merge-Helper +als bekannte Konfliktstelle behandeln. Nie mitten in der Gruppe. + +--- + +## Vorgaben für den Merge-Helper selbst + +Gilt für alle vier Gruppen: + +- **Agent-Modell:** `sonnet` für Implementierer und Reviewer. Nie haiku, nie opus, nie Fable. +- **`maxTurns` explizit auf 200 setzen.** `model_presets` ist NULL, sonst bekommt ein + sonnet-Task 30 Turns und stirbt mit „exited with code 1 and no result". +- **`serializeOnFileOverlap` auf der Gruppenliste einschalten.** Innerhalb einer Gruppe + fassen mehrere Pakete dieselben Dateien an. +- **Bauen:** `dotnet build ClaudeDo.slnx` schlägt auf .NET 8 fehl. Einzelprojekte mit + `-c Release` bauen (ein laufender Worker sperrt `Debug`): + ``` + dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release + dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj -c Release + ``` +- **Nie `git add -A`.** Immer explizit nach Pfad stagen und `git commit -- ` — der + `main`-Checkout ist von parallelen Sessions geteilt, ein blankes Commit fegt fremde WIP mit. +- **Nach dem Batch-Merge `main` selbst prüfen.** Approve baut und testet nicht. Neun grüne + Branches haben `main` schon zweimal zerlegt, davon einmal durch eine Test-Kollision, die ein + reiner src-Build nicht sieht. Also: Build **und** die betroffenen Testprojekte. +- **Vor Approve prüfen, ob der Branch überhaupt etwas enthält** (`changedFileCount`). Ein + blockierter Task wird sonst `Done` mit leerem Branch. +- **Keine EF-Migration** in diesem Vorhaben. Parallele Migrationen aus Geschwister-Branches + löschen sich beim SQLite-Table-Rebuild gegenseitig die Spalten, und die Tests bemerken es + nicht (`EnsureCreated`). +- **Keine Tests, die die echte `claude`-CLI starten.** +- **Visuelle Prüfung ist nicht durch Tests ersetzbar.** Jedes Paket in A und B endet mit einer + offenen visuellen Prüfung für den Nutzer. Nie behaupten, die UI funktioniere. + +--- + +## P0 — Fundament + +- [ ] **P0-1 — `OperationStatus` + `OperationIndicator` + Locale-Keys** +- [ ] **P0-2 — Timing-Hook für die Messung** +- [ ] **P0-3 — Regel in den CLAUDE.md-Dateien** + +**Vorbedingung:** keine. Muss allein landen, bevor A/B/C starten. + +### Der Vertrag (bindend — 19 Pakete hängen daran) + +`src/ClaudeDo.Ui/Services/OperationStatus.cs`, eine `ObservableObject`-Klasse: + +| Member | Verhalten | +|---|---| +| `bool IsRunning` | **sofort** `true` bei `Begin`. Treibt `CanExecute`, verhindert Doppelklick | +| `bool ShowIndicator` | erst nach **300 ms** `true`. Kein Flackern bei schnellen Calls | +| `string? Label` | lokalisierter Text | +| `string Elapsed` | `mm:ss`, **lokal getickt** | +| `bool IsStalled` | `true` nach **60 s ohne `Report`** | +| `IDisposable Begin(string label)` | startet; `Dispose` beendet **auch im Exception-Fall** | +| `void Report(string label)` | überschreibt `Label` mid-flight, setzt die Stall-Uhr zurück | + +**Nachbesserung 1 (bindend):** `IsStalled` bemisst sich an der Zeit **seit dem letzten +`Report`**, nicht an der Gesamtdauer. Ein regulär mehrminütiges Verify-Gate darf sich nicht +selbst als hängend melden. Eine Operation ohne jeden `Report` gilt nach 60 s als stalled — das +ist genau der Fall, der heute wie ein Absturz aussieht. + +Benutzung im ViewModel: + +```csharp +using var op = Approve.Begin(Loc.T("ops.merge.merging")); +var result = await _worker.ApproveReviewAsync(...); +``` + +### Weitere Vorgaben + +- **Zeitquelle injizierbar** über `TimeProvider` (in .NET 8 vorhanden). **Kein statischer + `DispatcherTimer`** — geteilter statischer State ist die Ursache der reihenfolgen-abhängigen + Flakiness in den `Ui.Tests`. Timer-Callbacks kommen vom Threadpool und müssen auf den + Dispatcher gepostet werden. +- **Mehrere `OperationStatus` pro VM sind erwünscht.** `WorktreesOverviewModalViewModel` + braucht getrennte für Refresh, Cleanup und Merge — sonst blockiert ein laufender Refresh die + Cleanup-Anzeige. +- **`OperationStatus` transportiert keine Fehler.** Fehlerbehandlung bleibt unverändert über + `ShowErrorAsync` / `ErrorReported` / `FlashFooterError`. +- **`OperationIndicator`** nutzt `Ellipse.spinner` aus `Design/IslandStyles.axaml` (14×14, + Accent, 0.9 s Rotation) plus Label und Elapsed. Ohne dieses Control wird die + Spinner-StackPanel aus `MergeModalView.axaml` acht Mal von Hand nachgebaut. Werte aus + `Tokens.axaml` verwenden, keine Inline-Zahlen. +- **Locale-Keys:** neuer Top-Level-Namespace `ops` in `en.json` und `de.json`, Parität wird von + `Localization.Tests` erzwungen. P0 legt die Keys für **A und C** vorab an — abgeleitet aus den + Paketlisten unten. + +**Nachbesserung 3 (bindend):** **Gruppe D braucht keine Locale-Keys.** +MCP-Progress-Meldungen gehen an Agenten, nicht an den Nutzer, und bleiben englische +Klartext-Strings. + +### P0-2: Messung an genau einer Stelle + +Zwei Hooks, nicht zwanzig: +1. In `WorkerClient` jeden Hub-Invoke mit Dauer loggen. +2. Im DB-Pfad der Islands dasselbe. + +Ein Tag Nutzung liefert eine sortierte Liste echter Ausreißer. **A5 und C werten sie aus**, +statt zu raten. Bild 2 und 4 brauchen keine Messung. + +Faustregel für den Zweifelsfall: instrumentiert wird, was git aufruft, Netz nutzt, einen +Prozess startet, oder O(n) über unbegrenzt viele DB-Zeilen läuft. Einzelzeilen-Reads nicht. + +### Definition of Done für P0 + +`OperationStatus` hat Unit-Tests mit einem Fake-`TimeProvider` für: 300-ms-Grace, 60-s-Stall +**ab letztem Report**, Elapsed-Formatierung, `Dispose` nach Exception, `Report` überschreibt +Label und setzt die Stall-Uhr zurück. `Localization.Tests` grün. Kein VM ist umgestellt — P0 +liefert nur das Werkzeug. + +--- + +## Gruppe A — Bild 1: UI-Stille + +- [ ] **A1 — Detail-Pane** ⚠️ *der gemeldete Fall* +- [ ] **A2 — WorktreesOverview** +- [ ] **A3 — Settings-Tabs** +- [ ] **A4 — Reports und Planning-UI** +- [ ] **A5 — Island-DB-Pfade nach Messdaten** + +**Vorbedingung:** P0 gemergt. + +### Vorgaben + +- **A1 zuerst.** `DetailsIslandViewModel.ApproveReviewAsync` ist der gemeldete Schmerz. Dazu im + selben Paket: `SubmitForReviewAsync`, `RejectReviewAsync`, `ParkReviewAsync` und + `MergeSectionViewModel.PreviewMergeAsync`. +- **A1 abonniert `IWorkerClient.MergeProgressEvent`**, um das Label mid-flight von „merging" auf + „verifying" zu schärfen. Das Event existiert seit `cad0582`. + + **Nachbesserung 2 (bindend):** A verlässt sich darauf, dass dieses Event **bestehen bleibt**. + C1 baut den generischen Kanal darunter, muss `MergeProgressEvent` aber als dünnen Forwarder + erhalten. Andernfalls müsste C `DetailsIslandViewModel` ändern — eine Datei aus Gruppe A — + und die Parallelität wäre zerstört. **A darf keinen anderen Kanal verwenden.** +- **Abo nur für die Dauer des Calls.** Der Worker broadcastet an alle Clients; ein dauerhaft + registriertes VM bekommt Events für fremde Tasks. `MergeModalViewModel` macht es richtig + vor: `+=` im `try`, `-=` im `finally`, plus `if (taskId != TaskId) return;`. +- **A2:** `IsBusy` existiert dort schon, schaltet aber nur `IsEnabled`. Die Anzeige fehlt + komplett. Betrifft Refresh, Cleanup, Reset, ForceRemove und Batch-Merge — Batch-Merge + zusätzlich mit Zeilen-Status, weil dort N Tasks sequenziell durchlaufen. +- **A3:** SessionSkill-Install ist ein `git clone` — der offensichtlichste Kandidat. Dazu + Update/Restore-Defaults, OnlineInbox-SignIn, RepoImport-Scan, Update-Check. +- **A4:** GenerateWeekReport, RunDailyPrepNow, BuildPlanningIntegrationBranch, + GetPlanningAggregate, Finalize/QueuePlanningSubtasks. `DiffViewerViewModel.IsLoadingCombined` + existiert bereits und bleibt — nicht doppeln. +- **A5 erst nach Auswertung von P0-2.** Nur Stellen umstellen, die die Messung als Ausreißer + zeigt. Kein Umstellen auf Verdacht. +- **`DetailsIslandViewModel` ist groß.** Nicht umstrukturieren, nur die Commands anfassen. +- **Bereits saubere Flächen nicht anfassen:** `MergeModalViewModel`, + `ConflictResolverViewModel`, `DiffViewerViewModel.IsLoadingCombined`, + `UsageMonitorModalViewModel`. + +### Definition of Done pro Paket + +Jeder umgestellte Command: `IsRunning` während des Laufs gesetzt, `CanExecute` gesperrt, +Zustand nach einer Exception zurückgesetzt — als Test in `ClaudeDo.Ui.Tests`. Kein +handgebauter Spinner, immer `OperationIndicator`. **Visuelle Prüfung offen und explizit +benannt** (Grace-Periode und Layout kann kein Test bestätigen). + +--- + +## Gruppe B — Bild 2: UI-Freeze + +- [ ] **B1 — `UnifiedDiffParser.Parse` auslagern** +- [ ] **B2 — `DiffAlignment.Build` Grenze** +- [ ] **B3 — Guard-Test gegen Dispatcher-Blockade** + +**Vorbedingung:** P0 gemergt (für den Indikator in B1). + +### Vorgaben + +- **Reihenfolge ist zwingend: erst auslagern, dann anzeigen.** Ein Spinner auf einem + blockierten UI-Thread wird nicht gezeichnet. B1 verschiebt `UnifiedDiffParser.Parse` nach + `Task.Run` und setzt danach den Indikator. +- **`UnifiedDiffParser` ist statisch und rein** — thread-safe, `Task.Run` ist unkritisch. Das + Zurückschreiben der Ergebnisse in Observable-Collections muss auf dem UI-Thread passieren. +- **B2 braucht eine Entscheidung, die der Merge-Helper trifft:** `DiffAlignment.Build` läuft in + `DiffTextView.axaml.cs` — in einem Control, nicht in einem VM. Entweder Grenze („ab N Zeilen + auslagern") oder inkrementeller Aufbau. Die Wahl gehört ins Paket, nicht hierher; die + Vorgabe ist nur: **eine sichtbare Grenze definieren, keine unbegrenzte Synchron-Arbeit.** +- **AvaloniaEdit-Fallen** (belegt, nicht neu ausprobieren): `this.TryGetResource` in einem + Control findet Brushes aus `Tokens.axaml` **nie** und scheitert lautlos; Brushes und Typeface + nicht im Konstruktor auflösen. `ScrollToVerticalOffset` ist in v12 ein No-op — schreiben über + `ScrollViewer.Offset`, lesen über `TextView.ScrollOffsetChanged`. +- Gemeinsames Editor-Boilerplate gehört in `Views/Controls/DiffEditorSetup.cs`, nicht ein + drittes Mal kopiert. + +### Definition of Done + +B3 ist der einzige Test im Vorhaben, der Blockade prüft: großes Diff-Fixture, Nachweis, dass +der Dispatcher weiter Nachrichten verarbeitet. Visuelle Prüfung offen (Verhalten bei großem +Diff, Indikator während des Parsens). + +--- + +## Gruppe C — Bild 3: Worker-Stille + +- [ ] **C1 — generischer `OperationProgress`-Kanal** +- [ ] **C2 — Startup-Recovery sichtbar** +- [ ] **C3 — Worktree-Anlage sichtbar** +- [ ] **C4 — Rebase-after-Merge und WorktreeMaintenance** +- [ ] **C5 — periodische Dienste** + +**Vorbedingung:** P0 gemergt. Die Abhängigkeit zur Parallelsession ist mit `cad0582` +aufgelöst. + +### Vorgaben + +- **Der Kanal:** + ``` + OperationProgress(string opKey, string phase, int current, int total) + ``` + `opKey` = TaskId bei task-gebundenen Operationen, sonst ein stabiler String + (`"worktree-cleanup"`, `"startup-recovery"`, `"planning-integration:"`). +- **Kein Elapsed auf der Leitung.** Das UI tickt lokal. Die heutige Implementierung lässt den + Worker alle 30 s ticken — damit zeigt sie die ersten 30 Sekunden nur „merging". C1 stellt das + um. +- **`MergeProgressEvent` bleibt als Forwarder** (siehe Nachbesserung 2 unter Gruppe A). Vier + Zeilen. Entfernen erst, wenn A und C beide gemergt sind — nicht in dieser Gruppe. +- **`current`/`total` wird als Text gezeigt, nie als Prozentbalken.** Die meisten Operationen + haben kein sinnvolles Total. +- **C3 zuerst prüfen, nicht bauen:** + `docs/superpowers/specs/2026-08-07-ui-reaktivitaet-und-listen-performance-design.md` führt + `WorktreeManager.cs:103` als DB-Write-ohne-Broadcast. Ob das inzwischen geschlossen ist, + gehört geprüft, bevor es doppelt behoben wird. +- **C5 ist der wackeligste Punkt im Design.** Periodische Dienste (Usage, OnlineSync, Prime, + Queue) dürfen **nur Aktivität und Fehler** melden, keinen Tick-Strom. Wenn der Merge-Helper + beim Umsetzen zum Schluss kommt, dass C5 nur Rauschen erzeugt: **weglassen und begründen**, + statt es durchzuziehen. +- **Test-Fakes wachsen mit.** Ein neues Event auf `IWorkerClient`/`WorkerHub` bricht + handgeschriebene Fakes in **beiden** Testprojekten — `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs` + und die UiVm-Tests unter `tests/ClaudeDo.Worker.Tests/`. +- **Broadcast-Sparsamkeit:** vor einem „fehlenden Broadcast" immer den Aufrufer prüfen. Im + 08-07-Spec war ein gemeldetes Loch in Wahrheit schon vom Aufrufer abgedeckt, und der Fix + wäre ein Duplikat gewesen. + +### Definition of Done + +Progress-Callbacks in `ClaudeDo.Worker.Tests` mit einem Fake gezählt — keine echten Timeouts, +keine echte CLI. UI-Seite: die Anzeige beim Worker-Start ersetzt „reconnecting" durch die +laufende Recovery-Phase. Visuelle Prüfung offen. + +--- + +## Gruppe D — Bild 4: MCP-Stille + +- [ ] **D1 — `ProgressReporter` extrahieren** +- [ ] **D2 — die 8 `batch_*`-Tools** +- [ ] **D3 — Worktree- und Diff-Tools** +- [ ] **D4 — Rest und Doku-Regel** + +**Vorbedingung:** keine. Kann sofort starten, parallel zu P0. + +### Vorgaben + +- **Warum das zählt:** ohne Progress bricht der MCP-Client bei Stille nach 300 s ab, während + der Worker weiterarbeitet. Das ist die dokumentierte Ursache dafür, dass ein Merge zwar + durchläuft, den Task aber nie auf `Done` bringt und Abhängige dauerhaft blockiert. +- **D1:** `TaskMergeService.RunReportingProgressAsync` ist die einzige existierende + Progress-Schleife. Als eigenständige Klasse extrahieren, plus ein Overload für + Element-Fortschritt (`i/n`). Nicht kopieren — extrahieren, damit es eine Implementierung + bleibt. +- **`TaskMergeService` ist die einzige Datei, die D mit C teilt.** D fasst dort ausschließlich + die Progress-Schleife an. Wenn beide Gruppen gleichzeitig laufen, ist das die Stelle, die der + Merge-Helper im Auge behalten muss. +- **Keine Locale-Keys** (Nachbesserung 3). Englische Klartext-Strings. +- **Zielumfang:** die 8 `batch_*`-Tools mit `i/n`, dann `cleanup_task_worktree`, + `batch_cleanup_task_worktrees`, `get_task_diff`, `preview_merge_set`. Bereits versorgt und + nicht anzufassen: die 5 Tools in `ExternalMcpService` mit `IProgress`, `TaskWaitMcpTools`, + `merge_task`/`review_task`. +- **D4 schreibt die Regel ins Worker-`CLAUDE.md`:** ein MCP-Tool, das über ~5 s laufen kann, + reportet Progress. Ohne die Regel wiederholt sich das Muster beim nächsten Tool. + +### Definition of Done + +Für jedes umgestellte Tool ein Test in `ClaudeDo.Worker.Tests`, der mit einem Fake-`IProgress` +zählt, dass Meldungen kommen — bei `batch_*` mindestens eine pro Element. Keine visuelle +Prüfung nötig, diese Gruppe hat keine UI. + +--- + +## Was nach allen vier Gruppen offen bleibt + +- **Visuelle Prüfung** für alle Pakete in A und B — der Nutzer, nicht ein Agent. +- **Auswertung von P0-2** steuert A5 und die Feinheiten in C. Vorher ist A5 absichtlich + unbestimmt. +- **`MergeProgressEvent`-Forwarder entfernen**, sobald A und C beide gemergt sind. Ein + Aufräum-Task, kein Paket. +- **Bewusst nicht gebaut:** Cancel, Footer-Anzeige für laufende Operationen, Prozentbalken, + Änderungen am Installer (der hat bereits eine vollständige Progress-Pipeline).