Files
ClaudeDo/docs/superpowers/plans/2026-08-11-operation-feedback.md
T

335 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 -- <pfade>` — 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:<taskId>"`).
- **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).