pal-e-docs

Review: Switch pal-e-app to adapter-static + nginx

review-414-2026-03-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- present but contains wrong issue reference (see Recommendation)
  • [x] Repo -- forgejo_admin/pal-e-app
  • [x] User Story -- well-formed
  • [x] Context -- clear motivation
  • [x] File Targets -- specific files listed with modify/remove/don't-touch sections
  • [x] Acceptance Criteria -- 6 items
  • [x] Test Expectations -- 3 items with run command
  • [x] Constraints -- 4 items
  • [x] Checklist -- present
  • [x] Related -- 3 references

Traceability

  • [ ] story:X label -- MISSING. Platform convention migration, not directly user-facing. Foundational work -- acceptable if intentional, but should be explicitly noted.
  • [x] arch:app label -- present (arch:app, arch:deploy)
  • [x] Forgejo issue -- forgejo_admin/pal-e-app#53, open

File Targets

  • [x] svelte.config.js -- verified: currently imports @sveltejs/adapter-node (line 1)
  • [x] Dockerfile -- verified: currently uses node:22-alpine multi-stage with Node.js runtime (27 lines)
  • [x] k8s/deployment.yaml -- verified: contains pal-e-auth-secrets secretRef (line 29-30), PAL_E_DOCS_API_URL env (line 32-33), PAL_E_DOCS_API_KEY env (line 34-38), containerPort 3000 (line 27), AUTH_TRUST_HOST env (line 39-40)
  • [x] package.json -- verified: has @sveltejs/adapter-node in devDependencies (line 18)
  • [x] k8s/pal-e-auth-secrets.enc.yaml -- verified: file exists, contains AUTH_SECRET
  • [x] Reference pattern ~/mcd-tracker-app/Dockerfile -- verified: nginx:alpine pattern exists and matches description

Repo Placement

OK. Issue is filed on forgejo_admin/pal-e-app and all file targets live in that repo. The k8s/ manifests are in-repo (not in pal-e-deployments), consistent with the Constraints section.

Dependencies

  • ISSUE: Wrong dependency reference. The Lineage section says "Depends on forgejo_admin/pal-e-app #2" but issue #2 is "Scaffold SvelteKit app with board pages and dev environment" (closed). The actual prerequisite is issue #52 ("Migrate pal-e-app auth + data fetching to client-side"), which is open and in backlog (board item #413).
  • Server-side code is extensive. Verified 14 +page.server.ts files and 3 +server.ts API routes that import from $lib/api, which uses $env/dynamic/private (server-only). The src/hooks.server.ts wires Auth.js server-side handle. The src/auth.ts uses @auth/sveltekit with server-side JWT callbacks. All of this MUST be removed/migrated by issue #52 before this ticket can execute.
  • Board item #412 (issue #51: "Write convention-sveltekit-spa convention note") is referenced in Related. This convention note does not yet exist in pal-e-docs (search returned empty). Not a hard blocker but the spec this ticket claims to follow doesn't exist yet.
  • Board item #413 (issue #52: client-side auth migration) is in backlog. This ticket cannot move to next_up until #52 is at minimum in_progress or done.

Acceptance Criteria

  • [x] "npm run build produces static files in build/ directory" -- verifiable by agent
  • [x] "Dockerfile builds nginx image that serves the SPA" -- verifiable via docker build
  • [x] "SPA fallback works" -- verifiable but test method not specified (needs curl or browser check)
  • [x] "k8s deployment has no secrets or server env vars" -- verifiable by grep on deployment.yaml
  • [ ] "ArgoCD syncs successfully with new image" -- NOT verifiable by agent in isolation. Requires deployed cluster. Should specify observable check command.
  • [x] "All existing routes work in production" -- verifiable via E2E tests
Test Expectations include npm run build && npm run test:e2e which is correct. However, E2E tests currently run against the live deployment (PLAYWRIGHT_BASE_URL: https://pal-e-app.tail5b443a.ts.net in .woodpecker.yaml), not against a local build. The test command alone won't validate the nginx image serves correctly.

Blast Radius

  • mcd-tracker-app already uses adapter-static + nginx pattern. Proven reference implementation. Low risk for the Dockerfile/adapter change itself.
  • westside-app and westside-contracts still use adapter-node. May follow the same pattern later. No immediate downstream impact.
  • Woodpecker CI (.woodpecker.yaml) -- build step runs npm run build (works for both adapters). The kaniko build pushes to pal-e-app/app matching Constraints. The update-deployment-tag step uses sed on k8s/deployment.yaml which still works. CI is safe.
  • @auth/sveltekit in package.json -- listed in dependencies (server-side auth library). The ticket says "don't touch src/" but package.json IS in scope. The agent will need clarity on whether @auth/sveltekit removal is this ticket's responsibility or was already handled by #52.

Recommendation

Three issues must be fixed before this ticket is READY:
  • Fix Lineage reference -- Change "Depends on forgejo_admin/pal-e-app #2" to "Depends on forgejo_admin/pal-e-app #52" (client-side auth + data fetching migration).
  • Add explicit blocker note -- This ticket CANNOT execute until issue #52 is complete. All 14 +page.server.ts files, 3 +server.ts API routes, hooks.server.ts, and auth.ts must be gone first. Consider adding: "BLOCKED: Do not start until #52 is merged and all server-side code is removed."
  • Clarify package.json scope -- The File Targets say to swap adapter-node for adapter-static, but @auth/sveltekit is also in dependencies. Explicitly state whether @auth/sveltekit removal is in scope here or was already handled by #52.
Optional improvements:
  • Add a story label if one applies, or note "foundational work" in the ticket.
  • Strengthen the ArgoCD acceptance criterion with a specific observable check command.
  • Note that convention-sveltekit-spa (#51) should ideally be written before this ticket executes, so the agent has the spec to follow.