Skip to content

chore: v1.2.12 #5630

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 81 commits into from
Jul 18, 2025
Merged

chore: v1.2.12 #5630

merged 81 commits into from
Jul 18, 2025

Conversation

wtfsayo
Copy link
Member

@wtfsayo wtfsayo commented Jul 18, 2025

No description provided.

github-actions bot and others added 30 commits July 17, 2025 08:19
Replace Node.js EventEmitter with Bun's native EventTarget implementation
to align with the project's Bun-first approach and reduce Node.js dependencies.

Changes:
- Convert InternalMessageBus to use EventTarget with EventEmitter-like API
- Update SimpleMigrationAgent to extend EventTarget instead of EventEmitter
- Maintain backward compatibility with existing event handling interfaces
- Add wrapper methods (on/off/emit) to preserve existing API contracts

This migration supports the project's goal of eliminating Node.js-specific
dependencies in favor of Bun's native APIs.

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

Co-Authored-By: Claude <[email protected]>
Document that EventEmitter should never be used due to Bun compatibility
issues. Add migration guidelines and examples for using Bun's native
EventTarget API instead.

This ensures future development maintains Bun compatibility and avoids
Node.js-specific APIs that may cause issues.

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

Co-Authored-By: Claude <[email protected]>
Implemented off, removeListener, removeAllListeners, listenerCount, and listeners methods in SimpleMigrationAgent for improved event handler management and EventEmitter compatibility. Also updated InternalMessageBus to prevent duplicate event handler registration.
Updated the emit methods in SimpleMigrationAgent and InternalMessageBus to return the result of dispatchEvent. This change improves consistency and allows callers to know if the event was canceled.
Updated event handler wrappers in SimpleMigrationAgent and InternalMessageBus to check if the event is a CustomEvent before accessing the detail property. This prevents potential runtime errors when non-CustomEvent objects are passed to the handler.
feat: migrate from EventEmitter to Bun native EventTarget API
Removed dependencies and lockfile entries for @elizaos/plugin-anthropic, @elizaos/plugin-discord, @elizaos/plugin-openai, and @elizaos/plugin-coinmarketcap, along with their related packages. This reduces the project footprint and eliminates unused or deprecated plugins.
- Replace 'any' with 'unknown' in simple-migration-agent.ts and bus.ts
- Replace generic Function type with explicit function signatures
- Fixes CodeFactor maintainability issues

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

Co-Authored-By: Claude <[email protected]>
fix: replace 'any' types with 'unknown' and explicit function types
- SimpleMigrationAgent.on() now returns 'this' for method chaining
- InternalMessageBus.on() now returns 'this' for method chaining
- Maintains backward compatibility with EventEmitter-style chaining
fix: add method chaining support to EventTarget-based on() methods
…get implementations

- Add unit tests for bus.ts EventEmitter compatibility in server package
- Add unit tests for SimpleMigrationAgent EventEmitter compatibility in CLI package
- Test all EventEmitter-like methods (emit, on, off, removeListener, etc.)
- Verify proper memory management and handler cleanup
- Test edge cases like duplicate handlers, undefined data, and error handling
- Add documentation explaining the compatibility pattern and test coverage

These tests ensure that EventTarget-based implementations maintain backward compatibility
with existing code expecting EventEmitter APIs while following ElizaOS guidelines.

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

Co-Authored-By: Claude <[email protected]>
Introduces a removeAllListeners method to InternalMessageBus for removing all or specific event listeners, improving test isolation and API compatibility. Updates tests to use this method and enhances error handling verification for event listeners that throw exceptions.
test: add EventEmitter compatibility tests for EventTarget implementations
Refactors test files for EventEmitter compatibility to use consistent code formatting, including multi-line arrow functions and improved spacing. Updates the EventEmitter-Compatibility-README.md for clarity and formatting. No functional changes to test logic.
- Add null safety check in InternalMessageBus off() method
- Add documentation explaining why classes are used (EventTarget requirement)
- Document intentional EventEmitter behavior deviation (duplicate prevention)
- Improve type safety with optional chaining in cleanup logic
fix: address EventTarget migration PR review issues
Standardized whitespace in doc comments for consistency in simple-migration-agent.ts and bus.ts. No functional changes were made.
Updated all internal @elizaos/* dependencies in package.json files to use the "workspace:*" protocol for better monorepo management and consistency. Also added 'bun' as a dependency in plugin-bootstrap. This change helps ensure local packages are always resolved from the workspace.
- Remove KNAP_WORKFLOW_FIX_SUMMARY.md as the Knip workflow has been fixed
- Remove docs/code-quality-analysis.md as it's no longer relevant
…250717-215958

chore: remove obsolete documentation files
wtfsayo and others added 22 commits July 18, 2025 15:53
Added ELIZA_SKIP_LOCAL_CLI_DELEGATION env var to test/CI detection in local-cli-delegation. Enhanced local-cli-delegation and server-manager unit tests to cover more scenarios, including update command detection, local CLI usage, argument passing, process event handling, and error cases. Updated log messages for clarity.
Replaces Node.js ChildProcess with Bun's Subprocess for server management in dev commands. Refactors the server manager to a functional pattern, updates the ServerProcess interface, and adds utility functions for process state and management. Improves environment setup and process lifecycle handling for Bun compatibility.
Converted deprecated sync functions in loader.ts to async, updating their signatures and internal module loading. Rewrote server-manager.test.ts to use a fully mocked, functional server manager interface, replacing child_process mocks with Bun.spawn mocks and improving test coverage for process lifecycle, environment setup, and utility functions.
Rewrites server-manager.test.ts to use custom in-memory mocks for process management, removing reliance on Bun.spawn and real process state. This improves test isolation, reliability, and clarity by simulating server lifecycle and behaviors directly. Also adds more comprehensive test coverage for edge cases and error handling.
Refactored local CLI delegation logic to add explicit environment variable checks, delegation depth tracking, and improved debug logging. Renamed process variable in server-manager.ts for clarity. Minor formatting updates in lerna.json.
Eliminates unnecessary debug logging and the ELIZA_DISABLE_LOCAL_CLI_DELEGATION environment variable check from local CLI delegation logic to simplify the code.
Updated test imports to use 'bun:test' instead of 'vitest' and improved formatting in test files for readability. Simplified logger.info statements in module-loader.ts for consistency. No functional changes to loader logic.
…esolution

feat: enhance ModuleLoader with local-first guarantees for consistent module resolution
Copy link
Contributor

coderabbitai bot commented Jul 18, 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 18, 2025

Claude finished @wtfsayo's task —— View job


PR Review - v1.2.12 Release

📋 Review Status: COMPLETE

Critical Issues ❌

CRITICAL: Wrong test framework usage → Fix: Replace vitest with bun:test

  • File: packages/cli/src/utils/module-loader.test.ts:1
  • Issue: import { describe, it, expect, beforeEach, vi } from 'vitest';
  • Fix: Change to import { describe, it, expect, beforeEach, mock } from 'bun:test';

CRITICAL: Mixed test frameworks → Fix: Remove jest imports, use bun:test only

  • File: packages/cli/tests/unit/utils/local-cli-delegation.test.ts:1
  • Issue: Mixing jest and bun:test imports
  • Fix: Remove jest import, use mock from bun:test instead of jest.spyOn

CRITICAL: Wrong process API usage → Fix: Use Bun.spawn() instead of Node.js spawn

  • File: packages/cli/src/utils/local-cli-delegation.ts:1
  • Issue: import { spawn } from 'node:child_process';
  • Fix: Replace with Bun.spawn() per CLAUDE.md rules

CRITICAL: Wrong process API usage → Fix: Use bun-exec utilities

  • Files: Multiple files using Node.js spawn from child_process
  • Issue: Violates CLAUDE.md requirement to use Bun.spawn() or bun-exec utilities
  • Fix: Replace with existing bun-exec utilities at packages/cli/src/utils/bun-exec.ts

Important Issues ⚠️

⚠️ IMPORTANT: Inconsistent error handling → Fix: Add proper error handling

  • File: packages/cli/src/utils/module-loader.ts:82
  • Issue: Dynamic import without comprehensive error handling
  • Fix: Add more specific error types and recovery strategies

⚠️ IMPORTANT: Missing TypeScript types → Fix: Add explicit types

  • File: packages/cli/src/utils/module-loader.ts:16
  • Issue: asyncCache = new Map<string, any>();
  • Fix: Replace any with proper generic type constraints

Suggestions 💡

💡 SUGGESTION: EventEmitter compatibility → Consider: Great implementation following CLAUDE.md guidelines

  • File: packages/server/src/bus.ts
  • Good: Properly uses EventTarget instead of EventEmitter
  • Suggestion: Consider adding JSDoc for better API documentation

💡 SUGGESTION: Test coverage → Consider: Add integration tests

  • Coverage: Good unit test coverage for new utilities
  • Suggestion: Add end-to-end tests for CLI delegation functionality

Summary

This release contains significant improvements to CLI functionality and EventEmitter compatibility. However, critical blocking issues require immediate attention:

  1. Test framework violations: Mixed jest/vitest with bun:test
  2. Process API violations: Node.js spawn usage instead of Bun.spawn()
  3. Architecture violations: Not following CLAUDE.md guidelines

Recommendation: 🚫 BLOCK - Fix critical issues before merge.

Files Requiring Immediate Fixes:

  • packages/cli/src/utils/module-loader.test.ts - vitest → bun:test
  • packages/cli/tests/unit/utils/local-cli-delegation.test.ts - jest mixing
  • packages/cli/src/utils/local-cli-delegation.ts - Node.js spawn usage
  • Multiple files using spawn from child_process

…placement

fix: (cli) prevent .elizadb inheritance in nested project creation
Copy link
Contributor

claude bot commented Jul 18, 2025

Claude finished @wtfsayo's task —— View job


PR Review - v1.2.12 Release

📋 Review Status:

  • Security audit - scan for hardcoded keys, SQL injection, XSS vulnerabilities
  • Verify all new code has comprehensive test coverage
  • Check package.json files use bun only (no npm/pnpm/yarn/jest/vitest)
  • Analyze for breaking changes without migration paths
  • Review TypeScript usage (no 'any' types, proper typing)
  • Verify @elizaos/core imports (not packages/core)
  • Review error handling implementation in new code
  • Review EventEmitter compatibility implementation
  • Review new CLI utilities and module loading changes
  • Review server manager refactoring

🚨 CRITICAL ISSUES

CRITICAL: Wrong process spawning API → Fix: Replace spawn from node:child_process with Bun.spawn()

  • File: packages/cli/src/utils/local-cli-delegation.ts:1
  • Issue: Uses forbidden Node.js spawn API instead of Bun.spawn
  • Fix: Replace with existing bun-exec utility or Bun.spawn() directly

CRITICAL: Jest usage in tests → Fix: Remove all jest imports and APIs, use bun test only

  • File: packages/cli/tests/unit/utils/local-cli-delegation.test.ts:1,55,61,78
  • Issue: Imports and uses jest APIs which are explicitly forbidden
  • Fix: Replace jest.spyOn, jest.restoreAllMocks with bun test equivalents

CRITICAL: npm command in script → Fix: Replace npx turbo with bun turbo

  • File: package.json:22
  • Issue: Uses npx turbo run clean instead of bun turbo run clean
  • Fix: Change to bun turbo run clean

⚠️ IMPORTANT ISSUES

⚠️ IMPORTANT: Class-based approach → Fix: Refactor to functional patterns

  • File: packages/cli/src/utils/module-loader.ts:13
  • Issue: Uses class ModuleLoader instead of functional patterns
  • Fix: Refactor to functional approach with closure-based state management

💡 POSITIVE FINDINGS

EventEmitter Compatibility: Correctly uses EventTarget instead of EventEmitter
Import Patterns: Properly uses @elizaos/core imports
TypeScript Usage: No any types found in new code
Security: No hardcoded secrets or keys found
Test Coverage: New functionality has comprehensive test coverage
Server Manager: Correctly uses Bun.spawn() instead of Node.js APIs

📋 VERSION CONSISTENCY

The PR title indicates v1.2.12 but package versions show 1.2.11-beta.9. This appears to be a normal pre-release versioning pattern.


🔄 RECOMMENDATION: Address the 4 critical issues before merging. The forbidden tooling usage (Node.js spawn, jest, npm) violates core project requirements and must be fixed.


Cleaned up unnecessary trailing whitespace in the createTEEProject and createProject functions for better code style consistency.
Copy link
Contributor

claude bot commented Jul 18, 2025

Claude finished @wtfsayo's task —— View job


PR Review - v1.2.12 Release

📋 Review Status: COMPLETE

Critical Issues ❌

CRITICAL: Wrong test framework usage → Fix: Replace vitest with bun:test

  • File: packages/cli/src/utils/module-loader.test.ts:1
  • Issue: import { describe, it, expect, beforeEach, vi } from 'vitest';
  • Fix: Change to import { describe, it, expect, beforeEach, mock } from 'bun:test';

CRITICAL: Mixed test frameworks → Fix: Remove jest imports, use bun:test only

  • File: packages/cli/tests/unit/utils/local-cli-delegation.test.ts:1
  • Issue: Mixing jest and bun:test imports
  • Fix: Remove jest import, use mock from bun:test instead of jest.spyOn

CRITICAL: Wrong process API usage → Fix: Use Bun.spawn() instead of Node.js spawn

  • File: packages/cli/src/utils/local-cli-delegation.ts:1
  • Issue: import { spawn } from 'node:child_process';
  • Fix: Replace with Bun.spawn() per CLAUDE.md rules

CRITICAL: Wrong process API usage → Fix: Use bun-exec utilities

  • Files: Multiple files using Node.js spawn from child_process
  • Issue: Violates CLAUDE.md requirement to use Bun.spawn() or bun-exec utilities
  • Fix: Replace with existing bun-exec utilities at packages/cli/src/utils/bun-exec.ts

Important Issues ⚠️

⚠️ IMPORTANT: Inconsistent error handling → Fix: Add proper error handling

  • File: packages/cli/src/utils/module-loader.ts:82
  • Issue: Dynamic import without comprehensive error handling
  • Fix: Add more specific error types and recovery strategies

⚠️ IMPORTANT: Missing TypeScript types → Fix: Add explicit types

  • File: packages/cli/src/utils/module-loader.ts:16
  • Issue: asyncCache = new Map<string, any>();
  • Fix: Replace any with proper generic type constraints

Suggestions 💡

💡 SUGGESTION: EventEmitter compatibility → Consider: Great implementation following CLAUDE.md guidelines

  • File: packages/server/src/bus.ts
  • Good: Properly uses EventTarget instead of EventEmitter
  • Suggestion: Consider adding JSDoc for better API documentation

💡 SUGGESTION: Test coverage → Consider: Add integration tests

  • Coverage: Good unit test coverage for new utilities
  • Suggestion: Add end-to-end tests for CLI delegation functionality

Summary

This release contains significant improvements to CLI functionality and EventEmitter compatibility. However, critical blocking issues require immediate attention:

  1. Test framework violations: Mixed jest/vitest with bun:test
  2. Process API violations: Node.js spawn usage instead of Bun.spawn()
  3. Architecture violations: Not following CLAUDE.md guidelines

Recommendation: 🚫 BLOCK - Fix critical issues before merge.

Files Requiring Immediate Fixes:

  • packages/cli/src/utils/module-loader.test.ts - vitest → bun:test
  • packages/cli/tests/unit/utils/local-cli-delegation.test.ts - jest mixing
  • packages/cli/src/utils/local-cli-delegation.ts - Node.js spawn usage
  • Multiple files using spawn from child_process

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Monorepo Version Mismatch Causes Deployment Issues

Monorepo version inconsistency: The PR title states "v1.2.12", but lerna.json is "1.2.11-beta.9", and individual packages have mixed versions (1.2.10 and 1.2.11-beta.9). This creates potential dependency resolution and deployment issues.

lerna.json#L1-L2

eliza/lerna.json

Lines 1 to 2 in 56f983f

{
"version": "1.2.11-beta.9",

packages/server/package.json#L2-L3

"name": "@elizaos/server",
"version": "1.2.11-beta.9",

Fix in CursorFix in Web


Was this report helpful? Give feedback by reacting with 👍 or 👎

@wtfsayo wtfsayo merged commit b089b9c into main Jul 18, 2025
21 of 28 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.

3 participants