fix(auth): gate email sign-in until its handlers are installed - #242
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a0a819a The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reached
Next review available in: 59 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚅 Deployed to the ePDS-pr-242 environment in ePDS
|
Coverage Report for CI Build 31088415828Coverage remained the same at 58.051%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR fixes a race on the auth-service login page where the email “Continue” button could be clicked before its JS submit handler was installed, causing the sign-in attempt to be silently dropped. It does this by disabling the submit button in the initial HTML and only enabling it after handler setup completes, and it adds coverage to lock in the ordering.
Changes:
- Render the email OTP submit button as
disabledon first paint and enable it only after installing the form submit handler. - Add a
<noscript>warning so users without JavaScript get an explicit explanation instead of a non-responsive flow. - Add unit tests that assert enablement happens after handler binding and that readiness is not driven by a timer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/auth-service/src/routes/login-page.ts |
Disables the email submit button in HTML, binds the submit handler via a stored form ref, then enables the button after handler setup; adds a noscript warning. |
packages/auth-service/src/__tests__/login-page.test.ts |
Adds tests that pin the “bind submit handler → enable button” ordering and ensure no timer-based readiness in that region. |
.changeset/sign-in-button-readiness.md |
Adds end-user-facing patch release notes describing the behavior change and the noscript experience. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Continue button was clickable from first paint, but its submit handler is installed at the end of a long inline script. A click landing in that window did nothing: the form had no handler yet, so the sign-in attempt was silently dropped. Fast e2e runs hit this routinely; real users on slow connections can too. Ship the button disabled and enable it as the last step of handler setup, so it becomes clickable exactly when it starts working. Add a noscript warning as well, since with scripting off the button would otherwise never respond and never explain why. The tests pin the ordering rather than the mechanism: the enable must follow the submit binding, and must not be driven by a timer. Split out of #148, which bundled this with unrelated enrichment work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b14ed8a to
a0a819a
Compare
|
Split 2 of 4 from #148. Independent of the other three — based directly on
main, mergeable in any order.The bug
The Continue button on the email sign-in form was clickable from first paint, but its submit handler is installed at the end of a long inline script. A click landing in that window did nothing at all: the form had no handler yet, so the sign-in attempt was silently dropped.
Fast e2e runs hit this routinely. Real users on slow connections can too.
The fix
Ship the button
disabledand enable it as the last step of handler setup, so it becomes clickable exactly when it starts working.Also adds a
<noscript>warning: with scripting off the button would otherwise never respond and never explain why.Tests
The tests pin the ordering rather than the mechanism — the enable must follow the submit binding, and must not be driven by a
setTimeout/setInterval. Anchoring on a timer would pass for the wrong reason, and the page legitimately usessetIntervalelsewhere for the PAR heartbeat, so the timer check is scoped to the gap between binding and enable.Verification
typecheck,lint,formatclean; 73 test files / 1124 tests pass.Series
epds_handle_modethrough the callback hop🤖 Generated with Claude Code