Set up Docker deployment with Caddy reverse proxy #14

New Issue

marcel · 2026-05-05T10:58:27+02:00

marcel commented

2026-05-05 10:58:27 +02:00

Task 14 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md`

As a developer/admin, I need the app packaged as a single Docker container that persists the SQLite DB and uploads across restarts, with a Caddy reverse-proxy for TLS on the root server.

Acceptance criteria

Dockerfile uses multi-stage build: node:22-alpine builder → node:22-alpine runner
sharp native bindings work in Alpine (vips-dev installed)
App binds on port 3000, CMD ["node", "build"]
docker-compose.yml mounts named volumes for db/ and uploads/
All secrets passed via environment variables (not baked into image):
- DATABASE_PATH=/app/db/erbstuecke.db
- UPLOAD_DIR=/app/uploads
- ADMIN_MARCEL_PASSWORD_HASH, ADMIN_RENATE_PASSWORD_HASH, ADMIN_BERIT_PASSWORD_HASH
.dockerignore excludes node_modules, .svelte-kit, build, *.db, uploads/
docker compose build && docker compose up runs and Gate Screen appears at http://localhost:3000
Full flow tested in Docker: create code → add article with photo → family login → reserve

Bcrypt hash generation (run locally before deployment)

node -e "const b = require('bcryptjs'); b.hash('PASSWORD', 12).then(h => console.log(h))"

Caddy config on host (not in repo)

erbstuecke.example.com {
    reverse_proxy localhost:3000
}

Files to create

Dockerfile
docker-compose.yml
.dockerignore

Depends on: all previous tasks | Size: S | Spec: system-design §2

## Task 14 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md` **As a developer/admin**, I need the app packaged as a single Docker container that persists the SQLite DB and uploads across restarts, with a Caddy reverse-proxy for TLS on the root server. ### Acceptance criteria - [ ] `Dockerfile` uses multi-stage build: `node:22-alpine` builder → `node:22-alpine` runner - [ ] `sharp` native bindings work in Alpine (`vips-dev` installed) - [ ] App binds on port 3000, `CMD ["node", "build"]` - [ ] `docker-compose.yml` mounts named volumes for `db/` and `uploads/` - [ ] All secrets passed via environment variables (not baked into image): - `DATABASE_PATH=/app/db/erbstuecke.db` - `UPLOAD_DIR=/app/uploads` - `ADMIN_MARCEL_PASSWORD_HASH`, `ADMIN_RENATE_PASSWORD_HASH`, `ADMIN_BERIT_PASSWORD_HASH` - [ ] `.dockerignore` excludes `node_modules`, `.svelte-kit`, `build`, `*.db`, `uploads/` - [ ] `docker compose build && docker compose up` runs and Gate Screen appears at `http://localhost:3000` - [ ] Full flow tested in Docker: create code → add article with photo → family login → reserve ### Bcrypt hash generation (run locally before deployment) ```bash node -e "const b = require('bcryptjs'); b.hash('PASSWORD', 12).then(h => console.log(h))" ``` ### Caddy config on host (not in repo) ``` erbstuecke.example.com { reverse_proxy localhost:3000 } ``` ### Files to create - `Dockerfile` - `docker-compose.yml` - `.dockerignore` **Depends on:** all previous tasks | **Size:** S | **Spec:** system-design §2

marcel added this to the v1.0 — MVP milestone 2026-05-05 10:58:27 +02:00

marcel commented

2026-05-05 11:26:55 +02:00

👤 Tobias Wendt — DevOps & Platform Engineer

Observations

The issue spec (§2) shows a docker-compose.yml without expose vs ports distinction — the compose snippet in the system design doesn't clarify this. Using ports: ["3000:3000"] would expose port 3000 directly to the internet, bypassing Caddy and all its security headers.
The issue acceptance criteria list SESSION_SECRET in the description of environment variables required but omit it from the explicit env-var bullet list. If the implementer follows that list literally, SESSION_SECRET will be missing from the compose file.
The issue says sharp requires vips-dev to be installed in Alpine — correct, but the multi-stage Dockerfile must install vips-dev in the builder stage only. The runner stage needs vips (the runtime shared library, not the dev headers) or libvips depending on Alpine version. Missing this distinction produces a working builder but a broken runtime image.
The acceptance criteria say CMD ["node", "build"] but do not mention USER node. Running as root inside a container is a known risk — the Tobias persona's standards require USER node in the runner stage.
No .env.example is listed in "Files to create." Without it, the first deployer will stare at the compose file and guess at variable names.
No health check is specified in the acceptance criteria. Caddy will route traffic to the container before Node finishes initializing if there is no health check.
The deploy procedure and backup strategy are completely absent from the issue — fine for a "Files to create" scope, but the issue should note that a backup cron and restore test are required before launch.

Recommendations

Add expose: ["3000"] (not ports) to the docker-compose.yml. Caddy on the host connects to localhost:3000. The port must never be reachable from the internet.
Add SESSION_SECRET to the explicit environment variable list in the acceptance criteria — it is required by the app and must come from .env.

Structure the Dockerfile as follows to handle sharp/vips on Alpine correctly:

FROM node:22-alpine AS builder
RUN apk add --no-cache vips-dev python3 make g++
WORKDIR /app
COPY package*.json ./
RUN npm ci
COPY . .
RUN npm run build

FROM node:22-alpine AS runner
RUN apk add --no-cache vips   # runtime library only — no headers
WORKDIR /app
ENV NODE_ENV=production
COPY --from=builder /app/build ./build
COPY --from=builder /app/package.json ./
RUN npm ci --omit=dev
USER node
EXPOSE 3000
CMD ["node", "build"]

Add a health check to docker-compose.yml:

healthcheck:
  test: ["CMD-SHELL", "wget -qO- http://localhost:3000/health || exit 1"]
  interval: 30s
  timeout: 5s
  retries: 3
  start_period: 15s

This requires a /health endpoint in SvelteKit returning 200 OK.

Add .env.example to "Files to create" — it documents every required variable and is the only onboarding artifact a new deployer needs.
Add restart: unless-stopped explicitly to the acceptance criteria (it is in the system design spec but not checked in the issue ACs).
Note in the issue that a backup restore test is required before the project goes live (it is not in scope for this task but must be tracked somewhere).

Open Decisions (omit if none)

Health endpoint scope — Should /health be implemented as part of this Docker task (Task 14) or tracked as a separate issue? If it is not implemented, the health check will always fail and the container will never be marked healthy. Options: (A) implement /health in this task, (B) disable the health check until a later task adds it. Cost of A: small extra scope. Cost of B: Caddy routes to an uninitialized container on first deploy.

## 👤 Tobias Wendt — DevOps & Platform Engineer ### Observations - The issue spec (§2) shows a `docker-compose.yml` without `expose` vs `ports` distinction — the compose snippet in the system design doesn't clarify this. Using `ports: ["3000:3000"]` would expose port 3000 directly to the internet, bypassing Caddy and all its security headers. - The issue acceptance criteria list `SESSION_SECRET` in the description of environment variables required but omit it from the explicit env-var bullet list. If the implementer follows that list literally, `SESSION_SECRET` will be missing from the compose file. - The issue says `sharp` requires `vips-dev` to be installed in Alpine — correct, but the multi-stage Dockerfile must install `vips-dev` in the **builder stage** only. The runner stage needs `vips` (the runtime shared library, not the dev headers) or `libvips` depending on Alpine version. Missing this distinction produces a working builder but a broken runtime image. - The acceptance criteria say `CMD ["node", "build"]` but do not mention `USER node`. Running as root inside a container is a known risk — the Tobias persona's standards require `USER node` in the runner stage. - No `.env.example` is listed in "Files to create." Without it, the first deployer will stare at the compose file and guess at variable names. - No health check is specified in the acceptance criteria. Caddy will route traffic to the container before Node finishes initializing if there is no health check. - The deploy procedure and backup strategy are completely absent from the issue — fine for a "Files to create" scope, but the issue should note that a backup cron and restore test are required before launch. ### Recommendations - Add `expose: ["3000"]` (not `ports`) to the docker-compose.yml. Caddy on the host connects to `localhost:3000`. The port must never be reachable from the internet. - Add `SESSION_SECRET` to the explicit environment variable list in the acceptance criteria — it is required by the app and must come from `.env`. - Structure the Dockerfile as follows to handle `sharp`/`vips` on Alpine correctly: ```dockerfile FROM node:22-alpine AS builder RUN apk add --no-cache vips-dev python3 make g++ WORKDIR /app COPY package*.json ./ RUN npm ci COPY . . RUN npm run build FROM node:22-alpine AS runner RUN apk add --no-cache vips # runtime library only — no headers WORKDIR /app ENV NODE_ENV=production COPY --from=builder /app/build ./build COPY --from=builder /app/package.json ./ RUN npm ci --omit=dev USER node EXPOSE 3000 CMD ["node", "build"] ``` - Add a health check to docker-compose.yml: ```yaml healthcheck: test: ["CMD-SHELL", "wget -qO- http://localhost:3000/health || exit 1"] interval: 30s timeout: 5s retries: 3 start_period: 15s ``` This requires a `/health` endpoint in SvelteKit returning 200 OK. - Add `.env.example` to "Files to create" — it documents every required variable and is the only onboarding artifact a new deployer needs. - Add `restart: unless-stopped` explicitly to the acceptance criteria (it is in the system design spec but not checked in the issue ACs). - Note in the issue that a backup restore test is required before the project goes live (it is not in scope for this task but must be tracked somewhere). ### Open Decisions _(omit if none)_ - **Health endpoint scope** — Should `/health` be implemented as part of this Docker task (Task 14) or tracked as a separate issue? If it is not implemented, the health check will always fail and the container will never be marked healthy. Options: (A) implement `/health` in this task, (B) disable the health check until a later task adds it. Cost of A: small extra scope. Cost of B: Caddy routes to an uninitialized container on first deploy.

marcel commented

2026-05-05 11:27:14 +02:00

👤 Felix Brandt — Fullstack Developer

Observations

The issue is infrastructure-only (Dockerfile, docker-compose.yml, .dockerignore) and correctly depends on all previous tasks. From a developer perspective the main risk is build-time incompatibility between the SvelteKit Node adapter output and what the Dockerfile copies.
The acceptance criteria say CMD ["node", "build"] — this is correct for @sveltejs/adapter-node which outputs a build/index.js entry point. However, the runner stage must copy more than just build/: adapter-node also requires the package.json and the production node_modules to be present at the same level as build/.
sharp is a native module compiled against the Node ABI and the platform's libvips. Rebuilding it in the runner stage via npm ci --omit=dev is necessary because the binaries compiled in the builder stage are not guaranteed to be compatible if the runner's Alpine patch version differs. The current approach of copying package.json and running npm ci --omit=dev in the runner is correct — but if sharp is not in dependencies (only in devDependencies by mistake), --omit=dev will exclude it and uploads will break at runtime.
The .dockerignore list in the ACs (node_modules, .svelte-kit, build, *.db, uploads/) is correct. However it should also exclude src/, static/, and docs/ from the runner stage context — these are large and only needed by the builder. Alternatively, use a proper .dockerignore that excludes everything not needed for the Docker build context.
The full-flow test AC ("create code → add article with photo → family login → reserve") is the right integration smoke test. But it should be run against http://localhost:3000 directly (bypassing any Caddy config) to isolate the container as the unit under test.
There is no mention of the DATABASE_PATH directory being created on first start. If the named volume db is empty and the app tries to open /app/db/erbstuecke.db, better-sqlite3 will fail if /app/db/ does not exist. The lib/db.ts implementation must create the directory before opening the database, or the Dockerfile must RUN mkdir -p /app/db /app/uploads.

Recommendations

Verify that sharp is listed under dependencies (not devDependencies) in package.json. It is a runtime dependency — npm ci --omit=dev in the runner stage must include it.
Add directory creation to either the Dockerfile runner stage or lib/db.ts:
```
# In runner stage, before USER node:
RUN mkdir -p /app/db /app/uploads && chown -R node:node /app/db /app/uploads
USER node
```
This ensures the volume mount points exist even on first deploy with empty named volumes.
The .dockerignore should be more aggressive — exclude src/, static/, docs/, *.md, tests/ from the build context. Only package*.json is needed before npm ci; the full source is needed only for npm run build. Use a layered approach in the Dockerfile (copy package files first, install, then copy source) to maximize Docker layer cache reuse on rebuilds.
Add an explicit test for the DATABASE_PATH directory creation in db.test.ts — confirm that getDb() with a path under a non-existent directory either creates the directory or throws a useful error, not a cryptic SQLITE_CANTOPEN.

Open Decisions (omit if none)

sharp dependency classification — The plan shows npm install sharp (runtime), but if a previous task accidentally placed it in devDependencies, this task will produce a runtime crash only visible inside Docker (not in dev). Should the acceptance criteria include an explicit check that sharp is in dependencies? (Raised by: Felix Brandt)

## 👤 Felix Brandt — Fullstack Developer ### Observations - The issue is infrastructure-only (Dockerfile, docker-compose.yml, .dockerignore) and correctly depends on all previous tasks. From a developer perspective the main risk is build-time incompatibility between the SvelteKit Node adapter output and what the Dockerfile copies. - The acceptance criteria say `CMD ["node", "build"]` — this is correct for `@sveltejs/adapter-node` which outputs a `build/index.js` entry point. However, the runner stage must copy more than just `build/`: adapter-node also requires the `package.json` and the production `node_modules` to be present at the same level as `build/`. - `sharp` is a native module compiled against the Node ABI and the platform's `libvips`. Rebuilding it in the runner stage via `npm ci --omit=dev` is necessary because the binaries compiled in the builder stage are not guaranteed to be compatible if the runner's Alpine patch version differs. The current approach of copying `package.json` and running `npm ci --omit=dev` in the runner is correct — but if `sharp` is not in `dependencies` (only in `devDependencies` by mistake), `--omit=dev` will exclude it and uploads will break at runtime. - The `.dockerignore` list in the ACs (node_modules, .svelte-kit, build, *.db, uploads/) is correct. However it should also exclude `src/`, `static/`, and `docs/` from the runner stage context — these are large and only needed by the builder. Alternatively, use a proper `.dockerignore` that excludes everything not needed for the Docker build context. - The full-flow test AC ("create code → add article with photo → family login → reserve") is the right integration smoke test. But it should be run against `http://localhost:3000` directly (bypassing any Caddy config) to isolate the container as the unit under test. - There is no mention of the `DATABASE_PATH` directory being created on first start. If the named volume `db` is empty and the app tries to open `/app/db/erbstuecke.db`, `better-sqlite3` will fail if `/app/db/` does not exist. The `lib/db.ts` implementation must create the directory before opening the database, or the Dockerfile must `RUN mkdir -p /app/db /app/uploads`. ### Recommendations - Verify that `sharp` is listed under `dependencies` (not `devDependencies`) in `package.json`. It is a runtime dependency — `npm ci --omit=dev` in the runner stage must include it. - Add directory creation to either the Dockerfile runner stage or `lib/db.ts`: ```dockerfile # In runner stage, before USER node: RUN mkdir -p /app/db /app/uploads && chown -R node:node /app/db /app/uploads USER node ``` This ensures the volume mount points exist even on first deploy with empty named volumes. - The `.dockerignore` should be more aggressive — exclude `src/`, `static/`, `docs/`, `*.md`, `tests/` from the build context. Only `package*.json` is needed before `npm ci`; the full source is needed only for `npm run build`. Use a layered approach in the Dockerfile (copy package files first, install, then copy source) to maximize Docker layer cache reuse on rebuilds. - Add an explicit test for the `DATABASE_PATH` directory creation in `db.test.ts` — confirm that `getDb()` with a path under a non-existent directory either creates the directory or throws a useful error, not a cryptic `SQLITE_CANTOPEN`. ### Open Decisions _(omit if none)_ - **`sharp` dependency classification** — The plan shows `npm install sharp` (runtime), but if a previous task accidentally placed it in `devDependencies`, this task will produce a runtime crash only visible inside Docker (not in dev). Should the acceptance criteria include an explicit check that `sharp` is in `dependencies`? _(Raised by: Felix Brandt)_

marcel commented

2026-05-05 11:27:34 +02:00

👤 Nora Steiner — Application Security Engineer

Observations

The acceptance criteria require all secrets to be passed via environment variables — correct. But the issue does not specify that .env must be gitignored or that the repo must never contain a committed .env. Without an explicit .gitignore entry and a .env.example, a deployer will likely create .env in the project root and accidentally commit it.
The system design spec §2 shows SESSION_SECRET in the compose file as a required variable, but the issue's acceptance criteria bullet list omits it. If the app fails-closed on missing SESSION_SECRET (as the persona standards require), a deployment without it will crash. If it does not fail-closed, it is a security defect.
The .dockerignore correctly excludes *.db and uploads/ — this prevents accidental baking of a local database or uploaded photos into the image. Good. But it should also exclude .env explicitly, since .env is not in the project root .gitignore by default in many SvelteKit scaffolds.
The issue mentions ADMIN_MARCEL_PASSWORD_HASH, ADMIN_RENATE_PASSWORD_HASH, and ADMIN_BERIT_PASSWORD_HASH by name. These being in environment variables is correct. The risk is that the bcrypt hash generation command (node -e "require('bcryptjs').hash('PASSWORD', 12).then(h => console.log(h))") suggests the operator substitutes their actual password in a shell command — which writes the plaintext password to shell history. The issue should recommend read -s PASSWORD && node -e ... to avoid history exposure.
The Caddyfile snippet in the issue uses erbstuecke.example.com as a placeholder — appropriate, but the comment says "not in repo." The security headers block (HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy) is absent from the Caddyfile shown. Without these headers, the deployed app has a weaker security posture despite TLS.
Running the Node process as root inside the container (no USER node in the Dockerfile) means a container escape or path traversal exploit runs as root on the host if the Docker daemon is not properly isolated. The current acceptance criteria do not require USER node.

Recommendations

Add .env to .dockerignore explicitly:

node_modules
.svelte-kit
build
*.db
uploads/
.env
.env.*

Add .env to .gitignore if not already present. This is a separate file from .dockerignore but equally critical.
Require SESSION_SECRET explicitly in the acceptance criteria env-var list, and require that lib/db.ts or hooks.server.ts fails to start if it is missing:
```
if (!process.env.SESSION_SECRET) {
  console.error('FATAL: SESSION_SECRET environment variable is required');
  process.exit(1);
}
```

Replace the plaintext-in-shell hash generation command with a history-safe version in the issue body:

read -rs PASSWORD && node -e "require('bcryptjs').hash(process.env.PW, 12).then(console.log)" PW=$PASSWORD
# Or simpler:
node -e "const r=require('readline').createInterface({input:process.stdin,terminal:false}); r.on('line',p=>require('bcryptjs').hash(p,12).then(console.log))"

Add the security headers block to the Caddyfile example:

erbstuecke.example.com {
    reverse_proxy localhost:3000
    header {
        Strict-Transport-Security "max-age=31536000; includeSubDomains"
        X-Content-Type-Options "nosniff"
        X-Frame-Options "DENY"
        Referrer-Policy "strict-origin-when-cross-origin"
        -Server
    }
}

Add USER node to the acceptance criteria checklist for the Dockerfile runner stage.

## 👤 Nora Steiner — Application Security Engineer ### Observations - The acceptance criteria require all secrets to be passed via environment variables — correct. But the issue does not specify that `.env` must be gitignored or that the repo must never contain a committed `.env`. Without an explicit `.gitignore` entry and a `.env.example`, a deployer will likely create `.env` in the project root and accidentally commit it. - The system design spec §2 shows `SESSION_SECRET` in the compose file as a required variable, but the issue's acceptance criteria bullet list omits it. If the app fails-closed on missing `SESSION_SECRET` (as the persona standards require), a deployment without it will crash. If it does not fail-closed, it is a security defect. - The `.dockerignore` correctly excludes `*.db` and `uploads/` — this prevents accidental baking of a local database or uploaded photos into the image. Good. But it should also exclude `.env` explicitly, since `.env` is not in the project root `.gitignore` by default in many SvelteKit scaffolds. - The issue mentions `ADMIN_MARCEL_PASSWORD_HASH`, `ADMIN_RENATE_PASSWORD_HASH`, and `ADMIN_BERIT_PASSWORD_HASH` by name. These being in environment variables is correct. The risk is that the bcrypt hash generation command (`node -e "require('bcryptjs').hash('PASSWORD', 12).then(h => console.log(h))"`) suggests the operator substitutes their actual password in a shell command — which writes the plaintext password to shell history. The issue should recommend `read -s PASSWORD && node -e ...` to avoid history exposure. - The Caddyfile snippet in the issue uses `erbstuecke.example.com` as a placeholder — appropriate, but the comment says "not in repo." The security headers block (HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy) is absent from the Caddyfile shown. Without these headers, the deployed app has a weaker security posture despite TLS. - Running the Node process as root inside the container (no `USER node` in the Dockerfile) means a container escape or path traversal exploit runs as root on the host if the Docker daemon is not properly isolated. The current acceptance criteria do not require `USER node`. ### Recommendations - Add `.env` to `.dockerignore` explicitly: ``` node_modules .svelte-kit build *.db uploads/ .env .env.* ``` - Add `.env` to `.gitignore` if not already present. This is a separate file from `.dockerignore` but equally critical. - Require `SESSION_SECRET` explicitly in the acceptance criteria env-var list, and require that `lib/db.ts` or `hooks.server.ts` fails to start if it is missing: ```typescript if (!process.env.SESSION_SECRET) { console.error('FATAL: SESSION_SECRET environment variable is required'); process.exit(1); } ``` - Replace the plaintext-in-shell hash generation command with a history-safe version in the issue body: ```bash read -rs PASSWORD && node -e "require('bcryptjs').hash(process.env.PW, 12).then(console.log)" PW=$PASSWORD # Or simpler: node -e "const r=require('readline').createInterface({input:process.stdin,terminal:false}); r.on('line',p=>require('bcryptjs').hash(p,12).then(console.log))" ``` - Add the security headers block to the Caddyfile example: ```caddyfile erbstuecke.example.com { reverse_proxy localhost:3000 header { Strict-Transport-Security "max-age=31536000; includeSubDomains" X-Content-Type-Options "nosniff" X-Frame-Options "DENY" Referrer-Policy "strict-origin-when-cross-origin" -Server } } ``` - Add `USER node` to the acceptance criteria checklist for the Dockerfile runner stage.

marcel commented

2026-05-05 11:27:52 +02:00

👤 Sara Holt — QA Engineer & Test Strategist

Observations

The issue has one integration AC: "Full flow tested in Docker: create code → add article with photo → family login → reserve." This is the right critical E2E journey, but it is written as a manual verification step, not an automated test. For a deployment task, manual smoke testing is acceptable — but there is no checklist of exactly what to verify at each step, so "tested" is ambiguous.
There is no acceptance criterion verifying that the named volumes actually persist across a docker compose down && docker compose up --build cycle. This is the most common mistake in Docker deployments — it is easy to verify manually and essential for the app's durability guarantee.
The docker compose build && docker compose up AC only verifies that the container starts, not that it is healthy. Without a health check endpoint, there is no programmatic way to confirm the app is actually serving requests before running the manual flow test.
The full flow AC covers the happy path. It does not cover the failure mode most likely to appear in Docker (and not in dev): a missing environment variable causing a silent crash or startup error. The smoke test should verify the container logs are clean (no errors, no missing env var warnings).
The .dockerignore exclusions are not currently verifiable by the acceptance criteria — there is no AC that confirms a *.db file in the project root is not baked into the built image.

Recommendations

Expand the smoke test AC into a concrete checklist:

[ ] docker compose build exits with code 0
[ ] docker compose up starts the app container
[ ] docker compose ps shows app is "running" (or "healthy" if health check is present)
[ ] curl http://localhost:3000/ returns HTTP 200 (Gate Screen HTML in body)
[ ] docker compose exec app ls /app/db /app/uploads shows both directories exist
[ ] docker compose logs app shows no ERROR or FATAL lines
[ ] Full flow: create code → add article with photo → family login → reserve

Add a persistence AC:

[ ] docker compose down (does NOT pass --volumes)
[ ] docker compose up --build
[ ] Data created before down is still present (article, code, reservation)

Add an isolation AC for .dockerignore:

[ ] touch test-local.db in project root before build
[ ] docker compose build
[ ] docker compose exec app ls /app | grep -v "test-local.db" (file is not in image)

If a /health endpoint is implemented, add:

[ ] curl http://localhost:3000/health returns HTTP 200 with body "ok"
[ ] docker compose ps shows status "healthy" after start_period elapses

Add a negative AC for missing env vars: run the container without DATABASE_PATH set, confirm it exits with a non-zero code and a readable error message rather than crashing silently or serving garbage.

Open Decisions (omit if none)

Manual vs. automated smoke test — The full-flow AC is currently written as a manual step. For a family-scale app with one deployment, manual is sufficient. But if this project will have multiple deploys (updates, new articles added by admins), automating the smoke test as a shell script (smoke-test.sh) would pay off quickly. Options: (A) keep it manual with the checklist above, (B) create a scripts/smoke-test.sh that runs the curl checks automatically. (Raised by: Sara Holt)

## 👤 Sara Holt — QA Engineer & Test Strategist ### Observations - The issue has one integration AC: "Full flow tested in Docker: create code → add article with photo → family login → reserve." This is the right critical E2E journey, but it is written as a manual verification step, not an automated test. For a deployment task, manual smoke testing is acceptable — but there is no checklist of exactly what to verify at each step, so "tested" is ambiguous. - There is no acceptance criterion verifying that the named volumes actually persist across a `docker compose down && docker compose up --build` cycle. This is the most common mistake in Docker deployments — it is easy to verify manually and essential for the app's durability guarantee. - The `docker compose build && docker compose up` AC only verifies that the container starts, not that it is healthy. Without a health check endpoint, there is no programmatic way to confirm the app is actually serving requests before running the manual flow test. - The full flow AC covers the happy path. It does not cover the failure mode most likely to appear in Docker (and not in dev): a missing environment variable causing a silent crash or startup error. The smoke test should verify the container logs are clean (no errors, no missing env var warnings). - The `.dockerignore` exclusions are not currently verifiable by the acceptance criteria — there is no AC that confirms a `*.db` file in the project root is not baked into the built image. ### Recommendations - Expand the smoke test AC into a concrete checklist: ``` [ ] docker compose build exits with code 0 [ ] docker compose up starts the app container [ ] docker compose ps shows app is "running" (or "healthy" if health check is present) [ ] curl http://localhost:3000/ returns HTTP 200 (Gate Screen HTML in body) [ ] docker compose exec app ls /app/db /app/uploads shows both directories exist [ ] docker compose logs app shows no ERROR or FATAL lines [ ] Full flow: create code → add article with photo → family login → reserve ``` - Add a persistence AC: ``` [ ] docker compose down (does NOT pass --volumes) [ ] docker compose up --build [ ] Data created before down is still present (article, code, reservation) ``` - Add an isolation AC for `.dockerignore`: ``` [ ] touch test-local.db in project root before build [ ] docker compose build [ ] docker compose exec app ls /app | grep -v "test-local.db" (file is not in image) ``` - If a `/health` endpoint is implemented, add: ``` [ ] curl http://localhost:3000/health returns HTTP 200 with body "ok" [ ] docker compose ps shows status "healthy" after start_period elapses ``` - Add a negative AC for missing env vars: run the container without `DATABASE_PATH` set, confirm it exits with a non-zero code and a readable error message rather than crashing silently or serving garbage. ### Open Decisions _(omit if none)_ - **Manual vs. automated smoke test** — The full-flow AC is currently written as a manual step. For a family-scale app with one deployment, manual is sufficient. But if this project will have multiple deploys (updates, new articles added by admins), automating the smoke test as a shell script (`smoke-test.sh`) would pay off quickly. Options: (A) keep it manual with the checklist above, (B) create a `scripts/smoke-test.sh` that runs the curl checks automatically. _(Raised by: Sara Holt)_

marcel commented

2026-05-05 11:28:11 +02:00

👤 Markus Keller — Application Architect

Observations

The Docker task is correctly scoped as infrastructure. The architecture decisions that matter here are: (1) volume mount paths must match the env vars the app expects, (2) the build output must include everything adapter-node requires, and (3) environment variables must be consumed exactly as lib/db.ts, lib/auth.ts, and lib/photos.ts expect them.
The system design spec §2 defines DATABASE_PATH=/app/db/erbstuecke.db and UPLOAD_DIR=/app/uploads. These must match exactly what lib/db.ts reads (process.env.DATABASE_PATH) and what lib/photos.ts reads (process.env.UPLOAD_DIR). If either lib uses a hardcoded fallback path, the container will silently write to the wrong location and data will be lost on rebuild.
The spec §2 compose snippet does not include SESSION_SECRET. The auth system requires it. If hooks.server.ts or the session layer silently falls back to a hardcoded secret, sessions will be insecure in production. If it fails-closed (as it should), the container will crash on first start without a clear error unless the error is logged.
The route GET /uploads/[...path]/+server.ts streams files from UPLOAD_DIR. In Docker, this path resolves inside the container's volume mount. The path bounds check (path.resolve(UPLOAD_DIR, params.path).startsWith(path.resolve(UPLOAD_DIR) + path.sep)) must use the env var value, not a hardcoded string, or it will reject all upload requests when the env var differs from the default.
The named volumes (db:, uploads:) survive docker compose down and docker compose up --build — this is the correct approach. However, the volume name prefix is determined by the Compose project name (defaults to the directory name). If the deployment directory is renamed, Docker creates new volumes and the old data appears lost. This is a common operational surprise.
The plan file shows the new project will be created at erbstuecke-wannsee/ (outside the spec repo). The Dockerfile, docker-compose.yml, and .dockerignore must be created in that project root, not in the spec repo (wannsee-kram/).

Recommendations

Confirm that lib/db.ts reads process.env.DATABASE_PATH without a fallback (or fails-closed if missing), and that lib/photos.ts reads process.env.UPLOAD_DIR without a fallback. Both should process.exit(1) with a clear message if their required env var is absent.
Add SESSION_SECRET to the required env vars in the compose file and to the acceptance criteria list.

Document the Compose project name risk: add a comment in docker-compose.yml noting that the volume names are project-name-prefixed, and that renaming the deployment directory requires migrating the volumes:

# NOTE: Named volumes are prefixed with the Compose project name (directory name by default).
# If you rename this directory, run: docker volume ls | grep erbstuecke
# and migrate data before deploying from the new location.

Verify the upload path bounds check in +server.ts uses UPLOAD_DIR from the environment, not a hardcoded string. This is an architecture correctness issue, not just a security one — wrong path = 404 for all photos in production.
The acceptance criteria should include verifying that docker compose exec app env shows all required variables with non-empty values before running the manual flow test.

## 👤 Markus Keller — Application Architect ### Observations - The Docker task is correctly scoped as infrastructure. The architecture decisions that matter here are: (1) volume mount paths must match the env vars the app expects, (2) the build output must include everything `adapter-node` requires, and (3) environment variables must be consumed exactly as `lib/db.ts`, `lib/auth.ts`, and `lib/photos.ts` expect them. - The system design spec §2 defines `DATABASE_PATH=/app/db/erbstuecke.db` and `UPLOAD_DIR=/app/uploads`. These must match exactly what `lib/db.ts` reads (`process.env.DATABASE_PATH`) and what `lib/photos.ts` reads (`process.env.UPLOAD_DIR`). If either lib uses a hardcoded fallback path, the container will silently write to the wrong location and data will be lost on rebuild. - The spec §2 compose snippet does not include `SESSION_SECRET`. The auth system requires it. If `hooks.server.ts` or the session layer silently falls back to a hardcoded secret, sessions will be insecure in production. If it fails-closed (as it should), the container will crash on first start without a clear error unless the error is logged. - The route `GET /uploads/[...path]/+server.ts` streams files from `UPLOAD_DIR`. In Docker, this path resolves inside the container's volume mount. The path bounds check (`path.resolve(UPLOAD_DIR, params.path).startsWith(path.resolve(UPLOAD_DIR) + path.sep)`) must use the env var value, not a hardcoded string, or it will reject all upload requests when the env var differs from the default. - The named volumes (`db:`, `uploads:`) survive `docker compose down` and `docker compose up --build` — this is the correct approach. However, the volume name prefix is determined by the Compose project name (defaults to the directory name). If the deployment directory is renamed, Docker creates new volumes and the old data appears lost. This is a common operational surprise. - The plan file shows the new project will be created at `erbstuecke-wannsee/` (outside the spec repo). The Dockerfile, docker-compose.yml, and .dockerignore must be created in that project root, not in the spec repo (`wannsee-kram/`). ### Recommendations - Confirm that `lib/db.ts` reads `process.env.DATABASE_PATH` without a fallback (or fails-closed if missing), and that `lib/photos.ts` reads `process.env.UPLOAD_DIR` without a fallback. Both should `process.exit(1)` with a clear message if their required env var is absent. - Add `SESSION_SECRET` to the required env vars in the compose file and to the acceptance criteria list. - Document the Compose project name risk: add a comment in `docker-compose.yml` noting that the volume names are project-name-prefixed, and that renaming the deployment directory requires migrating the volumes: ```yaml # NOTE: Named volumes are prefixed with the Compose project name (directory name by default). # If you rename this directory, run: docker volume ls | grep erbstuecke # and migrate data before deploying from the new location. ``` - Verify the upload path bounds check in `+server.ts` uses `UPLOAD_DIR` from the environment, not a hardcoded string. This is an architecture correctness issue, not just a security one — wrong path = 404 for all photos in production. - The acceptance criteria should include verifying that `docker compose exec app env` shows all required variables with non-empty values before running the manual flow test.

marcel commented

2026-05-05 11:28:25 +02:00

👤 Leonie Voss — UX Design Lead

Observations

This is a pure infrastructure task with no UI surface. There is nothing to review from a layout, typography, or accessibility perspective in the Docker files themselves.
The one indirect UX concern: if the /health endpoint is added to support the Docker health check, it must not be a publicly navigable page that returns HTML — it should return a plain 200 with a minimal body (ok). A health route that accidentally returns the Gate Screen HTML could confuse a monitoring tool or a curious user who lands on /health.
The full-flow acceptance criterion ("family login → reserve") will be the first time the complete user journey runs in a production-like environment. If any step fails silently (no error shown, page just reloads), it will be hard to diagnose without knowing the UX. Ensure the German error messages (already specified in the design) are visible in the Docker environment — particularly the "Dieser Artikel wurde soeben von jemand anderem reserviert" reservation conflict message, which depends on the SQLite UNIQUE constraint working correctly end-to-end.
The Gate Screen should render correctly with the @fontsource/lora and @fontsource/inter fonts served from the Node process. Fonts are bundled into the SvelteKit build output — no CDN needed. Confirm that the built image includes the font files (they are in node_modules/@fontsource/ and referenced in app.css, so they should be bundled by Vite at build time). If they are excluded or missing, the Gate Screen will fall back to Georgia/system-ui, which may not be noticed during the smoke test.

Recommendations

If a /health route is implemented, implement it as a minimal JSON or text endpoint, not a full SvelteKit page:
```
// src/routes/health/+server.ts
import type { RequestHandler } from './$types';
export const GET: RequestHandler = () => new Response('ok', { status: 200 });
```
This keeps it out of the user-facing route tree and avoids any layout or font rendering.
Add a visual check to the smoke test: after curl http://localhost:3000/ succeeds, open the Gate Screen in a browser on the deployment machine and confirm Lora serif renders (the code input box and the "Erbstücke Wannsee" heading use Lora — if fonts are missing the difference is immediately visible).
No open UX decisions for this task.

## 👤 Leonie Voss — UX Design Lead ### Observations - This is a pure infrastructure task with no UI surface. There is nothing to review from a layout, typography, or accessibility perspective in the Docker files themselves. - The one indirect UX concern: if the `/health` endpoint is added to support the Docker health check, it must not be a publicly navigable page that returns HTML — it should return a plain 200 with a minimal body (`ok`). A health route that accidentally returns the Gate Screen HTML could confuse a monitoring tool or a curious user who lands on `/health`. - The full-flow acceptance criterion ("family login → reserve") will be the first time the complete user journey runs in a production-like environment. If any step fails silently (no error shown, page just reloads), it will be hard to diagnose without knowing the UX. Ensure the German error messages (already specified in the design) are visible in the Docker environment — particularly the "Dieser Artikel wurde soeben von jemand anderem reserviert" reservation conflict message, which depends on the SQLite UNIQUE constraint working correctly end-to-end. - The Gate Screen should render correctly with the `@fontsource/lora` and `@fontsource/inter` fonts served from the Node process. Fonts are bundled into the SvelteKit build output — no CDN needed. Confirm that the built image includes the font files (they are in `node_modules/@fontsource/` and referenced in `app.css`, so they should be bundled by Vite at build time). If they are excluded or missing, the Gate Screen will fall back to Georgia/system-ui, which may not be noticed during the smoke test. ### Recommendations - If a `/health` route is implemented, implement it as a minimal JSON or text endpoint, not a full SvelteKit page: ```typescript // src/routes/health/+server.ts import type { RequestHandler } from './$types'; export const GET: RequestHandler = () => new Response('ok', { status: 200 }); ``` This keeps it out of the user-facing route tree and avoids any layout or font rendering. - Add a visual check to the smoke test: after `curl http://localhost:3000/` succeeds, open the Gate Screen in a browser on the deployment machine and confirm Lora serif renders (the code input box and the "Erbstücke Wannsee" heading use Lora — if fonts are missing the difference is immediately visible). - No open UX decisions for this task.

marcel commented

2026-05-05 11:28:42 +02:00

👤 Elicit — Requirements Engineer

Observations

The issue is well-structured and meets the Definition of Ready for an infrastructure task of size S: it has a clear title, explicit acceptance criteria with checkboxes, concrete commands, known dependencies, and a milestone. This is above-average issue quality.
One requirements gap: the issue states "All secrets passed via environment variables" but the acceptance criteria list omits SESSION_SECRET. This creates an incomplete specification — a developer implementing exactly to the AC list will produce a compose file that omits a required secret. The gap should be closed in the issue, not discovered at runtime.
The bcrypt hash generation command in the issue body instructs the operator to substitute their real password inline in a shell command. This is a process/workflow requirement that is underspecified: the issue does not say where to store the generated hashes, how to transfer them to the server, or that the .env file on the server must never be committed. For a solo operator this may be obvious, but it is a gap in the operational requirements.
The acceptance criterion "Full flow tested in Docker" is untestable as written — "tested" has no observable outcome. A well-formed AC would specify: given a running container with all env vars set, when the operator follows steps A-B-C, then the system responds with X-Y-Z. The checklist form (as recommended by Sara) would satisfy this.
The Caddy config is shown as a code block with erbstuecke.example.com as a placeholder and noted as "not in repo." This means there is no deliverable artifact for the Caddy config — it is documentation-only. This is fine for this project's scope, but it should be explicitly stated that the Caddyfile is the operator's responsibility and is not tracked in git.
The issue depends on "all previous tasks" but does not list specific issue numbers. This makes the dependency graph implicit. For a solo developer this is low risk, but worth noting.

Recommendations

Add SESSION_SECRET to the acceptance criteria env-var list. The requirement is already in the system design spec §2 — it is simply missing from this issue's AC.
Rewrite the "Full flow tested" AC as a numbered checklist of observable steps and outcomes (aligned with Sara's recommendation). This makes the criterion testable and unambiguous.
Add a note to the issue body clarifying the Caddyfile is not a repo artifact: "The Caddyfile is configured on the host server by the operator. The snippet above is a reference — adapt the domain name before use."
Add .env and .env.example to the operational notes. Specifically: .env is created on the server from .env.example, filled with real values, and never committed. This is an NFR-OPS requirement that is currently implicit.
Add explicit dependency issue numbers (e.g., "Depends on: #1, #2, ... #13") to make the dependency graph visible in the Gitea issue tracker.

## 👤 Elicit — Requirements Engineer ### Observations - The issue is well-structured and meets the Definition of Ready for an infrastructure task of size S: it has a clear title, explicit acceptance criteria with checkboxes, concrete commands, known dependencies, and a milestone. This is above-average issue quality. - One requirements gap: the issue states "All secrets passed via environment variables" but the acceptance criteria list omits `SESSION_SECRET`. This creates an incomplete specification — a developer implementing exactly to the AC list will produce a compose file that omits a required secret. The gap should be closed in the issue, not discovered at runtime. - The bcrypt hash generation command in the issue body instructs the operator to substitute their real password inline in a shell command. This is a process/workflow requirement that is underspecified: the issue does not say where to store the generated hashes, how to transfer them to the server, or that the `.env` file on the server must never be committed. For a solo operator this may be obvious, but it is a gap in the operational requirements. - The acceptance criterion "Full flow tested in Docker" is untestable as written — "tested" has no observable outcome. A well-formed AC would specify: given a running container with all env vars set, when the operator follows steps A-B-C, then the system responds with X-Y-Z. The checklist form (as recommended by Sara) would satisfy this. - The Caddy config is shown as a code block with `erbstuecke.example.com` as a placeholder and noted as "not in repo." This means there is no deliverable artifact for the Caddy config — it is documentation-only. This is fine for this project's scope, but it should be explicitly stated that the Caddyfile is the operator's responsibility and is not tracked in git. - The issue depends on "all previous tasks" but does not list specific issue numbers. This makes the dependency graph implicit. For a solo developer this is low risk, but worth noting. ### Recommendations - Add `SESSION_SECRET` to the acceptance criteria env-var list. The requirement is already in the system design spec §2 — it is simply missing from this issue's AC. - Rewrite the "Full flow tested" AC as a numbered checklist of observable steps and outcomes (aligned with Sara's recommendation). This makes the criterion testable and unambiguous. - Add a note to the issue body clarifying the Caddyfile is not a repo artifact: "The Caddyfile is configured on the host server by the operator. The snippet above is a reference — adapt the domain name before use." - Add `.env` and `.env.example` to the operational notes. Specifically: `.env` is created on the server from `.env.example`, filled with real values, and never committed. This is an NFR-OPS requirement that is currently implicit. - Add explicit dependency issue numbers (e.g., "Depends on: #1, #2, ... #13") to make the dependency graph visible in the Gitea issue tracker.

marcel commented

2026-05-05 11:29:02 +02:00

🗳️ Decision Queue — Action Required

3 decisions need your input before implementation starts.

Health Endpoint

Health endpoint scope — Should /health be implemented as part of this Docker task (Task 14) or tracked as a separate issue? Without it, the health check in docker-compose.yml will always fail and the container will never reach "healthy" status, meaning Caddy routes traffic to a potentially uninitialized Node process. Options: (A) implement /health as a minimal +server.ts route within this task — small scope, one file; (B) skip the health check in docker-compose.yml for now and add it in a follow-up task. (Raised by: Tobias Wendt)

Dependency Classification

sharp in dependencies vs devDependencies — sharp is a native runtime module that must survive npm ci --omit=dev in the Docker runner stage. If it was accidentally placed in devDependencies during scaffolding, the production image will crash on any photo upload — a failure mode that does not appear in npm run dev. Should the acceptance criteria include an explicit check (jq '.dependencies.sharp' package.json) to verify correct placement before the Docker build is considered passing? Options: (A) add this as an explicit AC checkpoint; (B) rely on the full-flow smoke test to catch it (it will, but only after a longer debug cycle). (Raised by: Felix Brandt)

Smoke Test Automation

Manual checklist vs. automated smoke script — The full-flow AC is currently written as a single manual verification step. For a project with a single production deployment this is sufficient. But admin users will add articles and codes over time, requiring periodic redeploys. Options: (A) keep it as a manual checklist (expanded per Sara's recommendations) — zero extra implementation cost; (B) create a scripts/smoke-test.sh that runs the curl checks and prints pass/fail — roughly 20 lines, reusable on every future deploy. (Raised by: Sara Holt)

## 🗳️ Decision Queue — Action Required _3 decisions need your input before implementation starts._ ### Health Endpoint - **Health endpoint scope** — Should `/health` be implemented as part of this Docker task (Task 14) or tracked as a separate issue? Without it, the health check in docker-compose.yml will always fail and the container will never reach "healthy" status, meaning Caddy routes traffic to a potentially uninitialized Node process. Options: (A) implement `/health` as a minimal `+server.ts` route within this task — small scope, one file; (B) skip the health check in docker-compose.yml for now and add it in a follow-up task. _(Raised by: Tobias Wendt)_ ### Dependency Classification - **`sharp` in `dependencies` vs `devDependencies`** — `sharp` is a native runtime module that must survive `npm ci --omit=dev` in the Docker runner stage. If it was accidentally placed in `devDependencies` during scaffolding, the production image will crash on any photo upload — a failure mode that does not appear in `npm run dev`. Should the acceptance criteria include an explicit check (`jq '.dependencies.sharp' package.json`) to verify correct placement before the Docker build is considered passing? Options: (A) add this as an explicit AC checkpoint; (B) rely on the full-flow smoke test to catch it (it will, but only after a longer debug cycle). _(Raised by: Felix Brandt)_ ### Smoke Test Automation - **Manual checklist vs. automated smoke script** — The full-flow AC is currently written as a single manual verification step. For a project with a single production deployment this is sufficient. But admin users will add articles and codes over time, requiring periodic redeploys. Options: (A) keep it as a manual checklist (expanded per Sara's recommendations) — zero extra implementation cost; (B) create a `scripts/smoke-test.sh` that runs the curl checks and prints pass/fail — roughly 20 lines, reusable on every future deploy. _(Raised by: Sara Holt)_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/wannsee-kram#14

Set up Docker deployment with Caddy reverse proxy #14

Task 14 — Plan reference: docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md

Acceptance criteria

Bcrypt hash generation (run locally before deployment)

Caddy config on host (not in repo)

Files to create

👤 Tobias Wendt — DevOps & Platform Engineer

Observations

Recommendations

Open Decisions (omit if none)

👤 Felix Brandt — Fullstack Developer

Observations

Recommendations

Open Decisions (omit if none)

👤 Nora Steiner — Application Security Engineer

Observations

Recommendations

👤 Sara Holt — QA Engineer & Test Strategist

Observations

Recommendations

Open Decisions (omit if none)

👤 Markus Keller — Application Architect

Observations

Recommendations

👤 Leonie Voss — UX Design Lead

Observations

Recommendations

👤 Elicit — Requirements Engineer

Observations

Recommendations

🗳️ Decision Queue — Action Required

Health Endpoint

Dependency Classification

Smoke Test Automation

Task 14 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md`