Skip to content

feat: rootId message injection & historyDedup reset on resume skip - #143

Merged
lishuceo merged 2 commits into
mainfrom
fix/rootid-and-dedup-reset
Mar 11, 2026
Merged

feat: rootId message injection & historyDedup reset on resume skip#143
lishuceo merged 2 commits into
mainfrom
fix/rootid-and-dedup-reset

Conversation

@lishuceo

Copy link
Copy Markdown
Owner

Summary

PR #142 的 squash merge 遗漏了后追加的 commits(rootId 注入和 _historyDedup 重置)。本 PR 补充这些修复。

  • rootId 引用消息注入: 用户回复某条消息并 @bot 时,显式获取被引用消息内容注入 prompt,确保 agent 能看到引用的 merge_forward 等内容
  • _historyDedup 重置: resume 因 hash 不匹配被跳过时,清除 afterMsgId,避免新 session 丢失历史上下文
  • sender open_id 格式统一: 实时 merge_forward 处理也改为 [名字](open_id) 格式

Test plan

  • TypeScript 类型检查通过
  • 全量测试 54/55 通过(1 个预存在的 flaky test)

🤖 Generated with Claude Code

lishuceo and others added 2 commits March 12, 2026 00:15
…ssage

When a user replies to a message (e.g. merge_forward) and @bot, the
referenced message content is now explicitly fetched via getMessageById
and prepended to the prompt. This ensures the agent can see the quoted
content even if it's not in the history window.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When system prompt hash changes and resume is skipped, the old
afterMsgId in _historyDedup would cause the new session to only see
messages after that point, losing earlier history context.

Now executor returns `resumeSkipped` flag, and executeDirectTask
clears the _historyDedup entry when resume is skipped.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Mar 11, 2026

Copy link
Copy Markdown

Claude finished @lishuceo's task in 1m 24s —— View job


Code Review: PR #143

  • Check for previous review comments
  • Gather context and read changed files
  • Review each changed file
  • Post review summary

Review Summary

Approved — The changes are well-structured and follow existing patterns in the codebase.

Overview

This PR adds three small but meaningful improvements:

  1. resumeSkipped flag (executor.ts, types.ts) — Clean propagation of whether a resume was skipped due to system prompt hash mismatch, added to all return paths in execute().

  2. _historyDedup reset on resume skip (event-handler.ts:1788-1793) — When resume is skipped, the dedup marker is cleared so the next query re-injects full history. This correctly prevents the new session from missing context that the old afterMsgId would have filtered out.

  3. rootId message injection (event-handler.ts:1695-1745) — When a user replies to a specific message (e.g. a merge_forward), the referenced message content is fetched and prepended to the prompt. This ensures the agent sees the context even if it's outside the history window.

  4. Sender open_id format change (event-handler.ts:2176-2178) — Aligns the real-time merge_forward sender format to [name](open_id), matching what the rootId injection already uses.

Detailed Notes

Things done well:

  • The rootId injection is wrapped in a try/catch with a warn-level log on failure — non-blocking and observable.
  • The rootId !== messageId guard prevents injecting the message itself.
  • The merge_forward expansion in rootId injection mirrors the existing pattern in parseMessage() and client.ts, keeping behavior consistent.
  • All four return paths in executor.execute() now include resumeSkipped.

Minor observations (not blocking):

🟡 Potential duplicate content (confidence: 65 — below reporting threshold, noting for awareness): If the rootId message is recent enough to already be included in buildDirectTaskHistory() results, it could appear twice in the prompt — once from history and once from the rootId injection. The comment at line 1696 acknowledges the primary use case is for messages outside the history window (like merge_forward), and the duplication is harmless (just extra context), so this is acceptable.

🟡 Code duplication: The merge_forward expansion logic in the rootId injection block (lines 1707-1732) closely mirrors the logic in parseMessage() (lines 2141-2186) and client.ts. This is a pattern already present in the codebase, and extracting a shared helper is out of scope for this PR.

Verdict

The changes are correct, follow existing conventions (ESM, logging patterns, error handling), and solve real problems. No bugs, security issues, or architectural concerns found.


@lishuceo
lishuceo merged commit 309db0b into main Mar 11, 2026
4 checks passed
@lishuceo
lishuceo deleted the fix/rootid-and-dedup-reset branch March 11, 2026 16:23
lishuceo added a commit that referenced this pull request Apr 8, 2026
feat: rootId message injection & historyDedup reset on resume skip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant