-
Notifications
You must be signed in to change notification settings - Fork 136
Adds documentation + CLI option to JS Moq Token library #847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe PR adds key generation and a CLI to the token package. It introduces a new generate module that creates keys for HMAC, RSA, RSA‑PSS, EC, and EdDSA algorithms and exports it from the package entry. A new CLI implements generate, sign, and verify commands. package.json was updated (version bump, CLI bin entry, build script, published files, and a commander dependency). The README was expanded with installation, CLI usage, generation/signing/verifying examples, and an updated API/claims description. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@js/token/README.md`:
- Line 10: Replace the npm install command in the README that currently reads
"npm add `@moq/token`" with the project-standard bun command "bun add `@moq/token`"
so package installation instructions follow the repository's bun-based package
management convention.
- Around line 18-20: Replace the npm global install example with the bun
equivalent to match project package management: change the code block containing
"npm install -g `@moq/token`" to use Bun's global add syntax, e.g. "bun add -g
`@moq/token`", and ensure the README section describing CLI installation
consistently references bun.
- Around line 57-72: The README example uses incorrect claim field names and a
relative import; update the Claims object to use the documented fields `publish`
and `subscribe` instead of `put` and `get`, and change the import to use the
package name import from "@moq/token" (keep using the same exported symbols
`Claims`, `load`, `sign`, `verify`); ensure the example still sets `exp` and
`iat` the same way and then calls `const token = await sign(key, claims)` as
before so the example matches the API docs.
In `@js/token/src/cli.ts`:
- Around line 13-16: The CLI hardcodes version "0.1.0" in the program.version
call which is out of sync with package.json ("0.1.1"); update program.version to
derive the version dynamically or align it with package.json. Locate the program
configuration in cli.ts (the program.name(...).version(...) chain) and either
import/require the package.json version field (e.g., read package.json.version
and pass it to program.version) or change the hardcoded string to "0.1.1" so
both sources match.
🧹 Nitpick comments (2)
js/token/README.md (2)
88-89: Use package import instead of relative path.Documentation examples should use the package name for clarity.
Suggested fix
-import { verify } from "../src/index.ts"; +import { verify } from "@moq/token";
111-116: Algorithm documentation is incomplete.The
generate.tsimplementation supports many more algorithms: RS256/384/512, PS256/384/512, ES256/384, and EdDSA. Consider documenting all supported algorithms or clarifying which are recommended.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
js/token/README.mdjs/token/package.jsonjs/token/src/cli.tsjs/token/src/generate.tsjs/token/src/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
js/**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bunfor all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Files:
js/token/src/index.tsjs/token/package.jsonjs/token/src/generate.tsjs/token/src/cli.ts
🧬 Code graph analysis (1)
js/token/src/generate.ts (2)
js/token/src/algorithm.ts (1)
Algorithm(20-20)js/token/src/key.ts (1)
Key(96-96)
🪛 markdownlint-cli2 (0.18.1)
js/token/README.md
14-14: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ 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). (1)
- GitHub Check: Check
🔇 Additional comments (9)
js/token/package.json (1)
11-20: LGTM! CLI setup and build script are correctly configured.The bin entry, files array, and build-cli script using
bun build --compileare properly set up. The compiled CLI artifact will be generated todist/clias expected.js/token/src/generate.ts (3)
8-33: Well-structured key generation with comprehensive algorithm support.The switch-based dispatch and separation of generation logic by key type (HMAC, RSA, EC, EdDSA) is clean and maintainable.
35-48: LGTM! HMAC key generation is correct.Proper byte lengths for each algorithm variant (32/48/64 bytes for HS256/384/512) and correct use of
crypto.getRandomValues()for secure random generation.
124-149: Ed25519 runtime support is now widespread; no additional error handling needed.As of January 2026, Ed25519 is widely supported across major JavaScript environments: Firefox 129+, Safari 17+, Chrome 137, and Edge 137 all include Web Crypto API support. The lack of internal error handling in
generateEdDsaKey()is consistent with other key generation functions (generateRsaKey,generateEcKey,generateHmacKey), which follow the same pattern of delegating error handling to the caller. The CLI layer properly catches and displays errors from thegenerate()function, so error propagation is handled appropriately.js/token/src/index.ts (1)
3-3: LGTM!Correctly exposes the new
generatefunction through the public API.js/token/src/cli.ts (4)
18-51: LGTM! Generate command is well-implemented.Good handling of both symmetric and asymmetric key types, with appropriate warning when a user requests a public key for symmetric algorithms.
53-83: LGTM! Sign command correctly maps CLI options to internal Claims fields.The mapping of
--publishtoputand--subscribetogetaligns with the internal Claims structure.
85-111: LGTM! Verify command with graceful key type fallback.The approach of trying public key first then falling back to symmetric key is reasonable for supporting both key types transparently.
113-119: LGTM! Clean timestamp parsing helper.Properly validates and parses unix timestamps with clear error messaging.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
js/token/README.md (2)
113-116: Algorithm list is incomplete.The documentation only lists HMAC algorithms, but the
generatefunction (and CLI) supports additional algorithms: RS256, RS384, RS512, PS256, PS384, PS512, ES256, ES384, and EdDSA.📝 Suggested addition
Supported algorithms: - `HS256` - HMAC with SHA-256 - `HS384` - HMAC with SHA-384 - `HS512` - HMAC with SHA-512 +- `RS256`, `RS384`, `RS512` - RSA with PKCS#1 v1.5 +- `PS256`, `PS384`, `PS512` - RSA-PSS +- `ES256`, `ES384` - ECDSA with P-256/P-384 +- `EdDSA` - Edwards-curve Digital Signature Algorithm
152-154:verifyfunction signature is incomplete.The documented signature shows 2 parameters, but the actual function accepts a third
rootparameter for path verification (as used in the CLI at line 105 of cli.ts).📝 Suggested fix
// Verify and decode a JWT -function verify(key: Key, token: string): Promise<Claims>; +function verify(key: Key, token: string, root?: string): Promise<Claims>;
🤖 Fix all issues with AI agents
In `@js/token/README.md`:
- Around line 39-49: Update the examples to await the async generate function
and fix the CLI flag placement: use "const key = await generate('HS256');"
(generate returns Promise<Key> so add await and ensure example is in an async
context) and change the CLI example to "moq-token generate --key key.jwk" so the
--key option is passed to the generate subcommand rather than the top-level
program.
- Around line 76-82: The example CLI invocation places --key before the sign
subcommand; move the --key option to follow the subcommand so the command is
"moq-token sign --key ..." instead of "moq-token --key ...", keeping the rest of
the flags (--root, --subscribe, --publish, --expires) unchanged and redirecting
output to "alice.jwt"; update the README example accordingly referencing the
moq-token CLI, the sign subcommand, and the --key option.
♻️ Duplicate comments (1)
js/token/README.md (1)
88-89: Import path should use package name.The import uses a relative path
"../src/index.ts"instead of the package name"@moq/token". This was acknowledged in the previous review.📝 Suggested fix
-import { verify } from "../src/index.ts"; +import { verify } from "@moq/token";
🧹 Nitpick comments (3)
js/token/src/cli.ts (2)
25-28: Consider validating the algorithm before use.The cast
options.algorithm as Algorithmbypasses type checking. If a user provides an invalid algorithm, the error will come fromgenerate()which may be less clear. A validation step would improve the error message.♻️ Suggested improvement
.action(async (options) => { try { - const algorithm = options.algorithm as Algorithm; + const validAlgorithms = ["HS256", "HS384", "HS512", "RS256", "RS384", "RS512", "PS256", "PS384", "PS512", "ES256", "ES384", "EdDSA"]; + if (!validAlgorithms.includes(options.algorithm)) { + throw new Error(`Unsupported algorithm: ${options.algorithm}. Valid options: ${validAlgorithms.join(", ")}`); + } + const algorithm = options.algorithm as Algorithm; const key = await generate(algorithm, options.id);
94-100: Empty catch block obscures the first error.If
loadPublicfails for a reason other than "it's a symmetric key" (e.g., malformed file), the error is silently discarded and thenloadis attempted. If both fail, only the second error is shown.♻️ Optional improvement
// Try to load as public key first (for asymmetric), fall back to symmetric key let key; try { key = loadPublic(keyEncoded); - } catch { - key = load(keyEncoded); + } catch (pubErr) { + try { + key = load(keyEncoded); + } catch (symErr) { + // If both fail, show the symmetric key error as it's more common + throw symErr; + } }js/token/README.md (1)
14-14: Heading level jumps from h2 to h4.Static analysis flagged that headings should increment one level at a time. Consider using
###(h3) instead of####(h4) for subsections under the h2 headings.♻️ Suggested fix
-#### CLI +### CLISimilarly for lines 36, 53, 84, 105.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
js/token/README.mdjs/token/src/cli.ts
🧰 Additional context used
📓 Path-based instructions (1)
js/**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bunfor all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Files:
js/token/src/cli.ts
🧠 Learnings (2)
📚 Learning: 2026-01-11T03:47:52.766Z
Learnt from: CR
Repo: moq-dev/moq PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T03:47:52.766Z
Learning: Applies to js/**/*.{ts,tsx,js,jsx,json} : Use `bun` for all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Applied to files:
js/token/README.md
📚 Learning: 2026-01-15T16:35:25.165Z
Learnt from: sb2702
Repo: moq-dev/moq PR: 847
File: js/token/README.md:10-10
Timestamp: 2026-01-15T16:35:25.165Z
Learning: User-facing documentation (like README installation instructions for npm packages) should use standard npm commands even if the project internally uses bun for development, since npm is more familiar to general JavaScript developers.
Applied to files:
js/token/README.md
🧬 Code graph analysis (1)
js/token/src/cli.ts (5)
js/token/src/algorithm.ts (1)
Algorithm(20-20)js/token/src/generate.ts (1)
generate(8-33)js/token/src/key.ts (3)
toPublicKey(102-122)sign(165-187)loadPublic(132-138)js/lite/src/util/error.ts (1)
error(2-4)js/token/src/claims.ts (1)
Claims(19-19)
🪛 markdownlint-cli2 (0.18.1)
js/token/README.md
14-14: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ 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). (1)
- GitHub Check: Check
🔇 Additional comments (3)
js/token/src/cli.ts (3)
1-16: LGTM on CLI setup and version.The shebang, imports, and program configuration look correct. The version has been updated to
0.1.1to match package.json.
53-83: LGTM on sign command.The sign command correctly loads the key, builds claims with optional fields using conditional spreading, and outputs the token to stdout. Error handling is appropriate.
113-121: LGTM on timestamp parsing and program execution.The
parseUnixTimestamphelper correctly validates input and provides a clear error message. Theprogram.parse()call follows standard Commander patterns.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@js/token/README.md`:
- Line 14: The README uses h4 headings for "CLI" and "Generation" which causes a
heading-increment (MD001) violation; change those headings from '#### CLI' and
'#### Generation' to '### CLI' and '### Generation' so the document hierarchy
moves from h2 to h3 correctly and resolves the static analysis warnings.
- Line 29: Replace the two-dot ellipsis in the command example "bunx `@moq/token`
generate .." with a three-dot ellipsis "..." so it matches the pattern used
elsewhere (lines with "..." such as the command examples on lines 23 and 30);
update the string in the README example accordingly (look for the exact token
command text "bunx `@moq/token` generate ..").
- Line 92: The type assertion on verifiedClaims uses incorrect syntax
("=<Claims> await ..."); update the expression to use TypeScript's preferred
"as" syntax and ensure await applies to verify first, e.g. replace the line
with: assign the result of await verify(key, token, rootPath) and append "as
Claims" (const verifiedClaims = await verify(key, token, rootPath) as Claims),
referencing verifiedClaims and the verify call.
♻️ Duplicate comments (1)
js/token/README.md (1)
89-89: Use package import for consistency.This example uses a relative import path
"../src/index.ts"while other examples in the README (lines 40, 58) correctly use the package name"@moq/token". User-facing documentation should consistently use the package name.📝 Suggested fix
-import { verify } from "../src/index.ts"; +import { load, verify } from "@moq/token";Note: Also added
loadto the import sincekeyis referenced on line 92 and would need to be created usingload()as shown in previous examples.
🧹 Nitpick comments (1)
js/token/README.md (1)
91-96: Improve error handling in example.The catch block captures the error but doesn't log it, giving users no information about why verification failed. This is particularly important for debugging token issues.
📝 Suggested enhancement
try{ const verifiedClaims = await verify(key, token, rootPath) as Claims; console.log("Valid token") } catch(e){ - console.log("Invalid token") + console.log("Invalid token:", e) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
js/token/README.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-11T03:47:52.766Z
Learnt from: CR
Repo: moq-dev/moq PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T03:47:52.766Z
Learning: Applies to js/**/*.{ts,tsx,js,jsx,json} : Use `bun` for all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Applied to files:
js/token/README.md
📚 Learning: 2026-01-15T16:35:25.165Z
Learnt from: sb2702
Repo: moq-dev/moq PR: 847
File: js/token/README.md:10-10
Timestamp: 2026-01-15T16:35:25.165Z
Learning: User-facing documentation (like README installation instructions for npm packages) should use standard npm commands even if the project internally uses bun for development, since npm is more familiar to general JavaScript developers.
Applied to files:
js/token/README.md
🪛 markdownlint-cli2 (0.18.1)
js/token/README.md
14-14: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (3)
js/token/README.md (3)
1-3: LGTM!The updated description provides clear context and the relative path to the authentication documentation is correct for the file location.
5-11: LGTM!The installation section correctly uses the scoped package name
@moq/tokenand provides clear context for library installation.
105-156: LGTM!The API documentation is clear and comprehensive. The reference to the working example provides users with a complete implementation to follow.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { verify } from "../src/index.ts"; | ||
| const rootPath = "rooms/meeting-123"; | ||
| try{ | ||
| const verifiedClaims =<Claims> await verify(key, token, rootPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type assertion syntax.
The type assertion is missing a space (=<Claims>) and should use TypeScript's preferred as syntax for better readability.
📝 Suggested fix
- const verifiedClaims =<Claims> await verify(key, token, rootPath);
+ const verifiedClaims = await verify(key, token, rootPath) as Claims;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const verifiedClaims =<Claims> await verify(key, token, rootPath); | |
| const verifiedClaims = await verify(key, token, rootPath) as Claims; |
🤖 Prompt for AI Agents
In `@js/token/README.md` at line 92, The type assertion on verifiedClaims uses
incorrect syntax ("=<Claims> await ..."); update the expression to use
TypeScript's preferred "as" syntax and ensure await applies to verify first,
e.g. replace the line with: assign the result of await verify(key, token,
rootPath) and append "as Claims" (const verifiedClaims = await verify(key,
token, rootPath) as Claims), referencing verifiedClaims and the verify call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@js/token/README.md`:
- Line 30: The README's Deno command references the CLI subpath but package.json
only exports the main entry; update package.json "exports" to include a "./cli"
export pointing to src/cli.ts (so npm:`@moq/token/cli` resolves), or alternatively
re-export the CLI from the main entry (src/index.ts) and keep the README command
as npm:`@moq/token` to match the current export; modify package.json's exports or
adjust the README command accordingly to ensure the referenced cli.ts is
reachable.
♻️ Duplicate comments (1)
js/token/README.md (1)
92-92: Use package import instead of relative path.For consistency with other examples in the documentation (lines 40, 61), the import should use the package name
@moq/tokeninstead of a relative path.♻️ Proposed fix
-import { verify } from "../src/index.ts"; +import { verify } from "@moq/token";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
js/token/README.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-11T03:47:52.766Z
Learnt from: CR
Repo: moq-dev/moq PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T03:47:52.766Z
Learning: Applies to js/**/*.{ts,tsx,js,jsx,json} : Use `bun` for all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Applied to files:
js/token/README.md
📚 Learning: 2026-01-15T16:35:25.165Z
Learnt from: sb2702
Repo: moq-dev/moq PR: 847
File: js/token/README.md:10-10
Timestamp: 2026-01-15T16:35:25.165Z
Learning: User-facing documentation (like README installation instructions for npm packages) should use standard npm commands even if the project internally uses bun for development, since npm is more familiar to general JavaScript developers.
Applied to files:
js/token/README.md
🪛 markdownlint-cli2 (0.18.1)
js/token/README.md
14-14: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ 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). (1)
- GitHub Check: Check
🔇 Additional comments (2)
js/token/README.md (2)
108-110: LGTM!The reference to the working example is clear and uses an appropriate relative path to an actual file in the repository.
112-158: LGTM - Clear and comprehensive API documentation.The API section provides well-structured documentation with clear type definitions and function signatures. The supported algorithms, Claims interface, and Key interface are all properly documented.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
js/token/README.md (2)
114-119: Algorithm list is incomplete.The API documentation only lists HMAC algorithms, but
generate.tssupports RS256/384/512, PS256/384/512, ES256/384, and EdDSA as well. Users won't know these are available.📝 Suggested additions
### Algorithm Supported algorithms: - `HS256` - HMAC with SHA-256 - `HS384` - HMAC with SHA-384 - `HS512` - HMAC with SHA-512 - `RS256` - RSA PKCS#1 v1.5 with SHA-256 - `RS384` - RSA PKCS#1 v1.5 with SHA-384 - `RS512` - RSA PKCS#1 v1.5 with SHA-512 - `PS256` - RSA-PSS with SHA-256 - `PS384` - RSA-PSS with SHA-384 - `PS512` - RSA-PSS with SHA-512 - `ES256` - ECDSA with P-256 and SHA-256 - `ES384` - ECDSA with P-384 and SHA-384 - `EdDSA` - Ed25519
140-148: Key interface documentation uses incorrect field names and structure.The documented
Keyinterface showsalgorithm,operations, andsecret, but the actualKeytype fromkey.tsis a discriminated union with:
alg(notalgorithm)key_ops(notoperations)kty(discriminator, completely missing from docs)- Type-specific fields:
kfor symmetric keys,n/e/dfor RSA,crv/x/y/dfor EC, etc.Update the README to accurately reflect the actual Key structure, including the union types and correct field names.
🤖 Fix all issues with AI agents
In `@js/token/README.md`:
- Line 30: The package.json currently lacks a "./cli" subpath in its "exports"
map which prevents imports like npm:`@moq/token/cli` from resolving; update the
"exports" field to include both "." -> "./src/index.ts" and "./cli" ->
"./dist/cli" (while keeping the existing "bin" entry intact) so the deno run -A
npm:`@moq/token/cli` generate ... command can resolve the CLI subpath.
- Line 92: Replace the relative import of the verify symbol in the README
example (currently written as import { verify } from "../src/index.ts") with the
package-name-based import used elsewhere in the docs (the same package string
used by the signing example), i.e., change the module specifier to the published
package name so the example imports verify from the package instead of a
relative path.
In `@js/token/src/generate.ts`:
- Around line 136-161: Wrap the crypto.subtle.generateKey call in
generateEdDsaKey with a try-catch and throw or log a clear, actionable error
that states Ed25519/WebCrypto is unsupported in the current runtime and suggests
minimum versions (e.g., Node 22.13.0+, Chrome 137+, Firefox 129+, Safari 17+) or
to upgrade/enable the appropriate feature; alternatively, add these minimum
runtime requirements to the README, but if catching, ensure the thrown error
includes the original error message for debugging and references
generateEdDsaKey/Ed25519 so callers can locate the issue.
♻️ Duplicate comments (3)
js/token/README.md (3)
95-95: Fix type assertion syntax.The type assertion has incorrect syntax (missing space, using legacy angle-bracket style). Use TypeScript's
assyntax for better readability.📝 Suggested fix
- const verifiedClaims =<Claims> await verify(key, token, rootPath); + const verifiedClaims = await verify(key, token, rootPath) as Claims;
14-14: Heading level skips from h2 to h4.Static analysis (MD001) flags this heading increment violation. This should be
### CLI(h3) to maintain proper document hierarchy.📝 Suggested fix
-#### CLI +### CLI
36-36: Heading level skips from h2 to h4.Same MD001 violation—should be
### Generation(h3).📝 Suggested fix
-#### Generation +### Generation
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
js/token/README.mdjs/token/src/generate.ts
🧰 Additional context used
📓 Path-based instructions (1)
js/**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bunfor all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Files:
js/token/src/generate.ts
🧠 Learnings (2)
📚 Learning: 2026-01-11T03:47:52.766Z
Learnt from: CR
Repo: moq-dev/moq PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T03:47:52.766Z
Learning: Applies to js/**/*.{ts,tsx,js,jsx,json} : Use `bun` for all TypeScript/JavaScript package management and script execution (not npm, yarn, or pnpm)
Applied to files:
js/token/README.md
📚 Learning: 2026-01-15T16:35:25.165Z
Learnt from: sb2702
Repo: moq-dev/moq PR: 847
File: js/token/README.md:10-10
Timestamp: 2026-01-15T16:35:25.165Z
Learning: User-facing documentation (like README installation instructions for npm packages) should use standard npm commands even if the project internally uses bun for development, since npm is more familiar to general JavaScript developers.
Applied to files:
js/token/README.md
🧬 Code graph analysis (1)
js/token/src/generate.ts (2)
js/token/src/algorithm.ts (1)
Algorithm(20-20)js/token/src/key.ts (1)
Key(96-96)
🪛 markdownlint-cli2 (0.18.1)
js/token/README.md
14-14: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ 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). (1)
- GitHub Check: Check
🔇 Additional comments (8)
js/token/src/generate.ts (5)
8-33: LGTM!The switch-based dispatch pattern is clean and provides exhaustive coverage of the supported algorithms with a clear error for unsupported cases.
38-51: LGTM!HMAC key generation is cryptographically sound—uses
crypto.getRandomValues(CSPRNG), correct key lengths matching algorithm requirements, and URL-safe base64 encoding appropriate for JWK.
56-98: LGTM!RSA key generation is correct with standard parameters (2048-bit modulus, 65537 exponent). The implementation properly exports the full private key JWK.
103-131: LGTM!EC key generation correctly uses P-256 for ES256 and P-384 for ES384 with standard ECDSA parameters.
166-171: LGTM!The hash mapping logic correctly handles RS/PS algorithm suffixes for its intended RSA key generation use case.
js/token/README.md (3)
1-11: LGTM!Installation instructions correctly use
npmfor user-facing documentation. Based on learnings, this is the appropriate choice even though the project internally usesbun.
56-85: LGTM!The signing examples are correct. The
put/getfield names are intentional to match the Rust CLI implementation.
108-110: LGTM!The relative link to the working example is correctly formatted.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| ```bash | ||
| npm add moq-token | ||
| bunx @moq/token generate ... | ||
| deno run -A npm:@moq/token/cli generate ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if package.json has /cli export
echo "=== Checking package.json exports ==="
cat js/token/package.json | jq '.exports'
echo -e "\n=== Checking bin entry ==="
cat js/token/package.json | jq '.bin'Repository: moq-dev/moq
Length of output: 180
🌐 Web query:
npm subpath exports package.json how to export cli subpath
💡 Result:
Short answer: define the CLI as a normal npm executable with the "bin" field, and only add an exports subpath if you also want the CLI file to be importable by other packages. Use "bin" for the command that npm/yarn will install; use "exports" to expose importable subpaths. Example package.json:
{
"name": "my-pkg",
"version": "1.0.0",
"main": "./dist/index.js",
"bin": {
"my-cli": "./dist/cli.js"
},
"exports": {
".": "./dist/index.js",
"./cli": "./dist/cli.js"
}
}
Notes:
- "bin" is required for npm to install the executable as a command on the PATH. [1]
- When "exports" is present, unlisted subpaths are blocked; list any subpath you want consumers to import (e.g. import "my-pkg/cli"). Use extensionless or extensioned specifiers consistently as Node expects. [2][3]
- If you only want the file to be a CLI and not importable, omit the "./cli" export (many bundlers/tooling default to excluding CLI entries). [4]
- For TypeScript consumers you may also need types/ typesVersions entries so editors can resolve exported subpaths. [5]
References:
[1] Node.js docs — Packages / bin and main behavior. (nodejs.org)
[2] Node.js docs — Subpath exports behavior and encapsulation. (nodejs.org)
[3] npm/package-exports guidance and common pitfalls. (npmjs.com)
[4] bunup docs — CLI entries are typically excluded from exports by build tools. (bunup.dev)
[5] Typescript workaround for subpath exports (typesVersions example). (skypack.dev)
If you want, tell me whether you need ESM/CJS conditional exports or an example with shebang + executable permissions for the CLI file.
Citations:
- 1: https://nodejs.org/api/packages.html?utm_source=openai
- 2: https://nodejs.org/download/release/v15.3.0/docs/api/packages.html?utm_source=openai
- 3: https://www.npmjs.com/package/package-exports?utm_source=openai
- 4: https://bunup.dev/docs/extra-options/exports.html?utm_source=openai
- 5: https://www.skypack.dev/view/typescript-subpath-exports-workaround?utm_source=openai
Add ./cli export to package.json exports field.
The command npm:@moq/token/cli`` requires an explicit "./cli" subpath in the exports field. The current `bin` entry doesn't make the subpath importable. Update the exports to:
"exports": {
".": "./src/index.ts",
"./cli": "./dist/cli"
}Without this, the documented deno run -A npm:@moq/token/cli generate ... command will not resolve.
🤖 Prompt for AI Agents
In `@js/token/README.md` at line 30, The package.json currently lacks a "./cli"
subpath in its "exports" map which prevents imports like npm:`@moq/token/cli` from
resolving; update the "exports" field to include both "." -> "./src/index.ts"
and "./cli" -> "./dist/cli" (while keeping the existing "bin" entry intact) so
the deno run -A npm:`@moq/token/cli` generate ... command can resolve the CLI
subpath.
| You can also verify a token | ||
|
|
||
| ```typescript | ||
| import { verify } from "../src/index.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use package name instead of relative import.
For documentation examples, use the package name for consistency with the signing example on line 61.
📝 Suggested fix
-import { verify } from "../src/index.ts";
+import { verify, type Claims } from "@moq/token";🤖 Prompt for AI Agents
In `@js/token/README.md` at line 92, Replace the relative import of the verify
symbol in the README example (currently written as import { verify } from
"../src/index.ts") with the package-name-based import used elsewhere in the docs
(the same package string used by the signing example), i.e., change the module
specifier to the published package name so the example imports verify from the
package instead of a relative path.
| async function generateEdDsaKey(alg: "EdDSA", kid?: string): Promise<Key> { | ||
| const keyPair = await crypto.subtle.generateKey( | ||
| { | ||
| name: "Ed25519", | ||
| }, | ||
| true, | ||
| ["sign", "verify"], | ||
| ); | ||
|
|
||
| const jwk = (await crypto.subtle.exportKey("jwk", keyPair.privateKey)) as { | ||
| kty: "OKP"; | ||
| crv: "Ed25519"; | ||
| x: string; | ||
| d: string; | ||
| }; | ||
|
|
||
| return { | ||
| kty: "OKP", | ||
| alg, | ||
| crv: "Ed25519", | ||
| x: jwk.x, | ||
| d: jwk.d, | ||
| key_ops: ["sign", "verify"], | ||
| ...(kid && { kid }), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Web Crypto Ed25519 support browser Node.js 2025
💡 Result:
Short answer — as of 2025: yes in Node.js and increasingly in browsers.
-
Node.js: Web Crypto supports Ed25519 (stable since Node v22.13.0; available via globalThis.crypto / require('node:crypto').webcrypto). [1]
-
Browsers: support landed at different times — Firefox added Ed25519 in Firefox 129 (Aug 2024) and is enabled in later releases; Safari/WebKit added support (Safari 17+ / WebKit updates); Chromium/Chrome support began rolling out around Chrome 137 (2025). Broad compatibility (major engines) is present by 2025, but check client versions before relying on it. [2][3][4]
Sources:
[1] Node.js Web Crypto API docs (history notes).
[2] Firefox 129 release notes / MDN (Ed25519 support).
[3] "SubtleCrypto sign: Ed25519" (Can I use) browser support table.
[4] MDN SubtleCrypto.sign (Ed25519 algorithm).
🏁 Script executed:
# Find package configuration files to understand target environments
find js/token -name "package.json" -o -name "tsconfig.json" -o -name ".browserslistrc" | head -20Repository: moq-dev/moq
Length of output: 131
🏁 Script executed:
# Check if there's any documentation about runtime compatibility
fd -e md js/token | head -20Repository: moq-dev/moq
Length of output: 392
🏁 Script executed:
# Read package.json to check target environments
cat js/token/package.jsonRepository: moq-dev/moq
Length of output: 810
🏁 Script executed:
# Read tsconfig.json to check compilation target
cat js/token/tsconfig.jsonRepository: moq-dev/moq
Length of output: 155
🏁 Script executed:
# Check if there are any runtime checks, error handling, or fallbacks around Ed25519
rg -A 5 -B 5 "Ed25519|EdDSA" js/token/src/Repository: moq-dev/moq
Length of output: 6934
🏁 Script executed:
# Check parent tsconfig for target settings
cat js/tsconfig.jsonRepository: moq-dev/moq
Length of output: 1387
🏁 Script executed:
# Search for any README or documentation about version requirements
fd -e md . js/token | xargs grep -l "node\|version\|compatibility" 2>/dev/null || echo "No matching docs found"Repository: moq-dev/moq
Length of output: 73
🏁 Script executed:
# Check root tsconfig as well
cat tsconfig.json 2>/dev/null || echo "No root tsconfig"Repository: moq-dev/moq
Length of output: 71
🏁 Script executed:
# Read the README for version/compatibility information
cat js/token/README.mdRepository: moq-dev/moq
Length of output: 3452
🏁 Script executed:
# Check if there's any error handling or version checks elsewhere in the codebase
rg "crypto\.subtle|generateKey" js/token/src/ -A 3 -B 3Repository: moq-dev/moq
Length of output: 3181
Document Ed25519 runtime requirements or add error handling.
Ed25519 support in Web Crypto is recent and varies by runtime: Node.js v22.13.0+ (stable), Chrome 137+, Firefox 129+, Safari 17+. Without documented version requirements or error handling, users on unsupported versions will encounter unhelpful runtime errors when calling generateEdDsaKey. Either document the minimum versions in the README or wrap the crypto.subtle.generateKey call with a try-catch that provides a clearer error message.
🤖 Prompt for AI Agents
In `@js/token/src/generate.ts` around lines 136 - 161, Wrap the
crypto.subtle.generateKey call in generateEdDsaKey with a try-catch and throw or
log a clear, actionable error that states Ed25519/WebCrypto is unsupported in
the current runtime and suggests minimum versions (e.g., Node 22.13.0+, Chrome
137+, Firefox 129+, Safari 17+) or to upgrade/enable the appropriate feature;
alternatively, add these minimum runtime requirements to the README, but if
catching, ensure the thrown error includes the original error message for
debugging and references generateEdDsaKey/Ed25519 so callers can locate the
issue.
|
I think theres some lint errors in the new files, I'll fix them Gimme a day or two, |
kixelated
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit about src vs dist.
| "files": [ | ||
| "./src" | ||
| "./src", | ||
| "./dist" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package.ts script rewrite src to dist. I would do that instead of bundling both.
| ".": "./src/index.ts" | ||
| }, | ||
| "bin": { | ||
| "moq-token": "./dist/cli" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same deal, I like specifying src for development and rewriting package.json for release.
npm add moq-tokenwas incorrect, fixed it tonpm add @moq/tokenbuild-clioption, should work asnpm install -g @moq/token, it should match the Rust CLIHopefully it should make this easier for JS/TS developers not familiar with Rust