Conversation
This adds a fetchWithX402() function to consume X402 protected resources that accept bitcoin through the lightning network.
|
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 X402 support and a unified fetch402 flow for L402/X402, introduces fetchWithX402 and fetch402 public modules, updates exports/rollup, adds examples and tests, and removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/App
participant Fetch as fetch402 / fetchWithX402
participant Cache as KVStorage
participant Server as HTTP Server
participant Wallet as Wallet
Client->>Fetch: fetch(url, fetchArgs, { wallet, store })
Fetch->>Fetch: validate wallet & init fetch args (no-store, CORS)
Fetch->>Cache: check cache for L402 token or X402 payment data
alt cached L402 token
Fetch->>Server: request + Authorization header (macaroon:preimage)
else cached X402 data
Fetch->>Server: request + payment-signature header
else no cache
Fetch->>Server: initial request (Accept-Authenticate)
end
Server-->>Fetch: response (WWW-Authenticate / PAYMENT-REQUIRED / OK)
alt WWW-Authenticate (L402)
Fetch->>Fetch: parse L402 challenge
Fetch->>Wallet: payInvoice({ invoice })
Wallet-->>Fetch: { preimage }
Fetch->>Cache: store token/preimage
Fetch->>Server: retry with Authorization
else PAYMENT-REQUIRED (X402)
Fetch->>Fetch: parse PAYMENT-REQUIRED (base64 JSON)
Fetch->>Wallet: payInvoice({ invoice })
Wallet-->>Fetch: { preimage }
Fetch->>Cache: store scheme, network, preimage, requirements
Fetch->>Server: retry with payment-signature
end
Server-->>Fetch: final response
Fetch-->>Client: return final response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
examples/x402.js (1)
1-1: Import path inconsistency with README documentation.The README examples use
@getalby/lightning-tools/x402, but this example imports from@getalby/lightning-tools/l402. For consistency with documentation, consider using the x402 import path:Suggested change
-import { fetchWithX402 } from "@getalby/lightning-tools/l402"; +import { fetchWithX402 } from "@getalby/lightning-tools/x402";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/x402.js` at line 1, The import in examples/x402.js uses the wrong package path; update the import of fetchWithX402 to use the documented module path by changing the module specifier from "@getalby/lightning-tools/l402" to "@getalby/lightning-tools/x402" so examples match the README and consumers can find fetchWithX402.src/l402/x402.ts (2)
33-34:unescape()is deprecated.The
unescape()function is deprecated. Consider using a more modern approach for UTF-8 safe base64 encoding.Alternative using TextEncoder (for environments that support it)
- // btoa only handles latin1; encode via UTF-8 to be safe - return btoa(unescape(encodeURIComponent(json))); + // btoa only handles latin1; encode via UTF-8 to be safe + const bytes = new TextEncoder().encode(json); + const binString = Array.from(bytes, (byte) => String.fromCodePoint(byte)).join(""); + return btoa(binString);Note: The current approach works and is widely used despite the deprecation. This is a low-priority refactor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 33 - 34, Replace the deprecated unescape(...) pattern used in the base64 encoding return statement (currently: return btoa(unescape(encodeURIComponent(json)))); with a modern UTF-8-safe approach: use TextEncoder to encode the JSON string to a Uint8Array, then convert that byte array into a base64 string (or use Buffer.from(...).toString('base64') in Node). Update the function that returns this expression to use TextEncoder (or Buffer) instead of unescape, preserving the same output but avoiding the deprecated API.
45-47: Redundant null check foroptions.The
optionsparameter is already destructured on line 41-43, so it cannot be nullish at this point. This check is unreachable.Remove redundant check
- if (!options) { - options = {}; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 45 - 47, Remove the unreachable null-check block that sets options to {} — the parameter named options is already destructured earlier, so delete the if (!options) { options = {}; } statement (and any empty-branch scaffolding) from x402.ts; ensure code continues to use the existing destructured values and that no other logic depends on reassigning options afterwards.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/x402.js`:
- Around line 21-26: The promise chain using
fetchWithX402(...).then(...).then(...) does not guarantee nwc.close() runs on
error; wrap the async flow so nwc.close() is always called (e.g., use
async/await with try/catch/finally or append .catch(...).finally(...)) and
ensure errors from fetchWithX402 or response.json() are caught and logged before
closing the NWC connection; update the code around fetchWithX402, response.json,
and nwc.close to perform final cleanup in a finally block (or .finally) and
surface/log any error.
In `@src/l402/x402.ts`:
- Around line 75-78: The catch block currently writes null via
store.setItem(url, null as unknown as string) which violates the KVStorage
string contract and will store the literal "null"; change the catch to either
call store.removeItem(url) if a deletion API exists or set an empty string via
store.setItem(url, "") and update the code that reads the cache (the
getItem/cache-check code that checks the stored value) to treat empty string as
a missing/corrupt value (i.e., skip cache and perform a fresh request). Ensure
you update references to store.setItem and the cache-check logic so both agree
on the chosen sentinel for "no entry."
- Around line 61-63: fetchArgs.headers may be a Headers instance or a string[][]
per RequestInit, so don't assume a plain object; add a small normalization step
that converts fetchArgs.headers (Headers | string[][] | Record<string,string> |
undefined) into a plain Record<string,string> (e.g., iterate Headers.entries()
or the array pairs and copy into a new object), use that plain object wherever
the code currently mutates fetchArgs.headers (the places referenced around the
current assignments at lines ~85 and ~148), and then set fetchArgs.headers =
normalizedHeaders so subsequent property assignments are safe and won't throw.
---
Nitpick comments:
In `@examples/x402.js`:
- Line 1: The import in examples/x402.js uses the wrong package path; update the
import of fetchWithX402 to use the documented module path by changing the module
specifier from "@getalby/lightning-tools/l402" to
"@getalby/lightning-tools/x402" so examples match the README and consumers can
find fetchWithX402.
In `@src/l402/x402.ts`:
- Around line 33-34: Replace the deprecated unescape(...) pattern used in the
base64 encoding return statement (currently: return
btoa(unescape(encodeURIComponent(json)))); with a modern UTF-8-safe approach:
use TextEncoder to encode the JSON string to a Uint8Array, then convert that
byte array into a base64 string (or use Buffer.from(...).toString('base64') in
Node). Update the function that returns this expression to use TextEncoder (or
Buffer) instead of unescape, preserving the same output but avoiding the
deprecated API.
- Around line 45-47: Remove the unreachable null-check block that sets options
to {} — the parameter named options is already destructured earlier, so delete
the if (!options) { options = {}; } statement (and any empty-branch scaffolding)
from x402.ts; ensure code continues to use the existing destructured values and
that no other logic depends on reassigning options afterwards.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ce96b42-3b5b-4dad-ad7a-ab791dc07a1d
📒 Files selected for processing (8)
README.mdexamples/x402.jspackage.jsonrollup.config.jssrc/l402/index.tssrc/l402/x402.test.tssrc/l402/x402.tssrc/x402/index.ts
We set the JSON in the storage. Let's assume it's valid and keep the code small.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/l402/x402.ts (1)
61-63:⚠️ Potential issue | 🟠 MajorHeaders type assumption still needs to be addressed.
As flagged previously,
fetchArgs.headerscan be aHeadersinstance,string[][], orRecord<string, string>per theRequestInittype. The direct property assignment on lines 80 and 143 (fetchArgs.headers["payment-signature"] = ...) will fail at runtime if headers is aHeadersinstance or array.♻️ Normalize headers to a plain object
- if (!fetchArgs.headers) { - fetchArgs.headers = {}; - } + // Normalize headers to a plain object for safe mutation + const normalizedHeaders: Record<string, string> = {}; + if (fetchArgs.headers) { + if (fetchArgs.headers instanceof Headers) { + fetchArgs.headers.forEach((value, key) => { + normalizedHeaders[key] = value; + }); + } else if (Array.isArray(fetchArgs.headers)) { + for (const [key, value] of fetchArgs.headers) { + normalizedHeaders[key] = value; + } + } else { + Object.assign(normalizedHeaders, fetchArgs.headers); + } + } + fetchArgs.headers = normalizedHeaders;,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 61 - 63, fetchArgs.headers may be a Headers instance or string[][] as well as a plain object, so normalize it to a Record<string,string> before writing to it: create a small helper (e.g. normalizeHeaders) that accepts fetchArgs.headers, returns {} if falsy, converts a Headers instance by iterating headers.entries(), converts string[][] to an object, and returns the original object if it's already a Record; call this helper where fetchArgs.headers is checked/initialized and before the assignments that reference fetchArgs.headers["payment-signature"] (the assignments on/around the lines you flagged) and then assign the normalized object back to fetchArgs.headers (you may cast to Record<string,string> to satisfy types).
🧹 Nitpick comments (2)
src/l402/x402.ts (2)
5-8: Consider unifying Wallet interface with l402.ts.The
Walletinterface here differs from the one inl402.ts(which has a requiredsendPaymentmethod). Having two distinctWalletinterfaces in the same package may confuse consumers. Consider exporting a shared interface fromutils.tsor documenting the differences clearly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 5 - 8, The Wallet interface in x402.ts declares optional sendPayment and payInvoice whereas l402.ts defines a Wallet with a required sendPayment, causing conflicting types; unify them by moving a single Wallet interface into a shared module (e.g., utils.ts) and export it for use in both x402.ts and l402.ts, ensuring the signature matches the canonical implementation (decide whether sendPayment is required or optional) and update imports in files that reference Wallet (e.g., adjust usages in functions/classes that call sendPayment or payInvoice to match the unified type).
33-34:unescapeis deprecated.The
unescape()function is deprecated. While thebtoa(unescape(encodeURIComponent(...)))pattern works, consider usingTextEncoderfor a more modern approach.♻️ Modern alternative
- // btoa only handles latin1; encode via UTF-8 to be safe - return btoa(unescape(encodeURIComponent(json))); + // Encode UTF-8 string to base64 + const bytes = new TextEncoder().encode(json); + const binary = Array.from(bytes, (b) => String.fromCharCode(b)).join(""); + return btoa(binary);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 33 - 34, Replace the deprecated unescape pattern used in the base64 encoding expression btoa(unescape(encodeURIComponent(json))) with a TextEncoder-based approach: use new TextEncoder().encode(json) to get a Uint8Array, convert that byte array to a binary string (e.g., map bytes to String.fromCharCode and join) and then call btoa on that binary string; update the expression where btoa(unescape(encodeURIComponent(json))) appears to use this TextEncoder -> Uint8Array -> binary string -> btoa flow instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/l402/x402.ts`:
- Line 73: Wrap the JSON.parse(cachedRaw) call in a try-catch so
corrupted/malformed cached data doesn't throw; inside the catch log or warn
(using the existing logger if present) and set cached to undefined/null (or
otherwise treat as cache miss) so the code falls through to the fresh request
logic; locate the parse in src/l402/x402.ts around the JSON.parse(cachedRaw)
line and update handling of variables cachedRaw and cached accordingly.
---
Duplicate comments:
In `@src/l402/x402.ts`:
- Around line 61-63: fetchArgs.headers may be a Headers instance or string[][]
as well as a plain object, so normalize it to a Record<string,string> before
writing to it: create a small helper (e.g. normalizeHeaders) that accepts
fetchArgs.headers, returns {} if falsy, converts a Headers instance by iterating
headers.entries(), converts string[][] to an object, and returns the original
object if it's already a Record; call this helper where fetchArgs.headers is
checked/initialized and before the assignments that reference
fetchArgs.headers["payment-signature"] (the assignments on/around the lines you
flagged) and then assign the normalized object back to fetchArgs.headers (you
may cast to Record<string,string> to satisfy types).
---
Nitpick comments:
In `@src/l402/x402.ts`:
- Around line 5-8: The Wallet interface in x402.ts declares optional sendPayment
and payInvoice whereas l402.ts defines a Wallet with a required sendPayment,
causing conflicting types; unify them by moving a single Wallet interface into a
shared module (e.g., utils.ts) and export it for use in both x402.ts and
l402.ts, ensuring the signature matches the canonical implementation (decide
whether sendPayment is required or optional) and update imports in files that
reference Wallet (e.g., adjust usages in functions/classes that call sendPayment
or payInvoice to match the unified type).
- Around line 33-34: Replace the deprecated unescape pattern used in the base64
encoding expression btoa(unescape(encodeURIComponent(json))) with a
TextEncoder-based approach: use new TextEncoder().encode(json) to get a
Uint8Array, convert that byte array to a binary string (e.g., map bytes to
String.fromCharCode and join) and then call btoa on that binary string; update
the expression where btoa(unescape(encodeURIComponent(json))) appears to use
this TextEncoder -> Uint8Array -> binary string -> btoa flow instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7372aa9-f581-46dc-bdc6-fd7c3fc31745
📒 Files selected for processing (2)
src/l402/x402.test.tssrc/l402/x402.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/l402/x402.test.ts
If no storage is provided we assume the payment proofs should not be stored. memory storage does not survive a restart and is also not really helpful actually.
This function allows to consume L402 and X402 endpoints. It handles both protocols
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
examples/402.js (1)
18-23: Add error handling to ensure wallet cleanup.If
fetch402orresponse.json()throws,nwc.close()is never called, potentially leaving the connection open. Consider usingfinallyor wrapping in try/catch.♻️ Suggested improvement
-fetch402(url, {}, { wallet: nwc }) - .then((response) => response.json()) - .then((data) => { - console.info(data); - nwc.close(); - }); +fetch402(url, {}, { wallet: nwc }) + .then((response) => response.json()) + .then((data) => { + console.info(data); + }) + .catch((error) => { + console.error("Error:", error); + }) + .finally(() => { + nwc.close(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/402.js` around lines 18 - 23, The promise chain around fetch402 and response.json does not guarantee nwc.close() runs on errors; update the logic that calls fetch402(...).then(...).then(...) so that nwc.close() is invoked in a finally block (or attach a .catch to handle errors and a .finally to call nwc.close()), ensuring the wallet is closed whether fetch402 or response.json() succeeds or throws; locate the callsite using the symbols fetch402, response.json, and nwc.close and move the cleanup into a finally/.finally or try/catch/finally structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 283: The README's description for the fetch402 option incorrectly states
the default storage as "a memory storage" — update the documentation for
fetch402 to state that the default store is NoStorage (the NoStorage
implementation) and mention that consumers can override it with a store
implementing getItem()/setItem() like localStorage; specifically change the
wording referencing "By default a memory storage is used" to explicitly name
NoStorage and briefly state its behavior and how to supply an alternative store
for fetch402.
- Line 236: The README claim that "By default a memory storage is used" is
inaccurate; update the documentation to state that fetchWithX402 and
fetchWithL402 default to NoStorage (which does not persist), or alternatively
change the function defaults to use the in-memory store implementation;
reference the constructors/exports fetchWithX402, fetchWithL402 and the
NoStorage symbol when making the change so the doc and code behavior remain
consistent.
In `@src/l402/fetch402.ts`:
- Around line 8-11: The Wallet interface in this file declares payInvoice as
payInvoice?(args: { invoice: string }) but x402.ts and the call sites in
fetch402 call wallet.payInvoice with a plain string; change the Wallet interface
declaration to payInvoice?(paymentRequest: string): Promise<{ preimage: string
}> so it matches x402.ts, then update the call sites in fetch402 where
wallet.payInvoice is invoked (the two places currently passing a string) to
continue passing the string unchanged; ensure type signatures for payInvoice and
any related handling (e.g., variable names) are consistent across Wallet,
x402.ts, and fetch402.ts.
---
Nitpick comments:
In `@examples/402.js`:
- Around line 18-23: The promise chain around fetch402 and response.json does
not guarantee nwc.close() runs on errors; update the logic that calls
fetch402(...).then(...).then(...) so that nwc.close() is invoked in a finally
block (or attach a .catch to handle errors and a .finally to call nwc.close()),
ensuring the wallet is closed whether fetch402 or response.json() succeeds or
throws; locate the callsite using the symbols fetch402, response.json, and
nwc.close and move the cleanup into a finally/.finally or try/catch/finally
structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ac4fa67-5367-47b7-af8b-fe1b325899c4
📒 Files selected for processing (7)
README.mdexamples/402.jspackage.jsonsrc/l402/fetch402.tssrc/l402/index.tssrc/l402/l402.tssrc/l402/x402.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/l402/index.ts
- src/l402/x402.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
236-236:⚠️ Potential issue | 🟡 MinorDefault storage behavior is still documented incorrectly.
Line 236 still says memory storage is the default, which conflicts with the current behavior (
NoStorageby default). Please align this line with thefetch402wording on Line 283.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 236, Update the README 'store' parameter description to reflect that the default storage is NoStorage (not an in-memory store); change the sentence that currently reads "By default a memory storage is used" to match the fetch402 wording used elsewhere (e.g., the fetch402 section) indicating NoStorage is the default. Locate the 'store' description paragraph (the one describing getItem()/setItem()) and replace the default-storage clause so it accurately states "By default NoStorage is used" or equivalent consistent phrasing with the fetch402 section.
🧹 Nitpick comments (1)
README.md (1)
224-229: Polish X402 wording for clarity.A quick wording cleanup would improve readability (e.g., “Similar to L402, X402…”, “other coins and networks”). This is docs-only but helps avoid ambiguity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 224 - 229, Update the README wording to improve clarity: add a comma after "Similar to L402", change "is an open protocol" to "X402 is an open protocol", pluralize "networks" in "other coins and networks", and rephrase the parenthetical to read e.g. "(Note: X402 also works with other coins and networks; this library supports X402 resources that accept Bitcoin on the Lightning Network)"; ensure the `fetchWithX402` mention remains and reads "This library includes a fetchWithX402 function to consume X402‑protected resources that support the Lightning Network."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 244-249: Update the misleading comment to reflect that the payment
proof is persisted to localStorage rather than kept only in memory: modify the
comment above the fetchWithX402 example to say the payment proof will be stored
in persistent storage (window.localStorage) and reused for subsequent requests,
and ensure the example continues to show fetchWithX402(..., { wallet: myWallet,
store: window.localStorage }).
---
Duplicate comments:
In `@README.md`:
- Line 236: Update the README 'store' parameter description to reflect that the
default storage is NoStorage (not an in-memory store); change the sentence that
currently reads "By default a memory storage is used" to match the fetch402
wording used elsewhere (e.g., the fetch402 section) indicating NoStorage is the
default. Locate the 'store' description paragraph (the one describing
getItem()/setItem()) and replace the default-storage clause so it accurately
states "By default NoStorage is used" or equivalent consistent phrasing with the
fetch402 section.
---
Nitpick comments:
In `@README.md`:
- Around line 224-229: Update the README wording to improve clarity: add a comma
after "Similar to L402", change "is an open protocol" to "X402 is an open
protocol", pluralize "networks" in "other coins and networks", and rephrase the
parenthetical to read e.g. "(Note: X402 also works with other coins and
networks; this library supports X402 resources that accept Bitcoin on the
Lightning Network)"; ensure the `fetchWithX402` mention remains and reads "This
library includes a fetchWithX402 function to consume X402‑protected resources
that support the Lightning Network."
this makes the implementation easier and webln is not widely used
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/l402/l402.ts (2)
30-33:⚠️ Potential issue | 🟡 MinorHandle corrupt cache entries as cache misses.
JSON.parse(cachedL402Data)can throw and currently breaks the request flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/l402.ts` around lines 30 - 33, The cachedL402Data parsing can throw and should be treated as a cache miss; wrap the JSON.parse(cachedL402Data) call in a try/catch, and on parse error remove or invalidate the stored entry (store.removeItem or store.setItem(url, null)) and proceed without using the cached value so the request continues normally; update the block around cachedL402Data / fetchArgs.headers["Authorization"] to only set the Authorization header when parsing succeeds.
27-29:⚠️ Potential issue | 🟠 MajorNormalize
fetchArgs.headersto a plain object before mutation.Direct bracket assignment (
fetchArgs.headers["Authorization"]) assumes a plain object. SinceRequestInit.headersacceptsHeaders, tuple arrays, or plain objects, bracket assignment will fail or silently skip setting headers when callers passHeadersobjects or tuple arrays. Initialize or convert to a plain object record instead of assigning an empty object only when falsy.Applies to lines: 27-29, 33-34, 38, 59-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/l402.ts` around lines 27 - 29, The code assumes fetchArgs.headers is a plain object when setting values; instead, normalize fetchArgs.headers to a plain object Record<string,string> before mutation (handle cases where it's undefined, a Headers instance, or a [string,string][] array), then assign Authorization and other headers on that object and reassign it back to fetchArgs.headers; update the logic around the fetchArgs variable where headers are handled (the block initializing fetchArgs.headers and the places that set fetchArgs.headers["Authorization"] and other keys) to perform this conversion and safe assignment.
♻️ Duplicate comments (2)
src/l402/x402.ts (2)
26-28:⚠️ Potential issue | 🟠 MajorNormalize
fetchArgs.headersbefore writing auth headers.
RequestInit.headersis a union type; direct property assignment is only safe for plain objects. WithHeaders/tuple input, the payment header can be silently omitted.Proposed fix
+const toHeaderRecord = ( + headers: HeadersInit | undefined, +): Record<string, string> => { + const out: Record<string, string> = {}; + if (!headers) return out; + if (headers instanceof Headers) { + headers.forEach((v, k) => (out[k] = v)); + return out; + } + if (Array.isArray(headers)) { + for (const [k, v] of headers) out[k] = v; + return out; + } + return { ...headers }; +}; if (!fetchArgs) { fetchArgs = {}; } @@ - if (!fetchArgs.headers) { - fetchArgs.headers = {}; - } + fetchArgs.headers = toHeaderRecord(fetchArgs.headers);Also applies to: 45-45, 101-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 26 - 28, fetchArgs.headers may be a Headers instance or header tuples so directly assigning properties can silently drop headers; before you add auth/payment headers normalize fetchArgs.headers to a plain object (e.g. const normalized = Object.fromEntries(new Headers(fetchArgs.headers || {})); then set fetchArgs.headers = normalized) so subsequent assignments to fetchArgs.headers['Authorization'] or the payment header write to a plain JS object. Apply this normalization wherever fetchArgs.headers is prepared/checked (the occurrences around fetchArgs.headers initialization and the places that add auth/payment headers).
30-39:⚠️ Potential issue | 🟡 MinorGuard cached JSON parsing to avoid hard failures on corrupt entries.
A malformed cache value currently throws and aborts the request path instead of falling back to a fresh fetch.
Proposed fix
const cachedRaw = store.getItem(url); if (cachedRaw) { @@ - cached = JSON.parse(cachedRaw); + try { + cached = JSON.parse(cachedRaw); + } catch (_) { + cached = null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.ts` around lines 30 - 39, Wrap the JSON.parse(cachedRaw) call in a try/catch so corrupt cache entries don't throw: when store.getItem(url) returns cachedRaw, attempt to parse into the existing cached variable inside a try block and on any error set cached = null and optionally call store.removeItem(url) to evict the bad entry; ensure subsequent logic that checks cached still works unchanged (references: cachedRaw, cached, JSON.parse, store.getItem, store.removeItem).
🧹 Nitpick comments (2)
src/l402/x402.test.ts (1)
385-405: Add coverage forHeadersand tuple-arrayheadersinputs.The current custom-header test only validates plain-object headers. Adding cases for
new Headers(...)and[["X-Custom","value"]]will catchHeadersInithandling regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/x402.test.ts` around lines 385 - 405, Update the "passes custom fetchArgs through to all fetch calls" test for fetchWithX402 to also validate HeadersInit variants (a Headers instance and an array-of-tuples) in addition to the plain object: change the test to run the same scenario for headers provided as { "X-Custom": "value" }, new Headers({ "X-Custom": "value" }), and [["X-Custom","value"]], then assert for each fetchMock.mock.calls that fetchInit.method is "POST" and that the resulting request headers include "X-Custom":"value"; this ensures fetchWithX402 correctly handles Headers and tuple-array inputs as well as plain objects.src/l402/fetch402.ts (1)
14-129: Consider extracting shared payment-flow helpers to reduce drift.
fetch402,fetchWithL402, andfetchWithX402now duplicate cache decode, wallet payment, and header mutation paths. Pulling shared helpers (header normalization, cache parse, wallet response validation) intoutils.tswill make protocol updates safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/l402/fetch402.ts` around lines 14 - 129, fetch402, fetchWithL402, and fetchWithX402 duplicate cache decoding, payment invocation, and header mutation logic; extract shared helpers into utils to centralize behavior. Create helper functions such as normalizeHeaders(headers), parseCacheEntry(raw: string) returning a discriminated union for L402/X402, validateWalletResponse(invResp) to ensure preimage exists, and applyAuthHeaders(fetchArgs.headers, auth) to set Authorization or payment-signature; replace duplicated blocks in fetch402 (see parseL402, buildX402PaymentSignature, wallet.payInvoice, store.getItem/setItem, HEADER_KEY) to call these helpers and persist to store via a single saveCache(url, entry) utility so all three fetch variants use the same decode, payment, and header-setting flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jest.config.ts`:
- Around line 1-4: The import and type alias in jest.config.ts use a
non-existent Config from "ts-jest"; replace that with the correct exported type
by importing JestConfigWithTsJest from "ts-jest" and update the config
variable's type from Config to JestConfigWithTsJest (i.e., change the import
line and change const config: Config to const config: JestConfigWithTsJest) so
TypeScript and CI accept the jest config.
In `@src/l402/fetch402.ts`:
- Around line 34-37: The current call to JSON.parse(cachedRaw) can throw and
abort the request; wrap the parse in a try/catch so JSON deserialization errors
are treated as a cache miss (i.e., fall through) rather than throwing.
Specifically, around the store.getItem(url) path where cachedRaw is parsed into
cached, catch any SyntaxError (or generic error), log or ignore it, and proceed
as if cached is null so the existing cached?.token && cached?.preimage check
only runs on a successful parse.
- Around line 66-77: The parsed L402 fields and wallet response are not
validated before being stored or used; update the flow around parseL402,
token/macaroon, invoice, wallet.payInvoice, and invResp.preimage to explicitly
check for presence and throw/return a meaningful error if any are missing (no-op
on missing token/macaroon, missing invoice, or missing invResp.preimage), and
only call store.setItem and set fetchArgs.headers["Authorization"] when token
and preimage are present; apply the same validations to the analogous block
handling lines 109-129 so you never persist invalid cache entries or construct
malformed Authorization headers.
- Around line 29-31: The code mutates fetchArgs.headers assuming it's a plain
object which breaks when headers is a Headers instance or a tuples array;
instead normalize to the standard Headers API, e.g. create const headers = new
Headers(fetchArgs.headers), call headers.set('Authorization', ...) or
headers.append(...) as needed, then assign fetchArgs.headers = headers (or leave
as Headers) so tuple/Headers inputs are handled safely; apply this pattern
everywhere you currently do direct object mutation of fetchArgs.headers (the
sites around the existing checks and assignments using fetchArgs.headers in this
file).
In `@src/l402/l402.ts`:
- Around line 45-50: The parsed L402 fields from parseL402 may be missing
token/macaroon or invoice, so before calling wallet.payInvoice or setting the
Authorization header you must validate those values (check that details.token ||
details.macaroon is a non-empty string and details.invoice is present); if
validation fails, bail out by throwing or returning an error/logging a clear
message instead of proceeding, and only construct Authorization: Bearer <token>
when token is defined; apply the same validation guard for the other usage
referenced around the wallet.payInvoice/Authorization logic (the second
occurrence using the parsed fields).
In `@src/l402/x402.ts`:
- Around line 89-99: Validate the wallet.payInvoice() response before persisting
or signing: after calling wallet.payInvoice({ invoice }) check that invResp
exists and that invResp.preimage is present and non-empty; if the preimage is
missing or invalid, abort (throw or return an error) and do not call
store.setItem or proceed to any signing logic (same validation should be added
for the subsequent block around lines 101-106), and include a clear error
message referencing wallet.payInvoice and invResp.preimage so callers can handle
the failed payment appropriately.
---
Outside diff comments:
In `@src/l402/l402.ts`:
- Around line 30-33: The cachedL402Data parsing can throw and should be treated
as a cache miss; wrap the JSON.parse(cachedL402Data) call in a try/catch, and on
parse error remove or invalidate the stored entry (store.removeItem or
store.setItem(url, null)) and proceed without using the cached value so the
request continues normally; update the block around cachedL402Data /
fetchArgs.headers["Authorization"] to only set the Authorization header when
parsing succeeds.
- Around line 27-29: The code assumes fetchArgs.headers is a plain object when
setting values; instead, normalize fetchArgs.headers to a plain object
Record<string,string> before mutation (handle cases where it's undefined, a
Headers instance, or a [string,string][] array), then assign Authorization and
other headers on that object and reassign it back to fetchArgs.headers; update
the logic around the fetchArgs variable where headers are handled (the block
initializing fetchArgs.headers and the places that set
fetchArgs.headers["Authorization"] and other keys) to perform this conversion
and safe assignment.
---
Duplicate comments:
In `@src/l402/x402.ts`:
- Around line 26-28: fetchArgs.headers may be a Headers instance or header
tuples so directly assigning properties can silently drop headers; before you
add auth/payment headers normalize fetchArgs.headers to a plain object (e.g.
const normalized = Object.fromEntries(new Headers(fetchArgs.headers || {}));
then set fetchArgs.headers = normalized) so subsequent assignments to
fetchArgs.headers['Authorization'] or the payment header write to a plain JS
object. Apply this normalization wherever fetchArgs.headers is prepared/checked
(the occurrences around fetchArgs.headers initialization and the places that add
auth/payment headers).
- Around line 30-39: Wrap the JSON.parse(cachedRaw) call in a try/catch so
corrupt cache entries don't throw: when store.getItem(url) returns cachedRaw,
attempt to parse into the existing cached variable inside a try block and on any
error set cached = null and optionally call store.removeItem(url) to evict the
bad entry; ensure subsequent logic that checks cached still works unchanged
(references: cachedRaw, cached, JSON.parse, store.getItem, store.removeItem).
---
Nitpick comments:
In `@src/l402/fetch402.ts`:
- Around line 14-129: fetch402, fetchWithL402, and fetchWithX402 duplicate cache
decoding, payment invocation, and header mutation logic; extract shared helpers
into utils to centralize behavior. Create helper functions such as
normalizeHeaders(headers), parseCacheEntry(raw: string) returning a
discriminated union for L402/X402, validateWalletResponse(invResp) to ensure
preimage exists, and applyAuthHeaders(fetchArgs.headers, auth) to set
Authorization or payment-signature; replace duplicated blocks in fetch402 (see
parseL402, buildX402PaymentSignature, wallet.payInvoice, store.getItem/setItem,
HEADER_KEY) to call these helpers and persist to store via a single
saveCache(url, entry) utility so all three fetch variants use the same decode,
payment, and header-setting flow.
In `@src/l402/x402.test.ts`:
- Around line 385-405: Update the "passes custom fetchArgs through to all fetch
calls" test for fetchWithX402 to also validate HeadersInit variants (a Headers
instance and an array-of-tuples) in addition to the plain object: change the
test to run the same scenario for headers provided as { "X-Custom": "value" },
new Headers({ "X-Custom": "value" }), and [["X-Custom","value"]], then assert
for each fetchMock.mock.calls that fetchInit.method is "POST" and that the
resulting request headers include "X-Custom":"value"; this ensures fetchWithX402
correctly handles Headers and tuple-array inputs as well as plain objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1b82ca8-615c-4ff9-9c1a-dd9efb1f6570
📒 Files selected for processing (7)
jest.config.tssrc/l402/fetch402.tssrc/l402/l402.test.tssrc/l402/l402.tssrc/l402/utils.tssrc/l402/x402.test.tssrc/l402/x402.ts
jest.config.ts
Outdated
Comment on lines
+1
to
+4
| import type { Config } from "ts-jest"; | ||
|
|
||
| const config: Config = { | ||
| preset: "ts-jest", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the jest.config.ts file
find . -name "jest.config.ts" -o -name "jest.config.js" | head -20Repository: getAlby/js-lightning-tools
Length of output: 86
🏁 Script executed:
# Check if ts-jest is in package.json and what version
if [ -f package.json ]; then
cat package.json | grep -A 5 -B 5 "ts-jest"
fiRepository: getAlby/js-lightning-tools
Length of output: 337
🏁 Script executed:
# Search for ts-jest type definitions or exports
if [ -d node_modules/ts-jest ]; then
echo "=== ts-jest package.json ==="
cat node_modules/ts-jest/package.json | head -50
fiRepository: getAlby/js-lightning-tools
Length of output: 52
🏁 Script executed:
# Look for type definitions in ts-jest
if [ -d node_modules/ts-jest ]; then
echo "=== Available type exports in ts-jest ==="
find node_modules/ts-jest -name "*.d.ts" | head -20
fiRepository: getAlby/js-lightning-tools
Length of output: 52
🏁 Script executed:
# Read the jest.config.ts file
cat -n ./jest.config.tsRepository: getAlby/js-lightning-tools
Length of output: 306
🏁 Script executed:
# Search for how ts-jest types are used elsewhere in the codebase
rg "from.*ts-jest" --type ts --type jsRepository: getAlby/js-lightning-tools
Length of output: 123
🌐 Web query:
ts-jest 29.4.1 type exports Config JestConfigWithTsJest
💡 Result:
In ts-jest 29.4.x (incl. 29.4.1):
JestConfigWithTsJestis exported byts-jest(it’s a helper type for a Jest config object that may includets-jest-specifictransformentries). Its shape is defined as anOmit<Config.InitialOptions, 'transform'>plus atransform?that allows['ts-jest', TsJestTransformerOptions]forms. [1]Configis not ats-jesttype export; it comes from Jest. Jest’s docs show typing configs withimport type { Config } from 'jest'. [2]
Typical usage:
import type { Config } from 'jest'
import type { JestConfigWithTsJest } from 'ts-jest'
const config: Config | JestConfigWithTsJest = {
preset: 'ts-jest',
testEnvironment: 'node',
}
export default configIf you specifically want the same Config namespace ts-jest builds against, it’s @jest/types (since ts-jest’s typings reference import type { Config } from '@jest/types'). [1]
Sources: [1] ts-jest type definitions (29.4.x) showing JestConfigWithTsJest and Config coming from @jest/types; [2] Jest configuration docs showing import type { Config } from 'jest'.
Fix invalid ts-jest type import to unblock CI.
ts-jest does not export Config; use JestConfigWithTsJest instead.
Proposed fix
-import type { Config } from "ts-jest";
+import type { JestConfigWithTsJest } from "ts-jest";
-const config: Config = {
+const config: JestConfigWithTsJest = {
preset: "ts-jest",📝 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.
Suggested change
| import type { Config } from "ts-jest"; | |
| const config: Config = { | |
| preset: "ts-jest", | |
| import type { JestConfigWithTsJest } from "ts-jest"; | |
| const config: JestConfigWithTsJest = { | |
| preset: "ts-jest", |
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Jest TS config parse failed. TS error: Module '"ts-jest"' has no exported member 'Config'. Did you mean to import Config from 'ts-jest' instead? (jest.config.ts:1,15)
🪛 GitHub Check: build
[failure] 1-1:
Module '"ts-jest"' has no exported member 'Config'. Did you mean to use 'import Config from "ts-jest"' instead?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jest.config.ts` around lines 1 - 4, The import and type alias in
jest.config.ts use a non-existent Config from "ts-jest"; replace that with the
correct exported type by importing JestConfigWithTsJest from "ts-jest" and
update the config variable's type from Config to JestConfigWithTsJest (i.e.,
change the import line and change const config: Config to const config:
JestConfigWithTsJest) so TypeScript and CI accept the jest config.
Comment on lines
+66
to
+77
| const details = parseL402(l402Header); | ||
| const token = details.token || details.macaroon; | ||
| const invoice = details.invoice; | ||
|
|
||
| const invResp = await wallet.payInvoice!({ invoice }); | ||
|
|
||
| store.setItem(url, JSON.stringify({ token, preimage: invResp.preimage })); | ||
|
|
||
| fetchArgs.headers["Authorization"] = | ||
| `${HEADER_KEY} ${token}:${invResp.preimage}`; | ||
|
|
||
| return await fetch(url, fetchArgs); |
There was a problem hiding this comment.
Validate parsed auth fields and wallet preimage before reuse/store.
Both branches should reject missing invoice/token/preimage explicitly; otherwise invalid cache entries and malformed auth headers can be generated.
Also applies to: 109-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/l402/fetch402.ts` around lines 66 - 77, The parsed L402 fields and wallet
response are not validated before being stored or used; update the flow around
parseL402, token/macaroon, invoice, wallet.payInvoice, and invResp.preimage to
explicitly check for presence and throw/return a meaningful error if any are
missing (no-op on missing token/macaroon, missing invoice, or missing
invResp.preimage), and only call store.setItem and set
fetchArgs.headers["Authorization"] when token and preimage are present; apply
the same validations to the analogous block handling lines 109-129 so you never
persist invalid cache entries or construct malformed Authorization headers.
Comment on lines
45
to
50
| const details = parseL402(header); | ||
| const token = details.token || details.macaroon; | ||
| const inv = details.invoice; | ||
| const invoice = details.invoice; | ||
|
|
||
| const invResp = await wallet.sendPayment(inv); | ||
| const invResp = await wallet.payInvoice({ invoice }); | ||
|
|
There was a problem hiding this comment.
Validate parsed L402 fields before paying and authorizing.
If www-authenticate is malformed, token/invoice can be missing and produce invalid wallet calls or Authorization: ...undefined.
Proposed fix
const details = parseL402(header);
const token = details.token || details.macaroon;
const invoice = details.invoice;
+ if (!token || !invoice) {
+ throw new Error("l402: invalid www-authenticate header");
+ }
const invResp = await wallet.payInvoice({ invoice });Also applies to: 59-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/l402/l402.ts` around lines 45 - 50, The parsed L402 fields from parseL402
may be missing token/macaroon or invoice, so before calling wallet.payInvoice or
setting the Authorization header you must validate those values (check that
details.token || details.macaroon is a non-empty string and details.invoice is
present); if validation fails, bail out by throwing or returning an
error/logging a clear message instead of proceeding, and only construct
Authorization: Bearer <token> when token is defined; apply the same validation
guard for the other usage referenced around the wallet.payInvoice/Authorization
logic (the second occurrence using the parsed fields).
some protocols (e.g. X402) work with millisats. To make it easier to for example validate the amount this directly exposes the millisats property.
This also adds amount validation to check the invoice amount is of the advertised amount.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a fetchWithX402() function to consume X402 protected resources that accept bitcoin through the lightning network.
it also adds a unified
fetch402()function to consume both. Not sure if it makes sense - it also has some code duplication.Summary by CodeRabbit
New Features
Public API
Documentation
Tests