* 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>
210 lines
12 KiB
Markdown
210 lines
12 KiB
Markdown
---
|
|
title: "Review - Bootstrap-wizard plan v2 met webresearch"
|
|
status: draft
|
|
date: 2026-05-13
|
|
source_plan: "/Users/janpetervisser/.claude/plans/als-ik-een-nieuwe-virtual-turtle.md"
|
|
previous_review: "docs/recommendations/bootstrap-wizard-plan-review-2026-05-13.md"
|
|
---
|
|
|
|
# Review - Bootstrap-wizard plan v2 met webresearch
|
|
|
|
## Conclusie
|
|
|
|
De eerdere aanbevelingen zijn grotendeels verwerkt, maar nog niet "goed" genoeg om dit plan direct naar implementatie te brengen. V2 lost de meeste oude schema-, enum-, status- en path-safety punten op papier op. De grootste resterende fout is dat het plan twee executor-modellen tegelijk beschrijft: eerst `scrum4me-docker` als deterministic runner, later de Next.js app als executor met een fire-and-forget background promise. Kies er een.
|
|
|
|
Mijn advies: maak de app niet de lange-running executor. Gebruik voor MVP een aparte `bootstrap-service` of breid de bestaande docker-runner expliciet uit met een veilig secret-contract. Vercel/Next fire-and-forget is te broos voor clone, file mutation, GitHub repo-create en push.
|
|
|
|
## Zijn de eerdere aanbevelingen verwerkt?
|
|
|
|
| Reviewpunt | Status | Oordeel |
|
|
|---|---:|---|
|
|
| Deterministic runtime ipv `model: null` | Ja | Goed concept, maar nog te veel gekoppeld aan `JobConfig` als de app uiteindelijk executor wordt. |
|
|
| Worker-eigenaar expliciet maken | Deels | V2 spreekt zichzelf tegen: docker-runner dispatch versus app-orchestrator. |
|
|
| Transactionele `BootstrapRun` + `ClaudeJob` status-sync | Ja | Goed. Hou notify na commit, niet in de DB-transaction zelf. |
|
|
| `ClaudeJobKind` exhaustive consumers | Ja | Goed opgenomen. |
|
|
| `BootstrapRun.claude_job_id @unique` + reverse relation | Ja | Goed. |
|
|
| Concurrency guard | Ja | Goed, vooral met DB-level partial unique index. |
|
|
| PAT secret-boundary | Deels | Docker krijgt geen DB/secrets meer, maar PAT wordt nu in memory doorgegeven aan een background promise. Dat is niet duurzaam. |
|
|
| Action-param validatie/path-safety | Ja | Goed, maar `condition: String?` blijft een risico. |
|
|
| Zes ADR-stubs in MVP | Ja | Goed. |
|
|
| App Router paden | Ja | Goed. |
|
|
| Niet-bestaand `npm run worker` | Ja | Gecorrigeerd. |
|
|
| `Product.repo_slug` | Ja | Goed begin, maar uniekheid moet eigenlijk per GitHub owner + slug, niet per Scrum4Me user. |
|
|
|
|
## Nieuwe bevindingen
|
|
|
|
### P1 - V2 heeft nog twee executor-architecturen tegelijk
|
|
|
|
Regels 49-66 beschrijven dispatch in `scrum4me-docker/bin/run-one-job.ts`, inclusief deterministic dispatch en ephemeral PAT op job-claim. Regels 200-212 kiezen daarna voor "App is executor" en laten docker `BOOTSTRAP_REPO` juist niet claimen. Regels 285-322 werken vervolgens app-side fire-and-forget uit.
|
|
|
|
Dat is geen detail; dit bepaalt wie claimt, wie secrets heeft, wie retries doet en wie eigenaar is van leases/timeouts. Maak de keuze expliciet:
|
|
|
|
- Optie A: `bootstrap-service` claimt alleen `BOOTSTRAP_REPO`, heeft `DATABASE_URL` + `BOOTSTRAP_ENCRYPTION_KEY`, decrypt zelf de PAT per run, en gebruikt dezelfde status-sync.
|
|
- Optie B: bestaande docker-runner claimt ook deterministic jobs, maar dan moet de secret-boundary worden aangepast en gedocumenteerd.
|
|
- Optie C: Next.js app voert inline uit, maar dan geen queue/claim-semantiek en geen 60 minuten timeout claimen.
|
|
|
|
Voor Scrum4Me past Optie A het best: klein apart Node-proces, geen Claude quota, wel durable retries.
|
|
|
|
### P1 - Fire-and-forget in de app is niet betrouwbaar genoeg
|
|
|
|
Het plan kiest `runBootstrapInBackground(runId, pat)` na de server-action response. Vercel documenteert dat niet-geawait async werk in Functions kan blijven hangen in een bevroren execution context; helpers als `waitUntil()` zijn bovendien nog steeds gebonden aan de maximale function timeout. Vercel Functions hebben harde duration-limieten; het plan noemt zelf een 60-minuten watchdog, wat niet past bij normale serverless limits.
|
|
|
|
Fix: vervang `fire-and-forget` door een echte worker:
|
|
|
|
- `startBootstrapAction` maakt alleen `BootstrapRun` + `ClaudeJob`.
|
|
- `bootstrap-service` claimt atomair `BOOTSTRAP_REPO` runs.
|
|
- Service decrypt de PAT op basis van `run.user_id`, voert de recipe uit, en sync't terminal status.
|
|
- UI blijft exact hetzelfde via SSE.
|
|
|
|
### P1 - PAT doorgeven aan een background promise is de verkeerde secret-shape
|
|
|
|
Regel 197 zegt dat `startBootstrapAction` decrypt voor enqueue, en regel 298 geeft `pat` door aan de background runner. Als het proces wegvalt, is de job niet hervatbaar zonder opnieuw vanuit user context te starten. Als logs of closures uitlekken, zit de PAT in app-memory buiten een duidelijk lifecycle-contract.
|
|
|
|
Fix: geef alleen `runId` door. De executor haalt `User.github_pat_encrypted` zelf op, decrypt binnen de execution boundary, zeroized daarna best-effort, en logt nooit token-materiaal. Voeg `github_pat_verified_at`, `github_pat_scopes` en `github_pat_expires_at` toe of overweeg later GitHub App/OAuth.
|
|
|
|
### P1 - Gebruik geen lange-running local git push in een serverless function
|
|
|
|
De v2-flow gebruikt `mkdtemp`, template clone, lokale git commit, repo create en push. Dat is prima voor een worker/service, maar kwetsbaar in serverless: tijdslimieten, file descriptor limieten, cleanup bij timeout, en onduidelijke rollback wanneer push half lukt.
|
|
|
|
Fix: zet dit in `bootstrap-service` of Vercel Sandbox/Workflow. Als je toch app-side wilt blijven, maak de eerste versie veel kleiner: GitHub template endpoint aanroepen, geen lokale mutaties, geen push, geen `RUN_BASH_TEMPLATE`.
|
|
|
|
### P2 - Voeg een dry-run/preview toe voor de wizard en admin-catalog
|
|
|
|
Backstage Scaffolder heeft dry-run support en een Template Editor waarmee templates in een echte omgeving getest kunnen worden zonder externe mutaties. Scrum4Me mist dit nog.
|
|
|
|
Aanbevolen toevoeging:
|
|
|
|
- `previewBootstrapAction(productId, selections)` bouwt `recipe_snapshot`, valideert acties, draait alle non-mutating file handlers in tmpdir, en retourneert file tree + action log + warnings.
|
|
- UI toont "Review" voor "Create repo".
|
|
- Admin-UI mag een recipe pas activeren nadat dry-run groen is.
|
|
- Tests draaien per action ook in dry-run mode.
|
|
|
|
Dit verlaagt het risico van DB-gedreven recipes sterk.
|
|
|
|
### P2 - Maak repository owner/slug een echte picker, geen impliciete username
|
|
|
|
Backstage gebruikt een repository picker met allowed hosts, owners en repos. Het plan heeft `repo_slug`, maar owner blijft impliciet `user.github_username` en staat zelfs nog als open punt.
|
|
|
|
Fix voor MVP:
|
|
|
|
- `Product.repo_owner` of `BootstrapRun.repo_owner_snapshot`.
|
|
- `repo_slug` uniqueness op `(repo_owner, repo_slug)`, niet op `(user_id, repo_slug)`.
|
|
- `saveGitHubPatAction` haalt beschikbare orgs op en bewaart geen owner zonder permissiecheck.
|
|
- Wizard laat owner + slug zien en doet preflight `GET /repos/{owner}/{repo}` of equivalente Octokit call.
|
|
|
|
### P2 - Gebruik GitHub template API bewust, of leg uit waarom niet
|
|
|
|
GitHub heeft een officieel endpoint om een repository uit een template te maken. Dat is eenvoudiger en veiliger dan zelf init/remote/push doen, maar het endpoint werkt met de template repo en repo-name/owner, niet met een willekeurige tag/ref zoals `template_version`.
|
|
|
|
Aanbevolen beslissing:
|
|
|
|
- Als `template_version` hard nodig is: blijf bij "download/clone tagged template, mutate, push", maar documenteer dat GitHub's template endpoint bewust niet gebruikt wordt.
|
|
- Als default-branch voldoende is: gebruik GitHub's template endpoint voor MVP en beperk v1 tot variabelen die later via follow-up commits kunnen.
|
|
|
|
Voor dit plan zou ik tag-pinning behouden, maar de trade-off expliciet maken.
|
|
|
|
### P2 - Voeg action-permissions toe, niet alleen admin CRUD
|
|
|
|
Backstage kan parameters, steps en actions autoriseren. Scrum4Me v2 heeft alleen "admin-UI fase 2" en path-safety. Dat beschermt niet tegen een legitieme recipe die te veel doet.
|
|
|
|
Voeg toe aan `BootstrapAction` of `BootstrapOption`:
|
|
|
|
- `risk_level: LOW | MEDIUM | HIGH`
|
|
- `requires_role: ADMIN | PRODUCT_OWNER`
|
|
- `enabled: boolean`
|
|
- `supports_dry_run: boolean`
|
|
- `side_effects: FILESYSTEM | GITHUB_REPO | GITHUB_SETTINGS | NETWORK`
|
|
|
|
`RUN_BASH_TEMPLATE` en GitHub-mutaties mogen standaard alleen admin-authored en dry-run getest zijn.
|
|
|
|
### P2 - Vervang `condition: String?` door een getypte mini-DSL of haal hem uit MVP
|
|
|
|
Een vrije condition string in DB is op termijn een tweede interpreter. Gebruik liever:
|
|
|
|
```ts
|
|
condition: {
|
|
allOf?: Array<{ category: string; option: string }>
|
|
anyOf?: Array<{ category: string; option: string }>
|
|
not?: Array<{ category: string; option: string }>
|
|
}
|
|
```
|
|
|
|
Valideer met Zod en snapshot de resolved action list. Voor MVP: geen conditions, alleen expliciete selected options.
|
|
|
|
### P2 - Maak template/catalog versioning scherper
|
|
|
|
Het plan heeft `template_version` en `recipe_snapshot`, maar mist nog:
|
|
|
|
- `template_source_sha` of release asset checksum.
|
|
- `catalog_version` of `recipe_hash`.
|
|
- `action_schema_version`.
|
|
- `generated_from` metadata in de nieuwe repo, bijvoorbeeld `.scrum4me/bootstrap.json`.
|
|
|
|
Dat maakt update-detection en latere "rerun/update repo" veel simpeler.
|
|
|
|
## Webresearch: vergelijkbare ideeen
|
|
|
|
### GitHub template repositories
|
|
|
|
GitHub ondersteunt "create repository using a template" via REST. Belangrijk: token scopes verschillen voor public/private repos; het endpoint accepteert `owner`, `name`, `include_all_branches` en `private`. Dit bevestigt dat owner/slug en token-scope preflight first-class moeten zijn.
|
|
|
|
Bron: <https://docs.github.com/en/rest/repos/repos#create-a-repository-using-a-template>
|
|
|
|
### Backstage Software Templates / Scaffolder
|
|
|
|
Backstage is het dichtstbijzijnde patroon: skeleton code laden, variabelen templaten, en publishen naar GitHub/GitLab. Het heeft ook built-in actions voor fetch/publish, een template editor, dry-run, secrets, repository picker en permission controls.
|
|
|
|
Relevante lessen:
|
|
|
|
- Scrum4Me's `BootstrapActionKind` lijkt sterk op Backstage scaffolder actions.
|
|
- Dry-run en template editor horen vroeg in het plan, niet pas na MVP.
|
|
- Secrets moeten apart van gewone parameters blijven.
|
|
- Repository owner/host/repo hoort een picker met policy te zijn.
|
|
- Action-level permissions zijn belangrijk als recipes in DB/admin UI leven.
|
|
|
|
Bronnen:
|
|
|
|
- <https://backstage.io/docs/features/software-templates/>
|
|
- <https://backstage.io/docs/features/software-templates/builtin-actions/>
|
|
- <https://backstage.io/docs/features/software-templates/writing-templates/>
|
|
- <https://backstage.io/docs/next/features/software-templates/dry-run-testing/>
|
|
- <https://backstage.io/docs/next/features/software-templates/authorizing-scaffolder-template-details/>
|
|
|
|
### Cookiecutter, Plop, Hygen
|
|
|
|
Cookiecutter bevestigt het template-repo model met prompts/context/replay. Plop en Hygen bevestigen het action/generator model, maar zijn vooral lokaal/dev-tooling, niet server-side repo provisioning.
|
|
|
|
Lessen voor Scrum4Me:
|
|
|
|
- Houd de action-set klein en composable.
|
|
- Zorg voor replay: bewaar parameters, template versie en recipe hash.
|
|
- Maak custom actions code-owned, niet vrij definieerbaar vanuit DB.
|
|
|
|
Bronnen:
|
|
|
|
- <https://cookiecutter.readthedocs.io/en/stable/>
|
|
- <https://plopjs.com/documentation/>
|
|
- <https://hygen.ecmascript.pizza/docs/create/>
|
|
|
|
### Vercel Functions
|
|
|
|
Omdat het plan de app als executor overweegt, zijn Vercel limits relevant. Vercel Functions hebben maximale duur en background helpers zijn nog steeds aan die max duration gebonden. Dat maakt app-side fire-and-forget ongeschikt als robuuste bootstrap-queue.
|
|
|
|
Bronnen:
|
|
|
|
- <https://vercel.com/docs/functions/limitations>
|
|
- <https://vercel.com/kb/guide/troubleshooting-inconsistent-logs-in-vercel-functions>
|
|
|
|
## Aangepaste aanbeveling voor het plan
|
|
|
|
Vervang de executor-sectie door deze keuze:
|
|
|
|
1. `BOOTSTRAP_REPO` blijft een `ClaudeJobKind` alleen voor uniforme UI/SSE/status.
|
|
2. `scrum4me-docker` claimt `BOOTSTRAP_REPO` niet.
|
|
3. Nieuwe `bootstrap-service` claimt alleen `BOOTSTRAP_REPO` of `BootstrapRun(PENDING)`.
|
|
4. Service heeft `DATABASE_URL`, `DIRECT_URL`, `BOOTSTRAP_ENCRYPTION_KEY`, geen Anthropic key nodig.
|
|
5. Service decrypt PAT per run, voert recipe uit, en gebruikt dezelfde transactionele status-sync.
|
|
6. Voeg `previewBootstrapAction` dry-run toe voor wizard en admin.
|
|
7. Voeg owner picker, action permissions, catalog versioning en `.scrum4me/bootstrap.json` toe.
|
|
|
|
Met die aanpassing wordt het plan duidelijker, veiliger en veel dichter bij bewezen scaffolder-patronen.
|