core: a protocol can be registered per client, and a client owns what it creates - #108
Merged
Merged
Conversation
… it creates A communication protocol registered in communication_protocols is one instance shared by every UtcpClient in the process, and so is any state it keeps. Right for a credential cache; wrong for connections: a caller creating a client per tenant, per user or per pooled connection was not actually isolating them, and no client could tear its own down — the client had no close() at all. New registry: communication_protocol_factories, filled through register_communication_protocol_factory. UtcpClient.create calls a factory once per client, and the client records the instance as OWNED. A factory wins over a shared instance of the same type, so a plugin migrates by moving its registration and callers change nothing. Shared instances are still looked up live, so late registration keeps working. Teardown is scoped to what the client owns. UtcpClient.close() (new, on the interface and the implementation) closes the owned instances and leaves shared ones to the process — every instance is closed even when one fails, then the failures are raised together as UtcpProtocolCloseError. create() adopts the factory instances inside a cleanup guard: nothing that needs closing is created in the constructor (it cannot await), a factory that raises part-way leaves the earlier instances closable, and any initialization failure closes them before re-raising, a failing cleanup being logged with the original error kept as the one the caller sees. CommunicationProtocol.close() is now part of the interface, a no-op by default, so the client can close any protocol uniformly. All of it carries REQUIRED docstrings, since the spec is generated from them. Tests (10, through the public surface): per-client routing, factory over shared, late shared registration, unknown type names both registries, close() scoped to own instance, close() waits for every instance, failed create closes what it created, a raising factory leaves earlier instances closed, shared survives a failed create, failing cleanup is reported. Each guard mutation-checked. Core 51/51, http 238, cli 62, text 11. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The MCP protocol holds live sessions and, for stdio, child processes. Registered as one shared instance, every client in the process dialled into one session cache and one client's close() drained everyone's. It now registers through register_communication_protocol_factory, so each UtcpClient gets its own instance, its own connections and its own teardown. The factory registry is a new core feature, so core goes to 1.2.0 and utcp-mcp to 1.2.0 requiring utcp>=1.2.0. MCP suite 48/48. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…te factory is adopted on first use Two findings from cubic on #108. register_manuals gathered its registrations first-failure-wins: when one raised UtcpVariableNotFound the caller got the failure while the sibling registrations were still running underneath it — and create(), which closes the client's protocols right after, would close them under a registration still in flight. Every registration now settles before the first failure is raised, so a caller holding the error holds a quiet client. The resolver consulted the factory registry only at creation, so a factory registered after a client existed was invisible to it while a shared instance registered late was not. Both registries are now consulted live: a late factory is adopted on first use, owned and closed like the rest. A type resolves the same way whenever it was registered. Tests: register_manuals raises only once every sibling has finished; a failed create closes its protocols only after every registration has finished; a late-registered factory is adopted on first use and owned. Each mutation-checked. Ownership 13/13, core 54/54, MCP 48/48, http 238. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The guidance said a protocol that holds connections registers a factory, while the WebSocket plugin — one live WebSocket per manual name and URL — still registered a shared instance: two clients registering the same manual used, and on deregistration closed, each other's connection, and one client's close() dropped everyone's. It now registers through register_communication_protocol_factory, so each client owns its connections. utcp-websocket goes to 1.2.0 requiring utcp>=1.2.0. Suite 38/38. The criterion itself was too blunt. 'Holds connections' would also sweep in the HTTP plugin's pooled aiohttp session, which is meant to be shared. The README, the interface docstring and the registry docstring now say what actually decides it: state that must not be shared between clients — sessions or connections keyed per manual, child processes, anything one client's use or close() would take away from another — goes in a factory; a credential cache or a pooled session stays an instance. Every shipped plugin is now on the side the criterion puts it: mcp and websocket are factories; http, sse, streamable_http, gql, cli, file, text, tcp and udp keep nothing per client and stay shared. Raised by cubic on #108. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference-implementation counterpart of typescript-utcp #52 and #54. The spec is generated from this repo's
REQUIREDdocstrings, so every new piece of surface carries one (checked withscripts/extract_required_docs.py).The gap
A protocol registered in
communication_protocolsis one instance shared by everyUtcpClientin the process, and so is any state it keeps. Right for a credential cache; wrong for connections. A caller creating a client per tenant, per user or per pooled connection was not actually isolating them — every client dialled into the same MCP session cache — and no client could tear its own connections down: the Python client had noclose()at all.The mechanism
communication_protocol_factories, filled throughregister_communication_protocol_factory(type, factory).UtcpClient.createcalls a factory once per client and the client records the instance as owned. A factory wins over a shared instance of the same type, so a plugin migrates by moving its registration and callers change nothing. Shared instances are still looked up live, so registering one after a client exists keeps working (existing tests rely on it).UtcpClient.close()(interface + implementation) closes the owned instances and leaves shared ones to the process. Every instance is closed even when one fails; the failures are then raised together asUtcpProtocolCloseError.create()adopts factories inside a cleanup guard. Nothing that needs closing is created in the constructor (it can't await). A factory that raises part-way leaves the earlier instances closable; any initialization failure closes them before re-raising; a failing cleanup is logged with the original error kept as the one the caller sees.CommunicationProtocol.close()joins the interface, a no-op by default, so the client can close any protocol uniformly.utcp-mcpregisters as a factory — sessions and stdio child processes belong to the client that opened them.utcp-httpstays shared: its OAuth cache is meant to be reused.Versions
Core →
1.2.0(new registry +close()on the client interface),utcp-mcp→1.2.0requiringutcp>=1.2.0.Tests
Ten, all through the public surface (
core/tests/client/test_client_protocol_ownership.py): per-client routing, factory over shared, late shared registration, unknown type names both registries,close()scoped to the own instance,close()waits for every instance even when one fails, failedcreate()closes what it created, a raising factory leaves earlier instances closed, a shared instance survives a failedcreate(), a failing cleanup is reported. Each guard mutation-checked — every mutation fails exactly its tests. Core 51/51, MCP 48/48, http 238, cli 62, text 11.🤖 Generated with Claude Code
Summary by cubic
Protocols can now be registered per client, so each
UtcpClientowns its protocol instances and can close them. Previously one protocol instance was shared by every client, which broke per-tenant isolation and left connections unclosed.Refactors
communication_protocol_factories;UtcpClient.createinstantiates each registered factory per client, and factories registered later are adopted and owned on first use.UtcpClient.close()to close owned protocol instances; shared instances remain process-wide.CommunicationProtocol.close()as an interface method with a no-op default.create()closes any owned instances before re-raising; close failures are collected inUtcpProtocolCloseError.register_manualswaits for every sibling registration to settle before raising, so a failedcreate()never closes a protocol another registration is still using.utcp-mcpandutcp-websocketnow register as factories;utcp-httpstays shared.Migration
register_communication_protocoltoregister_communication_protocol_factory.utcp,utcp-mcp, andutcp-websocketbecome 1.2.0, with the plugins requiringutcp>=1.2.0.Written for commit f4e87bd. Summary will update on new commits.