fix(coordinator): 关闭 cancel race 两处窗口期 (Codex HIGH #1 #2)#79
Merged
Conversation
closes Codex audit blockers on PR #78 ## HIGH #1 — begin_session 内 await 期间 cancel 被覆盖 背景：volcengine open_session().await + Recorder::start 都是异步，期间用户按 Esc 调用 cancel_session 把 phase 改回 Idle 并 cancelled=true。但原代码后续无 条件 `state.phase = SessionPhase::Listening`，把 Idle 又翻回 Listening → 用户的 cancel 被吞掉，session 仍然继续。 修： - 新增 `cancel_raced_during_starting(inner)` helper：持锁查 cancelled or phase != Starting - ASR open_session().await 后调用：如已 race，asr.cancel() + 回 Idle - Recorder::start Ok 分支用 BeginOutcome { Started / PendingStop / CancelRaced } 原子在同一 lock 内决定，CancelRaced 时清理 recorder + asr 资源不进 Listening ## HIGH #2 — cancelled check 与 inserter.insert 之间的窗口 背景：end_session 检查 cancelled 后到调用 inserter.insert 之间释放了 lock， 此时 Esc 触发 cancel_session 设 cancelled=true 已经晚了 — Cmd+V 即将发出， 撤销不掉。但 cancel_session 仍然 emit "已取消" → UI 与实际行为相反（已插入但 显示已取消）。 修： - 新增 SessionPhase::Inserting：表示「已过最后一次 cancel 检查、即将/正在 调用 inserter.insert」的窗口 - end_session：把 polish 后的 cancel check + phase 转换 atomic 在同一 lock 内：cancelled → Idle + return；否则 → phase=Inserting 后 release lock 走 insert - cancel_session：phase==Inserting → 直接 return（不设 cancelled、不 emit 取 消）。理由：物理上无法撤销 Cmd+V，硬装"已取消"只会让 UI 撒谎 cargo check 通过，13 个 warnings 全部是 pre-existing。
Reviewer's GuideThis PR hardens the coordinator’s session lifecycle against two cancel-vs-start/insert race conditions by adding an explicit Inserting phase, a BeginOutcome enum plus a cancel_raced_during_starting helper, and by making key phase transitions and cancel checks happen atomically under the state lock with proper rollback of ASR/recorder resources when a race is detected. Sequence diagram for begin_session with cancel race handlingsequenceDiagram
actor User
participant Coordinator
participant Inner
participant InnerState
participant ASR
participant Recorder
User->>Coordinator: begin_session(inner)
Coordinator->>InnerState: lock state
InnerState-->>Coordinator: phase = Starting
Coordinator-->>InnerState: unlock state
Coordinator->>ASR: open_session()
par
ASR-->>Coordinator: open_session completes
and
User->>Coordinator: cancel_session(inner)
Coordinator->>InnerState: lock state
InnerState-->>Coordinator: phase, cancelled
Coordinator->>InnerState: set cancelled = true or phase = Idle
Coordinator-->>InnerState: unlock state
end
Coordinator->>Coordinator: cancel_raced_during_starting(inner)
Coordinator->>InnerState: lock state
InnerState-->>Coordinator: cancelled, phase
alt cancel_raced_during_starting is true
Coordinator-->>InnerState: unlock state
Coordinator->>ASR: cancel()
Coordinator->>InnerState: lock state
InnerState->>InnerState: phase = Idle
Coordinator-->>InnerState: unlock state
Coordinator-->>User: Ok(())
else cancel_raced_during_starting is false
Coordinator-->>InnerState: unlock state
Coordinator->>Recorder: start(consumer, level_handler)
Recorder-->>Coordinator: Ok(rec)
Coordinator->>InnerState: lock state
alt state.cancelled or phase != Starting
Coordinator->>InnerState: determine BeginOutcome = CancelRaced
Coordinator-->>InnerState: unlock state
Coordinator->>Recorder: rec.stop()
Coordinator->>Inner: lock asr
alt ActiveAsr::Volcengine
Coordinator->>ASR: cancel()
else ActiveAsr::Whisper
Coordinator->>ASR: cancel()
end
Coordinator->>InnerState: lock state
InnerState->>InnerState: phase = Idle
Coordinator-->>InnerState: unlock state
Coordinator-->>User: Ok(())
else pending_stop is true
Coordinator->>InnerState: phase = Listening
Coordinator->>InnerState: pending_stop = false
Coordinator->>InnerState: determine BeginOutcome = PendingStop
Coordinator-->>InnerState: unlock state
Coordinator->>Inner: lock recorder
Inner->>Inner: recorder = Some(rec)
Coordinator-->>Inner: unlock recorder
Coordinator->>Coordinator: end_session(inner)
Coordinator-->>User: result of end_session
else no pending_stop
Coordinator->>InnerState: phase = Listening
Coordinator->>InnerState: pending_stop = false
Coordinator->>InnerState: determine BeginOutcome = Started
Coordinator-->>InnerState: unlock state
Coordinator->>Inner: lock recorder
Inner->>Inner: recorder = Some(rec)
Coordinator-->>Inner: unlock recorder
Coordinator-->>User: Ok(())
end
end
Class diagram for updated session lifecycle types and helpersclassDiagram
class SessionPhase {
<<enum>>
Idle
Starting
Listening
Processing
Inserting
}
class BeginOutcome {
<<enum>>
Started
PendingStop
CancelRaced
}
class InnerState {
SessionPhase phase
bool cancelled
bool pending_stop
}
class Inner {
Mutex~InnerState~ state
Mutex~Option~ recorder
Mutex~Option~ asr
}
class ActiveAsr {
<<enum>>
Volcengine
Whisper
}
class Coordinator {
+async begin_session(inner : Arc~Inner~) Result~(), String~
+async end_session(inner : Arc~Inner~) Result~(), String~
+cancel_session(inner : Arc~Inner~) void
+cancel_raced_during_starting(inner : Arc~Inner~) bool
}
Inner o-- InnerState : has
Inner o-- ActiveAsr : holds
Coordinator --> Inner : operates_on
Coordinator --> BeginOutcome : uses
InnerState --> SessionPhase : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
begin_session, the CancelRaced path afteropen_sessioncallsasr.cancel()but does not clearinner.asr, whereas the CancelRaced branch afterRecorder::startusesinner.asr.lock().take(); consider unifying the cleanup so you don’t leave a stale ASR handle around in one of the early-abort cases. - In
cancel_session, you readphasewithout a lock and then later lock only to mutatecancelled; since you’ve introduced a newInsertingphase that’s checked via that unlocked read, it would be safer and simpler to perform the phase check andcancelledmutation within a singlestatelock to avoid TOCTOU issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `begin_session`, the CancelRaced path after `open_session` calls `asr.cancel()` but does not clear `inner.asr`, whereas the CancelRaced branch after `Recorder::start` uses `inner.asr.lock().take()`; consider unifying the cleanup so you don’t leave a stale ASR handle around in one of the early-abort cases.
- In `cancel_session`, you read `phase` without a lock and then later lock only to mutate `cancelled`; since you’ve introduced a new `Inserting` phase that’s checked via that unlocked read, it would be safer and simpler to perform the phase check and `cancelled` mutation within a single `state` lock to avoid TOCTOU issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
appergb
pushed a commit
that referenced
this pull request
Apr 30, 2026
包含本轮所有合并： - Codex 终审两条 HIGH (cancel race) 修复 (PR #79) - 6 个 Cooper-X-Oak/Codex bot PRs 自动合并 (#44 #49 #53 #68 #72 #73) - 2 个有冲突 PR 本地 rebase 后合并 (#66 cancel + 空转写并存 / #67 Windows docs) - README 破图修复 (PR #80) - workflow-scope 受限的 #48 + #75 由用户在 GitHub UI 直接合并 3 处版本字段同步：package.json + tauri.conf.json + Cargo.toml
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.
修两条 audit HIGH
HIGH #1 — begin_session await 期间 cancel 被覆盖
修：新增
cancel_raced_during_startinghelper +BeginOutcome三态枚举（Started/PendingStop/CancelRaced），ASR open + Recorder start 后原子检查，CancelRaced 时清理资源并回 Idle。HIGH #2 — cancelled check 与 inserter.insert 不原子
修：新增
SessionPhase::Inserting+ 在同一 lock 内做 check + 转换。cancel_session在Inserting拒绝介入（Cmd+V 已发出，撤销不掉）。Test
Summary by Sourcery
Handle cancel/stop races in session lifecycle to keep UI state and actual insertion behavior consistent.
Bug Fixes:
Enhancements: