pal-e-docs

Review: Fix remaining CORB warnings on cross-origin player photo loads

review-537-2026-03-28 Review

review approved
Project

Verdict: APPROVED

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- References #207 / PR #208
  • [x] Repo -- forgejo_admin/basketball-api
  • [x] User Story -- WS-S6 (coach/admin draft board photos)
  • [x] Context -- Thorough root cause analysis with three contributing factors
  • [x] File Targets -- 4 files across 2 repos, all with specific descriptions
  • [x] Acceptance Criteria -- 5 criteria, all testable
  • [x] Test Expectations -- Unit + integration + regression
  • [x] Constraints -- Cross-repo shipping, no directory changes, container mime DB independence
  • [x] Checklist -- Present
  • [x] Related -- References parent issue, PR, board, and story

Traceability

  • [x] story:WS-S6 label -- Coach/admin draft board reliability
  • [x] arch:basketball-api label -- Correct primary target repo
  • [x] Forgejo issue -- forgejo_admin/basketball-api#210, open

File Targets

  • [x] src/basketball_api/main.py -- verified: StaticFiles mount at line 101, CORSMiddleware at line 53. No explicit MIME override exists. This is where the fix belongs.
  • [x] src/basketball_api/routes/upload.py -- verified: content_type validation at line 42, extension validation at lines 34-38. ALLOWED_EXTENSIONS: .jpg, .jpeg, .png, .webp. Upload uses uuid-based naming preserving original extension.
  • [x] src/routes/(app)/admin/teams/+page.svelte -- verified: cross-origin img at line 199 (ticket says line 196, off by 3 lines -- minor, same code block). Hardcoded https://basketball-api.tail5b443a.ts.net URL, no crossorigin attribute.
  • [x] src/routes/(app)/players/[id]/+page.svelte -- verified: cross-origin img at lines 265 and 277 (exact match). Uses {API_BASE}{player.photo_url} pattern, no crossorigin attribute.

Repo Placement

OK. The issue is filed on basketball-api (backend fix). The ticket correctly identifies that the frontend fix (crossorigin attribute) ships in a separate westside-app PR. Both repos are identified. Two PRs are needed -- this is documented in the Constraints section.

Dependencies

  • #532 (done) -- "Fix: CORS headers missing on photo static files" -- the predecessor that moved StaticFiles to module level. Already merged. No blocker.
  • #529 (done), #530 (done) -- Related WS-S6 draft board work, both complete.
  • The backend and frontend fixes are independent of each other -- the backend MIME fix and frontend crossorigin attribute fix can ship in either order. Both improve the situation independently.
  • No items currently in in_progress that block this ticket.

Acceptance Criteria

5 acceptance criteria, all verifiable:
  • AC1-2 (zero CORB warnings) -- verifiable via browser DevTools console, though requires manual cross-origin page load. Not fully automatable in CI without a browser test harness.
  • AC3 (correct Content-Type headers) -- fully testable with a unit test hitting the static file endpoint.
  • AC4-5 (no regression) -- verifiable by existing test suite + manual photo upload flow.
The integration test (cross-origin CORB check) may be difficult to automate in CI -- an agent can verify Content-Type headers programmatically but CORB is browser-only behavior. This is acceptable for scope; the unit tests cover the root cause.

Blast Radius

  • Coach photos -- teams/[id]/+page.svelte line 60 and coaches/[id]/+page.svelte line 50 use src={coach.photo_url} without API_BASE prefix. If coach photo_url contains a cross-origin URL, those img tags also need crossorigin. Not in scope for this ticket but worth tracking as discovered scope.
  • Register page photo upload -- uses /api/register/upload-photo endpoint (different from /upload/photo). The MIME fix on the StaticFiles mount would apply to all photos served from /uploads/photos/ regardless of upload route, so this is covered.
  • No other services consume photos from basketball-api's static files mount.
  • Rollback is straightforward -- revert either PR independently.

Decomposition Assessment

4 file targets across 2 repos, 5 acceptance criteria. The ticket already prescribes splitting into two PRs (basketball-api backend + westside-app frontend). Each PR touches 1-2 files. Each sub-task fits well within the 5-minute rule. No further decomposition needed -- the two-PR approach is the right granularity. Agent dispatches: one for basketball-api, one for westside-app.

Recommendation

No action needed. Scope is solid, all file targets verified, traceability complete, fits two single-agent passes.
  • [BODY] Nit: line reference for admin/teams/+page.svelte says "line 196" but the img tag is at line 199. Not blocking -- same code block, agent will find it via grep.