Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ await client.close()

A protocol plugin registers itself in one of two registries, and the choice decides who its state belongs to.

An instance registered with `register_communication_protocol` is **shared by every `UtcpClient` in the process**, and so is any state it keeps. That is right for state that is *meant* to be shared — a credential cache, a pooled HTTP session, a registry a decorator writes into — and wrong for state that belongs to one client. A shared instance lives as long as the process; no client closes it.
An instance registered with `register_communication_protocol` is **shared by every `UtcpClient` in the process**, and so is any state it keeps. That is right for state that is *meant* to be shared — a credential cache, a registry a decorator writes into — and wrong for state that belongs to one client. A shared instance lives as long as the process; no client closes it.

A protocol whose state **must not be shared between clients** registers a **factory** instead: live sessions or connections keyed per manual, child processes — anything one client's use or `close()` would take away from another. Each `UtcpClient` calls the factory once — at creation, or on first use if it was registered later — so each client gets its own instance, its own connections, and its own teardown on `close()`:

Expand All @@ -95,7 +95,7 @@ register_communication_protocol_factory("custom_type", CustomCommunicationProtoc

This is what makes "a client per tenant / per user / per pooled connection" actually isolate them, rather than every client reaching into one shared instance. A type registered as a factory wins over the same type registered as an instance, so a plugin migrates by moving its registration from one call to the other and callers change nothing.

`utcp-mcp` and `utcp-websocket` register this way — MCP sessions (and, for stdio, child processes) and per-manual WebSocket connections belong to the client that opened them. `utcp-http` (HTTP, SSE, streamable HTTP) and `utcp-gql` stay shared instances: what they keep is an OAuth token cache and a pooled HTTP session, both meant to be reused across clients.
`utcp-mcp` and `utcp-websocket` register this way — MCP sessions (and, for stdio, child processes) and per-manual WebSocket connections belong to the client that opened them. `utcp-http` (HTTP, SSE, streamable HTTP) and `utcp-gql` stay shared instances: the only state they keep is an OAuth token cache, which is meant to be reused across clients; every request opens its own session.

`client.close()` closes the instances the client created — every one of them, even if one fails, after which the failures are raised together as `UtcpProtocolCloseError`. If `UtcpClient.create` fails after creating them, they are closed the same way before the error is re-raised.

Expand Down
4 changes: 2 additions & 2 deletions core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ await client.close()

A protocol plugin registers itself in one of two registries, and the choice decides who its state belongs to.

An instance registered with `register_communication_protocol` is **shared by every `UtcpClient` in the process**, and so is any state it keeps. That is right for state that is *meant* to be shared — a credential cache, a pooled HTTP session, a registry a decorator writes into — and wrong for state that belongs to one client. A shared instance lives as long as the process; no client closes it.
An instance registered with `register_communication_protocol` is **shared by every `UtcpClient` in the process**, and so is any state it keeps. That is right for state that is *meant* to be shared — a credential cache, a registry a decorator writes into — and wrong for state that belongs to one client. A shared instance lives as long as the process; no client closes it.

A protocol whose state **must not be shared between clients** registers a **factory** instead: live sessions or connections keyed per manual, child processes — anything one client's use or `close()` would take away from another. Each `UtcpClient` calls the factory once — at creation, or on first use if it was registered later — so each client gets its own instance, its own connections, and its own teardown on `close()`:

Expand All @@ -95,7 +95,7 @@ register_communication_protocol_factory("custom_type", CustomCommunicationProtoc

This is what makes "a client per tenant / per user / per pooled connection" actually isolate them, rather than every client reaching into one shared instance. A type registered as a factory wins over the same type registered as an instance, so a plugin migrates by moving its registration from one call to the other and callers change nothing.

`utcp-mcp` and `utcp-websocket` register this way — MCP sessions (and, for stdio, child processes) and per-manual WebSocket connections belong to the client that opened them. `utcp-http` (HTTP, SSE, streamable HTTP) and `utcp-gql` stay shared instances: what they keep is an OAuth token cache and a pooled HTTP session, both meant to be reused across clients.
`utcp-mcp` and `utcp-websocket` register this way — MCP sessions (and, for stdio, child processes) and per-manual WebSocket connections belong to the client that opened them. `utcp-http` (HTTP, SSE, streamable HTTP) and `utcp-gql` stay shared instances: the only state they keep is an OAuth token cache, which is meant to be reused across clients; every request opens its own session.

`client.close()` closes the instances the client created — every one of them, even if one fails, after which the failures are raised together as `UtcpProtocolCloseError`. If `UtcpClient.create` fails after creating them, they are closed the same way before the error is re-raised.

Expand Down
123 changes: 81 additions & 42 deletions core/src/utcp/implementations/utcp_client_implementation.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@

logger = logging.getLogger(__name__)


def _sanitize_manual_name(name: str) -> str:
"""The name a manual is registered under: every non-word character becomes an underscore."""
return re.sub(r'[^\w]', '_', name)


class UtcpClientImplementation(UtcpClient):
"""REQUIRED
Implementation of the `UtcpClient` interface.
Expand Down Expand Up @@ -151,9 +157,14 @@ async def create(
client = cls(config, DefaultVariableSubstitutor(), root_dir)

# Everything from here on can fail, and the caller never receives a
# client it could close — so whatever this client CREATED is closed
# here before the failure is re-raised. Only what it created: the shared
# instances are in use by every other client and are not this one's.
# client it could close — so whatever this client CREATED is undone
# here before the failure is re-raised: the manuals this attempt
# registered are removed (the tool repository may be caller-supplied
# and shared), then the protocol instances it created are closed.
# Exactly what it registered — recorded as each registration
# succeeds, never inferred from repository state, which another client
# sharing the repository may have changed in the meantime.
registered_by_this_attempt: List[str] = []
try:
client._adopt_factory_protocols()

Expand All @@ -165,15 +176,29 @@ async def create(

# Load the manuals if any
if config.manual_call_templates:
await client.register_manuals(config.manual_call_templates)
results = await client._register_each(config.manual_call_templates, registered_by_this_attempt)
failures = [result for result in results if isinstance(result, BaseException)]
if failures:
raise failures[0]
except BaseException:
# Rollback first (deregistration needs the protocols open), then
# close — in a finally, so a cancellation that lands during the
# rollback still closes what this client created and then reaches
# the caller, as a cancellation must.
try:
await client._close_owned_protocols()
except Exception:
# The initialization failure stays the error the caller sees;
# the cleanup failure is reported rather than swallowed, because
# the caller has no client through which to retry it.
logger.error("UtcpClient.create failed, and closing the protocols it had created failed too", exc_info=True)
for name in registered_by_this_attempt:
try:
await client.deregister_manual(name)
except Exception:
logger.error(f"UtcpClient.create failed, and removing the manual '{name}' it had registered failed too", exc_info=True)
finally:
try:
await client._close_owned_protocols()
except Exception:
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
# The initialization failure stays the error the caller sees;
# the cleanup failure is reported rather than swallowed, because
# the caller has no client through which to retry it.
logger.error("UtcpClient.create failed, and closing the protocols it had created failed too", exc_info=True)
raise

return client
Expand Down Expand Up @@ -204,7 +229,7 @@ async def register_manual(self, manual_call_template: CallTemplate) -> RegisterM
ValueError: If manual name is already registered or communication protocol is not found.
"""
# Replace all non-word characters with underscore
manual_call_template.name = re.sub(r'[^\w]', '_', manual_call_template.name)
manual_call_template.name = _sanitize_manual_name(manual_call_template.name)
if await self.config.tool_repository.get_manual(manual_call_template.name) is not None:
raise ValueError(f"Manual {manual_call_template.name} already registered, please use a different name or deregister the existing manual")
manual_call_template = self._substitute_call_template_variables(manual_call_template, manual_call_template.name)
Expand Down Expand Up @@ -247,41 +272,55 @@ async def register_manuals(self, manual_call_templates: List[CallTemplate]) -> L
Returns:
A list of `RegisterManualResult` instances representing the results of the registration.
"""
# Create tasks for parallel CallTemplate registration
tasks = []
for manual_call_template in manual_call_templates:
async def try_register_manual(manual_call_template=manual_call_template):
try:
result = await self.register_manual(manual_call_template)
if result.success:
logger.info(f"Successfully registered manual '{manual_call_template.name}' with {len(result.manual.tools)} tools")
else:
logger.error(f"Error registering manual '{manual_call_template.name}': {result.errors}")
return result
except UtcpVariableNotFound as e:
raise e
except Exception as e:
logger.error(f"Error registering manual '{manual_call_template.name}': {traceback.format_exc()}")
return RegisterManualResult(
manual_call_template=manual_call_template,
manual=UtcpManual(manual_version="0.0.0", tools=[]),
success=False,
errors=[traceback.format_exc()]
)

tasks.append(try_register_manual())

# Wait for EVERY registration to settle, even when one raises. The
# caller receives the failure only once no sibling registration is
# still running underneath it — create(), for one, closes the
# protocols right after, and a registration still in flight would be
# using a closed one.
results = await asyncio.gather(*tasks, return_exceptions=True)
results = await self._register_each(manual_call_templates)
failures = [result for result in results if isinstance(result, BaseException)]
if failures:
raise failures[0]
return [p for p in results if p is not None]

async def _register_each(
self,
manual_call_templates: List[CallTemplate],
registered_names: Optional[List[str]] = None,
) -> List[Union[RegisterManualResult, BaseException]]:
"""Register every template in parallel and wait for EVERY one to settle.

A raised failure is returned in place of its result rather than
propagated at once, so no sibling registration is still running
underneath a caller that holds the failure — create(), for one, closes
the protocols right after, and a registration still in flight would be
using a closed one.

When ``registered_names`` is given, the registered name of each manual
that succeeded is appended to it AS it succeeds, so a caller that
fails part-way knows exactly what it registered — and only that.
"""
async def try_register_manual(manual_call_template: CallTemplate):
try:
result = await self.register_manual(manual_call_template)
if result.success:
if registered_names is not None:
registered_names.append(manual_call_template.name)
logger.info(f"Successfully registered manual '{manual_call_template.name}' with {len(result.manual.tools)} tools")
else:
logger.error(f"Error registering manual '{manual_call_template.name}': {result.errors}")
return result
except UtcpVariableNotFound as e:
raise e
except Exception as e:
logger.error(f"Error registering manual '{manual_call_template.name}': {traceback.format_exc()}")
return RegisterManualResult(
manual_call_template=manual_call_template,
manual=UtcpManual(manual_version="0.0.0", tools=[]),
success=False,
errors=[traceback.format_exc()]
)

return await asyncio.gather(
*(try_register_manual(manual_call_template) for manual_call_template in manual_call_templates),
return_exceptions=True,
)

async def deregister_manual(self, manual_name: str) -> bool:
"""REQUIRED
Deregister a manual from the client.
Expand Down Expand Up @@ -426,7 +465,7 @@ async def get_required_variables_for_manual_and_tools(self, manual_call_template
Returns:
A list of required variables for the manual and its tools.
"""
manual_call_template.name = re.sub(r'[^\w]', '_', manual_call_template.name)
manual_call_template.name = _sanitize_manual_name(manual_call_template.name)
variables_for_CallTemplate = self.variable_substitutor.find_required_variables(CallTemplateSerializer().to_dict(manual_call_template), manual_call_template.name)
if len(variables_for_CallTemplate) > 0:
try:
Expand Down
2 changes: 1 addition & 1 deletion core/src/utcp/interfaces/communication_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class CommunicationProtocol(ABC):
- `communication_protocols` holds an INSTANCE that is shared by every
`UtcpClient` in the process, and so is any state it keeps. That is the
right home for state that is meant to be shared (a credential cache, a
pooled HTTP session, a registry a decorator writes into). The instance
registry a decorator writes into). The instance
lives as long as the process that registered it; no client closes it.
- `communication_protocol_factories` holds a FACTORY, for a protocol whose
state must not be shared between clients: live sessions or connections
Expand Down
4 changes: 2 additions & 2 deletions core/src/utcp/plugins/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ def register_communication_protocol_factory(communication_protocol_type: str, fa
Use this instead of `register_communication_protocol` for a protocol whose
state must not be shared between clients: live sessions or connections
keyed per manual, child processes — anything one client's use or `close()`
would take away from another. (A credential cache or a pooled HTTP session
is meant to be shared and stays an instance.) Each `UtcpClient` calls the factory once — at
would take away from another. (A credential cache is meant to be shared
and stays an instance.) Each `UtcpClient` calls the factory once — at
creation, or on first use if the factory is registered later — and that
client's `close()` tears the instance down. A type registered as a factory
wins over the same type registered as an instance.
Expand Down
Loading
Loading