feat: Agent/Character cli methods + refactor server #3613
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR reorganizes character management and agent initialization across multiple modules. Redundant functions have been removed and type safety improved. The server now uses an Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai force review |
|
@wtfsayo: I'll perform a complete review of all changes in this PR. ✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/cli/src/commands/character.ts (1)
12-24: 🛠️ Refactor suggestionRevise the fee structure to maintain discount benefits.
The current implementation can result in a higher final bill after applying a discount, particularly for small purchases. For example, a $100 purchase with a 10% discount becomes $110 after the $20 fee, negating the discount benefit.
Consider these alternatives:
- Apply the fee before the discount
- Scale the fee based on the purchase amount
- Waive the fee for purchases below a threshold
Would you like me to propose a revised implementation?
🧹 Nitpick comments (10)
packages/agent/src/server/loader.ts (3)
48-67: Avoid abruptly exiting the process
loadCharactersFromUrlcallsprocess.exit(1)upon failure, which can hinder graceful error handling or retries by higher-level code.Throw an error or return a rejected promise so calling code can decide how to handle failures instead of terminating the process.
70-71: Avoid using 'any' to preserve type safety
Usinganyhere disables the benefits of type checking.Define a more specific parameter type or a generic constraint in place of
anyto improve maintainability.🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
209-225: Improve file upload error handling
Currently, no explicit error is surfaced to the caller if directory creation or writing fails.Consider passing an error to
cbin the callback if creation fails or using try/catch to surface meaningful feedback in case of a filesystem issue.packages/cli/src/commands/agent.ts (1)
127-130: Guard against overwriting files
The JSON file output is overwritten if it already exists, potentially causing data loss.Prompt the user or introduce a versioning scheme to prevent unintentional overwrites.
packages/agent/src/server/api.ts (2)
153-154: Clear two-step approach
Starting the agent and then ensuring the character's existence in the database is a good pattern. Consider wrapping both calls in a single try/catch block if needed for better error handling.Also applies to: 156-157
300-348: Handle edge case for empty agents
If there are no agents inagents.values(),anyAgentis undefined and the subsequent logic won't attempt the database. Possibly provide an explicit error message or fallback logic for a better user experience.packages/cli/src/utils/helpers.ts (1)
21-96: Comprehensive character display
displayCharactercovers various fields, logging them systematically. This greatly improves clarity when viewing character data.A small improvement could be to group or reuse repeated
logger.infocalls to reduce clutter in the output.packages/cli/src/commands/character.ts (1)
4-6: Add tests for the formula function.The TODO comment indicates missing test coverage. This is particularly important given the recent signature change.
Would you like me to generate unit tests for this function to ensure it works correctly with the new parameter?
packages/core/src/runtime.ts (2)
1377-1389: Optimize database operations.The method performs an update operation even when the character hasn't changed.
Consider this optimization:
async ensureCharacterExists(character: Character) { const characterExists = await this.databaseAdapter.getCharacter(character.name); if (!characterExists) { await this.databaseAdapter.createCharacter(character); + return; } const characterDiff = diffJson(characterExists, character); - logger.log(`[AgentRuntime][${this.character.name}] Character diff:`, characterDiff); - await this.databaseAdapter.updateCharacter(character.name, character); + if (characterDiff.length > 1) { // More than 1 indicates changes + logger.log(`[AgentRuntime][${this.character.name}] Character diff:`, characterDiff); + await this.databaseAdapter.updateCharacter(character.name, character); + } }
1391-1418: Use consistent logging.Some
console.logcalls remain while others were replaced withlogger.log.Apply this diff for consistent logging:
async ensureEmbeddingDimension() { logger.log(`[AgentRuntime][${this.character.name}] Starting ensureEmbeddingDimension`); if (!this.databaseAdapter) { throw new Error(`[AgentRuntime][${this.character.name}] Database adapter not initialized before ensureEmbeddingDimension`); } try { const model = this.getModel(ModelClass.TEXT_EMBEDDING); if (!model) { throw new Error(`[AgentRuntime][${this.character.name}] No TEXT_EMBEDDING model registered`); } - console.log(`[AgentRuntime][${this.character.name}] Getting embedding dimensions`); + logger.log(`[AgentRuntime][${this.character.name}] Getting embedding dimensions`); const embedding = await this.useModel(ModelClass.TEXT_EMBEDDING, null); if (!embedding || !embedding.length) { throw new Error(`[AgentRuntime][${this.character.name}] Invalid embedding received`); } - console.log(`[AgentRuntime][${this.character.name}] Setting embedding dimension: ${embedding.length}`); + logger.log(`[AgentRuntime][${this.character.name}] Setting embedding dimension: ${embedding.length}`); await this.databaseAdapter.ensureEmbeddingDimension(embedding.length, this.agentId); - console.log(`[AgentRuntime][${this.character.name}] Successfully set embedding dimension`); + logger.log(`[AgentRuntime][${this.character.name}] Successfully set embedding dimension`); } catch (error) { - console.log(`[AgentRuntime][${this.character.name}] Error in ensureEmbeddingDimension:`, error); + logger.error(`[AgentRuntime][${this.character.name}] Error in ensureEmbeddingDimension:`, error); throw error; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/agent/src/index.ts(7 hunks)packages/agent/src/server/api.ts(5 hunks)packages/agent/src/server/helper.ts(0 hunks)packages/agent/src/server/index.ts(1 hunks)packages/agent/src/server/loader.ts(1 hunks)packages/cli/src/commands/agent.ts(3 hunks)packages/cli/src/commands/character.ts(9 hunks)packages/cli/src/utils/helpers.ts(1 hunks)packages/core/src/runtime.ts(2 hunks)packages/plugin-drizzle/src/migrations.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/agent/src/server/helper.ts
✅ Files skipped from review due to trivial changes (1)
- packages/plugin-drizzle/src/migrations.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/agent/src/server/loader.ts
[error] 70-70: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🔇 Additional comments (19)
packages/agent/src/server/loader.ts (2)
11-17: Revisit return type or remove the null union
The function signaturetryLoadFileis: string | null, but it throws an error instead of returningnull. This may surprise callers expecting anullin case of failure.Either return
nullon error or update the signature to reflect that the function never returnsnullon failure.
122-145: Fallback path approach is robust
Multiple path checks for loading a character file is user-friendly and resilient.packages/agent/src/index.ts (3)
22-27: Centralized loading functions enhance maintainability
Extracting character load logic intoloader.tsclarifies responsibilities and simplifiesindex.ts.
38-38: Using RequestInit enhances clarity
Replacing a generic parameter withRequestInitimproves type safety for fetch options.
156-156: Verify database initialization before ensuring character
await runtime.ensureCharacterExists(character)assumes a fully connected database. Verify the DB adapter is properly initialized to avoid runtime errors.packages/cli/src/commands/agent.ts (4)
10-26: Flexible resolveAgentId function
Resolving by name, index, or direct ID is a robust approach that improves user experience.
65-67: Convenient alias and JSON output
Providing an alias (ls) and a JSON option for the list command serves both quick usage and script-friendly output.
100-107: Required option clarifies the 'get' command
-n, --nameeliminates ambiguity by making the needed identifier explicit.
150-201: Multi-pronged agent start logic
Supporting remote URLs, local paths, JSON, and name-based starts covers a broad range of use cases with clear code paths.packages/agent/src/server/api.ts (3)
16-16: No issues found.
The new type import forAgentServerlooks consistent with the rename.
51-51: Converted parameter type
Ensures that the second parameter is now strongly typed asAgentServer. Double-check that all references todirectClientremain valid.
76-76: Good modernization
Retrieving client keys withArray.from()is straightforward.packages/agent/src/server/index.ts (3)
22-22: Imported templates fromhelper.ts
No concerns here.
24-24: File upload utility
Importinguploadfromloader.tsis consistent with the new approach. Ensure that large file handling is properly configured upstream.
29-29: Renamed class
RenamingCharacterServertoAgentServerclarifies responsibilities for agent management.packages/cli/src/utils/helpers.ts (3)
1-4: Straightforward imports
ImportingCharacter,MessageExample, and logging modules is appropriate.
5-18: Well-structured conversation formatting
TheformatMessageExamplesfunction handles edge cases correctly and clearly formats messages.
100-117: Neat heading presentation
ThelogHeaderfunction effectively draws attention to the section titles. Great use of chalk to style.packages/cli/src/commands/character.ts (1)
12-12: Verify the helper functions import.The import statement references functions that were removed according to the AI summary.
Run the following script to verify the helper functions:
#!/bin/bash # Description: Check if helper functions exist in the utils/helpers file. ast-grep --pattern $'function displayCharacter($$$) { $$$ } function formatMessageExamples($$$) { $$$ }'Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
packages/agent/src/server/index.ts (3)
100-238: 🛠️ Refactor suggestionAdd validation for file size and type in message endpoint.
The message endpoint handles file attachments but lacks validation for:
- File size limits
- Allowed file types
- Maximum number of attachments
this.app.post( "/:agentId/message", - upload.single("file"), + upload.single("file", { + limits: { + fileSize: 10 * 1024 * 1024, // 10MB limit + }, + fileFilter: (req, file, cb) => { + const allowedTypes = ['image/jpeg', 'image/png', 'application/pdf']; + if (!allowedTypes.includes(file.mimetype)) { + cb(new Error('Invalid file type')); + return; + } + cb(null, true); + } + }),
510-611:⚠️ Potential issueSecure the fine-tune endpoints.
The fine-tune endpoints expose sensitive operations. Consider:
- Adding authentication middleware
- Validating request body schema
- Sanitizing file paths
+import { z } from 'zod'; + +const fineTuneSchema = z.object({ + // add your schema here +}); + this.app.post( "/fine-tune", + authenticate, async (req: express.Request, res: express.Response) => { + try { + const validatedData = fineTuneSchema.parse(req.body); // ... rest of the code
64-98: 💡 Verification agent❓ Verification inconclusive
Verify error handling in the whisper endpoint.
The whisper endpoint correctly handles file uploads and transcription, but consider adding cleanup for temporary audio files.
🏁 Script executed:
#!/bin/bash # Check if uploaded files are being cleaned up rg -A 5 'upload.single\("file"\)' | rg -A 5 'fs\.unlink|fs\.rm'Length of output: 65
Action Required: Implement Temporary File Cleanup in the Whisper Endpoint
- The endpoint processes and transcribes the uploaded audio file but does not remove the temporary file afterward.
- Please add a cleanup step (e.g., using
fs.unlinkorfs.unlinkSync) after reading the file to prevent file buildup.- Consider adding error handling around the cleanup operation to cover cases where file deletion might fail.
♻️ Duplicate comments (1)
packages/agent/src/server/loader.ts (1)
11-17: 🛠️ Refactor suggestionReturn type mismatch.
The function signature includesstring | null, but it never actually returnsnull. Instead, it always throws on error.
🧹 Nitpick comments (12)
packages/agent/src/server/loader.ts (6)
19-46: Check array concatenation merging behavior.
Merging arrays by concatenation may produce unexpected results if you want child arrays to override base arrays. Consider clarifying or customizing this logic.
48-67: Avoid terminating the entire process for URL fetch errors.
The use ofprocess.exit(1)upon network failure might be too aggressive for some use cases. Consider gracefully handling errors while continuing operation.
72-95: Confirm secrets merging logic.
Appending environment-based secrets to the incoming character might cause side effects if the object is shared in multiple contexts. Ensure this is intentional.
151-166: Potential concurrency note.
Simultaneous directory creation or reads could be an edge case. Consider using a single shared routine or synchronized approach if concurrency is expected.
173-205: Consistent fallback to default character.
UsingdefaultCharacterwhen no others are found is a sensible fallback, but keep in mind thatloadCharactersFromUrlexits the process on failure.
209-225: Secure file upload considerations.
Consider validating file types or file sizes in production settings to prevent malicious uploads.packages/cli/src/utils/helpers.ts (1)
24-96: Consider adding JSDoc documentation for displayCharacter function.While the function is well-implemented, it lacks comprehensive documentation of its parameters and behavior.
Add JSDoc documentation:
+/** + * Display character details in a formatted layout + * @param {Partial<Character>} data - Character data to display + * @param {string} [title="Character Review"] - Optional title for the display + */ export function displayCharacter(data: Partial<Character>, title = "Character Review"): void {packages/cli/src/commands/agent.ts (2)
11-26: Consider adding error handling for name resolution edge cases.The resolveAgentId function should handle the case where multiple agents have similar names.
async function resolveAgentId(nameOrIndex: string): Promise<string> { const listResponse = await fetch(`${AGENT_RUNTIME_URL}/agents`); const { agents } = await listResponse.json(); // Try to find agent by name (case insensitive) const agentByName = agents.find( agent => agent.name.toLowerCase() === nameOrIndex.toLowerCase() ); + // Check for similar names to warn user + const similarNames = agents + .filter(agent => agent.name.toLowerCase().includes(nameOrIndex.toLowerCase())) + .map(agent => agent.name); + + if (similarNames.length > 1) { + logger.warn(`Multiple agents found with similar names: ${similarNames.join(', ')}`); + } return agentByName ? agentByName.id : !Number.isNaN(Number(nameOrIndex)) ? await getAgentIdFromIndex(Number.parseInt(nameOrIndex)) : nameOrIndex; }
150-201: Add retry mechanism for agent start operations.The start command should handle temporary network failures.
+async function fetchWithRetry(url: string, options: RequestInit, retries = 3): Promise<Response> { + for (let i = 0; i < retries; i++) { + try { + const response = await fetch(url, options); + return response; + } catch (error) { + if (i === retries - 1) throw error; + await new Promise(resolve => setTimeout(resolve, 1000 * Math.pow(2, i))); + } + } + throw new Error('Failed after retries'); +} const response: Response = await (async () => { const payload: AgentStartPayload = {}; // ... existing code ... - return await fetch(url, options); + return await fetchWithRetry(url, options); })();packages/agent/src/server/index.ts (2)
486-508: Add rate limiting to the image generation endpoint.The
/imageendpoint could be vulnerable to abuse without rate limiting.+import rateLimit from 'express-rate-limit'; + +const imageLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 100 // limit each IP to 100 requests per windowMs +}); + this.app.post( "/:agentId/image", + imageLimiter, async (req: express.Request, res: express.Response) => {
752-825: Centralize text-to-speech configuration.The TTS endpoint has hardcoded configuration. Consider moving these to a configuration file or environment variables.
Create a new file
config/tts.ts:export const ttsConfig = { modelId: process.env.ELEVENLABS_MODEL_ID || 'eleven_multilingual_v2', voiceSettings: { stability: Number.parseFloat(process.env.ELEVENLABS_VOICE_STABILITY || '0.5'), similarityBoost: Number.parseFloat(process.env.ELEVENLABS_VOICE_SIMILARITY_BOOST || '0.9'), style: Number.parseFloat(process.env.ELEVENLABS_VOICE_STYLE || '0.66'), useSpeakerBoost: process.env.ELEVENLABS_VOICE_USE_SPEAKER_BOOST === 'true' } };packages/core/src/runtime.ts (1)
1392-1418: Replace console.log with logger.log for consistency.Some logging statements still use
console.loginstead of theloggerutility.- console.log(`[AgentRuntime][${this.character.name}] Getting embedding dimensions`); + logger.log(`[AgentRuntime][${this.character.name}] Getting embedding dimensions`); - console.log(`[AgentRuntime][${this.character.name}] Setting embedding dimension: ${embedding.length}`); + logger.log(`[AgentRuntime][${this.character.name}] Setting embedding dimension: ${embedding.length}`); - console.log(`[AgentRuntime][${this.character.name}] Successfully set embedding dimension`); + logger.log(`[AgentRuntime][${this.character.name}] Successfully set embedding dimension`); - console.log(`[AgentRuntime][${this.character.name}] Error in ensureEmbeddingDimension:`, error); + logger.error(`[AgentRuntime][${this.character.name}] Error in ensureEmbeddingDimension:`, error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/agent/src/index.ts(7 hunks)packages/agent/src/server/api.ts(5 hunks)packages/agent/src/server/helper.ts(0 hunks)packages/agent/src/server/index.ts(1 hunks)packages/agent/src/server/loader.ts(1 hunks)packages/cli/src/commands/agent.ts(3 hunks)packages/cli/src/commands/character.ts(9 hunks)packages/cli/src/utils/helpers.ts(1 hunks)packages/core/src/runtime.ts(2 hunks)packages/plugin-drizzle/src/migrations.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/agent/src/server/helper.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/agent/src/server/loader.ts
[error] 70-70: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🔇 Additional comments (21)
packages/agent/src/server/loader.ts (7)
97-104: Load function looks good.
This function is straightforward and aligns well with the intended usage ofjsonToCharacter.
106-110: Custom error handler is clean.
Centralizing error handling here keeps the code DRY.
112-120: Good error encapsulation.
safeLoadCharacterneatly wraps loading logic in a try-catch, improving resilience.
122-145: Robust path fallback logic.
Trying multiple paths is a helpful approach for flexible file locations.
147-149: Simple CSV string parsing.
This small utility is straightforward and readable.
168-171: Check for valid remote URLs.
This helper is concise and meets the immediate need.
224-226: Upload strategy is flexible.
Storing uploaded files works fine with the current disk-based approach.packages/agent/src/index.ts (4)
17-27: Reorganized imports and utilities.
Switching toAgentServer,hasValidRemoteUrls, etc., centralizes relevant logic in a single place, improving maintainability.
38-38: Enhanced type safety for fetch options.
ProvidingRequestInitincreases clarity and reduces runtime errors.
135-137: Improved agent initialization logic.
PassingAgentServertostartAgentclarifies ownership. Additionally,await runtime.ensureCharacterExists(character)ensures consistency in the database. Confirm that these calls align with your performance needs.Also applies to: 155-159
212-212: New server instantiation and agent registration.
Creating the server, assigning loader functions, and exposingserver.startAgentstreamline the flow of agent registration. The approach is consistent with the rest of your architecture.Also applies to: 218-220, 259-261
packages/agent/src/server/api.ts (4)
16-16: Switched toAgentServerparameter.
The direct client now reflects the updated server architecture, keeping naming consistent.Also applies to: 51-51
76-76: Array conversion for clients is neat.
Array.from(agent.getAllClients().keys())is a clear way to present currently connected clients.
156-156: Double-check character existence in DB.
Ensuring the character record in the database right after agent startup is a good safety measure.
300-348: New/agent/start/:characterNameendpoint.
This endpoint flexibly looks for the character in multiple sources, falling back to an existing agent. Nicely enhances usability.packages/plugin-drizzle/src/migrations.ts (1)
2-3: LGTM! Using node: protocol for built-in modules.The change to use the
node:protocol for built-in modules follows Node.js best practices and improves module resolution.packages/cli/src/utils/helpers.ts (2)
8-18: LGTM! Robust message formatting implementation.The function handles empty input gracefully and formats conversations clearly.
100-117: LGTM! Well-documented and visually appealing header implementation.The function is properly documented and uses chalk for visual enhancement.
packages/cli/src/commands/character.ts (1)
395-403: LGTM! Clean implementation of confirmation dialog.The confirmAction function is well-implemented and reusable.
packages/agent/src/server/index.ts (1)
29-58: LGTM! The class initialization and setup looks good.The class properly initializes Express middleware, sets up static file serving, and configures API routes.
🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/core/src/runtime.ts (1)
1382-1389: LGTM! Improved character diff tracking.The addition of character diff logging helps track changes in character configurations.
|
refactor lgtm |
wtfsayo
changed the title
|
Merging optimistically so that PRs don’t pile up |
CLI Features: ElizaOS Agent & Character Management
Agent Management (
agent.ts)Requires:
AGENT_RUNTIME_URLenvironment variable or server running on3000PORT locallyNew Commands:
elizaos agent list- List running agents with name/ID/client info (JSON/table output)elizaos agent get- Get agent details by ID/name/index with JSON export supportelizaos agent start- Start agents with multiple character spec options:--name- Use registry character name--path- Local character JSON file--remote- Remote character JSON URL--json- Raw JSON stringelizaos agent stop/remove- Stop or remove agents by ID/name/indexelizaos agent set- Update agent config via JSON/fileKey Features:
Character Management (
character.ts)Requires: Local database connection, Registry access for plugins
New Commands:
elizaos character create- Interactive character builder with:elizaos character edit- Modify existing characters with field-specific updateselizaos character import/export- JSON conversion with validationelizaos character list/remove- Character lifecycle managementSummary by CodeRabbit
New Features
Refactor