Skip to content

fix(api+frontend): harden SPA catch-all routing, tests, and dev script cleanup#34

Merged
Olino3 merged 2 commits intofeature/angular-scaffoldfrom
copilot/sub-pr-33
Mar 20, 2026
Merged

fix(api+frontend): harden SPA catch-all routing, tests, and dev script cleanup#34
Olino3 merged 2 commits intofeature/angular-scaffoldfrom
copilot/sub-pr-33

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 20, 2026

Addresses all review comments from PR #33 on the Angular SPA serving implementation.

src/api/main.py

  • Favicon: serve spa_dist_dir/favicon.ico when present; fall back to 204. spa_dist_dir computed before the favicon route to enable this.
  • Reserved prefix: /api//api — closes the gap where bare /api or /api/v3 bypassed the check.
  • Path traversal: replaced ".." not in full_path with Path.resolve() + relative_to() — properly rejects any path escaping spa_dist_dir.
  • Asset 404: switched from "any dot in filename" to mimetypes.guess_type().js/.css missing files return 404, but valid Angular routes like /user/john.doe still fall through to index.html.
  • Non-blocking index.html: replaced synchronous read_text() + HTMLResponse with FileResponse.
  • /v3 redirect: added explicit GET /v3 → 307 /v3/ inside the SPA block — the catch-all was preventing Starlette's redirect_slashes from firing for this path.
  • Removed now-unused HTMLResponse import.
# Before: weak traversal guard, sync read, wrong prefix, broken /v3 redirect
if full_path and ".." not in full_path:
    file_path = spa_dist_dir / full_path
    ...
return HTMLResponse(content=index_file.read_text())

# After: resolve + relative_to, FileResponse, mimetypes-based 404
resolved = (_resolved_spa_dist_dir / full_path).resolve()
resolved.relative_to(_resolved_spa_dist_dir)          # raises ValueError on escape
...
if mimetypes.guess_type(full_path)[0] is not None:    # known MIME → missing asset
    return Response(status_code=404)
return FileResponse(index_file, media_type="text/html")

test/test_api_spa_mount.py

  • API interception test: corrected path to /api/v3/system/status; added status == 200 and content-type: application/json assertions.
  • v3 interception test: follow_redirects=False + assert status == 307 (previously the test passed vacuously — the catch-all was returning 404, not the real route).
  • Added test_spa_catch_all_does_not_intercept_bare_api and test_spa_missing_asset_returns_404.

frontend/src/app/app.spec.ts

  • Removed broken "should render title" test — template only renders <router-outlet />, not an <h1>.

scripts/dev/run_frontend_dev.sh

  • cleanup(): replaced kill 0 (kills entire process group) with a targeted loop that kills then waits each captured PID.

⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.

Copilot AI changed the title [WIP] Scaffold Angular 21 project with FastAPI serving fix(api+frontend): harden SPA catch-all routing, tests, and dev script cleanup Mar 20, 2026
Copilot AI requested a review from Olino3 March 20, 2026 18:38
@Olino3 Olino3 marked this pull request as ready for review March 20, 2026 18:48
@Olino3 Olino3 merged commit 34494b0 into feature/angular-scaffold Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants