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

8.3 KiB

Raw Blame History

title	status	date	source_plan
Review - Bootstrap-wizard plan	draft	2026-05-13	/Users/janpetervisser/.claude/plans/als-ik-een-nieuwe-virtual-turtle.md

Review - Bootstrap-wizard plan

Korte conclusie

Het plan is functioneel sterk, maar niet uitvoerbaar zoals het nu geschreven is. De hoofdblokkade is dat ClaudeJob wordt gebruikt als deterministische queue, terwijl de huidige runner-architectuur ClaudeJob nog behandelt als een Claude CLI job met een verplicht model/config-pad. Trek dat eerst recht, anders eindigt de feature in typefouten, jobs die nooit terminal worden, of een worker die toch Claude probeert te starten.

Bevindingen

P1 - `BOOTSTRAP_REPO` met `model: null` breekt het huidige job-config contract

Het plan zet voor BOOTSTRAP_REPO expliciet model: null omdat er geen LLM draait (plan regels 101-106). In de huidige code is JobConfig.model niet nullable en beperkt tot ClaudeModel; snapshotFromConfig schrijft die waarde daarna naar ClaudeJob.requested_model als string (lib/job-config.ts regels 27-33 en 205-210). getJobConfigSnapshot is bovendien het bestaande enqueue-pad voor nieuwe jobs (lib/job-config-snapshot.ts regels 1-7 en 34-39).

Fix: maak deterministische jobs een expliciet ander runtime-pad. Bijvoorbeeld een discriminated union runtime: 'claude' | 'deterministic', of laat BOOTSTRAP_REPO de Claude config snapshot volledig overslaan. Alleen een KIND_DEFAULTS entry met model: null is onvoldoende.

P1 - De worker-eigenaar staat verkeerd of is te vaag

Het plan plaatst de dispatch in scrum4me-mcp/src/lib/job-runner.ts en noemt worker-bestanden in scrum4me-mcp (plan regels 172 en 235-238). De actuele runner-architectuur zegt iets anders: scrum4me-docker/bin/run-one-job.ts claimt jobs, resolve't config, bouwt CLI flags en spawnt claude; MCP levert tools/schema (docs/runbooks/worker-idempotency.md regels 170-176 en docs/runbooks/mcp-integration.md regel 12).

Als alleen Scrum4Me en scrum4me-mcp wijzigen, gaat de docker-runner de nieuwe kind nog steeds claimen en behandelen als Claude-job. Neem een expliciete wijziging op voor scrum4me-docker, of definieer een aparte bootstrap-executor. Let ook op: de huidige worker doet een Anthropic quota pre-flight voordat hij claimt (docs/runbooks/mcp-integration.md regels 80-93). Daardoor kan een no-LLM bootstrap-job onterecht wachten op quota.

P1 - De worker-flow sluit de `ClaudeJob` niet terminal af

In de pseudo-flow wordt bij succes alleen BootstrapRun en Product bijgewerkt, gevolgd door een generieke NOTIFY (plan regels 185-186). Bij fouten noemt het plan eveneens vooral BootstrapRun (plan regel 187). Het bestaande queue-protocol verwacht dat de job zelf naar DONE, FAILED of CANCELLED gaat en dat een claude_job_status event wordt verstuurd (docs/runbooks/mcp-integration.md regels 44-49).

Fix: maak BootstrapRun.status en ClaudeJob.status een transactionele status-sync. Bij succes: BootstrapRun.SUCCEEDED, ClaudeJob.DONE, finished_at, summary, repo_url/template_version. Bij failure/cancel: beide terminal, inclusief error, en een claude_job_status notify. Anders blijven jobs CLAIMED of RUNNING en grijpt stale recovery later fout in.

P1 - De enum-uitbreiding veroorzaakt build-fouten buiten de genoemde files

Het plan noemt ClaudeJobKind.BOOTSTRAP_REPO, maar niet alle plekken die exhaustief over ClaudeJobKind heen lopen. JobCard en JobsColumn gebruiken bijvoorbeeld Record<ClaudeJobKind, string> (components/jobs/job-card.tsx regels 28-34 en components/jobs/jobs-column.tsx regels 16-22). Na Prisma generate mist daar een key en faalt typecheck.

Fix: voeg jobs board labels/filters, initial SSE payloads, job detail rendering, cost/insight aggregaties en tests toe aan de scope. Dit is geen nice-to-have; het is build-path.

P1 - `BootstrapRun` koppeling mist relationele details

Het plan zet BootstrapRun.claude_job_id als nullable FK en laat de worker de run ophalen via run_id (plan regels 83-90 en 175), maar ClaudeJob heeft nu alleen task/idea/sprint koppelingen (prisma/schema.prisma regels 385-424). Zonder helder model blijft onduidelijk hoe de geclaimde job precies bij de run komt.

Fix: maak BootstrapRun.claude_job_id @unique, voeg relation names en een reverse relation op ClaudeJob toe, en indexeer product_id/status. Leg ook vast dat startBootstrapAction atomair voorkomt dat er meerdere actieve PENDING/RUNNING runs voor hetzelfde product ontstaan. Dit staat nu als open punt (plan regel 323), maar hoort in MVP.

P1 - PAT-encryptie botst met de huidige worker-secret boundary

Het plan staat encryptie met SESSION_SECRET of een optionele BOOTSTRAP_ENCRYPTION_KEY toe (plan regels 98-110), en laat de worker de PAT decrypten (plan regel 176). De docker-worker docs zeggen juist dat de worker geen DATABASE_URL, SESSION_SECRET of CRON_SECRET hoort te hebben (docs/manual/05-docker.md regels 52-64).

Fix: kies een expliciete credential-boundary. Waarschijnlijk moet BOOTSTRAP_ENCRYPTION_KEY verplicht worden voor app plus deterministische executor, of moet GitHub-side werk in de app/MCP-service gebeuren waar decryptie toegestaan is. Specificeer ook minimale PAT scopes, owner/namespace-keuze en voorkom dat de bestaande worker-level GITHUB_TOKEN per ongeluk repos onder de verkeerde account aanmaakt.

P1 - `BootstrapAction.params` is te vrij voor filesystem-acties

Het plan gebruikt params Json voor acties en noemt alleen een bash allowlist als securitymaatregel (plan regels 57-75 en 210-214). Maar COPY_FILE, WRITE_FILE, APPEND_TO_FILE en REPLACE_STRING kunnen ook schade doen: path traversal via ../, schrijven naar .git/config, absolute paden, te grote bestanden/logs, of onbedoelde workflow-mutaties.

Fix: valideer elke action-kind met een Zod-schema bij seed/admin-save en opnieuw bij uitvoering. Normaliseer paden en assert dat source/dest binnen de template root of output root blijven. Deny .git/**, absolute paden en parent traversal. Cap output_log, content en aantal acties per run.

P1 - MVP spreekt de verplichte zes ADR-stubs tegen

Het plan noemt zes verplichte ADR-stubs voor deploy/auth/DB/styling/state/testing (plan regel 25), maar de MVP seed bevat alleen deploy/auth/database (plan regels 253-260). De verificatie checkt ook alleen ADR-0001 tot ADR-0003 (plan regels 287-294).

Fix: genereer de zes core ADR-stubs onvoorwaardelijk in MVP, of neem alle zes categorieen op in Sprint 1. Anders is de MVP niet consistent met de eigen acceptatie.

P2 - Fysieke UI-paden kloppen niet met de App Router route groups

Het plan noemt fysieke files onder app/products, app/settings en app/admin (plan regels 159-164 en 240-244). In deze codebase zitten desktop routes onder app/(app)/... (docs/architecture/project-structure.md regels 18-42), bijvoorbeeld app/(app)/products/[id]/page.tsx.

Fix: corrigeer de filelijst naar app/(app)/products/[id]/..., app/(app)/settings/... en app/(app)/admin/.... De URL blijft hetzelfde; de fysieke implementatieplek niet.

P2 - Verificatie noemt een niet-bestaand worker-script

De verificatie zegt "manual: npm run worker" (plan regel 291), maar package.json heeft geen worker script (package.json regels 5-26). Dat maakt de E2E-stap niet reproduceerbaar.

Fix: verwijs naar het echte scrum4me-docker runnercommando of voeg bewust een dev-script toe als onderdeel van de feature.

P2 - Repo-slug en GitHub owner zijn nog onvoldoende gespecificeerd

De flow gebruikt <productSlug> en <owner> (plan regels 180-184), maar Product heeft nu name, optionele code en repo_url; geen slugveld (prisma/schema.prisma regels 196-227). code is bovendien niet hetzelfde als GitHub repo-validatie.

Fix: voeg repo_slug toe aan de wizard of maak een gesnapshotte derivatie met GitHub-regels, collision-check, owner-keuze en duidelijke foutmelding wanneer de repo al bestaat.

Aanbevolen aanpassing van de volgorde

Ontwerp eerst het deterministic-job contract: status-sync, runner-eigenaar, quota-bypass, config-bypass en BootstrapRun relation.
Voeg daarna schema + seed toe met path/action validatie en zes minimale ADR-stubs.
Bouw PAT settings en GitHub token test met expliciete scopes en owner-keuze.
Bouw pas daarna de wizard UI en E2E runner.

Met die volgorde blijft de UI dun en voorkom je dat het meeste risico pas in de worker-integratie zichtbaar wordt.

8.3 KiB Raw Blame History