Skip to content

fix: stabilize browser media sessions - #26

Open
lanyue-llk wants to merge 12 commits into
lex-mainfrom
codex/browser-face-media-gate-v2
Open

fix: stabilize browser media sessions#26
lanyue-llk wants to merge 12 commits into
lex-mainfrom
codex/browser-face-media-gate-v2

Conversation

@lanyue-llk

Copy link
Copy Markdown

Summary

  • preserve room-owned audio rendering across browser session transitions
  • make session stop idempotent and clean up resources reliably
  • verify browser echo-cancellation behavior
  • keep the activity-intelligence frontend deployment isolated from other Lex Voice instances

Verification

  • 264 frontend tests passed
  • lint passed
  • production build passed
  • local end-to-end Start Call → agent ready → End Call passed on the isolated deployment

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 59s —— View job


Todo

  • 获取 PR 上下文
  • 拉取 diff 并分析改动
  • 检查 LiveKit room/track 生命周期
  • 检查浏览器媒体权限失败路径
  • 检查 React hook 清理
  • 检查 Next.js 客户端/服务端边界、环境变量泄露
  • 输出 review 结论

Review 结论

有条件通过

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:165logBrowserAudioCaptureDiagnostics(captureTrack)(内部调用 assertBrowserEchoCancellationActive,见 lib/browser-audio-capture.ts:60-66)在 try 块(第 168 行开始)之前执行。一旦某些浏览器/设备上 getSettings().echoCancellation 未回报为 true(常见于部分移动端/Safari),该函数会直接 throw,此时已通过 createLocalAudioTrack 拿到的麦克风 audioTrack 不会被 stop()/mute(),导致麦克风硬件保持占用(热麦克风泄漏),且整个 ensureAudioPublished 直接失败,Start Call 流程中断。建议:把该诊断/断言调用移入 try 块内,或在抛出前显式停止 audioTrack,或将其改为仅记录日志、不 throw(毕竟目的是诊断而非硬性阻断)。
  • [P1] PR 描述提到"keep the activity-intelligence frontend deployment isolated from other Lex Voice instances",但本次 diff 的 7 个文件均不涉及部署/环境隔离相关改动,描述与实际变更不匹配,建议确认是否有遗漏的 commit 或描述需要更正。
  • [P1] components/livekit/filtered-audio-renderer.tsx:402-410hooks/useBrowserSourceClient.ts:165:新增的诊断日志使用裸 console.info/console.warn 无条件输出(前者在每次远端音轨开始播放时触发),而同文件其余诊断日志均通过 debugAudioLog(debugAudio, ...)(见 filtered-audio-renderer.tsx:24)按调试开关控制。建议保持一致,避免生产环境控制台噪音。
  • [P2] lib/session-stop.ts:26-33isLiveKitRoomNotFoundError 要求 status === 404 且 code === 'not_found' 同时成立,未见对 livekit-server-sdk 实际抛出错误的形态做注释/引用确认;若 SDK 版本升级后只设置其中一个字段,会静默退回到"删除失败"分支。建议补充来源说明或做更宽松的判断(如 status === 404 || code === 'not_found',需结合 SDK 语义确认)。
  • [P2] 新增测试(tests/browser-audio-capture.test.mjs)仅覆盖 lib/browser-audio-capture.ts 中的纯函数,未覆盖 useBrowserSourceClient.ts 中实际调用路径(即上述 P0 场景:assert 抛出时 track 是否被正确清理),建议补充针对 ensureAudioPublished 的集成测试。

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 24s —— View job


Todo

  • 获取 PR 上下文
  • 拉取 diff 并分析改动
  • 检查 LiveKit room/track 生命周期
  • 检查浏览器媒体权限失败路径
  • 检查 React hook 清理
  • 检查 Next.js 客户端/服务端边界、环境变量泄露
  • 输出 review 结论

Review 结论

有条件通过

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:165ensureAudioPublished 内):logBrowserAudioCaptureDiagnostics(captureTrack)try 块(第 168 行开始)之前调用,其内部 assertBrowserEchoCancellationActivelib/browser-audio-capture.ts:60-66)会在 getSettings().echoCancellation !== true 时直接 throw。此时 createLocalAudioTrack 已产生的 audioTrack 未进入 try 块,第 246-251 行的清理逻辑(stop()/mute())不会执行,导致麦克风硬件持续占用(热麦泄漏),ensureAudioPublished 直接失败、Start Call 中断。该问题在上一轮 review 中已指出,本次 push 未修复。建议:将诊断/断言调用移入 try 块内,或改为仅记录日志不 throw。
  • [P1] 本次 diff 相对 origin/lex-main 新增 10988 行、修改 58 个文件,但与 PR 描述(稳定浏览器媒体会话)直接相关的仅 hooks/useBrowserSourceClient.tslib/browser-audio-capture.tslib/session-stop.tscomponents/livekit/filtered-audio-renderer.tsxapp/api/session/stop/route.ts 等少数文件。其余约 4000+ 行为整套 components/agents-ui/*components/ai-elements/*components/ui/*(含 960 行 react-shader-toy.tsx)等 shadcn UI 组件库代码,以及 package.json 中新增十余个不相关依赖(@xyflow/reactaicmdkembla-carousel-reactmedia-chromeshikistreamdowntokenlens@rive-app/react-webgl2 等)、新增 app/api/token/route.tsapp/layout.tsx 中硬编码 "Built with Lexmount Agent Studio" 品牌 header。这些改动与"稳定浏览器媒体会话"目标无关,建议拆分为独立 PR,避免审查风险与合并冲突面被放大。
  • [P1] components/livekit/filtered-audio-renderer.tsx:402-410hooks/useBrowserSourceClient.ts:165:新增诊断日志使用裸 console.info/console.warn 无条件输出(前者在每次远端音轨开始播放时触发),而同文件其余诊断日志均通过 debugAudioLog(debugAudio, ...)(见 filtered-audio-renderer.tsx:24)按调试开关控制,风格不一致,会产生生产环境控制台噪音。建议统一走 debugAudioLog
  • [P2] lib/session-stop.ts:26-33isLiveKitRoomNotFoundError 要求 status === 404 && code === 'not_found' 同时成立,未注明依据的是 livekit-server-sdk 哪个版本的错误形态;若后续 SDK 只设置其中一个字段会静默退回"删除失败"分支。建议补充来源注释或放宽为 ||(需先确认 SDK 语义)。
  • [P2] tests/browser-audio-capture.test.mjs 仅覆盖 lib/browser-audio-capture.ts 纯函数,未覆盖 useBrowserSourceClient.tsensureAudioPublished 的实际调用路径,即上述 P0 场景(assert 抛出时 track 是否被清理)没有回归测试保护。建议补充针对该路径的集成测试。
  • [P2] app/api/token/route.ts 是新增的、与 app/api/connection-details/route.ts 功能重复的 token 路由(来自 shadcn agents-ui 模板),虽有 NODE_ENV !== 'development' 门禁,但与本 PR 目标无关,属于上述范围蔓延的一部分,建议随同其他无关文件一并移出本 PR。

Fix this →

@lanyue-llk
lanyue-llk changed the base branch from lex-main to main August 28, 2026 07:28
@lanyue-llk
lanyue-llk changed the base branch from main to lex-main August 28, 2026 09:23
@lanyue-llk
lanyue-llk force-pushed the codex/browser-face-media-gate-v2 branch from 473b555 to d16d644 Compare August 28, 2026 09:32
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 48s —— View job


Todo

  • 获取 PR 上下文
  • 拉取 diff 并分析改动(git diff origin/lex-main...HEAD,11 个文件,+295/-42)
  • 检查 LiveKit room/track 生命周期
  • 检查浏览器媒体权限失败路径
  • 检查 React hook 清理
  • 检查 Next.js 客户端/服务端边界、环境变量泄露
  • 输出 review 结论

Review 结论

不通过(此前两轮 review 已指出的 P0 问题本次 push 仍未修复)

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:165logBrowserAudioCaptureDiagnostics(captureTrack)(内部调用 assertBrowserEchoCancellationActive,见 lib/browser-audio-capture.ts:58-66)仍在 try 块(第 168 行开始)之前执行。当浏览器/设备上 getSettings().echoCancellation !== true 时会直接 throw,此时 createLocalAudioTrack 产生的 audioTrack 未进入 try 块,第 246-251 行的 stop()/mute() 清理逻辑不会执行,导致麦克风硬件持续占用(热麦泄漏),ensureAudioPublished 直接失败、Start Call 中断。该问题已连续两轮 review 提出,仍未修复。建议:把诊断/断言调用移入 try 块内,或让 assertBrowserEchoCancellationActive 仅记录日志不 throw。
  • [P1] components/livekit/filtered-audio-renderer.tsx:404-410hooks/useBrowserSourceClient.ts:717-721:新增诊断日志使用裸 console.info/console.warn 无条件输出(前者在每次远端音轨开始播放时触发),而同文件其余诊断日志均通过 debugAudioLog(debugAudio, ...)(见 filtered-audio-renderer.tsx:24)按调试开关控制,风格不一致,会在生产环境产生控制台噪音。建议统一走 debugAudioLog(或等价开关)。
  • [P2] tests/browser-audio-capture.test.mjs 仍只覆盖 lib/browser-audio-capture.ts 纯函数,未针对 hooks/useBrowserSourceClient.tsensureAudioPublished 的实际调用路径补充回归测试,即上述 P0 场景(assert 抛出时 track 是否被正确清理)没有测试保护。建议补充集成测试。
  • [P2] lib/session-stop.ts:26-33isLiveKitRoomNotFoundError 仍要求 status === 404 && code === 'not_found' 同时成立,未注明依据的是 livekit-server-sdk 哪个版本的错误形态;本次已补充单测(tests/session-stop.test.mjs),但若后续 SDK 只设置其中一个字段仍会静默退回“删除失败”分支。建议补充来源注释。

本轮改进(已确认解决)

  • 此前指出的“diff 混入约 4000+ 行无关 shadcn UI 组件/依赖”范围蔓延问题已解决,本次 diff 仅涉及与浏览器媒体会话相关的 11 个文件。
  • app/api/connection-details/route.ts 移除 RoomConfiguration 相关逻辑,避免与旧版 LiveKit server 的不兼容问题,改动合理且有对应测试更新。
  • lib/transcription-history.ts 新增跨 stream 用最终文本替换未完成片段的逻辑,并补充了对应单测,逻辑正确。

Fix this →

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 13s —— View job


Review 结论

不通过(连续第 4 轮 review,此前已指出的 P0 问题本次 push 仍未修复)

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:165logBrowserAudioCaptureDiagnostics(captureTrack)(内部调用 assertBrowserEchoCancellationActive,见 lib/browser-audio-capture.ts:58-66)仍在 try 块(第 168 行开始)之前执行。当 getSettings().echoCancellation !== true(部分移动端/Safari 常见)时会直接 throw,此时第 160 行 createLocalAudioTrack 产生的 audioTrack 未进入 try 块,第 246-251 行的 stop()/mute() 清理逻辑不会执行,导致麦克风硬件持续占用(热麦泄漏),ensureAudioPublished 直接失败、Start Call 中断。该问题已连续三轮 review 提出,本次 push 依旧未变动。建议:将该诊断/断言调用移入 try 块内,或让 assertBrowserEchoCancellationActive 仅记录日志不 throw。
  • [P1] components/livekit/filtered-audio-renderer.tsx:407-410:新增播放诊断日志使用裸 console.info/console.warn 无条件输出(每次远端音轨播放时触发),而同文件其余诊断日志均通过 debugAudioLog(debugAudio, ...)(见 filtered-audio-renderer.tsx:24)按调试开关控制。hooks/useBrowserSourceClient.ts:719logBrowserAudioCaptureDiagnostics 内的 console.info)同样存在此问题,此前两轮已指出,仍未修复,会在生产环境产生控制台噪音。建议统一走 debug 开关控制。
  • [P2] tests/browser-audio-capture.test.mjs 仍只覆盖 lib/browser-audio-capture.ts 纯函数,未针对 hooks/useBrowserSourceClient.tsensureAudioPublished 的实际调用路径补充回归测试,即上述 P0 场景(assert 抛出时 track 是否被正确清理)依旧没有测试保护。
  • [P2] lib/session-stop.ts:27-33isLiveKitRoomNotFoundError 仍要求 status === 404 && code === 'not_found' 同时成立,本轮已补充单测(tests/session-stop.test.mjs:48-59),但未注明依据的是 livekit-server-sdk 哪个版本的错误形态;若后续 SDK 只设置其中一个字段仍会静默退回"删除失败"分支,属于残余风险,可后续补充来源注释。

本轮改进(已确认)

  • app/api/connection-details/route.ts 移除 RoomConfiguration 相关逻辑,改动合理,避免与旧版 LiveKit server 不兼容。
  • lib/transcription-history.ts 用最终文本替换未完成片段的合并逻辑正确,已补充单测。
  • lib/session-stop.tsisLiveKitRoomNotFoundError 补充了单测。

Fix this →

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 39s —— View job


Review 结论

不通过(连续第 4 轮 review,此前指出的 P0 热麦克风泄漏问题本次 push 仍未修复)

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:165ensureAudioPublishedlogBrowserAudioCaptureDiagnostics(captureTrack)(内部调用 assertBrowserEchoCancellationActive,见 lib/browser-audio-capture.ts:58-66echoCancellation !== true 时 throw)仍在 try 块(第 168 行开始)之前执行。此时第 160 行 createLocalAudioTrack 已创建的 audioTrack 未进入 try,第 246-251 行的 stop()/mute() 清理逻辑不会执行,导致麦克风硬件持续占用(热麦泄漏),ensureAudioPublished 直接失败、Start Call 中断。该问题已连续四轮 review 提出,代码未变动。建议:把该诊断/断言调用移入 try 块内,或让 assertBrowserEchoCancellationActive 仅记录日志不 throw。Fix this →
  • [P1] components/livekit/filtered-audio-renderer.tsx:409hooks/useBrowserSourceClient.ts:720:诊断日志仍使用裸 console.info/console.warn 无条件输出(前者每次远端音轨播放时触发),而同文件其余日志均通过 debugAudioLog(debugAudio, ...) 按调试开关控制,风格不一致,会在生产环境产生控制台噪音。已连续三轮指出,未修复。建议统一走 debug 开关。
  • [P2] tests/browser-audio-capture.test.mjstests/browser-room-session.test.mjs 仍只覆盖纯函数或字符串匹配 hook 源码,未对 ensureAudioPublished 中 assert 抛出时 audioTrack 是否被清理的实际路径(即上述 P0 场景)做集成测试保护。
  • [P2] lib/session-stop.ts:27-33isLiveKitRoomNotFoundError 仍要求 status === 404 && code === 'not_found' 同时成立,未注明依据的是 livekit-server-sdk 哪个版本的错误形态;若后续 SDK 只设置其中一个字段会静默退回"删除失败"分支。已补充单测(tests/session-stop.test.mjs),残余风险较低,可后续补来源注释。

本轮新增的 hooks/useRoom.ts:266await browserSourceClient.stop(),在 waitForRoomDisconnected 前清理陈旧的 browser 媒体 runtime)逻辑合理:stop()hooks/useBrowserSourceClient.ts:411-415)在无 runtime 时直接返回,是幂等操作,且有对应测试更新,未发现问题。

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 4s —— View job


Todo

  • 获取 PR 上下文
  • 拉取 diff 并分析改动(git diff origin/lex-main...HEAD,18 个文件,+462/-70)
  • 重点检查此前多轮已指出的 P0 热麦克风泄漏问题是否修复
  • 检查 React hook 清理 / Next.js 边界 / 环境变量泄露
  • 输出 review 结论

Review 结论

不通过(连续第 5 轮 review,此前四轮已指出的 P0 热麦克风泄漏问题本次 push 仍未修复)

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:167-171captureTrack = audioTrack.mediaStreamTrack 后紧接调用 logBrowserAudioCaptureDiagnostics(captureTrack)(内部调用 assertBrowserEchoCancellationActive,见 lib/browser-audio-capture.ts:58-66echoCancellation !== true 时 throw),该调用仍位于 try 块(第 171 行开始)之前。第 163 行 createLocalAudioTrack 已创建的 audioTrack 一旦触发此 throw,第 249-253 行的 stop()/mute() 清理逻辑不会执行,导致麦克风硬件持续占用(热麦泄漏),ensureAudioPublished 直接失败、Start Call 中断。此问题已连续四轮 review 提出,代码逻辑(含行号偏移)未发生任何变化。建议:把该诊断/断言调用移入 try 块内,或让 assertBrowserEchoCancellationActive 仅记录日志不 throw。Fix this →
  • [P1] components/livekit/filtered-audio-renderer.tsx:405-410hooks/useBrowserSourceClient.ts:734-739:诊断日志仍使用裸 console.info/console.warn 无条件输出(前者每次远端音轨播放时触发),而同文件其余日志均通过 debugAudioLog(debugAudio, ...) 按调试开关控制,风格不一致,会在生产环境产生控制台噪音。已连续四轮提出,仍未修复。建议统一走 debug 开关。
  • [P2] tests/browser-audio-capture.test.mjstests/browser-room-session.test.mjs 仍只覆盖纯函数或字符串匹配 hook 源码,未对 ensureAudioPublished 中 assert 抛出时 audioTrack 是否被清理的实际路径(即上述 P0 场景)做集成测试保护。
  • [P2] lib/session-stop.ts:27-33isLiveKitRoomNotFoundError 仍要求 status === 404 && code === 'not_found' 同时成立,未注明依据的是 livekit-server-sdk 哪个版本的错误形态;已补充单测(tests/session-stop.test.mjs),残余风险较低,可后续补来源注释。

本轮改进(已确认)

  • lib/browser-media-capture-timeout.ts(新增)为摄像头采集/发布/媒体网关状态发布增加超时与资源释放(hooks/useBrowserSourceClient.ts 摄像头路径、lib/livekit-media-gate.ts:234-244),逻辑合理且有对应单测。
  • hooks/useRoom.ts:262-266waitForRoomDisconnected 前调用 browserSourceClient.stop() 清理陈旧的 browser 媒体 runtime,stop() 幂等,改动合理,有测试覆盖。
  • app/api/session/stop/route.ts + lib/session-stop.ts 对 LiveKit room 已不存在的情况做幂等处理,改动合理。

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 53s —— View job


Review 结论

不通过(连续第 6 轮 review,此前多轮已指出的 P0 热麦克风泄漏问题本次 push 仍未修复)

问题及建议

  • [P0] hooks/useBrowserSourceClient.ts:168logBrowserAudioCaptureDiagnostics(captureTrack)(内部调用 assertBrowserEchoCancellationActive,见 lib/browser-audio-capture.ts:58-66echoCancellation !== true 时 throw)仍在 try 块(第 171 行开始)之前执行。第 163 行 createLocalAudioTrack 已创建的 audioTrack 一旦触发此 throw,第 249-253 行的 stop()/mute() 清理逻辑不会执行,导致麦克风硬件持续占用(热麦泄漏),ensureAudioPublished 直接失败、Start Call 中断。此问题已连续五轮 review 提出,代码逻辑未变化。建议:把该诊断/断言调用移入 try 块内,或让 assertBrowserEchoCancellationActive 仅记录日志不 throw。Fix this →
  • [P1] components/livekit/filtered-audio-renderer.tsx:405-410hooks/useBrowserSourceClient.ts:734-739:诊断日志仍使用裸 console.info/console.warn 无条件输出(前者每次远端音轨播放时触发),而同文件其余日志均通过 debugAudioLog(debugAudio, ...)(见 filtered-audio-renderer.tsx:24)按调试开关控制,风格不一致,会在生产环境产生控制台噪音。已连续五轮提出,仍未修复。建议统一走 debug 开关。
  • [P2] tests/browser-audio-capture.test.mjstests/browser-room-session.test.mjs 仍只覆盖纯函数或字符串匹配 hook 源码,未对 ensureAudioPublished 中 assert 抛出时 audioTrack 是否被清理的实际路径(即上述 P0 场景)做集成测试保护。
  • [P2] lib/session-stop.ts:27-33isLiveKitRoomNotFoundError 仍要求 status === 404 && code === 'not_found' 同时成立,未注明依据的是 livekit-server-sdk 哪个版本的错误形态;已补充单测(tests/session-stop.test.mjs),残余风险较低,可后续补来源注释。

本轮改进(已确认)

  • hooks/useRoom.ts:262-266waitForRoomDisconnected 前调用 browserSourceClient.stop() 清理陈旧的 browser 媒体 runtime,避免 Room 意外 disconnect 后新会话拿不到麦克风,stop() 幂等,改动合理。
  • lib/browser-media-capture-timeout.ts(新增)为摄像头采集/发布/媒体网关状态发布增加超时与资源释放(hooks/useBrowserSourceClient.ts 摄像头路径、lib/livekit-media-gate.ts:234-247),逻辑合理且有对应单测。

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 4s —— View job


Review 结论

通过

问题及建议

未发现需要阻塞合并的问题。此前连续 6 轮 review 指出的 P0 热麦克风泄漏问题本次已修复:

  • hooks/useBrowserSourceClient.ts:168-170logBrowserAudioCaptureDiagnostics(内部含可能 throw 的 assertBrowserEchoCancellationActive)现已移入 runWithBrowserAudioTrackCleanup 回调内(lib/browser-audio-capture.ts:32-44),抛错时会先 mediaStreamTrack.enabled = falsemute()stop() 再重新抛出,麦克风硬件会被正确释放,热麦泄漏问题已解决。且新增单测 tests/browser-audio-capture.test.mjs:78-99 直接覆盖了该失败清理路径。
  • 此前指出的裸 console.info/console.warn 日志噪音问题也已修复:hooks/useBrowserSourceClient.ts:735-737 的诊断日志现受 debugAudio 参数门控;components/livekit/filtered-audio-renderer.tsx 中全部诊断日志均已统一走 debugAudioLog(debugAudio, ...)
  • lib/session-stop.ts:26-34isLiveKitRoomNotFoundError 已补充来源注释,说明依据 livekit-server-sdk 2.13.x TwirpError 形态判定 status/code 同时成立的原因。

残余风险:tests/browser-audio-capture.test.mjs 对清理逻辑的覆盖仍是针对 runWithBrowserAudioTrackCleanup helper 的单测,而非贯穿 ensureAudioPublished 完整调用路径的集成测试,属于 P2 级别可后续补充项,不影响本次合并。

@lanyue-llk

Copy link
Copy Markdown
Author

已处理本轮全部有效评论,提交为 5c45293

  • 浏览器音轨创建和严格 AEC 校验现在都在统一清理边界内;任一步失败都会立即禁用并停止已获取的麦克风轨道。mute() 只做尽力调用,不再等待它返回,避免权限撤销等异常下既泄漏麦克风又卡住 Start Call。
  • 采集与播放诊断日志只在 debugAudio 开启时输出。
  • 补充了 livekit-server-sdk 2.13.x 的错误字段约束说明,保留严格的 HTTP 404 + Twirp not_found 判定。
  • 新增行为回归测试,覆盖 mute() 永不返回时仍立即 stop 并保留原始异常。

验证:pnpm test 274 项通过,pnpm lintpnpm format:checkpnpm build 均通过。真实本地 Start Call 也已通过:浏览器发布 640×480 视频和带 AEC/NS/AGC 的音轨,人脸识别后麦克风开始发送音频,主动问候完整播放,浏览器控制台无告警或错误。

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 39s —— View job


Todo

  • 获取 PR 上下文
  • 拉取 diff 并分析改动(git diff origin/lex-main...HEAD,18 个文件,+534/-80)
  • 检查 LiveKit room/track 生命周期
  • 检查浏览器媒体权限失败路径
  • 检查 React hook 清理
  • 检查 Next.js 客户端/服务端边界、环境变量泄露
  • 输出 review 结论

Review 结论

通过

问题及建议

未发现需要阻塞合并的问题。核对说明:

  • 本次 origin/lex-main...HEAD diff 与上一轮(commit 5c45293)已通过的 review 内容一致,之间新增的两个提交(a887615、合并提交 7ea48c1)改动的是 app/api/session/session-dispatch-service.tstests/session-prewarm.test.mjs,这两个文件均已存在于 origin/lex-main,不属于本 PR 的实际改动范围。
  • 连续多轮指出的 P0 热麦克风泄漏问题已确认修复:hooks/useBrowserSourceClient.ts:168-171logBrowserAudioCaptureDiagnostics(含可能 throw 的 assertBrowserEchoCancellationActive)现已包裹在 runWithBrowserAudioTrackCleanuplib/browser-audio-capture.ts:20-31)回调内,失败时会先 enabled=falsemute()stop() 再重新抛出;tests/browser-audio-capture.test.mjs:78-99 覆盖了 mute() 永不 resolve 时仍立即 stop() 的路径。
  • 此前指出的裸 console.info/console.warn 噪音问题已修复:诊断日志均已通过 debugAudio 开关门控(hooks/useBrowserSourceClient.ts:732components/livekit/filtered-audio-renderer.tsx:406)。
  • lib/session-stop.ts:27-35isLiveKitRoomNotFoundError 已补充依据 livekit-server-sdk 2.13.x TwirpError 形态的来源注释。
  • 新增的 lib/browser-media-capture-timeout.ts(摄像头采集/发布/媒体网关状态发布超时+资源释放)逻辑合理,hooks/useBrowserSourceClient.ts:283-346lib/livekit-media-gate.ts:234-247 的超时后清理路径(disposeLateResult/catchstop()/unpublishTrack)均有对应处理,未见资源泄漏。
  • hooks/useRoom.ts:262-266waitForRoomDisconnected 前调用幂等的 browserSourceClient.stop(),避免 Room 意外断开后新会话拿不到麦克风,逻辑合理且有测试覆盖(tests/session-start-dispatch.test.mjs)。
  • lib/transcription-history.ts 跨 stream 用最终文本替换未完成片段的合并逻辑正确,tests/chat-message-filter.test.mjs 补充了对应用例。

残余风险(P2,不阻塞合并):tests/browser-audio-capture.test.mjs 对清理逻辑的覆盖仍是针对 runWithBrowserAudioTrackCleanup helper 的单测,而非贯穿 ensureAudioPublished 完整调用路径的集成测试,可后续补充。

@lanyue-llk

Copy link
Copy Markdown
Author

Review 结论

不建议当前合并。已基于最新 lex-maina887615)重新审查,并实际运行完整测试、lint、format check、production build;现有 281 项测试全部通过,但补充反例后确认有两个现有测试未覆盖的功能回退。

[P1] 音频启动仍然可能无限挂起

hooks/useBrowserSourceClient.ts:163-178createLocalAudioTrack(...) 和后续 publishTrack(...) 仍是无超时的直接 await;本 PR 新增的 awaitBrowserMediaCapture 只用于视频采集、视频发布和媒体状态发布。浏览器麦克风授权如果用户一直不处理,底层 getUserMedia 没有应用层截止时间,browserSourceClient.start() 就不会结束;同时 useRoom.tsPromise.allSettled([startLocalInputOrCancelDispatch(), dispatchAgentSession()]) 会一直等待,即使 agent dispatch 已经成功,Start Call 仍永久 pending,现有恢复/清理分支也不会进入。

建议将同一个有 late-result 清理能力的超时边界覆盖音频采集和音频发布:采集超时后停止迟到音轨;发布超时后停止并在迟到完成时 unpublish。否则“bound browser media startup”只修复了摄像头路径。

[P1] connection-details 静默丢弃已有 room_config 契约

app/api/connection-details/route.ts:34-46 删除了 RoomConfiguration.fromJsonat.roomConfig,因此所有调用方提交的合法非 agent 配置也被吞掉,不只是为避免重复 dispatch 而移除 agents。基线实现本来会保留 empty_timeoutdeparture_timeoutmax_participants、metadata、egress、playout delay 等字段,只清空 agents。

我实际调用当前 PR 的 POST route,传入 room_config: { empty_timeout: 123, max_participants: 7 },再解码返回 JWT;roomConfig.emptyTimeoutroomConfig.maxParticipants 均为 undefined。也就是说自定义 connection-details 调用方会得到 200,但房间生命周期、人数上限或 egress 等配置不再生效,是静默行为回退。

兼容旧 LiveKit server 不应无条件删除显式输入。最小修复是:内部默认请求继续不发送 room_config;仅当调用方显式提供时保留非 agent 字段并清空 agents,或通过明确的兼容配置选择是否省略,而不是静默忽略。

验证

  • PR head:7ea48c1;merge-base:最新 lex-main a887615
  • pnpm test:281/281 通过
  • pnpm lint:通过
  • pnpm format:check:通过
  • pnpm build:通过(仅 onnxruntime 既有动态 require warning)
  • 额外 JWT 反例:失败,确认 room_config 丢失

此前自动 review 的“通过”结论遗漏了以上两个行为边界。

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 8s —— View job


Todo

  • 获取 PR 上下文,确认最新 diff(origin/lex-main...HEAD,19 files, +591/-69)
  • 核对上一轮人工 review(P1: 音频启动无超时、connection-details 丢弃 room_config)是否已在最新 commit(1ebbf52)修复
  • 检查 LiveKit room/track 生命周期、浏览器媒体权限失败路径
  • 检查 React hook 清理、Next.js 客户端/服务端边界、环境变量泄露
  • 输出 review 结论

Review 结论

通过

问题及建议

未发现需要阻塞合并的问题。核对说明:

  • 上一轮人工 review 指出的两个 P1 问题已在最新提交 1ebbf52(fix: bound browser audio startup)中修复:
    • hooks/useBrowserSourceClient.ts:166-199createLocalAudioTrackpublishTrack 现均已包裹在 awaitBrowserMediaCapturelib/browser-media-capture-timeout.ts)中,分别设置 8s/5s 超时,超时后通过 disposeLateResult 停止迟到的麦克风轨道或 unpublish,音频路径与此前已修复的摄像头路径保持一致,不会无限挂起。
    • app/api/connection-details/route.ts:36-40,107-112buildTokenRoomConfig 现仅在显式传入 room_config 时剥离 agents 字段,empty_timeout/max_participants 等其他字段会被保留并写入 JWT 的 roomConfig;内部默认调用(hooks/useRoom.ts:121-123)仍不发送 room_config,对旧版 LiveKit server 的兼容性未受影响。tests/connection-details.test.mjs 新增用例验证了该行为。
  • 此前多轮已确认的修复保持稳定:lib/browser-audio-capture.tsrunWithBrowserAudioTrackCleanup 热麦清理、诊断日志统一走 debugAudio 开关、lib/session-stop.tsisLiveKitRoomNotFoundError 幂等判断及来源注释。
  • hooks/useRoom.ts:262-266 在复用 Room 前调用幂等的 browserSourceClient.stop(),避免断线重连后新会话拿不到麦克风,有测试覆盖(tests/browser-room-session.test.mjs)。
  • lib/transcription-history.tslk.segment_id 用最终文本替换未完成片段的合并逻辑正确,测试完整。

残余风险(P2,不阻塞合并):

  • tests/browser-audio-capture.test.mjstests/browser-media-capture-timeout.test.mjs 中部分用例仍以匹配 hook 源码字符串的方式断言(而非贯穿 ensureAudioPublished 的完整行为集成测试),可后续加强。

说明:本次审查环境未安装依赖(无 node_modules),且 shell 权限限制导致无法执行 pnpm lint/pnpm test/pnpm build,以上结论基于静态代码审查与此前多轮已验证通过的测试记录。

@lanyue-llk

Copy link
Copy Markdown
Author

已处理最新 review 的两个 P1,最新提交为 1ebbf52

  • 麦克风采集和发布现在分别有 8 秒、5 秒上限;超时后立即回收当前轨道,迟到结果也会停止并撤销发布,Start Call 不再无限挂起。
  • connection-details 保留调用方显式传入的 room_config,仅移除 agents,避免与 /api/session/dispatch 的显式派发重复。
  • 新增 JWT 行为测试,验证 emptyTimeoutmaxParticipants 被保留且 token 中没有 agents。

验证:282 passed,lint 通过,production build 通过。构建仍只有既有的 onnxruntime-web 静态分析 warning。

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