Implemented thread-level atomic elicitation counter for stopwatch pausing#12296
Implemented thread-level atomic elicitation counter for stopwatch pausing#12296
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54d4da7f29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b5279d1 to
675128b
Compare
e2aa79b to
5d6a86f
Compare
…sing
While trying to build out CLI-Tools for the agent to use under skills we
have found that those tools sometimes need to invoke a user elicitation.
These elicitations are handled out of band of the codex app-server but
need to indicate to the exec manager that the command running is
not going to progress on the usual timeout horizon.
Model calls universal exec:
`$ download-credit-card-history --start-date 2026-01-19 --end-date 2026-02-19 > credit_history.jsonl`
download-cred-card-history might hit a hosted/preauthenticated service to fetch data. That service might decide that the request requires an end user approval the access to the personal data. It should be able to signal to the running thread that the command in question is blocked on user elicitation. In that case we want the exec to continue, but the timeout to not expire on the tool call, essentially freezing time until the user approves or rejects the command at which point the tool would signal the app-server to decrement the outstanding elicitation count. Now timeouts would proceed as normal.
- New v2 RPC methods:
- thread/increment_elicitation
- thread/decrement_elicitation
- Protocol updates in:
- codex-rs/app-server-protocol/src/protocol/common.rs
- codex-rs/app-server-protocol/src/protocol/v2.rs
- App-server handlers wired in:
- codex-rs/app-server/src/codex_message_processor.rs
- Counter starts at 0 per thread.
- increment atomically increases the counter.
- decrement atomically decreases the counter; decrement at 0 returns invalid request.
- Transition rules:
- 0 -> 1: broadcast pause state, pausing all active stopwatches immediately.
- >0 -> >0: remain paused.
- 1 -> 0: broadcast unpause state, resuming stopwatches.
- Core thread/session logic:
- codex-rs/core/src/codex_thread.rs
- codex-rs/core/src/codex.rs
- codex-rs/core/src/mcp_connection_manager.rs
- Added centralized stopwatch tracking/controller:
- codex-rs/exec-server/src/posix/stopwatch_controller.rs
- Hooked pause/unpause broadcast handling + stopwatch registration:
- codex-rs/exec-server/src/posix/mcp.rs
- codex-rs/exec-server/src/posix/stopwatch.rs
- codex-rs/exec-server/src/posix.rs
- Add a session-local out-of-band elicitation pause signal in codex-rs/core, so core runtimes can observe the same paused
state that was previously only forwarded to MCP servers.
- Update CodexThread to set and clear that local pause state when the out-of-band elicitation count crosses zero, with
rollback if the MCP notification fails.
- Make unified exec subscribe to that session pause state for both the initial exec_command call and subsequent write_stdin
polls.
- Pause unified exec’s yield deadline while the thread is paused, then resume with the remaining budget once the pause lifts
instead of letting the 10s window expire in the background.
- Add a core regression test proving a paused unified exec call does not yield early and still returns the command output
after the pause clears.
Why
- The thread elicitation APIs are meant to block turn progress while an external binary is still holding the thread.
- Before this change, unified exec’s yield timer kept advancing even during an out-of-band elicitation pause, so exec_command
could return early and let the model continue reasoning before the underlying process finished.
90ed0c8 to
cf8c6be
Compare
|
@codex review |
celia-oai
left a comment
There was a problem hiding this comment.
chatted offline, this will be experimental & intended for internal only and we'll delete after proper solution here (maybe elicitation cli)
5e38ee3 to
cf8c6be
Compare
| params: v2::ThreadUnsubscribeParams, | ||
| response: v2::ThreadUnsubscribeResponse, | ||
| }, | ||
| #[experimental("thread/increment_elicitation")] |
There was a problem hiding this comment.
can we add a docstring here describe what these two endpoints do?
There was a problem hiding this comment.
& maybe make it clear that this only applies to unified exec as a workaround?
| Ok(()) | ||
| } | ||
|
|
||
| async fn notify_out_of_band_elicitation_state_change( |
There was a problem hiding this comment.
what's the next step for handling in this notification from the MCP side? Is there an MCP example that utilize this?
Purpose
While trying to build out CLI-Tools for the agent to use under skills we have found that those tools sometimes need to invoke a user elicitation. These elicitations are handled out of band of the codex app-server but need to indicate to the exec manager that the command running is not going to progress on the usual timeout horizon.
Example
Model calls universal exec:
$ download-credit-card-history --start-date 2026-01-19 --end-date 2026-02-19 > credit_history.jsonldownload-cred-card-history might hit a hosted/preauthenticated service to fetch data. That service might decide that the request requires an end user approval the access to the personal data. It should be able to signal to the running thread that the command in question is blocked on user elicitation. In that case we want the exec to continue, but the timeout to not expire on the tool call, essentially freezing time until the user approves or rejects the command at which point the tool would signal the app-server to decrement the outstanding elicitation count. Now timeouts would proceed as normal.
What's Added
Behavior
Exec-server stopwatch integration