Fixes/initial fixes #29
Conversation
Summary by CodeRabbit
WalkthroughThe changes introduce a new changelog file and enhance wallet path handling in configuration commands by normalizing and validating file paths, improving error messages, and ensuring consistent storage of absolute paths. Additionally, command index pages for core and toolbox modules are updated with detailed descriptions, usage examples, help flag definitions, and improved help output behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI_Command
participant FileSystem
participant Config
User->>CLI_Command: Run wallet add/set/new command with file path
CLI_Command->>FileSystem: Normalize path to absolute
CLI_Command->>FileSystem: Check if wallet file exists
alt File exists
CLI_Command->>Config: Store absolute path in config
CLI_Command-->>User: Success message
else File does not exist
CLI_Command-->>User: Error message (file not found)
end
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (2)
src/commands/config/wallets/new.ts (1)
94-108:⚠️ Potential issueAvoid manual backslash escaping – leads to double-escaped paths & duplicate-detection bugs
filePath.replace(/\\/g, '\\\\')stores a different string than the one you just compared against inexistingPath.
At runtime the parsed JSON already contains single back-slashes (they’re escaped only in the file), so the extra.replace():
- Produces a double-escaped value in memory → JSON ends up with quadruple back-slashes on every save.
- Breaks the
existingPathcheck, letting the same wallet be added twice on Windows.- const newWallet = { - name: flags.name, - path: filePath.replace(/\\/g, '\\\\'), // Escape backslashes for JSON + const newWallet = { + name: flags.name, + path: filePath, // JSON.stringify will escape as neededsrc/commands/config/wallets/add.ts (1)
39-46: 🧹 Nitpick (assertive)Path validation is case-sensitive –
.JSONfiles fail unexpectedly
endsWith('.json')rejects valid keypair files with an upper-case extension on Windows/macOS.-if (!args.path.endsWith('.json')) { +if (!args.path.toLowerCase().endsWith('.json')) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md(1 hunks)src/commands/config/wallets/add.ts(4 hunks)src/commands/config/wallets/new.ts(3 hunks)src/commands/config/wallets/set.ts(2 hunks)src/commands/core/asset/index.ts(1 hunks)src/commands/core/collection/index.ts(1 hunks)src/commands/core/index.ts(1 hunks)src/commands/core/plugins/index.ts(1 hunks)src/commands/toolbox/index.ts(1 hunks)src/commands/toolbox/sol/index.ts(1 hunks)src/commands/toolbox/token/index.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/commands/config/wallets/add.ts
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
src/commands/config/wallets/set.ts
[error] 6-6: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
[error] 8-8: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
8-8: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
15-15: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
25-25: Files should end with a single newline character
null
(MD047, single-trailing-newline)
| public async run(): Promise<void> { | ||
| const {args, flags} = await this.parse(ToolboxSol) | ||
|
|
||
| // Show help by default | ||
| this.log(ToolboxSol.description) | ||
| this.log('\nExamples:') | ||
| ToolboxSol.examples.forEach(example => { | ||
| this.log(` ${example}`) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Leverage Oclif’s built-in help instead of manual logging
Re-implementing help text risks divergence from actual flags/examples.
Consider delegating to Oclif to keep maintenance low:
- // Show help by default
- this.log(ToolboxSol.description)
- this.log('\nExamples:')
- ToolboxSol.examples.forEach(example => {
- this.log(` ${example}`)
- })
+ // Show generated help output
+ await this.showHelp(['--help'])🤖 Prompt for AI Agents
In src/commands/toolbox/sol/index.ts around lines 24 to 33, the run method
manually logs the help description and examples, which risks becoming outdated.
Replace this manual logging by invoking Oclif's built-in help command or method
to automatically display the current help text, ensuring consistency and
reducing maintenance.
There was a problem hiding this comment.
await this.showHelp(['--help']) doesn't exist.
There was a problem hiding this comment.
@tonyboylehub You’re right—showHelp isn’t part of @oclif/core’s Command API.
The usual ways to display help programmatically are:
-
Call the command’s own help generator:
// inside run() await this.parse(ToolboxSol); // keep your current parse await this.config.runCommand(this.id, ['--help']);
config.runCommand()re-invokes the command with--help, so the standard help system handles description, flags and examples. -
Or use the generic Help class:
import {Help} from '@oclif/core'; // … const help = new Help(this.config); await help.showHelp(['toolbox','sol']);
This instantiates the built-in Help renderer directly.
If you prefer to stay lightweight, keeping the custom this.log(...) block is perfectly valid—just be aware it may drift from the actual flag/description strings over time.
Let me know which route you’d like to take and I can adjust the suggestion accordingly.
| @@ -1,18 +1,33 @@ | |||
| import {Args, Command, Flags} from '@oclif/core' | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused Args import.
-import {Args, Command, Flags} from '@oclif/core'
+import {Command, Flags} from '@oclif/core'📝 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.
| import {Args, Command, Flags} from '@oclif/core' | |
| import {Command, Flags} from '@oclif/core' |
🤖 Prompt for AI Agents
In src/commands/core/index.ts at line 1, the imported `Args` from '@oclif/core'
is not used anywhere in the file. Remove `Args` from the import statement to
clean up unused imports and improve code clarity.
| const {args, flags} = await this.parse(CorePlugins) | ||
|
|
||
|
|
||
| // Show help by default | ||
| this.log(CorePlugins.description) | ||
| this.log('\nExamples:') | ||
| CorePlugins.examples.forEach(example => { | ||
| this.log(` ${example}`) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Drop unused destructuring from parse call
The command neither reads args nor flags, so the destructured variables are unused (and may trigger lint warnings).
You can keep the call—it validates input—while avoiding the unused bindings:
- const {args, flags} = await this.parse(CorePlugins)
+ await this.parse(CorePlugins) // validate CLI inputOptional: you can replace the custom logging with this._help(), letting oclif format the description & examples consistently across commands.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const {args, flags} = await this.parse(CorePlugins) | |
| // Show help by default | |
| this.log(CorePlugins.description) | |
| this.log('\nExamples:') | |
| CorePlugins.examples.forEach(example => { | |
| this.log(` ${example}`) | |
| }) | |
| } | |
| await this.parse(CorePlugins) // validate CLI input | |
| // Show help by default | |
| this.log(CorePlugins.description) | |
| this.log('\nExamples:') | |
| CorePlugins.examples.forEach(example => { | |
| this.log(` ${example}`) | |
| }) | |
| } |
🤖 Prompt for AI Agents
In src/commands/core/plugins/index.ts around lines 49 to 57, the destructured
variables 'args' and 'flags' from the parse call are unused and may cause lint
warnings. To fix this, remove the destructuring and just await the parse call
without assigning its result. Optionally, replace the manual logging of
description and examples with a call to this._help() to leverage oclif's
consistent help formatting.
| static override description = `MPL Core Plugin Management - Add and manage plugins for assets and collections | ||
|
|
||
| The plugin commands provide functionality for managing plugins that extend the capabilities of assets and collections on the Solana blockchain. | ||
|
|
||
| Available subcommands: | ||
| add - Add plugins to assets or collections (with wizard or JSON config) | ||
| remove - Remove plugins from assets or collections | ||
| generate - Generate reusable plugin configuration files for scripting | ||
|
|
||
| Available Plugin Types: | ||
| Common Plugins (Asset & Collection): | ||
| - Attributes | ||
| - Royalties | ||
| - Update Delegate | ||
| - Permanent Freeze | ||
| - Permanent Transfer | ||
| - Permanent Burn | ||
| - Add Blocker | ||
| - Immutable Metadata | ||
| - Autograph | ||
| - Verified Creators | ||
|
|
||
| Collection-specific Plugins: | ||
| - Master Edition | ||
|
|
||
| Asset-specific Plugins: | ||
| - Edition | ||
| - Freeze Delegate | ||
| - Burn Delegate | ||
| - Transfer Delegate` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Trim leading indentation in the multi-line description string
The template literal captures all the leading spaces that appear in the source file, so the CLI prints each line indented by two spaces. This makes the help text look slightly “shifted” compared with oclif’s built-in help for other commands.
A low-impact tweak is to strip the indentation at build time so the output is perfectly left-aligned:
- static override description = `MPL Core Plugin Management - Add and manage plugins for assets and collections
- ...
- - Transfer Delegate`
+ static override description = [
+ 'MPL Core Plugin Management - Add and manage plugins for assets and collections',
+ '',
+ 'The plugin commands provide functionality for managing plugins that extend the capabilities of assets and collections on the Solana blockchain.',
+ '',
+ 'Available subcommands:',
+ ' add - Add plugins to assets or collections (with wizard or JSON config)',
+ ' remove - Remove plugins from assets or collections',
+ ' generate - Generate reusable plugin configuration files for scripting',
+ '',
+ 'Available Plugin Types:',
+ ' Common Plugins (Asset & Collection):',
+ ' - Attributes',
+ ' - Royalties',
+ ' - Update Delegate',
+ ' - Permanent Freeze',
+ ' - Permanent Transfer',
+ ' - Permanent Burn',
+ ' - Add Blocker',
+ ' - Immutable Metadata',
+ ' - Autograph',
+ ' - Verified Creators',
+ '',
+ ' Collection-specific Plugins:',
+ ' - Master Edition',
+ '',
+ ' Asset-specific Plugins:',
+ ' - Edition',
+ ' - Freeze Delegate',
+ ' - Burn Delegate',
+ ' - Transfer Delegate',
+ ].join('\n')Any string-dedent helper (e.g. common-tags’ stripIndent) would achieve the same effect with less boilerplate.
📝 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.
| static override description = `MPL Core Plugin Management - Add and manage plugins for assets and collections | |
| The plugin commands provide functionality for managing plugins that extend the capabilities of assets and collections on the Solana blockchain. | |
| Available subcommands: | |
| add - Add plugins to assets or collections (with wizard or JSON config) | |
| remove - Remove plugins from assets or collections | |
| generate - Generate reusable plugin configuration files for scripting | |
| Available Plugin Types: | |
| Common Plugins (Asset & Collection): | |
| - Attributes | |
| - Royalties | |
| - Update Delegate | |
| - Permanent Freeze | |
| - Permanent Transfer | |
| - Permanent Burn | |
| - Add Blocker | |
| - Immutable Metadata | |
| - Autograph | |
| - Verified Creators | |
| Collection-specific Plugins: | |
| - Master Edition | |
| Asset-specific Plugins: | |
| - Edition | |
| - Freeze Delegate | |
| - Burn Delegate | |
| - Transfer Delegate` | |
| static override description = [ | |
| 'MPL Core Plugin Management - Add and manage plugins for assets and collections', | |
| '', | |
| 'The plugin commands provide functionality for managing plugins that extend the capabilities of assets and collections on the Solana blockchain.', | |
| '', | |
| 'Available subcommands:', | |
| ' add - Add plugins to assets or collections (with wizard or JSON config)', | |
| ' remove - Remove plugins from assets or collections', | |
| ' generate - Generate reusable plugin configuration files for scripting', | |
| '', | |
| 'Available Plugin Types:', | |
| ' Common Plugins (Asset & Collection):', | |
| ' - Attributes', | |
| ' - Royalties', | |
| ' - Update Delegate', | |
| ' - Permanent Freeze', | |
| ' - Permanent Transfer', | |
| ' - Permanent Burn', | |
| ' - Add Blocker', | |
| ' - Immutable Metadata', | |
| ' - Autograph', | |
| ' - Verified Creators', | |
| '', | |
| ' Collection-specific Plugins:', | |
| ' - Master Edition', | |
| '', | |
| ' Asset-specific Plugins:', | |
| ' - Edition', | |
| ' - Freeze Delegate', | |
| ' - Burn Delegate', | |
| ' - Transfer Delegate', | |
| ].join('\n') |
🤖 Prompt for AI Agents
In src/commands/core/plugins/index.ts between lines 5 and 34, the multi-line
description string includes leading spaces causing the CLI help text to be
indented and misaligned. To fix this, remove the leading indentation from each
line of the template literal so the output is left-aligned. You can do this by
using a string dedent helper like common-tags' stripIndent or manually trimming
the indentation in the string definition.
No description provided.