Skip to content

chore: 1.0.19 #5429

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

Merged
merged 14 commits into from
Jul 7, 2025
Merged

chore: 1.0.19 #5429

merged 14 commits into from
Jul 7, 2025

Conversation

wtfsayo
Copy link
Member

@wtfsayo wtfsayo commented Jul 7, 2025

Prepare for release 1.0.19

ai16z-demirix and others added 10 commits July 6, 2025 22:50
- Fixed CLI update to use global installation command (bun add -g)
- Modified update logic to skip package updates when not in a project directory
- Added test to verify no files are created in empty directories
- Shows clear error message if bun is not installed
- Ensures 'elizaos update' only updates global CLI when run outside projects

Fixes issue where running 'elizaos update' in non-project directories would create package.json, node_modules, etc.
- Changed path.join() to join() to match the import statement
- Fixes ReferenceError in test execution
- Change directory before running test instead of using cwd option
- Fix exit code check - runCliCommandSilently returns string not object
- Ensure directory is restored after test
- All update tests now pass locally
- Skip 'update succeeds outside a project' test in CI
- Skip 'update command should not create files' test in CI
- These tests attempt global CLI updates which aren't appropriate for CI
- Tests will still run locally for development validation
…-non-project-dir

fix: prevent CLI update from creating files in non-project directories
Remove explanatory comment about removed code check in socketio/index.ts. The code change is self-evident without the comment.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
fix: issue 5425 - removing check if sender name includes DM
- Add CommonJS shims for browser compatibility in Vite config
- Configure proper polyfills for crypto, buffer, and Node.js globals
- Update package versions: caniuse-lite, decimal.js, zod
- Fix async operation in server initialization
- Minor code formatting improvements
Copy link
Contributor

coderabbitai bot commented Jul 7, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch develop

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

claude bot commented Jul 7, 2025

Claude finished @wtfsayo's task —— View job


PR Review: chore: 1.0.19

  • Read and analyze all changed files
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Review performance considerations
  • Assess security concerns
  • Evaluate test coverage improvements
  • Provide comprehensive feedback

🔍 Code Review Summary

This PR includes several quality improvements and bug fixes across CLI update functionality and SocketIO handling. Overall, the changes demonstrate good engineering practices with improved error handling, better test coverage, and more precise logic.

📋 Code Quality & Best Practices

Strengths:

  • Error Handling: Excellent improvement in cli-update.ts with proper try-catch blocks and user-friendly error messages
  • Code Simplification: Removed dependency on executeInstallation utility in favor of direct execa usage, reducing complexity
  • Test Coverage: Added comprehensive test case for edge case behavior (non-project directory file creation)
  • CI Awareness: Proper skipIf conditions for CI environments where certain tests shouldn't run

⚠️ Minor Concerns:

  • Directory Detection Logic: The early directory detection in index.ts:29-31 is called twice - once for control flow and again later for package updates. Consider caching the result.

🐛 Bug Fixes & Improvements

Fixed Issues:

  1. SocketIO DM Detection: Removed overly broad senderName?.includes('DM') logic in socketio/index.ts:284 which could cause false positives
  2. CLI Update Flow: Better separation of concerns between CLI and package updates
  3. File Creation Prevention: New test ensures update command doesn't create unwanted files in non-project directories

🔧 Technical Improvements:

  • Better Error Messages: CLI update now provides clear installation instructions when bun is not found
  • Logging: Enhanced debug logging in CLI update process
  • Flow Control: More explicit logic for determining update behavior based on flags and context

🚀 Performance Considerations

Good:

  • Direct execa usage removes unnecessary abstraction layer
  • Early directory detection prevents unnecessary work in some cases

⚠️ Minor:

  • Directory detection is called twice in some execution paths - minimal impact but could be optimized

🔒 Security Assessment

Secure:

  • No security vulnerabilities identified
  • Proper input validation maintained
  • No credential exposure
  • Safe filesystem operations in tests with proper cleanup

🧪 Test Coverage

Excellent Additions:

  • New Test: update command should not create files in non-project directory (lines 274-310)
    • Tests 6 different file types that shouldn't be created
    • Proper cleanup with rmSync
    • CI environment awareness
    • Good isolation with temporary directories

Existing Tests:

  • Modified existing test to skip in CI environments appropriately
  • All tests maintain good structure and timeout handling

📝 Specific Code Comments

packages/cli/src/commands/update/actions/cli-update.ts

  • Lines 68-79: Excellent error handling with user-friendly bun installation instructions
  • Lines 52-63: Good fallback mechanism when bun migration fails

packages/cli/src/commands/update/index.ts

  • Lines 29-36: Smart early directory detection improves flow control
  • Lines 34-35: Clear logic for determining update behavior

packages/cli/tests/commands/update.test.ts

  • Lines 274-310: Comprehensive test for file creation prevention
  • Lines 293-297: Good coverage of different package manager files

packages/server/src/socketio/index.ts

  • Lines 282-284: More precise DM detection logic prevents false positives

🎯 Overall Assessment

Score: 8.5/10

This is a solid release preparation PR with meaningful improvements:

  • ✅ Better error handling and user experience
  • ✅ Bug fixes in DM detection logic
  • ✅ Enhanced test coverage for edge cases
  • ✅ Code simplification while maintaining functionality
  • ✅ No security concerns
  • ✅ Proper CI environment considerations

💡 Minor Suggestions

  1. Caching: Consider caching the directory detection result in index.ts to avoid duplicate calls
  2. Consistency: The error message format could be more consistent between different error scenarios

Approval Recommendation

This PR is ready for merge. The changes are well-tested, improve code quality, and fix actual issues without introducing new risks.


Copy link
Contributor

claude bot commented Jul 7, 2025

Claude finished @wtfsayo's task —— View job


PR Review - Release 1.0.19

Review Tasks:

  • Analyze CLI update command changes
  • Review test coverage additions
  • Examine server socketio modifications
  • Assess code quality and best practices
  • Check for potential bugs or security issues
  • Provide comprehensive feedback

🎯 Overall Assessment

This is a well-structured release PR with significant improvements to the CLI update functionality and comprehensive test coverage. The code follows TypeScript best practices and includes proper error handling throughout.

Code Quality & Best Practices

Strengths:

  • Strong TypeScript usage with proper type annotations and interfaces
  • Comprehensive error handling with try-catch blocks and fallback mechanisms
  • Good separation of concerns across modules
  • Consistent logging patterns using the @elizaos/core logger
  • Secure subprocess execution using execa instead of raw shell commands

Areas for Improvement:

  • Complex conditional logic in packages/cli/src/commands/update/index.ts:34-36 could be simplified for better readability
  • Method length: handleMessageSubmission in socketio/index.ts is quite long (~200 lines) and could benefit from decomposition

🔍 Potential Issues

Minor Issues:

  1. CLI Update Actions (cli-update.ts:53,69):

    • Fallback to npm install -g assumes npm is available
    • Error messages could be more actionable for users
  2. SocketIO Server (socketio/index.ts:208):

    • String comparison for UUID validation: serverId === DEFAULT_SERVER_ID should use UUID type checking
    • broadcastLog method doesn't validate the logEntry parameter

Edge Cases Handled Well:

  • ✅ NPM to Bun migration with proper fallback
  • ✅ Non-project directory detection
  • ✅ Empty project handling
  • ✅ CI environment considerations

🛡️ Security Assessment

Strong Security Practices:

  • ✅ Uses execa for secure subprocess execution (prevents command injection)
  • ✅ Proper UUID validation with validateUuid() function
  • ✅ Input sanitization in SocketIO message handling
  • ✅ No hardcoded credentials or sensitive data exposure

No security concerns identified.

Performance Considerations

Efficient Patterns:

  • ✅ Async/await usage is appropriate
  • ✅ Efficient event handling in SocketIO
  • ✅ Proper cleanup in disconnect handlers
  • ✅ Conditional execution to avoid unnecessary work

Minor Optimization:

  • Consider memoizing isRunningViaNpx() and isRunningViaBunx() calls if they're expensive

🧪 Test Coverage

Excellent Test Coverage:

  • 40+ new test lines with comprehensive scenarios
  • ✅ Tests CLI update, package updates, and combined operations
  • ✅ Covers edge cases: empty directories, non-ElizaOS projects, CI environments
  • ✅ Proper test isolation with setup/teardown
  • ✅ Timeout handling for async operations

Test Quality:

  • ✅ Well-structured test organization
  • ✅ Clear test descriptions and expectations
  • ✅ Appropriate use of skipIf for CI-specific tests

📋 Specific Recommendations

  1. Consider simplifying conditional logic in index.ts:34-36:

    // Current (complex)
    const updateCli = options.cli || (!options.cli && !options.packages);
    const updatePackages = options.packages || (!options.cli && !options.packages && isInProject);
    
    // Suggested (clearer)
    const updateCli = options.cli || (!options.packages);
    const updatePackages = options.packages || (!options.cli && isInProject);
  2. Add input validation to broadcastLog method:

    public broadcastLog(io: SocketIOServer, logEntry: any) {
      if (!logEntry || this.logStreamConnections.size === 0) return;
      // ... rest of method
    }
  3. Consider breaking down handleMessageSubmission into smaller, focused methods

🏆 Verdict

✅ APPROVE - This is a solid release PR with:

  • High-quality code improvements
  • Comprehensive test coverage
  • Good security practices
  • Proper error handling

The changes enhance the CLI update functionality significantly while maintaining code quality standards. The comprehensive test suite provides confidence in the reliability of the update mechanisms.

Ready for release pending any final integration tests.


…ackage-updates

fix: improve browser compatibility and update dependencies
Copy link
Contributor

claude bot commented Jul 7, 2025

Claude finished @wtfsayo's task —— View job


PR Review: chore: 1.0.19

  • Read and analyze all changed files
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Review performance considerations
  • Assess security concerns
  • Evaluate test coverage improvements
  • Provide comprehensive feedback

🔍 Code Review Summary

This PR includes several quality improvements and bug fixes across CLI update functionality and SocketIO handling. Overall, the changes demonstrate good engineering practices with improved error handling, better test coverage, and more precise logic.

📋 Code Quality & Best Practices

Strengths:

  • Error Handling: Excellent improvement in cli-update.ts with proper try-catch blocks and user-friendly error messages
  • Code Simplification: Removed dependency on executeInstallation utility in favor of direct execa usage, reducing complexity
  • Test Coverage: Added comprehensive test case for edge case behavior (non-project directory file creation)
  • CI Awareness: Proper skipIf conditions for CI environments where certain tests shouldn't run

⚠️ Minor Concerns:

  • Directory Detection Logic: The early directory detection in index.ts:29-31 is called twice - once for control flow and again later for package updates. Consider caching the result.

🐛 Bug Fixes & Improvements

Fixed Issues:

  1. SocketIO DM Detection: Removed overly broad senderName?.includes('DM') logic in socketio/index.ts:284 which could cause false positives
  2. CLI Update Flow: Better separation of concerns between CLI and package updates
  3. File Creation Prevention: New test ensures update command doesn't create unwanted files in non-project directories

🔧 Technical Improvements:

  • Better Error Messages: CLI update now provides clear installation instructions when bun is not found
  • Logging: Enhanced debug logging in CLI update process
  • Flow Control: More explicit logic for determining update behavior based on flags and context

🚀 Performance Considerations

Good:

  • Direct execa usage removes unnecessary abstraction layer
  • Early directory detection prevents unnecessary work in some cases

⚠️ Minor:

  • Directory detection is called twice in some execution paths - minimal impact but could be optimized

🔒 Security Assessment

Secure:

  • No security vulnerabilities identified
  • Proper input validation maintained
  • No credential exposure
  • Safe filesystem operations in tests with proper cleanup

🧪 Test Coverage

Excellent Additions:

  • New Test: update command should not create files in non-project directory (lines 274-310)
    • Tests 6 different file types that shouldn't be created
    • Proper cleanup with rmSync
    • CI environment awareness
    • Good isolation with temporary directories

Existing Tests:

  • Modified existing test to skip in CI environments appropriately
  • All tests maintain good structure and timeout handling

📝 Specific Code Comments

packages/cli/src/commands/update/actions/cli-update.ts

  • Lines 68-79: Excellent error handling with user-friendly bun installation instructions
  • Lines 52-63: Good fallback mechanism when bun migration fails

packages/cli/src/commands/update/index.ts

  • Lines 29-36: Smart early directory detection improves flow control
  • Lines 34-35: Clear logic for determining update behavior

packages/cli/tests/commands/update.test.ts

  • Lines 274-310: Comprehensive test for file creation prevention
  • Lines 293-297: Good coverage of different package manager files

packages/server/src/socketio/index.ts

  • Lines 282-284: More precise DM detection logic prevents false positives

🎯 Overall Assessment

Score: 8.5/10

This is a solid release preparation PR with meaningful improvements:

  • ✅ Better error handling and user experience
  • ✅ Bug fixes in DM detection logic
  • ✅ Enhanced test coverage for edge cases
  • ✅ Code simplification while maintaining functionality
  • ✅ No security concerns
  • ✅ Proper CI environment considerations

💡 Minor Suggestions

  1. Caching: Consider caching the directory detection result in index.ts to avoid duplicate calls
  2. Consistency: The error message format could be more consistent between different error scenarios

Approval Recommendation

This PR is ready for merge. The changes are well-tested, improve code quality, and fix actual issues without introducing new risks.


- Set protocolImports to false to prevent esbuild directory read errors
- Add crypto-browserify override to ensure proper browser polyfills
- Align vite.config.cypress.ts with main vite.config.ts polyfill settings
- Add process/browser alias to fix readable-stream import issues

This fixes the "Error reading node-stdlib-browser/esm/proxy/process: is a directory"
error that was preventing Cypress component tests from running in CI.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

claude bot commented Jul 7, 2025

Claude finished @wtfsayo's task —— View job


PR Review: chore: 1.0.19

  • Read and analyze all changed files
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Review performance considerations
  • Assess security concerns
  • Evaluate test coverage improvements
  • Provide comprehensive feedback

🔍 Code Review Summary

This PR includes several quality improvements and bug fixes across CLI update functionality and SocketIO handling. Overall, the changes demonstrate good engineering practices with improved error handling, better test coverage, and more precise logic.

📋 Code Quality & Best Practices

Strengths:

  • Error Handling: Excellent improvement in cli-update.ts with proper try-catch blocks and user-friendly error messages
  • Code Simplification: Removed dependency on executeInstallation utility in favor of direct execa usage, reducing complexity
  • Test Coverage: Added comprehensive test case for edge case behavior (non-project directory file creation)
  • CI Awareness: Proper skipIf conditions for CI environments where certain tests shouldn't run

⚠️ Minor Concerns:

  • Directory Detection Logic: The early directory detection in index.ts:29-31 is called twice - once for control flow and again later for package updates. Consider caching the result.

🐛 Bug Fixes & Improvements

Fixed Issues:

  1. SocketIO DM Detection: Removed overly broad senderName?.includes('DM') logic in socketio/index.ts:284 which could cause false positives
  2. CLI Update Flow: Better separation of concerns between CLI and package updates
  3. File Creation Prevention: New test ensures update command doesn't create unwanted files in non-project directories

🔧 Technical Improvements:

  • Better Error Messages: CLI update now provides clear installation instructions when bun is not found
  • Logging: Enhanced debug logging in CLI update process
  • Flow Control: More explicit logic for determining update behavior based on flags and context

🚀 Performance Considerations

Good:

  • Direct execa usage removes unnecessary abstraction layer
  • Early directory detection prevents unnecessary work in some cases

⚠️ Minor:

  • Directory detection is called twice in some execution paths - minimal impact but could be optimized

🔒 Security Assessment

Secure:

  • No security vulnerabilities identified
  • Proper input validation maintained
  • No credential exposure
  • Safe filesystem operations in tests with proper cleanup

🧪 Test Coverage

Excellent Additions:

  • New Test: update command should not create files in non-project directory (lines 274-310)
    • Tests 6 different file types that shouldn't be created
    • Proper cleanup with rmSync
    • CI environment awareness
    • Good isolation with temporary directories

Existing Tests:

  • Modified existing test to skip in CI environments appropriately
  • All tests maintain good structure and timeout handling

📝 Specific Code Comments

packages/cli/src/commands/update/actions/cli-update.ts

  • Lines 68-79: Excellent error handling with user-friendly bun installation instructions
  • Lines 52-63: Good fallback mechanism when bun migration fails

packages/cli/src/commands/update/index.ts

  • Lines 29-36: Smart early directory detection improves flow control
  • Lines 34-35: Clear logic for determining update behavior

packages/cli/tests/commands/update.test.ts

  • Lines 274-310: Comprehensive test for file creation prevention
  • Lines 293-297: Good coverage of different package manager files

packages/server/src/socketio/index.ts

  • Lines 282-284: More precise DM detection logic prevents false positives

🎯 Overall Assessment

Score: 8.5/10

This is a solid release preparation PR with meaningful improvements:

  • ✅ Better error handling and user experience
  • ✅ Bug fixes in DM detection logic
  • ✅ Enhanced test coverage for edge cases
  • ✅ Code simplification while maintaining functionality
  • ✅ No security concerns
  • ✅ Proper CI environment considerations

💡 Minor Suggestions

  1. Caching: Consider caching the directory detection result in index.ts to avoid duplicate calls
  2. Consistency: The error message format could be more consistent between different error scenarios

Approval Recommendation

This PR is ready for merge. The changes are well-tested, improve code quality, and fix actual issues without introducing new risks.


@wtfsayo wtfsayo requested a review from Copilot July 7, 2025 09:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR prepares the 1.0.19 release by updating runtime checks, improving build configurations, refining test execution conditions, and adjusting the CLI update flow.

  • Simplify DM channel detection and drop unnecessary await
  • Enhance Vite configs with CommonJS shims and refined polyfills
  • Introduce CI skips in update tests and refactor package vs CLI update logic; switch CLI installer to Bun

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/server/src/socketio/index.ts Remove senderName-based DM detection
packages/server/src/index.ts Drop redundant await on synchronous resolvePgliteDir
packages/client/vite.config.ts Add injectCommonJSShims plugin and explicit polyfill aliases
packages/client/vite.config.cypress.ts Tighten polyfill scopes and add process.browser definition
packages/cli/tests/commands/update.test.ts Skip certain tests in CI environments; add fresh non-project test
packages/cli/src/commands/update/index.ts Precompute directory context; guard package updates to projects
packages/cli/src/commands/update/actions/cli-update.ts Replace NPM installer with Bun via execa and better error hints
Comments suppressed due to low confidence (3)

packages/server/src/socketio/index.ts:282

  • Removing the senderName?.includes('DM') check may cause DMs to be misclassified when metadata is missing. Consider preserving a fallback or documenting why the senderName-based heuristic is no longer needed.
          const isDmChannel = metadata?.isDm || metadata?.channelType === ChannelType.DM;

packages/cli/tests/commands/update.test.ts:154

  • it.skipIf is not a standard Mocha/Jest API and may throw TypeError: it.skipIf is not a function. Use a supported conditional skip pattern (e.g., if (condition) { it.skip(...) } else { it(...) }).
  it.skipIf(process.env.CI === 'true' || process.env.GITHUB_ACTIONS === 'true')(

packages/client/vite.config.cypress.ts:84

  • Many modules were removed from the include array. Verify that essential polyfills for your tested codebase (e.g., crypto, streams) are still present and that this reduction doesn't break Cypress tests.
      'util',

@wtfsayo wtfsayo merged commit 3464099 into main Jul 7, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants