feat(core): coordinator holds the hub connection (gateway role) - #169
Merged
Merged
Conversation
Dials the hub when a bridge becomes the local mesh coordinator and drops the connection when it loses that role, idempotently. Not yet wired into MeshStore/PeerLifecycle or the transport's own hub session.
realHubOverWs and waitForCondition were private to the one integration test that needed them; a second hub integration test for the coordinator gateway needs the identical harness plus a connection count observable on the hub side, so it now lives in the shared helpers module both tests import.
WireMeshTransport.connectHub/disconnectHub delegate to the transport's own HubSession, which now exposes isConnected and disconnect() so a caller can observe and end the held hub session without reaching into private state. Both methods are optional on MeshTransport, matching listKnownDevices' existing precedent for a WireMeshTransport-only capability. Extracts mergeKnownDevices out of WireMeshTransport into its own gossip-directory module to keep the file under the repo's max-lines cap after this addition, mirroring the existing peer-lifecycle.ts / connection-approval.ts / room-router.ts / hub-session.ts splits.
MeshStore now constructs a CoordinatorGateway and calls onBecameCoordinator() from both places this store takes over as coordinator: init()'s own fresh-bind fallback and PeerLifecycle's handleBecomeCoordinator takeover. shutdown() calls onLostCoordinator() unconditionally; the gateway's own connected guard makes that a no-op unless this instance actually held the role. The hub URL defaults to DEFAULT_HUB_URL (mesh-store-shared.ts) and is overridable via a new MeshStore constructor parameter.
Every bridge builds its MeshStore through this factory; hubUrl now passes through to MeshStore's own constructor alongside the existing coordinatorPort parameter, so a test (or a future non-default deployment) can override which hub a bridge's coordinator gateway dials without touching MeshStore directly.
CoordinatorGateway.onBecameCoordinator previously let a connectHub rejection propagate all the way through MeshStore.init(), so a bridge whose becomeCoordinator() succeeded locally would still fail to start if the gateway's hub happened to be unreachable. The local mesh's own coordinator election has no dependency on internet or hub availability, so this coupled two things that were never meant to be coupled. A dial failure is now reported via an injected onError callback (wired through to MeshStore's own onError) and swallowed; isConnected stays false so the next onBecameCoordinator call retries.
MeshStore's coordinator now dials a hub on becoming coordinator (agent-comms#154); without an explicit override this smoke test would have started making a real outbound connection to the production hub on every CI run. Pointing it at a guaranteed-unreachable local address keeps the test offline and deterministic, and exercises CoordinatorGateway's own dial-failure isolation along the way.
Mearman
force-pushed
the
feat/coordinator-gateway
branch
from
September 17, 2026 14:43
ebae2a7 to
35ef7fe
Compare
Mearman
marked this pull request as ready for review
September 17, 2026 14:46
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 35ef7fe596
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
A Codex security review on PR #169 found that HubSession.consume() forwarded any hub-relayed state_sync/state_update straight into applyPatch/applyStateSync with no per-peer admission control. That path existed since #151, but #154's own change is what first made a coordinator dial the hub by default in production, turning a latent gap into a live one: any identity able to reach the configured hub could patch local mesh state, including queuing a spoofed delivery. Real per-peer admission control is #156's own deliverable. Until then, the two state-mutating message methods are dropped before they ever reach onMessage; every other message type this session relays was already inert on receipt (PeerLifecycle only reacts to these two), so this closes the whole exposure through this path.
Member
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Mearman
added a commit
that referenced
this pull request
Sep 17, 2026
… tests Deny-all by default (agent-comms#156) means the two transports here no longer discover or message each other, or exercise the state_sync/state_update filter, without each explicitly trusting the other's device-id first. Strengthens the #169 security-finding test along the way: it now proves the filter still applies even to a hub peer this side has explicitly trusted, not merely an anonymous one.
Mearman
added a commit
that referenced
this pull request
Sep 17, 2026
… tests Deny-all by default (agent-comms#156) means the two transports here no longer discover or message each other, or exercise the state_sync/state_update filter, without each explicitly trusting the other's device-id first. Strengthens the #169 security-finding test along the way: it now proves the filter still applies even to a hub peer this side has explicitly trusted, not merely an anonymous one.
This was referenced Sep 18, 2026
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
First leg of the cross-machine mesh epic (#153): attaches the gateway role to the existing local coordinator-election machinery. A bridge that becomes this machine's mesh coordinator, whether by a fresh bind (
MeshStore.init()) or a takeover (PeerLifecycle.handleBecomeCoordinator), now also dials the relay hub and holds the connection for as long as it holds the coordinator role. Losing the role drops the connection.CoordinatorGatewaycollaborator owns the connect/disconnect lifecycle, idempotently, and never lets a hub-dial failure propagate into local coordinator election. The local mesh must keep working with no internet or hub access, so a dial failure is reported viaonErrorand swallowed rather than thrown.MeshTransportgains optionalconnectHub/disconnectHub(same optionality precedent aslistKnownDevices), implemented byWireMeshTransporton top of its existingHubSession, which now also exposesisConnected/disconnect().MeshStore's constructor takes an optionalhubUrl, threaded throughcreateBridgeMesh/createBridgeMeshSyncalongside the existingcoordinatorPort, defaulting towss://mesh.exadev.io/(DEFAULT_HUB_URLinmesh-store-shared.ts).MeshStore.shutdown()unconditionally callsonLostCoordinator(); the gateway's own connected guard makes that a no-op unless this instance actually held the role. A full transport shutdown also still closes the hub session as part of its existing session cleanup, so the connection drops on both graceful shutdown and crash.Not in scope (tracked separately): forwarding local agents onto the hub / merging its directory back (#155), the gateway trust boundary / allowlist (#156).
Deliberately left open
PeerLifecycle.handleBecomeCoordinator's takeover path (a peer receiving abecome_coordinatorwire message) is wired into the gateway the same as the fresh-bind path and covered by unit tests, but nothing in this codebase currently sendsbecome_coordinator. I found no producer anywhere insrc/, despite the README describing "graceful handover to the longest-running peer" as live. That gap predates this issue and is outside #154's scope, since it's about the local coordinator-election protocol itself, not the gateway role riding on top of it, so I left it as-is rather than building a sender. Flagging it here since the epic's own text states the handover already exists.Test plan
pnpm lintpnpm typecheckpnpm test(815 tests)pnpm test:smoke(real multi-process, built-artifact smoke test, now hub-isolated)WireMeshTransport.connectHub/disconnectHub, and a fullcreateBridgeMeshbecomes coordinator, real local hub sees the connection, shutdown drops it testCloses #154