Repository navigation
STU-386: Restyle magic-link sign-in and preserve return-to-task - #82
Conversation
Applies the redesign's design tokens and accessible-modal contract (STU-374's shared sharedControls.js primitive) to the sign-in modal's markup and styles, and fixes two behavioral gaps in the underlying magic-link flow: - The submit button was permanently disabled after a successful send, so a user who wanted to resend or use a different address had no way back in. It now always re-enables, with a transient "Sent!" label. - Sign-in requests from the composer, plans, or billing surfaces had no memory of where they came from. openSignInModal() now records the current view (via the existing hash-based switchView router) in localStorage before opening, and a successful magic-link verification restores that surface and refreshes usage/plan state — without triggering checkout or deploy. - A failed/expired token now reopens the sign-in modal with the error shown inline (in addition to the existing toast), so retry is a click away instead of a dead end. - STU-149's alias-policy error already had a message path; it now also marks the input aria-invalid and preserves the rejected address. Existing auth endpoints, request/response contracts, and token stripping are unchanged. Scope is limited to sign-in markup/styles and the UI state adapter, per file ownership. Adds e2e/stu386-magic-link-signin.spec.js covering valid/invalid email, send failure + retry, sent + resend, alias-policy rejection, token failure, successful session refresh, and the Plans return-to-task round trip, plus new deterministic fixtures in e2e/fixtures/apiFixtures.js. Linear: STU-386
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| consumeSignInReturnSurface() | ||
| const message = err.message || 'Sign-in link invalid or expired.' | ||
| showToast(message, 'error') | ||
| // Surface the failure inline too, with the retry control right there, | ||
| // instead of leaving the user to hunt for a way back in. | ||
| openSignInModal() |
There was a problem hiding this comment.
When verification fails, consumeSignInReturnSurface() deletes the initiating surface before openSignInModal() records the callback page’s empty hash as #home. If a user started on Plans or Account, follows an expired link, and then successfully retries from the reopened modal, the new link returns them Home instead of to their original task. Preserve the stored surface through the inline retry rather than consuming and replacing it.
Greptile automatically discovered a related ticket stating that expired-link retry must preserve the initiating surface, which informed this comment.
Source Used: Linear — Restyle magic-link sign-in and preserve return-to-task behavior
Prompt To Fix With AI
This is a comment left during a code review.
Path: app.js
Line: 4278-4283
Comment:
**Retry loses return surface**
When verification fails, `consumeSignInReturnSurface()` deletes the initiating surface before `openSignInModal()` records the callback page’s empty hash as `#home`. If a user started on Plans or Account, follows an expired link, and then successfully retries from the reopened modal, the new link returns them Home instead of to their original task. Preserve the stored surface through the inline retry rather than consuming and replacing it.
Greptile automatically discovered a related ticket stating that expired-link retry must preserve the initiating surface, which informed this comment.
**Source Used:** Linear — [Restyle magic-link sign-in and preserve return-to-task behavior](https://linear.app/stuart-gardoll/issue/STU-386/restyle-magic-link-sign-in-and-preserve-return-to-task-behavior)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
…ui-ux' into sgardoll/stu-386-restyle-magic-link-sign-in-and-preserve-return-to-task # Conflicts: # dist/assets/index-D6csqwUm.js # dist/index.html
The tier label was unified to Free/Pro/Power by STU-382, which landed on the integration branch after this PR was cut. The assertion passed on this branch's original base (which rendered "Professional") and fails on the merged tree — the sole failure, 89/90. Found by an independent reviewer building the real merged tree rather than testing the branch in isolation. This merge commit also brings the branch up to date with the integration branch (merge, not rebase). The only conflicts were in generated dist/ artifacts, which CI rebuilds; they were resolved to the integration branch's version.
73290e3
into
milestone/stu-implement-updated-ui-ux
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| openSignInModal() | ||
| setSignInMessage(message, 'error') |
There was a problem hiding this comment.
🟡 First-visit token failure hides retry
For first-time free users, openSignInModal() displays the token error before showWalkthroughIfNeeded replaces it. The user sees onboarding instead of retry controls.
Learn more
The shared modal manager allows only one active modal. Opening a second modal forcibly closes the first through openModal. After token verification fails, these lines open the sign-in modal, but startup continues and showWalkthroughIfNeeded opens onboarding for users without hasSeenWalkthrough. The E2E setup masks this because every new test pre-populates that key.
Example: A new free user opens an expired link. The sign-in error appears during initializeAuth, then the walkthrough replaces it later in the same startup sequence. The expected inline retry is no longer visible.
Recommended fix: Suppress or defer showWalkthroughIfNeeded() when magic-link verification has opened the sign-in recovery modal. Add an E2E case without hasSeenWalkthrough.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (returnHash && !window.location.hash) window.location.hash = returnHash | ||
| } catch (err) { | ||
| showToast(err.message || 'Sign-in link invalid or expired.', 'error') | ||
| consumeSignInReturnSurface() |
There was a problem hiding this comment.
🟡 Expired-link retry loses return task
After a token fails, consumeSignInReturnSurface() deletes the initiating surface. openSignInModal() then records home, so a replacement link cannot return to the task.
Learn more
A return surface is consumed by reading and immediately deleting its storage entry. The failure path discards that value, then calls openSignInModal, which stores the callback page's hash or #home. A successful replacement link therefore restores the callback location instead of the surface where sign-in began.
Example: A user starts sign-in from Plans and opens an expired link without a hash. The failure consumes #plans, then retry records #home. After requesting and opening a new link, the app lands on Home instead of Plans.
Recommended fix: Preserve a valid return hash across verification failure and inline retry. Either leave the entry unconsumed on failure, or pass the consumed hash into the retry modal and write it back without replacing it with the callback location.
Was this helpful? React with 👍 or 👎 to provide feedback.
| localStorage.setItem(SIGNIN_RETURN_STORAGE_KEY, JSON.stringify({ | ||
| hash: window.location.hash || '#home', | ||
| ts: Date.now(), | ||
| })) |
There was a problem hiding this comment.
🟡 Concurrent sign-ins swap return destinations
Opening openSignInModal() in another tab overwrites the shared return record. Either tab's magic link then restores the most recently recorded surface.
Learn more
localStorage is shared by every tab on the same origin, and this implementation stores only one return hash. The magic-link callback has no flow-specific key, so it cannot associate a link with the tab or request that created it. Any later modal opening replaces the destination for all outstanding links.
Example: Alice opens sign-in from Plans in tab A, then opens sign-in from Account in tab B. Tab B overwrites #plans with #account. Opening tab A's email link now returns to Account.
Recommended fix: Correlate each magic-link request with a unique flow identifier carried through the callback and key return data by that identifier. If the backend contract cannot carry state, use per-tab storage and ensure links return to their originating tab rather than relying on a singleton origin-wide record.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const { hash, ts } = JSON.parse(raw) | ||
| if (!hash || typeof ts !== 'number' || Date.now() - ts > SIGNIN_RETURN_TTL_MS) return null | ||
| return hash |
There was a problem hiding this comment.
🔍 Stored return hashes lack router validation
The return record accepts any nonempty hash, while switchView() supports only home, account, and plans. Corrupt or legacy values can leave the URL inconsistent with the visible view.
Was this helpful? React with 👍 or 👎 to provide feedback.
| test("shows an inline alias-policy error and keeps normal addresses working", async ({ page }) => { | ||
| await applyDefaultRoutes(page, { | ||
| [ENDPOINTS.identity]: guestIdentity(), | ||
| [ENDPOINTS.getSubscription]: freeSubscription(), | ||
| [ENDPOINTS.authSendMagicLink]: magicLinkAliasRejected("user+tag@gmail.com"), | ||
| }); | ||
|
|
||
| await page.goto("/#account"); | ||
| await page.locator("#auth-signedout button", { hasText: "Sign In" }).click(); | ||
|
|
||
| const input = page.locator("#signin-email-input"); | ||
| const submit = page.locator("#signin-submit-btn"); | ||
| await input.fill("user+tag@gmail.com"); | ||
| await submit.click(); | ||
|
|
||
| // STU-149's alias-policy error is surfaced inline, not swallowed. | ||
| await expect(page.locator("#signin-message")).toContainText("primary email address"); | ||
| await expect(input).toHaveAttribute("aria-invalid", "true"); | ||
| await expect(submit).toBeEnabled(); | ||
| // The address is preserved so the user can see what was rejected. | ||
| await expect(input).toHaveValue("user+tag@gmail.com"); | ||
| }); |
There was a problem hiding this comment.
🔍 Alias recovery remains untested
The alias test only submits the rejected address, despite claiming normal addresses still work. It misses correction and successful resubmission after rejection.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Restyles the sign-in modal with the redesign's design tokens and STU-374's shared accessible-modal primitive, and fixes two behavioral gaps in the magic-link flow:
openSignInModal()now records the current surface (home/composer, plans, or account/billing — tracked via the app's existing hash-basedswitchViewrouter) inlocalStoragebefore opening. A successful magic-link verification restores that surface and refreshes usage/plan state, without triggering checkout or deploy.PLUS_ALIAS_REJECTEDmessage already flowed through; it now also marks the inputaria-invalidand preserves the rejected address so the user can see what was rejected.Existing auth endpoints, request/response contracts, and token-stripping behavior are unchanged. Scope is limited to sign-in markup/styles and the UI state adapter, per the ticket's file ownership — no changes to generation/pipeline, results/artifact inspection, deployment UI, or the billing/plans surfaces themselves beyond wiring their existing "Sign In" triggers.
Acceptance criteria
e2e/stu386-magic-link-signin.spec.js(7 new tests, all passing) and new fixtures ine2e/fixtures/apiFixtures.js(magicLinkAliasRejected,magicLinkSendFailure,magicLinkVerifyFailure).src/sharedControls.js(STU-374) unchanged;scripts/test-accessible-modals.mjs(which exercisessignin-modalspecifically) passes.Test plan
npm test— 288/288 unit tests passnpm run test:ui(Playwright) — 20/20 pass (7 new + 13 existing)npm run test:accessibility— passes, includingsignin-modalnpm run test:key-storage— passespython3 scripts/test-deploy-guard.py— 9/9 passnpm run build— succeeds;dist/regenerated and committed per repo conventionNotes / deviations
openSignInModal()(paywall in composer,startCheckoutfor plans, and the Account view's own "Sign In" button) needed no per-call-site changes — the return-surface capture readswindow.location.hashat the moment the modal opens, which the app's existing router already keeps in sync with the visible surface.openCustomerPortal()'s sign-in fallback exists for session-expiry edge cases but its own "Manage Subscription" button is only rendered once already signed in, so it wasn't independently exercised in e2e; the underlyingopenSignInModal()behavior it relies on is covered via the other call sites.Linear: STU-386
https://linear.app/stuart-gardoll/issue/STU-386