Skip to content

fix: Align session cleanup with split media topology - #20

Merged
lanyue-llk merged 5 commits into
lex-mainfrom
fix/xunfei-edge-session-stop
Aug 13, 2026
Merged

fix: Align session cleanup with split media topology#20
lanyue-llk merged 5 commits into
lex-mainfrom
fix/xunfei-edge-session-stop

Conversation

@lanyue-llk

@lanyue-llk lanyue-llk commented Aug 10, 2026

Copy link
Copy Markdown

Goal

Align End Call with LexVoice's supported split Edge Media and Video Processor topology so Xunfei sessions release their input resources completely and the next Start Call waits for the local Agent worker to become available.

Design

  • Use the single supported server-input topology: stop Video Processor first, then Edge Media, matching the reverse dependency order.
  • Remove the obsolete combined/legacy room-input URL fallback instead of maintaining two cleanup contracts; Browser sessions keep their browser-owned cleanup path.
  • Delete the LiveKit room only after server-input shutdown, then observe worker capacity transitions from the current Agent live.log.
  • Cover the actual sequential executor behavior so Edge stop cannot start before Processor stop has settled.

Validation

  • pnpm test

    ✅ passed: 241 tests

  • Static and formatting checks

    ✅ passed: ESLint, Prettier, and tsc --noEmit --incremental false

  • Production build

    ✅ passed: Next.js build generated all seven routes

  • Deployed Xunfei lifecycle

    ✅ passed: repeated Start/End cycles stopped Processor then Edge, deleted the room, and returned the Agent worker to available

Risk

  • This frontend now requires the backend split-media contract (VIDEO_PROCESSOR_URL and EDGE_MEDIA_URL); unsupported legacy combined room-input deployments are intentionally rejected rather than silently partially cleaned.
  • Local readiness parsing follows the current one-worker-per-run Agent log contract.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

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


Review 结论

有条件通过

问题及建议

  • [P1] lib/session-stop.ts:117-121:当配置了 EDGE_MEDIA_URL/VIDEO_PROCESSOR_URL 时函数直接 return,跳过了下方 frontdeskInputParticipantUrl/faceServiceUrl/genericCameraParticipantUrl 的停止调用(对比 lex-main 旧版本,这些 URL 原本在 xunfei/generic 场景下总会被调用)。如果这些服务在新拓扑下仍是独立于 Edge Media/Video Processor 的资源,会导致对应输入进程未被释放,与 PR "release input resources completely" 的目标相悖。建议确认新旧拓扑是否互斥,并补充一条测试覆盖"同时配置 split 与 legacy 专属 URL 时的预期行为"。

  • [P1] app/api/session/stop/route.ts:352-355stopRoomInputPromise.all 改为顺序 for...of 逐个 await。PR 设计只要求 Video Processor 先于 Edge Media(2 个 URL 之间有顺序依赖),但该改动把所有 stop URL(包括 legacy fallback 下可能同时存在的 audio/vision/frontdesk/face 等最多 4 个互不依赖的 URL)全部串行化。每个请求超时 ROOM_INPUT_STOP_TIMEOUT_MS 默认 3000ms,最坏情况下总耗时从并行的 ~3s 变为串行的 ~12s。在非 deferred(同步等待)路径下会直接拉长 API 响应时间。建议仅对有序列依赖的 video-processor→edge-media 保持顺序,其余 URL 仍并行执行。

  • [P2] lib/agent-worker-readiness.ts:12-23:新增的本地容量日志匹配(worker is below/at full capacity)未校验 agentName,与上方基于 agentName 的远程状态解析逻辑不一致。PR 描述已注明这是基于"one-worker-per-run"假设的已知风险,可接受,但如果未来同一 live.log 中出现多个 worker 的输出,会互相覆盖状态导致误判为 available。建议后续加固或在代码中加注释说明该假设。

残余风险:split 拓扑分支目前没有测试锁定"跳过 frontdesk/face/generic URL"的行为,如果这是非预期的,回归很难被现有测试发现。

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 2m 49s —— View job


Review 结论

有条件通过

问题及建议

  • [P1] app/api/session/stop/route.ts:159-163lib/session-stop.ts:9-10,82-110:commit 8796930 已将 ROOM_AUDIO_INPUT_URL/ROOM_VISION_INPUT_URL/ROOM_INPUT_URL/FRONTDESK_INPUT_PARTICIPANT_URL/FACE_SERVICE_URL/GENERIC_CAMERA_PARTICIPANT_URL 全部移除,resolveRoomInputStopUrls 现在只接受 videoProcessorUrl/edgeMediaUrl。这与 PR 描述中 "Risk" 部分所写的 "Legacy room-input URLs remain the fallback when split topology URLs are absent" 直接矛盾——legacy fallback 已被彻底删除,而非保留为 fallback。若生产环境尚未配置 VIDEO_PROCESSOR_URL/EDGE_MEDIA_URL(仍用旧变量名),resolveRoomInputStopUrls 会静默返回空数组,导致 xunfei/generic 场景下 stop 请求完全不再释放输入资源,且没有任何报错或日志提示。建议:确认这是有意为之的一次性切换(而非渐进式迁移),并同步更新 PR 描述与部署文档,避免遗留环境在合并后出现输入资源泄漏且无告警。

  • [P2] lib/agent-worker-readiness.ts:12-23:本地容量日志匹配(worker is below/at full capacity)仍未校验 agentName,与此前 review 意见一致(已知的 one-worker-per-run 假设风险),目前代码里也未加注释说明该假设,后续如果同一 live.log 出现多 worker 输出会互相覆盖状态。建议至少补充一行注释说明假设前提,便于后来者不会误以为这是按 agentName 过滤的。

  • [P2] tests/session-stop.test.mjs:新增测试覆盖了 split URL 全配置、顺序执行、browser 跳过等场景,但缺少"只配置 videoProcessorUrl/edgeMediaUrl 中的一个"这种部分配置场景的用例,建议补充以锁定该分支行为。

残余风险:stopRoomInput 的顺序执行(video-processor → edge-media)已将并发路径从 4 个独立 URL 收敛为 2 个,超时上限降至约 6s(2×ROOM_INPUT_STOP_TIMEOUT_MS),此前 review 中关于串行化拖慢响应的顾虑已基本解决。postRoomInputStop 内部 catch 所有异常,不会因单个 URL 失败中断后续 stop 调用,逻辑正确。

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 2m 16s —— View job


Todo

  • 读取上下文与历史 review 评论
  • 查看 CLAUDE.md 与相关约定
  • 分析 git diff origin/lex-main...HEAD(含最新提交 805492e
  • 逐文件审查变更
  • 输出最终 review 结论

Review 结论

有条件通过

最新提交 805492e(reject incomplete split media cleanup)已解决前两轮 review 中的核心 P1 问题:resolveRoomInputStopUrls 在 split 拓扑 URL 缺失/重复时不再静默返回空数组,而是 throw,由 stopRoomInput 捕获并作为 room_input_configurationfatal:true)失败项返回,API 响应状态码相应变为 502/partial,不再有"资源未释放但无告警"的风险。顺序执行改造(Video Processor → Edge Media)已收敛为仅 2 个 URL,超时上限可控,此前对串行化拖慢响应的顾虑已解决。

问题及建议

  • [P2] lib/agent-worker-readiness.ts:12-13:本地容量日志匹配(worker is below/at full capacity)仍未按 agentName 过滤,也未加注释说明"one-worker-per-run"假设。前两轮 review 已提出,本次提交未处理。建议至少补一行注释,避免后来者误判这是按 agentName 过滤的。

  • [P2] tests/session-stop.test.mjs:97-141("session stop reports invalid split media configuration and continues room cleanup"):该用例删除了 LIVEKIT_URL/LIVEKIT_API_KEY/LIVEKIT_API_SECRET,导致 livekit_room 结果因凭据缺失被 skipped,并未真正验证"room_input_configuration 失败后仍会尝试删除房间"这一核心行为(route.tsdeleteLiveKitRoom 是无条件在 roomInputResults 之后执行的)。建议补一个凭据齐全场景的用例,锁定真实的房间删除尝试行为。

  • [P2] lib/session-stop.ts:91-107:split 拓扑现在要求只要 usesServerInput 为真(哪怕只是 mixed 模式下 audio 或 vision 其中一路走 server),就必须同时配置 VIDEO_PROCESSOR_URLEDGE_MEDIA_URL。旧版针对 mixed 场景"仅停用选中角色"的测试已被删除且未替换为新拓扑下的等价用例,建议补充 mixed + 部分 server input 的用例以锁定该行为是否符合预期。

残余风险:room_input_configuration 失败时(P0 级输入未能显式停止),deleteLiveKitRoom 仍会尝试强制删除房间以断开参与者连接,这在语义上是可接受的兜底,但后端 Video Processor/Edge Media 进程是否会在连接断开后自行退出取决于后端实现,不在本 PR 可控范围内。

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

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


Todo

  • 读取上下文与历史 review 评论
  • 分析 git diff origin/lex-main...HEAD(含最新提交 fb19331
  • 逐文件审查变更
  • 输出最终 review 结论

Review 结论

有条件通过

最新提交 fb19331(test: lock mixed split cleanup contract)已解决上一轮遗留的两个 P2 问题:lib/agent-worker-readiness.ts:12 补充了 "one-worker-per-run" 假设的注释,tests/session-stop.test.mjs 新增了 mixed 模式下 audio/vision 任一角色走 server input 时仍锁定完整 split URL 的用例。此前多轮提出的 P1(legacy fallback 静默丢失资源释放、topology 矛盾描述)均已通过 805492e 的 throw-on-incomplete-config 方案解决,PR 描述与实现一致。

问题及建议

  • [P2] tests/session-stop.test.mjs:97-141("session stop reports invalid split media configuration and continues room cleanup"):该用例仍然删除了 LIVEKIT_URL/LIVEKIT_API_KEY/LIVEKIT_API_SECRET,导致 livekit_room 因凭据缺失被 skipped,未真正验证 room_input_configuration fatal 失败后 deleteLiveKitRoomapp/api/session/stop/route.ts:371)仍会被无条件调用这一核心行为。建议补一个凭据齐全场景的用例(mock RoomServiceClient 或断言尝试调用),锁定真实的房间删除尝试路径。

残余风险:room_input_configuration 校验失败(P0 输入未显式停止)时,deleteLiveKitRoom 仍会尝试强制删除房间断开参与者连接,这是可接受的兜底;但后端 Video Processor/Edge Media 进程是否会随连接断开自行退出取决于后端实现,不在本 PR 可控范围内。

@lanyue-llk
lanyue-llk merged commit 4cec10e into lex-main Aug 13, 2026
2 checks passed
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