Scrum4Me/docs/recommendations/bootstrap-wizard-plan-v3-4-review-2026-05-14.md
Janpeter Visser d84cdf664f
feat(PBI-67): IDEA_REVIEW_PLAN — iterative multi-model plan review (#199)
* 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>
2026-05-14 03:35:02 +02:00

7.1 KiB

Review — M8 bootstrap-wizard plan v3.4

Datum: 2026-05-14
Bronplan: docs/plans/M8-bootstrap-wizard.md
Scope: plan-review, geen implementatie uitgevoerd. Ik heb ook kort vergeleken met bestaande repo-contracten zoals prisma/schema.prisma, lib/job-status.ts, tsconfig.json en package.json.

Conclusie

De aanbevelingen uit de vorige review zijn grotendeels goed verwerkt. Ik zie geen P1-blocker meer in de laatste versie. De belangrijkste restpunten zitten in GitHub owner-permissies, catalog-hash determinisme en acceptatie-tests.

Findings

[P2] Org-owner preflight belooft meer zekerheid dan de beschreven checks kunnen leveren

Referentie: docs/plans/M8-bootstrap-wizard.md:50, docs/plans/M8-bootstrap-wizard.md:540-567

Het plan zegt dat RepoOwnerPicker alleen owners toont waarvoor een concrete repo-create-preflight slaagt. De uitgewerkte check doet echter GET /orgs/{org} plus membership-check. Dat bewijst lidmaatschap/zichtbaarheid, niet dat de PAT daadwerkelijk een private repo in die org mag maken.

GitHub documenteert voor org-repo creation dat de authenticated user org-lid moet zijn en dat classic PATs repo nodig hebben voor private repositories. Daarnaast kunnen org-instellingen repo creation beperken; de org API exposeert velden zoals members_can_create_repositories en members_allowed_repository_creation_type. De huidige plan-check gebruikt die velden niet en kan daardoor false positives of false negatives geven.

Aanbevolen wijziging:

  • Noem dit expliciet een best-effort owner discovery, niet een harde create-permission proof.
  • Valideer collision met GET /repos/{owner}/{repo}.
  • Laat de echte create-call in de service de finale autorisatie zijn en vertaal 403/422 naar een duidelijke wizard-fout.
  • Als je org-policy vooraf wilt meenemen: lees org creation settings waar beschikbaar, maar behandel ontbrekende rechten/SSO/admin-scope als onbekend in plaats van owner automatisch te verbergen.

Bronnen: GitHub REST docs voor repositories en organizations.

[P2] syncRunning mist expliciete timestamp-contracten

Referentie: docs/plans/M8-bootstrap-wizard.md:230, docs/plans/M8-bootstrap-wizard.md:418-420, docs/plans/M8-bootstrap-wizard.md:965-968

Het plan specificeert voor syncRunning alleen de status-overgang PENDING -> RUNNING en CLAIMED -> RUNNING. De modellen hebben started_at, en de verificatie sorteert later op started_at. Als syncRunning die velden niet atomair vult, worden metrics, UI-sortering en acceptatiequeries onbetrouwbaar.

Aanbevolen wijziging:

  • Zet in dezelfde transaction bootstrap_runs.started_at = now en claude_jobs.started_at = now.
  • Gebruik dezelfde now-waarde voor run en job.
  • Voeg een unit/integration-test toe voor CLAIMED/PENDING -> RUNNING inclusief started_at.

[P2] catalog_version is nog niet deterministisch genoeg gespecificeerd

Referentie: docs/plans/M8-bootstrap-wizard.md:603-634

recipe_hash is goed uitgewerkt, maar catalog_version blijft te vaag: SELECT md5(string_agg(...)) FROM bootstrap_options ... is zonder expliciete ordering niet deterministisch en lijkt alleen options te hashen. Catalog changes in categories, actions, params, roles, risk levels, enabled, archived of supports_dry_run kunnen dan gemist worden.

Aanbevolen wijziging:

  • Gebruik dezelfde canonical JSON-aanpak als recipe_hash.
  • Hash categories, options en actions samen.
  • Sorteer expliciet op category display_order/slug, option display_order/slug, action execution_order/id.
  • Include minstens: selection type, required/default flags, enabled/archived, action kind, action params, dry-run support, side effects, risk level en required role.
  • Gebruik sha256, niet ad-hoc md5(string_agg(...)).

[P2] De E2E-verificatiequery leest lease_until uit de verkeerde tabel

Referentie: docs/plans/M8-bootstrap-wizard.md:965-968

De query selecteert lease_until > NOW() uit bootstrap_runs, maar lease_until staat op claude_jobs. Deze acceptatiestap faalt zodra iemand het letterlijk uitvoert en kan lease-regressies maskeren.

Aanbevolen wijziging:

SELECT br.status,
       br.repo_url,
       br.recipe_hash,
       cj.lease_until > NOW() AS lease_active
FROM bootstrap_runs br
JOIN claude_jobs cj ON cj.id = br.claude_job_id
ORDER BY br.started_at DESC NULLS LAST, br.created_at DESC
LIMIT 1;

[P3] Startup stale-recovery uitleg is inconsistent met de worker-id definitie

Referentie: docs/plans/M8-bootstrap-wizard.md:93, docs/plans/M8-bootstrap-wizard.md:149-151

De worker-id bevat hostname, pid en start timestamp. Een herstartende service heeft dus niet dezelfde claimed_by_worker_id. De SQL in het plan is gelukkig globaal en kind-gefilterd, maar de uitleg zegt dat dezelfde service-instance zichzelf herkent via de oude hostname.

Aanbevolen wijziging:

  • Beschrijf startup recovery als globale recovery voor verlopen BOOTSTRAP_REPO leases.
  • Niet filteren op claimed_by_worker_id bij stale recovery.
  • Bewaar claimed_by_worker_id alleen voor renewal/observability.

[P3] Vendor-copy drift-mitigatie staat alleen als risico, niet als concrete sprint-taak

Referentie: docs/plans/M8-bootstrap-wizard.md:749-751, docs/plans/M8-bootstrap-wizard.md:1023-1028

Het plan erkent terecht dat vendor-copy drift tussen Scrum4Me en bootstrap-service gevaarlijk is. De mitigatie, een schema-hash CI-check, staat alleen bij accepted risks en niet bij fasering of verificatie.

Aanbevolen wijziging:

  • Maak de hash-check onderdeel van Sprint 1a of Sprint 1d.
  • Laat bootstrap-service bij startup loggen welke ActionSchema versie/hash geladen is.
  • Voeg een verificatiestap toe die faalt als packages/bootstrap-actions in de service niet overeenkomt met de Scrum4Me-bron.

[P3] ADD_DEPENDENCY.version regex is te smal voor normale npm specs

Referentie: docs/plans/M8-bootstrap-wizard.md:770-778

De regex accepteert alleen cijfers en operators. Geldige npm-versies zoals latest, prerelease labels (^1.2.3-beta.1), workspace:*, npm: aliases of git/tarball specs worden afgewezen. Voor MVP kan dit acceptabel zijn als seed-data alleen simpele semver gebruikt, maar het moet expliciet zijn.

Aanbevolen wijziging:

  • Documenteer MVP als "alleen exact/range semver".
  • Of gebruik een echte parser zoals npm-package-arg/semver en allowlist de toegestane spec-types.

Wat goed verwerkt is

  • Transactionele status-sync staat nu in één prisma.$transaction met post-commit NOTIFY.
  • FAILED_NEEDS_CLEANUP wordt alleen gebruikt bij bekende GitHub side-effects.
  • claimed_by_worker_id is terecht apart gehouden van claimed_by_token_id.
  • De @paralleldrive/cuid2 afhankelijkheid is verdwenen; Prisma cuid() blijft consistent met het bestaande schema.
  • Lowercase SSE-status via jobStatusToApi matcht het bestaande contract.
  • Stale recovery staat nu in Sprint 1d en is dus onderdeel van MVP.

Go/no-go

Go na verwerking van de P2-punten. De P3-punten kunnen mee in dezelfde planupdate, maar hoeven geen implementatie te blokkeren zolang ze expliciet als MVP-beperking of verificatietaak worden vastgelegd.