fix(build): unbreak production build — /hilfe/transkription prerender unreachable behind /login #472

Closed
opened 2026-05-07 17:28:47 +02:00 by marcel · 9 comments
Owner

Context

Pre-prod audit ran npm run build inside the running archive-frontend container. The build itself succeeded (162 chunks emitted), but the prerender step fails:

✓ built in 6.96s
  302 /hilfe/transkription -> /login

Error: The following routes were marked as prerenderable, but were not prerendered
because they were not found while crawling your app:
  - /hilfe/transkription

frontend/src/routes/hilfe/transkription/+page.ts:3 declares export const prerender = true. The route is gated by the global auth handler in hooks.server.ts (every non-public path redirects to /login), so the prerender crawler reaches /login and never finds the help page. The build exits non-zero.

Effect: today's npm run build fails by default. CI may currently run only tests + lint (audit confirmed .gitea/workflows/ci.yml has no build step), so this hasn't broken green merges, but the moment we add npm run build to CI (planned in the new container-images / devops(ci) issue), every PR fails until this is fixed.

Approach

Two reasonable fixes — pick one:

Tell the prerenderer about the route directly so it doesn't have to crawl into it:

// svelte.config.js
kit: {
    adapter: adapter(),
    prerender: {
        entries: ['/hilfe/transkription']
    }
}

This is the documented escape hatch for routes that crawling can't reach.

Option B — make the help route public

Add /hilfe/transkription (and any future help routes) to the PUBLIC_PATHS list in hooks.server.ts:8:

const PUBLIC_PATHS = [
    '/login', '/logout', '/forgot-password', '/reset-password', '/register',
    '/hilfe',   // help is public
];

Pro: lets users read the transcription guide without logging in (arguably desirable — part of the inviting-mom-to-transcribe story).
Con: scope creep into product behavior; dive into whether help should be public is a separate conversation.

Recommend Option A for this fix. Do Option B as a follow-up if the team wants public help.

Option C — remove prerender = true

If prerender is gone, the route gets SSR-rendered per request. Cheap operationally but loses the "ship as a static HTML file" benefit. Not recommended for what is intentionally a static help page.

Critical files

  • frontend/svelte.config.js — add kit.prerender.entries

Verification

  1. cd frontend && docker exec archive-frontend npm run build (or run with the container user) — exit 0.
  2. .svelte-kit/output/prerendered/pages/hilfe/transkription.html exists.
  3. npm run preview and curl -I http://localhost:4173/hilfe/transkription returns the prerendered HTML (or, on Option B, returns 200 without auth).

Acceptance criteria

  • npm run build exits 0.
  • /hilfe/transkription is prerendered to a static HTML file.
  • No regression on other prerendered routes (none currently — this is the only one).
  • Once green, add npm run build as a step in .gitea/workflows/ci.yml to prevent regression.

Effort

XS — 5 minutes for Option A. The CI step is a one-liner in the new devops(ci) issue.

Risk if not addressed

CI will fail the moment we add npm run build (or container build) to it. Production image build also fails until this is resolved. P0 because it currently blocks the prod-image work in #135 and the new CI gates issue.

Tracked in audit doc as F-35 (Medium → P0 via blocking dependency on #135 / CI gates).

## Context Pre-prod audit ran `npm run build` inside the running `archive-frontend` container. The build itself succeeded (162 chunks emitted), but the prerender step fails: ``` ✓ built in 6.96s 302 /hilfe/transkription -> /login Error: The following routes were marked as prerenderable, but were not prerendered because they were not found while crawling your app: - /hilfe/transkription ``` `frontend/src/routes/hilfe/transkription/+page.ts:3` declares `export const prerender = true`. The route is gated by the global auth handler in `hooks.server.ts` (every non-public path redirects to `/login`), so the prerender crawler reaches `/login` and never finds the help page. The build exits non-zero. Effect: **today's `npm run build` fails by default**. CI may currently run only tests + lint (audit confirmed `.gitea/workflows/ci.yml` has no build step), so this hasn't broken green merges, but the moment we add `npm run build` to CI (planned in the new container-images / `devops(ci)` issue), every PR fails until this is fixed. ## Approach Two reasonable fixes — pick one: ### Option A — explicit prerender entries (recommended) Tell the prerenderer about the route directly so it doesn't have to crawl into it: ```javascript // svelte.config.js kit: { adapter: adapter(), prerender: { entries: ['/hilfe/transkription'] } } ``` This is the documented escape hatch for routes that crawling can't reach. ### Option B — make the help route public Add `/hilfe/transkription` (and any future help routes) to the `PUBLIC_PATHS` list in `hooks.server.ts:8`: ```typescript const PUBLIC_PATHS = [ '/login', '/logout', '/forgot-password', '/reset-password', '/register', '/hilfe', // help is public ]; ``` Pro: lets users read the transcription guide without logging in (arguably desirable — part of the inviting-mom-to-transcribe story). Con: scope creep into product behavior; dive into whether help should be public is a separate conversation. **Recommend Option A for this fix.** Do Option B as a follow-up if the team wants public help. ### Option C — remove `prerender = true` If prerender is gone, the route gets SSR-rendered per request. Cheap operationally but loses the "ship as a static HTML file" benefit. Not recommended for what is intentionally a static help page. ## Critical files - `frontend/svelte.config.js` — add `kit.prerender.entries` ## Verification 1. `cd frontend && docker exec archive-frontend npm run build` (or run with the container user) — exit 0. 2. `.svelte-kit/output/prerendered/pages/hilfe/transkription.html` exists. 3. `npm run preview` and `curl -I http://localhost:4173/hilfe/transkription` returns the prerendered HTML (or, on Option B, returns 200 without auth). ## Acceptance criteria - [ ] `npm run build` exits 0. - [ ] `/hilfe/transkription` is prerendered to a static HTML file. - [ ] No regression on other prerendered routes (none currently — this is the only one). - [ ] Once green, add `npm run build` as a step in `.gitea/workflows/ci.yml` to prevent regression. ## Effort XS — 5 minutes for Option A. The CI step is a one-liner in the new devops(ci) issue. ## Risk if not addressed CI will fail the moment we add `npm run build` (or container build) to it. Production image build also fails until this is resolved. **P0 because it currently blocks the prod-image work in #135 and the new CI gates issue.** Tracked in audit doc as **F-35** (Medium → P0 via blocking dependency on #135 / CI gates).
marcel added the P0-criticalbug labels 2026-05-07 17:29:40 +02:00
Author
Owner

🏗️ Markus Keller — Application Architect

Observations

  • The root cause is a structural mismatch: prerender = true in +page.ts tells SvelteKit to ship this route as a static file, but the default kit.prerender.crawl can only discover routes reachable through followed links. The auth wall in hooks.server.ts intercepts the crawler before it reaches /hilfe/transkription, so the entry is never produced.
  • The issue correctly identifies three remediation vectors. Option A (explicit entries) is the right architectural answer for this case — it expresses intent in config without touching auth semantics.
  • The comment in +page.ts line 1 — // Safe: handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible — is actually misleading. Prerendered HTML is a static file served before any server hook runs. The redirect only fires when the Node adapter handles an SSR request, not when the CDN/Caddy serves the static file directly. This is worth correcting in the code comment, but does not change the recommended fix.
  • As the issue notes, /hilfe/transkription is currently the only prerendered route. The fix in svelte.config.js is XS scope.
  • The CI workflow (.gitea/workflows/ci.yml) currently has no npm run build step, which is why this hasn't surfaced as a build break. Adding it after this fix is the correct follow-through — and it should be a prerequisite gate before the frontend unit-test job, not a separate optional job.

Recommendations

  1. Implement Option A exactly as written in the issue. Add prerender: { entries: ['/hilfe/transkription'] } to kit in svelte.config.js. This is a one-line config change with no side-effects on auth or routing behavior.
  2. Fix the misleading comment in +page.ts. Replace the existing comment with something accurate: // Static prerendered page — no server-side auth runs against the static file. Access control for the live route is handled via hooks.server.ts for SSR fallback. Or simplest: remove the comment entirely since the prerender flag is self-documenting.
  3. Add npm run build to CI in the same PR. The issue suggests tracking the CI step in a separate devops issue, but the build gate is the whole point of this fix — ship both together. A passing build that doesn't gate CI provides no regression protection.
  4. Do not pursue Option B (making help public) in this PR. That is a product decision with UX implications (inviting external users to read the guide before registering). Open a separate issue if the team wants to evaluate it.

Open Decisions

  • Should the CI build step be part of this PR or a separate devops issue? The issue proposes separating it, but adding the CI step in the same PR eliminates the gap where the fix passes locally but regressions sneak back in. The tradeoff is PR scope vs. the principle of one logical change per commit. My recommendation is to include it here — the build check is integral to the fix, not an enhancement.
## 🏗️ Markus Keller — Application Architect ### Observations - The root cause is a structural mismatch: `prerender = true` in `+page.ts` tells SvelteKit to ship this route as a static file, but the default `kit.prerender.crawl` can only discover routes reachable through followed links. The auth wall in `hooks.server.ts` intercepts the crawler before it reaches `/hilfe/transkription`, so the entry is never produced. - The issue correctly identifies three remediation vectors. Option A (explicit `entries`) is the right architectural answer for this case — it expresses intent in config without touching auth semantics. - The comment in `+page.ts` line 1 — `// Safe: handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible` — is actually misleading. Prerendered HTML is a static file served before any server hook runs. The redirect only fires when the Node adapter handles an SSR request, not when the CDN/Caddy serves the static file directly. This is worth correcting in the code comment, but does not change the recommended fix. - As the issue notes, `/hilfe/transkription` is currently the only prerendered route. The fix in `svelte.config.js` is XS scope. - The CI workflow (`.gitea/workflows/ci.yml`) currently has no `npm run build` step, which is why this hasn't surfaced as a build break. Adding it after this fix is the correct follow-through — and it should be a prerequisite gate before the frontend unit-test job, not a separate optional job. ### Recommendations 1. **Implement Option A exactly as written in the issue.** Add `prerender: { entries: ['/hilfe/transkription'] }` to `kit` in `svelte.config.js`. This is a one-line config change with no side-effects on auth or routing behavior. 2. **Fix the misleading comment in `+page.ts`.** Replace the existing comment with something accurate: `// Static prerendered page — no server-side auth runs against the static file. Access control for the live route is handled via hooks.server.ts for SSR fallback.` Or simplest: remove the comment entirely since the prerender flag is self-documenting. 3. **Add `npm run build` to CI in the same PR.** The issue suggests tracking the CI step in a separate devops issue, but the build gate is the whole point of this fix — ship both together. A passing build that doesn't gate CI provides no regression protection. 4. **Do not pursue Option B (making help public) in this PR.** That is a product decision with UX implications (inviting external users to read the guide before registering). Open a separate issue if the team wants to evaluate it. ### Open Decisions - **Should the CI build step be part of this PR or a separate devops issue?** The issue proposes separating it, but adding the CI step in the same PR eliminates the gap where the fix passes locally but regressions sneak back in. The tradeoff is PR scope vs. the principle of one logical change per commit. My recommendation is to include it here — the build check is integral to the fix, not an enhancement.
Author
Owner

👨‍💻 Felix Brandt — Fullstack Developer

Observations

  • The fix surface is exactly three files: frontend/svelte.config.js (add prerender.entries), optionally +page.ts (fix the misleading comment), and .gitea/workflows/ci.yml (add the build step). No backend changes, no type regeneration, no Flyway migrations.
  • The existing +page.ts comment // Safe: handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible is factually wrong. Prerendered HTML is served as a static file — hooks.server.ts only runs for SSR requests, not for static file serving. The comment gives false confidence. It should be removed or replaced.
  • The existing test file page.svelte.spec.ts has good coverage of the rendered UI. It does not (and should not) test the prerender flag — that is a build-time concern, not a component concern.
  • There is no test for the build output itself. The only verification is npm run build exiting 0 — which is exactly what the CI gate provides.
  • The issue's verification step mentions checking for .svelte-kit/output/prerendered/pages/hilfe/transkription.html. This is a good sanity check during local development but not a test that can live in the suite.

Recommendations

  1. Make the svelte.config.js change in one commit, the CI step in a second. They are two logical changes: one fixes the bug, the other prevents regression. Atomic commits, one logical change each.
  2. Delete the misleading comment in +page.ts, do not replace it. The prerender = true export is self-documenting SvelteKit convention. A wrong comment is worse than no comment.
  3. Add a TDD note to the commit message: There is no "failing test first" to write here — the failing build output is the test. Document this in the PR description so the reviewer understands why TDD's red phase was the CI build run, not a new test file.
  4. Verify locally before pushing: Run cd frontend && npm run build and confirm .svelte-kit/output/prerendered/pages/hilfe/transkription.html exists. This takes 10 seconds and eliminates any doubt.
  5. Do not add export const ssr = false or any other flag to +page.ts. The route is already correctly configured — only svelte.config.js is missing the entries declaration.
## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - The fix surface is exactly three files: `frontend/svelte.config.js` (add `prerender.entries`), optionally `+page.ts` (fix the misleading comment), and `.gitea/workflows/ci.yml` (add the build step). No backend changes, no type regeneration, no Flyway migrations. - The existing `+page.ts` comment `// Safe: handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible` is factually wrong. Prerendered HTML is served as a static file — `hooks.server.ts` only runs for SSR requests, not for static file serving. The comment gives false confidence. It should be removed or replaced. - The existing test file `page.svelte.spec.ts` has good coverage of the rendered UI. It does not (and should not) test the prerender flag — that is a build-time concern, not a component concern. - There is no test for the build output itself. The only verification is `npm run build` exiting 0 — which is exactly what the CI gate provides. - The issue's verification step mentions checking for `.svelte-kit/output/prerendered/pages/hilfe/transkription.html`. This is a good sanity check during local development but not a test that can live in the suite. ### Recommendations 1. **Make the `svelte.config.js` change in one commit, the CI step in a second.** They are two logical changes: one fixes the bug, the other prevents regression. Atomic commits, one logical change each. 2. **Delete the misleading comment in `+page.ts`, do not replace it.** The `prerender = true` export is self-documenting SvelteKit convention. A wrong comment is worse than no comment. 3. **Add a TDD note to the commit message:** There is no "failing test first" to write here — the failing build output is the test. Document this in the PR description so the reviewer understands why TDD's red phase was the CI build run, not a new test file. 4. **Verify locally before pushing:** Run `cd frontend && npm run build` and confirm `.svelte-kit/output/prerendered/pages/hilfe/transkription.html` exists. This takes 10 seconds and eliminates any doubt. 5. **Do not add `export const ssr = false` or any other flag to `+page.ts`.** The route is already correctly configured — only `svelte.config.js` is missing the `entries` declaration.
Author
Owner

🚀 Tobias Wendt — DevOps & Platform Engineer

Observations

  • The CI workflow in .gitea/workflows/ci.yml currently runs three jobs: unit-tests (frontend Vitest + Playwright container), ocr-tests (Python), and backend-unit-tests (Maven). There is no npm run build step anywhere. The issue is correct: this gap has been masked because the build is never exercised in CI.
  • The frontend job already has the correct actions/cache@v4 setup for node_modules keyed to package-lock.json. Adding a build step to the existing unit-tests job is a one-line addition after the test run — no new job needed for XS scope.
  • The issue mentions running npm run build inside the running archive-frontend container in pre-prod. That's an auditing approach, not a CI approach. For CI, the build runs in the Playwright container that already installs dependencies.
  • The adapter-node is already configured — so npm run build produces a Node.js server bundle, not a static site. The prerendered routes are embedded in .svelte-kit/output/prerendered/ and bundled into the final artifact. The build failing non-zero is a reliable CI gate.
  • Production container builds (docker build for archive-frontend) would also fail until this is fixed, since the Dockerfile presumably runs npm run build inside the container image. Blocking #135 (container-images issue) is real.

Recommendations

  1. Add npm run build to the existing unit-tests job after the test run. Do not create a separate CI job — the dependency graph is already set up (same runner, same cached node_modules), and a separate job adds queue time with no benefit at this scale.

    - name: Run unit and component tests
      run: npm test
      working-directory: frontend
    
    - name: Build frontend
      run: npm run build
      working-directory: frontend
    
  2. Do not add a build artifact upload step. The build output is transient — its only purpose here is to verify the build exits 0 and prerendering completes. Uploading the artifact adds storage cost and CI time with no downstream consumer (the actual container image build uses a separate Dockerfile).

  3. Paraglide compilation must precede the build step, and it already does in the current workflow (npx @inlang/paraglide-js compile runs before tests). The build step fits naturally after the test step without reordering.

  4. Track this as a prerequisite for #135 — add a blocked-by: #472 link to the container-images issue so the dependency is visible in the backlog.

  5. After merging, verify the Gitea runner picks up the new step cleanly. The NAS runner runs Docker 24.x (the workflow already has DOCKER_API_VERSION: "1.43" workaround in the backend job — the frontend job uses a different container image and does not need this).

## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Observations - The CI workflow in `.gitea/workflows/ci.yml` currently runs three jobs: `unit-tests` (frontend Vitest + Playwright container), `ocr-tests` (Python), and `backend-unit-tests` (Maven). There is no `npm run build` step anywhere. The issue is correct: this gap has been masked because the build is never exercised in CI. - The frontend job already has the correct `actions/cache@v4` setup for `node_modules` keyed to `package-lock.json`. Adding a build step to the existing `unit-tests` job is a one-line addition after the test run — no new job needed for XS scope. - The issue mentions running `npm run build` inside the running `archive-frontend` container in pre-prod. That's an auditing approach, not a CI approach. For CI, the build runs in the Playwright container that already installs dependencies. - The `adapter-node` is already configured — so `npm run build` produces a Node.js server bundle, not a static site. The prerendered routes are embedded in `.svelte-kit/output/prerendered/` and bundled into the final artifact. The build failing non-zero is a reliable CI gate. - Production container builds (`docker build` for `archive-frontend`) would also fail until this is fixed, since the Dockerfile presumably runs `npm run build` inside the container image. Blocking `#135` (container-images issue) is real. ### Recommendations 1. **Add `npm run build` to the existing `unit-tests` job after the test run.** Do not create a separate CI job — the dependency graph is already set up (same runner, same cached `node_modules`), and a separate job adds queue time with no benefit at this scale. ```yaml - name: Run unit and component tests run: npm test working-directory: frontend - name: Build frontend run: npm run build working-directory: frontend ``` 2. **Do not add a build artifact upload step.** The build output is transient — its only purpose here is to verify the build exits 0 and prerendering completes. Uploading the artifact adds storage cost and CI time with no downstream consumer (the actual container image build uses a separate Dockerfile). 3. **Paraglide compilation must precede the build step**, and it already does in the current workflow (`npx @inlang/paraglide-js compile` runs before tests). The build step fits naturally after the test step without reordering. 4. **Track this as a prerequisite for `#135`** — add a `blocked-by: #472` link to the container-images issue so the dependency is visible in the backlog. 5. **After merging, verify the Gitea runner picks up the new step cleanly.** The NAS runner runs Docker 24.x (the workflow already has `DOCKER_API_VERSION: "1.43"` workaround in the backend job — the frontend job uses a different container image and does not need this).
Author
Owner

📋 Elicit — Requirements Engineer

Observations

  • The issue is well-specified for its scope. The problem, root cause, three options with pros/cons, affected files, verification steps, and acceptance criteria are all present. This is a green-light Definition of Ready for a P0 bugfix.
  • One hidden assumption needs surfacing: The +page.ts comment says // Safe: handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible. This implies a product decision: the transcription help page is currently intended to be auth-gated. If prerendered HTML is served as a static file (which it is with adapter-node + a Caddy proxy serving static assets directly), an unauthenticated user who knows the URL may be able to fetch the file depending on how Caddy is configured. The issue does not surface this as a risk.
  • Option B (making help public) is not purely scope creep. The project memory notes a specific user story: "inviting mom to transcribe." A user considering whether to register should be able to read the transcription guidelines without logging in. This is a real product question, not a technical one, and it deserves a separate issue rather than being dismissed in two lines.
  • The acceptance criteria are testable and complete for Option A. The criterion "No regression on other prerendered routes (none currently — this is the only one)" is accurate based on my inspection — grep -r "prerender = true" returns only frontend/src/routes/hilfe/transkription/+page.ts.
  • The effort estimate (XS, 5 minutes for Option A) is accurate for the config change. The CI step addition is another XS. Total: XS+.

Recommendations

  1. Accept the issue as-is and implement Option A immediately. All information needed is present. No further elicitation required for the fix.
  2. Create a follow-up issue for the public help question. User story: "As a prospective family member who has been invited to join, I want to read the transcription guide without logging in, so I can decide whether I want to participate before creating an account." This is a Should/Could-priority product decision with a clear JTBD. It should not be bundled into a P0 bugfix.
  3. Document the prerendering model in one sentence in the issue or PR. When adapter-node is used and Caddy serves .svelte-kit/output/prerendered/ as static files, server hooks do not run against those files. This is the root cause and it should be stated explicitly so the next developer understands why prerender.entries is needed even when the page is behind auth in the app.

Open Decisions

  • Is /hilfe/transkription intended to be accessible without authentication in production? If Caddy serves the prerendered static file directly (bypassing the Node server), it is currently publicly accessible regardless of the auth hook. This is a product decision: intentional (good UX) or unintentional (should be auth-gated at Caddy level)? Needs an explicit answer before shipping.
## 📋 Elicit — Requirements Engineer ### Observations - The issue is well-specified for its scope. The problem, root cause, three options with pros/cons, affected files, verification steps, and acceptance criteria are all present. This is a green-light Definition of Ready for a P0 bugfix. - **One hidden assumption needs surfacing:** The `+page.ts` comment says `// Safe: handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible`. This implies a product decision: the transcription help page is currently intended to be auth-gated. If prerendered HTML is served as a static file (which it is with `adapter-node` + a Caddy proxy serving static assets directly), an unauthenticated user who knows the URL may be able to fetch the file depending on how Caddy is configured. The issue does not surface this as a risk. - **Option B (making help public) is not purely scope creep.** The project memory notes a specific user story: "inviting mom to transcribe." A user considering whether to register should be able to read the transcription guidelines without logging in. This is a real product question, not a technical one, and it deserves a separate issue rather than being dismissed in two lines. - The acceptance criteria are testable and complete for Option A. The criterion "No regression on other prerendered routes (none currently — this is the only one)" is accurate based on my inspection — `grep -r "prerender = true"` returns only `frontend/src/routes/hilfe/transkription/+page.ts`. - The effort estimate (XS, 5 minutes for Option A) is accurate for the config change. The CI step addition is another XS. Total: XS+. ### Recommendations 1. **Accept the issue as-is and implement Option A immediately.** All information needed is present. No further elicitation required for the fix. 2. **Create a follow-up issue for the public help question.** User story: *"As a prospective family member who has been invited to join, I want to read the transcription guide without logging in, so I can decide whether I want to participate before creating an account."* This is a Should/Could-priority product decision with a clear JTBD. It should not be bundled into a P0 bugfix. 3. **Document the prerendering model in one sentence in the issue or PR.** When `adapter-node` is used and Caddy serves `.svelte-kit/output/prerendered/` as static files, server hooks do not run against those files. This is the root cause and it should be stated explicitly so the next developer understands why `prerender.entries` is needed even when the page is behind auth in the app. ### Open Decisions - **Is `/hilfe/transkription` intended to be accessible without authentication in production?** If Caddy serves the prerendered static file directly (bypassing the Node server), it is currently publicly accessible regardless of the auth hook. This is a product decision: intentional (good UX) or unintentional (should be auth-gated at Caddy level)? Needs an explicit answer before shipping.
Author
Owner

🔒 Nora "NullX" Steiner — Security Engineer

Observations

  • The fix itself (Option A) introduces no new security surface. Adding a route to prerender.entries only affects the build process, not runtime auth behavior.
  • The existing comment in +page.ts is a security documentation failure. It asserts handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible — but this is only true when the Node.js server handles the request. If Caddy (the reverse proxy) is configured to serve .svelte-kit/output/prerendered/ as static files directly (which is the production pattern described in the DevOps persona's canonical stack), then hooks.server.ts never runs and the page is publicly accessible regardless of the auth gate. This is not inherently a vulnerability — the page contains only transcription guidelines, no personal data — but the comment creates a false belief about the security model that could be cargo-culted to a future route containing sensitive content.
  • No IDOR, injection, or auth bypass introduced by this fix. The change is purely config-level.
  • The PUBLIC_PATHS list in hooks.server.ts correctly excludes /hilfe/transkription. This means SSR requests (when the static file is not cached/served by Caddy) will be redirected to login. The behavior is consistent with the existing auth model for Option A.
  • If Option B were chosen (adding /hilfe to PUBLIC_PATHS), that would be a deliberate auth relaxation. It is a minor security posture change with appropriate product justification, but it should be documented explicitly — not buried in a fix PR.

Recommendations

  1. Fix the misleading comment in +page.ts. Replace it with: // Prerendered to a static HTML file — the Node.js auth hook does NOT run when this file is served directly by the reverse proxy. This page intentionally contains no sensitive data. This documents the actual security model and sets a safe precedent for future prerendered routes.
  2. Add a note to the Caddy configuration (or the production deployment docs) that clarifies whether prerendered files are served statically or proxied through the Node server. If they go through the Node server, the auth hook applies. If they're served from the static output directory directly, they bypass it. This distinction matters for any future route marked prerender = true.
  3. Do not add /hilfe/transkription to PUBLIC_PATHS in this fix. Option A is safer because it does not touch the auth configuration — the behavioral change to the SSR path is zero.
  4. No security regression tests needed for this specific fix — the page has no auth-sensitive content and the change is config-only. If the team later moves a sensitive route to prerendered, that would warrant an E2E auth test at that time.
## 🔒 Nora "NullX" Steiner — Security Engineer ### Observations - The fix itself (Option A) introduces no new security surface. Adding a route to `prerender.entries` only affects the build process, not runtime auth behavior. - **The existing comment in `+page.ts` is a security documentation failure.** It asserts `handleAuth in hooks.server.ts redirects unauthenticated requests before prerendered HTML is visible` — but this is only true when the Node.js server handles the request. If Caddy (the reverse proxy) is configured to serve `.svelte-kit/output/prerendered/` as static files directly (which is the production pattern described in the DevOps persona's canonical stack), then `hooks.server.ts` never runs and the page is publicly accessible regardless of the auth gate. This is not inherently a vulnerability — the page contains only transcription guidelines, no personal data — but the comment creates a false belief about the security model that could be cargo-culted to a future route containing sensitive content. - **No IDOR, injection, or auth bypass introduced by this fix.** The change is purely config-level. - The `PUBLIC_PATHS` list in `hooks.server.ts` correctly excludes `/hilfe/transkription`. This means SSR requests (when the static file is not cached/served by Caddy) will be redirected to login. The behavior is consistent with the existing auth model for Option A. - If Option B were chosen (adding `/hilfe` to `PUBLIC_PATHS`), that would be a deliberate auth relaxation. It is a minor security posture change with appropriate product justification, but it should be documented explicitly — not buried in a fix PR. ### Recommendations 1. **Fix the misleading comment in `+page.ts`.** Replace it with: `// Prerendered to a static HTML file — the Node.js auth hook does NOT run when this file is served directly by the reverse proxy. This page intentionally contains no sensitive data.` This documents the actual security model and sets a safe precedent for future prerendered routes. 2. **Add a note to the Caddy configuration (or the production deployment docs) that clarifies whether prerendered files are served statically or proxied through the Node server.** If they go through the Node server, the auth hook applies. If they're served from the static output directory directly, they bypass it. This distinction matters for any future route marked `prerender = true`. 3. **Do not add `/hilfe/transkription` to `PUBLIC_PATHS` in this fix.** Option A is safer because it does not touch the auth configuration — the behavioral change to the SSR path is zero. 4. **No security regression tests needed** for this specific fix — the page has no auth-sensitive content and the change is config-only. If the team later moves a sensitive route to prerendered, that would warrant an E2E auth test at that time.
Author
Owner

🧪 Sara Holt — QA Engineer

Observations

  • The existing test file frontend/src/routes/hilfe/transkription/page.svelte.spec.ts has solid component-level coverage: 8 tests covering heading, intro paragraph, external link security attributes (including rel, referrerpolicy, and aria-label), section headings, rule cards, and clarification chips. These tests pass today and will continue to pass after the fix — the fix does not change rendered output.
  • There is no test that verifies the prerender output exists (i.e., that the build produces .svelte-kit/output/prerendered/pages/hilfe/transkription.html). This is correct — that is a CI gate responsibility, not a unit test responsibility.
  • The CI workflow currently has no npm run build step. This is the missing quality gate. Once added, any future prerender = true route that becomes unreachable will fail the build in CI, not just locally.
  • The test suite does not test the hooks.server.ts PUBLIC_PATHS list. There is no automated test that verifies /hilfe/transkription redirects to /login for unauthenticated users, or that it does not. This is a gap worth noting for the future — not a blocker for this fix, but relevant if Option B is ever pursued.
  • The verification steps in the issue are correct and complete for manual validation. They map to:
    1. Build exits 0 (CI gate)
    2. Static file exists in output (local verification)
    3. curl returns 200 (local or E2E)

Recommendations

  1. The fix requires no new tests beyond what already exists. The component tests cover rendered output. The CI build step is the regression gate for prerender correctness. Do not add a test that checks for a file on disk — that couples tests to build artifacts.
  2. Add one Playwright E2E test after the fix is shipped: test('hilfe/transkription loads without error for authenticated user', async ({ page }) => { await page.goto('/hilfe/transkription'); await expect(page.getByRole('heading', { level: 1 })).toBeVisible(); }). This is a lightweight smoke test that catches broken prerendered output in the full stack.
  3. Track the missing auth E2E test as a future follow-up: A test asserting GET /hilfe/transkription returns 200 for authenticated users and 302 to /login for unauthenticated users (via the SSR path) would complete the security posture verification. It can live in the E2E suite. Low priority for this P0 fix, but worth a separate issue.
  4. Verify the existing 8 component tests still pass after the fix. They should — no component code changes — but run npm test locally before pushing to confirm the CI will be clean.
## 🧪 Sara Holt — QA Engineer ### Observations - The existing test file `frontend/src/routes/hilfe/transkription/page.svelte.spec.ts` has solid component-level coverage: 8 tests covering heading, intro paragraph, external link security attributes (including `rel`, `referrerpolicy`, and `aria-label`), section headings, rule cards, and clarification chips. These tests pass today and will continue to pass after the fix — the fix does not change rendered output. - There is no test that verifies the prerender output exists (i.e., that the build produces `.svelte-kit/output/prerendered/pages/hilfe/transkription.html`). This is correct — that is a CI gate responsibility, not a unit test responsibility. - The CI workflow currently has no `npm run build` step. This is the missing quality gate. Once added, any future `prerender = true` route that becomes unreachable will fail the build in CI, not just locally. - The test suite does not test the `hooks.server.ts` `PUBLIC_PATHS` list. There is no automated test that verifies `/hilfe/transkription` redirects to `/login` for unauthenticated users, or that it does not. This is a gap worth noting for the future — not a blocker for this fix, but relevant if Option B is ever pursued. - The verification steps in the issue are correct and complete for manual validation. They map to: 1. Build exits 0 (CI gate) 2. Static file exists in output (local verification) 3. `curl` returns 200 (local or E2E) ### Recommendations 1. **The fix requires no new tests beyond what already exists.** The component tests cover rendered output. The CI build step is the regression gate for prerender correctness. Do not add a test that checks for a file on disk — that couples tests to build artifacts. 2. **Add one Playwright E2E test after the fix is shipped:** `test('hilfe/transkription loads without error for authenticated user', async ({ page }) => { await page.goto('/hilfe/transkription'); await expect(page.getByRole('heading', { level: 1 })).toBeVisible(); })`. This is a lightweight smoke test that catches broken prerendered output in the full stack. 3. **Track the missing auth E2E test as a future follow-up:** A test asserting `GET /hilfe/transkription` returns 200 for authenticated users and 302 to `/login` for unauthenticated users (via the SSR path) would complete the security posture verification. It can live in the E2E suite. Low priority for this P0 fix, but worth a separate issue. 4. **Verify the existing 8 component tests still pass after the fix.** They should — no component code changes — but run `npm test` locally before pushing to confirm the CI will be clean.
Author
Owner

🎨 Leonie Voss — UI/UX Design Lead

Observations

  • This is a build infrastructure fix. There are no UI changes, no layout modifications, and no component code touched. The rendered /hilfe/transkription page is not changing.
  • The prerendered help page is an interesting UX asset: it is a static HTML file, meaning it loads without JavaScript and without a backend round-trip. For the senior audience (60+) on slower connections, this is a genuine performance and reliability benefit — the page will still render even if the API is slow to respond.
  • Option B (making help public) has real UX value for the "invite mom to transcribe" user journey. A prospective contributor should be able to read the guidelines before deciding to register. This is currently blocked by the auth gate on every non-public path. The issue correctly defers this to a follow-up.
  • The current transcription help page has existing test coverage verifying that the Wikipedia external link has rel="noopener noreferrer", referrerpolicy="no-referrer", and an aria-label containing "öffnet in neuem Tab" — these are correct accessibility and security practices that are already in place.

Recommendations

  1. No UI changes are needed or appropriate for this fix. Implement Option A and move on.
  2. Open a follow-up UX issue for the public help page. The spec at docs/specs/transkriptions-richtlinien-spec.html likely has the design intent documented — check it to see if public access was ever part of the design. If so, add /hilfe to PUBLIC_PATHS in a separate PR with explicit product sign-off.
  3. If the team decides to make help public (Option B follow-up), consider adding a "Log in to start transcribing" CTA at the bottom of the help page — a call-to-action that converts readers into registered contributors. This is a one-component addition that fits naturally after the rule cards.
  4. Do not ship any visual regression snapshots in this fix PR. The page output is unchanged, so existing snapshots (if any) remain valid.
## 🎨 Leonie Voss — UI/UX Design Lead ### Observations - This is a build infrastructure fix. There are no UI changes, no layout modifications, and no component code touched. The rendered `/hilfe/transkription` page is not changing. - The prerendered help page is an interesting UX asset: it is a static HTML file, meaning it loads without JavaScript and without a backend round-trip. For the senior audience (60+) on slower connections, this is a genuine performance and reliability benefit — the page will still render even if the API is slow to respond. - Option B (making help public) has real UX value for the "invite mom to transcribe" user journey. A prospective contributor should be able to read the guidelines before deciding to register. This is currently blocked by the auth gate on every non-public path. The issue correctly defers this to a follow-up. - The current transcription help page has existing test coverage verifying that the Wikipedia external link has `rel="noopener noreferrer"`, `referrerpolicy="no-referrer"`, and an `aria-label` containing "öffnet in neuem Tab" — these are correct accessibility and security practices that are already in place. ### Recommendations 1. **No UI changes are needed or appropriate for this fix.** Implement Option A and move on. 2. **Open a follow-up UX issue for the public help page.** The spec at `docs/specs/transkriptions-richtlinien-spec.html` likely has the design intent documented — check it to see if public access was ever part of the design. If so, add `/hilfe` to `PUBLIC_PATHS` in a separate PR with explicit product sign-off. 3. **If the team decides to make help public (Option B follow-up), consider adding a "Log in to start transcribing" CTA** at the bottom of the help page — a call-to-action that converts readers into registered contributors. This is a one-component addition that fits naturally after the rule cards. 4. **Do not ship any visual regression snapshots in this fix PR.** The page output is unchanged, so existing snapshots (if any) remain valid.
Author
Owner

Decision Queue

Two open decisions were raised across the review. Both are genuine tradeoffs requiring human judgment.


Theme 1 — CI build step scope (raised by Markus / @mkeller)

Question: Should npm run build be added to .gitea/workflows/ci.yml in this PR, or tracked in the separate devops(ci) issue?

Why it matters: Without the CI gate, the config fix passes locally but provides no regression protection. A future prerender = true route that becomes unreachable behind auth would fail silently again until someone runs the build manually.

Options:

  • Include in this PR — the build gate is the point of the fix; shipping without it leaves the same gap open. Slightly wider PR scope.
  • Separate devops issue — preserves atomic commits; relies on the CI issue being picked up promptly before any new prerendered routes are added.

Recommendation from review: Include it. The CI step is integral to the fix, not an enhancement.


Theme 2 — Public accessibility of /hilfe/transkription in production (raised by Elicit and Nora)

Question: Is the transcription help page intended to be accessible without authentication in production?

Why it matters: With adapter-node and Caddy configured to serve .svelte-kit/output/prerendered/ as static files directly, hooks.server.ts does not run against those files — the page is publicly reachable regardless of PUBLIC_PATHS. The existing comment in +page.ts asserts the opposite. This is either:

  • (a) Intentionally public — good UX for the "invite mom to transcribe" onboarding journey, no sensitive data on the page.
  • (b) Unintentionally public — should be auth-gated at the Caddy level if private access is required.

This does not block the P0 fix (Option A is safe either way). But the answer determines whether:

  1. The misleading comment in +page.ts should say "intentionally public" or "should be gated at proxy level."
  2. A follow-up issue for public help (Option B) has product sign-off or is explicitly parked.

Recommendation from review: Decide and document the intent before or alongside this PR. The comment fix is cheap; the architectural clarity is valuable.

## Decision Queue Two open decisions were raised across the review. Both are genuine tradeoffs requiring human judgment. --- ### Theme 1 — CI build step scope (raised by Markus / @mkeller) **Question:** Should `npm run build` be added to `.gitea/workflows/ci.yml` in this PR, or tracked in the separate devops(ci) issue? **Why it matters:** Without the CI gate, the config fix passes locally but provides no regression protection. A future `prerender = true` route that becomes unreachable behind auth would fail silently again until someone runs the build manually. **Options:** - **Include in this PR** — the build gate is the point of the fix; shipping without it leaves the same gap open. Slightly wider PR scope. - **Separate devops issue** — preserves atomic commits; relies on the CI issue being picked up promptly before any new prerendered routes are added. **Recommendation from review:** Include it. The CI step is integral to the fix, not an enhancement. --- ### Theme 2 — Public accessibility of `/hilfe/transkription` in production (raised by Elicit and Nora) **Question:** Is the transcription help page intended to be accessible without authentication in production? **Why it matters:** With `adapter-node` and Caddy configured to serve `.svelte-kit/output/prerendered/` as static files directly, `hooks.server.ts` does not run against those files — the page is publicly reachable regardless of `PUBLIC_PATHS`. The existing comment in `+page.ts` asserts the opposite. This is either: - (a) Intentionally public — good UX for the "invite mom to transcribe" onboarding journey, no sensitive data on the page. - (b) Unintentionally public — should be auth-gated at the Caddy level if private access is required. **This does not block the P0 fix (Option A is safe either way).** But the answer determines whether: 1. The misleading comment in `+page.ts` should say "intentionally public" or "should be gated at proxy level." 2. A follow-up issue for public help (Option B) has product sign-off or is explicitly parked. **Recommendation from review:** Decide and document the intent before or alongside this PR. The comment fix is cheap; the architectural clarity is valuable.
Author
Owner

Implemented — PR #485

Implemented Option A as specified. Three changes across two commits:

Commit 1 — fix(build): add prerender entry for /hilfe/transkription

  • frontend/svelte.config.js: Added prerender: { entries: ['/hilfe/transkription'] } to kit config — tells SvelteKit to render the route directly without needing to crawl to it through links
  • frontend/src/routes/hilfe/transkription/+page.ts: Removed the misleading comment that claimed hooks.server.ts guards prerendered static files (it does not — the auth hook only runs for SSR requests)

Commit 2 — ci: add npm run build step to unit-tests job

  • .gitea/workflows/ci.yml: Added npm run build after the test step in the unit-tests job — the build gate is the regression protection for this fix

All 1835 frontend unit tests pass. CI will validate the actual build exit code on the PR run.

## ✅ Implemented — PR #485 Implemented Option A as specified. Three changes across two commits: **Commit 1 — `fix(build): add prerender entry for /hilfe/transkription`** - `frontend/svelte.config.js`: Added `prerender: { entries: ['/hilfe/transkription'] }` to `kit` config — tells SvelteKit to render the route directly without needing to crawl to it through links - `frontend/src/routes/hilfe/transkription/+page.ts`: Removed the misleading comment that claimed `hooks.server.ts` guards prerendered static files (it does not — the auth hook only runs for SSR requests) **Commit 2 — `ci: add npm run build step to unit-tests job`** - `.gitea/workflows/ci.yml`: Added `npm run build` after the test step in the `unit-tests` job — the build gate is the regression protection for this fix All 1835 frontend unit tests pass. CI will validate the actual build exit code on the PR run.
Sign in to join this conversation.
No Label P0-critical bug
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#472