fix(acp-bridge): keep event attribution through turn compaction

[he-yufeng]

What this PR does

Turn-boundary compaction now keeps promptId and originatorClientId on merged events. Refs #7457 (DAEMON-007).

Why it's needed

session_update chunks carry those two fields at the envelope level, and replay consumers use them: the bridge client groups transcript text by originator, and echo suppression compares event.originatorClientId with the subscriber's own client id. Both merge helpers rebuild the envelope from scratch and drop the fields, so after compaction a resuming client loses prompt correlation and can see its own messages echoed back, while the live stream for the same turn was fine.

Text/thought slots now track the latest promptId / originatorClientId alongside the existing lastMeta / lastEnvelopeMeta and copy them onto the merged event. Folded tool calls take them from the incoming event, falling back to the existing one, same as the id merge. Events whose sources carried no attribution still emit none, and pass-through paths are untouched.

Scope note: #7457 lists three defects. DAEMON-001 is covered by #7458, and DAEMON-008's bare catch {} is the documented never-throws contract for the best-effort engine. This PR doesn't touch either.

Reviewer Test Plan

How to verify

npx vitest run src/compactionEngine.test.ts in packages/acp-bridge: 60 tests pass, three of them new (merged text keeps both fields, folded tool calls keep them with incoming winning, sources without attribution still emit none). Full package: 898 passed across 19 files.

Evidence (Before & After)

N/A (non-UI change; coverage is the new unit tests)

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	⚠️
🐧 Linux	⚠️

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓ — the What / Why / Reviewer Test Plan sections are all there and detailed. Minor housekeeping note: the formal ## Risk & Scope and ## Linked Issues headings and the Chinese <details> translation from the template are absent (the issue is linked inline as "Refs #7457" and the scope is covered in prose, so this isn't blocking — just worth folding in if you touch the body again).

Problem: real, not theoretical. This is DAEMON-007 from the open audit issue #7457, with a concrete mechanism: makeMergedSessionUpdateEvent and mergeToolCallEvent rebuild the envelope from scratch and drop the top-level promptId / originatorClientId. I confirmed the consumers depend on them — bridgeClient.ts groups replayed transcript text by originatorClientId and uses it for echo suppression, so a compacted replay genuinely diverges from the live stream. The new unit tests fail without the change.

Direction: aligned. This is a correctness fix in the daemon (qwen serve) replay path, squarely in scope, and it touches no auth/sandbox/model-selection/telemetry/public-contract surface. CHANGELOG has no direct reference, but the daemon replay area is active (the issue itself is a follow-up to #7385 / #7399).

Size: not applicable — packages/acp-bridge is not a core-protected path. 28 production lines + 49 test lines across 2 files.

Approach: the scope feels right and the discipline is good — the PR deliberately addresses only DAEMON-007's envelope attribution, leaving DAEMON-001 to #7458 and DAEMON-008's documented never-throws contract alone. The change mirrors the existing lastMeta / lastEnvelopeMeta / id bookkeeping exactly, so it reads as a natural extension rather than new machinery. One thing worth confirming, not a blocker: #7457 also lists data.sessionId among the dropped stamps, and this PR doesn't touch it. If text session_update events carry a data.sessionId that the merge drops, that gap would survive this fix — is that intentionally deferred, or does the data-level spread in mergeToolCallEvent already cover it?

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓ —— What / Why / Reviewer Test Plan 三部分齐全且详尽。一个小的格式提醒：模板里的 ## Risk & Scope、## Linked Issues 标题以及中文 <details> 翻译没有写（issue 已在正文以 "Refs #7457" 内联关联，scope 也在正文说明了，所以不阻塞——如果后续再改 PR body 时顺手补上即可）。

问题：真实存在，非理论性。这是开放审计 issue #7457 中的 DAEMON-007，机制具体：makeMergedSessionUpdateEvent 与 mergeToolCallEvent 从头重建 envelope，丢掉了顶层的 promptId / originatorClientId。我确认了消费方依赖这两个字段——bridgeClient.ts 按 originatorClientId 对重放的 transcript 文本分组并用于回声抑制，因此压缩后的重放确实会与实时流不一致。新增单测在缺少该改动时会失败。

方向：对齐。这是 daemon（qwen serve）重放路径上的正确性修复，完全在范围内，且不触及 auth/sandbox/模型选择/telemetry/公共契约。CHANGELOG 无直接引用，但 daemon 重放区域是活跃的（该 issue 本身就是 #7385 / #7399 的后续）。

规模：不适用 —— packages/acp-bridge 不属于核心保护路径。2 个文件，28 行生产代码 + 49 行测试。

方案：范围合理，纪律良好——PR 只处理 DAEMON-007 的 envelope 归属，把 DAEMON-001 留给 #7458，DAEMON-008 的"永不抛错"契约保持不动。改动完全沿用了现有 lastMeta / lastEnvelopeMeta / id 的记账模式，读起来是自然扩展而非新机制。有一点值得确认（非阻塞）：#7457 还把 data.sessionId 列为被丢掉的标记之一，而本 PR 未涉及。如果 text session_update 事件携带 data.sessionId 且被合并丢掉，那么这个缺口在本修复后仍会存在——这是有意推迟，还是 mergeToolCallEvent 里 data 层的 spread 已经覆盖了？

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 223963b3e1b7b5914855342ca924f272c6b8eb6b · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

I formed my independent take first: to keep attribution through the merge, track the latest promptId / originatorClientId on the text/thought slot (exactly like the existing lastEnvelopeMeta) and spread them onto the merged event present-only-if-set; for tool-call folding, take incoming-with-existing-fallback mirroring how id is merged. The PR does precisely this, so it matches the simplest path I'd have taken — no simpler alternative was missed.

The change is minimal and idiomatic. It threads the two fields through all four ingest sites (subagent merge/new, top-level merge/new), the snapshot emit, and both merge helpers, mirroring the surrounding bookkeeping so it reads as a natural extension. The spread is present-only-if-set, so attribution-less sources still emit nothing (one of the new tests guards exactly this). Type-correct against BridgeEvent (both fields are optional top-level), and the two added comments explain the non-obvious why (replay consumers key echo suppression / grouping on them) rather than narrating the code. No correctness, security, or regression concerns; no AGENTS.md violations. ESLint and tsc --noEmit are both clean.

On the data.sessionId question from Stage 1: looking at the code, text session_update events here carry data shaped { update: {...} } with no top-level data.sessionId, and mergeToolCallEvent already preserves any top-level data fields via its {...existingData, ...incomingData} spread. So the practical gap looks narrower than the issue's wording implies — but I'd still like the author to confirm whether any real event shape carries a data.sessionId that makeMergedSessionUpdateEvent (which rebuilds data fresh) would drop. Non-blocking.

Testing

This is a non-UI change in the daemon (qwen serve) replay/compaction path — the attribution fields are envelope-internal and not observable in terminal UI, so the meaningful evidence is the unit coverage plus a before/after at the test level. Ran in tmux on the PR checkout.

Before (base commit, fix reverted): the two new "keeps …" tests fail, demonstrating the bug — merged events come back with no attribution:

 × basic compaction > keeps promptId and originatorClientId on merged text chunks
   → expected undefined to be 'prompt-1' // Object.is equality
 × tool call folding > keeps promptId and originatorClientId on folded tool calls
   → expected undefined to be 'prompt-1' // Object.is equality
 Test Files  1 failed (1)
      Tests  2 failed | 58 skipped (60)

After (this PR): the targeted file passes in full:

 RUN  v3.2.4 .../packages/acp-bridge

 ✓ src/compactionEngine.test.ts (60 tests) 21ms

 Test Files  1 passed (1)
      Tests  60 passed (60)
   Duration  326ms

Full package regression run (tmux capture-pane):

 ✓ src/bridge.test.ts (435 tests) 5269ms

 Test Files  19 passed (19)
      Tests  898 passed (898)
   Start at  18:59:43
   Duration  9.68s (transform 5.89s, setup 0ms, collect 40.72s, tests 5.91s, environment 3ms, prepare 2.15s)

898 passed across 19 files, matching the PR's claim. No regressions.

中文说明

代码审查

我先独立形成判断：要在合并中保留归属信息，就在 text/thought slot 上追踪最新的 promptId / originatorClientId（与现有 lastEnvelopeMeta 完全一致），并以"有值才展开"的方式贴到合并事件上；tool-call 折叠则采用"incoming 优先、existing 兜底"，与 id 的合并方式一致。PR 正是这么做的，与我能想到的最简路径一致——没有遗漏更简单的方案。

改动最小且符合惯例。它在四个 ingest 位点（subagent 合并/新建、top-level 合并/新建）、snapshot 输出以及两个 merge helper 中贯通了这两个字段，沿用了周围的记账模式，读起来是自然扩展。展开是"有值才贴"，因此无归属的源仍然不输出任何东西（其中一个新单测正是守护这一点）。对 BridgeEvent 类型正确（两个字段都是顶层可选），新增的两处注释解释的是非显而易见的为什么（重放消费方依赖它们做回声抑制/分组），而非复述代码。无正确性、安全或回归隐患；无 AGENTS.md 违规。ESLint 与 tsc --noEmit 均干净。

关于 Stage 1 提到的 data.sessionId：看代码，这里的 text session_update 事件 data 形如 { update: {...} }，没有顶层 data.sessionId，而 mergeToolCallEvent 已通过 {...existingData, ...incomingData} 展开保留了任何顶层 data 字段。所以实际缺口看起来比 issue 措辞暗示的更窄——但仍希望作者确认是否存在某种真实事件形态携带 data.sessionId，会被（从头重建 data 的）makeMergedSessionUpdateEvent 丢掉。非阻塞。

测试

这是 daemon（qwen serve）重放/压缩路径上的非 UI 改动——归属字段在 envelope 内部，终端 UI 不可见，因此有意义的证据是单测覆盖加测试层面的 before/after。在 PR checkout 上用 tmux 运行。

Before（base 提交，回退修复）： 两个新的 "keeps …" 测试失败，证明 bug 存在——合并事件返回时没有归属信息（expected undefined to be 'prompt-1'，2 failed | 58 skipped）。

After（本 PR）： 目标文件全部通过（60 passed）。

整包回归（tmux capture-pane）：19 个文件 898 个测试全部通过，与 PR 声明一致，无回归。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 223963b3e1b7b5914855342ca924f272c6b8eb6b · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — a clean, minimal, well-tested correctness fix; the only open item is a non-blocking confirmation about data.sessionId.

Stepping back: this is exactly the kind of PR the gate should let through. The problem is real and verified — I confirmed in code that the merge helpers rebuilt the envelope from scratch and dropped promptId / originatorClientId, and that bridgeClient genuinely keys echo suppression and transcript grouping on them. The two new tests fail on the base commit (expected undefined to be 'prompt-1') and pass with the fix, so the coverage actually guards the bug rather than just exercising the happy path.

The implementation matches the simplest path I'd have proposed independently — thread the latest values through the existing slot bookkeeping and spread them present-only-if-set — and it mirrors the surrounding lastEnvelopeMeta / id handling so precisely that it reads like it was always there. Scope discipline is good: it fixes DAEMON-007's envelope attribution and deliberately leaves DAEMON-001 (in #7458) and DAEMON-008's documented never-throws contract alone, with no drive-by edits. Full package suite is green (898/898), lint and typecheck clean.

The one reservation, carried over from Stage 1 and non-blocking: #7457 also names data.sessionId as a dropped stamp. From the code, text session_update events don't appear to carry a top-level data.sessionId and mergeToolCallEvent already preserves top-level data fields, so the gap looks narrower than the issue wording suggests — but I'd like the author to confirm no real event shape is affected. That's a comment, not a blocker.

If I had to maintain this in six months I'd thank the author. Approving.

中文说明

置信度：4/5 —— 一个干净、最小、测试充分的正确性修复；唯一未决项是关于 data.sessionId 的非阻塞确认。

退一步看：这正是 gate 应当放行的那类 PR。问题真实且已验证——我在代码中确认 merge helper 从头重建 envelope 并丢掉了 promptId / originatorClientId，且 bridgeClient 确实依赖它们做回声抑制和 transcript 分组。两个新单测在 base 提交上失败（expected undefined to be 'prompt-1'），加上修复后通过，因此覆盖确实守护了这个 bug，而非只跑 happy path。

实现与我独立提出的最简路径一致——把最新值贯通到现有 slot 记账并以"有值才展开"的方式贴上——并且与周围 lastEnvelopeMeta / id 的处理高度一致，读起来像本来就在那里。范围纪律良好：只修 DAEMON-007 的 envelope 归属，刻意不动 DAEMON-001（在 #7458）与 DAEMON-008 文档化的"永不抛错"契约，无顺手改动。整包测试全绿（898/898），lint 与 typecheck 干净。

唯一保留意见（承自 Stage 1，非阻塞）：#7457 还把 data.sessionId 列为被丢掉的标记。从代码看，text session_update 事件似乎不携带顶层 data.sessionId，且 mergeToolCallEvent 已保留顶层 data 字段，所以缺口看起来比 issue 措辞更窄——但希望作者确认没有真实事件形态受影响。这是评论，不是阻塞项。

如果半年后由我来维护，我会感谢作者。批准。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 223963b3e1b7b5914855342ca924f272c6b8eb6b · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

[wenshao]

@qwen-code /resolve

[qwen-code-dev-bot]

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge Conflict Resolution — PR #7499

Root cause

PR #7458 (fix(serve): detect stale SSE cursors across daemon restarts via epoch token; preserve turn attribution and surface compaction failures in replay) landed on main and modified compactionEngine.ts to preserve event attribution through compaction — the exact same problem PR #7499 independently solves. Both PRs added prompt/originator tracking fields to CompactedSlot and spread them into merged events, but with different data shapes.

Textual or semantic

Semantic conflict. Both sides modified the same logic (attribution preservation) with incompatible representations:

• PR fix(acp-bridge): keep event attribution through turn compaction #7499 (ours): flat fields lastPromptId?: string, lastOriginatorClientId?: string on the slot, passed individually to makeMergedSessionUpdateEvent.
• PR fix(serve): detect stale SSE cursors across daemon restarts via epoch token; preserve turn attribution and surface compaction failures in replay #7458 (main): structured lastTurn?: Pick<BridgeEvent, 'promptId' | 'originatorClientId'> + lastSessionId?: string, plus helper functions captureTurnFields() and captureSessionId().

Main's approach is a superset — it captures everything PR #7499 does plus sessionId. Resolution takes main's representation throughout.

The auto-merge also produced duplicate spread operators in two functions:

// makeMergedSessionUpdateEvent had both turn?.promptId AND bare promptId spreads
// mergeToolCallEvent had promptId/originatorClientId spreads twice (before and after _meta)

Both were cleaned up — keeping only main's versions.

What is load-bearing

• The captureTurnFields() helper implements per-field ?? fallback so a chunk carrying only promptId doesn't drop a previously captured originatorClientId (and vice versa). This matches the mergeToolCallEvent path's per-field merge semantics.
• In makeMergedSessionUpdateEvent, the turn?.promptId spreads must appear before envelopeMeta (matching main's ordering), and the sessionId spread lives inside data — not at the envelope level.
• In mergeToolCallEvent, the promptId/originatorClientId spreads appear before mergedMeta, not after (main's ordering).

What I could not verify

• The auto-merged test file compactionEngine.test.ts did not conflict, but it may contain tests referencing PR fix(acp-bridge): keep event attribution through turn compaction #7499's field names (lastPromptId, lastOriginatorClientId) that no longer exist. The PR's own CI will catch this.
• No build or typecheck was run — the resolution is purely textual/semantic.

中文说明

合并冲突解决 — PR #7499

根因

PR #7458（fix(serve): 通过 epoch token 检测过期 SSE 游标；保留 turn 归因并在重放中暴露压缩失败）已合入 main，修改了 compactionEngine.ts 以在压缩时保留事件归因——这正是 PR #7499 独立解决的同一问题。两个 PR 都在 CompactedSlot 上添加了 prompt/originator 追踪字段并将其展开到合并事件中，但数据结构不同。

文本冲突还是语义冲突

语义冲突。 双方修改了相同逻辑，但表示方式不兼容：

• PR fix(acp-bridge): keep event attribution through turn compaction #7499（ours）： 扁平字段 lastPromptId?: string、lastOriginatorClientId?: string。
• PR fix(serve): detect stale SSE cursors across daemon restarts via epoch token; preserve turn attribution and surface compaction failures in replay #7458（main）： 结构化 lastTurn?: Pick<BridgeEvent, 'promptId' | 'originatorClientId'> + lastSessionId?: string，加上辅助函数 captureTurnFields() 和 captureSessionId()。

main 的方案是超集，额外捕获了 sessionId。解决方案全面采用 main 的表示方式。

自动合并还在 makeMergedSessionUpdateEvent 和 mergeToolCallEvent 中产生了重复的展开运算符，已清理。

关键约束

• captureTurnFields() 实现逐字段 ?? 回退，确保仅携带 promptId 的 chunk 不会丢失之前捕获的 originatorClientId。
• makeMergedSessionUpdateEvent 中 turn?.promptId 展开必须在 envelopeMeta 之前，sessionId 在 data 内部。
• mergeToolCallEvent 中 promptId/originatorClientId 展开在 mergedMeta 之前。

未能验证的部分

• 自动合并的测试文件 compactionEngine.test.ts 可能引用了不再存在的 PR fix(acp-bridge): keep event attribution through turn compaction #7499 字段名，需 CI 验证。
• 未运行构建或类型检查。

[wenshao]

Review: fix(acp-bridge): keep event attribution through turn compaction

Verdict: LGTM — no Critical / High findings. The fix is correct, minimal, and well-scoped. Everything below is Suggestion-or-lower, mostly about test precision and the accuracy of the PR description.

Verification performed

• npm ci in a clean worktree at the PR head, then npx vitest run in packages/acp-bridge → 19 files / 898 tests pass, matching the PR body.
• A/B toggle: reverted only compactionEngine.ts to the parent commit, keeping the new tests → 2 of the 3 new tests fail (keeps promptId and originatorClientId on merged text chunks, ... on folded tool calls); restored → green. The tests genuinely pin the fix rather than passing incidentally. (The third, omits attribution ..., passes both ways by design — it's a negative guard.)
• tsc --noEmit -p packages/acp-bridge clean.
• Traced the source of the attribution: BridgeClient.prepareSessionUpdateFrames stamps every live session_update frame with { promptId, originatorClientId } from the entry's active prompt (bridgeClient.ts:778-790), so agent chunks and tool calls really do carry it on the wire, and compaction was the only place it was lost. Confirmed.

Code quality

The merge policy is internally consistent: text/thought slots use the sticky event.x ?? slot.lastX shape already used for lastMeta / lastEnvelopeMeta, and the tool fold uses incoming ?? existing exactly like the id merge one line above. Because every frame within a turn shares the same turn object from prepareSessionUpdateFrames, a cross-originator merge cannot occur on the live path, so "latest wins" is safe in practice. Comments explain the why and stay in house style. No any, no new deps, source change is 12 lines.

---

1. (Suggestion) The described user impact isn't reachable through either merge path

The PR body says a resuming client "can see its own messages echoed back". Echo suppression only fires for sessionUpdate: 'user_message_chunk' (packages/sdk-typescript/src/daemon/ui/normalizer.ts:652-659), and compaction never merges that type — it falls through classifySessionUpdate's default arm into a misc slot and is replayed verbatim with its envelope intact. I verified this with a throwaway probe: a user_message_chunk ingested with originatorClientId: 'client-a' comes back out of snapshot().compactedTurns with client-a on both the old and new code.

Similarly, "the bridge client groups transcript text by originator" (bridgeClient.ts:1002-1016) builds fresh mid_turn_message_injected frames from the live mid-turn queue, not from replayed compacted frames. And createBase (normalizer.ts:~560) never lifts envelope promptId onto the normalized UI event at all, so nothing downstream reads it off a session_update today.

None of this makes the change wrong — the correct and sufficient justification is narrower: a replayed compacted frame must carry the same envelope as the live frame it stands in for. Today a session/load consumer sees assistant.text.delta with originatorClientId present live and absent after compaction, which is a contract violation and a trap for the next consumer that starts reading it. Worth rewording the description so a future maintainer doesn't go looking for an echo-suppression regression test that can't exist.

2. (Suggestion) The "incoming wins" test doesn't test incoming-wins

compactionEngine.test.ts:399-415 gives both the tool_call and the tool_call_update the same prompt-1 / client-a, so it passes identically under incoming ?? existing, existing ?? incoming, or "existing only when set" — it cannot distinguish the documented policy from its inverse. Changing the second event to client-b / prompt-2 and asserting those values would actually pin it. I checked: the implementation does return client-b, so this is a one-line strengthening, not a bug.

3. (Suggestion) The subagent merge branch got the fix but no test

mergeTextSlot's parentToolCallId branch (compactionEngine.ts:334-359) received the same two-line treatment, but neither new text test sets _meta.parentToolCallId, so lines 343-345 and 357-358 are exercised only by pre-existing tests that don't assert attribution. This is the path that matters most for the parallel-subagent grouping story in the description. A probe confirms it behaves correctly — it just isn't guarded.

4. (Nit) The sticky ?? fallback is a real policy choice with no test

slot.lastPromptId = event.promptId ?? slot.lastPromptId means once a slot has attribution, a later unattributed chunk can't clear it. That's the right behavior and it matches lastMeta / lastEnvelopeMeta, but nothing pins it. Cheap to fold into #2's test.

5. (Nit / scope) The fix is inert on the ACP transport

AcpHttpDispatcher.translateEvent (packages/cli/src/serve/acp-http/dispatch.ts:4218-4228) forwards only event.data as notification('session/update', …), dropping the whole envelope — including both of these fields — for every session_update. So this change lands only on the HTTP path: snapshotReplay() → compactedReplay in the session/load response (bridge.ts:3955-3971) → DaemonSessionClient.load() → normalizeDaemonEvent. Not a defect in this PR, but worth knowing if #7457 (DAEMON-007) expects ACP parity.

6. (Nit) Small replay-window cost

Merged events now serialize roughly two extra UUID-sized fields, and serializedBridgeEventByteLength feeds the maxReplayBytes budget, so the window retains marginally fewer turns. Immaterial against the default cap, and obviously preferable to emitting wrong envelopes — just flagging that it is a real behavior delta.

---

Scope note (agreed)

The DAEMON-008 deferral checks out: notifyReplayWindowEviction (compactionEngine.ts:502-508) and the EventBus call sites both carry the documented never-throws / best-effort contract for the bare catch {}, so leaving it alone is right.

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

[Suggestion] Missing test for data.sessionId preservation — Issue #7457 DAEMON-007 names three dropped stamps: promptId, originatorClientId, and data.sessionId. The production code (via #7458) correctly preserves all three, but this PR only tests two.

Failure scenario: a future refactor removing lastSessionId from the slot type or the sessionId parameter from makeMergedSessionUpdateEvent would regress silently — no test here would catch it.

Concrete cost: data.sessionId is the one stamp whose preservation this PR leaves unverified.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

[Suggestion] No test exercises the per-field ?? fallback with mixed attribution — Both captureTurnFields (text path) and mergeToolCallEvent (tool path) use event.promptId ?? previous?.promptId, but all three tests provide uniform attribution across every chunk.

Failure scenario: if the ?? fallback were replaced with a direct assignment, all three tests still pass — but in production, streaming commonly delivers attribution on the first chunk only. The merged event silently loses promptId.

Concrete cost: the core invariant documented in JSDoc ("Each field falls back independently") is untested.

— qwen3.7-max via Qwen Code /review

[he-yufeng]

Closing this out. #7458 landed the same attribution preservation and goes further: merged text/thought events and folded tool calls now keep promptId, originatorClientId and data.sessionId on main, with tests covering both paths. Nothing left here worth rebasing.