Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/livekit-server-sdk/package.json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 No code changes accompany the major version bump

This PR bumps jose from v5 to v6 (a major version) but includes zero code changes. Major version bumps typically include breaking changes. While the existing tests may pass if all used APIs remain compatible, the lack of any accompanying code changes warrants careful review of the jose v6 migration guide to ensure no behavioral changes affect JWT generation or verification.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 CJS export path may break on Node.js < 22 with jose v6

The package declares a CJS export at dist/index.cjs (line 24). jose v6 explicitly states that CJS-style require only works when require(esm) support is present in the Node.js runtime, which was introduced in Node.js 22. If tsup does NOT bundle jose into the CJS output (which is the default behavior for packages listed in dependencies), then CJS consumers on Node.js 18–21 will get a runtime error when the generated require('jose') call fails. This should be verified by checking the tsup configuration (tsup.config.ts or the tsup field in package.json) to see if noExternal: ['jose'] or similar bundling configuration is in place.

(Refers to lines 22-25)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 No changeset included for the jose major version bump

The three .changeset/*.md files in this PR are all for @livekit/rtc-node (patch-level changes). There is no changeset entry for livekit-server-sdk despite bumping a major dependency (jose v5→v6). While this doesn't cause a runtime bug, it means this dependency change won't be reflected in the changelog or trigger a version bump for livekit-server-sdk. If jose v6 is considered a meaningful change for downstream consumers (e.g., due to transitive dependency resolution or peer dependency constraints), a changeset should be added.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"@bufbuild/protobuf": "^1.10.1",
"@livekit/protocol": "^1.46.3",
"camelcase-keys": "^9.0.0",
"jose": "^5.1.2"
"jose": "^6.0.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Package engines field claims Node.js >=18 but jose v6 drops Node.js 18 support and breaks CJS on Node <22

Updating jose to ^6.0.0 without updating the engines field creates a compatibility mismatch. The package declares "node": ">=18" but jose v6 explicitly drops support for Node.js 18.x and earlier. More critically, jose v6 states: "CJS-style require is now only possible when require(esm) support is present in the Node.js runtime" (require(esm) was added in Node.js 22).

Impact on CJS consumers

The package exports a CJS entry point at packages/livekit-server-sdk/package.json:24 ("default": "./dist/index.cjs"). Since jose is a runtime dependency (not bundled), the CJS build will attempt to require('jose') at runtime. On Node.js versions 18–21, this will fail because jose v6 ships only ESM and those runtimes do not support require(esm). Users on Node.js 18 or 20 (both current LTS lines) who consume livekit-server-sdk via require() will get a runtime error.

The engines field at line 65 should be updated to reflect the actual minimum Node.js version required by the dependency graph (at minimum >=20, or >=22 if CJS support is needed).

Prompt for agents
Update the engines field in packages/livekit-server-sdk/package.json (currently at line 65: "node": ">=18") to reflect the minimum Node.js version required by jose v6. At minimum, change it to "node": ">=20" (jose v6 drops Node 18 support). If CJS consumers need to be supported, change it to "node": ">=22" since jose v6's CJS-style require only works with Node.js require(esm) support (added in Node 22). Alternatively, consider configuring tsup to bundle jose into the CJS output so CJS consumers on older Node.js versions are not affected.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 clockTolerance parameter type allows string but jose v6 only accepts number

In jose v6, the clockTolerance option in jwtVerify only accepts a number (seconds), not a string. However, TokenVerifier.verify() at AccessToken.ts:228 declares clockTolerance: string | number, which allows callers to pass a string (e.g., '10s'). While the default value (defaultClockToleranceSeconds = 10) is a number and won't cause issues for default usage, any caller that passes a string will get a runtime error from jose v6.

Prompt for agents
In packages/livekit-server-sdk/src/AccessToken.ts line 228, change the clockTolerance parameter type from 'string | number' to just 'number' to match jose v6's API. The current signature is:

  async verify(token: string, clockTolerance: string | number = defaultClockToleranceSeconds): Promise<ClaimGrants>

It should become:

  async verify(token: string, clockTolerance: number = defaultClockToleranceSeconds): Promise<ClaimGrants>

Note: this is a public API change for the SDK, so callers passing strings will need to be updated. This is a breaking change in the SDK's own API surface.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 engines field claims Node.js 18 support but jose v6 drops it

The engines field at packages/livekit-server-sdk/package.json:65 specifies "node": ">=18", but jose v6's release notes explicitly state "drop support for Node.js 18.x and earlier." While Node.js 18 reached EOL in April 2025, the SDK still advertises support. The engines field should likely be updated to >=20 to match the actual minimum supported runtime. This is a documentation/configuration concern rather than a code bug — actual runtime failures depend on which Node.js 18 features jose v6 relies on (e.g., WebCryptoAPI completeness).

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

},
"devDependencies": {
"@changesets/cli": "^2.27.1",
Expand Down
10 changes: 5 additions & 5 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading