Open
Conversation
Add an optional timeout parameter to NTQQGroupApi.getGroupMember (default 15000) and pass it to the invoke listener. In MessageEncoder, import isNonNullable, declare support for 'at' messages, and implement handling for OB11 At elements: resolve display string from data.name, handle 'all' as '@全体成员', or resolve a user name by converting uin->uid and fetching group member info (with a short 50ms timeout) and falling back to user simple info on failure. Construct a text child for the mention and append it to the message preview.
Replace the logical-OR fallback with nullish coalescing for nick resolution in two encoders. Uses `this.name ?? selfInfo.nick` instead of `this.name || selfInfo.nick || 'QQ用户'`, so empty-string names are no longer treated as missing and the hardcoded `'QQ用户'` fallback was removed. Affected files: src/milky/transform/message/outgoing.ts and src/onebot11/helper/createMultiMessage.ts. Note: if both values are null/undefined, `nick` will now be undefined.
Update README badge to reflect a new minimum Node.js runtime (>=24.x). This change updates the documentation only and does not modify code or functionality.
Update dependency @hono/node-server from ^1.19.14 to ^2.0.0. This is a major version upgrade.
Add a logger.info(process.argv) call in the main onLoad initialization so runtime arguments are recorded centrally. Remove the identical logger.info from PMHQBase constructor to avoid duplicate logs and keep argument logging localized to the main startup flow.
Import ChatType from ntqqapi types and replace ctx.ntFileApi.getVideoUrl(...) with ctx.ntFileApi.getVideoUrlByPacket(element.videoElement!.fileUuid, message.chatType === ChatType.Group). This simplifies video temp URL retrieval by using the file UUID and an isGroup flag instead of passing chat-specific ids (peerUid/guildId/msgId).
Introduce a new SetInputStatus action (src/onebot11/action/llbot/user/SetInputStatus.ts) and register it in the action map. Add ActionName.SetInputStatus enum entry. Expose sendShowInputStatusReq in NTQQMsgApi and declare it on NodeIKernelMsgService (including ChatType import). The action resolves a user's uid from uin and calls the new API (ChatType.C2C) and errors on non-zero results.
- Start two LLBot instances connecting to pmhq1/pmhq2 - Wait for OB11 HTTP endpoints to be ready - Run OneBot11 API tests with dual accounts - Output logs and upload test report on completion
Contributor
审阅者指南新增用于单元测试、构建和 OneBot11 端到端（E2E）测试的 CI 工作流；扩展 OneBot11 对 @ 提及和输入状态处理的支持；更新 NTQQ API 和消息转换逻辑；引入单元测试及其测试配置；并刷新 Node/Yarn/依赖包版本以及日志行为。 OneBot11
|
| 变更 | 详情 | 文件 |
|---|---|---|
| 扩展 OneBot11 消息编码与动作以支持 @ 提及和输入状态事件。 |
|
src/onebot11/helper/createMultiMessage.tssrc/milky/transform/message/outgoing.tssrc/onebot11/action/index.tssrc/onebot11/action/types.tssrc/onebot11/action/llbot/user/SetInputStatus.ts |
| 增强 NTQQ API，以支持群成员查询、输入状态和视频 URL。 |
|
src/ntqqapi/api/group.tssrc/ntqqapi/api/msg.tssrc/ntqqapi/services/NodeIKernelMsgService.tssrc/milky/transform/message/incoming.ts |
| 引入基于 Vitest 的单元测试基础设施，并为 OneBot11 事件过滤和其他子系统增加覆盖。 |
|
vitest.config.tspackage.jsontest/unit/eventfilter.test.tstest/unit/milky/api.test.tstest/unit/milky/transform.test.tstest/unit/satori/server.test.tstest/unit/satori/utils.test.tstest/unit/setup.ts |
| 新增 GitHub Actions CI 工作流，用于测试、构建验证、Docker 构建和带报告的 OneBot11 E2E 测试。 |
|
.github/workflows/test.ymldocker/Dockerfile.test |
| 更新工具链、运行时和 Yarn 配置，以适配 CI 并提升安全性。 |
|
README.mdpackage.json.yarnrc.yml |
| 微调启动时进程参数的日志记录，仅输出到主日志记录器。 |
|
src/main/main.tssrc/main/pmhq/base.ts |
| 轻量的仓库整理和文档路径调整。 |
|
a/doc/教学资料.txtb/doc/教学资料.txtpackage-dist.json |
提示与命令
与 Sourcery 交互
- 触发新的审查： 在 Pull Request 中评论
@sourcery-ai review。 - 继续讨论： 直接回复 Sourcery 的审查评论。
- 从审查评论生成 GitHub Issue： 在审查评论下回复，要求 Sourcery 从该评论创建 issue。也可以直接回复
@sourcery-ai issue，从该评论创建 issue。 - 生成 Pull Request 标题： 在 Pull Request 标题的任意位置写上
@sourcery-ai，即可随时生成标题。也可以在 Pull Request 中评论@sourcery-ai title来（重新）生成标题。 - 生成 Pull Request 摘要： 在 Pull Request 正文的任意位置写上
@sourcery-ai summary，即可在对应位置生成 PR 摘要。也可以在 Pull Request 中评论@sourcery-ai summary来（重新）生成摘要。 - 生成审阅者指南： 在 Pull Request 中评论
@sourcery-ai guide，即可（重新）生成审阅者指南。 - 解决所有 Sourcery 评论： 在 Pull Request 中评论
@sourcery-ai resolve，即可将所有 Sourcery 评论标记为已解决。如果你已经处理完所有评论且不希望再看到它们，此命令很有用。 - 撤销所有 Sourcery 审查： 在 Pull Request 中评论
@sourcery-ai dismiss，即可撤销所有现有的 Sourcery 审查。如果你希望从一次全新的审查开始，尤其有用——别忘了随后评论@sourcery-ai review以触发新的审查！
自定义你的体验
访问你的 控制面板 以：
- 启用或禁用审查特性，例如 Sourcery 自动生成的 Pull Request 摘要、审阅者指南等。
- 更改审查语言。
- 添加、删除或编辑自定义审查指令。
- 调整其他审查设置。
获取帮助
Original review guide in English
Reviewer's Guide
Adds CI workflows for unit, build, and OneBot11 E2E tests; extends OneBot11 support with @ mentions and input status handling; updates NTQQ APIs and message transforms; introduces unit tests and test configuration; and refreshes Node/Yarn/package dependencies and logging behavior.
Sequence diagram for OneBot11 set_input_status action
sequenceDiagram
actor OneBotClient
participant OneBotAdapter
participant SetInputStatus
participant Context
participant NTUserApi
participant NTQQMsgApi
participant PMHQ
participant NodeIKernelMsgService
OneBotClient->>OneBotAdapter: send ActionName.SetInputStatus
OneBotAdapter->>SetInputStatus: dispatch payload{user_id,event_type}
SetInputStatus->>SetInputStatus: validate payloadSchema
SetInputStatus->>Context: access ntUserApi
SetInputStatus->>NTUserApi: getUidByUin(uin)
NTUserApi-->>SetInputStatus: uid
SetInputStatus->>SetInputStatus: ensure uid exists
SetInputStatus->>Context: access ntMsgApi
SetInputStatus->>NTQQMsgApi: sendShowInputStatusReq(ChatType.C2C,eventType,uid)
NTQQMsgApi->>PMHQ: invoke nodeIKernelMsgService/sendShowInputStatusReq
PMHQ->>NodeIKernelMsgService: sendShowInputStatusReq(ChatType,eventType,toUid)
NodeIKernelMsgService-->>PMHQ: GeneralCallResult
PMHQ-->>NTQQMsgApi: GeneralCallResult
NTQQMsgApi-->>SetInputStatus: GeneralCallResult
SetInputStatus->>SetInputStatus: check result
SetInputStatus-->>OneBotAdapter: success(null) or error
OneBotAdapter-->>OneBotClient: action response
Sequence diagram for MessageEncoder handling @ mentions in group messages
sequenceDiagram
actor OneBotClient
participant MessageEncoder
participant Context
participant NTUserApi
participant NTGroupApi
OneBotClient->>MessageEncoder: add segment type=At data{qq,name}
MessageEncoder->>MessageEncoder: detect type At
MessageEncoder->>MessageEncoder: check isGroup
alt not group chat
MessageEncoder-->>OneBotClient: ignore At segment
else group chat
alt data.name isNonNullable
MessageEncoder->>MessageEncoder: str = "@" + data.name
else data.qq == all
MessageEncoder->>MessageEncoder: str = "@全体成员"
else resolve by qq
MessageEncoder->>Context: access ntUserApi
MessageEncoder->>NTUserApi: getUidByUin(data.qq, peer.peerUid)
NTUserApi-->>MessageEncoder: uid
MessageEncoder->>Context: access ntGroupApi
MessageEncoder->>NTGroupApi: getGroupMember(peer.peerUid, uid, false, 50)
NTGroupApi-->>MessageEncoder: GroupMemberInfo
alt member info available
MessageEncoder->>MessageEncoder: str = "@" + (cardName or nick)
else fallback user info
MessageEncoder->>NTUserApi: getUserSimpleInfo(uid)
NTUserApi-->>MessageEncoder: UserSimpleInfo
MessageEncoder->>MessageEncoder: str = "@" + coreInfo.nick
end
end
MessageEncoder->>MessageEncoder: push child text{str}
MessageEncoder->>MessageEncoder: append str to preview
end
Class diagram for updated NTQQ messaging APIs and OneBot11 actions
classDiagram
class MessageEncoder {
+static support: string[]
+results: Msg_Message_Input[]
+children: Msg_Elem_Input[]
+content: Buffer
+name: string
+isGroup: boolean
+peer: Peer
+preview: string
+ctx: Context
+flush(): Promise~void~
+handleSegment(type: OB11MessageDataType, data: any): Promise~void~
}
class NTQQGroupApi {
+getGroupMember(groupCode: string, uid: string, forceUpdate: boolean, timeout: number): Promise~GroupMemberInfo~
}
class NTQQMsgApi {
+setContactLocalTop(peer: Peer, isTop: boolean): Promise~GeneralCallResult~
+sendShowInputStatusReq(chatType: ChatType, eventType: number, toUid: string): Promise~GeneralCallResult~
}
class NodeIKernelMsgService {
<<interface>>
+setContactLocalTop(peer: Peer, isTop: boolean): Promise~GeneralCallResult~
+sendShowInputStatusReq(chatType: ChatType, eventType: number, toUid: string): Promise~GeneralCallResult~
}
class SetInputStatus {
<<BaseAction<Payload,null>>>
+actionName: ActionName
+payloadSchema: Schema
+_handle(payload: Payload): Promise~null~
}
class Payload {
+user_id: number | string
+event_type: number | string
}
class Context {
+ntUserApi: NTUserApi
+ntMsgApi: NTQQMsgApi
+ntGroupApi: NTQQGroupApi
}
class NTUserApi {
+getUidByUin(uin: string, peerUid: string): Promise~string~
+getUserSimpleInfo(uid: string): Promise~UserSimpleInfo~
}
class NTGroupApi {
+getGroupMember(groupCode: string, uid: string, forceUpdate: boolean, timeout: number): Promise~GroupMemberInfo~
}
class OB11MessageDataType {
}
class Peer {
+peerUid: string
}
class Msg_Message_Input {
}
class Msg_Elem_Input {
}
class GroupMemberInfo {
+cardName: string
+nick: string
}
class UserSimpleInfoCore {
+nick: string
}
class UserSimpleInfo {
+coreInfo: UserSimpleInfoCore
}
class GeneralCallResult {
+result: number
+errMsg: string
}
class ActionName {
<<enum>>
+SetInputStatus
}
class ChatType {
<<enum>>
+C2C
+Group
}
MessageEncoder --> Context : uses
MessageEncoder --> NTUserApi : resolves_uid_for_at
MessageEncoder --> NTGroupApi : resolves_group_member_for_at
NTQQMsgApi ..|> NodeIKernelMsgService : invokes_via_pmhq
SetInputStatus --> Context : uses
SetInputStatus --> NTQQMsgApi : calls_sendShowInputStatusReq
SetInputStatus --> NTUserApi : calls_getUidByUin
SetInputStatus --> Payload : handles
SetInputStatus --> ActionName : identifies_as
NTGroupApi --> GroupMemberInfo : returns
NTUserApi --> UserSimpleInfo : returns
UserSimpleInfo --> UserSimpleInfoCore : contains
Context --> NTUserApi
Context --> NTQQMsgApi
Context --> NTGroupApi
File-Level Changes
| Change | Details | Files |
|---|---|---|
| Extend OneBot11 message encoding and actions to support @ mentions and input status events. |
|
src/onebot11/helper/createMultiMessage.tssrc/milky/transform/message/outgoing.tssrc/onebot11/action/index.tssrc/onebot11/action/types.tssrc/onebot11/action/llbot/user/SetInputStatus.ts |
| Enhance NTQQ APIs for group member lookups, input status, and video URLs. |
|
src/ntqqapi/api/group.tssrc/ntqqapi/api/msg.tssrc/ntqqapi/services/NodeIKernelMsgService.tssrc/milky/transform/message/incoming.ts |
| Introduce Vitest-based unit test infrastructure and add coverage for OneBot11 event filtering and other subsystems. |
|
vitest.config.tspackage.jsontest/unit/eventfilter.test.tstest/unit/milky/api.test.tstest/unit/milky/transform.test.tstest/unit/satori/server.test.tstest/unit/satori/utils.test.tstest/unit/setup.ts |
| Add GitHub Actions CI workflow for tests, build verification, Docker build, and OneBot11 E2E tests with reporting. |
|
.github/workflows/test.ymldocker/Dockerfile.test |
| Update tooling, runtime, and Yarn configuration for CI compatibility and security posture. |
|
README.mdpackage.json.yarnrc.yml |
| Tweak logging of process arguments during startup to main logger only. |
|
src/main/main.tssrc/main/pmhq/base.ts |
| Minor repository housekeeping and doc path adjustments. |
|
a/doc/教学资料.txtb/doc/教学资料.txtpackage-dist.json |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
Test Report
✅ All tests passed |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 5 个问题，并留下了一些整体性的反馈：
- 在
NTQQGroupApi.getGroupMember中，新增加的timeout参数默认是 15000ms，但在createMultiMessage里的调用传的是false, 50，这现在意味着超时时间变成了 50ms，而不是之前的dataSource值——这很可能过短，会导致频繁超时；建议重新调整参数顺序/含义，或者改用更清晰的 options 对象。 - 将
vitest.config.ts中的setupFiles从test/webui/setup.ts改成test/unit/setup.ts，会导致 WebUI 测试不再执行之前的初始化；如果这些测试依赖那部分初始化逻辑，建议改为为每个项目/每个命令配置独立的 setup，而不是使用单一的全局文件。 - e2e 测试步骤对
npm test使用了continue-on-error: true，因此即使 OneBot11 API 测试失败，e2e-test这个 job 也依然可以成功；如果 e2e 测试是用来作为 PR 的阻塞条件，应该移除continue-on-error，或显式地处理失败情况。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- 在 `NTQQGroupApi.getGroupMember` 中，新增加的 `timeout` 参数默认是 15000ms，但在 `createMultiMessage` 里的调用传的是 `false, 50`，这现在意味着超时时间变成了 50ms，而不是之前的 `dataSource` 值——这很可能过短，会导致频繁超时；建议重新调整参数顺序/含义，或者改用更清晰的 options 对象。
- 将 `vitest.config.ts` 中的 `setupFiles` 从 `test/webui/setup.ts` 改成 `test/unit/setup.ts`，会导致 WebUI 测试不再执行之前的初始化；如果这些测试依赖那部分初始化逻辑，建议改为为每个项目/每个命令配置独立的 setup，而不是使用单一的全局文件。
- e2e 测试步骤对 `npm test` 使用了 `continue-on-error: true`，因此即使 OneBot11 API 测试失败，`e2e-test` 这个 job 也依然可以成功；如果 e2e 测试是用来作为 PR 的阻塞条件，应该移除 `continue-on-error`，或显式地处理失败情况。
## Individual Comments
### Comment 1
<location path=".yarnrc.yml" line_range="1-4" />
<code_context>
+approvedGitRepositories:
+ - "**"
+
+enableScripts: true
+
nodeLinker: node-modules
</code_context>
<issue_to_address>
**🚨 issue (security):** 全局启用脚本并允许所有 git 仓库会显著增加供应链风险。
使用 `approvedGitRepositories: [
</issue_to_address>
### Comment 2
<location path="src/main/main.ts" line_range="175" />
<code_context>
ctx.inject(['logger'], (ctx) => {
ctx.logger.exporter(new Log(ctx, true))
ctx.logger.info(`LLBot ${version}`)
+ ctx.logger.info(process.argv)
})
// setFFMpegPath(config.ffmpeg || '')
</code_context>
<issue_to_address>
**🚨 issue (security):** 在启动时记录 `process.argv` 可能会无意中泄露敏感信息。
CLI 参数中往往包含 token、密码或其他机密信息。每次启动都记录这些参数，会有将凭据泄露到日志或外部日志聚合系统的风险。如果你只是为了排查问题需要这条日志，请将其放在调试开关之后，并/或对敏感值做脱敏处理，或者干脆删除这条日志。
</issue_to_address>
### Comment 3
<location path="src/onebot11/action/llbot/user/SetInputStatus.ts" line_range="18-21" />
<code_context>
+ const uin = payload.user_id.toString()
+ const uid = await this.ctx.ntUserApi.getUidByUin(uin)
+ if (!uid) throw new Error('无法获取用户信息')
+ const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, +payload.event_type, uid)
+ if (result.result !== 0) {
+ throw new Error(result.errMsg)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 使用一元 `+` 强制转换 `event_type` 可能会将 `NaN` 或意外的值传给 API。
由于 `event_type` 的类型是 `number | string`，`+payload.event_type` 依赖隐式类型转换，对于非法字符串会产生 `NaN`，并被直接传入 `sendShowInputStatusReq`。建议显式地解析和校验这个值（例如使用 `parseInt` 再做范围/白名单检查），如果无效就尽早失败，而不是向 API 发送错误的状态码。
```suggestion
const uin = payload.user_id.toString()
const uid = await this.ctx.ntUserApi.getUidByUin(uin)
if (!uid) throw new Error('无法获取用户信息')
const eventType =
typeof payload.event_type === 'number'
? payload.event_type
: Number(payload.event_type)
if (!Number.isFinite(eventType)) {
throw new Error('无效的 event_type 参数')
}
const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, eventType, uid)
```
</issue_to_address>
### Comment 4
<location path="test/unit/eventfilter.test.ts" line_range="176-180" />
<code_context>
+ })
+ })
+
+ describe('无效过滤器', () => {
+ it('无效 filter 应返回 false', () => {
+ expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false)
+ })
</code_context>
<issue_to_address>
**suggestion (testing):** 建议扩展无效过滤器测试用例，覆盖字段条件内部的错误操作符用法以及结构畸形的情况。
当前的“无效过滤器”测试只覆盖了一个未知的顶层操作符（`{ $invalidOp: 123 }`）。为了更好地覆盖校验逻辑，请补充类似如下的用例：
- 字段下的未知操作符：`{ post_type: { $foo: 'bar' } }`。
- 操作符使用了错误类型的值（例如：`$in` 的值不是数组，`$gt` 使用了不支持的类型）。
- 混合有效和无效条件：`{ post_type: 'message', invalid: { $bad: 1 } }`，并断言无效部分会不会导致整个过滤器被拒绝。
这将有助于明确对畸形过滤器的行为预期，并防止校验逻辑在未来回归。
```suggestion
describe('无效过滤器', () => {
it('无效顶层操作符应返回 false', () => {
expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false)
})
it('字段内部未知操作符应返回 false', () => {
expect(matchEventFilter(
{ post_type: { $foo: 'bar' } } as any,
makeEvent()
)).toBe(false)
})
it('$in 的值不是数组应返回 false', () => {
expect(matchEventFilter(
{ post_type: { $in: 'message' as any } },
makeEvent()
)).toBe(false)
})
it('$gt 使用不支持的值类型应返回 false', () => {
expect(matchEventFilter(
{ message_id: { $gt: { not: 'a primitive' } as any } },
makeEvent()
)).toBe(false)
})
it('混合有效和无效条件时应整体返回 false（无效条件使整个过滤器失效）', () => {
expect(matchEventFilter(
{
post_type: 'message', // 有效条件
invalid: { $bad: 1 } as any // 无效条件
},
makeEvent()
)).toBe(false)
})
it('字段结构畸形（非对象且包含操作符样式字段）应返回 false', () => {
expect(matchEventFilter(
{ post_type: [] as any },
makeEvent()
)).toBe(false)
})
})
```
</issue_to_address>
### Comment 5
<location path="src/onebot11/helper/createMultiMessage.ts" line_range="390" />
<code_context>
this.content = extra
}
this.preview += `[文件] ${fileName}`
+ } else if (type === OB11MessageDataType.At) {
+ if (!this.isGroup) {
+ return
</code_context>
<issue_to_address>
**issue (complexity):** 建议将新增的 `At` 处理逻辑抽取到一个返回 @ 文本的辅助方法中，并通过 early return 简化主流程控制。
你可以通过将 `At` 分支中的逻辑抽取到一个独立的 helper 中，并使用 early return 来减少嵌套，从而降低新增逻辑带来的复杂度。这样可以让 `_handle` 更专注于组装 `children`/`preview`，而把 API 调用和错误处理等细节移出主流程。
例如：
```ts
// inside MessageEncoder
private async resolveAtText(data: AtData): Promise<string | undefined> {
if (!this.isGroup) return
if (isNonNullable(data.name)) {
return `@${data.name}`
}
if (data.qq === 'all') {
return '@全体成员'
}
const uid = await this.ctx.ntUserApi.getUidByUin(
data.qq,
this.isGroup ? this.peer.peerUid : undefined,
)
try {
const info = await this.ctx.ntGroupApi.getGroupMember(this.peer.peerUid, uid, false, 50)
return `@${info.cardName || info.nick}`
} catch {
const info = await this.ctx.ntUserApi.getUserSimpleInfo(uid)
return `@${info.coreInfo.nick}`
}
}
```
随后主分支可以变得更简单：
```ts
} else if (type === OB11MessageDataType.At) {
const str = await this.resolveAtText(data)
if (!str) return
this.children.push({
text: { str },
})
this.preview += str
}
```
这样可以在保留现有行为（包括所有 API 调用和回退逻辑）的同时，降低嵌套层次，让主方法更易阅读和扩展。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据反馈改进后续的评审。
Original comment in English
Hey - I've found 5 issues, and left some high level feedback:
- In
NTQQGroupApi.getGroupMemberthe newtimeoutparameter defaults to 15000ms, but the call increateMultiMessagepassesfalse, 50, which now means a 50ms timeout instead of the previousdataSourcevalue — this is likely far too short and will cause frequent timeouts; consider reordering/adjusting arguments or using a clearer options object. - Changing
vitest.config.tssetupFilesfromtest/webui/setup.tstotest/unit/setup.tsmeans the WebUI tests no longer run their previous setup; if those tests depend on that initialization, you may want to configure per-project/per-command setup instead of a single global file. - The e2e test step uses
continue-on-error: truefornpm test, so thee2e-testjob can succeed even when the OneBot11 API tests fail; if e2e tests are intended to gate PRs, you should removecontinue-on-erroror handle failures explicitly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NTQQGroupApi.getGroupMember` the new `timeout` parameter defaults to 15000ms, but the call in `createMultiMessage` passes `false, 50`, which now means a 50ms timeout instead of the previous `dataSource` value — this is likely far too short and will cause frequent timeouts; consider reordering/adjusting arguments or using a clearer options object.
- Changing `vitest.config.ts` `setupFiles` from `test/webui/setup.ts` to `test/unit/setup.ts` means the WebUI tests no longer run their previous setup; if those tests depend on that initialization, you may want to configure per-project/per-command setup instead of a single global file.
- The e2e test step uses `continue-on-error: true` for `npm test`, so the `e2e-test` job can succeed even when the OneBot11 API tests fail; if e2e tests are intended to gate PRs, you should remove `continue-on-error` or handle failures explicitly.
## Individual Comments
### Comment 1
<location path=".yarnrc.yml" line_range="1-4" />
<code_context>
+approvedGitRepositories:
+ - "**"
+
+enableScripts: true
+
nodeLinker: node-modules
</code_context>
<issue_to_address>
**🚨 issue (security):** Enabling scripts globally and approving all git repositories can increase supply-chain risk.
With `approvedGitRepositories: [
</issue_to_address>
### Comment 2
<location path="src/main/main.ts" line_range="175" />
<code_context>
ctx.inject(['logger'], (ctx) => {
ctx.logger.exporter(new Log(ctx, true))
ctx.logger.info(`LLBot ${version}`)
+ ctx.logger.info(process.argv)
})
// setFFMpegPath(config.ffmpeg || '')
</code_context>
<issue_to_address>
**🚨 issue (security):** Logging `process.argv` on startup may unintentionally expose sensitive information.
CLI arguments often include tokens, passwords, or other secrets. Logging them on every start risks leaking credentials to logs and external aggregators. If you need this for troubleshooting, please gate it behind a debug flag and/or redact sensitive values, or remove it entirely.
</issue_to_address>
### Comment 3
<location path="src/onebot11/action/llbot/user/SetInputStatus.ts" line_range="18-21" />
<code_context>
+ const uin = payload.user_id.toString()
+ const uid = await this.ctx.ntUserApi.getUidByUin(uin)
+ if (!uid) throw new Error('无法获取用户信息')
+ const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, +payload.event_type, uid)
+ if (result.result !== 0) {
+ throw new Error(result.errMsg)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Coercing `event_type` with unary `+` may pass `NaN` or unexpected values to the API.
Because `event_type` is `number | string`, `+payload.event_type` depends on implicit coercion and may produce `NaN` for invalid strings, which is then passed into `sendShowInputStatusReq`. Consider explicitly parsing and validating this value (e.g. `parseInt` plus range/whitelist checks) and failing fast if it’s invalid, rather than sending a bad code to the API.
```suggestion
const uin = payload.user_id.toString()
const uid = await this.ctx.ntUserApi.getUidByUin(uin)
if (!uid) throw new Error('无法获取用户信息')
const eventType =
typeof payload.event_type === 'number'
? payload.event_type
: Number(payload.event_type)
if (!Number.isFinite(eventType)) {
throw new Error('无效的 event_type 参数')
}
const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, eventType, uid)
```
</issue_to_address>
### Comment 4
<location path="test/unit/eventfilter.test.ts" line_range="176-180" />
<code_context>
+ })
+ })
+
+ describe('无效过滤器', () => {
+ it('无效 filter 应返回 false', () => {
+ expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false)
+ })
</code_context>
<issue_to_address>
**suggestion (testing):** Broaden invalid filter tests to include bad operator usage inside field conditions and malformed structures
The current "无效过滤器" tests only an unknown top-level operator (`{ $invalidOp: 123 }`). To better cover validation, please also add cases like:
- Unknown operator under a field: `{ post_type: { $foo: 'bar' } }`.
- Operator with a value of the wrong type (e.g. `$in` with non-array, `$gt` with unsupported type).
- Mixed valid/invalid filter: `{ post_type: 'message', invalid: { $bad: 1 } }`, asserting whether the invalid part causes the whole filter to be rejected.
This will clarify behavior for malformed filters and protect against regressions in validation logic.
```suggestion
describe('无效过滤器', () => {
it('无效顶层操作符应返回 false', () => {
expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false)
})
it('字段内部未知操作符应返回 false', () => {
expect(matchEventFilter(
{ post_type: { $foo: 'bar' } } as any,
makeEvent()
)).toBe(false)
})
it('$in 的值不是数组应返回 false', () => {
expect(matchEventFilter(
{ post_type: { $in: 'message' as any } },
makeEvent()
)).toBe(false)
})
it('$gt 使用不支持的值类型应返回 false', () => {
expect(matchEventFilter(
{ message_id: { $gt: { not: 'a primitive' } as any } },
makeEvent()
)).toBe(false)
})
it('混合有效和无效条件时应整体返回 false（无效条件使整个过滤器失效）', () => {
expect(matchEventFilter(
{
post_type: 'message', // 有效条件
invalid: { $bad: 1 } as any // 无效条件
},
makeEvent()
)).toBe(false)
})
it('字段结构畸形（非对象且包含操作符样式字段）应返回 false', () => {
expect(matchEventFilter(
{ post_type: [] as any },
makeEvent()
)).toBe(false)
})
})
```
</issue_to_address>
### Comment 5
<location path="src/onebot11/helper/createMultiMessage.ts" line_range="390" />
<code_context>
this.content = extra
}
this.preview += `[文件] ${fileName}`
+ } else if (type === OB11MessageDataType.At) {
+ if (!this.isGroup) {
+ return
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the new `At` handling logic into a helper method that returns the mention text and using early returns to simplify the main control flow.
You can reduce the added complexity in the `At` branch by extracting the logic into a helper and using early returns to flatten the control flow. This keeps `_handle` focused on assembling `children`/`preview` and moves API/error-handling out.
For example:
```ts
// inside MessageEncoder
private async resolveAtText(data: AtData): Promise<string | undefined> {
if (!this.isGroup) return
if (isNonNullable(data.name)) {
return `@${data.name}`
}
if (data.qq === 'all') {
return '@全体成员'
}
const uid = await this.ctx.ntUserApi.getUidByUin(
data.qq,
this.isGroup ? this.peer.peerUid : undefined,
)
try {
const info = await this.ctx.ntGroupApi.getGroupMember(this.peer.peerUid, uid, false, 50)
return `@${info.cardName || info.nick}`
} catch {
const info = await this.ctx.ntUserApi.getUserSimpleInfo(uid)
return `@${info.coreInfo.nick}`
}
}
```
Then the main branch becomes much simpler:
```ts
} else if (type === OB11MessageDataType.At) {
const str = await this.resolveAtText(data)
if (!str) return
this.children.push({
text: { str },
})
this.preview += str
}
```
This preserves behavior (including all the API calls and fallbacks) while reducing nesting and making the main method easier to read and extend.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1
to
+4
| approvedGitRepositories: | ||
| - "**" | ||
|
|
||
| enableScripts: true |
Contributor
There was a problem hiding this comment.
🚨 issue (security): 全局启用脚本并允许所有 git 仓库会显著增加供应链风险。
使用 `approvedGitRepositories: [
Original comment in English
🚨 issue (security): Enabling scripts globally and approving all git repositories can increase supply-chain risk.
With `approvedGitRepositories: [
| ctx.inject(['logger'], (ctx) => { | ||
| ctx.logger.exporter(new Log(ctx, true)) | ||
| ctx.logger.info(`LLBot ${version}`) | ||
| ctx.logger.info(process.argv) |
Contributor
There was a problem hiding this comment.
🚨 issue (security): 在启动时记录 process.argv 可能会无意中泄露敏感信息。
CLI 参数中往往包含 token、密码或其他机密信息。每次启动都记录这些参数，会有将凭据泄露到日志或外部日志聚合系统的风险。如果你只是为了排查问题需要这条日志，请将其放在调试开关之后，并/或对敏感值做脱敏处理，或者干脆删除这条日志。
Original comment in English
🚨 issue (security): Logging process.argv on startup may unintentionally expose sensitive information.
CLI arguments often include tokens, passwords, or other secrets. Logging them on every start risks leaking credentials to logs and external aggregators. If you need this for troubleshooting, please gate it behind a debug flag and/or redact sensitive values, or remove it entirely.
Comment on lines
+18
to
+21
| const uin = payload.user_id.toString() | ||
| const uid = await this.ctx.ntUserApi.getUidByUin(uin) | ||
| if (!uid) throw new Error('无法获取用户信息') | ||
| const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, +payload.event_type, uid) |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): 使用一元 + 强制转换 event_type 可能会将 NaN 或意外的值传给 API。
由于 event_type 的类型是 number | string，+payload.event_type 依赖隐式类型转换，对于非法字符串会产生 NaN，并被直接传入 sendShowInputStatusReq。建议显式地解析和校验这个值（例如使用 parseInt 再做范围/白名单检查），如果无效就尽早失败，而不是向 API 发送错误的状态码。
Suggested change
| const uin = payload.user_id.toString() | |
| const uid = await this.ctx.ntUserApi.getUidByUin(uin) | |
| if (!uid) throw new Error('无法获取用户信息') | |
| const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, +payload.event_type, uid) | |
| const uin = payload.user_id.toString() | |
| const uid = await this.ctx.ntUserApi.getUidByUin(uin) | |
| if (!uid) throw new Error('无法获取用户信息') | |
| const eventType = | |
| typeof payload.event_type === 'number' | |
| ? payload.event_type | |
| : Number(payload.event_type) | |
| if (!Number.isFinite(eventType)) { | |
| throw new Error('无效的 event_type 参数') | |
| } | |
| const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, eventType, uid) |
Original comment in English
suggestion (bug_risk): Coercing event_type with unary + may pass NaN or unexpected values to the API.
Because event_type is number | string, +payload.event_type depends on implicit coercion and may produce NaN for invalid strings, which is then passed into sendShowInputStatusReq. Consider explicitly parsing and validating this value (e.g. parseInt plus range/whitelist checks) and failing fast if it’s invalid, rather than sending a bad code to the API.
Suggested change
| const uin = payload.user_id.toString() | |
| const uid = await this.ctx.ntUserApi.getUidByUin(uin) | |
| if (!uid) throw new Error('无法获取用户信息') | |
| const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, +payload.event_type, uid) | |
| const uin = payload.user_id.toString() | |
| const uid = await this.ctx.ntUserApi.getUidByUin(uin) | |
| if (!uid) throw new Error('无法获取用户信息') | |
| const eventType = | |
| typeof payload.event_type === 'number' | |
| ? payload.event_type | |
| : Number(payload.event_type) | |
| if (!Number.isFinite(eventType)) { | |
| throw new Error('无效的 event_type 参数') | |
| } | |
| const result = await this.ctx.ntMsgApi.sendShowInputStatusReq(ChatType.C2C, eventType, uid) |
Comment on lines
+176
to
+180
| describe('无效过滤器', () => { | ||
| it('无效 filter 应返回 false', () => { | ||
| expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
suggestion (testing): 建议扩展无效过滤器测试用例，覆盖字段条件内部的错误操作符用法以及结构畸形的情况。
当前的“无效过滤器”测试只覆盖了一个未知的顶层操作符（{ $invalidOp: 123 }）。为了更好地覆盖校验逻辑，请补充类似如下的用例：
- 字段下的未知操作符：
{ post_type: { $foo: 'bar' } }。 - 操作符使用了错误类型的值（例如：
$in的值不是数组，$gt使用了不支持的类型）。 - 混合有效和无效条件：
{ post_type: 'message', invalid: { $bad: 1 } }，并断言无效部分会不会导致整个过滤器被拒绝。
这将有助于明确对畸形过滤器的行为预期，并防止校验逻辑在未来回归。
Suggested change
| describe('无效过滤器', () => { | |
| it('无效 filter 应返回 false', () => { | |
| expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false) | |
| }) | |
| }) | |
| describe('无效过滤器', () => { | |
| it('无效顶层操作符应返回 false', () => { | |
| expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false) | |
| }) | |
| it('字段内部未知操作符应返回 false', () => { | |
| expect(matchEventFilter( | |
| { post_type: { $foo: 'bar' } } as any, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('$in 的值不是数组应返回 false', () => { | |
| expect(matchEventFilter( | |
| { post_type: { $in: 'message' as any } }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('$gt 使用不支持的值类型应返回 false', () => { | |
| expect(matchEventFilter( | |
| { message_id: { $gt: { not: 'a primitive' } as any } }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('混合有效和无效条件时应整体返回 false（无效条件使整个过滤器失效）', () => { | |
| expect(matchEventFilter( | |
| { | |
| post_type: 'message', // 有效条件 | |
| invalid: { $bad: 1 } as any // 无效条件 | |
| }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('字段结构畸形（非对象且包含操作符样式字段）应返回 false', () => { | |
| expect(matchEventFilter( | |
| { post_type: [] as any }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| }) |
Original comment in English
suggestion (testing): Broaden invalid filter tests to include bad operator usage inside field conditions and malformed structures
The current "无效过滤器" tests only an unknown top-level operator ({ $invalidOp: 123 }). To better cover validation, please also add cases like:
- Unknown operator under a field:
{ post_type: { $foo: 'bar' } }. - Operator with a value of the wrong type (e.g.
$inwith non-array,$gtwith unsupported type). - Mixed valid/invalid filter:
{ post_type: 'message', invalid: { $bad: 1 } }, asserting whether the invalid part causes the whole filter to be rejected.
This will clarify behavior for malformed filters and protect against regressions in validation logic.
Suggested change
| describe('无效过滤器', () => { | |
| it('无效 filter 应返回 false', () => { | |
| expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false) | |
| }) | |
| }) | |
| describe('无效过滤器', () => { | |
| it('无效顶层操作符应返回 false', () => { | |
| expect(matchEventFilter({ $invalidOp: 123 } as any, makeEvent())).toBe(false) | |
| }) | |
| it('字段内部未知操作符应返回 false', () => { | |
| expect(matchEventFilter( | |
| { post_type: { $foo: 'bar' } } as any, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('$in 的值不是数组应返回 false', () => { | |
| expect(matchEventFilter( | |
| { post_type: { $in: 'message' as any } }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('$gt 使用不支持的值类型应返回 false', () => { | |
| expect(matchEventFilter( | |
| { message_id: { $gt: { not: 'a primitive' } as any } }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('混合有效和无效条件时应整体返回 false（无效条件使整个过滤器失效）', () => { | |
| expect(matchEventFilter( | |
| { | |
| post_type: 'message', // 有效条件 | |
| invalid: { $bad: 1 } as any // 无效条件 | |
| }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| it('字段结构畸形（非对象且包含操作符样式字段）应返回 false', () => { | |
| expect(matchEventFilter( | |
| { post_type: [] as any }, | |
| makeEvent() | |
| )).toBe(false) | |
| }) | |
| }) |
| this.content = extra | ||
| } | ||
| this.preview += `[文件] ${fileName}` | ||
| } else if (type === OB11MessageDataType.At) { |
Contributor
There was a problem hiding this comment.
issue (complexity): 建议将新增的 At 处理逻辑抽取到一个返回 @ 文本的辅助方法中，并通过 early return 简化主流程控制。
你可以通过将 At 分支中的逻辑抽取到一个独立的 helper 中，并使用 early return 来减少嵌套，从而降低新增逻辑带来的复杂度。这样可以让 _handle 更专注于组装 children/preview，而把 API 调用和错误处理等细节移出主流程。
例如：
// inside MessageEncoder
private async resolveAtText(data: AtData): Promise<string | undefined> {
if (!this.isGroup) return
if (isNonNullable(data.name)) {
return `@${data.name}`
}
if (data.qq === 'all') {
return '@全体成员'
}
const uid = await this.ctx.ntUserApi.getUidByUin(
data.qq,
this.isGroup ? this.peer.peerUid : undefined,
)
try {
const info = await this.ctx.ntGroupApi.getGroupMember(this.peer.peerUid, uid, false, 50)
return `@${info.cardName || info.nick}`
} catch {
const info = await this.ctx.ntUserApi.getUserSimpleInfo(uid)
return `@${info.coreInfo.nick}`
}
}随后主分支可以变得更简单：
} else if (type === OB11MessageDataType.At) {
const str = await this.resolveAtText(data)
if (!str) return
this.children.push({
text: { str },
})
this.preview += str
}这样可以在保留现有行为（包括所有 API 调用和回退逻辑）的同时，降低嵌套层次，让主方法更易阅读和扩展。
Original comment in English
issue (complexity): Consider extracting the new At handling logic into a helper method that returns the mention text and using early returns to simplify the main control flow.
You can reduce the added complexity in the At branch by extracting the logic into a helper and using early returns to flatten the control flow. This keeps _handle focused on assembling children/preview and moves API/error-handling out.
For example:
// inside MessageEncoder
private async resolveAtText(data: AtData): Promise<string | undefined> {
if (!this.isGroup) return
if (isNonNullable(data.name)) {
return `@${data.name}`
}
if (data.qq === 'all') {
return '@全体成员'
}
const uid = await this.ctx.ntUserApi.getUidByUin(
data.qq,
this.isGroup ? this.peer.peerUid : undefined,
)
try {
const info = await this.ctx.ntGroupApi.getGroupMember(this.peer.peerUid, uid, false, 50)
return `@${info.cardName || info.nick}`
} catch {
const info = await this.ctx.ntUserApi.getUserSimpleInfo(uid)
return `@${info.coreInfo.nick}`
}
}Then the main branch becomes much simpler:
} else if (type === OB11MessageDataType.At) {
const str = await this.resolveAtText(data)
if (!str) return
this.children.push({
text: { str },
})
this.preview += str
}This preserves behavior (including all the API calls and fallbacks) while reducing nesting and making the main method easier to read and extend.
…network and volumes
Replace all @llbot/ imports with local type copies to avoid ts-jest compilation failures caused by @/ path aliases in the main project source.
This reverts commit 9de3d06.
- Install ffmpeg+curl in Dockerfile.test for voice message tests - Restart PMHQ with workspace volume mount so QQ can read test files - Restore original PMHQ containers after tests
- Mount workspace/data into PMHQ so QQ can read LLBot temp files - Add 'config' to OB11 adapter inject for get_config/set_config actions
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
2 participants
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.
Summary by Sourcery
添加 OneBot11 输入状态支持，扩展消息编码功能，并引入全面的测试与 CI 工作流。
新功能：
功能增强：
构建：
CI：
文档：
测试：
杂务：
Original summary in English
Summary by Sourcery
Add OneBot11 input status support, extend message encoding, and introduce comprehensive testing and CI workflows.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: