fix(codex): steer active turns for /ps guidance - #1665
Open
AaronZ345 wants to merge 6 commits into
Open
Conversation
chenhg5
approved these changes
Aug 13, 2026
chenhg5
left a comment
Owner
There was a problem hiding this comment.
结论: Approve
总体判断: 一个干净的 protocol-level fix——把 /ps guidance 从「启动新 turn」改成「steer 现有 turn」,复用 Codex 0.144.1 的 turn/steer JSON-RPC。架构变化很合理,race fix 正确,测试覆盖三个独立分支。建议合入。
Review 范围:
- 看了
agent/codex/appserver_session.go中 Send 的 refactor(activeTurn 捕获 + preamble 守卫修改 + 新增 steerTurn 函数)。 - 看了
turnSteerResponse类型 +steerTurn的 turnId 校验逻辑。 - 看了新增 3 个测试:
SendSteersActiveTurn/SendStartsIdleTurn/SendRejectsMismatchedSteeringTurn。 - 看了 hermetic test fixes(launchd_test.go / gateway_test.go / poll_test.go / cuj_test.go / engine_test.go)—— 全部为环境隔离(
t.Setenv("HOME", tempDir)等),无逻辑改动。 - CI: run 31399427489 全绿(lint 1m57s / unit-test 4m25s / smoke 26s / regression 27s / performance 57s)。
✅ 做得好的地方:
- Race 修得精准:原 bug 是
Send在turn/start调用时把currentTurn重置 + buffered messages 清空,导致 race 下Send后异步 turn 完整跑完但 cc-connect 已经丢弃了 state,结果是「/ps触发了,但响应空」。修复后 steer 不重置 turn state,自然消除 race。 - Preamble 守卫修正顺手做了:
if activeTurn == "" && !s.preambleSent同时修了「/ps重复 prepend preamble」的问题——preamble 本意是「首次 turn 提示 AI 工作模式」,重复 prepend 既浪费 token 也干扰 steering 语义。这条 fix 不在 race 范围内但与 steer 行为耦合(preamble 重复会让/ps看起来「没生效」),所以作者一起做是合理的。 - TurnID 校验两段式:steer response 必须 turnId 非空 + 必须等于 expectedTurnID。这条防御了 Codex 未来 schema 变化时「返回错误 turnId 导致错误引导」的 silent failure。
- Hermetic test fixes:4 个 test 文件的环境隔离改动(
t.Setenv("HOME", ...)等)让本地go test ./...全过不再依赖 launchd / real env。这是「顺手修一个无关 race」——做法很对,因为这次改 codex 不应该让本地其他包的测试跟着 flaky。 - Supersedes #1530:作者明确标注了取代 #1530,避免 reviewer 困惑「为什么不用之前的尝试」。
🚨/🔴 必须处理:
- 未发现。
🟠 建议改进(不阻塞):
- Hermetic test fixes 的独立性:
launchd_test.go/gateway_test.go/poll_test.go/cuj_test.go的改动其实是独立的 flaky-test 修复,但被绑在一个 codex PR 里。建议 merge 后另开一个 follow-up PR 把这些 fix 单独 cherry-pick 到 main(让 main 上的go test ./...立即稳定,不必等 codex PR 合并)。低优先。 steerTurn的 input 格式假设:当前实现把input []map[string]any直接序列化(与 startTurn 保持一致)。如果 Codex 0.144+ 的 steer 与 start 对 input schema 略有不同(例如 steer 不接受某些字段),会导致运行时错。建议作者加一个_ = schema注释或测试覆盖 input schema 兼容性。currentTurn状态管理:steer 后currentTurn是否更新?看 diff 没动。如果 Codex 在 steer 后会发出新的 turnId(同一 thread 多次 steer),cc-connect 应该跟新——否则后续/ps仍 steer 旧 turn。建议 author 加一个测试覆盖「连续两次 /ps」的 currentTurn 更新行为。- Pre-merge 真实环境验证:Codex 0.144.1 升级到更新版本时
turn/steerschema 可能变化。建议作者在 post-merge 留意 Codex release notes。
🔵 可选优化:
docs/superpowers/plans/2026-07-11-codex-ps-turn-steer.md177 行 implementation plan——这种 plan 文件建议放仓库外(个人 PR-author 笔记),但仓库内也 OK,作为「为什么这样设计」的历史记录有价值。保留。expectedTurnIDmismatch 时返回的具体错误信息codex app-server turn/steer returned turn id %q, want %q——对 debug 友好。
Testing / Risk:
- 已看到的验证: 3 个新测试覆盖三个分支(active / idle / mismatched);CI 全绿;
go test -race通过。 - 未覆盖风险: 真实 Codex 0.144.1 跑多次
/ps时 turnId 行为是否一致(作者没贴 raw wire log)。建议 post-merge 用真实 Codex 0.144.1 跑 10 次连续/ps看 steer 是否都成功。 - Blast radius: 仅
agent/codex/appserver_session.go,不影响其他 agent。
Next step:
- 建议 owner 直接 merge。Scope 小、架构对、测试全,race 修得精准。
- post-merge 验证: 用飞书连一个 Codex 0.144.1+ session,发 5+ 次连续
/ps,确认每次 steer 都生效、buffered text 不丢失、最终响应完整。
24 tasks
AaronZ345
force-pushed
the
agent/fix-codex-ps-steer-20260810
branch
3 times, most recently
from
August 15, 2026 14:44
c21000b to
482b53b
Compare
AaronZ345
force-pushed
the
agent/fix-codex-ps-steer-20260810
branch
from
August 16, 2026 14:44
482b53b to
17b0075
Compare
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
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.
Summary
turn/steerinstead of starting a second turn/psguidanceturn/startbehavior for idle sessions and reject mismatched steering responsesWhy
/pscallsAgentSession.Sendwhile the current turn is still running. The Codex app-server implementation previously treated everySendasturn/start, reset the active-turn state, and cleared buffered messages. That race could emit an empty result even though Codex later completed a full answer.Codex exposes
turn/steerfor same-turn guidance. This change uses it with the activeexpectedTurnId, matching Codex's native guidance behavior without turning the guidance into a separate user turn.Testing
go test ./...go test -race ./agent/codex ./core ./platform/cloud-webgo vet ./...go build ./...turn/steerrequest and response fieldsSupersedes #1530.