Make route guards await the current user - #11
Merged
Conversation
The teacher guard decided from auth.user() synchronously while that value is populated by an asynchronous lookup, so on a cold load the decision could be made before the answer arrived. It now waits for the lookup, which is fetched at most once and shared. Note this was not reproducible: a browser test that reloads teacher URLs with /api/auth/me delayed by 700ms passes both with and without the change, so the race does not appear to fire in practice. The change is kept because a guard that races the very fact it depends on is wrong regardless, and the test is kept as regression cover for cold loads — reloads, bookmarks, and the second window used for projecting. The smoke test now also asserts that reloading a teacher view leaves no error banner behind, which nothing covered before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3wNzXNdrPw2Z59WjGHYv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Frontend-only hardening plus regression cover for cold loads.
This does not fix the "Session not found." / broken QR report — see the honest note below before merging with that expectation.
Change
teacherGuarddecided fromauth.user()synchronously, while that value is filled in by an asynchronous/api/auth/melookup. It now waits for that lookup, whichAuthService.ensureLoaded()performs at most once and shares between callers.A guard that races the very fact it depends on is wrong regardless of whether the race currently fires, and the shared fetch also removes a duplicate request on startup.
Honest assessment
I claimed this was a confirmed cause of the reported symptom. It is not, and the test proves it:
e2e/cold-load.spec.mjsreloads teacher URLs (/join,/control,/teacher) with/api/auth/medelayed by 700 ms to simulate a real connectionSo the race does not appear to fire in practice — Angular's initial navigation evidently does not beat the lookup, even when the lookup is slow. The change is kept on its own merits, not as a fix.
Regression cover added
cold-load.spec.mjs— cold loads of teacher URLs must not bounce a logged-in teacher to the login page, and the view must render without an error banner. Nothing covered bookmarks, reloads, or the second window used for projecting.smoke.spec.mjsnow asserts that reloading a session view leaves no error banner and the QR still renders. The suite previously never checked the error banner was absent, which is precisely the symptom reported.Testing
61 backend tests, 5 frontend unit tests, 5 browser specs — all green.
Still open
The reported "Session not found." with a broken QR, while the join URL renders, is unexplained.
/join-urlsucceeded on the same page and with the same cookie that/stateand/qr.svgapparently failed on. The status codes of those two requests from the browser's network tab would identify it: 401 points at an expired cookie (the session cookie lasts 12 hours), 404 at the session missing from the database, 500 at a server error with a traceback in the deployment log.🤖 Generated with Claude Code
https://claude.ai/code/session_018u3wNzXNdrPw2Z59WjGHYv
Generated by Claude Code