From 1612f06a4c19c53b6d8b02b02cd30d14a1858d28 Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Fri, 17 Jul 2026 03:08:09 +0530 Subject: [PATCH] fix(ccr): don't crash tool-call detection on a null function/functionCall (#2269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description CCR tool-call detection crashes when an upstream response carries a tool call whose `function` (or `functionCall`) field is explicitly `null`. `is_ccr_tool_call` and `parse_tool_call` both read the nested name like this: ```python tool_call.get("function", {}).get("name") tool_call.get("functionCall", {}).get("name") ``` `dict.get("function", {})` only substitutes `{}` when the key is **missing**. When the key is present but `null` — `{"id": "call_1", "type": "function", "function": null}`, which upstreams (and gateways like LiteLLM/OpenRouter) emit for a partial or streamed tool call — the result is `None`, and `None.get("name")` raises `AttributeError`. These functions run over the untrusted upstream response (`has_ccr_tool_calls` → `is_ccr_tool_call` for every tool call, and `parse_tool_call` on the retrieve path), so a single malformed tool call takes down CCR detection for the whole response. The sibling `tool_call_id_for_provider` in the same module already guards this shape (`if isinstance(function_call, dict)`); these two paths just weren't updated to match. ## Fix Coalesce with `or {}` so a `null` (or any falsy) value collapses to `{}`: ```python (tool_call.get("function") or {}).get("name") (tool_call.get("functionCall") or {}).get("name") ``` and in `parse_tool_call`: ```python function = tool_call.get("function") or {} function_call = tool_call.get("functionCall") or {} ``` A null tool call now reports "not a CCR call" and is passed through as a normal tool, and real CCR calls are still detected. Closes # ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/ccr/tool_calls.py`: `is_ccr_tool_call` coalesces `function` / `functionCall` with `or {}`. - `headroom/ccr/tool_injection.py`: `parse_tool_call` coalesces `function` (openai) and `functionCall` (google) with `or {}`. - `tests/test_ccr_tool_calls.py`, `tests/test_ccr_tool_injection.py`: new tests covering a null-function tool call in detection and parsing. - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 check headroom/ccr/tool_calls.py headroom/ccr/tool_injection.py tests/test_ccr_tool_calls.py tests/test_ccr_tool_injection.py All checks passed! $ uvx mypy@1.20.2 --ignore-missing-imports headroom/ccr/tool_calls.py headroom/ccr/tool_injection.py Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17` / `uvx mypy@1.20.2`. A full `pytest` OOM-kills this box (ML stack import), so I reproduced the detection logic with a dependency-free script and left the full pytest to CI. - Exact command / steps: ran an OpenAI tool call `{"function": null}` (plus a real CCR call) through the OLD `get("function", {})` form and the NEW `get("function") or {}` form. - Observed result: OLD raises `AttributeError` on the null function; NEW returns `False`/`None` for it and still detects the real CCR call and both `functionCall`/`name` shapes. - Not tested: a live upstream emitting a null-function tool call; full local `pytest` deferred to CI (OOM). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The "unit tests pass locally" box is unchecked because the full suite imports the ML stack, which I can't run here. The new tests live alongside the existing CCR tool-call tests so they run under the normal CI pytest job; behaviour is additionally verified by the standalone proof above. --------- Co-authored-by: Tejas Chopra --- headroom/ccr/tool_calls.py | 4 ++-- headroom/ccr/tool_injection.py | 8 ++++++-- tests/test_ccr_tool_calls.py | 30 ++++++++++++++++++++++++++++++ tests/test_ccr_tool_injection.py | 6 ++++++ 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/headroom/ccr/tool_calls.py b/headroom/ccr/tool_calls.py index 16fa18878..46e5feacb 100644 --- a/headroom/ccr/tool_calls.py +++ b/headroom/ccr/tool_calls.py @@ -73,8 +73,8 @@ def is_ccr_tool_call(tool_call: dict[str, Any]) -> bool: """Return true when a provider-native tool call names the CCR retrieval tool.""" return ( tool_call.get("name") == CCR_TOOL_NAME - or tool_call.get("function", {}).get("name") == CCR_TOOL_NAME - or tool_call.get("functionCall", {}).get("name") == CCR_TOOL_NAME + or (tool_call.get("function") or {}).get("name") == CCR_TOOL_NAME + or (tool_call.get("functionCall") or {}).get("name") == CCR_TOOL_NAME ) diff --git a/headroom/ccr/tool_injection.py b/headroom/ccr/tool_injection.py index 90a83c9a6..20337285d 100644 --- a/headroom/ccr/tool_injection.py +++ b/headroom/ccr/tool_injection.py @@ -467,7 +467,10 @@ def parse_tool_call( name = tool_call.get("name") input_data = tool_call.get("input", {}) elif provider == "openai": - function = tool_call.get("function", {}) + # `get("function", {})` returns None for an explicit {"function": null} + # (the default only applies to a missing key), so `.get` below would + # raise AttributeError on a malformed/partial tool call. Coalesce to {}. + function = tool_call.get("function") or {} name = function.get("name") # OpenAI passes args as JSON string args_str = function.get("arguments", "{}") @@ -478,7 +481,8 @@ def parse_tool_call( input_data = {} elif provider == "google": # Google/Gemini format: {"functionCall": {"name": "...", "args": {...}}} - function_call = tool_call.get("functionCall", {}) + # Coalesce to {} so an explicit {"functionCall": null} does not crash. + function_call = tool_call.get("functionCall") or {} name = function_call.get("name") input_data = function_call.get("args", {}) elif provider == "openai_responses": diff --git a/tests/test_ccr_tool_calls.py b/tests/test_ccr_tool_calls.py index b70b16320..bf2522b42 100644 --- a/tests/test_ccr_tool_calls.py +++ b/tests/test_ccr_tool_calls.py @@ -58,6 +58,36 @@ def test_has_ccr_tool_calls_uses_provider_native_names() -> None: ) +def test_ccr_detection_survives_null_function_tool_call() -> None: + # A partial/streamed OpenAI tool call with an explicit {"function": null} + # must not crash detection: dict.get("function", {}) returns None for a + # present-but-null key, and .get on None raises AttributeError. + response = { + "choices": [ + { + "message": { + "tool_calls": [ + {"id": "call_1", "type": "function", "function": None}, + { + "id": "call_2", + "type": "function", + "function": { + "name": CCR_TOOL_NAME, + "arguments": '{"hash": "' + HASH + '"}', + }, + }, + ] + } + } + ] + } + + assert has_ccr_tool_calls(response, "openai") + ccr_calls, other_calls = parse_ccr_tool_calls(response, "openai") + assert ccr_calls == [CCRToolCall(tool_call_id="call_2", hash_key=HASH)] + assert other_calls == [{"id": "call_1", "type": "function", "function": None}] + + def test_parse_ccr_tool_calls_splits_retrievals_from_other_tools() -> None: response = { "content": [ diff --git a/tests/test_ccr_tool_injection.py b/tests/test_ccr_tool_injection.py index d4506bc33..6bf2ccc13 100644 --- a/tests/test_ccr_tool_injection.py +++ b/tests/test_ccr_tool_injection.py @@ -310,6 +310,12 @@ class TestParseToolCall: assert hash_key == "def456abc123def456abc123" + def test_parse_null_function_returns_none_without_crashing(self): + """A tool call with an explicit {"function": null} / {"functionCall": null} + must return None, not raise AttributeError.""" + assert parse_tool_call({"id": "c1", "function": None}, "openai") is None + assert parse_tool_call({"functionCall": None}, "google") is None + def test_parse_normalises_uppercase_hash_to_lowercase(self): """An uppercase hash echoed by the model must be lowercased so it matches the store (which keys entries by a lowercase hash)."""