314 lines
17 KiB
Markdown
314 lines
17 KiB
Markdown
# 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) ← nach P0
|
||
```
|
||
|
||
| 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 — inzwischen committet als
|
||
`cad0582 fix(merge): conventional merge-commit default and live verify progress`:
|
||
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").
|
||
- Der Blocker ist damit aufgelöst: C hängt nur noch an P0, nicht mehr an der Parallelsession.
|
||
|
||
## 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.
|