Review: Cleanup: Keycloak link nits (DRY, coach role, a11y)

review-269-2026-03-27 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — plan-wkq Phase 11
  • [x] Repo — forgejo_admin/westside-app
  • [x] User Story — present
  • [x] Context — present
  • [x] File Targets — present (but paths are wrong)
  • [x] Acceptance Criteria — present (4 items)
  • [x] Test Expectations — present
  • [x] Constraints — present
  • [x] Checklist — present
  • [x] Related — present

Traceability

  • [ ] story:X label — missing from board item labels. Labels are type:cleanup,scope:phase-11. No story label.
  • [ ] arch:X label — missing from board item labels. Should reference the westside-app arch component.
  • [x] Forgejo issue — westside-app#69, open

File Targets

  • [ ] src/routes/+layout.svelte — ISSUE: Wrong path. This file is only 7 lines (bare root layout with no Keycloak references). The actual file is src/routes/(app)/+layout.svelte (144 lines, contains the hardcoded Keycloak account URL at line 132 and the role-based nav blocks).
  • [ ] src/routes/admin/+page.svelte — ISSUE: Wrong path. This file does not exist. The actual file is src/routes/(app)/admin/+page.svelte (111 lines, contains the hardcoded Keycloak admin console URL at line 102).

Constraint Issue: Constants Not Exported

The ticket says "Use existing KEYCLOAK_URL and REALM constants from $lib/keycloak.js." However, these constants are not exported — they are module-private const declarations (lines 9-10 of src/lib/keycloak.js). The agent will need to add export to these constants, which is a code change not mentioned in File Targets. src/lib/keycloak.js should be listed as a file target.

Acceptance Criteria Detail

  • [x] "Keycloak URLs imported from $lib/keycloak.js constants (DRY)" — Verifiable. Two hardcoded URLs confirmed: layout line 132, admin line 102. But requires exporting the constants first (see above).
  • [x] "Account link visible for coach role (not just player/parent)" — Verifiable. Coach nav block (layout lines 115-125) currently has only "Team" and "Sign Out". Player nav (lines 126-141) has the Account link. Adding to coach is straightforward.
  • [x] "Inline style replaced with CSS class" — Verifiable. Admin page line 101 has style="margin-top: 0.5rem;". However, the ticket should specify WHICH inline style — there are 3 inline styles in the admin page (lines 71, 101, 107). Lines 71 and 107 are dynamic/contextual; line 101 is the likely target but should be explicit.
  • [x] "aria-label on external links indicating new tab" — Verifiable. Neither external link currently has an aria-label.

Repo Placement

OK. Issue is filed on westside-app and all changes are in westside-app. Single repo.

Dependencies

No blocking dependencies found. Nothing in in_progress column. The two items in next_up are unrelated (Svelte promotion prep and basketball-api reconciliation fix). PR #68 (referenced parent) is already merged.

Blast Radius

Low. Exporting KEYCLOAK_URL and REALM from keycloak.js is additive — existing internal references won't break. Adding an Account link to coach nav is isolated to the layout. The inline style change is cosmetic.
No other files in the app hardcode the Keycloak base URL beyond the two identified.

Decomposition

Not needed. 3 file targets (corrected), single repo, 4 acceptance criteria, well under 5 minutes of agent work.

Recommendation

Four issues must be fixed before this ticket is READY:
  • Fix file paths: Change src/routes/+layout.svelte to src/routes/(app)/+layout.svelte and src/routes/admin/+page.svelte to src/routes/(app)/admin/+page.svelte.
  • Add missing file target: Add src/lib/keycloak.js — export the KEYCLOAK_URL and REALM constants (currently module-private).
  • Clarify inline style target: Specify which inline style on the admin page to replace (line 101 margin-top: 0.5rem is the likely candidate; lines 71 and 107 are dynamic).
  • Add traceability labels: Board item needs story:X and arch:X labels.