Merge branch 'main' of github.com:openai/codex into dev/mzeng/tool_suggest_fix

codex-rs/core/src/tools/handlers/tool_suggest.rs

@@ -27,6 +27,8 @@ use crate::tools::registry::ToolKind;
 
 pub struct ToolSuggestHandler;
 
+const INVALID_SUGGEST_REASON_MESSAGE: &str = "suggest_reason must be a specific one-line user-facing reason for the current request; do not use placeholders like \"placeholder\".";
+
 impl ToolHandler for ToolSuggestHandler {
     type Output = FunctionToolOutput;
 
@@ -58,10 +60,8 @@ impl ToolHandler for ToolSuggestHandler {
 
         let args: ToolSuggestArgs = parse_arguments(&arguments)?;
         let suggest_reason = args.suggest_reason.trim();
-        if suggest_reason.is_empty() {
-            return Err(FunctionCallError::RespondToModel(
-                "suggest_reason must not be empty".to_string(),
-            ));
+        if let Some(message) = invalid_suggest_reason_message(suggest_reason) {
+            return Err(FunctionCallError::RespondToModel(message.to_string()));
         }
         if args.action_type != DiscoverableToolAction::Install {
             return Err(FunctionCallError::RespondToModel(
@@ -158,6 +158,57 @@ impl ToolHandler for ToolSuggestHandler {
     }
 }
 
+fn invalid_suggest_reason_message(suggest_reason: &str) -> Option<&'static str> {
+    if suggest_reason.trim().is_empty() {
+        return Some("suggest_reason must not be empty");
+    }
+
+    let normalized = normalize_suggest_reason_for_stub_check(suggest_reason);
+    if normalized.ends_with("goes here")
+        || matches!(
+            normalized.as_str(),
+            "placeholder"
+                | "placeholder text"
+                | "todo"
+                | "tbd"
+                | "n/a"
+                | "na"
+                | "none"
+                | "null"
+                | "nil"
+                | "reason"
+                | "suggest reason"
+                | "suggestion reason"
+                | "tool suggestion reason"
+                | "concise one-line user-facing reason"
+                | "concise one-line user-facing reason why this tool can help with the current request"
+                | "specific one-line user-facing reason"
+                | "specific one-line user-facing reason why this tool can help with the current request"
+        )
+    {
+        return Some(INVALID_SUGGEST_REASON_MESSAGE);
+    }
+
+    None
+}
+
+fn normalize_suggest_reason_for_stub_check(suggest_reason: &str) -> String {
+    const WRAPPER_CHARS: &[char] = &[
+        '"', '\'', '`', '<', '>', '[', ']', '(', ')', '{', '}', '.', ',', ':', ';', '-', '_', '*',
+        ' ',
+    ];
+
+    suggest_reason
+        .trim()
+        .trim_matches(WRAPPER_CHARS)
+        .split_whitespace()
+        .map(|word| word.trim_matches(WRAPPER_CHARS))
+        .filter(|word| !word.is_empty())
+        .collect::<Vec<_>>()
+        .join(" ")
+        .to_ascii_lowercase()
+}
+
 async fn verify_tool_suggestion_completed(
     session: &crate::session::session::Session,
     turn: &crate::session::turn_context::TurnContext,

codex-rs/core/src/tools/handlers/tool_suggest_tests.rs

@@ -8,6 +8,46 @@ use crate::plugins::test_support::write_plugins_feature_config;
 use codex_utils_absolute_path::AbsolutePathBuf;
 use tempfile::tempdir;
 
+#[test]
+fn invalid_suggest_reason_message_rejects_placeholder_reasons() {
+    for reason in [
+        "",
+        "   ",
+        "placeholder",
+        " Placeholder ",
+        "<placeholder>",
+        "[TODO]",
+        "TBD",
+        "n/a",
+        "reason",
+        "suggestion reason",
+        "reason goes here",
+        "Concise one-line user-facing reason why this tool can help with the current request",
+        "Specific one-line user-facing reason why this tool can help with the current request",
+    ] {
+        assert!(
+            invalid_suggest_reason_message(reason).is_some(),
+            "expected {reason:?} to be rejected"
+        );
+    }
+}
+
+#[test]
+fn invalid_suggest_reason_message_allows_specific_reasons() {
+    for reason in [
+        "Search Slack messages related to the release plan.",
+        "Find and reference issues in GitHub for this bug report.",
+        "Use Google Calendar to compare attendee availability.",
+        "Use Figma to inspect placeholder text in the selected design.",
+    ] {
+        assert_eq!(
+            invalid_suggest_reason_message(reason),
+            None,
+            "expected {reason:?} to be allowed"
+        );
+    }
+}
+
 #[tokio::test]
 async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
     let codex_home = tempdir().expect("tempdir should succeed");

codex-rs/core/templates/search_tool/tool_suggest_description.md

@@ -6,6 +6,11 @@ Use this ONLY when:
 - You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.
 - For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.
 - For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.
+- The `suggest_reason` must be a specific one-line user-facing reason for the current request. Do not use placeholders like `placeholder`.
+
+There are two types of allowed suggestions:
+1. Suggest a plugin needed in the context that explicitly and unambiguously fits the user intent but is not installed, tool_type = "plugin", action_type = "install"
+2. Suggest a connector needed in the context but not installed (even when its plugin is installed), tool_type = "connector", action_type = "install"
 
 Tool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.
 
@@ -14,13 +19,13 @@ Discoverable tools:
 
 Workflow:
 
-1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.
-2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.
+1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent. If tool search is available, tool search should happen before tool suggestion.
+2. If no available or searchable tool is found, match the user's intent against the discoverable tools list above. Decide if any of the discoverable tools match the user intent explicitly and unambiguously. Suggest a tool only when it qualifies all the conditions.
 3. If one tool clearly fits, call `tool_suggest` with:
    - `tool_type`: `connector` or `plugin`
    - `action_type`: `install` or `enable`
    - `tool_id`: exact id from the discoverable tools list above
-   - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request
+   - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request, must not be empty and must not be a placeholder
 4. After the suggestion flow completes:
    - if the user finished the install or enable flow, continue by searching again or using the newly available tool
    - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks you to.

codex-rs/tools/src/tool_discovery.rs

@@ -272,11 +272,6 @@ pub fn collect_tool_search_source_infos<'a>(
 }
 
 pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> ToolSpec {
-    let discoverable_tool_ids = discoverable_tools
-        .iter()
-        .map(|tool| tool.id.as_str())
-        .collect::<Vec<_>>()
-        .join(", ");
     let properties = BTreeMap::from([
         (
             "tool_type".to_string(),
@@ -293,14 +288,14 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool
         ),
         (
             "tool_id".to_string(),
-            JsonSchema::string(Some(format!(
-                "Connector or plugin id to suggest. Must be one of: {discoverable_tool_ids}."
-            ))),
+            JsonSchema::string(Some(
+                "Connector or plugin id to suggest. Must be one of the discoverable tool ids.".to_string(),
+            )),
         ),
         (
             "suggest_reason".to_string(),
             JsonSchema::string(Some(
-                "Concise one-line user-facing reason why this tool can help with the current request."
+                "Concise one-line user-facing reason why this tool can help with the current request, must not be empty and must not be a placeholder."
                     .to_string(),
             )),
         ),
@@ -308,7 +303,7 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool
 
     let discoverable_tools = format_discoverable_tools(discoverable_tools);
     let description = format!(
-        "# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `{TOOL_SEARCH_TOOL_NAME}` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n{discoverable_tools}\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.\n2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.\n3. If one tool clearly fits, call `{TOOL_SUGGEST_TOOL_NAME}` with:\n   - `tool_type`: `connector` or `plugin`\n   - `action_type`: `install` or `enable`\n   - `tool_id`: exact id from the discoverable tools list above\n   - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request\n4. After the suggestion flow completes:\n   - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n   - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it."
+        "# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `{TOOL_SEARCH_TOOL_NAME}` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n- The `suggest_reason` must be a specific one-line user-facing reason for the current request. Do not use placeholders like `placeholder`.\n\nThere are two types of allowed suggestions:\n1. Suggest a plugin needed in the context that explicitly and unambiguously fits the user intent but is not installed, tool_type = \"plugin\", action_type = \"install\"\n2. Suggest a connector needed in the context but not installed (even when its plugin is installed), tool_type = \"connector\", action_type = \"install\"\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n{discoverable_tools}\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent. If tool search is available, tool search should happen before tool suggestion.\n2. If no available or searchable tool is found, match the user's intent against the discoverable tools list above. Decide if any of the discoverable tools match the user intent explicitly and unambiguously. Suggest a tool only when it qualifies all the conditions.\n3. If one tool clearly fits, call `{TOOL_SUGGEST_TOOL_NAME}` with:\n   - `tool_type`: `connector` or `plugin`\n   - `action_type`: `install` or `enable`\n   - `tool_id`: exact id from the discoverable tools list above\n   - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request, must not be empty and must not be a placeholder\n4. After the suggestion flow completes:\n   - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n   - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it."
     );
 
     ToolSpec::Function(ResponsesApiTool {

codex-rs/tools/src/tool_discovery_tests.rs

@@ -50,69 +50,100 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_sources() {
 
 #[test]
 fn create_tool_suggest_tool_uses_plugin_summary_fallback() {
+    let tool = create_tool_suggest_tool(&[
+        ToolSuggestEntry {
+            id: "slack@openai-curated".to_string(),
+            name: "Slack".to_string(),
+            description: None,
+            tool_type: DiscoverableToolType::Connector,
+            has_skills: false,
+            mcp_server_names: Vec::new(),
+            app_connector_ids: Vec::new(),
+        },
+        ToolSuggestEntry {
+            id: "github".to_string(),
+            name: "GitHub".to_string(),
+            description: None,
+            tool_type: DiscoverableToolType::Plugin,
+            has_skills: true,
+            mcp_server_names: vec!["github-mcp".to_string()],
+            app_connector_ids: vec!["github-app".to_string()],
+        },
+    ]);
+    let ToolSpec::Function(ResponsesApiTool {
+        name,
+        description,
+        strict,
+        defer_loading,
+        parameters,
+        output_schema,
+    }) = tool
+    else {
+        panic!("expected function tool");
+    };
+
+    assert_eq!(name, "tool_suggest");
+    assert!(!strict);
+    assert_eq!(defer_loading, None);
+    assert_eq!(output_schema, None);
+    assert!(
+        description.contains(
+            "You've already tried to find a matching available tool for the user's request"
+        )
+    );
+    assert!(description.contains("This includes `tool_search` (if available) and other means."));
+    assert!(description.contains("There are two types of allowed suggestions:"));
+    assert!(description.contains("tool_type = \"plugin\", action_type = \"install\""));
+    assert!(description.contains("tool_type = \"connector\", action_type = \"install\""));
+    assert!(
+        description.contains("- GitHub (id: `github`, type: plugin, action: install): skills; MCP servers: github-mcp; app connectors: github-app")
+    );
+    assert!(
+        description.contains("- Slack (id: `slack@openai-curated`, type: connector, action: install): No description provided.")
+    );
+    assert!(description.contains("placeholders like `placeholder`"));
+
     assert_eq!(
-        create_tool_suggest_tool(&[
-            ToolSuggestEntry {
-                id: "slack@openai-curated".to_string(),
-                name: "Slack".to_string(),
-                description: None,
-                tool_type: DiscoverableToolType::Connector,
-                has_skills: false,
-                mcp_server_names: Vec::new(),
-                app_connector_ids: Vec::new(),
-            },
-            ToolSuggestEntry {
-                id: "github".to_string(),
-                name: "GitHub".to_string(),
-                description: None,
-                tool_type: DiscoverableToolType::Plugin,
-                has_skills: true,
-                mcp_server_names: vec!["github-mcp".to_string()],
-                app_connector_ids: vec!["github-app".to_string()],
-            },
-        ]),
-        ToolSpec::Function(ResponsesApiTool {
-            name: "tool_suggest".to_string(),
-            description: "# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n- GitHub (id: `github`, type: plugin, action: install): skills; MCP servers: github-mcp; app connectors: github-app\n- Slack (id: `slack@openai-curated`, type: connector, action: install): No description provided.\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.\n2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.\n3. If one tool clearly fits, call `tool_suggest` with:\n   - `tool_type`: `connector` or `plugin`\n   - `action_type`: `install` or `enable`\n   - `tool_id`: exact id from the discoverable tools list above\n   - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request\n4. After the suggestion flow completes:\n   - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n   - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it.".to_string(),
-            strict: false,
-            defer_loading: None,
-            parameters: JsonSchema::object(BTreeMap::from([
-                    (
-                        "action_type".to_string(),
-                        JsonSchema::string(Some(
-                                "Suggested action for the tool. Use \"install\" or \"enable\"."
-                                    .to_string(),
-                            ),),
-                    ),
-                    (
-                        "suggest_reason".to_string(),
-                        JsonSchema::string(Some(
-                                "Concise one-line user-facing reason why this tool can help with the current request."
-                                    .to_string(),
-                            ),),
-                    ),
-                    (
-                        "tool_id".to_string(),
-                        JsonSchema::string(Some(
-                                "Connector or plugin id to suggest. Must be one of: slack@openai-curated, github."
-                                    .to_string(),
-                            ),),
-                    ),
-                    (
-                        "tool_type".to_string(),
-                        JsonSchema::string(Some(
-                                "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"."
-                                    .to_string(),
-                            ),),
-                    ),
-                ]), Some(vec![
-                    "tool_type".to_string(),
+        parameters,
+        JsonSchema::object(
+            BTreeMap::from([
+                (
                     "action_type".to_string(),
-                    "tool_id".to_string(),
+                    JsonSchema::string(Some(
+                        "Suggested action for the tool. Use \"install\" or \"enable\"."
+                            .to_string(),
+                    )),
+                ),
+                (
                     "suggest_reason".to_string(),
-                ]), Some(false.into())),
-            output_schema: None,
-        })
+                    JsonSchema::string(Some(
+                        "Concise one-line user-facing reason why this tool can help with the current request, must not be empty and must not be a placeholder."
+                            .to_string(),
+                    )),
+                ),
+                (
+                    "tool_id".to_string(),
+                    JsonSchema::string(Some(
+                        "Connector or plugin id to suggest. Must be one of the discoverable tool ids."
+                            .to_string(),
+                    )),
+                ),
+                (
+                    "tool_type".to_string(),
+                    JsonSchema::string(Some(
+                        "Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"."
+                            .to_string(),
+                    )),
+                ),
+            ]),
+            Some(vec![
+                "tool_type".to_string(),
+                "action_type".to_string(),
+                "tool_id".to_string(),
+                "suggest_reason".to_string(),
+            ]),
+            Some(false.into()),
+        )
     );
 }
 

codex-rs/tools/src/tool_registry_plan_tests.rs

@@ -1575,7 +1575,7 @@ fn tool_suggest_can_be_registered_without_search_tool() {
         "Suggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin"
     ));
     assert!(description.contains(
-        "You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means."
+        "If tool search is available, tool search should happen before tool suggestion."
     ));
 }
 
@@ -1646,27 +1646,22 @@ fn tool_suggest_description_lists_discoverable_tools() {
     assert!(description.contains("Plan events and schedules."));
     assert!(description.contains("Find and summarize email threads."));
     assert!(description.contains("id: `sample@test`, type: plugin, action: install"));
-    assert!(description.contains("`action_type`: `install` or `enable`"));
+    assert!(description.contains("tool_type = \"plugin\", action_type = \"install\""));
+    assert!(description.contains("tool_type = \"connector\", action_type = \"install\""));
     assert!(
         description.contains("skills; MCP servers: sample-docs; app connectors: connector_sample")
     );
-    assert!(
-        description.contains(
-            "You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means."
-        )
-    );
     assert!(description.contains(
-        "For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely."
+        "If tool search is available, tool search should happen before tool suggestion."
+    ));
+    assert!(description.contains("There are two types of allowed suggestions:"));
+    assert!(description.contains(
+        "Suggest a plugin needed in the context that explicitly and unambiguously fits the user intent but is not installed"
     ));
     assert!(description.contains(
-        "For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself."
-    ));
-    assert!(description.contains(
-        "Do not suggest a plugin just because one of its connectors or capabilities seems relevant."
-    ));
-    assert!(description.contains(
-        "Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard."
+        "Suggest a connector needed in the context but not installed (even when its plugin is installed)"
     ));
+    assert!(description.contains("Suggest a tool only when it qualifies all the conditions."));
     assert!(description.contains("DO NOT explore or recommend tools that are not on this list."));
     assert!(!description.contains("{{discoverable_tools}}"));
     assert!(!description.contains("tool_search fails to find a good match"));