-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: coinbase agentkit plugin for eliza #2298
base: develop
Are you sure you want to change the base?
Conversation
@coderabbitai review |
@wtfsayo here's my review of the agentkit plugin implementation:
console.log("sweetman------------------------");
console.log("sweetman client------------------------", client);
console.log("sweetman actions------------------------", actions);
export async function getclient(): promise {
const config = {
networkid: "base-sepolia", // should use config parameter
};
return await cdpagentkit.configurewithwallet(config);
}
let me know if you'd like me to explain any of these points in more detail. 🔗 analysis chain 🏁 scripts executed the following scripts were executed for the analysis: script:
length of output: 51199 script:
length of output: 8869 ✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughA new plugin for Eliza, named Changes
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (8)
packages/plugin-agentkit/tsup.config.ts (1)
8-8
: Fix misleading comment about CommonJS formatThe comment suggests CommonJS, but the format is set to ESM only. This inconsistency could confuse other developers.
- format: ["esm"], // Ensure you're targeting CommonJS + format: ["esm"], // Using ESM format for modern module compatibilitypackages/plugin-agentkit/src/actions.ts (3)
32-32
: Remove debugging console.log statementsConsole.log statements at lines 32, 48, and 94 are likely for debugging purposes and should be removed to clean up the code.
Also applies to: 48-48, 94-94
114-114
: Specify 'tool' parameter typeIn
composeParameterContext
, replacetool: any
withtool: Tool
to improve type safety.
100-100
: Refine 'parameters' type in 'executeToolAction'Avoid using
any
forparameters
. Use a more specific type orunknown
to enhance type safety.packages/plugin-agentkit/README.md (4)
56-76
: Add language specifications to code blocks.Enhance documentation readability by specifying languages for the code blocks.
-``` +```text Can you show me my wallet details?-
+
text
Deploy a new NFT collection called "Music NFTs" with symbol "MUSIC"-``` +```text Create a new WOW token called "Artist Token" with symbol "ART"
-
+
text
What's my current balance?🧰 Tools
🪛 Markdownlint (0.37.0)
56-56: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
62-62: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
68-68: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
74-74: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
94-97
: Add version specifications for dependencies.Specify version requirements for each dependency to ensure compatibility.
-@elizaos/core -@coinbase/cdp-agentkit-core -@coinbase/cdp-langchain -@langchain/core +@elizaos/core: ^1.0.0 +@coinbase/cdp-agentkit-core: ^1.0.0 +@coinbase/cdp-langchain: ^1.0.0 +@langchain/core: ^1.0.0
103-104
: Document specific network IDs.Add the exact network IDs for configuration clarity.
-Base Sepolia (default) -Base Mainnet +Base Sepolia (default, networkId: "base-sepolia") +Base Mainnet (networkId: "base-mainnet")
123-123
: Add link to MIT license.Include a link to the full MIT license text.
-MIT +MIT - [Full License](https://opensource.org/licenses/MIT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.gitignore
(1 hunks)packages/plugin-agentkit/README.md
(1 hunks)packages/plugin-agentkit/package.json
(1 hunks)packages/plugin-agentkit/src/actions.ts
(1 hunks)packages/plugin-agentkit/src/index.ts
(1 hunks)packages/plugin-agentkit/src/provider.ts
(1 hunks)packages/plugin-agentkit/tsconfig.json
(1 hunks)packages/plugin-agentkit/tsup.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- packages/plugin-agentkit/tsconfig.json
- packages/plugin-agentkit/package.json
🧰 Additional context used
🪛 LanguageTool
packages/plugin-agentkit/README.md
[uncategorized] ~39-~39: Loose punctuation mark.
Context: ... AgentKit tools: - GET_WALLET_DETAILS
: Get wallet information - DEPLOY_NFT
: ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Markdownlint (0.37.0)
packages/plugin-agentkit/README.md
56-56: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
62-62: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
74-74: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
packages/plugin-agentkit/tsup.config.ts (1)
9-20
: LGTM! Well-structured externals configurationGood practice to externalize Node.js built-ins and third-party dependencies. This keeps the bundle size minimal and prevents duplicate dependencies.
can you fix issues @sweetmantech |
fixing now |
I add updated files.
Sweets/pnpm
fixed current errors @wtfsayo ✅ Could I please get another round of review feedback? 🧑🔧 |
@coderabbitai review |
1 similar comment
@coderabbitai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/plugin-agentkit/src/index.ts (1)
8-10
: Consider making networkId configurableThe networkId is hardcoded to "base-sepolia". Consider making it configurable through the plugin parameters.
-async function agentKitPlugin(): Promise<Plugin> { +async function agentKitPlugin(networkId: string = "base-sepolia"): Promise<Plugin> { const actions = await getAgentKitActions({ getClient, config: { - networkId: "base-sepolia", + networkId, }, });packages/plugin-agentkit/src/actions.ts (2)
80-88
: Enhance error handling with more contextThe error handling could provide more details about the failed operation.
} catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); callback?.({ - text: `Error executing action ${tool.name}: ${errorMessage}`, + text: `Error executing action ${tool.name} with parameters ${JSON.stringify(parameters)}: ${errorMessage}`, content: { error: errorMessage }, }); return false;
94-108
: Consider caching the toolkit instanceCreating a new CdpToolkit instance for each action execution could be inefficient.
+let toolkitInstance: CdpToolkit | null = null; + async function executeToolAction( tool: Tool, parameters: any, client: CdpAgentkit ): Promise<unknown> { - const toolkit = new CdpToolkit(client); - const tools = toolkit.getTools(); + if (!toolkitInstance) { + toolkitInstance = new CdpToolkit(client); + } + const tools = toolkitInstance.getTools(); const selectedTool = tools.find((t) => t.name === tool.name);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugin-agentkit/src/actions.ts
(1 hunks)packages/plugin-agentkit/src/index.ts
(1 hunks)
🔇 Additional comments (3)
packages/plugin-agentkit/src/index.ts (1)
5-21
: Clean implementation of the plugin interfaceThe plugin structure is well-organized and properly implements the Plugin interface.
packages/plugin-agentkit/src/actions.ts (2)
15-20
: Well-structured type definitionThe GetAgentKitActionsParams type is clear and properly handles optional config.
134-168
: Comprehensive response templateThe response template is well-structured and includes all necessary context for generating responses.
@coderabbitai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/plugin-agentkit/src/actions.ts (1)
80-88
: Enhance error handling with more context.The error handling could be more informative by including the parameters that caused the failure.
} catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); + const errorDetails = { + error: errorMessage, + tool: tool.name, + parameters, + }; callback?.({ - text: `Error executing action ${tool.name}: ${errorMessage}`, - content: { error: errorMessage }, + text: `Error executing action ${tool.name}: ${errorMessage}. Parameters: ${JSON.stringify(parameters)}`, + content: errorDetails, }); return false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugin-agentkit/src/actions.ts
(1 hunks)
🔇 Additional comments (2)
packages/plugin-agentkit/src/actions.ts (2)
1-20
: Well-structured imports and type definitions!The type definitions are clean and flexible, with good use of optional parameters.
134-168
: LGTM! Well-structured response template.The response template is comprehensive and includes all necessary sections for generating contextual responses.
What's next @wtfsayo ? 🧑🔧 |
Can you share a demo / screenshots of this works |
|
@coderabbitai final review please |
@wtfsayo i've reviewed the latest changes and can confirm that:
while the network id is still hardcoded, the overall implementation is solid and ready for production. we can address the network id configuration in a future enhancement. i recommend approving this pr. 🔗 analysis chain 🏁 scripts executed the following scripts were executed for the analysis: script:
length of output: 6455 ✏️ learnings added
✅ Actions performedComments resolved. |
@tcm390 |
Head branch was pushed to by a user without write access
Could you please try running the smoke tests again @wtfsayo ?
|
Relates to
Adding AgentKit EVM support to Eliza
Risks
Background
What does this PR do?
Adding AgentKit EVM support to Eliza
What kind of change is this?
Documentation changes needed?
Testing
Where should a reviewer start?
Detailed testing steps