fix(auth): no sign-in error toast on storage-blocking browsers; App Store 1.0 metadata & screenshots - #163
Merged
Conversation
…rage Kiosk and e-ink browsers that block site data make the window.localStorage getter throw, so Clerk.load() rejected and pushed an error toast on devices where sign-in can never work. Probe storage access before loading and treat denial as the same deliberate disabled state as a missing publishable key. - probe site-storage access in clerkAuthentication before importing the SDK - add regression tests for the storage-denied and missing-key gates
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Clerk authentication loader now checks browser storage access before loading Clerk. It keeps authentication disabled when storage is unavailable or no publishable key exists. Tests cover both paths and verify logging, toast, state, and promise behavior. ChangesClerk authentication loading
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR prevents a persistent “Clerk failed to load” sign-in error toast on browsers that deny site storage (e.g., hardened/kiosk environments) by probing storage access before loading the Clerk SDK, and treating “cannot ever work here” as a deliberate auth-disabled state (similar to builds without a publishable key).
Changes:
- Add a site-storage access probe and gate
loadClerkAuthentication()early to skip loading Clerk (and its chunks) when storage is denied. - Preserve existing “toast + rethrow” behavior for genuine Clerk load failures.
- Add Vitest regression tests for the storage-denied and missing-publishable-key quiet-disable paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/lib/composables/clerkAuthentication.svelte.ts | Adds a storage-access probe and early-return gate to disable auth quietly (and avoid Clerk chunk downloads) when storage is denied. |
| frontend/src/lib/composables/clerkAuthentication.svelte.test.ts | Adds regression tests covering the new environment gates (storage denied / missing publishable key). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/lib/composables/clerkAuthentication.svelte.test.ts (1)
53-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that both gates avoid Clerk module imports.
These tests verify the quiet disabled state, but they do not prove that
@clerk/clerk-js,@clerk/ui, and@clerk/ui/themeswere not imported. Add module mocks or import spies that fail when any Clerk module evaluates. This protects the no-SDK-download objective. Vitest supports mocking modules loaded throughimport(). (vitest.dev)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/composables/clerkAuthentication.svelte.test.ts` around lines 53 - 85, Update both tests around loadClerkAuthentication to mock or spy on `@clerk/clerk-js`, `@clerk/ui`, and `@clerk/ui/themes` before dynamically importing the authentication module, with mocks that fail if any Clerk module evaluates. Verify the storage-access-denied and missing-VITE_CLERK_PUBLISHABLE_KEY paths complete without importing those modules while preserving their existing quiet-state assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/lib/composables/clerkAuthentication.svelte.test.ts`:
- Around line 28-32: Update restoreSiteStorageAccess() to handle an undefined
originalLocalStorageDescriptor by removing the own localStorage override
installed by denySiteStorageAccess(). Keep the existing descriptor restoration
path unchanged when a descriptor was originally saved, so inherited storage is
also restored for later tests.
---
Nitpick comments:
In `@frontend/src/lib/composables/clerkAuthentication.svelte.test.ts`:
- Around line 53-85: Update both tests around loadClerkAuthentication to mock or
spy on `@clerk/clerk-js`, `@clerk/ui`, and `@clerk/ui/themes` before dynamically
importing the authentication module, with mocks that fail if any Clerk module
evaluates. Verify the storage-access-denied and
missing-VITE_CLERK_PUBLISHABLE_KEY paths complete without importing those
modules while preserving their existing quiet-state assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 45a62c58-4d21-48c8-b868-1678278882cb
📒 Files selected for processing (2)
frontend/src/lib/composables/clerkAuthentication.svelte.test.tsfrontend/src/lib/composables/clerkAuthentication.svelte.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6045c4293e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ifacts The App Store Connect CLI writes run-state, reports, and release checkpoints under .asc/, and the screenshot pipeline produces raw captures, rejected candidates, and review sheets under screenshots/. These are reproducible working artifacts, not source. - Ignore .asc/runs/, .asc/reports/, .asc/release/checkpoints/ - Ignore screenshots/raw/, framed-test/, rejected/, review/, asc-review/ - Keep .asc/*.json config and screenshots/final|captions tracked
Release preflight and screenshot capture were run from local, untracked configuration, making the release process non-reproducible across machines. asc treats .asc/workflow.json as repo-local config that belongs in version control. - Add .asc/workflow.json with release-assets and iOS/Mac preflight workflows - Add .asc/screenshots.json capture plan for the iPhone welcome shot - Add .asc/shots.settings.json pipeline settings (raw -> final framing)
The App Store listing copy for the 1.0 release existed only on App Store Connect and in local working files, with no version-controlled source of truth. The asc metadata workflow treats ./metadata as the canonical local copy for validate/push/pull. - Add app-info/en-US.json with name, subtitle, and privacy policy URL - Add version/1.0/en-US.json with description, keywords, and URLs - Add release-notes/1.0/en-US.md What's New copy - Add RELEASE_READINESS.md summarizing 1.0 submission state
The final framed screenshot sets referenced by .asc/workflow.json were untracked, so the release-assets validation workflow had no reproducible inputs in version control. - Add five iPhone screenshots (IPHONE_69) under screenshots/final/en-US - Add five iPad screenshots (IPAD_PRO_3GEN_129) - Add three Mac screenshots (DESKTOP) - Add captions/en-US.json framing caption source
Browser-automation dogfood runs write annotated element-capture PNGs (normal-load.png, signin-modal.png) directly into screenshots/, next to the App Store asset directories. They are ephemeral debug evidence, on par with the ignored tmp/ and dogfood-output/ captures. - Ignore /screenshots/*.png (root level only; subdirectories unaffected)
…avior A missing publishable key returns quietly (deliberate disabled state) and never threw; only Clerk.load() rejections propagate. Document only the real thrown path.
…ests restoreSiteStorageAccess re-defined the saved own descriptor but left the throwing getter installed when window.localStorage came from the prototype chain, leaking the denial into later suites. Delete the override when no own descriptor existed so the inherited accessor is re-exposed.
WilliamAGH
changed the title
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
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.
Summary
Browsers that deny site storage (kiosk/e-ink display frames like TRMNL, or Chrome with "block all cookies") showed a persistent "Sign-in is unavailable: Clerk failed to load" error toast on every page load, even though the chat works fine unauthenticated there. The app now detects storage denial up front and quietly disables auth in those environments, exactly like a build without a publishable key. The PR also carries the complete App Store Connect 1.0 submission assets (metadata, screenshots, asc workflow).
Changes
Bug Fixes
window.localStorageproperty getter itself throws a SecurityError;@clerk/clerk-js6.25.10 reads it unguarded duringClerk.load(), so the load rejected and the catch pushed the user-facing toast. A storage-access probe now runs before the SDK is loaded, and a denied environment returns quietly — no toast, auth controls stay hidden, and aconsole.inforecords the browser's actual SecurityError for diagnostics (clerkAuthentication.svelte.ts—probeSiteStorageAccess,loadClerkAuthentication)Clerk.load()keep the existing toast + rethrow path; only the cannot-ever-work environment is gatedPerformance
loadClerkAuthentication)Tests
isLoadedstays false, SecurityError logged) and missing publishable key; the restore helper re-exposes an inheritedlocalStorageaccessor after the denial override so later suites see normal storage (clerkAuthentication.svelte.test.ts)Documentation
@throwscontract:loadClerkAuthenticationdocuments only the path that actually throws —Clerk.load()rejections — since missing-key and storage-denied environments return quietly by design (clerkAuthentication.svelte.ts)App Store release assets (chore)
metadata/for the App Store submissionscreenshots/(.asc/screenshots.json,.asc/shots.settings.jsonconfigure capture/framing).asc/workflow.json); transient asc run state and stray debug captures are now gitignored (.gitignore)Breaking Changes
None
Related Issues
None