Skip to content

test(docker): prove per-destination routing over E-RTMP, not just SRT - #124

Merged
rainmanjam merged 14 commits into
mainfrom
test/ertmp-routing-coverage
Aug 8, 2026
Merged

test(docker): prove per-destination routing over E-RTMP, not just SRT#124
rainmanjam merged 14 commits into
mainfrom
test/ertmp-routing-coverage

Conversation

@rainmanjam

Copy link
Copy Markdown
Owner

Step 4 proved per-destination audio routing by content — for SRT only. Nothing proved it for RTMP.

Step 5 claimed to and never ingested an RTMP packet. drive mode rtmp wrote settings.ingest.mode, which the engine overwrites from the source row and which Target.Ready does not consult, so every publish was refused. It passed because it asserted >= 1 track and an un-probed source reports the six-track placeholder layout.

Step 4b publishes three tones over rtmp:// — which is E-RTMP multitrack — and runs the same content assertions. My first version was a false pass measuring step 4's SRT files; the recordings are cleared first now, and the measurements differ, which is how you can tell it ran.

Known gap, documented in place: after a mode change the probe does not re-run promptly — the publisher is admitted and holds its session while the source still reports the placeholder layout. Step 5's assertion is left weak with that written next to it rather than tightened into a failure about something else.

38 passed, 0 failed.

Asked whether all three protocols had been tested with audio routing. They had
not, and finding out took several wrong answers.

WHAT WAS ALREADY TRUE: step 4 proves per-destination routing for SRT, by
content — three tones in, three differently-routed destinations, each one
measured by bandpass energy. That is the right shape of test and it was only
ever pointed at one transport.

WHAT WAS NOT: nothing proved it for RTMP. Step 5 claimed to cover the RTMP path
and never ingested a single RTMP packet in its life. `drive mode rtmp` wrote
settings.ingest.mode, which the engine overwrites from the source row and which
neither the listener gate nor rtmpserver's Target.Ready consults — so every
publish was refused for having no ready target. The step passed anyway because
it asserted the probe reported ">= 1" track, and an un-probed source reports the
six-track PLACEHOLDER layout. Six is not zero. Green for years, proving nothing.

Step 4b now publishes three tones over rtmp:// — which IS Enhanced RTMP
multitrack, since FFmpeg 7.1+ writes multitrack FLV whenever more than one audio
stream is mapped — and runs the same content assertions. It matters that this is
separate from SRT: SRT hands datagrams straight into the hub, while RTMP goes
through rtmpserver's setup cache, which is where multitrack broke once already.

MY FIRST VERSION OF IT WAS A FALSE PASS, and the way it failed is the reason the
recordings are now deleted first. It measured step 4's SRT files and reported
four green lines about a transport it had never touched — identical to six
decimal places, -24.095344 / -51.165581 / -70.757469 in both steps. The fixed
version measures -24.130838 / -51.196428 / -70.754045, which is how you can tell
it ran.

Two other things it took a while to stop blaming the product for: publish() ends
in `-f mpegts` because it was built for SRT, so pointing it at an rtmp:// URL
sent TS bytes down an RTMP connection and the server correctly dropped the
session with "invalid message type: 255"; and the listener check scraped
`docker logs` for a line printed once at startup, which was intermittent enough
to look like a listener that never came up. It is a TCP connect now.

Step 5 keeps its weak assertion, with the reason written next to it. The half
that is fixed is fixed; the half that is not — the probe does not re-run
promptly after a mode change, so a source reports the placeholder layout while
its publisher is admitted and holding a session — is recorded rather than
tightened into a failure about something else.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
Copilot AI lite review requested due to automatic review settings August 7, 2026 20:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the docker-based acceptance suite to prove per-destination audio routing over RTMP, specifically Enhanced RTMP (E-RTMP) multitrack, closing a gap where the existing “RTMP” step could pass without ever ingesting RTMP packets.

Changes:

  • Add an E-RTMP multitrack publisher (publish_ertmp) and a new Step 4b that clears prior recordings and re-runs the same content-based routing assertions over rtmp://.
  • Fix ingest mode switching in the acceptance driver by updating per-source ingest mode via /sources/:id instead of writing the settings singleton.
  • Correct Step 5 to publish to the per-source token URL and document the known post-mode-change probe lag.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
scripts/acceptance-docker.sh Adds E-RTMP multitrack routing proof (Step 4b), refines RTMP fallback step, and introduces an E-RTMP publisher.
scripts/acceptance_docker_driver.go Fixes setMode to update ingest mode on the source rows via the sources API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +304 to +306
if docker run --rm --network "$NET" --entrypoint sh "$IMAGE" -c \
"nc -z -w2 $CTR 1935" >/dev/null 2>&1; then bound=yes; break; fi
sleep 1
Comment on lines +333 to +335
sleep 20
drive stopall >/dev/null
sleep 12
Comment on lines +419 to +430
id, _ := src["id"].(float64)
ing, _ := src["ingest"].(map[string]any)
if ing == nil {
die("source carried no ingest block")
}
ing["mode"] = mode
// Only the ingest block. handleUpdateSource decodes over the stored row,
// so a partial body is the supported shape — and sending the whole view
// back fails, because /sources returns a row wrapped with fields like
// `destinations` that the source itself does not have.
code, body := do(http.MethodPut, fmt.Sprintf("/sources/%d", int64(id)),
map[string]any{"ingest": ing})
… measured

Both found by tracing why a source reported six tracks while a publisher held a
clean 30-second session. Six is routing.DefaultSource() — the placeholder that
exists so the routing editor has something to draw before a stream arrives.

DESTINATIONS COMPILED AGAINST IT. reconcileMeters and stemPlanFor both refuse on
an unprobed layout, with a comment explaining that a zero-track check cannot
catch it because the placeholder HAS tracks. Destinations read e.source raw —
the one process-building consumer that did, and the one that matters most.

Two failures came out of that, and the quiet one is why this is a guard rather
than a warning. A profile naming a track the stream lacks emits `[0:a:5]`,
FFmpeg refuses, the destination crash-loops: loud and findable. But the
placeholder also claims Channels: 2 on every track, so a real 5.1 track compiles
to `pan=stereo|c0=c0|c1=c1` — valid FFmpeg. The destination starts, stays up,
and publishes front L/R only, discarding centre, where dialogue lives. No error
at any layer.

READY DID NOT MEAN WHAT ITS OWN COMMENT SAID. "Ready is the counterpart of
srtserver's `Sink != nil`: it must mean an RTMP SUBSCRIBER exists for this
source, not merely that an engine does" — sitting directly above
`eng != nil && s.Ingest.Mode == db.IngestRTMP`, which reads a database row and
an engine record and never asks whether anything is reading. Between the two
sits every state where the ingest child is absent or crash-looping, including
reconcileIngest's own early return for a source with no publish token. In all of
them a publisher was admitted, held as long as it liked, and delivered into a
stream with no reader: encoder green, no output, nothing logged, because from
the server's side nothing had gone wrong.

rtmpserver.HasSubscriber closes it by asking the listener. Subscribe-before-
publish is the normal order — the ingest child dials in when the source is
enabled, well before anyone hits Start in an encoder — so the ordinary case is
unaffected, which the container suite confirms: E-RTMP ingest with three tracks
still routes per destination, verified by tone content.

TestAnRTMPTargetIsNotReadyUntilItsEngineIs asserted the wrong contract and is
renamed to the right one. Its fixture cannot exec FFmpeg, so the engine exists
and nothing ever subscribes — under the old rule that was Ready.

The driver reported the placeholder's LENGTH while decoding the probed flag and
discarding it, so "6" came back for a source that had never seen a packet. It
says "unprobed" now, which is what the engine was saying all along.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
…ified

The guard added in 1a16eb3 -- hold destination planning until the ingest
layout has been measured -- was one line and shipped three bugs. CI caught
one, a control run against the parent caught the second, and two subagents
reviewing the diff caught the third. Two further defects of the same family
turned out to predate the branch. All five are here.

1. The hold returned from reconcileOutputs outright, so the selector, the
   silence tier, the renditions and playout stopped reconciling too. Killing
   the primary encoder clears e.probed after three idle rounds, so it fired
   for exactly the window the selector exists to cover: the late reconcile
   put a backwards DTS step in the output, which is the discontinuity a
   receiving platform drops the connection on.

2. It asked `probed` -- "a layout is arriving right now" -- when it meant
   "a layout has ever been measured". probeLoop clears probed when a stream
   stops but deliberately leaves e.source alone, so a destination added
   during a failover could not start until the primary came back. Split into
   two flags; only ingest start puts the placeholder back.

3. It skipped stopDestinations while held. Everything below still replaces
   hubs, and closing a hub stops delivery without ending the process, so a
   destination sat alive and subscribed to a hub nobody wrote to -- 76
   seconds, zero bytes, no error. Reproduced about one run in two.

4. reconcileIngest returns early for SRT, for IngestUnset and for RTMP with
   no token, all before the reset that ingest start performs. Switching a
   probed RTMP source to SRT left the RTMP stream's layout in e.source,
   still measured. Clearing it in those returns is worse than the bug -- the
   SRT branch runs on every reconcile -- so measuredMode records the mode a
   layout was measured under and invalidation is conditional on a change.

5. stopDestinations keeps a destination whose running spec matches its
   planned one. Planning against the placeholder while held meant a graph
   built for a real STEREO layout matched, because the placeholder is
   stereo -- so it survived an unmeasured window and would publish front L/R
   from a 5.1 stream, discarding centre. A held pass now plans nothing.

Also: the RTMP standby target was Ready: true unconditionally, directly
below a comment describing the opposite contract. Ready was fixed for the
primary in 1a16eb3 and missed here, so a crash-looping backup ingest
admitted publishers into a stream with no reader.

Tests. Six new, each mutation-verified by reintroducing the exact defect and
confirming it applied before trusting the kill. The drift test now asserts
that clearing `measured` is PAIRED with restoring the placeholder, rather
than pinning a count -- the count version failed on a correct change.

acceptance-failover now asserts the mismatch destination wrote bytes. It was
a note, and a note is why bug 3 stayed green: the destination never
restarted, so the restart counter read 0 and every other check passed.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
…urvey

Both playlist rows in COMPARISON.md read "Partial -- no playlist sequencing"
long after sequencing shipped: internal/playlistmedia normalises every upload
to one fixed profile on import, the concat list runs under -stream_loop -1,
and playlist.start / playlist.stop are scheduler actions with drift tests.
RESEARCH-COMPETITIVE.md carried a third copy of the same claim.

Corrected with the caveat stated rather than buried -- the playlist rides
failover.playlist and goes on air when no encoder is delivering, so it is
the fill tier, not a channel you can programme a day of content into.

The re-verification note now records the DIRECTION of the drift: every stale
row found so far has understated the product, because the page gets written
when a gap is found and nobody returns to it when the gap closes.

Also adds the Castr.com survey the correction came out of. The genuine gaps
are teams/roles/SSO, cloud production and WHIP output; most of the rest is
Castr hosting the audience and polyemesis not. Nothing in their 82 features
is per-destination audio routing -- their multistream page states "every
destination receives the same produced feed" as a feature.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
`npm audit --audit-level=high` began failing on GHSA-2v37-7h3g-55p8 --
nanoid below 3.3.17 loops indefinitely when a custom generator is given
size zero. Transitive, three deep: vite -> postcss -> nanoid, so nothing in
package.json names it and no frontend change provoked it. The advisory was
published against a version the lockfile already pinned.

Lockfile only (npm audit fix --package-lock-only): 3.3.16 -> 3.3.18, no
package.json change and no other package moved. Verified by reinstalling
with the same `npm ci --ignore-scripts` CI uses and running its whole
sequence -- tsc, lint, build, 92 tests, audit -- rather than trusting the
audit line alone, since audit reads the lockfile while the tests read
node_modules and those can disagree.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
A review by codex, agy, Fable and Opus over commits ea1821f..69bb9fc. Findings
they agreed on, plus the ones only one of them saw.

1. probeOnce committed its result unconditionally after a multi-second unlocked
   ffprobe, so a probe in flight during an ingest-mode change landed AFTER the
   invalidation and re-certified the dead transport's layout -- stamping it with
   the NEW mode, which satisfied the guard permanently. All four reviewers found
   this independently; Opus traced a second trigger the others missed, where
   token rotation or an RTMP app edit hits it with no mode change at all.
   sourceGen is captured before the read and checked before the write, which
   also covers the same-mode ingest restart that measuredMode cannot see.

2. probeLoop called reconcileOutputs without reconcileMu. Benign on main, where
   both passes computed identical plans; not now, because `measured` flips
   inside the probeOnce that triggers the second pass. A Reconcile holding an
   empty plan set races a probe pass that starts every destination, then tears
   down what it just started -- and nothing restarts them, because the layout is
   stable so no later probe reports `changed` and Reconcile has no ticker.
   Taken in probeLoop, not in reconcileOutputs: Reconcile already holds it there
   and the mutex is not reentrant. Both codex and agy verified that separately.

3. wantSilence read `probed` literally -- the same mistake the `measured` split
   was introduced to remove, in a place the fix never reached. A video-only
   source going idle tore down the silence tier, and planning then ran against a
   zero-track layout with no synthTrack() substitution, so routing.Compile
   answered ErrNoAudio and every destination was torn down for as long as the
   encoder was quiet.

4. The RTMP standby was Ready: true unconditionally, directly below a comment
   describing the opposite contract. Fixed for the primary in 1a16eb3 and missed
   here, so a crash-looping backup ingest admitted publishers into a stream with
   no reader.

5. probeOnce swallowed every probe error silently. Destinations are held until a
   layout is measured, so a probe that can NEVER land left every destination
   down with nothing anywhere saying why. Logged on the transition, with a
   recovery line, because the retry cadence is 3s and an unconditional line
   would bury the log.

6. measuredMode outlived the layout it described. Inert -- the guard is gated on
   `measured` -- but the invariant "measuredMode is the mode e.source was
   measured under" was false whenever measured was false, and that is what made
   defect 1 hard to see.

7. m.rtmp was assigned after s.Start(), so the listener accepted connections in
   a window where its own lookup could not see it and scored Ready false.
   Microseconds, but Ready had no dependency on m.rtmp before this branch.

Plus scripts: the docker driver's all() discarded every HTTP status and printed
START_OK regardless, so a 500 on start-all reached the shell as success.

TESTS. wantSilence's table had no row for the case that broke it -- it covered
probed and unprobed but never measured-but-idle, which is the whole gap. Added,
and the fixture renamed, because it was silently setting a field the function no
longer reads. Mutation-verified.

Defects 1 and 2 ship WITHOUT dedicated tests. Both are timing races across
dropped locks that the existing fixtures cannot stage, and a test that cannot
fail on the broken code is worse than none. They are verified by reading and by
-race staying clean, which is weaker than the bar the rest of this branch holds.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
… to run

Every destination path returns a compiled filterComplex so the editor can render
"Tracks 1, 2, 4 -> stereo" and the filter string without a second round trip.
All of them compiled against s.eng().Source(), which is effectiveSource() -- the
variant that DISCARDS whether the layout was measured.

So before a stream had ever arrived, the operator was handed a graph built from
routing.DefaultSource(): six stereo tracks that exist so the editor has
something to draw. It contains [0:a:5] for a profile selecting track 5, and
2-channel pans for tracks that may be 5.1. reconcileOutputs refuses to start
that exact graph. The screen and the process disagreed, in the direction that
makes the placeholder look authoritative -- and it is the screen the operator
configures from.

Adds Engine.SourceKnown, and the list, get and update paths now mark such a
response `routingProvisional`.

FLAGGED, NOT WITHHELD. refuseIfSilent argues the case directly above these
handlers: configuring a destination before going live is when most people
configure them, so refusing a preview would make the product harder to set up
than the bug ever made it to operate. The preview stays; it just has to admit
what it was compiled from.

Found by an independent review agent, which reached it while checking something
else and called it the live defect -- correctly. It predates the guard work on
this branch; the guard only made the disagreement reachable more often.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
…pawn

Ready requires a live subscriber, and the ingest child that provides one is a
supervised FFmpeg carrying no reconnect flags: it EXITS whenever its publisher
does and the supervisor respawns it on a 500ms-5s backoff. So the most ordinary
failure there is -- an encoder auto-reconnecting after a network blip -- arrives
precisely while nothing is subscribed, and was refused. RTMP carries no typed
rejection, so the encoder sees only a failed connect; one without aggressive
retry stays down until somebody notices. Token rotation has the same window,
because it changes the ingest signature and tears the child down.

A publisher that has already proved it holds a valid key is now HELD for up to
6s while its subscriber comes back, and admitted if it does. Only that verdict
waits: an unknown key or a disabled source is answered at once, so the grace
cannot be used to hold connections open against the listener.

The handshake deadline is extended before waiting, not left alone. It is set
before the handshake and only cleared once admission succeeds, so the wait would
otherwise spend the SAME budget the handshake already drew on -- a slow
handshake plus a full grace blows it, and the session is admitted and then fails
its first read with i/o timeout. That would have been worse than the refusal
being fixed, and it was caught in review rather than by me.

Tests: the grace admits when a subscriber appears, expires when none ever does,
and only refuseNotReady waits at all. KNOWN GAP, stated rather than hidden: they
call awaitReady directly, so they would still pass if the call site were removed
from the serve path. The wiring is verified by reading. A mutant that deleted the
call did not apply cleanly and nothing went red, which is how the gap was found.

Also in this commit:

- The cross-platform smoke test now publishes over E-RTMP and over SRT, not only
  into the relay hub, and measures per-destination audio for each. Both
  publishers are pure Go -- gortmplib and datarhei/gosrt, already dependencies --
  so the one-port listener, the readiness gate and multitrack FLV demux run on
  macOS and Windows too. The hub injection existed because a runner's FFmpeg is
  not guaranteed to carry libsrt; this Mac's Homebrew build genuinely does not,
  checked. FFmpeg is left muxing, which every build can do.

  It must now run from the repo root: importing gosrt needs module context. CI
  already does.

- Two tests for the reconcile race and the stale-probe commit, which shipped
  untested in 9ff1ab6. They use a controlled fake ffprobe to force the
  interleaving rather than sleeping and hoping. They prove the fix holds under
  the interleaving staged; they cannot prove there is no third one.

- scripts/seed_dests.go, build-tagged: seeds routed destinations straight into a
  database through the project's own db package, for verifying a server where no
  API credentials are held. It bypasses every API validation, which is the point
  and also the hazard.

- The review notes both agent passes produced, and the 0.5.0 changelog.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
The port prompt comes BEFORE the TLS choice, so an operator picks one without
yet knowing they will be serving HTTPS. Left at the 8080 default that gives a
working but unlovely install: the :80 redirect correctly sends browsers to
https://host:8080, every link carries the port, and nothing listens on the port
people actually try first.

Now offered once TLS is on, and only when the port is still the untouched
default -- a port given on the command line or typed at the prompt is a decision
and is left alone.

The firewall rule follows HTTP_PORT rather than opening 443 unconditionally.
Opening a port nothing binds looks like working TLS and serves nothing, which is
harder to diagnose than a closed port.

AND THE CAPABILITY, which is the part that would have bitten. CAP_NET_BIND_SERVICE
was granted only when tls.mode was acme -- correct for the :80 challenge, wrong
for everything else. Ports below 1024 are privileged and the unit runs
unprivileged, so an operator choosing selfsigned (the DEFAULT) and taking the
443 offer would have got a unit that could not bind the port the installer had
just written into its own ExecStart: bind: permission denied, on a fresh
install, from following the prompts. Now granted for acme, for 443, or for 80,
whichever applies.

Found while checking whether the OVH box could bind 443 at all -- which it can,
because that unit already carries the capability from an acme install. The
selfsigned path had never been walked.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
The edit anchored on "### Testing", which appears in every release section, so
the assertion failed and only install.sh was committed. Recording it because a
changelog that silently misses a change is worse than one that is late.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
INSTALL.SH HAD NO TESTS AT ALL, and the first careful reading of it found a real
bug: CAP_NET_BIND_SERVICE was granted only when tls.mode was acme, so an
operator choosing selfsigned -- the DEFAULT -- and taking the 443 offer got a
unit that could not bind the port the installer had just written into its own
ExecStart. scripts/acceptance-install.sh drives the decisions without
installing: the full nine-case capability matrix, that the 443 offer fires only
on an untouched default, that the firewall follows the chosen port rather than
opening 443 unconditionally, and that acme still opens 80. Reverting the gate to
`acme:*` fails it.

THE GRACE WAIT WAS NOT PROVEN TO BE WIRED IN. The tests called awaitReady
directly and would all have passed with the call site deleted -- which is how
the gap was found: a mutant that removed it failed to apply and nothing went
red, indistinguishable from a passing test. There is now an end-to-end test
where a real FFmpeg publishes to a listener whose target is NOT ready and a
subscriber appears 1.2s later. Deleting only the call site makes it fail in
0.05s with `exit status 224` instead of passing in 5.6s: the production failure,
reproduced. A source-level check remains as a cheap backstop for the two
properties that make the wait safe -- only refuseNotReady waits, and the
handshake deadline is extended before it.

ROUTINGPROVISIONAL WAS PINNED BY SOURCE TEXT, which proves the code was written,
not that a client sees the field. Two tests now drive the real handlers and read
the JSON, on both the list and the single-destination paths -- separate handlers
with separate copies of the decision, the kind that get updated in one place and
not the other. They also fail if the preview is REFUSED rather than flagged,
because withholding it would break configuring a destination before going live.

Every mutant here was checked for having actually applied and compiled before
its kill was believed. Three mutants earlier today silently failed to apply, and
a mutant that does not apply looks exactly like a test that passed.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
Both reconcile-race tests wrote their fake ffprobe as `#!/bin/sh` with no
extension. Every Unix runs that; Windows cannot exec it at all. So on
windows-latest the probe never happened and both tests failed with "timed out
waiting for the probe to measure the source" -- which reads like a race that
only manifests on Windows rather than a fixture that never ran, and would have
sent whoever picked it up looking in the engine.

The fake is now COMPILED, with .exe on Windows, and driven by environment
variables so one program serves both tests: FAKE_PROBE_JSON to print,
FAKE_PROBE_ENTERED to touch on entry, FAKE_PROBE_RELEASE to wait for. It stays
a real process rather than becoming a mock, because probeOnce owns that
boundary deliberately and both regressions need a probe held in flight on the
far side of it.

Both mutants re-verified after the rewrite, since a fixture change can quietly
turn a test into one that passes for a new reason: removing reconcileMu from
probeLoop still fails the first, and disabling the sourceGen guard still fails
the second.

Also: the alert encoders cap their own item count, and nothing tested that they
ACCOUNT for what they cut. The existing overflow test sets Delivery.Overflow
from the caller; this covers the other source of one, where the encoder
truncates at discordMaxEmbeds or slackMaxAttachments and adds the remainder
itself. Discord rejects a payload carrying more than ten embeds outright, so a
regression there is a 400 on every alert once a burst gets big enough --
precisely when alerts matter most, and silent, because the retry classifier
correctly abandons a 4xx. Raising the cap or dropping the accounting both fail
it now.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
…o PATH

SonarCloud's quality gate went red on new_security_rating: two go:S4036 in the
E-RTMP and SRT phases added earlier, both "make sure the PATH variable only
contains fixed, unwriteable directories".

It is a fair finding. This program is run by CI and by operators, and
exec.Command("ffmpeg", ...) defers resolution to PATH at exec time -- so a
writable directory ordered ahead of the real ffmpeg is a way to have something
else run with their privileges. The repo already resolves it properly in the
rtmpserver tests; the smoketest did not.

Resolved once through exec.LookPath, with every call naming the result. All
four call sites, not only the two the gate flagged: the other two are older code
that was never new enough to trip it, and leaving one of four resolved
differently is how the next person learns the wrong pattern from the file.

Verified end to end after the change -- all three phases still pass, hub
injection, E-RTMP and SRT, each measuring per-destination audio.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
countLinks was called on the NORMALISED message. Normalise collapses runs of
repeated letters -- the step that folds "sssspam" to "spam" -- so it also folds
"http://" to "htp://", "https://" to "htps://" and "www." to "w.". None of the
three strings countLinks searches for can survive it, so it returned 0 for every
message ever sent.

MaxLinks defaults to 3, so this is on for every install: a limit every operator
had configured and none of them had. The scheme-less "www." fallback was the
most thoroughly dead part of it, and it is the one that exists BECAUSE dropping
the scheme is the obvious way past a filter that only counts schemes.

Counted from the raw text at insert now, kept as an int on the entry rather than
a second string beside norm -- this package bounds its memory under a raid on
purpose, and holding the raw message per entry would double the cost of the
thing it is defending against. The repeat detector still compares norm, which is
what normalisation is for.

Nothing caught this because countLinks had no test and no test mentioned links,
so there was never an assertion for the always-zero to fail. Three added: the
schemed case, the bare-domain fallback, and ordinary conversation not tripping
it. Putting the count back on norm fails the first two with empty findings.

Also four gaps in rules.go that the existing tests in checkers_test.go and
engine_test.go do not reach: an uncompiled Rule reaching Match (nil *regexp --
the mutant panics rather than failing), a nil *RuleSet (the DEFAULT state, no
rules configured), an empty pattern (which compiles fine and matches EVERY
message, unlike the unparseable one already covered), and the fact that
run-collapsing folds legitimate doubles -- "moon" to "mon" -- which is only
survivable because Check also matches raw text.

Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
@sonarqubecloud

sonarqubecloud Bot commented Aug 8, 2026

Copy link
Copy Markdown

@rainmanjam
rainmanjam merged commit ba128db into main Aug 8, 2026
34 checks passed
@rainmanjam
rainmanjam deleted the test/ertmp-routing-coverage branch August 14, 2026 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants