Skip to content

fix(acp): prevent white-screen crash from stale session restore#1791

Merged
piorpua merged 4 commits intomainfrom
fix/session-restore-crash
Mar 29, 2026
Merged

fix(acp): prevent white-screen crash from stale session restore#1791
piorpua merged 4 commits intomainfrom
fix/session-restore-crash

Conversation

@kaizhou-lab
Copy link
Copy Markdown
Collaborator

Summary

  • Validate ACP session ownership before restore to prevent cross-conversation binding
  • Handle user_message_chunk protocol update to eliminate unknown-type warnings during session restore
  • Add renderer crash circuit-breaker: skip destroyed windows in emit loop and auto-reload on render-process-gone

Changes

Session restore ownership (src/process/agent/acp/index.ts, AcpAgentManager.ts, storage.ts)

  • Store acpSessionConversationId alongside acpSessionId when persisting session data
  • Before resuming, check if the stored session belongs to the current conversation
  • If mismatched, discard stale session and create a fresh one

Protocol handling (src/process/agent/acp/AcpAdapter.ts)

  • Add explicit user_message_chunk case in the session update switch
  • Silently ignore these echoed-back user messages during restore (already displayed from local DB)

Crash circuit-breaker (src/common/adapter/main.ts, src/index.ts)

  • Guard webContents.send() with isDestroyed() check, auto-remove dead windows from adapter list
  • On render-process-gone, reload the renderer instead of leaving a white screen

Tests (tests/unit/)

  • acpSessionOwnership.test.ts — 5 cases covering ownership mismatch, same-conversation resume, legacy data, fresh session, and resume failure
  • acpAdapterUserMessageChunk.test.ts — 3 cases for silent handling and warning behavior
  • adapterEmitGuard.test.ts — 4 cases for destroyed window skipping

Related Issue

Closes #1790

Test Plan

  • Verify bunx tsc --noEmit passes
  • Verify bun run test — 12 new tests pass (3 files)
  • Manual: open conversation A with ACP session, switch to conversation B, verify B gets a fresh session (not A's)
  • Manual: simulate renderer crash (kill renderer process), verify window auto-reloads instead of white screen

@piorpua piorpua added the bot:reviewing Review in progress (mutex) label Mar 29, 2026
@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 29, 2026

Code Review:fix(acp): prevent white-screen crash from stale session restore (#1791)

变更概述

本 PR 修复了三类独立问题:①ACP session 跨会话错误绑定导致会话恢复到错误对话;②session/load restore 时 user_message_chunk 类型未处理导致 unknown-type 警告;③渲染进程崩溃后白屏(render-process-gone 未做恢复)。改动涵盖 src/process/agent/acp/src/common/adapter/main.tssrc/index.ts,并新增 12 个单元测试。


方案评估

结论:✅ 方案合理

三个问题的修复各自独立、精准。Session 所有权校验通过在存储时写入 acpSessionConversationId + 恢复时比对,逻辑简洁;兼容无 acpSessionConversationId 的遗留数据(legacy backward compat 由 resumeConversationId 为 falsy 时绕过判断实现)。adapter/main.ts 改为反向迭代以安全 splice 是标准模式。render-process-gone 处理器正确区分了开发/生产模式,且通过 isDestroyed() 前置检查防止二次崩溃。无过度工程化,无明显设计盲点。


问题清单

✅ 未发现新引入的问题。以下是对各维度的检查结论:

  • 正确性:逻辑无误。createOrResumeSession 中所有权检查失败后会正确走到 newSession 分支,之后 saveAcpSessionId 会同步写入正确的 acpSessionConversationId
  • 不可变性adapterWindowList.splice() 是模块级列表的维护操作,与现有 off() 清理逻辑一致,不违反不可变原则。
  • 错误处理:所有 loadURL/loadFile 调用均有 .catch()createOrResumeSession 失败路径已有 fallback。
  • 测试:新增 3 个测试文件共 12 个用例,完整覆盖了所有权不匹配、同会话恢复、遗留数据、无 session ID、恢复失败五个场景,以及 user_message_chunk 和 emit guard。
  • Lint:变更代码行无新增 lint 警告(oxlint 输出中的所有 warning 均为改动前已存在的行)。

汇总

# 严重级别 文件 问题

结论

批准合并 — 无阻塞性问题,代码质量良好,测试覆盖充分。


本报告由本地 pr-review skill 生成,包含完整项目上下文,无截断限制。

CONCLUSION: APPROVED
IS_CRITICAL_PATH: false
PR_NUMBER: 1791

@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 29, 2026

✅ 已自动 review,无阻塞性问题,正在触发自动合并。

@piorpua piorpua merged commit ae30254 into main Mar 29, 2026
17 checks passed
@piorpua piorpua deleted the fix/session-restore-crash branch March 29, 2026 06:14
@piorpua piorpua added bot:done Auto-merged by bot and removed bot:reviewing Review in progress (mutex) labels Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:done Auto-merged by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: session restore crash causes white screen on conversation switch

2 participants