Review: T3 — basketball-api POST /api/jersey-public-orders

Verdict: NEEDS_REFINEMENT

Template Completeness

• [x] Type — Feature
• [x] Lineage — references #429, arch-jersey-intake, feedback_funnel_requires_auth
• [x] Repo — forgejo_admin/basketball-api
• [x] User Story — parent/player via westside-basketball realm
• [x] Context — explains revision from public to auth
• [x] File Targets — present (issues flagged below)
• [x] Acceptance Criteria — 12 items
• [x] Test Expectations — 9 tests + run command
• [x] Constraints — documented
• [x] Checklist — present
• [x] Related — story, arch, deps

Traceability

• [x] story:WS-S31 — verified in project-westside-basketball user-stories (stories-admin-list): "As an admin, I want a long-standing public jersey order link..."
• [x] arch:jersey-intake label on board item
• [ ] arch note MISSING — [SCOPE] Create architecture note arch-jersey-intake. search_notes returned empty.
• [x] Forgejo issue #430 valid and open
• [ ] [SCOPE] feedback_funnel_requires_auth note is cited as policy justification but search_notes returns empty. Either create the feedback note or correct the reference.

File Targets

• [x] src/basketball_api/routes/jersey_public.py — new file, correct placement under routes/.
• [x] src/basketball_api/main.py — exists; router registration pattern confirmed at lines 17, 67-94.
• [ ] src/basketball_api/schemas/jersey_public.py — ISSUE: the directory src/basketball_api/schemas/ does not exist. Repo convention (see routes/checkout.py lines 38-63) is inline Pydantic models per route module. Creating a new top-level schemas/ package breaks convention with no justification. [BODY] Drop the schemas file and declare models inline in jersey_public.py.
• [ ] src/basketball_api/routes/__init__.py — ISSUE: existing __init__.py is empty; routers are imported directly in main.py (e.g. line 17 from basketball_api.routes.jersey import router as jersey_router). No export needed. [BODY] Drop the __init__.py modification from File Targets.
• CRITICAL routes/checkout.py auth-pattern reference — see Blast Radius.

Repo Placement

Dependencies

• [ ] T2 / #429 — migration 014 for jersey_public_orders — CRITICAL: alembic slot 014 is already occupied by 014_add_password_reset_tokens.py. Repo is at 030 (030_add_registration_type_to_registrations.py). Next free slot is 031. The "014" label in T2 and T3 titles is factually wrong. This is primarily a T2 problem but T3's Lineage repeats the claim. [BODY] Update Lineage to reference the corrected migration number once T2 is refined; drop "014".
• [x] T3 board item 948 (backlog), T2 board item 947 (backlog). Wave sequencing correct (T2 wave:1 → T3 wave:2).

Acceptance Criteria

• Auth: 401 on missing header, 401 on invalid token — covered.
• Happy path: 201 with id/status/created_at — covered.
• submitter_keycloak_sub populated from JWT sub — covered.
• player_name/email NOT overridden from JWT — explicitly covered (important parent-for-child distinction).
• Field validation (kq, tier, preferred numbers, required fields) — covered.
• submission_ip from X-Forwarded-For first entry — covered.
• CORS for westsidekingsandqueens.tail5b443a.ts.net — covered.

Blast Radius

• grep -rn "keycloak_user" src/basketball_api/ returns ZERO hits. No such name exists.
• routes/checkout.py uses only Depends(get_db) and Depends(require_admin) (line 265). It has no Keycloak user dep on its public endpoints — checkout is unauth or admin-only.
• The actual JWT-validating dep is get_current_user in src/basketball_api/auth.py lines 77-159. It returns a User dataclass with fields sub, email, username, roles, validates RS256 via JWKS from the westside-basketball realm, accepts internal and external issuer URLs, raises 401 on missing/invalid token. This IS the correct primitive — user.sub maps directly to submitter_keycloak_sub.
• Real reference files that use the pattern: routes/account.py, routes/players.py, routes/subscriptions.py, routes/teams.py, routes/coaches_api.py, routes/tryouts.py, routes/upload.py.

• Dependency: get_current_user from basketball_api.auth
• Reference module: routes/account.py (or any of the routes listed above) as the real pattern
• Usage: user: User = Depends(get_current_user), then persist user.sub as submitter_keycloak_sub
• Remove the "System C pattern" framing — checkout is not a Keycloak auth reference.

Decomposition Assessment

Recommendation

• [BODY] Replace keycloak_user and routes/checkout.py auth-pattern references with get_current_user from basketball_api.auth. Point at routes/account.py as the reference. Remove "System C pattern" framing.
• [BODY] Drop src/basketball_api/schemas/jersey_public.py from File Targets; declare Pydantic models inline in jersey_public.py (matches checkout.py convention).
• [BODY] Drop the routes/__init__.py modification — file is empty and unused.
• [BODY] Update Lineage migration number from "014" to the correct next slot (031) once T2 is refined. Propagate to T2 review.
• [BODY] Reword test mock guidance to "use app.dependency_overrides[get_current_user] to inject a fake User; do NOT hit real Keycloak or JWKS in CI."
• [SCOPE] Create architecture note arch-jersey-intake in pal-e-docs (referenced but missing).
• [SCOPE] Create or verify the feedback_funnel_requires_auth note — cited as policy justification but not found via search.

Re-Review Verdict: APPROVED

Fix Verification

• [x] Context documents verified auth primitive get_current_user in src/basketball_api/auth.py lines 77-159 with User dataclass fields
• [x] File Targets reduced to only src/basketball_api/routes/jersey_public.py (new) and src/basketball_api/main.py (modify). No schemas/ file. No routes/__init__.py. Explicit 'do NOT touch' list added including routes/__init__.py
• [x] Keycloak integration section uses from basketball_api.auth import User, get_current_user and user: User = Depends(get_current_user) verbatim
• [x] Pydantic schemas section header states '(inline, in routes/jersey_public.py)' with full inline code block — no schemas/ directory referenced
• [x] Context explicitly states 'routes/checkout.py is NOT a valid reference for Keycloak user auth — it only uses get_db and require_admin'
• [x] Lineage references migration 031. Context states 'Migration head is 030... #429 creates migration 031'. No '014' anywhere in the body
• [x] Reference module is routes/subscriptions.py line 367 (cited in both Context and Related sections). Not checkout.py for auth
• [x] arch-jersey-intake exists in pal-e-docs (note id 1406, updated 2026-04-10). Earlier semantic_search false negative — note is present and covers all three systems plus the System B Keycloak-gated flow

Residual Notes

• Constraints still cite 'Match inline-Pydantic convention from routes/subscriptions.py and routes/checkout.py'. This is accurate — checkout.py does declare Pydantic models inline, which is distinct from the (now-corrected) auth-pattern claim. Not blocking.
• Pydantic code example uses constr(regex=...) which is Pydantic v1 syntax. If basketball-api is on Pydantic v2, the implementing agent will need constr(pattern=...). Not a scope-review blocker — agent can reconcile against installed version.
• feedback_funnel_requires_auth still cited in Lineage; verification of that note was not part of this re-review brief. Flagging as carry-over [SCOPE] from the first pass.