Skip to content

Conversation

@shuhuiluo
Copy link
Collaborator

@shuhuiluo shuhuiluo commented Nov 25, 2025

Convert dbReady auto-running promise to explicit runMigrations() function. Prevents migrations from running during tests that import the db module.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Refactored database initialization process to ensure migrations execute reliably at startup with guaranteed single execution.

✏️ Tip: You can customize this high-level summary in your review settings.

Convert dbReady auto-running promise to explicit runMigrations()
function. Prevents migrations from running during tests that import
the db module.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

The pull request refactors the database migration initialization pattern. The dbReady export—a constant that executed migrations upon module import—is replaced with an explicit runMigrations() function. This function uses a private module-scoped variable to ensure migrations execute only once, regardless of how many times the function is invoked. The main entry point in src/index.ts is updated to import and explicitly call await runMigrations() during startup instead of awaiting the previous dbReady constant. Error handling for migrations is preserved through logging and rethrowing within the guarded execution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The changes are localized to two files with a straightforward refactoring pattern
  • Logic remains consistent; the shift is from auto-executing on import to explicit function invocation
  • Homogeneous changes across files reduce complexity
  • Key review areas:
    • Verify the idempotent guard (private migrationPromise variable) correctly prevents multiple migrations
    • Confirm error handling paths are equivalent to the previous implementation
    • Ensure the initialization sequence in src/index.ts properly awaits migrations before database operations

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: converting auto-running migrations on module import to explicit on-demand execution at app startup.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 fix/lazy-db-migrations

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0fa0f8c and de33676.

📒 Files selected for processing (2)
  • src/db/index.ts (1 hunks)
  • src/index.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use <@{userId}> for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)

Files:

  • src/db/index.ts
  • src/index.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)

Files:

  • src/db/index.ts
  • src/index.ts
🧬 Code graph analysis (1)
src/index.ts (1)
src/db/index.ts (1)
  • runMigrations (69-79)
🔇 Additional comments (2)
src/db/index.ts (1)

63-79: Lazy, single-flight migration orchestration looks correct

The migrationPromise guard plus runMigrations() API cleanly removes import-time side effects while ensuring migrations only run once per process. Logging and rethrowing in the .catch keeps failures visible without swallowing errors. Note that if migrations fail in a given process, later calls will keep returning the same rejected promise (i.e., no in-process retry); that’s usually fine for startup-style code where you fix the issue and restart the app, but worth keeping in mind if you ever plan to call runMigrations() from other code paths.

src/index.ts (1)

8-8: Explicit startup migration call aligns with the new DB API

Importing runMigrations and awaiting it at the top of the entrypoint preserves the old “migrate on startup” behavior while removing the side effect from the ./db module itself. This keeps tests free to import db without triggering migrations, and the startup sequence (migrate → log → init bot/services) remains clear and ordered.

Also applies to: 24-25


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

@shuhuiluo shuhuiluo merged commit ddaeb65 into main Nov 25, 2025
2 checks passed
@shuhuiluo shuhuiluo deleted the fix/lazy-db-migrations branch November 25, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants