Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions backend/druks/api/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,12 @@ async def mcp_not_found(path: str) -> None:


def serve_spa(app: FastAPI, dist: Path = _SPA_DIST) -> None:
"""Serve the platform's own dashboard the way an app's ``dist/`` is
served — FastAPI's low-priority frontend routes, so every API path (and every
app frontend, registered earlier) wins, and unknown paths fall back to
index.html for client-side routing. A bare pip install ships no SPA build;
then the API serves JSON only."""
"""Serve the platform's own dashboard the way Druks serves an app's ``dist/``.
FastAPI serves both from low-priority frontend routes. Every API path wins over
them. Every app frontend registers earlier and also wins. An unknown path falls
back to index.html for client-side routing. The index.html fallback needs a
navigation request with an ``Accept: text/html`` header. A bare pip install
ships no SPA build. Without a build, the API serves JSON only."""
if (dist / "index.html").is_file():
app.frontend("/", directory=dist)

Expand Down
10 changes: 6 additions & 4 deletions backend/tests/test_spa_serving.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from druks.db import db_session
from fastapi import Depends, FastAPI, HTTPException
from fastapi.testclient import TestClient
from sqlalchemy.orm import Session


def _app_with_spa(dist: Path) -> FastAPI:
Expand All @@ -30,7 +31,7 @@ def test_spa_serves_index_assets_and_client_routes(tmp_path):
assert "druks" in client.get("/").text
assert client.get("/assets/app.js").status_code == 200
# An unknown path is a client-side route: index.html, not a 404.
assert "druks" in client.get("/work_items/42").text
assert "druks" in client.get("/work_items/42", headers={"Accept": "text/html"}).text


def test_api_paths_win_over_the_spa(tmp_path):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium. After the 0.162 bump, this test passes without the code it protects. TestClient sends Accept: */*. On 0.162, FastAPI never serves the index.html fallback for that header. So /api/nope returns 404 whether the /api/{path:path} catch-all exists or not.

I checked this: I deleted api_not_found from _app_with_spa and ran the file. On 0.139.0, this test fails, as it should. On 0.162.0, all 5 tests pass. The case the catch-all protects, which the comment on line 40 describes, is a browser navigation to an unknown /api/* path. Line 41 needs the same header as the client-route requests:

assert client.get("/api/nope", headers={"Accept": "text/html"}).status_code == 404

This goes past AC2, which kept this test unchanged. But it has the same cause and the same fix as AC1, and review.md item 14 requires a test to fail when the code it guards is gone. With the header, the test fails without the catch-all on both 0.139 and 0.162.

Expand All @@ -55,15 +56,16 @@ def test_cache_policy_lives_with_the_server(tmp_path):
assert asset.headers["cache-control"] == "public, max-age=31536000, immutable"
# index.html must revalidate every load or a deploy leaves stale bundles.
assert client.get("/").headers["cache-control"] == "no-cache"
assert client.get("/work_items/42").headers["cache-control"] == "no-cache"
assert (
client.get("/work_items/42", headers={"Accept": "text/html"}).headers["cache-control"]
== "no-cache"
)
# API responses carry no frontend cache policy; nor does a missing asset.
assert "cache-control" not in client.get("/api/thing").headers
assert "cache-control" not in client.get("/assets/gone.js").headers


def test_request_that_never_touches_the_db_opens_no_connection(monkeypatch):
from sqlalchemy.orm import Session

connected = []
original = Session._connection_for_bind

Expand Down
Loading