fix(daemon): preserve user message source metadata - #6385
Merged
Conversation
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: The data loss is real — Direction: Aligned with the active cron/loop feature work. The session timeline needing source metadata for future UI is a reasonable motivator — and even without a downstream consumer today, the normalizer should preserve Size: Cross-package change (cli → sdk-typescript → web-shell), 32 production lines + 63 test lines. Well under any escalation threshold. Approach: Scope is tight — each of the 8 files does exactly one small thing, and the whole diff reads like someone traced the data pipeline and added one field at each hop. No scope creep, no drive-by refactors. The layered approach mirrors the existing data flow. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：数据丢失确实存在—— 方向：与正在进行的 cron/loop 功能开发一致。会话时间轴未来需要 source metadata 来做 UI 渲染是合理的动机——而且即使目前没有消费方，normalizer 本来就应该 一致地保留 规模：跨包改动（cli → sdk-typescript → web-shell），32 行生产代码 + 63 行测试。远低于任何上报阈值。 方案：范围紧凑——8 个文件每个只做一件小事，整个 diff 读起来像是作者追踪了数据管道然后在每一跳加了一个字段。没有范围蔓延，没有顺手重构。分层方式与现有数据流一致。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal: To preserve Findings: No critical issues. The implementation is clean and uses existing patterns throughout:
All three test files cover the new paths. The test in Reuse check: All new logic reuses existing helpers. No parallel utilities or duplicated patterns. Real-Scenario TestingThis change is a data pipeline modification with no user-visible behavior — the timeline UI that would render
No critical blockers found. Code is minimal, well-tested, and follows project conventions. 中文说明代码审查独立方案： 为了端到端保留 发现： 没有关键问题。实现干净，完全使用现有模式：
三个测试文件都覆盖了新路径。 复用检查： 所有新逻辑都复用了现有辅助函数。没有平行的工具或重复的模式。 真实场景测试此变更是无可用户可见行为的数据管道修改——会渲染
未发现关键阻碍。 代码精简，测试充分，遵循项目规范。 — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back to think about this one. The motivation is sound — cron/loop prompts already stamp My independent proposal matched the PR's approach, which is a good sign — there isn't a materially simpler path that was missed. The code reuses existing helpers ( The one reservation: no downstream consumer renders this I'm approving this. It's a clean, minimal data-pipeline fix. ✅ 中文说明退一步想想。 动机是合理的——cron/loop prompt 已经在 user message 上标记了 我的独立方案与 PR 的方案一致，这是个好信号——没有被遗漏的更简路径。代码复用了现有辅助函数（ 唯一的保留意见：目前没有下游消费方渲染这个 我批准这个 PR。干净、精简的数据管道修复。 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jul 6, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
| await this.messageEmitter.emitUserMessage( | ||
| displayText, | ||
| record.timestamp, | ||
| record.subtype === 'cron' ? { source: 'cron' } : undefined, |
Collaborator
There was a problem hiding this comment.
[Suggestion] Two concerns on this line:
1. Hardcoded 'cron' loses 'loop' on replay. The live path (Session.ts:2812) sends source: 'loop' for loop prompts (where cronExpr === '@wakeup'), but both cron and loop prompts are recorded with subtype: 'cron' by chatRecordingService.ts. On replay, loop-originated messages will incorrectly carry source: 'cron' instead of source: 'loop'. No current UI consumer distinguishes the two, but this contradicts the PR's goal of preserving source metadata end-to-end.
Consider persisting the original source value on the chat record (e.g., in systemPayload) and reading it back during replay:
Suggested change
| record.subtype === 'cron' ? { source: 'cron' } : undefined, | |
| record.subtype === 'cron' | |
| ? { source: (record.systemPayload as { source?: string } | undefined)?.source ?? 'cron' } | |
| : undefined, |
2. No test coverage. The conditional that tags cron-subtype user messages with source metadata during replay is not tested in HistoryReplayer.test.ts. The downstream layers are individually tested, but this integration point is not — a future refactor that drops or inverts the ternary would go undetected. Consider adding test cases for (a) cron-subtype records emitting _meta.source: 'cron' and (b) notification-subtype records emitting no source.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /triage |
Collaborator
Author
|
@qwen-code /review |
Contributor
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/28844314321)._ |
Collaborator
✅ Verification — reproduced end-to-end on Linux (author tested macOS only)I built a real cross-package harness that drives the actual production code at every seam of the pipeline this PR touches — the emitter, the SDK normalizer + transcript reducer, and the web-shell adapter — plus a revert-proof pass to confirm each new test is load-bearing. Everything is green and the fix does exactly what it claims. LGTM, safe to merge. Env: Linux, Node What the PR wires (the
|
| Pipeline stage (package) | ALL-PR | base normalizer only | base adapter only |
|---|---|---|---|
② normalize → user.text.delta.meta (sdk) |
source:cron |
dropped ❌ | source:cron |
③ reduce → block.meta (sdk) |
source:cron |
dropped | source:cron |
④ adapt → message.source (web-shell) |
"cron" ✅ |
absent | absent ❌ |
→ The normalizer change is the gate at seam ②; the adapter change is the gate at seam ④. Both are individually necessary; the reducer in the middle already carried meta (seam ③ shows source present with PR-normalizer + base-adapter).
Notes for the reviewer
- Both live and replay cron turns are fixed.
Session.ts:3046already emitted_meta:{source}for live cron/loop echoes; they were silently dropped at the normalizer until now. Good catch that this also unblocks the replay path. - Extra-keys risk (self-flagged) is benign.
normalizercopies the whole_metaonto every user block'smeta(so normal turns now carrymeta:{timestamp}). I checked the consumers: the terminal renderer reads no text-blockmeta, and the only meta-gated hiding (isBackgroundNotificationAssistantBlock) targets assistant blocks and requiressource==='background_notification' + qwenDiscreteMessage + backgroundTask— a cron user block (source:'cron') cannot match. No regression. - Typecheck is neutral: the PR adds 0 type errors in any file it touches (verified via a base-vs-PR differential; the only errors in my sandbox are pre-existing
events.tsenv drift, identical on both sides).
Optional nit (non-blocking)
normalizer attaches the entire _meta (via extractUpdateMeta) rather than just source, mirroring the existing agent_message_chunk path — consistent and harmless, but if you'd prefer to be surgical you could project only the keys user blocks actually need. Fine to leave as-is for parity.
🇨🇳 中文版（点击展开）
✅ 验证结论 — 已在 Linux 上端到端复现（作者仅测了 macOS）
我搭了一个真实的跨包 harness，逐段驱动这个 PR 涉及的真实生产代码：emitter → SDK normalizer + transcript reducer → web-shell adapter，并做了 revert-proof 证明每个新增测试都是有效的（能真正卡住行为）。全部通过，修复行为与描述一致。LGTM，可以合并。
环境：Linux，Node v22.22.2，PR head 67dba9d8，merge-base 5c8af1a1。
这个 PR 接通的 source 链路
改动前，SDK normalizer 在处理 user_message_chunk 时会丢掉 _meta，所以实时 cron 路径（Session.ts:3046 已有 _meta:{source}）和历史回放路径（HistoryReplayer.ts:116，本 PR 新增）在这一段都会丢失 source。reducer、block.meta 类型、以及 agent_message_chunk 携带完整 _meta 的逻辑本来就存在——本 PR 只是把这套已有模式扩展到 user chunk，并让 web-shell adapter 去读它。
证据
- Layer A — 三个 PR 测试套件在 PR head 全绿（252 / 98 / 21）。
- Layer B — revert-proof：只把某个源文件回退到 base（保留新测试）→ 新测试失败，证明测试确实在守护该行为。
- Layer C — 单进程串联真实的
MessageEmitter→normalizeDaemonEvent→reduceDaemonTranscriptEvents→transcriptBlocksToDaemonMessages。这正是单元测试覆盖不到的：emitter 真实产出的_meta形状，正好是 normalizer 读取的；其meta正好是 reducer 合并的；其block.meta正好是 adapter 读取的。
（终端截图见上方英文版 console 代码块；A/B 表格逐段回退，可定位每处修复的作用点。）
Layer C A/B 逐段定位： normalizer 改动是 ② 处的关键闸门，adapter 改动是 ④ 处的关键闸门，两者缺一不可；中间的 reducer 本就已经透传 meta（base-adapter 那一列里 block.meta.source 仍在）。
给 reviewer 的说明
- 实时与回放两条 cron 链路都被修复了。
Session.ts:3046早就为实时 cron/loop echo 打了_meta:{source}，但一直在 normalizer 处被丢弃；这个 PR 同时打通了实时和回放两条路。 - PR 自己标注的「多余 key」风险是无害的。 normalizer 会把整个
_meta拷到每个 user block 的meta上（普通消息现在也带meta:{timestamp}）。我核对了消费方：终端渲染不读文本块的meta；唯一基于 meta 的隐藏逻辑（isBackgroundNotificationAssistantBlock）只作用于 assistant 块且需要source==='background_notification' + qwenDiscreteMessage + backgroundTask——cron user 块（source:'cron'）不可能命中。无回归。 - 类型检查中性： PR 在其改动的任何文件中都没有引入类型错误（用 base vs PR 差分验证；我沙箱里仅有的报错是
events.ts的既有环境漂移，两侧完全一致）。
可选的小建议（不阻塞）
normalizer 通过 extractUpdateMeta 拷贝了整个 _meta 而非仅 source，这与已有的 agent_message_chunk 路径保持一致，无害；如果想更精简，可只投影 user block 真正需要的字段。保持现状与既有逻辑对齐也完全可以。
| ) { | ||
| return []; | ||
| } | ||
| const meta = extractUpdateMeta(update); |
Collaborator
There was a problem hiding this comment.
[Suggestion] meta is extracted here before the content-type branch, but only propagated on user.text.delta events. The user.image.delta return (in the image branch above) omits meta entirely. This creates an asymmetric contract — the PR aims for end-to-end metadata preservation, but if a scheduled user message ever carries an image attachment, its source metadata is silently dropped.
No live bug today (cron/loop prompts are text-only), but future image-bearing scheduled messages would lose provenance without warning.
Consider propagating meta on the image path too, or adding a comment noting the intentional omission.
— qwen3.7-max via Qwen Code /review
| }); | ||
| }); | ||
|
|
||
| it('should include source metadata when provided', async () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] This test covers source + timestamp together, but the implementation has two independent conditional spreads inside _meta. The source-only branch (no timestamp) is never exercised.
A future caller passing source without timestamp — a valid parameter combination per the signature — has unverified behavior.
Suggested change
| it('should include source metadata when provided', async () => { | |
| it('should include source metadata when provided', async () => { | |
| await emitter.emitUserMessage('scheduled prompt', 1_700_000_000_000, { | |
| source: 'cron', | |
| }); | |
| expect(sendUpdateSpy).toHaveBeenCalledWith({ | |
| sessionUpdate: 'user_message_chunk', | |
| content: { type: 'text', text: 'scheduled prompt' }, | |
| _meta: { | |
| timestamp: 1_700_000_000_000, | |
| source: 'cron', | |
| }, | |
| }); | |
| }); | |
| it('should include source metadata without timestamp', async () => { | |
| await emitter.emitUserMessage('scheduled prompt', undefined, { | |
| source: 'cron', | |
| }); | |
| expect(sendUpdateSpy).toHaveBeenCalledWith({ | |
| sessionUpdate: 'user_message_chunk', | |
| content: { type: 'text', text: 'scheduled prompt' }, | |
| _meta: { source: 'cron' }, | |
| }); | |
| }); |
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jul 7, 2026
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
3 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.
What this PR does
This PR preserves source metadata on user-message transcript events end to end. Cron and loop prompts already emit
_meta.source, and this change keeps that metadata through SDK normalization, transcript blocks, web-shell message adaptation, and history replay.Why it's needed
The session timeline needs to know when a user-message turn came from a scheduled task before it can render scheduled-task-specific UI. The daemon was already marking cron-fired user chunks, but the user-message normalizer dropped
_meta, so the web shell could not distinguish cron turns from ordinary user prompts.Reviewer Test Plan
How to verify
Create or replay a session with a cron-fired prompt and confirm the corresponding user transcript block carries
source: "cron"through the SDK and web-shell adapter. The downstream timeline UI can then identify the turn as scheduled-task-originated.Evidence (Before & After)
Before:
user_message_chunk._meta.sourcewas not copied onto user text delta events, so web-shell user messages had nosourcefield.After: user text delta events and transcript blocks preserve
_meta.source, and replayed cron prompts can emit the same source metadata.Tested on
Environment (optional)
Local Node.js workspace on macOS. Verified with
cd packages/sdk-typescript && npx vitest run test/unit/daemonUi.test.ts,cd packages/web-shell && npx vitest run client/adapters/transcriptToMessages.test.ts,cd packages/cli && npx vitest run src/acp-integration/session/emitters/MessageEmitter.test.ts, andnpm run typecheck.Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
这个 PR 端到端保留 user message 的 source metadata。cron 和 loop prompt 已经会发出
_meta.source，这里把这个 metadata 继续保留到 SDK normalize、transcript block、web-shell message adapter，以及历史回放路径。为什么需要
左侧会话时间轴需要知道一个 user turn 是否来自定时任务，后续才能渲染定时任务专属 UI。daemon 已经给 cron 触发的 user chunk 打了标记，但 user-message normalizer 会丢掉
_meta，导致 web shell 无法区分 cron turn 和普通用户输入。Reviewer Test Plan
如何验证
创建或回放一个 cron 触发的 prompt，确认对应 user transcript block 会保留
source: "cron"，并能通过 SDK 和 web-shell adapter 传递。下游时间轴 UI 就可以据此识别该 turn 来自定时任务。证据（Before & After）
Before：
user_message_chunk._meta.source没有被复制到 user text delta event 上，所以 web-shell user message 没有source字段。After：user text delta event 和 transcript block 都会保留
_meta.source，历史回放的 cron prompt 也可以发出同样的 source metadata。Tested on
环境（可选）
macOS 本地 Node.js workspace。已用
cd packages/sdk-typescript && npx vitest run test/unit/daemonUi.test.ts、cd packages/web-shell && npx vitest run client/adapters/transcriptToMessages.test.ts、cd packages/cli && npx vitest run src/acp-integration/session/emitters/MessageEmitter.test.ts和npm run typecheck验证。风险和范围
关联 Issue
N/A