Review: Basketball-API client with Keycloak auth (re-review)

Verdict: READY

Re-review Context

Template Completeness

• [x] Type — Feature
• [x] Lineage — depends on #4 (scaffold), Keycloak client provisioned by #1
• [x] Repo — forgejo_admin/westside-ai-assistant
• [x] User Story — clear who/what/why for Marcus as admin
• [x] Context — Keycloak client credentials flow, caching, typed functions, tenant slug hardcoded
• [x] File Targets — specific files to create and modify, plus do-not-touch boundaries
• [x] Acceptance Criteria — 5 testable conditions
• [x] Endpoint Reference Table — NEW. Full HTTP method + path for all 14 operations
• [x] Test Expectations — 4 unit tests plus run command
• [x] Constraints — httpx, internal URL, Keycloak token URL, tenant slug, bulk note, client-only
• [x] Checklist — PR, tests, no unrelated changes
• [x] Related — project page and architecture doc linked

Traceability

• [x] story:read-ops — read operations for program data
• [x] story:write-ops — write operations for program data
• [x] arch:A3 — Basketball-API Client component in domain model
• [x] Forgejo issue — forgejo_admin/westside-ai-assistant#5, open

Previous Recommendations — Resolution

• [x] [BODY] Tenant resolution strategy — RESOLVED. Context says "Tenant slug is hardcoded to 'westside-kings-queens'". Constraints repeats it. Endpoint table bakes the slug into paths.
• [x] [BODY] Endpoint reference table — RESOLVED. Full table added with 7 reads and 7 writes, all with HTTP method + path.
• [x] [BODY] Rename assign_tryout_number to plural — RESOLVED. Now bulk_assign_tryout_numbers with explicit BULK note in Constraints.
• [x] [BODY] Clarify Keycloak client provisioning — RESOLVED. Lineage now says "Keycloak client provisioned by #1."

File Targets

• [x] app/basketball.py — to be created. Depends on #4 scaffold creating the app/ directory. Documented in Lineage.
• [x] tests/test_basketball.py — to be created. Test file for the client module.
• [x] requirements.txt — to be modified. httpx should already be present from #4.
• [x] Do-not-touch boundaries clearly stated (basketball-api repo, app/groupme.py).

Endpoint Table Deep Verification

• [x] GET /admin/dashboard — admin router prefix /admin + route /dashboard. No query params needed (auto-resolves tenant).
• [x] GET /admin/players — admin router prefix /admin + route /players. No query params needed.
• [x] GET /players/{player_id} — players router mounted at both /api/players and /players (SPA alias). Path works via SPA alias. Requires any auth.
• [x] GET /admin/teams — admin router prefix /admin + route /teams. Auto-resolves tenant. Returns teams+players for draft board.
• [x] GET /tenants/westside-kings-queens/roster — roster router prefix /tenants + route /{tenant_slug}/roster. Correct.
• [x] GET /api/subscriptions/overview — subscriptions router prefix /api/subscriptions + route /overview. Correct.
• [x] GET /api/subscriptions — subscriptions router prefix /api/subscriptions + route "". Correct.

• [x] PUT /players/{player_id} — SPA alias mount works. Requires any auth.
• [x] POST /teams/{team_id}/players — SPA alias mount works. Requires admin + ?tenant_id= query param. Body is {"player_ids": [...]} (list, not single). See Nit 1.
• [x] DELETE /teams/{team_id}/players/{player_id} — SPA alias mount works. Requires admin + ?tenant_id= query param. See Nit 1.
• [x] PATCH /admin/players/{player_id}/visibility — admin router. Correct.
• [x] POST /api/teams — teams router. Body includes tenant_id field. Correct.
• [x] POST /api/roster/westside-kings-queens/check-in/{player_id} — tryouts router (no prefix). Correct.
• [x] POST /tryouts/admin/westside-kings-queens/assign-numbers — tryouts router (no prefix). Correct. BULK operation documented.

Nits (non-blocking)

• Query param on team player operations: POST /teams/{team_id}/players and DELETE /teams/{team_id}/players/{player_id} both require a ?tenant_id= query parameter in the basketball-api code. Since the client hardcodes the tenant slug, the implementing agent needs to resolve the tenant's numeric ID or pass the slug. The existing endpoint accepts str for tenant_id, so passing the slug string should work. Not blocking because the agent will discover this from the basketball-api OpenAPI spec or error responses.
• Path prefix inconsistency: Some write paths use /teams/ and /players/ (SPA alias mounts) while others use /api/teams and /api/players. Both work — the SPA aliases are registered at lines 82-83 of main.py. Cosmetic only.
• assign_player_to_team body schema: The actual endpoint accepts {"player_ids": [...]} (a list), not a single player_id. The function name assign_player_to_team (singular) is fine as a client wrapper that takes a single ID and wraps it in a list. The agent can handle this.

Repo Placement

Dependencies

• [x] #4 (FastAPI scaffold + GroupMe webhook handler) — board item #596, column: backlog. This ticket cannot execute until #4 merges. Documented in Lineage.
• [x] #1 (Provision infrastructure) — provisions the Keycloak client westside-ai-bot. Now documented in Lineage.
• [x] #9 (basketball-api default-deny NetworkPolicy) — discovered scope item on the board. The AI assistant pod needs network access to basketball-api. This is an infra dependency that must be resolved before runtime validation, but not before code implementation.

Acceptance Criteria

Blast Radius

Decomposition

Recommendation