全面代码评审：24 项数据安全/崩溃/并发/功能修复（附完整评审报告） - #170
Merged
Merged
Conversation
同一 runloop 内多次 mutation（如一次走子触发 ensureFenId+ensureMove）原先会让 dataVersion 多次自增，破坏 checkpoint=dataVersion-1 的版本仲裁假设，引发虚假 版本告警、忽略远端真实更新；mutation 后立即 save() 也会因 isDirty 未置位被跳过。 现 markDirty/markClean 在主线程同步生效，非主线程调用保持异步派发。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
启动时若存档文件存在但读取/解码失败（损坏/未下载/schema 不兼容），原先静默创建 空库且保存时无任何确认，空库可直接覆盖云端真实数据。现在： - DatabaseStorage.databaseFileExists() 区分全新安装与加载失败（含 .icloud 占位文件） - Database 记录 loadFailedAtStartup 状态 - saveToDefault 在版本号不可读且存档存在时弹确认，确认后先把原存档备份到本地 Application Support/XiangqiNotebook/OverwriteBackups 再写入 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
原先 willTerminateNotification 只停引擎不保存，Cmd+Q 或 iOS 被系统杀掉时 所有未手动保存的修改（含引擎评估结果）静默丢失。现 macOS 退出前、iOS 进 后台时自动保存；并复用 saveToDefault 的安全护栏：远端版本更新或版本不可 读时跳过自动保存，留给用户手动走确认流程。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
删除组或路径时原先只在删的恰好是选中项时重置选择；选中靠后的项再删除 靠前的项会留下 stale index，pathGroups[selectedGroupIndex] 越界崩溃。 现删除后平移/重置选中索引，并以 selectionIsValid 统一做边界保护。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PGN 的 [FEN] 头原先未经任何校验经 ensureFenId 入库，空串或行列数错误的 FEN 会在 fenToPiecesBySquare（数组越界）或着法列表显示（String.index 越界） 时崩溃，用户可用一份畸形 PGN 文件触发。现在： - 新增 XiangqiBoardUtils.isValidBoardFen 结构校验（10 行 × 9 列、字符集） - generateFenSequence 对外部 FEN 头校验失败返回 nil（按导入错误计数） - fenToPiecesBySquare/findDiffColumns/normalizeFen 对畸形输入防御性返回 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
playRandomGame 强制解包 allGamePaths 并在空范围上 Int.random：黑方开局库 为空的用户按随机一局（随机到黑方筛选时）当前局面不在视图范围内，生成不出 任何路径，直接崩溃。现无可用路径时返回 nil 并保持当前局面。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
棋局可同时挂在多本棋书下；deleteBook 删除棋局对象但不清理其他棋书的 gameIds 引用，之后 getGamesInBookUnfiltered 的强制解包即崩溃。改用 compactMap 容忍悬空引用。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 文件选择弹窗 .actionSheet 改 .alert：iPad 上未配置 popover 锚点的 actionSheet 在 present 时抛 NSGenericException 直接崩溃 - presentingViewController 不可用时统一回调 completion(nil/false)： 原先静默不回调，recoverFromUserChoice 的 withCheckedContinuation 永久泄漏 - openFile/recoverData 主线程化（原先在调用方线程直接操作 UIKit） 注：本机 Xcode 未安装 iOS 平台支持，此 iOS-only 文件经 swiftc -parse 语法 校验并对照同文件既有模式人工核对；macOS 全量测试通过。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- iPadContentView 补 setViewModel 接线与全局 .alert modifier（此前 showAlert/showWarningAlert 全部 no-op，练习模式提示无声消失） - presentingViewController 改为呈现时惰性解析：init 时机 scene 尚未 foregroundActive，注入值常为 nil；并下钻到最顶层 presented VC， 避免已有弹窗时呈现失败 注：iOS-only 代码经 swiftc -parse 校验与既有模式核对；macOS 全量测试通过。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
queryFenScore/pikafishQuickMove 的 catch 分支会从后台任务线程调用 showWarningAlert，dismissCurrentAlert 在调用方线程直接触碰 AppKit （orderOut/NSApp.stopModal）并无同步地读写共享状态，违反 AppKit 主线程 约定。现三个弹窗方法整体先收敛到主线程再执行。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
markAsRemoved 只把 targetFenId 置 nil（软删除），moveToId 映射残留； 重新走该招时 ensureMove 返回僵尸 move，addMoveIfNeeded 与 autoExtendGame 均拒绝，棋子无声弹回且本会话内无法恢复（重启后 rebuildIndexes 排除僵尸 才正常）。现在： - ensureMove 跳过 targetFenId 为 nil 的僵尸映射，直接新建（兼容老数据遗留状态） - removeCurrentStep 删招时同步清理 moveToId 条目 - 僵尸对象保留在 moveObjects 中，维持 count+1 发号方案的 ID 唯一性 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
start() 原先 guard process == nil，对引擎崩溃后残留的死进程引用直接 no-op；evaluatePosition 的重启路径形同虚设，之后每次评估都 10 秒超时， 只能重启 app 恢复。现检测到进程已死时清理重启；启动流程失败时终止进程 并清空状态，保证下次可干净重试。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cancelAll 原先立即置 nil processingTask，但 cancel 不会中断在飞的 evaluatePosition；紧接着的 enqueue 会启动第二个处理任务，与旧任务并发 驱动同一个无锁的 PikafishService（交错 UCI 命令、竞争 outputBuffer）。 被取消任务的收尾还会误删新队列的去重键、覆盖发布状态。现在： - 引入取消代数 generation，过期任务不得触碰新一代队列的共享状态 - processingTask 槽位由任务退出时自行释放；取消期间入队的新请求 在旧任务退出后自动补启动处理 - cancelAll 后在飞任务退出前队列不算 idle（同时收紧 pikafishQuickMove 的 isIdle 防并发护栏） 测试：更新 testCancelAll 以反映新语义（取消后等待旧任务退出再断言 idle）， 新增取消后立即重新入队的回归测试。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
两者经快捷键闭包的无结构 Task 调用，原先整段在全局执行器上运行： 惰性创建 pikafishService/evaluationQueue 的 ivar 写入、queue.isIdle （MainActor 状态）与 session 读取均与主线程竞争。现标记 @mainactor， 仅引擎/网络的 await 在后台挂起，原 MainActor.run 包装随之内联。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
importPGNFile 修改 Database 并切换 @published dataChanged，原先在 PGNHttpServer 私有队列与 DispatchQueue.global 上直接调用，属于离主线程 发布 + 与主线程的数据竞争（与 #130 修复的同类问题）。HTTP 回调用 main.sync 等待结果以便写回响应；文件导入路径读文件留后台、导入回主线程。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
新增的取消语义测试依赖固定时长等待，负载波动下偶发失败；改为带超时 的条件轮询。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- {} 注释与 () 变着用跨行状态机剥离：注释内形如坐标的 token（如
{ h2e2 better }）原先会被当真实着法插入主线，静默污染棋局
- 同时含 [Game] 与 [Event] 头的标准 PGN 原先每局被拆成幽灵局+真实局
两份；现仅在当前局已读到着法时才开新局
- parseDate 设 en_US_POSIX locale，避免非公历系统日历下解析错误
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
标准 SM-2 规定 q<3 时只重置重复次数与间隔、不更新 EF；原实现对失败评分 也执行 EF 公式（.again 每次 -0.54、.hard -0.32），叠加间隔重置形成双重 惩罚，失败项 EF 速降钉死在下限 1.3，间隔再也长不起来。现仅成功评分 （q>=3）更新 EF。同步更新编码旧语义的测试。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
原先随机调用 toggleFilterRed/BlackOpeningOnly：已在红方开局筛选时随机到 红方会把筛选关掉、在全库上随机。意图是切换到目标筛选，改用显式 setFilters（与 practiceRedOpening 等同模式一致）。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
循环内 guard else return 会让一个局面的数据缺失（targetFenObject 不在 范围内）中止整个批量操作并返回部分统计且无提示，改为 continue。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
toggleStepLimitation 与 toggleShowLastMove 都注册了 sequence(",l")，
查找表后注册者覆盖前者，步数限制的快捷键静默失效。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
旧实现注释称选择最新版本，实际无条件用 gained 的冲突版本覆盖当前文件 （冲突版本可能更旧），随后 removeOtherVersionsOfItem 把被覆盖的版本永久 删除；操作未经写协调、未标记 isResolved，还强制解包 presentedItemURL。 现比较 modificationDate 仅在冲突版本较新时经协调写替换，并对所有未解决 冲突版本置 isResolved。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
StatusBarView 经 viewModel.evaluationQueue.state 读取进度，但没有视图 直接观察 EvaluationQueue 这个 ObservableObject，ViewModel 也不转发其 objectWillChange——取消评估后进度条悬挂到下一次无关更新才消失。现在 ensureEvaluationQueue 时把队列的 objectWillChange 转发给 ViewModel。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scrollPosition(id:) 缺少配套的 scrollTargetLayout()，且 MoveListView 的 scrollPosition 误挂在外层 VStack 而非 ScrollView 上；VariantListView 的 ForEach 身份（Move）与 scrollPosition 值类型（Int 索引）不匹配。三处 修正后长棋局导航时列表能正确跟随当前着法滚动。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Database.reload() 与 restoreFromBackup() 替换 databaseData 后未失效 realGamesByFenId，isRealGamesIndexReady 仍为 true，实战列表会按旧库 内容显示错误数据。索引失效后查询自动走线性扫描兜底路径。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
exportPGNCurrentDatabaseView/exportPGNCurrentGame 的实现包在 #if os(macOS) 内（使用 NSSavePanel），但动作注册没有平台守卫，iOS 编译报 no member 错误。main 分支即存在（安装 iOS SDK 后首次构建发现）， 自该功能引入后 iOS target 一直无法编译。 验证：iOS Simulator 构建 BUILD SUCCEEDED；macOS 全量单元测试通过。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
防止 macOS-only API 漏进共享代码（macOS 端日常构建与测试查不出此类 问题，见 #171 的存量案例）。模拟器构建用 ad-hoc 签名，无需证书。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GitHub 警告 Node.js 20 actions 将于 2026-06-16 起强制运行在 Node 24 上， checkout@v4 可能失效；v5 原生支持 Node 24。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 移除 .github/workflows/ci.yml - 新增版本化的 .githooks/pre-push：push 前运行 iOS Simulator 构建， 防止 macOS-only API 漏进共享代码（存量案例见 #171） - CLAUDE.md 记录启用方式（git config core.hooksPath .githooks） 与跳过方式（git push --no-verify） Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
按用户反馈调整 #144：保留「不自动保存、Cmd+Q 即丢弃未保存改动」的工作流， 改为崩溃保护—— - 撤销退出/进后台自动写 database.json 的行为 - 周期性（30s，仅脏且版本变化时）把当前数据库写入本地固定恢复文件 Application Support/XiangqiNotebook/recovery.json（不碰正式存档、不走 iCloud） - macOS 干净退出（willTerminate）清除快照——干净退出视为主动丢弃， 只有崩溃/被杀（不触发回调）才会留下快照 - iOS 进后台强制写一次快照（防挂起期间被系统杀） - 手动保存成功 / 冲突解决后清除快照 - 启动时若快照版本 > 已保存版本，弹确认（恢复/丢弃）；恢复后保持 dirty 待手动保存 新增 PlatformService.showConfirmAlert 自定义按钮文案重载（Mac/iOS 各自实现， 带默认实现以兼容测试 mock），供恢复对话框用「恢复/丢弃」按钮。 测试：StorageTests 新增恢复快照读写/版本/缺失文件用例；macOS 全量测试通过。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 12, 2026
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
1 participant
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.
概述
基于一次全面代码评审（5 个维度：存储与并发、Views 层、Services 与网络、测试套件、象棋领域逻辑），本 PR 修复其中 24 项可安全自动化修复的问题 + 1 个存量构建问题 + 1 个崩溃恢复增强，每项一个独立 commit、引用对应 issue。
文档（包含在本 PR 中）
修复清单
数据安全（P0）
崩溃（P0）
并发与线程
功能正确性
构建（评审后续验证中发现的存量问题）
工程
.githooks/pre-push：push 前本地跑 iOS Simulator 构建，防止 macOS-only API 再次漏进共享代码（启用：git config core.hooksPath .githooks）actions/checkout升级 v4 → v5（GitHub 6/16 起强制 Node 24）不在本 PR、已建 issue 跟踪
验证
BUILD SUCCEEDED（修复 [CR-31] iOS target 无法编译：PGN 导出动作注册缺少平台守卫 #171 前 main 与本分支均无法编译 iOS target）🤖 Generated with Claude Code