refactor: unify 15 bridge callbacks into single onEvent channel

[genni613]

User description

概述

• 当前 15 个独立回调分散在 App.tsx 的 15 个 useCallback 中，状态逻辑难以推理、调试和扩展。本次重构将其统一为单一 onEvent(type, payload)事件通道，引入 eventReducer 纯函数集中管理状态转换，为后续 Phase 2（前端消息分组、多轮工具调用）奠定基础
• 将 App.tsx 中的状态逻辑提取为纯函数 eventReducer
• 保持所有现有行为不变 — 纯重构，无用户可感知变化

改动

文件	改动
BridgeHandler.kt	新增 buildEventJS 辅助方法，重写 notifyXxx 内部实现，新增 notifyRoundEnd 占位方法
useBridge.ts	单一 onEvent 处理器替代 15 个回调参数
App.tsx	单一 handleEvent + useState<AppState> 替代 15 个 useCallback
eventReducer.ts	新增纯函数，将事件类型映射到状态变更
bridge.d.ts	Bridge 接口更新为单一 onEvent
ChatService.kt	零改动

测试计划

• 新增 21 个 eventReducer 测试用例，覆盖全部事件类型
• 完整测试套件通过（36/36）
• 手动验证：单轮对话、工具调用、审批流程、续写、会话恢复

---

PR Type

Enhancement, Tests

---

Description

• Unify 15 bridge callbacks into single onEvent(type, payload) channel with eventReducer

• Add micro-batching with 16ms timer flush for token/log streaming to reduce EDT overhead

• Introduce round_end event and lazy bubble creation to fix execution card ordering

• Add BridgeHandlerBatchingTest and eventReducer.test.mjs covering batching and state logic

---

Diagram Walkthrough

flowchart LR
  A[Kotlin Backend] -->|"notifyXxx(msgId, data)"| B[BridgeHandler]
  B -->|"buildEventJS(type, payload)"| C{Event Type}
  C -->|"token/log"| D[enqueueJS - buffer]
  C -->|"start/end/error"| E[flushAndPush - immediate]
  D -->|"16ms timer"| F[flushPendingBuffer]
  F --> G[Browser JS]
  E --> G
  G -->|"onEvent(type, payloadJson)"| H[useBridge]
  H --> I[eventReducer]
  I --> J[App State]

Loading

File Walkthrough

	Relevant files
Enhancement	6 files BridgeHandler.kt Add buildEventJS and buffering logic                                          +161/-50 App.tsx Replace 15 useCallback with handleEvent + eventReducer      +51/-187 MessageBubble.tsx Export Message type from eventReducer                                        +2/-7      eventReducer.ts New pure function for event-to-state mapping                          +160/-0  useBridge.ts Single onEvent handler instead of 15 callbacks                      +13/-71  bridge.d.ts Update Bridge interface with single onEvent                            +3/-14
Bug fix	1 files ChatService.kt Remove bridgeNotifiedStart, add lazy bubble logic                +19/-9
Tests	3 files BridgeHandlerBatchingTest.kt Test micro-batching mechanism                                                        +264/-0  CommandExecutionServiceTest.kt Add rapid multi-line output test                                                  +18/-0    eventReducer.test.mjs Test all eventReducer cases                                                            +262/-0
Documentation	2 files 2026-04-19-phase1-unified-event-channel-design.md Add Phase 1 unified event channel spec                                      +391/-0  2026-04-19-phase2-message-grouping-design.md Add Phase 2 message grouping design spec                                  +488/-0
Configuration changes	2 files package.json Add eventReducer test to test command                                        +1/-1      tsconfig.test.json Add eventReducer to test build include                                      +3/-2
Additional files	1 files index.html +39/-39

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns Potential JavaScript injection: The type parameter in buildEventJS (line 318) is directly interpolated into the generated JavaScript string without escaping quotes: '${type}'. If the type contains a single quote, it could break out of the string and inject arbitrary JS. While the type values are currently controlled by internal Kotlin code, this is a defensive programming risk.

⚡ Recommended focus areas for review Potential JS Injection via Event Type At line 318, the type parameter is directly interpolated into the JavaScript string without escaping: window.__bridge.onEvent('${type}', ...). If type contains a single quote, it could break out of the string and inject arbitrary JavaScript. While type is controlled internally by Kotlin code, this is a defensive programming concern. Use type.quoted() or similar escaping like the old code did. return "window.__bridge.onEvent('${type}', ${json.encodeToString(payloadStr)})" Unused Mutable Set After Refactor The bridgeNotifiedStart mutable set (used for the lazy notifyStart hack) remains in the code but is now partially replaced by new logic. Lines 422-425 add tokens lazily, but the set is no longer fully utilized. This creates dead code that should be cleaned up in Phase 2 or now if the hack is no longer needed. onToken = { token -> if (activeMessageId == msgId) { responseBuffer.append(token) // Lazily create the assistant bubble on first token. // If this round ends up producing tool_calls, onFinishReason will // remove the bubble so execution cards stay in front. if (msgId !in bridgeNotifiedStart) { bridgeHandler?.notifyStart(msgId) bridgeNotifiedStart.add(msgId) } bridgeHandler?.notifyToken(token) } }, onEnd = { if (activeMessageId == msgId && !truncationHandler.isPendingContinuation) { Double JSON Encoding in Payload The buildEventJS method at line 316-318 encodes the payload to JSON string (obj.toString()) and then wraps it with json.encodeToString(payloadStr). This results in the payload being double-encoded (the JSON string itself gets JSON-escaped). The frontend then receives a string that requires an extra JSON.parse to get the actual payload values, which is demonstrated in the tests but adds unnecessary overhead. Consider whether this double-encoding is intentional. private fun buildEventJS(type: String, builderAction: JsonObjectBuilder.() -> Unit): String { val obj = buildJsonObject(builderAction) val payloadStr = obj.toString() return "window.__bridge.onEvent('${type}', ${json.encodeToString(payloadStr)})" }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Move JS execution outside synchronized block The executeJS call is made inside the synchronized block. Since executeJS invokes browser.cefBrowser.executeJavaScript() which can be slow (involves IPC to the renderer process), this holds the lock for an unnecessarily long time. Consider moving the execution outside the lock while keeping the queue drain atomic. src/main/kotlin/com/github/codeplangui/BridgeHandler.kt [369-380] internal fun flushPendingBuffer() { + val batch: List<String> synchronized(flushLock) { - val batch = mutableListOf<String>() - while (true) { - val item = pendingJs.poll() ?: break - batch.add(item) + batch = buildList { + while (true) { + val item = pendingJs.poll() ?: break + add(item) + } } - if (batch.isNotEmpty()) { - executeJS(batch.joinToString(";")) - } + } + if (batch.isNotEmpty()) { + executeJS(batch.joinToString(";")) } } Suggestion importance[1-10]: 6 __ Why: This is a valid performance improvement. Holding a lock while executing JavaScript (which involves IPC to the renderer process) is unnecessarily expensive. The suggested change moves executeJS outside the synchronized block while still keeping the queue drain atomic. This improves concurrency without changing correctness.	Low
	Ensure onEvent ref updates before bridge setup The onEventRef.current = onEvent assignment runs on every render, which is correct for capturing the latest callback. However, this occurs outside the useEffect that sets up window.__bridge.onEvent, creating a potential stale closure if the bridge setup happens on first render before onEventRef is updated. Ensure the ref is always current before the setup function runs. webview/src/hooks/useBridge.ts [8-9] const onEventRef = useRef(onEvent) -onEventRef.current = onEvent +useEffect(() => { + onEventRef.current = onEvent +}, [onEvent]) +const setup = useCallback(() => { + // ... existing setup code +}, []) + Suggestion importance[1-10]: 4 __ Why: The suggested change moves the ref update into a useEffect, but in practice this doesn't change behavior because onEventRef.current = onEvent executes synchronously before the useEffect that calls setup() runs. The original code is functionally correct—there's no stale closure issue here since the ref assignment happens before any async work.	Low
Possible issue	Fix race condition in flush scheduling logic There's a race condition in the flush scheduling. After setting flushPending.set(false), the check pendingJs.isNotEmpty() is performed outside the synchronization of flushPendingBuffer(), and before the recursive scheduleFlush() can acquire the lock via compareAndSet. Items added during this window won't trigger a new flush. Consider checking pendingJs.isNotEmpty() inside the synchronized block before releasing the lock, or reorder the operations. src/main/kotlin/com/github/codeplangui/BridgeHandler.kt [351-363] private fun scheduleFlush() { if (flushPending.compareAndSet(false, true)) { flushTimer.schedule(object : TimerTask() { override fun run() { - flushPendingBuffer() - flushPending.set(false) - if (pendingJs.isNotEmpty()) { + val hasMore: Boolean + synchronized(flushLock) { + flushPendingBuffer() + hasMore = pendingJs.isNotEmpty() + flushPending.set(false) + } + if (hasMore) { scheduleFlush() } } }, FLUSH_INTERVAL_MS) } } Suggestion importance[1-10]: 5 __ Why: The suggested change addresses a minor race condition where items added to the queue between flushPending.set(false) and the isNotEmpty() check could delay processing until the next flush cycle. However, this is a minor timing edge case that doesn't cause data loss—items will eventually be processed on the next scheduled flush. The fix is reasonable but not critical.	Low