refactor(streams): build duplex streams from a single transport shape - #1138
Merged
rekmarks-consensys-1 merged 3 commits intoOct 2, 2026
Merged
Conversation
rekmarks-consensys-1
added this pull request to stack #1139
October 1, 2026 19:19
rekmarks-consensys-1
marked this pull request as ready for review
October 1, 2026 19:20
rekmarks-consensys-1
force-pushed
the
refactor/streams-consolidate-transports
branch
from
October 1, 2026 19:42
d1ae14a to
8ec9dde
Compare
rekmarks-consensys-1
marked this pull request as draft
October 1, 2026 19:44
Base automatically changed from
fix/streams-writer-end-on-remote-close
to
main
October 2, 2026 16:17
`BaseDuplexStream` now takes `{ listen, onDispatch, onEnd }` and builds its
own reader and writer, so each transport is one class instead of three.
`BaseReader` receives input through `listen` rather than a protected
`getReceiveInput()`, `BaseWriter` ends without the mutually recursive
dispatch/throw path, and `split` uses a single variadic signature. The
local `Reader`/`Writer` types replace the `@endo/stream` dependency.
The per-transport `*Reader` and `*Writer` classes are no longer exported,
and `NodePort` requires `off` so the Node stream removes its listener on end.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rekmarks-consensys-1
force-pushed
the
refactor/streams-consolidate-transports
branch
from
October 2, 2026 16:17
8d3730e to
5a2ca2e
Compare
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rekmarks-consensys-1
marked this pull request as ready for review
October 2, 2026 17:00
ci-belphegor
approved these changes
Oct 2, 2026
ci-belphegor
left a comment
ci-belphegor
left a comment
Contributor
There was a problem hiding this comment.
These lines will not be missed
rekmarks-consensys-1
deleted the
refactor/streams-consolidate-transports
branch
October 2, 2026 21:22
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.
Stacked on #1137.
Every transport in
@metamask/streamshad a Reader class, a Writer class, and a duplex class that wired the two together by hand. The four copies had drifted:NodeWorkerReadernever removed its listener and dropped the error passed toonEnd.MessagePortReaderignored thenameit was given.SplitStreamnamed itself"Function".PostMessageDuplexStreamacceptedonEnd.This PR moves the wiring into
BaseDuplexStream, which takes one transport shape and builds its own reader and writer:listen(receiveInput)subscribes to the transport and may return an unsubscribe function, which the reader calls when it ends. The writer ends last, so the final Done or Error signal is dispatched beforeonEndcloses the transport. #1137 fixed that ordering forPostMessageDuplexStream; it now holds for every transport, andonEndruns exactly once.Other changes:
BaseReadergets input throughlisteninstead of the protectedgetReceiveInput(), which removesSplitReader. Input that fails validation or signal parsing ends the reader with that error, as before.BaseWriterdrops the mutually recursive#dispatch/#throw. A failed write sends the underlying error to the remote once and always rejects with"<name> experienced a dispatch failure". Previously a second failure surfaced the raw transport error.splithas one variadic signature in place of four overloads, and it narrows any number of splits.Reader/Writerare defined locally, so the@endo/streamdependency is gone.remote-iterablesstill uses it, so the only lockfile change is the@metamask/streamsentry.TestDuplexStreamtakes a singleonEndin place ofreaderOnEnd/writerOnEnd, and its unusedonDispatchgetter is removed. The two tests that used the old options are updated.packages/streamsnon-test source goes from 2,191 to 1,581 lines (−28%), and tests plus mocks from 2,768 to 2,155.Breaking changes
*Reader/*Writerclasses for MessagePort, PostMessage, ChromeRuntime, and NodeWorker are no longer exported. Nothing in this monorepo imports them.BaseReaderandBaseWriterare now exported in their place, for one-way streams over any transport.NodePortrequiresoff. Node'sWorkerandworker_threadsMessagePortboth provide it.Testing
@metamask/streams: 145 tests pass with 99% coverage. The transport tests were ported from the removed classes to the duplex streams, and there are new cases for singleonEnd, dispatch-before-onEndordering, and listener removal.yarn buildsucceeds. The tests forlogger,kernel-node-runtime,ocap-kernel,kernel-browser-runtime,kernel-test,kernel-language-model-service,extension, andomnium-gatherumpass.🤖 Generated with Claude Code
Note
High Risk
Breaking export and transport-interface changes plus reworked stream end/error semantics affect all kernel IPC paths that depend on
@metamask/streams.Overview
Refactors
@metamask/streamsso every transport implements one shape (listen,onDispatch, optionalvalidateInput/onEnd) andBaseDuplexStreamwires its ownBaseReader/BaseWriter, instead of hand-rolling separate Reader/Writer classes per transport.Lifecycle and errors: Duplex streams expose a single
onEnd(replacingreaderOnEnd/writerOnEndin tests and mocks). The writer shuts down last so the final done/error signal is dispatched beforeonEndruns.BaseReadersubscribes vialisten(nogetReceiveInput()); bad input ends the reader.BaseWriterdrops recursive dispatch/retry—failed writes emit one error signal and reject with"<name> experienced a dispatch failure".Public API (breaking): Per-transport
*Reader/*Writerexports are removed; use duplex streams or newly exportedBaseReader/BaseWriter.NodePortmust implementofffor listener cleanup.splitis variadic with per-predicate type narrowing.ChromeRuntimeDuplexStreamthrows in the constructor when local and remote targets match.@endo/streamis removed;Reader/Writertypes live in-package.Downstream test helpers (
TestDuplexStream,VatSupervisor, internal-comms mocks) switch toonEnd. Transport tests consolidate on duplex streams with coverage for listener removal and end ordering.Reviewed by Cursor Bugbot for commit 5a2ca2e. Bugbot is set up for automated code reviews on this repo. Configure here.