Skip to content

Conversation

@odilitime
Copy link
Collaborator

@odilitime odilitime commented Nov 6, 2025

Note

Aligns server tests and routers with new ElizaOS interfaces, clarifies plugin auto-injection behavior, and deflakes/modernizes middleware, loader, socket, and utility tests.

  • Server/API alignment:
    • Update agentExistsMiddleware, Socket.IO router, Jobs and Agent Runs routers to use ElizaOS getters instead of agent maps.
    • Adjust message flow and log streaming tests to new event/filter semantics.
  • Plugin loading behavior:
    • Clarify that server auto-injects sql plugin; bootstrap injection handled at character level; ensure deduping and order expectations.
  • Test refactors/deflakes:
    • Replace heavy mock.module with spies and lightweight mocks; remove brittle fs mocks.
    • Skip flaky/integration suites causing interference (version, DB ops, message bus, agent runs, some jobs tests).
  • Utilities and middleware:
    • Update tests for authMiddleware, validation, security headers, rate limits.
    • Adjust sanitizeFilename expectations and resolvePgliteDir behavior; strengthen path handling assertions.
  • Misc:
    • Minor type/interface tweaks (e.g., runtime methods) and improved express server boot/teardown in tests.

Written by Cursor Bugbot for commit a988400. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Improved plugin loading flexibility: server now recognizes both short and full bootstrap plugin identifiers during initialization.
    • Enhanced SQL plugin auto-injection and plugin deduplication logic.
  • Refactor

    • Removed internal loader APIs: tryLoadFile and loadCharacterTryPath functions are no longer available.
    • Updated core routing infrastructure for improved agent management.
    • Simplified test infrastructure by reducing mock dependencies and improving test reliability across environments.

@graphite-app
Copy link

graphite-app bot commented Nov 6, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • merge-queue-hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@odilitime odilitime marked this pull request as ready for review November 6, 2025 04:07
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This pull request refactors the test infrastructure across multiple server test files, removing global mock modules and replacing them with spy-based assertions and an ElizaOS-centered dependency injection pattern. Test files shift from isolated mocking to integration-style testing, and key APIs like SocketIORouter change from accepting agent Maps to accepting elizaOS instances.

Changes

Cohort / File(s) Summary
Mock removal & spy migration
packages/server/src/__tests__/authMiddleware.test.ts, packages/server/src/__tests__/middleware.test.ts, packages/server/src/__tests__/validation.test.ts
Replaced mock.module logger blocks with spyOn-based spy for logger.warn; adjusted assertions to reference loggerWarnSpy instead of direct logger.warn calls
Comprehensive mock elimination
packages/server/src/__tests__/agent-server-database.test.ts, packages/server/src/__tests__/agent-server-errors.test.ts, packages/server/src/__tests__/agent-server-initialization.test.ts, packages/server/src/__tests__/agent-server-middleware.test.ts, packages/server/src/__tests__/ui-disable-feature.test.ts
Removed mock.module blocks for @elizaos/core and @elizaos/plugin-sql; tests now rely on real or simplified dependencies
API test suite restructuring
packages/server/src/__tests__/api.test.ts
Eliminated mock setup for plugin-sql, filesystem, and Socket.IO; removed entire test harness scaffold for server initialization
ElizaOS agent injection pattern
packages/server/src/__tests__/agents-runs.test.ts, packages/server/src/__tests__/socketio-router.test.ts
Replaced Map<UUID, IAgentRuntime> with mockElizaOS object exposing getAgent(); updated router initialization and agent retrieval logic
SocketIORouter API change
packages/server/src/socketio.ts
Constructor signature changed from (agents: Map<UUID, IAgentRuntime>, serverInstance) to (elizaOS, serverInstance)
Test suite skipping
packages/server/src/__tests__/integration/database-operations.test.ts, packages/server/src/__tests__/integration/jobs-message-flow.test.ts, packages/server/src/__tests__/message-bus.test.ts, packages/server/src/api/system/__tests__/version.test.ts
Marked test suites or specific tests with describe.skip/it.skip; added explanatory comments noting timeout/interference issues
Exported function removal
packages/server/src/__tests__/loader.test.ts
Removed tryLoadFile and loadCharacterTryPath from loader module exports; tests refactored to skip or update correspondingly
Plugin presence validation relaxation
packages/server/src/__tests__/bootstrap-autoload.test.ts
Relaxed strict plugin ordering checks; replaced specific bootstrap/SQL order assertions with flexible presence checks accounting for server auto-injection
Test isolation and filesystem handling
packages/server/src/__tests__/file-utils.test.ts, packages/server/src/__tests__/utils.test.ts
Removed mocks for node:fs and dotenv; adjusted assertions to validate structure/edge cases rather than strict filesystem operations; added process.cwd preservation
Suite removal
packages/server/src/__tests__/agent-plugin-reload.test.ts
Removed entire test suite (plugin change detection, validation, agent restart scenarios); file reduced to bare imports
Suite removal
packages/server/src/__tests__/agent-server-management.test.ts
Removed mock.module blocks and test scaffolding; test behavior shifts to real dependency usage
Minor formatting
packages/server/src/__tests__/basic-functionality.test.ts
Cosmetic adjustments: constructor formatting and middleware parameter rename from _req to req
Simple mock removal
packages/server/src/__tests__/simple-validation.test.ts
Removed mock import and mock.module('@elizaos/core', ...) block; tests now use direct expandTildePath without global mocks
Async lifecycle management
packages/server/src/api/system/__tests__/version.test.ts
Upgraded beforeEach/afterEach to async/await with explicit server startup/shutdown handling and port-release delay

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test Suite
    participant Router as SocketIORouter
    participant ElizaOS as elizaOS Instance
    
    rect rgb(200, 220, 255)
    Note over Test,Router: New Pattern (Post-PR)
    Test->>ElizaOS: Create mockElizaOS with getAgent()
    Test->>Router: Initialize with (elizaOS, serverInstance)
    Test->>Router: Request agent via socket
    Router->>ElizaOS: getAgent(agentId)
    ElizaOS-->>Router: Return IAgentRuntime
    end
    
    rect rgb(255, 220, 200)
    Note over Test,Router: Old Pattern (Pre-PR)
    Test->>Test: Create Map<UUID, IAgentRuntime>
    Test->>Router: Initialize with (agentsMap, serverInstance)
    Test->>Router: Request agent via socket
    Router->>Test: Direct map.get(agentId)
    end
Loading
sequenceDiagram
    participant Test as Test File
    participant Logger as logger Module
    participant Spy as spyOn(logger, 'warn')
    
    rect rgb(200, 220, 255)
    Note over Test,Spy: New Spy-Based Approach (Post-PR)
    Test->>Spy: Create loggerWarnSpy in beforeEach
    Test->>Logger: Code invokes logger.warn()
    Logger->>Spy: Captured by spy
    Test->>Spy: Assert loggerWarnSpy was called
    Test->>Spy: Restore spy in afterEach
    end
    
    rect rgb(255, 220, 200)
    Note over Test,Logger: Old Mock-Based Approach (Pre-PR)
    Test->>Logger: mock.module(`@elizaos/core`) with logger shim
    Test->>Logger: Code invokes mocked logger.warn()
    Test->>Logger: Assert on mock.warn calls
    Test->>Logger: mock.restore() after test
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • API signature change: SocketIORouter constructor refactored to accept elizaOS object instead of agents Map; requires understanding both old and new patterns
  • Exported function removal: tryLoadFile and loadCharacterTryPath removed from loader; verify removal doesn't break external callers
  • Heterogeneous changes: Mix of mock elimination, spy introduction, test skipping, and logic adjustments across 20+ files; each requires separate reasoning
  • Integration-style shift: Tests move from isolated mocking to real dependency usage; verify test isolation doesn't degrade and false positives aren't introduced
  • Plugin validation relaxation: bootstrap-autoload tests accept broader conditions; confirm intent and that edge cases aren't overlooked

Possibly related PRs

Suggested labels

1.x, Epic 1, Epic 6

Suggested reviewers

  • ChristopherTrimboli
  • wtfsayo
  • 0xbbjoker

Poem

🧪 Tests shed their mocks like morning dew,
Spy-based spying keeps tabs anew,
ElizaOS steps in, neat and clean,
Agent maps fade from the test-suite scene,
Simpler fixtures, leaner code—
Testing truth on a brighter road. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning PR description lacks required template sections including Risks, Background detail, Documentation changes, and Testing information. Complete the PR description using the repository template: add Risks section, expand Background (What/Kind), specify Documentation changes needed, and provide Testing details with review guidance and steps.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: align server tests with ElizaOS API changes' directly and specifically describes the main objective of the PR, which involves refactoring server tests to align with ElizaOS API changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch odi-server-tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@odilitime
Copy link
Collaborator Author

@CodeRabbit review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@odilitime
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@claude
Copy link
Contributor

claude bot commented Nov 6, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)
packages/server/src/__tests__/agent-server-initialization.test.ts (1)

1-9: Empty test file—delete it or restore the tests.

The entire test suite has been removed, leaving only unused imports and a comment. This creates dead code and confusion.

If initialization tests are no longer needed, delete this file entirely. If they should be restored (even as describe.skip), add them back with proper test cases.

Apply this action:

#!/bin/bash
# Verify if there's any actual test code beyond line 9
cat packages/server/src/__tests__/agent-server-initialization.test.ts | wc -l
echo "---"
cat packages/server/src/__tests__/agent-server-initialization.test.ts
packages/server/src/__tests__/message-bus.test.ts (1)

112-112: Undefined variable mock will fail if tests are re-enabled.

The mock.restore() call references an undefined mock variable. This is a latent bug that will cause a ReferenceError when the suite is re-enabled.

Apply this diff to remove the invalid call:

   afterEach(() => {
-    mock.restore();
+    // Cleanup handled by individual test mocks
   });
packages/server/src/__tests__/agent-server-errors.test.ts (1)

1-10: Delete this empty test file.

The file contains no test suites or test cases—only unused imports and stale comments. According to the AI summary, all AgentServer error handling tests were removed, leaving dead code.

Do you want me to open an issue to track removing this file, or would you prefer to delete it in this PR?

packages/server/src/__tests__/ui-disable-feature.test.ts (1)

74-74: Replace any with specific types.

Functions use any for parameters, violating the TypeScript guideline requiring specific types. Define minimal interfaces or use Pick from Express types.

Apply this pattern to fix:

+interface MockApp {
+  use: (middleware: string) => void;
+}
+
+interface MockResponse {
+  sendStatus: (status: number) => void;
+}
+
-const configureStaticFiles = (app: any, uiEnabled: boolean) => {
+const configureStaticFiles = (app: MockApp, uiEnabled: boolean) => {

-const handleUIDisabledRequest = (res: any, uiEnabled: boolean) => {
+const handleUIDisabledRequest = (res: MockResponse, uiEnabled: boolean) => {

-const configureSPAFallback = (app: any, uiEnabled: boolean) => {
+const configureSPAFallback = (app: MockApp, uiEnabled: boolean) => {

Also applies to: 97-97, 119-119

packages/server/src/__tests__/bootstrap-autoload.test.ts (3)

34-58: Test name doesn't match implementation.

The test is named "should automatically inject bootstrap plugin by default" but now verifies SQL plugin presence instead of bootstrap. Based on the comments, bootstrap injection happens at the character level via buildCharacterPlugins(), not by the server. Either rename this test to reflect SQL plugin verification or add an explicit bootstrap check.

Consider renaming to match the actual assertion:

-    it('should automatically inject bootstrap plugin by default', async () => {
+    it('should automatically inject SQL plugin by default', async () => {
       const testCharacter: Character = {
         name: 'TestAgent',
         bio: ['Test agent for bootstrap injection'],
         plugins: [], // No plugins specified
       };
 
       const runtimes = await server.startAgents([{ character: testCharacter }], {
         isTestMode: true,
       });
 
       expect(runtimes).toHaveLength(1);
       const runtime = runtimes[0];
 
       // Verify server loaded required plugins
       // Note: Bootstrap plugin auto-injection happens at character level via buildCharacterPlugins()
       // The server itself only auto-injects SQL plugin
       const pluginNames = runtime.plugins.map((p) => p.name);
       
       // Server should have at least SQL plugin
       const hasSQL = runtime.plugins.some(
         (p) => p.name === 'sql' || p.name === '@elizaos/plugin-sql'
       );
       expect(hasSQL).toBe(true);
       expect(runtime.plugins.length).toBeGreaterThan(0);
     });

Or add a bootstrap check if both should be verified:

       // Server should have at least SQL plugin
       const hasSQL = runtime.plugins.some(
         (p) => p.name === 'sql' || p.name === '@elizaos/plugin-sql'
       );
+      const hasBootstrap = runtime.plugins.some(
+        (p) => p.name === 'bootstrap' || p.name === '@elizaos/plugin-bootstrap'
+      );
       expect(hasSQL).toBe(true);
+      expect(hasBootstrap).toBe(true);
       expect(runtime.plugins.length).toBeGreaterThan(0);

177-214: Test doesn't verify the promised plugin order.

The test is named "should maintain correct plugin order: bootstrap -> character -> runtime -> SQL" but the assertions no longer verify any ordering. The code only checks that plugins are present, not their order. Either update the test to verify the ordering or rename it to reflect what it actually tests.

Option 1: Rename to match the actual assertions:

-  it('should maintain correct plugin order: bootstrap -> character -> runtime -> SQL', async () => {
+  it('should load all plugins successfully', async () => {

Option 2: Add ordering assertions if the order should be verified:

     // Verify plugins are present (server only auto-injects SQL)
     expect(runtimePluginIndex).not.toBe(-1);
     expect(sqlIndex).not.toBe(-1);

-    // Verify plugins loaded successfully
+    // Verify plugin order: runtime plugin should come before SQL
+    expect(runtimePluginIndex).toBeLessThan(sqlIndex);
     expect(runtime.plugins.length).toBeGreaterThan(0);

218-238: Test name mentions bootstrap but doesn't verify it.

The test is named "should inject bootstrap and SQL for all agents" but only verifies SQL plugin presence. If bootstrap should be verified for all agents, add a check for it. Otherwise, update the test name.

Option 1: Add bootstrap verification:

       // Verify both agents have SQL plugin (server auto-injects)
       for (const runtime of runtimes) {
         // Plugin names can be either short ('sql') or full package name ('@elizaos/plugin-sql')
         const hasSQL = runtime.plugins.some(
           (p) => p.name === 'sql' || p.name === '@elizaos/plugin-sql'
         );
+        const hasBootstrap = runtime.plugins.some(
+          (p) => p.name === 'bootstrap' || p.name === '@elizaos/plugin-bootstrap'
+        );
 
         expect(hasSQL).toBe(true);
+        expect(hasBootstrap).toBe(true);
         expect(runtime.plugins.length).toBeGreaterThan(0);
       }

Option 2: Update the test name to match what's verified:

-  it('should inject bootstrap and SQL for all agents', async () => {
+  it('should inject SQL for all agents', async () => {
packages/server/src/__tests__/agent-plugin-reload.test.ts (1)

1-16: Remove stale header comment or restore tests.

The file header describes extensive plugin reload tests (change detection, restart logic, validation), but no test implementation exists. Either restore the tests with describe.skip if they're temporarily disabled, or remove the outdated header comment.

If tests are permanently removed:

-/**
- * Agent plugin reload tests
- * Tests plugin change detection and agent restart logic for PATCH /api/agents/:agentId endpoint
- * Addresses issues:
- * - Plugin change detection using proper array comparison
- * - Agent restart with error recovery
- * - Input validation for plugins array
- */
-
 import { describe, it, expect, beforeEach, jest } from 'bun:test';
 import type { Character } from '@elizaos/core';
-
-// Type for plugins (string or object with name)
-type PluginType = string | { name: string };
-
-// Mock logger to avoid console output during tests
🧹 Nitpick comments (14)
packages/server/src/__tests__/agent-server-initialization.test.ts (1)

5-5: Remove unused jest import.

If all mocks and tests have been removed, the jest import from bun:test is unnecessary.

packages/server/src/__tests__/ui-disable-feature.test.ts (1)

9-254: Consider testing actual server implementation.

Tests define and verify inline utility functions rather than importing actual server code. This documents expected behavior but doesn't verify the implementation.

Consider importing and testing the actual server functions or clarifying that this is a specification test.

packages/server/src/__tests__/simple-validation.test.ts (1)

34-71: Extract duplicate validateUuidPattern helper.

The validateUuidPattern function is defined identically in two test cases (lines 37-40 and 50-53). Extract it to the describe block level to eliminate duplication.

Apply this diff:

 describe('Basic validation functionality', () => {
+  const validateUuidPattern = (id: string): boolean => {
+    const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+    return uuidRegex.test(id);
+  };
+
   it('should validate UUID format', () => {
     // Test the validation logic directly
-    const validateUuidPattern = (id: string): boolean => {
-      const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
-      return uuidRegex.test(id);
-    };

     const validUuid = '123e4567-e89b-12d3-a456-426614174000';
     const invalidUuid = 'invalid-uuid';

     expect(validateUuidPattern(validUuid)).toBe(true);
     expect(validateUuidPattern(invalidUuid)).toBe(false);
   });

   it('should detect various UUID formats', () => {
-    const validateUuidPattern = (id: string): boolean => {
-      const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
-      return uuidRegex.test(id);
-    };

     const testCases = [
packages/server/src/api/system/__tests__/version.test.ts (2)

5-5: Remove unused import.

setSystemTime is imported but never used in the file.

-import { describe, it, expect, beforeEach, afterEach, setSystemTime } from 'bun:test';
+import { describe, it, expect, beforeEach, afterEach } from 'bun:test';

63-64: Extract magic number to named constant.

The 10ms delay aids port release, but the hardcoded value reduces clarity.

Consider extracting to a named constant:

+const PORT_RELEASE_DELAY_MS = 10;
+
 afterEach(async () => {
   if (server && typeof server.close === 'function') {
     await new Promise<void>((resolve) => {
       server.close(() => {
         server = null;
         resolve();
       });
     });
-    // Add small delay to ensure port is fully released
-    await new Promise(resolve => setTimeout(resolve, 10));
+    await new Promise(resolve => setTimeout(resolve, PORT_RELEASE_DELAY_MS));
   }
 });
packages/server/src/__tests__/integration/jobs-message-flow.test.ts (1)

66-107: Skipping the test is appropriate, but consider removing it entirely.

The test skip aligns with the PR's goal to remove flaky integration tests. However, the assertion on line 106 (expect(messageReceived || true).toBe(true)) always passes, rendering the test meaningless even before it was skipped. Consider removing this test entirely rather than keeping it as it.skip.

packages/server/src/__tests__/loader.test.ts (2)

200-265: Clean up or document skipped test block dependencies.

This describe.skip block contains tests that reference fs.readFileSync without any imports or mock setup. If someone attempts to re-enable these tests, they'll fail immediately.

Consider either:

  1. Removing the test bodies entirely and leaving a comment about integration-level coverage, or
  2. Adding a detailed comment listing required dependencies (fs mocking, specific imports) to re-enable.

Apply this diff to clarify what's needed:

-  describe.skip('loadCharacterTryPath', () => {
+  /* SKIPPED: loadCharacterTryPath tests
+   * These require fs mocking (fs.readFileSync) which causes test hangs in Bun test runner.
+   * To re-enable: import and mock fs module, set up fs.readFileSync mock in beforeEach.
+   * File loading is validated through integration tests instead.
+   */
+  describe.skip('loadCharacterTryPath', () => {

293-420: Clean up skipped test block with unmocked dependencies.

This describe.skip block contains extensive test code referencing unmocked dependencies:

  • fs.readFileSync, fs.promises.mkdir, fs.promises.readdir (no imports or mocks)
  • logger.error, logger.info, logger.warn (no spy setup in lines 354, 390, 404, 407-410)

These tests will fail immediately if re-enabled without significant rework.

Consider applying a similar approach to the comment block at lines 193-198—replace the test bodies with a brief comment:

-  describe.skip('loadCharacters', () => {
-    it('should load characters from comma-separated paths', async () => {
-      const char1 = { name: 'Character 1', id: 'char-1' };
-      ...
-    });
-    ...
-  });
+  /* SKIPPED: loadCharacters tests
+   * These require fs and logger mocking which causes test hangs in Bun test runner.
+   * To re-enable: import and mock fs module, set up logger spies, configure fs.readFileSync/promises mocks in beforeEach.
+   * File loading is validated through integration tests instead.
+   */
packages/server/src/__tests__/file-utils.test.ts (1)

164-167: Remove redundant it.skip.

This test is already skipped by the describe.skip on line 138.

-    it.skip('should handle file deletion errors', () => {
+    it('should handle file deletion errors', () => {
packages/server/src/__tests__/utils.test.ts (3)

5-5: Remove unused jest import.

The jest import is not used anywhere in this file after the mocking refactor.

Apply this diff:

-import { describe, it, expect, beforeEach, jest } from 'bun:test';
+import { describe, it, expect, beforeEach } from 'bun:test';

77-79: Consider removing empty beforeEach hook.

This hook is now empty and could be removed. The comment provides documentation value, but that could be captured in a test suite description instead.


124-141: Consolidate duplicate test cases.

These two tests verify nearly identical behavior (that calling resolvePgliteDir with an explicit path sets PGLITE_DATA_DIR). The second test (lines 133-141) adds little value beyond the first (lines 124-131).

Consider merging them or making the second test verify a distinct scenario, such as:

  • Explicit path overriding an existing PGLITE_DATA_DIR value
  • Behavior when the path doesn't exist
  • Normalization of the path before setting the env var
packages/server/src/__tests__/authMiddleware.test.ts (1)

29-33: Refine types for header access.

The as any cast on Line 32 bypasses type checking. Consider typing the header lookup properly using Express's IncomingHttpHeaders or a compatible type.

Apply this diff:

       get: jest.fn((name: string) => {
         // Express normalizes header names to lowercase
         const lowerName = name.toLowerCase();
-        return (mockRequest.headers as any)?.[lowerName];
+        return mockRequest.headers?.[lowerName as keyof typeof mockRequest.headers];
       }),
packages/server/src/__tests__/validation.test.ts (1)

149-157: Define a proper type for mockElizaOS.

Repeatedly using as any for mockElizaOS undermines type safety. Consider defining a mock interface at the file level.

Add a type definition at the top of the describe block:

type MockElizaOS = {
  getAgent: jest.Mock<(agentId: UUID) => IAgentRuntime | null>;
};

Then update the mock creation:

-      const mockRuntime = { id: 'test-runtime' } as unknown as IAgentRuntime;
-      const mockElizaOS = {
-        getAgent: jest.fn(() => mockRuntime),
-      } as any;
+      const mockRuntime: IAgentRuntime = { id: 'test-runtime' } as unknown as IAgentRuntime;
+      const mockElizaOS: MockElizaOS = {
+        getAgent: jest.fn(() => mockRuntime),
+      };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e087e7 and 2478695.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (23)
  • packages/server/src/__tests__/agent-plugin-reload.test.ts (1 hunks)
  • packages/server/src/__tests__/agent-server-database.test.ts (1 hunks)
  • packages/server/src/__tests__/agent-server-errors.test.ts (1 hunks)
  • packages/server/src/__tests__/agent-server-initialization.test.ts (1 hunks)
  • packages/server/src/__tests__/agent-server-management.test.ts (1 hunks)
  • packages/server/src/__tests__/agent-server-middleware.test.ts (1 hunks)
  • packages/server/src/__tests__/agents-runs.test.ts (2 hunks)
  • packages/server/src/__tests__/api.test.ts (1 hunks)
  • packages/server/src/__tests__/authMiddleware.test.ts (8 hunks)
  • packages/server/src/__tests__/basic-functionality.test.ts (2 hunks)
  • packages/server/src/__tests__/bootstrap-autoload.test.ts (5 hunks)
  • packages/server/src/__tests__/file-utils.test.ts (6 hunks)
  • packages/server/src/__tests__/integration/database-operations.test.ts (1 hunks)
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts (1 hunks)
  • packages/server/src/__tests__/loader.test.ts (6 hunks)
  • packages/server/src/__tests__/message-bus.test.ts (1 hunks)
  • packages/server/src/__tests__/middleware.test.ts (11 hunks)
  • packages/server/src/__tests__/simple-validation.test.ts (1 hunks)
  • packages/server/src/__tests__/socketio-router.test.ts (4 hunks)
  • packages/server/src/__tests__/ui-disable-feature.test.ts (1 hunks)
  • packages/server/src/__tests__/utils.test.ts (5 hunks)
  • packages/server/src/__tests__/validation.test.ts (6 hunks)
  • packages/server/src/api/system/__tests__/version.test.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: TypeScript for all code
Never use any, never, or unknown types - always opt for specific types that accurately represent the data
Ensure code is free of TypeScript errors or warnings - code must compile without issues

**/*.{ts,tsx}: Use TypeScript with ESM; prefer Bun APIs and avoid Node-only modules when Bun equivalents exist
Formatting must follow Prettier settings: 2 spaces, semicolons, single quotes, trailing comma es5, print width 100
Naming: use camelCase for variables and functions
Naming: use PascalCase for types and React components

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

Unit tests must use bun:test with standard primitives and test individual components in isolation

Unit test files must be named *.test.ts(x) and colocated near sources

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

packages/**/*.{ts,tsx}: Never use execa or Node child_process (execSync, spawnSync, exec, spawn); use Bun.spawn() or the bun-exec utilities instead
Prefer the existing bun-exec utility (packages/cli/src/utils/bun-exec.ts) for process execution
Do not use Node.js EventEmitter; always use Bun's EventTarget API
TypeScript only: provide proper specific types; never use any, never, or unknown in new code
Ensure code compiles without TypeScript errors or warnings
Use camelCase for variables and functions
Props interfaces use PascalCase and end with 'Props' (e.g., DashboardMenuProps)
File names should match the main export (e.g., DashboardMenu.tsx matches DashboardMenu)
Actions must return Promise and use callback() for user messages while returning ActionResult for chaining

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
packages/**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

In tests, do not use Node child_process APIs; use bun-exec utilities or Bun.spawn

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
packages/!(core)/**/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

In package code outside core, import @elizaos/core (not 'packages/core')

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
packages/**

📄 CodeRabbit inference engine (AGENTS.md)

Place all source code under packages/* in the monorepo

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
packages/server/**

📄 CodeRabbit inference engine (AGENTS.md)

packages/server implements the Express API; server-specific code should live here

Files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
  • packages/server/src/__tests__/integration/jobs-message-flow.test.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: wtfsayo
Repo: elizaOS/eliza PR: 0
File: :0-0
Timestamp: 2025-06-22T17:42:23.153Z
Learning: In the elizaOS/eliza repository, always use bun:test for testing instead of jest or vitest. Import testing utilities from 'bun:test' rather than 'jest/globals' or other testing frameworks.
📚 Learning: 2025-08-08T14:26:42.742Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-08T14:26:42.742Z
Learning: Applies to **/*.test.{ts,tsx} : Unit tests must use bun:test with standard primitives and test individual components in isolation

Applied to files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-08-08T14:26:42.742Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-08T14:26:42.742Z
Learning: Applies to **/e2e/**/*.{ts,tsx} : E2E tests must use the actual runtime and cannot use bun:test state

Applied to files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-08-28T20:11:19.074Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:11:19.074Z
Learning: Applies to packages/**/*.test.ts : In tests, do not use Node child_process APIs; use bun-exec utilities or Bun.spawn

Applied to files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-07-28T23:42:39.951Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/plugin-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:39.951Z
Learning: Applies to packages/plugin-starter/tests/**/*.{test,spec}.ts : Write tests for core functionality

Applied to files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/basic-functionality.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/integration/database-operations.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-06-22T17:42:23.153Z
Learnt from: wtfsayo
Repo: elizaOS/eliza PR: 0
File: :0-0
Timestamp: 2025-06-22T17:42:23.153Z
Learning: In the elizaOS/eliza repository, always use bun:test for testing instead of jest or vitest. Import testing utilities from 'bun:test' rather than 'jest/globals' or other testing frameworks.

Applied to files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-09-17T22:22:35.096Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Client testing: unit with Bun/Testing Library; e2e with Cypress (bun run cypress:open)

Applied to files:

  • packages/server/src/__tests__/agent-server-errors.test.ts
  • packages/server/src/__tests__/validation.test.ts
  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/agent-server-database.test.ts
  • packages/server/src/__tests__/ui-disable-feature.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/authMiddleware.test.ts
  • packages/server/src/__tests__/socketio-router.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/middleware.test.ts
  • packages/server/src/__tests__/agent-server-middleware.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-09-17T22:22:35.096Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript with ESM; prefer Bun APIs and avoid Node-only modules when Bun equivalents exist

Applied to files:

  • packages/server/src/__tests__/loader.test.ts
📚 Learning: 2025-08-28T20:11:19.074Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:11:19.074Z
Learning: Applies to **/package.json : Use bun:test exclusively; do not add jest, vitest, or mocha to dependencies/devDependencies

Applied to files:

  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/api/system/__tests__/version.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/simple-validation.test.ts
  • packages/server/src/__tests__/utils.test.ts
  • packages/server/src/__tests__/api.test.ts
📚 Learning: 2025-09-17T22:22:35.096Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Applies to **/*.test.{ts,tsx} : Unit test files must be named *.test.ts(x) and colocated near sources

Applied to files:

  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
📚 Learning: 2025-01-17T12:46:49.173Z
Learnt from: wtfsayo
Repo: elizaOS/eliza PR: 2389
File: packages/client-direct/src/api.ts:181-191
Timestamp: 2025-01-17T12:46:49.173Z
Learning: When implementing file storage in the character system, ensure the target directory exists using `fs.promises.mkdir` with `recursive: true` option, and use `elizaLogger` for proper error logging instead of console.log.

Applied to files:

  • packages/server/src/__tests__/loader.test.ts
  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/utils.test.ts
📚 Learning: 2025-07-28T23:42:53.140Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/project-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:53.140Z
Learning: Applies to packages/project-starter/characters/*.json : Store character variants as JSON files in the `characters/` directory (e.g., `production.json`, `development.json`, `specialized.json`)

Applied to files:

  • packages/server/src/__tests__/loader.test.ts
📚 Learning: 2025-07-28T23:42:53.140Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/project-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:53.140Z
Learning: Applies to packages/project-starter/src/characters.ts : Store multi-character management logic in `src/characters.ts`

Applied to files:

  • packages/server/src/__tests__/loader.test.ts
📚 Learning: 2025-07-28T23:42:39.951Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/plugin-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:39.951Z
Learning: Applies to packages/plugin-starter/src/services/*.ts : Service classes must implement proper initialization logic in an async initialize(runtime: IAgentRuntime) method

Applied to files:

  • packages/server/src/__tests__/message-bus.test.ts
  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-server-initialization.test.ts
📚 Learning: 2025-07-28T23:42:53.140Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/project-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:53.140Z
Learning: Applies to packages/project-starter/src/character.ts : Include required plugins `elizaos/plugin-bootstrap` and `elizaos/plugin-sql` in the agent's plugin configuration

Applied to files:

  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
📚 Learning: 2025-07-28T23:42:53.140Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/project-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:53.140Z
Learning: Applies to packages/project-starter/src/character.ts : Integrate custom plugins by importing them into `src/character.ts` and adding them to the `plugins` array

Applied to files:

  • packages/server/src/__tests__/bootstrap-autoload.test.ts
📚 Learning: 2025-07-28T23:42:39.951Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/plugin-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:39.951Z
Learning: Applies to packages/plugin-starter/src/index.ts : All services must be registered in the plugin's services array in src/index.ts

Applied to files:

  • packages/server/src/__tests__/bootstrap-autoload.test.ts
📚 Learning: 2025-07-28T23:42:39.951Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/plugin-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:39.951Z
Learning: Applies to packages/plugin-starter/src/index.ts : The main plugin export must follow the specified pattern in src/index.ts, exporting a Plugin object with services, actions, and optional providers/evaluators

Applied to files:

  • packages/server/src/__tests__/bootstrap-autoload.test.ts
📚 Learning: 2025-01-21T10:42:58.166Z
Learnt from: wtfsayo
Repo: elizaOS/eliza PR: 0
File: :0-0
Timestamp: 2025-01-21T10:42:58.166Z
Learning: New plugins must be imported in both `agent/package.json` (as a dependency) and `agent/src/index.ts` (for initialization) to be properly integrated into the system.

Applied to files:

  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
📚 Learning: 2025-01-21T10:42:58.166Z
Learnt from: wtfsayo
Repo: elizaOS/eliza PR: 0
File: :0-0
Timestamp: 2025-01-21T10:42:58.166Z
Learning: New plugins must be added to both `agent/package.json` (as a workspace dependency) and `agent/src/index.ts` (imported and initialized in the plugins array) to be properly integrated into the system.

Applied to files:

  • packages/server/src/__tests__/bootstrap-autoload.test.ts
  • packages/server/src/__tests__/agent-plugin-reload.test.ts
📚 Learning: 2025-07-28T23:42:53.140Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/project-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:53.140Z
Learning: Applies to packages/project-starter/**/package.json : Use `elizaos` CLI scripts in `package.json` for starting, developing, testing, and building the agent

Applied to files:

  • packages/server/src/__tests__/agents-runs.test.ts
  • packages/server/src/__tests__/agent-server-management.test.ts
📚 Learning: 2025-08-28T20:11:19.074Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:11:19.074Z
Learning: Applies to packages/**/*.{ts,tsx} : Prefer the existing bun-exec utility (packages/cli/src/utils/bun-exec.ts) for process execution

Applied to files:

  • packages/server/src/__tests__/file-utils.test.ts
  • packages/server/src/__tests__/utils.test.ts
📚 Learning: 2025-07-28T23:42:39.951Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/plugin-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:39.951Z
Learning: Applies to packages/plugin-starter/src/services/*.ts : Do not hardcode credentials; use environment variables for secrets and API keys

Applied to files:

  • packages/server/src/__tests__/utils.test.ts
📚 Learning: 2025-09-17T22:22:35.096Z
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Applies to packages/server/** : packages/server implements the Express API; server-specific code should live here

Applied to files:

  • packages/server/src/__tests__/api.test.ts
🔇 Additional comments (26)
packages/server/src/__tests__/message-bus.test.ts (1)

5-5: LGTM: Jest-compatible API from bun:test.

The jest import from 'bun:test' is correct—Bun provides jest-compatible mocking APIs, and this aligns with the project's requirement to use bun:test.

packages/server/src/__tests__/ui-disable-feature.test.ts (1)

5-5: Import aligns with PR objectives.

The jest import is used only for lightweight mock functions (jest.fn()), which is consistent with the PR's goal to replace heavy module mocking with minimal, targeted mocks.

packages/server/src/__tests__/simple-validation.test.ts (1)

9-136: Well-structured test suite.

The test organization is clear and comprehensive, covering edge cases and security considerations. The inline validation logic aligns with the PR's goal of simplifying tests and removing complex mocks.

packages/server/src/__tests__/basic-functionality.test.ts (1)

146-146: LGTM! Correct removal of unused-parameter prefix.

The parameter is actually used (line 147), so removing the underscore prefix is accurate.

packages/server/src/api/system/__tests__/version.test.ts (1)

10-13: Verify the test interference issue and track re-enablement.

The entire test suite is skipped, eliminating coverage for the version endpoint. While the comment explains port conflicts or state pollution during full test runs, ensure this is tracked for resolution so these tests can be re-enabled.

Consider opening a follow-up issue to investigate and resolve the underlying test interference, as these tests validate critical API functionality.

packages/server/src/__tests__/bootstrap-autoload.test.ts (2)

100-102: LGTM!

Correctly checks for both bootstrap plugin name variants when verifying the IGNORE_BOOTSTRAP behavior.


120-122: LGTM!

Correctly handles both bootstrap plugin name variants when verifying deduplication.

packages/server/src/__tests__/agent-server-database.test.ts (1)

5-5: Remove unused jest import from line 5.

The jest identifier is imported from bun:test but never referenced in the file. This aligns with the PR's shift away from mocking toward real implementations. Per coding guidelines, use bun:test exclusively without importing unused test utilities.

Update line 5 to:

import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
⛔ Skipped due to learnings
Learnt from: wtfsayo
Repo: elizaOS/eliza PR: 0
File: :0-0
Timestamp: 2025-06-22T17:42:23.153Z
Learning: In the elizaOS/eliza repository, always use bun:test for testing instead of jest or vitest. Import testing utilities from 'bun:test' rather than 'jest/globals' or other testing frameworks.
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:11:19.074Z
Learning: Applies to **/package.json : Use bun:test exclusively; do not add jest, vitest, or mocha to dependencies/devDependencies
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-08T14:26:42.742Z
Learning: Applies to **/*.test.{ts,tsx} : Unit tests must use bun:test with standard primitives and test individual components in isolation
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:11:19.074Z
Learning: Applies to packages/**/*.test.ts : In tests, do not use Node child_process APIs; use bun-exec utilities or Bun.spawn
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-08T14:26:42.742Z
Learning: Applies to **/e2e/**/*.{ts,tsx} : E2E tests must use the actual runtime and cannot use bun:test state
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: packages/plugin-starter/CLAUDE.md:0-0
Timestamp: 2025-07-28T23:42:39.951Z
Learning: Applies to packages/plugin-starter/tests/**/*.{test,spec}.ts : Write tests for core functionality
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Client testing: unit with Bun/Testing Library; e2e with Cypress (bun run cypress:open)
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Use Bun for all install/build/dev/test commands (e.g., bun install, bun run build, bun run dev, bun run test)
Learnt from: CR
Repo: elizaOS/eliza PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-17T22:22:35.096Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript with ESM; prefer Bun APIs and avoid Node-only modules when Bun equivalents exist
packages/server/src/__tests__/agents-runs.test.ts (1)

22-24: Ensure skipped tests are tracked and temporary.

The entire test suite is skipped due to test interference. While this aligns with the PR objective, skipped tests don't verify functionality and should only be a temporary measure.

Please confirm:

  1. Is there an issue tracking the root cause of the test interference?
  2. Is there a plan to unskip these tests once the interference is resolved?
packages/server/src/__tests__/agent-server-management.test.ts (1)

5-5: Remove unused jest import.

The jest object is imported but not used in the file. Remove it from line 5 to keep imports clean.

import { describe, it, expect, beforeEach, afterEach } from 'bun:test';

If you plan to use jest.spyOn() or similar utilities when adding test implementations, add the import back then.

packages/server/src/__tests__/integration/jobs-message-flow.test.ts (1)

109-239: The search confirmed that hardcoded setTimeout calls are a known anti-pattern for flaky tests. Now I need to inspect the actual test file to verify the conditions.

Based on hardcoded sleep statements in asynchronous tests are flaky because there's no guarantee that sleeping for N seconds will be enough for the system to process data, and given the codebase has a recent history of flaky test fixes, the remaining tests warrant closer inspection. Your original concern about test stability is valid.

Replace hardcoded setTimeout calls with deterministic waiting mechanisms; verify test stability.

Evaluate timing and synchronization by analyzing whether tests assume specific timing for operations, and introduce flexible wait conditions or timeouts to stabilize tests. The three tests (lines 109–239) all use fixed setTimeout delays (100ms, 200ms, 1500ms) which can cause non-deterministic behavior across different host systems depending on timer queue ordering and system load.

Additionally, recent CI failures blocking vitest→bun:test migration show integration tests timing out with SIGTERM across multiple platforms, with macOS failing after 8+ minutes. This suggests the test suite has timing reliability issues that should be resolved before these tests pass.

  • Use Bun's setSystemTime() mocking instead of real sleeps
  • Replace message bus timing assertions with polling/retry logic
  • Increase test timeouts if needed, or refactor to avoid timing-dependent expectations
packages/server/src/__tests__/loader.test.ts (4)

5-29: LGTM! Clean test setup with lightweight mocking.

The test setup follows best practices: imports from bun:test, uses a lightweight global fetch mock instead of heavy module mocking, and properly isolates tests via beforeEach.


33-101: LGTM! Comprehensive URL loading test coverage.

This suite thoroughly tests URL-based character loading with proper coverage of success paths, HTTP errors, invalid JSON, and network failures.


103-191: LGTM! Thorough character transformation tests.

This suite comprehensively tests jsonToCharacter including environment secret injection, merging behavior, auto-generated IDs, and settings preservation.


267-291: LGTM! Clean environment variable validation tests.

The hasValidRemoteUrls tests properly verify URL validation logic for HTTP/HTTPS URLs and edge cases.

packages/server/src/__tests__/integration/database-operations.test.ts (1)

12-12: Skip is justified—tests are well-structured despite bun:test isolation limitations.

The test file has comprehensive setup/teardown: unique DB paths, environment cleanup, random ports, and proper resource release delays. The "test interference" occurs when multiple test suites run together (a known bun:test runner limitation), not due to poor test isolation. This skip aligns with ElizaOS's active bun:test migration work.

packages/server/src/__tests__/file-utils.test.ts (3)

16-27: LGTM! Clean test setup.

The process.cwd mocking approach is lightweight and properly restores state after each test.


29-86: LGTM! Improved path assertions.

The switch from hardcoded absolute paths to structural checks (.toContain, .toEndWith) makes tests more portable.


94-95: Test expectations are correct; no issues found.

All sanitizeFilename test cases match the actual implementation behavior:

  • Null bytes are removed (line 95: 'test\0file.jpg' → 'testfile.jpg')
  • Consecutive dots normalize to single dot (line 109: '...test.jpg' → '.test.jpg'; line 111: '.. .test.jpg' → '. .test.jpg')
  • Empty/whitespace filenames default to 'unnamed' (lines 124-125)
packages/server/src/__tests__/socketio-router.test.ts (1)

408-408: Log level mapping verified as correct.

The comment at line 408 accurately reflects the logger configuration: customLevels.info = 30 is confirmed in packages/core/src/logger.ts:156.

packages/server/src/__tests__/api.test.ts (1)

1-10: api.test.ts doesn't need describe.skip — tests were reorganized, not deleted.

API endpoint coverage exists across specialized test files: health-endpoints.test.ts, agent-server-middleware.test.ts, socketio-router.test.ts, integration tests, and messaging API tests. The original api.test.ts was refactored into more maintainable, focused test suites covering the same surface area.

Likely an incorrect or invalid review comment.

packages/server/src/__tests__/middleware.test.ts (5)

108-143: LGTM!

The validateUuidMiddleware tests correctly use the logger spy and verify both valid UUID handling and security logging for invalid inputs.


145-183: LGTM!

The validateChannelIdMiddleware tests correctly verify missing, valid, and invalid channel ID scenarios with appropriate security logging.


185-249: LGTM!

The securityMiddleware tests comprehensively verify security headers and detection of suspicious patterns (User-Agent, path traversal, XSS, SQL injection) using the logger spy appropriately.


251-309: LGTM!

The validateContentTypeMiddleware tests correctly verify content type validation across different HTTP methods and edge cases.


311-323: LGTM!

The rate limiting tests appropriately verify that the middleware factory functions are defined and return functions.

Comment on lines 106 to 113
it('should use default directory when no options provided', () => {
const expected = path.join(process.cwd(), '.eliza', '.elizadb');

const result = resolvePgliteDir();

expect(result).toBe(expected);
// Should return some valid path
expect(result).toBeDefined();
expect(typeof result).toBe('string');
expect(result.length).toBeGreaterThan(0);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Test assertions are too generic to verify correctness.

This test now only confirms a non-empty string is returned, without verifying it's actually the correct default directory. Consider at least checking that the result is a valid absolute path or contains expected path components.

Example improvement:

-      // Should return some valid path
       expect(result).toBeDefined();
       expect(typeof result).toBe('string');
       expect(result.length).toBeGreaterThan(0);
+      expect(path.isAbsolute(result)).toBe(true);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/server/src/__tests__/utils.test.ts around lines 106 to 113, the test
only asserts a non-empty string and is too generic; update it to assert the
returned path is an absolute path (use Node's path.isAbsolute) and to verify
expected path components (for example that it ends with or contains the
configured default directory name, e.g. '.pglite' or the known default segment
used by resolvePgliteDir), so the test verifies both form and expected location
rather than just non-empty text.

Comment on lines +143 to 149
it('should handle environment file loading', () => {
// Environment file loading is handled internally
const result = resolvePgliteDir();

expect(dotenv.default.config).not.toHaveBeenCalled();
expect(result).toBeDefined();
expect(typeof result).toBe('string');
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Test provides insufficient verification.

This test only confirms the function returns a defined string, without verifying any behavior related to environment file loading. If environment file loading can't be tested without mocks, consider either:

  • Marking this test as it.skip with a TODO
  • Removing it entirely
  • Adding indirect verification (e.g., checking that specific env vars are present after the call)

Comment on lines 172 to 194
it('should handle empty string inputs', () => {
// Empty string gets passed through and expandTildePath returns it unchanged
const expected = '';

delete process.env.PGLITE_DATA_DIR;
delete process.env.ELIZA_DATABASE_DIR;
const result = resolvePgliteDir('');

expect(result).toBe(expected);
// Empty string is passed through expandTildePath, which returns it
// Then getDatabaseDir() is called since no dir is set
expect(result).toBeDefined();
expect(typeof result).toBe('string');
});

it('should handle null/undefined inputs', () => {
const expected = path.join(process.cwd(), '.eliza', '.elizadb');
it('should handle undefined inputs', () => {
delete process.env.PGLITE_DATA_DIR;
delete process.env.ELIZA_DATABASE_DIR;

const result1 = resolvePgliteDir(null as any);
const result2 = resolvePgliteDir(undefined);
const result = resolvePgliteDir(undefined);

expect(result1).toBe(expected);
expect(result2).toBe(expected);
// Should return default path from getDatabaseDir()
expect(result).toBeDefined();
expect(typeof result).toBe('string');
expect(result.length).toBeGreaterThan(0);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Strengthen edge case assertions.

Both edge case tests (empty string and undefined inputs) only verify that a string is returned, without confirming correct default/fallback behavior. Consider verifying:

  • The result matches expected fallback behavior
  • The result is a valid absolute path
  • Environment variables are set or unset as expected

Example improvement:

       const result = resolvePgliteDir('');
 
-      // Empty string is passed through expandTildePath, which returns it
-      // Then getDatabaseDir() is called since no dir is set
       expect(result).toBeDefined();
       expect(typeof result).toBe('string');
+      expect(path.isAbsolute(result)).toBe(true);
+      // Verify it falls back to default behavior
+      expect(result).toBe(resolvePgliteDir(undefined));
🤖 Prompt for AI Agents
In packages/server/src/__tests__/utils.test.ts around lines 172 to 194, the two
edge-case tests only assert that a string is returned; strengthen them to assert
correct fallback behavior by (1) explicitly clearing PGLITE_DATA_DIR and
ELIZA_DATABASE_DIR at test start, (2) for the undefined input test, compare the
result to the known default from getDatabaseDir() (or assert it starts with that
default prefix), and (3) for the empty-string input test, assert the returned
path is an absolute path (use path.isAbsolute) and not an empty string; also
assert environment vars remain unset during the test. Ensure you import/require
path and getDatabaseDir in the test file if not already present.

Comment on lines 33 to 52
beforeEach(async () => {
app = express();
app.use('/api/system/version', createVersionRouter());

// Find an available port
server = app.listen(0, () => {
port = server.address().port;
done();
// Use promise-based approach for better async handling
await new Promise<void>((resolve, reject) => {
server = app.listen(0, () => {
const address = server.address();
if (address && typeof address === 'object') {
port = address.port;
resolve();
} else {
reject(new Error('Failed to get server address'));
}
});

server.on('error', (err: any) => {
reject(err);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix type safety: avoid any type.

The async/await refactor improves readability and error handling. However, line 49 uses any type which violates the coding guidelines.

Apply this diff to use a proper type:

-      server.on('error', (err: any) => {
+      server.on('error', (err: Error) => {
         reject(err);
       });
📝 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.

Suggested change
beforeEach(async () => {
app = express();
app.use('/api/system/version', createVersionRouter());
// Find an available port
server = app.listen(0, () => {
port = server.address().port;
done();
// Use promise-based approach for better async handling
await new Promise<void>((resolve, reject) => {
server = app.listen(0, () => {
const address = server.address();
if (address && typeof address === 'object') {
port = address.port;
resolve();
} else {
reject(new Error('Failed to get server address'));
}
});
server.on('error', (err: any) => {
reject(err);
});
});
beforeEach(async () => {
app = express();
app.use('/api/system/version', createVersionRouter());
// Use promise-based approach for better async handling
await new Promise<void>((resolve, reject) => {
server = app.listen(0, () => {
const address = server.address();
if (address && typeof address === 'object') {
port = address.port;
resolve();
} else {
reject(new Error('Failed to get server address'));
}
});
server.on('error', (err: Error) => {
reject(err);
});
});

@odilitime odilitime changed the title fix: server tests fix: align server tests with ElizaOS API changes' Nov 6, 2025
@odilitime odilitime changed the title fix: align server tests with ElizaOS API changes' fix: align server tests with ElizaOS API changes Nov 6, 2025
@claude
Copy link
Contributor

claude bot commented Nov 6, 2025

Claude finished @odilitime's task —— View job


PR Review: Comprehensive Security & Code Quality Analysis

CRITICAL: Multiple type safety violations → Fix: Replace any types with specific interfaces

  • packages/server/src/__tests__/authMiddleware.test.ts:32 - Replace (mockRequest.headers as any) with proper typing
  • packages/server/src/__tests__/agents-runs.test.ts:34 - Replace server: any with proper Server type
  • packages/server/src/__tests__/ui-disable-feature.test.ts:74,97,119 - Replace function params any with specific interfaces
  • packages/server/src/api/system/__tests__/version.test.ts:49 - Replace err: any with err: Error

⚠️ IMPORTANT: Test coverage severely reduced by skipping → Fix: Add tracking issues for re-enabling

  • packages/server/src/__tests__/message-bus.test.ts:18 - MessageBusService completely untested
  • packages/server/src/__tests__/agents-runs.test.ts:32 - Agent runs API untested
  • packages/server/src/api/system/__tests__/version.test.ts:13 - Version endpoint untested
  • packages/server/src/__tests__/integration/ - Multiple integration suites skipped

⚠️ IMPORTANT: Missing test cleanup in middleware.test.ts → Fix: Add afterEach spy restoration

afterEach(() => {
  loggerWarnSpy?.mockRestore();
});

💡 SUGGESTION: Empty test files should be deleted → Consider: Remove scaffolding files

  • packages/server/src/__tests__/agent-server-errors.test.ts - Only imports, no tests
  • packages/server/src/__tests__/agent-server-initialization.test.ts - Only imports, no tests
  • packages/server/src/__tests__/api.test.ts - Only imports, no tests

💡 SUGGESTION: Unused import cleanup → Consider: Remove unused test imports

  • Remove jest from files only using describe/it/expect
  • Remove setSystemTime import from version.test.ts

✅ Security Assessment - PASSED

  • No hardcoded secrets/keys found in test files
  • Auth middleware tests properly validate security scenarios
  • Environment variable handling secure (test-only values)
  • No SQL injection or XSS vulnerabilities

✅ Framework Compliance - PASSED

  • All test files use bun:test (no jest/vitest/mocha frameworks)
  • Proper @elizaos/core imports (no packages/core usage found)

⚠️ Breaking Changes Assessment

  • Router API changes (SocketIORouter constructor signature) properly tested
  • Plugin auto-injection logic changes covered in bootstrap tests
  • Migration from Map-based to ElizaOS-based agent access tested

Recommendation: CONDITIONAL APPROVAL

The PR achieves its goal of eliminating heavy mocking and modernizing the test infrastructure. However, the extensive test skipping creates significant coverage gaps that should be tracked for resolution.


@claude
Copy link
Contributor

claude bot commented Nov 6, 2025

Claude finished @odilitime's task —— View job


PR Review: Server Tests Migration to ElizaOS API

Todo List:

  • Analyze key test files for security and type safety violations
  • Check test coverage impact from skipped tests
  • Verify bun:test usage and framework compliance
  • Review breaking changes and API migrations
  • Provide actionable feedback

❌ CRITICAL: Multiple type safety violations → Fix: Replace any types with specific interfaces

  • packages/server/src/__tests__/authMiddleware.test.ts:32 - Replace (mockRequest.headers as any) with proper typing
  • packages/server/src/__tests__/agents-runs.test.ts:9,14,22-23 - Replace function params any with specific interfaces
  • packages/server/src/__tests__/middleware.test.ts:26,71,86 - Replace spy variables and mockElizaOS any with proper types
  • packages/server/src/__tests__/message-bus.test.ts:15,112 - Replace mockFetch and mock.restore() with proper types

⚠️ IMPORTANT: Test coverage severely reduced by skipping → Fix: Add tracking issues for re-enabling

  • packages/server/src/__tests__/message-bus.test.ts:18 - MessageBusService completely untested
  • packages/server/src/__tests__/agents-runs.test.ts:32 - Agent runs API untested
  • packages/server/src/api/system/__tests__/version.test.ts:13 - Version endpoint untested
  • packages/server/src/__tests__/integration/ - Multiple integration suites skipped

⚠️ IMPORTANT: Missing test cleanup causes test pollution → Fix: Add proper spy restoration

  • packages/server/src/__tests__/middleware.test.ts - Missing afterEach(() => { loggerWarnSpy?.mockRestore(); })
  • packages/server/src/__tests__/message-bus.test.ts:112 - References undefined mock variable in restore

💡 SUGGESTION: Empty test files should be deleted → Consider: Remove scaffolding files

  • packages/server/src/__tests__/agent-server-errors.test.ts - Only imports, no tests
  • packages/server/src/__tests__/agent-server-initialization.test.ts - Only imports, no tests
  • packages/server/src/__tests__/api.test.ts - Only imports, no tests

💡 SUGGESTION: Unused import cleanup → Consider: Remove unused test imports

  • Remove jest from files only using describe/it/expect
  • Remove unused imports from multiple test files

✅ Security Assessment - PASSED

  • No hardcoded secrets/keys found in test files
  • Auth middleware tests properly validate security scenarios
  • Environment variable handling secure (test-only values)
  • No SQL injection or XSS vulnerabilities

✅ Framework Compliance - PASSED

  • All test files use bun:test (no jest/vitest/mocha frameworks)
  • Proper @elizaos/core imports (no packages/core usage found)
  • ElizaOS-based agent access pattern correctly implemented

⚠️ Breaking Changes Assessment

  • Router API changes (SocketIORouter constructor signature) properly tested
  • Plugin auto-injection logic changes covered in bootstrap tests
  • Migration from Map-based to ElizaOS-based agent access tested
  • Risk: Extensive test skipping creates coverage gaps

Recommendation: CONDITIONAL APPROVAL

The PR achieves its goal of eliminating heavy mocking and modernizing the test infrastructure. However, the extensive test skipping and type safety violations must be addressed.

Required fixes before merge:

  1. Replace all any types with specific interfaces
  2. Add proper test cleanup (spy restoration)
  3. Fix undefined mock variable reference
  4. Create tracking issues for skipped test suites

(p) => p.name === 'sql' || p.name === '@elizaos/plugin-sql'
);
expect(hasSQL).toBe(true);
expect(runtime.plugins.length).toBeGreaterThan(0);
Copy link

Choose a reason for hiding this comment

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

Bug: Bootstrap Index Validation Fails Test Reliability

In the test "should inject bootstrap before character plugins", the test checks if bootstrapIndex < openaiIndex only when openaiIndex !== -1, but it doesn't verify that bootstrapIndex !== -1 (i.e., that bootstrap plugin actually exists). If bootstrap plugin is not found, bootstrapIndex will be -1, and the condition -1 < openaiIndex will be true for any positive index, causing a false positive test pass. The test should also check bootstrapIndex !== -1 before making the comparison, or use an unconditional assertion that both plugins exist and bootstrap comes first.

Fix in Cursor Fix in Web

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