Skip to content

[Fix] Recover conversation migration state safely#816

Closed
dingyi222666 wants to merge 1 commit intoChatLunaLab:v1-devfrom
dingyi222666:fix/conversation-system-1
Closed

[Fix] Recover conversation migration state safely#816
dingyi222666 wants to merge 1 commit intoChatLunaLab:v1-devfrom
dingyi222666:fix/conversation-system-1

Conversation

@dingyi222666
Copy link
Copy Markdown
Member

This pr hardens the built-in conversation migration flow so startup recovery can safely handle an existing legacy sentinel, avoid re-reading already-purged legacy tables, and preserve current ChatLuna data.

New Features

  • None.

Bug fixes

  • Skip admin-only conversation management checks for personal routes.
  • Persist migration completion metadata before purging legacy tables so recovery can resume from a finished state.
  • Treat an existing legacy sentinel as startup recovery, including fresh-install and already-migrated data cases.
  • Recognize cannot resolve table as a missing-table error during legacy cleanup.
  • Add migration coverage for sentinel recovery and missing-table cleanup behavior.

Other Changes

  • Simplify conversation creation by removing the redundant manage-permission assertion from the service path.

Skip admin-only conversation checks for personal routes and persist migration completion metadata before legacy tables are purged. This lets startup recovery handle an existing legacy sentinel without re-reading missing tables or discarding existing ChatLuna data.

Tests: not run (not requested)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e3e76bee-1b0d-4da0-bdef-9ac9f0cf9eb5

📥 Commits

Reviewing files that changed from the base of the PR and between b6082d4 and 97b451a.

📒 Files selected for processing (7)
  • packages/core/src/middlewares/system/conversation_manage.ts
  • packages/core/src/middlewares/system/wipe.ts
  • packages/core/src/migration/legacy_tables.ts
  • packages/core/src/migration/room_to_conversation.ts
  • packages/core/src/migration/validators.ts
  • packages/core/src/services/conversation.ts
  • packages/core/tests/conversation-migration.spec.ts
💤 Files with no reviewable changes (1)
  • packages/core/src/services/conversation.ts

Walkthrough

该PR改进了ChatLuna的房间到对话迁移流程,引入了sentinel哨兵机制支持迁移恢复,优化了权限检查逻辑,并增强了数据库清理与迁移验证功能。

Changes

Cohort / File(s) Summary
迁移流程核心逻辑
packages/core/src/migration/room_to_conversation.ts, packages/core/src/migration/validators.ts
添加sentinel哨兵检查用于迁移恢复;引入createPassedValidationResult()助手函数;重构迁移完成流程,将"done标志"写入提前至清理前;新增BUILTIN_SCHEMA_VERSION常量与writeMigrationDone()助手。
数据库与错误处理
packages/core/src/migration/legacy_tables.ts, packages/core/src/middlewares/system/wipe.ts
扩展表丢失错误检测以捕获"cannot resolve table"错误;完善wipe流程添加chatluna_meta清理和迁移元数据写入(schema版本、验证结果、完成标志、时间戳)。
权限与会话管理
packages/core/src/middlewares/system/conversation_manage.ts, packages/core/src/services/conversation.ts
紧化admin权限守卫,仅在非personal路由模式下要求管理员权限;移除ensureActiveConversation中冗余的管理权限验证。
迁移测试覆盖
packages/core/tests/conversation-migration.spec.ts
新增sentinel恢复场景测试、不完整迁移元数据处理测试、表存在性检查测试,验证迁移验证与完成状态更新。

Sequence Diagram(s)

sequenceDiagram
    participant App as 应用启动
    participant FS as 文件系统
    participant Migrator as 迁移器
    participant DB as 数据库
    participant Meta as 迁移元数据
    
    App->>FS: 检查sentinel文件存在?
    alt Sentinel存在
        FS-->>Migrator: sentinel已存在
        Migrator->>Meta: 检查验证结果是否已通过
        alt 验证已通过
            Migrator->>Meta: 更新schema_version和完成时间
            Migrator-->>App: 恢复完成,跳过迁移
        else 验证未通过
            Migrator->>DB: 探测ChatLuna数据存在
            Migrator->>Meta: 记录通过的验证结果
            Migrator->>Meta: 设置完成标志和时间戳
            Migrator-->>App: 迁移验证完成
        end
    else Sentinel不存在
        FS-->>Migrator: 需执行完整迁移
        Migrator->>DB: 从room/message执行迁移
        Migrator->>Meta: 写入完成标志
        Migrator->>DB: 清理遗留表
        Migrator->>Meta: 记录迁移完成时间
        Migrator->>FS: 创建sentinel文件
        Migrator-->>App: 迁移完全完成
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 迁移的森林有了新灯塔,
Sentinel守卫着恢复的路,
权限的栅栏更加精妙,
数据的清理变得有序,
ChatLuna的家园焕然一新~ ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确总结了PR的核心变化:加强对话迁移流程以安全处理启动恢复,与代码变更直接相关。
Description check ✅ Passed 描述详细列举了所有主要变更(跳过管理员检查、迁移元数据持久化、处理legacy sentinel、识别表不存在错误等),与代码变更紧密相关。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the migration logic from legacy rooms to conversations by introducing a sentinel check and recovery mechanism to handle interrupted migrations or existing data. Key updates include refactoring metadata persistence, improving error detection for missing tables, and refining access control logic. Feedback was provided to parallelize database queries during the migration validation process to improve performance.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97b451a961

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +49 to +50
if (existsSync(getLegacySchemaSentinel(ctx.baseDir))) {
return ensureMigrationValidated(ctx, config)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid skipping migration on sentinel alone

This early return makes the sentinel file authoritative even when legacy chathub_* tables still contain data, so a stale sentinel (for example after restoring an old DB while keeping data/chatluna/temp/legacy-schema-disabled.json) will bypass migration entirely. In that case ensureMigrationValidated() marks validation as passed and writes migration-done metadata without reading legacy rows, leaving legacy conversations/messages unmigrated and effectively lost to ChatLuna.

Useful? React with 👍 / 👎.

@dingyi222666
Copy link
Copy Markdown
Member Author

Superseded by #817

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.

1 participant