d84cdf664f
* 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>
73 lines
4.6 KiB
Markdown
73 lines
4.6 KiB
Markdown
---
|
|
title: "Review - Bootstrap-wizard plan v3.3"
|
|
status: draft
|
|
date: 2026-05-14
|
|
source_plan: "/Users/janpetervisser/.claude/plans/als-ik-een-nieuwe-virtual-turtle.md"
|
|
---
|
|
|
|
# Review - Bootstrap-wizard plan v3.3
|
|
|
|
## Conclusie
|
|
|
|
V3.3 verwerkt de v3.2-review goed. De claim-identiteit, shared package locatie, GitHub side-effect checkpoints, stale-recovery filter, action-level permissions, classic PAT-keuze en env/docs zijn nu expliciet. Dit plan is dicht bij implementatieklaar.
|
|
|
|
Nog verwerken vóór uitvoering: de status-sync voorbeeldcode is nog niet echt transactioneel, stale-recovery zet runs te breed op `FAILED_NEEDS_CLEANUP`, en er staat nog een niet-bestaande ID-generator in het enqueue-voorbeeld.
|
|
|
|
## Bevindingen
|
|
|
|
### P1 - Status-sync is nog niet transactioneel genoeg
|
|
|
|
De sectie heet "transactional + post-commit NOTIFY", maar `syncSuccess` doet eerst `bootstrapRun.updateMany(...)` buiten een transaction en daarna pas een transaction met `claudeJob.updateMany(...)` en `product.update(...)`. Als de tweede transaction faalt, staat de run al op `SUCCEEDED`. Als de job-update `count=0` oplevert, wordt het product alsnog bijgewerkt en wordt alsnog `DONE` genotify'd.
|
|
|
|
Fix: doe run-update, job-update en product-update in één `prisma.$transaction(async tx => ...)`, check beide `updateMany.count` waarden, en notify pas na een volledig geslaagde commit. Zet ook `lease_until` en `claimed_by_worker_id` terminal op `null`.
|
|
|
|
### P1 - Stale recovery zet alle verlopen runs op `FAILED_NEEDS_CLEANUP`
|
|
|
|
De SQL zet alle bijbehorende `bootstrap_runs` op `FAILED_NEEDS_CLEANUP`, terwijl de tekst zegt dat dit alleen moet wanneer `github_repo_full_name IS NOT NULL`. Voor runs zonder externe side effects hoort status `FAILED` te zijn.
|
|
|
|
Fix: split recovery in twee updates:
|
|
|
|
- `FAILED_NEEDS_CLEANUP` alleen waar `github_repo_full_name IS NOT NULL` of `github_repo_created_at IS NOT NULL`.
|
|
- `FAILED` waar beide leeg zijn.
|
|
|
|
Hou de `kind='BOOTSTRAP_REPO'` filter; die is goed.
|
|
|
|
### P1 - Enqueue gebruikt `@paralleldrive/cuid2`, maar die dependency bestaat niet
|
|
|
|
Het plan importeert `createId` uit `@paralleldrive/cuid2`, maar deze repo heeft die dependency niet. De bestaande schema's gebruiken Prisma `cuid()` defaults; applicatiecode genereert die IDs nu niet zelf.
|
|
|
|
Fix: gebruik de transaction callback-vorm en laat Prisma de IDs genereren, of voeg expliciet een dependency toe en leg vast dat alle nieuwe ID-validatie `z.string().cuid()` blijft accepteren. Mijn voorkeur: transaction callback, geen nieuwe ID-library.
|
|
|
|
### P2 - Nieuwe non-null arrayvelden op `User` hebben defaults nodig
|
|
|
|
`github_pat_scopes String[]` is niet nullable en heeft geen default. Op een bestaande database met users maakt dat de migration lastig of onmogelijk zonder backfill.
|
|
|
|
Fix: maak dit `github_pat_scopes String[] @default([])` of gebruik `Json?` als je fine-grained tokenmetadata later flexibeler wilt opslaan.
|
|
|
|
### P2 - NOTIFY-status casing moet expliciet API-lowercase zijn
|
|
|
|
De voorbeelden sturen `status: 'DONE'` en `status: 'QUEUED'`. Bestaande helpers mappen jobstatussen naar lowercase API-strings (`done`, `queued`, etc.). Sommige bestaande paden sturen al lowercase via `jobStatusToApi`.
|
|
|
|
Fix: spreek af dat NOTIFY payloads API-lowercase gebruiken, en DB-writes UPPER_SNAKE houden. Dus `status: 'done'` in payload, `status: 'DONE'` in DB.
|
|
|
|
### P2 - Stale recovery hoort niet pas fase 2 te zijn
|
|
|
|
De service gebruikt leases in MVP, maar de verificatie noemt stale recovery "in fase-2". Zonder recovery kan een crash een job langdurig in `CLAIMED`/`RUNNING` laten hangen.
|
|
|
|
Fix: neem minimale stale recovery op in Sprint 1d: markeer verlopen `BOOTSTRAP_REPO` jobs en runs correct als `FAILED` of `FAILED_NEEDS_CLEANUP`.
|
|
|
|
### P2 - Org-owner preflight moet endpoint-gedreven zijn
|
|
|
|
Voor classic PAT MVP is `repo` scope helder, maar repo creation in een org hangt ook af van de daadwerkelijke org-permissions. Scope-check alleen is niet genoeg.
|
|
|
|
Fix: laat `RepoOwnerPicker` alleen owners tonen waarvoor de concrete Octokit preflight slaagt, en behandel de response als authority. Documenteer dat org-eigenaarschap/permissies via GitHub worden gevalideerd, niet afgeleid uit alleen scopes.
|
|
|
|
## Aanbevolen minimale patch op het plan
|
|
|
|
1. Herschrijf `syncSuccess/syncFailed/syncRunning` als één transaction callback met count-checks.
|
|
2. Split stale recovery in `FAILED` vs `FAILED_NEEDS_CLEANUP`.
|
|
3. Vervang pre-generated `createId()` door een transaction callback of voeg de dependency expliciet toe.
|
|
4. Voeg `@default([])` toe aan `github_pat_scopes`.
|
|
5. Maak NOTIFY statuswaarden lowercase.
|
|
|
|
Daarna is v3.3 goed genoeg om naar `docs/plans/M8-bootstrap-wizard.md` te promoveren.
|