Repository navigation
feat(studio): SRT "Via relay" and dial-in addresses from the backend - #259
Conversation
|
This PR looks like part of a multi-PR dependency stack together with, in dependency order: Eyevinn/strom#1258 (Strom resolves Related: Eyevinn/open-live#628 (spec merged in #630) builds on this stack's relay-status cache, and its implementation waits for #629 to merge. |
ApiOutput.connect was typed as a string, but open-live returns the
{ uri, host, port, mode, source, reason? } object from lib/srt-connect.ts.
Add SrtConnect, use it on outputs, and carry the same field on sources
(newer backends add it there too), keeping it in the sources store. Also
type server-info's srtRelay ({ configured, public }).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When server-info reports srtRelay.configured, the SRT direction choice gets a third option next to Listener and Caller. It submits srt+relay://auto (plus the passphrase, if entered); open-live generates the stream ID once and keeps it, also when an edit sends auto again. The form recommends a passphrase, since the relay port is public. Rows, the edit dialogs and the production's SRT OUT copy button show the relay dial-in address from the backend's connect.uri (srt://<relay>?streamid=<id>, "with the passphrase"), or say the relay address is not known yet. An edit without a new passphrase leaves the stored relay address alone, like a listener. An existing relay document keeps its relay choice even if the relay is no longer reported. Listener and caller behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rm fixes Code-review follow-ups on the SRT relay branch: - dialInAddress() takes the backend's `connect` for listener and relay addresses alike (SRT_PUBLIC_HOST, private-host refusal, relay public address), and only falls back to the Strom host when an older backend sends no `connect`. The production's SRT OUT copy uses the same helper instead of its own copy, so Studio and backend no longer compute the public address two ways. - Rows and the relay edit fields say why a dial-in address is unknown (no relay configured, Strom not answering, public host not set) instead of always "relay address not known"; one shared srtRowLabel() for both panels. - Edit only offers "leave blank to keep" for the passphrase when the stored address has the chosen direction, so switching a relay address with a passphrase to Listener no longer drops it silently. - The form checks the passphrase is 10-79 characters (raw, before encoding) for listener and relay addresses, with a clear message. - Cleanup: shared relayAddressPatch(); the stored direction and passphrase are read once per edit (EditState); useSrtRelayConfigured() returns only what callers use; `connect` is excluded from the source create/update input types; SrtConnect.reason and srtRelay.state are typed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f801c22 to
cef0fc3
Compare
…to the Strom host dialInAddress returned only the backend's connect.uri. On a private or localhost STROM_URL (a LAN Strom, reason srt-host-not-loopback-reachable) that is null, so listener sources and outputs lost the dial-in address they used to show, built from the Strom host. A listener with connect.uri null now uses connect.host (or stromHost) with the mode flipped, and the row says it is local network only, with the SRT_PUBLIC_HOST hint. Relay addresses with uri null stay unknown: there is no fallback for the relay's public address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-host listeners The pill on an active production switched to dialInAddress, which is null for caller outputs and was null for a listener whose connect.uri is null, so the pill disappeared for both. A caller output copies its stored address without the passphrase again, as before; a listener uses the dial-in fallback to the Strom host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
buildRelayAddress and buildListenerAddress encoded the passphrase with encodeURIComponent, which leaves ! * ' ( ) ~ as they are. The backend accepts only [A-Za-z0-9._-=&%+] in an SRT query, so a passphrase such as "my!secret(2024)" got a 400 "Invalid SRT relay address format" with no hint at the cause. One helper, encodePassphrase, now encodes those too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
passphraseError counted UTF-16 code units, while Strom checks the decoded passphrase in bytes. A passphrase of 45 "å" passed Studio (45 units) and then failed the Strom build (90 bytes), so the production did not start. It now counts UTF-8 bytes. The limit stays 79: Strom allows 80 bytes, but the Open Live backend refuses more than 79. The help texts and the error say bytes, and that å counts as 2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A query key without '=' (srt://host:9000?latency) crashes GStreamer when the SRT element starts; the backend now refuses it with a 400. Caller addresses typed in the source and output dialogs are checked before they are sent, naming the key. A stored address that is not sent again stays saveable. The listener and relay builders only ever write key=value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backend replaced the unreleased server-info `srtRelay` with an
always-present `srt` { state, modes, relays, inboundReachable, ports }
read from Strom's capabilities (open-live#629). The studio now reads it:
- "Via relay" is offered when `srt.relays` is non-empty. A relay picker
shows when there is more than one relay, or the only one is not named
`default`. A new address on a named relay is `srt+relay://auto@<name>`;
the default relay is written without `@`. On edit, renewing the
passphrase sends `auto` (keeps the stored id and relay), switching relay
sends `auto@<name>` (also `auto@default`), keeping the id. A switch with
a stored passphrase asks for it again, since the sent address replaces
the stored one and the stored passphrase is masked.
- Listener is greyed out, and a new rendezvous caller address refused,
only when Strom answered (`state: ok`) that the mode cannot work, with
"This Strom cannot be reached from outside; use caller or Via relay".
While the state is unknown, unreachable or unsupported every direction
is offered; unreachable adds a subtle note. A stored listener stays
selectable in Edit (open-live#628).
- `connect.warning` (srt-listener-unsupported, srt-rendezvous-unsupported)
is shown on the source/output row and in its Edit dialog; `connect.relay`
and the reason `srt-relay-unknown` name the missing relay and the
configured ones.
The unreleased `srtRelay` field is no longer read, so this needs the
backend from open-live#629; without `srt` no relay is offered.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- 422 `srt-relay-not-configured` and `srt-relay-unknown` on a source or output save read as operator messages, the latter with the relays Strom has (the body's `relays`). - The backend 400 for a query key without a value names the key, in the same words as the studio's own check. - New Source now creates quietly and shows a failed save inline, keeping the dialog open, as New Output and Edit Source already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Open Live now keeps the stored passphrase on an srt+relay://auto[@<relay>] patch without a passphrase key (the relay never sees it; the stream stays encrypted end to end). A relay switch with a blank passphrase field now just sends auto@<relay>, and the switch note says the passphrase stays. The "Type the passphrase again" check is gone. "Remove passphrase" sends an empty passphrase=, so it works on the stored relay and on a relay switch alike. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review: feat(studio): SRT "Via relay" and dial-in addresses from the backend
Verdict: REQUEST CHANGES
Reviewed at head c3a1cbd9. This is the studio half of open-live#628. It has to merge together with open-live#629 (head f4105d75).
Prior findings
Sources: every review and comment on this PR (no reviews, no inline comments, one issue comment) and on open-live#628 (six triage comments), plus the acceptance criteria and test cases in #628.
| # | Finding (source) | Status | Evidence |
|---|---|---|---|
| 1 | Hold off: the PR is part of a stack with strom#1258 and open-live#629, and they are drafts (stack-flag comment on this PR, 2026-11-11) | FIXED | Both PRs left draft at 15:24Z. Strom shipped this as strom#1260 and #1262 (both merged, v0.8.2 released), and #1258 is closed as superseded. The PR body now names the real dependencies. |
| 2 | The PR bodies still say the Strom branches "have no PR numbers yet" (#628 triage, 2026-11-13 ~15:18Z) | FIXED | The PR body now cites strom#1260 and #1262. |
| 3 | Before merge, team-lead must sign off on the amendment in open-live#629 (f554d143) that warns instead of refusing with 422 (#628 triages, 12:20Z / 15:18Z / 16:17Z) |
OPEN | open-live#629 has no reviews and no team-lead sign-off. This PR builds on the amended contract: it has no srt-direction-unsupported handling, and a stored listener can be re-picked in Edit. So it cannot merge before the amendment is signed off. |
| 4 | The spec header needs updating (Status Implemented, Strom 0.8.2, drop the stale strom#1258 references) and ADR-014 must be set to Accepted (#628 triages, 08:18Z / 16:17Z) | OPEN (on #629) | At f4105d75 the spec still says Status: Draft and still cites strom#1258 as open. This is fixed on #629, not here, but the pair merges together. |
| 5 | #628 proposal 2 and test case "Strom with STROM_PORTS: the listener port choice comes from those ports". Spec §6 "Port picker": when srtPorts is null and srt.ports is non-null, offer ports from those ranges and warn inline when a port is outside them |
OPEN | SrtCapabilities.ports is typed (src/lib/api.ts:832) but nothing reads it. useListenerPortMode (src/components/ui/SrtDirectionFields.tsx:20) still looks only at the legacy srtPorts/srtPortState. #628's triage says the issue is closed once #629 and #259 merge, with no separate implementation, so without this the criterion is silently dropped. No human has waived it. |
| 6 | #628 proposal 2: offer Listener only when Strom supports it, Via relay only when there is a relay, Caller always | FIXED | srtOffer / offeredDirection (src/lib/srt.ts:360, :386). Listener is disabled on create (SourcesPanel.tsx:1090, OutputsPanel.tsx:612), and Via relay shows only when srt.relays is non-empty (commit 24af66a5). |
| 7 | #628 proposal 4: existing records stay listed and keep working; Edit does not offer an unsupported direction as a new choice | FIXED | The lists are not filtered. A stored listener stays selectable, and every other record gets Listener disabled (SourcesPanel.tsx:781, OutputsPanel.tsx:696). connect.warning is shown on rows and in Edit (SourcesPanel.tsx:694, :786; OutputsPanel.tsx:497, :700). Commit 24af66a5. |
| 8 | #628 proposal 5 / test case: with an older Strom or default settings, behaviour is unchanged | FIXED | srtOffer offers every mode unless state === 'ok' (src/lib/srt.ts:360). Covered in srt.test.ts and *.srtCapabilities.test.tsx. |
| 9 | #628 test case: STROM_INBOUND_REACHABLE=false with a relay, so the studio offers Caller and Via relay |
FIXED | Covered in SourcesPanel.srtCapabilities.test.tsx and OutputsPanel.srtCapabilities.test.tsx (commit 24af66a5). |
Critical issues (must fix before merge)
- Missing acceptance criterion (row 5): Listener ports from
srt.ports. Implement spec §6 "Port picker". IfsrtPorts(the legacy reservation) is non-null, keep today's behaviour. Otherwise, ifsrt.portsis non-null, offer ports from those{first,last}spans; the spec suggests defaulting to the lowest port not used by a listed source or output. Allow free entry, with an inline warning when the port is outside the spans. Add a component test for the #628STROM_PORTStest case. If the team decides to descope this, a human with authority must say so on #628 and a follow-up issue must be filed. Until then it stays open. - SPEC DIVERGENCE: server-info is never refetched. Spec §6 says the studio reads
server-info.srt"fetched as today, plus a refetch when a create/edit dialog opens".useServerInfocaches a single promise for the life of the page (src/hooks/useServerInfo.ts:13). The backend answersstate: unknown, relays: []until its first capabilities fetch succeeds, andunreachablewhile Strom is down. Failure case: a studio tab opened during a backend restart never offers "Via relay", and never greys out Listener, until the operator reloads the page. Before this PR that cache was fine, because server-info only changed on restart. Now it carries live Strom state. Refetch (or invalidate the cache) when the New/Edit source and output dialogs open. - Merge gate (row 3):
team-leadsign-off on the open-live#629 warn-instead-of-422 amendment. The studio code matches the amended contract, so nothing needs to change here. It must not merge before that sign-off.
Warnings (should fix)
-
srtSaveErrorMessagefor 422srt-relay-not-configuredsays "Choose listener or caller." (src/lib/srt.ts:428). It says this even when Strom has just reported that Listener cannot work. UseunreachableFromOutsideText-style wording based on the currentoffer.listener.
Nits
- On a stored listener record whose Strom reports
listener: false, spec §6 asks for the direction to be labelled "not supported by this Strom" and says switching back should not be offered. This PR shows theconnect.warningline instead and lets you switch back. That is reasonable under the #629 amendment ("reported, never refused"), but write it into the spec amendment so the two agree.
What was verified
- Read the full diff (13 files). Checked against open-live#629 at
f4105d75:- The
server-info.srtshape{state, modes, relays, inboundReachable, ports}matchessrc/routes/status.tsandservices/strom-capabilities.ts. SrtConnect.relay/streamid/reason/warningand the reason and warning values matchlib/srt-connect.ts.- The relay name and id regexes match
SRT_RELAY_NAME_RE/SRT_RELAY_ID_REinlib/srt-relay.ts. - The 400 text matched by
srtSaveErrorMessage(SRT parameter '<key>' has no value) is the text inlib/srt-uri-check.ts:104. - The 422 body
{error, statusCode, code, relays}matches #629's description. ApiError.bodyexists insrc/lib/api.ts.- No fabricated fields found.
- The
- Security: no secrets, tokens or credentials. The passphrase is never put into the dial-in address, and it is percent-encoded into the backend's query charset. No new endpoints, no
innerHTML, no validation weakened: the passphrase checks and the new query-key and rendezvous checks only add restrictions. No dependency changes (package.jsonand the lockfile are untouched). - Local run at
c3a1cbd9:pnpm install --frozen-lockfile,lint(0 warnings),typecheck,test(53 files, 605 tests passed) andbuildall pass. - CI:
build-and-lintpassed. - Every commit has a Co-Authored-By trailer. The PR is on branch
feat/srt-relay, notmain.
…rding Address the review of #259 (open-live#628, spec section 6): - Listener ports from server-info srt.ports: without a legacy reservation, the listener port field offers Strom's STROM_PORTS spans, defaults (when left blank) to the lowest port no listed source or output listens on, and allows free entry with an inline warning when the port is outside the spans. The legacy srtPorts reservation still wins. - connect.warning srt-port-outside-strom-ports (open-live#629) is typed and shown in the source/output rows and Edit dialogs. - useServerInfo no longer caches one answer per page: the New/Edit source and output dialogs refetch it when they open, so a tab opened while the backend still reported state unknown offers Via relay without a reload. A failed refetch keeps the last answer. - The 422 srt-relay-not-configured message follows the current offer: "Choose listener or caller." only when Listener is offered, otherwise it says Strom cannot be reached from outside and points to caller. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
daily-backlog-pr Phase 3: first re-review after the REQUEST_CHANGES verdict, on the new head with the fix commits pushed by the Phase 2 re-entry. |
There was a problem hiding this comment.
PR Review: feat(studio): SRT "Via relay" and dial-in addresses from the backend (re-review at 4b2431af)
Verdict: APPROVE
First re-review after the REQUEST CHANGES at c3a1cbd9. Commit 4b2431af fixes both Critical issues and the Warning; the Edit-dialog nit and the merge-gate sign-off were settled on open-live#629. Checked against open-live#629 at acef6547 (src/ and docs/openapi.yaml): the studio reads no field the backend does not send.
Prior findings
No human comment withdraws or waives any finding.
| # | Finding (source) | Status | Evidence |
|---|---|---|---|
| 1 | Stack hold with strom#1258 / open-live#629 drafts | FIXED | Both out of draft; strom#1260/#1262 merged (v0.8.2); #1258 closed. |
| 2 | PR body lacked Strom PR numbers | FIXED | Body cites strom#1260, #1262. |
| 3 | Merge gate: team-lead sign-off on the #629 amendment | FIXED | 6859992148. |
| 4 | Spec header / ADR-014 Accepted (on #629) | FIXED (on #629) | acef6547. |
| 5 | Critical: listener ports from srt.ports |
FIXED | 4b2431af: spans mode src/components/ui/SrtDirectionFields.tsx:46, freePortsInSpans src/lib/srt.ts:462, portOutsideSpansWarning SrtDirectionFields.tsx:83,197; tests in SourcesPanel/OutputsPanel.srtCapabilities.test.tsx. |
| 6 | Listener only when supported, relay only with relays, Caller always | FIXED | Unchanged since 24af66a5. |
| 7 | Existing records keep working; Edit doesn't offer unsupported direction | FIXED | SourcesPanel.tsx:786 / OutputsPanel.tsx. |
| 8 | Older Strom: behaviour unchanged | FIXED | spans needs non-empty srt.ports. |
| 9 | STROM_INBOUND_REACHABLE=false + relay |
FIXED | Unchanged since 24af66a5. |
| 10 | Critical: server-info never refetched | FIXED | 4b2431af: src/hooks/useServerInfo.ts:44,50; refetch on dialog open SourcesPanel.tsx:188, OutputsPanel.tsx:210; serverInfoRefetch.test.tsx. |
| 11 | Warning: srt-relay-not-configured text ignores unsupported Listener |
FIXED | 4b2431af: src/lib/srt.ts:432. |
| 12 | Nit: stored-listener Edit behaviour into spec | FIXED | Spec on acef6547 :64, :425, :606. |
| 13 | connect.warning: srt-port-outside-strom-ports |
FIXED | src/lib/api.ts:303, src/lib/srt.ts:411. |
Critical issues
None.
Warnings
None.
Nits (optional)
- The suggested free port only skips ports listed in this studio; on a shared Strom without a reservation it could suggest a port another instance uses. The backend's legacy reservation covers that case.
What was verified
- Contract against open-live#629
acef6547:server-info.srt = { state, modes, relays, inboundReachable, ports }(src/routes/status.ts:37-43),{first,last}inclusive spans,connect.warningenum (docs/openapi.yaml:602), relay 422 codes (docs/openapi.yaml:1285-1294). - Local at
4b2431af: lint, typecheck, test (54 files, 623 tests) and build pass. CIbuild-and-lintgreen. - Merge order: open-live#629 first, then the
server-info.srtcheck on staging, then this PR.
Needs the backend, Eyevinn/open-live#629, and Strom 0.8.2 (Eyevinn/strom#1260, #1262, merged). Merge together with #629.
What
SRT directions from Strom's capabilities (Eyevinn/open-live#628)
The studio reads
server-info.srt(state,modes,relays,inboundReachable,ports). It no longer reads the unreleasedsrtRelayfield, which the backend has removed.state: ok) thatmodes.listeneris false. The dialog says why: "This Strom cannot be reached from outside; use caller or Via relay." (without relays: "…use caller."). A new dialog starts on Caller in that case.mode=rendezvousis refused in the form whenmodes.rendezvousis false andstateisok.stateisunknown,unreachableorunsupported, or the backend sends nosrt, every direction is offered.unreachableadds a small muted note.connect.warning(srt-listener-unsupported,srt-rendezvous-unsupported) shows as an amber line on the row and in the Edit dialog."Via relay (one public port)", now with named relays
srt.relaysis non-empty.default. Each option shows its name and publichost:port.defaultcomes first and is preselected.srt+relay://autoon the default relay (written without@), orsrt+relay://auto@<name>on another relay. The passphrase is added when given.autowithout@, so the backend keeps the stored id and relay.auto@<name>, orauto@defaultto go back to the default relay. The id stays the same.auto@<name>and the backend carries the passphrase over. The switch note says "the stream ID and the passphrase stay the same."auto[@<name>]?passphrase=(empty value), on the same relay or a new one.connect.urifor the stored relay. After a relay switch is picked, the dialog shows the new relay's public address as the one used after save.Errors
srt-relay-not-configured/srt-relay-unknownon save: "Strom has no SRT relay of that name. Configured relays: default, eu-north. Pick one of them." (taken from the body'srelays).connect.reasonsrt-relay-unknownon a row:via relay eu-north · stream ID … · Strom has no SRT relay named "eu-north" (configured: default); edit it and pick another relay.=(for example?latency, which crashes GStreamer) is refused in the source and output forms, naming the key. The backend's 400 for it is shown in the same words. The listener and relay builders only writekey=value. Passphrase handling is unchanged (10–80 bytes, percent-encoded).Earlier in this PR
connect.uri) for listener and relay addresses. A listener on a private or loopback Strom withoutSRT_PUBLIC_HOSTfalls back to the Strom host, marked "local network only". The production "SRT OUT" copy buttons use the same helper.! * ' ( ) ~are percent-encoded. Switching a relay with a passphrase to Listener no longer claims to keep the passphrase.connectis typed as the object the backend returns.Verified
eslint(0 warnings),tsc --noEmitandvitest(53 files, 605 tests) after the passphrase-keep change;buildpassed earlier the same day.src/lib/srt.test.tscoversrtOffer,offeredDirection,relayAddressPatch(auto,auto@<name>,auto@default),chosenRelay,bareQueryKey,rendezvousError,srtConnectWarningTextandsrtSaveErrorMessage.SourcesPanel.srtCapabilities.test.tsxandOutputsPanel.srtCapabilities.test.tsxcover when the picker shows, Listener disabled per state, warnings and error messages.Depends on
server-info.srt, named relays,connect.warning/connect.relay, the relay 422s, the 400 for a key without a value).GET /api/capabilities) and #1262 (named relays). The studio no longer usesGET /api/srt-relayfrom feat(srt): dial out through an SRT relay with srt+relay:// (one public UDP port) strom#1258.🤖 Generated with Claude Code