From 0e9373c083cbced3b627fbd72356cfbacee50a36 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 09:56:36 +0200 Subject: [PATCH] docs(spec): unit branch and draft PR design --- .../2026-10-01-unit-branch-draft-pr-design.md | 215 ++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-01-unit-branch-draft-pr-design.md diff --git a/docs/superpowers/specs/2026-10-01-unit-branch-draft-pr-design.md b/docs/superpowers/specs/2026-10-01-unit-branch-draft-pr-design.md new file mode 100644 index 00000000..de7b073f --- /dev/null +++ b/docs/superpowers/specs/2026-10-01-unit-branch-draft-pr-design.md @@ -0,0 +1,215 @@ +# Unit-Branch + Draft-PR — Design + +**Datum:** 2026-10-01 · **Status:** Entwurf, Review durch Mika ausstehend +**Diagramme:** Obsidian `07 - Plattform & Technik/ClaudeDo/ClaudeDo - Branch und PR Ablauf.md` +**Ersetzt:** die offene Entscheidung aus `2026-08-06-planning-chain-fork-base-design.md` + +## Ziel + +Aufgaben starten und später zu fertigen PRs zurückkommen. In Firmen-Repos (Azure DevOps Server) +heißen Branches nach dem work-conventions-Schema, und das Review endet in einem **Draft-PR** statt +in einem lokalen Merge auf `main`. Nebenbei sehen Planning-Children endlich die Arbeit ihres +Vorgängers. + +## Begriffe + +- **Unit:** ein einzelner Task, oder ein Parent (Planning oder Improvement) samt Children. +- **Unit-Branch:** der eine Branch, auf dem die ganze Arbeit einer Unit landet. Er gehört dem + Worktree des Unit-Roots (Einzeltask bzw. Parent). +- **Firmen-Repo:** Die `origin`-URL enthält `company_remote_pattern` (`worker.config.json`, + Default `/_git/`, nur per Hand editierbar). Geprüft wird live über `git remote get-url origin` + und ohne Cache. + +## Entscheidungen + +| Frage | Entscheidung | +|---|---| +| Branchname | Einheitlich `/-`. Die Nummer ist die Ticket-Id aus `TicketRef` (beim Child die des Unit-Roots), sonst privat die Task-Nummer. Im Firmen-Repo ohne Ticket entfällt sie (`/`). `claudedo/{nr}-…` fällt weg. Planning-Session-Worktrees bleiben `claudedo/planning/{guid}`. | +| Typ | `feat`→`feature`. `fix`, `refactor`, `docs`, `test`, `chore` bleiben, wie sie sind. Alles andere (`ci`, `perf`, `style`, `build`) wird `chore`. | +| Slug | `CommitMessageBuilder.ToSlug` plus Umlaut-Auflösung (`ä→ae`, `ö→oe`, `ü→ue`, `ß→ss`), ≤40 Zeichen wie heute. | +| Children | Starten vom aktuellen Head des Unit-Branches. Ist ein Child fertig, wird der Unit-Branch sofort per **fast-forward** auf den Head des Childs gezogen. Das gilt privat und in Firmen-Repos. | +| Review-Ende privat | Unverändert: Approve merged den Unit-Branch auf `main`, dann läuft Verify auf `main`. | +| Review-Ende Firma | Approve führt Verify im Worktree des Unit-Roots aus, dann Push und Draft-PR. Der Task bleibt `WaitingForReview` mit PR-Link. `Done` setzt Mika per Toggle nach dem ADO-Merge. | +| PR-Automatik | Kein Auto-PR bei einem Statuswechsel. Automatisiert wird über den Merge-Helper, dessen `approve` dann den PR erzeugt. | +| ADO-Zugang | Eigener PAT (Scope Code Read & Write). Er wird per `DpapiTokenStore` in `~/.claudeDo/azdo.pat` gespeichert, ohne Abhängigkeit auf das work-conventions-Plugin. | +| dependsOn über Units | Wartet bewusst auf den echten Merge. Abhängige Arbeit gehört in eine Unit mit einem PR. | + +## 1. Branch-Naming (`WorktreeManager`) + +- `NameFor(task, ticketId, isCompany)` liefert den Leaf `-` bzw. ``, und + `BranchFor` liefert `/`. Der Ordnername des Worktrees bleibt der Leaf. +- **Kollisionsprüfung** (heute `Contains($"claudedo/{name}")`, Zeile 83) prüft den neuen vollen + Branchnamen. Bei einem Treffer kommt das Id-Suffix dran, wie heute. +- **Self-Heal** (`git branch -D` nach „already exists“) läuft nur noch, wenn der Branch in einer + `worktrees`-Zeile steht, deren Task der aktuelle Task ist. Sonst gibt es keinen Delete, sondern + das Id-Suffix, und der Branch des Nutzers bleibt unangetastet. Ohne das Präfix `claudedo/` ist + das Pflicht, sonst löscht ClaudeDo einen fremden `feature/…`-Branch. +- **`UntrackedWorktreeRecovery`** filtert statt nach dem Präfix nach „Worktree-Pfad liegt unter + einer ClaudeDo-Worktree-Root (`.claudedo-worktrees` bzw. Central-Root), aber nicht unter + `planning/`“. +- `PlanningAggregator.CleanupIntegrationBranchAsync:137` (Präfix-Check beim Checkout-Fallback) + wird mit Abschnitt 3 ohnehin hinfällig. +- Bestehende Worktrees behalten ihren Namen. Der Branchname steht in der DB und wird nie neu + abgeleitet, eine Migration ist also nicht nötig. +- **Bugfix nebenbei:** `DetailsIslandViewModel.cs:1223/1385` baut für Dialogtexte + `claudedo/{Task.Id}`, das ist schon heute falsch. Künftig steht dort `worktree.BranchName`. Das + literale `"← main"` (`:670/:933`) zeigt das echte Ziel an. + +## 2. Unit-Branch und Children + +**Wer besitzt den Unit-Branch?** +- **Einzeltask:** sein eigener Worktree. Am Ablauf ändert sich nichts. +- **Improvement-Parent:** sein eigener Worktree, den er schon heute hat. Seine Children starten + schon heute von `parent.HeadCommit`. +- **Planning-Parent:** hat heute **keinen** Worktree. Neu: `QueuePlanAsync` legt per + `WorktreeManager.CreateAsync` einen Worktree für den Parent an, auf dem Unit-Branch und mit + `main` HEAD als Basis. Damit hat jeder Parent eine `worktrees`-Zeile, die den Unit-Branch und + später die PR-URL trägt. + +**Basis eines Childs** (`ResolveBaseCommitAsync`): Das ist immer der aktuelle Head des +Unit-Branches (`rev-parse `). Der Sonderfall „nur bei `PlanningPhase.None`“ fällt weg. + +**Absorbieren eines Childs:** Neu ist `UnitBranchService.AbsorbChildAsync(childId)`, das in +`git -C merge --ff-only ` mündet. +- Die Kette ist seriell (`BlockedByTaskId`), und das Child startet vom Head des Unit-Branches. + Deshalb ist es **immer** ein fast-forward, Konflikte sind ausgeschlossen. Lässt es sich nicht + fast-forwarden, z.B. weil jemand ein Child manuell parallel per „Run now“ gestartet hat, wird + das Child `Failed` mit einer klaren Meldung, ohne Merge-Versuch. + `ponytail: kein Merge-Fallback; einbauen, falls parallele Children ein echter Fall werden.` +- Danach wird der Worktree des Childs entfernt, der Branch gelöscht und die Zeile auf `Merged` + gesetzt (`MergeCommit` = null, denn es gibt keinen Merge-Commit). `revert_merge` greift auf + Child-Ebene deshalb nicht, revertiert wird die ganze Unit. +- Aufgerufen wird es aus `TaskRunner.HandleSuccess` direkt vor `_state.CompleteAsync`. Ein + ff-Fehler wird so zu `FailAsync` statt zu einem Unblock des Nachfolgers. +- Interaktive Children landen in `WaitingForReview`, und ihr Approve geht durch + `TaskMergeService.ApproveAndMergeAsync`. Dort gilt neu: Hat der Task einen Parent mit eigenem + Worktree, wird `AbsorbChildAsync` aufgerufen statt auf das Ziel gemergt. +- Kein Git-Code in `TaskStateService` oder `PlanningChainCoordinator`, die bleiben git-frei. + +**Was dabei wegfällt:** Den Unit-Merge Child für Child am Ende gibt es für neue Units nicht mehr. +Ein Parent wird beim Approve wie ein Einzeltask behandelt, über seinen eigenen Worktree in +`TaskMergeService.ApproveAndMergeAsync`. Auch der Integration-Branch +(`PlanningAggregator`, `planning/{slug}-integration`, dafür `checkout -b` im Main-Checkout) ist +für neue Units unnötig, denn der kombinierte Diff ist `unitBase..unitHead`. + +**Routing beim Approve** (`WorkerHub.ApproveReview`, `ExternalMcpService.ReviewTask`): Heute +geht jeder Task mit Children an den Orchestrator. Neu gilt: Gibt es ein `Done`-Child mit einem +`Active`-Worktree, also eins, das nicht absorbiert wurde, läuft der Legacy-Orchestrator. Sonst +geht es über `ApproveAndMergeAsync` auf den Worktree des Parents. + +**Altbestand:** Parents ohne eigenen Worktree (vor dem Rollout geplant) laufen weiter über +`PlanningMergeOrchestrator` und den bestehenden Aggregator. Diesen Legacy-Pfad entfernen wir in +einem Folge-Task, sobald keine solchen Units mehr offen sind. + +## 3. Approve im Firmen-Repo + +`TaskMergeService.ApproveAndMergeAsync` verzweigt nach `IsCompanyRepo(list)`: + +1. **Verify** im Worktree des Unit-Roots (`IVerifyCommandRunner.RunAsync(worktreePath, …)`), nicht + im Main-Checkout. Ohne Verify-Command entfällt der Schritt. Rot heißt: Ergebnis + `verify_failed`, der Task bleibt `WaitingForReview`, kein Push. `skipVerify` wird im + Firmen-Modus ignoriert, weil kein Merge landet, auf den man das Verify verschieben könnte. +2. **Push** mit `git -C push -u origin `. Der PAT geht als + `http.extraHeader` über `GIT_CONFIG_COUNT`/`GIT_CONFIG_KEY_0`/`GIT_CONFIG_VALUE_0` in der + Prozess-Umgebung mit. Er steht also nicht in der Kommandozeile, und es gibt keinen + Credential-Manager-Prompt im Hintergrundprozess. +3. **PR**, neuer `AzureDevOpsClient`, portiert aus `pr-create.ps1`: + - Der Remote wird an `/_git/` zerlegt: API-Basis = alles davor, Repo = Segment danach. + Eingebettete Zugangsdaten fliegen raus. + - `GET …/pullrequests?searchCriteria.sourceRefName=refs/heads/&searchCriteria.status=active`. + Gibt es schon einen PR, wird er wiederverwendet (idempotent, auch wenn der Skill in der + Session schon einen angelegt hat). + - Sonst `POST …/pullrequests` mit `isDraft: true` und + `targetRefName = ` (`GET …/repositories/`). + - Die API-Version fällt auf die zurück, die der Server in seiner Fehlermeldung nennt, wie im + Skript. +4. Die **PR-URL** wird gespeichert, in der neuen nullable Spalte `worktrees.pull_request_url` + (Migration). Danach folgen `WorktreeUpdated` und der Rückgabestatus `pr_created` bzw. + `pr_updated`. +5. Der Task bleibt `WaitingForReview`, der Worktree bleibt `Active`, und das Ticket bleibt auf + `InBearbeitung`. + +**PR-Inhalt:** +- **Titel:** `(): , Resolves #`. Ohne Ticket entfällt + `Resolves`. +- **Beschreibung:** + - `Resolves [#412]()`. Die Projekt-Id kommt über `BandelTicketClient.GetTicketAsync` + → `ParentTicketID` (bei Tasks eine Ebene höher). Ohne Id gibt es keinen Link. + - Leerzeile, dann die Task-Beschreibung. + - Leerzeile, dann `Geprüft: grün` bzw. `Geprüft: nichts automatisch`. + +**Commits im Firmen-Repo:** `CommitMessageBuilder` lässt den Trailer `ClaudeDo-Task:` weg. Er +wird nirgends gelesen, und die Konvention verbietet Signatur-Trailer. + +**Rerun nach Änderungswunsch:** Ein normaler Reject mit Feedback, der auf demselben Worktree +weiterläuft. Der nächste Approve pusht nach, und der vorhandene PR aktualisiert sich. + +**Fehler:** Fehlt der PAT, wird der Push abgelehnt oder scheitert ein REST-Aufruf, dann landen +Statusmeldung und Footer-Strip (`FlashFooterError`) beim Nutzer, und der Task bleibt +`WaitingForReview`. Scheitert der PR nach einem erfolgreichen Push, sagt die Meldung das genau +so: „gepusht, PR fehlgeschlagen: …“. + +**Laufzeit:** Verify plus Push können länger als 5 s dauern. Der MCP-Pfad meldet deshalb Progress +über `ProgressReporter`, sonst bricht der MCP-Client nach 300 s Leerlauf ab. + +## 4. Merge-Helper + +Die Prompts in `PromptFiles` (merge und merge_final) bekommen einen Firmen-Repo-Absatz: +- `review_task approve` liefert `pr_created`/`pr_updated` mit URL. +- `verify_merges` entfällt, weil das Verify bereits im Approve läuft. +- Die Abschluss-Zusammenfassung listet die PR-Links auf. +- Die Regel „erst mergen, dann neu starten“ bleibt harmlos, da `main` sich nicht bewegt. + +## 5. UI + +- Der Settings-Reiter „Ticketsystem“ heißt künftig **„Externe Dienste“**. Er bekommt einen + zweiten Abschnitt **Azure DevOps** mit PAT-Feld, Test-Button und Löschen-Button, gespiegelt vom + Ticket-PAT. +- Neue Hub-Methoden: `GetAzureDevOpsSettings`, `SetAzureDevOpsToken`, `ClearAzureDevOpsToken`, + `TestAzureDevOpsConnection`. Neuer DI-Marker `AzureDevOpsPatStore`. +- Der Test braucht ein Firmen-Repo als Kontext: Er nimmt die erste Liste, deren `origin` passt, + und ruft `GET …/repositories/` auf. +- Im Detail-Pane heißt der Approve-Button im Firmen-Repo **„Draft-PR erstellen“**. Er ist weiter + hinter „Diff öffnen“ gegated, und das Ziel-Dropdown ist ausgeblendet. Ist eine PR-URL gesetzt, + erscheint ein Link und der Button heißt „PR aktualisieren“. +- Locale-Keys kommen in `en.json` und `de.json` gleichzeitig hinzu. + +## 6. Bewusst draußen + +- PR-Status-Polling bzw. automatisches `Done` nach dem ADO-Merge. +- Andere PR-Provider als Azure DevOps. `company_remote_pattern` schaltet nur das Schema um, die + PR-API ist ADO-spezifisch. +- Aufräumen von Worktree und lokalem Branch, wenn ein Firmen-Task auf `Done` gesetzt wird. Das + geht über die bestehende Worktree-Übersicht. +- `git fetch` vor dem Anlegen des Unit-Branches. Die Basis ist der lokale `main`-HEAD wie heute, + siehe offene Frage. +- Eine UI für `company_remote_pattern`. + +## 7. Tests + +Keine Tests mit der echten Claude-CLI und keine gegen das echte ADO. + +- **Naming:** Typ-Mapping, Nummernregel (Ticket, privat, Firma ohne Ticket), Umlaute, + Kollision mit einem fremden Branch (der Branch überlebt, das Suffix greift), Self-Heal löscht + nur DB-eigene Branches. Die betroffenen Asserts in `WorktreeManagerTests` (`:69`, `:131/138`, + `:160`, `:314`) passe ich an. +- **Unit-Branch** (echtes git, Worker.Tests): + - Ein Child startet vom Head des Unit-Branches. + - Zwei serielle Children werden per fast-forward absorbiert, und Child 2 sieht die Dateien von + Child 1. + - Ein nicht fast-forwardbares Child wird `Failed`. + - Ein Planning-Parent bekommt seinen Worktree bei `QueuePlanAsync`. +- **Approve Firma:** + - Ein lokales Bare-Repo dient als `origin`, und das Muster wird auf dessen Pfad gesetzt. + - `AzureDevOpsClient` wird gefakt: PR anlegen, vorhandenen PR wiederverwenden, Verify rot + heißt kein Push, PR-Fehler nach dem Push. +- **Remote-Parsing:** die Fälle aus dem `-SelfTest` des Skripts. +- **Manueller Smoke-Test (Mika):** PAT eintragen, Test-Button. Ein Ticket-Task in einem echten + Firmen-Repo bis zum Draft-PR durchspielen, danach eine Planning-Unit mit 2 Children. + +## Offene Frage + +- **Basis im Firmen-Repo:** Der lokale `main` ist in Firmen-Repos oft veraltet. Vorschlag für + später: `git fetch origin ` und dann `origin/` als Basis des Unit-Branches. + In v1 ist das draußen. Wenn es dich jetzt schon stört, ist es ein kleiner Zusatz in + `ResolveBaseCommitAsync`.