* feat(ideas): upload-plan knop — short-circuit van Make-Plan AI-flow Voegt een 'Upload plan' knop toe in idea-row-actions (verschijnt in zowel list als idea-detail). Klik → file picker → kies .md → server-side parse + opslaan; idea-status springt naar PLAN_READY. Vandaaruit de bestaande 'Maak PBI' knop voor materialize. Server (uploadPlanMdAction): - Toegestaan vanuit DRAFT, GRILLED, PLAN_FAILED, PLAN_READY - DRAFT → skip-grill: status gaat direct naar PLAN_READY - PLAN_READY overschrijft het bestaande plan (consistent met updatePlanMdAction, geen confirmation) - Geblokkeerd in GRILLING/PLANNING (job loopt), PLANNED (al gematerialiseerd) - Parse-failure → 422 + details (NIET opslaan, zodat een onparseerbaar plan nooit in de DB belandt) - Empty / >100k chars → 422 - Schrijft IdeaLog NOTE met from_status + length - Rate-limit + demo-guard + ownership-check via loadOwnedIdea (zelfde patroon als updatePlanMdAction) UI (idea-row-actions.tsx): - Hidden <input type=file accept=".md,.markdown,text/markdown,text/plain"> - FileReader → text → action - Toast bij success + router.refresh() - Blocked-tooltip in andere statussen Tests: 10 nieuwe in __tests__/actions/ideas-crud.test.ts dekkend voor: happy paths (DRAFT/GRILLED/PLAN_READY-overwrite/PLAN_FAILED), blocks (PLANNED/GRILLING), validation (empty/oversized/parse-fail), 404. Full suite groen: 849/849. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add reviews for Bootstrap-wizard plans v3.2 to v3.4 - Review v3.2: Addressed executor model, fire-and-forget issues, and PAT handling. - Review v3.3: Improved transaction handling, stale recovery, and ID generation. - Review v3.4: Finalized GitHub permissions, catalog versioning, and E2E verification queries. - Updated recommendations for each version to enhance implementation readiness. * docs(plans): M8 bootstrap-wizard upload-variant v1.4 — backtick-paden Upload-variant van het volledige technische plan (docs/plans/M8-bootstrap-wizard.md), bedoeld voor de "Upload plan"-functie. Genereert 1 PBI + 4 Stories + 22 Tasks via materializeIdeaPlanAction. v1.4-aanpassingen tov eerdere generatie-iteratie: - Alle bestandspaden in implementation_plan in backticks (path-extractor matchen) - Expliciete "Bestanden:" blok per task vóór de stappen - Alle tasks op verify_required: ALIGNED_OR_PARTIAL (was deels ALIGNED — te strict voor ADR-stubs en multi-file edits) Fixt forward-only: T-963 cancelled_by_self door DIVERGENT verifier-verdict. Re-upload van dit bestand produceert tasks die door verify_task_against_plan als ALIGNED of PARTIAL geclassificeerd kunnen worden. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PBI-67: Add review-plan support to Idea model and job config - Add plan_review_log and reviewed_at fields to Idea model - Add REVIEWING_PLAN, PLAN_REVIEW_FAILED, PLAN_REVIEWED to IdeaStatus enum - Add IDEA_REVIEW_PLAN to ClaudeJobKind enum - Add IDEA_REVIEW_PLAN config to job-config.ts with model=opus, thinking_budget=6000 - Create migration record for schema changes (applied via db push) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * PBI-67 Phase 2: Add update-idea-plan-reviewed MCP tool - Create src/tools/update-idea-plan-reviewed.ts: saves review-log and transitions idea status to PLAN_REVIEWED - Add PLAN_REVIEW_RESULT to IdeaLogType enum (both repos) - Register tool in src/index.ts - Update Prisma schemas (both repos): add plan_review_log and reviewed_at fields to Idea model - Add REVIEWING_PLAN, PLAN_REVIEW_FAILED, PLAN_REVIEWED to IdeaStatus enum (MCP schema) - Add IDEA_REVIEW_PLAN to ClaudeJobKind enum (MCP schema) - Tool includes transaction safety and convergence metrics logging Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat(PBI-67): IDEA_REVIEW_PLAN Phases 3-6 — server actions, UI components, prompt & tests - Phase 3: startReviewPlanJobAction, cancelIdeaJobAction, status transitions (REVIEWING_PLAN / PLAN_REVIEWED / PLAN_REVIEW_FAILED), status colors, job-card/jobs-column filters, idea-list status tabs - Phase 4: review-plan-job.md prompt (multi-model orchestration with codex injection + active plan revision via update_idea_plan_md after each round), runbook, 13 unit tests - Phase 5: ReviewLogViewer component (rounds, convergence, approval, issues), idea-detail integration, proper ReviewLog TypeScript types exported from component - Phase 6.1: wait-for-job discriminator wired (IDEA_REVIEW_PLAN), plan-revision step made mandatory in prompt (was previously optional/missing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
109 lines
6.5 KiB
Markdown
109 lines
6.5 KiB
Markdown
---
|
|
title: "Review - Bootstrap-wizard plan v3.2"
|
|
status: draft
|
|
date: 2026-05-14
|
|
source_plan: "/Users/janpetervisser/.claude/plans/als-ik-een-nieuwe-virtual-turtle.md"
|
|
---
|
|
|
|
# Review - Bootstrap-wizard plan v3.2
|
|
|
|
## Conclusie
|
|
|
|
V3.2 is een stevige verbetering. De grote architectuurfout uit v2 is opgelost: er is nu één executor-model met een aparte `bootstrap-service`, geen app-side fire-and-forget. Ook snake_case tables, het bestaande SSE payload-contract, `lease_until`, owner/slug en tag-pinning zijn goed verwerkt.
|
|
|
|
Nog niet direct implementeren zonder de punten hieronder te verwerken. De belangrijkste resterende blokkades zitten in claim-identiteit, deploybaarheid van het gedeelde package, en recovery wanneer GitHub-repo-aanmaak/push half slaagt.
|
|
|
|
## Bevindingen
|
|
|
|
### P1 - Claim-query gebruikt een niet-bestaand `claimed_by` veld
|
|
|
|
Het claim-protocol zet `claimed_by = ${WORKER_ID}` op `claude_jobs`. Het huidige `ClaudeJob`-model heeft `claimed_by_token_id`, `claimed_at` en `lease_until`, maar geen `claimed_by`. Dit faalt in SQL/migratie tenzij je een nieuw veld toevoegt.
|
|
|
|
Fix: kies expliciet:
|
|
|
|
- Re-use `claimed_by_token_id` met een dedicated service `ApiToken`, of
|
|
- voeg `claimed_by_worker_id String?` / `claimed_by_service String?` toe, of
|
|
- laat claim-identiteit weg en vertrouw op `lease_until`.
|
|
|
|
Mijn voorkeur: voeg `claimed_by_worker_id String?` toe voor `bootstrap-service`, zodat je logs en recovery kunt correleren zonder `ApiToken`-semantiek te misbruiken.
|
|
|
|
### P1 - `file:../bootstrap-service/...` dependency maakt de app niet deploybaar
|
|
|
|
V3.2 kiest voor een shared package onder `~/Development/bootstrap-service/packages/bootstrap-actions/` en een lokale `file:` link vanuit de Scrum4Me-app. Dat werkt lokaal, maar niet in een normale Vercel/GitHub build van de Scrum4Me repo: de sibling-directory zit niet in de repository checkout.
|
|
|
|
Fix voor MVP:
|
|
|
|
- Zet `packages/bootstrap-actions/` in de Scrum4Me repo, want dit package bevat geen secrets.
|
|
- Laat `bootstrap-service` dit package consumeren via git/package release, of tijdelijk via copied source met een sync-script.
|
|
- Of publiceer meteen naar GitHub Packages en pin een versie.
|
|
|
|
Niet doen: de app afhankelijk maken van een sibling path buiten de repo.
|
|
|
|
### P1 - Crash-recovery na externe GitHub-mutaties is nog onvoldoende
|
|
|
|
De happy path en catch-path verwijderen een aangemaakte repo bij errors, maar er is geen duurzaam checkpoint als de service crasht nadat de repo is aangemaakt en voordat `SUCCEEDED` is opgeslagen. Stale recovery markeert dan alleen DB-statussen `FAILED`; de GitHub repo kan blijven bestaan als orphan.
|
|
|
|
Fix: voeg expliciete externe side-effect checkpoints toe op `BootstrapRun`:
|
|
|
|
- `github_repo_created_at`
|
|
- `github_repo_id`
|
|
- `github_repo_full_name`
|
|
- `push_completed_at`
|
|
|
|
Stale recovery kan dan beslissen: compensating delete proberen, of `FAILED_NEEDS_CLEANUP`/manual intervention markeren. Zonder dit is rollback niet betrouwbaar.
|
|
|
|
### P1 - Stale recovery moet strikt op `BOOTSTRAP_REPO` filteren
|
|
|
|
De stale-recovery beschrijving update `claude_jobs` waar status `CLAIMED/RUNNING` en `lease_until < NOW`. Dat mag niet generiek op alle job kinds draaien, want de bestaande Claude/sprint runner gebruikt dezelfde tabel.
|
|
|
|
Fix: filter altijd `kind = 'BOOTSTRAP_REPO'`, en update alleen de bijbehorende `bootstrap_runs`. Laat bestaande cleanup voor andere job kinds ongemoeid.
|
|
|
|
### P1 - Transaction-array kan geen generated `jobId` doorgeven aan `BootstrapRun`
|
|
|
|
De atomische enqueue pseudo-code gebruikt `prisma.$transaction([claudeJob.create(...), bootstrapRun.create({ claude_job_id }))])`. Als `jobId` door Prisma wordt gegenereerd, is die waarde in array-form niet beschikbaar voor de tweede create.
|
|
|
|
Fix: gebruik een transaction callback en pregenereer IDs, of maak eerst de job in de transaction en gebruik de returned ID voor de run. Bijvoorbeeld `const jobId = createId()` vooraf en beide records met expliciete IDs schrijven.
|
|
|
|
### P2 - Cancel kan alsnog door succes worden overschreven
|
|
|
|
`cancelBootstrapAction` zet `ClaudeJob.status='CANCELLED'`; de service "detecteert per-action". Dat is goed, maar `syncSuccess` moet ook conditioneel zijn. Anders kan een cancel tussen de laatste checkpoint en success-sync alsnog eindigen als `DONE/SUCCEEDED`.
|
|
|
|
Fix: voor terminal transitions eerst current job/run status lezen of conditional `updateMany` gebruiken. Als `CANCELLED`, geen success meer schrijven.
|
|
|
|
### P2 - `last_bootstrap_run_id` mist relationele details
|
|
|
|
Het plan noemt `Product.last_bootstrap_run_id String?`, maar niet de Prisma relation naar `BootstrapRun` met `onDelete: SetNull`. Voeg die expliciet toe, inclusief relation name om ambiguiteit met `Product.bootstrap_runs` te voorkomen.
|
|
|
|
### P2 - Action permissions staan op option-niveau, maar risico kan action-niveau zijn
|
|
|
|
`risk_level` en `requires_role` staan nu op `BootstrapOption`, terwijl `RUN_BASH_TEMPLATE` een action-kind is. Als een optie meerdere acties bevat, moet de optie-risk altijd afgeleid worden uit de zwaarste action, of je hebt action-level permissions nodig.
|
|
|
|
Fix: ofwel permissions verplaatsen naar `BootstrapAction`, of `BootstrapOption.risk_level`/`requires_role` server-side afleiden en niet handmatig laten driften.
|
|
|
|
### P2 - Houd ID-strategie consistent met de codebase
|
|
|
|
Nieuwe modellen gebruiken `@default(uuid())`, terwijl bestaande Scrum4Me-tabellen vrijwel overal `@default(cuid())` gebruiken. Technisch kan UUID, maar het wijkt af zonder duidelijke reden.
|
|
|
|
Fix: gebruik `cuid()` tenzij er een externe reden is voor UUID.
|
|
|
|
### P2 - Fine-grained GitHub PATs passen niet netjes in alleen `repo` scope
|
|
|
|
De verificatie verwacht `repo` in `x-oauth-scopes`. Dat is prima voor classic PATs, maar fine-grained PATs werken met repository permissions en tonen niet altijd hetzelfde scope-model.
|
|
|
|
Fix: maak MVP expliciet "classic PAT met `repo` scope" of ondersteun fine-grained tokens met aparte permission checks. Zet dit ook in de settings UI-copy.
|
|
|
|
### P2 - `.env.example` en deployment docs ontbreken in de filelijst
|
|
|
|
`BOOTSTRAP_ENCRYPTION_KEY` wordt verplicht in de app en service. Voeg `.env.example`, deployment runbook en bootstrap-service README setup toe aan de scope, anders breken lokale onboarding en CI/deploy snel.
|
|
|
|
## Aanbevolen aanpassing
|
|
|
|
Verwerk vóór implementatie minimaal:
|
|
|
|
1. Vervang `claimed_by` door een bestaand of nieuw veld.
|
|
2. Verplaats het shared package naar de Scrum4Me repo of publiceer het.
|
|
3. Voeg GitHub side-effect checkpoints toe.
|
|
4. Filter stale recovery hard op `kind='BOOTSTRAP_REPO'`.
|
|
5. Maak enqueue transaction-ID handling concreet.
|
|
|
|
Daarna is het plan implementatieklaar genoeg om naar `docs/plans/M8-bootstrap-wizard.md` te verplaatsen.
|