feat(core): replace room system with conversation runtime#805
Open
dingyi222666 wants to merge 20 commits intov1-devfrom
Open
feat(core): replace room system with conversation runtime#805dingyi222666 wants to merge 20 commits intov1-devfrom
dingyi222666 wants to merge 20 commits intov1-devfrom
Conversation
Migrate legacy room data into a conversation runtime with archive support, and update chat, memory, and adapter flows to resolve conversations directly.
…structure - Rewrite ChatChainDependencyGraph with lifecycle slot-based scheduling, replacing topological sort with a rule-based dependency engine that supports before/after lifecycle anchors and produces better error messages - Flatten command hierarchy: move chatluna.chat.* sub-commands to chatluna.* (rollback, stop, voice) and clean up preset option choices - Add autoSummarizeTitle to ChatInterface for automatic title generation after the first exchange - Refactor conversation service: simplify API, reduce complexity across conversation_runtime, middlewares, and conversation_manage - Improve room-to-conversation migration validators and legacy table handling - Add inputImageTokens/outputImageTokens tracking to adapter-gemini - Extensive i18n updates for en-US and zh-CN locales
Contributor
Walkthrough本次 PR 将系统从“房间(room)”中心迁移为“会话(conversation)”中心：新增 ConversationService/ConversationRuntime、会话路由与约束、迁移/验证/归档/恢复与清理流程；删除大量 room 相关中间件/命令；调整中间件生命周期为 request_conversation；并在适配器与使用元数据上做兼容性扩展。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReadMsg as read_chat_message
participant Resolve as resolve_conversation
participant ConvSvc as ConversationService
participant ConvRuntime as ConversationRuntime
participant ChatIface as ChatInterface
participant DB as Database
User->>ReadMsg: 发送消息
ReadMsg->>Resolve: 解析 preset/presetLane，设置 context.options
Resolve->>ConvSvc: resolveContext / ensureActiveConversation(conversationId/presetLane)
ConvSvc-->>Resolve: 返回 resolvedConversation 与 resolvedContext
Resolve-->>ReadMsg: 填充 context.options.conversationId/resolvedConversationContext
ReadMsg->>ConvRuntime: 调用 chat(session, conversation, message, ...)
ConvRuntime->>ChatIface: 创建或复用 ChatInterface，获取锁并执行请求
ChatIface-->>ConvRuntime: 返回 AI 响应与 usage
ConvRuntime->>DB: 更新 latestMessageId、记录使用与压缩结果（如有）
ConvRuntime-->>User: 返回生成消息（含 usage 元数据）
sequenceDiagram
participant Admin
participant ConvManage as conversation_manage
participant ConvSvc as ConversationService
participant FS as Filesystem
participant DB as Database
participant Runtime as ConversationRuntime
Admin->>ConvManage: 发起 archive/export 请求
ConvManage->>ConvSvc: 权限校验并调用 exportConversation/archiveConversation
ConvSvc->>FS: 写 conversation.md 与 messages.jsonl.gz
ConvSvc->>FS: 写 manifest（含 SHA-256）
ConvSvc->>DB: upsert chatluna_archive，更新 conversation.status=archived
ConvSvc->>Runtime: 清除运行时接口/缓存
ConvSvc-->>ConvManage: 返回 success / archiveId
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
dingyi222666
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the core architecture by replacing the legacy room system with a comprehensive conversation management framework, including new services for runtime handling, ACL, and data migration. The update introduces a rule-based dependency graph for middlewares and adds image token tracking for the Gemini adapter. Technical feedback highlights critical performance and reliability concerns, such as potential Out-Of-Memory errors during message migration, resource leaks in the interface cache, and race conditions in sequence allocation. Additionally, the reviewer recommends optimizing several database operations to avoid inefficient full table scans and suggests using more reliable utility functions for authority checks.
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a major architectural refactor, replacing the legacy room-based system with a new conversation-based model, including a comprehensive migration path for existing data. Key changes involve the introduction of ConversationService and ConversationRuntime to manage lifecycles, ACL constraints, and archiving, as well as updates to model adapters for enhanced token tracking. Feedback primarily addresses critical performance risks, specifically identifying multiple scenarios where loading entire database tables into memory could lead to Out-Of-Memory errors during migration and cleanup. The reviewer also highlighted a race condition in conversation sequence allocation and recommended using filtered database queries to improve scalability and data consistency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5abb2b437a
ℹ️ 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".
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a major architectural shift by replacing the legacy 'room' concept with a more robust 'conversation' system, including a comprehensive migration path and a new ACL-based permission model. The changes span across the core requester logic, command structures, and memory management, while also adding support for image tokens in the Gemini adapter. Key feedback focuses on optimizing database operations to avoid full table scans in the ConversationService, addressing code duplication in command utilities, and ensuring that cross-route access via ACLs is not inadvertently blocked by strict route checks. Additionally, the migration validation logic should be reviewed for memory efficiency when handling large datasets.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/core/src/middlewares/model/set_default_embeddings.ts (1)
34-56:⚠️ Potential issue | 🔴 Critical缺少 return 语句导致运行时错误。
当
targetEmbeddings.length > 1或=== 0时，代码设置了错误消息但没有返回，导致继续执行到第 56 行访问targetEmbeddings[0]。当没有找到匹配的 embeddings 时，这将导致 undefined 错误。对比
set_default_vectorstore.ts的正确实现，该文件在第 45 和 48 行正确返回了ChainMiddlewareRunStatus.STOP。🐛 建议修复
if (targetEmbeddings.length > 1) { const buffer: string[] = [] buffer.push(session.text('.multiple_models_found.header')) for (const embedding of targetEmbeddings) { buffer.push(embedding.toModelName()) } buffer.push(session.text('.multiple_models_found.footer')) buffer.push( session.text('.multiple_models_found.example', [ targetEmbeddings[0] ]) ) context.message = buffer.join('\n') + return ChainMiddlewareRunStatus.STOP } else if (targetEmbeddings.length === 0) { context.message = session.text('.model_not_found') + return ChainMiddlewareRunStatus.STOP } const fullName = targetEmbeddings[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/model/set_default_embeddings.ts` around lines 34 - 56, The handler in set_default_embeddings.ts sets context.message when targetEmbeddings.length > 1 or === 0 but fails to stop execution, leading to accessing targetEmbeddings[0] when undefined; modify the branches that set context.message to return ChainMiddlewareRunStatus.STOP (the same enum used in set_default_vectorstore.ts) immediately after setting the message so execution does not proceed to the line that reads targetEmbeddings[0]; locate the conditional blocks around targetEmbeddings, context.message, and add the return statements there.packages/extension-long-memory/src/plugins/edit_memory.ts (1)
44-58:⚠️ Potential issue | 🟡 Minor先解析 memory scope，再提示用户输入。
getMemoryScope()失败是可以在交互前确定的，但当前顺序会先session.prompt()，最后再用.edit_failed结束。这样用户输入会被白白消耗掉，体验上像“已经开始编辑但最终失败”。把 scope 解析提前即可避免这类空提示。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-long-memory/src/plugins/edit_memory.ts` around lines 44 - 58, The current flow prompts the user (session.prompt) before resolving the memory scope, so if getMemoryScope(ctx, session, {conversationId, presetLane, type}) returns null you end up consuming a user input and then sending .edit_failed; change the order by calling getMemoryScope(...) immediately after session.send(session.text('.edit_memory_start')) (or even before sending the prompt), check for null and return ChainMiddlewareRunStatus.STOP with context.message = session.text('.edit_failed') before calling session.prompt(), so the prompt is only shown when the scope is valid.packages/core/src/llm-core/chat/app.ts (1)
45-57:⚠️ Potential issue | 🟠 Major
dispose()现在不会在上下文释放时自动执行。构造函数里已经没有把它挂到
ctx.on('dispose')之类的生命周期钩子上了，所以这里新增的清理逻辑只能依赖外部调用者手动触发。一旦ChatInterface所属上下文直接卸载，链路/记忆相关对象就不会被清理，这是这次重构引入的生命周期回归。建议修改
constructor( public ctx: Context, input: ChatInterfaceInput ) { this._input = input + ctx.on('dispose', () => this.dispose()) }Based on learnings, 在 ChatInterface 类中，响应式 watch 调用通过
ctx.effect()包装来自动处理清理工作，并添加ctx.on('dispose')处理器提供额外的清理保障，避免内存泄漏。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/chat/app.ts` around lines 45 - 57, 构造函数现在没有把清理逻辑注册到上下文生命周期，导致当 Context 被卸载时 ChatInterface 的链路/记忆不会自动释放：在 ChatInterface 的 constructor 中使用 ctx.effect(...) 包装那些会创建或依赖 this._chain / this._embeddings / this._historyMemory / this._infiniteContextManager 的响应式或副作用逻辑，以便 ctx.effect 返回的清理函数自动处理释放；另外在 constructor 中添加 ctx.on('dispose', () => this.dispose()) 作为额外保障，确保在 Context 全局销毁时会调用 dispose() 清理这些字段。
🟠 Major comments (25)
packages/core/src/utils/error.ts-21-31 (1)
21-31:⚠️ Potential issue | 🟠 Major移除了 logger 改用 console.error，违反编码规范
此处将
logger.error替换为console.error是一个回退。项目编码规范明确要求"Use try/catch around LLM calls and file I/O; log vialogger.error(e)"，而console.error无法接入 Koishi 日志框架进行统一管理和配置。建议恢复使用
logger.error，或在ChatLunaError构造函数中注入 logger 参数以便正确处理错误日志。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/error.ts` around lines 21 - 31, The recent change replaced logger.error calls with console.error inside the ChatLunaError handling code; revert this by using the project's logger instead: in the ChatLunaError class/constructor and any methods referencing console.error (look for usages in ChatLunaError and its stack/origin handling), restore calls to logger.error or add a logger parameter to the ChatLunaError constructor so the instance can call this.logger.error(...) for originError, originError.cause and the fallback `this`; ensure the logger is passed in or imported from the common logging module so logs flow through the Koishi logger framework.packages/extension-agent/src/service/skills.ts-95-100 (1)
95-100:⚠️ Potential issue | 🟠 Major归档/恢复时清空技能状态会让会话忘掉已加载技能。
_active和_requested是这项服务里唯一的会话态来源。现在一旦归档或恢复就删除对应 entry，后续 prompt 就不再注入之前已激活或待注入的 skill，恢复后的会话会直接“失忆”。除非这里另有显式恢复逻辑，否则不应该挂在conversation-after-archive/conversation-after-restore上。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-agent/src/service/skills.ts` around lines 95 - 100, The current handlers call clear(...) on the 'chatluna/conversation-after-archive' and 'chatluna/conversation-after-restore' events which deletes the session entries backing _active and _requested and causes restored conversations to lose previously loaded/requested skills; remove or stop using the clear(...) calls in those two event handlers and instead only clear state on an event that truly means the conversation is gone (e.g., conversation-delete or conversation-before-destroy) or add explicit restore logic that rehydrates _active/_requested when a conversation is restored; update the handlers around clear, conversation-after-archive, conversation-after-restore, and any restore logic to preserve or rehydrate _active/_requested rather than deleting them.packages/core/src/llm-core/prompt/context_manager.ts-304-309 (1)
304-309:⚠️ Potential issue | 🟠 Major归档/恢复不应该直接清空 persistent injections。
clearConversation()会删除_conversationPersistent和_conversationQueue。把它挂到conversation-after-archive/conversation-after-restore上之后，恢复后的会话会永久丢失 prompt injection 状态；这和本类对 persistent injection 的语义是冲突的。这里最多应该清派生缓存，不应该把会话级注入存储直接抹掉。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/prompt/context_manager.ts` around lines 304 - 309, 当前在 chatluna/conversation-after-archive 与 conversation-after-restore 事件中直接调用 clear(conversationId)（即 clearConversation），但 clearConversation 会删除 _conversationPersistent（会话级持久注入）和 _conversationQueue，导致恢复后丢失 persistent 注入。改为不要删除持久注入：在这两个事件处理器中调用或添加一个只清除派生缓存/队列的函数（例如 clearConversationQueue 或 clearDerivedCache(conversationId)），保留 _conversationPersistent 不变；若不存在此函数，请实现一个仅清空 _conversationQueue/衍生缓存的 helper 并在事件回调中使用它，而不要调用 clear 或 clearConversation。packages/core/src/middlewares/chat/chat_time_limit_save.ts-34-39 (1)
34-39:⚠️ Potential issue | 🟠 Major不要在保存阶段静默放过缺失的限额上下文。
这里一旦
conversationId、chatLimit或chatLimitCache缺失就直接CONTINUE，chatLimit.count不会递增，也不会写回缓存。这样会把上游 conversation wiring 的问题变成 fail-open，实际效果就是限额统计被绕过。这里应该由上游保证这些字段必定存在，或者在这里显式失败，而不是静默跳过。 As per coding guidelines "Do not add defensive/fallback checks; do not guess what types or structures might be; use the most probable type directly".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/chat_time_limit_save.ts` around lines 34 - 39, The middleware currently returns ChainMiddlewareRunStatus.CONTINUE when conversationId, chatLimit, or chatLimitCache are missing, which silently bypasses incrementing and persisting chatLimit; instead, change the behavior in chat_time_limit_save.ts to assert these required values and fail loudly: detect missing conversationId, chatLimit, or chatLimitCache and throw a descriptive Error (including which symbol is missing) rather than returning ChainMiddlewareRunStatus.CONTINUE so the caller sees the wiring issue and the limit count is not silently skipped.packages/core/src/llm-core/platform/service.ts-56-65 (1)
56-65:⚠️ Potential issue | 🟠 Major这里把单会话事件变成了全局 cache eviction。
这些监听都是某个
conversation的生命周期事件，但clear()直接this._tmpVectorStores.clear()。LRUCache.clear()会触发上面的dispose，所以一个会话的 clear/archive/delete/restore/cache-clear/compress 会对所有会话的临时 vector store 执行free()。这会打掉无关会话的缓存，严重时还会在并发请求里释放仍在使用的实例。至少要按conversationId定位并删除该会话对应项，而不是清空整个 LRU。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/platform/service.ts` around lines 56 - 65, 当前把每个 conversation 的生命周期事件（chatluna/conversation-after-clear-history 等）统一绑定到一个不带上下文的 clear()，直接调用 this._tmpVectorStores.clear()，导致单会话操作清空全局 LRU 并触发所有项的 dispose。修改事件处理器让它接收事件载荷、从中提取 conversationId（或 conversation.id），然后仅针对该 id 从 this._tmpVectorStores 删除或释放对应条目（例如调用 this._tmpVectorStores.delete(conversationId) 或先取到条目并调用其 free 方法），并在没有 conversationId 时安全地跳过操作；保持 LRUCache 的 dispose 行为不变以正确回收资源。packages/adapter-dify/src/requester.ts-412-416 (1)
412-416:⚠️ Potential issue | 🟠 Major
user现在是动态的，但会话清理时仍然使用固定的'chatluna'。在第133行和243行，
resolveDifyUser(params)被调用以获取动态的 user 值，该值被发送到 Dify 的创建/续用会话接口。但在dispose()方法中（第737行），删除请求仍然硬编码{ user: 'chatluna' }。根据 Dify API 文档，DELETE /conversations/{conversation_id} 接口要求 body 中的 user 必须与创建该会话时使用的 user 完全一致，否则删除会失败。需要在缓存创建会话时同时保存 user 值（不仅保存 difyConversationId），然后在
dispose()中从缓存中取出并使用相同的 user 值来删除会话。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-dify/src/requester.ts` around lines 412 - 416, The dispose() flow currently hardcodes user:'chatluna' while session creation uses resolveDifyUser(params) dynamically; fix by persisting the user with the conversation id when creating the session (where difyConversationId is cached) and update dispose() to read that cached user and send it in the DELETE body instead of the literal 'chatluna'; reference resolveDifyUser(params), the caching logic that stores difyConversationId, and dispose() to add retrieval of the saved user (with a fallback to resolveDifyUser(params) only if the cache entry is missing) before calling the Dify DELETE /conversations/{conversation_id} API.packages/core/src/utils/model.ts-1-6 (1)
1-6:⚠️ Potential issue | 🟠 MajorTuple 契约与实现不一致，严重影响下游代码安全性。
函数签名承诺
[string, string]，但实现会返回：
[undefined, undefined]当输入为空（第 3 行）- 单元素数组
['platform/model']当输入仅含单个/或不含/（第 6 行，因正则寻找//双斜线）- 仅当输入含
//时才正确返回双元素数组调用方直接解构
[platform, modelName]后使用两个值，会导致非法undefined向下传播。虽然部分调用点已添加防御式检查（如middlewares/chat/read_chat_message.ts:483），但这表明该bug已在生产中暴露。加之tsconfig.base.json禁用了strictNullChecks，TypeScript 无法捕获这类违约。🔧 建议修正
-export function parseRawModelName(modelName: string): [string, string] { - if (modelName == null || modelName.trim().length < 1) { +export function parseRawModelName( + modelName: string +): [string | undefined, string | undefined] { + if (modelName == null || modelName.trim().length < 1) { return [undefined, undefined] } - return modelName.split(/(?<=^[^\/]+)\//) as [string, string] + const i = modelName.indexOf('/') + if (i < 1 || i === modelName.length - 1) { + return [undefined, undefined] + } + return [modelName.slice(0, i), modelName.slice(i + 1)] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/model.ts` around lines 1 - 6, The function parseRawModelName currently declares a return type of [string, string] but can return undefineds or single-element arrays; change the contract and implementation to reliably return a two-tuple of possibly-undefined values: update the signature to parseRawModelName(modelName: string): [string | undefined, string | undefined] and implement splitting with a simple first-slash split (e.g., const parts = modelName?.trim()?.split('/', 2)); return [undefined, undefined] for null/empty input, return [undefined, parts[0]] when there's no slash, and return [parts[0], parts[1]] when there is one—trim parts and ensure the function always returns exactly two array slots so callers can safely destructure platform and modelName.packages/core/src/index.ts-215-232 (1)
215-232:⚠️ Potential issue | 🟠 Major不要把会话标题批量写进成功日志。
这里记录的是
conversation.title，而标题现在本质上是用户对话内容的摘要。自动归档 / 清理任务把这些标题原样落盘，会把敏感提示词、个人信息或业务内容带进日志。这里最多记录数量，必要时记录内部 id，不要记录标题文本。Also applies to: 267-282
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 215 - 232, The success logging currently records conversation.title (sensitive user content); change the archive loop to avoid storing or logging titles—keep only a counter (or, if needed, non-sensitive identifiers like conversation.id) by replacing success.push(conversation.title) with success.push(conversation.id) or incrementing a numeric counter, and update the logger.success call to print only the count (and optional ids) instead of title text; apply the same change for the other occurrence that uses the success array around functions conversation.archiveConversationById and logger.success.packages/core/src/utils/chat_request.ts-6-8 (1)
6-8:⚠️ Potential issue | 🟠 Major缓存键不要用
'-'直接拼接。这个 key 不是唯一映射：例如
['a-b', 'c', 'd']和['a', 'b-c', 'd']都会得到同一个字符串。这样getRequestId()/deleteRequestId()就可能命中错误请求，导致 stop 或覆盖串到别的会话。这里至少应改成JSON.stringify([session.userId, session.guildId ?? '', conversationId])，或者改成分层Map。🛠️ 建议修改
function getRequestCacheKey(session: Session, conversationId: string) { - return session.userId + '-' + (session.guildId ?? '') + '-' + conversationId + return JSON.stringify([ + session.userId, + session.guildId ?? '', + conversationId + ]) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/chat_request.ts` around lines 6 - 8, getRequestCacheKey currently concatenates session.userId, session.guildId and conversationId with '-' which can produce collisions (e.g. ['a-b','c','d'] vs ['a','b-c','d']) and cause getRequestId()/deleteRequestId() to operate on wrong requests; change getRequestCacheKey to produce an unambiguous key (for example return JSON.stringify([session.userId, session.guildId ?? '', conversationId])) or refactor the request cache to use a nested Map keyed by userId -> guildId -> conversationId so lookups in getRequestId/deleteRequestId use the new non-colliding key structure.packages/core/src/index.ts-202-207 (1)
202-207:⚠️ Potential issue | 🟠 Major给这两个后台任务加外层
try/catch。现在只有单个 conversation 的处理被包在
try/catch里，但前面的ctx.database.get(...)一旦抛错，execute()就会直接 reject；而这里又在初始化阶段同步await execute()。一次瞬时数据库故障就能中断插件启动，定时器回调里也会留下未处理的 rejection。建议把整个execute()外层包起来并统一logger.error(e)。Also applies to: 235-239, 252-259, 285-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 202 - 207, 将整个后台定时任务的执行体（即包含对 ctx.database.get(...) 查询和随后对每个会话处理的循环/调用 execute() 的外层）用一个外层 try/catch 包裹起来；当前只对单个 conversation 做了 try/catch，但如果 ctx.database.get 抛错或 execute() 在初始化时被 await，会导致未捕获的 rejection。修改涉及调用 ctx.database.get(...)、execute() 等的函数体，在 catch 中统一使用 logger.error(e) 记录完整错误信息并阻止异常冒泡；同样对文件中其他类似的定时任务位置（调用 ctx.database.get、处理会话或调用 execute 的地方）作相同处理。packages/core/src/llm-core/chat/default.ts-21-37 (1)
21-37:⚠️ Potential issue | 🟠 Major事件处理器中的异步清理未被等待。
forEach(async ...)不会等待clearConversationInterface()完成，导致清理任务作为悬空 Promise 执行。模型移除或工具更新事件发出后，事件处理器立即返回，旧的 conversation interface 可能还留在缓存里，失败时也不会被事件监听器捕获。应改为 async 事件监听器，并用Promise.allSettled()或for...of循环等待所有清理任务完成。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/chat/default.ts` around lines 21 - 37, The event handler registered with ctx.on('chatluna/model-removed') starts asynchronous clears via ctx.chatluna.conversationRuntime.clearConversationInterface inside a forEach(async ...) without awaiting them, leaving dangling Promises; change the listener to an async function and gather the cleanup promises from ctx.chatluna.conversationRuntime.getCachedConversations().filter(...).map(([id, entry]) => ctx.chatluna.conversationRuntime.clearConversationInterface(entry.conversation)) then await Promise.allSettled(...) (or iterate with for...of and await each) and log results via logger?.debug, keeping parseRawModelName(...) and the same filter logic intact.packages/core/src/middlewares/system/wipe.ts-20-49 (1)
20-49:⚠️ Potential issue | 🟠 Major
purge_legacy也需要二次确认。
wipe已经要求用户做一次挑战确认，但这里一旦validation_result.passed为真就会立即清空 legacy 表。这个动作虽然范围更小，但一样是不可逆的，会直接抹掉 rollback 数据，误触成本很高。建议复用现有的 challenge / prompt 流程，至少再确认一次。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/system/wipe.ts` around lines 20 - 49, The 'purge_legacy' middleware currently proceeds to purge once validation_result.passed is true; add a second user confirmation using the existing challenge/prompt flow instead of auto-executing purge: inside the middleware (identified by the 'purge_legacy' chain entry) call the same challenge prompt/confirmation routine used by the wipe flow (reuse the same function or middleware hook that performs the user challenge) and only call purgeLegacyTables, writeMetaValue(..., 'legacy_purged_at'), writeMetaValue(..., LEGACY_RETENTION_META_KEY) and return STOP if the user explicitly confirms; if the confirmation fails or is cancelled, set context.message to an appropriate abort message and return SKIPPED or STOP without mutating data. Ensure you use the same challenge API to keep UX consistent and reference readMetaValue, purgeLegacyTables, writeMetaValue, createLegacyTableRetention and ChainMiddlewareRunStatus when making the change.packages/core/src/utils/compression.ts-16-28 (1)
16-28:⚠️ Potential issue | 🟠 Major
gzipEncode()和gzipDecode()现在不是对称 API。
gzipEncode()支持返回base64/hex字符串，但gzipDecode()只接受ArrayBuffer | ArrayBufferView。调用方一旦用了非默认编码，这个模块自己就无法完成 round-trip，后续很容易出现“能压缩、不能解压”的回归。建议让gzipDecode()同步接受字符串并显式带上输入编码，或者直接去掉字符串编码分支。🧩 建议修改
-export async function gzipDecode(data: ArrayBuffer | ArrayBufferView) { - const buffer = ArrayBuffer.isView(data) ? Buffer.from(data.buffer, data.byteOffset, data.byteLength) : Buffer.from(data) +export async function gzipDecode( + data: ArrayBuffer | ArrayBufferView | string, + encoding: 'base64' | 'hex' = 'base64' +) { + const buffer = + typeof data === 'string' + ? Buffer.from(data, encoding) + : ArrayBuffer.isView(data) + ? Buffer.from(data.buffer, data.byteOffset, data.byteLength) + : Buffer.from(data) return (await gunzipAsync(buffer)).toString() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/compression.ts` around lines 16 - 28, The gzipEncode/gzipDecode APIs are asymmetric: gzipEncode supports returning encoded strings (e.g. 'base64'/'hex') while gzipDecode only accepts ArrayBuffer/ArrayBufferView, preventing a round-trip when callers use a string encoding. Update gzipDecode (the function named gzipDecode) to accept either a string plus an optional encoding parameter or existing ArrayBuffer/ArrayBufferView; detect when input is a string and convert it to a Buffer via Buffer.from(input, encoding) (defaulting encoding to 'base64' or require an explicit encoding), otherwise keep the current ArrayBuffer handling, then pass the Buffer into gunzipAsync and return the decoded string—this restores symmetry with gzipEncode without removing existing functionality.packages/core/src/utils/archive.ts-28-50 (1)
28-50:⚠️ Potential issue | 🟠 Major不要用整行
upsert回写 binding 快照。这里先全表读取
chatluna_binding，再把命中的binding整对象展开回写。归档清理是后台任务，若别的请求在这期间更新了同一条 binding 的其它字段，这里的 stale snapshot 会把新值覆盖回去；而且每次 purge 都要扫全表，数据量一大就会明显退化。请改成只更新命中的记录，并且只写activeConversationId、lastConversationId和updatedAt。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/archive.ts` around lines 28 - 50, Currently the code reads all rows from ctx.database.get('chatluna_binding') and writes back entire binding objects via ctx.database.upsert, which can overwrite concurrent changes and is inefficient; instead find only records that match conversation.id (or query with a filter for activeConversationId/lastConversationId) and perform a targeted update per record that sets only activeConversationId, lastConversationId and updatedAt (e.g. call your DB update API or an upsert/update on 'chatluna_binding' using the binding primary key/id and a payload with only { activeConversationId: ..., lastConversationId: ..., updatedAt: new Date() }) so you don’t overwrite other fields or scan the whole table.packages/core/src/middlewares/model/resolve_model.ts-40-45 (1)
40-45:⚠️ Potential issue | 🟠 Major先校验
parseRawModelName()的结果，再查平台模型。
platformName/rawModelName的判空是在listPlatformModels()之后才做。只要库里残留一个迁移过来的非法conversation.model，这里就会先抛异常，最后落到catch静默STOP，用户收不到.unavailable提示。🧩 建议修改
const [platformName, rawModelName] = parseRawModelName(modelName) - const platformModels = ctx.chatluna.platform.listPlatformModels( - platformName, - ModelType.llm - ).value const presetExists = ctx.chatluna.preset.getPreset(conversation.preset, false) .value != null if ( - modelName !== 'empty' && - platformName != null && - rawModelName != null && - platformModels.length > 0 && - platformModels.some((it) => it.name === rawModelName) && - presetExists - ) { - return ChainMiddlewareRunStatus.CONTINUE - } - - await context.send( - session.text('chatluna.conversation.messages.unavailable', [ - modelName - ]) - ) - return ChainMiddlewareRunStatus.STOP + modelName === 'empty' || + platformName == null || + rawModelName == null + ) { + await context.send( + session.text( + 'chatluna.conversation.messages.unavailable', + [modelName] + ) + ) + return ChainMiddlewareRunStatus.STOP + } + + const platformModels = ctx.chatluna.platform.listPlatformModels( + platformName, + ModelType.llm + ).value + + if ( + platformModels.length > 0 && + platformModels.some((it) => it.name === rawModelName) && + presetExists + ) { + return ChainMiddlewareRunStatus.CONTINUE + } + + await context.send( + session.text('chatluna.conversation.messages.unavailable', [ + modelName + ]) + ) + return ChainMiddlewareRunStatus.STOPAlso applies to: 50-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/model/resolve_model.ts` around lines 40 - 45, Validate the output of parseRawModelName immediately after calling it (check platformName and rawModelName are non-empty/valid) before invoking ctx.chatluna.platform.listPlatformModels; if the parse result is invalid return or throw a controlled error (so the caller can map to .unavailable) instead of letting listPlatformModels run and raise an unexpected exception. Specifically, in resolve_model.ts around the code using parseRawModelName, add guards for platformName/rawModelName and short-circuit with a handled error or return value prior to calling ctx.chatluna.platform.listPlatformModels(ModelType.llm), and apply the same validation pattern to the later similar block (lines ~50-69) that repeats this logic.packages/core/src/services/conversation_types.ts-162-184 (1)
162-184:⚠️ Potential issue | 🟠 Major不要把缺失标识折叠成固定的 binding key 片段。
这里的
?? 'unknown'和routeKey ?? 'default'会把本该暴露出来的上下文缺失静默变成真实 routing key。结果是不同会话、不同群组，甚至不同 custom route，都可能被并到同一个 conversation 命名空间里，直接影响历史归属和隔离性。这里更安全的做法是 fail-fast，而不是生成兜底 key。
As per coding guidelines, "Do NOT add defensive/fallback checks; do not guess what types or structures might be; use the most probable type directly".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation_types.ts` around lines 162 - 184, computeBaseBindingKey currently masks missing identifiers with defaults (platform/selfId/guildId/userId and routeKey) which can collapse distinct conversations; instead validate required fields and fail-fast: in computeBaseBindingKey check session.platform, session.selfId, and depending on routeMode and session.isDirect assert/throw when session.guildId or session.userId are missing, and when routeMode === 'custom' require a non-null routeKey (do not default to 'default'); remove all '??' fallbacks and return binding strings only after those validations so missing context surfaces as an error.packages/core/src/middlewares/chat/rollback_chat.ts-77-108 (1)
77-108:⚠️ Potential issue | 🟠 Major不要把回滚轮次写死成“每轮 2 条消息”。
rollbackRound * 2只覆盖最简单的 user/assistant 往返。新 runtime 下一个回合里可能包含 tool call、tool result、额外 assistant 节点等，当前算法会把latestMessageId回退到一轮中间，留下半截尾链和错误的历史头。这里应该沿parentId回溯，直到消费完 N 个“用户消息边界”为止，再删除该边界之后的全部节点。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 77 - 108, The current rollback logic assumes each round equals exactly 2 messages and uses rollbackRound * 2, which breaks when rounds contain tool calls/results or extra assistant nodes; change the traversal to walk parentId backwards counting user-message boundaries (e.g., nodes where role === 'user' or type indicates user input) until you have consumed rollbackRound user boundaries, collect all nodes encountered, then set previousLatestId to the parentId immediately before the first of those consumed user nodes and remove all collected nodes; update the calls around ctx.database.get ('chatluna_message'), ctx.database.upsert ('chatluna_conversation') and the deletion call (e.g., ctx.database.delete or upsert batch) to use this computed previousLatestId and to delete all nodes after that boundary rather than assuming 2 messages per round.packages/core/src/middlewares/chat/rollback_chat.ts-112-123 (1)
112-123:⚠️ Potential issue | 🟠 Major回滚重放时不要把历史多模态输入压成纯文本。
Line 122 把
decodeMessageContent(humanMessage)的结果再次经过getMessageContent()+h.text()压平了。原始回合里的图片、文件、音频会在 rollback 重放时全部消失，重试出来的请求和用户当时发送的内容已经不是同一条了。这里应直接复用解码后的 structured content，或至少把 typed parts 一并恢复。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 112 - 123, The rollback currently flattens multimodal history by calling getMessageContent(humanContent) and h.text() after decodeMessageContent(humanMessage) (in rollback_chat.ts inside the block that sets context.options.inputMessage), which strips images/files/audio; instead pass the decoded structured content (humanContent) or reconstruct its typed parts into the transformer input so multimodal parts are preserved: replace the h.text(getMessageContent(...)) step and feed ctx.chatluna.messageTransformer.transform(session, humanContent /* or mapped typed parts */, reResolved.effectiveModel ?? conversation.model) (or otherwise ensure typed parts from decodeMessageContent are included when building context.options.inputMessage).packages/core/src/middlewares/chat/read_chat_message.ts-60-90 (1)
60-90:⚠️ Potential issue | 🟠 Major不要在识别预设前缀后把整条消息重建成纯文本。
Line 90 直接把
message改成[h.text(parsed.content)]，会把原消息里的图片、文件、音频等元素全部丢掉。这样用户一旦发送“预设前缀 + 附件”，后面的 transformer 看到的就只剩文本了。这里应只剥离前缀文本，保留其余元素。💡 一个最小修正方向
- message = [h.text(parsed.content)] + const next: h[] = [] + let replaced = false + for (const el of message as h[]) { + if (!replaced && el.type === 'text') { + replaced = true + if (parsed.content.length > 0) { + next.push(h.text(parsed.content)) + } + continue + } + next.push(el) + } + message = next🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/read_chat_message.ts` around lines 60 - 90, The code currently replaces the entire message with a single text node (message = [h.text(parsed.content)]), which drops non-text elements (images, files, audio); instead, preserve all original h elements and only replace/trim the text portion that contained the preset prefix: locate the block using parsed and message in read_chat_message.ts, build a new message array by iterating the original message (the h[]), for text nodes remove the detected prefix and replace their text with parsed.content (or split the first matching text node into the prefix-removed remainder), and keep all non-text nodes unchanged; assign that new array back to message so downstream transformers still see attachments while the prefix is stripped.packages/core/src/llm-core/chat/app.ts-237-243 (1)
237-243:⚠️ Potential issue | 🟠 Major自动标题摘要有竞态，会重复调用模型并互相覆盖结果。
这里是 fire-and-forget，而
autoTitle要等摘要完成后才会写回false。after-chat监听已经可以在这之前放行下一轮请求，所以新会话连续两轮时，很容易同时跑进autoSummarizeTitle()：白白多一次 LLM 调用，且最后一次写入还会覆盖第一次标题。建议在发起摘要前先把这个开关做成幂等关闭，或把保护下沉到 conversation service。Also applies to: 453-488
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/chat/app.ts` around lines 237 - 243, autoSummarizeTitle is being called fire-and-forget which races with the autoTitle flag and allows multiple concurrent LLM calls that overwrite each other; before invoking autoSummarizeTitle from the after-chat listener, atomically flip the conversation's autoTitle state to "in-progress" (or false) so subsequent requests see it disabled, or move that idempotency check into the conversation service (e.g., add a method like claimAutoTitle(conversationId) that does a conditional DB/update and returns whether this caller owns the job); only call autoSummarizeTitle when claimAutoTitle succeeds and ensure the completion handler clears the in-progress marker, avoiding concurrent LLM calls and overwritten titles.packages/core/src/middlewares/system/conversation_manage.ts-783-821 (1)
783-821:⚠️ Potential issue | 🟠 Major压缩命令检查的是“当前路由”的锁，不是“目标会话”的锁。
resolved.constraint是在解析当前上下文时拿到的约束；但conversation可能是通过 ACL 解析到的跨路由目标会话。这样一来，目标会话本身被锁时这里仍可能放行，反过来也可能被当前路由的锁误拦截。压缩前应基于conversation.id再解析一次目标会话的 constraint，或把这层校验下沉到 service/runtime。建议修改
if (conversation == null) { context.message = session.text(`${key}.no_conversation`) return ChainMiddlewareRunStatus.STOP } - if (resolved.constraint.lockConversation) { + const targetResolved = await ctx.chatluna.conversation.resolveContext( + session, + { presetLane, conversationId: conversation.id } + ) + + if (targetResolved.constraint.lockConversation) { context.message = session.text(`${key}.failed`, [ conversation.title, conversation.id, session.text('chatluna.conversation.messages.action_locked', [ session.text('chatluna.conversation.action.compress')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/system/conversation_manage.ts` around lines 783 - 821, The lock check is using resolved.constraint (constraints for the current route) instead of the target conversation's constraints, so re-check the target conversation's constraint after resolving the target conversation: call ctx.chatluna.conversation.resolveContext or an equivalent constraint-resolution method using conversation.id (after resolveTargetConversation returns) and then inspect that returned.constraint.lockConversation before allowing the compress action; update the block that currently reads resolved.constraint.lockConversation to use the freshly resolved constraint for the target (and return the same stop/status and message flow if that target constraint indicates a lock).packages/core/src/middlewares/system/conversation_manage.ts-160-164 (1)
160-164:⚠️ Potential issue | 🟠 Major
Pagination单例会串掉并发请求的格式和内容。这个实例挂在
apply()级别，但conversation_list每次都会覆写formatItem/formatString，而缓存 key 还是共享的bindingKey。两个会话并发执行 list 时，后一条请求会把前一条请求的本地化文本和格式函数覆盖掉，返回内容会串语言、串状态。这里应该按请求创建Pagination，不要把可变分页状态做成全局单例。Also applies to: 342-356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/system/conversation_manage.ts` around lines 160 - 164, The Pagination instance is created once in apply() and shared across requests, causing concurrent conversation_list calls to overwrite formatItem/formatString via the shared bindingKey; change the code to instantiate a fresh Pagination inside the request-handling path (e.g., inside conversation_list) so formatItem and formatString are per-request (or include a request-specific suffix in bindingKey) and avoid keeping mutable pagination state as a global/singleton; update any references to Pagination, apply, conversation_list, formatItem, formatString, and bindingKey accordingly to ensure pagination and localization are scoped to each request.packages/core/src/migration/legacy_tables.ts-45-62 (1)
45-62:⚠️ Potential issue | 🟠 Major不要在 drop 失败后仍写入 legacy sentinel。
现在任意一张
chathub_*表 drop 失败时，只会记一条 warning，然后仍然落盘legacy-schema-disabled.json。下次启动就会把遗留表视为已彻底清理，迁移/清理逻辑可能被跳过，但数据库里其实还残留旧表。建议修改
export async function purgeLegacyTables(ctx: Context) { + const failed: string[] = [] + for (const table of [ ...LEGACY_MIGRATION_TABLES, ...LEGACY_RUNTIME_TABLES ]) { try { await ctx.database.drop(table) } catch (error) { ctx.logger.warn(`purge legacy ${table}: ${error}`) + failed.push(table) } } + if (failed.length > 0) { + throw new Error(`Failed to purge legacy tables: ${failed.join(', ')}`) + } + const sentinel = getLegacySchemaSentinel(ctx.baseDir) await fs.mkdir(getLegacySchemaSentinelDir(ctx.baseDir), { recursive: true }) await fs.writeFile( sentinel, JSON.stringify({ purgedAt: new Date().toISOString() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/migration/legacy_tables.ts` around lines 45 - 62, The purgeLegacyTables function currently writes the legacy sentinel even if any table drop failed; change purgeLegacyTables so that when iterating over [...LEGACY_MIGRATION_TABLES, ...LEGACY_RUNTIME_TABLES] you track whether any ctx.database.drop(table) threw (e.g., set a boolean hadDropError or collect errors inside the try/catch for purgeLegacyTables), and only call getLegacySchemaSentinelDir/getLegacySchemaSentinel and write the sentinel when no drop errors occurred; if any drop failed, keep the existing warning logs but skip creating/writing legacy-schema-disabled.json (or propagate/return an error) so the system does not falsely consider legacy tables purged. Ensure references to purgeLegacyTables, LEGACY_MIGRATION_TABLES, LEGACY_RUNTIME_TABLES, getLegacySchemaSentinelDir and getLegacySchemaSentinel are used to locate the change.packages/core/test/conversation-runtime.test.ts-1382-1384 (1)
1382-1384:⚠️ Potential issue | 🟠 Major缺少
await，断言无法真正阻塞测试
assert.rejects()返回 Promise，未等待时测试会继续执行，即使断言失败也不会使用例失败。修改建议
- assert.rejects(fs.access(archiveDir)) + await assert.rejects(fs.access(archiveDir))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/conversation-runtime.test.ts` around lines 1382 - 1384, The test calls assert.rejects(fs.access(archiveDir)) without awaiting it, so the assertion's Promise isn't awaited and won't fail the test; update the test to await the rejection check (e.g., await assert.rejects(...)) when verifying purgeArchivedConversation(...) removed the archive, and ensure the value passed to assert.rejects is a Promise (such as fs.access(archiveDir) or an async wrapper) so the awaited assertion correctly blocks and fails on success.packages/core/src/services/conversation_runtime.ts-470-489 (1)
470-489:⚠️ Potential issue | 🟠 Major
formatUsageMetadataMessage中 output 数组使用了错误的字段来源。audio 和 image 应该从
output_token_details读取，但当前代码从input_token_details读取。此外，video和document字段在 UsageMetadata 结构中根本不存在，应该完全移除。根据
createUsageMetadata的实现，有效的字段为：
- input_token_details: audio, image, cache_read, cache_creation
- output_token_details: audio, image, reasoning
🐛 建议修复
const output = [ - ...(usage.input_token_details?.audio != null && - usage.input_token_details?.audio > 0 - ? [`audio=${usage.input_token_details.audio}`] + ...(usage.output_token_details?.audio != null && + usage.output_token_details?.audio > 0 + ? [`audio=${usage.output_token_details.audio}`] : []), - ...(usage.input_token_details?.image != null && - usage.input_token_details?.image > 0 - ? [`image=${usage.input_token_details.image}`] + ...(usage.output_token_details?.image != null && + usage.output_token_details?.image > 0 + ? [`image=${usage.output_token_details.image}`] : []), - ...(usage.input_token_details?.video != null && - usage.input_token_details?.video > 0 - ? [`video=${usage.input_token_details.video}`] - : []), - ...(usage.input_token_details?.document > 0 - ? [`document=${usage.input_token_details.document}`] - : []), ...(usage.output_token_details?.reasoning != null ? [`reasoning=${usage.output_token_details.reasoning}`] : []) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation_runtime.ts` around lines 470 - 489, In formatUsageMetadataMessage update the output array to use the correct fields: read audio and image from usage.output_token_details (not usage.input_token_details), remove any references to non-existent fields video and document, and retain reasoning from usage.output_token_details; align the code with createUsageMetadata's schema by only emitting output entries for output_token_details.audio, output_token_details.image, and output_token_details.reasoning while leaving input_token_details-related fields unchanged.
🟡 Minor comments (7)
packages/core/src/llm-core/memory/message/database_history.ts-421-439 (1)
421-439:⚠️ Potential issue | 🟡 Minor缺少
autoTitle字段应与标准创建路径保持一致。在会话不存在时的回退创建（行 421-439）中，缺少了
autoTitle字段。虽然该字段在类型定义中是可选的，但标准的会话创建路径（conversation.ts:354）会明确设置autoTitle: true。为保持一致性，应补充此字段。此外，bindingKey: this.conversationId将绑定键设置为会话ID本身，这在系统创建的会话中可能是设计意图，但应确保这是预期行为。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/memory/message/database_history.ts` around lines 421 - 439, When creating the fallback conversation in database_history (the call to this._ctx.database.create('chatluna_conversation', ...)), add the missing autoTitle: true property so the fallback matches the standard creation path (conversation.ts:354); also verify that keeping bindingKey: this.conversationId is intentional for system-created conversations and adjust if a different bindingKey is required.packages/core/src/middlewares/chat/chat_time_limit_check.ts-111-115 (1)
111-115:⚠️ Potential issue | 🟡 Minor
oldChatLimitCheck返回值不一致：应返回ChainMiddlewareRunStatus.CONTINUE而非隐式undefined第 28-30 行在
target == null时返回ChainMiddlewareRunStatus.CONTINUE，但第 113-114 行在相同条件下只有return（隐式返回undefined）。这两处应保持一致的行为。🔧 建议修复
if (target == null) { - return + return ChainMiddlewareRunStatus.CONTINUE }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/chat_time_limit_check.ts` around lines 111 - 115, oldChatLimitCheck currently returns ChainMiddlewareRunStatus.CONTINUE in one branch but implicitly returns undefined in another; update the branch after calling resolveConversationTarget(session, context) so that when target == null it explicitly returns ChainMiddlewareRunStatus.CONTINUE (matching the behavior at lines 28–30). Locate the oldChatLimitCheck function and the resolveConversationTarget call and replace the bare return with return ChainMiddlewareRunStatus.CONTINUE to ensure consistent middleware return semantics.packages/core/src/index.ts-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor删除未使用的
fs/promises导入。当前文件里没有任何
fs调用，这里只会增加一次 lint / 静态检查告警。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` at line 3, Remove the unused import "import fs from 'fs/promises'": locate the import statement "import fs from 'fs/promises'" at the top of the module (in packages core index module) and delete it so the file no longer imports fs/promises; then run the linter/type-check to ensure no other references to fs remain.packages/core/src/middlewares/system/conversation_manage.ts-44-56 (1)
44-56:⚠️ Potential issue | 🟡 Minor自定义路由 scope 会被格式化成
undefined。
formatRouteScope()默认把 key 当成shared/personal的 4/5 段结构解析。像custom:team-alpha这种两段 key 走到这里会直接拼出带undefined的字符串，conversation_rule_show的展示会坏掉。对非shared/personal路由应直接返回原始 key，或单独处理custom。Also applies to: 755-759
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/system/conversation_manage.ts` around lines 44 - 56, formatRouteScope currently assumes bindingKey is the 4/5-part "shared"/"personal" form and splits into [mode,platform,selfId,scope,userId], causing "custom:team-alpha" to produce "undefined" parts; update formatRouteScope to detect non-conforming keys (e.g., when split length < 4 or mode not 'shared'/'personal' and not containing ':preset:') and simply return the original bindingKey (or specifically handle 'custom' by returning it unchanged) instead of building the `${mode} ${platform}/...` string; adjust the logic around the existing bindingKey.includes(':preset:') check and ensure conversation_rule_show and any callers receive the unmodified key for unknown modes.packages/core/src/commands/conversation.ts-290-307 (1)
290-307:⚠️ Potential issue | 🟡 Minor
renameCommand变量已赋值但未使用。静态分析提示此变量未被使用。如果不需要对该命令做后续配置（如
setChoices），可以移除变量赋值。🔧 建议修复
- const renameCommand = ctx - .command('chatluna.rename <title:text>', { + ctx.command('chatluna.rename <title:text>', { authority: 1 }) .option('preset', '-p <preset:string>') .action(async ({ options, session }, title) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/commands/conversation.ts` around lines 290 - 307, The declared constant renameCommand is assigned but never used; either remove the assignment and call ctx.command(...).option(...).action(...) directly, or keep the variable and use it for further configuration (e.g., renameCommand.setChoices(...) or other chaining). Locate the code around the renameCommand declaration and either drop the "const renameCommand =" prefix, leaving the chained call to ctx.command(...) intact, or reference renameCommand in subsequent setup so the variable is used; ensure chain.receiveCommand, normalizeTarget and ctx remain unchanged in the action body.packages/core/src/services/conversation.ts-376-385 (1)
376-385:⚠️ Potential issue | 🟡 Minor格式化问题：多余的括号。
静态分析提示第 379 行有多余的括号。
🔧 建议修复
const payload: BindingRecord = { bindingKey, activeConversationId: conversationId, lastConversationId: prev != null && prev !== conversationId ? prev - : (current?.lastConversationId ?? null), + : current?.lastConversationId ?? null, updatedAt: new Date() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 376 - 385, The object construction for payload contains an extra pair of parentheses around the expression (current?.lastConversationId ?? null) for the lastConversationId property; remove the unnecessary parentheses so it reads current?.lastConversationId ?? null within the payload assignment (the code that builds payload and assigns lastConversationId before calling this.ctx.database.upsert in the same function).packages/core/src/services/conversation.ts-782-786 (1)
782-786:⚠️ Potential issue | 🟡 Minor格式化问题：多余的括号。
静态分析提示 784-785 行有格式问题。
🔧 建议修复
const conversation = options.conversationId - ? ((await this.getConversation(options.conversationId)) ?? - resolved.conversation) + ? (await this.getConversation(options.conversationId)) ?? + resolved.conversation : resolved.conversation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 782 - 786, 格式化问题：在会话赋值表达式中存在多余的括号，清理语法即可。修改 conversation 的赋值表达式（涉及 resolveContext(session, options)、getConversation(options.conversationId) 和 resolved.conversation）去掉包裹整个子表达式的多余圆括号，使三元/空合并表达式保持简洁且通过静态分析。
🧹 Nitpick comments (8)
packages/core/src/utils/koishi.ts (1)
31-43:permissions.test异常可能导致权限检查失败。如果
session.app.permissions.test抛出异常（例如权限系统未正确初始化），整个checkAdmin调用会失败，而不是降级到authority检查。考虑是否需要 try-catch 包装权限测试部分。♻️ 可选的防御性处理
export async function checkAdmin(session: Session) { - const tested = await session.app.permissions.test('chatluna:admin', session) - - if (tested) { - return true + try { + const tested = await session.app.permissions.test('chatluna:admin', session) + if (tested) { + return true + } + } catch { + // fallback to authority check } const user = await session.getUser<User.Field>(session.userId, [ 'authority' ]) return user?.authority >= 3 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/koishi.ts` around lines 31 - 43, The permissions.test call in checkAdmin can throw and currently aborts the function; wrap the session.app.permissions.test('chatluna:admin', session) invocation in a try-catch inside the checkAdmin function so that on any exception you fall back to the existing authority lookup (await session.getUser<User.Field>(session.userId, ['authority'])) and return user?.authority >= 3; optionally log the caught error before continuing.packages/core/src/chains/chain.ts (1)
831-836: 双重非空断言!需注意。
this._tasks.get(name)!.middleware!使用了两个非空断言。虽然在当前逻辑下是安全的（因为topo只包含已验证存在的节点名，且addNode总是设置middleware），但如果未来逻辑变化可能导致运行时错误。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/chains/chain.ts` around lines 831 - 836, The code uses double non-null assertions this._tasks.get(name)!.middleware! when building levels which is fragile; replace these assertions with a safe access and explicit error handling: retrieve the task via this._tasks.get(name), check that the task exists and that task.middleware is defined (or throw a clear Error mentioning the missing task/middleware), or use optional chaining and filter out undefined middleware before pushing to levels; update the code paths that assume middleware always exists (e.g., topo generation and addNode) to either guarantee middleware or rely on the explicit runtime check to surface a helpful error.packages/core/src/middlewares/preset/delete_preset.ts (1)
56-66: Dead code:defaultPreset未被使用。根据 AI 摘要，此代码之前用于更新关联房间的 preset，但该逻辑已被移除。现在
defaultPreset获取后未被使用，可以删除。♻️ 建议移除未使用的代码
await fs.rm(presetTemplate.path) - let defaultPreset: PresetTemplate - try { - defaultPreset = preset.getDefaultPreset().value - if (!defaultPreset || !defaultPreset.triggerKeyword?.length) { - throw new Error('Default preset is invalid') - } - } catch (e) { - logger.error('Failed to get default preset:', e) - await context.send(session.text('.failed_to_get_default')) - return ChainMiddlewareRunStatus.STOP - } - context.message = session.text('.success', [presetName])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/preset/delete_preset.ts` around lines 56 - 66, The code fetches a defaultPreset via preset.getDefaultPreset() in delete_preset.ts but never uses defaultPreset (dead code); remove the try/catch block and the defaultPreset variable so the function no longer fetches or logs failures for the unused value, and ensure any early-return behavior tied to that block (the await context.send('.failed_to_get_default') and returning ChainMiddlewareRunStatus.STOP) is also removed or adjusted so control flow remains correct without defaultPreset.packages/core/src/commands/chat.ts (1)
145-147: 把normalizeTarget()内联到调用点。这里只有一次
trim()+ 空串判断，单独抽成normalizeXXXhelper 和仓库当前约定相反，而且这里只有本文件内部使用。直接在调用点内联会更贴合现有风格。
As per coding guidelines, "Do NOT createresolveXXX,normalizeXXX,ensureXXX,toSafeXXXfunctions—these are banned patterns".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/commands/chat.ts` around lines 145 - 147, Remove the helper function normalizeTarget and inline its logic at the single call site in this file: replace the call to normalizeTarget(value) with the equivalent expression that returns undefined for null/empty/whitespace (value == null || value.trim().length < 1 ? undefined : value.trim()), and delete the normalizeTarget function declaration so the file follows the project's guideline against creating normalize/resolve helper functions.packages/core/src/middlewares/conversation/resolve_conversation.ts (1)
59-69: 复用前序中间件已经算好的会话上下文。
read_chat_message已经把resolvedConversationContext写回context.options。这里再无条件调用一次resolveContext()，会让每条消息多跑一遍会话/约束解析，热路径上是纯重复开销。可以像前一个中间件那样仅在缓存缺失时再解析。♻️ 建议改动
- const resolved = await ctx.chatluna.conversation.resolveContext( - session, - { - conversationId: context.options.conversationId, - presetLane - } - ) + const resolved = + context.options.resolvedConversationContext ?? + (await ctx.chatluna.conversation.resolveContext(session, { + conversationId: context.options.conversationId, + presetLane + }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/conversation/resolve_conversation.ts` around lines 59 - 69, The code always calls ctx.chatluna.conversation.resolveContext even when a previous middleware (read_chat_message) already populated context.options.resolvedConversationContext; change the logic in the resolve_conversation middleware to first check context.options.resolvedConversationContext (and/or context.options.resolvedConversation) and only invoke ctx.chatluna.conversation.resolveContext(session, { conversationId: context.options.conversationId, presetLane }) when that cached value is missing, then assign context.options.resolvedConversation and context.options.resolvedConversationContext from the resolved result as currently done.packages/core/src/commands/conversation.ts (1)
68-70: 模型列表在模块加载时初始化，不会动态更新。
models数组在apply函数执行时捕获一次，如果之后有新平台/模型注册，命令的 choices 不会更新。如果这是预期行为可以忽略，否则考虑使用响应式方案或延迟获取。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/commands/conversation.ts` around lines 68 - 70, models is captured once when the command module loads so choices for the command won't reflect platforms/models registered later; move the model lookup out of module initialization and into a runtime provider by calling ctx.chatluna.platform.listAllModels(ModelType.llm).value.map(...) inside the command's apply handler (or a lazy/reactive choices resolver) so the list is recomputed each time the command is invoked; update references to the models constant and ensure the apply function uses the newly fetched array rather than the stale captured models variable.packages/core/src/services/chat.ts (1)
1-3: 导入顺序不符合编码规范。根据编码指南，导入顺序应为：Node 内置模块 → 第三方包 → Koishi 框架 → ChatLuna 导入 → 本地导入。
parseRawModelName是 ChatLuna 导入，应放在fs和path之后。🔧 建议修复
+import fs from 'fs' +import path from 'path' import { parseRawModelName } from 'koishi-plugin-chatluna/llm-core/utils/count_tokens' -import fs from 'fs' -import path from 'path'As per coding guidelines: "Use ESM import syntax at the top of the file; group imports in order: Node built-ins, third-party packages, Koishi framework, ChatLuna imports, local imports"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/chat.ts` around lines 1 - 3, The import order is wrong: move the ChatLuna import of parseRawModelName after the Node built-ins (fs, path) so imports follow the required grouping (Node built-ins → third-party → Koishi → ChatLuna → local); locate the top-of-file imports (references: parseRawModelName, fs, path) and reorder them so fs and path come before the parseRawModelName import, preserving existing ESM syntax and any side-effect imports.packages/core/src/migration/room_to_conversation.ts (1)
193-268: 房间迁移逻辑完整，但conversationId类型断言需注意。第 224 行将
room.conversationId断言为string，但在循环中并未显式检查。虽然filterValidRooms已过滤掉无效的conversationId，但这里的类型断言依赖于该前置过滤。建议添加注释说明此依赖关系。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/migration/room_to_conversation.ts` around lines 193 - 268, The code casts room.conversationId to string when building the ConversationRecord (conversation.id) without an explicit local check; either add a short runtime guard or a clarifying comment referencing the earlier filter to make the dependency explicit. Update the loop around building conversation (where conversation: ConversationRecord is created) to either: 1) perform a simple runtime validation like if (!room.conversationId) continue before using room.conversationId, or 2) add a terse comment above the cast stating that filterValidRooms guarantees conversationId is present; reference room.conversationId and the ConversationRecord construction so reviewers can see why the assertion is safe.
Keep conversation state, target resolution, and provider-specific context in sync across the new conversation flow while cleaning up the refactor fallout.
Keep the conversation target helper out of the generated commands entry so process-dynamic-import no longer treats it as a command module that must export apply.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/adapter-dify/src/requester.ts (1)
300-306:⚠️ Potential issue | 🟡 Minor错误消息中引用了错误的服务名称。
错误消息显示 "qwen completion"，但这是 Dify 适配器。这是一个复制粘贴错误，应修复以避免调试时混淆。
🔧 建议修复
throw new ChatLunaError( ChatLunaErrorCode.API_REQUEST_FAILED, new Error( - 'error when calling qwen completion, Result: ' + chunk + 'error when calling dify completion, Result: ' + chunk ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-dify/src/requester.ts` around lines 300 - 306, The thrown ChatLunaError in the requester (use ChatLunaError and ChatLunaErrorCode.API_REQUEST_FAILED) contains the wrong service name ("qwen completion"); update the error message to reference the Dify adapter/service (e.g., "dify completion" or "Dify API") so logs correctly identify the failing service—modify the Error passed into the ChatLunaError constructor where the chunk-based failure is raised.
🧹 Nitpick comments (3)
packages/core/src/llm-core/memory/message/database_history.ts (1)
421-440: 会话创建时使用硬编码的空值可能导致数据不一致当
_loadConversation创建新会话时，model、preset、chatMode等字段被设置为空字符串，createdBy被设置为'system'。这可能与通过ConversationService正常创建的会话数据结构不一致。建议确认此处创建的会话是否仅作为临时占位符，或者是否需要与其他创建路径保持一致的默认值。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/memory/message/database_history.ts` around lines 421 - 440, The new-conversation creation in _loadConversation uses hardcoded empty strings and 'system' (create('chatluna_conversation') with model/preset/chatMode = '' and createdBy = 'system'), which can diverge from ConversationService-created records; update _loadConversation to either call the existing ConversationService (or shared factory) to create a conversation with the same default values, or replace the hardcoded empty strings with the canonical defaults (or null where appropriate) used by ConversationService so stored rows remain consistent with other creation paths; ensure you update the fields model, preset, chatMode, createdBy, and any other defaults (autoTitle, additional_kwargs, compression) to match ConversationService behavior.packages/core/src/middlewares/model/resolve_model.ts (1)
32-38: 硬编码的中文字符'无'可能影响国际化模型名称检查中包含硬编码的中文字符
'无'。如果这是用户可输入的值，建议通过 i18n 机制处理，或在文档中明确说明此约定。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/model/resolve_model.ts` around lines 32 - 38, The conditional that sets modelName in resolve_model.ts currently checks conversation.model against a hard-coded Chinese string '无'; replace this hard-coded literal by using a configurable/centralized value or i18n lookup to avoid embedding language-specific text. Update the logic around modelName and conversation.model to compare against a constant/array (e.g., EMPTY_MODEL_VALUES) or call your i18n helper (e.g., i18n.t('empty')) so all accepted "empty" tokens are configurable and documented, and ensure unit tests for resolve_model cover the localized empty token.packages/core/src/utils/koishi.ts (1)
36-53:checkAdmin在用户获取失败时可能返回false当
session.getUser返回null或undefined时，user?.authority >= 3会返回false。如果这是预期行为（用户不存在则非管理员），则逻辑正确；但如果获取用户失败应视为错误情况，当前实现会静默失败。此外，空的
catch {}块会吞掉所有错误。建议至少记录 debug 级别日志以便排查问题。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/koishi.ts` around lines 36 - 53, The empty catch in checkAdmin swallows permission-check errors and there is no explicit handling when session.getUser returns null; change the catch{} around session.app.permissions.test to log the caught error (use session.app.logger.debug or error) and rethrow or surface it instead of silently swallowing, and after calling session.getUser explicitly handle a null/undefined user (e.g. return false for non-existent user or throw a clear error) rather than relying on user?.authority >= 3; update the function checkAdmin to reference session.app.permissions.test, the catch block, session.getUser, and the user.authority check accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/middlewares/chat/read_chat_message.ts`:
- Around line 71-90: The code normalizes the matched preset lane to
preset.triggerKeyword[0], which collapses aliases; instead preserve the user's
actual matched lane from parsed.preset when recording the lane. Replace usages
of preset.triggerKeyword[0] (where setting context.options.presetLane and
context.options.conversation_manage.presetLane) with the parsed.preset value
(the output of parsePresetLaneInput) so subsequent parsing/switching uses the
original alias the user provided.
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 29-53: 当前直接信任 context.options.resolvedConversation 会绕过 manage
权限校验；在 rollback_chat 中即使存在 context.options.resolvedConversation，也要对目标会话重新按
manage 权限进行解析或校验。修改逻辑：当 context.options.resolvedConversation 存在时，不直接使用它作为最终
conversation，而应调用 ctx.chatluna.conversation.resolveCommandConversation
或等价的权限检查接口（使用 resolvedConversation 中的
conversationId/targetConversation/presetLane）来确保 permission:
'manage'，只有通过该管理权限解析后再赋值给 conversation；其它分支保持不变（getCurrentConversation 用法不变）。
- Around line 83-106: The parent-chain traversal in rollback_chat (the loop
using parentId, currentMessage, humanCount, rollbackRound and ctx.database.get)
can infinite-loop on corrupt data with cycles; modify the while loop to detect
and break cycles by tracking visited message ids (e.g., a Set of seen
parentId/currentMessage.id) and break if an id repeats, and/or enforce a hard
hop limit (e.g., MAX_HOPS) to abort after too many iterations; when breaking for
a cycle or hop-limit, stop adding messages, optionally log a warning including
the offending id, and ensure humanCount/rollbackRound logic still behaves
correctly.
In `@packages/core/src/middlewares/system/wipe.ts`:
- Around line 81-94: The wipe logic is not idempotent because it unconditionally
calls ctx.database.drop on LEGACY_MIGRATION_TABLES and LEGACY_RUNTIME_TABLES
which throws if a legacy table was already removed; update the wipe middleware
to guard each drop by checking existence or swallowing missing-table errors:
iterate the arrays LEGACY_MIGRATION_TABLES and LEGACY_RUNTIME_TABLES and for
each table call a safe-drop helper (e.g., check ctx.database.hasTable(table) or
wrap ctx.database.drop(table) in a try/catch and only ignore not-found errors),
leaving other errors propagated; ensure the same safe-drop behavior is used for
the primary drops (chatluna_* calls) to make the entire wipe function
idempotent.
In `@packages/core/src/migration/room_to_conversation.ts`:
- Around line 74-89: The code writes migration "done" flags and
validation_result before actually purging legacy tables, so if
purgeLegacyTables() fails the migration appears complete and
ensureMigrationValidated() will skip retries; move the writes of
'room_migration_done', 'message_migration_done', LEGACY_RETENTION_META_KEY
(createLegacyTableRetention('purged')) and migration_finished_at until after
purgeLegacyTables() succeeds. Concretely: keep calling
validateRoomMigration(ctx, config) and write validation_result, then call
purgeLegacyTables(ctx); only after purgeLegacyTables() returns without error
call writeMetaValue for migration_finished_at, room_migration_done,
message_migration_done and writeMetaValue(ctx, LEGACY_RETENTION_META_KEY,
createLegacyTableRetention('purged')), so failures during purge will allow
retries.
- Around line 462-467: 当前迁移无条件调用 add({conversationId, principalType: 'user',
principalId: room.roomMasterId, permission: 'manage'})，会在 room.roomMasterId
为空时写入 principalId === '' 的脏 ACL；请在调用 add(...) 之前检查 room.roomMasterId 是否非空（例如
room.roomMasterId && room.roomMasterId.length > 0），只有在该条件成立时才插入这条 manage
权限记录，以确保迁移结果与现有校验逻辑一致。
In `@packages/core/src/migration/validators.ts`:
- Around line 57-73: The message counts are currently scanning entire tables and
must use the same room filter as validRooms; update the readTableBatches calls
(for both 'chathub_message' counting legacyMessageCount and 'chatluna_message'
counting migratedLegacyMessageCount) to apply filterValidRooms()’s criteria
(exclude conversationId null/0 and only include conversationIds present in
validRooms) so only messages from valid legacy rooms are counted; preserve the
existing createdAt null check for migratedLegacyMessageCount but restrict the
rows passed into the callback by the same filter logic used earlier.
- Around line 184-196: The validator currently treats every chathub_user's
defaultRoomId as requiring a binding and pushes invalid or skipped room IDs into
missingBindingConversationIds, causing false failures; update the loop over
users (where conversationsByRoomId, missingBindingConversationIds,
createLegacyBindingKey, routeModes, migratedBindingKeys, and missingBindingKeys
are used) to first skip users whose defaultRoomId is falsy or equals 0 or whose
defaultRoomId is not present in conversationsByRoomId (i.e., only process users
with a valid conversation entry), and only then generate the bindingKey and
check migratedBindingKeys — do not push entries for users with invalid/absent
defaultRoomId.
In `@packages/core/src/services/chat.ts`:
- Around line 879-1007: The schema for chatluna_constraint lacks a uniqueness
constraint on name, causing ConversationService.getManagedConstraint() and
updateManagedConstraint() (which locate constraints by name) to be ambiguous
under concurrent writes; add a unique index/constraint on the name column in the
ctx.database.extend call for 'chatluna_constraint' (i.e., make
chatluna_constraint.name unique) and ensure any migrations or DB setup code
applies the unique constraint so create/insert operations will fail on duplicate
names rather than producing multiple rows with the same name.
In `@packages/core/src/services/conversation_runtime.ts`:
- Around line 140-174: The runtime is discarding AIMessage.additional_kwargs
when it returns only { content, additionalReplyMessages }, which loses provider
metadata; update the return to include the original aiMessage.additional_kwargs
(preserving existing additionalReplyMessages and content) so metadata injected
by adapters stays available downstream—locate the aiMessage from
chainValues.message and ensure the returned object includes additional_kwargs:
aiMessage.additional_kwargs (alongside usageMetadata/formatUsageMetadataMessage
and service.currentConfig.showThoughtMessage handling).
In `@packages/core/src/services/conversation.ts`:
- Around line 841-845: Ensure the selected archive actually belongs to the
current conversation: after resolving archive via getArchive(options.archiveId)
or getArchive(conversation.archiveId) or
getArchiveByConversationId(conversation.id), verify archive.conversationId ===
conversation.id (or, if your domain allows null/undefined, use a clear ownership
check) and throw or reject (e.g., return an error) when it does not match; apply
the same ownership validation to the other archive-loading logic in the same
function (the block around lines 877-905) so you never restore messages from an
archive that belongs to a different conversation.
- Around line 481-512: 当前实现把跨 binding 的目标会话写回到当前 route 的
resolved.bindingKey，导致其它 route 的 activeConversationId 被污染；在 conversation 切换路径（函数
resolveTargetConversation -> 使用 setActiveConversation(...) 的位置）改为：当
conversation.bindingKey !== resolved.bindingKey 时，针对目标会话所属的 binding 调用
setActiveConversation(conversation.bindingKey, conversation.id)（而不是始终使用
resolved.bindingKey），并在触发事件 'chatluna/conversation-before-switch' 和
'chatluna/conversation-after-switch' 时传递正确的 bindingKey（使用 resolved.bindingKey
表示原 binding，使用 conversation.bindingKey 表示目标 binding）；同样把相同保护逻辑应用到
reopenConversation() 的非归档分支，避免把跨 binding 的目标会话写回当前 binding。
- Around line 720-808: The archive flow can race with in-flight chat requests
because the runtime lock is only taken in
this.ctx.chatluna.conversationRuntime.clearConversationInterface; wrap the
persistent state changes (creating ArchiveRecord + upsert, touchConversation,
unbindConversation, removing chatluna_message, writing files) and the final
clearConversationInterface call inside a conversation-level synchronization
provided by the runtime (e.g. use a conversation-level lock API on
this.ctx.chatluna.conversationRuntime such as
withConversationLock/withConversationSync or similar), or first call an explicit
stop/abort of active requests on the runtime (e.g.
stopActiveRequests/abortActiveRequests) before performing the DB/file changes
and then call clearConversationInterface; ensure you move the calls to
touchConversation, unbindConversation, ctx.database.remove('chatluna_message'),
ctx.database.upsert('chatluna_archive') and clearConversationInterface into that
synchronized block so no in-flight chat can write messages after
archival/deletion.
- Around line 207-212: The effectivePreset selection currently places
options.presetLane before constraint.fixedPreset, letting callers bypass
admin-fixed presets; update the logic in the effectivePreset assignment (inside
conversation.ts where effectivePreset is computed) so constraint.fixedPreset
takes precedence over options.presetLane (i.e., if constraint.fixedPreset is
set, use it and ignore options.presetLane), then fall back to
allowedConversation?.preset, constraint.defaultPreset, and
this.config.defaultPreset in that order; ensure you reference the symbols
effectivePreset, options.presetLane, constraint.fixedPreset,
allowedConversation?.preset, constraint.defaultPreset, and
this.config.defaultPreset when making the change.
In `@packages/core/test/conversation-runtime.test.ts`:
- Around line 71-93: FakeDatabase.get currently only filters by table/query
which breaks callers that rely on pagination and ordering (validateRoomMigration
-> readTableBatches, runRoomToConversationMigration, allocateConversationSeq,
recordCompression). Enhance FakeDatabase.get to extract and remove special
pagination/sort keys from the incoming query (e.g., $offset, $limit, $sort),
apply sorting to the filtered rows before paging, then apply offset/limit to
return the correct page. Implement $sort handling to accept either an array of
[key, 'asc'|'desc'] tuples or an object mapping key->'asc'|'desc' (compare
values and handle undefined), and ensure $offset/$limit are coerced to integers
and defaulted (offset 0, limit = all) so tests that depend on readTableBatches
and ordering match production behavior.
---
Outside diff comments:
In `@packages/adapter-dify/src/requester.ts`:
- Around line 300-306: The thrown ChatLunaError in the requester (use
ChatLunaError and ChatLunaErrorCode.API_REQUEST_FAILED) contains the wrong
service name ("qwen completion"); update the error message to reference the Dify
adapter/service (e.g., "dify completion" or "Dify API") so logs correctly
identify the failing service—modify the Error passed into the ChatLunaError
constructor where the chunk-based failure is raised.
---
Nitpick comments:
In `@packages/core/src/llm-core/memory/message/database_history.ts`:
- Around line 421-440: The new-conversation creation in _loadConversation uses
hardcoded empty strings and 'system' (create('chatluna_conversation') with
model/preset/chatMode = '' and createdBy = 'system'), which can diverge from
ConversationService-created records; update _loadConversation to either call the
existing ConversationService (or shared factory) to create a conversation with
the same default values, or replace the hardcoded empty strings with the
canonical defaults (or null where appropriate) used by ConversationService so
stored rows remain consistent with other creation paths; ensure you update the
fields model, preset, chatMode, createdBy, and any other defaults (autoTitle,
additional_kwargs, compression) to match ConversationService behavior.
In `@packages/core/src/middlewares/model/resolve_model.ts`:
- Around line 32-38: The conditional that sets modelName in resolve_model.ts
currently checks conversation.model against a hard-coded Chinese string '无';
replace this hard-coded literal by using a configurable/centralized value or
i18n lookup to avoid embedding language-specific text. Update the logic around
modelName and conversation.model to compare against a constant/array (e.g.,
EMPTY_MODEL_VALUES) or call your i18n helper (e.g., i18n.t('empty')) so all
accepted "empty" tokens are configurable and documented, and ensure unit tests
for resolve_model cover the localized empty token.
In `@packages/core/src/utils/koishi.ts`:
- Around line 36-53: The empty catch in checkAdmin swallows permission-check
errors and there is no explicit handling when session.getUser returns null;
change the catch{} around session.app.permissions.test to log the caught error
(use session.app.logger.debug or error) and rethrow or surface it instead of
silently swallowing, and after calling session.getUser explicitly handle a
null/undefined user (e.g. return false for non-existent user or throw a clear
error) rather than relying on user?.authority >= 3; update the function
checkAdmin to reference session.app.permissions.test, the catch block,
session.getUser, and the user.authority check accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2406a86d-49f7-4bf3-aa3e-b05a6a40f685
📒 Files selected for processing (62)
packages/adapter-dify/src/requester.tspackages/core/src/chains/chain.tspackages/core/src/command.tspackages/core/src/commands/chat.tspackages/core/src/commands/conversation.tspackages/core/src/commands/utils.tspackages/core/src/index.tspackages/core/src/llm-core/agent/agent.tspackages/core/src/llm-core/agent/executor.tspackages/core/src/llm-core/agent/sub-agent.tspackages/core/src/llm-core/chat/app.tspackages/core/src/llm-core/chat/default.tspackages/core/src/llm-core/memory/message/database_history.tspackages/core/src/llm-core/platform/service.tspackages/core/src/llm-core/platform/types.tspackages/core/src/llm-core/prompt/context_manager.tspackages/core/src/llm-core/utils/count_tokens.tspackages/core/src/middlewares/chat/chat_time_limit_check.tspackages/core/src/middlewares/chat/chat_time_limit_save.tspackages/core/src/middlewares/chat/read_chat_message.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/conversation/request_conversation.tspackages/core/src/middlewares/conversation/resolve_conversation.tspackages/core/src/middlewares/model/resolve_model.tspackages/core/src/middlewares/model/set_default_embeddings.tspackages/core/src/middlewares/preset/delete_preset.tspackages/core/src/middlewares/system/conversation_manage.tspackages/core/src/middlewares/system/lifecycle.tspackages/core/src/middlewares/system/wipe.tspackages/core/src/migration/legacy_tables.tspackages/core/src/migration/room_to_conversation.tspackages/core/src/migration/validators.tspackages/core/src/services/chat.tspackages/core/src/services/conversation.tspackages/core/src/services/conversation_runtime.tspackages/core/src/services/conversation_types.tspackages/core/src/utils/archive.tspackages/core/src/utils/chat_request.tspackages/core/src/utils/compression.tspackages/core/src/utils/error.tspackages/core/src/utils/koishi.tspackages/core/src/utils/lock.tspackages/core/src/utils/model.tspackages/core/test/conversation-runtime.test.tspackages/extension-agent/client/index.tspackages/extension-agent/src/cli/dispatch.tspackages/extension-agent/src/cli/render.tspackages/extension-agent/src/computer/backends/local/store.tspackages/extension-agent/src/computer/tools/bash.tspackages/extension-agent/src/config/defaults.tspackages/extension-agent/src/service/computer.tspackages/extension-agent/src/service/index.tspackages/extension-agent/src/service/permissions.tspackages/extension-agent/src/service/skills.tspackages/extension-agent/src/service/sub_agent.tspackages/extension-agent/src/skills/scan.tspackages/extension-agent/src/sub-agent/builtin.tspackages/extension-agent/src/sub-agent/render.tspackages/extension-agent/src/sub-agent/run.tspackages/extension-agent/src/sub-agent/session.tspackages/extension-agent/src/sub-agent/tool.tspackages/extension-long-memory/src/plugins/edit_memory.ts
💤 Files with no reviewable changes (2)
- packages/extension-agent/src/sub-agent/render.ts
- packages/extension-agent/src/service/sub_agent.ts
✅ Files skipped from review due to trivial changes (18)
- packages/core/src/llm-core/agent/executor.ts
- packages/extension-agent/src/cli/dispatch.ts
- packages/extension-agent/src/sub-agent/run.ts
- packages/extension-agent/src/computer/backends/local/store.ts
- packages/core/src/llm-core/agent/agent.ts
- packages/extension-agent/src/service/computer.ts
- packages/extension-agent/src/sub-agent/builtin.ts
- packages/extension-agent/src/service/index.ts
- packages/extension-agent/src/cli/render.ts
- packages/extension-agent/src/sub-agent/tool.ts
- packages/extension-agent/src/service/skills.ts
- packages/extension-agent/src/sub-agent/session.ts
- packages/extension-agent/src/config/defaults.ts
- packages/core/src/llm-core/platform/types.ts
- packages/extension-agent/src/computer/tools/bash.ts
- packages/core/src/llm-core/agent/sub-agent.ts
- packages/core/src/utils/chat_request.ts
- packages/extension-agent/client/index.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/core/src/middlewares/model/set_default_embeddings.ts
- packages/core/src/middlewares/chat/chat_time_limit_save.ts
- packages/core/src/utils/error.ts
- packages/core/src/llm-core/platform/service.ts
- packages/core/src/utils/lock.ts
- packages/core/src/command.ts
- packages/core/src/utils/model.ts
- packages/core/src/llm-core/prompt/context_manager.ts
- packages/core/src/index.ts
- packages/core/src/utils/archive.ts
- packages/core/src/utils/compression.ts
- packages/core/src/middlewares/chat/chat_time_limit_check.ts
- packages/core/src/llm-core/chat/default.ts
- packages/core/src/middlewares/conversation/resolve_conversation.ts
- packages/core/src/middlewares/system/lifecycle.ts
- packages/core/src/chains/chain.ts
- packages/extension-long-memory/src/plugins/edit_memory.ts
- packages/core/src/llm-core/chat/app.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/utils/conversation.ts (1)
24-32: 可选：避免先插入空字符串再过滤Line 28 当前会在
seq缺失时先产生''，随后再被 Line 32 过滤。可以直接在flatMap中跳过该值，减少一次无效集合元素构造。建议修改
const expect = Array.from( new Set( - conversations.flatMap((conversation) => [ - conversation.id, - String(conversation.seq ?? ''), - conversation.title - ]) + conversations.flatMap((conversation) => + conversation.seq == null + ? [conversation.id, conversation.title] + : [conversation.id, String(conversation.seq), conversation.title] + ) ) ).filter((item) => item.length > 0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/conversation.ts` around lines 24 - 32, The code builds expect by flatMapping id, seq, and title but currently pushes String(conversation.seq ?? '') which creates empty strings when seq is missing and later filtered out; update the flatMap in the conversations.flatMap callback (the logic that produces the array for each conversation) to only include seq when conversation.seq is not null/undefined (e.g., conditionally add String(conversation.seq) or push it inside an if), keeping the existing inclusion of conversation.id and conversation.title, so no empty '' entries are produced before the final filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/utils/conversation.ts`:
- Around line 24-32: The code builds expect by flatMapping id, seq, and title
but currently pushes String(conversation.seq ?? '') which creates empty strings
when seq is missing and later filtered out; update the flatMap in the
conversations.flatMap callback (the logic that produces the array for each
conversation) to only include seq when conversation.seq is not null/undefined
(e.g., conditionally add String(conversation.seq) or push it inside an if),
keeping the existing inclusion of conversation.id and conversation.title, so no
empty '' entries are produced before the final filter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cf24435c-a3f3-4c60-ab17-44f1816d68bf
📒 Files selected for processing (3)
packages/core/src/commands/chat.tspackages/core/src/commands/conversation.tspackages/core/src/utils/conversation.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/commands/conversation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/commands/chat.ts
Align the core conversation tests with Koishi's Mocha-based setup so they run cleanly in the current CJS test environment. Split the suite by concern and include the runtime/source fixes needed for the new harness.
Keep conversation state, preset routing, and archive flows consistent as the room-to-conversation migration finishes. Tighten cleanup and validation paths so legacy data and admin-only actions behave predictably.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/adapter-dify/src/requester.ts (1)
224-232:⚠️ Potential issue | 🟡 Minor在
message_end事件写缓存前，添加对conversation_id的显式验证。Dify 官方 OpenAPI schema 中，
message_end事件通过StreamEventBase组合定义了conversation_id字段，官方流式响应示例也显示该字段应当存在。然而，schema 并未明确标记该字段为必需。虽然设计上conversation_id应在message_end事件中出现，但由于没有强制要求保证，建议在 line 227 缓存写入前添加对conversation_id的存在性验证，避免字段缺失时导致会话关联丢失。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-dify/src/requester.ts` around lines 224 - 232, Before calling updateDifyConversationId in the message_end branch, explicitly check that data.conversation_id (assigned to updatedDifyConversationId) is non-null/defined; if it's missing, skip the cache write and log or handle the condition (e.g., warn via processLogger or return early) to avoid writing a null/undefined conversation id—update the code around the message_end branch where updatedDifyConversationId is set and used to call updateDifyConversationId(conversationId, config.workflowName, updatedDifyConversationId, difyUser).packages/core/src/middlewares/chat/rollback_chat.ts (1)
100-188:⚠️ Potential issue | 🔴 Critical回滚过程没有和运行中的请求串行化。
这段代码会清 interface、重写
latestMessageId并删除消息，但整个过程没有进入conversationRuntime.withConversationSync(...)，也没有先停掉 active request。若同一会话仍在流式写入，回滚后的旧链会立刻被后续落库重新污染。至少把 Line 102-188 的读写放进 runtime 的会话级同步块，并把 Line 102-104 换成 locked 版的 interface 清理。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 100 - 188, 将对话回滚的读写逻辑（从设置 context.options.conversationId 并调用 ctx.chatluna.conversationRuntime.clearConversationInterface(...) 到最后的 ctx.database.remove('chatluna_message', ...)）包裹到会话级同步块里，使用 conversationRuntime.withConversationSync(...) 执行所有对 conversation 的读写操作，并把对 clearConversationInterface 的调用换成受锁/阻塞写入版本（例如 clearConversationInterfaceLocked 或 conversationRuntime.clearConversationInterfaceLocked(...)），同时在进入同步块前确保通过 conversationRuntime 的接口先停止或阻塞任何 active request，以避免正在流式写入的请求与回滚并发写回旧链。
🧹 Nitpick comments (7)
packages/core/src/middlewares/preset/delete_preset.ts (1)
62-79: 会话迁移逻辑已添加，解决了之前的孤立会话问题 👍删除预设前，代码现在会将所有引用该预设的会话迁移到
nextPreset，这解决了之前评审中提到的会话不可用问题。不过，这里有一个潜在的性能问题：
ctx.database.get('chatluna_conversation', {})会获取所有会话记录，然后在内存中过滤。对于大型系统，这可能会比较慢。如果数据库支持，可以考虑使用$in查询来直接过滤。♻️ 可选优化：使用数据库层过滤
- const conversations = (await ctx.database.get( - 'chatluna_conversation', - {} - )) as ConversationRecord[] - const updatedAt = new Date() - const patched = conversations - .filter((conversation) => - presetTemplate.triggerKeyword.includes(conversation.preset) - ) - .map((conversation) => ({ - ...conversation, - preset: nextPreset, - updatedAt - })) + const conversations = (await ctx.database.get( + 'chatluna_conversation', + { preset: { $in: presetTemplate.triggerKeyword } } + )) as ConversationRecord[] + const updatedAt = new Date() + const patched = conversations.map((conversation) => ({ + ...conversation, + preset: nextPreset, + updatedAt + }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/preset/delete_preset.ts` around lines 62 - 79, The current migration loads every conversation via ctx.database.get('chatluna_conversation', {}) and filters in memory which will scale poorly; change the logic to query the DB for conversations whose preset is in presetTemplate.triggerKeyword (use a $in or equivalent filter supported by your DB) so you only fetch matching records, then build the patched array (preserve updatedAt and set preset to nextPreset) and call ctx.database.upsert('chatluna_conversation', patched) as before; look for usages of ctx.database.get, presetTemplate.triggerKeyword, nextPreset, patched, and ctx.database.upsert to update the query.packages/core/src/llm-core/memory/message/database_history.ts (1)
259-273:updatedMessages缺少createdAt字段在
removeAllToolAndFunctionMessages中构造的updatedMessages对象缺少createdAt字段。虽然 upsert 操作可能不需要所有字段，但为了类型一致性和避免潜在问题，建议保留原始的createdAt值。♻️ 建议添加 createdAt 字段
const updatedMessages = this._serializedChatHistory.map((msg) => ({ id: msg.id, parentId: msg.parentId, content: msg.content, role: msg.role, conversationId: msg.conversationId, name: msg.name, tool_call_id: msg.tool_call_id, tool_calls: msg.tool_calls, additional_kwargs_binary: msg.additional_kwargs_binary, - rawId: msg.rawId + rawId: msg.rawId, + createdAt: msg.createdAt }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/memory/message/database_history.ts` around lines 259 - 273, The constructed updatedMessages array in removeAllToolAndFunctionMessages (using this._serializedChatHistory and assigned to updatedMessages) omits the original createdAt value; update the mapping to include createdAt: msg.createdAt for each message so the objects passed to this._ctx.database.upsert('chatluna_message', updatedMessages) retain the original timestamp and type consistency.packages/core/tests/conversation-e2e.spec.ts (1)
10-13: 临时目录未清理测试创建了临时目录但在测试结束后没有清理。虽然操作系统最终会清理
tmpdir，但显式清理是更好的做法。♻️ 添加目录清理
} finally { await app.stop() + await fs.rm(dir, { recursive: true, force: true }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/conversation-e2e.spec.ts` around lines 10 - 13, 测试中通过 fs.mkdtemp 创建的临时目录（变量 dir）未被清理；在测试结束时添加清理逻辑以删除该目录。修改 conversation-e2e.spec.ts：在使用 createMemoryService(...) 后，将 dir 保存在测试上下文并在 afterEach 或 afterAll 钩子中调用 fs.promises.rm(dir, { recursive: true, force: true })（或等效的 fs.rmSync）以递归删除临时目录，确保即使测试失败也在 finally/钩子中执行清理；参考符号：dir、fs.mkdtemp、createMemoryService、afterEach/afterAll。packages/core/tests/conversation-source.spec.ts (2)
27-46: 硬编码的跨包路径可能导致脆弱的测试这些测试依赖于硬编码的相对路径来访问其他扩展包的源文件。如果包结构或文件位置发生变化，测试会失败但可能不是因为实际的代码问题。
建议考虑：
- 使用 glob 模式动态查找文件
- 或在测试描述中注明这些路径依赖
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/conversation-source.spec.ts` around lines 27 - 46, The test currently hardcodes relative paths in the files array to inspect other extensions' source, which is fragile; replace the static files list with a glob-based discovery (e.g., using fast-glob or glob) to find the target files under ../../extension-*/src/** (or narrow to service/*.ts and plugins/*.ts) and iterate those results in the same loop, keeping the existing assertions that check for the event strings 'chatluna/clear-chat-history' and 'chatluna/conversation-after-clear-history'; alternatively, if you prefer not to change discovery, add a clear comment in the test describing the path dependency and why it must be kept, referencing this spec file and the checked filenames (memory.ts, skills.ts, service.ts, todos.ts) so future maintainers understand the coupling.
48-58: 正则表达式匹配可能因格式变化而失效测试依赖于精确的代码格式（如
for (const table of LEGACY_MIGRATION_TABLES)）。如果代码格式化工具修改了空格或换行，测试可能会失败。建议使用更宽松的匹配模式或只检查关键标识符的存在。
♻️ 更健壮的匹配方案
- assert.match(source, /for \(const table of LEGACY_MIGRATION_TABLES\)/) - assert.match(source, /for \(const table of LEGACY_RUNTIME_TABLES\)/) + assert.match(source, /for\s*\(\s*const\s+table\s+of\s+LEGACY_MIGRATION_TABLES\s*\)/) + assert.match(source, /for\s*\(\s*const\s+table\s+of\s+LEGACY_RUNTIME_TABLES\s*\)/)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/conversation-source.spec.ts` around lines 48 - 58, The test currently relies on exact spacing in the source regexes (e.g. /for \(const table of LEGACY_MIGRATION_TABLES\)/) which will break if formatting changes; update the assertions to use more flexible patterns that allow arbitrary whitespace around tokens (e.g. use a regex like /for\s*\(\s*const\s+table\s+of\s+LEGACY_MIGRATION_TABLES\s*\)/) and do the same for the LEGACY_RUNTIME_TABLES loop check, or alternatively remove the exact loop regex and only assert presence of the identifiers LEGACY_MIGRATION_TABLES and LEGACY_RUNTIME_TABLES plus a loose loop check; update the test assertions that reference LEGACY_MIGRATION_TABLES and LEGACY_RUNTIME_TABLES accordingly.packages/core/src/migration/room_to_conversation.ts (1)
393-396: 移除多余的括号（静态分析提示）Prettier 检测到
(current?.lastConversationId ?? null)外层有多余的括号。♻️ 格式修复
const lastConversationId = prevActive != null && prevActive !== conversation.id ? prevActive - : (current?.lastConversationId ?? null) + : current?.lastConversationId ?? null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/migration/room_to_conversation.ts` around lines 393 - 396, 在计算 lastConversationId 时移除多余的外层括号：替换当前三元表达式中的 `(current?.lastConversationId ?? null)` 为 `current?.lastConversationId ?? null`，保持变量名 prevActive、conversation.id 和 current 不变以修复静态分析/Prettier 报告的多余括号问题并保持原有逻辑。packages/core/src/migration/legacy_tables.ts (1)
45-54: 字符串匹配方式可能产生误判
isMissingTableError使用宽泛的字符串匹配（如'not found'）可能会错误地将其他类型的错误识别为"表不存在"错误。例如，"configuration not found" 或 "user not found" 等错误也会被匹配。建议考虑更严格的匹配模式，或者在上层调用时做额外验证。
♻️ 可选的更严格匹配方案
function isMissingTableError(error: unknown) { const message = String(error).toLowerCase() return ( message.includes('no such table') || message.includes('unknown table') || - message.includes('not found') || + message.includes('table not found') || + message.includes('relation') && message.includes('does not exist') || message.includes("doesn't exist") || message.includes('does not exist') ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/migration/legacy_tables.ts` around lines 45 - 54, The isMissingTableError function is too permissive (e.g., matching 'not found') and can misclassify unrelated errors; tighten detection by matching database-specific error codes or structured properties when available and/or use a stricter regex that requires the word "table" (e.g., /\b(table).*(not found|does not exist|no such)\b/i) or check for DB error classes/properties (e.g., error.code, error.errno, or instanceof SqliteError) before falling back to string checks; update isMissingTableError to prefer these structured checks and document that callers should perform additional validation if they rely on ambiguous messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adapter-dify/src/requester.ts`:
- Around line 744-748: 在构造 cacheKey 前先校验 config 是否存在并在为空时立即返回（或安全处理），避免在访问
config.workflowName 导致运行时异常从而中断后续 dispose 流程；具体在使用 conversationId 和
config.workflowName 构建 cacheKey（变量名 cacheKey）并调用 this.ctx.chatluna.cache.get
之前，检查 config（和必要的字段如 workflowName），如果为 null/undefined，则记录日志并早返回或使用安全默认值以保证
dispose 不会在未知模型下崩溃。
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 81-98: The current checks use resolvedContext (from the current
route binding) but must instead fetch the target conversation's constraint using
its bindingKey before enforcing admin/lock checks: after resolving the initial
context (resolvedContext) and before the manageMode/lockConversation checks,
call ctx.chatluna.conversation.resolveContext(session, { conversationId:
conversation.id, presetLane: context.options.presetLane, bindingKey:
conversation.bindingKey }) or otherwise resolve the context using
conversation.bindingKey to get the target constraint, then use that returned
constraint to evaluate resolvedConstraint.constraint.manageMode and
resolvedConstraint.constraint.lockConversation and only then run
checkAdmin(session) and set context.message/return ChainMiddlewareRunStatus.STOP
as appropriate.
In `@packages/core/src/middlewares/chat/stop_chat.ts`:
- Around line 61-77: The code is resolving constraints using the current route's
binding instead of the target conversation's binding; change the resolveContext
call to resolve against the target conversation's binding by passing
conversation.bindingKey (e.g. call
ctx.chatluna.conversation.resolveContext(session, { conversationId:
conversation.id, bindingKey: conversation.bindingKey })) and then use that
resulting resolvedContext.constraint for the manageMode and lockConversation
checks (keep using checkAdmin(session) as before). Ensure all references to
resolvedContext in this block use the target-binding-resolved context so the
target conversation's admin/lock rules are enforced.
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Line 87: The call to fs.rm(presetTemplate.path) lacks error handling and can
crash after the DB migration; wrap the await fs.rm(presetTemplate.path) in a
try/catch, and in the catch call logger.error(e) to record the failure; do not
let the exception abort the surrounding flow (so the preset migration/database
changes remain consistent) unless you explicitly want to fail—if you must fail
instead, rethrow after logging.
In `@packages/core/src/services/conversation.ts`:
- Around line 656-676: The export path currently calls
exportMarkdown(conversation) which only reads chatluna_message rows, so archived
conversations (whose messages are removed during archive) export only metadata;
update exportMarkdown (or add a helper used by exportConversation) to detect
conversation.status === 'archived' and load message content from the
conversation's archive payload (instead of chatluna_message) when present;
ensure the exported markdown includes archived messages and metadata
consistently, and add a regression test that archives a conversation then calls
exportConversation({... includeArchived: true ...}) to assert the full message
body is present.
- Around line 478-500: The code verifies manage permission using the current
route's resolved.constraint but then acts on a possibly different conversation
binding (resolved via resolveTargetConversation and
getManagedConstraintByBindingKey), so the target binding's manageMode isn't
enforced; update the flow after obtaining target (from
getManagedConstraintByBindingKey(conversation.bindingKey)) to also enforce its
admin gate by calling assertManageAllowed(session, targetConstraint) or
otherwise re-checking manageMode from target (e.g., prefer target?.manageMode
over resolved.constraint.manageMode) before proceeding with actions like
switch/archive/restore/delete/rename/update/export; apply this same fix in all
similar blocks (the other ranges noted) so authorization uses the target
binding's constraint rather than only resolved.constraint.
- Around line 1334-1356: The exact-ID branch in switchConversation logic returns
a Conversation from getConversation without checking its status, allowing
deleted/broken (and potentially archived) conversations to be reopened; update
the branch (the options.conversationId path that calls getConversation and
hasConversationPermission) to reject conversations whose conversation.status is
'deleted' or 'broken' (and apply the same default archived-filter behavior as
list/fuzzy paths — i.e., treat archived as excluded unless an explicit
includeArchived flag is set), returning null or throwing as appropriate. Ensure
checks reference the retrieved conversation object (conversation.status) and
preserve the existing permission check via hasConversationPermission.
---
Outside diff comments:
In `@packages/adapter-dify/src/requester.ts`:
- Around line 224-232: Before calling updateDifyConversationId in the
message_end branch, explicitly check that data.conversation_id (assigned to
updatedDifyConversationId) is non-null/defined; if it's missing, skip the cache
write and log or handle the condition (e.g., warn via processLogger or return
early) to avoid writing a null/undefined conversation id—update the code around
the message_end branch where updatedDifyConversationId is set and used to call
updateDifyConversationId(conversationId, config.workflowName,
updatedDifyConversationId, difyUser).
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 100-188: 将对话回滚的读写逻辑（从设置 context.options.conversationId 并调用
ctx.chatluna.conversationRuntime.clearConversationInterface(...) 到最后的
ctx.database.remove('chatluna_message', ...)）包裹到会话级同步块里，使用
conversationRuntime.withConversationSync(...) 执行所有对 conversation 的读写操作，并把对
clearConversationInterface 的调用换成受锁/阻塞写入版本（例如 clearConversationInterfaceLocked 或
conversationRuntime.clearConversationInterfaceLocked(...)），同时在进入同步块前确保通过
conversationRuntime 的接口先停止或阻塞任何 active request，以避免正在流式写入的请求与回滚并发写回旧链。
---
Nitpick comments:
In `@packages/core/src/llm-core/memory/message/database_history.ts`:
- Around line 259-273: The constructed updatedMessages array in
removeAllToolAndFunctionMessages (using this._serializedChatHistory and assigned
to updatedMessages) omits the original createdAt value; update the mapping to
include createdAt: msg.createdAt for each message so the objects passed to
this._ctx.database.upsert('chatluna_message', updatedMessages) retain the
original timestamp and type consistency.
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Around line 62-79: The current migration loads every conversation via
ctx.database.get('chatluna_conversation', {}) and filters in memory which will
scale poorly; change the logic to query the DB for conversations whose preset is
in presetTemplate.triggerKeyword (use a $in or equivalent filter supported by
your DB) so you only fetch matching records, then build the patched array
(preserve updatedAt and set preset to nextPreset) and call
ctx.database.upsert('chatluna_conversation', patched) as before; look for usages
of ctx.database.get, presetTemplate.triggerKeyword, nextPreset, patched, and
ctx.database.upsert to update the query.
In `@packages/core/src/migration/legacy_tables.ts`:
- Around line 45-54: The isMissingTableError function is too permissive (e.g.,
matching 'not found') and can misclassify unrelated errors; tighten detection by
matching database-specific error codes or structured properties when available
and/or use a stricter regex that requires the word "table" (e.g.,
/\b(table).*(not found|does not exist|no such)\b/i) or check for DB error
classes/properties (e.g., error.code, error.errno, or instanceof SqliteError)
before falling back to string checks; update isMissingTableError to prefer these
structured checks and document that callers should perform additional validation
if they rely on ambiguous messages.
In `@packages/core/src/migration/room_to_conversation.ts`:
- Around line 393-396: 在计算 lastConversationId 时移除多余的外层括号：替换当前三元表达式中的
`(current?.lastConversationId ?? null)` 为 `current?.lastConversationId ??
null`，保持变量名 prevActive、conversation.id 和 current 不变以修复静态分析/Prettier
报告的多余括号问题并保持原有逻辑。
In `@packages/core/tests/conversation-e2e.spec.ts`:
- Around line 10-13: 测试中通过 fs.mkdtemp 创建的临时目录（变量 dir）未被清理；在测试结束时添加清理逻辑以删除该目录。修改
conversation-e2e.spec.ts：在使用 createMemoryService(...) 后，将 dir 保存在测试上下文并在
afterEach 或 afterAll 钩子中调用 fs.promises.rm(dir, { recursive: true, force: true
})（或等效的 fs.rmSync）以递归删除临时目录，确保即使测试失败也在
finally/钩子中执行清理；参考符号：dir、fs.mkdtemp、createMemoryService、afterEach/afterAll。
In `@packages/core/tests/conversation-source.spec.ts`:
- Around line 27-46: The test currently hardcodes relative paths in the files
array to inspect other extensions' source, which is fragile; replace the static
files list with a glob-based discovery (e.g., using fast-glob or glob) to find
the target files under ../../extension-*/src/** (or narrow to service/*.ts and
plugins/*.ts) and iterate those results in the same loop, keeping the existing
assertions that check for the event strings 'chatluna/clear-chat-history' and
'chatluna/conversation-after-clear-history'; alternatively, if you prefer not to
change discovery, add a clear comment in the test describing the path dependency
and why it must be kept, referencing this spec file and the checked filenames
(memory.ts, skills.ts, service.ts, todos.ts) so future maintainers understand
the coupling.
- Around line 48-58: The test currently relies on exact spacing in the source
regexes (e.g. /for \(const table of LEGACY_MIGRATION_TABLES\)/) which will break
if formatting changes; update the assertions to use more flexible patterns that
allow arbitrary whitespace around tokens (e.g. use a regex like
/for\s*\(\s*const\s+table\s+of\s+LEGACY_MIGRATION_TABLES\s*\)/) and do the same
for the LEGACY_RUNTIME_TABLES loop check, or alternatively remove the exact loop
regex and only assert presence of the identifiers LEGACY_MIGRATION_TABLES and
LEGACY_RUNTIME_TABLES plus a loose loop check; update the test assertions that
reference LEGACY_MIGRATION_TABLES and LEGACY_RUNTIME_TABLES accordingly.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1fc3d534-5561-4b45-84a9-ccee348cd873
⛔ Files ignored due to path filters (6)
package.jsonis excluded by!**/*.jsonpackages/core/package.jsonis excluded by!**/*.jsonpackages/core/src/locales/en-US.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.ymlis excluded by!**/*.ymlpackages/core/tests/tsconfig.jsonis excluded by!**/*.jsonyakumo.ymlis excluded by!**/*.yml
📒 Files selected for processing (23)
packages/adapter-dify/src/requester.tspackages/core/src/llm-core/agent/react/output_parser.tspackages/core/src/llm-core/memory/message/database_history.tspackages/core/src/middlewares/chat/read_chat_message.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/chat/stop_chat.tspackages/core/src/middlewares/preset/delete_preset.tspackages/core/src/middlewares/system/wipe.tspackages/core/src/migration/legacy_tables.tspackages/core/src/migration/room_to_conversation.tspackages/core/src/migration/validators.tspackages/core/src/services/chat.tspackages/core/src/services/conversation.tspackages/core/src/services/conversation_runtime.tspackages/core/src/utils/koishi.tspackages/core/tests/conversation-archive.spec.tspackages/core/tests/conversation-e2e.spec.tspackages/core/tests/conversation-migration.spec.tspackages/core/tests/conversation-runtime.spec.tspackages/core/tests/conversation-service.spec.tspackages/core/tests/conversation-source.spec.tspackages/core/tests/conversation-utils.spec.tspackages/core/tests/helpers.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/llm-core/agent/react/output_parser.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/utils/koishi.ts
- packages/core/src/services/conversation_runtime.ts
- packages/core/src/migration/validators.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 7 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 4 file(s) based on 7 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
packages/core/src/middlewares/preset/delete_preset.ts (1)
57-59: 确认输入建议改为大小写不敏感Line 57 仅接受
'Y'，用户输入y或带空白会被误判为取消，影响交互体验。建议修改
- if (result !== 'Y') { + if (result.trim().toUpperCase() !== 'Y') { context.message = session.text('.cancelled', [presetName]) return ChainMiddlewareRunStatus.STOP }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/preset/delete_preset.ts` around lines 57 - 59, The prompt currently only accepts 'Y' exactly which treats 'y' or inputs with surrounding whitespace as cancel; update the confirmation check in delete_preset.ts to perform a case-insensitive, trimmed comparison of the user's response (e.g. normalize result with result.trim().toUpperCase()) before comparing to 'Y', so the existing branch that sets context.message to session.text('.cancelled', [presetName]) and returns ChainMiddlewareRunStatus.STOP still triggers correctly for any capitalization or extra whitespace.packages/core/src/services/conversation.ts (5)
1016-1037: 已归档会话导出逻辑已改进，但存在类型转换隐患。归档会话现在会从存档文件读取消息内容，解决了之前评审中提到的问题。但
as unknown as MessageRecord[]的类型断言可能会掩盖潜在的类型不匹配问题，特别是SerializedMessageRecord与MessageRecord之间可能存在的其他字段类型差异（如content、additional_kwargs_binary等二进制字段）。建议使用
deserializeMessage函数来保持与restoreConversation中一致的反序列化逻辑：建议修复
try { const payload = await this.readArchivePayload(archive.path) - messages = payload.messages.map((msg) => ({ - ...msg, - createdAt: new Date(msg.createdAt), - conversationId: conversation.id - })) as unknown as MessageRecord[] + messages = payload.messages.map((msg) => ({ + ...deserializeMessage(msg), + conversationId: conversation.id + })) } catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 1016 - 1037, The exportMarkdown flow casts archived payload messages with "as unknown as MessageRecord[]" which can hide mismatches; instead, when reading messages from readArchivePayload(archive.path) in exportMarkdown, map payload.messages through the same deserializeMessage used by restoreConversation to properly convert SerializedMessageRecord fields (e.g., content, additional_kwargs_binary) into MessageRecord, then set createdAt = new Date(msg.createdAt) and conversationId = conversation.id (falling back to listMessages if deserialization fails) so types and binary fields are handled consistently with restoreConversation.
1880-1887: 锁对象未清理可能导致内存泄漏。
_bindingLocks和_titleLocks中的锁对象会持续累积，从不被清理。虽然在实际使用中 binding key 和 conversation id 的数量相对有限，但长时间运行可能积累大量已不活跃的锁。可以考虑使用 WeakMap 或添加定期清理机制，但鉴于实际影响有限，这是一个可选的改进。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 1880 - 1887, The getLock function creates and stores ObjectLock instances in maps (used by _bindingLocks and _titleLocks) but never removes them, risking memory growth; update the implementation to avoid unbounded accumulation by either switching those maps to WeakMap where keys are objects (or use wrapper objects for keys) or implement a cleanup strategy inside getLock/ObjectLock (e.g., track last-used timestamp and remove stale locks, or remove the lock when its held count reaches zero) and reference getLock and ObjectLock (and the _bindingLocks/_titleLocks usages) when applying the change so locks are created only when needed and removed when inactive.
397-401: 静态分析：多余的括号。静态分析工具提示此处有多余的括号，但这不影响功能正确性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 397 - 401, Remove the unnecessary parentheses around the nullish-coalescing expression in the object assignment for lastConversationId; specifically update the expression setting lastConversationId (which currently uses prev, conversationId and (current?.lastConversationId ?? null)) to drop the surrounding parentheses so it reads prev != null && prev !== conversationId ? prev : current?.lastConversationId ?? null, leaving updatedAt untouched.
651-655: ACL 删除可优化为批量操作。当前实现在循环中逐条调用
ctx.database.remove，如果需要删除多条 ACL 记录，会产生多次数据库往返。可以考虑使用$in查询或其他批量删除方式。建议优化
- for (const item of removed) { - await this.ctx.database.remove('chatluna_acl', item) + if (removed.length > 0) { + for (const item of removed) { + await this.ctx.database.remove('chatluna_acl', { + conversationId: item.conversationId, + principalType: item.principalType, + principalId: item.principalId, + permission: item.permission + }) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 651 - 655, The loop deletes ACL entries one-by-one (for (const item of removed) await this.ctx.database.remove('chatluna_acl', item)) causing many DB round-trips; replace it with a single bulk delete that targets all removed IDs in one query (e.g., collect the primary key or id values from removed, then call this.ctx.database.remove once with a query using $in and conversationId as needed), and keep the final return this.listAcl(conversationId); update the call site to use the correct key name from the removed objects and ensure the query includes conversationId to avoid accidental global deletes.
1552-1590: 多次顺序数据库查询可合并优化。
findAccessibleConversations方法中对chatluna_acl表进行了多次顺序查询（按 user/guild 和 manage/view 权限组合）。可以考虑使用$or查询条件合并这些请求以减少数据库往返。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 1552 - 1590, The current code in findAccessibleConversations issues multiple sequential ctx.database.get('chatluna_acl', ...) calls to collect ACL records into acl for different principalType/principalId/permission combos; replace those repeated calls with a single combined query using an $or array of predicates (each predicate specifying principalType, principalId and permission) so you fetch all matching ACLRecord items in one ctx.database.get call, cast the result to ACLRecord[] and assign/concat into acl, and ensure you build the predicates conditionally (include guild predicates only if guildId != null and include view permissions only when required === 'view').packages/adapter-dify/src/requester.ts (1)
761-766: 空 catch 块静默忽略 JSON 解析错误。如果缓存中的数据不是有效 JSON，解析错误会被静默忽略。虽然后续使用了默认值
'chatluna'，但记录一条 debug 日志有助于排查问题。建议修复
if (cached != null) { try { difyUser = (JSON.parse(cached) as { user?: string }).user ?? 'chatluna' - } catch {} + } catch { + this.ctx.logger.debug('Dify dispose: failed to parse cached user info') + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-dify/src/requester.ts` around lines 761 - 766, The empty catch silently swallows JSON.parse errors when reading cached; update the block around cached/difyUser to catch the parse error, set difyUser to the default 'chatluna' and emit a debug-level log including the parse error and the offending cached string (e.g., logger.debug or processLogger.debug) so failures to parse are visible; keep the try/catch around JSON.parse in the same location and only add the debug logging inside the catch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/middleware.ts`:
- Line 104: 文件 packages/core/src/middleware.ts 在末尾缺少换行符（当前文件以单个 "}"
结束），导致静态分析/prettier 报错；请在该文件末尾添加一个单一的换行符以确保以 newline 结尾（不要添加多余空行），保存后用项目的
prettier/formatter 检查确认通过。
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Line 109: 文件末尾缺少换行（EOF newline）——在
packages/core/src/middlewares/preset/delete_preset.ts 的文件末尾（紧跟最后一个右花括号 '}'，即
delete preset 中间件/函数的结尾）添加一个换行符即可，保存后静态检查应通过。
---
Nitpick comments:
In `@packages/adapter-dify/src/requester.ts`:
- Around line 761-766: The empty catch silently swallows JSON.parse errors when
reading cached; update the block around cached/difyUser to catch the parse
error, set difyUser to the default 'chatluna' and emit a debug-level log
including the parse error and the offending cached string (e.g., logger.debug or
processLogger.debug) so failures to parse are visible; keep the try/catch around
JSON.parse in the same location and only add the debug logging inside the catch.
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Around line 57-59: The prompt currently only accepts 'Y' exactly which treats
'y' or inputs with surrounding whitespace as cancel; update the confirmation
check in delete_preset.ts to perform a case-insensitive, trimmed comparison of
the user's response (e.g. normalize result with result.trim().toUpperCase())
before comparing to 'Y', so the existing branch that sets context.message to
session.text('.cancelled', [presetName]) and returns
ChainMiddlewareRunStatus.STOP still triggers correctly for any capitalization or
extra whitespace.
In `@packages/core/src/services/conversation.ts`:
- Around line 1016-1037: The exportMarkdown flow casts archived payload messages
with "as unknown as MessageRecord[]" which can hide mismatches; instead, when
reading messages from readArchivePayload(archive.path) in exportMarkdown, map
payload.messages through the same deserializeMessage used by restoreConversation
to properly convert SerializedMessageRecord fields (e.g., content,
additional_kwargs_binary) into MessageRecord, then set createdAt = new
Date(msg.createdAt) and conversationId = conversation.id (falling back to
listMessages if deserialization fails) so types and binary fields are handled
consistently with restoreConversation.
- Around line 1880-1887: The getLock function creates and stores ObjectLock
instances in maps (used by _bindingLocks and _titleLocks) but never removes
them, risking memory growth; update the implementation to avoid unbounded
accumulation by either switching those maps to WeakMap where keys are objects
(or use wrapper objects for keys) or implement a cleanup strategy inside
getLock/ObjectLock (e.g., track last-used timestamp and remove stale locks, or
remove the lock when its held count reaches zero) and reference getLock and
ObjectLock (and the _bindingLocks/_titleLocks usages) when applying the change
so locks are created only when needed and removed when inactive.
- Around line 397-401: Remove the unnecessary parentheses around the
nullish-coalescing expression in the object assignment for lastConversationId;
specifically update the expression setting lastConversationId (which currently
uses prev, conversationId and (current?.lastConversationId ?? null)) to drop the
surrounding parentheses so it reads prev != null && prev !== conversationId ?
prev : current?.lastConversationId ?? null, leaving updatedAt untouched.
- Around line 651-655: The loop deletes ACL entries one-by-one (for (const item
of removed) await this.ctx.database.remove('chatluna_acl', item)) causing many
DB round-trips; replace it with a single bulk delete that targets all removed
IDs in one query (e.g., collect the primary key or id values from removed, then
call this.ctx.database.remove once with a query using $in and conversationId as
needed), and keep the final return this.listAcl(conversationId); update the call
site to use the correct key name from the removed objects and ensure the query
includes conversationId to avoid accidental global deletes.
- Around line 1552-1590: The current code in findAccessibleConversations issues
multiple sequential ctx.database.get('chatluna_acl', ...) calls to collect ACL
records into acl for different principalType/principalId/permission combos;
replace those repeated calls with a single combined query using an $or array of
predicates (each predicate specifying principalType, principalId and permission)
so you fetch all matching ACLRecord items in one ctx.database.get call, cast the
result to ACLRecord[] and assign/concat into acl, and ensure you build the
predicates conditionally (include guild predicates only if guildId != null and
include view permissions only when required === 'view').
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cfea233d-df52-443f-af6d-848175abe699
📒 Files selected for processing (4)
packages/adapter-dify/src/requester.tspackages/core/src/middleware.tspackages/core/src/middlewares/preset/delete_preset.tspackages/core/src/services/conversation.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Resolve stop and rollback checks against the selected conversation binding so cross-route admin and lock rules are applied consistently.
![codefactor-io[bot]](https://avatars.githubusercontent.com/in/25603?s=60&v=4){kind=small}
Co-authored-by: codefactor-io[bot] <47775046+codefactor-io[bot]@users.noreply.github.com>
Add trailing newlines to touched source files so the working tree stays clean and consistent across builds.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/middlewares/chat/stop_chat.ts (1)
89-96:⚠️ Potential issue | 🟡 Minor
stopRequest返回布尔值，status === null永远不会为真。根据
packages/core/src/services/conversation_runtime.ts:335-345，stopRequest(requestId)返回boolean（true表示成功，false表示未找到）。status === null的检查是死代码。🔧 建议简化状态检查
const status = await ctx.chatluna.conversationRuntime.stopRequest(requestId) - if (status === null) { - context.message = session.text('.no_active_chat') - } else if (!status) { + if (!status) { context.message = session.text('.stop_failed') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/stop_chat.ts` around lines 89 - 96, stopRequest returns a boolean so the null check is dead code; remove the status === null branch and treat status as boolean: call stopRequest(requestId) into status, and if (!status) set context.message = session.text('.no_active_chat') (since false means not found), otherwise handle the successful stop case (set a success message or clear context.message) — update the code around stopRequest/ status / context.message to reflect this boolean logic.
♻️ Duplicate comments (1)
packages/core/src/middleware.ts (1)
104-104:⚠️ Potential issue | 🟡 Minor补上文件末尾换行。
Line 104 这里仍然缺少文件末尾换行，当前 lint / prettier 会继续失败。
🔧 修复建议
for (const middleware of middlewares) { await middleware(ctx, config, ctx.chatluna.chatChain) } -} +} +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middleware.ts` at line 104, The file middleware.ts is missing a trailing newline at EOF; add a single newline character after the final closing brace '}' at the end of the file so linters/prettier stop failing (ensure the file ends with a newline, not extra spaces).
🧹 Nitpick comments (5)
packages/core/src/services/conversation.ts (2)
1057-1061:deserializeMessage返回的类型与MessageRecord不完全匹配。
payload.messages经过map后直接断言为MessageRecord[]，但SerializedMessageRecord的createdAt是字符串，转换后的对象缺少完整的类型安全。当前实现手动转换了createdAt，但使用as unknown as MessageRecord[]是一个类型安全的隐患。♻️ 建议使用 deserializeMessage 保持一致性
messages = payload.messages.map((msg) => ({ - ...msg, - createdAt: new Date(msg.createdAt), + ...deserializeMessage(msg), conversationId: conversation.id - })) as unknown as MessageRecord[] + }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 1057 - 1061, The code unsafely asserts payload.messages.map(...) as MessageRecord[]—instead of a blanket cast, call the existing deserializeMessage function for each item (e.g., map payload.messages through deserializeMessage) to convert SerializedMessageRecord -> MessageRecord and ensure createdAt and other fields are typed correctly; replace the manual createdAt conversion and the "as unknown as MessageRecord[]" cast with a proper map to deserializeMessage so the messages variable is a true MessageRecord[].
684-689:removeAcl中循环删除效率较低。当
records数组较大时，逐条调用ctx.database.remove会产生大量数据库往返。可以考虑批量删除。♻️ 可选优化：批量删除
for (const item of removed) { - await this.ctx.database.remove('chatluna_acl', item) + await this.ctx.database.remove('chatluna_acl', { + conversationId: item.conversationId, + principalType: item.principalType, + principalId: item.principalId, + permission: item.permission + }) }或者如果 ACL 记录有唯一 ID，可以使用
$in批量删除。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 684 - 689, The loop in removeAcl that awaits this.ctx.database.remove for each item (variables: removeAcl, removed, this.ctx.database.remove) is inefficient for large records; replace the per-item deletes with a single bulk delete call (e.g., use a database API that accepts a filter like { id: { $in: removed.map(r => r.id) } } or a batchRemove/transaction method) and then return this.listAcl(conversationId) as before; ensure you handle empty removed arrays and propagate/throw DB errors consistently.packages/core/src/middlewares/chat/rollback_chat.ts (1)
14-24:decodeMessageContent的异常处理较为宽松。当
gzipDecode或JSON.parse失败时，函数默默返回message.text ?? ''。这对于回滚操作来说是合理的容错，但可能隐藏数据损坏问题。考虑在 catch 块中添加日志记录。♻️ 建议添加警告日志
async function decodeMessageContent(message: MessageRecord) { try { return JSON.parse( message.content ? await gzipDecode(message.content) : (message.text ?? '""') ) } catch { + logger.warn(`Failed to decode message content: ${message.id}`) return message.text ?? '' } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 14 - 24, The decodeMessageContent function currently swallows errors from gzipDecode/JSON.parse; update its catch to capture the error (e.g., catch (err)) and emit a warning log including the error and identifying info from the MessageRecord (such as message.id or message.metadata) along with the raw message.text/content to aid debugging; keep the existing fallback return (message.text ?? '') but ensure the warning uses the project's logger (e.g., logger.warn or processLogger.warn) rather than console so logs integrate with existing telemetry.packages/core/src/middlewares/preset/delete_preset.ts (1)
62-79: 加载全部会话进行过滤可能影响大规模部署性能。当前实现加载所有
chatluna_conversation记录后在内存中过滤。对于拥有大量会话的部署，这可能造成性能问题。如果数据库支持，可以考虑在查询时直接过滤preset字段。♻️ 可选优化：数据库层面过滤
如果 Koishi 的数据库驱动支持
$in操作符：- const conversations = (await ctx.database.get( - 'chatluna_conversation', - {} - )) as ConversationRecord[] - const updatedAt = new Date() - const patched = conversations - .filter((conversation) => - presetTemplate.triggerKeyword.includes(conversation.preset) - ) + const conversations = (await ctx.database.get( + 'chatluna_conversation', + { + preset: { $in: presetTemplate.triggerKeyword } + } + )) as ConversationRecord[] + const updatedAt = new Date() + const patched = conversations .map((conversation) => ({ ...conversation, preset: nextPreset, updatedAt }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/preset/delete_preset.ts` around lines 62 - 79, The current code fetches all chatluna_conversation rows then filters in memory which will not scale; change the logic in the deletion flow so you query the database for only conversations whose preset is in presetTemplate.triggerKeyword (use ctx.database.get / query with an "$in" or equivalent operator for the preset field) and then map those returned rows to set preset = nextPreset and updatedAt before calling ctx.database.upsert('chatluna_conversation', patched); reference the existing identifiers ctx.database.get, chatluna_conversation, presetTemplate.triggerKeyword, nextPreset, patched, and ctx.database.upsert when making this change.packages/adapter-dify/src/requester.ts (1)
240-267:_workflowStream与_agentStream的difyUser处理方式不一致。
_agentStream接收difyUser作为参数（line 132），而_workflowStream仍在内部调用resolveDifyUser(params)（line 246）。建议统一两者的处理模式，由调用方传入difyUser。♻️ 建议修改
private async *_workflowStream( params: ModelRequestParams, conversationId: string | undefined, - config: { apiKey: string; workflowName: string; workflowType: string } + config: { apiKey: string; workflowName: string; workflowType: string }, + difyUser: string ): AsyncGenerator<ChatGenerationChunk> { const lastMessage = params.input[params.input.length - 1] as | BaseMessage | undefined - const difyUser = this.resolveDifyUser(params) const { files, chatlunaMultimodal } = await this.prepareFiles(调用处相应修改：
- iter = this._workflowStream(params, conversationId, config) + iter = this._workflowStream(params, conversationId, config, difyUser)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-dify/src/requester.ts` around lines 240 - 267, The _workflowStream function currently calls resolveDifyUser(params) internally while _agentStream accepts difyUser as a parameter; make them consistent by changing _workflowStream to accept a difyUser parameter (matching _agentStream) and remove the internal call to resolveDifyUser; update usages inside _workflowStream to pass that difyUser into prepareFiles and the request body (replace internal resolveDifyUser usage), and adjust any callers to pass the resolved difyUser; ensure related helpers referenced (prepareFiles, buildInputs) still receive the difyUser where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/adapter-dify/src/requester.ts`:
- Line 792: 该文件在末尾缺少换行符导致 CI 失败；请在 packages/adapter-dify/src/requester.ts
文件的末尾（文件结束处）添加一个单独的空行/换行符，确保文件以换行结束以通过静态检查和流水线验证。
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Line 109: 文件末尾缺少换行符：在 packages/core/src/middlewares/preset/delete_preset.ts
的文件结束处（在最后一个右花括号 `}` 之后）添加一个换行符以满足静态分析工具要求并确保文件以换行结尾。
---
Outside diff comments:
In `@packages/core/src/middlewares/chat/stop_chat.ts`:
- Around line 89-96: stopRequest returns a boolean so the null check is dead
code; remove the status === null branch and treat status as boolean: call
stopRequest(requestId) into status, and if (!status) set context.message =
session.text('.no_active_chat') (since false means not found), otherwise handle
the successful stop case (set a success message or clear context.message) —
update the code around stopRequest/ status / context.message to reflect this
boolean logic.
---
Duplicate comments:
In `@packages/core/src/middleware.ts`:
- Line 104: The file middleware.ts is missing a trailing newline at EOF; add a
single newline character after the final closing brace '}' at the end of the
file so linters/prettier stop failing (ensure the file ends with a newline, not
extra spaces).
---
Nitpick comments:
In `@packages/adapter-dify/src/requester.ts`:
- Around line 240-267: The _workflowStream function currently calls
resolveDifyUser(params) internally while _agentStream accepts difyUser as a
parameter; make them consistent by changing _workflowStream to accept a difyUser
parameter (matching _agentStream) and remove the internal call to
resolveDifyUser; update usages inside _workflowStream to pass that difyUser into
prepareFiles and the request body (replace internal resolveDifyUser usage), and
adjust any callers to pass the resolved difyUser; ensure related helpers
referenced (prepareFiles, buildInputs) still receive the difyUser where needed.
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 14-24: The decodeMessageContent function currently swallows errors
from gzipDecode/JSON.parse; update its catch to capture the error (e.g., catch
(err)) and emit a warning log including the error and identifying info from the
MessageRecord (such as message.id or message.metadata) along with the raw
message.text/content to aid debugging; keep the existing fallback return
(message.text ?? '') but ensure the warning uses the project's logger (e.g.,
logger.warn or processLogger.warn) rather than console so logs integrate with
existing telemetry.
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Around line 62-79: The current code fetches all chatluna_conversation rows
then filters in memory which will not scale; change the logic in the deletion
flow so you query the database for only conversations whose preset is in
presetTemplate.triggerKeyword (use ctx.database.get / query with an "$in" or
equivalent operator for the preset field) and then map those returned rows to
set preset = nextPreset and updatedAt before calling
ctx.database.upsert('chatluna_conversation', patched); reference the existing
identifiers ctx.database.get, chatluna_conversation,
presetTemplate.triggerKeyword, nextPreset, patched, and ctx.database.upsert when
making this change.
In `@packages/core/src/services/conversation.ts`:
- Around line 1057-1061: The code unsafely asserts payload.messages.map(...) as
MessageRecord[]—instead of a blanket cast, call the existing deserializeMessage
function for each item (e.g., map payload.messages through deserializeMessage)
to convert SerializedMessageRecord -> MessageRecord and ensure createdAt and
other fields are typed correctly; replace the manual createdAt conversion and
the "as unknown as MessageRecord[]" cast with a proper map to deserializeMessage
so the messages variable is a true MessageRecord[].
- Around line 684-689: The loop in removeAcl that awaits
this.ctx.database.remove for each item (variables: removeAcl, removed,
this.ctx.database.remove) is inefficient for large records; replace the per-item
deletes with a single bulk delete call (e.g., use a database API that accepts a
filter like { id: { $in: removed.map(r => r.id) } } or a batchRemove/transaction
method) and then return this.listAcl(conversationId) as before; ensure you
handle empty removed arrays and propagate/throw DB errors consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0c4b6061-1600-424a-ab9c-107e04f8a8da
📒 Files selected for processing (8)
packages/adapter-dify/src/requester.tspackages/core/src/middleware.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/chat/stop_chat.tspackages/core/src/middlewares/preset/delete_preset.tspackages/core/src/services/conversation.tspackages/core/src/services/conversation_types.tspackages/core/tests/conversation-service.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/tests/conversation-service.spec.ts
- packages/core/src/services/conversation_types.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/middlewares/preset/delete_preset.ts (1)
62-75: 考虑使用数据库查询条件过滤会话。当前实现加载所有会话后在内存中过滤。如果会话数量较大，可以考虑使用数据库 WHERE 条件直接过滤，减少内存占用。
不过由于预设删除是低频操作，当前实现也是可接受的。
♻️ 可选优化
- const conversations = (await ctx.database.get( - 'chatluna_conversation', - {} - )) as ConversationRecord[] - const updatedAt = new Date() - const patched = conversations - .filter((conversation) => - presetTemplate.triggerKeyword.includes(conversation.preset) - ) - .map((conversation) => ({ - ...conversation, - preset: nextPreset, - updatedAt - })) + const conversations = (await ctx.database.get( + 'chatluna_conversation', + { preset: presetTemplate.triggerKeyword } + )) as ConversationRecord[] + const updatedAt = new Date() + const patched = conversations.map((conversation) => ({ + ...conversation, + preset: nextPreset, + updatedAt + }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/preset/delete_preset.ts` around lines 62 - 75, The current code loads all conversations into memory then filters them by presetTemplate.triggerKeyword; change the database call (ctx.database.get) to push the filter into the query so only matching conversations are returned (use the same predicate that currently uses presetTemplate.triggerKeyword), then map those results to update preset to nextPreset and updatedAt as before; update references to conversations, ctx.database.get, presetTemplate.triggerKeyword, ConversationRecord, nextPreset and patched so the in-memory filter is removed and the DB does the WHERE filtering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/middleware.ts`:
- Around line 76-77: 为 request_conversation 和 resolve_conversation
两个中间件添加命令守卫：在各自的 middleware 回调最开始检查 context.command 是否存在，如果不为 null 则直接返回
ChainMiddlewareRunStatus.SKIPPED；这样可以避免 request_conversation 在管理命令没有
inputMessage 时访问 inputMessage.content 导致崩溃，以及避免 resolve_conversation 在处理
conversation_manage 相关命令时误读
context.options.conversation_manage?.targetConversation 并返回 STOP。参照
conversation_manage.ts 中 middleware() 的写法，在 request_conversation 和
resolve_conversation 的入口处加入该检查并返回 SKIPPED。
---
Nitpick comments:
In `@packages/core/src/middlewares/preset/delete_preset.ts`:
- Around line 62-75: The current code loads all conversations into memory then
filters them by presetTemplate.triggerKeyword; change the database call
(ctx.database.get) to push the filter into the query so only matching
conversations are returned (use the same predicate that currently uses
presetTemplate.triggerKeyword), then map those results to update preset to
nextPreset and updatedAt as before; update references to conversations,
ctx.database.get, presetTemplate.triggerKeyword, ConversationRecord, nextPreset
and patched so the in-memory filter is removed and the DB does the WHERE
filtering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b1594904-0ddf-4e4d-8683-d2ebbb5d6247
📒 Files selected for processing (3)
packages/adapter-dify/src/requester.tspackages/core/src/middleware.tspackages/core/src/middlewares/preset/delete_preset.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/adapter-dify/src/requester.ts
Use per-user variables only in personal route mode so shared chats keep the same Dify user, and rely on typed additional_kwargs access when reading conversation IDs.
Drop legacy auth and balance flows so the room-to-conversation migration only keeps the legacy ChatHub tables that still need to survive until purge.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/adapter-dify/src/requester.ts (1)
769-786:⚠️ Potential issue | 🟠 Major只在 Dify 删除成功后再删本地缓存。
这里非 2xx 的 DELETE 也会继续执行
cache.delete()。一旦 Dify 因user不匹配或临时错误拒绝删除，conversationId/user映射就丢了，后面既无法重试清理，也没法再精确定位这条远端会话。把缓存删除放到res.ok分支里更安全。建议修复
if (difyConversationId) { - await this._plugin - .fetch(this.concatUrl('/conversations/' + difyConversationId), { - headers: this._buildHeaders(config.apiKey), - method: 'DELETE', - body: JSON.stringify({ user: difyUser }) - }) - .then(async (res) => { - if (res.ok) { - this.ctx.logger.info('Dify clear: success') - } else { - this.ctx.logger.warn( - 'Dify clear: failed: ' + (await res.text()) - ) - } - }) - - await this.ctx.chatluna.cache.delete('chatluna/keys', cacheKey) + const res = await this._plugin.fetch( + this.concatUrl('/conversations/' + difyConversationId), + { + headers: this._buildHeaders(config.apiKey), + method: 'DELETE', + body: JSON.stringify({ user: difyUser }) + } + ) + + if (res.ok) { + this.ctx.logger.info('Dify clear: success') + await this.ctx.chatluna.cache.delete( + 'chatluna/keys', + cacheKey + ) + } else { + this.ctx.logger.warn( + 'Dify clear: failed: ' + (await res.text()) + ) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapter-dify/src/requester.ts` around lines 769 - 786, The cache is being deleted regardless of the Dify DELETE result; change the logic in the block using difyConversationId and this._plugin.fetch(...) so that await this.ctx.chatluna.cache.delete('chatluna/keys', cacheKey) is executed only inside the res.ok branch (i.e., after confirming a successful DELETE), keeping the existing logging in both branches and awaiting cache.delete only on success to avoid losing the conversationId/user mapping when the remote delete fails.
♻️ Duplicate comments (2)
packages/core/src/index.ts (1)
252-275:⚠️ Potential issue | 🔴 Critical定时 purge 前要重新串行化并复核当前状态。
这里先查出一批 archived 会话，再按查询快照直接
purgeArchivedConversation(ctx, conversation)。如果某个会话在查询之后、循环到它之前已经被 restore 成 active，这里仍会把刚恢复的会话和归档一起删掉；而packages/core/src/utils/archive.ts的 Line 4-68 也没有接入conversationRuntime.withConversationSync()。建议在 purge 前进入 conversation 级同步，并重新读取当前记录确认status === 'archived'后再删。可参考的修正
for (const conversation of conversations) { try { - await purgeArchivedConversation(ctx, conversation) + await ctx.chatluna.conversationRuntime.withConversationSync( + conversation, + async () => { + const current = + await ctx.chatluna.conversation.getConversation( + conversation.id + ) + if ( + current == null || + current.status !== 'archived' + ) { + return + } + await purgeArchivedConversation(ctx, current) + } + ) success += 1 } catch (e) { logger.error(e) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 252 - 275, The purge loop currently fetches a snapshot of archived conversations and calls purgeArchivedConversation(ctx, conversation) directly, which can delete conversations that were restored after the query; to fix, wrap per-conversation work in the conversation-level sync using conversationRuntime.withConversationSync(conversation.id, ...), inside the sync re-read the latest conversation record from the DB (e.g., via ctx.database.get or ctx.database.findOne for the conversation id) and only call purgeArchivedConversation(ctx, conversation) if the fresh record.status === 'archived'; ensure conversationRuntime.withConversationSync is used for each conversation and that purgeArchivedConversation is only invoked after this re-check.packages/core/src/services/conversation.ts (1)
1198-1224:⚠️ Potential issue | 🔴 Critical不要用
view解析路径来执行会话配置写入。当调用方带
conversationId时，这里先走ensureActiveConversation()；而这条路径在resolveContext()里只要求hasConversationPermission(..., 'view', ...)。结果是拿到 ACLview的跨 binding 会话，也能在这里改model/preset/chatMode；同时这里也少了renameConversation()、deleteConversation()那样对目标 binding 再跑一遍assertManageAllowed()的校验。建议改成和其它管理入口一致，先按permission: 'manage'解析目标，再校验目标 binding 的 managed constraint。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 1198 - 1224, The current path uses ensureActiveConversation which can resolve a conversation via a 'view' permission path (via resolveContext/hasConversationPermission) allowing callers with only view access to change model/preset/chatMode; update the code to resolve the target conversation with permission: 'manage' (i.e. replace the ensureActiveConversation/resolveContext('view') usage with a resolve that requires 'manage' or call the existing resolve method that accepts a permission parameter), then call assertManageAllowed on the resolved conversation's binding/constraint again before applying changes (keep getManagedConstraintByBindingKey and the lockConversation check), and finally call touchConversation to persist the updates; mirror the same binding-level manage validation done in renameConversation/deleteConversation so only callers with manage rights can update model/preset/chatMode.
🧹 Nitpick comments (1)
packages/core/src/services/chat.ts (1)
594-597: 给chatluna_message.conversationId加索引。新 runtime 的历史加载、压缩、归档基本都会按
conversationId查消息；这里只有主键索引，消息量上来后会很快退化成全表扫描。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/chat.ts` around lines 594 - 597, 当前 chatluna_message 表只在主键上有索引，需为 conversationId 字段添加非唯一索引以避免按 conversationId 查询时全表扫描；在定义 conversationId 的模型/迁移处（字段名 conversationId、类型 'char' length 255）新增索引声明或新增一条数据库迁移/索引创建语句为表 chatluna_message 创建一个普通索引（非唯一），并确保相应 ORM/模式定义和迁移脚本一致且在部署时运行该迁移。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/index.ts`:
- Around line 197-223: The current auto-archive loop fetches conversations via
ctx.database.get using updatedAt < cutoff (Date.now() -
config.autoArchiveTimeout*1000) but then calls
ctx.chatluna.conversation.archiveConversationById(conversation.id) without
re-checking inactivity, which can archive conversations that became active after
the query; fix by revalidating the conversation's updatedAt against the same
cutoff immediately before archiving (or move that validation into
archiveConversationById so it always checks current DB state), i.e., fetch the
latest record for conversation.id or call a validation method to ensure
updatedAt < cutoff before calling archiveConversationById and only proceed if
the check passes.
In `@packages/core/src/middlewares/chat/chat_time_limit_check.ts`:
- Around line 124-149: resolveConversationTarget currently returns a target only
when context.options.conversationId exists, letting default chats bypass time
limits; update it to also try the middleware-populated resolution fields (e.g.
context.resolvedConversation, context.resolvedConversationContext or the values
that packages/core/src/middlewares/conversation/resolve_conversation.ts writes
back) when conversationId is null so a conversationId is derived before skipping
limits, using ctx.chatluna.conversation.resolveContext as fallback and returning
{ model, conversationId } or null accordingly; ensure this prevents default
requests from falling through to CONTINUE and misrouting to the
chat_time_limit_save branch.
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 167-178: The replay path currently calls
ctx.chatluna.conversation.resolveContext(session, { conversationId:
conversation.id }) which loses the original binding/preset lane and can cause
effectiveModel to be resolved incorrectly; update the logic in rollback_chat.ts
so that when reconstructing inputMessage you reuse the earlier resolvedContext
(the one resolved using conversation.bindingKey + presetLane) or pass the same
bindingKey and presetLane into resolveContext (keep reResolved variable
semantics), then feed its effectiveModel into
ctx.chatluna.messageTransformer.transform(transformMessageContentToElements(humanContent),
...) so transformMessageContentToElements and context.options.inputMessage are
built with the correct model and binding context (refer to resolveContext,
transformMessageContentToElements, context.options.inputMessage, humanMessage,
reResolved/resolvedContext, conversation.bindingKey, presetLane).
In `@packages/core/src/middlewares/system/wipe.ts`:
- Around line 104-113: The wipe middleware currently removes the entire chatluna
temp directory (via the fs.rm call in wipe.ts), which also deletes the
LEGACY_SCHEMA_SENTINEL used by defineLegacyMigrationTables(); update the wipe
logic to preserve that sentinel by either excluding the sentinel file from
deletion or by re-creating it immediately after the deletion: import
LEGACY_SCHEMA_SENTINEL from migration/legacy_tables and after the temp-directory
fs.rm succeeds, write the sentinel back to its original filename (using fs.mkdir
with { recursive: true } then fs.writeFile) so defineLegacyMigrationTables()
will not re-register legacy tables.
In `@packages/core/src/migration/room_to_conversation.ts`:
- Around line 239-274: The code currently writes room.conversationId straight
into the ConversationRecord (the id field) and then upserts via
ctx.database.upsert('chatluna_conversation'), which lets later legacy rooms
overwrite existing conversations; instead, build a stable
roomId->newConversationId mapping before creating any ConversationRecord: when
processing rooms (the logic around creating conversation and existingByRoomId),
generate or assign a newConversationId (use a new UUID when room.conversationId
is missing or already mapped to another room), store that mapping, and use
newConversationId as the ConversationRecord.id and for any bindings/messages
migration (migrateMessages(), binding creation) so all upserts and message
inserts reuse the mapped id rather than raw room.conversationId; update
existingByRoomId lookups and the upsert call to reference the mapped id to
prevent overwrites.
In `@packages/core/src/services/chat.ts`:
- Around line 615-620: The schema currently defines tool_call_id as a required
column causing ordinary human/assistant/system messages to need fake values;
update the schema entry for tool_call_id (the field named "tool_call_id" in the
same object where "name" is defined) to be optional/nullable by changing it from
a plain 'string' to an object with type 'string' and nullable: true (e.g.,
tool_call_id: { type: 'string', nullable: true }) so only tool messages populate
it.
- Around line 259-276: The createChatInterface function is not forwarding
ConversationRecord.autoTitle into the ChatInterface options, so each
conversation's auto-title setting is ignored; update the ChatInterface
instantiation in createChatInterface to include autoTitle:
conversation.autoTitle (and any similarly missing flag usage in the other
createChatInterface occurrence referenced), ensuring the ChatInterface
constructor receives the autoTitle flag from the ConversationRecord so
per-conversation auto-title behavior is preserved.
- Around line 112-114: ChatLunaService 在构造函数中通过
_defineDatabase/this.ctx.on('ready') 调用 model.extend() 和
_dedupeConstraintNames() 导致唯一约束（chatluna_constraint.name）在 ready 阶段才声明，从而被
Koishi 的自动 schema 同步绕过并在存在旧数据重复时阻断升级；将约束声明同步迁移到 apply() 中使用
ctx.database.extend()（而不是在 ready 回调里 model.extend()），并把去重逻辑移入数据库迁移钩子（migration
hook 或类似在 apply() 同步执行的迁移步骤）以在创建唯一索引前清理旧重复数据（参考函数/方法：ChatLunaService,
_defineDatabase, _dedupeConstraintNames, apply, ctx.database.extend,
model.extend）。
In `@packages/core/src/services/conversation.ts`:
- Around line 1658-1678: Validate the archive's manifest checksum and size
before decoding messages: read messages.jsonl.gz as a Buffer (use path
archivePath and filename 'messages.jsonl.gz'), compute its byte length and
SHA-256, and compare them against the manifest fields (the SHA-256 and size
written earlier in manifest). If either check fails, throw a clear error to stop
restoreConversation()/exportMarkdown() from consuming corrupted data; only call
gzipDecode and JSON.parse into SerializedMessageRecord after the checks pass.
---
Outside diff comments:
In `@packages/adapter-dify/src/requester.ts`:
- Around line 769-786: The cache is being deleted regardless of the Dify DELETE
result; change the logic in the block using difyConversationId and
this._plugin.fetch(...) so that await
this.ctx.chatluna.cache.delete('chatluna/keys', cacheKey) is executed only
inside the res.ok branch (i.e., after confirming a successful DELETE), keeping
the existing logging in both branches and awaiting cache.delete only on success
to avoid losing the conversationId/user mapping when the remote delete fails.
---
Duplicate comments:
In `@packages/core/src/index.ts`:
- Around line 252-275: The purge loop currently fetches a snapshot of archived
conversations and calls purgeArchivedConversation(ctx, conversation) directly,
which can delete conversations that were restored after the query; to fix, wrap
per-conversation work in the conversation-level sync using
conversationRuntime.withConversationSync(conversation.id, ...), inside the sync
re-read the latest conversation record from the DB (e.g., via ctx.database.get
or ctx.database.findOne for the conversation id) and only call
purgeArchivedConversation(ctx, conversation) if the fresh record.status ===
'archived'; ensure conversationRuntime.withConversationSync is used for each
conversation and that purgeArchivedConversation is only invoked after this
re-check.
In `@packages/core/src/services/conversation.ts`:
- Around line 1198-1224: The current path uses ensureActiveConversation which
can resolve a conversation via a 'view' permission path (via
resolveContext/hasConversationPermission) allowing callers with only view access
to change model/preset/chatMode; update the code to resolve the target
conversation with permission: 'manage' (i.e. replace the
ensureActiveConversation/resolveContext('view') usage with a resolve that
requires 'manage' or call the existing resolve method that accepts a permission
parameter), then call assertManageAllowed on the resolved conversation's
binding/constraint again before applying changes (keep
getManagedConstraintByBindingKey and the lockConversation check), and finally
call touchConversation to persist the updates; mirror the same binding-level
manage validation done in renameConversation/deleteConversation so only callers
with manage rights can update model/preset/chatMode.
---
Nitpick comments:
In `@packages/core/src/services/chat.ts`:
- Around line 594-597: 当前 chatluna_message 表只在主键上有索引，需为 conversationId
字段添加非唯一索引以避免按 conversationId 查询时全表扫描；在定义 conversationId 的模型/迁移处（字段名
conversationId、类型 'char' length 255）新增索引声明或新增一条数据库迁移/索引创建语句为表 chatluna_message
创建一个普通索引（非唯一），并确保相应 ORM/模式定义和迁移脚本一致且在部署时运行该迁移。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 237c66ee-d09c-45f3-af4e-ed059cd36873
⛔ Files ignored due to path filters (4)
packages/core/src/locales/en-US.schema.ymlis excluded by!**/*.ymlpackages/core/src/locales/en-US.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.schema.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.ymlis excluded by!**/*.yml
📒 Files selected for processing (32)
packages/adapter-dify/src/requester.tspackages/core/src/authorization/service.tspackages/core/src/authorization/types.tspackages/core/src/command.tspackages/core/src/commands/auth.tspackages/core/src/config.tspackages/core/src/index.tspackages/core/src/middleware.tspackages/core/src/middlewares/auth/add_user_to_auth_group.tspackages/core/src/middlewares/auth/black_list.tspackages/core/src/middlewares/auth/create_auth_group.tspackages/core/src/middlewares/auth/kick_user_form_auth_group.tspackages/core/src/middlewares/auth/list_auth_group.tspackages/core/src/middlewares/auth/set_auth_group.tspackages/core/src/middlewares/chat/chat_time_limit_check.tspackages/core/src/middlewares/chat/chat_time_limit_save.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/chat/stop_chat.tspackages/core/src/middlewares/conversation/request_conversation.tspackages/core/src/middlewares/system/clear_balance.tspackages/core/src/middlewares/system/query_balance.tspackages/core/src/middlewares/system/restart.tspackages/core/src/middlewares/system/set_balance.tspackages/core/src/middlewares/system/wipe.tspackages/core/src/migration/legacy_tables.tspackages/core/src/migration/room_to_conversation.tspackages/core/src/services/chat.tspackages/core/src/services/conversation.tspackages/core/src/utils/error.tspackages/core/src/utils/koishi.tspackages/core/tests/conversation-source.spec.tspackages/core/tests/helpers.ts
💤 Files with no reviewable changes (12)
- packages/core/src/commands/auth.ts
- packages/core/src/middlewares/auth/set_auth_group.ts
- packages/core/src/middlewares/auth/black_list.ts
- packages/core/src/authorization/types.ts
- packages/core/src/middlewares/system/query_balance.ts
- packages/core/src/middlewares/auth/list_auth_group.ts
- packages/core/src/middlewares/auth/add_user_to_auth_group.ts
- packages/core/src/middlewares/system/set_balance.ts
- packages/core/src/middlewares/auth/kick_user_form_auth_group.ts
- packages/core/src/middlewares/system/clear_balance.ts
- packages/core/src/middlewares/auth/create_auth_group.ts
- packages/core/src/authorization/service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/utils/error.ts
- packages/core/src/middlewares/chat/stop_chat.ts
Track route-level preset lanes and defaults so switching, listing, migration, export, and chat limits stay consistent across conversations. Also keep Dify cache entries when remote conversation deletion fails.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/core/src/services/types.ts (1)
114-133: Prettier 格式问题需要修复。静态分析提示此区域存在格式问题（缺少换行和缩进）。建议运行
prettier --write修复。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/types.ts` around lines 114 - 133, The block defining ListConversationsOptions, ResolveTargetConversationOptions and SerializedMessageRecord has Prettier formatting issues (missing expected newlines/indentation); run a formatter (e.g., run prettier --write on this file) or reformat the block so each interface member and type parameter is on its own line with proper indentation, ensuring the Omit<> multi-line formatting is preserved for SerializedMessageRecord and the optional fields (content, additional_kwargs_binary, createdAt) are each on separate indented lines.packages/core/src/index.ts (1)
332-376:dedupeConstraintNames与chat.ts中的实现重复。此函数在
packages/core/src/services/chat.ts:450-491有完全相同的实现。建议提取到共享模块或只保留一处调用。此外，第 375 行的空
catch {}块会静默吞掉所有错误，建议至少记录日志。建议修复空 catch 块
- } catch {} + } catch (e) { + logger.warn('Failed to dedupe constraint names', e) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 332 - 376, dedupeConstraintNames duplicates logic already present in services/chat.ts (same function body) so extract the shared logic into a single exported helper (e.g., move dedupeConstraintNames implementation into a new shared utility module or into an existing shared service) and replace both implementations with a single import + call; also replace the empty catch block in dedupeConstraintNames with proper error logging (use the existing logger.error and include the caught error) so failures aren't silently swallowed. Ensure references to dedupeConstraintNames and any call sites in chat.ts are updated to import the shared function.packages/core/src/middlewares/chat/rollback_chat.ts (1)
14-24:decodeMessageContent中的括号可以移除静态分析提示 Line 18-19 有多余的括号，可以简化：
🔧 建议修复
async function decodeMessageContent(message: MessageRecord) { try { return JSON.parse( message.content ? await gzipDecode(message.content) - : (message.text ?? '""') + : message.text ?? '""' ) } catch { return message.text ?? '' } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 14 - 24, The decodeMessageContent function contains unnecessary parentheses around the fallback operand in the ternary expression; update the function (decodeMessageContent) so the JSON.parse call uses message.content ? await gzipDecode(message.content) : message.text ?? '""' (remove the extra parentheses around message.text ?? '""') while keeping the existing behavior and imports (MessageRecord, gzipDecode) intact.packages/core/src/middlewares/conversation/resolve_conversation.ts (1)
13-25: 辅助函数可以内联根据编码指南，少于 5 行的函数逻辑应该内联。
getPresetLane和getTargetConversation只在一处使用，可以直接内联到调用点。♻️ 可选重构
export function apply(ctx: Context, config: Config, chain: ChatChain) { chain .middleware('resolve_conversation', async (session, context) => { - const presetLane = getPresetLane(context) - const targetConversation = getTargetConversation(context) + const presetLane = + context.options.conversation_manage?.presetLane ?? + context.options.presetLane + const targetConversation = + context.options.conversation_manage?.targetConversation ?? + context.options.targetConversation const useRoutePresetLane =并移除 Lines 13-25 的辅助函数定义。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/conversation/resolve_conversation.ts` around lines 13 - 25, Inline the two small helper functions getPresetLane and getTargetConversation into their single call sites: replace calls to getPresetLane(context) with context.options.conversation_manage?.presetLane ?? context.options.presetLane and replace calls to getTargetConversation(context) with context.options.conversation_manage?.targetConversation ?? context.options.targetConversation, then remove the now-unused helper function definitions (getPresetLane and getTargetConversation) from the file.packages/core/src/services/conversation.ts (1)
476-484: 可移除 prettier 标记的多余括号静态分析提示多处有多余括号，可以简化：
- Line 482:
(current?.lastConversationId ?? null)- Line 1513:
(session.guildId ?? session.channelId ?? null)这些是格式化小问题，不影响功能。
Also applies to: 1511-1516
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 476 - 484, The code contains unnecessary parentheses around nullish-coalescing expressions—remove the extra parentheses in the BindingRecord creation for lastConversationId (the expression currently written as (current?.lastConversationId ?? null)) and similarly remove the extra parentheses around the session ID expression (session.guildId ?? session.channelId ?? null) found near session handling; update the expressions where bindingKey, activeConversationId, lastConversationId, and updatedAt are set so they use the bare nullish-coalescing expressions without surrounding parentheses to satisfy the linter/prettier suggestions.packages/core/src/migration/room_to_conversation.ts (1)
473-479: 可移除多余括号静态分析提示 Line 478 有多余的括号：
🔧 建议修复
const lastConversationId = prevActive != null && prevActive !== conversation.id ? prevActive - : (current?.lastConversationId ?? null) + : current?.lastConversationId ?? null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/migration/room_to_conversation.ts` around lines 473 - 479, 在 lastConversationId 的三元表达式中移除多余括号：将当前的 prevActive != null && prevActive !== conversation.id ? prevActive : (current?.lastConversationId ?? null) 改为 prevActive != null && prevActive !== conversation.id ? prevActive : current?.lastConversationId ?? null，直接引用 current?.lastConversationId ?? null 即可（相关标识符：prevActive、lastConversationId、current、conversation）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/services/types.ts`:
- Around line 196-200: The interface ToolMaskArg renamed the field from source
to bindingKey but sub_agent.ts still destructures and accesses source, causing
undefined runtime errors; update the destructuring at the top of the function
(replace "{ session, source }" with "{ session, bindingKey }"), and replace all
uses of ctx.source with ctx.bindingKey (or map ctx.bindingKey to a local
variable named source if backward compatibility is required) so code
consistently uses the ToolMaskArg.bindingKey property.
---
Nitpick comments:
In `@packages/core/src/index.ts`:
- Around line 332-376: dedupeConstraintNames duplicates logic already present in
services/chat.ts (same function body) so extract the shared logic into a single
exported helper (e.g., move dedupeConstraintNames implementation into a new
shared utility module or into an existing shared service) and replace both
implementations with a single import + call; also replace the empty catch block
in dedupeConstraintNames with proper error logging (use the existing
logger.error and include the caught error) so failures aren't silently
swallowed. Ensure references to dedupeConstraintNames and any call sites in
chat.ts are updated to import the shared function.
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 14-24: The decodeMessageContent function contains unnecessary
parentheses around the fallback operand in the ternary expression; update the
function (decodeMessageContent) so the JSON.parse call uses message.content ?
await gzipDecode(message.content) : message.text ?? '""' (remove the extra
parentheses around message.text ?? '""') while keeping the existing behavior and
imports (MessageRecord, gzipDecode) intact.
In `@packages/core/src/middlewares/conversation/resolve_conversation.ts`:
- Around line 13-25: Inline the two small helper functions getPresetLane and
getTargetConversation into their single call sites: replace calls to
getPresetLane(context) with context.options.conversation_manage?.presetLane ??
context.options.presetLane and replace calls to getTargetConversation(context)
with context.options.conversation_manage?.targetConversation ??
context.options.targetConversation, then remove the now-unused helper function
definitions (getPresetLane and getTargetConversation) from the file.
In `@packages/core/src/migration/room_to_conversation.ts`:
- Around line 473-479: 在 lastConversationId 的三元表达式中移除多余括号：将当前的 prevActive !=
null && prevActive !== conversation.id ? prevActive :
(current?.lastConversationId ?? null) 改为 prevActive != null && prevActive !==
conversation.id ? prevActive : current?.lastConversationId ?? null，直接引用
current?.lastConversationId ?? null
即可（相关标识符：prevActive、lastConversationId、current、conversation）。
In `@packages/core/src/services/conversation.ts`:
- Around line 476-484: The code contains unnecessary parentheses around
nullish-coalescing expressions—remove the extra parentheses in the BindingRecord
creation for lastConversationId (the expression currently written as
(current?.lastConversationId ?? null)) and similarly remove the extra
parentheses around the session ID expression (session.guildId ??
session.channelId ?? null) found near session handling; update the expressions
where bindingKey, activeConversationId, lastConversationId, and updatedAt are
set so they use the bare nullish-coalescing expressions without surrounding
parentheses to satisfy the linter/prettier suggestions.
In `@packages/core/src/services/types.ts`:
- Around line 114-133: The block defining ListConversationsOptions,
ResolveTargetConversationOptions and SerializedMessageRecord has Prettier
formatting issues (missing expected newlines/indentation); run a formatter
(e.g., run prettier --write on this file) or reformat the block so each
interface member and type parameter is on its own line with proper indentation,
ensuring the Omit<> multi-line formatting is preserved for
SerializedMessageRecord and the optional fields (content,
additional_kwargs_binary, createdAt) are each on separate indented lines.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4cd792a2-de24-43b8-aac1-c6b0289f1cfd
⛔ Files ignored due to path filters (2)
packages/core/src/locales/en-US.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.ymlis excluded by!**/*.yml
📒 Files selected for processing (23)
packages/adapter-dify/src/requester.tspackages/core/src/commands/conversation.tspackages/core/src/config.tspackages/core/src/index.tspackages/core/src/llm-core/chat/app.tspackages/core/src/middlewares/chat/allow_reply.tspackages/core/src/middlewares/chat/chat_time_limit_check.tspackages/core/src/middlewares/chat/chat_time_limit_save.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/conversation/request_conversation.tspackages/core/src/middlewares/conversation/resolve_conversation.tspackages/core/src/middlewares/model/resolve_model.tspackages/core/src/middlewares/system/conversation_manage.tspackages/core/src/middlewares/system/wipe.tspackages/core/src/migration/room_to_conversation.tspackages/core/src/migration/validators.tspackages/core/src/services/chat.tspackages/core/src/services/conversation.tspackages/core/src/services/conversation_types.tspackages/core/src/services/types.tspackages/core/src/utils/conversation.tspackages/core/src/utils/koishi.tspackages/core/src/utils/message_content.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/utils/message_content.ts
- packages/core/src/commands/conversation.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/src/middlewares/chat/allow_reply.ts
- packages/core/src/utils/conversation.ts
- packages/core/src/utils/koishi.ts
- packages/core/src/services/conversation_types.ts
- packages/core/src/middlewares/system/conversation_manage.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Allow conversation management and target completion to work across canonical and legacy preset routes while showing preset names in list output.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/core/src/middlewares/chat/rollback_chat.ts (1)
171-180:⚠️ Potential issue | 🟠 Major这里不要再回退到旧的
resolvedConversationContext。上面已经按目标会话的
bindingKey重新resolveContext()过了；这里再优先拿缓存值，跨 binding / preset lane 回滚仍会用当前 route 的effectiveModel去重放输入。建议修改
- const reResolved = - context.options.resolvedConversationContext ?? - resolvedContext + const reResolved = resolvedContext🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 171 - 180, 当前代码用 context.options.resolvedConversationContext ?? resolvedContext 来决定 reResolved，会导致跨 binding/preset 回退时仍优先使用旧缓存的 resolvedConversationContext；请改为始终使用新计算的 resolvedContext（即把 reResolved 直接设为 resolvedContext），并在后续调用 ctx.chatluna.messageTransformer.transform/transformMessageContentToElements 时使用该 reResolved 的 effectiveModel（fallback 到 conversation.model 仅在 reResolved.effectiveModel 为空时保留），以确保按目标会话的 bindingKey 重放输入。packages/core/src/services/conversation.ts (1)
2159-2172:⚠️ Potential issue | 🟠 Major同一路由不同 preset lane 仍会写回当前 binding。
matchesBindingKey()这里按 base key 认“同一路由”，所以目标会话只要来自另一条 lane，就会把activeConversationId写回当前 lane。结果是laneA的 current/list/request 会被laneB的会话污染。建议修改
function pickBindingKey( session: Session, resolved: ResolvedConversationContext, conversation: ConversationRecord, - allPresetLanes = false, - sameRouteBase = false + allPresetLanes = false ) { const keys = getLookupKeys(session, resolved.constraint.bindingKey, true) - if (!matchesBindingKey(conversation.bindingKey, keys)) { + if ( + conversation.bindingKey !== resolved.bindingKey || + !matchesBindingKey(conversation.bindingKey, keys) + ) { return conversation.bindingKey } - return sameRouteBase ? conversation.bindingKey : resolved.bindingKey + return resolved.bindingKey }// 调用点也要去掉 `sameRouteBase` 实参 const bindingKey = pickBindingKey( session, resolved, conversation, options.allPresetLanes )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 2159 - 2172, pickBindingKey currently treats "same route" by base key (via matchesBindingKey) and the sameRouteBase flag lets sessions from other preset lanes be considered the same route, causing activeConversationId to be written back across lanes; change pickBindingKey to only consider an exact binding key match (e.g., compare conversation.bindingKey === resolved.bindingKey or otherwise include lane in the comparison) instead of matchesBindingKey, remove the sameRouteBase parameter and its branching, and update all call sites (remove the sameRouteBase argument) so only truly identical binding keys (including lane) allow writing back.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/middlewares/conversation/resolve_conversation.ts`:
- Around line 64-74: The code currently reuses the route-level
resolvedConversationContext after resolving a target conversation, which can
misapply route bindings and turn a valid resolved.conversation into null; change
resolve_conversation.ts so that after obtaining resolved from
ctx.chatluna.conversation.resolveContext(session, {...}), if
resolved.conversation exists and carries its own binding/preset, call
ctx.chatluna.conversation.resolveContext a second time using the
resolved.conversation.id and the conversation's own binding/preset (e.g. pass
conversationId: resolved.conversation.id, presetLane:
resolved.conversation.binding?.presetLane and set useRoutePresetLane: false) and
then assign context.options.resolvedConversationContext to that final resolved
result and context.options.resolvedConversation to finalResolved.conversation
(falling back to the first resolved only if no conversation.bindings exist).
In `@packages/core/src/services/conversation.ts`:
- Around line 1691-1696: The global exact-title branch currently picks the first
match from globalMatches (via globalExactTitle) and returns it, which silently
misroutes when multiple sessions share the same normalized title across
bindings/ACLs; change this to perform the same ambiguity check used in the
local-title branch: filter globalMatches for entries whose
title.toLocaleLowerCase() === normalized, and if the filtered list has length >
1 throw an ambiguity error (same error type/message the local branch uses),
otherwise return the single match. Ensure you reference globalMatches,
globalExactTitle (replace with the filtered result), and normalized when
implementing the check.
- Around line 1586-1597: 当前严格用 resolved.bindingKey 去复核会话权限导致当
resolved.allPresetLanes 为 true 的路由跨 lane 会话被误判无权限；在 conversation 权限复核（调用
hasConversationPermission(session, conversation, options.permission ?? 'view',
resolved.bindingKey)）前，先检查 resolved.allPresetLanes，如果为 true 则允许按路由放行（即不要要求完全匹配
bindingKey），或者改为传入能够表示“路由通配”的标志给 hasConversationPermission（例如传入 undefined 或专门的
allowAllPresetLanes 标志）并在 hasConversationPermission 内处理该标志以放行同一路由跨 lane
的会话；引用符号：hasConversationPermission, session, conversation, options.permission,
resolved.bindingKey, resolved.allPresetLanes, conversationId。
In `@packages/core/src/utils/conversation.ts`:
- Around line 18-52: The current validation builds expect only from
listConversationEntries and blocks values that are actually accessible via
broader ACL/cross-binding logic in resolveTargetConversation, causing valid
targets to be suggested away; update this code in conversation.ts to either (a)
expand expect to include IDs/titles returned by the full access-resolution path
(i.e., consult the same ACL/cross-binding lookup used by
resolveTargetConversation) or (b) remove the early session.suggest rejection
here and let resolveTargetConversation perform final validation/suggestion;
adjust references to entries, expect, listConversationEntries,
resolveTargetConversation and session.suggest accordingly so shared/ACL-visible
conversations are not incorrectly rejected.
---
Duplicate comments:
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 171-180: 当前代码用 context.options.resolvedConversationContext ??
resolvedContext 来决定 reResolved，会导致跨 binding/preset 回退时仍优先使用旧缓存的
resolvedConversationContext；请改为始终使用新计算的 resolvedContext（即把 reResolved 直接设为
resolvedContext），并在后续调用
ctx.chatluna.messageTransformer.transform/transformMessageContentToElements 时使用该
reResolved 的 effectiveModel（fallback 到 conversation.model 仅在
reResolved.effectiveModel 为空时保留），以确保按目标会话的 bindingKey 重放输入。
In `@packages/core/src/services/conversation.ts`:
- Around line 2159-2172: pickBindingKey currently treats "same route" by base
key (via matchesBindingKey) and the sameRouteBase flag lets sessions from other
preset lanes be considered the same route, causing activeConversationId to be
written back across lanes; change pickBindingKey to only consider an exact
binding key match (e.g., compare conversation.bindingKey === resolved.bindingKey
or otherwise include lane in the comparison) instead of matchesBindingKey,
remove the sameRouteBase parameter and its branching, and update all call sites
(remove the sameRouteBase argument) so only truly identical binding keys
(including lane) allow writing back.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4d926739-8954-4985-a562-f43070d5ee86
⛔ Files ignored due to path filters (2)
packages/core/src/locales/en-US.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.ymlis excluded by!**/*.yml
📒 Files selected for processing (14)
packages/core/src/commands/chat.tspackages/core/src/commands/conversation.tspackages/core/src/middlewares/chat/read_chat_message.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/chat/stop_chat.tspackages/core/src/middlewares/conversation/resolve_conversation.tspackages/core/src/middlewares/system/conversation_manage.tspackages/core/src/services/conversation.tspackages/core/src/services/conversation_types.tspackages/core/src/services/types.tspackages/core/src/utils/conversation.tspackages/core/tests/conversation-service.spec.tspackages/core/tests/conversation-target.spec.tspackages/extension-agent/src/service/sub_agent.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/src/commands/chat.ts
- packages/core/src/middlewares/chat/stop_chat.ts
- packages/core/src/middlewares/system/conversation_manage.ts
- packages/core/src/commands/conversation.ts
- packages/core/src/middlewares/chat/read_chat_message.ts
packages/core/src/middlewares/conversation/resolve_conversation.ts
Outdated
Show resolved
Hide resolved
Allow exact ID and accessible cross-route targets without rebinding the current lane. Reject ambiguous exact-title matches so switches and exports resolve the intended conversation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/middlewares/chat/rollback_chat.ts (1)
106-188:⚠️ Potential issue | 🟠 Major把整段回滚修改包进
withConversationSync()。现在只有
clearConversationInterface()本身是加锁的，后面的 parent 链遍历、latestMessageIdupsert 和消息删除都在锁外。并发 chat 只要在清缓存之后抢到同一会话，就能把新消息重新写回去，回滚结果会被重新污染。这里最好像archiveConversationById()/restoreConversation()一样，在 sync 块里重新读取当前会话后再完成整段回滚。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/rollback_chat.ts` around lines 106 - 188, The rollback sequence (after clearConversationInterface) must be executed inside the same withConversationSync() lock so the parent-chain traversal, latestMessageId upsert and message removal are atomic; modify the rollback_chat logic to call withConversationSync(conversation.id, async () => { ... }) and move the parentId traversal, humanCount/humanMessage selection, previousLatestId upsert, preparing context.options.inputMessage (using decodeMessageContent and ctx.chatluna.messageTransformer.transform), and the ctx.database.remove('chatluna_message', ...) into that synced block; follow the pattern used by archiveConversationById()/restoreConversation() by re-reading the current conversation inside the sync block before applying the upsert/remove to avoid races.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 14-23: The current decodeMessageContent calls JSON.parse on a
fallback plain-text which causes numbers/booleans/null/objects to become
non-string/non-array types and later breaks transformMessageContentToElements;
fix decodeMessageContent so JSON.parse is only used when message.content exists
(i.e., parse await gzipDecode(message.content) only if message.content is
truthy), otherwise immediately return message.text ?? '' as a string;
additionally, keep the try/catch but on parse failure fall back to message.text
?? '' to ensure transformMessageContentToElements always receives a string or
message-array.
In `@packages/core/src/services/conversation.ts`:
- Around line 649-652: The activation path that calls
setActiveConversation(pickBindingKey(resolved, conversation), conversation.id)
updates only the target binding's activeConversationId but not the managed
activePresetLane, so calls across preset lanes leave activePresetLane stale and
cause subsequent non-lane conversations to revert to the old lane; update the
same code paths where switchConversation handles same-route-base and the other
branch (also at the similar location around the second occurrence) to also set
the binding's activePresetLane (e.g., by computing the target lane from resolved
/ conversation or pickBindingKey result and writing it into the managed state
alongside activeConversationId) so both activeConversationId and
activePresetLane are synced whenever activating a conversation across preset
lanes.
- Around line 1216-1224: The current flow marks a conversation as 'deleted'
using touchConversation but leaves its archive metadata (archiveId,
chatluna_archive row and archive files) orphaned so it can neither be reopened
nor purged by purgeArchivedConversation; update the deletion path in
conversation.ts: either call purgeArchivedConversation(current.id) instead of
switching status to 'deleted', or when you must mark deleted, also clear
archive-related data by setting archiveId to null in touchConversation, remove
the chatluna_archive DB record (delete from 'chatluna_archive' for current.id)
and delete the archive files on disk (same logic used by
purgeArchivedConversation), then proceed with unbindConversation, removing
messages and removeAcl so no orphaned archives remain; reference functions:
touchConversation, purgeArchivedConversation, unbindConversation, removeAcl and
DB table 'chatluna_archive' and field archiveId to locate the code to change.
- Around line 1511-1527: The current check in the block inside conversation.ts
compares base binding keys and calls hasConversationPermission using
resolved.bindingKey, which falsely rejects legacy-vs-new bindings that belong to
the same route family; change the logic to compare route-family identifiers
instead of raw matched binding keys: derive a route family id for both
conversation.bindingKey and resolved.bindingKey (e.g. via an existing
getRouteFamily or by normalizing to the family key), use that family id in the
allPresetLanes short-circuit check (replace getBaseBindingKey(...) comparison),
and pass the family id (not the resolved matched binding) into
hasConversationPermission so legacy and new bindings in the same route-family
are treated as the same route. Ensure you update references around
listConversationEntries / completeConversationTarget flows, keeping identifiers
like conversation.bindingKey, resolved.bindingKey, getBaseBindingKey, and
hasConversationPermission to locate and change the code.
---
Outside diff comments:
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 106-188: The rollback sequence (after clearConversationInterface)
must be executed inside the same withConversationSync() lock so the parent-chain
traversal, latestMessageId upsert and message removal are atomic; modify the
rollback_chat logic to call withConversationSync(conversation.id, async () => {
... }) and move the parentId traversal, humanCount/humanMessage selection,
previousLatestId upsert, preparing context.options.inputMessage (using
decodeMessageContent and ctx.chatluna.messageTransformer.transform), and the
ctx.database.remove('chatluna_message', ...) into that synced block; follow the
pattern used by archiveConversationById()/restoreConversation() by re-reading
the current conversation inside the sync block before applying the upsert/remove
to avoid races.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 785acf41-ac1e-425a-9432-fc7083d127c8
📒 Files selected for processing (7)
packages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/conversation/resolve_conversation.tspackages/core/src/services/conversation.tspackages/core/src/utils/archive.tspackages/core/src/utils/conversation.tspackages/core/tests/conversation-service.spec.tspackages/core/tests/conversation-target.spec.ts
Use route-family lookup keys when switching or restoring conversations, remove stale archives on delete, and run rollback under the conversation sync lock to avoid inconsistent history state.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/middlewares/chat/rollback_chat.ts`:
- Around line 219-246: The code updates chatluna_conversation.latestMessageId
before reconstructing the input message and deleting messages, which can leave
the conversation pointer inconsistent if decodeMessageContent or transform
throws; fix by first performing decodeMessageContent(humanMessage) and
ctx.chatluna.messageTransformer.transform(...) (i.e., build inputMessage when
context.options.inputMessage is missing) and only after that perform the
persistence operations: remove the messages via
ctx.database.remove('chatluna_message', { id: messages.map(m=>m.id) }) and
upsert the conversation latestMessageId in a single atomic step; ideally wrap
the remove and upsert into one transaction if ctx.database supports transactions
to ensure both delete and latestMessageId update succeed or both roll back.
- Around line 49-68: The code currently falls back to the current conversation
whenever resolve/lookup returns null, which silently applies a rollback to the
wrong convo when the caller explicitly provided a
conversationId/targetConversation; change the behavior in rollback_chat.ts so
that after attempting to resolve the provided conversation (via
ctx.chatluna.conversation.resolveCommandConversation with conversationId and
presetLane), if conversation is still null AND the caller explicitly supplied a
target (e.g., context.targetConversation or an explicit conversationId param),
do not call ctx.chatluna.conversation.getCurrentConversation and instead
abort/throw an error (or return a failure) so the rollback does not apply to the
current conversation; only allow falling back to getCurrentConversation when no
explicit target was provided. Ensure checks reference the variables used in the
diff (conversation, session, context.options.presetLane/allPresetLanes,
ctx.chatluna.conversation.resolveCommandConversation) so the change is
localized.
In `@packages/core/src/services/conversation.ts`:
- Around line 1216-1218: When manually renaming a conversation, also disable the
automatic titling so the auto-summary won't overwrite the user's change: update
the call to touchConversation(conversation.id, { title: options.title.trim(),
autoTitle: false }) (or set the conversation.autoTitle flag to false before
persisting) so that the autoTitle field is turned off when handling the manual
rename in the rename handler that currently calls touchConversation.
- Around line 998-1022: The code directly fetches a conversation via
getConversation(options.conversationId) and skips ACL/route/admin checks,
allowing restoration by ID; replace that direct fetch with the same
permission-checked resolution used elsewhere (call
resolveTargetConversation(options.conversationId, { permission: 'manage' }) or
the project’s equivalent) to obtain the target conversation and its constraint,
then use that resolved constraint for assertManageAllowed and subsequent archive
ownership checks (keep using getArchive/getArchiveByConversationId only after
the target conversation is permission-checked).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8bdae7cf-77cc-4a18-9f0e-06c867f68dd5
📒 Files selected for processing (8)
packages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/conversation/resolve_conversation.tspackages/core/src/services/conversation.tspackages/core/src/utils/archive.tspackages/core/src/utils/conversation.tspackages/core/tests/conversation-service.spec.tspackages/core/tests/conversation-target.spec.tspackages/core/tests/rollback-chat.spec.ts
Prevent explicit rollback and restore targets from falling back or bypassing route checks, and avoid overwriting manual titles. Add regression coverage for rollback failure paths and restore ACL checks.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/services/conversation.ts (1)
489-505:allPresetLanes场景下全表扫描存在性能隐患。当
options.allPresetLanes为true时，此处从chatluna_conversation表拉取全部记录后在内存过滤。随着会话数量增长，这会成为瓶颈。建议改为使用数据库层的
$or/ 前缀匹配条件，或者限制返回条数并分页。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation.ts` around lines 489 - 505, The current allPresetLanes branch pulls the entire chatluna_conversation table into memory and filters by bindingKey, causing a full-table scan; update the this.ctx.database.get call inside the options.allPresetLanes branch to push the filter into the DB by building a query that uses an $or over each key: one clause matching bindingKey === key and another matching bindingKey starts-with (prefix) key + ':preset:' (or the DB's equivalent like regex/$like), e.g. produce an array of { bindingKey: key } and { bindingKey: { $startsWith: key + ':preset:' } } for all keys and pass that to this.ctx.database.get; also add a sensible limit and optionally paginate results if the table can grow large.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/services/conversation.ts`:
- Around line 489-505: The current allPresetLanes branch pulls the entire
chatluna_conversation table into memory and filters by bindingKey, causing a
full-table scan; update the this.ctx.database.get call inside the
options.allPresetLanes branch to push the filter into the DB by building a query
that uses an $or over each key: one clause matching bindingKey === key and
another matching bindingKey starts-with (prefix) key + ':preset:' (or the DB's
equivalent like regex/$like), e.g. produce an array of { bindingKey: key } and {
bindingKey: { $startsWith: key + ':preset:' } } for all keys and pass that to
this.ctx.database.get; also add a sensible limit and optionally paginate results
if the table can grow large.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b6b4fec2-687b-4941-8892-518eb3551b96
📒 Files selected for processing (5)
packages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/services/conversation.tspackages/core/tests/conversation-archive.spec.tspackages/core/tests/conversation-e2e.spec.tspackages/core/tests/rollback-chat.spec.ts
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR replaces the legacy room-based architecture with a new conversation runtime system, providing a cleaner, more flexible model for managing chat sessions.
New Features
ConversationServicereplacing the room-basedRoomService, with a cleaner API for managing conversation sessionsConversationRuntimelayer that handles lifecycle, archiving, compression, and scheduling of chat sessionsChatChainDependencyGraphrewritten with lifecycle slot-based scheduling, replacing topological sort with a rule-based dependency engine supportingbefore/afterlifecycle anchors and better error messagesautoSummarizeTitletoChatInterfacefor automatic conversation title generation after the first exchangearchive.tsandcompression.tsutilities for conversation data managementinputImageTokens/outputImageTokenstracking toadapter-geminichatluna conversationcommand replacing the room commands with a streamlined interfaceBug Fixes
chatluna.chat.*sub-commands (rollback,stop,voice) to top-levelchatluna.*for simpler accessOther Changes
create_room,set_room,resolve_room,invite_room,join_room,leave_room,kick_member,transfer_room,delete_room,list_room,clear_room,compress_room,check_room,switch_room,room_info,room_permission,set_auto_update_room)room_to_conversation.tsmigration script for upgrading existing data from the room schema to the conversation schemaextension-long-memory,extension-agent, andextension-toolsto resolve conversations via the new service APIpackages/core/test/conversation-runtime.test.tswith comprehensive tests for the new runtime