test(repo): [DO NOT MERGE] exercise snapi breaking-change detector#8669
test(repo): [DO NOT MERGE] exercise snapi breaking-change detector#8669jacekradko wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 8f4036c The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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.
|
API Changes Report
Summary
@clerk/backendVersion: 3.4.14 → 3.4.14 Subpath
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis pull request refactors shared file utilities by renaming Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/file.ts (1)
12-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle JSON parse failures by rejecting the Promise.
At Line 13,
JSON.parsecan throw inside theloadevent callback. That exception is not converted into a Promise rejection here, which can leave callers without a propercatchpath.Suggested fix
export function parseJSONFile(file: File): Promise<unknown> { return new Promise((resolve, reject) => { const reader = new FileReader(); reader.addEventListener('load', function () { - const result = JSON.parse(reader.result as string); - resolve(result); + try { + const result = JSON.parse(reader.result as string); + resolve(result); + } catch (err) { + reject(err); + } }); reader.addEventListener('error', reject); reader.readAsText(file); }); }🤖 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 `@packages/shared/src/file.ts` around lines 12 - 15, The load handler for the FileReader currently calls JSON.parse directly and can throw without rejecting the surrounding Promise; update the reader.addEventListener('load', function () { ... }) callback to wrap JSON.parse(reader.result as string) in a try/catch and call reject(error) when parsing fails, and also ensure you attach a reader.addEventListener('error', ...) that calls reject with the reader.error so the Promise rejects on both parse and read errors (referencing the existing resolve/reject used in this module).
🧹 Nitpick comments (3)
packages/shared/src/file.ts (2)
33-37: ⚡ Quick winAdd JSDoc for newly exported public APIs.
MimeTypeExtensionandextensionare exported but undocumented. Please add JSDoc blocks for both public exports.As per coding guidelines, "Maintain comprehensive JSDoc comments for public APIs" and "All public APIs must be documented with JSDoc".
🤖 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 `@packages/shared/src/file.ts` around lines 33 - 37, Add JSDoc comments for the exported type MimeTypeExtension and the exported function extension: document what MimeTypeExtension represents (a union type of file extensions derived from MimeTypeToExtensionMap for SupportedMimeType) and annotate the extension function with param and return descriptions — include `@param` for mimeType (SupportedMimeType) and optional fallback (string), explain behavior when mapping is missing, and include `@returns` string; place the JSDoc immediately above the MimeTypeExtension type alias and the extension function declaration.
9-37: ⚡ Quick winPlease add tests for the renamed and behavior-changed exports.
This change renames a public function and changes
extension()fallback behavior; tests should be added/updated to cover both paths.As per coding guidelines, "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 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 `@packages/shared/src/file.ts` around lines 9 - 37, The PR renamed/changed exported behavior but has no tests; add/modify unit tests to cover the renamed JSON parser (parseJSONFile) and the changed extension() fallback behavior: create tests that (1) call parseJSONFile with a mock File/Blob containing valid JSON and assert the resolved value, (2) verify extension(mime) returns the correct mapped extension for each SupportedMimeType, and (3) verify extension(mime, fallback) returns the provided fallback when mime is not in MimeTypeToExtensionMap and that extension(mime) without fallback returns an empty string; reference parseJSONFile, MimeTypeToExtensionMap, SupportedMimeType, and extension in your tests.packages/backend/src/webhooks.ts (1)
15-18: ⚡ Quick winAdd tests for the new webhook option contract.
Since this PR adds a new
VerifyWebhookOptionsfield, please include tests that cover its expected behavior (including default vs explicit tolerance values).As per coding guidelines "
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."🤖 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 `@packages/backend/src/webhooks.ts` around lines 15 - 18, Add unit tests exercising the new VerifyWebhookOptions.timestampToleranceSeconds behavior: create tests that call the webhook verification function (e.g., verifyWebhook in packages/backend/src/webhooks.ts) with (1) no timestampToleranceSeconds to assert the default tolerance is applied, and (2) an explicit timestampToleranceSeconds value to assert the provided tolerance is used. Include cases that are just inside and just outside the tolerance window to verify acceptance/rejection behavior, and add assertions for both default vs explicit outcomes using the same signing/timestamp fixtures.
🤖 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 `@packages/backend/src/webhooks.ts`:
- Around line 15-18: The public option timestampToleranceSeconds is defined but
unused; update the verification flow in verifyWebhook to accept and apply this
tolerance when checking the webhook timestamp against the replay-window (i.e.,
add a parameter or read the option in verifyWebhook and expand the allowed delta
by timestampToleranceSeconds when comparing now - incomingTimestamp to the
configured replayWindow), or remove/deprecate timestampToleranceSeconds from the
public API; modify the verifyWebhook function (and any callers that construct
its options) so the replay-time check uses timestampToleranceSeconds to increase
the permitted difference.
---
Outside diff comments:
In `@packages/shared/src/file.ts`:
- Around line 12-15: The load handler for the FileReader currently calls
JSON.parse directly and can throw without rejecting the surrounding Promise;
update the reader.addEventListener('load', function () { ... }) callback to wrap
JSON.parse(reader.result as string) in a try/catch and call reject(error) when
parsing fails, and also ensure you attach a reader.addEventListener('error',
...) that calls reject with the reader.error so the Promise rejects on both
parse and read errors (referencing the existing resolve/reject used in this
module).
---
Nitpick comments:
In `@packages/backend/src/webhooks.ts`:
- Around line 15-18: Add unit tests exercising the new
VerifyWebhookOptions.timestampToleranceSeconds behavior: create tests that call
the webhook verification function (e.g., verifyWebhook in
packages/backend/src/webhooks.ts) with (1) no timestampToleranceSeconds to
assert the default tolerance is applied, and (2) an explicit
timestampToleranceSeconds value to assert the provided tolerance is used.
Include cases that are just inside and just outside the tolerance window to
verify acceptance/rejection behavior, and add assertions for both default vs
explicit outcomes using the same signing/timestamp fixtures.
In `@packages/shared/src/file.ts`:
- Around line 33-37: Add JSDoc comments for the exported type MimeTypeExtension
and the exported function extension: document what MimeTypeExtension represents
(a union type of file extensions derived from MimeTypeToExtensionMap for
SupportedMimeType) and annotate the extension function with param and return
descriptions — include `@param` for mimeType (SupportedMimeType) and optional
fallback (string), explain behavior when mapping is missing, and include
`@returns` string; place the JSDoc immediately above the MimeTypeExtension type
alias and the extension function declaration.
- Around line 9-37: The PR renamed/changed exported behavior but has no tests;
add/modify unit tests to cover the renamed JSON parser (parseJSONFile) and the
changed extension() fallback behavior: create tests that (1) call parseJSONFile
with a mock File/Blob containing valid JSON and assert the resolved value, (2)
verify extension(mime) returns the correct mapped extension for each
SupportedMimeType, and (3) verify extension(mime, fallback) returns the provided
fallback when mime is not in MimeTypeToExtensionMap and that extension(mime)
without fallback returns an empty string; reference parseJSONFile,
MimeTypeToExtensionMap, SupportedMimeType, and extension in your tests.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9dd4ff24-7de6-4c52-870e-6fc8f2907940
📒 Files selected for processing (3)
.changeset/test-snapi-detection-do-not-merge.mdpackages/backend/src/webhooks.tspackages/shared/src/file.ts
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
…nd/jwt Adds two more snapi-visible changes on an explicit (non-wildcard) subpath: - BREAKING: drop hasValidSignature from the @clerk/backend/jwt barrel - ADDITIVE: export a new JwtAlgorithm type from the same barrel The existing changes to @clerk/shared/file land under the './*' wildcard export, which the current snapi build skips, so this PR otherwise wouldn't surface anything breaking once the snapi pin is bumped.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/jwt/index.ts (1)
8-9: ⚡ Quick winAdd JSDoc for the new public
JwtAlgorithmtype export.This adds a new public API type without API docs in
packages/**/src.Proposed patch
-export type JwtAlgorithm = 'HS256' | 'HS384' | 'HS512' | 'RS256' | 'RS384' | 'RS512' | 'ES256' | 'ES384' | 'ES512'; +/** + * Supported JWT signing algorithms for Clerk backend JWT helpers. + * + * `@example` + * const alg: JwtAlgorithm = 'RS256'; + */ +export type JwtAlgorithm = + | 'HS256' + | 'HS384' + | 'HS512' + | 'RS256' + | 'RS384' + | 'RS512' + | 'ES256' + | 'ES384' + | 'ES512';As per coding guidelines, "packages//src//*.{ts,tsx}: Maintain comprehensive JSDoc comments for public APIs" and "All public APIs must be documented with JSDoc".
🤖 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 `@packages/backend/src/jwt/index.ts` around lines 8 - 9, Add a JSDoc block for the exported type JwtAlgorithm explaining that it represents supported JWT signing algorithms and list the allowed string literal values ('HS256', 'HS384', 'HS512', 'RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512'); include a one-line description, `@public` (or `@internal` if not public), and a short example or usage note referencing where it's used (e.g., in JWT creation/verification functions) so the public API in packages/backend/src/jwt/index.ts is documented.
🤖 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.
Nitpick comments:
In `@packages/backend/src/jwt/index.ts`:
- Around line 8-9: Add a JSDoc block for the exported type JwtAlgorithm
explaining that it represents supported JWT signing algorithms and list the
allowed string literal values ('HS256', 'HS384', 'HS512', 'RS256', 'RS384',
'RS512', 'ES256', 'ES384', 'ES512'); include a one-line description, `@public` (or
`@internal` if not public), and a short example or usage note referencing where
it's used (e.g., in JWT creation/verification functions) so the public API in
packages/backend/src/jwt/index.ts is documented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: de55a71e-1264-42ae-9bc6-04a4837989c5
📒 Files selected for processing (2)
packages/backend/src/__tests__/exports.test.tspackages/backend/src/jwt/index.ts
💤 Files with no reviewable changes (1)
- packages/backend/src/tests/exports.test.ts
- drop the paths: filter on the push trigger so publish-baseline runs on every main/release-branch commit, keeping the cache that check-api restores hot. - bump SNAPI_PACKAGE to the current snapi main HEAD; the previous pin predates subpath snapshot support, so changes on subpath exports (e.g. @clerk/backend/webhooks) were never detected.
1 participant
Caution
DO NOT MERGE. This PR exists only to exercise snapi's breaking-change detector against this repo. The diff intentionally introduces breaking, non-breaking, and additive API changes and is not meant to ship. Close once the report has been collected.
Mixes the three change categories the detector cares about so we can see how each is classified. Three of the original changes land under
@clerk/shared/file, which is exposed via the./*wildcard export and is currently skipped by snapi, so two more changes were added on the explicit@clerk/backend/jwtsubpath to make sure the detector actually fires.Breaking (explicit subpath, detected)
@clerk/backend/jwt:hasValidSignatureis no longer re-exported from the subpath barrel.Breaking (wildcard subpath, currently hidden)
@clerk/shared/file:readJSONFilerenamed toparseJSONFile.Non-breaking (explicit subpath, detected)
@clerk/backend/webhooks:VerifyWebhookOptionsgains an optionaltimestampToleranceSeconds?: numberfield.Non-breaking (wildcard subpath, currently hidden)
@clerk/shared/file:extension(mimeType)gains an optionalfallback?: stringsecond parameter.Additive (explicit subpath, detected)
@clerk/backend/jwt: new exported typeJwtAlgorithm.Additive (wildcard subpath, currently hidden)
@clerk/shared/file: new exported typeMimeTypeExtension.Wildcard handling is tracked separately on the snapi side. Modeled on clerk/snapi#21, which did the same exercise against snapi itself. Changeset is intentionally empty since nothing here will be released.