feat(ai): draft lifecycle - send/discard/view, retention, fetch timeout+retry#16
Merged
Merged
Conversation
Walkthrough在 ChangesAI 草稿生命周期与投递流程
Sequence Diagram(s)sequenceDiagram
participant Admin as 管理员 Telegram
participant handleTopicCommand
participant AiDraftService
participant DeliveryService
participant ExternalUser as 外部用户 Telegram
rect rgba(70, 130, 180, 0.5)
note over Admin,ExternalUser: /draft send 流程
Admin->>handleTopicCommand: /draft send
handleTopicCommand->>AiDraftService: findReady(conversationId)
AiDraftService-->>handleTopicCommand: DraftRow
handleTopicCommand->>DeliveryService: create(pending)
handleTopicCommand->>ExternalUser: sendMessage(draftText)
alt 发送成功
handleTopicCommand->>DeliveryService: markSent(deliveryId)
handleTopicCommand->>AiDraftService: markSent(draftId)
else 发送失败
handleTopicCommand->>DeliveryService: markFailed(deliveryId, error)
handleTopicCommand-->>Admin: 发送失败，可重试 /draft send
end
end
rect rgba(60, 179, 113, 0.5)
note over Admin,AiDraftService: /draft discard / view 流程
Admin->>handleTopicCommand: /draft discard
handleTopicCommand->>AiDraftService: findReady(conversationId)
handleTopicCommand->>AiDraftService: markDiscarded(draftId)
handleTopicCommand-->>Admin: 草稿已丢弃
Admin->>handleTopicCommand: /draft view
handleTopicCommand->>AiDraftService: findReady(conversationId)
handleTopicCommand-->>Admin: 回显草稿文本
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 6
🤖 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 `@src/channels/telegram/commands.ts`:
- Around line 307-327: In the `subCommand === "send"` flow of `commands.ts`, the
draft send path skips the existing blocklist guard used by the normal outbound
path. Reuse the same `isBlocked(contact.id)` check from the Telegram outbound
handling in `messages.ts` before calling `ctx.api.sendMessage`, and return the
same blocked-user reply so `/draft send` cannot bypass `/ban`.
- Around line 314-323: In the Telegram send flow inside the command handler that
uses ctx.api.sendMessage, persist a new outbound text message before creating
the delivery record instead of reusing draft.sourceMessageId. Update the logic
around deps.deliveries.createPending, markSent, and markFailed so the delivery
is tied to the newly created message id, matching the standard flow used in
telegram/messages.ts. Keep deps.aiDrafts.markSent after a successful send, but
ensure the outbound message is written first so history, export, and AI context
include the real reply.
In `@src/domain/ai-drafts.ts`:
- Around line 167-188: The current DraftRow workflow in ai-drafts.ts is still a
non-atomic read-then-write sequence, so concurrent `/draft send` calls can both
consume the same ready draft. Replace the split `findReady()` +
`markSent()`/`markDiscarded()` flow with a single compare-and-swap style claim
operation on `ai_drafts` that atomically changes `ready` to a temporary
in-flight state (for example `sending`) and returns success only for the winner;
then complete with `sent` or roll back to a terminal failure state on error.
Keep `findReady`, `markSent`, and `markDiscarded` aligned with the new state
machine so duplicate sends are prevented under concurrent access.
- Around line 124-149: 4xx errors are still being retried in the AI draft fetch
flow. In the retry loop inside ai-drafts.ts, update the catch path around the
fetch/parse logic so the thrown 4xx error from the response handling is
recognized as non-retryable and immediately rethrown or exits without waiting
for AI_FETCH_RETRY_DELAY_MS. Use the existing draft-fetch retry code in the same
block where lastError, attempts, and response.status are handled to ensure only
transient failures retry while AI provider 4xx responses fail fast.
In `@src/domain/retention.ts`:
- Around line 14-18: `cleanupExpired` is mixing two time sources, so all cutoff
timestamps should be derived from the same `now` argument. Update
`RetentionService.cleanupExpired` to compute both `staleCutoff` and
`retentionCutoff` from the passed-in `now` value instead of `Date.now()`,
keeping `ai_drafts` and `messages` cleanup on a consistent time basis. Use the
existing `nowIso`/`cleanupExpired` flow as the reference point and keep the
cutoff calculations aligned throughout the method.
- Around line 41-47: The retention cleanup in retention.ts is clearing
draft_text/error for records that can still be selected by AiDrafts.findReady()
because they remain in status = 'ready'. Update the soft cleanup query to
exclude ready drafts, or if ready drafts must be anonymized, also transition
them to a terminal status so /draft view and /draft send cannot fetch a
visible-but-empty draft. Use the existing retention cleanup path and the
findReady() behavior in ai-drafts.ts to keep the lifecycle consistent.
🪄 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: 041d3bfa-98b6-4f87-884f-ad312e150a46
📒 Files selected for processing (8)
src/channels/telegram/commands.tssrc/channels/telegram/menu.tssrc/domain/ai-drafts.tssrc/domain/deliveries.tssrc/domain/retention.tssrc/runtime/main.tssrc/tools/retention-cleanup.tstest/core.test.ts
Comment on lines
+307
to
+327
| if (subCommand === "send") { | ||
| const draft = deps.aiDrafts.findReady(conversation.id); | ||
| if (!draft || !draft.draftText) { | ||
| await ctx.reply("当前没有可发送的草稿，使用 /draft 生成。"); | ||
| return true; | ||
| } | ||
| try { | ||
| const deliveryId = await deps.deliveries.createPending(draft.sourceMessageId ?? undefined, `telegram-user:${contact.externalUserId}`); | ||
| try { | ||
| await ctx.api.sendMessage(Number(contact.externalUserId), draft.draftText); | ||
| await deps.deliveries.markSent(deliveryId); | ||
| } catch (error) { | ||
| const errMsg = error instanceof Error ? error.message : String(error); | ||
| await deps.deliveries.markFailed(deliveryId, errMsg, 3); | ||
| throw error; | ||
| } | ||
| deps.aiDrafts.markSent(draft.id); | ||
| await ctx.reply("草稿已发送给外部用户。"); | ||
| } catch (error) { | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| await ctx.reply(`草稿发送失败：${msg}。草稿保留，可重试 /draft send。`); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
/draft send 需要复用现有的封禁检查。
普通外发路径在 src/channels/telegram/messages.ts:300-340 里会先拦截 isBlocked(contact.id)；这里直接对 contact.externalUserId 执行 sendMessage，会让 /ban 对草稿发送失效，管理员仍可向已封禁联系人外发消息。发送前应复用同一套封禁判断，并返回与普通回复一致的提示。
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 307 - 327, In the `subCommand
=== "send"` flow of `commands.ts`, the draft send path skips the existing
blocklist guard used by the normal outbound path. Reuse the same
`isBlocked(contact.id)` check from the Telegram outbound handling in
`messages.ts` before calling `ctx.api.sendMessage`, and return the same
blocked-user reply so `/draft send` cannot bypass `/ban`.
Source: Path instructions
Comment on lines
+314
to
+323
| const deliveryId = await deps.deliveries.createPending(draft.sourceMessageId ?? undefined, `telegram-user:${contact.externalUserId}`); | ||
| try { | ||
| await ctx.api.sendMessage(Number(contact.externalUserId), draft.draftText); | ||
| await deps.deliveries.markSent(deliveryId); | ||
| } catch (error) { | ||
| const errMsg = error instanceof Error ? error.message : String(error); | ||
| await deps.deliveries.markFailed(deliveryId, errMsg, 3); | ||
| throw error; | ||
| } | ||
| deps.aiDrafts.markSent(draft.id); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
发送成功后缺少 outbound message 落库。
常规外发流程在 src/channels/telegram/messages.ts:300-340 会先 createMessage(...)，再用新消息 id 创建 delivery。这里直接拿 draft.sourceMessageId 建 delivery 并调用 sendMessage，没有写入新的 outbound message 记录。结果是历史/导出/后续 AI 上下文都会缺少这条真实回复，而且 delivery 会错误关联到触发草稿生成的旧消息。发送前应先持久化一条新的 outbound text message，再把它的 id 传给 createPending()。
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 314 - 323, In the Telegram
send flow inside the command handler that uses ctx.api.sendMessage, persist a
new outbound text message before creating the delivery record instead of reusing
draft.sourceMessageId. Update the logic around deps.deliveries.createPending,
markSent, and markFailed so the delivery is tied to the newly created message
id, matching the standard flow used in telegram/messages.ts. Keep
deps.aiDrafts.markSent after a successful send, but ensure the outbound message
is written first so history, export, and AI context include the real reply.
Source: Path instructions
Comment on lines
+124
to
+149
| if (response.status >= 400 && response.status < 500) { | ||
| throw new Error(`AI provider returned HTTP ${response.status}`); | ||
| } | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`AI provider returned HTTP ${response.status}`); | ||
| } | ||
|
|
||
| const data = (await response.json()) as { choices?: Array<{ message?: { content?: string } }> }; | ||
| const rawText = data.choices?.[0]?.message?.content?.trim(); | ||
| if (!rawText) { | ||
| throw new Error("AI provider returned an empty draft."); | ||
| } | ||
|
|
||
| const data = (await response.json()) as { choices?: Array<{ message?: { content?: string } }> }; | ||
| const text = data.choices?.[0]?.message?.content?.trim(); | ||
| if (!text) { | ||
| throw new Error("AI provider returned an empty draft."); | ||
| const text = truncateText(rawText); | ||
| this.db | ||
| .prepare("UPDATE ai_drafts SET status = 'ready', draft_text = ?, updated_at = ? WHERE id = ?") | ||
| .run(text, nowIso(), draftId); | ||
| const elapsed = Date.now() - startTime; | ||
| void elapsed; | ||
| return { status: "ready", text }; | ||
| } catch (error) { | ||
| lastError = error instanceof Error ? error : new Error(String(error)); | ||
| if (attempts < 2) { | ||
| await new Promise((resolve) => setTimeout(resolve, AI_FETCH_RETRY_DELAY_MS)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
4xx 失败现在仍会进入重试分支。
Line 124-125 已把 4xx 当成不可恢复错误抛出，但 Line 145-149 的 catch 没有区分错误类型，所以鉴权/参数类失败依然会再等 2 秒重试一次。这和本 PR“4xx 不重试”的目标不一致，也会平白消耗一次 provider 调用。
建议修复
+class NonRetryableAiError extends Error {}
+
while (attempts < 2) {
attempts += 1;
try {
const response = await fetch(url, {
method: "POST",
@@
- if (response.status >= 400 && response.status < 500) {
- throw new Error(`AI provider returned HTTP ${response.status}`);
+ if (response.status >= 400 && response.status < 500) {
+ throw new NonRetryableAiError(`AI provider returned HTTP ${response.status}`);
}
@@
} catch (error) {
lastError = error instanceof Error ? error : new Error(String(error));
- if (attempts < 2) {
+ if (!(lastError instanceof NonRetryableAiError) && attempts < 2) {
await new Promise((resolve) => setTimeout(resolve, AI_FETCH_RETRY_DELAY_MS));
}
}
}📝 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 (response.status >= 400 && response.status < 500) { | |
| throw new Error(`AI provider returned HTTP ${response.status}`); | |
| } | |
| if (!response.ok) { | |
| throw new Error(`AI provider returned HTTP ${response.status}`); | |
| } | |
| const data = (await response.json()) as { choices?: Array<{ message?: { content?: string } }> }; | |
| const rawText = data.choices?.[0]?.message?.content?.trim(); | |
| if (!rawText) { | |
| throw new Error("AI provider returned an empty draft."); | |
| } | |
| const data = (await response.json()) as { choices?: Array<{ message?: { content?: string } }> }; | |
| const text = data.choices?.[0]?.message?.content?.trim(); | |
| if (!text) { | |
| throw new Error("AI provider returned an empty draft."); | |
| const text = truncateText(rawText); | |
| this.db | |
| .prepare("UPDATE ai_drafts SET status = 'ready', draft_text = ?, updated_at = ? WHERE id = ?") | |
| .run(text, nowIso(), draftId); | |
| const elapsed = Date.now() - startTime; | |
| void elapsed; | |
| return { status: "ready", text }; | |
| } catch (error) { | |
| lastError = error instanceof Error ? error : new Error(String(error)); | |
| if (attempts < 2) { | |
| await new Promise((resolve) => setTimeout(resolve, AI_FETCH_RETRY_DELAY_MS)); | |
| } | |
| class NonRetryableAiError extends Error {} | |
| if (response.status >= 400 && response.status < 500) { | |
| throw new NonRetryableAiError(`AI provider returned HTTP ${response.status}`); | |
| } | |
| if (!response.ok) { | |
| throw new Error(`AI provider returned HTTP ${response.status}`); | |
| } | |
| const data = (await response.json()) as { choices?: Array<{ message?: { content?: string } }> }; | |
| const rawText = data.choices?.[0]?.message?.content?.trim(); | |
| if (!rawText) { | |
| throw new Error("AI provider returned an empty draft."); | |
| } | |
| const text = truncateText(rawText); | |
| this.db | |
| .prepare("UPDATE ai_drafts SET status = 'ready', draft_text = ?, updated_at = ? WHERE id = ?") | |
| .run(text, nowIso(), draftId); | |
| const elapsed = Date.now() - startTime; | |
| void elapsed; | |
| return { status: "ready", text }; | |
| } catch (error) { | |
| lastError = error instanceof Error ? error : new Error(String(error)); | |
| if (!(lastError instanceof NonRetryableAiError) && attempts < 2) { | |
| await new Promise((resolve) => setTimeout(resolve, AI_FETCH_RETRY_DELAY_MS)); | |
| } |
🤖 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/domain/ai-drafts.ts` around lines 124 - 149, 4xx errors are still being
retried in the AI draft fetch flow. In the retry loop inside ai-drafts.ts,
update the catch path around the fetch/parse logic so the thrown 4xx error from
the response handling is recognized as non-retryable and immediately rethrown or
exits without waiting for AI_FETCH_RETRY_DELAY_MS. Use the existing draft-fetch
retry code in the same block where lastError, attempts, and response.status are
handled to ensure only transient failures retry while AI provider 4xx responses
fail fast.
Comment on lines
+167
to
+188
| findReady(conversationId: number): DraftRow | undefined { | ||
| const row = this.db | ||
| .prepare( | ||
| `SELECT * FROM ai_drafts | ||
| WHERE conversation_id = ? AND status = 'ready' | ||
| ORDER BY created_at DESC LIMIT 1`, | ||
| ) | ||
| .get(conversationId) as Record<string, unknown> | undefined; | ||
| return row ? draftFromRow(row) : undefined; | ||
| } | ||
|
|
||
| markSent(draftId: number): void { | ||
| this.db | ||
| .prepare("UPDATE ai_drafts SET status = 'sent', updated_at = ? WHERE id = ?") | ||
| .run(nowIso(), draftId); | ||
| } | ||
|
|
||
| markDiscarded(draftId: number): void { | ||
| this.db | ||
| .prepare("UPDATE ai_drafts SET status = 'discarded', updated_at = ? WHERE id = ?") | ||
| .run(nowIso(), draftId); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
给草稿发送增加原子“领取”步骤。
当前新增 API 仍然要求调用方先 findReady()，再单独 markSent()/markDiscarded()。在共享 Topic 里，两个管理员几乎同时执行 /draft send 时，都能读到同一条 ready 草稿，并在任一状态更新前各自发出一次消息，导致重复外发。这里需要 compare-and-swap 式的领取流程（例如先把 ready 原子改成临时 sending，成功后转 sent，失败时回滚/置 failed），不能继续依赖“先读后写”的两步流。
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/domain/ai-drafts.ts` around lines 167 - 188, The current DraftRow
workflow in ai-drafts.ts is still a non-atomic read-then-write sequence, so
concurrent `/draft send` calls can both consume the same ready draft. Replace
the split `findReady()` + `markSent()`/`markDiscarded()` flow with a single
compare-and-swap style claim operation on `ai_drafts` that atomically changes
`ready` to a temporary in-flight state (for example `sending`) and returns
success only for the winner; then complete with `sent` or roll back to a
terminal failure state on error. Keep `findReady`, `markSent`, and
`markDiscarded` aligned with the new state machine so duplicate sends are
prevented under concurrent access.
Source: Path instructions
Comment on lines
14
to
+18
| async cleanupExpired(now = nowIso()): Promise<number> { | ||
| let cleaned = 0; | ||
|
|
||
| const staleCutoff = new Date(Date.now() - STALE_PENDING_THRESHOLD_MS).toISOString(); | ||
| const staleResult = this.db |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
请统一用传入的 now 计算所有截止时间。
staleCutoff（Line 17）和 retentionCutoff（Line 31）现在都基于 Date.now()，但后面的 messages 清理走的是 now 参数。这样 cleanupExpired("2999-01-01T00:00:00.000Z") 之类的显式时间只会影响一半逻辑，同一次清理会对 ai_drafts 和 messages 使用不同时间基准，结果不一致。
建议修改
async cleanupExpired(now = nowIso()): Promise<number> {
let cleaned = 0;
+ const nowMs = new Date(now).getTime();
- const staleCutoff = new Date(Date.now() - STALE_PENDING_THRESHOLD_MS).toISOString();
+ const staleCutoff = new Date(nowMs - STALE_PENDING_THRESHOLD_MS).toISOString();
const staleResult = this.db
.prepare(
`UPDATE ai_drafts
SET status = 'failed', error = 'Draft generation timed out (process may have restarted)', updated_at = ?
WHERE status = 'pending' AND created_at < ?`,
)
.run(now, staleCutoff);
if (this.retentionDays > 0) {
- const retentionCutoff = new Date(Date.now() - this.retentionDays * 86400 * 1000).toISOString();
+ const retentionCutoff = new Date(nowMs - this.retentionDays * 86400 * 1000).toISOString();Also applies to: 30-31
🤖 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/domain/retention.ts` around lines 14 - 18, `cleanupExpired` is mixing two
time sources, so all cutoff timestamps should be derived from the same `now`
argument. Update `RetentionService.cleanupExpired` to compute both `staleCutoff`
and `retentionCutoff` from the passed-in `now` value instead of `Date.now()`,
keeping `ai_drafts` and `messages` cleanup on a consistent time basis. Use the
existing `nowIso`/`cleanupExpired` flow as the reference point and keep the
cutoff calculations aligned throughout the method.
Comment on lines
+41
to
+47
| const softCleaned = this.db | ||
| .prepare( | ||
| `UPDATE ai_drafts | ||
| SET draft_text = NULL, error = NULL, updated_at = ? | ||
| WHERE created_at < ? AND (draft_text IS NOT NULL OR error IS NOT NULL)`, | ||
| ) | ||
| .run(now, retentionCutoff); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
不要保留 ready 状态却把草稿内容清空。
这里会匹配所有超期且有 draft_text / error 的草稿，包括 status = 'ready'。但 src/domain/ai-drafts.ts 里的 findReady() 仍然只按 status = 'ready' 取最新记录，所以 /draft view / /draft send 会有机会拿到一个“可见但无内容”的草稿。最小修复是排除 ready；如果产品上必须脱敏 ready，这里需要同步把状态转成终态，避免后续继续把它当可发送草稿。
最小安全修复示例
const softCleaned = this.db
.prepare(
`UPDATE ai_drafts
SET draft_text = NULL, error = NULL, updated_at = ?
- WHERE created_at < ? AND (draft_text IS NOT NULL OR error IS NOT NULL)`,
+ WHERE created_at < ?
+ AND status <> 'ready'
+ AND (draft_text IS NOT NULL OR error IS NOT NULL)`,
)
.run(now, retentionCutoff);As per path instructions, "重点审查 Telegram 双向转发、管理员白名单、限流、封禁、会话销毁和数据库写入逻辑。任何可能导致非白名单成员代发、bot 自己消息回环、Topic 错路由或用户数据泄露的问题都应标为高优先级。"
📝 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
| const softCleaned = this.db | |
| .prepare( | |
| `UPDATE ai_drafts | |
| SET draft_text = NULL, error = NULL, updated_at = ? | |
| WHERE created_at < ? AND (draft_text IS NOT NULL OR error IS NOT NULL)`, | |
| ) | |
| .run(now, retentionCutoff); | |
| const softCleaned = this.db | |
| .prepare( | |
| `UPDATE ai_drafts | |
| SET draft_text = NULL, error = NULL, updated_at = ? | |
| WHERE created_at < ? | |
| AND status <> 'ready' | |
| AND (draft_text IS NOT NULL OR error IS NOT NULL)`, | |
| ) | |
| .run(now, retentionCutoff); |
🤖 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/domain/retention.ts` around lines 41 - 47, The retention cleanup in
retention.ts is clearing draft_text/error for records that can still be selected
by AiDrafts.findReady() because they remain in status = 'ready'. Update the soft
cleanup query to exclude ready drafts, or if ready drafts must be anonymized,
also transition them to a terminal status so /draft view and /draft send cannot
fetch a visible-but-empty draft. Use the existing retention cleanup path and the
findReady() behavior in ai-drafts.ts to keep the lifecycle consistent.
Source: Path instructions
…imeout+retry - Add /draft send (creates Delivery record, sendMessage to external user, marks draft as sent on success) - Add /draft discard (marks latest ready draft as discarded) - Add /draft view (shows latest ready draft without regenerating) - Add AiDraftService.findReady, markSent, markDiscard methods - Extend draft status with 'sent' and 'discarded' values - Add AI fetch 15s timeout (AbortSignal.timeout) + 1 retry on 5xx/timeout - 4xx errors skip retry, immediately mark as failed - Truncate draft text to 4000 chars, limit AI context to 12000 chars - RetentionService now cleans ai_drafts: - Stale pending (>5min) recovered as failed - Terminal drafts (sent/discarded/failed) hard-deleted past retention - Active drafts soft-cleaned (null out text/error, keep row) - Update help text and menu description for /draft subcommands - Add DeliveryService.stats() for status aggregation - 5 new test cases covering findReady, markSent/markDiscarded, stale pending recovery, terminal draft deletion, delivery stats Co-authored-by: monkeycode-ai <monkeycode-ai@chaitin.com> Co-authored-by: monkeycode-ai <monkeycode-ai@chaitin.com>
monkeycode-ai
Bot
force-pushed
the
260629-feat-ai-draft-lifecycle
branch
from
cde1313 to
01d1646
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/core.test.ts`:
- Around line 711-747: The test for AiDraftService currently only checks that
findReady() stops returning a draft, which would still pass if markSent() and
markDiscarded() accidentally set the same status. Update this test to assert the
persisted status values after calling AiDraftService.markSent() and
AiDraftService.markDiscarded(), using the draft records created via the
ai_drafts insert and the draft IDs returned by findReady() so the new state
semantics are explicitly covered.
🪄 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: a77a1bdb-4f7b-46c7-b07e-ea456b172de9
📒 Files selected for processing (7)
src/channels/telegram/commands.tssrc/channels/telegram/menu.tssrc/domain/ai-drafts.tssrc/domain/retention.tssrc/runtime/main.tssrc/tools/retention-cleanup.tstest/core.test.ts
Comment on lines
+711
to
+747
| it("marks draft as sent and discarded", async () => { | ||
| const conversations = new ConversationService(handle.db, 30); | ||
| const config = loadConfig({ | ||
| TELEGRAM_BOT_TOKEN: "token", | ||
| TELEGRAM_MANAGEMENT_CHAT_ID: "-1001", | ||
| TELEGRAM_UPDATE_MODE: "polling", | ||
| TELEGRAM_ADMIN_USER_IDS: "1", | ||
| }); | ||
| const aiDrafts = new AiDraftService(handle.db, conversations, config); | ||
| const bundle = await conversations.getOrCreateConversation({ | ||
| platform: "telegram", | ||
| externalUserId: "123", | ||
| displayName: "Test", | ||
| }); | ||
|
|
||
| handle.db | ||
| .prepare( | ||
| `INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at) | ||
| VALUES (?, 'ready', 'hello', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z')`, | ||
| ) | ||
| .run(bundle.conversation.id); | ||
| const draft = aiDrafts.findReady(bundle.conversation.id); | ||
| assert.ok(draft); | ||
|
|
||
| aiDrafts.markSent(draft.id); | ||
| assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined); | ||
|
|
||
| handle.db | ||
| .prepare( | ||
| `INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at) | ||
| VALUES (?, 'ready', 'world', '2026-01-02T00:00:00Z', '2026-01-02T00:00:00Z')`, | ||
| ) | ||
| .run(bundle.conversation.id); | ||
| const draft2 = aiDrafts.findReady(bundle.conversation.id); | ||
| assert.ok(draft2); | ||
| aiDrafts.markDiscarded(draft2.id); | ||
| assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
补上对持久化状态值的断言。
这里现在只验证 findReady() 不再返回草稿；如果 markSent() 和 markDiscarded() 都误写成同一个终态，这个用例仍然会通过，覆盖不到本 PR 新增的状态语义。
建议补丁
aiDrafts.markSent(draft.id);
assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined);
+ const sentRow = handle.db
+ .prepare("SELECT status FROM ai_drafts WHERE id = ?")
+ .get(draft.id) as { status: string };
+ assert.equal(sentRow.status, "sent");
handle.db
.prepare(
`INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at)
VALUES (?, 'ready', 'world', '2026-01-02T00:00:00Z', '2026-01-02T00:00:00Z')`,
@@
const draft2 = aiDrafts.findReady(bundle.conversation.id);
assert.ok(draft2);
aiDrafts.markDiscarded(draft2.id);
assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined);
+ const discardedRow = handle.db
+ .prepare("SELECT status FROM ai_drafts WHERE id = ?")
+ .get(draft2.id) as { status: string };
+ assert.equal(discardedRow.status, "discarded");📝 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
| it("marks draft as sent and discarded", async () => { | |
| const conversations = new ConversationService(handle.db, 30); | |
| const config = loadConfig({ | |
| TELEGRAM_BOT_TOKEN: "token", | |
| TELEGRAM_MANAGEMENT_CHAT_ID: "-1001", | |
| TELEGRAM_UPDATE_MODE: "polling", | |
| TELEGRAM_ADMIN_USER_IDS: "1", | |
| }); | |
| const aiDrafts = new AiDraftService(handle.db, conversations, config); | |
| const bundle = await conversations.getOrCreateConversation({ | |
| platform: "telegram", | |
| externalUserId: "123", | |
| displayName: "Test", | |
| }); | |
| handle.db | |
| .prepare( | |
| `INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at) | |
| VALUES (?, 'ready', 'hello', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z')`, | |
| ) | |
| .run(bundle.conversation.id); | |
| const draft = aiDrafts.findReady(bundle.conversation.id); | |
| assert.ok(draft); | |
| aiDrafts.markSent(draft.id); | |
| assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined); | |
| handle.db | |
| .prepare( | |
| `INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at) | |
| VALUES (?, 'ready', 'world', '2026-01-02T00:00:00Z', '2026-01-02T00:00:00Z')`, | |
| ) | |
| .run(bundle.conversation.id); | |
| const draft2 = aiDrafts.findReady(bundle.conversation.id); | |
| assert.ok(draft2); | |
| aiDrafts.markDiscarded(draft2.id); | |
| assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined); | |
| it("marks draft as sent and discarded", async () => { | |
| const conversations = new ConversationService(handle.db, 30); | |
| const config = loadConfig({ | |
| TELEGRAM_BOT_TOKEN: "token", | |
| TELEGRAM_MANAGEMENT_CHAT_ID: "-1001", | |
| TELEGRAM_UPDATE_MODE: "polling", | |
| TELEGRAM_ADMIN_USER_IDS: "1", | |
| }); | |
| const aiDrafts = new AiDraftService(handle.db, conversations, config); | |
| const bundle = await conversations.getOrCreateConversation({ | |
| platform: "telegram", | |
| externalUserId: "123", | |
| displayName: "Test", | |
| }); | |
| handle.db | |
| .prepare( | |
| `INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at) | |
| VALUES (?, 'ready', 'hello', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z')`, | |
| ) | |
| .run(bundle.conversation.id); | |
| const draft = aiDrafts.findReady(bundle.conversation.id); | |
| assert.ok(draft); | |
| aiDrafts.markSent(draft.id); | |
| assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined); | |
| const sentRow = handle.db | |
| .prepare("SELECT status FROM ai_drafts WHERE id = ?") | |
| .get(draft.id) as { status: string }; | |
| assert.equal(sentRow.status, "sent"); | |
| handle.db | |
| .prepare( | |
| `INSERT INTO ai_drafts (conversation_id, status, draft_text, created_at, updated_at) | |
| VALUES (?, 'ready', 'world', '2026-01-02T00:00:00Z', '2026-01-02T00:00:00Z')`, | |
| ) | |
| .run(bundle.conversation.id); | |
| const draft2 = aiDrafts.findReady(bundle.conversation.id); | |
| assert.ok(draft2); | |
| aiDrafts.markDiscarded(draft2.id); | |
| assert.equal(aiDrafts.findReady(bundle.conversation.id), undefined); | |
| const discardedRow = handle.db | |
| .prepare("SELECT status FROM ai_drafts WHERE id = ?") | |
| .get(draft2.id) as { status: string }; | |
| assert.equal(discardedRow.status, "discarded"); |
🤖 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 `@test/core.test.ts` around lines 711 - 747, The test for AiDraftService
currently only checks that findReady() stops returning a draft, which would
still pass if markSent() and markDiscarded() accidentally set the same status.
Update this test to assert the persisted status values after calling
AiDraftService.markSent() and AiDraftService.markDiscarded(), using the draft
records created via the ai_drafts insert and the draft IDs returned by
findReady() so the new state semantics are explicitly covered.
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
/draft send— creates Delivery record, sendMessage to external user, marks draft assenton success/draft discard— marks latest ready draft asdiscarded/draft view— shows latest ready draft without regenerating (saves AI calls)/draft(no args) keeps existing behavior (regenerate)AiDraftService.findReady,markSent,markDiscardedmethodssentanddiscardedvalues (no schema migration needed)AbortSignal.timeout) + 1 retry on 5xx/timeout; 4xx skips retryRetentionServicenow cleansai_drafts:failedwith timeout error messagesent/discarded/failed) hard-deleted past retention perioddraft_text/error, keep row for audit)/draftsubcommandsDeliveryService.stats()for status aggregationProblem
Drafts had no send/discard/view commands — admins had to copy-paste from Telegram.
ai_draftstable grew indefinitely with no cleanup. AI fetch had no timeout, so slow AI APIs blocked message processing. Crashedpendingdrafts stayed pending forever.Verification
npm run verifypassed (check + 30 tests + audit 0 vulns)Summary by CodeRabbit
新功能
Bug 修复