Ga naar inhoud

Review-fixes fase 1

Prompt voor een agent-sessie. Voortgekomen uit een code-review van fase 0 t/m 1.9 op 10 augustus 2026. Geef dit document als opdracht mee; werk het niet bij als werkdocument — de status van taken staat in Taken.

Lees eerst AGENTS.md en Architectuur. De harde regels daarin gelden onverkort: huisstijl komt uit data, geen kleuren/fonts/radii in componenten, types niet dupliceren, tenant-context expliciet via astro:env, code in het Engels. Voeg geen dependencies toe (geen linters, testrunners, CI).

Een code-review van fase 0 t/m 1.9 leverde de punten hieronder op. Werk deel A volledig af. Deel B bevat keuzes: volg per punt de aanbeveling, tenzij je een sterker argument hebt — leg dat dan uit in plaats van stilzwijgend af te wijken. Deel C niet aanpassen, alleen documenteren.

Werk in kleine, losse commits per punt. Blijf binnen de scope: taak 1.10 (blok-editor) hoort hier niet bij, ook niet gedeeltelijk.


Deel A — bugs, oppakken zoals beschreven

Section titled “Deel A — bugs, oppakken zoals beschreven”

A1. De Media-sectie van het CMS is onbereikbaar (blocker)

Section titled “A1. De Media-sectie van het CMS is onbereikbaar (blocker)”

cmsNavItems in apps/site-template/src/lib/cms-nav.ts bevat /media, maar CMS_SECTION_PREFIXES in apps/site-template/src/lib/hosts.ts niet. Daardoor geeft toInternalAdminPath("/media") null, en valt src/middleware.ts door naar de laatste branch (“onbekend pad op de admin-host”) die naar / redirect. Gevolg: de sidebar-link, /media/new en de redirect na upload landen allemaal op het dashboard. Taak 1.9 is hierdoor in de praktijk niet te gebruiken.

Voeg /media toe aan CMS_SECTION_PREFIXES.

Voorkom daarnaast dat dit opnieuw uit elkaar loopt: zorg dat de sidebar en de routing dezelfde bron gebruiken, of dat een mismatch onmogelijk is. Los dit op zonder een testrunner toe te voegen — bijvoorbeeld door cmsNavItems als bron te nemen voor de prefixes, of andersom.

A2. Open redirect op de loginpagina (server-side)

Section titled “A2. Open redirect op de loginpagina (server-side)”

In apps/site-template/src/pages/login.astro staat in de frontmatter:

return Astro.redirect(redirectTo.startsWith("/") ? redirectTo : "/");

//evil.com begint met / en is protocol-relatief, dus de browser gaat naar https://evil.com. Het clientscript onderaan hetzelfde bestand doet het al goed (redirectTo.startsWith("/") && !redirectTo.startsWith("//")).

Trek de validatie in één helper (bijv. safeRedirectPath() in lib/hosts.ts) en gebruik die op beide plekken, zodat ze niet meer los kunnen lopen. Check ook de andere auth-pagina’s op hetzelfde patroon.

A3. Media verwijderen laat kapotte blokken achter

Section titled “A3. Media verwijderen laat kapotte blokken achter”

mediaId in de blocks-jsonb heeft geen foreign key (kan ook niet). Verwijder je media die in een blok gebruikt wordt, dan faalt imageBlockSchema (waar mediaId verplicht is) bij het renderen, en BlockRenderer rendert in productie stil nietsreturn null buiten dev. De redacteur krijgt geen enkele waarschuwing.

Pas deleteMedia in apps/site-template/src/actions/cms-media.ts aan: zoek vóór het verwijderen naar verwijzingen naar dit id in pages.blocks, services.blocks en news.blocks, en blokkeer de delete met een ActionError (CONFLICT) die benoemt waar de media nog gebruikt wordt. Hergebruik collectMediaIds uit lib/media.ts als dat past; anders een jsonb-query, maar houd de logica op één plek. De bestaande FK-kolommen (services.imageId, news.imageId, testimonials.logoId) horen dezelfde nette melding te geven — zie A5.

A4. Sessielookup op élk request, ook op assets

Section titled “A4. Sessielookup op élk request, ook op assets”

src/middleware.ts roept getAuth().api.getSession() aan vóór de isAssetPath()-check. Elke /_astro/*-request doet dus een databasequery.

Verplaats de asset-check naar boven de sessie-call. Let op: isAssetPath matcht ook /_actions, en Astro Actions leunen op context.locals.user (via requireCmsUser). Zorg dus dat voor /_actions de sessie wel geladen blijft; alleen echte statische assets mogen de lookup overslaan.

A5. FK-conflict geeft 500 in plaats van 409

Section titled “A5. FK-conflict geeft 500 in plaats van 409”

throwIfUniqueViolation in apps/site-template/src/lib/cms/errors.ts matcht alleen Postgres-code 23505 (unique violation). Een foreign-key-violation is 23503 en valt nu door naar INTERNAL_SERVER_ERROR.

Behandel 23503 als CONFLICT met een bruikbare melding. Hernoem de helper als de naam daardoor misleidend wordt.

A6. Sitemap bevat overzichtspagina’s die leeg kunnen zijn

Section titled “A6. Sitemap bevat overzichtspagina’s die leeg kunnen zijn”

src/pages/sitemap.xml.ts voegt /diensten, /nieuws en /referenties onvoorwaardelijk toe, ook zonder gepubliceerde items. Neem ze alleen op als er minstens één gepubliceerd item is (referenties: minstens één rij).


Deel B — dode configuratie, kies en maak consistent

Section titled “Deel B — dode configuratie, kies en maak consistent”

Regel: een instelling die je in het CMS kunt zetten moet effect hebben, of niet in het formulier staan. Nu geldt voor drie velden geen van beide.

B1. theme.logo.mediaId en logo.mediaIdDark renderen nergens

Section titled “B1. theme.logo.mediaId en logo.mediaIdDark renderen nergens”

Instelbaar in src/pages/admin/instellingen/index.astro, maar Header.astro zet alleen siteName als tekst.

Aanbeveling: render logo.mediaId in Header.astro (via MediaImage uit @platform/blocks), met siteName als fallback wanneer er geen logo is; houd siteName als alt. Haal logo.mediaIdDark uit het formulier — er is geen dark mode, dus het veld is misleidend. Laat het veld wél in themeLogoSchema staan (optioneel) zodat bestaande data niet breekt, en noteer in Taken onder “Losse punten” dat dark-mode-logo’s nog open staan.

B2. --font-scale en --spacing-base hebben geen consument

Section titled “B2. --font-scale en --spacing-base hebben geen consument”

themeToCssVars in packages/contract/src/theme.ts emitteert beide, maar apps/site-template/src/styles/global.css mapt ze niet in @theme en geen enkel blok raakt ze aan. typography.scale in de database heeft dus geen effect.

Aanbeveling: maak ze werkend in plaats van ze te schrappen — scale is een zinnig huisstijl-knopje. Consumeer --font-scale op precies één plek (de root-fontsize in global.css) en map --spacing-base op Tailwind’s basis- spacingvariabele in het bestaande @theme inline-blok. Verifieer daarna dat een scale van bijv. 1.15 in de database de site zichtbaar verandert zonder code-aanpassing — dat is hetzelfde bewijs dat taak 1.1 vroeg.

B3. Admin en redacteur hebben identieke rechten

Section titled “B3. Admin en redacteur hebben identieke rechten”

Elke mutatie gaat via requireCmsUser (src/lib/cms/require-cms-user.ts), en die accepteert admin én editor. Een redacteur kan dus de huisstijl overschrijven, de formulierontvanger wijzigen en alles verwijderen. editor = ac.newRole({}) in lib/auth-permissions.ts wordt nergens geraadpleegd; adminRoles: ["admin"] in lib/auth.ts beschermt uitsluitend Better Auth’s eigen user-management-endpoints. Taak 1.6 leverde de rollen op, 1.8 heeft ze nooit gebruikt.

Aanbeveling — dit is de grens, wijk hier niet van af zonder te melden:

  • editor mag content beheren: pages, services, news, testimonials, navigation, media (create/update/delete).
  • Alleen admin mag updateSiteSettings en updateFormRecipient aanroepen.

Bouw dit als één expliciete guard naast requireCmsUser (bijv. requireCmsAdmin) en gebruik die in de betreffende actions. Verberg of disable de admin-only onderdelen ook in de UI, zodat een redacteur niet in een formulier belandt dat bij verzenden faalt. Als je permissions via createAccessControl wilt afdwingen in plaats van via een losse guard: prima, maar dan wel op één plek en niet half.


TENANT_ID en SITE_DOMAIN staan in astro.config.mjs als envField.string({ access: "public" }). Astro valideert en inlinet die tijdens de build — pnpm --filter site-template run build faalt zonder deze variabelen. Gevolg: één image kan geen twee tenants dienen, en elke klant heeft een eigen build nodig. Dat raakt taak 1.11 (Dockerfile) en 2.9 (template-update naar álle klanten) direct.

Verander hier niets. Voeg in Taken bij taak 1.11 een korte notitie toe dat dit vóór 1.11 een expliciete keuze vraagt: per-tenant build, of deze twee variabelen naar runtime-config verhuizen. Bouw de keuze niet zelf in.


Draai en laat de uitvoer zien:

pnpm install
pnpm turbo run typecheck
pnpm turbo run build # met TENANT_ID, SITE_DOMAIN, DATABASE_URL en BETTER_AUTH_SECRET gezet
pnpm run check:tokens

Controleer daarnaast handmatig, met docker compose up -d en een geseede database:

  1. Sidebar → Media opent /media, uploaden werkt, redirect na upload landt op de detailpagina (A1).
  2. admin.localhost:4321/login?redirect=//example.com terwijl je ingelogd bent redirect naar /, niet naar example.com (A2).
  3. Media die in een blok gebruikt wordt, kan niet verwijderd worden en geeft een melding die benoemt waar hij gebruikt wordt (A3).
  4. Inloggen als editor@demo.local: instellingen zijn niet bereikbaar of niet opslaanbaar; content wel (B3).
  5. typography.scale in de database wijzigen verandert de site zonder deploy (B2).

Meld aan het eind, conform AGENTS.md:

  • welke versies je hebt vastgesteld (draai npm view <pkg> dist-tags waar je configuratie aanraakt; neem niets aan uit je trainingsdata);
  • wat je anders hebt gedaan dan hier gevraagd, en waarom;
  • waar je twijfelde over een ontwerpkeuze — met name bij B1 t/m B3;
  • welke verificatiestap je niet hebt kunnen uitvoeren, en waarom.

Vink in Taken niets extra af. Noteer wel per punt kort wat er is gewijzigd, in de stijl van dat document (“vink af, noteer afwijkingen”).