fix(nextjs): Keyless drift detection telemetry bug from usage of async methods #6594
Conversation
🦋 Changeset detectedLatest commit: b8b72af 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.
|
📝 WalkthroughWalkthroughA new changeset file .changeset/sweet-vans-wonder.md was added documenting a patch release for @clerk/nextjs fixing keyless drift detection failure. In packages/nextjs/src/server/keyless-telemetry.ts, async fs calls in tryMarkTelemetryEventAsFired were replaced with synchronous variants: nodeFsOrThrow now exposes mkdirSync and writeFileSync; directory creation uses mkdirSync(..., { recursive: true }); flag file creation uses writeFileSync(..., { flag: 'wx' }). Control flow, return values, and telemetry/drift-detection logic are unchanged. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
heatlikeheatwave
changed the title
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.changeset/sweet-vans-wonder.md (1)
5-5: Polish the changeset summary for clarity and tense.Current phrasing is a bit clunky. Consider tightening and explicitly naming the sync APIs used.
-fix(nextjs): Fix for keyless drift detection telemetry event not firing due to usage of async methods in the keyless telemetry logic +fix(nextjs): Keyless drift detection telemetry event did not fire because async Node fs APIs were used; replace with mkdirSync/writeFileSync in keyless telemetry logicpackages/nextjs/src/server/keyless-telemetry.ts (5)
52-59: Remove stray await keywords around synchronous fs calls.
mkdirSyncandwriteFileSyncare synchronous and do not return Promises; awaiting them is a no-op and may confuse future readers and linters.Apply this diff:
- // Ensure the directory exists before attempting to write the file - await mkdirSync(flagDirectory, { recursive: true }); + // Ensure the directory exists before attempting to write the file + mkdirSync(flagDirectory, { recursive: true }); const flagData = { firedAt: new Date().toISOString(), event: EVENT_KEYLESS_ENV_DRIFT_DETECTED, }; - await writeFileSync(flagFilePath, JSON.stringify(flagData, null, 2), { flag: 'wx' }); + writeFileSync(flagFilePath, JSON.stringify(flagData, null, 2), { flag: 'wx' });Optional follow-up (if you want to fully de-async the helper): make
tryMarkTelemetryEventAsFirednon-async and drop theawaitat the single call site.-async function tryMarkTelemetryEventAsFired(): Promise<boolean> { +function tryMarkTelemetryEventAsFired(): boolean {- const shouldFireEvent = await tryMarkTelemetryEventAsFired(); + const shouldFireEvent = tryMarkTelemetryEventAsFired();
175-177: Reuse the EVENT_SAMPLING_RATE constant for consistency.Avoid magic numbers and single-source the sampling rate.
- telemetry: { - samplingRate: 1, - }, + telemetry: { + samplingRate: EVENT_SAMPLING_RATE, + },
10-31: Consider moving the flag file under os.tmpdir() to improve portability.Writing to
process.cwd()/.clerk/.tmpmay fail on some serverless hosts (read-only deploy assets). Using the OS temp directory reduces the chance of EACCES while keeping the per-process “once” semantics.If you want, I can open a follow-up PR to:
- switch to
join(os.tmpdir(), 'clerk', 'telemetry.json'),- ensure recursive dir creation,
- and update tests accordingly.
92-96: Nit: duplicate canUseKeyless guard.
detectKeylessEnvDriftalready early-returns when!canUseKeyless. The secondif (canUseKeyless)guard insidetryMarkTelemetryEventAsFiredis redundant unless you expect this helper to be reused elsewhere.Two options:
- Keep it for defensive programming, or
- Remove the inner guard to simplify control flow.
180-191: Add a targeted test to prevent regressions (event fired once, not more).This change fixes a production issue; adding a small test helps ensure we don’t regress. Suggested coverage:
- When drift is present, the first call records the event, subsequent calls do not.
- The helper gracefully handles pre-existing flag file (EEXIST) without throwing.
I can provide a Jest test that:
- stubs
nodeFsOrThrowto use real sync fs,- points
process.cwd()to a temporary directory,- spies on
clerkClient.telemetry.record,- asserts the single-record behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/sweet-vans-wonder.md(1 hunks)packages/nextjs/src/server/keyless-telemetry.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/sweet-vans-wonder.md
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/nextjs/src/server/keyless-telemetry.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/nextjs/src/server/keyless-telemetry.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: pr-title-lint
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
.changeset/sweet-vans-wonder.md (1)
2-2: Package and bump type look correct.Patch bump for @clerk/nextjs matches the scope and impact of the change.
packages/nextjs/src/server/keyless-telemetry.ts (1)
55-57: Good use of atomic creation via 'wx'.Using
{ flag: 'wx' }ensures only the first caller creates the flag file; subsequent attempts fail withEEXIST, which you already handle below.
| try { | ||
| if (canUseKeyless) { | ||
| const { mkdir, writeFile } = nodeFsOrThrow(); | ||
| const { mkdirSync, writeFileSync } = nodeFsOrThrow(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify nodeFsOrThrow exposes sync variants in all runtimes.
We now rely on mkdirSync and writeFileSync. Ensure nodeFsOrThrow() always returns these in supported Next.js server runtimes (Node.js only), and its type definition reflects that.
Run this repo-scoped script to confirm the implementation and typings of nodeFsOrThrow:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate nodeFsOrThrow definition and ensure it returns mkdirSync/writeFileSync:"
rg -nP -C3 'export\s+(function|const)\s+nodeFsOrThrow\b|nodeFsOrThrow\s*\(' --type ts
echo
echo "Check for sync fs members in the returned object:"
rg -nP -C2 'mkdirSync|writeFileSync' --type ts | sed -n '1,200p'Length of output: 10556
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate utils.ts in packages/nextjs/src/server/fs:"
fd --base-directory packages/nextjs/src/server/fs -t f utils.ts
echo
echo "Display first 200 lines of utils.ts to inspect nodeFsOrThrow definition and its typing:"
sed -n '1,200p' packages/nextjs/src/server/fs/utils.tsLength of output: 1302
Enforce FS sync methods in types
At runtime nodeFsOrThrow() returns the real Node.js fs module (which includes mkdirSync, writeFileSync, etc.), so your usages in keyless-telemetry.ts are safe. However, because we import #safe-node-apis with @ts-ignore, nodeFsOrThrow() is currently typed as any and does not guarantee those sync methods at compile time.
To fully verify and enforce that callers always have access to the sync variants, please:
- In
packages/nextjs/src/server/fs/utils.ts, change the signature ofnodeFsOrThrowto explicitly return Node’sfstype:// before const nodeFsOrThrow = () => { if (!nodeRuntime.fs) throwMissingFsModule('fs') return nodeRuntime.fs } // after const nodeFsOrThrow = (): typeof import('fs') => { if (!nodeRuntime.fs) throwMissingFsModule('fs') return nodeRuntime.fs }
- Add a TypeScript declaration for
#safe-node-apisso that itsfsexport is correctly typed. For example, create apackages/nextjs/src/server/fs/utils.d.ts(or in your global types) with:declare module '#safe-node-apis' { export const fs: typeof import('fs') export const path: typeof import('path') export const cwd: () => string }
These changes ensure the compiler will flag any missing sync methods immediately and align the types with the actual runtime behavior.
🤖 Prompt for AI Agents
In packages/nextjs/src/server/fs/utils.ts around the nodeFsOrThrow definition
(used by packages/nextjs/src/server/keyless-telemetry.ts line 47), the function
is currently untyped and returns any; change its signature to explicitly return
Node’s fs type (i.e., nodeFsOrThrow(): typeof import("fs")) and keep the
existing runtime checks, and add a TypeScript declaration file (e.g.,
packages/nextjs/src/server/fs/utils.d.ts) declaring module '#safe-node-apis'
that exports fs: typeof import("fs"), path: typeof import("path"), and cwd: ()
=> string so callers get proper typings and the compiler will enforce presence
of sync methods.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/sweet-vans-wonder.md (1)
5-5: Make the release note more specific about the root cause and the fix.Suggest explicitly calling out the async→sync switch and the affected telemetry to aid changelog readers.
-Fix keyless drift detection failure caused by invalid fs module methods. +Fix KEYLESS_ENV_DRIFT_DETECTED not being sent due to use of async fs methods. +Replaced fs.mkdir/fs.writeFile with mkdirSync/writeFileSync in the keyless telemetry flag writer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.changeset/sweet-vans-wonder.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/sweet-vans-wonder.md
🪛 LanguageTool
.changeset/sweet-vans-wonder.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...ure caused by invalid fs module methods.
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.changeset/sweet-vans-wonder.md (1)
1-4: Changeset front-matter looks correct.Package name and bump type are valid for a patch release.
Description
The telemetry event
KEYLESS_ENV_DRIFT_DETECTEDis not sent due to the errorFailed to create telemetry flag file: TypeError: mkdir is not a function. This PR changesmkdir, andwriteFiletomkdirSync, andwriteFileSyncin the keyless telemetry logic.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Bug Fixes
Chores