Skip to content

Commit 2b9eb62

Browse files
authored
Merge branch 'main' into fix/add-security-risk-to-tool-examples
2 parents c73477b + fe9b8ff commit 2b9eb62

File tree

10 files changed

+366
-21
lines changed

10 files changed

+366
-21
lines changed

examples/03_github_workflows/02_pr_review/prompt.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,21 @@
3636
PROMPT = """{skill_trigger}
3737
/github-pr-review
3838
39-
When posting a review, keep the review body brief unless your active review instructions
40-
require a longer structured format.
39+
When posting a review, keep the review body brief unless your active review
40+
instructions require a longer structured format.
41+
42+
## Review decision policy (eval / benchmark risk)
43+
44+
You MAY approve clearly low-risk changes (docs, typo fixes, formatting, or
45+
pure refactors with no behavior changes).
46+
47+
Do NOT submit an APPROVE review when the PR changes agent behavior or anything
48+
that affects benchmark/evaluation performance.
49+
Examples include: prompt templates, tool calling/execution, planning/loop logic,
50+
memory/condenser behavior, terminal/stdin/stdout handling, or evaluation harness code.
51+
52+
If a PR is in this category (or you are uncertain), leave a COMMENTED review and
53+
explicitly flag it for a human maintainer to decide after running lightweight evals.
4154
4255
Review the PR changes below and identify issues that need to be addressed.
4356

openhands-agent-server/openhands/agent_server/config.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ class Config(BaseModel):
140140
le=65535,
141141
description="Port on which VSCode server should run",
142142
)
143+
vscode_base_path: str | None = Field(
144+
default=None,
145+
description=(
146+
"Base path for VSCode server (used in path-based routing). "
147+
"For example, '/{runtime_id}/vscode' when using path-based routing."
148+
),
149+
)
143150
enable_vnc: bool = Field(
144151
default=False,
145152
description="Whether to enable VNC desktop functionality",

openhands-agent-server/openhands/agent_server/conversation_router.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,20 @@ async def update_conversation(
275275
@conversation_router.post(
276276
"/{conversation_id}/generate_title",
277277
responses={404: {"description": "Item not found"}},
278+
deprecated=True,
278279
)
279280
async def generate_conversation_title(
280281
conversation_id: UUID,
281282
request: GenerateTitleRequest,
282283
conversation_service: ConversationService = Depends(get_conversation_service),
283284
) -> GenerateTitleResponse:
284-
"""Generate a title for the conversation using LLM."""
285+
"""Generate a title for the conversation using LLM.
286+
287+
Deprecated since v1.11.5 and scheduled for removal in v1.14.0.
288+
289+
Prefer enabling `autotitle` in `StartConversationRequest` to have the server
290+
generate and persist the title automatically from the first user message.
291+
"""
285292
title = await conversation_service.generate_conversation_title(
286293
conversation_id, request.max_length, request.llm
287294
)

openhands-agent-server/openhands/agent_server/conversation_service.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
ConversationExecutionStatus,
2626
ConversationState,
2727
)
28+
from openhands.sdk.event import MessageEvent
2829
from openhands.sdk.event.conversation_state import ConversationStateUpdateEvent
2930
from openhands.sdk.utils.cipher import Cipher
3031

@@ -505,6 +506,10 @@ async def _start_event_service(self, stored: StoredConversation) -> EventService
505506
)
506507
# Create subscribers...
507508
await event_service.subscribe_to_events(_EventSubscriber(service=event_service))
509+
if stored.autotitle and stored.title is None:
510+
await event_service.subscribe_to_events(
511+
AutoTitleSubscriber(service=event_service)
512+
)
508513
asyncio.gather(
509514
*[
510515
event_service.subscribe_to_events(
@@ -548,6 +553,35 @@ async def __call__(self, _event: Event):
548553
update_last_execution_time()
549554

550555

556+
@dataclass
557+
class AutoTitleSubscriber(Subscriber):
558+
service: EventService
559+
560+
async def __call__(self, event: Event) -> None:
561+
# Only act on incoming user messages
562+
if not isinstance(event, MessageEvent) or event.source != "user":
563+
return
564+
# Guard: skip if a title was already set (e.g. by a concurrent task)
565+
if self.service.stored.title is not None:
566+
return
567+
568+
async def _generate_and_save() -> None:
569+
try:
570+
title = await self.service.generate_title()
571+
if title and self.service.stored.title is None:
572+
self.service.stored.title = title
573+
self.service.stored.updated_at = utc_now()
574+
await self.service.save_meta()
575+
except Exception:
576+
logger.warning(
577+
f"Auto-title generation failed for "
578+
f"conversation {self.service.stored.id}",
579+
exc_info=True,
580+
)
581+
582+
asyncio.create_task(_generate_and_save())
583+
584+
551585
@dataclass
552586
class WebhookSubscriber(Subscriber):
553587
conversation_id: UUID

openhands-agent-server/openhands/agent_server/models.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,13 @@ class StartConversationRequest(BaseModel):
126126
"hooks."
127127
),
128128
)
129+
autotitle: bool = Field(
130+
default=True,
131+
description=(
132+
"If true, automatically generate a title for the conversation from "
133+
"the first user message using the conversation's LLM."
134+
),
135+
)
129136

130137

131138
class StoredConversation(StartConversationRequest):

openhands-agent-server/openhands/agent_server/vscode_service.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ def __init__(
1818
self,
1919
port: int = 8001,
2020
connection_token: str | None = None,
21+
server_base_path: str | None = None,
2122
):
2223
"""Initialize VSCode service.
2324
@@ -26,9 +27,11 @@ def __init__(
2627
workspace_path: Path to the workspace directory
2728
create_workspace: Whether to create the workspace directory if it doesn't
2829
exist
30+
server_base_path: Base path for the server (used in path-based routing)
2931
"""
3032
self.port: int = port
3133
self.connection_token: str | None = connection_token
34+
self.server_base_path: str | None = server_base_path
3235
self.process: asyncio.subprocess.Process | None = None
3336
self.openvscode_server_root: Path = Path("/openhands/.openvscode-server")
3437
self.extensions_dir: Path = self.openvscode_server_root / "extensions"
@@ -147,12 +150,18 @@ async def _start_vscode_process(self) -> None:
147150
if self.extensions_dir.exists()
148151
else ""
149152
)
153+
base_path_arg = (
154+
f"--server-base-path {self.server_base_path} "
155+
if self.server_base_path
156+
else ""
157+
)
150158
cmd = (
151159
f"exec {self.openvscode_server_root}/bin/openvscode-server "
152160
f"--host 0.0.0.0 "
153161
f"--connection-token {self.connection_token} "
154162
f"--port {self.port} "
155163
f"{extensions_arg}"
164+
f"{base_path_arg}"
156165
f"--disable-workspace-trust\n"
157166
)
158167

@@ -229,6 +238,8 @@ def get_vscode_service() -> VSCodeService | None:
229238
if config.session_api_keys:
230239
connection_token = config.session_api_keys[0]
231240
_vscode_service = VSCodeService(
232-
port=config.vscode_port, connection_token=connection_token
241+
port=config.vscode_port,
242+
connection_token=connection_token,
243+
server_base_path=config.vscode_base_path,
233244
)
234245
return _vscode_service

openhands-workspace/openhands/workspace/docker/dev_workspace.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ class DockerDevWorkspace(DockerWorkspace):
2828
result = workspace.execute_command("ls -la")
2929
"""
3030

31+
# Override parent's server_image default to None so that callers
32+
# providing base_image don't need to explicitly pass server_image=None.
33+
server_image: str | None = Field(
34+
default=None,
35+
description="Pre-built agent server image. Mutually exclusive with base_image.",
36+
)
37+
3138
# Add base_image support
3239
base_image: str | None = Field(
3340
default=None,

tests/agent_server/test_conversation_router.py

Lines changed: 89 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,6 @@ def test_generate_conversation_title_success(
10471047
):
10481048
"""Test generate_conversation_title endpoint with successful generation."""
10491049

1050-
# Mock the service response
10511050
mock_conversation_service.generate_conversation_title.return_value = (
10521051
"Generated Title"
10531052
)
@@ -1068,12 +1067,11 @@ def test_generate_conversation_title_success(
10681067
data = response.json()
10691068
assert data["title"] == "Generated Title"
10701069

1071-
# Verify service was called with correct parameters
10721070
mock_conversation_service.generate_conversation_title.assert_called_once()
10731071
call_args = mock_conversation_service.generate_conversation_title.call_args
10741072
assert call_args[0][0] == sample_conversation_id
1075-
assert call_args[0][1] == 30 # max_length
1076-
assert call_args[0][2] is None # llm (default)
1073+
assert call_args[0][1] == 30
1074+
assert call_args[0][2] is None
10771075
finally:
10781076
client.app.dependency_overrides.clear()
10791077

@@ -1083,7 +1081,6 @@ def test_generate_conversation_title_with_llm(
10831081
):
10841082
"""Test generate_conversation_title endpoint with custom LLM."""
10851083

1086-
# Mock the service response
10871084
mock_conversation_service.generate_conversation_title.return_value = (
10881085
"Custom LLM Title"
10891086
)
@@ -1111,12 +1108,11 @@ def test_generate_conversation_title_with_llm(
11111108
data = response.json()
11121109
assert data["title"] == "Custom LLM Title"
11131110

1114-
# Verify service was called
11151111
mock_conversation_service.generate_conversation_title.assert_called_once()
11161112
call_args = mock_conversation_service.generate_conversation_title.call_args
11171113
assert call_args[0][0] == sample_conversation_id
1118-
assert call_args[0][1] == 40 # max_length
1119-
assert call_args[0][2] is not None # llm provided
1114+
assert call_args[0][1] == 40
1115+
assert call_args[0][2] is not None
11201116
finally:
11211117
client.app.dependency_overrides.clear()
11221118

@@ -1126,7 +1122,6 @@ def test_generate_conversation_title_failure(
11261122
):
11271123
"""Test generate_conversation_title endpoint with generation failure."""
11281124

1129-
# Mock the service response - generation failed
11301125
mock_conversation_service.generate_conversation_title.return_value = None
11311126

11321127
client.app.dependency_overrides[get_conversation_service] = (
@@ -1141,9 +1136,7 @@ def test_generate_conversation_title_failure(
11411136
json=request_data,
11421137
)
11431138

1144-
assert response.status_code == 500 # Internal Server Error
1145-
1146-
# Verify service was called
1139+
assert response.status_code == 500
11471140
mock_conversation_service.generate_conversation_title.assert_called_once()
11481141
finally:
11491142
client.app.dependency_overrides.clear()
@@ -1159,25 +1152,36 @@ def test_generate_conversation_title_invalid_params(
11591152
)
11601153

11611154
try:
1162-
# Test with max_length too low
11631155
request_data = {"max_length": 0}
11641156
response = client.post(
11651157
f"/api/conversations/{sample_conversation_id}/generate_title",
11661158
json=request_data,
11671159
)
1168-
assert response.status_code == 422 # Validation error
1160+
assert response.status_code == 422
11691161

1170-
# Test with max_length too high
11711162
request_data = {"max_length": 201}
11721163
response = client.post(
11731164
f"/api/conversations/{sample_conversation_id}/generate_title",
11741165
json=request_data,
11751166
)
1176-
assert response.status_code == 422 # Validation error
1167+
assert response.status_code == 422
11771168
finally:
11781169
client.app.dependency_overrides.clear()
11791170

11801171

1172+
def test_generate_title_endpoint_is_deprecated_in_openapi(client):
1173+
response = client.get("/openapi.json")
1174+
assert response.status_code == 200
1175+
1176+
openapi_schema = response.json()
1177+
operation = openapi_schema["paths"][
1178+
"/api/conversations/{conversation_id}/generate_title"
1179+
]["post"]
1180+
1181+
assert operation.get("deprecated") is True
1182+
assert "scheduled for removal" in operation["description"]
1183+
1184+
11811185
def test_start_conversation_with_tool_module_qualnames(
11821186
client, mock_conversation_service, sample_conversation_info
11831187
):
@@ -1284,6 +1288,75 @@ def test_start_conversation_without_tool_module_qualnames(
12841288
client.app.dependency_overrides.clear()
12851289

12861290

1291+
def test_start_conversation_autotitle_defaults_to_true(
1292+
client, mock_conversation_service, sample_conversation_info
1293+
):
1294+
"""autotitle defaults to True when not supplied in the request."""
1295+
mock_conversation_service.start_conversation.return_value = (
1296+
sample_conversation_info,
1297+
True,
1298+
)
1299+
client.app.dependency_overrides[get_conversation_service] = (
1300+
lambda: mock_conversation_service
1301+
)
1302+
1303+
try:
1304+
request_data = {
1305+
"agent": {
1306+
"llm": {
1307+
"model": "gpt-4o",
1308+
"api_key": "test-key",
1309+
"usage_id": "test-llm",
1310+
},
1311+
"tools": [{"name": "TerminalTool"}],
1312+
},
1313+
"workspace": {"working_dir": "/tmp/test"},
1314+
}
1315+
response = client.post("/api/conversations", json=request_data)
1316+
1317+
assert response.status_code == 201
1318+
call_args = mock_conversation_service.start_conversation.call_args
1319+
request_arg = call_args[0][0]
1320+
assert request_arg.autotitle is True
1321+
finally:
1322+
client.app.dependency_overrides.clear()
1323+
1324+
1325+
def test_start_conversation_autotitle_false(
1326+
client, mock_conversation_service, sample_conversation_info
1327+
):
1328+
"""autotitle=False is forwarded correctly to the service."""
1329+
mock_conversation_service.start_conversation.return_value = (
1330+
sample_conversation_info,
1331+
True,
1332+
)
1333+
client.app.dependency_overrides[get_conversation_service] = (
1334+
lambda: mock_conversation_service
1335+
)
1336+
1337+
try:
1338+
request_data = {
1339+
"agent": {
1340+
"llm": {
1341+
"model": "gpt-4o",
1342+
"api_key": "test-key",
1343+
"usage_id": "test-llm",
1344+
},
1345+
"tools": [{"name": "TerminalTool"}],
1346+
},
1347+
"workspace": {"working_dir": "/tmp/test"},
1348+
"autotitle": False,
1349+
}
1350+
response = client.post("/api/conversations", json=request_data)
1351+
1352+
assert response.status_code == 201
1353+
call_args = mock_conversation_service.start_conversation.call_args
1354+
request_arg = call_args[0][0]
1355+
assert request_arg.autotitle is False
1356+
finally:
1357+
client.app.dependency_overrides.clear()
1358+
1359+
12871360
def test_set_conversation_security_analyzer_success(
12881361
client,
12891362
sample_conversation_id,

0 commit comments

Comments
 (0)