Skip to content

test(server): regression tests for SPA fallback on client-side routes (fabro-q5 brief) - #41

Open
zenprocess wants to merge 1 commit into
mainfrom
fix/spa-fallback-runs
Open

test(server): regression tests for SPA fallback on client-side routes (fabro-q5 brief)#41
zenprocess wants to merge 1 commit into
mainfrom
fix/spa-fallback-runs

Conversation

@zenprocess

Copy link
Copy Markdown
Owner

Summary

Regression tests for the SPA fallback that serves the app shell for client-side routes (/runs, /runs/<id>/stages, etc.). Brief: ao-company#306.

The server-side SPA fallback already lives in lib/apps/fabro-server/src/static_files.rs (the if accepts_html(headers) { load_asset_for_mode("index.html", …) } block at the end of serve_with_mode) and is wired into the router at lib/apps/fabro-server/src/server.rs:1878 (the else if web_enabled && matches!(req.method(), &Method::GET | &Method::HEAD) branch of the fallback service).

This PR adds the missing regression surface: tests that pin both halves of the contract, so a future revert of the fallback is caught.

Why

The operator's bookmarked /runs link currently works because the fallback is in place — but no test in the suite exercised it. Only a behavioral test catches a future revert. Four tests now do.

Acceptance

ulimit -n 4096 && cargo nextest run -p fabro-server spa_fallback_for_client_routes::

Expected: 4 passed, 0 failed.

Two-state proof (test fails when fallback is reverted)

Test run #1 — with the fallback REVERTED in lib/apps/fabro-server/src/static_files.rs (the if accepts_html(headers) { … } block commented out, no other code changed):

cargo test -p fabro-server --lib spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link

thread 'server::tests::spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link' (15384548) panicked at lib/apps/fabro-server/src/server/tests.rs:16808:9:
assertion `left == right` failed: GET /runs must return the SPA shell, not a 404 — deep links and bookmarks depend on this fallback
  left: 404
 right: 200

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 780 filtered out

Test run #2 — fallback restored, all four tests:

cargo test -p fabro-server --lib spa_fallback_for_client_routes

PASS [0.112s] spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link
PASS [0.122s] spa_fallback_for_client_routes::runs_route_404s_for_post_requests
PASS [0.131s] spa_fallback_for_client_routes::runs_route_404s_for_non_html_clients_to_avoid_silent_shells
PASS [0.141s] spa_fallback_for_client_routes::runs_detail_route_serves_spa_index_for_bookmarked_link

Summary [0.145s] 4 tests run: 4 passed, 777 skipped

This is the regression surface: a revert of the fallback produces a non-zero exit (base_rc != 0); with the fallback in place, exit 0 (head_rc = 0).

What the four tests pin

  • runs_route_serves_spa_index_for_bookmarked_link — GET /runs with browser Accept: text/html,… returns 200 + the embedded SPA shell. Primary regression guard.
  • runs_detail_route_serves_spa_index_for_bookmarked_link — deep route /runs/<id>/stages returns the same. The deep-link case is the operator's actual bookmark shape.
  • runs_route_404s_for_non_html_clients_to_avoid_silent_shells — GET /runs with Accept: application/json returns 404. Prevents widening the fallback so a stray fetch() / typo'd API request silently receives a 25 KB HTML shell.
  • runs_route_404s_for_post_requests — POST /runs returns 404. Only GET/HEAD reach the SPA fallback; this test fails if the method match is ever widened.

Diff scope

lib/apps/fabro-server/src/server/tests.rs | 106 ++++++++++++++++++++++++++++++
1 file changed, 106 insertions(+)

No production code change. The fallback was already correct; the brief's "hash-router" premise was wrong (the repo has used createBrowserRouter since 9b9ebdf), and the server already handled browser-router URLs.

Live path / unrelated notes

  • The Cargo.lock change that builds up under this worktree is an unrelated fabro-referee workspace entry that regenerates on build. Not part of this PR; reverted locally before the branch was pushed.
  • Brief's second change (gate POSTing /api/v1/runs/registrations, live since fabro server 38b26303 with zero prior callers) is being tracked separately on ao-company#306. End-to-end live path there is unproven today (sandbox-creation failures with firecracker snapshot-load 400); the brief instructed using a local mock stand-in and stating plainly that the live path is unproven — that change lives in the ao-company repo, not here.
  • qa-pipeline reports no-tests on this repo and exits 0 — a manufactured green. Not cited. Real runner is cargo nextest run -p fabro-server per the brief.

🤖 Generated with Claude Code

… (fabro-q5 brief)

The deployed build is a browser-router (createBrowserRouter in
apps/fabro-web/app/entry.tsx), not the hash-router the brief assumed.
URLs like /runs and /runs/<id>/stages are real server paths, and the
server already has the HTML fallback wired through static_files.rs.

These four tests lock that behavior in so a future revert is caught:

  runs_route_serves_spa_index_for_bookmarked_link
  runs_detail_route_serves_spa_index_for_bookmarked_link
  runs_route_404s_for_non_html_clients_to_avoid_silent_shells
  runs_route_404s_for_post_requests

Together they pin both halves of the contract (HTML routes resolve to
index.html; non-HTML and non-GET/HEAD keep getting 404). No server
code change: this is a pure regression-test-only addition per the
brief's thin-CI scope.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@zenprocess

Copy link
Copy Markdown
Owner Author

Evidence — reviewer could not fetch the diff blob

I am re-posting the acceptance evidence here so review does not have to wait for git push problems to clear. This is what re-running on a clean checkout should produce; the coach will independently re-run before accepting.

Acceptance command

ulimit -n 4096 && cargo nextest run -p fabro-server spa_fallback_for_client_routes::

Per the brief: cargo nextest run -p fabro-server (per-crate, not workspace). qa-pipeline reports no-tests on this repo and exits 0 — a manufactured green, not cited.

head_rc

PASS [0.112s] spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link
PASS [0.122s] spa_fallback_for_client_routes::runs_route_404s_for_post_requests
PASS [0.131s] spa_fallback_for_client_routes::runs_route_404s_for_non_html_clients_to_avoid_silent_shells
PASS [0.141s] spa_fallback_for_client_routes::runs_detail_route_serves_spa_index_for_bookmarked_link

Summary [0.145s] 4 tests run: 4 passed, 777 skipped

head_rc = 0.

base_rc — two-state proof that the test FAILS when the fallback is reverted

To produce this, I commented out the SPA fallback (if accepts_html(headers) { … } block at the end of serve_with_mode in lib/apps/fabro-server/src/static_files.rs), left all other code unchanged, and re-ran the relevant subset. The unmodified git tree is restored after the proof; this is captured for reproducibility.

State 1 — fallback REVERTED (Cargo.lock unchanged, server/tests.rs unchanged, only the if accepts_html(headers) { … } block commented out in static_files.rs):

cargo test -p fabro-server --lib spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link

thread 'server::tests::spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link' (15384548) panicked at lib/apps/fabro-server/src/server/tests.rs:16808:9:
assertion `left == right` failed: GET /runs must return the SPA shell, not a 404 — deep links and bookmarks depend on this fallback
  left: 404
 right: 200

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 780 filtered out

base_rc != 0 — the test fails when the fallback is removed.

State 2 — fallback RESTORED (this commit's source tree, as-pushed):

cargo test -p fabro-server --lib spa_fallback_for_client_routes

Summary [0.145s] 4 tests run: 4 passed, 777 skipped

head_rc = 0 — the test passes when the fallback is in place.

So the test produces base_rc != 0 under a hypothetical revert and head_rc = 0 on the committed tree. That is the regression surface the brief required.

How to reproduce independently

# Clean clone / fresh worktree, on this branch.
ulimit -n 4096 && cargo nextest run -p fabro-server spa_fallback_for_client_routes::
# expect: 4 passed, head_rc == 0

# To see the test catch a revert:
git checkout -b reproduce-tmp-fix/spa-fallback-runs
# edit lib/apps/fabro-server/src/static_files.rs and wrap the
# `if accepts_html(headers) { load_asset_for_mode("index.html", …) … }`
# block in /* … */ comments.
cargo test -p fabro-server --lib spa_fallback_for_client_routes::runs_route_serves_spa_index_for_bookmarked_link
# expect: 1 FAILED with `left: 404, right: 200`
git checkout fix/spa-fallback-runs
git branch -D reproduce-tmp-fix/spa-fallback-runs

What this PR contains

lib/apps/fabro-server/src/server/tests.rs | 106 ++++++++++++++++++++++++++++++
1 file changed, 106 insertions(+)

No production code change. The brief's "hash-router" premise was wrong (the repo has used createBrowserRouter since 9b9ebdf50), and the SPA fallback was already implemented at lib/apps/fabro-server/src/static_files.rs:132-141. The contribution is the regression surface.

🤖 Generated with Claude Code

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.

1 participant