diff --git a/docs/superpowers/specs/2026-08-11-operation-feedback-design.md b/docs/superpowers/specs/2026-08-11-operation-feedback-design.md new file mode 100644 index 00000000..02d5404f --- /dev/null +++ b/docs/superpowers/specs/2026-08-11-operation-feedback-design.md @@ -0,0 +1,311 @@ +# Feedback für langlaufende Operationen + +**Datum:** 2026-08-11 +**Status:** Design freigegeben, Implementierung offen +**Auslöser:** Ein Merge über das Detail-Pane blockierte minutenlang ohne jede Anzeige. + +## Problem + +Langlaufende Operationen geben kein Feedback. Der Nutzer klickt, nichts passiert sichtbar, +die App liest sich als abgestürzt. Der gemeldete Fall war ein Merge mit Verify-Gate, aber das +ist ein Symptom eines Musters: Feedback wird **pro Fall ad-hoc** gebaut +(`PrepStarted/Line/Finished`, `RefineStarted/Finished`, `PlanningMerge*`, `MergeProgress`), +und dabei geht regelmäßig eine Hälfte verloren oder eine Fläche wird vergessen. + +Belegend: `HubBroadcaster.MergeProgress` existierte bereits, während `WorkerClient` noch +keinen Listener hatte — Sender ohne Empfänger. Das wurde parallel zu diesem Design in einer +anderen Session nachgezogen (siehe *Abhängigkeit zur Parallelarbeit*), deckt aber nur das +MergeModal ab, nicht den Approve-Pfad im Detail-Pane. + +## Vier Fehlerbilder + +Die Analyse trennt vier Klassen, die **verschiedene** Lösungen brauchen. Sie zusammen als +„fehlendes Feedback" zu behandeln war der Fehler der bisherigen Einzelfall-Fixes. + +### Bild 1 — Stiller Await im UI + +Ein `[RelayCommand]` awaitet einen Call, ohne Busy-State, ohne Anzeige. Der Button bleibt +klickbar (Doppelklick-Risiko), es gibt keinen Hinweis, dass etwas läuft. + +| Stelle | Befund | +|---|---| +| `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs:1206` `ApproveReviewAsync` | **Der gemeldete Fall.** Kein Busy-Flag, kein Indikator; das Verify-Gate kann den Call Minuten halten | +| `DetailsIslandViewModel.cs:1244` `SubmitForReviewAsync` | kein Busy-Flag (`MissionControlViewModel` hat für denselben Call ein `IsSubmitPending`, das Detail-Pane nicht) | +| `DetailsIslandViewModel.cs` `RejectReviewAsync`, `ParkReviewAsync` | kein Busy-Flag; Fehler werden verschluckt (`catch { return; }`) | +| `src/ClaudeDo.Ui/ViewModels/Islands/MergeSectionViewModel.cs:121` `PreviewMergeAsync` | kein Indikator | +| `WorktreesOverviewModalViewModel` (`IsBusy` vorhanden) | `IsBusy` schaltet in `WorktreesOverviewModalView.axaml:107-108` **nur** Buttons aus — kein Spinner, kein Text. Betrifft Cleanup, Reset, ForceRemove, Refresh, Batch-Merge | +| Settings-Tabs: `SessionSkillsSettingsTabViewModel`, `FilesSettingsTabViewModel`, `OnlineInboxSettingsViewModel` | dito — `IsEnabled="{Binding !IsBusy}"`, keine Anzeige. Skill-Install ist ein `git clone` | +| `WeeklyReportModalViewModel`, `RepoImportModalViewModel` | `IsBusy` nur als CanExecute-Gate | +| Islands ↔ SQLite direkt | Die Islands sprechen direkt mit der DB (`new TaskRepository(ctx)` u.ä., ~15 Stellen allein in `DetailsIslandViewModel`). Meist schnell; `ClearCompleted`, Drag-Reorder über viele Zeilen und das Laden großer Listen nicht zwingend | + +Bereits sauber gelöst und **nicht** anzufassen: `MergeModalViewModel` (Spinner + Phase + +Elapsed), `ConflictResolverViewModel` (`IsBusy` + Text), `DiffViewerViewModel.IsLoadingCombined`, +`UsageMonitorModalViewModel` (Spinner). + +### Bild 2 — UI-Thread-Freeze + +Eine andere Fehlerklasse: hier hilft kein Indikator, weil der Dispatcher blockiert ist und +der Spinner nicht gezeichnet werden könnte. Erst auslagern, dann anzeigen. + +| Stelle | Befund | +|---|---| +| `src/ClaudeDo.Ui/ViewModels/Modals/DiffViewerViewModel.cs:182` und `:260` | `UnifiedDiffParser.Parse(raw)` läuft **synchron auf dem UI-Thread** | +| `src/ClaudeDo.Ui/Views/Controls/DiffTextView.axaml.cs:102` | `DiffAlignment.Build(File?.Lines)` ebenfalls | + +Im gesamten `ClaudeDo.Ui`-Projekt gibt es **zwei** `Task.Run`-Offloads (`RepoScanner`, +`RestartWorkerService`). CPU-Arbeit auf dem UI-Thread ist damit die Norm, nicht die Ausnahme. + +### Bild 3 — Worker-Stille + +Operationen ohne UI-Auslöser. Kein Client erfährt, dass sie laufen; die UI zeigt im +Startfall nur „reconnecting". + +| Stelle | Warum langsam | +|---|---| +| 6× `src/ClaudeDo.Worker/Lifecycle/*Recovery.cs` beim Worker-Start (`OrphanRecovery`, `StaleTaskRecovery`, `PromptFileRecovery`, `AttachmentOrphanRecovery`, `PlanningLineageRecovery`, `LegacyWorktreeFolderRecovery`) | git + DB über alle Tasks/Worktrees | +| `src/ClaudeDo.Worker/Runner/WorktreeManager.cs` — Worktree-Anlage beim Task-Start | `git worktree add` + Branch; stille Lücke zwischen `Queued` und erster Ausgabe | +| `src/ClaudeDo.Worker/Worktrees/WorktreeMaintenanceService.cs` | Hintergrund-git über alle Worktrees | +| `TaskMergeService.RebaseOthersAfterMergeAsync` | läuft **innerhalb** des Merge-Calls, **nach** dem Phasen-Broadcast — vollständig unsichtbar | +| `PlanningAggregator`, `PlanningMergeOrchestrator` | ein Merge pro Subtask | +| `OnlineSyncService`, `UsageMonitorService`, `PrimeScheduler`, `QueueService` | periodisch, Netz/git | + +Querverweis: `docs/superpowers/specs/2026-08-07-ui-reaktivitaet-und-listen-performance-design.md` +führt `WorktreeManager.cs:103` als DB-Write-ohne-Broadcast (Loch 3). C3 muss prüfen, ob das +inzwischen geschlossen ist, statt es doppelt zu beheben. + +### Bild 4 — MCP-Stille + +Trifft nicht die UI, sondern Agenten-Runs. Nur drei Dateien reporten Progress: +`ExternalMcpService` (5 Tools), `TaskWaitMcpTools`, `TaskMergeService`. Ohne Progress laufen +u.a. die 8 `batch_*`-Tools, `cleanup_task_worktree`, `get_task_diff`, `preview_merge_set`. + +Das ist die dokumentierte Ursache des Traps „MCP 300s idle abort leaves merges stuck": der +Client bricht bei Stille ab, während der Worker weiterarbeitet, und der Task bleibt hängen. + +### Bewusst ausgeschlossen: der Installer + +`ClaudeDo.Installer` hat eine vollständige `IProgress`-Step-Pipeline mit Live-Anzeige +(`Core/InstallerService.cs`, `Steps/*`, `Pages/InstallPage`). Das ist die beste Progress-Fläche +im Repo und braucht nichts. + +## Design + +### 1. UI-Primitive: `OperationStatus` + +Eine `ObservableObject`-Klasse, die ein ViewModel als Property exponiert. Mehrere pro VM sind +erlaubt und erwünscht — `WorktreesOverviewModalViewModel` braucht getrennte für Refresh, +Cleanup und Merge, sonst blockiert ein laufender Refresh die Cleanup-Anzeige. + +| Property | Verhalten | +|---|---| +| `IsRunning` | sofort `true` → treibt `CanExecute`, verhindert Doppelklick | +| `ShowIndicator` | erst nach **300 ms** `true` → kein Flackern bei schnellen Calls | +| `Label` | lokalisierter Text, **mid-flight überschreibbar** (`Report(label)`) | +| `Elapsed` | `mm:ss`, **lokal getickt** | +| `IsStalled` | `true`, wenn **60 s ohne Aktualisierung** vergangen sind → Hinweis „läuft weiter, Worker antwortet noch" | + +`IsStalled` bemisst sich an der Zeit seit dem letzten `Report`, **nicht** an der Gesamtdauer. +Sonst würde ein regulär mehrminütiges Verify-Gate fälschlich als hängend gemeldet. Eine +Operation ohne jeden `Report` gilt nach 60 s als stalled — das ist der Fall, der heute wie ein +Absturz aussieht. + +Benutzung: + +```csharp +using var op = Approve.Begin(Loc.T("ops.merge.merging")); +var result = await _worker.ApproveReviewAsync(...); +``` + +`Dispose` beendet die Operation auch im Exception-Fall. + +**Zeitquelle ist injizierbar** (`TimeProvider`, in .NET 8 vorhanden), kein statischer +`DispatcherTimer`. Ein geteilter statischer Timer wäre genau die Sorte Shared State, die die +`Ui.Tests` reihenfolgen-abhängig flaky macht. Timer-Callbacks kommen vom Threadpool und müssen +auf den Dispatcher gepostet werden. + +**Fehlerbehandlung bleibt unverändert.** Weiter über `ShowErrorAsync` / `ErrorReported` / +`FlashFooterError`. `OperationStatus` transportiert keine Fehler. + +### 2. UI-Control: `OperationIndicator` + +`src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml` — Spinner (`Ellipse.spinner` aus +`IslandStyles`) + Label + Elapsed, gebunden an eine `OperationStatus`. Ohne das Control wird +die Spinner-StackPanel aus `MergeModalView.axaml:28-30` acht Mal von Hand nachgebaut. + +Alle Texte über Locale-Keys im Namespace `ops.*`; en/de-Parität erzwingen die +`Localization.Tests`. + +### 3. Worker-Kanal: ein generisches Progress-Event + +Der Worker sendet nur, was das UI **nicht erraten kann** — interne Phasen und Zählstände. Beim +Klick weiß das UI selbst, was es angestoßen hat. + +``` +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 tickt das UI +- `current`/`total` erlaubt echten Fortschritt („Worktree 3/12"), was das heutige + `MergeProgress` nicht kann + +Dieses Event **ersetzt** `MergeProgress`. Begründung, dass sich die Verallgemeinerung lohnt: +es gibt drei Produzenten (Merge-Phasen, Worktree-Cleanup, Planning-Integration), nicht einen. + +**Aber `IWorkerClient.MergeProgressEvent` bleibt als dünner Forwarder bestehen.** Sonst +zerstört C1 die Gruppen-Isolation: A1 abonniert dieses Event, und ein Rename würde C zwingen, +`DetailsIslandViewModel` zu ändern — eine Datei, die Gruppe A besitzt. Der Forwarder kostet +vier Zeilen und hält die vier Sessions unabhängig. Er darf später entfernt werden, wenn A und +C beide gemergt sind. + +### 4. MCP: `ProgressReporter` als eigene Klasse + +`TaskMergeService.RunReportingProgressAsync` (Zeile ~175) ist die einzige existierende +Progress-Schleife. Sie wird als eigenständige Klasse extrahiert, sodass jedes MCP-Tool sie +nutzen kann, statt die Schleife zu kopieren. Zusätzlich ein Overload für Element-Fortschritt +(`i/n`) für die `batch_*`-Tools. + +## Festgelegte Grenzen + +Bewusst **nicht** Teil dieses Designs: + +- **Kein Cancel.** Nur Sichtbarkeit. Ein Abbruch wäre bei einem Merge ohnehin irreführend: das + Verify-Gate läuft, wenn der Merge-Commit längst geschrieben ist — „abbrechen" könnte nur + *aufhören zu warten* bedeuten, nicht zurückrollen. Ohne Cancel entfällt der + `opId`-Handshake; die Korrelation läuft über `taskId` bzw. das offene Modal. +- **Keine Footer-Anzeige für laufende Operationen.** Anzeige am auslösenden Control und an der + Task-Zeile / im Detail-Pane. Der Footer bleibt für Fehler (`FlashFooterError`) und den + Worker-Log. +- **Keine Prozent-Balken.** Die meisten Operationen haben kein sinnvolles Total. + `current/total` wird als Text gezeigt, nicht als Balken. +- **Keine EF-Migration.** Damit entfällt der Trap paralleler Migrationen. + +## Messung + +„Erst messen, dann instrumentieren" gilt für Bild 1 und 3 — aber an **einer** Stelle, nicht an +zwanzig: + +1. Ein Timing-Hook in `WorkerClient` loggt jeden Hub-Invoke mit Dauer. +2. Ein zweiter im DB-Pfad der Islands. + +Ein Tag Nutzung liefert eine sortierte Liste echter Ausreißer. A5 und C entscheiden danach, +statt zu raten. Bild 2 und 4 brauchen keine Messung — der Befund ist aus dem Code eindeutig. + +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. + +## Parallelisierung: vier Merge-Helper-Gruppen + +Der Zuschnitt folgt dem **Datei-Eigentum**, nicht der Fachlichkeit — nur so können vier +Sessions gleichzeitig laufen, ohne sich zu überschreiben. + +``` +P0 (Fundament, muss allein zuerst landen) + ├── Gruppe A Bild 1: UI-Stille (5 Pakete) + └── Gruppe B Bild 2: UI-Freeze (3 Pakete) + +Gruppe D Bild 4: MCP (4 Pakete) ← braucht P0 NICHT, kann sofort starten +Gruppe C Bild 3: Worker (5 Pakete) ← wartet auf den Peer-Commit +``` + +| Gruppe | Exklusiv besessene Dateien | +|---|---| +| **P0** | `OperationStatus.cs`, `OperationIndicator.axaml`, **beide `locales/*.json`** | +| **A** | Island-VMs, Modal-VMs und deren AXAML | +| **B** | `DiffViewerViewModel.cs`, `DiffTextView.axaml.cs` | +| **C** | `HubBroadcaster`, `WorkerHub`, `WorkerClient`, `IWorkerClient`, `StubWorkerClient`, `Lifecycle/*Recovery`, `WorktreeManager` | +| **D** | `Worker/External/*McpTools`, `ExternalMcpService` | + +**Der einzige echte Konfliktkandidat sind die Locale-Dateien.** A und C brauchen Keys — D +nicht: MCP-Progress-Meldungen gehen an Agenten, nicht an den Nutzer, und bleiben englische +Klartext-Strings ohne Locale-Eintrag. + +Regel: **P0 legt die Keys für A und C vorab an**, danach fasst keine Gruppe die JSONs mehr an. +Braucht eine Gruppe doch einen nicht vorgesehenen Key, hängt sie ihn als **letzte** Änderung +des Pakets an und behandelt ihn im Merge-Helper als bekannte Konfliktstelle. + +Drei erzwungene Serialisierungen: +- `TaskMergeService` — die Parallelsession arbeitet dort; C1 danach. +- `WorkerClient.cs` — P0-2 setzt dort den Timing-Hook, C ändert dieselbe Datei. P0 liegt + ohnehin vor C, damit unkritisch — aber C darf nicht vor P0 starten. +- Innerhalb jeder Gruppe `serializeOnFileOverlap` auf der Liste setzen. + +Ergänzung zum Datei-Eigentum: `Services/UpdateCheckService.cs` gehört zu **A** (Paket A3), +`Services/WorkerClient.cs` zu **C**. Beide liegen im selben Ordner, sind aber verschiedene +Dateien. + +### Abhängigkeit zur Parallelarbeit + +Während dieses Designs hat eine andere Session im gemeinsamen `main`-Checkout das +Merge-Feedback für das **MergeModal** implementiert: Phasen-Broadcast in `TaskMergeService`, +`HubBroadcaster.MergeProgress`, `IWorkerClient.MergeProgressEvent`, +`MergeModalViewModel.ProgressMessage`, Spinner in `MergeModalView.axaml`, plus Tests. + +Konsequenzen: +- Der Merge-Fall im MergeModal ist **Vorarbeit**, nicht neu zu bauen. +- Zwei Nachbesserungen bleiben: A1 schließt den Approve-Pfad im Detail-Pane an, C1 stellt + Elapsed auf UI-lokalen Tick um (die Implementierung lässt den Worker alle 30 s ticken — + die ersten 30 Sekunden zeigt sie nur „merging"). +- C darf erst starten, wenn dieser Commit liegt. + +## Pakete + +### P0 — Fundament + +| # | Inhalt | +|---|---| +| P0-1 | `OperationStatus` + `OperationIndicator` + `ops.*`-Keys **für alle vier Gruppen vorab** + `Ui.Tests` mit Fake-`TimeProvider` | +| P0-2 | Timing-Hook in `WorkerClient` + im DB-Pfad der Islands | +| P0-3 | Regel in `src/ClaudeDo.Ui/CLAUDE.md` und `src/ClaudeDo.Worker/CLAUDE.md`: jeder `IWorkerClient`-Call in einem `[RelayCommand]` läuft durch eine `OperationStatus`; MCP-Tools über ~5 s reporten Progress | + +### Gruppe A — Bild 1: UI-Stille + +| # | Inhalt | +|---|---| +| A1 | Detail-Pane: Approve & Merge, Submit for review, Reject, Park, Preview-Merge. Abonniert das Merge-Phasen-Event zum Nachschärfen des Labels | +| A2 | WorktreesOverview: Refresh, Cleanup, Reset, ForceRemove, Batch-Merge (inkl. Zeilen-Status) | +| A3 | Settings-Tabs: SessionSkill install/update, Restore-Defaults, OnlineInbox-SignIn, RepoImport-Scan, Update-Check | +| A4 | Reports und Planning-UI: GenerateWeekReport, RunDailyPrepNow, BuildPlanningIntegrationBranch, GetPlanningAggregate, Finalize/QueuePlanningSubtasks | +| A5 | Island-DB-Pfade — **nur die, die P0-2 als Ausreißer zeigt** | + +### Gruppe B — Bild 2: UI-Freeze + +| # | Inhalt | +|---|---| +| B1 | `UnifiedDiffParser.Parse` nach `Task.Run` + Indikator im DiffViewer | +| B2 | `DiffAlignment.Build`: Grenze definieren (ab N Zeilen auslagern oder inkrementell aufbauen) | +| B3 | Guard-Test: Parse/Build über ein großes Fixture blockiert den Dispatcher nicht | + +### Gruppe C — Bild 3: Worker-Stille + +| # | Inhalt | +|---|---| +| C1 | `OperationProgress(opKey, phase, current, total)` ersetzt `MergeProgress`; Elapsed auf UI-lokalen Tick | +| C2 | Startup-Recovery sichtbar (6 Services) — Anzeige im Shell statt nur „reconnecting" | +| C3 | Worktree-Anlage beim Task-Start sichtbar an der Task-Zeile. Vorher prüfen, ob der Broadcast-Gap aus dem 08-07-Spec schon geschlossen ist | +| C4 | `RebaseOthersAfterMergeAsync` und `WorktreeMaintenanceService` in den Kanal | +| C5 | Periodische Dienste (Usage, OnlineSync, Prime, Queue): nur Aktivität und Fehler, kein Tick-Spam | + +### Gruppe D — Bild 4: MCP + +| # | Inhalt | +|---|---| +| D1 | `ProgressReporter` aus `TaskMergeService.RunReportingProgressAsync` extrahieren, inkl. `i/n`-Overload | +| D2 | Die 8 `batch_*`-Tools mit Element-Fortschritt | +| D3 | Worktree- und Diff-Tools: `cleanup_task_worktree`, `batch_cleanup_task_worktrees`, `get_task_diff`, `preview_merge_set` | +| D4 | Restliche Long-Runner + Doku-Regel im Worker-`CLAUDE.md` | + +## Test-Strategie + +- **`OperationStatus`**: reine Unit-Tests mit Fake-`TimeProvider` — 300-ms-Grace, 60-s-Stall, + Elapsed-Formatierung, `Dispose` im Exception-Fall, `Report` überschreibt Label. +- **VM-Tests** (`Ui.Tests`, headless): Command setzt `IsRunning`, `CanExecute` sperrt während + des Laufs, Zustand ist nach einer Exception zurückgesetzt. Fakes: `StubWorkerClient` muss + bei jedem neuen Event mitwachsen. +- **B3** ist der einzige Test, der Blockierung prüft: großes Diff-Fixture, Messung, dass der + Dispatcher weiter Nachrichten verarbeitet. +- **Worker/MCP** (`Worker.Tests`): Progress-Callbacks werden mit einem Fake-`IProgress` + gezählt; keine echte Claude-CLI, keine echten Timeouts. +- **Visuell**: jedes Paket in A und B hat eine offene visuelle Prüfung. Spinner, Grace-Periode + und Layout kann kein Test bestätigen — das läuft über den Nutzer.