From a6b35523628747aa353d4d80804106f783f08115 Mon Sep 17 00:00:00 2001 From: kuku Date: Thu, 15 Oct 2026 09:58:16 +0800 Subject: [PATCH] fix(models): preserve Codex tool_choice in requests (#6898) * fix(models): preserve Codex tool_choice in requests Signed-off-by: kuku <186267668+xbzz1018@users.noreply.github.com> * fix(models): validate Codex tool selection before HTTP Signed-off-by: kuku <186267668+xbzz1018@users.noreply.github.com> --------- Signed-off-by: kuku <186267668+xbzz1018@users.noreply.github.com> Co-authored-by: kuku <186267668+xbzz1018@users.noreply.github.com> --- README.md | 1 + README_zh.md | 1 + .../harness/deerflow/models/AGENTS.md | 10 + .../deerflow/models/openai_codex_provider.py | 46 ++++- backend/tests/test_codex_tool_choice.py | 195 ++++++++++++++++++ 5 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 backend/tests/test_codex_tool_choice.py diff --git a/README.md b/README.md index df07b3f02f..223aac59ee 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,7 @@ streams simulate chunks from a non-streaming response, while no-tool streams sta - Codex CLI reads `~/.codex/auth.json` - Codex function tools preserve explicit `strict: true` or `strict: false` in wrapped or flat dictionary definitions. Missing or null settings keep the provider default. `bind_tools` applies the same conversion to dictionaries and `BaseTool` schemas. + - Codex honors `tool_choice` in `bind_tools` and direct calls: `auto`, `none`, `required`, or a function name/selection. `any` and `True` mean `required`; `None` and `False` keep the provider default. Per-call choices override the bound policy. Bound choices are validated when binding; direct calls and overrides are validated before HTTP. `required` needs at least one function tool, and named selections must match a supplied function. `auto` and `none` also work without tools. - Completed Codex responses still return their text and tool calls when token usage is null, omitted, or empty; usage metadata remains unavailable. - The Codex model provider returns completed responses without waiting for the SSE connection to close. Failed or incomplete responses report the provider's error or reason; partial output is not returned as a successful answer. Non-object error details are reported as text. - Claude Code accepts `CLAUDE_CODE_OAUTH_TOKEN`, `ANTHROPIC_AUTH_TOKEN`, `CLAUDE_CODE_CREDENTIALS_PATH`, or `~/.claude/.credentials.json` diff --git a/README_zh.md b/README_zh.md index 8b8abd95da..3ebd837a43 100644 --- a/README_zh.md +++ b/README_zh.md @@ -228,6 +228,7 @@ YAML 同名模型优先，已有任务保留原配置快照。托管模型使用 ``` - Codex CLI 会读取 `~/.codex/auth.json` + - Codex 的 `bind_tools` 和直接调用支持 `tool_choice`：`auto`、`none`、`required` 或指定函数名称/对象。`any` 和 `True` 表示 `required`；`None` 和 `False` 保留服务端默认策略。单次调用可覆盖绑定策略。绑定时校验绑定策略；直接调用及覆盖参数在 HTTP 请求前校验。`required` 至少需要一个函数工具，指定函数必须存在于工具列表中。无工具时仍支持 `auto` 和 `none`。 - Claude Code 支持 `CLAUDE_CODE_OAUTH_TOKEN`、`ANTHROPIC_AUTH_TOKEN`、`CLAUDE_CODE_CREDENTIALS_PATH`，或 `~/.claude/.credentials.json` - ACP agent 条目与 model provider 是分开配置的——如果你配置了 `acp_agents.codex`，请把它指向一个 Codex ACP 适配器，例如 `npx -y @zed-industries/codex-acp` - MiniMax Code 原生支持 ACP，不需要额外适配器。先安装并登录，再把它配置成 ACP agent： diff --git a/backend/packages/harness/deerflow/models/AGENTS.md b/backend/packages/harness/deerflow/models/AGENTS.md index cf57efcf39..731a6e53f5 100644 --- a/backend/packages/harness/deerflow/models/AGENTS.md +++ b/backend/packages/harness/deerflow/models/AGENTS.md @@ -109,6 +109,16 @@ share `_convert_tools` so binding cannot discard the setting. Keep caller schemas and definitions unchanged. Offline sync/async request and tool-followup coverage: `tests/test_codex_tool_strict.py`. +Codex forwards `tool_choice` from `bind_tools` and direct sync/async calls. +Normalize `any`/`True` to `required`, named or wrapped function selections to +Responses' flat `{type: function, name: ...}` shape, and omit `None`/`False`. +Validate bound selections at `bind_tools`; validate invocation overrides again +before HTTP. `required` needs at least one converted function tool, and a named +selection must match a converted function. `auto` and `none` remain valid without +tools. Per-call choices override bound choices without mutating caller definitions +or history. Offline HTTP serialization and tool-followup coverage: +`tests/test_codex_tool_choice.py`. + ### Codex SSE termination (`packages/harness/deerflow/models/openai_codex_provider.py`) Completed responses with null, omitted, or empty `usage` retain their text, diff --git a/backend/packages/harness/deerflow/models/openai_codex_provider.py b/backend/packages/harness/deerflow/models/openai_codex_provider.py index c3471b160f..cac7dfbe2a 100644 --- a/backend/packages/harness/deerflow/models/openai_codex_provider.py +++ b/backend/packages/harness/deerflow/models/openai_codex_provider.py @@ -34,6 +34,31 @@ def _is_valid_call_id(call_id: Any) -> bool: return isinstance(call_id, str) and bool(call_id.strip()) +def _convert_tool_choice(tool_choice: str | dict | bool | None) -> str | dict | None: + """Normalize LangChain function selection to the Responses API format.""" + if tool_choice is None or tool_choice is False: + return None + if tool_choice is True or tool_choice == "any": + return "required" + if isinstance(tool_choice, str) and tool_choice: + if tool_choice in ("auto", "none", "required"): + return tool_choice + return {"type": "function", "name": tool_choice} + if isinstance(tool_choice, dict) and tool_choice.get("type") == "function": + function = tool_choice.get("function", tool_choice) + if isinstance(function, dict) and isinstance(function.get("name"), str) and function["name"]: + return {"type": "function", "name": function["name"]} + raise ValueError("tool_choice must be auto, none, required, any, a function name/selection, a boolean, or None") + + +def _validate_tool_choice(tool_choice: str | dict | None, tools: list[dict]) -> None: + """Check forced choices against the function tools that will be sent.""" + if tool_choice == "required" and not tools: + raise ValueError("tool_choice='required' needs at least one function tool") + if isinstance(tool_choice, dict) and not any(tool["name"] == tool_choice["name"] for tool in tools): + raise ValueError("tool_choice must select a function present in tools") + + def _build_usage_metadata(oai_usage: dict) -> dict: """Convert Codex/Responses API usage dict to LangChain usage_metadata format. @@ -234,7 +259,7 @@ class CodexChatModel(BaseChatModel): responses_tools.append(converted) return responses_tools - def _call_codex_api(self, messages: list[BaseMessage], tools: list[dict] | None = None) -> dict: + def _call_codex_api(self, messages: list[BaseMessage], tools: list[dict] | None = None, tool_choice: str | dict | bool | None = None) -> dict: """Call the Codex Responses API and return the completed response.""" instructions, input_items = self._convert_messages(messages) @@ -247,8 +272,13 @@ class CodexChatModel(BaseChatModel): "reasoning": {"effort": self.reasoning_effort, "summary": "detailed"} if self.reasoning_effort != "none" else {"effort": "none"}, } - if tools: - payload["tools"] = self._convert_tools(tools) + converted_tools = self._convert_tools(tools or []) + converted_choice = _convert_tool_choice(tool_choice) + _validate_tool_choice(converted_choice, converted_tools) + if converted_tools: + payload["tools"] = converted_tools + if converted_choice is not None: + payload["tool_choice"] = converted_choice headers = { "Authorization": f"Bearer {self._access_token}", @@ -465,15 +495,16 @@ class CodexChatModel(BaseChatModel): ) -> ChatResult: """Generate a response using Codex Responses API.""" tools = kwargs.get("tools", None) - response = self._call_codex_api(messages, tools=tools) + response = self._call_codex_api(messages, tools=tools, tool_choice=kwargs.get("tool_choice")) return self._parse_response(response) - def bind_tools(self, tools: list, **kwargs: Any) -> Any: - """Bind tools for function calling.""" + def bind_tools(self, tools: list, *, tool_choice: str | dict | bool | None = None, **kwargs: Any) -> Any: + """Bind function tools and their selection policy.""" from langchain_core.runnables import RunnableBinding from langchain_core.tools import BaseTool from langchain_core.utils.function_calling import convert_to_openai_function + converted_choice = _convert_tool_choice(tool_choice) formatted_tools = [] for tool in tools: if isinstance(tool, BaseTool): @@ -495,4 +526,5 @@ class CodexChatModel(BaseChatModel): else: formatted_tools.append(tool) - return RunnableBinding(bound=self, kwargs={"tools": formatted_tools}, **kwargs) + _validate_tool_choice(converted_choice, self._convert_tools(formatted_tools)) + return RunnableBinding(bound=self, kwargs={"tools": formatted_tools, "tool_choice": converted_choice}, **kwargs) diff --git a/backend/tests/test_codex_tool_choice.py b/backend/tests/test_codex_tool_choice.py new file mode 100644 index 0000000000..1b3477aa90 --- /dev/null +++ b/backend/tests/test_codex_tool_choice.py @@ -0,0 +1,195 @@ +"""Preserve tool selection through public Codex calls and real HTTP serialization.""" + +from __future__ import annotations + +import asyncio +import copy +import json + +import httpx +import pytest +from langchain_core.messages import HumanMessage, ToolMessage + +from deerflow.models import openai_codex_provider as provider +from deerflow.models.credential_loader import CodexCliCredential + + +@pytest.mark.parametrize( + "tool_choice,expected", + [ + (None, None), + (False, None), + (True, "required"), + ("auto", "auto"), + ("none", "none"), + ("required", "required"), + ("any", "required"), + ("read_report", {"type": "function", "name": "read_report"}), + ({"type": "function", "name": "read_report"}, {"type": "function", "name": "read_report"}), + ({"type": "function", "function": {"name": "read_report"}}, {"type": "function", "name": "read_report"}), + ], +) +@pytest.mark.parametrize("binding", ["direct", "bound"]) +@pytest.mark.parametrize("invocation", ["invoke", "ainvoke"]) +def test_tool_choice_survives_request_and_tool_followup(monkeypatch, tool_choice, expected, binding, invocation): + definition = {"type": "function", "function": {"name": "read_report", "description": "Read a prepared report.", "parameters": {"type": "object", "properties": {}}}} + original_definition = copy.deepcopy(definition) + original_choice = copy.deepcopy(tool_choice) + monkeypatch.setattr(provider, "load_codex_cli_credential", lambda: CodexCliCredential("fake-token", "fake-account")) + model = provider.CodexChatModel(retry_max_attempts=1) + runnable = model.bind_tools([definition], tool_choice=tool_choice) if binding == "bound" else model + kwargs = {} if binding == "bound" else {"tools": [definition], "tool_choice": tool_choice} + requests = [] + clients = [] + responses = [] + client_class = httpx.Client + + def handle(request): + payload = json.loads(request.content) + requests.append(payload) + if len(requests) == 1 and payload.get("tool_choice") != "none": + output = [{"type": "function_call", "name": "read_report", "arguments": "{}", "call_id": "call-report"}] + else: + output = [{"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "Report ready."}]}] + event = {"type": "response.completed", "response": {"id": "response-report", "status": "completed", "output": output, "usage": None}} + response = httpx.Response(200, headers={"content-type": "text/event-stream"}, content=f"data: {json.dumps(event)}\n\n".encode()) + responses.append(response) + return response + + def make_client(**options): + client = client_class(transport=httpx.MockTransport(handle), **options) + clients.append(client) + return client + + monkeypatch.setattr(provider.httpx, "Client", make_client) + history = [HumanMessage(content="Read the quarterly report.")] + + if invocation == "invoke": + first = runnable.invoke(history, **kwargs) + followup = [*history, first, ToolMessage(content="Report contents", tool_call_id="call-report")] if first.tool_calls else None + answer = runnable.invoke(followup, **{**kwargs, "tool_choice": "none"}) if followup else first + else: + + async def exchange(): + first = await runnable.ainvoke(history, **kwargs) + followup = [*history, first, ToolMessage(content="Report contents", tool_call_id="call-report")] if first.tool_calls else None + answer = await runnable.ainvoke(followup, **{**kwargs, "tool_choice": "none"}) if followup else first + return first, followup, answer + + first, followup, answer = asyncio.run(exchange()) + + if expected is None: + assert "tool_choice" not in requests[0] + else: + assert requests[0].get("tool_choice") == expected + assert answer.content == "Report ready." + if expected == "none": + assert not first.tool_calls + assert len(requests) == 1 + else: + assert first.tool_calls == [{"name": "read_report", "args": {}, "id": "call-report", "type": "tool_call"}] + assert len(requests) == 2 + assert requests[1]["tool_choice"] == "none" + assert requests[1]["input"][-2:] == [ + {"type": "function_call", "name": "read_report", "arguments": "{}", "call_id": "call-report"}, + {"type": "function_call_output", "call_id": "call-report", "output": "Report contents"}, + ] + assert definition == original_definition + assert tool_choice == original_choice + assert history == [HumanMessage(content="Read the quarterly report.")] + assert all(client.is_closed for client in clients) + assert all(response.is_closed for response in responses) + + +@pytest.mark.parametrize("tool_choice", [123, [], "", {"type": "file_search"}, {"type": "function"}, {"type": "function", "function": "read_report"}]) +def test_invalid_tool_choice_fails_before_http(monkeypatch, tool_choice): + monkeypatch.setattr(provider, "load_codex_cli_credential", lambda: CodexCliCredential("fake-token", "fake-account")) + model = provider.CodexChatModel(retry_max_attempts=1) + definition = {"name": "read_report", "parameters": {"type": "object", "properties": {}}} + + def unexpected_http(**options): + pytest.fail("Invalid tool_choice must fail before creating an HTTP client") + + monkeypatch.setattr(provider.httpx, "Client", unexpected_http) + with pytest.raises(ValueError, match="tool_choice"): + model.invoke([HumanMessage(content="Read the report.")], tools=[definition], tool_choice=tool_choice) + + +@pytest.mark.parametrize("tool_choice", [123, [], "", {"type": "file_search"}, {"type": "function"}, {"type": "function", "function": "read_report"}]) +def test_invalid_bound_choice_fails_at_bind_time(monkeypatch, tool_choice): + monkeypatch.setattr(provider, "load_codex_cli_credential", lambda: CodexCliCredential("fake-token", "fake-account")) + model = provider.CodexChatModel(retry_max_attempts=1) + with pytest.raises(ValueError, match="tool_choice"): + model.bind_tools([{"name": "read_report"}], tool_choice=tool_choice) + + +@pytest.mark.parametrize( + "tools,tool_choice", + [ + (None, "required"), + ([], "any"), + ([], "read_report"), + ([{"type": "web_search"}], True), + ([{"type": "web_search"}], {"type": "function", "function": {"name": "read_report"}}), + ([{"name": "other_function"}], "read_report"), + ([{"name": "other_function"}], {"type": "function", "name": "read_report"}), + ], +) +@pytest.mark.parametrize("invocation", ["invoke", "ainvoke", "bind_tools"]) +def test_forced_selection_needs_available_function(monkeypatch, tools, tool_choice, invocation): + monkeypatch.setattr(provider, "load_codex_cli_credential", lambda: CodexCliCredential("fake-token", "fake-account")) + model = provider.CodexChatModel(retry_max_attempts=1) + + def unexpected_http(**options): + pytest.fail("Invalid selection must be rejected before creating an HTTP client") + + monkeypatch.setattr(provider.httpx, "Client", unexpected_http) + with pytest.raises(ValueError, match="tool_choice"): + if invocation == "bind_tools": + model.bind_tools(tools or [], tool_choice=tool_choice) + elif invocation == "ainvoke": + asyncio.run(model.ainvoke([HumanMessage(content="Read the report.")], tools=tools, tool_choice=tool_choice)) + else: + model.invoke([HumanMessage(content="Read the report.")], tools=tools, tool_choice=tool_choice) + + +@pytest.mark.parametrize("override", [{"tools": []}, {"tool_choice": "missing_function"}, {"tool_choice": {"type": "function"}}]) +def test_bound_call_revalidates_invocation_overrides(monkeypatch, override): + monkeypatch.setattr(provider, "load_codex_cli_credential", lambda: CodexCliCredential("fake-token", "fake-account")) + model = provider.CodexChatModel(retry_max_attempts=1) + runnable = model.bind_tools([{"name": "read_report"}], tool_choice="required") + + def unexpected_http(**options): + pytest.fail("Invocation overrides must be validated before creating an HTTP client") + + monkeypatch.setattr(provider.httpx, "Client", unexpected_http) + with pytest.raises(ValueError, match="tool_choice"): + runnable.invoke([HumanMessage(content="Read the report.")], **override) + + +@pytest.mark.parametrize( + "tools,tool_choice,invocation", + [(None, None, "invoke"), ([], False, "ainvoke"), ([], "auto", "bound_invoke"), ([], "none", "bound_ainvoke"), ([{"type": "web_search"}], "none", "invoke"), ([{"type": "web_search"}], "auto", "bound_invoke")], +) +def test_no_function_tools_preserves_nonforcing_choices(monkeypatch, tools, tool_choice, invocation): + monkeypatch.setattr(provider, "load_codex_cli_credential", lambda: CodexCliCredential("fake-token", "fake-account")) + model = provider.CodexChatModel(retry_max_attempts=1) + payloads = [] + client_class = httpx.Client + + def handle(request): + payloads.append(json.loads(request.content)) + event = {"type": "response.completed", "response": {"output": [{"type": "message", "content": [{"type": "output_text", "text": "OK"}]}]}} + return httpx.Response(200, headers={"content-type": "text/event-stream"}, content=f"data: {json.dumps(event)}\n\n".encode()) + + monkeypatch.setattr(provider.httpx, "Client", lambda **options: client_class(transport=httpx.MockTransport(handle), **options)) + runnable = model.bind_tools(tools or [], tool_choice=tool_choice) if invocation.startswith("bound_") else model + kwargs = {} if invocation.startswith("bound_") else {"tools": tools, "tool_choice": tool_choice} + answer = asyncio.run(runnable.ainvoke([HumanMessage(content="Say OK.")], **kwargs)) if invocation.endswith("ainvoke") else runnable.invoke([HumanMessage(content="Say OK.")], **kwargs) + assert answer.content == "OK" + assert len(payloads) == 1 + assert not payloads[0].get("tools") + if tool_choice in ("auto", "none"): + assert payloads[0]["tool_choice"] == tool_choice + else: + assert "tool_choice" not in payloads[0]