feat!: backend-owned project lifecycle over stable per-project channels - #88
Open
gmaclennan wants to merge 9 commits into
Open
feat!: backend-owned project lifecycle over stable per-project channels#88gmaclennan wants to merge 9 commits into
gmaclennan wants to merge 9 commits into
Conversation
Per-project channels are now keyed by the project's public id — stable
across close/re-open cycles and server restarts — instead of a
per-open-lifetime instance id. The server owns instance lifecycle
behind the channel: a per-project host re-opens the project on demand
when a message arrives for a closed instance, buffers calls during the
open, and replays recorded event subscriptions into the fresh
rpc-reflector server (server-side subscriptions die with each
per-instance server), so client references and listeners survive
server-side closes they never hear about.
Left projects are the one deliberate exception: core re-opens a left
project as a live-but-gutted instance, so the host checks
listProjects({ includeLeft: true }) before opening and rejects with a
new ProjectLeftError until the project is re-joined. A leaveProject
call routed through the server also closes the gutted instance core
leaves cached (core only cleans that up inside addProject).
Client side this deletes the lifecycle machinery that has churned
through five PRs (#47, #49, #61, #74, #83/#84): instance-id
revalidation on every getProject, close interception, close-event
eviction, closed proxies, and ProjectClosedError. project.close() is
removed from the reflected surface — the client has no lifecycle verbs
at all — and project references are permanent. Server-side, the
instance-id mint, tombstone set, and closed-project stub are replaced
by the host. This also removes the unsound instance-id currency check
across server restarts (ids came from a counter a restarted server
could remint), which previously forced restart recovery to hard-close
every cached wrapper.
Unsubscribe methods (off / removeListener / removeAllListeners) on a
closed client are now chainable no-ops instead of throwing: removing a
listener from a dead client is correct teardown (React effect cleanup
runs against stale references). Subscribe methods still throw.
BREAKING CHANGE: project.close() is gone from the client surface;
ProjectClosedError is removed and ProjectLeftError added; getProject
never returns a new reference for a previously-fetched id.
gmaclennan
added a commit
to digidem/rpc-reflector
that referenced
this pull request
Aug 15, 2026
The @comapeo/ipc transport-reset design that needed it (hard-closing project wrappers and firing their 'close' event locally, digidem/comapeo-ipc#87) has been superseded by backend-owned project lifecycle (digidem/comapeo-ipc#88/#89), where project references survive a server restart and nothing emits locally. Reverts d64b36d rather than rewriting history; a squash merge lands this PR with no trace of the API.
RangerMauve
requested changes
Aug 17, 2026
| // `NotFoundError` / `ProjectLeftError`) rather than on the first method | ||
| // call, and so the server opens the project eagerly. The returned | ||
| // wrapper is the same object across calls. | ||
| await projectRoutingClient.assertProjectExists(projectPublicId) |
Contributor
There was a problem hiding this comment.
Do we really need this anymore?
| projectChannel.start() | ||
|
|
||
| return instanceId | ||
| const managerRequestHook = (request, next) => { |
Contributor
There was a problem hiding this comment.
I don't think we should be adding project leave specific logic to IPC, What is the purpose of this? If we have some specific behaviour I think it should go in core. We should probably listen for close still and know to call getProiject1 again.
Contributor
There was a problem hiding this comment.
still don't think we should be adding project leave logic here
| * - **open** — an rpc-reflector server for the current instance is bound to | ||
| * the channel; the host only records subscription state. | ||
| * | ||
| * Subscription tape: the host snoops ON/OFF frames on the channel and keeps |
Contributor
There was a problem hiding this comment.
If the process with the server side is dying anyway how is this info getting preserved? I think re-subscribe logic should live entirely on the client since it knows when it reconnects.
gmaclennan
added a commit
to digidem/rpc-reflector
that referenced
this pull request
Aug 19, 2026
* feat: add createClient.rejectPending and createClient.resubscribe Give the transport owner hooks to recover from a server restart without tearing down the client: rejectPending(client, error) fails every in-flight call fast with a caller-supplied (distinguishable) error while keeping the client usable, and resubscribe(client) replays an ON message for every event (root and nested sub-clients) that still has listeners, restoring subscriptions a restarted server has lost. Both are no-ops on a closed client. * feat: add createClient.emitLocal for local-only event delivery Review follow-up: give the transport owner a way to deliver an event that a dead server can no longer send (e.g. firing 'close' teardown listeners when hard-closing a client after the server process died). Listeners are stored under encoded names (propArray + eventName), so a plain emit on the client cannot reach them; emitLocal encodes the root propArray and emits to locally-registered root listeners only, with no wire traffic. No-op (returning false) on a closed client. Also drop the unreachable listenerCount guard in resubscribe: eventemitter3's eventNames() only lists events with listeners. * revert: drop createClient.emitLocal — no remaining consumer The @comapeo/ipc transport-reset design that needed it (hard-closing project wrappers and firing their 'close' event locally, digidem/comapeo-ipc#87) has been superseded by backend-owned project lifecycle (digidem/comapeo-ipc#88/#89), where project references survive a server restart and nothing emits locally. Reverts d64b36d rather than rewriting history; a squash merge lands this PR with no trace of the API. * docs: document createClient.rejectPending and createClient.resubscribe * test: cover rejectPending and resubscribe via the public API The tests for these two methods asserted on the over-the-wire message shape on the MessagePort — sniffing msgIds off REQUEST messages to forge a late RESPONSE, and deep-comparing the ON messages resubscribe() emits. Rewrite them against a real client/server pair following the pattern in e2e.test.js, so they run over both a real MessageChannel and the MessagePort-like fake. The late-response case now has the server answer a call that rejectPending() has already given up on, and resubscribe() is checked by restarting the server and asserting the server-side emitters regain exactly the expected listeners and that events reach the client again.
This was referenced Aug 20, 2026
Closed
…d handlers The per-project rpc-reflector server is now created with a handler factory (rpc-reflector 4.5.0): rpc-reflector owns path walking, the subscription registry, and re-attaching subscriptions to each fresh instance before any awaited frame dispatches. The ProjectHost keeps only the comapeo-specific parts: the interim left-project guard, the close-in-flight retry around manager.getProject, and detachHandler() on the instance's close event. Deletes the project facade, the host-side subscription registries, and unreferenced logger groundwork. Requires rpc-reflector ^4.5.0.
getProject validates a project id only on first acquisition; after one success the permanent wrapper is returned directly. Concurrent first calls still share one validation, a failed lookup is still retried, and a project left after acquisition rejects on method calls over the wire.
notifyTransportReset(client) rejects every in-flight call — manager, project routing, per-project, or services — with rpc-reflector's ChannelClosedError (RPC_CHANNEL_CLOSED); resubscribe(client) replays event subscriptions once the transport is reconnected. Both accept the core client or the services client, are safe to call repeatedly, and no-op on a closed client. The phases stay split: ON frames written into a down transport can nudge the native connection into reconnect loops. A replayed project subscription re-opens the project via the host's late-bound handler factory.
Rewrites the lifecycle section around rpc-reflector's late-bound handlers, documents notifyTransportReset/resubscribe and the two-phase recovery rule, and notes the interim left-guard/leaveProject-hook pair (digidem/comapeo-core#1313).
gmaclennan
marked this pull request as draft
August 20, 2026 17:50
gmaclennan
marked this pull request as ready for review
August 20, 2026 17:50
Review fixes: the project host factory re-checks leftness after the open resolves, closing the window where a racing leave (which can take up to its sync wait) would bind calls to the gutted instance. The client close routine now flags closing at its start and getProject re-checks after its validation round trip, so a wrapper can no longer be minted after the sweep (leaking a port listener and an unclosed rpc client) — such calls reject with ClientClosedError. Nested closed proxies return undefined for 'then' so awaiting a namespace after close resolves instead of hanging on a thenable that never settles. README: document the server/client opts params (including the interim leave hook wrapping a consumer onRequestHook) and the rpc-reflector 4.5.0 release-order requirement.
FakeProject now mirrors ready-resource (async close with a closing promise, closed flag after) and core's double close emit, with a holdClose gate for close-in-flight windows. New cells: leave racing an in-flight open rejects with ProjectLeftError; getProject during client close rejects with ClientClosedError; awaiting a nested namespace after close does not hang; double/stray close emits are harmless; an open during a close-in-flight waits it out and binds the fresh instance; the retry gives up after repeated closes; re-invite after leaving a never-acquired project succeeds; parallel first getProject shares one validation round trip; a consumer onRequestHook composes with the interim leave hook; a failed leaveProject still triggers the cleanup close. The never-validated-reference test now asserts the per-request error frame actually arrives on the unknown project channel.
gmaclennan
force-pushed
the
feat/backend-owned-lifecycle
branch
from
August 20, 2026 18:02
155e501 to
d219b40
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
RangerMauve
requested changes
Aug 26, 2026
| // `resubscribe` once the transport is back up. Project references stay | ||
| // valid: their channels are keyed by project id, which a restarted | ||
| // server serves identically. | ||
| createClient.rejectPending(managerClient, new RpcChannelClosedError()) |
Contributor
There was a problem hiding this comment.
minor nit, it's a bit strange to have methods on a function. Is it possible to have something like managerClient.rejectPending instead? Or use a symbol if we really don't want to use regular methods.
| projectChannel.start() | ||
|
|
||
| return instanceId | ||
| const managerRequestHook = (request, next) => { |
Contributor
There was a problem hiding this comment.
still don't think we should be adding project leave logic here
| assert.equal(again, project, 'cached wrapper returned without validation') | ||
| }) | ||
|
|
||
| test('leaveProject: calls reject with ProjectLeftError and the gutted instance is closed', async (t) => { |
Contributor
There was a problem hiding this comment.
this should really only be a concern for core.
| // layer itself (the host detaches rpc-reflector's handler when the instance | ||
| // closes; the subscription registry holds no instance references). | ||
| // | ||
| // Runs only when `global.gc` is available (npm test passes --expose-gc). |
Contributor
There was a problem hiding this comment.
We should set this flag in tests.
| ) | ||
| }) | ||
|
|
||
| test('project.close is not exposed on the client surface', async (t) => { |
Contributor
There was a problem hiding this comment.
Remind me again why the frontend should not be allowed to initiate a close?
| ) | ||
| }) | ||
|
|
||
| test('Gives up after the project keeps closing while opening', async (t) => { |
Contributor
There was a problem hiding this comment.
What edge case is this trying to catch? did we actually see this in the wild?
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
2 participants
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.
Rewrites project-instance lifecycle so the backend owns it entirely. Per-project channels are keyed by
projectPublicId— stable across instance close/re-open cycles and across backend restarts — and the client-side project reference is permanent:getProjectalways returns the same wrapper, and the backend closes and re-opensMapeoProjectinstances behind it invisibly (leave/re-join today, memory eviction later).This replaces v9's per-open instance ids and the client-side machinery they required (revalidation on every
getProject, close-event cache eviction, closed proxies, server-side tombstones). Each of those was a fix for the previous one's race, and the class of bug is structural: any client-visible instance lifecycle makes every client-side cache of a reference a stale-reference hazard. The v9 design is also unsound across a server restart — instance ids come from a per-connection counter, so a restarted (or merely reconnected) server can remint the exact id a cached wrapper is bound to, and revalidation falsely passes. Stable channels remove the whole class.Server
The server no longer inspects wire frames or replays recorded subscriptions. Each project channel gets one long-lived rpc-reflector server created with a handler factory (rpc-reflector's late-bound handlers, digidem/rpc-reflector#55): the factory guards against left projects, then opens via
manager.getProjectwith a retry that waits out core's close-in-flight window;project.once('close')detaches the handler, returning the channel to dormant. rpc-reflector keeps the subscription registry across instance swaps and re-attaches it before dispatching the work that triggered a re-open, so a call's events can never be missed. Subscribing alone wakes a dormant project — which is what lets a listen-only client resume transparently after a backend restart. The routing channel'sassertProjectExiststriggers an eager open (ensureHandler) so events emitted before the first project-channel frame are not lost.Two pieces are explicitly interim, removed together when comapeo-core ships a typed left-project error (digidem/comapeo-core#1313): the left-guard in the factory, and the
leaveProjectrequest hook that closes the gutted instance core leaves cached. Once core owns left-state, this package carries zero leave-specific logic.Client
project.close()is no longer exposed — a client cannot close an instance out from under other consumers.getProjectvalidates existence over the wire only on first acquisition per project; afterwards the permanent wrapper is returned with no round trip. A project left after acquisition keeps resolving the same wrapper, whose prohibited calls reject withProjectLeftError(PROJECT_LEFT); first acquisition of a left project rejects. Re-invite is invisible: the same reference simply works again.Transport reset
Two new exports support process-restart recovery (previously #87/#89, folded in here):
notifyTransportReset(client)rejects every pending call — manager, services, and all project clients — with rpc-reflector'sChannelClosedError(RPC_CHANNEL_CLOSED), andresubscribe(client)re-sends subscriptions after the transport reconnects. The two-phase rule matters: reject at drop time, resubscribe only once reconnected (resubscribing into a down transport feeds the native reconnect a hot loop). Under stable channels this is the entire restart story — no wrapper invalidation, no generation guards.Breaking changes
project.close()removed;ProjectClosedErrorremoved;ProjectLeftError(PROJECT_LEFT) added;getProjectreturns a permanent reference with the semantics above. Requires rpc-reflector ≥4.5.0 (digidem/rpc-reflector#55). Until that releases, the dependency is git-pinned to the PR branch commit so CI installs and runs the real thing; swapping the pin back to^4.5.0and regenerating the lockfile is part of the release checklist.Testing
66 tests. The lifecycle suite pins the contract from the outside (public client + a fake manager that models core's real behavior, including its double
closeemit and async closing/closed windows): calls transparently survive backend-side closes, subscriptions — including nested$sync— survive close/re-open, subscribe-alone re-opens, leave →PROJECT_LEFT→ re-invite revives the same reference, parallel first acquisition costs one round trip, and repeated close/re-open cycles retain zero instances (WeakRef under--expose-gc). The transport-reset suite covers reject-at-drop and resubscribe-after-restart with a fresh server on the same channels.