Skip to content

fix(codex): accept a steer only when the turn consumes it - #344

Merged
Tryanks merged 3 commits into
mainfrom
fix/codex-steer-acceptance
Sep 6, 2026
Merged

Tryanks merged 3 commits into
mainfrom
fix/codex-steer-acceptance

Conversation

@Tryanks

@Tryanks Tryanks commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Behaviour

Codex steers showed Steered the moment `turn/steer` was sent. That reply only confirms the app-server enqueued the input; the running turn consumes it later, before its next model request. Until then the message is still Steering… in reality.

Codex core (`session/turn.rs` → `record_pending_input` → `record_user_prompt_and_emit_turn_item`) echoes the drained input back as a `userMessage` item, so that echo is the real consumption signal. The Codex client now:

  • keeps a successfully acknowledged steer as pending `(request_id, text)` instead of emitting `SteerAccepted` on the RPC reply;
  • emits `SteerAccepted` when a parent-thread `userMessage` item echo's text matches a pending steer (oldest match first, so a late echo of the turn's own prompt is not mistaken for the steer);
  • still refuses acceptance on an RPC error, even if matching text is later echoed.

The `SteerAccepted` contract doc in `agent/src/lib.rs` now describes the Codex signal alongside Claude's.

Tests

`steer_acceptance_waits_for_the_consumed_user_message_echo` replaces the RPC-reply test and protects: no acceptance on the `turn/steer` reply, no acceptance on a non-matching `userMessage` echo, exactly one acceptance across the started/completed echo pair, and no acceptance after an RPC error.

Checks run

  • `cargo fmt --all --check` ✅
  • `cargo clippy --workspace --all-targets --locked -- -D warnings` ✅
  • `cargo test --workspace --locked` ✅
  • Live probe against codex-cli 0.153.4: `cargo run -p agent --example probe -- codex "Run sleep 30, then reply DONE" /tmp/smoke --steer "Stop and reply PONG"` → `SteerAccepted` observed before `TurnCompleted`, marker PONG present, TurnStarted=1 TurnCompleted=1.
  • Not run locally: Windows/Linux, iOS/Android/Web and `cargo machete` (CI).

…run with

Subagent items now carry optional `model` and `effort`, merged like
`summary` so later lifecycle snapshots never erase them.

Claude: the Agent tool's `model` alias is recorded at spawn; the resolved
model and effort are read from the child's assistant records, both when
they stream over stdout with `parent_tool_use_id` and when a background
task's transcript is tailed (the tail hands the values back to the actor
so the re-emitted spawn item keeps its live status).

Codex: `spawn_agent`'s `model` / `reasoning_effort` overrides are used
when present, otherwise the child inherits the parent thread's settings.

The subagent row renders a `model · effort` chip next to the agent type.
The turn/steer reply only means the app-server enqueued the input, so the
bubble flipped from Steering… to Steered the instant the request was sent.
Codex drains pending input before its next model request and echoes it as
a userMessage item; use that echo (matched by text against the acknowledged
steers) as the acceptance signal instead.
@Tryanks
Tryanks enabled auto-merge (squash) September 6, 2026 17:13
@Tryanks
Tryanks merged commit 14f283c into main Sep 6, 2026
5 checks passed
@Tryanks
Tryanks deleted the fix/codex-steer-acceptance branch September 6, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant