docs(specs): Design für Feedback bei langlaufenden Operationen

2026-08-11 19:16:55 +02:00
parent cad0582b37
commit ba1a745e09
1 changed files with 311 additions and 0 deletions
@@ -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<string>`-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:<taskId>"`)
+- **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.