mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Improve Default mode prompt (less confusion with Plan mode) (#10545)
## Summary
This PR updates `request_user_input` behavior and Default-mode guidance
to match current collaboration-mode semantics and reduce model
confusion.
## Why
- `request_user_input` should be explicitly documented as **Plan-only**.
- Tool description and runtime availability checks should be driven by
the **same centralized mode policy**.
- Default mode prompt needed stronger execution guidance and explicit
instruction that `request_user_input` is unavailable.
- Error messages should report the **actual mode name** (not aliases
that can read as misleading).
## What changed
- Centralized `request_user_input` mode policy in `core` handler logic:
- Added a single allowed-modes config (`Plan` only).
- Reused that policy for:
- runtime rejection messaging
- tool description text
- Updated tool description to include availability constraint:
- `"This tool is only available in Plan mode."`
- Updated runtime rejection behavior:
- `Default` -> `"request_user_input is unavailable in Default mode"`
- `Execute` -> `"request_user_input is unavailable in Execute mode"`
- `PairProgramming` -> `"request_user_input is unavailable in Pair
Programming mode"`
- Strengthened Default collaboration prompt:
- Added explicit execution-first behavior
- Added assumptions-first guidance
- Added explicit `request_user_input` unavailability instruction
- Added concise progress-reporting expectations
- Simplified formatting implementation:
- Inlined allowed-mode name collection into `format_allowed_modes()`
- Kept `format_allowed_modes()` output for 3+ modes as CSV style
(`modes: a,b,c`)
This commit is contained in:
committed by
GitHub
Unverified
parent
d9ad5c3c49
commit
998eb8f32b
@@ -27,6 +27,7 @@ pub use mcp_resource::McpResourceHandler;
|
||||
pub use plan::PlanHandler;
|
||||
pub use read_file::ReadFileHandler;
|
||||
pub use request_user_input::RequestUserInputHandler;
|
||||
pub(crate) use request_user_input::request_user_input_tool_description;
|
||||
pub use shell::ShellCommandHandler;
|
||||
pub use shell::ShellHandler;
|
||||
pub use test_sync::TestSyncHandler;
|
||||
|
||||
@@ -10,6 +10,56 @@ use crate::tools::registry::ToolKind;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||
|
||||
const REQUEST_USER_INPUT_ALLOWED_MODES: [ModeKind; 1] = [ModeKind::Plan];
|
||||
|
||||
fn request_user_input_mode_name(mode: ModeKind) -> &'static str {
|
||||
match mode {
|
||||
ModeKind::Plan => "Plan",
|
||||
ModeKind::Default => "Default",
|
||||
ModeKind::Execute => "Execute",
|
||||
ModeKind::PairProgramming => "Pair Programming",
|
||||
}
|
||||
}
|
||||
|
||||
fn format_allowed_modes() -> String {
|
||||
let mut mode_names = Vec::with_capacity(REQUEST_USER_INPUT_ALLOWED_MODES.len());
|
||||
for mode in REQUEST_USER_INPUT_ALLOWED_MODES {
|
||||
let name = request_user_input_mode_name(mode);
|
||||
if !mode_names.contains(&name) {
|
||||
mode_names.push(name);
|
||||
}
|
||||
}
|
||||
|
||||
match mode_names.as_slice() {
|
||||
[] => "no modes".to_string(),
|
||||
[mode] => format!("{mode} mode"),
|
||||
[first, second] => format!("{first} or {second} mode"),
|
||||
[..] => format!("modes: {}", mode_names.join(",")),
|
||||
}
|
||||
}
|
||||
|
||||
fn request_user_input_is_available_in_mode(mode: ModeKind) -> bool {
|
||||
REQUEST_USER_INPUT_ALLOWED_MODES.contains(&mode)
|
||||
}
|
||||
|
||||
pub(crate) fn request_user_input_unavailable_message(mode: ModeKind) -> Option<String> {
|
||||
if request_user_input_is_available_in_mode(mode) {
|
||||
None
|
||||
} else {
|
||||
let mode_name = request_user_input_mode_name(mode);
|
||||
Some(format!(
|
||||
"request_user_input is unavailable in {mode_name} mode"
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn request_user_input_tool_description() -> String {
|
||||
let allowed_modes = format_allowed_modes();
|
||||
format!(
|
||||
"Request user input for one to three short questions and wait for the response. This tool is only available in {allowed_modes}."
|
||||
)
|
||||
}
|
||||
|
||||
pub struct RequestUserInputHandler;
|
||||
|
||||
#[async_trait]
|
||||
@@ -37,14 +87,8 @@ impl ToolHandler for RequestUserInputHandler {
|
||||
};
|
||||
|
||||
let mode = session.collaboration_mode().await.mode;
|
||||
if !matches!(mode, ModeKind::Plan | ModeKind::PairProgramming) {
|
||||
let mode_name = match mode {
|
||||
ModeKind::Default | ModeKind::Execute => "Default",
|
||||
ModeKind::Plan | ModeKind::PairProgramming => unreachable!(),
|
||||
};
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"request_user_input is unavailable in {mode_name} mode"
|
||||
)));
|
||||
if let Some(message) = request_user_input_unavailable_message(mode) {
|
||||
return Err(FunctionCallError::RespondToModel(message));
|
||||
}
|
||||
|
||||
let mut args: RequestUserInputArgs = parse_arguments(&arguments)?;
|
||||
@@ -82,3 +126,54 @@ impl ToolHandler for RequestUserInputHandler {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn request_user_input_mode_availability_is_plan_only() {
|
||||
assert_eq!(
|
||||
request_user_input_is_available_in_mode(ModeKind::Plan),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
request_user_input_is_available_in_mode(ModeKind::Default),
|
||||
false
|
||||
);
|
||||
assert_eq!(
|
||||
request_user_input_is_available_in_mode(ModeKind::Execute),
|
||||
false
|
||||
);
|
||||
assert_eq!(
|
||||
request_user_input_is_available_in_mode(ModeKind::PairProgramming),
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_user_input_unavailable_messages_use_default_name_for_default_modes() {
|
||||
assert_eq!(request_user_input_unavailable_message(ModeKind::Plan), None);
|
||||
assert_eq!(
|
||||
request_user_input_unavailable_message(ModeKind::Default),
|
||||
Some("request_user_input is unavailable in Default mode".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
request_user_input_unavailable_message(ModeKind::Execute),
|
||||
Some("request_user_input is unavailable in Execute mode".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
request_user_input_unavailable_message(ModeKind::PairProgramming),
|
||||
Some("request_user_input is unavailable in Pair Programming mode".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_user_input_tool_description_mentions_plan_only() {
|
||||
assert_eq!(
|
||||
request_user_input_tool_description(),
|
||||
"Request user input for one to three short questions and wait for the response. This tool is only available in Plan mode.".to_string()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::tools::handlers::apply_patch::create_apply_patch_json_tool;
|
||||
use crate::tools::handlers::collab::DEFAULT_WAIT_TIMEOUT_MS;
|
||||
use crate::tools::handlers::collab::MAX_WAIT_TIMEOUT_MS;
|
||||
use crate::tools::handlers::collab::MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::tools::handlers::request_user_input_tool_description;
|
||||
use crate::tools::registry::ToolRegistryBuilder;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
@@ -623,9 +624,7 @@ fn create_request_user_input_tool() -> ToolSpec {
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "request_user_input".to_string(),
|
||||
description:
|
||||
"Request user input for one to three short questions and wait for the response."
|
||||
.to_string(),
|
||||
description: request_user_input_tool_description(),
|
||||
strict: false,
|
||||
parameters: JsonSchema::Object {
|
||||
properties,
|
||||
|
||||
Reference in New Issue
Block a user