Review: Migrate pal-e-app auth + data fetching to client-side

review-413-2026-03-26 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- depends on pal-e-app #1
  • [x] Repo -- forgejo_admin/pal-e-app
  • [x] User Story -- well-formed As/I want/So that
  • [x] Context -- good background, references Phase 29 and mcd-tracker-app pattern
  • [x] File Targets -- present but incomplete (see below)
  • [x] Acceptance Criteria -- 7 criteria
  • [x] Test Expectations -- e2e tests + run command
  • [x] Constraints -- 5 constraints listed
  • [x] Checklist -- standard 3-item
  • [x] Related -- 3 references

Traceability

  • [ ] story:X label -- MISSING. Board item #413 has labels type:feature,arch:auth,arch:app but no story label. This is a significant feature that should map to a user story.
  • [x] arch:X label -- arch:auth,arch:app present, correctly identifies affected architecture components
  • [x] Forgejo issue -- forgejo_admin/pal-e-app#52, open

File Targets

Files to create (verified):
  • [x] src/lib/keycloak.js -- does not exist yet, model file ~/mcd-tracker-app/src/lib/keycloak.js confirmed present
  • [x] src/lib/api-client.js -- does not exist yet, model file ~/mcd-tracker-app/src/lib/api.js confirmed present
Files to modify (verified with issues):
  • [x] src/routes/+layout.svelte -- exists, uses $page.data.session and Auth.js session pattern
  • [x] src/routes/+page.svelte -- exists
  • [x] src/routes/boards/+page.svelte -- exists
  • [x] src/routes/boards/[slug]/+page.svelte -- exists
  • [x] src/routes/notes/+page.svelte -- exists
  • [x] src/routes/notes/[slug]/+page.svelte -- exists
  • [x] src/routes/projects/+page.svelte -- exists
  • [x] src/routes/projects/[slug]/+page.svelte -- exists
  • [x] src/routes/repos/+page.svelte -- exists
  • [x] src/routes/search/+page.svelte -- exists
  • [x] src/routes/dashboard/+page.svelte -- exists
  • [x] src/routes/tags/+page.svelte -- exists
  • [x] package.json -- has @auth/sveltekit: ^1.11.1
  • [ ] MISSING: src/routes/notes/[slug]/edit/+page.svelte -- exists, has server data, needs client-side migration
  • [ ] MISSING: src/routes/tags/[name]/+page.svelte -- exists, has server data, needs client-side migration
  • [ ] MISSING: src/lib/components/QuickJot.svelte -- uses fetch('/api/notes') which will break when src/routes/api/ is removed. Must migrate to api-client.js
  • [ ] MISSING: src/lib/slugCache.ts -- imports from $lib/api which is being removed. Needs migration or removal
Files to remove (verified with issues):
  • [x] src/routes/+layout.server.ts -- exists, loads session + projects via server-side API
  • [x] src/routes/+page.server.ts -- exists
  • [x] src/routes/boards/+page.server.ts -- exists
  • [x] src/routes/boards/[slug]/+page.server.ts -- exists
  • [x] src/routes/notes/+page.server.ts -- exists
  • [x] src/routes/notes/[slug]/+page.server.ts -- exists
  • [x] src/routes/projects/+page.server.ts -- exists
  • [x] src/routes/projects/[slug]/+page.server.ts -- exists
  • [x] src/routes/repos/+page.server.ts -- exists
  • [x] src/routes/search/+page.server.ts -- exists
  • [x] src/routes/dashboard/+page.server.ts -- exists
  • [x] src/routes/tags/+page.server.ts -- exists
  • [x] src/routes/signin/ -- exists (2 files)
  • [x] src/routes/signout/ -- exists (2 files)
  • [x] src/routes/api/ -- exists (4 server files across boards + notes)
  • [x] src/lib/api.ts -- exists, uses $env/dynamic/private (server-only)
  • [ ] MISSING: src/auth.ts -- Auth.js config file with SvelteKitAuth, Keycloak provider, JWT/session callbacks. Must be removed.
  • [ ] MISSING: src/hooks.server.ts -- imports and re-exports Auth.js handle. Must be removed.
  • [ ] MISSING: src/routes/notes/[slug]/edit/+page.server.ts -- exists, loads note + projects + tags for edit form
  • [ ] MISSING: src/routes/tags/[name]/+page.server.ts -- exists, loads notes filtered by tag name
Type export concern: Multiple components import types from $lib/api (BlockRenderer.svelte, NoteLayout.svelte, QuickJot.svelte, +page.svelte files). When api.ts is removed, these type imports break. The ticket should specify where types move (likely a new src/lib/types.ts or co-located in api-client.js).

Repo Placement

OK -- issue filed on forgejo_admin/pal-e-app, all file targets are within that repo.

Dependencies

  • BLOCKER: Issue #51 (convention-sveltekit-spa) -- explicitly declared dependency in Lineage. Issue #51 is still open. The convention-sveltekit-spa note does not exist in pal-e-docs. This ticket cannot proceed until the convention is written and approved. Board item #412 tracks this.
  • Board item #414 (adapter-static switch) -- downstream of this ticket. Correctly scoped as separate. No conflict.
  • Board item #297 (kanban prototype) -- in_progress, no conflict.
  • Keycloak client creation -- acceptance criteria says "Keycloak client pal-e-app created with correct redirect URIs." This is infrastructure work that should either be documented as a prerequisite or explicitly included in the file targets (e.g., a script or Terraform resource).

Acceptance Criteria

  • [x] "keycloak-js handles login/logout/token refresh" -- testable via e2e
  • [x] "All routes load data client-side with Bearer tokens" -- testable via e2e
  • [x] "No +page.server.ts files remain" -- testable via find/glob
  • [x] "Auth.js fully removed from package.json and code" -- testable via grep
  • [x] "Public routes still work without login" -- testable via e2e
  • [x] "Authenticated routes redirect to Keycloak login" -- testable via e2e
  • [ ] "Keycloak client pal-e-app created with correct redirect URIs" -- NOT testable by agent without Keycloak admin access. Needs clarification: is this done via Terraform, manual admin console, or a script? Which redirect URIs?
  • [ ] MISSING criterion: "No $env/dynamic/private imports remain" -- server-only env vars must all be replaced with VITE_ prefixed public env vars
  • [ ] MISSING criterion: "Type imports resolve after api.ts removal" -- components using types from $lib/api must still compile

Blast Radius

  • QuickJot component -- uses fetch('/api/notes') proxy route. When src/routes/api/ is removed, QuickJot breaks. Not mentioned in file targets.
  • Keycloak realm mismatch -- current auth.ts uses master realm. The issue says "pal-e realm." If the realm hasn't been created yet, this is additional infrastructure work not scoped in the ticket.
  • $env/dynamic/private to public env migration -- api.ts uses PAL_E_DOCS_API_URL from server-side env. The client-side replacement needs VITE_PAL_E_DOCS_API_URL or equivalent. This affects k8s deployment config (even though k8s/ is out of scope, the env var naming must be documented).
  • Type system -- 6 files import TypeScript types from $lib/api. The new api-client.js is JavaScript, not TypeScript. Type definitions need a new home.
  • No other pal-e repos affected -- this is frontend-only, no backend API changes needed.

Recommendation

Seven issues must be resolved before this ticket is READY:
  • Add missing file targets: src/auth.ts, src/hooks.server.ts, notes/[slug]/edit/, tags/[name]/, QuickJot.svelte, slugCache.ts
  • Document type migration strategy: Where do TypeScript types go when api.ts is removed? Recommend a new src/lib/types.ts
  • Clarify Keycloak realm: Is it pal-e (stated) or master (current)? How is the client created?
  • Add missing acceptance criteria: No $env/dynamic/private remaining; type imports resolve
  • Add story label: Board item #413 needs a story:X label for traceability
  • Wait for #51: convention-sveltekit-spa must be written first (declared dependency)
  • Document env var naming: Which VITE_ env vars replace server-side env vars?