refactor(core): 增加机器人级 turn admission/mutation gate - #596
Conversation
deepcoldy
left a comment
There was a problem hiding this comment.
首次 Review(Claude)— 结论:✅ 可合入(等 codex 复审 + 申晗确认后再合码)
按 CLAUDE.md 影响范围要求做了跨面评估,并跑了对抗式验证。
这个 PR 在做什么(白话)
它给每个机器人(按 larkAppId 隔离)加了一把「准入/排空/独占变更」闸门原语。背景:Codex App 生命周期修复、RIFF 安全关停都需要「普通消息 turn 可以并发进,但替换 worker generation / 关停这类 mutation 必须先挡住新 turn、等已进入的 turn 排空、再独占执行」,master 里没有可复用的原语,所以先把这块基础模块单独拆出来(本 PR 只加模块 + 测试,不接入任何运行时路径)。
四个对外 API:
withBotTurnAdmission— 一次普通 turn,可并发;有 mutation 持闸时阻塞;同 bot 嵌套调用是可重入的(靠 AsyncLocalStorage 认领的 lease)。withBotTurnMutation— 独占:关闸(mutating=true)→ 等所有在飞 admission 排空 → 执行 → 重新开闸。难点是 upgrade:mutation 若发现自己是在一个「已准入的 handler」里被调用(典型:消息/卡片 handler 里触发关停),就把外层这条 admission「释放 → 排空其它 turn → 独占变更 → 在开闸前原子地重新认领同一条 lease」,这样调用方在状态变更后还能安全地继续用原 admission 收尾(投递/日志)。tryWithBotTurnMutation— 带绝对截止时间的有界版本,保证 返回{acquired:false}之后 action 绝不会迟到执行(移除超时 waiter + 回滚已关的闸)。runDetached*— fire-and-forget 用;清空两个 ALS context,让浮起来的子任务去认领全新计数的 lease,而不是冒用祖先那条「已被吊销但仍挂在 ALS 里」的 lease。
为什么设计成这样(几个关键不变量)
- 两个 mutation 不可能同时进场 ——
while(state.mutating)检查与state.mutating=true之间没有 await,单线程下这条 check→set 是原子的,被wakeAll同批唤醒的多个 mutation 里只有第一个微任务能关闸,其余重新回到openWaiters。 - upgrade 的「先重认领、后开闸」次序是核心 ——
finally里顺序是lease.active=false→ (upgrading && !ownerFinished时)activeAdmissions++ / inheritedAdmission.active=true→mutating=false→wakeAll。因为重认领在开闸之前,被唤醒的排队 mutation 会看到activeAdmissions>0,正确地在drainWaiters上继续等 upgrade 那条 handler 的尾部工作排空。 ownerFinished守卫防「幽灵计数」 —— 如果 upgrade 的外层 admission 已经先返回了(fire-and-forget 浮起的 mutation),就不能再把计数加回去,否则activeAdmissions永远停在 1、之后所有 mutation 全部饿死。- 进程模型对齐:worker 是
fork()子进程,这把闸是 daemon 单进程内的 module-level 单例,只守 daemon 侧的 inbound/HTTP turn 准备阶段,按larkAppId分桶,不影响其它 bot,也不需要跨进程协调 —— 粒度正确。
我做的验证
pnpm install && pnpm build:✅ 通过pnpm exec vitest run test/bot-turn-mutation-gate.test.ts:✅ 16/16 通过- 影响面:
git diff master...HEAD仅 2 个新增文件(296 + 400 行),全仓无其它模块 import 该文件,master 上无同类原语 —— 纯新增、运行时行为不变,与 PR 描述一致。 - 对抗式可执行探针:对 6 个维度(upgrade 竞态 / 有界一致性 / ALS 冒用 / 计数不变量 / 唤醒公平性 / 截止时间边界)各写 4 条会失败于 bug 的 vitest 探针,独立跑真实仓库。5 个维度全部完成 = 20 条探针 20 PASS,0 真 bug。
- 其中
count-invariants维度做了变异测试:把!ownerFinished守卫删掉后,探针出现真实 30s 死锁(activeAdmissions被 stranded 在 1)—— 证明这条守卫在无界和有界两条路径上都是 load-bearing 的,不是死代码。 deadline-timer维度的 verifier 因真实定时器 sleep 过长在 6 次尝试里都 stall(harness 超时假象,非代码缺陷);且我最关心的「截止时间之后被唤醒的 drain waiter 必须拒绝、action 绝不执行」这条已在bounded-consistency维度独立覆盖并 PASS,PR 自带测试里也有两条 timer 用例。
- 其中
非阻塞观察(P3,可留 follow-up)
upgrade_conflict返回值 0 覆盖:现有 16 条测试里没有一条驱动「admission 内调用tryWithBotTurnMutation且已有别的 mutation 持闸」这条早退路径(探针证明它行为正确、admission 保持不变)。建议后续接入 PR 补一条回归测试锚定契约。- 模块目前无任何 runtime importer,属「基础先行、后续接入」拆分模式 —— 合理,但需确保后续 Codex App / RIFF 接入 PR 真正落地,避免长期空挂 bitrot。接入时才是真正验证 drain 语义对 live turn 流的地方(PtyBackend / TmuxBackend、话题会话 / 群会话 / adopt-restore 各组合)。
结论:代码质量高、注释准确描述了每个 hazard、对抗验证零真 bug,可合入。已 @ codex 复审。未获申晗确认前不合码。
deepcoldy
left a comment
There was a problem hiding this comment.
⚠️ 首审结论撤回:Codex 复审发现阻塞性自锁,我确认属实
Codex 在复审中发现一个阻塞性并发自锁,我已在 head dcf37d4 独立复现,撤回此前的「可合入」结论。
独立复现(我跑的探针)
在一个 admission 内用 Promise.all 并发起两个 withBotTurnMutation(均被父 action 结构化 await,非 fire-and-forget):
- 无界:
DEADLOCK-timeout, events=["m1"]—— 永久挂死,只有 m1 执行 - 该 bot 永久锁死:之后任何 admission 都
blocked - 有界对照:
r2={acquired:false,reason:'timeout'},m2 从不执行 - 顺序调用对照组正常:
m1,m2,outer-done
根因(与 Codex 一致,bot-turn-mutation-gate.ts:143-154,168-173)
两个 mutation 共享同一条 admission lease:
- m1 把共享 lease 的
active改为false并关闸。 - 同步发起的 m2 看到
active=false,把upgrading永久快照为 false,随后等第一把闸。 - m1 完成时先把外层 admission 重认领为
activeAdmissions=1,再唤醒 m2。 - m2 醒来关闸并等
activeAdmissions===0,但这个计数只能由正在await Promise.all的外层释放 → 环形等待,mutating=true卡死,该 bot 后续全挡。
为什么我的首审漏了
我的 upgrade-races 对抗探针测的是「两个各自独立 admission 的 upgrader」(各持自己的 lease,能正确串行化),而这个 bug 需要两个 mutation 共享同一条 lease——upgrading 只在入口快照 active 是不够的。这是我探针设计的真实盲区,教训记下。
下一步
正在按 Codex 建议实现修复:让同一 admission lease 识别/串行化 in-flight upgrade(用 lease 上的 in-flight 计数,而非只在入口快照 active),并补回归测试(同 lease 并发无界 mutation、无界+bounded 混合)。修复验证通过后再单独更新 + @codex 复审。仍然不合码。
|
To use Codex here, create a Codex account and connect to github. |
修复计划(按 Codex 验收标准实现中)根因已确认: 修复思路:在 lease 上串行化 in-flight upgrade给
按 Codex 验收标准补的回归
实现 + 自测(含 Codex 死锁探针翻绿)通过后带新 commit SHA 更新。 |
deepcoldy
left a comment
There was a problem hiding this comment.
重新 Review:作者修复提交 78a5e1d4 —— ✅ 死锁已修复,实测通过
针对 Codex 复审发现的并发自锁,作者推了 78a5e1d4c fix(core): serialize same-admission mutation upgrades。我基于 PR 当前 head 重新完整 review + 独立对抗验证,结论:修复正确、测试充分。
修复机制(读懂了)
AdmissionLease 新增 pendingUpgrades / upgradeLocked / upgradeWaiters,把「是否 upgrade」从「入口一次性快照 active」改为按 lease 串行排队:
shouldQueueAdmissionUpgrade的关键:active || pendingUpgrades > 0才排队 —— 兄弟 mutation 即使看到被先行者临时释放的active=false,只要pendingUpgrades>0仍会正确进队列,不再误判成独立 top-level mutation 去等自己那个挂起的外层 admission。acquireAdmissionUpgrade/releaseAdmissionUpgrade:直接把 lock 所有权移交下一个排队者(不清upgradeLocked),保证同 lease 的 action 串行不重叠。- bounded 版
acquireAdmissionUpgradeBefore:deadline-aware 排队,超时精确摘除自身 waiter;handoff 与 timer 竞争时按Date.now() < deadlineMs判acquired/expired_owner,过期只释放槽、action 绝不迟到执行。
我的实测验证
pnpm build✅;PR 测试 23/23 ✅(作者新增 5 条同-lease 用例)- Codex 的原始死锁探针:
outcome=completed, events=["m1","m2","outer-done"]—— 不再挂死 ✅ - 作者回归测试是 discriminating 的:把源码回退到旧 head、只跑作者的
serializes concurrent mutations that share one admission lease,30s 超时死锁;换回修复源码则通过 —— 证明测试真能抓 bug、修复真能解 ✅ - 我另写 9 条独立对抗探针全 PASS,覆盖 Codex 六条验收标准:
- A:独立 top-level mutation 与在飞同-lease upgrade 竞争 —— 我最担心会引入新死锁的场景,实测
["m1","indep","m2","outer-done"]正常交错,无死锁 - B/末:3~5 个同-lease 并发 mutation 全 exactly-once、
maxConcurrent===1严格不重叠 - C:bounded 兄弟排在挂死的 unbounded 兄弟后,按真实 30ms deadline 超时
{acquired:false,reason:'timeout'},b-ran从不执行(不因等自己外层 lease 而必然超时,也不迟到) - D/E:第一个 / 第二个 action 抛错 → 队列继续推进、
activeAdmissions/mutating完整恢复 - F:fire-and-forget 兄弟 upgrade、owner 先返回 → 仍执行、无幽灵计数
- G:跨 bot 隔离,两 bot 各自并发 mutation 不互相串行化
- H/I:混合 bounded+unbounded 及后续 admission/mutation 均可正常进入
- A:独立 top-level mutation 与在飞同-lease upgrade 竞争 —— 我最担心会引入新死锁的场景,实测
- 作者两条最微妙的新用例(
rejects an overdue same-lease queue handoff before its timer callback用 fake timer 造 handoff-vs-timer 竞争、does not restore a ghost admission when a queued upgrade outlives its owner)设计严谨,正好覆盖 deadline + fire-and-forget 边界。
范围与回归
- 仍只 2 个文件、无任何 runtime importer、build 全绿、本地树与 PR head
78a5e1d4完全一致(diff 为空)。纯新增基础模块,运行时行为不变。
结论:修复正确、验收标准逐条满足,可合入。 已 @ Codex 复审确认。
背景
Codex App 生命周期修复与 RIFF 安全关停都需要同一个机器人维度的并发边界:普通 turn 可以并发进入,替换 worker generation 或关停等 mutation 必须先阻止新 turn、等待已进入 turn 排空,再独占执行。现有 master 没有可复用的原语。
改动
影响范围
纯新增基础模块;当前运行时行为不变。按 larkAppId 隔离,不影响其它机器人。
验证
git diff --check:通过pnpm build:通过pnpm exec vitest run test/bot-turn-mutation-gate.test.ts:1 个文件、16 项测试全部通过未验证
本 PR 尚未接入 daemon/worker,因此无需 live daemon 或 PM2 验证;运行时接入将在后续独立 PR 中验证。