Treat literal 'null'/'none' strings as absent for optional tool args (#1164)

* Treat literal 'null'/'none' strings as absent for optional tool args

Models routinely pass the literal string "null" or "none" instead of
omitting an optional argument. Taken at face value it becomes a filter
that matches nothing, so tools like list_notes / list_reports /
list_requests silently return no results.

Coerce such values to None in the central argument-coercion layer, but
only for parameters the schema allows to be null (or that are absent from
a declared "required" list), so required strings keep the literal value.
The list/filter helpers normalize the same values too, so a direct call
can't regress.

* Limit nullish coercion to query tools and keep literal tags

A literal "null"/"none" is only a mistake where the argument is a filter, so
gate the coercion on read-only query tools; a tool that writes keeps the value,
which stops update_note(content="none") from being read as "leave unchanged".

Stop dropping nullish entries from a notes tag filter too: tags are free-form,
so a literal "none" tag stays filterable and mixed tag queries keep every
branch.

tests/test_agent_factory_tool_arguments.py

@@ -9,24 +9,30 @@ import pytest
 from agents.tool import FunctionTool
 
 from strix.agents import factory
+from strix.tools.notes.tools import list_notes
+from strix.tools.reporting.tool import list_reports
 
 
-def _capturing_tool(captured: dict[str, str], schema: dict[str, Any]) -> FunctionTool:
+def _capturing_tool(
+    captured: dict[str, str], schema: dict[str, Any], name: str = "probe"
+) -> FunctionTool:
     async def invoke(_ctx: Any, raw_input: str) -> str:
         captured["raw_input"] = raw_input
         return "ok"
 
     return FunctionTool(
-        name="probe",
+        name=name,
         description="test tool",
         params_json_schema={"type": "object", "properties": schema},
         on_invoke_tool=invoke,
     )
 
 
-async def _roundtrip(schema: dict[str, Any], payload: dict[str, Any]) -> dict[str, Any]:
+async def _roundtrip(
+    schema: dict[str, Any], payload: dict[str, Any], name: str = "probe"
+) -> dict[str, Any]:
     captured: dict[str, str] = {}
-    wrapped = factory._with_coerced_arguments(_capturing_tool(captured, schema))
+    wrapped = factory._with_coerced_arguments(_capturing_tool(captured, schema, name))
     assert await wrapped.on_invoke_tool(cast("Any", None), json.dumps(payload)) == "ok"
     return cast("dict[str, Any]", json.loads(captured["raw_input"]))
 
@@ -144,3 +150,88 @@ async def test_coercion_is_applied_once_per_tool() -> None:
     tool = factory._with_coerced_arguments(_capturing_tool(captured, _ARRAY))
 
     assert factory._with_coerced_arguments(tool) is tool
+
+
+_NULLABLE_STRING = {"category": {"anyOf": [{"type": "string"}, {"type": "null"}]}}
+_NULLABLE_CONTENT = {"content": {"anyOf": [{"type": "string"}, {"type": "null"}]}}
+_NULLABLE_STRING_TYPE_LIST = {"category": {"type": ["string", "null"]}}
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("schema", [_NULLABLE_STRING, _NULLABLE_STRING_TYPE_LIST])
+@pytest.mark.parametrize("value", ["null", "none", "NULL", " None ", "nil", "undefined"])
+async def test_nullish_string_on_a_nullable_parameter_becomes_none(
+    schema: dict[str, Any], value: str
+) -> None:
+    parsed = await _roundtrip(schema, {"category": value}, "list_probes")
+
+    assert parsed["category"] is None
+
+
+@pytest.mark.asyncio
+async def test_nullish_string_on_a_required_parameter_is_untouched() -> None:
+    schema = {"content": {"type": "string"}}
+    captured: dict[str, str] = {}
+    tool = _capturing_tool(captured, schema, "list_probes")
+    tool.params_json_schema["required"] = ["content"]
+    wrapped = factory._with_coerced_arguments(tool)
+
+    assert await wrapped.on_invoke_tool(cast("Any", None), json.dumps({"content": "none"})) == "ok"
+    assert json.loads(captured["raw_input"])["content"] == "none"
+
+
+@pytest.mark.asyncio
+async def test_a_parameter_absent_from_required_is_treated_as_nullable() -> None:
+    captured: dict[str, str] = {}
+    tool = _capturing_tool(captured, {"category": {"type": "string"}}, "list_probes")
+    tool.params_json_schema["required"] = []
+    wrapped = factory._with_coerced_arguments(tool)
+
+    assert await wrapped.on_invoke_tool(cast("Any", None), json.dumps({"category": "null"})) == "ok"
+    assert json.loads(captured["raw_input"])["category"] is None
+
+
+@pytest.mark.asyncio
+async def test_nullish_string_without_a_required_list_is_untouched() -> None:
+    parsed = await _roundtrip(_STRING, {"todos": "none"}, "list_probes")
+
+    assert parsed["todos"] == "none"
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("name", ["update_note", "create_note", "record_coverage"])
+@pytest.mark.parametrize("value", ["null", "none"])
+async def test_a_nullish_value_survives_on_a_tool_that_writes(name: str, value: str) -> None:
+    parsed = await _roundtrip(_NULLABLE_CONTENT, {"content": value}, name)
+
+    assert parsed["content"] == value
+
+
+@pytest.mark.asyncio
+async def test_nullish_looking_content_is_not_coerced() -> None:
+    parsed = await _roundtrip(
+        _NULLABLE_STRING, {"category": "none of the endpoints reflect input"}, "list_probes"
+    )
+
+    assert parsed["category"] == "none of the endpoints reflect input"
+
+
+@pytest.mark.asyncio
+async def test_empty_string_on_a_nullable_string_parameter_is_untouched() -> None:
+    parsed = await _roundtrip(_NULLABLE_STRING, {"category": ""})
+
+    assert parsed["category"] == ""
+
+
+@pytest.mark.asyncio
+async def test_nullish_string_on_a_nullable_array_parameter_becomes_none() -> None:
+    parsed = await _roundtrip(_NULLABLE_ARRAY, {"tags": "null"}, "list_probes")
+
+    assert parsed["tags"] is None
+
+
+def test_real_tool_schemas_declare_optional_filters_as_nullable() -> None:
+    for tool, params in ((list_notes, ("category", "search")), (list_reports, ("target",))):
+        schema = tool.params_json_schema
+        for param in params:
+            assert factory._is_nullable(param, schema["properties"][param], schema)

tests/test_list_reports.py

@@ -357,3 +357,25 @@ def test_get_report_no_state_returns_error(monkeypatch: pytest.MonkeyPatch) -> N
     result = _do_get_report("vuln-0001")
     assert result["success"] is False
     assert result["report"] is None
+
+
+@pytest.mark.parametrize("nullish", ["null", "none", "NULL", " None ", "undefined", "nil"])
+def test_list_reports_ignores_nullish_filter_strings(
+    report_state: ReportState, nullish: str
+) -> None:
+    _seed(report_state)
+    unfiltered = _do_list_reports(
+        severity=None, finding_class=None, target=None, search=None, include_details=False
+    )
+    assert unfiltered["filtered_count"] == 3
+
+    assert (
+        _do_list_reports(
+            severity=nullish,
+            finding_class=nullish,
+            target=nullish,
+            search=nullish,
+            include_details=False,
+        )
+        == unfiltered
+    )

tests/test_notes.py

@@ -101,3 +101,33 @@ def test_get_note_flags_caller_ownership() -> None:
     assert mine["note"]["agent_name"] == "Agent One"
     theirs = notes_tools._get_note_impl(note_id, caller_agent_id="agent-9")
     assert "by_you" not in theirs["note"]
+
+
+@pytest.mark.parametrize("nullish", ["null", "none", "NULL", " None ", "undefined", "nil"])
+def test_list_notes_ignores_nullish_filter_strings(nullish: str) -> None:
+    notes_tools._create_note_impl("recon", "content", category="findings", tags=["auth"])
+    notes_tools._create_note_impl("other", "content", category="general")
+
+    unfiltered = notes_tools._list_notes_impl()
+    assert unfiltered["filtered_count"] == 2
+
+    assert notes_tools._list_notes_impl(category=nullish) == unfiltered
+    assert notes_tools._list_notes_impl(search=nullish) == unfiltered
+
+
+@pytest.mark.parametrize("tag", ["null", "none"])
+def test_list_notes_filters_on_a_literal_nullish_tag(tag: str) -> None:
+    notes_tools._create_note_impl("tagged", "content", tags=[tag])
+    notes_tools._create_note_impl("other", "content", tags=["auth"])
+
+    assert [n["title"] for n in notes_tools._list_notes_impl(tags=[tag])["notes"]] == ["tagged"]
+    mixed = notes_tools._list_notes_impl(tags=[tag, "auth"])
+    assert sorted(n["title"] for n in mixed["notes"]) == ["other", "tagged"]
+
+
+def test_list_notes_still_filters_on_real_values() -> None:
+    notes_tools._create_note_impl("recon", "content", category="findings")
+    notes_tools._create_note_impl("other", "content", category="general")
+
+    result = notes_tools._list_notes_impl(category="findings")
+    assert [n["title"] for n in result["notes"]] == ["recon"]