Support steering active Codex turns from queued messages#472
Support steering active Codex turns from queued messages#472Liu-KM wants to merge 2 commits intotiann:mainfrom
Conversation
There was a problem hiding this comment.
Findings
- [Major] Messages queued before the turn id is known are not re-checked for steering, so they can fall through to the next turn. Evidence
cli/src/codex/codexRemoteLauncher.ts:261,cli/src/codex/codexRemoteLauncher.ts:771.
Suggested fix:if (turnId) { this.currentTurnId = turnId; void maybeSteerQueuedMessages(); }
- [Major]
await turnSettledcan hang indefinitely if the app-server exits afterturn/startsucceeds but before a terminal event arrives. Evidencecli/src/codex/codexRemoteLauncher.ts:762,cli/src/codex/codexRemoteLauncher.ts:776, contextcli/src/codex/codexAppServerClient.ts:99.
Suggested fix:appServerClient.setDisconnectHandler(() => { turnInFlight = false; activeTurnModeHash = null; this.currentTurnId = null; settleTurnInFlight(); });
Summary
Review mode: initial
The new steering path still misses one active-turn window and can deadlock the remote launcher on app-server disconnect. Residual testing gap: not found coverage for messages queued before turn_id becomes available or for disconnects after turn/start.
Testing
Not run (automation; static review only per prompt constraints)
HAPI Bot
| if (this.shouldExit || turnInFlight === false || steeringInFlight) { | ||
| return; | ||
| } | ||
| if (!this.currentThreadId || !this.currentTurnId || !activeTurnModeHash) { |
There was a problem hiding this comment.
[MAJOR] This early return means a same-mode message that lands after turnInFlight = true but before currentTurnId is populated will miss the steering window. setOnMessage() fires once, exits here, and nothing re-runs maybeSteerQueuedMessages() when startTurn() or turn/started later provides the turn id, so that message waits for a later turn instead of steering the active one.
Suggested fix:
if (turnId) {
this.currentTurnId = turnId;
void maybeSteerQueuedMessages();
}| } else if (!this.currentTurnId) { | ||
| allowAnonymousTerminalEvent = true; | ||
| } | ||
| await turnSettled; |
There was a problem hiding this comment.
[MAJOR] turnSettled is only resolved from terminal notifications. If the app-server exits after turn/start returns but before turn/completed, CodexAppServerClient only rejects pending RPCs (cli/src/codex/codexAppServerClient.ts:99), so this await never resolves and the session stays stuck in thinking while cleanup never runs.
Suggested fix:
appServerClient.setDisconnectHandler(() => {
turnInFlight = false;
activeTurnModeHash = null;
this.currentTurnId = null;
settleTurnInFlight();
});
Summary
Add support for steering an active Codex app-server turn with queued messages.
turn/steersupport to the Codex app-server clientturn/steerWhy
Codex can accept additional instructions while a task is running. HAPI previously queued those messages for a later turn, and could also start another turn before the active turn had emitted a terminal event, which could leave sessions stuck in a thinking state.
Validation
cd cli && bun x vitest run src/codex/codexRemoteLauncher.test.tsNotes
AI-assisted with GPT-5.4.