Keep request_user_input direct-model only - #27316
Conversation
There was a problem hiding this comment.
This gives extensions ability to disable or enable other tools per turn over time. Here we do two things:
- disable the tool for the turn.
- if the tool itself is still reachable for some reason, it can emit the unavailability reason to the model.
There was a problem hiding this comment.
Update - we are no long disabling the tool for the turn. We will rely on the fact that when the tool is not available we will emit an error.
| let mode = session.collaboration_mode().await.mode; | ||
| if let Some(message) = request_user_input_unavailable_message(mode, &self.available_modes) { | ||
| return Err(FunctionCallError::RespondToModel(message)); | ||
| } |
There was a problem hiding this comment.
There will be a separate opportunity for us to clean this up.
| if goal.status == codex_state::ThreadGoalStatus::Active | ||
| && input.collaboration_mode.mode == ModeKind::Default | ||
| { | ||
| input | ||
| .turn_store | ||
| .get_or_init(ToolAvailability::default) | ||
| .mark_unavailable( | ||
| ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME), | ||
| ACTIVE_GOAL_REQUEST_USER_INPUT_UNAVAILABLE_MESSAGE, | ||
| ); | ||
| } |
There was a problem hiding this comment.
On the turn level we store the tool unavailability status and the reason.
There was a problem hiding this comment.
The idea here is that the request user input tool will not be available if there is a goal objective that is currently in active status.
THE ONLY EXCEPTION (which the inline code comment also mentioned) is that if plan collaboration mode is also enabled then we will have request user input tool. BUT technically when plan mode is enabled we actually never reach this point. We early terminate at line 209.
| pub struct ToolAvailability { | ||
| unavailability_reason_by_tool_name: Mutex<HashMap<ToolName, String>>, | ||
| } |
There was a problem hiding this comment.
Maybe I should call this ToolUnavailability? cause we are storing tools that are not available but it just feels weird calling it that way.
53afbeb to
07c9996
Compare
| async fn request_user_input_unavailability_reason_for_turn( | ||
| session: &crate::session::session::Session, | ||
| turn: &crate::session::turn_context::TurnContext, | ||
| available_modes: &[ModeKind], | ||
| ) -> Option<String> { | ||
| if turn.session_source.is_non_root_agent() { | ||
| return Some("request_user_input can only be used by the root thread".to_string()); | ||
| } | ||
|
|
||
| if let Some(unavailability_reason) = | ||
| turn.extension_data | ||
| .get::<ToolUnavailability>() | ||
| .and_then(|unavailability| { | ||
| unavailability.unavailability_reason(&ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME)) | ||
| }) | ||
| { | ||
| return Some(unavailability_reason); | ||
| } | ||
|
|
||
| let mode = session.collaboration_mode().await.mode; | ||
| request_user_input_unavailable_message(mode, available_modes) | ||
| } |
There was a problem hiding this comment.
Better organize tool unavailability reasons.
07c9996 to
3f3c5f0
Compare
77a01d3 to
e1ca683
Compare
Why
request_user_inputhas direct blocking semantics when invoked by the model. When it is exposed as a nested code-mode tool, the call has to flow through code-mode waiting and continuation behavior instead, which is not the behavior we want for this user-input request surface.What changed
request_user_inputwithToolExposure::DirectModelOnlywhen registering the core utility tool.request_user_inputdirect-model visible, including in code-mode-only planning.spec_plan_testscoverage that verifiesrequest_user_inputremains visible and registered as direct-model-only, while it is omitted from the nested code-mode tool description.No active goal suppression or runtime unavailability behavior is included in this PR.
Validation
just test -p codex-core request_user_inputpassed.