feat(web-shell): show message time on hover#5079
Merged
Merged
Conversation
…istory History replay re-emits each persisted record with its original epoch-ms time nested in update._meta, but BridgeClient.sessionUpdate published the frame without lifting it to the envelope. EventBus.publish then stamped envelope _meta.serverTimestamp with publish-time Date.now(), which the client's extractServerTimestamp picks up at higher priority than the nested original — so a resumed session rendered every historical message at the resume moment instead of when it was sent. Lift update._meta.timestamp (or serverTimestamp) to the envelope serverTimestamp so EventBus preserves it. Live updates without such a timestamp keep the Date.now() fallback unchanged.
Carry each transcript block's wall-clock time (serverTimestamp ?? clientReceivedAt) onto every message and reveal it as a CSS-only hover tooltip in the message list. Same-day messages show HH:mm:ss; older ones show yyyy-MM-dd HH:mm:ss (local time, zero-padded).
qwen-code-ci-bot
requested changes
Jun 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Hey @wenshao — thanks for the detailed work here, the bug analysis and test coverage look solid.
Unfortunately the PR body doesn't follow our PR template. The required sections are:
- What this PR does — description of the change
- Why it's needed — motivation / user-facing benefit
- Reviewer Test Plan — with "How to verify", "Evidence (Before & After)", and "Tested on" subsections
- Risk & Scope — main risk, what's out of scope, breaking changes
- Linked Issues — closes/fixes references
- 中文说明 — full Chinese translation in a
<details>block
The current body has good content under different headings (Summary, What changed, The resume bug, Testing, Verify) — most of it can be reorganized into the template sections without rewriting. The "Tested on" OS table and "Risk & Scope" are genuinely new info we need.
Could you reformat the body to match the template? Happy to help if anything's unclear. 🙏
中文说明
@wenshao 你好——感谢这个详细的贡献，bug 分析和测试覆盖看起来都很扎实。
不过 PR 正文没有按照我们的 PR 模板 来写。必需的章节是：
- What this PR does — 变更描述
- Why it's needed — 动机 / 用户价值
- Reviewer Test Plan — 包含 "How to verify"、"Evidence (Before & After)" 和 "Tested on" 小节
- Risk & Scope — 主要风险、不在范围内的内容、破坏性变更
- Linked Issues — closes/fixes 引用
- 中文说明 — 在
<details>块中提供完整的中文翻译
当前正文的内容质量不错，只是用了不同的标题（Summary、What changed、The resume bug、Testing、Verify）——大部分内容可以直接重新组织到模板章节中，不需要重写。"Tested on" 的操作系统表格和 "Risk & Scope" 是我们确实需要的新信息。
能否按照模板重新整理一下正文？有任何不清楚的地方随时问。🙏
— Qwen Code · qwen3.7-max
Collaborator
Author
|
Thanks for the review — reformatted the body to match the template: What this PR does / Why it's needed / Reviewer Test Plan (How to verify · Evidence · Tested on · Environment) / Risk & Scope / Linked Issues, plus a full 中文说明 One honest caveat on Evidence: this is the daemon web-shell and I couldn't launch it in my environment, so I haven't attached before/after screenshots — the behavior is covered by the new unit tests (acp-bridge timestamp preservation + web-shell |
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. |
Collaborator
Author
|
@qwen-code /triage |
pomelo-nwu
approved these changes
Jun 13, 2026
Collaborator
|
Thanks for the PR, @wenshao! Template looks good ✓ — all required headings present with bilingual content. On direction: solid. Hover timestamps on the web-shell is a straightforward UX win — reviewing a session naturally calls for knowing when things happened. Catching and fixing the daemon-side bug where On approach: scope feels right. Two packages for two coupled problems — the UI tooltip and the daemon timestamp preservation. The CSS-only tooltip ( One observation: the Moving on to code review and tests. 🔍 中文说明感谢贡献，@wenshao！ 模板完整 ✓ — 所有必需的标题都已包含，并有中英双语内容。 方向：很好。为 web-shell 添加悬停时间戳是一个直观的 UX 改进——回看会话时自然会想知道什么时候发生了某件事。发现并修复 daemon 端 方案：范围合理。两个包对应两个耦合问题——UI tooltip 和 daemon 时间戳保留。纯 CSS 的 tooltip（ 一个观察： 进入代码审查和测试 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
Code ReviewRead the diff against an independent proposal: the problem is (1) daemon overwrites original timestamps on resume, (2) web-shell has no per-message time display. My approach would have been: lift Correctness — no issues found.
No AGENTS.md violations. Code is in the right packages, no over-abstraction, no speculative features. Test ResultsUnit TestsAll pass. New tests cover:
Typecheck & Lint
Build
Real-Scenario TestingThe daemon web-shell is a browser-based UI — it requires starting the daemon server and interacting via a real browser. The Unit tests thoroughly cover the logic on both sides (daemon timestamp preservation + UI formatter/rendering). A reviewer with access to the daemon can verify end-to-end by running 中文说明代码审查将 diff 与独立方案对比：问题是 (1) daemon 在 resume 时覆盖原始时间戳，(2) web-shell 没有消息时间显示。我的方案：在 正确性 — 未发现问题。
无 AGENTS.md 违规。 代码在正确的包中，无过度抽象，无投机性功能。 测试结果
真实场景测试Daemon web-shell 是基于浏览器的 UI——需要启动 daemon 服务器并通过真实浏览器交互。 单元测试充分覆盖了双方逻辑（daemon 时间戳保留 + UI 格式化/渲染）。有 daemon 访问权限的审查者可以通过运行 — Qwen Code · qwen3.7-max |
Collaborator
|
This PR does two things cleanly and both are worth shipping. The daemon bug fix is the more important half. History replay was already carrying the original timestamp in The UI side is straightforward: a CSS-only hover tooltip, no JS state, no new dependencies. The My independent proposal matched the PR's approach almost exactly — same lift in the daemon, same threading through message types, same lightweight render. The implementation is what I would have written. Tests are thorough: 7 new tests across both packages, all passing. Typecheck and lint are clean. Build succeeds. The only thing not verified is the end-to-end browser experience, which requires the daemon — but the unit tests cover every code path that feeds into it. No concerns. Approving. ✅ 中文说明这个 PR 干净地做了两件事，都值得合入。 Daemon 端 bug 修复是更重要的一半。历史重放已经在 UI 端很直接：纯 CSS 悬停 tooltip，无 JS 状态，无新依赖。 独立方案与 PR 的实现几乎一致——同样的 daemon 提升、同样的消息类型传递、同样的轻量渲染。实现符合预期。 测试充分：两个包共 7 个新测试，全部通过。类型检查和 lint 干净。构建成功。唯一未验证的是端到端浏览器体验，需要 daemon——但单元测试覆盖了所有相关代码路径。 无顾虑。批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 13, 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. ✅
ytahdn
reviewed
Jun 13, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
Additional suggestion: mergeCompactToolGroups in MessageList.tsx constructs merged tool_group messages without propagating the new timestamp field. In compact mode (default for short tool sequences), merged groups will have timestamp: undefined and render no hover tooltip — while non-merged groups and all other message types display it correctly. Consider adding timestamp: mergeableGroups[0].timestamp to the merged object.
Also, the serverTimestamp branch of blockTime = block.serverTimestamp ?? block.clientReceivedAt in transcriptToMessages.ts has no dedicated test — all test fixtures set only clientReceivedAt. A test asserting that serverTimestamp takes precedence when both are set would cover the core design contract of the timestamp feature.
| * depending on the wall clock. | ||
| */ | ||
| export function formatTimestamp(ts: number, now: Date = new Date()): string { | ||
| const d = new Date(ts); |
Collaborator
There was a problem hiding this comment.
[Suggestion] formatTimestamp does not guard against invalid input (NaN, Infinity, or values outside the valid Date range). If a corrupt timestamp reaches this function, the tooltip renders "NaN:NaN:NaN". Consider adding a defensive check:
export function formatTimestamp(ts: number, now: Date = new Date()): string {
if (!Number.isFinite(ts)) return '';
const d = new Date(ts);
// ...
}The upstream bridgeClient.ts validates with Number.isFinite(), but defense-in-depth at the render boundary is safer.
— qwen3.7-max via Qwen Code /review
| // the EventBus `Date.now()` fallback unchanged. | ||
| const updateMeta = (params.update as { _meta?: Record<string, unknown> }) | ||
| ._meta; | ||
| const originalTs = |
Collaborator
There was a problem hiding this comment.
[Suggestion] updateMeta?.['serverTimestamp'] appears to be dead code today — no current emitter sets serverTimestamp on the inner update._meta (they all use timestamp). This creates a naming collision with the envelope-level _meta.serverTimestamp that could confuse future developers about which level is authoritative.
Consider either:
- Removing the
serverTimestampbranch and only readingupdateMeta?.['timestamp'](with a comment explaining the inner field is emitter-set), or - Adding a comment documenting that
serverTimestampon the inner_metais a defensive fallback for a future emitter.
— qwen3.7-max via Qwen Code /review
ytahdn
approved these changes
Jun 13, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM! ✅ — qwen3.7-max via Qwen Code /review
wenshao
commented
Jun 13, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
3 Suggestions from qwen3.7-max review (2 inline, 1 below):
transcriptToMessages.test.ts—blockTime = serverTimestamp ?? clientReceivedAtfallback untested. No fixture setsserverTimestamp; if??precedence were swapped all tests would still pass.bridgeClient.test.ts—updateMeta?.['serverTimestamp'] ?? updateMeta?.['timestamp']prefers theserverTimestampkey, but the test only exercises thetimestamppath.ToolCallEmitter.ts:117(not in diff) —PlanEmitter.emitPlan(todos)dropsparams.timestamp, so plan messages still show resume time on session replay. The PR's core bug fix is incomplete for the plan path.PlanEmitter.emitPlanneeds to accept atimestampparameter and include_meta: { timestamp }in the sent update.
— qwen3.7-max via Qwen Code /review
| @@ -148,6 +148,7 @@ describe('transcriptBlocksToDaemonMessages', () => { | |||
| { | |||
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The blockTime = block.serverTimestamp ?? block.clientReceivedAt fallback at transcriptToMessages.ts:91 is never directly tested. No test fixture sets serverTimestamp on a block — all tests only set clientReceivedAt. If the ?? precedence were accidentally swapped, every existing test would still pass.
Add a test with a block where serverTimestamp: 1000 and clientReceivedAt: 2000, then assert the output message has timestamp: 1000 (proving serverTimestamp wins).
— qwen3.7-max via Qwen Code /review
| update: { | ||
| sessionUpdate: 'user_message_chunk', | ||
| content: { type: 'text', text: 'hi' }, | ||
| _meta: { timestamp: original }, |
Collaborator
Author
There was a problem hiding this comment.
[Suggestion] The production code at bridgeClient.ts:445 reads updateMeta?.['serverTimestamp'] ?? updateMeta?.['timestamp'], preferring the serverTimestamp key. But this test only provides _meta: { timestamp: original } — the serverTimestamp key path (which takes priority) is never exercised.
Add a test case with _meta: { serverTimestamp: 1_700_000_000_000 } (no timestamp key) and assert the envelope gets serverTimestamp: 1_700_000_000_000. Optionally also test that when both keys are present, serverTimestamp wins.
— qwen3.7-max via Qwen Code /review
wenshao
added a commit
that referenced
this pull request
Jun 13, 2026
…#5084) The message-time-on-hover feature (#5079) wraps each transcript message with MessageTimestamp, but two sub-agent surfaces were left out and so looked inconsistent with the rest of the transcript: - The "Parallel agents · N/N done" box (ParallelAgentsGroup) renders directly in MessageList, bypassing MessageItem/MessageTimestamp, so it showed no time. Carry the first grouped launch's timestamp onto the parallel_agents display item and wrap the box in MessageTimestamp. - Each sub-tool row inside a SubAgentPanel's Tools list showed no time. Wrap each row in a scoped hover tooltip (.toolTimeRow/.toolTimeTip, kept separate from MessageTimestamp's .row/.tip so the nested tooltip stays independent of the enclosing message's) keyed off the tool's startTime. Both reuse formatTimestamp for an identical HH:mm:ss (or dated) format revealed on hover in the top-right corner, matching the main transcript.
doudouOUC
pushed a commit
that referenced
this pull request
Jun 15, 2026
* fix(acp-bridge): preserve original timestamp when replaying session history History replay re-emits each persisted record with its original epoch-ms time nested in update._meta, but BridgeClient.sessionUpdate published the frame without lifting it to the envelope. EventBus.publish then stamped envelope _meta.serverTimestamp with publish-time Date.now(), which the client's extractServerTimestamp picks up at higher priority than the nested original — so a resumed session rendered every historical message at the resume moment instead of when it was sent. Lift update._meta.timestamp (or serverTimestamp) to the envelope serverTimestamp so EventBus preserves it. Live updates without such a timestamp keep the Date.now() fallback unchanged. * feat(web-shell): show each history message's time on hover Carry each transcript block's wall-clock time (serverTimestamp ?? clientReceivedAt) onto every message and reveal it as a CSS-only hover tooltip in the message list. Same-day messages show HH:mm:ss; older ones show yyyy-MM-dd HH:mm:ss (local time, zero-padded).
doudouOUC
pushed a commit
that referenced
this pull request
Jun 15, 2026
…#5084) The message-time-on-hover feature (#5079) wraps each transcript message with MessageTimestamp, but two sub-agent surfaces were left out and so looked inconsistent with the rest of the transcript: - The "Parallel agents · N/N done" box (ParallelAgentsGroup) renders directly in MessageList, bypassing MessageItem/MessageTimestamp, so it showed no time. Carry the first grouped launch's timestamp onto the parallel_agents display item and wrap the box in MessageTimestamp. - Each sub-tool row inside a SubAgentPanel's Tools list showed no time. Wrap each row in a scoped hover tooltip (.toolTimeRow/.toolTimeTip, kept separate from MessageTimestamp's .row/.tip so the nested tooltip stays independent of the enclosing message's) keyed off the tool's startTime. Both reuse formatTimestamp for an identical HH:mm:ss (or dated) format revealed on hover in the top-right corner, matching the main transcript.
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
4 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
Adds an on-hover timestamp to every message in the daemon web-shell, and fixes a daemon-side bug that made resumed sessions lose each message's original time. In the UI, every transcript message now carries its block's wall-clock time and reveals it as a CSS-only hover tooltip (anchored inside the row's top-right with
pointer-events: none, so theoverflow-y: automessage list never clips it and inner interactions still pass through). The time is shown in local time, zero-padded: same-day messages asHH:mm:ss, older ones asyyyy-MM-dd HH:mm:ss. On the daemon side,BridgeClient.sessionUpdatenow lifts the replayedupdate._meta.timestampup to the event envelope'sserverTimestampso it survives to the client.Why it's needed
When reviewing a conversation it is useful to see when each message was sent, and the web-shell had no such affordance. While adding it I found that resumed history showed the wrong time: every message from a previous-day session displayed today's time (the moment of resume) rather than when it was actually sent — so the new tooltip would have been misleading exactly where it is most useful. History replay (
MessageEmitter) re-emits each persisted record with its original epoch-ms time nested inupdate._meta.timestamp, butsessionUpdatepublished the frame without lifting it to the envelope, soEventBus.publishstamped_meta.serverTimestampwith publish-timeDate.now(), and the client'sextractServerTimestampreads the envelope value at higher priority than the nested original. Live messages were unaffected (theirDate.now()≈ the real time), which is why only/resumeexposed it.Reviewer Test Plan
How to verify
Unit tests cover both halves. acp-bridge:
cd packages/acp-bridge && npx vitest run→ 445 passed (incl. 2 new inbridgeClient.test.ts— one asserts a replayedupdate._meta.timestampreaches the envelopeserverTimestamp, the other asserts live updates without a timestamp emit no envelope_metaso theDate.now()fallback is unchanged). web-shell:cd packages/web-shell && npx vitest run→ 250 passed (incl. 5 new forformatTimestampsame-day / previous-day / previous-year branches andMessageTimestamprendering). Typecheck, eslint and prettier are clean for all touched files. For the end-to-end UI: restart the daemon (rebuild if you run fromdist;dev:daemonruns from source via tsx),/resumea session created on a previous day, then hover its messages — older ones should readyyyy-MM-dd HH:mm:ss, today'sHH:mm:ss.Evidence (Before & After)
Before: after resuming a previous-day session, every message's hover tooltip showed today's date/time (the resume moment). After: each message shows its original send time — same-day
HH:mm:ss, olderyyyy-MM-dd HH:mm:ss. Screenshots are not attached: this is the daemon web-shell, which I could not launch in my working environment, so the visual before/after needs a reviewer running the daemon. The underlying behavior is verified by the unit tests above (timestamp preservation in acp-bridge plus the formatter branches in web-shell).Tested on
Environment (optional)
Unit tests only (vitest), on macOS. The daemon was not launched locally; the logic is platform-independent (TypeScript + CSS) and CI runs the suites on all three OSes.
Risk & Scope
serverTimestampfromupdate._meta.timestampfor everysession_update, not only replay. It is backward-compatible — live updates that carry no such timestamp pass no envelope_meta, soEventBus.publishkeeps its existingDate.now()fallback; ordering is unaffected because the client orders byeventId, notserverTimestamp.ParallelAgentsGroup) is not routed throughMessageItemand so has no hover tooltip — it aggregates multiple tool groups and has no single per-message time.Linked Issues
N/A
中文说明
What this PR does
为 daemon web-shell 的每条消息增加鼠标悬停显示时间，并修复一个导致 resume 历史会话丢失每条消息原始时间的 daemon 端 bug。在 UI 中，每条 transcript 消息现在都携带其 block 的墙钟时间，并以纯 CSS 的悬停 tooltip 呈现（定位在消息行右上角内侧，
pointer-events: none，这样overflow-y: auto的消息列表不会裁切它，内部交互也能正常穿透）。时间按本地时区、补零显示：当天消息为HH:mm:ss，更早的为yyyy-MM-dd HH:mm:ss。在 daemon 端，BridgeClient.sessionUpdate现在会把重放所携带的update._meta.timestamp提升到事件 envelope 的serverTimestamp，使其能够传到客户端。Why it's needed
回看一段会话时，知道每条消息的发送时间很有用，而 web-shell 此前没有这个能力。在添加这个功能时我发现 resume 的历史显示了错误的时间：往日会话的每条消息都显示成了当天时间（resume 的时刻），而不是其实际发送时间——也就是说，恰恰在最需要它的场景下，这个新 tooltip 反而会产生误导。历史重放（
MessageEmitter）会把每条持久化记录连同其原始的 epoch 毫秒时间一起重新发出，嵌在update._meta.timestamp中，但sessionUpdate在发布帧时没有把它提升到 envelope，于是EventBus.publish用发布时刻的Date.now()覆盖了_meta.serverTimestamp，而客户端的extractServerTimestamp读取 envelope 值的优先级高于嵌套的原始值。实时消息不受影响（其Date.now()≈ 真实时间），这就是为什么只有/resume才会暴露这个问题。Reviewer Test Plan
How to verify
单元测试覆盖了两部分。acp-bridge：
cd packages/acp-bridge && npx vitest run→ 445 通过（含bridgeClient.test.ts中 2 个新测试——一个断言重放的update._meta.timestamp能到达 envelope 的serverTimestamp，另一个断言不带时间戳的实时更新不会发出 envelope_meta，从而保持Date.now()回退不变）。web-shell：cd packages/web-shell && npx vitest run→ 250 通过（含 5 个新测试，覆盖formatTimestamp的当天 / 往日 / 往年分支以及MessageTimestamp渲染）。所有改动文件的 typecheck、eslint、prettier 均干净。端到端 UI：重启 daemon（若从dist运行需先重新构建；dev:daemon通过 tsx 从源码运行），/resume一个往日创建的会话，然后悬停其消息——更早的应显示yyyy-MM-dd HH:mm:ss，当天的显示HH:mm:ss。Evidence (Before & After)
之前：resume 一个往日会话后，每条消息的悬停 tooltip 显示当天日期/时间（resume 时刻）。之后：每条消息显示其原始发送时间——当天
HH:mm:ss，更早yyyy-MM-dd HH:mm:ss。未附截图：这是 daemon web-shell，我在工作环境中无法启动它，因此视觉上的前后对比需要 reviewer 运行 daemon 来确认。底层行为已由上述单元测试验证（acp-bridge 的时间戳保留，以及 web-shell 的格式化分支）。Tested on
Environment (optional)
仅单元测试（vitest），在 macOS 上运行。未在本地启动 daemon；该逻辑与平台无关（TypeScript + CSS），且 CI 会在三个操作系统上运行测试套件。
Risk & Scope
session_update（不仅是重放）从update._meta.timestamp设置 envelopeserverTimestamp。它是向后兼容的——不带该时间戳的实时更新不会传 envelope_meta，因此EventBus.publish保持其原有的Date.now()回退；排序不受影响，因为客户端按eventId排序，而非serverTimestamp。ParallelAgentsGroup）不经过MessageItem，因此没有悬停 tooltip——它聚合了多个 tool group，没有单一的每条消息时间。Linked Issues
N/A