From fffa5a47d2d1467b655f37da4ea5cb743cb62ee8 Mon Sep 17 00:00:00 2001 From: Janpeter Visser <30029041+madhura68@users.noreply.github.com> Date: Thu, 7 May 2026 13:32:30 +0200 Subject: [PATCH] docs: archiveer sprint-pr-worktree state-machines advies (#140) Het advies-document dat als input diende voor PBI-50 is nu in docs/plans/ opgeborgen voor traceability. INDEX.md regenerated om hem op te nemen. Co-authored-by: Claude Opus 4.7 (1M context) --- docs/INDEX.md | 1 + .../sprint-pr-worktree-state-machines.md | 345 ++++++++++++++++++ 2 files changed, 346 insertions(+) create mode 100644 docs/plans/sprint-pr-worktree-state-machines.md diff --git a/docs/INDEX.md b/docs/INDEX.md index 41c7ddd..59962d6 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -49,6 +49,7 @@ Auto-generated on 2026-05-07 from front-matter and headings. | [M12 — Idea entity + Grill/Plan Claude jobs](./plans/M12-ideas.md) | planned | — | | [M9 — Actief Product Backlog](./plans/M9-active-product-backlog.md) | active | 2026-05-03 | | [PBI-11 — Mobile-shell met landscape-lock (settings + backlog + solo)](./plans/PBI-11-mobile-shell.md) | — | — | +| [Advies - SprintRun, PR en worktree lifecycle als state machines](./plans/sprint-pr-worktree-state-machines.md) | draft | 2026-05-06 | | [ST-1109 — PBI krijgt een status (Ready / Blocked / Done)](./plans/ST-1109-pbi-status.md) | active | 2026-05-03 | | [ST-1110 — Demo gebruiker read-only](./plans/ST-1110-demo-readonly.md) | active | 2026-05-03 | | [ST-1111 — Voer uit-knop met Claude Code job queue](./plans/ST-1111-claude-job-trigger.md) | active | 2026-05-03 | diff --git a/docs/plans/sprint-pr-worktree-state-machines.md b/docs/plans/sprint-pr-worktree-state-machines.md new file mode 100644 index 0000000..afc8349 --- /dev/null +++ b/docs/plans/sprint-pr-worktree-state-machines.md @@ -0,0 +1,345 @@ +--- +title: "Advies - SprintRun, PR en worktree lifecycle als state machines" +status: draft +audience: [ai-agent, developer] +language: nl +last_updated: 2026-05-06 +--- + +# Advies - SprintRun, PR en worktree lifecycle als state machines + +## Context + +Het combinatieplan voor F3 auto-merge worker-flow en persistente product-worktrees raakt meerdere lifecycles tegelijk: + +- `SprintRun`: queue, running, paused, done, failed, cancelled. +- `ClaudeJob`: claim, run, verify, push, done, failed, cancelled. +- Worktree/lock: acquire, create/reuse, sync, ready, release. +- PR-flow: push branch, create PR, wait for scope completion, checks, merge/ready/pause. + +De grootste maintainability-risico's ontstaan waar deze lifecycles elkaar kruisen. Voorbeelden: + +- Een PR krijgt auto-merge terwijl latere story-tasks nog niet klaar zijn. +- Een job wordt `DONE`, maar de product-worktree lock blijft hangen. +- Een `SprintRun` wordt `PAUSED`, maar de UI/server action kan alleen een failed sprint hervatten. +- Per-task verificatie gebruikt `origin/main...HEAD`, terwijl meerdere tasks dezelfde story- of sprint-branch hergebruiken. + +Het advies is om deze lifecycles expliciet te modelleren als state machines, met centrale transition-regels en declaratieve side effects. + +## Kernadvies + +Houd de worker zo dom mogelijk. De worker voert werk uit in de meegegeven context en rapporteert via MCP-tools. De MCP-server is eigenaar van lifecycle-transitions, cleanup, locks, PR-status, pause/resume en terminal states. + +Concreet: + +1. Maak pure transition-modules in `scrum4me-mcp`. +2. Laat `wait-for-job.ts` en `update-job-status.ts` transitions aanroepen. +3. Laat transitions declaratieve effects teruggeven. +4. Voer effects idempotent server-side uit. +5. Persistente waarheid blijft Postgres. + +Niet doen: + +- Lock-release afhankelijk maken van een worker-prompt-instructie. +- PR/merge-flow deels in worker hooks en deels in MCP-tools stoppen. +- Auto-merge activeren voordat de volledige scope klaar is. +- Worktree internals rechtstreeks muteren zonder Git-commando's zoals `git rev-parse --git-path`. + +## Voorgestelde Machines + +### 1. WorktreeLeaseMachine + +Doel: product-worktrees en locks betrouwbaar beheren. + +States: + +```text +idle + -> acquiring_lock + -> creating_or_reusing + -> syncing + -> ready + -> releasing + -> released + +error: + -> lock_timeout + -> sync_failed + -> stale_released +``` + +Belangrijke regels: + +- Acquire lock voordat de worktree wordt aangemaakt of gesynct. +- Gebruik een lock-pad dat buiten de worktree bestaat, bijvoorbeeld: + `~/.scrum4me-agent-worktrees/_locks/product-{productId}`. +- Release locks server-side bij: + - `update_job_status(done)` + - `update_job_status(failed)` + - job cancellation + - stale reset + - process shutdown waar mogelijk +- Gebruik `proper-lockfile` alleen als single-host/single-filesystem aanname klopt. +- Overweeg PostgreSQL advisory locks of een DB-lease-tabel als meerdere MCP-processen of machines dezelfde product-worktrees kunnen beheren. + +### 2. PrFlowMachine + +Doel: PR's consistent aanmaken, bijwerken, ready zetten en eventueel auto-mergen. + +States: + +```text +none + -> branch_pushed + -> pr_opened + -> waiting_for_scope_done + -> waiting_for_checks + -> auto_merge_enabled + -> merged + +draft path: + -> draft_opened + -> ready_for_review + +failure/pause: + -> checks_failed + -> merge_conflict_paused +``` + +Regels per `PrStrategy`: + +| Strategie | Eerste task | Tijdens scope | Laatste task | +|---|---|---|---| +| `STORY` | Branch push + PR openen | PR open houden, geen auto-merge | Na laatste story-task: checks groen -> auto-merge | +| `SPRINT` | Branch push + draft PR openen | Commits blijven op sprint-branch | Na sprint DONE: PR ready-for-review, geen auto-merge | + +Belangrijke regels: + +- PR openen mag vroeg; auto-merge activeren mag pas wanneer de scope klaar is. +- Gebruik GitHub branch protection, required checks en eventueel merge queue als merge-gate. +- Gebruik bij merge-acties waar mogelijk een head-SHA guard, bijvoorbeeld via `gh pr merge --match-head-commit`. +- Maak auto-merge/merge-fouten typed: + - `CHECKS_FAILED` + - `MERGE_CONFLICT` + - `GH_AUTH_ERROR` + - `AUTO_MERGE_NOT_ALLOWED` + - `UNKNOWN` +- Alleen `MERGE_CONFLICT` hoort naar `SprintRun.PAUSED`; CI rood hoort naar task/sprint failure. + +### 3. SprintRunMachine + +Doel: sprint-run status niet verspreid over UI, server actions en MCP-tools laten ontstaan. + +States: + +```text +queued + -> running + -> paused_merge_conflict + -> running + -> done + +failure: + -> failed + +manual: + -> cancelled +``` + +Events: + +```ts +type SprintRunEvent = + | { type: 'CLAIM_FIRST_JOB' } + | { type: 'TASK_DONE'; taskId: string } + | { type: 'TASK_FAILED'; taskId: string; error: string } + | { type: 'MERGE_CONFLICT'; prUrl: string; files: string[] } + | { type: 'USER_RESUMED' } + | { type: 'USER_CANCELLED' } +``` + +PAUSED moet context hebben: + +- `pause_reason = 'MERGE_CONFLICT'` +- `pr_url` +- conflict-bestanden +- `claude_question_id` +- eventueel `resume_instructions` + +Zonder die context wordt PAUSED lastig te onderhouden in UI, MCP en worker-flow. + +## Pure Transitions En Declaratieve Effects + +Een transition-functie moet geen GitHub-call, Prisma-write of filesystem-operatie direct doen. Laat de functie teruggeven wat er moet gebeuren. + +Voorbeeld: + +```ts +type FlowEffect = + | { type: 'CREATE_CLAUDE_QUESTION'; payload: { sprintRunId: string; prUrl: string; files: string[] } } + | { type: 'SET_SPRINT_RUN_STATUS'; sprintRunId: string; status: 'PAUSED' | 'RUNNING' | 'DONE' | 'FAILED' } + | { type: 'ENABLE_AUTO_MERGE'; prUrl: string; expectedHeadSha: string } + | { type: 'MARK_PR_READY'; prUrl: string } + | { type: 'RELEASE_WORKTREE_LOCKS'; jobId: string } + +type TransitionResult = { + nextState: State + effects: FlowEffect[] +} +``` + +Daarna voert een executor de effects idempotent uit: + +```ts +async function executeEffects(effects: FlowEffect[]) { + for (const effect of effects) { + switch (effect.type) { + case 'RELEASE_WORKTREE_LOCKS': + await releaseJobLocks(effect.jobId) + break + case 'MARK_PR_READY': + await markPullRequestReady({ prUrl: effect.prUrl }) + break + // enzovoort + } + } +} +``` + +Voordelen: + +- Transitions zijn unit-testbaar zonder GitHub, Git of database. +- Side effects zijn apart idempotent te testen. +- Verboden transitions worden expliciet. +- UI en tools kunnen dezelfde statusbetekenis gebruiken. + +## Per-Job Verificatie: Base SHA Vastleggen + +De huidige verificatie met `git diff origin/main...HEAD` is niet geschikt wanneer meerdere jobs dezelfde story- of sprint-branch hergebruiken. Task 2 ziet dan ook wijzigingen van task 1. + +Aanbevolen wijziging: + +- Leg bij claim `ClaudeJob.base_sha` vast. +- Verifieer job-scope met: + - `git diff ...HEAD`, of + - `git diff ..HEAD` als lineaire task-commits verplicht zijn. +- Leg na succesvolle push `ClaudeJob.head_sha` vast. +- Gebruik die SHA ook als guard voor PR/merge-acties. + +Dit maakt task-verificatie, PR lifecycle en auto-merge veel voorspelbaarder. + +## XState, Eigen Module Of Temporal + +### Eigen pure TypeScript module + +Beste eerste stap. + +Gebruik dit wanneer: + +- De workflow lokaal in MCP-tools draait. +- Postgres de persistente waarheid blijft. +- Je vooral transitions en guards wilt centraliseren. + +Voordeel: weinig dependency- en runtime-complexiteit. + +### XState + +XState is passend wanneer: + +- transitions complexer worden; +- nested states nuttig zijn; +- je visualisatie of model-based tests wilt; +- meerdere lifecycles als actors gaan samenwerken. + +Gebruik XState in deze fase bij voorkeur als pure transition layer, niet als long-running runtime. Persistente status blijft in Postgres. + +Bron: [XState docs](https://stately.ai/docs) + +### Temporal + +Temporal pas overwegen als de orchestration echt distributed en long-running wordt. + +Gebruik dit wanneer: + +- workflows uren/dagen lopen; +- meerdere worker-machines betrokken zijn; +- retries, timers en signals crash-proof moeten zijn; +- je wilt dat workflow-code exact verdergaat na server crash. + +Niet kiezen als eerste stap: het brengt eigen infra, deployment, determinisme-regels en workflow/activity-splitsing mee. + +Bron: [Temporal docs](https://docs.temporal.io/) + +## Aanbevolen Implementatievolgorde + +### Stap 1 - Documenteer huidige transitions + +Maak een kleine inventarisatie: + +- Welke code wijzigt `SprintRun.status`? +- Welke code wijzigt `ClaudeJob.status`? +- Welke code maakt/verwijdert worktrees? +- Welke code maakt PR's, zet PR's ready of enablet auto-merge? + +### Stap 2 - Introduceer pure machines + +Nieuwe bestanden in `scrum4me-mcp`: + +- `src/flow/worktree-lease-machine.ts` +- `src/flow/pr-flow-machine.ts` +- `src/flow/sprint-run-machine.ts` +- `src/flow/effects.ts` + +### Stap 3 - Verplaats beslislogica uit tools + +Laat `update-job-status.ts` niet zelf bepalen welke lifecycle-actie volgt, maar: + +1. laad context uit DB; +2. stuur event naar machine; +3. persist next state; +4. voer effects uit; +5. emit SSE. + +### Stap 4 - Maak effects idempotent + +Elke effect moet veilig opnieuw uitvoerbaar zijn: + +- PR bestaat al -> return bestaande URL. +- PR is al ready -> success. +- Lock bestaat niet meer -> success. +- SprintRun staat al terminal -> geen mutatie. + +### Stap 5 - Voeg transition-tests toe + +Test vooral verboden of gevoelige paden: + +- STORY: auto-merge niet bij eerste task. +- STORY: auto-merge pas na laatste task en groene checks. +- SPRINT: draft PR blijft draft tot sprint DONE. +- MERGE_CONFLICT: SprintRun wordt PAUSED met question/context. +- CI rood: task wordt FAILED, niet PAUSED. +- Product-worktree: lock acquire gebeurt vóór create/sync. +- Stale reset: lock release wordt altijd uitgevoerd. + +## Aanpassing Aan Het Combinatieplan + +Het plan zou voor implementatie worden aangescherpt op deze punten: + +1. F3 niet implementeren als worker post-task hook, maar als MCP-owned PR-flow. +2. STORY auto-merge uitstellen tot story-scope klaar is. +3. Per-job `base_sha` en `head_sha` toevoegen voor verificatie en merge guards. +4. Product-worktree lock acquire vóór `getOrCreateProductWorktree`. +5. Lock-release niet via worker-prompt, maar via server-side terminal transitions. +6. PAUSED resume-path expliciet maken in server action en UI. +7. `PLAN_CHAT` alleen opnemen als die jobflow end-to-end bestaat. +8. Delete-only verifierverwachting corrigeren: delete-only is niet `EMPTY` als er daadwerkelijk bestanden zijn verwijderd. + +## Bronnen + +- [Git worktree documentation](https://git-scm.com/docs/git-worktree.html) +- [GitHub protected branches](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches) +- [GitHub merge queue](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue) +- [GitHub CLI `gh pr merge`](https://cli.github.com/manual/gh_pr_merge) +- [PostgreSQL explicit locking](https://www.postgresql.org/docs/current/explicit-locking.html) +- [XState documentation](https://stately.ai/docs) +- [Temporal documentation](https://docs.temporal.io/)