feat(audit): audit log for admin command operations#21
Merged
Conversation
- Add audit_logs table with conversation_id and admin_id indexes - Add AuditService with log/list/listByConversation methods - Add audit instrumentation to 17 admin commands: ban/unban/assign/ priority/close/reopen/mute/delete/reset/expire/ai_on/ai_off/ draft_send/draft_discard/tag/untag/note - Add /audit [N] command to view recent audit entries in Topic - Add /operations/audit web page with admin_id and action filters - Audit logging failures are silently swallowed to avoid breaking command flow - 4 new test cases covering AuditService CRUD, filters, web auth, and HTML rendering Co-authored-by: monkeycode-ai <monkeycode-ai@chaitin.com>
Walkthrough新增完整审计日志功能：创建 Changes审计日志功能
估计代码审查工作量🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.monkeycode/specs/audit-log/design.md:
- Around line 78-81: `AuditService.log()` 的失败处理与设计不一致：当前实现是空 catch，没有输出任何错误信息。请在
`AuditService` 的 `log()` 中捕获写入失败后至少执行一次 `console.error`，并保留继续执行的行为；同时把设计文档里对“只
console.error”的描述与实际实现统一，确保 `log()`、`list()` 以及 `src/domain/audit.ts` 的职责表述一致并符合
REQ-7。
- Around line 204-218: listAuditLogs currently instantiates a new AuditService
on every call, which is inconsistent with the Telegram flow that reuses a
singleton. Update the web layer so the AuditService used by listAuditLogs is
created once during startup in main.ts and injected/reused through the handle
path, then have listAuditLogs reference that shared instance instead of calling
new AuditService(handle.db) each request.
In @.monkeycode/specs/audit-log/requirements.md:
- Line 52: The audit-log requirements document describes an admin_id
single-column index, but the actual migration defines a composite index with
created_at DESC. Update the index requirement text in the audit-log spec to
match the real schema, referencing the audit log performance requirement entry
so it clearly states the (admin_id, created_at DESC) index alongside the
existing (conversation_id, created_at DESC) index.
In `@src/channels/telegram/commands.ts`:
- Around line 294-296: Move the delete audit in `commands.ts` so it only happens
after `deps.conversations.deleteConversationData(conversation.id)` succeeds;
currently `deps.audit.log({ action: "delete" })` runs too early in the Telegram
topic delete flow. Update the `deleteConversation`/topic teardown sequence to
keep `deleteConversationData()` as the commit point, then emit the delete audit
using a persistence path that does not rely on the already-removed conversation
row, and make sure any failure before that point does not produce a delete audit
entry.
In `@src/runtime/web-console.ts`:
- Around line 389-394: The audit page handler in web-console should validate
`page` as a finite positive integer before using it. Update the
`/operations/audit` GET branch in `web-console` to parse
`url.searchParams.get("page")`, reject `NaN`/non-integer values, and fall back
to a safe default like 1 before calling `options.listAuditLogs` and
`renderAuditLogs`. Make sure the normalization happens at the `pageNum`
assignment so invalid values never propagate into the `main.ts` pagination path.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f4d4c4db-cce5-46c4-a551-79ec43fa5e75
📒 Files selected for processing (10)
.monkeycode/specs/audit-log/design.md.monkeycode/specs/audit-log/requirements.mdsrc/channels/telegram/bot.tssrc/channels/telegram/commands.tssrc/channels/telegram/messages.tssrc/domain/audit.tssrc/runtime/main.tssrc/runtime/web-console.tssrc/storage/migrations/0001_initial.tstest/core.test.ts
Comment on lines
+78
to
+81
| - `log()` 使用 `INSERT INTO audit_logs (admin_id, conversation_id, action, detail, created_at) VALUES (?, ?, ?, ?, ?)` | ||
| - `created_at` 使用 `new Date().toISOString()` | ||
| - `log()` 内部 try/catch，失败时只 console.error（不依赖 pino，因为 AuditService 在命令处理中调用，保持与 ConversationService 同级的纯 domain 服务） | ||
| - `list()` 动态拼接 WHERE 子句，按 `created_at DESC` 排序 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
审计写入失败时的错误处理与实现不一致
设计文档写明 log() 失败时“只 console.error”，但 src/domain/audit.ts 的实际实现是空 catch（不记录任何信息）。这与 REQ-7“记录错误日志并继续执行”也存在偏差。请统一设计文档与实际实现，确保审计写入失败时至少输出错误日志以便排查。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.monkeycode/specs/audit-log/design.md around lines 78 - 81,
`AuditService.log()` 的失败处理与设计不一致：当前实现是空 catch，没有输出任何错误信息。请在 `AuditService` 的
`log()` 中捕获写入失败后至少执行一次 `console.error`，并保留继续执行的行为；同时把设计文档里对“只
console.error”的描述与实际实现统一，确保 `log()`、`list()` 以及 `src/domain/audit.ts` 的职责表述一致并符合
REQ-7。
Comment on lines
+204
to
+218
| ```typescript | ||
| listAuditLogs: (opts) => { | ||
| const audit = new AuditService(handle.db); | ||
| const result = audit.list({ | ||
| adminId: opts.adminId || undefined, | ||
| action: opts.action || undefined, | ||
| limit: opts.pageSize, | ||
| offset: (opts.page - 1) * opts.pageSize, | ||
| }); | ||
| return { | ||
| items: result.items.map(a => ({ ...a })), | ||
| total: result.total, | ||
| }; | ||
| }, | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
建议 Web 层复用 AuditService 单例
设计文档中 listAuditLogs 每次请求都 new AuditService(handle.db)，而 Telegram 侧（bot.ts）是注入单例。建议 main.ts 也在启动时创建单例并复用，保持两层实例化模式一致，避免不必要的对象创建。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.monkeycode/specs/audit-log/design.md around lines 204 - 218, listAuditLogs
currently instantiates a new AuditService on every call, which is inconsistent
with the Telegram flow that reuses a singleton. Update the web layer so the
AuditService used by listAuditLogs is created once during startup in main.ts and
injected/reused through the handle path, then have listAuditLogs reference that
shared instance instead of calling new AuditService(handle.db) each request.
|
|
||
| ## 5. 非功能性需求 | ||
|
|
||
| - 性能：审计日志查询使用 `(conversation_id, created_at DESC)` 复合索引和 `(admin_id)` 索引 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
索引描述与实际定义不一致
非功能性需求中写的 (admin_id) 单列索引，但迁移 SQL 实际创建的是 (admin_id, created_at DESC) 复合索引。建议更新文档以准确反映索引结构，避免后续维护者误解。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.monkeycode/specs/audit-log/requirements.md at line 52, The audit-log
requirements document describes an admin_id single-column index, but the actual
migration defines a composite index with created_at DESC. Update the index
requirement text in the audit-log spec to match the real schema, referencing the
audit log performance requirement entry so it clearly states the (admin_id,
created_at DESC) index alongside the existing (conversation_id, created_at DESC)
index.
Comment on lines
294
to
296
| await ctx.api.deleteForumTopic(Number(topicContext.topic.managementChatId), topicContext.topic.messageThreadId); | ||
| deps.audit.log({ adminId, conversationId: conversation.id, action: "delete" }); | ||
| await deps.conversations.deleteConversationData(conversation.id); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
把 delete 审计放到数据库清理成功之后。
这里先删 Telegram Topic，再写 delete 审计，最后才执行数据库清理。只要最后一步失败，就会同时留下“已删除”的审计记录、已删的 Topic 和未删的会话数据，审计结果会变成假阳性。删除审计必须跟成功提交绑定，至少要放到 deleteConversationData() 成功之后，并配套一个不依赖已删除会话行的落库方案。As per path instructions, src/**/*.ts 需重点审查 Telegram 会话销毁和数据库写入逻辑。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/channels/telegram/commands.ts` around lines 294 - 296, Move the delete
audit in `commands.ts` so it only happens after
`deps.conversations.deleteConversationData(conversation.id)` succeeds; currently
`deps.audit.log({ action: "delete" })` runs too early in the Telegram topic
delete flow. Update the `deleteConversation`/topic teardown sequence to keep
`deleteConversationData()` as the commit point, then emit the delete audit using
a persistence path that does not rely on the already-removed conversation row,
and make sure any failure before that point does not produce a delete audit
entry.
Source: Path instructions
Comment on lines
+389
to
+394
| if (url.pathname === "/operations/audit" && req.method === "GET") { | ||
| const pageNum = Math.max(1, Number(url.searchParams.get("page") ?? "1")); | ||
| const adminId = url.searchParams.get("admin_id") ?? undefined; | ||
| const action = url.searchParams.get("action") ?? undefined; | ||
| const data = options.listAuditLogs({ page: pageNum, adminId, action, pageSize: 50 }); | ||
| renderAuditLogs(res, data, pageNum, adminId, action); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
先把 page 规范成有限正整数。
Number("abc") 会得到 NaN，而这里的 Math.max(1, NaN) 仍然是 NaN。这样 /operations/audit?page=abc 会把非法页码继续传到 src/runtime/main.ts Line 386，最终把 offset=NaN 送进查询并把页面打成 500。
建议改法
- const pageNum = Math.max(1, Number(url.searchParams.get("page") ?? "1"));
+ const rawPage = Number(url.searchParams.get("page") ?? "1");
+ const pageNum = Number.isFinite(rawPage) && rawPage > 0 ? Math.floor(rawPage) : 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (url.pathname === "/operations/audit" && req.method === "GET") { | |
| const pageNum = Math.max(1, Number(url.searchParams.get("page") ?? "1")); | |
| const adminId = url.searchParams.get("admin_id") ?? undefined; | |
| const action = url.searchParams.get("action") ?? undefined; | |
| const data = options.listAuditLogs({ page: pageNum, adminId, action, pageSize: 50 }); | |
| renderAuditLogs(res, data, pageNum, adminId, action); | |
| if (url.pathname === "/operations/audit" && req.method === "GET") { | |
| const rawPage = Number(url.searchParams.get("page") ?? "1"); | |
| const pageNum = Number.isFinite(rawPage) && rawPage > 0 ? Math.floor(rawPage) : 1; | |
| const adminId = url.searchParams.get("admin_id") ?? undefined; | |
| const action = url.searchParams.get("action") ?? undefined; | |
| const data = options.listAuditLogs({ page: pageNum, adminId, action, pageSize: 50 }); | |
| renderAuditLogs(res, data, pageNum, adminId, action); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/web-console.ts` around lines 389 - 394, The audit page handler in
web-console should validate `page` as a finite positive integer before using it.
Update the `/operations/audit` GET branch in `web-console` to parse
`url.searchParams.get("page")`, reject `NaN`/non-integer values, and fall back
to a safe default like 1 before calling `options.listAuditLogs` and
`renderAuditLogs`. Make sure the normalization happens at the `pageNum`
assignment so invalid values never propagate into the `main.ts` pagination path.
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
audit_logstable with(conversation_id, created_at DESC)and(admin_id, created_at DESC)indexesAuditServicewithlog()/list()/listByConversation()— log failures silently swallowed to never break command flow/audit [N]command in Topic — view recent audit entries (default 20, max 50)/operations/auditweb page — paginated audit log with admin_id and action filtersProblem
All admin commands (ban/assign/delete/reset/close etc) had no persistent record. In multi-admin scenarios, there was no way to answer "who closed this conversation" or "who banned this contact".
Verification
npm run verifypassed (check + 45 tests + audit 0 vulns)Summary by CodeRabbit
新功能
/audit命令，可快速查询当前会话的审计记录。改进