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>
…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>
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>
…-client-protocol-factories core: a protocol can be registered per client, and a client owns what it creates
Four defects in scripts/extract_required_docs.py, each surfacing as a wrong page in the generated spec (cubic, utcp-specification #66): - A prose line containing a colon that is not a 'name: description' pair was silently DROPPED: '(note: ...)' sentences, quoted URLs after '(e.g.', anything with a colon mid-sentence. Such a line is now ordinary text. - Any line ending in a colon became a title-cased section header, code spans included ('Inheritance is controlled by `inherit_env_vars`:' -> '**Inheritance Is Controlled By `Inherit_Env_Vars`**'; 'def tool1():' inside an example -> '**Def Tool1()**'). A header is now a known Google-style one or a short title of words, digits, spaces and hyphens. - A docstring with no section header at all was never flushed, so 62 REQUIRED docstrings across the spec rendered as '*No ... documentation available*' (UtcpClient, every auth serializer, the plugin loader, all socket methods, ...). The bogus headers above had been flushing some of them by accident, which the fix exposed. - Cross-reference links were inserted inside inline code spans, where Markdown shows them as literal brackets. Links now stop at code spans; the field-list placeholder backticks are unwrapped before the pass so fields keep their links. Also: a class whose own docstring is not REQUIRED but whose methods are now renders those methods (the index already counted them), and index links are POSIX paths on every platform. Two docstrings corrected on the way: the CLI template claimed tool_args are 'shell-quoted' - the mechanism is per-invocation environment variables, as the same docstring explains - and 'OAuth2Auth.cache_key' is now written so the class links and cache_key stays code. Regenerated against the published pages: 43 pages change; every removed line is a placeholder, a bogus header, a link-in-code-span or a mangled example fragment; 360 lines of previously invisible documentation come back. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three findings from cubic on #110, each a heuristic that was right on the cases in front of me and wrong one step over: - The inline-code splitter closed a span at the first backtick run, so ``a`b`` ended at the inner tick and class names still inside the span were linked. A span now closes only on a run of the same length. - A bare URL line ("https://example.com") still parsed as a definition: "https" is short, alphanumeric and has no space. A definition has a space after its colon; a URL has "//". That is the whole difference, so it is the rule. - The header heuristic (short, words only) admitted "Use the following:" and rejected "Return Values (Complex):" or anything over 41 chars. A custom header is a Title-Cased line ending in a colon: every word starts with a capital or a digit, no code span, any length. Sentence- case captions ("Basic command step:") now stay under their real "Examples" header instead of replacing it. Unit-checked on the boundary cases; regenerated and compared with the previous run -- the only movement is captions becoming paragraphs under the section header they belong to. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ractor-fidelity docs: the spec extractor renders what the docstrings say
# Conflicts: # plugins/communication_protocols/websocket/pyproject.toml
![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
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…lugins first Four findings from cubic on the release PR (#111): - create() rolled back the protocols it created but not the manuals it registered: when one manual of a batch fails after a sibling has saved, the sibling stayed in the tool repository -- which may be caller-supplied and shared. The manuals THIS attempt added are now deregistered before the owned protocols are closed. Only this attempt's: a manual already present before (whose re-registration fails as a duplicate without raising) is left alone. The manual-name rule lives in one helper now, so create() computes the same names register_manual will use. - The two registry-isolating fixtures swapped the class-level dicts before plugins had loaded. Plugins load lazily on the first create() of the session and register into whatever dict holds the attribute at that moment -- so a session whose first create() happened inside an isolated test wrote every shared instance into the throwaway dict and lost it on restore, silently unregistering http/sse/streamable_http for the rest of the session. Both fixtures load plugins first. - The extractor's inline-code scanner ran per line, so a span crossing a newline was linked inside. Everything between fences is now one segment for the scanner. - The README and two docstrings said utcp-http/utcp-gql keep a pooled HTTP session. They do not: every request opens its own session; the only shared state is the OAuth token cache. Said so. Tests: a manual registered by a failed create() is removed from the caller's repository; a manual present before the attempt survives it; the multi-line span case is unit-checked. Mutation-checked. Core 56/56. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…protocols even when cancelled Two findings from cubic on #112, both about the rollback added there. It inferred "what this attempt registered" from repository state -- names absent before, present after -- which is wrong on a shared repository: a manual another client registered in the meantime looks identical to one of ours, and the rollback would delete it. The batch registration now appends each registered name to a list AS the registration succeeds (_register_each, shared with register_manuals), and create() rolls back exactly that list. A name another client took first fails our registration as a duplicate and is never on the list. And the cleanup was not cancellation-safe: a CancelledError landing in the rollback bypassed the close, leaking the factory-created protocols. The close now runs in a finally after the rollback, so a cancellation still closes what this client created and then reaches the caller, as a cancellation must. Tests: a manual another client registered during the attempt survives the rollback (the other client registers from inside the batch, so the interleaving is deterministic); cancelling create() while its rollback is blocked in deregistration still closes the owned protocol and the caller sees the CancelledError. Mutation-checked: inferring from the repository again fails exactly the two "survives" tests; moving the close out of the finally fails exactly the cancellation test. Core 59/59. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ase-review-111 core/docs: a failed create() leaves no manual behind; fixtures load plugins first
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.
Summary by cubic
Adds per-client ownership for stateful communication protocols. Previously all protocols used process-wide instances and clients had no
close(); factory-registered protocols now get isolated instances and teardown, while shared protocols keep their existing behavior and remain process-owned.New Features
register_communication_protocol_factory, with factory instances taking precedence and late registrations adopted on first use.UtcpProtocolCloseError; every close attempt is awaited and failures are raised together.create()calls close the protocols this client made and deregister only the manuals this attempt registered, even when cancelled.utcp-mcpandutcp-websocketto per-client factories; HTTP-based protocols remain shared.utcp,utcp-mcp, andutcp-websocketto1.2.0.Refactors
Written for commit e120001. Summary will update on new commits.