-
Notifications
You must be signed in to change notification settings - Fork 512
feat(tool-streams): add tool streams #1584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||||||||||||||||||||||||||||||
| # Copyright 2026 Dimensional Inc. | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||
| # You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||
| # See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||
| # limitations under the License. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||
| import uuid | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from dimos.core.transport import pLCMTransport | ||||||||||||||||||||||||||||||||||
| from dimos.utils.logging_config import setup_logger | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| logger = setup_logger() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| _TOOL_STREAM_TOPIC = "/tool_streams" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class ToolStream: | ||||||||||||||||||||||||||||||||||
| """A streaming channel for sending updates from a running skill to the agent. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Each `ToolStream` publishes messages on a shared LCM topic. The agent | ||||||||||||||||||||||||||||||||||
| (or the MCP server SSE endpoint) subscribes once and receives all updates. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def __init__(self, tool_name: str) -> None: | ||||||||||||||||||||||||||||||||||
| self.tool_name = tool_name | ||||||||||||||||||||||||||||||||||
| self.id = str(uuid.uuid4()) | ||||||||||||||||||||||||||||||||||
| self._closed = False | ||||||||||||||||||||||||||||||||||
| self._transport: pLCMTransport[dict[str, Any]] | None = None | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def start(self) -> None: | ||||||||||||||||||||||||||||||||||
| self._transport = pLCMTransport(_TOOL_STREAM_TOPIC) | ||||||||||||||||||||||||||||||||||
| self._transport.start() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def send(self, message: str) -> None: | ||||||||||||||||||||||||||||||||||
| if self._closed: | ||||||||||||||||||||||||||||||||||
| logger.error("Attempted to send on closed ToolStream", stream_id=self.id) | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| if self._transport is None: | ||||||||||||||||||||||||||||||||||
| logger.error("ToolStream transport not initialized", stream_id=self.id) | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| self._transport.broadcast( | ||||||||||||||||||||||||||||||||||
| None, | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| "stream_id": self.id, | ||||||||||||||||||||||||||||||||||
| "tool_name": self.tool_name, | ||||||||||||||||||||||||||||||||||
| "type": "update", | ||||||||||||||||||||||||||||||||||
| "text": message, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def stop(self) -> None: | ||||||||||||||||||||||||||||||||||
| if self._closed: | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| self._closed = True | ||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||
| self._transport.broadcast( | ||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if self._transport is None (calling stop after failed start) wouldn't the stop throw? The finally would clean up but still would let the exception out |
||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| "stream_id": self.id, | ||||||||||||||||||||||||||||||||||
| "tool_name": self.tool_name, | ||||||||||||||||||||||||||||||||||
| "type": "close", | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+67
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong argument count in
Suggested change
|
||||||||||||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||||||||||||
| if self._transport is not None: | ||||||||||||||||||||||||||||||||||
| self._transport.stop() | ||||||||||||||||||||||||||||||||||
| self._transport = None | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||
| def is_closed(self) -> bool: | ||||||||||||||||||||||||||||||||||
| return self._closed | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| __all__ = ["_TOOL_STREAM_TOPIC", "ToolStream"] | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition on
sse_queueslistapp.state.sse_queuesis a plainlistthat is mutated from multiple concurrent contexts without any synchronisation:streams_sse_endpointappends a queue, and theevent_generatorfinaliser removes it._on_tool_stream_message(called from the RxPY subscriber thread) iterates over the list withfor queue in app.state.sse_queues.Python's GIL protects individual atomic operations, but iterating over a list while another coroutine or thread appends/removes items from it can still raise
RuntimeError: list changed size during iterationor silently skip/double-process entries.Consider protecting mutations and the iteration with a
threading.Lock, or replacing the list with asetguarded by a lock, to make concurrent access safe.