Skip to content

Commit 7422fc5

Browse files
Add conversation_id to automation completion callback (#3012)
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 468d098 commit 7422fc5

5 files changed

Lines changed: 152 additions & 0 deletions

File tree

openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,8 @@ def __init__(
768768
)
769769
self._id = uuid.UUID(cid)
770770

771+
workspace.register_conversation(str(self._id))
772+
771773
# Initialize the remote state
772774
self._state = RemoteState(
773775
self._client,

openhands-sdk/openhands/sdk/workspace/remote/base.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,25 @@ def default_conversation_tags(self) -> dict[str, str] | None:
210210
Dictionary of tag key-value pairs, or None if no default tags.
211211
"""
212212
return None
213+
214+
def register_conversation(self, conversation_id: str) -> None:
215+
"""Register a conversation ID with this workspace.
216+
217+
Called by RemoteConversation after creation to associate the conversation
218+
with the workspace. Subclasses can override to track conversation IDs
219+
for callbacks or other purposes.
220+
221+
Args:
222+
conversation_id: The conversation ID to register
223+
"""
224+
# Default implementation is a no-op
225+
pass
226+
227+
@property
228+
def conversation_id(self) -> str | None:
229+
"""Get the most recently registered conversation ID.
230+
231+
Returns:
232+
The conversation ID if one has been registered, None otherwise.
233+
"""
234+
return None

openhands-workspace/openhands/workspace/cloud/workspace.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ class OpenHandsCloudWorkspace(RemoteWorkspace):
151151
_exposed_urls: list[dict[str, Any]] | None = PrivateAttr(default=None)
152152
_automation_callback_url: str | None = PrivateAttr(default=None)
153153
_automation_run_id: str | None = PrivateAttr(default=None)
154+
_conversation_id: str | None = PrivateAttr(default=None)
154155

155156
@property
156157
def default_conversation_tags(self) -> dict[str, str]:
@@ -796,6 +797,28 @@ def _send_settings_request(
796797

797798
return response
798799

800+
def register_conversation(self, conversation_id: str) -> None:
801+
"""Register a conversation ID with this workspace.
802+
803+
Called by RemoteConversation after creation to associate the conversation
804+
with the workspace. The conversation ID is included in the completion
805+
callback sent to the automation service.
806+
807+
Args:
808+
conversation_id: The conversation ID to register
809+
"""
810+
self._conversation_id = conversation_id
811+
logger.debug(f"Registered conversation: {conversation_id}")
812+
813+
@property
814+
def conversation_id(self) -> str | None:
815+
"""Get the registered conversation ID.
816+
817+
Returns:
818+
The conversation ID if one has been registered, None otherwise.
819+
"""
820+
return self._conversation_id
821+
799822
def __del__(self) -> None:
800823
self.cleanup()
801824

@@ -813,6 +836,9 @@ def _send_completion_callback(
813836
814837
Called by ``__exit__`` before ``cleanup()``. Does nothing when
815838
``AUTOMATION_CALLBACK_URL`` env var was not set.
839+
840+
Includes ``conversation_id`` in the payload if one was registered via
841+
``register_conversation()``.
816842
"""
817843
try:
818844
callback_url = self._automation_callback_url
@@ -829,6 +855,10 @@ def _send_completion_callback(
829855
if exc_val is not None:
830856
payload["error"] = str(exc_val)
831857

858+
# Include conversation_id if one was registered
859+
if self._conversation_id is not None:
860+
payload["conversation_id"] = self._conversation_id
861+
832862
try:
833863
headers = {"Authorization": f"Bearer {self.cloud_api_key}"}
834864
with httpx.Client(timeout=10.0) as cb_client:

tests/sdk/conversation/remote/test_remote_conversation.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,3 +1321,25 @@ def test_remote_conversation_execute_tool_not_implemented(self, mock_ws_client):
13211321
conversation.execute_tool("any_tool", mock_action)
13221322

13231323
assert "not yet supported for RemoteConversation" in str(exc_info.value)
1324+
1325+
@patch(
1326+
"openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient"
1327+
)
1328+
def test_remote_conversation_calls_register_conversation(self, mock_ws_client):
1329+
"""Test RemoteConversation.__init__ calls workspace.register_conversation."""
1330+
conversation_id = str(uuid.uuid4())
1331+
self.setup_mock_client(conversation_id=conversation_id)
1332+
1333+
mock_ws_instance = Mock()
1334+
mock_ws_client.return_value = mock_ws_instance
1335+
1336+
# Patch register_conversation at the class level to verify it gets called
1337+
with patch.object(RemoteWorkspace, "register_conversation") as mock_register:
1338+
# Create RemoteConversation - this should call register_conversation
1339+
_conversation = RemoteConversation(
1340+
agent=self.agent,
1341+
workspace=self.workspace,
1342+
)
1343+
1344+
# Verify register_conversation was called with the conversation ID
1345+
mock_register.assert_called_once_with(conversation_id)

tests/workspace/test_cloud_workspace.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,3 +579,79 @@ def test_callback_failure_does_not_raise(monkeypatch):
579579

580580
# Should not raise
581581
ws.__exit__(None, None, None)
582+
583+
584+
# --- conversation_id registration tests ---
585+
586+
587+
def test_register_conversation_sets_conversation_id():
588+
"""register_conversation sets the _conversation_id attribute."""
589+
ws = _make_local_workspace()
590+
591+
ws.register_conversation("conv-123")
592+
593+
assert ws._conversation_id == "conv-123"
594+
assert ws.conversation_id == "conv-123"
595+
596+
597+
def test_conversation_id_property_returns_none_initially():
598+
"""conversation_id property returns None when no conversation registered."""
599+
ws = _make_local_workspace()
600+
601+
assert ws.conversation_id is None
602+
603+
604+
def test_callback_includes_conversation_id_when_registered(monkeypatch):
605+
"""Callback payload includes conversation_id when registered."""
606+
monkeypatch.setenv("AUTOMATION_CALLBACK_URL", "https://svc.test/complete")
607+
monkeypatch.setenv("AUTOMATION_RUN_ID", "run-42")
608+
ws = _make_local_workspace()
609+
610+
# Register a conversation
611+
ws.register_conversation("conv-xyz")
612+
613+
mock_resp = MagicMock()
614+
mock_resp.status_code = 200
615+
616+
with patch("httpx.Client") as MockClient:
617+
mock_client = MagicMock()
618+
mock_client.post.return_value = mock_resp
619+
mock_client.__enter__ = MagicMock(return_value=mock_client)
620+
mock_client.__exit__ = MagicMock(return_value=False)
621+
MockClient.return_value = mock_client
622+
623+
ws.__exit__(None, None, None)
624+
625+
# Check the POST payload includes conversation_id
626+
mock_client.post.assert_called_once()
627+
payload = mock_client.post.call_args.kwargs["json"]
628+
assert payload["status"] == "COMPLETED"
629+
assert payload["run_id"] == "run-42"
630+
assert payload["conversation_id"] == "conv-xyz"
631+
632+
633+
def test_callback_omits_conversation_id_when_not_registered(monkeypatch):
634+
"""Callback payload omits conversation_id when not registered."""
635+
monkeypatch.setenv("AUTOMATION_CALLBACK_URL", "https://svc.test/complete")
636+
monkeypatch.setenv("AUTOMATION_RUN_ID", "run-42")
637+
ws = _make_local_workspace()
638+
639+
# Do not register a conversation
640+
641+
mock_resp = MagicMock()
642+
mock_resp.status_code = 200
643+
644+
with patch("httpx.Client") as MockClient:
645+
mock_client = MagicMock()
646+
mock_client.post.return_value = mock_resp
647+
mock_client.__enter__ = MagicMock(return_value=mock_client)
648+
mock_client.__exit__ = MagicMock(return_value=False)
649+
MockClient.return_value = mock_client
650+
651+
ws.__exit__(None, None, None)
652+
653+
# Check the POST payload does NOT include conversation_id
654+
mock_client.post.assert_called_once()
655+
payload = mock_client.post.call_args.kwargs["json"]
656+
assert payload["status"] == "COMPLETED"
657+
assert "conversation_id" not in payload

0 commit comments

Comments
 (0)