refactor(core): delete TlsTransport, step 6 of P2's substrate-swap sequencing - #61
Merged
Conversation
…Listener Both stored the literal port argument they were called with rather than reading the actual bound port back from the listener, exactly the bug already fixed for TlsTransport (3876df5) but never carried over to WireMeshTransport's own independent implementation. Only matters when called with port 0 for an OS-assigned free port -- listListeners() would silently report 0 instead of the real port. Added a shared listenerPort() helper (mirroring startDataServer's own already-correct pattern) and used it in both places. Migrated become-coordinator-actual-port.integration.test.ts's own regression test onto WireMeshTransport and extended it to cover addListener too. Also drops connect_accepted, connect_rejected, and pong from the MeshMessage union: none has had a real constructor since the substrate swap (connect_request's own accept/reject rides sendManageRequest's request/response round trip directly, and peer identity now comes from the TLS-authenticated connection itself, never a self-reported pong), so route()'s own defensive drop case for them is now just dead cases for variants that no longer exist.
…eMeshTransport TlsTransport was agent-comms' own hand-rolled MeshTransport implementation over X.509 certificates and newline-delimited JSON framing -- the substrate every bridge ran on before the P2 migration. With all six bridges cut over to WireMeshTransport (over @exadev/wire-mesh-core) and its own connection-approval quarantine gate closing the security gap that substrate swap introduced, nothing constructs a TlsTransport any more. test-transport.ts's wireTestTransport and wireWireMeshTestTransport wired two different transports onto a test MeshStore for exactly this migration period; with only one transport left, they collapse into one function (kept the wireTestTransport name, the one every existing test already called). Every test that built its own peer directly against TlsTransport (broadcast-window, downtime-replay.integration, identity-restart) now does the same against WireMeshTransport, with fingerprint-based identity comparisons swapped for deviceId, matching the canonical identity createBridgeMesh already uses in production. listener-policy.integration.test.ts's one test that spoke TlsTransport's own wire format directly (a raw tls.connect() writing newline-delimited JSON) is rewritten against a real WireMeshTransport client session instead, and no longer reaches into MeshStore's internals through an unsafe cast to observe the result -- it constructs its own transport directly with a hand-rolled TransportEvents object instead, the same pattern become-coordinator-actual-port.integration.test.ts already used. peer-id-verification.integration.test.ts is deleted outright rather than migrated: the vulnerability class it guarded against (a peer's self-reported peerId not matching the certificate it authenticated with) cannot occur under WireMeshTransport by construction, since peer identity is never self-reported in the first place -- it's read directly from the TLS-authenticated connection's own peerDeviceId. wire-mesh-transport.integration.test.ts and wire-mesh-transport-approval.integration.test.ts were written during the migration specifically to prove WireMeshTransport reached parity with TlsTransport's own behaviour. Now that wireTestTransport wires WireMeshTransport for every test, they duplicate mesh-e2e.integration.test.ts and approval.integration.test.ts/listener-policy.integration.test.ts scenario-for-scenario. Deleted, after moving the one genuinely unique test in the approval file over: the quarantine-gate regression test proving a forged state_update from an unapproved connection is refused. Fixed along the way: approval.integration.test.ts's own "mesh_pending lists pending connections" test shut down storeB immediately once its own assertions were satisfied, without waiting for storeB's own fire-and-forget connectToRemote continuation (the mesh-formation handshake) to actually settle. Under WireMeshTransport's real round-trip timing this let the test's own shutdown race ahead of that continuation, which then tried to track a session into an already-shut-down transport and never closed it -- the same convergence wait "accept establishes the peer connection" already uses fixes it.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
|
🎉 This PR is included in version 1.33.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Sep 11, 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.
Step 6 of #47's sequencing: deletes TlsTransport and the fingerprint-based identity path now that every bridge runs on WireMeshTransport (steps 4/5, #58/#59, plus the connection-approval quarantine fix in #60).
Two commits:
becomeCoordinator/addListenernever got the same "report the OS-assigned port, not the literal 0 requested" fix TlsTransport already had (3876df5). Fixed both, extended the existing regression test to cover both, and droppedconnect_accepted/connect_rejected/pongfrom the wire protocol union now that nothing constructs them any more.tls-transport.tsremoved;test-transport.ts's two wiring helpers collapse into one (wireTestTransport, now wiring WireMeshTransport); every test that built a peer directly against TlsTransport (broadcast-window, downtime-replay.integration, identity-restart) migrated to WireMeshTransport with device-id identity comparisons;listener-policy.integration.test.ts's one raw-TLS-wire-format probe rewritten against a real WireMeshTransport client session, dropping an unsafe cast into MeshStore's internals along the way;peer-id-verification.integration.test.tsdeleted outright since the vulnerability class it guarded against can't occur under WireMeshTransport by construction (peer identity is never self-reported);wire-mesh-transport.integration.test.tsandwire-mesh-transport-approval.integration.test.tsdeleted as fully redundant withmesh-e2e.integration.test.ts/approval.integration.test.tsnow that there's only one substrate to test, after moving the one genuinely unique test (the quarantine-gate regression check) intoapproval.integration.test.ts.Also fixed while verifying:
approval.integration.test.ts's "mesh_pending lists pending connections" test shut down storeB immediately once its own assertions passed, without waiting for storeB's own fire-and-forgetconnectToRemotecontinuation to actually settle -- under WireMeshTransport's real round-trip timing this let shutdown race ahead of it, tracking a session into an already-shut-down transport that never got closed and hung the test runner. Fixed with the same convergence wait "accept establishes the peer connection" already uses.85/85 tests pass, plus
test:smokeandtest:delivery, typecheck and lint clean.