Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds Bubblegum V2 compressed-NFT support: CLI commands and prompts for Merkle tree and NFT lifecycle (create, fetch, update, transfer, burn), on-disk tree persistence, UMI middleware registration for mpl-bubblegum and DAS API, package dependency additions, extensive docs, and new tests (some DAS-dependent tests skipped). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant TreeStorage
participant DASAPI
participant UMI
participant Explorer
User->>CLI: bg nft create [--wizard | flags]
alt wizard
CLI->>User: interactive prompts (metadata, files, tree)
CLI->>TreeStorage: resolve saved tree by name
else flags/files
CLI->>TreeStorage: resolve tree (name/address) or prompt manual
end
CLI->>DASAPI: upload asset & metadata (optional)
DASAPI-->>CLI: asset/uri response
CLI->>UMI: invoke mintV2 (mpl-bubblegum)
UMI-->>CLI: tx signature
CLI->>CLI: derive assetId (attempt from tx)
CLI->>Explorer: build explorer URL
CLI-->>User: print summary / JSON
sequenceDiagram
participant User
participant CLI
participant DASAPI
participant FS
User->>CLI: bg nft fetch <assetId> [--proof-only] [--download]
CLI->>DASAPI: getAsset(assetId) & getAssetProof(assetId) (parallel)
DASAPI-->>CLI: asset data, merkle proof
alt download
CLI->>FS: write <assetId>-asset.json / <assetId>-proof.json
else
CLI->>CLI: format & display asset + proof
end
CLI-->>User: output (human or JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Points needing extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
docs/bubblegum/README.md(1 hunks)docs/bubblegum/nft-burn.md(1 hunks)docs/bubblegum/nft-create.md(1 hunks)docs/bubblegum/nft-fetch.md(1 hunks)docs/bubblegum/nft-transfer.md(1 hunks)docs/bubblegum/nft-update.md(1 hunks)docs/bubblegum/quick-reference.md(1 hunks)docs/bubblegum/tree-create.md(1 hunks)docs/bubblegum/tree-list.md(1 hunks)docs/bubblegum/troubleshooting.md(1 hunks)package.json(1 hunks)src/commands/bg/index.ts(1 hunks)src/commands/bg/nft/burn.ts(1 hunks)src/commands/bg/nft/create.ts(1 hunks)src/commands/bg/nft/fetch.ts(1 hunks)src/commands/bg/nft/index.ts(1 hunks)src/commands/bg/nft/transfer.ts(1 hunks)src/commands/bg/nft/update.ts(1 hunks)src/commands/bg/tree/create.ts(1 hunks)src/commands/bg/tree/index.ts(1 hunks)src/commands/bg/tree/list.ts(1 hunks)src/commands/tm/create.ts(1 hunks)src/lib/Context.ts(5 hunks)src/lib/treeStorage.ts(1 hunks)src/lib/util.ts(1 hunks)src/prompts/createBubblegumMetadataPrompt.ts(1 hunks)src/prompts/selectTreePrompt.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-28T23:08:38.782Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 18
File: src/commands/core/asset/burn.ts:179-182
Timestamp: 2025-05-28T23:08:38.782Z
Learning: Burned/destroyed assets should not include explorer URLs for the asset itself in CLI output because burned assets no longer exist on the blockchain and would result in "not found" errors when viewed in explorers.
Applied to files:
docs/bubblegum/nft-burn.mdsrc/commands/bg/nft/burn.ts
📚 Learning: 2025-03-27T21:39:44.269Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 4
File: src/commands/core/asset/index.ts:1-18
Timestamp: 2025-03-27T21:39:44.269Z
Learning: In this project, base commands rely on OCLIF's built-in help system (--help flag) to display available subcommands rather than implementing custom help text in the run method.
Applied to files:
src/commands/bg/nft/index.ts
📚 Learning: 2025-05-28T23:09:37.361Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 18
File: test/commands/core/core.create.test.ts:92-97
Timestamp: 2025-05-28T23:09:37.361Z
Learning: In the Metaplex CLI test suite, the user prefers to avoid redundant assertions in tests when helper functions like `createCoreAsset` and `createCoreCollection` already perform comprehensive validation (exit codes, success messages, ID format validation, and throw errors on failure). The test can rely on the helper functions to fail appropriately rather than duplicating assertions.
Applied to files:
src/prompts/createBubblegumMetadataPrompt.ts
📚 Learning: 2025-06-05T23:37:11.513Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 27
File: src/lib/Context.ts:152-153
Timestamp: 2025-06-05T23:37:11.513Z
Learning: In src/lib/Context.ts, the non-null assertions on config.rpcUrl! and config.commitment! are safe because consolidateConfigs() merges DEFAULT_CONFIG (which provides both values) with user config and overrides, only overwriting with non-undefined values, ensuring these properties are always present.
Applied to files:
src/lib/Context.ts
🧬 Code graph analysis (8)
src/commands/bg/tree/list.ts (1)
src/lib/treeStorage.ts (1)
listTrees(119-127)
src/commands/bg/nft/burn.ts (3)
src/lib/LedgerSigner.ts (1)
publicKey(39-41)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
src/prompts/selectTreePrompt.ts (1)
src/lib/treeStorage.ts (1)
listTrees(119-127)
src/commands/bg/nft/update.ts (3)
src/commands/core/asset/update.ts (1)
uploadJson(172-182)src/lib/util.ts (2)
txSignatureToString(13-13)chain(123-131)src/explorers.ts (1)
generateExplorerUrl(34-37)
src/commands/bg/tree/create.ts (3)
src/lib/treeStorage.ts (3)
isValidTreeName(150-153)getNetworkInfo(34-43)saveTree(85-102)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
src/commands/bg/nft/fetch.ts (2)
src/lib/file.ts (1)
ensureDirectoryExists(13-15)src/lib/util.ts (1)
jsonStringify(15-16)
src/commands/bg/nft/transfer.ts (3)
src/lib/LedgerSigner.ts (1)
publicKey(39-41)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
src/commands/bg/nft/create.ts (4)
src/lib/util.ts (2)
txSignatureToString(13-13)chain(123-131)src/explorers.ts (1)
generateExplorerUrl(34-37)src/lib/treeStorage.ts (1)
getTreeByNameOrAddress(104-117)src/prompts/createBubblegumMetadataPrompt.ts (1)
CreateBubblegumMetadataPromptResult(10-20)
🪛 LanguageTool
docs/bubblegum/README.md
[grammar] ~11-~11: Ensure spelling is correct
Context: ...ures - Massive Cost Savings: After tree rent, minting costs ~0.000005 SOL per N...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/bubblegum/troubleshooting.md
10-10: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
17-17: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
36-36: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
36-36: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
43-43: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
54-54: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
68-68: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
75-75: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
88-88: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
104-104: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
111-111: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
130-130: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
153-153: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
160-160: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
183-183: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
183-183: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
190-190: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
201-201: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
216-216: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
216-216: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
223-223: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
240-240: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
240-240: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
262-262: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
280-280: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
280-280: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
289-289: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
308-308: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
308-308: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
315-315: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
332-332: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
332-332: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
339-339: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
350-350: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
350-350: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
357-357: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
377-377: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
377-377: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
384-384: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
401-401: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
401-401: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
408-408: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
422-422: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
422-422: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
429-429: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
444-444: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
460-460: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
476-476: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
476-476: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
483-483: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
497-497: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
497-497: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
504-504: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
518-518: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
518-518: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
523-523: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
541-541: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
551-551: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
551-551: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
558-558: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
575-575: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
588-588: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
607-607: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
619-619: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
619-619: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
624-624: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/tree-list.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
95-95: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
175-175: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
175-175: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
203-203: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
217-217: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
227-227: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/tree-create.md
64-64: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
64-64: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
116-116: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
116-116: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
152-152: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
261-261: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
261-261: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
280-280: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
280-280: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
294-294: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
306-306: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
319-319: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
329-329: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/nft-fetch.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
95-95: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
175-175: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
175-175: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
203-203: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
217-217: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
227-227: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/nft-burn.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
132-132: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
145-145: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
158-158: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
167-167: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/nft-create.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
95-95: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
175-175: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
175-175: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
203-203: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
217-217: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
227-227: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/README.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
95-95: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
95-95: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
175-175: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
175-175: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
203-203: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
217-217: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/nft-update.md
221-221: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
232-232: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
245-245: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
255-255: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/bubblegum/nft-transfer.md
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
133-133: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
146-146: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
158-158: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
171-171: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
⏰ 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). (3)
- GitHub Check: unit-tests (ubuntu-latest, lts/*)
- GitHub Check: unit-tests (ubuntu-latest, latest)
- GitHub Check: unit-tests (ubuntu-latest, lts/-1)
🔇 Additional comments (15)
docs/bubblegum/quick-reference.md (2)
24-29: Verify cost estimates for accuracy.The cost estimates shown in the table should be verified against current network conditions:
- 16,384 NFTs: ~0.2 SOL
- 131,072 NFTs: ~1.5 SOL
- 1,048,576 NFTs: ~12 SOL
These costs can vary based on rent exemption requirements and network conditions. Consider adding a note that these are approximate estimates and may vary.
#!/bin/bash # Verify current rent costs for tree creation on devnet # This script would need the actual tree creation parameters to calculate exact costs echo "Note: Verify these cost estimates against current Solana rent calculations" echo "Tree costs depend on:" echo "- Max depth (affects account size)" echo "- Buffer size (affects account size)" echo "- Canopy depth (affects account size)" echo "- Current rent exemption rates"Also consider adding a disclaimer:
| Max NFTs | Approximate Cost | |----------|-----------------| | 16,384 | ~0.2 SOL | | 131,072 | ~1.5 SOL | | 1,048,576 | ~12 SOL | +*Note: Costs are approximate and may vary based on network conditions and exact tree configuration.*
1-472: Excellent quick reference documentation!This is a well-organized, comprehensive quick reference that covers all major Bubblegum commands, workflows, and use cases. The authority reference table and common workflows sections are particularly valuable for users.
Key strengths:
- Clear command examples with all options
- Common workflows provide end-to-end guidance
- Authority reference table clarifies permissions
- Use-case-specific examples (music, video, 3D NFTs)
- Troubleshooting quick fixes
src/commands/tm/create.ts (1)
143-148: Good defensive programming fix!The guard prevents a potential runtime error when
jsonFile.properties.filesis undefined or empty. This is a solid improvement that makes the code more robust.The fix properly uses optional chaining and checks array length before accessing the first element.
src/commands/bg/nft/index.ts (1)
1-57: Well-structured NFT command index!The implementation provides helpful, comprehensive guidance for users while following the project's established patterns. The recommended workflow section effectively guides users through the typical Bubblegum NFT lifecycle.
Key strengths:
- Clear subcommand descriptions
- Step-by-step workflow with command examples
- Note about Metaplex Core collections requirement
- References to --help for detailed subcommand documentation
src/commands/bg/index.ts (1)
1-49: Well-structured namespace command with clear guidance.The Bg command provides a comprehensive overview of Bubblegum functionality with clear examples and workflow guidance. The help text effectively guides users through the complete process from tree creation to NFT operations.
src/commands/bg/tree/index.ts (1)
1-29: Clear and concise tree management command.The BgTreeCommand provides appropriate help text for tree management operations with relevant examples and clear subcommand descriptions.
src/commands/bg/tree/list.ts (1)
21-54: Well-implemented tree listing command with good UX.The implementation handles empty states gracefully, formats data appropriately for display, and provides helpful contextual information. The use of emojis to highlight public trees (line 43) and the tip about network filtering (lines 51-53) enhance user experience.
package.json (1)
31-32: No issues found—versions are valid and secure.The specified package versions exist and are free from known vulnerabilities:
@metaplex-foundation/digital-asset-standard-api@^2.0.0resolves to 2.0.0 (only available version in range)@metaplex-foundation/mpl-bubblegum@^5.0.2resolves to 5.0.2 (latest in range)- No security advisories detected for either package
src/commands/bg/nft/burn.ts (1)
55-70: Burn flow and explorer output align with ownership rules and prior burn guidance.The burn command correctly:
- Verifies the signer is either the owner or delegate before calling
burnV2.- Uses the transaction signature (not the asset address) when generating explorer URLs in both the summary and JSON output, so users don’t get broken asset links after a burn. This matches the earlier guidance to avoid asset explorer URLs for destroyed assets. Based on learnings.
Also applies to: 79-100, 107-123
src/commands/bg/nft/update.ts (2)
301-305: Add error handling around JSON upload spinner in flag-based update path.In the flag-based path you start a spinner for uploading JSON but don’t handle failures, so on
uploadJsonrejection the spinner never.fail()and the user gets no clear “upload failed” message:const jsonUploadSpinner = ora('Uploading JSON file...').start() newMetadataUri = await uploadJson(umi, newMetadata) jsonUploadSpinner.succeed('Uploaded JSON')Mirror the interactive path and fail the spinner on error:
- const jsonUploadSpinner = ora('Uploading JSON file...').start() - newMetadataUri = await uploadJson(umi, newMetadata) - jsonUploadSpinner.succeed('Uploaded JSON') + const jsonUploadSpinner = ora('Uploading JSON file...').start() + try { + newMetadataUri = await uploadJson(umi, newMetadata) + jsonUploadSpinner.succeed('Uploaded JSON') + } catch (error) { + jsonUploadSpinner.fail('JSON upload failed') + throw error + }⛔ Skipped due to learnings
Learnt from: tonyboylehub Repo: metaplex-foundation/cli PR: 32 File: src/commands/toolbox/storage/upload.ts:57-63 Timestamp: 2025-07-18T21:03:17.621Z Learning: In the Metaplex CLI toolbox storage upload command (src/commands/toolbox/storage/upload.ts), failed uploads are intentionally included in the cache with empty URIs using `uri || ''`. This behavior is designed to maintain a complete record of all upload attempts and preserve index mapping between files and cache entries.
30-45: Disregard the original suggestion to pass the full Collection struct.According to mpl-bubblegum documentation, when calling
updateMetadataV2, thecollectionfield should receive the collection'sPublicKey(ornone()/nullto indicate no collection), not the full Collection struct. The practical examples show passingcollectionOption?.key ?? null.The current code does pass
collectionData.key(albeit wrapped insome()), which aligns with the correct approach. However, verify whether thesome()wrapper is necessary or if the field should receive the key directly.The original review's suggested fix (passing
{ key, verified }) would make the code more incorrect, not less.Likely an incorrect or invalid review comment.
src/lib/Context.ts (1)
2-27: Context Umi extension and DAS/Bubblegum middleware wiring are correct.Widening
umitoUmi & { rpc: DasApiInterface }is safe becausedasApi()registers RPC helpers onumi.rpcwhen the middleware is invoked, and your code unconditionally applies.use(mplBubblegum()).use(dasApi())before returning. The existing non‑null assertions onconfig.rpcUrl!andconfig.commitment!remain valid per prior learnings—consolidateConfigs(DEFAULT_CONFIG, ...)guarantees these fields are always present.src/commands/bg/nft/fetch.ts (1)
130-231: Fetch flow, error handling, and output look solidAsset ID validation, DAS RPC calls (with clear errors on unsupported endpoints), download vs. inline display, and JSON output handling are all wired cleanly. The asset/proof summary covers key compression, ownership, royalty, and proof details and should be very useful for users.
Also applies to: 291-324
src/prompts/createBubblegumMetadataPrompt.ts (1)
89-107: Review comment is incorrect—tilde expansion works as implementedThe review's core technical claim is unfounded. You asserted that
path.join(os.homedir(), value.slice(1))fails on POSIX becausepath.join('/home/user', '/foo')yields/foo, dropping the home directory.Actual test results show:
path.join('/home/user', '/foo')=/home/user/foo✓- For input
'~/dir/file': bothvalue.slice(1)andvalue.slice(2)produce/home/user/dir/file✓The current code with
value.slice(1)expands tilde paths correctly in the tested environment. The paths are validated, files are checked, and stored paths are normalized properly as implemented.While extracting tilde expansion into a helper function would reduce duplication across
createBubblegumMetadataPrompt.tsandcreateTokenMetadataPrompt.ts(6 instances total), this is a stylistic refactor, not a correctness fix. The claimed bug does not exist.Likely an incorrect or invalid review comment.
src/commands/bg/nft/create.ts (1)
141-214: Overall run() orchestration looks solidTree selection, owner/metadata resolution, mintV2 invocation, and result reporting (including optional JSON + explorer URL) are wired coherently and defensively. No blocking issues in the main flow.
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (2)
src/commands/bg/nft/transfer.ts (1)
57-58: Unify spinner handling to avoid leavingtransferSpinneractive on errorsYou currently start a
spinnerfor fetch and a separatetransferSpinnerfor verification/transfer, but thecatchonly failsspinner. If an error occurs aftertransferSpinnerstarts, it can remain “spinning” while the earlier spinner is flipped to failed, which is confusing UX.Simplest fix: reuse a single spinner and just update its
textacross phases:- const spinner = ora('Fetching asset and proof data...').start() + const spinner = ora('Fetching asset and proof data...').start() @@ - spinner.succeed('Asset and proof data fetched') - - const transferSpinner = ora('Verifying ownership...').start() + spinner.text = 'Verifying ownership...' @@ - if (signerKey !== ownerKey && signerKey !== delegateKey) { - transferSpinner.fail('Transfer failed') + if (signerKey !== ownerKey && signerKey !== delegateKey) { + spinner.fail('Transfer failed') @@ - transferSpinner.text = 'Executing transfer...' + spinner.text = 'Executing transfer...' @@ - transferSpinner.succeed('Compressed NFT transferred successfully!') + spinner.succeed('Compressed NFT transferred successfully!') @@ - } catch (error) { - spinner.fail('Failed to transfer compressed NFT') + } catch (error) { + spinner.fail('Failed to transfer compressed NFT') throw error }This guarantees whichever phase fails/succeeds is always reflected by the same spinner.
Also applies to: 65-65, 67-67, 85-85, 97-97, 117-120
src/commands/bg/nft/create.ts (1)
236-244: HandleRpcChain.Testnetexplicitly ingetNetworkLabel
NetworkLabelincludes'testnet', andgetTreeByNameOrAddressaccepts'testnet' | 'localnet' | ..., butgetNetworkLabelonly mapsMainnetandDevnetand sends everything else to'localnet'. IfRpcChainhas aTestnetvariant, testnet will be misclassified as localnet:
- Tree lookups will read/write under the wrong network label.
- Error messages will incorrectly say “on localnet” when actually on testnet.
Add an explicit
Testnetcase:private getNetworkLabel(chain: RpcChain): NetworkLabel { switch (chain) { case RpcChain.Mainnet: return 'mainnet' case RpcChain.Devnet: return 'devnet' + case RpcChain.Testnet: + return 'testnet' default: return 'localnet' } }If
RpcChaingenuinely lacks aTestnetvariant in your codebase, this can be skipped, but given the existingNetworkLabeland tree storage support, it’s likely intended to be handled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
.amman/mpl_account_compression.sois excluded by!**/*.so.amman/mpl_bubblegum.sois excluded by!**/*.so.amman/mpl_core.sois excluded by!**/*.so.amman/mpl_noop.sois excluded by!**/*.so.amman/spl_account_compression.sois excluded by!**/*.so.amman/spl_noop.sois excluded by!**/*.so
📒 Files selected for processing (19)
.validator.cjs(1 hunks)src/commands/bg/collection/create.ts(1 hunks)src/commands/bg/nft/burn.ts(1 hunks)src/commands/bg/nft/create.ts(1 hunks)src/commands/bg/nft/transfer.ts(1 hunks)src/commands/bg/nft/update.ts(1 hunks)src/commands/bg/tree/create.ts(1 hunks)src/lib/treeStorage.ts(1 hunks)src/prompts/selectTreePrompt.ts(1 hunks)test/commands/bg/README.md(1 hunks)test/commands/bg/bg.collection.create.test.ts(1 hunks)test/commands/bg/bg.integration.test.ts(1 hunks)test/commands/bg/bg.nft.burn.test.ts(1 hunks)test/commands/bg/bg.nft.create.test.ts(1 hunks)test/commands/bg/bg.nft.transfer.test.ts(1 hunks)test/commands/bg/bg.nft.update.test.ts(1 hunks)test/commands/bg/bg.tree.create.test.ts(1 hunks)test/commands/bg/bgcollectionhelpers.ts(1 hunks)test/commands/bg/bghelpers.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-28T23:09:37.361Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 18
File: test/commands/core/core.create.test.ts:92-97
Timestamp: 2025-05-28T23:09:37.361Z
Learning: In the Metaplex CLI test suite, the user prefers to avoid redundant assertions in tests when helper functions like `createCoreAsset` and `createCoreCollection` already perform comprehensive validation (exit codes, success messages, ID format validation, and throw errors on failure). The test can rely on the helper functions to fail appropriately rather than duplicating assertions.
Applied to files:
test/commands/bg/bg.nft.update.test.tstest/commands/bg/README.mdtest/commands/bg/bg.integration.test.tstest/commands/bg/bg.collection.create.test.tstest/commands/bg/bghelpers.tstest/commands/bg/bgcollectionhelpers.tstest/commands/bg/bg.nft.create.test.tstest/commands/bg/bg.nft.burn.test.tstest/commands/bg/bg.nft.transfer.test.tssrc/commands/bg/nft/create.ts
📚 Learning: 2025-05-28T23:08:38.782Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 18
File: src/commands/core/asset/burn.ts:179-182
Timestamp: 2025-05-28T23:08:38.782Z
Learning: Burned/destroyed assets should not include explorer URLs for the asset itself in CLI output because burned assets no longer exist on the blockchain and would result in "not found" errors when viewed in explorers.
Applied to files:
src/commands/bg/nft/burn.tstest/commands/bg/bg.nft.burn.test.ts
📚 Learning: 2025-05-30T13:45:51.808Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 19
File: src/prompts/pluginInquirer.ts:0-0
Timestamp: 2025-05-30T13:45:51.808Z
Learning: In src/prompts/pluginInquirer.ts, changing `let` to `const` for authority variables in plugin configurations causes errors upon blockchain operations. The variables should remain declared with `let` even though they appear to only be assigned once, due to blockchain-specific requirements.
Applied to files:
src/prompts/selectTreePrompt.tssrc/commands/bg/nft/create.ts
🧬 Code graph analysis (16)
src/commands/bg/nft/update.ts (2)
src/lib/util.ts (2)
txSignatureToString(13-13)chain(123-131)src/explorers.ts (1)
generateExplorerUrl(34-37)
test/commands/bg/bg.nft.update.test.ts (1)
test/runCli.ts (1)
runCli(6-58)
src/commands/bg/tree/create.ts (3)
src/lib/treeStorage.ts (4)
isValidTreeName(147-150)getNetworkInfo(35-44)loadTrees(63-74)saveTree(82-99)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
src/commands/bg/collection/create.ts (3)
src/commands/cm/create.ts (1)
createCollection(368-414)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
src/commands/bg/nft/burn.ts (3)
src/lib/LedgerSigner.ts (1)
publicKey(39-41)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
test/commands/bg/bg.integration.test.ts (3)
test/runCli.ts (1)
runCli(6-58)test/commands/bg/bghelpers.ts (2)
createBubblegumTree(169-169)createCompressedNFT(170-170)test/commands/bg/bgcollectionhelpers.ts (1)
createBubblegumCollection(68-68)
test/commands/bg/bg.collection.create.test.ts (1)
test/runCli.ts (1)
runCli(6-58)
src/commands/bg/nft/transfer.ts (3)
src/lib/LedgerSigner.ts (1)
publicKey(39-41)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
test/commands/bg/bghelpers.ts (1)
test/runCli.ts (1)
runCli(6-58)
src/prompts/selectTreePrompt.ts (1)
src/lib/treeStorage.ts (1)
listTrees(116-124)
test/commands/bg/bgcollectionhelpers.ts (1)
test/runCli.ts (1)
runCli(6-58)
test/commands/bg/bg.nft.create.test.ts (1)
test/runCli.ts (1)
runCli(6-58)
test/commands/bg/bg.nft.burn.test.ts (2)
test/runCli.ts (1)
runCli(6-58)test/commands/bg/bghelpers.ts (4)
createBubblegumTree(169-169)createCompressedNFT(170-170)stripAnsi(174-174)extractSignature(173-173)
test/commands/bg/bg.tree.create.test.ts (1)
test/runCli.ts (1)
runCli(6-58)
test/commands/bg/bg.nft.transfer.test.ts (2)
test/runCli.ts (1)
runCli(6-58)test/commands/bg/bghelpers.ts (4)
createBubblegumTree(169-169)createCompressedNFT(170-170)stripAnsi(174-174)extractSignature(173-173)
src/commands/bg/nft/create.ts (4)
src/lib/util.ts (2)
txSignatureToString(13-13)chain(123-131)src/explorers.ts (1)
generateExplorerUrl(34-37)src/lib/treeStorage.ts (1)
getTreeByNameOrAddress(101-114)src/prompts/createBubblegumMetadataPrompt.ts (1)
CreateBubblegumMetadataPromptResult(10-20)
🪛 Biome (2.1.2)
test/commands/bg/bghelpers.ts
[error] 5-5: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
test/commands/bg/bgcollectionhelpers.ts
[error] 5-5: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🪛 markdownlint-cli2 (0.18.1)
test/commands/bg/README.md
9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
45-45: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
61-61: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
82-82: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
120-120: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
⏰ 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). (3)
- GitHub Check: unit-tests (ubuntu-latest, lts/-1)
- GitHub Check: unit-tests (ubuntu-latest, latest)
- GitHub Check: unit-tests (ubuntu-latest, lts/*)
🔇 Additional comments (11)
.validator.cjs (1)
25-49: I need to verify the remaining two program IDs. Let me search for those:Based on the verification results, here's the rewritten review comment:
All configurations verified successfully.
✓ All five
.sofiles exist in the.amman/directory
✓ Program IDs confirmed:
- BGUMAp9Gq7iTEuizy4pqaxsTyUCBK68MDfK752saRPUY is the Metaplex Bubblegum program for creating and managing compressed NFTs
- noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV is the SPL Noop program used with SPL Account Compression
- cmtDvXumGCrqC1Age74AVPhSRVXJMd8PJS91L8KbNCK is the SPL Account Compression program ID
The configuration adds proper Bubblegum support with corresponding noop and compression programs. The Metaplex fork provides SDKs for both mpl-account-compression and mpl-noop programs, which are referenced in the validator configuration.
src/lib/treeStorage.ts (1)
1-150: LGTM! Tree storage implementation is well-structured.The file implements a robust persistence layer for Bubblegum tree metadata with proper error handling, duplicate prevention, and network scoping. The use of
os.homedir()(line 2, 47) addresses the past review comment and provides reliable cross-platform home directory resolution.test/commands/bg/bg.nft.create.test.ts (1)
1-168: LGTM! Comprehensive test coverage for NFT creation.The test suite effectively covers multiple NFT creation scenarios including royalties (0%, 5%, 100%), symbols, collections, and multiple NFTs. The tests rely on helper functions for validation and include appropriate assertions for transaction outputs.
Based on learnings.
test/commands/bg/bg.nft.update.test.ts (1)
1-218: LGTM! Well-structured test scaffolding for future enablement.The skipped test suite is properly structured with clear documentation explaining the DAS API dependency. The defensive checks for missing asset IDs and try/catch error handling demonstrate good test design for when these tests are eventually enabled.
test/commands/bg/bg.nft.transfer.test.ts (1)
1-144: LGTM! Test scaffolding ready for DAS-enabled environments.The skipped test suite properly documents DAS API requirements and includes appropriate defensive checks. The use of
generateSignerfor creating new owner addresses demonstrates good test design.test/commands/bg/bg.nft.burn.test.ts (1)
1-168: LGTM! Burn test scaffolding is well-designed.The skipped test suite appropriately documents DAS API requirements and includes defensive checks for missing asset IDs. The tests verify distinct signatures for sequential burns and include proper output validation. The Explorer URL assertion (line 102) correctly refers to the transaction explorer link, which is appropriate for burn operations.
Based on learnings.
test/commands/bg/bg.collection.create.test.ts (1)
1-159: LGTM! Thorough collection creation test coverage.The test suite comprehensively covers collection creation scenarios including various royalty percentages (0-100%), validation of required flags, output formatting, and usage instructions. The tests follow established patterns and rely on helper functions for validation.
Based on learnings.
src/commands/bg/nft/update.ts (3)
80-112: Editor fallback is now correctly implemented.The implementation properly uses
findAvailableEditor(['nano', 'vi'])(line 108) to probe for available editors before falling back. This addresses the past review comment and ensures nano is tried first with vi as a fallback.
233-351: Robust metadata update logic with comprehensive error handling.The
updateWithFlagsmethod properly handles:
- Asset validation and proof fetching
- Metadata fetch failure scenarios with clear error messages (lines 280-286)
- Image upload with mime type inference (lines 289-296)
- Metadata structure preservation including additional files (lines 299-316)
The defensive requirement for all fields when metadata fetch fails (line 281-284) is appropriate, ensuring the NFT isn't updated with incomplete metadata.
353-401: Clean command orchestration with proper validation.The
runmethod correctly:
- Routes between interactive editor mode and flag-based updates
- Validates that at least one update flag is provided (lines 370-374)
- Generates appropriate transaction explorer URLs
- Supports JSON output mode
src/commands/bg/tree/create.ts (1)
125-175: Tree name validation and early duplicate checks are solidBoth the wizard and flag flows now:
- Trim and validate names via
isValidTreeName.- Check for per‑network duplicate names using
getNetworkInfo+loadTreesbefore starting the transaction.- Still leave a secondary safeguard in
saveTreeif the pre‑check cannot run.This aligns the
--namebehavior with the wizard and avoids wasted transactions on duplicate names while keeping error messages consistent. Nicely done.Also applies to: 270-298
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
test/commands/bg/bgcollectionhelpers.ts (1)
71-92: Redundant exit code assertion.
runClirejects on any non-zero exit code (see test/runCli.ts:33-36), so by the time this helper reaches line 88,codeis guaranteed to be0. The explicit assertion and destructuring are unnecessary.Based on learnings.
Remove the redundant check:
- const { stdout, stderr, code } = await runCli(cliInput) + const { stdout, stderr } = await runCli(cliInput) const cleanStderr = stripAnsi(stderr) const cleanStdout = stripAnsi(stdout) const combined = cleanStdout + '\n' + cleanStderr const collectionId = extractCollectionId(combined) if (!collectionId) { console.log('Collection creation output:', combined) throw new Error('Collection ID not found in output') } // Extract signature const sigMatch = combined.match(/Transaction: ([a-zA-Z0-9]+)/) const signature = sigMatch ? sigMatch[1] : '' - expect(code).to.equal(0) expect(combined).to.contain('Collection created with Bubblegum V2 plugin') expect(collectionId).to.match(/^[a-zA-Z0-9]+$/)test/commands/bg/bghelpers.ts (1)
167-193: Redundant exit code assertion in NFT helper.Same pattern:
runClirejects on non-zero exit, making the assertion on line 185 unnecessary.Based on learnings.
- const { stdout, stderr, code } = await runCli(cliInput) + const { stdout, stderr } = await runCli(cliInput) const cleanStderr = stripAnsi(stderr) const cleanStdout = stripAnsi(stdout) const combined = cleanStdout + '\n' + cleanStderr const assetId = extractAssetId(combined) const signature = extractSignature(combined) // Extract owner (should be the test keypair address) const ownerMatch = combined.match(/Owner: ([a-zA-Z0-9]+)/) const owner = ownerMatch ? ownerMatch[1] : '' if (!signature) { console.log('NFT creation output:', combined) throw new Error('Signature not found in output') } - expect(code).to.equal(0) expect(combined).to.contain('Compressed NFT created')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/commands/bg/collection/create.ts(1 hunks)src/commands/bg/tree/create.ts(1 hunks)test/commands/bg/bg.collection.create.test.ts(1 hunks)test/commands/bg/bgcollectionhelpers.ts(1 hunks)test/commands/bg/bghelpers.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-28T23:09:37.361Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 18
File: test/commands/core/core.create.test.ts:92-97
Timestamp: 2025-05-28T23:09:37.361Z
Learning: In the Metaplex CLI test suite, the user prefers to avoid redundant assertions in tests when helper functions like `createCoreAsset` and `createCoreCollection` already perform comprehensive validation (exit codes, success messages, ID format validation, and throw errors on failure). The test can rely on the helper functions to fail appropriately rather than duplicating assertions.
Applied to files:
test/commands/bg/bg.collection.create.test.tstest/commands/bg/bgcollectionhelpers.tstest/commands/bg/bghelpers.ts
📚 Learning: 2025-05-30T13:45:51.808Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 19
File: src/prompts/pluginInquirer.ts:0-0
Timestamp: 2025-05-30T13:45:51.808Z
Learning: In src/prompts/pluginInquirer.ts, changing `let` to `const` for authority variables in plugin configurations causes errors upon blockchain operations. The variables should remain declared with `let` even though they appear to only be assigned once, due to blockchain-specific requirements.
Applied to files:
src/commands/bg/collection/create.ts
🧬 Code graph analysis (5)
src/commands/bg/tree/create.ts (3)
src/lib/treeStorage.ts (4)
isValidTreeName(147-150)getNetworkInfo(35-44)loadTrees(63-74)saveTree(82-99)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
test/commands/bg/bg.collection.create.test.ts (3)
test/runCli.ts (1)
runCli(6-58)test/commands/bg/bgcollectionhelpers.ts (2)
createBubblegumCollection(95-95)stripAnsi(95-95)test/commands/bg/bghelpers.ts (1)
stripAnsi(201-201)
test/commands/bg/bgcollectionhelpers.ts (1)
test/runCli.ts (1)
runCli(6-58)
test/commands/bg/bghelpers.ts (1)
test/runCli.ts (1)
runCli(6-58)
src/commands/bg/collection/create.ts (3)
src/commands/cm/create.ts (1)
createCollection(368-414)src/lib/util.ts (1)
txSignatureToString(13-13)src/explorers.ts (1)
generateExplorerUrl(34-37)
⏰ 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). (3)
- GitHub Check: unit-tests (ubuntu-latest, lts/*)
- GitHub Check: unit-tests (ubuntu-latest, latest)
- GitHub Check: unit-tests (ubuntu-latest, lts/-1)
🔇 Additional comments (3)
test/commands/bg/bgcollectionhelpers.ts (1)
5-32: LGTM! Scanner approach resolves Biome lint error.The
stripAnsiimplementation correctly avoids control characters in regex by using a character-by-character scanner, addressing the previousnoControlCharactersInRegexlint violation.src/commands/bg/collection/create.ts (1)
59-84: LGTM! Royalties plugin correctly wired and configured.All issues from the previous review have been addressed:
- Line 67: Basis points correctly computed as
Math.round(flags.royalties * 100)(percentage → basis points)- Line 71: Creator percentage correctly set to
100(single creator receives full share)- Line 83: The
pluginsarray (including optional Royalties) is now passed tocreateCollection- Line 97: Plugin summary dynamically reflects the actual plugins configured
The royalties flag now has the intended on-chain effect.
src/commands/bg/tree/create.ts (1)
24-32: TREE_CONFIG definitions look consistent and well structuredThe predefined configurations (depth, buffer, canopy, maxNfts, costs, proofs) are coherent (maxNfts are 2^maxDepth, etc.) and give a clear UX in the wizard. No issues spotted here.
|
Thank you very much!
2. same for "mplx bg". Why is namespace mentioned?
3. I like the description of the workflow, but the "Note" seems to be redundant since it would be step 1 already?
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
src/commands/bg/index.ts(1 hunks)
⏰ 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). (3)
- GitHub Check: unit-tests (ubuntu-latest, latest)
- GitHub Check: unit-tests (ubuntu-latest, lts/*)
- GitHub Check: unit-tests (ubuntu-latest, lts/-1)
🔇 Additional comments (1)
src/commands/bg/index.ts (1)
15-42: Code displays clear help documentation with appropriate workflow structure.The
run()method insrc/commands/bg/index.tsprovides well-organized help text with:
- Clear listing of available commands (lines 19–26)
- Well-structured three-step workflow (lines 28–36)
- Appropriate "(Optional)" marker for step 1
- Helpful references to subcommand documentation (lines 38–40)
No issues identified in the code snippet.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
src/commands/bg/index.ts (1)
1-43: LGTM! Description updated per previous feedback.The description has been shortened to a single sentence, aligning with codebase conventions. The run() method provides a helpful workflow overview for users.
One observation: The PR reviewer noted the CLI description appears "strange" with a namespace mention. If the issue persists, consider whether the
summaryfield (Line 7) is being rendered unexpectedly in the help output—you may need to adjust or remove it depending on how oclif displays both fields.src/commands/bg/tree/create.ts (2)
253-302: LGTM! Name validation now aligned between wizard and flag paths.The
--nameflag handling now properly:
- Uses
flags.name !== undefinedto catch empty string edge case (Line 272)- Validates with
isValidTreeNamebefore transaction (Line 279)- Checks for duplicate names early to avoid wasting the transaction (Lines 284-291)
This addresses the previous review feedback about inconsistent validation between wizard and flag flows.
125-150: Network info fetched on every validation keystroke.The
validatecallback callsgetNetworkInfo(this.context.umi)on each validation run, resulting in an RPC call per keystroke. While functionally correct, this could be optimized by fetching the network info once before theinput()call and capturing it in the closure.This is a minor performance nitpick; current behavior is acceptable.
test/commands/bg/bgcollectionhelpers.ts (1)
43-65: Redundant exit code assertion.The
expect(code).to.equal(0)assertion on line 60 is redundant becauserunClialready rejects on non-zero exit codes or when stderr contains error indicators. By the time execution reaches line 43, the code is guaranteed to be 0.Based on learnings.
Apply this diff to remove the redundant assertion:
- const { stdout, stderr, code } = await runCli(cliInput) + const { stdout, stderr } = await runCli(cliInput) const cleanStderr = stripAnsi(stderr) const cleanStdout = stripAnsi(stdout) const combined = cleanStdout + '\n' + cleanStderr const collectionId = extractCollectionId(combined) if (!collectionId) { console.log('Collection creation output:', combined) throw new Error('Collection ID not found in output') } // Extract signature const sigMatch = combined.match(/Transaction: ([a-zA-Z0-9]+)/) const signature = sigMatch ? sigMatch[1] : '' - expect(code).to.equal(0) expect(combined).to.contain('Collection created with Bubblegum V2 plugin') expect(collectionId).to.match(/^[a-zA-Z0-9]+$/) return { collectionId, signature }test/commands/bg/bghelpers.ts (1)
77-102: Redundant exit code assertion in tree helper.The
expect(code).to.equal(0)assertion on line 96 is redundant becauserunClirejects on non-zero exit codes. By the time line 77 resolves, the code is guaranteed to be 0.Based on learnings.
Apply this diff to remove the redundant assertion:
- const { stdout, stderr, code } = await runCli(cliInput) + const { stdout, stderr } = await runCli(cliInput) const cleanStderr = stripAnsi(stderr) const cleanStdout = stripAnsi(stdout) const combined = cleanStdout + '\n' + cleanStderr const treeAddress = extractTreeAddress(combined) const signature = extractSignature(combined) if (!treeAddress) { console.log('Tree creation output:', combined) throw new Error('Tree address not found in output') } if (!signature) { console.log('Tree creation output:', combined) throw new Error('Signature not found in output') } - expect(code).to.equal(0) expect(combined).to.contain('Merkle tree created') expect(treeAddress).to.match(/^[a-zA-Z0-9]{32,44}$/) expect(signature).to.match(/^[a-zA-Z0-9]{32,}$/) return { treeAddress, signature }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/commands/bg/index.ts(1 hunks)src/commands/bg/tree/create.ts(1 hunks)test/commands/bg/bgcollectionhelpers.ts(1 hunks)test/commands/bg/bghelpers.ts(1 hunks)test/commands/bg/common.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-28T23:09:37.361Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 18
File: test/commands/core/core.create.test.ts:92-97
Timestamp: 2025-05-28T23:09:37.361Z
Learning: In the Metaplex CLI test suite, the user prefers to avoid redundant assertions in tests when helper functions like `createCoreAsset` and `createCoreCollection` already perform comprehensive validation (exit codes, success messages, ID format validation, and throw errors on failure). The test can rely on the helper functions to fail appropriately rather than duplicating assertions.
Applied to files:
test/commands/bg/bgcollectionhelpers.tstest/commands/bg/bghelpers.ts
🧬 Code graph analysis (3)
test/commands/bg/bgcollectionhelpers.ts (2)
test/runCli.ts (1)
runCli(6-58)test/commands/bg/common.ts (1)
stripAnsi(2-29)
src/commands/bg/tree/create.ts (2)
src/lib/treeStorage.ts (4)
isValidTreeName(147-150)getNetworkInfo(35-44)loadTrees(63-74)saveTree(82-99)src/explorers.ts (1)
generateExplorerUrl(34-37)
test/commands/bg/bghelpers.ts (2)
test/runCli.ts (1)
runCli(6-58)test/commands/bg/common.ts (1)
stripAnsi(2-29)
⏰ 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). (3)
- GitHub Check: unit-tests (ubuntu-latest, lts/*)
- GitHub Check: unit-tests (ubuntu-latest, latest)
- GitHub Check: unit-tests (ubuntu-latest, lts/-1)
🔇 Additional comments (4)
test/commands/bg/common.ts (1)
1-29: LGTM! Proper ANSI-stripping implementation.The scanner-based approach correctly handles CSI sequences and avoids the Biome control-character-in-regex error. Centralizing
stripAnsihere addresses the duplication concern from past reviews.test/commands/bg/bgcollectionhelpers.ts (1)
7-19: LGTM! Robust extraction logic.The multiple-pattern approach handles CLI output variations gracefully.
test/commands/bg/bghelpers.ts (2)
6-45: LGTM! Consistent extraction utilities.All three extraction helpers use a sensible multi-pattern approach to handle CLI output variations.
165-172: LGTM! Clean exports.All test utilities are properly exported for reuse across the test suite.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.validator.cjs(1 hunks)package.json(1 hunks)src/lib/Context.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use utilities in src/lib/file.ts for file handling operations
Files:
src/lib/Context.ts
src/lib/Context.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/Context.ts: Extend ConfigJson type for new configuration options and update CONFIG_KEYS array when adding new config fields
Use consolidateConfigs function for merging configuration sources
Files:
src/lib/Context.ts
🧠 Learnings (4)
📚 Learning: 2025-12-02T16:21:02.107Z
Learnt from: CR
Repo: metaplex-foundation/cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T16:21:02.107Z
Learning: Applies to src/commands/**/*.ts : Access Umi instance through this.context.umi for blockchain interactions
Applied to files:
src/lib/Context.ts
📚 Learning: 2025-12-02T16:21:02.107Z
Learnt from: CR
Repo: metaplex-foundation/cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T16:21:02.107Z
Learning: Use Metaplex Umi as the core blockchain interaction layer
Applied to files:
src/lib/Context.ts
📚 Learning: 2025-12-02T16:21:02.107Z
Learnt from: CR
Repo: metaplex-foundation/cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T16:21:02.107Z
Learning: Applies to src/lib/Context.ts : Extend ConfigJson type for new configuration options and update CONFIG_KEYS array when adding new config fields
Applied to files:
src/lib/Context.ts
📚 Learning: 2025-06-05T23:37:11.513Z
Learnt from: tonyboylehub
Repo: metaplex-foundation/cli PR: 27
File: src/lib/Context.ts:152-153
Timestamp: 2025-06-05T23:37:11.513Z
Learning: In src/lib/Context.ts, the non-null assertions on config.rpcUrl! and config.commitment! are safe because consolidateConfigs() merges DEFAULT_CONFIG (which provides both values) with user config and overrides, only overwriting with non-undefined values, ensuring these properties are always present.
Applied to files:
src/lib/Context.ts
⏰ 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). (3)
- GitHub Check: unit-tests (ubuntu-latest, latest)
- GitHub Check: unit-tests (ubuntu-latest, lts/-1)
- GitHub Check: unit-tests (ubuntu-latest, lts/*)
🔇 Additional comments (3)
src/lib/Context.ts (3)
1-2: LGTM!The imports for
mplBubblegumanddasApi/DasApiInterfaceare correctly added and align with the new package dependencies.Also applies to: 27-27
160-168: LGTM!The middleware registration for
mplBubblegum()anddasApi()follows the established pattern. PlacingdasApi()at the end is appropriate since it augments the RPC layer rather than registering program-specific functionality.
55-55: Error handling already implemented; DAS API availability is documented.The type augmentation correctly exposes DAS API methods on
umi.rpc. Error handling for DAS API calls is already in place:
transfer.tsandburn.tswrapgetAssetWithProofin try-catch blocks with error handlersfetch.tsincludes specific error messages mentioning "RPC does not support DAS API"update.tsuses.catch()with error handlingThe troubleshooting documentation (
docs/bubblegum/troubleshooting.md) already addresses DAS API RPC requirements with a dedicated section recommending DAS-enabled providers like Helius and Triton, noting that standard Solana RPCs don't support DAS.Likely an incorrect or invalid review comment.
| "@metaplex-foundation/digital-asset-standard-api": "^2.0.0", | ||
| "@metaplex-foundation/mpl-bubblegum": "^5.0.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions and peer dependencies
echo "=== Checking latest versions on npm ==="
echo "digital-asset-standard-api:"
npm view @metaplex-foundation/digital-asset-standard-api version 2>/dev/null || echo "Failed to fetch"
echo -e "\nmpl-bubblegum:"
npm view @metaplex-foundation/mpl-bubblegum version 2>/dev/null || echo "Failed to fetch"
echo -e "\n=== Checking peer dependencies ==="
echo "mpl-bubblegum peer deps:"
npm view @metaplex-foundation/mpl-bubblegum peerDependencies 2>/dev/null || echo "Failed to fetch"
echo -e "\ndigital-asset-standard-api peer deps:"
npm view @metaplex-foundation/digital-asset-standard-api peerDependencies 2>/dev/null || echo "Failed to fetch"Repository: metaplex-foundation/cli
Length of output: 362
Peer dependency incompatibility detected.
These packages have conflicting Umi version requirements. The digital-asset-standard-api@2.0.0 requires @metaplex-foundation/umi >= 0.8.2 <= 1, but the project uses Umi v1.4.1, which exceeds the upper bound. The mpl-bubblegum@5.0.2 is compatible, but the DAS API package will cause installation/runtime issues. Downgrade digital-asset-standard-api to a version compatible with Umi v1.4.1, or upgrade the entire Umi ecosystem.
🤖 Prompt for AI Agents
In package.json around lines 31-32, the dependency
"@metaplex-foundation/digital-asset-standard-api": "^2.0.0" declares a Umi peer
that is incompatible with the project's installed @metaplex-foundation/umi
v1.4.1; resolve by either (A) pinning
"@metaplex-foundation/digital-asset-standard-api" to a release that explicitly
supports Umi v1.4.1 (find the correct version in that package's
changelog/registry and update the version specifier), or (B) upgrade the
project's Umi ecosystem (@metaplex-foundation/umi and any related
@metaplex-foundation packages) to a version within the peer range required by
digital-asset-standard-api (then run a full install and smoke tests). After
changing package.json, run npm/yarn install, update lockfile, and run the test
suite to verify no runtime breakage.
2 participants
No description provided.