Review: hostPath mount for basketball-api email templates

review-929-2026-04-10 Doc

review needs-refinement

Verdict: NEEDS_REFINEMENT

Board item #929 on board-westside-basketball — Forgejo: forgejo_admin/pal-e-deployments#105. Issue is substantively sound and executable; refinements are metadata/traceability only. Scope, file targets, and technical approach all verified against the live filesystem.

Template Completeness (template-issue-feature)

  • [x] Type — "Feature"
  • [x] Lineage — standalone, 2026-04-10 Westside Ops session, Lucas quote captured
  • [x] Repo — forgejo_admin/pal-e-deployments
  • [x] User Story — Lucas iterating on email copy, in-session
  • [x] Context — exceptionally thorough; names current state, code references, pattern precedent, misnamed overlay disambiguation
  • [x] File Targets — 1 file to modify + archbox directory to create, explicit "do NOT touch" list
  • [x] Acceptance Criteria — 8 criteria, all observable
  • [x] Test Expectations — 5 manual tests + regression list
  • [x] Constraints — 5 constraints including readOnly, keep baked-in fallback, no code changes
  • [x] Checklist — 6 items
  • [x] Related — project, unblocks, precedent files

Traceability

  • [x] Forgejo issue — forgejo_admin/pal-e-deployments#105, open
  • [~] story:ops-hygiene label — NOT in project-westside-basketball user-stories taxonomy. All stories use the WS-S{n} numbered convention. The work actually maps to story:WS-S1 (As superadmin, I want to deploy platform changes via IaC so that infrastructure is reproducible and auditable) or arguably story:WS-S7 (admin wants to send branded email announcements — the feedback loop for that story). "ops-hygiene" appears to be an ad-hoc label.
  • [ ] story note verified — MISSING. No ops-hygiene entry on project-westside-basketball user-stories section.
  • [x] arch:basketball-api label — matches the repo that owns the email sender
  • [ ] arch note verified — MISSING. search_notes(query="arch-basketball-api") returned zero results. This gap is foundational (affects every basketball-api ticket) — acceptable to defer but worth flagging.

File Targets

  • [x] pal-e-deployments/overlays/basketball-api/prod/deployment-patch.yaml — verified present. Lines 72-73 contain BASKETBALL_EMAIL_TEMPLATES_DIR=/app/templates/email/compiled/ exactly as the ticket claims.
  • [x] basketball-api/src/basketball_api/services/email.py:1107 — verified: def load_email_template(template_name: str, data: dict) -> str: exists at line 1107. Docstring confirms it reads from the env-configured dir. Ticket correctly says no code changes required.
  • [x] Precedent: overlays/svelte-playground/prod/deployment.yaml:50 uses hostPath: /home/ldraney/svelte-playground/dist — pattern match confirmed.
  • [x] Precedent: overlays/pal-e-streamlit/dev/deployment.yaml:93 uses hostPath: /home/ldraney/pal-e-streamlit — pattern match confirmed.

Repo Placement

OK. Overlay lives in pal-e-deployments; the Forgejo issue is correctly filed there. No basketball-api code changes, so no second issue needed.

Dependencies

Prerequisite sequencing is called out inside the ticket itself: initial templates MUST be copied from the running pod to /home/ldraney/basketball-api-email-templates/ before the overlay is rolled out, or sends will fall back to plain text. This is documented in both Context and Constraints sections. No external board-item dependencies — self-contained.

Acceptance Criteria

All 8 ACs are observable. The critical loop — edit template → test send to draneylucas@gmail.com → confirm text change → revert → re-send — is the strongest part of the ticket because it proves the "no caching" claim from Context. The "readOnly mount" and "baked-in fallback preserved" constraints are verifiable by inspecting the merged manifest and the container filesystem post-rollout.

Blast Radius

Low. Scope is a single overlay, single service, single namespace. Rolling update window ~30s. Concerns worth noting in the PR review (not blockers):
  • Single-node k3s — hostPath is safe here; flag if the cluster ever scales out.
  • 9 email send functions listed in Test Expectations regression check — good coverage.
  • The misnamed westside-email overlay is explicitly scoped OUT, with a recommendation for a separate rename ticket. This is correct discipline.

Decomposition

No decomposition needed. 1 file target across 1 repo. 8 ACs, but ACs 4-6 are one logical test flow (edit, send, verify, revert) and ACs 7-8 are pre-rollout setup checks. Estimated agent work: <5 min for the YAML change, plus manual kubectl cp + test-send (which Lucas/Ava performs, not the agent). Fits the 5-minute rule.

Recommendation

  • [LABEL] Replace story:ops-hygiene with story:WS-S1 (IaC/superadmin deploy) on board item #929. If Ava prefers, story:WS-S7 (admin branded email) is also defensible — this is the admin-facing feedback loop that story depends on. Ava decides.
  • [SCOPE] (optional, foundational) Create backing story note: if ops-hygiene is meant to be a cross-cutting "keep the lights on" story category, add it to project-westside-basketball user-stories section under Superadmin. Otherwise retire the label.
  • [SCOPE] (optional, foundational) Create architecture note arch-basketball-api in pal-e-docs. This gap affects every basketball-api ticket, not just this one — worth a separate backlog item rather than blocking #929.
  • [BODY] (nit, not required) The ticket says "update BASKETBALL_EMAIL_TEMPLATES_DIR env var to point at the mount path" but also mentions pointing /app/templates/email/live → hostPath. Recommend the dev agent standardize on a single target path (e.g. /app/templates/email/live/) and update the env var to match, keeping /app/templates/email/compiled/ as the baked-in fallback inside the image. The ticket already implies this, just make it one explicit line in the PR.
Summary for Ava: Ship it after the label swap. The technical scope is excellent — Lucas wrote this one with the verified code references baked in, and the hostPath precedent is real. The only true refinement is the story:ops-hygiene label which doesn't trace to the WS-S taxonomy. Don't block on the missing arch-basketball-api note; that's a platform-wide gap, not a this-ticket gap.