Skip to content

Commit 01646be

Browse files
committed
Revert "always default to adding risk prediction"
This reverts commit 160a6a2.
1 parent 8405598 commit 01646be

File tree

10 files changed

+95
-20
lines changed

10 files changed

+95
-20
lines changed

openhands-sdk/openhands/sdk/agent/agent.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ def init_state(
9999
source="agent",
100100
system_prompt=TextContent(text=self.system_message),
101101
# Always include security_risk field in tools
102-
tools=[t.to_openai_tool() for t in self.tools_map.values()],
102+
tools=[
103+
t.to_openai_tool(add_security_risk_prediction=True)
104+
for t in self.tools_map.values()
105+
],
103106
)
104107
on_event(event)
105108

@@ -175,13 +178,15 @@ def step(
175178
tools=list(self.tools_map.values()),
176179
include=None,
177180
store=False,
181+
add_security_risk_prediction=True,
178182
extra_body=self.llm.litellm_extra_body,
179183
)
180184
else:
181185
llm_response = self.llm.completion(
182186
messages=_messages,
183187
tools=list(self.tools_map.values()),
184188
extra_body=self.llm.litellm_extra_body,
189+
add_security_risk_prediction=True,
185190
)
186191
except FunctionCallValidationError as e:
187192
logger.warning(f"LLM generated malformed function call: {e}")

openhands-sdk/openhands/sdk/llm/llm.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ def completion(
433433
messages: list[Message],
434434
tools: Sequence[ToolDefinition] | None = None,
435435
_return_metrics: bool = False,
436+
add_security_risk_prediction: bool = False,
436437
**kwargs,
437438
) -> LLMResponse:
438439
"""Generate a completion from the language model.
@@ -466,7 +467,12 @@ def completion(
466467
# Convert Tool objects to ChatCompletionToolParam once here
467468
cc_tools: list[ChatCompletionToolParam] = []
468469
if tools:
469-
cc_tools = [t.to_openai_tool() for t in tools]
470+
cc_tools = [
471+
t.to_openai_tool(
472+
add_security_risk_prediction=add_security_risk_prediction
473+
)
474+
for t in tools
475+
]
470476

471477
use_mock_tools = self.should_mock_tool_calls(cc_tools)
472478
if use_mock_tools:
@@ -566,6 +572,7 @@ def responses(
566572
include: list[str] | None = None,
567573
store: bool | None = None,
568574
_return_metrics: bool = False,
575+
add_security_risk_prediction: bool = False,
569576
**kwargs,
570577
) -> LLMResponse:
571578
"""Alternative invocation path using OpenAI Responses API via LiteLLM.
@@ -582,7 +589,16 @@ def responses(
582589

583590
# Convert Tool objects to Responses ToolParam
584591
# (Responses path always supports function tools)
585-
resp_tools = [t.to_responses_tool() for t in tools] if tools else None
592+
resp_tools = (
593+
[
594+
t.to_responses_tool(
595+
add_security_risk_prediction=add_security_risk_prediction
596+
)
597+
for t in tools
598+
]
599+
if tools
600+
else None
601+
)
586602

587603
# Normalize/override Responses kwargs consistently
588604
call_kwargs = select_responses_options(

openhands-sdk/openhands/sdk/llm/router/base.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ def completion(
5151
messages: list[Message],
5252
tools: Sequence[ToolDefinition] | None = None,
5353
return_metrics: bool = False,
54+
add_security_risk_prediction: bool = False,
5455
**kwargs,
5556
) -> LLMResponse:
5657
"""
@@ -68,6 +69,7 @@ def completion(
6869
messages=messages,
6970
tools=tools,
7071
_return_metrics=return_metrics,
72+
add_security_risk_prediction=add_security_risk_prediction,
7173
**kwargs,
7274
)
7375

openhands-sdk/openhands/sdk/mcp/tool.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ def to_mcp_tool(
242242

243243
def to_openai_tool(
244244
self,
245+
add_security_risk_prediction: bool = False,
245246
action_type: type[Schema] | None = None,
246247
) -> ChatCompletionToolParam:
247248
"""Convert a Tool to an OpenAI tool.
@@ -250,6 +251,12 @@ def to_openai_tool(
250251
from the MCP tool input schema, and pass it to the parent method.
251252
It will use the .model_fields from this pydantic model to
252253
generate the OpenAI-compatible tool schema.
254+
255+
Args:
256+
add_security_risk_prediction: Whether to add a `security_risk` field
257+
to the action schema for LLM to predict. This is useful for
258+
tools that may have safety risks, so the LLM can reason about
259+
the risk level before calling the tool.
253260
"""
254261
if action_type is not None:
255262
raise ValueError(
@@ -259,5 +266,6 @@ def to_openai_tool(
259266
assert self.name == self.mcp_tool.name
260267
mcp_action_type = _create_mcp_action_type(self.mcp_tool)
261268
return super().to_openai_tool(
269+
add_security_risk_prediction=add_security_risk_prediction,
262270
action_type=mcp_action_type,
263271
)

openhands-sdk/openhands/sdk/tool/tool.py

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,42 +360,72 @@ def to_mcp_tool(
360360

361361
def _get_tool_schema(
362362
self,
363+
add_security_risk_prediction: bool = False,
363364
action_type: type[Schema] | None = None,
364365
) -> dict[str, Any]:
365366
action_type = action_type or self.action_type
366-
action_type_with_risk = _create_action_type_with_risk(action_type)
367-
schema = action_type_with_risk.to_mcp_schema()
367+
368+
if add_security_risk_prediction:
369+
# Always include security_risk field when prediction is enabled
370+
# This ensures consistent tool schemas regardless of tool type
371+
# (including read-only tools)
372+
action_type_with_risk = _create_action_type_with_risk(action_type)
373+
schema = action_type_with_risk.to_mcp_schema()
374+
else:
375+
schema = action_type.to_mcp_schema()
376+
368377
return schema
369378

370379
def to_openai_tool(
371380
self,
381+
add_security_risk_prediction: bool = False,
372382
action_type: type[Schema] | None = None,
373383
) -> ChatCompletionToolParam:
374-
"""Convert a Tool to an OpenAI tool."""
384+
"""Convert a Tool to an OpenAI tool.
385+
386+
Args:
387+
add_security_risk_prediction: Whether to include the `security_risk`
388+
field in the tool schema. When enabled, the field is included
389+
for all tool types (including read-only tools).
390+
action_type: Optionally override the action_type to use for the schema.
391+
This is useful for MCPTool to use a dynamically created action type
392+
based on the tool's input schema.
393+
"""
375394
return ChatCompletionToolParam(
376395
type="function",
377396
function=ChatCompletionToolParamFunctionChunk(
378397
name=self.name,
379398
description=self.description,
380-
parameters=self._get_tool_schema(action_type),
399+
parameters=self._get_tool_schema(
400+
add_security_risk_prediction, action_type
401+
),
381402
),
382403
)
383404

384405
def to_responses_tool(
385406
self,
407+
add_security_risk_prediction: bool = False,
386408
action_type: type[Schema] | None = None,
387409
) -> FunctionToolParam:
388410
"""Convert a Tool to a Responses API function tool (LiteLLM typed).
389411
390412
For Responses API, function tools expect top-level keys:
391413
{ "type": "function", "name": ..., "description": ..., "parameters": ... }
414+
415+
Args:
416+
add_security_risk_prediction: Whether to include the `security_risk`
417+
field in the tool schema. When enabled, the field is included
418+
for all tool types (including read-only tools).
419+
action_type: Optionally override the action_type to use for the schema.
392420
"""
393421

394422
return {
395423
"type": "function",
396424
"name": self.name,
397425
"description": self.description,
398-
"parameters": self._get_tool_schema(action_type),
426+
"parameters": self._get_tool_schema(
427+
add_security_risk_prediction, action_type
428+
),
399429
"strict": False,
400430
}
401431

tests/cross/test_remote_conversation_live_server.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ def fake_completion(
147147
messages,
148148
tools,
149149
return_metrics=False,
150+
add_security_risk_prediction=False,
150151
**kwargs,
151152
): # type: ignore[no-untyped-def]
152153
from openhands.sdk.llm.llm_response import LLMResponse
@@ -447,6 +448,7 @@ def fake_completion_with_cost(
447448
messages,
448449
tools,
449450
return_metrics=False,
451+
add_security_risk_prediction=False,
450452
**kwargs,
451453
): # type: ignore[no-untyped-def]
452454
from openhands.sdk.llm.llm_response import LLMResponse

tests/sdk/llm/test_llm_completion.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ def test_llm_completion_non_function_call_mode(mock_completion):
349349
tools = list(_MockTool.create())
350350

351351
# Verify that tools should be mocked (non-function call path)
352-
cc_tools = [t.to_openai_tool() for t in tools]
352+
cc_tools = [t.to_openai_tool(add_security_risk_prediction=False) for t in tools]
353353
assert llm.should_mock_tool_calls(cc_tools)
354354

355355
# Call completion - this should go through the prompt-based tool calling path

tests/sdk/mcp/test_mcp_security_risk.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def test_mcp_tool_to_openai_with_security_risk():
6666
tool = tools[0]
6767

6868
# Generate OpenAI tool schema WITH security risk prediction
69-
openai_tool = tool.to_openai_tool()
69+
openai_tool = tool.to_openai_tool(add_security_risk_prediction=True)
7070

7171
function_params = openai_tool["function"]["parameters"] # type: ignore[typeddict-item]
7272
properties = function_params["properties"]

tests/sdk/tool/test_to_responses_tool_security.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def test_to_responses_tool_security_gating():
4848
observation_type=None,
4949
annotations=ToolAnnotations(readOnlyHint=True),
5050
)
51-
t = readonly.to_responses_tool()
51+
t = readonly.to_responses_tool(add_security_risk_prediction=True)
5252
params = t["parameters"]
5353
assert isinstance(params, dict)
5454
props = params.get("properties") or {}
@@ -62,7 +62,7 @@ def test_to_responses_tool_security_gating():
6262
observation_type=None,
6363
annotations=ToolAnnotations(readOnlyHint=False),
6464
)
65-
t2 = writable.to_responses_tool()
65+
t2 = writable.to_responses_tool(add_security_risk_prediction=True)
6666
params2 = t2["parameters"]
6767
assert isinstance(params2, dict)
6868
props2 = params2.get("properties") or {}
@@ -76,7 +76,7 @@ def test_to_responses_tool_security_gating():
7676
observation_type=None,
7777
annotations=None,
7878
)
79-
t3 = noflag.to_responses_tool()
79+
t3 = noflag.to_responses_tool(add_security_risk_prediction=False)
8080
params3 = t3["parameters"]
8181
assert isinstance(params3, dict)
8282
props3 = params3.get("properties") or {}

tests/sdk/tool/test_tool_definition.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,9 @@ def test_security_risk_added_for_all_tools_when_enabled(self):
579579
)
580580

581581
# Test read-only tool - security_risk should be added when enabled
582-
readonly_openai_tool = readonly_tool.to_openai_tool()
582+
readonly_openai_tool = readonly_tool.to_openai_tool(
583+
add_security_risk_prediction=True
584+
)
583585
readonly_function = readonly_openai_tool["function"]
584586
assert "parameters" in readonly_function
585587
readonly_params = readonly_function["parameters"]
@@ -588,27 +590,35 @@ def test_security_risk_added_for_all_tools_when_enabled(self):
588590
) # Included for read-only tools too
589591

590592
# Test writable tool - security_risk SHOULD be added
591-
writable_openai_tool = writable_tool.to_openai_tool()
593+
writable_openai_tool = writable_tool.to_openai_tool(
594+
add_security_risk_prediction=True
595+
)
592596
writable_function = writable_openai_tool["function"]
593597
assert "parameters" in writable_function
594598
writable_params = writable_function["parameters"]
595599
assert "security_risk" in writable_params["properties"]
596600

597601
# Test tool with no annotations - security_risk SHOULD be added
598-
no_annotations_openai_tool = no_annotations_tool.to_openai_tool()
602+
no_annotations_openai_tool = no_annotations_tool.to_openai_tool(
603+
add_security_risk_prediction=True
604+
)
599605
no_annotations_function = no_annotations_openai_tool["function"]
600606
assert "parameters" in no_annotations_function
601607
no_annotations_params = no_annotations_function["parameters"]
602608
assert "security_risk" in no_annotations_params["properties"]
603609

604610
# Test that when add_security_risk_prediction=False, no security_risk is added
605-
readonly_no_risk = readonly_tool.to_openai_tool()
611+
readonly_no_risk = readonly_tool.to_openai_tool(
612+
add_security_risk_prediction=False
613+
)
606614
readonly_no_risk_function = readonly_no_risk["function"]
607615
assert "parameters" in readonly_no_risk_function
608616
readonly_no_risk_params = readonly_no_risk_function["parameters"]
609617
assert "security_risk" not in readonly_no_risk_params["properties"]
610618

611-
writable_no_risk = writable_tool.to_openai_tool()
619+
writable_no_risk = writable_tool.to_openai_tool(
620+
add_security_risk_prediction=False
621+
)
612622
writable_no_risk_function = writable_no_risk["function"]
613623
assert "parameters" in writable_no_risk_function
614624
writable_no_risk_params = writable_no_risk_function["parameters"]
@@ -633,7 +643,7 @@ def test_security_risk_is_required_field_in_schema(self):
633643
observation_type=ToolMockObservation,
634644
)
635645

636-
openai_tool = tool.to_openai_tool()
646+
openai_tool = tool.to_openai_tool(add_security_risk_prediction=True)
637647
function_chunk = openai_tool["function"]
638648
assert "parameters" in function_chunk
639649
function_params = function_chunk["parameters"]
@@ -657,7 +667,9 @@ def test_security_risk_is_required_field_in_schema(self):
657667
annotations=writable_annotations,
658668
)
659669

660-
writable_openai_tool = writable_tool.to_openai_tool()
670+
writable_openai_tool = writable_tool.to_openai_tool(
671+
add_security_risk_prediction=True
672+
)
661673
writable_function_chunk = writable_openai_tool["function"]
662674
assert "parameters" in writable_function_chunk
663675
writable_function_params = writable_function_chunk["parameters"]

0 commit comments

Comments
 (0)