Mitigate tool call error due to missing "detail" field in the gatekeeper response (#448)

fix: handle null or missing detail in gatekeeper response

Add default empty string for GatekeeperResult.detail so responses that
omit the field (common when status is OK) no longer cause errors.
Introduce GatekeeperResultStrict for structured LLM output and unify
JSON parsing through model_validate_json, removing the manual fallback.

Co-authored-by: Owen Taylor <otaylor@fishsoup.net>

Author: Jazzcort

src/linux_mcp_server/gatekeeper/__init__.py

@@ -1,6 +1,7 @@
 from linux_mcp_server.gatekeeper.check_run_script import check_run_script
 from linux_mcp_server.gatekeeper.check_run_script import GatekeeperResult
+from linux_mcp_server.gatekeeper.check_run_script import GatekeeperResultStrict
 from linux_mcp_server.gatekeeper.check_run_script import GatekeeperStatus
 
 
-__all__ = ["check_run_script", "GatekeeperStatus", "GatekeeperResult"]
+__all__ = ["check_run_script", "GatekeeperStatus", "GatekeeperResult", "GatekeeperResultStrict"]

src/linux_mcp_server/gatekeeper/check_run_script.py

@@ -1,4 +1,3 @@
-import json
 import logging
 
 from litellm import Choices
@@ -96,6 +95,8 @@ def get_model() -> str:
 what is wrong with the script. Be specific to allow the language model to correct
 the problem.
 
+If status is OK, the detail should be an empty string.
+
 If the script seems buggy but does not fall into any of the categories above, return
 a status of `OK`.
 
@@ -116,7 +117,7 @@ class GatekeeperStatus(StrEnum):
 
 class GatekeeperResult(BaseModel):
     status: GatekeeperStatus
-    detail: str
+    detail: str = ""
 
     @property
     def description(self) -> str:
@@ -175,6 +176,10 @@ def parse_from_description(cls, description: str) -> "GatekeeperResult":
             return cls(status=status, detail=detail)
 
 
+class GatekeeperResultStrict(GatekeeperResult):
+    detail: str  # type:ignore
+
+
 def check_run_script(description: str, script_type: str, script: str, *, readonly: bool) -> GatekeeperResult:
     # Check that the script does what is described
     if "start_of_script" in script.lower() or "end_of_script" in script.lower():
@@ -200,7 +205,7 @@ def check_run_script(description: str, script_type: str, script: str, *, readonl
 
     params = get_supported_openai_params(model=get_model())
     if params is not None and "response_format" in params:
-        response_format = GatekeeperResult
+        response_format = GatekeeperResultStrict
     else:
         response_format = None
 
@@ -211,22 +216,4 @@ def check_run_script(description: str, script_type: str, script: str, *, readonl
 
     logger.info(f"Gatekeeper response: {response_text}")
 
-    if response_format is not None:
-        return GatekeeperResult.model_validate_json(response_text)
-
-    try:
-        response_data = json.loads(response_text)
-    except json.JSONDecodeError:
-        raise RuntimeError("Failed to parse response from gatekeeper model")
-
-    if not isinstance(response_data, dict):
-        raise RuntimeError("Invalid response format from gatekeeper model")
-
-    try:
-        status = GatekeeperStatus(response_data.get("status", ""))
-    except ValueError:
-        raise RuntimeError("Bad status in gatekeeper model response")
-
-    detail = response_data.get("detail", "")
-
-    return GatekeeperResult(status=status, detail=detail)
+    return GatekeeperResult.model_validate_json(response_text)

tests/gatekeeper/test_check_run_script.py

@@ -6,8 +6,10 @@
 
 from litellm import Choices
 from litellm import ModelResponse
+from pydantic import ValidationError
 
 from linux_mcp_server.gatekeeper import GatekeeperResult
+from linux_mcp_server.gatekeeper import GatekeeperResultStrict
 from linux_mcp_server.gatekeeper import GatekeeperStatus
 from linux_mcp_server.gatekeeper.check_run_script import check_run_script
 from linux_mcp_server.gatekeeper.check_run_script import get_model
@@ -123,22 +125,31 @@ def test_response_format_handling(self, mock_litellm, supported_params, expect_r
 
         call_kwargs = mock_completion.call_args.kwargs
         if expect_response_format:
-            assert call_kwargs["response_format"] is GatekeeperResult
+            assert call_kwargs["response_format"] is GatekeeperResultStrict
         else:
             assert call_kwargs["response_format"] is None
 
+    def test_missing_detail_defaults_to_empty(self, mock_litellm):
+        mock_completion, mock_get_params = mock_litellm
+        mock_get_params.return_value = ["response_format"]
+        mock_completion.return_value = self._make_response('{"status": "OK"}')
+
+        result = check_run_script(description="test", script_type="bash", script="echo hi", readonly=True)
+        assert result.status == GatekeeperStatus.OK
+        assert result.detail == ""
+
     @pytest.mark.parametrize(
-        "response_text,error_match",
+        "response_text,error_type,error_match",
         [
-            ("not valid json", "Failed to parse response"),
-            ('"just a string"', "Invalid response format"),
-            ('{"status": "INVALID_STATUS"}', "Bad status"),
+            ("not valid json", ValidationError, "input_value='not valid json'"),
+            ('"just a string"', ValidationError, "input_value='just a string'"),
+            ('{"status": "INVALID_STATUS"}', ValidationError, "input_value='INVALID_STATUS'"),
         ],
     )
-    def test_parse_errors(self, mock_litellm, response_text, error_match):
+    def test_parse_errors(self, mock_litellm, response_text, error_type, error_match):
         mock_completion, mock_get_params = mock_litellm
         mock_get_params.return_value = None  # No response_format support
         mock_completion.return_value = self._make_response(response_text)
 
-        with pytest.raises(RuntimeError, match=error_match):
+        with pytest.raises(error_type, match=error_match):
             check_run_script(description="test", script_type="bash", script="echo hi", readonly=True)