Address 25 issues reported by golangci-lint following the structural
refactor:
- cyclop: split common.Reassembler.Push into upsert/storeChunk/deliver
helpers (12→5). Move seichannel option-default fill into Options.
withDefaults so New stays under the limit.
- exhaustive: enumerate ResultPartial / ResultIgnore explicitly in
seichannel and videochannel switches over common.Result.
- gosec G115: annotate the test-fixture int→uint16/uint32 conversions
in common_test.go with //nolint:gosec.
- lll: break up the 130+ character one-liners in transport
unit/integration tests and the videochannel track-ID construction.
- nolintlint: drop the stale //nolint:cyclop in mobile_test.go where
the underlying complexity already cleared the limit.
- wrapcheck: wrap errors returned from internal/framing and
internal/runtime in their public callers (handshake, control,
server.setupCipher, client.setupCipher) so they carry the layer name.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
internal/transport/vp8channel/transport.go opened with a 24-line block
of Russian profanity addressed at Yandex SFU engineers (the engine
vp8channel targets to evade). It served no engineering purpose and is
not suitable for an open-source codebase.
Replace with a one-paragraph package doc summarising what the package
actually does (KCP-over-VP8-keyframes byte transport for SFUs that
validate VP8 conformance).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
session.Config used to spread 16 per-transport tuning fields across its
top level (VideoWidth/Height/.../VP8FPS/.../SEIAckTimeoutMS). The flat
layout meant every caller had to know which fields belong to which
transport, and the YAML→session bridge in internal/config repeated the
same name 32 times across Apply/ApplyProfile.
Group them under VideoConfig/VP8Config/SEIConfig structs hung off
session.Config. internal/config now does e.g.
dst.Video.Width = pickInt(dst.Video.Width, f.Video.Width)
instead of touching dst.VideoWidth. session.ApplyTransportDefaults,
validateVideoChannel/VP8Channel/SEIChannel and buildTransportOptions
read through cfg.Video.*/cfg.VP8.*/cfg.SEI.* in the same way.
The YAML schema itself was already grouped (Video / VP8 / SEI sections);
this commit lines session.Config up with it so the Apply functions can
mirror the YAML structure 1:1 instead of unpacking it into 32 flat
assignments.
All session/config/e2e/cmd/main tests that referenced cfg.VideoX directly
are updated to cfg.Video.X.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
server.go and client.go each carried byte-identical copies of
smuxConfig (~20 lines), setupCipher (~18 lines), and the health
bookkeeping pair recordSession/Pong/Missed/Unhealthy/Reconnect plus a
private healthMu+status+notifyHealth scaffold. Same code, twice.
Add internal/runtime exposing:
- SetupCipher, SmuxConfig, MaxPayload — common construction helpers,
ErrKeyRequired/ErrKeySize re-exported from runtime so existing
errors.Is checks on server.ErrKeyRequired etc. keep working.
- HealthTracker — nil-safe wrapper around control.Status with
RecordSession/Pong/Missed/Unhealthy/Reconnect that publishes through an
OnHealth callback supplied at construction.
server and client now hold a *runtime.HealthTracker instead of their own
mu+status+notify scaffolds. recordX methods on Server/Client are now
one-liners that forward to the tracker. smuxConfig(0) replaces the prior
variadic smuxConfig() in test call sites; nil-safe Status()/update() on
HealthTracker means tests that build raw &Server{}/&Client{} no longer
need to wire up a tracker for the records to be no-ops.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
videochannel, seichannel and vp8channel each carried independent copies of
randomID(), fragmentPayload(), inboundMessage + upsertInbound +
assembleMessage + ackWaiters/ackMu. The reassembly logic was almost
byte-identical across videochannel and seichannel; vp8channel only needed
randomID. Three copies of the same idea.
Add internal/transport/common with:
- RandomID(): 8-char hex per-peer ID (Jitsi msid uniqueness requirement).
- FragmentPayload(): split bytes into max-size chunks.
- Reassembler: stores in-flight messages keyed by Seq, validates CRC, and
reports Partial / Delivered / Duplicate / Ignore via a Result enum.
- AckRegistry: Register/Unregister/Resolve for ack waiters.
videochannel and seichannel now hold *common.AckRegistry and
*common.Reassembler instead of raw maps + mutexes. Their Send paths route
through acks.Register/Unregister; their handleInboundFrame is a 20-line
switch over reassembler.Push. vp8channel keeps its KCP framing but reuses
common.RandomID.
Tests that constructed raw streamTransport with inbound/delivered/ackWaiters
maps are updated to instantiate the new common types instead. Two now-
redundant low-level tests (upsertInbound out-of-range, assembleMessage)
collapse into the new TestInboundRejectsBadCRC.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
internal/carrier and internal/carrier/builtin sat between transports and
engines, wrapping every engine.Session in carrier.Session +
engineByteStream/engineVideoTrack adapters that mechanically proxied every
method. That layer existed solely to translate Capabilities/AddTrack names;
no behaviour lived above engine.
Replace with internal/engine/builtin: a name-keyed registry that calls
auth.Issue and engine.New directly. Transports look up engine.Session via
enginebuiltin.Open, then type-assert engine.VideoTrackCapable for video
transports. A small per-transport engineVideoSession adapter unifies the
reconnect callback signature (engine uses func(*webrtc.DataChannel); the
transports want func()).
Updates:
- internal/engine/builtin/builtin.go: new Register/Open registry + auth
pass-through ("none") + auth-driven factories for jazz/telemost/wbstream/jitsi.
- internal/transport/datachannel/transport.go: uses engine.Session directly
via Capabilities().ByteStream check.
- internal/transport/{seichannel,videochannel,vp8channel}: each gains an
engineVideoSession adapter and routes Connect/Send/Close/AddTrack through
the engine session.
- internal/app/session: imports enginebuiltin; carrier.Available() →
enginebuiltin.Available().
- pkg/olcrtc/olcrtc.go: switches to enginebuiltin.RegisterDefaults.
- internal/carrier and internal/carrier/builtin: deleted.
- Tests rewritten to register a fakeEngineSession (implements engine.Session
+ engine.VideoTrackCapable) through enginebuiltin.Register. The e2e
memoryStream gains the same dual interface so memorySession is gone.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
internal/link and internal/link/direct were a single-implementation
abstraction layer where directLink mechanically proxied every method to
transport.Transport — only Features() lived above transport.Transport,
and even that was a Features() alias. Six layers of plumbing for zero
behavioural value.
Drop the layer entirely:
- muxconn.Conn now takes a transport.Transport directly.
- server.Server and client.Client store transport.Transport, call
transport.New, and expose Features() through transport.Transport's
built-in method.
- server.Config and client.Config lose their Link string field.
- session.Config loses Link + validateLink + ErrLinkRequired/ErrUnsupportedLink.
- config.File and config.Profile lose the link YAML key.
- pkg/olcrtc/tunnel.Config loses Link.
- mobile drops defaultLink, SetLink, and mobileConfig.link.
Two e2e tests that exercised link.New directly are renamed to call
transport.New (TestTransportCreatesAllProviderTransportCombinations and
TestTransportConnectsFastProviderTransportMatrix); behaviour is unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
transport.Config used to carry a flat union of video+vp8+sei tuning fields
that every transport ignored except its own. Replace with an opaque
transport.Options marker interface and per-transport Options structs
(videochannel.Options, vp8channel.Options, seichannel.Options). Datachannel
keeps an unset Options.
link.Config gains TransportOptions and drops the 16 transport-specific
fields. server.Config and client.Config follow suit. session.Config is
left untouched in this commit — buildTransportOptions packs its existing
flat fields into the typed Options bundle before calling server/client
(session.Config is rebuilt in a later commit when YAML config moves to
typed sections).
Tests that synthesized link/server/client/transport configs are updated
to pass typed Options bundles. The shared e2eTransportOptions helper
replaces three copies of the flat field bundle in e2e/tunnel_test.go.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
handshake and control duplicated the same 4-byte BE length + body framing
with independent ErrFrameTooLarge constants. Centralize in internal/framing
and have both callers delegate. ErrFrameTooLarge is re-exported so existing
errors.Is checks keep working.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
When bringUpLink errored — a handshake timeout against a wedged transport,
for instance — Run/RunWithReady returned straight to the caller without
calling shutdown, so the carrier link that had already joined the MUC
was never closed. The result was a ghost participant lingering on
Jicofo/JVB until idle timeout, which the next test in the same room
inherited as stale endpoints in 'bridge open'.
The clue from logs was that failing seichannel runs produced one
'leave-muc handshake ok' instead of two: the server's normal
ctx-cancel path got there cleanly, but the client's bringUpLink
returned early and skipped its defer.
Both paths now register shutdown before the bringUpLink call. shutdown
is nil-safe and idempotent so it works whether or not bringUpLink
actually populated link/session fields. server's wg.Wait moves into
the same defer so wg goroutines spawned by partial setup also drain
before Run returns.
