Conversation
🦋 Changeset detectedLatest commit: 4547ca5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
a4df941 to
55b6696
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a keyless workflow to the Astro integration: a file-backed keyless service and runtime key resolution utilities with a development-time keyless fallback. Middleware performs per-request keyless resolution and stores keyless data in Astro locals; server/client env merging and get-safe-env prefer injected keyless publishable keys. Integration config/schema accept optional PUBLIC_CLERK_KEYLESS_CLAIM_URL, PUBLIC_CLERK_KEYLESS_API_KEYS_URL, and PUBLIC_CLERK_KEYLESS_DISABLED and make PUBLIC_CLERK_PUBLISHABLE_KEY and CLERK_SECRET_KEY optional. Exports canUseKeyless, adds Playwright tests for keyless flow, and updates Vite and tsup externals. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
📝 WalkthroughWalkthroughThis pull request introduces keyless mode support for the Astro framework. Changes include adding new environment configuration properties for keyless URLs and disabled flags, implementing keyless service initialization with file storage, creating utilities for resolving keys with a keyless fallback mechanism, integrating keyless resolution into the middleware for per-request handling, and adding feature flags to control keyless availability based on environment and filesystem support. A new end-to-end test suite validates the keyless workflow, and the build configuration externals list is updated to include vite. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/astro/package.json`:
- Around line 98-102: The lockfile is out of sync with package.json
devDependencies: add vite@^7.1.0; regenerate and commit the updated lockfile
(e.g., run your package manager's install to update lockfile so it includes the
new "vite" entry) and ensure the changed lockfile is included in the PR; verify
the lockfile now contains the vite@^7.1.0 entry corresponding to the
"devDependencies" section in package.json.
In `@packages/astro/src/env.d.ts`:
- Around line 24-25: The InternalEnv interface is missing definitions for
PUBLIC_CLERK_KEYLESS_CLAIM_URL and PUBLIC_CLERK_KEYLESS_API_KEYS_URL which are
referenced by getContextEnvVar in get-safe-env.ts; add both as readonly optional
string properties to the InternalEnv interface (e.g.,
PUBLIC_CLERK_KEYLESS_CLAIM_URL?: string and PUBLIC_CLERK_KEYLESS_API_KEYS_URL?:
string) so keyof InternalEnv matches the keys used and TypeScript compilation
succeeds.
In `@packages/astro/src/server/get-safe-env.ts`:
- Around line 42-43: The calls to getContextEnvVar with
PUBLIC_CLERK_KEYLESS_CLAIM_URL and PUBLIC_CLERK_KEYLESS_API_KEYS_URL fail
TypeScript because those keys are not declared on the InternalEnv type; update
env.d.ts to add these two keys (PUBLIC_CLERK_KEYLESS_CLAIM_URL and
PUBLIC_CLERK_KEYLESS_API_KEYS_URL) to the InternalEnv interface so
getContextEnvVar(...) compiles, ensuring the names and types match other
PUBLIC_* URL entries already defined.
| readonly PUBLIC_CLERK_KEYLESS_DISABLED?: string; | ||
| } |
There was a problem hiding this comment.
Missing type definitions for keyless URL environment variables.
The InternalEnv interface is missing PUBLIC_CLERK_KEYLESS_CLAIM_URL and PUBLIC_CLERK_KEYLESS_API_KEYS_URL. These are used in get-safe-env.ts (lines 42-43, 61-62) with getContextEnvVar(), which has a parameter typed as keyof InternalEnv. This will cause TypeScript compilation errors.
🐛 Proposed fix
readonly PUBLIC_CLERK_TELEMETRY_DEBUG?: string;
readonly PUBLIC_CLERK_KEYLESS_DISABLED?: string;
+ readonly PUBLIC_CLERK_KEYLESS_CLAIM_URL?: string;
+ readonly PUBLIC_CLERK_KEYLESS_API_KEYS_URL?: string;
}🤖 Prompt for AI Agents
In `@packages/astro/src/env.d.ts` around lines 24 - 25, The InternalEnv interface
is missing definitions for PUBLIC_CLERK_KEYLESS_CLAIM_URL and
PUBLIC_CLERK_KEYLESS_API_KEYS_URL which are referenced by getContextEnvVar in
get-safe-env.ts; add both as readonly optional string properties to the
InternalEnv interface (e.g., PUBLIC_CLERK_KEYLESS_CLAIM_URL?: string and
PUBLIC_CLERK_KEYLESS_API_KEYS_URL?: string) so keyof InternalEnv matches the
keys used and TypeScript compilation succeeds.
54d9338 to
fc1064b
Compare
The options parameter in clerkClient was not providing any value because getSafeEnv(context) already reads all necessary configuration from both process.env and context.locals (which includes keyless values set by middleware). Changes: 1. clerk-client.ts: - Remove optional options parameter from clerkClient signature - Keep createClerkClientWithOptions internal signature for potential future use - Client configuration fully driven by getSafeEnv(context) 2. keyless/index.ts: - Remove ClerkAstroMiddlewareOptions import (no longer needed) - Remove options parameter from keyless() function - Remove options from clerkClient() calls - Simplify function signature 3. keyless/utils.ts: - Remove ClerkAstroMiddlewareOptions import - Remove options parameter from resolveKeysWithKeylessFallback() - Update keyless() call to not pass options 4. clerk-middleware.ts: - Remove options parameter from resolveKeysWithKeylessFallback() call Why this is correct: - getSafeEnv(context) reads from process.env via getContextEnvVar() - getSafeEnv(context) reads keyless values from context.locals - All necessary config (keys, urls, telemetry) is already available - The spread ...options at end of createClerkClient was never overriding anything useful - Simplifies the API and removes unnecessary parameters Benefits: ✅ Simpler API surface ✅ No unused parameters ✅ Clear single source of truth (getSafeEnv) ✅ Less confusing for developers ✅ All type checks pass
| PUBLIC_CLERK_KEYLESS_CLAIM_URL: envField.string({ | ||
| context: 'client', | ||
| access: 'public', | ||
| optional: true, | ||
| url: true, | ||
| }), | ||
| PUBLIC_CLERK_KEYLESS_API_KEYS_URL: envField.string({ | ||
| context: 'client', | ||
| access: 'public', | ||
| optional: true, | ||
| url: true, | ||
| }), |
| throw new Error( | ||
| 'Keyless mode requires a Node.js runtime with file system access. ' + | ||
| 'Set PUBLIC_CLERK_KEYLESS_DISABLED=1 or CLERK_KEYLESS_DISABLED=1 to disable keyless mode.', | ||
| ); |
There was a problem hiding this comment.
We should gracefully degrade here, not hard error.
There was a problem hiding this comment.
Fixed. Now falls back to in-memory storage for non-Node.js runtimes (e.g., edge environments) instead of throwing. Keys will persist for the process lifetime but won't be written to disk, allowing keyless mode to work gracefully in environments without file system access. 🤦🏼
There was a problem hiding this comment.
fixed! updated react-router one as well to match tanstack start's createFileStorage behavior
Description
Brings keyless mode support to
@clerk/astro, removing the need for API keys during development setup.Changes
@clerk/shared/keyless.env, you'll have to hard reload the browser for keyless to pick it up. I went with runtime middleware instead, similar to react-router and tanstack start keyless implementation.Data Flow
Server (Middleware):
resolveKeysWithKeylessFallback()- Checks for user keys, falls back to keyless servicekeyless()service - Creates/retrieves accountless application, persists to.clerk/context.locals(Astro's request-scoped storage)decorateRequest()- Transform HTML response, inject<script id="__CLERK_ASTRO_SAFE_VARS__">Client (Browser):
runInjectionScript()- Read config from DOMmergeEnvVarsWithParams()- Merge with integration paramsclerk.load()with keyless URLs for dev promptsChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Configuration
Infrastructure
Tests
Chores