fix: cli create command directory display and cleanup on interruption #5321
+279
−59
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRobust cleanup logic was added to the project and plugin creation commands, ensuring incomplete directories are removed if the process is interrupted or fails. A utility for user-friendly directory display was introduced, dependency installation was refactored for better signal handling, and a test verifies cleanup on interruption. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant OS
User->>CLI: Run create command
CLI->>CLI: Call withCleanupOnInterrupt
CLI->>CLI: Start project/plugin creation
CLI->>CLI: Install dependencies (execa bun install)
OS-->>CLI: (Possible SIGINT/SIGTERM)
alt Interrupted or Error
CLI->>CLI: Cleanup handler deletes target directory
else Success
CLI->>CLI: Complete creation
end
CLI->>User: Show result
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate Unit 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
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. |
|
@coderabbitai full review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/cli/src/commands/create/actions/creators.ts(8 hunks)packages/cli/src/commands/create/actions/setup.ts(3 hunks)packages/cli/src/utils/build-project.ts(2 hunks)packages/cli/src/utils/helpers.ts(1 hunks)packages/cli/tests/commands/create.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: wtfsayo
PR: elizaOS/eliza#0
File: :0-0
Timestamp: 2025-01-18T14:10:19.334Z
Learning: PR #2485 includes significant changes beyond the initially described chat-bubble and version.sh updates, including a new plugin-di package, Solana plugin updates, and i18n improvements.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Ensure proper plugin structure by exporting a Plugin object with name, version, actions, providers, evaluators, and services.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{test,spec}.{ts,tsx} : Create comprehensive unit and integration tests: test all actions, providers, evaluators, service initialization and lifecycle, event handling, memory operations, and error handling.
packages/cli/src/utils/helpers.ts (5)
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.{ts,tsx} : Descriptive variable and function names
Learnt from: wtfsayo
PR: elizaOS/eliza#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.
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to packages/{cli,client,app,server,api-client,plugin-bootstrap,plugin-sql,plugin-starter,project-starter,project-tee-starter,autodoc,docs,create-eliza}/src/**/*.{ts,tsx} : Use `@elizaos/core` for imports in package code, and `packages/core` for internal references
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : All @elizaos imports must be updated to use the new paths (e.g., import { logger } from '@elizaos/core'; instead of import { elizaLogger } from '@elizaos/core';).
Learnt from: wtfsayo
PR: elizaOS/eliza#0
File: :0-0
Timestamp: 2025-01-16T10:45:27.955Z
Learning: The AgentKit plugin for Eliza supports Base Sepolia network and provides integration with CDP AgentKit tools for managing NFTs and tokens. The implementation includes proper error handling, wallet data persistence, and comprehensive context templates for natural language interactions.
packages/cli/src/utils/build-project.ts (15)
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.{ts,tsx} : Don't comment change notes
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.{ts,tsx} : Comment complex logic, but do not comment change notes
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.{ts,tsx} : Never omit code or add an "// ..." as it risks breaking the codebase
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.{ts,tsx} : Never omit code or add '// ...' as it risks breaking the codebase
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.{ts,tsx} : Comment complex logic
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Ensure proper plugin structure by exporting a Plugin object with name, version, actions, providers, evaluators, and services.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Update all type references: Account → Entity, userId → entityId, room → world (agent-side abstraction only), IAgentRuntime → AgentRuntime.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/package.json : Update package.json dependencies to use '@elizaos/core' version ^1.0.0 and update scripts to use vitest and tsup.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Implement proper event emission and handling using this.runtime.emit and this.runtime.on.
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to packages/cli/src/index.ts : CLI entry point must be in `packages/cli/src/index.ts`
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Do not use the `elizaos` CLI within the `eliza` monorepo itself; use `bun` commands for monorepo development
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.test.{ts,tsx} : Use Bun's built-in test runner for all tests
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: NEVER USE `npm` OR `pnpm` for package management or script execution; ALWAYS USE `bun`
Learnt from: wtfsayo
PR: elizaOS/eliza#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
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/!(*.e2e).test.{ts,tsx} : Unit tests must use Bun test with standard primitives
packages/cli/tests/commands/create.test.ts (10)
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{test,spec}.{ts,tsx} : Create comprehensive unit and integration tests: test all actions, providers, evaluators, service initialization and lifecycle, event handling, memory operations, and error handling.
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.test.{ts,tsx} : Verify tests are complete and passing before declaring changes correct
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Ensure proper plugin structure by exporting a Plugin object with name, version, actions, providers, evaluators, and services.
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.{ts,tsx} : Work on files until they are perfect, looping testing and fixing until all tests pass
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/{unit,component}/**/*.test.{ts,tsx} : Unit tests use vitest with standard primitives and test individual components in isolation
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/!(*.e2e).test.{ts,tsx} : Unit tests must use Bun test with standard primitives
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.test.{ts,tsx} : Use Bun's built-in test runner for all tests
Learnt from: simsaidev
PR: elizaOS/eliza#2614
File: packages/client-simsai/src/index.ts:0-0
Timestamp: 2025-01-21T17:38:50.416Z
Learning: Stop methods in clients should include comprehensive error handling with try-catch blocks, proper cleanup (e.g., calling stop on all sub-components), error logging, and success confirmation logs. Example pattern:
```typescript
try {
await cleanup1();
await cleanup2();
log.info("Cleanup successful");
} catch (error) {
log.error("Cleanup failed:", error);
throw error;
}
```
Learnt from: wtfsayo
PR: elizaOS/eliza#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
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.e2e.test.{ts,tsx} : E2E tests must use actual runtime with real integrations
packages/cli/src/commands/create/actions/setup.ts (14)
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to packages/cli/src/index.ts : CLI entry point must be in `packages/cli/src/index.ts`
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : All @elizaos imports must be updated to use the new paths (e.g., import { logger } from '@elizaos/core'; instead of import { elizaLogger } from '@elizaos/core';).
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Implement proper event emission and handling using this.runtime.emit and this.runtime.on.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/package.json : Update package.json dependencies to use '@elizaos/core' version ^1.0.0 and update scripts to use vitest and tsup.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Convert generateText calls to runtime.useModel with the new message-based API.
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Do not use the `elizaos` CLI within the `eliza` monorepo itself; use `bun` commands for monorepo development
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Update all type references: Account → Entity, userId → entityId, room → world (agent-side abstraction only), IAgentRuntime → AgentRuntime.
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to packages/{cli,client,app,server,api-client,plugin-bootstrap,plugin-sql,plugin-starter,project-starter,project-tee-starter,autodoc,docs,create-eliza}/src/**/*.{ts,tsx} : Use `@elizaos/core` for imports in package code, and `packages/core` for internal references
Learnt from: wtfsayo
PR: elizaOS/eliza#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
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Ensure proper plugin structure by exporting a Plugin object with name, version, actions, providers, evaluators, and services.
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: NEVER USE `npm` OR `pnpm` for package management or script execution; ALWAYS USE `bun`
Learnt from: wtfsayo
PR: elizaOS/eliza#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.
Learnt from: yungalgo
PR: elizaOS/eliza#4813
File: packages/project-starter/package.json:9-9
Timestamp: 2025-05-28T16:15:33.419Z
Learning: Project starter packages like @elizaos/project-starter are templates that users copy rather than install as dependencies, so minor path inconsistencies in package.json entry points are less critical than for published packages.
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.test.{ts,tsx} : Use Bun's built-in test runner for all tests
packages/cli/src/commands/create/actions/creators.ts (13)
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Ensure proper plugin structure by exporting a Plugin object with name, version, actions, providers, evaluators, and services.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{test,spec}.{ts,tsx} : Create comprehensive unit and integration tests: test all actions, providers, evaluators, service initialization and lifecycle, event handling, memory operations, and error handling.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Convert generateText calls to runtime.useModel with the new message-based API.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Services must now extend the base Service class from '@elizaos/core' and implement lifecycle methods: initialize, start, and stop.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : All @elizaos imports must be updated to use the new paths (e.g., import { logger } from '@elizaos/core'; instead of import { elizaLogger } from '@elizaos/core';).
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Implement proper event emission and handling using this.runtime.emit and this.runtime.on.
Learnt from: CR
PR: elizaOS/eliza#0
File: packages/cli/src/utils/upgrade/CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:49.972Z
Learning: Applies to packages/cli/src/utils/upgrade/**/*.{ts,tsx} : Update memory operations to use the new API with table names (use runtime.memory.create with entityId, worldId, content, and tableName).
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to packages/cli/src/index.ts : CLI entry point must be in `packages/cli/src/index.ts`
Learnt from: CR
PR: elizaOS/eliza#0
File: .cursorrules:0-0
Timestamp: 2025-06-30T09:27:56.759Z
Learning: Applies to **/*.{ts,tsx} : Comprehensive error handling required
Learnt from: CR
PR: elizaOS/eliza#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T09:28:32.208Z
Learning: Applies to **/*.{ts,tsx} : Comprehensive error handling required
Learnt from: wtfsayo
PR: elizaOS/eliza#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.
Learnt from: wtfsayo
PR: elizaOS/eliza#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.
Learnt from: yungalgo
PR: elizaOS/eliza#4813
File: packages/project-starter/package.json:9-9
Timestamp: 2025-05-28T16:15:33.419Z
Learning: Project starter packages like @elizaos/project-starter are templates that users copy rather than install as dependencies, so minor path inconsistencies in package.json entry points are less critical than for published packages.
🧬 Code Graph Analysis (2)
packages/cli/src/utils/build-project.ts (1)
packages/cli/src/utils/run-bun.ts (1)
runBunCommand(10-44)
packages/cli/tests/commands/create.test.ts (2)
packages/cli/tests/commands/test-utils.ts (1)
crossPlatform(503-529)packages/cli/tests/test-timeouts.ts (1)
TEST_TIMEOUTS(12-106)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: databaseIntegrationTests
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: test (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/cli/src/commands/create/actions/setup.ts (1)
340-351: Good subprocess handling with proper SIGINT support!The direct use of
execawithreject: falseand explicit exit code handling is the right approach for graceful interruption support.packages/cli/src/commands/create/actions/creators.ts (2)
295-301: Smart handling of current directory case!Good design decision to skip cleanup wrapper for current directory to prevent accidental deletion.
23-33: Add existence check to prevent unnecessary cleanup attemptsThe cleanup function should verify the directory exists before attempting removal to avoid errors.
const cleanup = () => { - if (directoryCreated && existsSync(targetDir)) { + // Only cleanup if we created it and it still exists + if (directoryCreated && existsSync(targetDir)) { console.info(colors.red(`\n\nInterrupted! Cleaning up ${displayName}...`)); try { rmSync(targetDir, { recursive: true, force: true }); console.info('Cleanup completed.'); } catch (error) { - console.error(colors.red('Error during cleanup:'), error); + // Only log if directory still exists (could be race condition) + if (existsSync(targetDir)) { + console.error(colors.red('Error during cleanup:'), error); + } } } };⛔ Skipped due to learnings
Learnt from: simsaidev PR: elizaOS/eliza#2614 File: packages/client-simsai/src/index.ts:0-0 Timestamp: 2025-01-21T17:38:50.416Z Learning: Stop methods in clients should include comprehensive error handling with try-catch blocks, proper cleanup (e.g., calling stop on all sub-components), error logging, and success confirmation logs. Example pattern: ```typescript try { await cleanup1(); await cleanup2(); log.info("Cleanup successful"); } catch (error) { log.error("Cleanup failed:", error); throw error; } ```Learnt from: wtfsayo PR: elizaOS/eliza#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.
yungalgo
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR improves the elizaos create command by displaying user-friendly directory names and automatically cleaning up partially created directories when interrupted.
- Introduces
getDisplayDirectory()to simplify prompts by showing only the directory basename. - Adds
withCleanupOnInterrupt()wrapper to handle SIGINT/SIGTERM and remove incomplete directories. - Updates install and build flows to delegate cleanup and adjusts tests to cover interruption scenarios.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/tests/commands/create.test.ts | Added a test case to verify directory cleanup on SIGINT interrupt |
| packages/cli/src/utils/helpers.ts | Implemented getDisplayDirectory() helper for prompt messages |
| packages/cli/src/utils/build-project.ts | Updated comments to note cleanup is handled upstream |
| packages/cli/src/commands/create/actions/setup.ts | Switched installDependencies to execa and removed local cleanup |
| packages/cli/src/commands/create/actions/creators.ts | Added withCleanupOnInterrupt and hooked it into all create flows |
Comments suppressed due to low confidence (3)
packages/cli/src/utils/helpers.ts:8
- The doc comment specifically mentions mapping "." to "Desktop", but the implementation returns
path.basename(process.cwd()). Update the comment to reflect that it returns the current working directory's base name rather than always "Desktop".
* converts paths like "." to "Desktop" or "/Users/me/Desktop" to "Desktop"
packages/cli/src/commands/create/actions/creators.ts:301
- [nitpick] The
displayNameparameter is passed asprojectName, which may not clearly represent the directory being cleaned up. Consider passing a more descriptive label (e.g., the directory basename ordisplayProjectName) for clearer cleanup messages.
await withCleanupOnInterrupt(projectTargetDir, projectName, createFn);
packages/cli/tests/commands/create.test.ts:356
- Using fixed
setTimeoutdelays can lead to flaky tests if the process startup time varies. Consider polling for the directory or listening to process output/events to determine when to send SIGINT.
await new Promise((resolve) => setTimeout(resolve, 1500));
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@wtfsayo fixed the bugbot nitpick |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…type-text fix: (cli) show correct type in create command messages
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
bugbot run |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix CLI create command directory display and cleanup on interruption
Problem
Two minor bugs with the
elizaos createcommand:Confusing directory display: When creating a project/plugin, the confirmation prompt showed
"Create plugin 'testing' in plugin-testing?"instead of the actual directory name (e.g.,"Create 'plugin-testing' in Desktop?")Abandoned directories on Ctrl-C: Pressing Ctrl-C during
bun installduring a plugin or project installation left partially created directories behind, requiring manual cleanupSolution
Directory display: Added
getDisplayDirectory()helper that gives the current directory name and simplifies absolute paths to just the directory nameCleanup on interruption: Implemented
withCleanupOnInterrupt()wrapper that:Details
Implementation
bun install) receive signals directlyinstallDependenciesandbuildProjectfunctions since cleanup is now handled at a higher levelFiles Changed
src/commands/create/actions/creators.ts- Added cleanup wrapper and updated all create functionssrc/commands/create/actions/setup.ts- Simplified after moving cleanup logic upsrc/utils/helpers.ts- AddedgetDisplayDirectory()helpertests/commands/create.test.ts- Added test for cleanup behaviorTesting
Run the full test suite:
bun test tests/commands/create.test.tsManual test:
Summary by CodeRabbit
New Features
Bug Fixes
Tests