SDK: Fix getNFT tests, Playground: Fix build #8242
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
WalkthroughTests updated to adjust Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Route as API Route (paywall/chat)
participant Env as Environment
participant SDK as thirdweb Client / thirdwebAI
participant Facilitator as twFacilitator
participant Backend as Payment/Engine
Client->>Route: HTTP request (GET or POST)
Route->>Env: read SECRET_KEY and other env vars
Route->>SDK: instantiate thirdweb client / thirdwebAI (per-request)
Route->>Facilitator: build/rebuild facilitator with client + env
Route->>Backend: process payment or handle chat stream
Backend-->>Route: result / stream
Route-->>Client: HTTP response / stream
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
size-limit report 📦
|
MananTank
force-pushed
the
10-13-sdk_fix_getnfts.test
branch
from
0d3b80a to
fb3e423
Compare
MananTank
force-pushed
the
10-13-sdk_fix_getnfts.test
branch
from
fb3e423 to
060d435
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8242 +/- ##
==========================================
- Coverage 55.02% 55.02% -0.01%
==========================================
Files 919 919
Lines 60583 60583
Branches 4126 4127 +1
==========================================
- Hits 33338 33335 -3
- Misses 27141 27145 +4
+ Partials 104 103 -1
🚀 New features to boost your workflow:
|
MananTank
force-pushed
the
10-13-sdk_fix_getnfts.test
branch
from
8d02352 to
126a370
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/playground-web/src/app/api/chat/route.ts (1)
10-10: Remove the debug log that exposes the secret key.This console.log statement creates a critical security vulnerability by logging the SECRET_KEY, which may be exposed through logging systems, CI/CD pipelines, or monitoring tools.
Apply this diff:
-console.log("DEBUG api/chat: SECRET_KEY", SECRET_KEY); -apps/playground-web/src/app/api/paywall/route.ts (1)
13-13: Remove the debug log that exposes the secret key.This console.log statement creates a critical security vulnerability by logging the SECRET_KEY, which may be exposed through logging systems, CI/CD pipelines, or monitoring tools.
Apply this diff:
-console.log("DEBUG api/paywall: SECRET_KEY", SECRET_KEY); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/playground-web/src/app/api/chat/route.ts(1 hunks)apps/playground-web/src/app/api/paywall/route.ts(1 hunks)packages/thirdweb/src/extensions/erc721/read/getNFT.test.ts(1 hunks)packages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.tsapps/playground-web/src/app/api/paywall/route.tsapps/playground-web/src/app/api/chat/route.tspackages/thirdweb/src/extensions/erc721/read/getNFT.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.tspackages/thirdweb/src/extensions/erc721/read/getNFT.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.tsapps/playground-web/src/app/api/paywall/route.tsapps/playground-web/src/app/api/chat/route.tspackages/thirdweb/src/extensions/erc721/read/getNFT.test.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.tspackages/thirdweb/src/extensions/erc721/read/getNFT.test.ts
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/playground-web/src/app/api/paywall/route.tsapps/playground-web/src/app/api/chat/route.ts
🧬 Code graph analysis (2)
apps/playground-web/src/app/api/paywall/route.ts (2)
apps/playground-web/src/app/ai/api/types.ts (1)
API_URL(1-1)packages/thirdweb/src/x402/facilitator.ts (1)
facilitator(107-262)
apps/playground-web/src/app/api/chat/route.ts (2)
packages/ai-sdk-provider/src/exports/thirdweb.ts (1)
createThirdwebAI(2-2)packages/ai-sdk-provider/src/provider.ts (1)
createThirdwebAI(608-610)
🪛 Gitleaks (8.28.0)
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts
[high] 51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 125-125: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 162-162: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 199-199: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
packages/thirdweb/src/extensions/erc721/read/getNFT.test.ts
[high] 47-47: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (2)
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/thirdweb/src/extensions/erc721/read/getNFT.test.ts (1)
47-47: LGTM! Snapshot update reflects correct tokenAddress casing.The tokenAddress casing update in this test snapshot aligns with the expected checksum format for Ethereum addresses. This is a valid test data update.
Note: The static analysis warning about a "Generic API Key" is a false positive—this is a legitimate Ethereum contract address used in tests.
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts (1)
51-51: LGTM! Snapshot updates reflect correct tokenAddress casing.The tokenAddress casing updates in these test snapshots align with the expected checksum format for Ethereum addresses. These are valid test data updates consistent with the changes in getNFT.test.ts.
Note: The static analysis warnings about "Generic API Key" are false positives—these are legitimate Ethereum contract addresses used in tests.
Also applies to: 88-88, 125-125, 162-162, 199-199
MananTank
force-pushed
the
10-13-sdk_fix_getnfts.test
branch
from
126a370 to
3f03bbf
Compare
MananTank
changed the title
Merge activity
|
<!--
## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes"
If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000):
## Notes for the reviewer
Anything important to call out? Be sure to also clarify these in your comments.
## How to test
Unit tests, playground, etc.
-->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR primarily focuses on updating the `tokenAddress` format in the tests and modifying the instantiation of the `thirdwebAI` and `thirdwebClient` to use a consistent `SECRET_KEY` variable for better readability and maintainability.
### Detailed summary
- Updated `tokenAddress` format in `getNFT.test.ts` and `getNFTs.test.ts`.
- Changed `thirdwebAI` instantiation in `route.ts` to use `SECRET_KEY` variable.
- Modified `thirdwebClient` instantiation in `paywall/route.ts` to use `SECRET_KEY` variable.
- Maintained existing environment variable usage for wallet addresses and API URLs.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **New Features**
* Added a new public GET paywall endpoint.
* **Tests**
* Updated NFT test snapshots to correct token address casing.
* Removed an obsolete single-line test comment.
* **Chores**
* API handlers now initialize needed services per request and read required environment secrets.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
10-13-sdk_fix_getnfts.test
branch
from
3f03bbf to
1a19e14
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/playground-web/src/app/api/paywall/route.ts (1)
1-9: Add missingexport const dynamic = "force-dynamic";andimport "server-only";directives.According to the PR description, this file should export
const dynamic = "force-dynamic";for runtime configuration. Additionally, per coding guidelines, server components inapps/playground-webshould start withimport "server-only";.Apply this diff:
+import "server-only"; + import { type NextRequest, NextResponse } from "next/server"; import { createThirdwebClient, defineChain } from "thirdweb"; import { toUnits } from "thirdweb/utils"; import { facilitator, settlePayment } from "thirdweb/x402"; import { token } from "../../payments/x402/components/constants"; +export const dynamic = "force-dynamic"; + // Allow streaming responses up to 5 minutes export const maxDuration = 300;As per coding guidelines
♻️ Duplicate comments (1)
apps/playground-web/src/app/api/paywall/route.ts (1)
21-22: Replace hardcoded localhost URL with environment variable.The hardcoded
localhost:3030URL will break in production and other environments. This was previously flagged in past review comments.Apply this diff:
- // const API_URL = `https://${process.env.NEXT_PUBLIC_API_URL || "api.thirdweb.com"}`; - const API_URL = "http://localhost:3030"; + const API_URL = `https://${process.env.NEXT_PUBLIC_API_URL || "api.thirdweb.com"}`;Alternatively, import the constant from
apps/playground-web/src/app/ai/api/types.ts:+import { API_URL } from "../../ai/api/types"; + import { type NextRequest, NextResponse } from "next/server"; // ... rest of importsThen remove lines 21-22:
const ENGINE_VAULT_ACCESS_TOKEN = process.env .ENGINE_VAULT_ACCESS_TOKEN as string; - // const API_URL = `https://${process.env.NEXT_PUBLIC_API_URL || "api.thirdweb.com"}`; - const API_URL = "http://localhost:3030";
🧹 Nitpick comments (1)
apps/playground-web/src/app/api/paywall/route.ts (1)
11-29: Hoist Thirdweb client and facilitator to module scope
This API route instantiatescreateThirdwebClientandfacilitatorper request, adding latency. If configuration is static, move them to module scope or memoize; otherwise benchmark to confirm acceptable overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/playground-web/src/app/api/chat/route.ts(1 hunks)apps/playground-web/src/app/api/paywall/route.ts(1 hunks)packages/thirdweb/src/extensions/erc721/read/getNFT.test.ts(1 hunks)packages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/playground-web/src/app/api/chat/route.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
apps/playground-web/src/app/api/paywall/route.tspackages/thirdweb/src/extensions/erc721/read/getNFT.test.tspackages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/playground-web/src/app/api/paywall/route.tspackages/thirdweb/src/extensions/erc721/read/getNFT.test.tspackages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/playground-web/src/app/api/paywall/route.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/extensions/erc721/read/getNFT.test.tspackages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/extensions/erc721/read/getNFT.test.tspackages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts
🧬 Code graph analysis (1)
apps/playground-web/src/app/api/paywall/route.ts (2)
apps/playground-web/src/app/ai/api/types.ts (1)
API_URL(1-1)packages/thirdweb/src/x402/facilitator.ts (1)
facilitator(107-262)
🪛 Gitleaks (8.28.0)
packages/thirdweb/src/extensions/erc721/read/getNFT.test.ts
[high] 47-47: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts
[high] 51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 125-125: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 162-162: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 199-199: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (4)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/thirdweb/src/extensions/erc721/read/getNFT.test.ts (1)
47-47: LGTM! Snapshot updated to checksummed address format.The tokenAddress snapshot has been correctly updated to use EIP-55 checksummed format (mixed-case), which is the standard for Ethereum addresses. This aligns the test expectation with the actual implementation output.
Note: The Gitleaks warning flagging this as an API key is a false positive—this is an Ethereum contract address.
packages/thirdweb/src/extensions/erc721/read/getNFTs.test.ts (1)
51-51: LGTM! Snapshots updated to checksummed address format.All tokenAddress snapshots have been correctly updated to use EIP-55 checksummed format (mixed-case). This is consistent with the changes in
getNFT.test.tsand aligns the test expectations with the actual implementation output.Note: The Gitleaks warnings flagging these as API keys are false positives—these are Ethereum contract addresses.
Also applies to: 88-88, 125-125, 162-162, 199-199
| const client = createThirdwebClient({ | ||
| secretKey: process.env.THIRDWEB_SECRET_KEY as string, | ||
| }); | ||
| export async function GET(request: NextRequest) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return type to GET function.
Per coding guidelines, TypeScript functions should have explicit return types for clarity and type safety.
Apply this diff:
-export async function GET(request: NextRequest) {
+export async function GET(request: NextRequest): Promise<NextResponse> {As per coding guidelines
🤖 Prompt for AI Agents
In apps/playground-web/src/app/api/paywall/route.ts around line 10, the exported
async GET function lacks an explicit return type; update its signature to
include a clear TypeScript return type (for example: export async function
GET(request: NextRequest): Promise<Response> { ... } or use
Promise<NextResponse> if you return NextResponse) and ensure the function body
returns a value matching that type.
2 participants
PR-Codex overview
This PR focuses on updating the
tokenAddressformat in NFT tests and improving the handling of theTHIRDWEB_SECRET_KEYin API routes.Detailed summary
tokenAddressformat ingetNFT.test.tsandgetNFTs.test.tsfrom"0x8a90cab2b38dba80c64b7734e58ee1db38b8992e"to"0x8a90CAb2b38dba80c64b7734e58Ee1dB38B8992e".thirdwebAIinitialization inchat/route.tsto useSECRET_KEY.paywall/route.tsto initializeclientwithSECRET_KEY.Summary by CodeRabbit
New Features
Tests
Chores