core/docs: a failed create() leaves no manual behind; fixtures load plugins first - #112
Merged
Merged
Conversation
…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>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…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>
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
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.
Into
dev, ahead of thedev → mainrelease (#111) — cubic's four findings on that PR.create()left a sibling manual behind on failureIt 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 (deregistration needs them open). 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 (
_sanitize_manual_name), socreate()computes the same namesregister_manualwill use.The registry-isolating fixtures could silently unregister every plugin
Both 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 if the session's firstcreate()ran inside an isolated test, every shared instance landed in the throwaway dict and was lost on restore. Both fixtures now callensure_plugins_initialized()first.Extractor: an inline code span crossing a newline was linked inside
The scanner ran per line and reset at each. Everything between fences is now handed to it as one segment.
Docs: no "pooled HTTP session"
utcp-httpandutcp-gqlopen a session per request; the only shared state is the OAuth token cache. README, interface docstring and registry docstring corrected.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 is unit-checked. Mutation-checked: dropping the rollback fails exactly the two rollback tests. Core 56/56.🤖 Generated with Claude Code
Summary by cubic
Failed
UtcpClient.create()previously rolled back its owned protocols but left manuals registered during the attempt; it now removes exactly those manuals before closing protocols. Rollback remains safe when cancelled, so shared repositories keep pre-existing or concurrently registered manuals and created protocols are still closed.Bug Fixes
Documentation
utcp-httpandutcp-gqlnow describe per-request sessions with shared OAuth token caches.Written for commit 47821be. Summary will update on new commits.