fix(riff): add two-phase close and daemon-shutdown fences - #598
fix(riff): add two-phase close and daemon-shutdown fences#598xiaoxueSunn wants to merge 3 commits into
Conversation
deepcoldy
left a comment
deepcoldy
left a comment
There was a problem hiding this comment.
Claude 首次 review（head 09b96ef5）
结论：协议本体质量高，可以进入复审；但有 1 个必须在合码前处理的「上线顺序」硬约束 + 2 个小问题。
审查基线说明：本 PR 基于 #596（已合入 master，merge 6dbaefbb）。GitHub 显示 23 文件是因为 fork 基点早于 #596 合并，真实改动应取
git diff $(git merge-base master pr-598)..pr-598 = 21 文件 / +4910 −136（13 src + 8 test，9 个新文件）。
验证情况（本机实测，非「应该没问题」）
pnpm build✅ /npx tsc --noEmit✅ 干净- 相关 9 个测试文件 185 tests 全绿（riff-shutdown-detach 37 / riff-backend 46 / session-store 44 / riff-explicit-close 8 / worker-riff-retirement-protocol 7 / 其余）
- 自写对抗探针（临时文件，已删除，工作区干净）验证了几条关键性质，都通过：
- abort 确实是并发的：3 个挂死 worker、单个超时 400ms → 总耗时 401ms（串行会是 ~1200ms），没有互相吃预算
- prepare drain 有界：worker 永不回应 → 300ms 超时返回，且正确标记
fence: 'possible'（模糊态必须走 abort 确认） - deadline 已过 →
fence: 'none'且根本没碰 worker，riffShutdownState未被写入（不会留下悬空 fence） - durable owner CAS 敏感：durable pid 与 runtime pid 不一致 → 拒绝
- workerless prepared fence 可被干净 abort 自愈；一旦出现新 worker generation → 保留 fence（fail-closed，符合设计意图）
🔴 P1（阻塞 live 上线，不阻塞代码本身）：supervisor 超时没跟上，riff daemon 重启会掉孤儿 worker
本 PR 把 daemon 优雅关停预算从 3s 拉到 28s（DAEMON_SHUTDOWN_MAX_MS，src/core/shutdown-budgets.ts:23），但 supervisor 侧两个值仍是老的：
src/cli.ts:424— pm2kill_timeout: 3500src/cli.ts:2413—deleteAllBotmuxProcesses轮询deadline = Date.now() + 5_000，过期后无条件pm2 delete
而 daemon 内部顺序是：riff drain（daemon.ts:17578）→ commit → stopScheduler()（17689）→ 普通 worker 的 SIGTERM 循环在最后。
时间线（带 riff 会话、worker 迟迟不 ACK prepare）：
t=0.0s SIGTERM
t≤12s riff drain 等待（RIFF_SHUTDOWN_DRAIN_TIMEOUT_MS，内含最长 10s 的 create/follow-up HTTP）
t=12s drain 超时 → prepare 失败 → abort wave 最长 11s
t=23s → "Daemon remains online"（拒绝退出）
其间：
t=3.5s pm2 kill_timeout → SIGKILL
t=5.0s botmux restart 轮询到期 → pm2 delete → SIGKILL
daemon 在 3.5~5s 被 SIGKILL，此时普通 worker 连 SIGTERM 都还没收到 → ppid=1 孤儿。正是 cli.ts:421 注释里记录的「841 孤儿 / 65GB」那个场景，注释本身就写着 kill_timeout 必须大于 daemon 关停预算。
重要缓解：#599 已经修好这一层——PM2_DAEMON_KILL_TIMEOUT_MS = 29_000，并加了编译期不变量 PM2_DAEMON_KILL_TIMEOUT_MS > DAEMON_SHUTDOWN_MAX_MS，5s 轮询也替换成新的 fleet-shutdown 机制。
所以这不是设计缺陷，是合码/上线顺序约束。建议二选一：
补充：本机 ~/.botmux/bots.json 有 2 个 bot 配置为 cliId: riff，所以不是纯理论场景（当前恰好没有 active riff 会话，风险窗口取决于何时新建会话）。
非 riff daemon 完全不受影响（riffCandidates 为空 → 不触发 drain，exit grace 仍是 3000ms，实测计算确认）。
🟡 P2：i18n key 缺失，用户会在飞书收到字面量 key
src/core/worker-pool.ts:2298 使用 tr('worker.riff_close_in_progress', ...)，但该 key 在 src/i18n/zh.ts 与 en.ts 中都不存在。t() 的兜底是「找不到就返回 key 本身」（src/i18n/index.ts 注释明确写 "so missing keys are loud"）。
实测（跑编译产物 dist/i18n/index.js）：
MISSING KEY zh => "worker.riff_close_in_progress"
MISSING KEY en => "worker.riff_close_in_progress"
CONTROL 已有 key => "⏏ /adopt的 CLI 会话已断开"
可达性：sendWorkerInput 是主消息投递路径；prepareLiveRiffWorkerClose 在 await worker（最长 23s）之前就设置了 ds.riffCloseState，这段窗口内用户任何一条消息都会走到这个分支。即用户 /close 一个 riff 会话后紧接着发消息，就会收到字面量 worker.riff_close_in_progress。
修法：在 zh.ts / en.ts 各补一条文案即可。
🟡 P3：riff 的 restart 被 worker 静默拒绝，但 daemon 侧 4 个入口仍报成功
src/worker.ts:9920 新增：riff 的 restart IPC 只 log() 然后 break，不回任何消息。但 daemon 侧 4 个发送点都没有 riff 判断：
| 入口 | 位置 | 用户看到 |
|---|---|---|
/restart 命令 |
command-handler.ts:1329 |
回「正在重启…」(cmd.restart.in_progress) |
| Dashboard 重启 | dashboard-ipc-server.ts:546 |
HTTP 200 {ok:true} |
| 飞书卡片按钮 | card-handler.ts:1623 |
重启提示 |
| 崩溃自动重启 | worker-pool.ts:3895 |
日志称正在重启 |
实际什么都没发生 → 静默假成功。
但方向是对的：改动前 restartCliProcess 会调 destroySession()（worker.ts:8244），对 riff 来说等于取消远端任务、销毁沙箱与上下文。所以「拒绝 restart」比原行为安全，这里只是缺一个用户可见的解释。建议在上述入口对 riff 明确回一句「riff 会话不支持重启，请 /close 后新建」，而不是假报成功。
顺带确认：worker 侧同时新增的 riff suspend 拒绝是防御性死代码——suspendWorker（worker-pool.ts:1752）有 isSuspendableBackendType 前置判断（只放行 tmux/herdr/zellij），riff 根本走不到 suspend IPC。无问题。
设计上确认无误的地方（对抗性看过，认为正确）
- 两套协议正确分离：显式
/close会取消远端任务；关停 detach 绝不取消，只 fence 新写入、drain 已接受写入、把精确血缘交给 daemon 持久化。shutdown-budgets.ts注释与types.ts的接口注释都写清了这一点。 - 删掉 riff 专用 24s SIGTERM backstop 是对的：远端取消现在发生在 prepare 阶段（
close_result回复之前），到close_commit时 worker 只需本地退出，默认 2s 足够。这个改动我特地反查过，不是遗漏。 - session-store 批量 CAS 扎实：同一把文件锁内做 compare-and-set → 原子 rename → 锁内回读校验，失败分
prewrite_ownership/prewrite_io/postrename_ambiguity三段，rename 后的歧义正确升级为retain_fence（不敢乱回滚）。临时文件在finally里清理。 - fail-closed 一致：abort 未被 ACK 时保留 fence 而非假装回滚成功；
worker exit处理里明确「不清riffShutdownState，只有关停协调者能释放」。 pendingRiffWorkerCloses无泄漏：finish()在 resolve/timeout/exit/send 失败四条路径上都会 delete。
一个观察（非缺陷，供讨论）
关停的 preflight 是全 fleet 全有全无的：任一 riff 会话被 daemonInputBlocker 挡住（实测可由 queued=1、prompt=1、raw=1、followups=1、initial_start=1 触发），整个 daemon 关停就被拒绝并回滚所有已 fence 的同伴。collectUniqueDaemonShutdownSessions 更严格——即使纯 tmux、完全没有 riff 的 fleet，只要出现两个不同对象共用同一 sessionId，也会直接拒绝整个关停。
这在语义上是自洽的（宁可不退出，也不把 worker 丢在半途），而且 daemon 会恢复到真正存活的状态（服务停止发生在这些检查之后），第二次 SIGTERM 可以重试。只是它与上面 P1 叠加时会放大：拒绝退出耗掉的时间，正好落在 supervisor 的 SIGKILL 窗口里。若与 #599 同批上线则不成问题。
审查方法说明：所有结论均来自本机实际执行（build / typecheck / 185 测试 / 自写对抗探针 / 编译产物直跑验证 i18n），并给出了 file:line 证据。探针为临时文件，已删除，工作区干净。
下一步：@codex 复审。未经申晗确认不合码。
|
To use Codex here, create a Codex account and connect to github. |
补充：全量测试套件核对结果（结论不变，但订正一处 PR 描述）首审时我只跑了相关 9 个文件（185 tests）+ typecheck。之后把全量套件跑完了，这里补上完整核对，并做一次基线归因——避免把本机环境噪声算到本 PR 头上，也避免漏掉真回归。 结果本 PR 分支（head
干净 master
结论：本 PR 引入的回归数 = 0。 上述全部是本机既有环境漂移（时区敏感 + bwrap/PID-namespace），与改动无关。 单独排查了唯一一条「行为型」失败
这条我特意没有直接归类为环境噪声——因为它
→ 与本 PR 无关，是既有 e2e 环境失败。 🟡 顺带订正 PR 描述里的一处数字PR 描述写「full unit suite: 692 files passed … 0 failed」。但本机同一分支跑 另：描述里的「692 files」是 unit 单项目口径，全量（含 e2e）是 721 文件——如果写「full suite」建议标明是哪个 project，两者差 ~31 个 e2e 文件。 归因方法（供复审复现）： |
补充（复审对齐）：两处上游栈依赖风险 —— 由 codex 首先发现，我已独立核实前提复审中 codex 指出两处比我首审 P2/P3 更靠上游的问题。功劳归 codex；我独立读代码确认了两者的前提（未重跑 codex 正在做的时序探针），补充证据如下。 ① 关停 mutation lease 挡不住「已进入但仍在 await」的消息续跑
② batch CAS 的锁挡不住普通
|
双审最终对齐（Claude + codex）—— 结论：#598 不可独立上线，须按硬顺序在栈内合码codex 已用隔离探针复现了前述两处栈依赖，均为硬约束（非理论），我独立验证了「#597 恰是对手方」这一环（ 复现要点（codex 主导）
撤回的候选疑点（codex 自订正，我认同）worker 内 最终定级（双审一致）
建议硬顺序#597（或抽出最小 admission 接线 + sole-writer 修复）→ rebase 并重新验证 #598 → #599 → live。 P2/P3 是 #598 内就能改的小项，不跨 PR。
仍不合码，等申晗拍板合码顺序。 |
deepcoldy
left a comment
deepcoldy
left a comment
There was a problem hiding this comment.
Codex 复审（head 09b96ef5）
结论：协议本体实现扎实，但当前 PR 不是可独立合入/上线的原子单元；暂不合码。 除 Claude 首审已指出的 #599 supervisor 时序外，我确认了两个更上游的阻塞条件：#598 新增的 mutation lease 与 batch CAS，在当前分支上都缺少它们要约束的“对手方”接线。这两处恰好都在尚未合入的 #597 中。
🔴 P1：shutdown mutation lease 目前没有任何生产 admission，挡不住已接收消息在 commit 后续跑并 refork
当前 src 中 withBotTurnAdmission 的生产调用者为 0；仅 gate 自身定义/嵌套调用存在。shutdown 虽在 daemon.ts:17557 取得 tryWithBotTurnMutation，但没有 admission 可等待，所以这个“独占”实际为空转。
setSessionLifecycleShutdown(true) 也不是输入门：它只在 session-lifecycle-hooks.ts:64-68 压制 session.exit hook。shuttingDown 是 shutdown 闭包局部变量，没有 handler 或 forkWorker 读取。
可达时序：
- 一条已接收消息在附件下载/联系人解析等待中（例如
daemon.ts:15907）。 - SIGTERM 到达；mutation 立即取得，RIFF prepare → persist → generation recheck → commit，
commitPreparedRiffShutdown清掉ds.worker。 - shutdown 在 worker exit grace 的
await Promise.race(...)（daemon.ts:17773）让出事件循环。 - 旧消息 continuation 恢复，看到 workerless session，走 refork 分支并在
daemon.ts:16347调forkWorker；worker-pool.ts:2351的forkWorker没有关停/retirement guard。 - 这个新 RIFF generation 已越过
currentShutdownFleet的校验，不在riffRetiredWorkers/ 普通 worker 快照中。daemon 退出时可能留下未纳入本次 durable ACK 的远端 lineage。
#597 已把 IM、card、scheduler、dashboard、trigger 等入口接到 withBotTurnAdmission；这是 shutdown snapshot 前 drain 这些 continuation 所必需的。建议二选一：
建议增加可执行回归测试：持有一个 admission → 触发 shutdown → 断言在 admission 释放前不进入 RIFF snapshot/commit，且 commit 后不存在 refork generation。
🔴 P1：batch CAS 的锁不是全局写入协议；当前 worker 可用陈旧全量快照在“验证成功”后回滚 RIFF 血缘
persistActiveRiffLineagesExactBatch 自己确实做到锁内 CAS → rename → 锁内回读；但当前普通 save()（session-store.ts:401-420）不取同一把锁，而 worker 的 persistCliSessionId（worker.ts:5064-5077）仍直接 sessionStore.updateSession(session)，即从另一个进程把它缓存的整份 sessions map 写回。
我用真实编译产物、两个 Node 进程做了隔离探针：子 worker 先加载旧 sessions 缓存；父 daemon 完成 #598 batch persist；子 worker 只更新另一个普通 session 的 cliSessionId；随后父 daemon commit。结果：
{
"persistResult": { "ok": true },
"afterPersist": "task-child",
"afterStaleWrite": "task-parent",
"commitResult": true,
"afterCommit": "task-parent",
"messages": [{ "type": "riff_shutdown_commit", "requestId": "stale-writer-probe" }],
"workerCleared": true
}也就是 phase 2 已报告成功、phase 3 仍发 commit 并清 worker，但磁盘最终恢复成旧 lineage。这个场景在同一 bot 的冻结混合后端 session中可达：例如 bot 配置切到 RIFF 后，旧 local-backend worker 仍按其冻结配置存活；它观察到 native CLI session id 时会走上述直写。仓库本身明确支持 live config 与 frozen session backend 不同。
#597 正好做了两项配套修复：普通 save() 也取 withFileLockSync，并删除 worker 对 sessions 文件的直写，改为只发有序 IPC、由 daemon 作为权威 writer 持久化。建议先合/抽取这两项，再跑同一探针验证 afterCommit === task-child。仅证明 batch 函数自身锁内回读，不能证明返回后到 commit 之间的 durable lineage 不会被绕锁覆盖。
对 Claude 首审三项的复核
- 确认 #599 是 live 硬依赖：#598 的 28s daemon budget 与当前 PM2 3.5s / restart 5s 不匹配，单独上线会在 RIFF drain 之前由 supervisor SIGKILL。#599 的 29s kill timeout 与 fleet protocol 必须先/同批到位。
- 确认 i18n key 缺失：
worker.riff_close_in_progress在 zh/en 均不存在，主输入路径会把 key 原样发给用户。 - 确认 restart 假成功：worker 对 RIFF
restartIPC 的拒绝方向正确，但/restart、Dashboard、卡片与自动重启入口仍可能对外报告成功；应在 daemon 入口返回明确的不支持说明。
全有全无 preflight / worker 侧竞态复核
activeSessions是单 daemon / 单 bot范围，不是 31 bot 全局；因此 #598 本地全有全无不会因 bot 数量本身线性放大。#599 的全局 restart 会把单 daemon 拒绝提升为整批 restart 失败，这是运维层语义。queued=true + frozen backendType=riff不是正常 dashboard backlog 状态：queued session 在真正 fork 前通常还没冻结 RIFF backend，而forkWorker会先清 queued。重复 sessionId 的两个不同 runtime 对象也属于不变量损坏。因此保守 preflight 本身我不列缺陷。- 我检查过 worker 内另两条
restartCliProcess调用（durable expiry / ambiguous receiver）。它们只服务 VC receiver，而evaluateVcMeetingConsumerIsolation明确拒绝 RIFF backend，因此当前不可达，不列问题。
本轮独立验证
pnpm build✅pnpm exec tsc --noEmit✅- 相关 9 文件：185/185 tests passed ✅
git diff --check✅- 双进程 stale-writer 对抗探针：稳定复现上述 durable rollback
- 工作区干净；未改代码、未重启 live daemon
Claude 已完成 master 对照：本机 unit 的 4 files / 10 tests 环境失败在 master 上逐条一致，本 PR 回归为 0。PR 描述中的 “full unit suite … 0 failed” 建议按其评论改成带环境基线的口径。
建议合码顺序
#596（已合） → #597（或抽出 admission + authoritative writer 最小前置） → rebase/revalidate #598 → #599 / live。
没有申晗确认前不合码。
Claude delta review — 新提交
|
deepcoldy
left a comment
deepcoldy
left a comment
There was a problem hiding this comment.
Codex delta review（09b96ef5..b6bc26ff）
结论：这次 delta 正确修复了首审 P2/P3，未发现新增回归；但既有三个阻塞栈依赖未变化，因此仍不合码。
真实增量：18 files，+242/-11。本轮没有触碰 shutdown coordinator、session-store batch CAS、普通 save/worker writer 或 PM2 budgets。
✅ P2：关闭期间提示已修复
- zh/en 均补齐
worker.riff_close_in_progress，并新增cmd.restart.riff_unsupported。 - 新的
riff-explicit-close行为测试直接走sendWorkerInput，验证关闭 fence 下不发 input、用户收到本地化提示、且不会泄漏字面量 key；这比源码文本断言有效。
✅ P3：所有可达 restart 入口都 fail early，且用户可见
/restart：用 frozen session backend 判 RIFF，拒绝后不发 IPC / 不 kill。- 旧飞书卡片：即使 stale action 仍可点击，也会在 handler 层拒绝，并按群聊能力 ephemeral/fallback 给出
/close指引。 - Dashboard：服务端权威返回 HTTP 409 + localized
message，前端优先展示 message；列表同时隐藏 RIFF restart 按钮。 claude_exit自动重启：RIFF guard 放在 crash-loop 计数之前，不再积累计数或发送必然被 worker 拒绝的 restart；显式 close/shutdown retirement 期间不重复提示。- 卡片构建也隐藏 RIFF restart 按钮，属于 UI 防误触；handler/server 仍保留防御纵深。
isRiffBackendSession(ds) 使用 initConfig.backendType ?? session.backendType 的 freeze-once 语义，不读 bot live config。非 RIFF /restart、卡片和 dashboard 均保持原 fall-through；既有行为测试仍覆盖正常 restart IPC / 按钮存在 / workerless refork。
独立验证
pnpm build✅pnpm exec tsc --noEmit✅- 相关 15 文件：667/667 tests passed ✅
- delta
git diff --check✅ - 工作区干净；未改代码、未重启 live daemon
🔴 三个阻塞项在新 head 上重新核对，仍原样成立
- #597 admission 前置仍缺失：
src中withBotTurnAdmission生产调用者仍为 0；只有 gate 自身两处。shutdown 的 mutation lease 仍无法 drain 已进入的异步 handler，commit 后 continuation refork 逃逸窗口未变。 - #597 authoritative writer 前置仍缺失：我在
b6bc26ff编译产物上重跑双进程探针，结果仍为：
{
"persistResult": { "ok": true },
"afterPersist": "task-child",
"afterStaleWrite": "task-parent",
"commitResult": true,
"afterCommit": "task-parent",
"workerCleared": true
}即旁路 worker 的陈旧全量 save 仍能在 batch fresh-read 成功后回滚 lineage，而 commit 仍退休 worker。
3. #599 supervisor 前置仍缺失：cli.ts 仍是 PM2 kill_timeout: 3500，restart deadline 仍是 Date.now() + 5_000，与 28s daemon budget 不匹配。
PR 当前 mergeable=CONFLICTING，需要处理 base drift；但应在依赖顺序确定后再 rebase，避免重复解冲突与无效验证。
建议顺序不变：#596（已合）→ #597（或抽最小 admission + authoritative writer 前置）→ rebase/revalidate #598 → #599 → live。 未经申晗确认不合码。
2 participants
Summary
Extract the RIFF lifecycle shutdown work from the original mixed PR into a dedicated PR.
This PR:
Stack
#596 has merged. GitHub reports this PR cleanly mergeable with the current
master, including the gate review fix.The PM2 fleet protocol remains in the separate stacked PR #599.
Validation
Latest
mastermerge-state validation:tsc --noEmit)git diff --checkThe isolated merge worktree initially lacked
dist/codex-app-runner.js; after generating the normal TypeScript build output, all real-tmux integration coverage passed. This was a test-worktree setup issue, not a product regression.No RIFF worker, daemon, or PM2 process was restarted during validation.