EdClient adopts the thread default context instead of clobbering it#31
Open
dsrw wants to merge 1 commit into
Open
EdClient adopts the thread default context instead of clobbering it#31dsrw wants to merge 1 commit into
dsrw wants to merge 1 commit into
Conversation
EdClient.reconnect always minted a brand-new EdContext on first connect and overwrote Ed.thread_ctx, discarding whatever context the caller had already established on the thread. First connect now ADOPTS an existing context: a caller-supplied `ctx_override` if given, else the thread's default (Ed.thread_ctx). The client inherits that context's id as its stable identity. It only mints a private context when it can't adopt -- the caller pinned an `id` the default doesn't carry (backward-compatible with id-driven callers), or the only default available is a spent, closed context. A genuine reconnect (the client already has a live context) still mints a FRESH context under the same stable id: an existing object's CREATE never re-broadcasts, so reusing a context would strand this client's objects as ghosts a restarted peer never learns about. Either way Ed.thread_ctx ends up pointing at the live context. Adds EdContext.closed (set by close) so a spent thread default is never re-adopted, and Ed.thread_ctx_or_nil to probe the thread default without forcing one into being.
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.
What
EdClient.reconnectalways minted a brand-newEdContexton first connect and assigned it toEd.thread_ctx, discarding whatever context the caller had already established on the thread. This makes the client respect the thread's default context by default, with an opt-in override for multi-context setups.Behavior
First connect now ADOPTS an existing context rather than minting one and clobbering the thread pointer:
ctx_override(new optional field) if the caller supplied one; elseEd.thread_ctx).The client inherits the adopted context's
idas its stable identity, so later reconnects reuse it. It falls back to minting a private context only when it genuinely can't adopt:idthat the thread default doesn't carry (keeps id-driven callers — e.g. the MCP server — byte-for-byte backward compatible: with no pre-existing matching default they mint under their own id, exactly as before), orclosed context (must not be resurrected).First-connect vs reconnect (the trap)
A genuine reconnect (the client already holds a live context) must NOT reuse it: an existing object's CREATE never re-broadcasts, so reusing the context would strand this client's objects as ghosts a restarted peer never learns about. So the reconnect path is unchanged — it still mints a fresh context under the same stable
id, and the peer supersedes the prior session by that id.The two are cleanly separated by whether
self.ctxis already set:self.ctx == nil→ first connect → adopt (or mint only if un-adoptable).self.ctx != nil→ reconnect → always mint fresh under the same id.Either way
Ed.thread_ctxends up pointing at the client's live context (a no-op assignment in the adopt-the-default case), soon_connectand helpers that readEd.thread_ctxsee the right one.Supporting changes
EdContext.closed— set byclose; lets first-connect refuse to re-adopt a spent thread default (whose objects would otherwise linger as ghosts).Ed.thread_ctx_or_nil— probes the thread default without auto-creating one, so adoption can tell "a default exists" from "force one into being".Backward compatibility
No caller changes required. Existing callers that pass a stable
id(Enu'sEnu.client, the MCP server, demos) and ed's own tests behave identically: with no pre-established matching thread default they still mint a context under their own id on first connect. Adoption only activates when a context is genuinely already present (or an override is passed).Tests
tests/client_tests.nim: first connect adopts a pre-setEd.thread_ctx(and doesn't clobber it) while a subsequent reconnect still mints fresh;ctx_overridewins over the thread default; an explicit id the default can't provide mints its own context.tests/tests.nim+tests/threading_tests.nimsuites pass. Verified downstream by building and smoke-testing the Enu client end to end (connect to a live headless Enu, query units).🤖 Generated with Claude Code