From 82b8194e67ee3238b6fdbd8be53c18381806c29a Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Tue, 21 Jul 2026 16:34:42 -0700 Subject: [PATCH 01/20] fix: debug recordings never reached S3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs, both from the July 4 S3 debug-recording change (bba1d2ca) meeting the June 6 worker-isolated recording change (f2c72925): 1. Ingest workers never got main's S3 config. buildWorkerConfig handed the worker only Record + DataDir, and both workers built a minimal config.CLI with no S3 fields — so S3Configured() was always false in the worker and DebugRecordingCreate silently fell back to local disk under DataDir. Every isolated-ingest recording (MKV/RTMP and WHIP, i.e. production) has been landing on the node's disk, not the bucket. The S3 config now rides the startup handshake (cfg.S3, sent only when recording), the same pipe that already carries the signing keys, and a shared workerCLI() applies it on both worker paths. 2. Nothing ever committed the S3 upload. The MKV tee's pipe writer was never closed, so the dump goroutine's io.Copy never returned and UploadWriter.Close — which is what completes the multipart upload — never ran. Harmless on local disk (bytes flush as written), fatal for S3 (the object never appears; this also broke the in-process path where main DOES have S3 config). The tee wiring is now a shared recordTee() whose finalize closes the pipe and waits, bounded, for the commit; MKVIngest and RunMKVIngestWorker both finalize at teardown. Same story on WHIP: rtcrec's delayed file.Close raced worker-process exit, so FinalizeRecording() now lets the WHIP worker block until the recording is committed. DebugRecordingCreate detaches the upload from the session ctx (WithoutCancel) so teardown's cancel can't abort the in-flight commit. TestRunMKVIngestWorkerRecordsToS3 covers the production shape end to end against a fake path-style S3 server: cfg.S3 over the handshake, the object committed to the bucket verbatim, nothing on local disk. Co-Authored-By: Claude Fable 5 --- pkg/config/config.go | 17 +++- pkg/media/ingest_supervisor.go | 8 ++ pkg/media/ingest_worker.go | 54 +++++++----- pkg/media/ingest_worker_test.go | 113 +++++++++++++++++++++++++ pkg/media/mkv_ingest.go | 43 ++++++++-- pkg/media/whip_worker.go | 10 ++- pkg/rtcrec/recording_peerconnection.go | 45 ++++++++-- pkg/s3/s3.go | 14 +-- 8 files changed, 261 insertions(+), 43 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 1ba3931b..107157e3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1440,6 +1440,17 @@ func (cli *CLI) S3Config() s3.Config { } } +// SetS3Config applies an s3.Config to the CLI's S3 fields — the inverse of +// S3Config, for processes (ingest workers) that receive the S3 destination over +// a handshake instead of from flags. +func (cli *CLI) SetS3Config(c s3.Config) { + cli.S3Endpoint = c.Endpoint + cli.S3Bucket = c.Bucket + cli.S3AccessKeyID = c.AccessKeyID + cli.S3SecretAccessKey = c.SecretAccessKey + cli.S3Region = c.Region +} + // DebugRecordingFile is the write target returned by DebugRecordingCreate: an // *os.File on local disk, or an S3 upload that commits on Close. Name() reports // the destination (path or object key) for logging. @@ -1458,7 +1469,11 @@ type DebugRecordingFile interface { func (cli *CLI) DebugRecordingCreate(ctx context.Context, fpath []string, contentType string, overwrite bool) (DebugRecordingFile, error) { if cli.S3Configured() { key := strings.Join(fpath, "/") - return s3.NewUploadWriter(ctx, s3.NewClient(cli.S3Config()), cli.S3Bucket, key, contentType) + // The recording outlives the ingest session's ctx: Close commits the upload + // during teardown, after that ctx is typically cancelled — a cancelled ctx + // here would abort the upload and lose the object. Callers bound the commit + // with their own finalize waits instead. + return s3.NewUploadWriter(context.WithoutCancel(ctx), s3.NewClient(cli.S3Config()), cli.S3Bucket, key, contentType) } return cli.DataFileCreate(fpath, overwrite) } diff --git a/pkg/media/ingest_supervisor.go b/pkg/media/ingest_supervisor.go index 5f09a984..f268d08f 100644 --- a/pkg/media/ingest_supervisor.go +++ b/pkg/media/ingest_supervisor.go @@ -278,6 +278,14 @@ func (mm *MediaManager) buildWorkerConfig(ctx context.Context, ms MediaSigner) ( } else if rec { cfg.Record = true cfg.DataDir = mm.cli.DataDir + // The worker writes the recording, so it needs main's S3 destination too — + // without it, DebugRecordingCreate inside the worker would silently fall + // back to local disk under DataDir. Only sent when recording, to keep the + // S3 secret out of handshakes that don't need it. + if mm.cli.S3Configured() { + s3cfg := mm.cli.S3Config() + cfg.S3 = &s3cfg + } } // Node transcode signer lets the worker complete to dual-codec itself. If it's // unavailable, the worker emits single-codec (the node doesn't re-transcode the diff --git a/pkg/media/ingest_worker.go b/pkg/media/ingest_worker.go index 8221738a..7125a12e 100644 --- a/pkg/media/ingest_worker.go +++ b/pkg/media/ingest_worker.go @@ -13,6 +13,7 @@ import ( "stream.place/streamplace/pkg/gstinit" "stream.place/streamplace/pkg/log" "stream.place/streamplace/pkg/muxl" + "stream.place/streamplace/pkg/s3" ) // manifestHolder holds the worker's current C2PA manifest. It starts as the @@ -90,14 +91,18 @@ type IngestWorkerConfig struct { Chunked bool `json:"chunked,omitempty"` // Record, when true, makes the worker write a debug recording of this session - // (the MKV/RTMP push body, or the WHIP session) under - // DataDir/debug-recordings//. main evaluates the per-stream DebugRecording - // setting (which needs the DB) and the worker carries it out — so debug - // recording keeps working on the isolated paths without main being in the data - // path, and a recording even survives a main restart. DataDir is set (only when - // Record) to the node data dir the worker writes recordings under. - Record bool `json:"record,omitempty"` - DataDir string `json:"data_dir,omitempty"` + // (the MKV/RTMP push body, or the WHIP session). main evaluates the per-stream + // DebugRecording setting (which needs the DB) and the worker carries it out — + // so debug recording keeps working on the isolated paths without main being in + // the data path, and a recording even survives a main restart. The recording + // streams to S3 under debug-recordings// when S3 is set (production), and + // falls back to DataDir/debug-recordings// on local disk otherwise (dev). + // DataDir and S3 are set (only when Record) from main's config; S3 carries the + // secret key, which is fine here — the handshake exists to carry key material + // off argv/env. + Record bool `json:"record,omitempty"` + DataDir string `json:"data_dir,omitempty"` + S3 *s3.Config `json:"s3,omitempty"` // Transport selects the worker's ingest source: "" / "mkv" reads MKV media // (stdin or InputFD); "whip" makes the worker own the WebRTC PeerConnection, @@ -112,6 +117,18 @@ type IngestWorkerConfig struct { // IngestTransportWHIP is the cfg.Transport value selecting the WHIP worker. const IngestTransportWHIP = "whip" +// workerCLI assembles the minimal config.CLI a worker runs with: the +// broadcaster identity plus the debug-recording destination (S3 when main +// handed its config over the handshake, else local disk under DataDir). Shared +// by the MKV and WHIP workers so both record to the same place main would. +func (cfg IngestWorkerConfig) workerCLI() *config.CLI { + cli := &config.CLI{BroadcasterHost: cfg.BroadcasterHost, DataDir: cfg.DataDir} + if cfg.S3 != nil { + cli.SetS3Config(*cfg.S3) + } + return cli +} + // WorkerInput reconstructs the raw media stream the gst pipeline reads from the // fd-passed push connection: prepend any bytes main already read past the headers // (Prebuf), then de-chunk if the push used chunked transfer-encoding. For stdin @@ -211,23 +228,22 @@ func RunMKVIngestWorker(ctx context.Context, cfg IngestWorkerConfig, stdin io.Re // Minimal manager: the broadcaster identity the transcode completion // (finishTranscodedSegment) stamps into the node-signed AAC track, plus the - // data dir for an optional debug recording. - mm := &MediaManager{cli: &config.CLI{BroadcasterHost: cfg.BroadcasterHost, DataDir: cfg.DataDir}} + // destination for an optional debug recording. + mm := &MediaManager{cli: cfg.workerCLI()} onSegment, flush := mm.workerSegmentSink(ctx, cfg, frames) - // Debug recording: tee the ingest media to a file before it reaches gst. main - // decided this (cfg.Record) and handed us DataDir; recording here keeps main + // Debug recording: tee the ingest media before it reaches gst. main decided + // this (cfg.Record) and handed us the destination; recording here keeps main // out of the data path and lets the recording survive a main restart. media := stdin if cfg.Record { log.Log(ctx, "recording ingest media to file", "streamer", cfg.StreamerDID) - pr, pw := io.Pipe() - media = io.TeeReader(stdin, pw) - go func() { - if derr := mm.dumpToFile(ctx, pr, cfg.StreamerDID, ".rtmp.mkv"); derr != nil { - log.Error(ctx, "ingest worker: dump recording to file", "error", derr) - } - }() + var finalize func() + media, finalize = mm.recordTee(ctx, stdin, cfg.StreamerDID, ".rtmp.mkv") + // Registered before the pipeline's SetState(Null) defer, so it runs after + // the pipeline stops reading — and before this worker process exits, which + // would otherwise strand an uncommitted S3 upload. + defer finalize() } signerElem, done, err := muxlSignSegmentElem(ctx, mm.cli, workerSignStream(cfg, getManifest), onSegment) diff --git a/pkg/media/ingest_worker_test.go b/pkg/media/ingest_worker_test.go index 2615a631..74336d8f 100644 --- a/pkg/media/ingest_worker_test.go +++ b/pkg/media/ingest_worker_test.go @@ -6,9 +6,12 @@ import ( "errors" "fmt" "io" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" + "sync" "testing" "time" @@ -19,6 +22,7 @@ import ( "stream.place/streamplace/pkg/gstinit" "stream.place/streamplace/pkg/ingestframe" "stream.place/streamplace/pkg/muxl" + "stream.place/streamplace/pkg/s3" ) // TestWorkerInputDeframes checks the body-deframing the worker applies to the @@ -219,6 +223,115 @@ func TestRunMKVIngestWorkerRecords(t *testing.T) { }, 10*time.Second, 25*time.Millisecond, "worker records the ingest media verbatim") } +// fakeS3Server is a minimal path-style S3 endpoint speaking just enough of the +// multipart-upload protocol for UploadWriter: initiate → upload parts → +// complete. Completed objects land in objects keyed by "/". +type fakeS3Server struct { + mu sync.Mutex + parts map[string][]byte // "#" → body + objects map[string][]byte // completed "/" → body +} + +func newFakeS3Server() *fakeS3Server { + return &fakeS3Server{parts: map[string][]byte{}, objects: map[string][]byte{}} +} + +func (f *fakeS3Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + path := strings.TrimPrefix(r.URL.Path, "/") + q := r.URL.Query() + switch { + case r.Method == "POST" && q.Has("uploads"): + fmt.Fprintf(w, `test-upload`) + case r.Method == "PUT" && q.Has("partNumber"): + body, _ := io.ReadAll(r.Body) + f.parts[path+"#"+q.Get("partNumber")] = body + w.Header().Set("ETag", `"part-`+q.Get("partNumber")+`"`) + case r.Method == "POST" && q.Has("uploadId"): + var buf []byte + for i := 1; ; i++ { + part, ok := f.parts[fmt.Sprintf("%s#%d", path, i)] + if !ok { + break + } + buf = append(buf, part...) + } + f.objects[path] = buf + fmt.Fprintf(w, `%s`, path) + default: + w.WriteHeader(http.StatusBadRequest) + } +} + +// objectWithPrefix finds a completed object whose key starts with prefix and +// ends with suffix (the recording's timestamped filename isn't predictable). +func (f *fakeS3Server) objectWithPrefix(prefix, suffix string) ([]byte, bool) { + f.mu.Lock() + defer f.mu.Unlock() + for path, b := range f.objects { + if strings.HasPrefix(path, prefix) && strings.HasSuffix(path, suffix) { + return b, true + } + } + return nil, false +} + +// TestRunMKVIngestWorkerRecordsToS3 proves the debug recording streams to S3 +// when main hands its S3 config over the handshake (cfg.S3) — the production +// shape. Without that plumbing the worker's minimal CLI has no S3 fields and +// DebugRecordingCreate silently falls back to local disk, which is exactly the +// regression this guards against: recordings must land in the bucket, not under +// DataDir. +func TestRunMKVIngestWorkerRecordsToS3(t *testing.T) { + ctx := context.Background() + ms := newBareSegmentSigner(t) + + keyPEM, err := signers.MarshalES256KPrivateKeyPEM(ms.Signer) + require.NoError(t, err) + manifest, err := ms.buildManifest(ctx, time.Now().UnixMilli()) + require.NoError(t, err) + + fake := newFakeS3Server() + srv := httptest.NewServer(fake) + defer srv.Close() + + dataDir := t.TempDir() + cfg := IngestWorkerConfig{ + StreamerDID: ms.Streamer(), + KeyPEM: keyPEM, + CertPEM: ms.Cert, + Manifest: manifest, + BroadcasterHost: "test.example.com", + Record: true, + DataDir: dataDir, + S3: &s3.Config{ + Endpoint: srv.URL, + Bucket: "debug-bucket", + AccessKeyID: "test-access", + SecretAccessKey: "test-secret", + Region: "auto", + }, + } + + mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + + require.NoError(t, RunMKVIngestWorker(ctx, cfg, bytes.NewReader(mkv), ingestframe.NewWriter(io.Discard), func() []byte { return cfg.Manifest })) + + // The upload commits asynchronously (the dump goroutine's Close); the object + // must appear at debug-bucket/debug-recordings//.rtmp.mkv holding + // exactly the ingested media. + wantPrefix := "debug-bucket/debug-recordings/" + ms.Streamer() + "/" + require.Eventually(t, func() bool { + got, ok := fake.objectWithPrefix(wantPrefix, ".rtmp.mkv") + return ok && bytes.Equal(got, mkv) + }, 10*time.Second, 25*time.Millisecond, "worker streams the recording to the S3 bucket verbatim") + + // And nothing fell back to local disk. + matches, _ := filepath.Glob(filepath.Join(dataDir, "debug-recordings", "*", "*")) + require.Empty(t, matches, "recording must go to S3, not DataDir") +} + // TestRunMKVIngestWorkerSelfWatchdog proves the worker's OWN watchdog contains a // wedge. This is the only wedge containment on the detached/WHIP paths, where // main can't kill a detached worker — so the worker has to notice it's stuck and diff --git a/pkg/media/mkv_ingest.go b/pkg/media/mkv_ingest.go index 64182fcf..2e9451a5 100644 --- a/pkg/media/mkv_ingest.go +++ b/pkg/media/mkv_ingest.go @@ -22,14 +22,9 @@ func (mm *MediaManager) MKVIngest(ctx context.Context, input io.Reader, ms Media } if shouldRecord { log.Log(ctx, "recording RTMP stream to file", "streamer", ms.Streamer()) - pr, pw := io.Pipe() - input = io.TeeReader(input, pw) - go func() { - err := mm.dumpToFile(ctx, pr, ms.Streamer(), ".rtmp.mkv") - if err != nil { - log.Error(ctx, "error dumping to file", "error", err) - } - }() + var finalize func() + input, finalize = mm.recordTee(ctx, input, ms.Streamer(), ".rtmp.mkv") + defer finalize() } else { log.Log(ctx, "not recording RTMP stream to file", "streamer", ms.Streamer()) } @@ -127,6 +122,38 @@ func buildMKVIngestPipeline(ctx context.Context, input io.Reader, signerElem *gs return pipeline, nil } +// debugRecordingFlushTimeout bounds how long ingest teardown waits for a debug +// recording to finalize — for S3 the commit only happens at Close, so an +// unbounded wait could wedge teardown while an unwaited exit loses the object. +const debugRecordingFlushTimeout = 30 * time.Second + +// recordTee wires up a debug recording: everything read through the returned +// reader is teed into an asynchronous dumpToFile. The returned finalize ends +// the dump (closing the tee's pipe — the dump's io.Copy never sees EOF +// otherwise, since a TeeReader doesn't propagate one) and waits, bounded, for +// it to commit. Callers MUST finalize after ingest ends: on the S3 path the +// object only exists once Close commits the upload, so skipping it (e.g. a +// worker process exiting) silently loses the recording. +func (mm *MediaManager) recordTee(ctx context.Context, r io.Reader, user string, filesuffix string) (io.Reader, func()) { + pr, pw := io.Pipe() + done := make(chan struct{}) + go func() { + defer close(done) + if err := mm.dumpToFile(ctx, pr, user, filesuffix); err != nil { + log.Error(ctx, "error dumping to file", "error", err, "streamer", user) + } + }() + finalize := func() { + pw.Close() + select { + case <-done: + case <-time.After(debugRecordingFlushTimeout): + log.Error(ctx, "debug recording did not finalize in time", "streamer", user) + } + } + return io.TeeReader(r, pw), finalize +} + func (mm *MediaManager) dumpToFile(ctx context.Context, r io.Reader, user string, filesuffix string) error { now := aqtime.FromTime(time.Now()) filename := fmt.Sprintf("%s%s", now.FileSafeString(), filesuffix) diff --git a/pkg/media/whip_worker.go b/pkg/media/whip_worker.go index de598bcb..61a98e08 100644 --- a/pkg/media/whip_worker.go +++ b/pkg/media/whip_worker.go @@ -7,7 +7,6 @@ import ( "os" "github.com/pion/webrtc/v4" - "stream.place/streamplace/pkg/config" "stream.place/streamplace/pkg/gstinit" "stream.place/streamplace/pkg/log" "stream.place/streamplace/pkg/rtcrec" @@ -59,11 +58,11 @@ func ServeWHIPIngestWorkerSocket(ctx context.Context, cfg IngestWorkerConfig) er return runErr } - mm := &MediaManager{cli: &config.CLI{BroadcasterHost: cfg.BroadcasterHost, DataDir: cfg.DataDir}} + mm := &MediaManager{cli: cfg.workerCLI()} // The worker owns the PeerConnection (its own UDP sockets), built with the // same codec/interceptor setup as the in-process server. Debug recording is - // decided by main (cfg.Record) and written by the worker under cfg.DataDir. + // decided by main (cfg.Record) and written by the worker (S3 or cfg.DataDir). api, webrtcConfig, err := newWebRTCAPI() if err != nil { return finish(fmt.Errorf("webrtc api: %w", err)) @@ -110,5 +109,10 @@ func ServeWHIPIngestWorkerSocket(ctx context.Context, cfg IngestWorkerConfig) er cancel() <-signerDone flush() + // The recording commits asynchronously after pc.Close (drain sleep + S3 + // commit); wait for it, or this process exits and the object never appears. + if rpc, ok := pc.(*rtcrec.RecordingPeerConnection); ok { + rpc.FinalizeRecording(ctx) + } return finish(streamErr) } diff --git a/pkg/rtcrec/recording_peerconnection.go b/pkg/rtcrec/recording_peerconnection.go index 29de5eac..34a8347d 100644 --- a/pkg/rtcrec/recording_peerconnection.go +++ b/pkg/rtcrec/recording_peerconnection.go @@ -3,6 +3,7 @@ package rtcrec import ( "context" "fmt" + "sync" "time" "github.com/pion/rtcp" @@ -13,10 +14,12 @@ import ( ) type RecordingPeerConnection struct { - enabled bool - pionpc *webrtc.PeerConnection - file config.DebugRecordingFile - stream *RecorderStream + enabled bool + pionpc *webrtc.PeerConnection + file config.DebugRecordingFile + stream *RecorderStream + closeOnce sync.Once + recDone chan struct{} // closed once the recording file/upload is committed } func NewRecordingPeerConnection(ctx context.Context, cli config.CLI, user string, pionpc *webrtc.PeerConnection, enabled bool) (PeerConnection, error) { @@ -43,6 +46,7 @@ func NewRecordingPeerConnection(ctx context.Context, cli config.CLI, user string file: f, stream: stream, enabled: enabled, + recDone: make(chan struct{}), }, nil } @@ -53,14 +57,43 @@ func (pc *RecordingPeerConnection) Do(f func()) { } func (pc *RecordingPeerConnection) Close() error { - pc.Do(func() { + pc.Do(pc.finishRecording) + return pc.pionpc.Close() +} + +// finishRecording drains stragglers, commits the recording (for S3, Close IS +// the commit), and signals recDone. Idempotent — Close on the disconnect path +// and FinalizeRecording at worker exit can both trigger it. +func (pc *RecordingPeerConnection) finishRecording() { + pc.closeOnce.Do(func() { // This is sloppy but there might be other goroutines still writing so let's chill for a sec time.Sleep(10 * time.Second) pc.file.Close() + close(pc.recDone) }) - return pc.pionpc.Close() } +// FinalizeRecording blocks until the debug recording is committed (bounded). +// Call it before process exit on paths like the WHIP ingest worker: Close only +// *starts* the drain+commit on a goroutine, and a process that exits first +// strands an uncommitted S3 upload — the object never appears. No-op when not +// recording. +func (pc *RecordingPeerConnection) FinalizeRecording(ctx context.Context) { + if !pc.enabled { + return + } + go pc.finishRecording() // in case nothing called Close (e.g. pipeline error) + select { + case <-pc.recDone: + case <-time.After(recordingFinalizeTimeout): + log.Error(ctx, "debug recording did not finalize in time", "file", pc.file.Name()) + } +} + +// recordingFinalizeTimeout bounds FinalizeRecording: the 10s straggler drain in +// finishRecording plus generous headroom for the S3 commit. +const recordingFinalizeTimeout = 40 * time.Second + func (pc *RecordingPeerConnection) CreateAnswer(options *webrtc.AnswerOptions) (webrtc.SessionDescription, error) { now := time.Now() ret, err := pc.pionpc.CreateAnswer(options) diff --git a/pkg/s3/s3.go b/pkg/s3/s3.go index 2351f97c..6bad8e1a 100644 --- a/pkg/s3/s3.go +++ b/pkg/s3/s3.go @@ -15,13 +15,15 @@ import ( "stream.place/streamplace/pkg/log" ) -// Config holds the configuration for an S3-compatible upload target. +// Config holds the configuration for an S3-compatible upload target. The json +// tags exist because it rides the ingest-worker startup handshake (a dedicated +// pipe fd, never argv/env — it carries the secret key). type Config struct { - Endpoint string - Bucket string - AccessKeyID string - SecretAccessKey string - Region string + Endpoint string `json:"endpoint,omitempty"` + Bucket string `json:"bucket,omitempty"` + AccessKeyID string `json:"access_key_id,omitempty"` + SecretAccessKey string `json:"secret_access_key,omitempty"` + Region string `json:"region,omitempty"` } // Recorder is an optional persistence hook for S3Uploader. RecordStart is -- 2.51.2 From 1c69d6787f61da5da498526e83e957aa3f25d108 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Tue, 21 Jul 2026 17:19:18 -0700 Subject: [PATCH 02/20] s3: bound multipart ops; rtcrec: log failed recording commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two Greptile P1s on #1209: - MultipartWriter's S3 calls had no deadline (the SDK's default HTTP client has none), so a stalled connection could block its caller forever — notably a debug-recording commit, whose writer deliberately runs on a non-cancellable ctx so session teardown can't abort it. Every operation now carries a generous per-op timeout (10m per part, 2m for create/complete/abort), so a genuine stall errors out and surfaces instead of leaking a wedged goroutine + dangling multipart upload. - rtcrec's finishRecording discarded pc.file.Close()'s error, so a failed S3 commit looked identical to success. The outcome is now logged either way; recDone still means "attempt finished" — with the session over there's nothing better to do with a failure than say so loudly. Co-Authored-By: Claude Fable 5 --- pkg/rtcrec/recording_peerconnection.go | 24 ++++++++++++++++-------- pkg/s3/multipart_writer.go | 21 ++++++++++++++++++++- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/pkg/rtcrec/recording_peerconnection.go b/pkg/rtcrec/recording_peerconnection.go index 34a8347d..3e43f0dc 100644 --- a/pkg/rtcrec/recording_peerconnection.go +++ b/pkg/rtcrec/recording_peerconnection.go @@ -18,8 +18,9 @@ type RecordingPeerConnection struct { pionpc *webrtc.PeerConnection file config.DebugRecordingFile stream *RecorderStream + logCtx context.Context // for finishRecording's logs (it outlives the session) closeOnce sync.Once - recDone chan struct{} // closed once the recording file/upload is committed + recDone chan struct{} // closed once the finalize ATTEMPT is over — check the logs for commit failures } func NewRecordingPeerConnection(ctx context.Context, cli config.CLI, user string, pionpc *webrtc.PeerConnection, enabled bool) (PeerConnection, error) { @@ -46,6 +47,7 @@ func NewRecordingPeerConnection(ctx context.Context, cli config.CLI, user string file: f, stream: stream, enabled: enabled, + logCtx: context.WithoutCancel(ctx), recDone: make(chan struct{}), }, nil } @@ -63,21 +65,27 @@ func (pc *RecordingPeerConnection) Close() error { // finishRecording drains stragglers, commits the recording (for S3, Close IS // the commit), and signals recDone. Idempotent — Close on the disconnect path -// and FinalizeRecording at worker exit can both trigger it. +// and FinalizeRecording at worker exit can both trigger it. recDone means the +// attempt finished, not that it succeeded: a failed commit is logged loudly +// (there is nothing better to do with it at this point — the session is over). func (pc *RecordingPeerConnection) finishRecording() { pc.closeOnce.Do(func() { // This is sloppy but there might be other goroutines still writing so let's chill for a sec time.Sleep(10 * time.Second) - pc.file.Close() + if err := pc.file.Close(); err != nil { + log.Error(pc.logCtx, "debug recording commit FAILED; the recording is lost", "file", pc.file.Name(), "error", err) + } else { + log.Log(pc.logCtx, "debug recording committed", "file", pc.file.Name()) + } close(pc.recDone) }) } -// FinalizeRecording blocks until the debug recording is committed (bounded). -// Call it before process exit on paths like the WHIP ingest worker: Close only -// *starts* the drain+commit on a goroutine, and a process that exits first -// strands an uncommitted S3 upload — the object never appears. No-op when not -// recording. +// FinalizeRecording blocks until the debug recording's commit attempt finishes +// (bounded; failures are logged by finishRecording). Call it before process +// exit on paths like the WHIP ingest worker: Close only *starts* the +// drain+commit on a goroutine, and a process that exits first strands an +// uncommitted S3 upload — the object never appears. No-op when not recording. func (pc *RecordingPeerConnection) FinalizeRecording(ctx context.Context) { if !pc.enabled { return diff --git a/pkg/s3/multipart_writer.go b/pkg/s3/multipart_writer.go index 714f07c0..9891030c 100644 --- a/pkg/s3/multipart_writer.go +++ b/pkg/s3/multipart_writer.go @@ -37,6 +37,17 @@ const MultipartPartSize = 16 * 1024 * 1024 // so a 1.5 GB upload dragged on for tens of minutes. const multipartUploadConcurrency = 8 +// Per-operation deadlines for MultipartWriter's S3 calls. The SDK's default +// HTTP client has no request timeout, so without these a stalled connection +// blocks its caller forever — e.g. a debug-recording commit, whose writer +// deliberately runs on a non-cancellable ctx (config.DebugRecordingCreate) so +// session teardown can't abort it. Values are far above healthy operation +// times; only genuine stalls hit them. +const ( + s3PartOpTimeout = 10 * time.Minute // one ≤MultipartPartSize UploadPart + s3ControlOpTimeout = 2 * time.Minute // create/complete/abort/empty-put +) + // multipartAPI is the subset of *s3.Client that MultipartWriter calls. // Pulled out so tests can inject a fake; *s3.Client satisfies it. type multipartAPI interface { @@ -105,7 +116,9 @@ func newMultipartWriter(ctx context.Context, client multipartAPI, bucket, key, c if contentType != "" { in.ContentType = aws.String(contentType) } - resp, err := client.CreateMultipartUpload(ctx, in) + cctx, cancel := context.WithTimeout(ctx, s3ControlOpTimeout) + defer cancel() + resp, err := client.CreateMultipartUpload(cctx, in) if err != nil { span.RecordError(err) return nil, fmt.Errorf("create multipart upload s3://%s/%s: %w", bucket, key, err) @@ -175,6 +188,8 @@ func (w *MultipartWriter) uploadPart(num int32, body []byte) { attribute.Int("part_size_bytes", len(body)), )) defer span.End() + ctx, cancel := context.WithTimeout(ctx, s3PartOpTimeout) + defer cancel() resp, err := w.client.UploadPart(ctx, &s3.UploadPartInput{ Bucket: aws.String(w.bucket), Key: aws.String(w.key), @@ -244,6 +259,8 @@ func (w *MultipartWriter) Complete() error { return err } span.SetAttributes(attribute.Int("part_count", len(w.parts))) + ctx, cancel := context.WithTimeout(ctx, s3ControlOpTimeout) + defer cancel() if len(w.parts) == 0 { // Zero-byte upload: S3 won't accept an empty CompletedMultipartUpload, // so abort and create an empty object via PutObject. @@ -308,6 +325,8 @@ func (w *MultipartWriter) Abort() error { attribute.Int("parts_pending", len(w.parts)), )) defer span.End() + ctx, cancel := context.WithTimeout(ctx, s3ControlOpTimeout) + defer cancel() _, err := w.client.AbortMultipartUpload(ctx, &s3.AbortMultipartUploadInput{ Bucket: aws.String(w.bucket), Key: aws.String(w.key), -- 2.51.2 From f199a718c8fc8238cf86150ca00650882f28aeaa Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Tue, 21 Jul 2026 17:24:51 -0700 Subject: [PATCH 03/20] media/rtcrec: give recording finalize a slow-uplink-sized window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile round two: the 30/40s finalize waits were shorter than the s3 per-op timeouts, so a slow-but-working upload could be abandoned by a worker exiting even though waiting would have saved the recording. At teardown up to ~128 MB of backpressured parts can still be uploading; give them 5 minutes — a post-stream worker lingering is cheap, a lost recording isn't. A genuinely stalled connection stays bounded by the per-op timeouts and is unrecoverable under any window; past the wait the recording is abandoned and logged, and bucket lifecycle rules should reap the dangling multipart. Co-Authored-By: Claude Fable 5 --- pkg/media/mkv_ingest.go | 9 ++++++++- pkg/rtcrec/recording_peerconnection.go | 9 +++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/media/mkv_ingest.go b/pkg/media/mkv_ingest.go index 2e9451a5..49cc5c63 100644 --- a/pkg/media/mkv_ingest.go +++ b/pkg/media/mkv_ingest.go @@ -125,7 +125,14 @@ func buildMKVIngestPipeline(ctx context.Context, input io.Reader, signerElem *gs // debugRecordingFlushTimeout bounds how long ingest teardown waits for a debug // recording to finalize — for S3 the commit only happens at Close, so an // unbounded wait could wedge teardown while an unwaited exit loses the object. -const debugRecordingFlushTimeout = 30 * time.Second +// Generous on purpose: at teardown there can be up to ~128 MB of backpressured +// parts still uploading (multipartUploadConcurrency × MultipartPartSize), and a +// slow-but-working uplink deserves the time to land them — a post-stream worker +// lingering is cheap, a lost recording isn't. A genuinely stalled connection is +// bounded separately by the s3 package's per-operation timeouts; past this +// window the recording is abandoned (logged by the dump goroutine when its op +// timeouts fire; bucket lifecycle rules should reap the dangling multipart). +const debugRecordingFlushTimeout = 5 * time.Minute // recordTee wires up a debug recording: everything read through the returned // reader is teed into an asynchronous dumpToFile. The returned finalize ends diff --git a/pkg/rtcrec/recording_peerconnection.go b/pkg/rtcrec/recording_peerconnection.go index 3e43f0dc..b7395638 100644 --- a/pkg/rtcrec/recording_peerconnection.go +++ b/pkg/rtcrec/recording_peerconnection.go @@ -99,8 +99,13 @@ func (pc *RecordingPeerConnection) FinalizeRecording(ctx context.Context) { } // recordingFinalizeTimeout bounds FinalizeRecording: the 10s straggler drain in -// finishRecording plus generous headroom for the S3 commit. -const recordingFinalizeTimeout = 40 * time.Second +// finishRecording plus generous headroom for the S3 commit — enough for a +// slow-but-working uplink to land any backpressured parts (a post-stream worker +// lingering is cheap, a lost recording isn't). A genuinely stalled connection +// is bounded separately by the s3 package's per-operation timeouts; past this +// window the recording is abandoned and the commit failure logged when those +// fire. +const recordingFinalizeTimeout = 5 * time.Minute func (pc *RecordingPeerConnection) CreateAnswer(options *webrtc.AnswerOptions) (webrtc.SessionDescription, error) { now := time.Now() -- 2.51.2 From 2177a18eaa20d8dc93950a14c96f756b4a65bc61 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Tue, 21 Jul 2026 17:45:11 -0700 Subject: [PATCH 04/20] media: ingest fMP4 from MistServer instead of MKV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Mist ingest bridge was MKV: an MKVExec process on the Mist side piped the stream into `streamplace live`, which POSTed it to /live. Matroska blocks carry only presentation timestamps, so ingest had to RECONSTRUCT decode timestamps with h264timestamper — which needs to guess a reorder window. For streams whose SPS declares none (VideoToolbox writes no bitstream_restriction), it assumes the worst-case full DPB and mints a constant spurious PTS-DTS offset on streams that never reorder, pushing every GoP's presentation past its segment's declared window; WebRTC playback (push-mode qtdemux in packetize clips to that window) dropped the tail of every GoP — a visible jump at each keyframe. The earlier B-frame fix (fc52def0) and this bug were two horns of the same dilemma: with MKV you either trust a guessed window (stretch B-frame streams) or guess harder (corrupt no-reorder streams). Sidestep the whole class: never use MKV. MP4 track fragments carry real decode timestamps (tfdt/trun + ctts), so nothing is reconstructed. - On PUSH_REWRITE (already the auth point — it mints and caches the signer), main now PULLS Mist's live fMP4 HTTP output for the stream it just named (MistPullIngest). The GET is issued by hand so the raw connection can be fd-passed to the detached worker with prebuf + chunked flag — the exact contract the old hijacked POST provided, with the connection pointing the other way. Zero-downtime detach/reattach, watchdog, ban enforcement, manifest refresh all unchanged. - buildMKVIngestPipeline -> buildMP4IngestPipeline: matroskademux -> qtdemux, h264timestamper deleted. MKV*/RunMKV* identifiers renamed MP4*. - docker/mistserver.json: MKVExec process removed (deploys must set SP_MIST_HTTP_PORT to Mist's HTTP port, 28080 in this config). The /live route and `streamplace live` CLI remain, now speaking fMP4. - Tests ported to fMP4 synthesis (mp4mux fragment-duration=500). The sparse-video wedge and B-frame regressions carry over; new videoPTSDTSOffsets probe asserts the archival property directly: TestMP4IngestMistRealSample runs a real VideoToolbox 720p Mist .mp4 capture (new remote fixture mist-vt-720p.mp4, the stream that repro'd the bug) and requires PTS == DTS on every video sample of every signed segment; TestMP4IngestBFramesValidate requires real reorder offsets are PRESERVED for B-frame streams. MKV-specific nyc-* fixture tests (M_JSON metadata track, matroskademux wedge cuts) retired with the format. Co-Authored-By: Claude Fable 5 --- docker/mistserver.json | 10 - pkg/api/api_internal.go | 46 ++-- pkg/cmd/live.go | 6 +- pkg/cmd/streamplace.go | 10 +- pkg/media/frame_server.go | 10 +- pkg/media/frame_socket_e2e_test.go | 8 +- pkg/media/ingest_daemon.go | 10 +- pkg/media/ingest_daemon_test.go | 4 +- pkg/media/ingest_subprocess_test.go | 30 +-- pkg/media/ingest_supervisor.go | 10 +- pkg/media/ingest_worker.go | 26 +- pkg/media/ingest_worker_test.go | 87 +++---- pkg/media/key_revocation_test.go | 12 +- pkg/media/mist_mkv_ingest_test.go | 274 -------------------- pkg/media/mist_mp4_ingest_test.go | 284 +++++++++++++++++++++ pkg/media/mist_pull.go | 154 +++++++++++ pkg/media/mist_pull_test.go | 113 ++++++++ pkg/media/{mkv_ingest.go => mp4_ingest.go} | 62 ++--- pkg/media/segmenter.go | 2 +- pkg/media/whip_worker.go | 2 +- pkg/media/whip_worker_test.go | 4 +- pkg/media/worker_watchdog.go | 2 +- 22 files changed, 723 insertions(+), 443 deletions(-) delete mode 100644 pkg/media/mist_mkv_ingest_test.go create mode 100644 pkg/media/mist_mp4_ingest_test.go create mode 100644 pkg/media/mist_pull.go create mode 100644 pkg/media/mist_pull_test.go rename pkg/media/{mkv_ingest.go => mp4_ingest.go} (60%) diff --git a/docker/mistserver.json b/docker/mistserver.json index dfb0f830..580b5511 100644 --- a/docker/mistserver.json +++ b/docker/mistserver.json @@ -115,16 +115,6 @@ "stream": { "debug": 5, "name": "stream", - "processes": [ - { - "debug": 5, - "exec": "streamplace live $wildcard", - "exit_unmask": false, - "inconsequential": false, - "process": "MKVExec", - "restart_type": "fixed" - } - ], "source": "push://", "stop_sessions": false, "tags": [] diff --git a/pkg/api/api_internal.go b/pkg/api/api_internal.go index 1b92b862..bde37570 100644 --- a/pkg/api/api_internal.go +++ b/pkg/api/api_internal.go @@ -10,7 +10,6 @@ import ( "net/http" "net/http/pprof" "os" - "regexp" "runtime" rtpprof "runtime/pprof" "strconv" @@ -45,19 +44,16 @@ func (a *StreamplaceAPI) ServeInternalHTTP(ctx context.Context) error { }) } -// lightweight way to authenticate push requests to ourself -var mkvRE *regexp.Regexp - -func init() { - mkvRE = regexp.MustCompile(`^\d+\.mkv$`) -} - func (a *StreamplaceAPI) InternalHandler(ctx context.Context) (http.Handler, error) { router := httprouter.New() broker := misttriggers.NewTriggerBroker() + // serverCtx outlives any single trigger request — the Mist pull ingests + // spawned below run for the life of their stream, not the life of the + // PUSH_REWRITE request that announced it. + serverCtx := ctx broker.OnPushRewrite(func(ctx context.Context, payload *misttriggers.PushRewritePayload) (string, error) { - log.Log(ctx, "got push out start", "streamName", payload.StreamName, "url", payload.URL.String()) + log.Log(ctx, "got push rewrite", "streamName", payload.StreamName, "url", payload.URL.String()) // Extract the last part of the URL path urlPath := payload.URL.Path parts := strings.Split(urlPath, "/") @@ -77,6 +73,20 @@ func (a *StreamplaceAPI) InternalHandler(ctx context.Context) (http.Handler, err a.SignerCacheMu.Unlock() log.Log(ctx, "added key to cache", "mist-stream", out, "streamer", mediaSigner.Streamer()) + // The push is authed and named — ingest it by pulling Mist's live fMP4 + // output for the stream we just named. This replaces the old Mist-side + // MKVExec process (`streamplace live` POSTing MKV back to /live): fMP4 + // carries real decode timestamps, so ingest no longer reconstructs DTS. + // Mist accepts the push right after this trigger returns, so the pull + // retries briefly while the stream boots (mistPullConnect). + go func() { + if perr := a.MediaManager.MistPullIngest(serverCtx, out, mediaSigner); perr != nil { + log.Error(serverCtx, "mist pull ingest ended", "mist-stream", out, "streamer", mediaSigner.Streamer(), "error", perr) + } else { + log.Log(serverCtx, "mist pull ingest ended cleanly", "mist-stream", out, "streamer", mediaSigner.Streamer()) + } + }() + return out, nil }) triggerCollection := misttriggers.NewMistCallbackHandlersCollection(a.CLI, broker) @@ -209,21 +219,22 @@ func (a *StreamplaceAPI) InternalHandler(ctx context.Context) (http.Handler, err _, _ = io.ReadFull(bufrw.Reader, prebuf) } chunked := len(httpReq.TransferEncoding) > 0 && httpReq.TransferEncoding[0] == "chunked" - if derr := a.MediaManager.MKVIngestDetached(reqCtx, conn, prebuf, chunked, mediaSigner); derr != nil { + if derr := a.MediaManager.MP4IngestDetached(reqCtx, conn, prebuf, chunked, mediaSigner); derr != nil { log.Log(reqCtx, "isolated stream ended", "error", derr) } return // connection hijacked; the HTTP response is ours now } // The isolated path needs a hijackable HTTP/1.1 connection (which the - // only real MKV/RTMP-push client — a co-located MistServer pushing over - // localhost — always is). We don't support a non-hijack fallback: it - // couldn't receive mid-stream manifest updates and would stay stuck - // pre-live, so refuse. Such a client can use WHIP instead. + // real /live clients — the `streamplace live` CLI and tests pushing + // over localhost — always are; the Mist ingest itself now arrives via + // MistPullIngest, not this route). We don't support a non-hijack + // fallback: it couldn't receive mid-stream manifest updates and would + // stay stuck pre-live, so refuse. Such a client can use WHIP instead. log.Error(reqCtx, "isolated ingest requires a hijackable HTTP/1.1 connection; refusing push") errors.WriteHTTPInternalServerError(w, "isolated ingest requires a hijackable HTTP/1.1 connection; use WHIP", fmt.Errorf("connection is not hijackable")) return } else { - err = a.MediaManager.MKVIngest(reqCtx, r, mediaSigner) + err = a.MediaManager.MP4Ingest(reqCtx, r, mediaSigner) } if err != nil { @@ -234,7 +245,10 @@ func (a *StreamplaceAPI) InternalHandler(ctx context.Context) (http.Handler, err log.Log(reqCtx, "stream success", "url", httpReq.URL.String()) } - // route to accept an incoming mkv stream from OBS, segment it, and push the segments back to this HTTP handler + // route to accept an incoming fragmented-MP4 stream (the `streamplace live` + // CLI piping from stdin), segment it, and validate the signed segments. + // The co-located MistServer's streams are ingested by pulling its fMP4 + // output instead (MistPullIngest, kicked off from PUSH_REWRITE above). router.POST("/live/:key", handleIncomingStream) router.PUT("/live/:key", handleIncomingStream) diff --git a/pkg/cmd/live.go b/pkg/cmd/live.go index 1a814b16..6229ad59 100644 --- a/pkg/cmd/live.go +++ b/pkg/cmd/live.go @@ -8,7 +8,9 @@ import ( ) func Live(streamKey string, httpInternalAddr string) error { - // Create the URL for the live stream endpoint + // Live POSTs a fragmented-MP4 stream from stdin to the node's /live ingest + // route. (The Mist ingest bridge no longer uses this — the node pulls Mist's + // fMP4 output directly; see MistPullIngest.) url := fmt.Sprintf("http://%s/live/%s", httpInternalAddr, streamKey) // Create a new HTTP request with POST method @@ -18,7 +20,7 @@ func Live(streamKey string, httpInternalAddr string) error { } // Set appropriate headers if needed - req.Header.Set("Content-Type", "video/x-matroska") // Assuming MKV format, adjust if needed + req.Header.Set("Content-Type", "video/mp4") // fragmented MP4 from stdin // Create HTTP client and send the request client := &http.Client{} diff --git a/pkg/cmd/streamplace.go b/pkg/cmd/streamplace.go index fcdfdde9..9e58fb4c 100644 --- a/pkg/cmd/streamplace.go +++ b/pkg/cmd/streamplace.go @@ -866,8 +866,8 @@ func makeStreamCommand(build *config.BuildFlags) *urfavecli.Command { } // makeIngestWorkerCommand is the per-stream isolated ingest worker (Stage 1: -// MKV/RTMP push). The node spawns it; it is not meant for direct use. It reads -// the config handshake from fd 3, the MKV media from stdin, runs the mux + sign +// fMP4 / Mist pull). The node spawns it; it is not meant for direct use. It reads +// the config handshake from fd 3, the fragmented-MP4 media from stdin, runs the mux + sign // pipeline, and writes signed canonical .m4s frames to fd 4 — dedicated fds so // stray stdout/stderr can't corrupt the frame stream. A clean run ends with an // End frame; a fatal error emits an Error frame before exiting non-zero. @@ -920,7 +920,7 @@ func makeIngestWorkerCommand(build *config.BuildFlags) *urfavecli.Command { defer f.Close() raw = f } - return media.ServeMKVIngestWorkerSocket(ctx, cfg, media.WorkerInput(cfg, raw)) + return media.ServeMP4IngestWorkerSocket(ctx, cfg, media.WorkerInput(cfg, raw)) } framesFile := os.NewFile(4, "ingest-frames") @@ -933,7 +933,7 @@ func makeIngestWorkerCommand(build *config.BuildFlags) *urfavecli.Command { // This fd-4 path has no back-channel for manifest updates, so the // manifest stays whatever main built at spawn. It's not used in prod // (api requires a hijackable connection); kept for the worker self-test. - if err := media.RunMKVIngestWorker(ctx, cfg, os.Stdin, frames, func() []byte { return cfg.Manifest }); err != nil { + if err := media.RunMP4IngestWorker(ctx, cfg, os.Stdin, frames, func() []byte { return cfg.Manifest }); err != nil { _ = frames.Error(err.Error()) return err } @@ -995,7 +995,7 @@ func makeRTMPPushWorkerCommand(build *config.BuildFlags) *urfavecli.Command { func makeLiveCommand(build *config.BuildFlags) *urfavecli.Command { cli := config.CLI{Build: build} liveCmd := cli.NewCommand("live") - liveCmd.Usage = "start live stream" + liveCmd.Usage = "start live stream (pipe fragmented MP4 to stdin)" liveCmd.ArgsUsage = "[stream-key]" liveCmd.Action = func(ctx context.Context, cmd *urfavecli.Command) error { args := cmd.Args() diff --git a/pkg/media/frame_server.go b/pkg/media/frame_server.go index 6447e823..a60d7e38 100644 --- a/pkg/media/frame_server.go +++ b/pkg/media/frame_server.go @@ -27,7 +27,7 @@ const workerDrainGrace = 60 * time.Second // FrameWriter is the worker's segment sink. Stage 1 uses a direct framed pipe // (*ingestframe.Writer); the zero-downtime path uses *frameServer, which buffers // across a disconnected main and replays on reconnect. Structurally satisfied by -// *ingestframe.Writer, so RunMKVIngestWorker is agnostic to which it gets. +// *ingestframe.Writer, so RunMP4IngestWorker is agnostic to which it gets. type FrameWriter interface { Segment(seg []byte) error End() error @@ -165,15 +165,15 @@ func (s *frameServer) detachConn(conn net.Conn) { } } -// ServeMKVIngestWorkerSocket runs the ingest worker, delivering its signed +// ServeMP4IngestWorkerSocket runs the ingest worker, delivering its signed // segments to main over a per-session unix socket at cfg.SocketPath with // buffered reconnect — the zero-downtime path. It listens, serves the frame // stream (buffering across any main disconnect), runs the ingest, frames a // trailing End/Error, then lingers until main has drained the buffer before // removing the socket and returning. -func ServeMKVIngestWorkerSocket(ctx context.Context, cfg IngestWorkerConfig, stdin io.Reader) error { +func ServeMP4IngestWorkerSocket(ctx context.Context, cfg IngestWorkerConfig, stdin io.Reader) error { if cfg.SocketPath == "" { - return fmt.Errorf("ServeMKVIngestWorkerSocket: empty socket path") + return fmt.Errorf("ServeMP4IngestWorkerSocket: empty socket path") } ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -192,7 +192,7 @@ func ServeMKVIngestWorkerSocket(ctx context.Context, cfg IngestWorkerConfig, std manifest := newManifestHolder(cfg.Manifest) go serveFrameSocket(ctx, ln, srv, manifest) - runErr := RunMKVIngestWorker(ctx, cfg, stdin, srv, manifest.get) + runErr := RunMP4IngestWorker(ctx, cfg, stdin, srv, manifest.get) if runErr != nil { _ = srv.Error(runErr.Error()) } else { diff --git a/pkg/media/frame_socket_e2e_test.go b/pkg/media/frame_socket_e2e_test.go index 33734803..2c201509 100644 --- a/pkg/media/frame_socket_e2e_test.go +++ b/pkg/media/frame_socket_e2e_test.go @@ -15,7 +15,7 @@ import ( ) // TestWorkerServesFramesOverSocket drives the zero-downtime transport end-to-end -// with a REAL ingest: ServeMKVIngestWorkerSocket runs the full mux+sign+transcode +// with a REAL ingest: ServeMP4IngestWorkerSocket runs the full mux+sign+transcode // pipeline and serves the resulting signed dual-codec segments over a unix // socket; a client connects and reads them through to a clean End. This proves // the socket path carries real signed media (the frameServer reconnect tests @@ -40,10 +40,10 @@ func TestWorkerServesFramesOverSocket(t *testing.T) { SocketPath: sock, } - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) serveDone := make(chan error, 1) - go func() { serveDone <- ServeMKVIngestWorkerSocket(ctx, cfg, bytes.NewReader(mkv)) }() + go func() { serveDone <- ServeMP4IngestWorkerSocket(ctx, cfg, bytes.NewReader(mp4)) }() // Connect once the worker's listener is up (retry the dial briefly). var conn net.Conn @@ -85,7 +85,7 @@ func TestWorkerServesFramesOverSocket(t *testing.T) { case serveErr := <-serveDone: require.NoError(t, serveErr) case <-time.After(30 * time.Second): - t.Fatal("ServeMKVIngestWorkerSocket did not return after the stream drained") + t.Fatal("ServeMP4IngestWorkerSocket did not return after the stream drained") } t.Logf("worker served %d signed segments + End over the socket", segs) } diff --git a/pkg/media/ingest_daemon.go b/pkg/media/ingest_daemon.go index 3cee3517..cab8825b 100644 --- a/pkg/media/ingest_daemon.go +++ b/pkg/media/ingest_daemon.go @@ -62,7 +62,7 @@ func SpawnIngestWorkerDetached(cfg IngestWorkerConfig, media *os.File) (*os.Proc // identifiable in a process listing; key material stays on fd 3. cmd := exec.Command(exe, "ingest-worker", cfg.StreamerDID) setDetached(cmd) // own session, survives a main restart (Linux) - // fd 3 = config; fd 4 = the fd-passed media connection (MKV/RTMP). WHIP owns + // fd 3 = config; fd 4 = the fd-passed media connection (the Mist fMP4 pull, or an fMP4 push). WHIP owns // its own PeerConnection, so it passes no media fd. cmd.ExtraFiles = []*os.File{cfgR} if media != nil { @@ -245,7 +245,7 @@ func (mm *MediaManager) ingestWorkerSocketDir() (string, error) { return dir, nil } -// MKVIngestDetached is the production zero-downtime entry: main has authed the +// MP4IngestDetached is the production zero-downtime entry: main has authed the // push and hijacked its connection; this fd-passes that connection to a DETACHED // worker (own session, survives a main restart) which ingests the media directly // and serves signed segments over a per-session unix socket, and then consumes @@ -256,7 +256,7 @@ func (mm *MediaManager) ingestWorkerSocketDir() (string, error) { // breaks the ingest nor loses output: the worker keeps signing into its buffer, // and the restarted main rediscovers the socket (DiscoverWorkerSockets) and // drains it. -func (mm *MediaManager) MKVIngestDetached(ctx context.Context, conn net.Conn, prebuf []byte, chunked bool, ms MediaSigner) error { +func (mm *MediaManager) MP4IngestDetached(ctx context.Context, conn net.Conn, prebuf []byte, chunked bool, ms MediaSigner) error { cfg, err := mm.buildWorkerConfig(ctx, ms) if err != nil { return err @@ -285,7 +285,7 @@ func (mm *MediaManager) MKVIngestDetached(ctx context.Context, conn net.Conn, pr if err != nil { return fmt.Errorf("spawn detached worker: %w", err) } - spmetrics.IngestWorkerStarts.WithLabelValues("mkv").Inc() + spmetrics.IngestWorkerStarts.WithLabelValues("mp4").Inc() // Ban / key revocation: the detached worker can't notice it itself (no // bus/model), so main watches and kills it. proc.Kill (not ctx cancel) so the @@ -301,7 +301,7 @@ func (mm *MediaManager) MKVIngestDetached(ctx context.Context, conn net.Conn, pr start := time.Now().UnixMilli() manifestSource := func() ([]byte, error) { return mm.streamerManifest(ctx, ms.Streamer(), start) } err = mm.ConsumeWorkerSocket(ctx, cfg.SocketPath, ms.Streamer(), mm.validateSegment(ctx), manifestSource) - recordWorkerExit("mkv", err, ctx.Err()) + recordWorkerExit("mp4", err, ctx.Err()) // Reap the worker unless we're deliberately leaving it running across a main // restart (ctx cancel). On a clean end OR a crash the worker has exited, so // Wait() clears the zombie; only on main shutdown do we let it stay detached diff --git a/pkg/media/ingest_daemon_test.go b/pkg/media/ingest_daemon_test.go index 18e36c4d..d683ce0e 100644 --- a/pkg/media/ingest_daemon_test.go +++ b/pkg/media/ingest_daemon_test.go @@ -59,7 +59,7 @@ func TestDetachedWorkerZeroDowntime(t *testing.T) { InputFD: 4, } - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) mediaR, mediaW, err := os.Pipe() require.NoError(t, err) @@ -67,7 +67,7 @@ func TestDetachedWorkerZeroDowntime(t *testing.T) { require.NoError(t, err) mediaR.Close() // the worker holds its own dup go func() { - _, _ = mediaW.Write(mkv) + _, _ = mediaW.Write(mp4) mediaW.Close() }() diff --git a/pkg/media/ingest_subprocess_test.go b/pkg/media/ingest_subprocess_test.go index 26a3d1d2..31c2115e 100644 --- a/pkg/media/ingest_subprocess_test.go +++ b/pkg/media/ingest_subprocess_test.go @@ -21,7 +21,7 @@ import ( // runIngestWorkerHelper is what the test binary becomes when re-exec'd with the // `ingest-worker` arg (see TestMain). It mirrors makeIngestWorkerCommand exactly: -// config on fd 3, frames on fd 4, MKV on stdin; clean run ends with End, a fatal +// config on fd 3, frames on fd 4, fMP4 on stdin; clean run ends with End, a fatal // error with an Error frame and a non-zero exit. func runIngestWorkerHelper() int { // Test hook: a worker-shaped process that just sleeps (same argv layout as a @@ -57,7 +57,7 @@ func runIngestWorkerHelper() int { defer f.Close() raw = f } - if err := ServeMKVIngestWorkerSocket(context.Background(), cfg, WorkerInput(cfg, raw)); err != nil { + if err := ServeMP4IngestWorkerSocket(context.Background(), cfg, WorkerInput(cfg, raw)); err != nil { return 1 } return 0 @@ -69,7 +69,7 @@ func runIngestWorkerHelper() int { } defer framesFile.Close() frames := ingestframe.NewWriter(framesFile) - if err := RunMKVIngestWorker(context.Background(), cfg, os.Stdin, frames, func() []byte { return cfg.Manifest }); err != nil { + if err := RunMP4IngestWorker(context.Background(), cfg, os.Stdin, frames, func() []byte { return cfg.Manifest }); err != nil { _ = frames.Error(err.Error()) return 1 } @@ -78,8 +78,8 @@ func runIngestWorkerHelper() int { } // TestIngestWorkerSubprocess exercises the real process boundary: it spawns the -// worker as an actual subprocess (config over fd 3, MKV over stdin, frames over -// fd 4 — the exact wiring MKVIngestIsolated uses) and verifies the worker +// worker as an actual subprocess (config over fd 3, fMP4 over stdin, frames over +// fd 4 — the exact wiring MP4IngestIsolated uses) and verifies the worker // produces valid signed segments, a clean End frame, and a zero exit. This is // the part the in-process worker test can't cover: fd passing, the framed wire // protocol over a real pipe, and process lifecycle. @@ -99,14 +99,14 @@ func TestIngestWorkerSubprocess(t *testing.T) { }) require.NoError(t, err) - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) exe, err := os.Executable() require.NoError(t, err) cmd := exec.CommandContext(ctx, exe, "ingest-worker") // Quiet gst in the child (it inherits the parent test's verbose leak-tracer env). cmd.Env = append(os.Environ(), "GST_DEBUG=0", "GST_TRACERS=") - cmd.Stdin = bytes.NewReader(mkv) + cmd.Stdin = bytes.NewReader(mp4) cmd.Stderr = os.Stderr cfgR, cfgW, err := os.Pipe() @@ -153,14 +153,14 @@ func TestIngestWorkerSubprocess(t *testing.T) { t.Logf("worker subprocess emitted %d valid signed segments + clean End", segs) } -// TestMKVIngestIsolatedWedgeContained is the isolation guarantee: an audio-only -// MKV starves the fMP4 muxer's video pad of both data and EOS, so the native +// TestMP4IngestIsolatedWedgeContained is the isolation guarantee: an audio-only +// fMP4 starves the fMP4 muxer's video pad of both data and EOS, so the native // pipeline wedges with no frames and no EOS — exactly the kind of native // wedge that would hang (or, with a runaway buffer, OOM-kill) an in-process // ingest and take the node with it. Run in a worker, it must be contained: the -// watchdog kills the worker and MKVIngestIsolated returns an error, bounded in +// watchdog kills the worker and MP4IngestIsolated returns an error, bounded in // time, with THIS process — the node — still running to assert it. -func TestMKVIngestIsolatedWedgeContained(t *testing.T) { +func TestMP4IngestIsolatedWedgeContained(t *testing.T) { old := ingestWorkerWatchdog ingestWorkerWatchdog = 6 * time.Second defer func() { ingestWorkerWatchdog = old }() @@ -168,10 +168,10 @@ func TestMKVIngestIsolatedWedgeContained(t *testing.T) { mm, _ := getStaticTestMediaManager(t) ms := newBareSegmentSigner(t) - wedge := makeAudioOnlyAACMKV(t, context.Background(), 5) + wedge := makeAudioOnlyAACFMP4(t, context.Background(), 5) start := time.Now() - err := mm.MKVIngestIsolated(context.Background(), bytes.NewReader(wedge), ms) + err := mm.MP4IngestIsolated(context.Background(), bytes.NewReader(wedge), ms) elapsed := time.Since(start) require.Error(t, err, "a wedged worker must surface as an error, not a hang") @@ -208,7 +208,7 @@ func TestWorkerIngestsFromPassedFD(t *testing.T) { }) require.NoError(t, err) - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) exe, err := os.Executable() require.NoError(t, err) @@ -233,7 +233,7 @@ func TestWorkerIngestsFromPassedFD(t *testing.T) { cfgW.Close() }() go func() { - _, _ = mediaW.Write(mkv) + _, _ = mediaW.Write(mp4) mediaW.Close() }() diff --git a/pkg/media/ingest_supervisor.go b/pkg/media/ingest_supervisor.go index 5f09a984..d82091e8 100644 --- a/pkg/media/ingest_supervisor.go +++ b/pkg/media/ingest_supervisor.go @@ -27,7 +27,7 @@ import ( // shorten it. var ingestWorkerWatchdog = 30 * time.Second -// MKVIngestIsolated is the process-isolated counterpart to MKVIngest. Instead of +// MP4IngestIsolated is the process-isolated counterpart to MP4Ingest. Instead of // running the demux + sign pipeline in this process — where a native gst fault, // OOM, or deadlock would take the whole node down — it spawns a dedicated // `ingest-worker` subprocess that owns the pipeline and streams signed canonical @@ -38,7 +38,7 @@ var ingestWorkerWatchdog = 30 * time.Second // Per the locked design the worker signs everything, so main hands it the // streamer key + cert + a once-built manifest over a dedicated config fd (kept // off argv/env). See buildWorkerConfig for the interim key-custody note. -func (mm *MediaManager) MKVIngestIsolated(ctx context.Context, input io.Reader, ms MediaSigner) error { +func (mm *MediaManager) MP4IngestIsolated(ctx context.Context, input io.Reader, ms MediaSigner) error { cfg, err := mm.buildWorkerConfig(ctx, ms) if err != nil { return err @@ -104,7 +104,7 @@ func (mm *MediaManager) MKVIngestIsolated(ctx context.Context, input io.Reader, } cfgR.Close() // the child holds its own copy now framesW.Close() // ditto; the parent only reads framesR - spmetrics.IngestWorkerStarts.WithLabelValues("mkv-fd").Inc() + spmetrics.IngestWorkerStarts.WithLabelValues("mp4-fd").Inc() go func() { _, _ = cfgW.Write(cfgJSON) @@ -160,7 +160,7 @@ func (mm *MediaManager) MKVIngestIsolated(ctx context.Context, input io.Reader, } else if werr != nil && !sawEnd { exitErr = werr } - recordWorkerExit("mkv-fd", exitErr, ctx.Err()) + recordWorkerExit("mp4-fd", exitErr, ctx.Err()) switch { case readErr != nil: @@ -245,7 +245,7 @@ func streamWorkerLogs(ctx context.Context, stderr io.Reader, streamer string) { // buildWorkerConfig extracts the handshake the worker needs to sign on main's // behalf. INTERIM key custody: requires a software MediaSignerLocal — the -// MKV/RTMP push path always provides one; anything else errors so the caller can +// Mist-pull/RTMP path always provides one; anything else errors so the caller can // fall back to the in-process path. func (mm *MediaManager) buildWorkerConfig(ctx context.Context, ms MediaSigner) (IngestWorkerConfig, error) { local, ok := ms.(*MediaSignerLocal) diff --git a/pkg/media/ingest_worker.go b/pkg/media/ingest_worker.go index 8221738a..bdd17c42 100644 --- a/pkg/media/ingest_worker.go +++ b/pkg/media/ingest_worker.go @@ -50,7 +50,7 @@ func (h *manifestHolder) set(b []byte) { // approach; the detach/reattach work will revisit how a worker holds keys. type IngestWorkerConfig struct { StreamerDID string `json:"streamer_did"` - // KeyPEM is the streamer's ES256K signing key in PEM. The MKV/RTMP push path + // KeyPEM is the streamer's ES256K signing key in PEM. The Mist-pull/RTMP path // always yields a software key, which is what muxl-sign wants here; it is // forwarded verbatim, no reconstruction. KeyPEM []byte `json:"key_pem"` @@ -90,7 +90,7 @@ type IngestWorkerConfig struct { Chunked bool `json:"chunked,omitempty"` // Record, when true, makes the worker write a debug recording of this session - // (the MKV/RTMP push body, or the WHIP session) under + // (the fMP4 ingest body, or the WHIP session) under // DataDir/debug-recordings//. main evaluates the per-stream DebugRecording // setting (which needs the DB) and the worker carries it out — so debug // recording keeps working on the isolated paths without main being in the data @@ -99,9 +99,9 @@ type IngestWorkerConfig struct { Record bool `json:"record,omitempty"` DataDir string `json:"data_dir,omitempty"` - // Transport selects the worker's ingest source: "" / "mkv" reads MKV media - // (stdin or InputFD); "whip" makes the worker own the WebRTC PeerConnection, - // built from OfferSDP — no media fd to pass. + // Transport selects the worker's ingest source: "" / "mp4" reads fragmented + // MP4 media (stdin or InputFD); "whip" makes the worker own the WebRTC + // PeerConnection, built from OfferSDP — no media fd to pass. Transport string `json:"transport,omitempty"` // OfferSDP is the WHIP client's SDP offer (transport "whip"). The worker // generates the answer and emits it as the first frame (ingestframe.Answer) @@ -131,7 +131,7 @@ func WorkerInput(cfg IngestWorkerConfig, raw io.Reader) io.Reader { // the streamer key PEM + cert straight to muxl-sign, no MediaSigner / model / DB // needed. The manifest is read FRESH per GoP from the holder, so a manifest main // pushes mid-stream (e.g. pre-live → live) takes effect on the next GoP — the -// same fresh-per-GoP shape as the in-process signer. Shared by the MKV and WHIP +// same fresh-per-GoP shape as the in-process signer. Shared by the MP4 and WHIP // workers. func workerSignStream(cfg IngestWorkerConfig, getManifest func() []byte) SignSegmentStreamFunc { return func(ctx context.Context, input io.Reader, eventCh chan *muxl.MuxlEvent) error { @@ -152,7 +152,7 @@ func workerSignStream(cfg IngestWorkerConfig, getManifest func() []byte) SignSeg // segment); flush Closes that transcoder so its ~1-GoP tail is framed before the // worker exits. The transcoder runs on a non-cancellable context so draining the // signer can't kill it early. One process == one session, so the per-DID -// transcoder-reuse hazard can't arise. Shared by the MKV and WHIP workers. +// transcoder-reuse hazard can't arise. Shared by the MP4 and WHIP workers. func (mm *MediaManager) workerSegmentSink(ctx context.Context, cfg IngestWorkerConfig, frames FrameWriter) (onSegment func(context.Context, []byte) error, flush func()) { var transcoder *streamTranscoder onSegment = func(_ context.Context, segment []byte) error { @@ -187,9 +187,9 @@ func (mm *MediaManager) workerSegmentSink(ctx context.Context, cfg IngestWorkerC return onSegment, flush } -// RunMKVIngestWorker is the body of the `ingest-worker` subcommand. It reads an -// MKV stream from stdin, runs the same demux + Opus re-encode + muxl-sign -// pipeline as the in-process MKVIngest, and emits each signed canonical .m4s +// RunMP4IngestWorker is the body of the `ingest-worker` subcommand. It reads a +// fragmented-MP4 stream from stdin, runs the same demux + Opus re-encode + muxl-sign +// pipeline as the in-process MP4Ingest, and emits each signed canonical .m4s // segment to frames; the main process reads those frames and runs ValidateMP4 // over each, exactly as if onSegment had called it directly. // @@ -197,7 +197,7 @@ func (mm *MediaManager) workerSegmentSink(ctx context.Context, cfg IngestWorkerC // caller frames End or Error accordingly. All segment frames are guaranteed // flushed before it returns, so a trailing End can never race ahead of the last // Segment. -func RunMKVIngestWorker(ctx context.Context, cfg IngestWorkerConfig, stdin io.Reader, frames FrameWriter, getManifest func() []byte) error { +func RunMP4IngestWorker(ctx context.Context, cfg IngestWorkerConfig, stdin io.Reader, frames FrameWriter, getManifest func() []byte) error { gstinit.InitGST() ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -224,7 +224,7 @@ func RunMKVIngestWorker(ctx context.Context, cfg IngestWorkerConfig, stdin io.Re pr, pw := io.Pipe() media = io.TeeReader(stdin, pw) go func() { - if derr := mm.dumpToFile(ctx, pr, cfg.StreamerDID, ".rtmp.mkv"); derr != nil { + if derr := mm.dumpToFile(ctx, pr, cfg.StreamerDID, ".rtmp.mp4"); derr != nil { log.Error(ctx, "ingest worker: dump recording to file", "error", derr) } }() @@ -234,7 +234,7 @@ func RunMKVIngestWorker(ctx context.Context, cfg IngestWorkerConfig, stdin io.Re if err != nil { return fmt.Errorf("build signer element: %w", err) } - pipeline, err := buildMKVIngestPipeline(ctx, media, signerElem) + pipeline, err := buildMP4IngestPipeline(ctx, media, signerElem) if err != nil { return fmt.Errorf("build pipeline: %w", err) } diff --git a/pkg/media/ingest_worker_test.go b/pkg/media/ingest_worker_test.go index 2615a631..ee1bd5b3 100644 --- a/pkg/media/ingest_worker_test.go +++ b/pkg/media/ingest_worker_test.go @@ -25,7 +25,7 @@ import ( // fd-passed push connection: prebuf (bytes main read past the headers) is // prepended, and a chunked transfer-encoding is decoded back to the raw media. func TestWorkerInputDeframes(t *testing.T) { - payload := []byte("the-actual-media-bytes-pretend-this-is-mkv-data") + payload := []byte("the-actual-media-bytes-pretend-this-is-mp4-data") // A textbook chunked body: one chunk then the zero terminator. body := []byte(fmt.Sprintf("%x\r\n%s\r\n0\r\n\r\n", len(payload), payload)) @@ -41,18 +41,15 @@ func TestWorkerInputDeframes(t *testing.T) { require.Equal(t, payload, got) } -// makeH264AACMKV builds a clean, single-track, streamable H264+AAC MKV from an -// H264+Opus MP4 fixture (video passed through, audio transcoded Opus→AAC). The -// repo's only AAC fixture (sample-stream.mkv) carries four audio tracks, which -// the single-audio ingest pipeline leaves three of unlinked — wedging -// matroskademux with no EOS. This produces exactly the 1-video-1-audio AAC MKV -// the RTMP push path actually delivers. -func makeH264AACMKV(t *testing.T, ctx context.Context, srcMP4 string) []byte { +// makeH264AACFMP4 builds a clean, 1-video-1-audio fragmented H264+AAC MP4 from +// an H264+Opus MP4 fixture (video passed through, audio transcoded Opus→AAC) — +// exactly the shape MistServer's live .mp4 output delivers for an RTMP push. +func makeH264AACFMP4(t *testing.T, ctx context.Context, srcMP4 string) []byte { t.Helper() gstinit.InitGST() desc := strings.Join([]string{ "filesrc location=" + srcMP4 + " ! qtdemux name=d", - "d. ! queue ! h264parse ! matroskamux name=mux streamable=true ! appsink name=sink", + "d. ! queue ! h264parse ! mp4mux name=mux fragment-duration=500 ! appsink name=sink", "d. ! queue ! opusdec ! audioconvert ! audioresample ! fdkaacenc ! aacparse ! mux.", }, "\n") pipeline, err := gst.NewPipelineFromString(desc) @@ -69,23 +66,21 @@ func makeH264AACMKV(t *testing.T, ctx context.Context, srcMP4 string) []byte { go func() { busErr <- HandleBusMessages(ctx, pipeline) }() require.NoError(t, pipeline.SetState(gst.StatePlaying)) defer func() { _ = pipeline.SetState(gst.StateNull) }() - require.NoError(t, <-busErr, "remux to H264+AAC MKV") - require.NotEmpty(t, buf.Bytes(), "remux produced an MKV") + require.NoError(t, <-busErr, "remux to fragmented H264+AAC MP4") + require.NotEmpty(t, buf.Bytes(), "remux produced an fMP4") return buf.Bytes() } -// makeAudioOnlyAACMKV synthesizes an AAC-audio-only streamable MKV — the +// makeAudioOnlyAACFMP4 synthesizes an AAC-audio-only fragmented MP4 — the // canonical WEDGE input for watchdog/containment tests. The ingest pipeline -// hardwires a video and an audio branch; with no video track, matroskademux -// never creates a video pad, the fMP4 aggregator's video pad never sees data -// OR EOS, and the pipeline hangs forever with no frames and no EOS — a true -// native wedge that no queue sizing can fix. (The 4-audio sample-stream.mkv -// previously used for this stopped wedging once the ingest branches moved to -// Queue2Big: its wedge was really the 1s default-queue interleave deadlock.) -func makeAudioOnlyAACMKV(t *testing.T, ctx context.Context, seconds int) []byte { +// hardwires a video and an audio branch; with no video track, qtdemux never +// creates a video pad, the fMP4 aggregator's video pad never sees data OR +// EOS, and the pipeline hangs forever with no frames and no EOS — a true +// native wedge that no queue sizing can fix. +func makeAudioOnlyAACFMP4(t *testing.T, ctx context.Context, seconds int) []byte { t.Helper() gstinit.InitGST() - desc := fmt.Sprintf("audiotestsrc num-buffers=%d samplesperbuffer=1024 ! audio/x-raw,rate=48000,channels=2 ! audioconvert ! fdkaacenc ! aacparse ! matroskamux streamable=true ! appsink name=sink", seconds*47) + desc := fmt.Sprintf("audiotestsrc num-buffers=%d samplesperbuffer=1024 ! audio/x-raw,rate=48000,channels=2 ! audioconvert ! fdkaacenc ! aacparse ! mp4mux fragment-duration=500 ! appsink name=sink", seconds*47) pipeline, err := gst.NewPipelineFromString(desc) require.NoError(t, err) @@ -100,18 +95,18 @@ func makeAudioOnlyAACMKV(t *testing.T, ctx context.Context, seconds int) []byte go func() { busErr <- HandleBusMessages(ctx, pipeline) }() require.NoError(t, pipeline.SetState(gst.StatePlaying)) defer func() { _ = pipeline.SetState(gst.StateNull) }() - require.NoError(t, <-busErr, "synthesize audio-only MKV") + require.NoError(t, <-busErr, "synthesize audio-only fMP4") require.NotEmpty(t, buf.Bytes()) return buf.Bytes() } -// TestRunMKVIngestWorkerProducesValidSignedFrames drives the isolated ingest -// worker's core directly (no subprocess): feed it an H264+AAC MKV, collect the +// TestRunMP4IngestWorkerProducesValidSignedFrames drives the isolated ingest +// worker's core directly (no subprocess): feed it an H264+AAC fMP4, collect the // framed output, and verify every emitted segment is a valid signed canonical // .m4s. This is the contract the supervisor relies on — frames it can hand // straight to ValidateMP4. (The real subprocess spawn + fault injection is // Stage 3.) -func TestRunMKVIngestWorkerProducesValidSignedFrames(t *testing.T) { +func TestRunMP4IngestWorkerProducesValidSignedFrames(t *testing.T) { ctx := context.Background() ms := newBareSegmentSigner(t) @@ -132,13 +127,13 @@ func TestRunMKVIngestWorkerProducesValidSignedFrames(t *testing.T) { BroadcasterHost: "test.example.com", } - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) - // All frame writes complete before RunMKVIngestWorker returns (it waits on + // All frame writes complete before RunMP4IngestWorker returns (it waits on // the signer drain), so reading the buffer single-threaded afterwards is safe. var buf bytes.Buffer frames := ingestframe.NewWriter(&buf) - require.NoError(t, RunMKVIngestWorker(ctx, cfg, bytes.NewReader(mkv), frames, func() []byte { return cfg.Manifest })) + require.NoError(t, RunMP4IngestWorker(ctx, cfg, bytes.NewReader(mp4), frames, func() []byte { return cfg.Manifest })) r := ingestframe.NewReader(&buf) var segs int @@ -175,12 +170,12 @@ func TestRunMKVIngestWorkerProducesValidSignedFrames(t *testing.T) { t.Logf("worker emitted %d valid dual-codec segments", segs) } -// TestRunMKVIngestWorkerRecords proves debug recording works INSIDE the worker: +// TestRunMP4IngestWorkerRecords proves debug recording works INSIDE the worker: // with cfg.Record set and a DataDir handed over, the worker tees its ingest -// media to debug-recordings//.rtmp.mkv. This is what keeps debug +// media to debug-recordings//.rtmp.mp4. This is what keeps debug // recording working on the isolated paths where main is out of the data path // (it can't tee the bytes itself), so main decides and the worker records. -func TestRunMKVIngestWorkerRecords(t *testing.T) { +func TestRunMP4IngestWorkerRecords(t *testing.T) { ctx := context.Background() ms := newBareSegmentSigner(t) @@ -200,33 +195,33 @@ func TestRunMKVIngestWorkerRecords(t *testing.T) { DataDir: dataDir, } - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) // Frames are irrelevant here (recording is on the input side); discard them. - require.NoError(t, RunMKVIngestWorker(ctx, cfg, bytes.NewReader(mkv), ingestframe.NewWriter(io.Discard), func() []byte { return cfg.Manifest })) + require.NoError(t, RunMP4IngestWorker(ctx, cfg, bytes.NewReader(mp4), ingestframe.NewWriter(io.Discard), func() []byte { return cfg.Manifest })) - // The recording lands at debug-recordings//.rtmp.mkv and + // The recording lands at debug-recordings//.rtmp.mp4 and // must contain exactly the media the worker ingested. The dump goroutine // flushes asynchronously, so allow it a moment to finish the last write. - glob := filepath.Join(dataDir, "debug-recordings", "*", "*.rtmp.mkv") + glob := filepath.Join(dataDir, "debug-recordings", "*", "*.rtmp.mp4") require.Eventually(t, func() bool { matches, _ := filepath.Glob(glob) if len(matches) != 1 { return false } got, rerr := os.ReadFile(matches[0]) - return rerr == nil && bytes.Equal(got, mkv) + return rerr == nil && bytes.Equal(got, mp4) }, 10*time.Second, 25*time.Millisecond, "worker records the ingest media verbatim") } -// TestRunMKVIngestWorkerSelfWatchdog proves the worker's OWN watchdog contains a +// TestRunMP4IngestWorkerSelfWatchdog proves the worker's OWN watchdog contains a // wedge. This is the only wedge containment on the detached/WHIP paths, where // main can't kill a detached worker — so the worker has to notice it's stuck and -// exit itself. An audio-only MKV starves the muxer's video pad of both data and +// exit itself. An audio-only fMP4 starves the muxer's video pad of both data and // EOS, so the pipeline wedges with no frames; the watchdog must tear it down // and return rather than hang forever. (The fd-4 path's supervisor-side -// watchdog is covered separately by TestMKVIngestIsolatedWedgeContained.) -func TestRunMKVIngestWorkerSelfWatchdog(t *testing.T) { +// watchdog is covered separately by TestMP4IngestIsolatedWedgeContained.) +func TestRunMP4IngestWorkerSelfWatchdog(t *testing.T) { old := ingestWorkerWatchdog ingestWorkerWatchdog = 3 * time.Second defer func() { ingestWorkerWatchdog = old }() @@ -245,12 +240,12 @@ func TestRunMKVIngestWorkerSelfWatchdog(t *testing.T) { BroadcasterHost: "test.example.com", } - wedge := makeAudioOnlyAACMKV(t, ctx, 5) + wedge := makeAudioOnlyAACFMP4(t, ctx, 5) start := time.Now() done := make(chan error, 1) go func() { - done <- RunMKVIngestWorker(ctx, cfg, bytes.NewReader(wedge), ingestframe.NewWriter(io.Discard), func() []byte { return cfg.Manifest }) + done <- RunMP4IngestWorker(ctx, cfg, bytes.NewReader(wedge), ingestframe.NewWriter(io.Discard), func() []byte { return cfg.Manifest }) }() select { case <-done: @@ -258,16 +253,16 @@ func TestRunMKVIngestWorkerSelfWatchdog(t *testing.T) { require.Less(t, elapsed, 25*time.Second, "watchdog bounded the wedge") t.Logf("worker self-terminated on wedge in %s", elapsed.Round(time.Second)) case <-time.After(30 * time.Second): - t.Fatal("worker-side watchdog did not contain the wedge (RunMKVIngestWorker hung)") + t.Fatal("worker-side watchdog did not contain the wedge (RunMP4IngestWorker hung)") } } -// TestRunMKVIngestWorkerSignsWithSuppliedManifest is the core of the pre-live → +// TestRunMP4IngestWorkerSignsWithSuppliedManifest is the core of the pre-live → // live fix: the worker signs each GoP with whatever the manifest getter returns, // NOT a frozen one. Two runs of the same media differ only in the getter — a // pre-live manifest yields unpublished segments; a live manifest (the same plus // a c2pa.published action, as main would push on go-live) yields published ones. -func TestRunMKVIngestWorkerSignsWithSuppliedManifest(t *testing.T) { +func TestRunMP4IngestWorkerSignsWithSuppliedManifest(t *testing.T) { ctx := context.Background() ms := newBareSegmentSigner(t) keyPEM, err := signers.MarshalES256KPrivateKeyPEM(ms.Signer) @@ -285,11 +280,11 @@ func TestRunMKVIngestWorkerSignsWithSuppliedManifest(t *testing.T) { []byte(`{"action":"c2pa.created"},{"action":"c2pa.published"}`), 1) require.NotEqual(t, string(prelive), string(live), "the live manifest must add c2pa.published") - mkv := makeH264AACMKV(t, ctx, getFixture("5sec.mp4")) + mp4 := makeH264AACFMP4(t, ctx, getFixture("5sec.mp4")) firstSegmentPublished := func(manifest []byte) bool { var buf bytes.Buffer - require.NoError(t, RunMKVIngestWorker(ctx, cfg, bytes.NewReader(mkv), + require.NoError(t, RunMP4IngestWorker(ctx, cfg, bytes.NewReader(mp4), ingestframe.NewWriter(&buf), func() []byte { return manifest })) r := ingestframe.NewReader(&buf) typ, payload, rerr := r.ReadFrame() diff --git a/pkg/media/key_revocation_test.go b/pkg/media/key_revocation_test.go index 065d2baf..35cfd91c 100644 --- a/pkg/media/key_revocation_test.go +++ b/pkg/media/key_revocation_test.go @@ -87,20 +87,20 @@ func TestWatchKeyRevocationStreamKick(t *testing.T) { } } -// TestMKVIngestIsolatedBanContained proves the fix end to end: banning a streamer +// TestMP4IngestIsolatedBanContained proves the fix end to end: banning a streamer // mid-ingest tears their isolated worker down. The watchdog is set generously -// (60s) and the input is a wedging audio-only MKV that never ends on its own — +// (60s) and the input is a wedging audio-only fMP4 that never ends on its own — // so a timely return can only be the ban kill, not the watchdog or a natural -// EOS. (The 4-audio sample-stream.mkv previously used here now ingests to +// EOS. (The 4-audio sample-stream.mp4 previously used here now ingests to // completion in a few seconds, which would race the ban.) -func TestMKVIngestIsolatedBanContained(t *testing.T) { +func TestMP4IngestIsolatedBanContained(t *testing.T) { old := ingestWorkerWatchdog ingestWorkerWatchdog = 60 * time.Second defer func() { ingestWorkerWatchdog = old }() mm, _ := getStaticTestMediaManager(t) ms := newBareSegmentSigner(t) - wedge := makeAudioOnlyAACMKV(t, context.Background(), 5) + wedge := makeAudioOnlyAACFMP4(t, context.Background(), 5) // Ban the streamer once the worker is up and the watcher has subscribed. go func() { @@ -112,7 +112,7 @@ func TestMKVIngestIsolatedBanContained(t *testing.T) { }() start := time.Now() - err := mm.MKVIngestIsolated(context.Background(), bytes.NewReader(wedge), ms) + err := mm.MP4IngestIsolated(context.Background(), bytes.NewReader(wedge), ms) elapsed := time.Since(start) require.Error(t, err, "a banned stream is torn down, surfaced as an error") diff --git a/pkg/media/mist_mkv_ingest_test.go b/pkg/media/mist_mkv_ingest_test.go deleted file mode 100644 index 6b67609d..00000000 --- a/pkg/media/mist_mkv_ingest_test.go +++ /dev/null @@ -1,274 +0,0 @@ -package media - -import ( - "bytes" - "context" - "errors" - "fmt" - "io" - "os" - "strings" - "testing" - "time" - - "github.com/go-gst/go-gst/gst" - "github.com/go-gst/go-gst/gst/app" - "github.com/stretchr/testify/require" - "stream.place/streamplace/pkg/crypto/signers" - "stream.place/streamplace/pkg/gstinit" - "stream.place/streamplace/pkg/ingestframe" - "stream.place/streamplace/test/remote" -) - -// runMKVThroughIngestWorker feeds an MKV byte stream through the isolated -// ingest worker and returns how many signed canonical segments it emitted. The -// worker watchdog is shortened so a wedged pipeline returns promptly instead of -// hanging the test (override via SP_TEST_WATCHDOG to e.g. park a wedge for a -// stack dump). With transcode=true the node keys are supplied so the worker -// completes to dual-codec, as production does. -func runMKVThroughIngestWorker(t *testing.T, mkv []byte, transcode bool) (int, error) { - segs, err := runMKVThroughIngestWorkerSegments(t, mkv, transcode) - return len(segs), err -} - -// runMKVThroughIngestWorkerSegments is runMKVThroughIngestWorker returning the -// raw segment payloads, for tests that validate the emitted segments rather -// than just count them. -func runMKVThroughIngestWorkerSegments(t *testing.T, mkv []byte, transcode bool) ([][]byte, error) { - t.Helper() - old := ingestWorkerWatchdog - ingestWorkerWatchdog = 10 * time.Second - if wd := os.Getenv("SP_TEST_WATCHDOG"); wd != "" { - d, perr := time.ParseDuration(wd) - require.NoError(t, perr) - ingestWorkerWatchdog = d - } - defer func() { ingestWorkerWatchdog = old }() - - ctx := context.Background() - ms := newBareSegmentSigner(t) - keyPEM, err := signers.MarshalES256KPrivateKeyPEM(ms.Signer) - require.NoError(t, err) - manifest, err := ms.buildManifest(ctx, time.Now().UnixMilli()) - require.NoError(t, err) - cfg := IngestWorkerConfig{ - StreamerDID: ms.Streamer(), - KeyPEM: keyPEM, - CertPEM: ms.Cert, - Manifest: manifest, - BroadcasterHost: "test.example.com", - } - if transcode { - cfg.NodeCertPEM = ms.Cert - cfg.NodeKeyPEM = keyPEM - } - - var buf bytes.Buffer - runErr := RunMKVIngestWorker(ctx, cfg, bytes.NewReader(mkv), ingestframe.NewWriter(&buf), func() []byte { return cfg.Manifest }) - - r := ingestframe.NewReader(&buf) - var segs [][]byte - for { - typ, payload, rerr := r.ReadFrame() - if errors.Is(rerr, io.EOF) { - break - } - require.NoError(t, rerr) - if typ == ingestframe.Segment { - segs = append(segs, payload) - } - } - return segs, runErr -} - -// makeSparseVideoAACMKV synthesizes the stream shape that wedged production -// ingest: video that degrades to keyframe-only at a low rate (here 0.5fps — -// MistServer drops all delta frames when a push falls behind, leaving ~1s-apart -// keyframes) alongside continuous 48kHz AAC audio, in a streamable MKV. The -// audio branch must buffer a full video-frame gap while matroskademux walks to -// the next video frame; gst's default 1s-capped queue can't, and the -// aggregator-based fMP4 muxer deadlocks (see buildMKVIngestPipeline). -func makeSparseVideoAACMKV(t *testing.T, ctx context.Context, seconds int) []byte { - t.Helper() - gstinit.InitGST() - desc := strings.Join([]string{ - fmt.Sprintf("videotestsrc num-buffers=%d ! video/x-raw,width=320,height=240,framerate=1/2 ! x264enc key-int-max=1 tune=zerolatency speed-preset=ultrafast ! h264parse ! matroskamux name=mux streamable=true ! appsink name=sink", (seconds+1)/2), - fmt.Sprintf("audiotestsrc num-buffers=%d samplesperbuffer=1024 ! audio/x-raw,rate=48000,channels=2 ! audioconvert ! fdkaacenc ! aacparse ! mux.", seconds*47), - }, "\n") - pipeline, err := gst.NewPipelineFromString(desc) - require.NoError(t, err) - - sinkEle, err := pipeline.GetElementByName("sink") - require.NoError(t, err) - var buf bytes.Buffer - app.SinkFromElement(sinkEle).SetCallbacks(&app.SinkCallbacks{ - NewSampleFunc: WriterNewSample(ctx, &buf), - }) - - busErr := make(chan error, 1) - go func() { busErr <- HandleBusMessages(ctx, pipeline) }() - require.NoError(t, pipeline.SetState(gst.StatePlaying)) - defer func() { _ = pipeline.SetState(gst.StateNull) }() - require.NoError(t, <-busErr, "synthesize sparse-video MKV") - require.NotEmpty(t, buf.Bytes()) - return buf.Bytes() -} - -// TestMKVIngestSparseVideoNoWedge is the regression test for a production -// ingest wedge: a stream whose video goes sparse (keyframe-only, ≥1s between -// video frames) deadlocked the MKV ingest pipeline — audio backpressure -// through the default 1s-capped queue blocked the demux, the fMP4 aggregator -// starved on its video pad, and the stream hung with no EOS until the -// watchdog killed it. With Queue2Big on both ingest branches the same stream -// must segment to completion. -func TestMKVIngestSparseVideoNoWedge(t *testing.T) { - ctx := context.Background() - mkv := makeSparseVideoAACMKV(t, ctx, 12) - - segs, err := runMKVThroughIngestWorker(t, mkv, false) - require.NoError(t, err, "sparse-video stream ingests cleanly (wedge → watchdog → context canceled)") - // 12s of 2s-apart keyframes ≈ 6 GoPs; wedging yields 0-1 segments. - require.GreaterOrEqual(t, segs, 4, "sparse-video stream emits its segments") - t.Logf("sparse-video stream: %d segments", segs) -} - -// The nyc-* fixtures are cuts of a real 164s production MistServer MKV push -// whose video degrades to keyframe-only at ~140s (behind-push frame-drop) — -// the capture that wedged production ingest. head = first ~10s; head-nojson = -// the same bytes with the 30-byte M_JSON TrackEntry stripped; tail135 = from -// 135s (~5s before the keyframe-only transition); full = the whole capture. - -// TestMKVIngestMistMetadataTrack: MistServer's MKV push declares a -// live-metadata track (CodecID M_JSON, TrackType 3) as track 1, ahead of the -// AAC audio and H264 video tracks. It was the initial suspect for the -// production wedge but proved benign — matroskademux ignores the unknown -// codec, and the same media segments identically with the 30-byte M_JSON -// TrackEntry stripped (the control). Kept as a canary for the MistServer -// track layout. (The real wedge: TestMKVIngestSparseVideoNoWedge.) -func TestMKVIngestMistMetadataTrack(t *testing.T) { - control, err := os.ReadFile(remote.RemoteFixture("3284ef5658e7864bce326c296a909e985c4167d0b9a445b2ce944c2f0171c71e/nyc-head-nojson.mkv")) - require.NoError(t, err) - mist, err := os.ReadFile(remote.RemoteFixture("c0989e044f3350c55f1e129b76252bfb2859914058bb17d2431a605db9693467/nyc-head.mkv")) - require.NoError(t, err) - - segs, err := runMKVThroughIngestWorker(t, control, false) - require.NoError(t, err, "control (M_JSON TrackEntry stripped) ingests cleanly") - require.GreaterOrEqual(t, segs, 1, "control emits segments") - t.Logf("control: %d segments", segs) - - segs, err = runMKVThroughIngestWorker(t, mist, false) - require.NoError(t, err, "MistServer MKV (with M_JSON track) ingests cleanly") - require.GreaterOrEqual(t, segs, 1, "MistServer MKV emits segments") - t.Logf("with M_JSON track: %d segments", segs) -} - -// TestMKVIngestMistFullSample runs the entire 164s production capture through -// the worker with node transcode keys — the closest in-process approximation -// of the production ingest. The capture degrades to keyframe-only video at -// ~140s (MistServer behind-push frame-drop), which is what wedged production; -// with Queue2Big on the ingest branches the whole capture must segment. -func TestMKVIngestMistFullSample(t *testing.T) { - // This once "progressively slowed until the watchdog fired, then hung in - // the post-cancel drain" and was skip-gated as known-hanging — that was - // the muxl-event-drain-vs-cancel deadlock (see muxlSignSegmentElem's - // drainCtx); with the drain non-cancellable the full capture transcodes - // at full speed (~13s). - mkv, err := os.ReadFile(remote.RemoteFixture("3e4e5d9758e67053908e523379a3e2ef2cf60679d0657a940daf96590e866015/nyc-full.mkv")) - require.NoError(t, err) - segs, err := runMKVThroughIngestWorker(t, mkv, true) - t.Logf("full sample: %d segments, err=%v", segs, err) - require.NoError(t, err, "full production sample ingests cleanly") - // 171 GoPs in the capture (~1s each); wedging yielded ~144. - require.GreaterOrEqual(t, segs, 160, "full sample emits the whole stream's segments") -} - -// TestMKVIngestMistTail is the fast sample-based wedge check: the tail sample -// starts at 135s, ~5s before the capture goes keyframe-only, so an unfixed -// pipeline wedges within seconds (4 segments) instead of minutes. -func TestMKVIngestMistTail(t *testing.T) { - mkv, err := os.ReadFile(remote.RemoteFixture("03df698a342f1ab89dccc20ce0a0283e1270104e6382703575686c9f4a88881e/nyc-tail135.mkv")) - require.NoError(t, err) - segs, err := runMKVThroughIngestWorker(t, mkv, false) - t.Logf("tail sample: %d segments, err=%v", segs, err) - require.NoError(t, err, "tail of production sample ingests cleanly") - // 135s..164s at ~1s GoPs ≈ 29 segments; wedging yields ~5. - require.GreaterOrEqual(t, segs, 20, "tail sample emits segments past the keyframe-only transition") -} - -// makeBFrameAACMKV synthesizes an H264 stream WITH B-frames (PTS ≠ DTS) -// alongside AAC audio in a streamable MKV — the shape a hardware encoder or -// non-zerolatency x264 push produces. Matroska blocks carry only presentation -// timestamps, so on demux the reordered video arrives with dts=none; without -// DTS reconstruction the fMP4 muxer stretches the video track and every -// segment fails validation downstream (see buildMKVIngestPipeline's -// h264timestamper). b-adapt=false forces x264 to actually emit the configured -// B-frames rather than deciding per-scene. -func makeBFrameAACMKV(t *testing.T, ctx context.Context, seconds int) []byte { - t.Helper() - gstinit.InitGST() - desc := strings.Join([]string{ - fmt.Sprintf("videotestsrc num-buffers=%d ! video/x-raw,width=320,height=240,framerate=30/1 ! x264enc bframes=2 b-adapt=false key-int-max=30 speed-preset=veryfast ! h264parse ! matroskamux name=mux streamable=true ! appsink name=sink", seconds*30), - fmt.Sprintf("audiotestsrc num-buffers=%d samplesperbuffer=1024 ! audio/x-raw,rate=48000,channels=2 ! audioconvert ! fdkaacenc ! aacparse ! mux.", seconds*47), - }, "\n") - pipeline, err := gst.NewPipelineFromString(desc) - require.NoError(t, err) - - sinkEle, err := pipeline.GetElementByName("sink") - require.NoError(t, err) - var buf bytes.Buffer - app.SinkFromElement(sinkEle).SetCallbacks(&app.SinkCallbacks{ - NewSampleFunc: WriterNewSample(ctx, &buf), - }) - - busErr := make(chan error, 1) - go func() { busErr <- HandleBusMessages(ctx, pipeline) }() - require.NoError(t, pipeline.SetState(gst.StatePlaying)) - defer func() { _ = pipeline.SetState(gst.StateNull) }() - require.NoError(t, <-busErr, "synthesize B-frame MKV") - require.NotEmpty(t, buf.Bytes()) - return buf.Bytes() -} - -// TestMKVIngestBFramesValidate is the regression test for B-frame MKV ingest: -// every emitted segment must survive the full ValidateMP4Media chokepoint. -// Before DTS reconstruction, mp4mux treated the reordered (dts=none) B-frame -// PTS as monotonic timing, stretching the video track ~2.2×; the segments -// LOOKED fine (both tracks present, signatures valid) but the video/audio -// duration mismatch made push-mode qtdemux EOS the audio pad before the audio -// bytes arrived — muxl's flat wrap is non-interleaved, video first — and every -// segment was rejected with "no audio in segment". -func TestMKVIngestBFramesValidate(t *testing.T) { - ctx := context.Background() - mkv := makeBFrameAACMKV(t, ctx, 8) - - segs, err := runMKVThroughIngestWorkerSegments(t, mkv, false) - require.NoError(t, err, "B-frame stream ingests cleanly") - require.GreaterOrEqual(t, len(segs), 6, "B-frame stream emits its segments") - - sawBFrames := false - for i, seg := range segs { - res, verr := ValidateMP4Media(ctx, seg) - require.NoError(t, verr, "segment %d validates (video+audio both present)", i) - dur := time.Duration(res.MediaData.Duration) - require.Greater(t, dur, 500*time.Millisecond, "segment %d duration sane", i) - require.Less(t, dur, 2*time.Second, "segment %d duration not stretched", i) - if res.MediaData.Video[0].BFrames { - sawBFrames = true - } - } - require.True(t, sawBFrames, "synthesized stream actually contains B-frames — if this fails the test no longer exercises the reorder path") - t.Logf("B-frame stream: %d segments, all validated", len(segs)) -} - -// TestMKVIngestMistFullSampleNoTranscode is TestMKVIngestMistFullSample -// without node keys (segment+sign only). During diagnosis this proved the -// wedge was in the core ingest pipeline, not the dual-codec transcode stage — -// both variants wedged at the same GoP. -func TestMKVIngestMistFullSampleNoTranscode(t *testing.T) { - mkv, err := os.ReadFile(remote.RemoteFixture("3e4e5d9758e67053908e523379a3e2ef2cf60679d0657a940daf96590e866015/nyc-full.mkv")) - require.NoError(t, err) - segs, err := runMKVThroughIngestWorker(t, mkv, false) - t.Logf("full sample (no transcode): %d segments, err=%v", segs, err) - require.NoError(t, err, "full production sample ingests cleanly without transcode") - require.GreaterOrEqual(t, segs, 70, "full sample emits the whole stream's segments") -} diff --git a/pkg/media/mist_mp4_ingest_test.go b/pkg/media/mist_mp4_ingest_test.go new file mode 100644 index 00000000..13a60bcc --- /dev/null +++ b/pkg/media/mist_mp4_ingest_test.go @@ -0,0 +1,284 @@ +package media + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "os" + "strings" + "testing" + "time" + + "github.com/go-gst/go-gst/gst" + "github.com/go-gst/go-gst/gst/app" + "github.com/stretchr/testify/require" + "stream.place/streamplace/pkg/crypto/signers" + "stream.place/streamplace/pkg/gstinit" + "stream.place/streamplace/pkg/ingestframe" + "stream.place/streamplace/pkg/muxl" + "stream.place/streamplace/test/remote" +) + +// runMP4ThroughIngestWorker feeds a fragmented-MP4 byte stream through the +// isolated ingest worker and returns how many signed canonical segments it +// emitted. The worker watchdog is shortened so a wedged pipeline returns +// promptly instead of hanging the test (override via SP_TEST_WATCHDOG to e.g. +// park a wedge for a stack dump). With transcode=true the node keys are +// supplied so the worker completes to dual-codec, as production does. +func runMP4ThroughIngestWorker(t *testing.T, mp4 []byte, transcode bool) (int, error) { + segs, err := runMP4ThroughIngestWorkerSegments(t, mp4, transcode) + return len(segs), err +} + +// runMP4ThroughIngestWorkerSegments is runMP4ThroughIngestWorker returning the +// raw segment payloads, for tests that validate the emitted segments rather +// than just count them. +func runMP4ThroughIngestWorkerSegments(t *testing.T, mp4 []byte, transcode bool) ([][]byte, error) { + t.Helper() + old := ingestWorkerWatchdog + ingestWorkerWatchdog = 10 * time.Second + if wd := os.Getenv("SP_TEST_WATCHDOG"); wd != "" { + d, perr := time.ParseDuration(wd) + require.NoError(t, perr) + ingestWorkerWatchdog = d + } + defer func() { ingestWorkerWatchdog = old }() + + ctx := context.Background() + ms := newBareSegmentSigner(t) + keyPEM, err := signers.MarshalES256KPrivateKeyPEM(ms.Signer) + require.NoError(t, err) + manifest, err := ms.buildManifest(ctx, time.Now().UnixMilli()) + require.NoError(t, err) + cfg := IngestWorkerConfig{ + StreamerDID: ms.Streamer(), + KeyPEM: keyPEM, + CertPEM: ms.Cert, + Manifest: manifest, + BroadcasterHost: "test.example.com", + } + if transcode { + cfg.NodeCertPEM = ms.Cert + cfg.NodeKeyPEM = keyPEM + } + + var buf bytes.Buffer + runErr := RunMP4IngestWorker(ctx, cfg, bytes.NewReader(mp4), ingestframe.NewWriter(&buf), func() []byte { return cfg.Manifest }) + + r := ingestframe.NewReader(&buf) + var segs [][]byte + for { + typ, payload, rerr := r.ReadFrame() + if errors.Is(rerr, io.EOF) { + break + } + require.NoError(t, rerr) + if typ == ingestframe.Segment { + segs = append(segs, payload) + } + } + return segs, runErr +} + +// runSynthPipeline runs a gst-launch description whose sink is an appsink +// named "sink" and returns everything the sink produced. +func runSynthPipeline(t *testing.T, ctx context.Context, desc string) []byte { + t.Helper() + gstinit.InitGST() + pipeline, err := gst.NewPipelineFromString(desc) + require.NoError(t, err) + + sinkEle, err := pipeline.GetElementByName("sink") + require.NoError(t, err) + var buf bytes.Buffer + app.SinkFromElement(sinkEle).SetCallbacks(&app.SinkCallbacks{ + NewSampleFunc: WriterNewSample(ctx, &buf), + }) + + busErr := make(chan error, 1) + go func() { busErr <- HandleBusMessages(ctx, pipeline) }() + require.NoError(t, pipeline.SetState(gst.StatePlaying)) + defer func() { _ = pipeline.SetState(gst.StateNull) }() + require.NoError(t, <-busErr, "synthesize test stream") + require.NotEmpty(t, buf.Bytes()) + return buf.Bytes() +} + +// makeSparseVideoAACFMP4 synthesizes the stream shape that wedged production +// ingest back when the bridge format was MKV: video that degrades to +// keyframe-only at a low rate (here 0.5fps — MistServer drops all delta frames +// when a push falls behind, leaving ~1s-apart keyframes) alongside continuous +// 48kHz AAC audio, in a fragmented MP4. The wedge mechanism is +// demux-agnostic — the audio branch must buffer a full video-frame gap while +// the demux walks the byte stream to the next video frame, and the +// aggregator-based fMP4 muxer downstream consumes nothing until every pad has +// data — so the regression carries over to the qtdemux pipeline (see +// buildMP4IngestPipeline's Queue2Big comment). +func makeSparseVideoAACFMP4(t *testing.T, ctx context.Context, seconds int) []byte { + t.Helper() + desc := strings.Join([]string{ + fmt.Sprintf("videotestsrc num-buffers=%d ! video/x-raw,width=320,height=240,framerate=1/2 ! x264enc key-int-max=1 tune=zerolatency speed-preset=ultrafast ! h264parse ! mp4mux name=mux fragment-duration=500 ! appsink name=sink", (seconds+1)/2), + fmt.Sprintf("audiotestsrc num-buffers=%d samplesperbuffer=1024 ! audio/x-raw,rate=48000,channels=2 ! audioconvert ! fdkaacenc ! aacparse ! mux.", seconds*47), + }, "\n") + return runSynthPipeline(t, ctx, desc) +} + +// TestMP4IngestSparseVideoNoWedge is the regression test for a production +// ingest wedge: a stream whose video goes sparse (keyframe-only, ≥1s between +// video frames) deadlocked the ingest pipeline — audio backpressure through +// the default 1s-capped queue blocked the demux, the fMP4 aggregator starved +// on its video pad, and the stream hung with no EOS until the watchdog killed +// it. With Queue2Big on both ingest branches the same stream must segment to +// completion. +func TestMP4IngestSparseVideoNoWedge(t *testing.T) { + ctx := context.Background() + mp4 := makeSparseVideoAACFMP4(t, ctx, 12) + + segs, err := runMP4ThroughIngestWorker(t, mp4, false) + require.NoError(t, err, "sparse-video stream ingests cleanly (wedge → watchdog → context canceled)") + // 12s of 2s-apart keyframes ≈ 6 GoPs; wedging yields 0-1 segments. + require.GreaterOrEqual(t, segs, 4, "sparse-video stream emits its segments") + t.Logf("sparse-video stream: %d segments", segs) +} + +// videoPTSDTSOffsets flat-wraps a canonical segment and returns every video +// sample's PTS−DTS composition offset. This is the direct probe for the class +// of bug that motivated the fMP4 ingest rewrite: MKV ingest reconstructed DTS +// with h264timestamper, whose SPS fallback minted a constant spurious offset +// for streams that declare no reorder window (VideoToolbox) — pushing every +// GoP's presentation past its segment's declared window and breaking WebRTC +// playback at each keyframe. fMP4 ingest reads the container's real DTS, so a +// no-reorder stream must come out with PTS == DTS on every sample. +func videoPTSDTSOffsets(t *testing.T, ctx context.Context, segment []byte) []time.Duration { + t.Helper() + gstinit.InitGST() + var flat bytes.Buffer + require.NoError(t, muxl.RunMuxlWrap(ctx, bytes.NewReader(segment), "flat", &flat)) + + desc := strings.Join([]string{ + "appsrc name=src ! qtdemux name=demux", + "demux.video_0 ! queue ! h264parse ! appsink name=sink sync=false", + "demux.audio_0 ! queue ! fakesink sync=false", + }, "\n") + pipeline, err := gst.NewPipelineFromString(desc) + require.NoError(t, err) + + srcEle, err := pipeline.GetElementByName("src") + require.NoError(t, err) + app.SrcFromElement(srcEle).SetCallbacks(&app.SourceCallbacks{ + NeedDataFunc: ReaderNeedDataIncremental(ctx, bytes.NewReader(flat.Bytes())), + }) + + sinkEle, err := pipeline.GetElementByName("sink") + require.NoError(t, err) + var offsets []time.Duration + app.SinkFromElement(sinkEle).SetCallbacks(&app.SinkCallbacks{ + NewSampleFunc: func(sink *app.Sink) gst.FlowReturn { + sample := sink.PullSample() + if sample == nil { + return gst.FlowEOS + } + buf := sample.GetBuffer() + pts, dts := buf.PresentationTimestamp(), buf.DecodingTimestamp() + if pts != gst.ClockTimeNone && dts != gst.ClockTimeNone { + offsets = append(offsets, time.Duration(int64(pts)-int64(dts))) + } + return gst.FlowOK + }, + }) + + busErr := make(chan error, 1) + go func() { busErr <- HandleBusMessages(ctx, pipeline) }() + require.NoError(t, pipeline.SetState(gst.StatePlaying)) + defer func() { _ = pipeline.SetState(gst.StateNull) }() + require.NoError(t, <-busErr, "demux flat-wrapped segment") + require.NotEmpty(t, offsets, "segment has video samples with timestamps") + return offsets +} + +// TestMP4IngestMistRealSample runs a real MistServer live-MP4 capture through +// the worker: a VideoToolbox (macOS hardware encoder) 720p H264 + AAC RTMP +// push, pulled from Mist's HTTP .mp4 output — exactly what production ingest +// consumes since the MKV→fMP4 bridge rewrite. VideoToolbox is the interesting +// encoder here because its SPS declares no reorder window, which is what sent +// the old MKV path's h264timestamper into its spurious-offset fallback; this +// stream must instead come through with its real timestamps: PTS == DTS on +// every video sample of every signed segment. +func TestMP4IngestMistRealSample(t *testing.T) { + ctx := context.Background() + mp4, err := os.ReadFile(remote.RemoteFixture("ee4d7f8f9b267ba8229314162ef268186048f91ac4242b13fce3f5ee955b97ae/mist-vt-720p.mp4")) + require.NoError(t, err) + + segs, err := runMP4ThroughIngestWorkerSegments(t, mp4, false) + require.NoError(t, err, "real Mist fMP4 capture ingests cleanly") + require.GreaterOrEqual(t, len(segs), 3, "capture emits its segments") + + for i, seg := range segs { + res, verr := ValidateMP4Media(ctx, seg) + require.NoError(t, verr, "segment %d validates (video+audio both present)", i) + dur := time.Duration(res.MediaData.Duration) + require.Greater(t, dur, 200*time.Millisecond, "segment %d duration sane", i) + require.Less(t, dur, 6*time.Second, "segment %d duration not stretched", i) + require.False(t, res.MediaData.Video[0].BFrames, "VideoToolbox capture has no B-frames") + for _, off := range videoPTSDTSOffsets(t, ctx, seg) { + require.Equal(t, time.Duration(0), off, "segment %d: no-reorder stream must keep PTS == DTS — a nonzero offset means ingest invented a reorder delay", i) + } + } + t.Logf("real Mist capture: %d segments, all validated with PTS == DTS", len(segs)) +} + +// makeBFrameAACFMP4 synthesizes an H264 stream WITH B-frames (PTS ≠ DTS) +// alongside AAC audio in a fragmented MP4 — the shape a hardware encoder or +// non-zerolatency x264 push produces, as delivered by MistServer's live .mp4 +// output. Unlike Matroska, MP4 track fragments carry real decode timestamps, +// so the ingest pipeline needs no DTS reconstruction for the fMP4 muxer to mux +// the reordered stream correctly. b-adapt=false forces x264 to actually emit +// the configured B-frames rather than deciding per-scene. +func makeBFrameAACFMP4(t *testing.T, ctx context.Context, seconds int) []byte { + t.Helper() + desc := strings.Join([]string{ + fmt.Sprintf("videotestsrc num-buffers=%d ! video/x-raw,width=320,height=240,framerate=30/1 ! x264enc bframes=2 b-adapt=false key-int-max=30 speed-preset=veryfast ! h264parse ! mp4mux name=mux fragment-duration=500 ! appsink name=sink", seconds*30), + fmt.Sprintf("audiotestsrc num-buffers=%d samplesperbuffer=1024 ! audio/x-raw,rate=48000,channels=2 ! audioconvert ! fdkaacenc ! aacparse ! mux.", seconds*47), + }, "\n") + return runSynthPipeline(t, ctx, desc) +} + +// TestMP4IngestBFramesValidate is the regression test for B-frame ingest: +// every emitted segment must survive the full ValidateMP4Media chokepoint with +// a sane duration, and the stream's real reorder offsets must be preserved. +// (On the old MKV path this scenario originally lost DTS entirely — mp4mux +// stretched the video track ~2.2× and every segment failed validation with +// "no audio in segment"; the h264timestamper fix for that in turn minted +// spurious offsets on no-reorder streams. fMP4's container DTS sidesteps the +// whole trade-off, and this test pins the B-frame half of it.) +func TestMP4IngestBFramesValidate(t *testing.T) { + ctx := context.Background() + mp4 := makeBFrameAACFMP4(t, ctx, 8) + + segs, err := runMP4ThroughIngestWorkerSegments(t, mp4, false) + require.NoError(t, err, "B-frame stream ingests cleanly") + require.GreaterOrEqual(t, len(segs), 6, "B-frame stream emits its segments") + + sawBFrames := false + sawReorderOffset := false + for i, seg := range segs { + res, verr := ValidateMP4Media(ctx, seg) + require.NoError(t, verr, "segment %d validates (video+audio both present)", i) + dur := time.Duration(res.MediaData.Duration) + require.Greater(t, dur, 500*time.Millisecond, "segment %d duration sane", i) + require.Less(t, dur, 2*time.Second, "segment %d duration not stretched", i) + if res.MediaData.Video[0].BFrames { + sawBFrames = true + } + for _, off := range videoPTSDTSOffsets(t, ctx, seg) { + if off > 0 { + sawReorderOffset = true + } + } + } + require.True(t, sawBFrames, "synthesized stream actually contains B-frames — if this fails the test no longer exercises the reorder path") + require.True(t, sawReorderOffset, "B-frame stream keeps its real PTS−DTS reorder offsets through ingest") + t.Logf("B-frame stream: %d segments, all validated", len(segs)) +} diff --git a/pkg/media/mist_pull.go b/pkg/media/mist_pull.go new file mode 100644 index 00000000..528dcad4 --- /dev/null +++ b/pkg/media/mist_pull.go @@ -0,0 +1,154 @@ +package media + +import ( + "bufio" + "context" + "fmt" + "io" + "net" + "net/http" + "net/url" + "strings" + "time" + + "stream.place/streamplace/pkg/log" +) + +// mistPullConnectGrace bounds how long we retry the initial GET while Mist +// boots the freshly-pushed stream: PUSH_REWRITE fires BEFORE Mist accepts the +// push, so the stream may 404 (or refuse the connection) for a moment before +// media flows. Var (not const) so tests can shorten it. +var mistPullConnectGrace = 30 * time.Second + +// mistPullRetryBackoff paces those initial connect retries. +var mistPullRetryBackoff = 500 * time.Millisecond + +// MistPullIngest ingests a live stream by PULLING MistServer's fragmented-MP4 +// HTTP output for mistStreamName — the replacement for the old MKVExec push +// bridge (Mist exec'ing `streamplace live` and POSTing MKV to /live). Pulling +// .mp4 instead of receiving .mkv matters: MP4 fragments carry real decode +// timestamps, so ingest no longer has to reconstruct DTS from the H264 +// bitstream (see buildMP4IngestPipeline). +// +// It's kicked off from the PUSH_REWRITE trigger — the moment we've authed an +// incoming Mist push and minted its signer — and runs for the life of the +// stream: the GET body ends when the push ends. The isolated path hands the +// raw response connection to a detached worker (fd-passing, exactly like the +// old hijacked-POST path), so a main restart doesn't interrupt the ingest. +func (mm *MediaManager) MistPullIngest(ctx context.Context, mistStreamName string, ms MediaSigner) error { + hostport := fmt.Sprintf("127.0.0.1:%d", mm.cli.MistHTTPPort) + // PathEscape leaves '+' (a legal path character) alone, but HTTP servers + // commonly decode it as a space — Mist wildcard names are full of them + // (stream+_), so escape it explicitly. + path := "/" + strings.ReplaceAll(url.PathEscape(mistStreamName), "+", "%2B") + ".mp4" + ctx = log.WithLogValues(ctx, "streamer", ms.Streamer(), "mist-stream", mistStreamName) + + conn, prebuf, chunked, err := mistPullConnect(ctx, hostport, path) + if err != nil { + return fmt.Errorf("mist pull: %w", err) + } + log.Log(ctx, "mist pull connected", "url", hostport+path) + + if mm.cli.IsolatedIngest { + // Zero-downtime path: the detached worker owns the pull connection (so + // it survives a main restart) and serves signed segments back over its + // socket — the same machinery as the old hijacked inbound push, with + // the connection pointing the other way. + return mm.MP4IngestDetached(ctx, conn, prebuf, chunked, ms) + } + defer conn.Close() + body := WorkerInput(IngestWorkerConfig{Prebuf: prebuf, Chunked: chunked}, conn) + return mm.MP4Ingest(ctx, body, ms) +} + +// mistPullConnect dials Mist's HTTP output and issues the GET by hand — not +// through http.Client — because the isolated path needs the raw *net.TCPConn +// to fd-pass to the worker. It consumes the response headers and returns the +// connection positioned at the body, plus any body bytes the header read +// buffered past the headers (prebuf) and whether the body is chunked — the +// same (conn, prebuf, chunked) shape the old hijacked-POST ingest produced, so +// the downstream machinery is shared unchanged. +// +// It retries while Mist boots the stream (mistPullConnectGrace): a refused +// connection or a non-200 just means the push hasn't started flowing yet. +func mistPullConnect(ctx context.Context, hostport, path string) (*net.TCPConn, []byte, bool, error) { + giveUp := time.Now().Add(mistPullConnectGrace) + for { + conn, prebuf, chunked, err := tryMistGET(ctx, hostport, path) + if err == nil { + return conn, prebuf, chunked, nil + } + if time.Now().After(giveUp) { + return nil, nil, false, fmt.Errorf("stream never came up at %s%s: %w", hostport, path, err) + } + select { + case <-ctx.Done(): + return nil, nil, false, ctx.Err() + case <-time.After(mistPullRetryBackoff): + } + } +} + +// tryMistGET is one attempt: dial, send the GET, read the response headers. +// On a 200 it hands back the connection + buffered body bytes; anything else +// is an error and the connection is closed. +func tryMistGET(ctx context.Context, hostport, path string) (*net.TCPConn, []byte, bool, error) { + d := net.Dialer{Timeout: 5 * time.Second} + raw, err := d.DialContext(ctx, "tcp", hostport) + if err != nil { + return nil, nil, false, err + } + conn, ok := raw.(*net.TCPConn) + if !ok { + raw.Close() + return nil, nil, false, fmt.Errorf("expected TCP connection, got %T", raw) + } + // Connection: close — one stream per connection, body runs to EOF (or + // chunked-EOS) when the Mist stream ends. No keepalive reuse to reason about. + req := "GET " + path + " HTTP/1.1\r\n" + + "Host: " + hostport + "\r\n" + + "User-Agent: streamplace-ingest\r\n" + + "Accept: video/mp4\r\n" + + "Connection: close\r\n\r\n" + if err := conn.SetDeadline(time.Now().Add(10 * time.Second)); err != nil { + conn.Close() + return nil, nil, false, err + } + if _, err := io.WriteString(conn, req); err != nil { + conn.Close() + return nil, nil, false, err + } + br := bufio.NewReader(conn) + resp, err := http.ReadResponse(br, nil) + if err != nil { + conn.Close() + return nil, nil, false, err + } + if resp.StatusCode != http.StatusOK { + conn.Close() + return nil, nil, false, fmt.Errorf("mist returned %s", resp.Status) + } + if err := conn.SetDeadline(time.Time{}); err != nil { // clear; streaming has no deadline + conn.Close() + return nil, nil, false, err + } + chunked := false + for _, te := range resp.TransferEncoding { + if te == "chunked" { + chunked = true + } + } + // The header read buffered some raw body bytes; peel them off the bufio so + // the caller can prepend them to the (otherwise unbuffered) connection. + // resp.Body is deliberately never read — it would de-chunk, and the worker + // wants the raw stream + the chunked flag. + var prebuf []byte + if n := br.Buffered(); n > 0 { + prebuf = make([]byte, n) + if _, err := io.ReadFull(br, prebuf); err != nil { + conn.Close() + return nil, nil, false, err + } + } + return conn, prebuf, chunked, nil +} diff --git a/pkg/media/mist_pull_test.go b/pkg/media/mist_pull_test.go new file mode 100644 index 00000000..8316b816 --- /dev/null +++ b/pkg/media/mist_pull_test.go @@ -0,0 +1,113 @@ +package media + +import ( + "context" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestMistPullConnect drives mistPullConnect against a fake Mist: the first +// request 404s (the push hasn't started flowing yet — PUSH_REWRITE fires +// before Mist accepts the push), the second streams a chunked body. The +// connector must retry through the 404, then hand back the raw connection + +// buffered bytes + chunked flag such that WorkerInput reconstructs exactly the +// media bytes — the same contract the old hijacked-POST path provided. +func TestMistPullConnect(t *testing.T) { + payload := make([]byte, 256*1024) // big enough to outsize any header-read buffering + for i := range payload { + payload[i] = byte(i) + } + + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/stream%2Bdid:test:abc_123.mp4", r.URL.EscapedPath()) + if calls.Add(1) == 1 { + http.Error(w, "stream not ready", http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "video/mp4") + fl := w.(http.Flusher) + // Stream in pieces with flushes so Go's server chunks the response — + // the shape Mist's live output has. + for i := 0; i < len(payload); i += 4096 { + end := min(i+4096, len(payload)) + _, err := w.Write(payload[i:end]) + require.NoError(t, err) + fl.Flush() + } + })) + defer srv.Close() + + oldGrace, oldBackoff := mistPullConnectGrace, mistPullRetryBackoff + mistPullConnectGrace, mistPullRetryBackoff = 5*time.Second, 10*time.Millisecond + defer func() { mistPullConnectGrace, mistPullRetryBackoff = oldGrace, oldBackoff }() + + hostport := srv.Listener.Addr().String() + conn, prebuf, chunked, err := mistPullConnect(context.Background(), hostport, "/stream%2Bdid:test:abc_123.mp4") + require.NoError(t, err) + defer conn.Close() + require.GreaterOrEqual(t, calls.Load(), int32(2), "connector retried through the 404") + require.True(t, chunked, "streamed live body is chunked") + + got, err := io.ReadAll(WorkerInput(IngestWorkerConfig{Prebuf: prebuf, Chunked: chunked}, conn)) + require.NoError(t, err) + require.Equal(t, payload, got, "prebuf + raw conn de-frames to the exact media bytes") +} + +// TestMistPullConnectGivesUp: a stream that never comes up must fail within +// the connect grace instead of retrying forever. +func TestMistPullConnectGivesUp(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "no such stream", http.StatusNotFound) + })) + defer srv.Close() + + oldGrace, oldBackoff := mistPullConnectGrace, mistPullRetryBackoff + mistPullConnectGrace, mistPullRetryBackoff = 300*time.Millisecond, 20*time.Millisecond + defer func() { mistPullConnectGrace, mistPullRetryBackoff = oldGrace, oldBackoff }() + + _, _, _, err := mistPullConnect(context.Background(), srv.Listener.Addr().String(), "/nope.mp4") + require.Error(t, err) + require.Contains(t, err.Error(), "never came up") +} + +// TestMistPullConnectRefusedThenUp: Mist itself may not even be listening yet +// (or between restarts); a refused connection is retried like a 404. +func TestMistPullConnectRefusedThenUp(t *testing.T) { + // Reserve a port, then close the listener so the first dials are refused. + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + hostport := l.Addr().String() + require.NoError(t, l.Close()) + + oldGrace, oldBackoff := mistPullConnectGrace, mistPullRetryBackoff + mistPullConnectGrace, mistPullRetryBackoff = 5*time.Second, 20*time.Millisecond + defer func() { mistPullConnectGrace, mistPullRetryBackoff = oldGrace, oldBackoff }() + + go func() { + time.Sleep(200 * time.Millisecond) + l2, lerr := net.Listen("tcp", hostport) + if lerr != nil { + return // port raced away; the test will fail on the connect error + } + _ = http.Serve(l2, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "media") + w.(http.Flusher).Flush() + })) + }() + + conn, prebuf, chunked, err := mistPullConnect(context.Background(), hostport, "/late.mp4") + require.NoError(t, err) + defer conn.Close() + got, err := io.ReadAll(WorkerInput(IngestWorkerConfig{Prebuf: prebuf, Chunked: chunked}, conn)) + require.NoError(t, err) + require.Equal(t, []byte("media"), got) +} diff --git a/pkg/media/mkv_ingest.go b/pkg/media/mp4_ingest.go similarity index 60% rename from pkg/media/mkv_ingest.go rename to pkg/media/mp4_ingest.go index 64182fcf..77314fda 100644 --- a/pkg/media/mkv_ingest.go +++ b/pkg/media/mp4_ingest.go @@ -14,24 +14,25 @@ import ( "stream.place/streamplace/pkg/log" ) -// ingest a H264+AAC MKV stream (prolly from an RTMP server) -func (mm *MediaManager) MKVIngest(ctx context.Context, input io.Reader, ms MediaSigner) error { +// ingest a H264+AAC fragmented-MP4 stream (the MistServer live .mp4 output, or +// an fMP4 push to /live) +func (mm *MediaManager) MP4Ingest(ctx context.Context, input io.Reader, ms MediaSigner) error { shouldRecord, err := mm.shouldRecord(ctx, ms.Streamer()) if err != nil { return err } if shouldRecord { - log.Log(ctx, "recording RTMP stream to file", "streamer", ms.Streamer()) + log.Log(ctx, "recording ingest stream to file", "streamer", ms.Streamer()) pr, pw := io.Pipe() input = io.TeeReader(input, pw) go func() { - err := mm.dumpToFile(ctx, pr, ms.Streamer(), ".rtmp.mkv") + err := mm.dumpToFile(ctx, pr, ms.Streamer(), ".rtmp.mp4") if err != nil { log.Error(ctx, "error dumping to file", "error", err) } }() } else { - log.Log(ctx, "not recording RTMP stream to file", "streamer", ms.Streamer()) + log.Log(ctx, "not recording ingest stream to file", "streamer", ms.Streamer()) } ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -40,7 +41,7 @@ func (mm *MediaManager) MKVIngest(ctx context.Context, input io.Reader, ms Media if err != nil { return err } - pipeline, err := buildMKVIngestPipeline(ctx, input, signer) + pipeline, err := buildMP4IngestPipeline(ctx, input, signer) if err != nil { return err } @@ -64,41 +65,42 @@ func (mm *MediaManager) MKVIngest(ctx context.Context, input io.Reader, ms Media return <-busErr } -// buildMKVIngestPipeline builds the H264+AAC MKV demux graph (video → h264parse, -// audio → Opus re-encode) and links both branches into signerElem — the muxl -// signing bin that emits one bare canonical .m4s per GoP. Shared by the -// in-process MKVIngest and the isolated ingest worker, which differ only in +// buildMP4IngestPipeline builds the H264+AAC fragmented-MP4 demux graph (video +// → h264parse, audio → Opus re-encode) and links both branches into signerElem +// — the muxl signing bin that emits one bare canonical .m4s per GoP. Shared by +// the in-process MP4Ingest and the isolated ingest worker, which differ only in // where signerElem routes its segments (ValidateMP4 vs. a frame writer to the // main process). -func buildMKVIngestPipeline(ctx context.Context, input io.Reader, signerElem *gst.Element) (*gst.Pipeline, error) { - // Queue sizing: matroskademux feeds both branches from one thread, and the - // fMP4 muxer downstream is an aggregator — it consumes NOTHING until every - // pad has data. If the video track goes sparse (e.g. MistServer drops all - // delta frames when a push falls behind, leaving ~1s-apart keyframes), the - // audio branch must buffer a full video-frame gap while the demux walks the - // byte stream to the next video frame. gst's default queue caps at +// +// The source is fMP4, not MKV, very much on purpose: MP4 track fragments carry +// both decode (tfdt/trun) and presentation (ctts) timestamps, so qtdemux hands +// us the encoder's real DTS. Matroska carries only presentation timestamps, so +// the old MKV ingest had to *reconstruct* DTS with h264timestamper — which +// guesses a worst-case full-DPB reorder window for streams whose SPS doesn't +// declare one (notably VideoToolbox), minting a constant spurious PTS−DTS +// offset that pushed every GoP's presentation past its segment's declared +// window and broke WebRTC playback at every keyframe. Real DTS in the +// container means no reconstruction and no guessing. +func buildMP4IngestPipeline(ctx context.Context, input io.Reader, signerElem *gst.Element) (*gst.Pipeline, error) { + // Queue sizing: qtdemux feeds both branches from one thread, and the fMP4 + // muxer downstream is an aggregator — it consumes NOTHING until every pad + // has data. If the video track goes sparse (e.g. MistServer drops all delta + // frames when a push falls behind, leaving ~1s-apart keyframes), the audio + // branch must buffer a full video-frame gap while the demux walks the byte + // stream to the next video frame. gst's default queue caps at // max-size-time=1s, so a ≥1s video gap fills the audio queue, blocks the // demux, starves the muxer's video pad, and deadlocks the whole graph with // no EOS — a live stream wedges until the watchdog kills it. Use the shared // Queue2Big preset (no time/buffer cap, generous byte cap) like the other // demux-fed pipelines (transcode, rtmp_push, packetize, media_data_parser). - // - // h264timestamper: Matroska blocks carry only presentation timestamps, so - // for B-frame streams (PTS ≠ DTS) matroskademux emits reordered PTS with - // dts=none — and h264parse does not reconstruct DTS. The fMP4 muxer needs - // DTS to mux a reordered stream; without it it treats the jumbled PTS as - // monotonic timing and stretches the video track (~2.2× on a real capture), - // which downstream makes qtdemux EOS the audio pad early and every segment - // fails validation with "no audio in segment". h264timestamper rebuilds - // DTS from the H264 picture order count. pipelineSlice := []string{ - "appsrc name=streamsrc ! matroskademux name=demux", - "demux. ! " + constants.Queue2Big + " ! h264parse ! h264timestamper name=videoout", + "appsrc name=streamsrc ! qtdemux name=demux", + "demux. ! " + constants.Queue2Big + " ! h264parse name=videoout", "demux. ! " + constants.Queue2Big + " ! fdkaacdec ! audioresample ! opusenc name=audioenc", } pipeline, err := gst.NewPipelineFromString(strings.Join(pipelineSlice, "\n")) if err != nil { - return nil, fmt.Errorf("error creating MKVIngest pipeline: %w", err) + return nil, fmt.Errorf("error creating MP4Ingest pipeline: %w", err) } srcele, err := pipeline.GetElementByName("streamsrc") if err != nil { @@ -132,7 +134,7 @@ func (mm *MediaManager) dumpToFile(ctx context.Context, r io.Reader, user string filename := fmt.Sprintf("%s%s", now.FileSafeString(), filesuffix) // Streams to S3 when configured (production), else a local file under DataDir // (dev). Close finalizes either target — for S3 it commits the upload. - f, err := mm.cli.DebugRecordingCreate(ctx, []string{"debug-recordings", user, filename}, "video/x-matroska", false) + f, err := mm.cli.DebugRecordingCreate(ctx, []string{"debug-recordings", user, filename}, "video/mp4", false) if err != nil { return fmt.Errorf("failed to create debug recording: %w", err) } diff --git a/pkg/media/segmenter.go b/pkg/media/segmenter.go index 65fdabbc..9269efb2 100644 --- a/pkg/media/segmenter.go +++ b/pkg/media/segmenter.go @@ -236,7 +236,7 @@ func SegmentUnsigned(ctx context.Context, cli *config.CLI, streamer string, inpu } pipeline, err := gst.NewPipelineFromString(strings.Join(pipelineSlice, "\n")) if err != nil { - return fmt.Errorf("error creating MKVIngest pipeline: %w", err) + return fmt.Errorf("error creating SegmentUnsigned pipeline: %w", err) } srcele, err := pipeline.GetElementByName("appsrc") diff --git a/pkg/media/whip_worker.go b/pkg/media/whip_worker.go index de598bcb..be8ad040 100644 --- a/pkg/media/whip_worker.go +++ b/pkg/media/whip_worker.go @@ -14,7 +14,7 @@ import ( ) // ServeWHIPIngestWorkerSocket is the WHIP counterpart of -// ServeMKVIngestWorkerSocket. Unlike MKV there's no socket/fd to pass in: the +// ServeMP4IngestWorkerSocket. Unlike the fMP4 worker there is no socket/fd to pass in: the // worker OWNS the PeerConnection, so it creates it from cfg.OfferSDP (binding its // own UDP sockets), generates the SDP answer, and emits it as the FIRST frame on // the unix socket — main reads that Answer frame and returns it to the WHIP diff --git a/pkg/media/whip_worker_test.go b/pkg/media/whip_worker_test.go index 717ebe96..481e3496 100644 --- a/pkg/media/whip_worker_test.go +++ b/pkg/media/whip_worker_test.go @@ -47,7 +47,7 @@ func whipClientOffer(t *testing.T) (*webrtc.PeerConnection, *webrtc.TrackLocalSt // an offer it builds the PeerConnection, generates the SDP answer, and emits it // as the FIRST frame on its socket — the synchronous reply main returns to the // WHIP client. (Media flow → signed segments rides the same webRTCIngestPipeline -// the in-process path uses, plus the transcoder/frame machinery the MKV tests +// the in-process path uses, plus the transcoder/frame machinery the fMP4 ingest tests // already cover.) func TestWHIPWorkerAnswersOffer(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) @@ -149,7 +149,7 @@ func produceWHIPMedia(t *testing.T, ctx context.Context, video, audio *webrtc.Tr // TestWHIPWorkerLoopback is the full WHIP media path: a pion client offers, // connects to the worker (which owns the PeerConnection), and streams real // H264+Opus RTP; the worker must mux+sign+transcode it and serve a valid signed -// dual-codec segment over its socket — the WHIP parity of the MKV worker e2e +// dual-codec segment over its socket — the WHIP parity of the fMP4 ingest worker e2e // test. func TestWHIPWorkerLoopback(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) diff --git a/pkg/media/worker_watchdog.go b/pkg/media/worker_watchdog.go index ea5cd176..8d3ec3ff 100644 --- a/pkg/media/worker_watchdog.go +++ b/pkg/media/worker_watchdog.go @@ -15,7 +15,7 @@ import ( // fires onWedge — the worker's context cancel — tearing the pipeline down so the // process exits and the fault stays contained to this subprocess. // -// This is the worker-side counterpart to MKVIngestIsolated's main-side watchdog, +// This is the worker-side counterpart to MP4IngestIsolated's main-side watchdog, // and the ONLY wedge containment on the detached and WHIP paths: those workers // are detached (not tied to main's context), so main can't kill a stuck one — // the worker has to notice and exit itself. -- 2.51.2 From 857036b7cd7c7458773fd3e0cbff376844356287 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Wed, 22 Jul 2026 13:06:21 -0700 Subject: [PATCH 05/20] media: serialize the mist pull request with net/http, not by hand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tryMistGET still dials its own conn — that part is load-bearing (the raw fd gets passed to the detached worker, which http.Client can't provide) — but the request itself is now a real *http.Request written with req.Write instead of a concatenated header string, and http.ReadResponse gets the request for context. Same wire bytes, stdlib framing. The %2B-escaped wildcard path survives req.Write via URL.RawPath (covered by TestMistPullConnect's EscapedPath assertion). Co-Authored-By: Claude Fable 5 --- pkg/media/mist_pull.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pkg/media/mist_pull.go b/pkg/media/mist_pull.go index 528dcad4..66d473be 100644 --- a/pkg/media/mist_pull.go +++ b/pkg/media/mist_pull.go @@ -103,23 +103,28 @@ func tryMistGET(ctx context.Context, hostport, path string) (*net.TCPConn, []byt raw.Close() return nil, nil, false, fmt.Errorf("expected TCP connection, got %T", raw) } - // Connection: close — one stream per connection, body runs to EOF (or + // A real *http.Request serialized by the stdlib — we only own the conn by + // hand (it gets fd-passed to the worker), not the HTTP framing. req.Close + // sends Connection: close: one stream per connection, body runs to EOF (or // chunked-EOS) when the Mist stream ends. No keepalive reuse to reason about. - req := "GET " + path + " HTTP/1.1\r\n" + - "Host: " + hostport + "\r\n" + - "User-Agent: streamplace-ingest\r\n" + - "Accept: video/mp4\r\n" + - "Connection: close\r\n\r\n" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://"+hostport+path, nil) + if err != nil { + conn.Close() + return nil, nil, false, err + } + req.Close = true + req.Header.Set("User-Agent", "streamplace-ingest") + req.Header.Set("Accept", "video/mp4") if err := conn.SetDeadline(time.Now().Add(10 * time.Second)); err != nil { conn.Close() return nil, nil, false, err } - if _, err := io.WriteString(conn, req); err != nil { + if err := req.Write(conn); err != nil { conn.Close() return nil, nil, false, err } br := bufio.NewReader(conn) - resp, err := http.ReadResponse(br, nil) + resp, err := http.ReadResponse(br, req) if err != nil { conn.Close() return nil, nil, false, err -- 2.51.2 From aa11bba08fe27294cc0dda308ebdd9b33aaf94d0 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Wed, 22 Jul 2026 13:20:10 -0700 Subject: [PATCH 06/20] media: diagnose legacy MKV pushes; default mist-http-port to 28080 Field report from the first live test of the fMP4 ingest: a MistServer container still running the legacy MKVExec process config POSTed MKV to /live on a 4s restart loop, and each attempt died as an instant cryptic qtdemux failure (a pile of ~200-byte truncated debug recordings). Meanwhile the pull ingest dialed the old default port 18080 while Mist listened on 28080, so it never connected at all. - buildMP4IngestPipeline now peeks the stream and rejects the EBML magic with an error that names the actual problem (legacy MKVExec config) instead of letting qtdemux die confusingly. - mist-http-port default 18080 -> 28080, matching docker/mistserver.json (the generated dev config derives Mist's listener from the same flag, so both worlds stay consistent). Co-Authored-By: Claude Fable 5 --- pkg/config/config.go | 4 ++-- pkg/media/mist_mp4_ingest_test.go | 12 ++++++++++++ pkg/media/mp4_ingest.go | 32 +++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 1ba3931b..03a78af3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1032,8 +1032,8 @@ func (cli *CLI) NewCommand(name string) *urfavecli.Command { }) cmd.Flags = append(cmd.Flags, &urfavecli.IntFlag{ Name: "mist-http-port", - Usage: "MistServer HTTP port (internal use only)", - Value: 18080, + Usage: "MistServer HTTP port (internal use only) — ingest pulls Mist's live fMP4 output from this port, so it must match the running Mist config (docker/mistserver.json uses 28080, the default here)", + Value: 28080, Destination: &cli.MistHTTPPort, Sources: urfavecli.EnvVars("SP_MIST_HTTP_PORT"), }) diff --git a/pkg/media/mist_mp4_ingest_test.go b/pkg/media/mist_mp4_ingest_test.go index 13a60bcc..d933ca18 100644 --- a/pkg/media/mist_mp4_ingest_test.go +++ b/pkg/media/mist_mp4_ingest_test.go @@ -282,3 +282,15 @@ func TestMP4IngestBFramesValidate(t *testing.T) { require.True(t, sawReorderOffset, "B-frame stream keeps its real PTS−DTS reorder offsets through ingest") t.Logf("B-frame stream: %d segments, all validated", len(segs)) } + +// TestMP4IngestRejectsMatroskaWithDiagnosis: an MKV stream landing on the +// fMP4 ingest (a MistServer still running the legacy MKVExec process config) +// must fail immediately with a message that names the actual problem — not a +// generic qtdemux parse error on an endless Mist-side restart loop. +func TestMP4IngestRejectsMatroskaWithDiagnosis(t *testing.T) { + mkvish := append(append([]byte{}, matroskaMagic...), make([]byte, 1024)...) + _, err := runMP4ThroughIngestWorker(t, mkvish, false) + require.Error(t, err) + require.Contains(t, err.Error(), "Matroska") + require.Contains(t, err.Error(), "MKVExec") +} diff --git a/pkg/media/mp4_ingest.go b/pkg/media/mp4_ingest.go index 77314fda..776e804b 100644 --- a/pkg/media/mp4_ingest.go +++ b/pkg/media/mp4_ingest.go @@ -1,7 +1,10 @@ package media import ( + "bufio" + "bytes" "context" + "errors" "fmt" "io" "strings" @@ -81,7 +84,36 @@ func (mm *MediaManager) MP4Ingest(ctx context.Context, input io.Reader, ms Media // offset that pushed every GoP's presentation past its segment's declared // window and broke WebRTC playback at every keyframe. Real DTS in the // container means no reconstruction and no guessing. +// matroskaMagic is the EBML header every Matroska/WebM stream opens with. +var matroskaMagic = []byte{0x1A, 0x45, 0xDF, 0xA3} + +// rejectMatroska peeks at the ingest stream and fails fast with a diagnosis if +// it's Matroska. MKV was this pipeline's previous bridge format, so the most +// likely stray MKV source is a MistServer still running the legacy MKVExec +// process config (`streamplace live` POSTing MKV to /live on a restart loop) — +// without the sniff that just looks like qtdemux dying instantly, over and +// over, which is a miserable thing to debug. Returns a reader that includes +// the peeked bytes. +func rejectMatroska(input io.Reader) (io.Reader, error) { + br := bufio.NewReader(input) + head, err := br.Peek(len(matroskaMagic)) + if err != nil { + if errors.Is(err, io.EOF) { + return br, nil // shorter than the magic; let the pipeline EOS/complain + } + return nil, fmt.Errorf("peek ingest stream: %w", err) + } + if bytes.Equal(head, matroskaMagic) { + return nil, fmt.Errorf("ingest input is Matroska (MKV), but this node ingests fragmented MP4 — a MistServer running the legacy MKVExec process config is probably still pushing MKV to /live; update its config (see docker/mistserver.json)") + } + return br, nil +} + func buildMP4IngestPipeline(ctx context.Context, input io.Reader, signerElem *gst.Element) (*gst.Pipeline, error) { + input, err := rejectMatroska(input) + if err != nil { + return nil, err + } // Queue sizing: qtdemux feeds both branches from one thread, and the fMP4 // muxer downstream is an aggregator — it consumes NOTHING until every pad // has data. If the video track goes sparse (e.g. MistServer drops all delta -- 2.51.2 From 966180cb2c1544d014f0bce5dee97732ff3dc547 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Wed, 22 Jul 2026 14:28:55 -0700 Subject: [PATCH 07/20] feat: add Web Push notifications alongside Firebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Web Push (VAPID + Push API) as a second notification transport, so web users get the same livestream/beta-invite pushes mobile users get. Backend: - Add Type column to the notifications table ("firebase"/"web", defaults to "firebase" for backward compat). CreateNotification now takes a notifType; GetManyNotifications returns full rows with type info; DeleteNotification prunes a subscription on opt-out. - Introduce a unified Notifier interface taking []NotificationTarget (token + type) instead of []string. FirebaseNotifier filters to firebase targets; new WebPushNotifier (SherClockHolmes/webpush-go) handles web targets, fanning out in parallel and surfacing 410 Gone as ExpiredSubscriptionError so dead subscriptions can be pruned. - MultiNotifier wraps both transports and fans each target to the correct one — the single Notifier the rest of the codebase holds. - VAPID keys auto-generate on first run and persist in the Config table via EnsureVAPIDKeys, following the EnsureJWK pattern. No env vars required; keys survive restarts so subscriptions stay valid. - New API endpoints: DELETE /api/notification (prune on opt-out) and GET /api/notification/vapid-public-key (hand the public key to the browser). POST /api/notification now accepts a type field. - Both blast call sites (livestream-goes-live, beta invite) updated to load typed targets and blast through the unified notifier. Frontend: - Implement the web platformSlice (was a stub): initPushNotifications registers a service worker; enableWebNotifications requests permission, fetches the VAPID key, subscribes via PushManager, and registers the subscription; disableWebNotifications unsubscribes + prunes the server row. - Add public/sw.js service worker handling push events (show notification) and notificationclick (focus/navigate to the stream). - Add a Notifications settings screen with a toggle that works on both web (subscribe/unsubscribe) and mobile (reflects OS permission state). Wired into the settings navigator, linking config, and nav types. Translations added for all 7 locales. Co-Authored-By: Claude Opus 4.8 --- go.mod | 1 + go.sum | 10 +- .../notifications-category-settings.tsx | 110 ++++++++++++++ js/app/components/settings/settings.tsx | 7 + js/app/features/platform/shared.tsx | 1 + js/app/public/sw.js | 70 +++++++++ js/app/src/linking-config.ts | 1 + js/app/src/navigation-types.ts | 1 + js/app/src/shell.tsx | 6 + js/app/store/hooks.ts | 6 + js/app/store/slices/platformSlice.native.ts | 21 ++- js/app/store/slices/platformSlice.ts | 137 +++++++++++++++++- js/components/locales/en-US/settings.ftl | 6 + js/components/locales/es-ES/settings.ftl | 1 + js/components/locales/fr-FR/settings.ftl | 1 + js/components/locales/pt-BR/settings.ftl | 1 + js/components/locales/ro-RO/settings.ftl | 1 + js/components/locales/zh-Hans/settings.ftl | 1 + js/components/locales/zh-Hant/settings.ftl | 1 + pkg/api/api.go | 61 +++++++- pkg/api/api_internal.go | 12 +- pkg/api/api_test.go | 2 +- pkg/atproto/firehose.go | 2 +- pkg/atproto/sync.go | 12 +- pkg/cmd/streamplace.go | 18 ++- pkg/notifications/firebase.go | 26 +++- pkg/notifications/multi.go | 52 +++++++ pkg/notifications/notifier.go | 34 +++++ pkg/notifications/webpush.go | 135 +++++++++++++++++ pkg/notifications/webpush_test.go | 127 ++++++++++++++++ pkg/statedb/notification.go | 58 +++++++- pkg/statedb/notification_test.go | 60 +++++++- pkg/statedb/queue_processor.go | 15 +- pkg/statedb/statedb.go | 4 +- pkg/statedb/vapid.go | 59 ++++++++ 35 files changed, 1013 insertions(+), 47 deletions(-) create mode 100644 js/app/components/settings/notifications-category-settings.tsx create mode 100644 js/app/public/sw.js create mode 100644 pkg/notifications/multi.go create mode 100644 pkg/notifications/notifier.go create mode 100644 pkg/notifications/webpush.go create mode 100644 pkg/notifications/webpush_test.go create mode 100644 pkg/statedb/vapid.go diff --git a/go.mod b/go.mod index 16a3cf5a..f775d595 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( firebase.google.com/go/v4 v4.14.1 github.com/99designs/gqlgen v0.17.64 github.com/NYTimes/gziphandler v1.1.1 + github.com/SherClockHolmes/webpush-go v1.4.0 github.com/ThalesGroup/crypto11 v0.0.0-00010101000000-000000000000 github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d github.com/aws/aws-sdk-go-v2 v1.41.4 diff --git a/go.sum b/go.sum index 7f141140..5f890882 100644 --- a/go.sum +++ b/go.sum @@ -142,6 +142,8 @@ github.com/ProtonMail/go-crypto v1.0.0/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azu github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMzGrKSKYe4AqU6PDYYpjk= github.com/RussellLuo/slidingwindow v0.0.0-20200528002341-535bb99d338b h1:5/++qT1/z812ZqBvqQt6ToRswSuPZ/B33m6xVHRzADU= github.com/RussellLuo/slidingwindow v0.0.0-20200528002341-535bb99d338b/go.mod h1:4+EPqMRApwwE/6yo6CxiHoSnBzjRr3jsqer7frxP8y4= +github.com/SherClockHolmes/webpush-go v1.4.0 h1:ocnzNKWN23T9nvHi6IfyrQjkIc0oJWv1B1pULsf9i3s= +github.com/SherClockHolmes/webpush-go v1.4.0/go.mod h1:XSq8pKX11vNV8MJEMwjrlTkxhAj1zKfxmyhdV7Pd6UA= github.com/StackExchange/wmi v1.2.1 h1:VIkavFPXSjcnS+O8yTq7NI32k0R5Aj+v39y29VYDOSA= github.com/StackExchange/wmi v1.2.1/go.mod h1:rcmrprowKIVzvc+NUiLncP2uuArMWLCbu9SBzvHz7e8= github.com/VictoriaMetrics/fastcache v1.13.0 h1:AW4mheMR5Vd9FkAPUv+NH6Nhw+fmbTMGMsNAoA/+4G0= @@ -568,6 +570,7 @@ github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69 github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang-jwt/jwt/v4 v4.5.2 h1:YtQM7lnr8iZ+j5q71MGKkNw9Mn7AjHM68uc9g5fXeUI= github.com/golang-jwt/jwt/v4 v4.5.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9vvo= github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 h1:au07oEsX2xN0ktxqI+Sida1w446QrXBRJ0nee3SNZlA= @@ -1372,8 +1375,6 @@ github.com/streamplace/atmoq/go v0.0.4-0.20260701223355-13757de4ae08 h1:NiTRz8AX github.com/streamplace/atmoq/go v0.0.4-0.20260701223355-13757de4ae08/go.mod h1:3P8eSwKAGH7uh3SX5z1jlt/JgPTilJTUZngQJKhWY5s= github.com/streamplace/atproto-oauth-golang v0.0.0-20260413212710-98956064d06c h1:IzEPU2O4iL58Nb7aw+7lB9ttnesEwOVVE5oV9NEXemM= github.com/streamplace/atproto-oauth-golang v0.0.0-20260413212710-98956064d06c/go.mod h1:9LlKkqciiO5lRfbX0n4Wn5KNY9nvFb4R3by8FdW2TWc= -github.com/streamplace/glex v0.0.0-20260715231618-ee553e32d7c7 h1:MSBBIH+QMR9AVfC0RuBLbBm/o1RAl7+bacek7txswDo= -github.com/streamplace/glex v0.0.0-20260715231618-ee553e32d7c7/go.mod h1:LRaoeSMvSgOrhFX8s7ygjRlyka7wXdDa1s7JJ9o1IzY= github.com/streamplace/glex v0.0.0-20260716203108-f73ed7cc31c9 h1:HbIhx8i7wytiNg9mWg2EuHG59r8FXTVlDqlkZObjqbQ= github.com/streamplace/glex v0.0.0-20260716203108-f73ed7cc31c9/go.mod h1:LRaoeSMvSgOrhFX8s7ygjRlyka7wXdDa1s7JJ9o1IzY= github.com/streamplace/go-dpop v0.0.0-20250510031900-c897158a8ad4 h1:L1fS4HJSaAyNnkwfuZubgfeZy8rkWmA0cMtH5Z0HqNc= @@ -1604,6 +1605,7 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= +golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1742,6 +1744,7 @@ golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -1806,6 +1809,7 @@ golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= @@ -1822,6 +1826,7 @@ golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= +golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -1840,6 +1845,7 @@ golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/js/app/components/settings/notifications-category-settings.tsx b/js/app/components/settings/notifications-category-settings.tsx new file mode 100644 index 00000000..9ef4f303 --- /dev/null +++ b/js/app/components/settings/notifications-category-settings.tsx @@ -0,0 +1,110 @@ +import { + MenuContainer, + MenuGroup, + Text, + View, + zero, +} from "@streamplace/components"; +import { useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Platform, ScrollView } from "react-native"; +import { useStore } from "store"; +import { SettingToggle } from "./components/setting-toggle"; + +// NotificationsCategorySettings is the opt-in surface for push notifications. +// +// On web, the toggle calls enableWebNotifications (which requests permission, +// subscribes the browser's PushManager, and registers the subscription with the +// backend) or disableWebNotifications (unsubscribes + prunes the server row). +// +// On native, push is driven by initPushNotifications at app start (FCM), so +// the toggle reflects the OS permission state and, when supported, links the +// user to the system settings to change it. The toggle itself can't grant +// permission on native — the OS prompt already ran at startup — but it shows +// the current state honestly. +export function NotificationsCategorySettings() { + const { t } = useTranslation("settings"); + const enableWebNotifications = useStore( + (state) => state.enableWebNotifications, + ); + const disableWebNotifications = useStore( + (state) => state.disableWebNotifications, + ); + const webNotificationPermission = useStore( + (state) => state.webNotificationPermission, + ); + const notificationToken = useStore((state) => state.notificationToken); + + const isWeb = Platform.OS === "web"; + const [busy, setBusy] = useState(false); + + // The "enabled" state: on web it's permission === granted AND we have a + // registered subscription token. On native it's whether the OS granted + // permission (the FCM token is acquired separately at startup). + const permission = isWeb ? webNotificationPermission() : "granted"; + const enabled = isWeb + ? permission === "granted" && !!notificationToken + : permission === "granted"; + + // Re-check permission when the screen gains focus (the user may have + // changed it in browser settings). + const [refreshKey, setRefreshKey] = useState(0); + useEffect(() => { + if (!isWeb) return; + const interval = setInterval(() => setRefreshKey((k) => k + 1), 1000); + return () => clearInterval(interval); + }, [isWeb]); + // touch refreshKey so the linter doesn't complain and the re-render happens + void refreshKey; + + const handleToggle = async (value: boolean) => { + if (busy) return; + setBusy(true); + try { + if (value) { + if (isWeb) { + await enableWebNotifications(); + } + // native: permission was already requested at startup; nothing to do + } else { + if (isWeb) { + await disableWebNotifications(); + } + } + } finally { + setBusy(false); + } + }; + + return ( + + + + + + + + {isWeb && permission === "denied" && ( + + + {t("notifications-blocked-help")} + + + )} + + + + + ); +} diff --git a/js/app/components/settings/settings.tsx b/js/app/components/settings/settings.tsx index e778534c..9fd245f4 100644 --- a/js/app/components/settings/settings.tsx +++ b/js/app/components/settings/settings.tsx @@ -18,6 +18,7 @@ import { import { ImageBackground } from "expo-image"; import { Award, + Bell, Brush, Globe, Info, @@ -141,6 +142,12 @@ export function Settings() { screen="PrivacyCategory" icon={Shield} /> + + )} {danmuUnlocked && ( diff --git a/js/app/features/platform/shared.tsx b/js/app/features/platform/shared.tsx index a3b086fd..c4aa024c 100644 --- a/js/app/features/platform/shared.tsx +++ b/js/app/features/platform/shared.tsx @@ -13,4 +13,5 @@ export const initialState: PlatformState = { export type RegisterNotificationTokenBody = { token: string; repoDID?: string; + type?: "firebase" | "web"; }; diff --git a/js/app/public/sw.js b/js/app/public/sw.js new file mode 100644 index 00000000..b4c5b98c --- /dev/null +++ b/js/app/public/sw.js @@ -0,0 +1,70 @@ +// Streamplace service worker for Web Push. +// +// This file is served from /sw.js (see public/) and registered by the web +// platform slice on app mount. It does two things: +// +// 1. push — when a push message arrives, show it as a system notification. +// The payload is the JSON-serialized NotificationBlast from the server +// ({title, body, data}). If the payload is empty (a "silent" push), we +// still show a notification with a generic title, since browsers require +// a visible notification for every push. +// +// 2. notificationclick — focus an existing tab (or open a new one) and +// navigate it to the path encoded in the notification's data, so tapping +// a "🔴 @user is LIVE!" notification opens that stream. + +self.addEventListener("push", (event) => { + let data = { title: "Streamplace", body: "", data: {} }; + try { + if (event.data) { + const parsed = event.data.json(); + data = { + title: parsed.title || data.title, + body: parsed.body || data.body, + data: parsed.data || data.data, + }; + } + } catch (e) { + // Payload wasn't JSON; fall back to raw text if present. + if (event.data) { + data.body = event.data.text(); + } + } + event.waitUntil( + self.registration.showNotification(data.title, { + body: data.body, + data: data.data, + icon: "/favicon.ico", + badge: "/favicon.ico", + }), + ); +}); + +self.addEventListener("notificationclick", (event) => { + event.notification.close(); + const path = event.notification.data && event.notification.data.path; + const targetUrl = path ? path : "/"; + + event.waitUntil( + (async () => { + const all = await self.clients.matchAll({ + type: "window", + includeUncontrolled: true, + }); + // Focus an existing tab if one is open. + for (const client of all) { + if ("focus" in client) { + client.focus(); + if ("navigate" in client) { + await client.navigate(targetUrl); + } + return; + } + } + // Otherwise open a new one. + if (self.clients.openWindow) { + await self.clients.openWindow(targetUrl); + } + })(), + ); +}); diff --git a/js/app/src/linking-config.ts b/js/app/src/linking-config.ts index cdfac859..3cead5a3 100644 --- a/js/app/src/linking-config.ts +++ b/js/app/src/linking-config.ts @@ -121,6 +121,7 @@ export const SCREEN_PATHS = { WebhooksSettings: "settings/streaming/webhooks", RecommendationsSettings: "settings/streaming/recommendations", PrivacyCategory: "settings/privacy", + NotificationsCategory: "settings/notifications", DanmuCategory: "settings/danmu", AdvancedCategory: "settings/advanced", DeveloperSettings: "settings/developer", diff --git a/js/app/src/navigation-types.ts b/js/app/src/navigation-types.ts index b3712f43..431aefc1 100644 --- a/js/app/src/navigation-types.ts +++ b/js/app/src/navigation-types.ts @@ -9,6 +9,7 @@ export type SettingsStackParamList = { WebhooksSettings: undefined; BackupSettings: undefined; PrivacyCategory: undefined; + NotificationsCategory: undefined; DanmuCategory: undefined; AdvancedCategory: undefined; LanguagesCategory: undefined; diff --git a/js/app/src/shell.tsx b/js/app/src/shell.tsx index ed990807..927defc0 100644 --- a/js/app/src/shell.tsx +++ b/js/app/src/shell.tsx @@ -33,6 +33,7 @@ import { DanmuCategorySettings } from "components/settings/danmu-category-settin import KeyManager from "components/settings/key-manager"; import { LanguagesCategorySettings } from "components/settings/languages-category-settings"; import MultistreamManager from "components/settings/multistream-manager"; +import { NotificationsCategorySettings } from "components/settings/notifications-category-settings"; import { PrivacyCategorySettings } from "components/settings/privacy-category-settings"; import RecommendationsManager from "components/settings/recommendations-manager"; import { StreamingCategorySettings } from "components/settings/streaming-category-settings"; @@ -369,6 +370,11 @@ function SettingsNavigator() { component={PrivacyCategorySettings} options={{ title: "Privacy & Security" }} /> + useStore((state) => state.notificationToken); export const useNotificationDestination = () => useStore((state) => state.notificationDestination); +export const useEnableWebNotifications = () => + useStore((state) => state.enableWebNotifications); +export const useDisableWebNotifications = () => + useStore((state) => state.disableWebNotifications); +export const useWebNotificationPermission = () => + useStore((state) => state.webNotificationPermission); diff --git a/js/app/store/slices/platformSlice.native.ts b/js/app/store/slices/platformSlice.native.ts index abb1ebef..860b5f47 100644 --- a/js/app/store/slices/platformSlice.native.ts +++ b/js/app/store/slices/platformSlice.native.ts @@ -14,6 +14,12 @@ export interface PlatformSlice { openLoginLink: (url: string) => Promise; initPushNotifications: () => Promise; registerNotificationToken: () => Promise; + // web-only actions; no-ops on native. Present so the shared PlatformSlice + // type is identical across platforms and the settings toggle can call + // them unconditionally. + enableWebNotifications: () => Promise; + disableWebNotifications: () => Promise; + webNotificationPermission: () => NotificationPermission; } const checkApplicationPermission = async () => { @@ -170,7 +176,10 @@ export const createPlatformSlice: StateCreator< return; } - const body: { token: string; repoDID?: string } = { token }; + const body: { token: string; type: string; repoDID?: string } = { + token, + type: "firebase", + }; const did = oauthSession?.did; if (did) { @@ -196,4 +205,14 @@ export const createPlatformSlice: StateCreator< console.error("registerNotificationToken error", e); } }, + enableWebNotifications: async () => { + // web-only; native uses FCM via initPushNotifications + return "denied"; + }, + disableWebNotifications: async () => { + // web-only + }, + webNotificationPermission: () => { + return "denied"; + }, }); diff --git a/js/app/store/slices/platformSlice.ts b/js/app/store/slices/platformSlice.ts index cbcf76f6..75e4b284 100644 --- a/js/app/store/slices/platformSlice.ts +++ b/js/app/store/slices/platformSlice.ts @@ -1,3 +1,4 @@ +import { AppStore } from "store"; import { StateCreator } from "zustand"; export interface PlatformSlice { @@ -10,14 +11,39 @@ export interface PlatformSlice { openLoginLink: (url: string) => Promise; initPushNotifications: () => Promise; registerNotificationToken: () => Promise; + // web-only: subscribe/unsubscribe the browser's PushManager. Returns the + // permission state so the settings toggle can reflect reality. + enableWebNotifications: () => Promise; + disableWebNotifications: () => Promise; + webNotificationPermission: () => NotificationPermission; } -export const createPlatformSlice: StateCreator = (set, get) => ({ +// VAPID public key must be converted from base64url to a Uint8Array for the +// PushManager.subscribe() applicationServerKey argument. +function urlBase64ToUint8Array(base64String: string): Uint8Array { + const padding = "=".repeat((4 - (base64String.length % 4)) % 4); + const base64 = (base64String + padding).replace(/-/g, "+").replace(/_/g, "/"); + const rawData = atob(base64); + const output = new Uint8Array(rawData.length); + for (let i = 0; i < rawData.length; ++i) { + output[i] = rawData.charCodeAt(i); + } + return output; +} + +export const createPlatformSlice: StateCreator< + AppStore, + [], + [], + PlatformSlice +> = (set, get) => ({ status: "idle", notificationToken: null, notificationDestination: null, handleNotification: (payload) => { - // notification handling logic + if (!payload) return; + if (typeof payload.path !== "string") return; + set({ notificationDestination: payload.path }); }, clearNotification: () => { set({ notificationDestination: null }); @@ -32,9 +58,112 @@ export const createPlatformSlice: StateCreator = (set, get) => ({ } }, initPushNotifications: async () => { - // mobile-only, web notifications someday + // Register the service worker that receives push events. This must + // happen early (on app mount) so that pushes delivered while the tab is + // backgrounded still surface as system notifications. The actual + // subscription + permission request is deferred to the settings toggle + // (enableWebNotifications) because browsers require a user gesture for + // the permission prompt. + if (!("serviceWorker" in navigator)) { + return; + } + try { + await navigator.serviceWorker.register("/sw.js"); + } catch (e) { + console.log("service worker registration failed", e); + } }, registerNotificationToken: async () => { - // notification token registration + // On web, token registration is driven by enableWebNotifications (which + // subscribes and posts the subscription). This no-op keeps the shared + // shell effect happy without double-registering. + }, + enableWebNotifications: async () => { + const url = get().url; + if (!url) { + console.log( + "no streamplace url configured, cannot enable web notifications", + ); + return "denied"; + } + try { + const permission = await Notification.requestPermission(); + if (permission !== "granted") { + return permission; + } + + // Make sure the service worker is active before subscribing. + const reg = await navigator.serviceWorker.ready; + + // Fetch the server's VAPID public key. + const vapidRes = await fetch(`${url}/api/notification/vapid-public-key`); + if (!vapidRes.ok) { + throw new Error(`failed to fetch vapid public key: ${vapidRes.status}`); + } + const { publicKey } = await vapidRes.json(); + + // Subscribe the browser's PushManager. + const subscription = await reg.pushManager.subscribe({ + userVisibleOnly: true, + applicationServerKey: urlBase64ToUint8Array(publicKey) as BufferSource, + }); + const subJSON = JSON.stringify(subscription); + set({ notificationToken: subJSON }); + + // Register the subscription with the backend. + const { oauthSession } = get(); + const body: { token: string; type: string; repoDID?: string } = { + token: subJSON, + type: "web", + }; + if (oauthSession?.did) { + body.repoDID = oauthSession.did; + } + const res = await fetch(`${url}/api/notification`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + console.log("web notification registration status:", res.status); + return permission; + } catch (e) { + console.error("enableWebNotifications error", e); + return "denied"; + } + }, + disableWebNotifications: async () => { + const url = get().url; + const { notificationToken } = get(); + try { + if (notificationToken) { + // Unsubscribe the browser side so it stops accepting pushes. + const sub = JSON.parse(notificationToken); + // We need the live PushSubscription object to call unsubscribe(); get + // it from the service worker registration by matching endpoint. + const reg = await navigator.serviceWorker.ready; + const existing = await reg.pushManager.getSubscription(); + if (existing && existing.endpoint === sub.endpoint) { + await existing.unsubscribe(); + } + // Tell the server to drop the row. + if (url) { + await fetch(`${url}/api/notification`, { + method: "DELETE", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ token: notificationToken }), + }); + } + } + } catch (e) { + console.error("disableWebNotifications error", e); + } finally { + set({ notificationToken: null }); + } + }, + webNotificationPermission: () => { + if (typeof Notification === "undefined") { + return "denied"; + } + return Notification.permission; }, }); diff --git a/js/components/locales/en-US/settings.ftl b/js/components/locales/en-US/settings.ftl index 3711dcdc..9f522a9c 100644 --- a/js/components/locales/en-US/settings.ftl +++ b/js/components/locales/en-US/settings.ftl @@ -55,6 +55,12 @@ developer = Developer languages = Languages privacy-security = Privacy & Security streaming = Streaming +notifications = Notifications +notifications-title = Push Notifications +notifications-web-description = Get notified when streamers you follow go live. +notifications-mobile-description = Push notifications are managed by your device settings. +notifications-blocked-description = Blocked — you denied notification permission in your browser. +notifications-blocked-help = To re-enable, update site permissions in your browser settings, then toggle this on again. ## Common Actions cancel = Cancel diff --git a/js/components/locales/es-ES/settings.ftl b/js/components/locales/es-ES/settings.ftl index 21db93e9..027991e0 100644 --- a/js/components/locales/es-ES/settings.ftl +++ b/js/components/locales/es-ES/settings.ftl @@ -91,6 +91,7 @@ developer = Desarrollador languages = Idiomas privacy-security = Privacidad y Seguridad streaming = Transmisión +notifications = Notificaciones ## Acciones Comunes cancel = Cancelar diff --git a/js/components/locales/fr-FR/settings.ftl b/js/components/locales/fr-FR/settings.ftl index 2f338953..b38df888 100644 --- a/js/components/locales/fr-FR/settings.ftl +++ b/js/components/locales/fr-FR/settings.ftl @@ -89,6 +89,7 @@ developer = Développeur languages = Langues privacy-security = Confidentialité et Sécurité streaming = Diffusion +notifications = Notifications ## Actions Courantes cancel = Annuler diff --git a/js/components/locales/pt-BR/settings.ftl b/js/components/locales/pt-BR/settings.ftl index 08712eeb..253bf6b8 100644 --- a/js/components/locales/pt-BR/settings.ftl +++ b/js/components/locales/pt-BR/settings.ftl @@ -89,6 +89,7 @@ developer = Desenvolvedor languages = Idiomas privacy-security = Privacidade e Segurança streaming = Transmissão +notifications = Notificações ## Ações Comuns cancel = Cancelar diff --git a/js/components/locales/ro-RO/settings.ftl b/js/components/locales/ro-RO/settings.ftl index 6fa630eb..a58dcb53 100644 --- a/js/components/locales/ro-RO/settings.ftl +++ b/js/components/locales/ro-RO/settings.ftl @@ -51,6 +51,7 @@ developer = Dezvoltator languages = Limbi privacy-security = Confidențialitate și securitate streaming = Streaming +notifications = Notificări ## Common Actions cancel = Anulare diff --git a/js/components/locales/zh-Hans/settings.ftl b/js/components/locales/zh-Hans/settings.ftl index 9235e910..a6041461 100644 --- a/js/components/locales/zh-Hans/settings.ftl +++ b/js/components/locales/zh-Hans/settings.ftl @@ -53,6 +53,7 @@ developer = 开发者 languages = 语言 privacy-security = 隐私与安全 streaming = 串流 +notifications = 通知 ## Common Actions cancel = 取消 diff --git a/js/components/locales/zh-Hant/settings.ftl b/js/components/locales/zh-Hant/settings.ftl index f0522c9c..47446a79 100644 --- a/js/components/locales/zh-Hant/settings.ftl +++ b/js/components/locales/zh-Hant/settings.ftl @@ -90,6 +90,7 @@ developer = 開發者 languages = 語言 privacy-security = 隱私與安全 streaming = 串流 +notifications = 通知 ## 常用動作 cancel = 取消 diff --git a/pkg/api/api.go b/pkg/api/api.go index 9309331d..331ab77c 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -64,7 +64,7 @@ type StreamplaceAPI struct { Updater *Updater Signer *eip712.EIP712Signer Mimes map[string]string - FirebaseNotifier notifications.FirebaseNotifier + Notifier notifications.Notifier MediaManager *media.MediaManager MediaSigner media.MediaSigner UploadManager *upload.Manager @@ -99,7 +99,7 @@ type WebsocketTracker struct { mu sync.RWMutex } -func MakeStreamplaceAPI(cli *config.CLI, mod model.Model, statefulDB *statedb.StatefulDB, noter notifications.FirebaseNotifier, mm *media.MediaManager, ms media.MediaSigner, bus *bus.Bus, atsync *atproto.ATProtoSynchronizer, d *director.Director, op *oatproxy.OATProxy, ldb localdb.LocalDB, um *upload.Manager, playbackStore blob.Store, viewLog *viewlog.Writer) (*StreamplaceAPI, error) { +func MakeStreamplaceAPI(cli *config.CLI, mod model.Model, statefulDB *statedb.StatefulDB, noter notifications.Notifier, mm *media.MediaManager, ms media.MediaSigner, bus *bus.Bus, atsync *atproto.ATProtoSynchronizer, d *director.Director, op *oatproxy.OATProxy, ldb localdb.LocalDB, um *upload.Manager, playbackStore blob.Store, viewLog *viewlog.Writer) (*StreamplaceAPI, error) { updater, err := PrepareUpdater(cli) if err != nil { return nil, err @@ -108,7 +108,7 @@ func MakeStreamplaceAPI(cli *config.CLI, mod model.Model, statefulDB *statedb.St Model: mod, StatefulDB: statefulDB, Updater: updater, - FirebaseNotifier: noter, + Notifier: noter, MediaManager: mm, MediaSigner: ms, UploadManager: um, @@ -188,6 +188,8 @@ func (a *StreamplaceAPI) Handler(ctx context.Context) (http.Handler, error) { router.Handler("GET", "/.well-known/assetlinks.json", a.HandleAndroidAssetLinks(ctx)) apiRouter := httprouter.New() addFunc(apiRouter, "POST", "/api/notification", a.HandleNotification(ctx)) + addFunc(apiRouter, "DELETE", "/api/notification", a.HandleNotificationDelete(ctx)) + addFunc(apiRouter, "GET", "/api/notification/vapid-public-key", a.HandleVapidPublicKey(ctx)) // old clients addFunc(router, "GET", "/app-updates", a.HandleAppUpdates(ctx)) // new ones @@ -556,6 +558,7 @@ func (a *StreamplaceAPI) RedirectHandler(ctx context.Context) (http.Handler, err type NotificationPayload struct { Token string `json:"token"` RepoDID string `json:"repoDID"` + Type string `json:"type"` } func (a *StreamplaceAPI) HandleAPI404(ctx context.Context) http.HandlerFunc { @@ -579,7 +582,7 @@ func (a *StreamplaceAPI) HandleNotification(ctx context.Context) http.HandlerFun w.WriteHeader(400) return } - err = a.StatefulDB.CreateNotification(n.Token, n.RepoDID) + err = a.StatefulDB.CreateNotification(n.Token, n.RepoDID, statedb.NotificationType(n.Type)) if err != nil { log.Log(ctx, "error creating notification", "error", err) w.WriteHeader(400) @@ -598,6 +601,56 @@ func (a *StreamplaceAPI) HandleNotification(ctx context.Context) http.HandlerFun } } +// HandleNotificationDelete removes a push token (web or mobile). Used by the +// web client when a user disables notifications — the browser subscription is +// unsubscribed locally and the server row is pruned so we stop pushing to a +// dead endpoint. +func (a *StreamplaceAPI) HandleNotificationDelete(ctx context.Context) http.HandlerFunc { + return func(w http.ResponseWriter, req *http.Request) { + payload, err := io.ReadAll(req.Body) + if err != nil { + log.Log(ctx, "error reading notification delete", "error", err) + w.WriteHeader(400) + return + } + n := NotificationPayload{} + if err := json.Unmarshal(payload, &n); err != nil { + log.Log(ctx, "error unmarshalling notification delete", "error", err) + w.WriteHeader(400) + return + } + if n.Token == "" { + w.WriteHeader(400) + return + } + if err := a.StatefulDB.DeleteNotification(n.Token); err != nil { + log.Log(ctx, "error deleting notification", "error", err) + w.WriteHeader(400) + return + } + log.Log(ctx, "successfully deleted notification", "token", n.Token) + w.WriteHeader(200) + } +} + +// HandleVapidPublicKey returns the server's Web Push VAPID public key. The web +// client needs it to subscribe the browser's PushManager. The key is generated +// on first access (via EnsureVAPIDKeys) and stays stable thereafter. +func (a *StreamplaceAPI) HandleVapidPublicKey(ctx context.Context) http.HandlerFunc { + return func(w http.ResponseWriter, req *http.Request) { + keys, err := a.StatefulDB.EnsureVAPIDKeys(ctx) + if err != nil { + log.Error(ctx, "error ensuring vapid keys", "error", err) + apierrors.WriteHTTPInternalServerError(w, "unable to get vapid public key", err) + return + } + w.Header().Set("Content-Type", "application/json") + if _, err := w.Write([]byte(`{"publicKey":"` + keys.PublicKey + `"}`)); err != nil { + log.Error(ctx, "error writing vapid public key", "error", err) + } + } +} + func (a *StreamplaceAPI) HandleSegment(ctx context.Context) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { err := a.MediaManager.ValidateMP4(ctx, req.Body, false) diff --git a/pkg/api/api_internal.go b/pkg/api/api_internal.go index 1b92b862..7f0cd158 100644 --- a/pkg/api/api_internal.go +++ b/pkg/api/api_internal.go @@ -430,15 +430,15 @@ func (a *StreamplaceAPI) InternalHandler(ctx context.Context) (http.Handler, err errors.WriteHTTPInternalServerError(w, "unable to get notifications", err) return } - if a.FirebaseNotifier == nil { - errors.WriteHTTPInternalServerError(w, "firebase notifier not initialized", nil) + if a.Notifier == nil { + errors.WriteHTTPInternalServerError(w, "notifier not initialized", nil) return } - tokens := []string{} - for _, not := range notifications { - tokens = append(tokens, not.Token) + targets := make([]notificationpkg.NotificationTarget, len(notifications)) + for i, not := range notifications { + targets[i] = notificationpkg.NotificationTarget{Token: not.Token, Type: not.Type} } - err = a.FirebaseNotifier.Blast(ctx, tokens, &payload) + err = a.Notifier.Blast(ctx, targets, &payload) if err != nil { errors.WriteHTTPInternalServerError(w, "unable to blast notifications", err) return diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 48bc12f9..c68276a5 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -81,7 +81,7 @@ func TestRedirectHandler(t *testing.T) { type MockFirebase struct { } -func (m *MockFirebase) Blast(ctx context.Context, nots []string, nb *notifications.NotificationBlast) error { +func (m *MockFirebase) Blast(ctx context.Context, targets []notifications.NotificationTarget, nb *notifications.NotificationBlast) error { return nil } diff --git a/pkg/atproto/firehose.go b/pkg/atproto/firehose.go index a54c3cf5..aed0aa0a 100644 --- a/pkg/atproto/firehose.go +++ b/pkg/atproto/firehose.go @@ -47,7 +47,7 @@ type ATProtoSynchronizer struct { CLI *config.CLI Model model.Model StatefulDB *statedb.StatefulDB - Noter notificationpkg.FirebaseNotifier + Noter notificationpkg.Notifier Bus *bus.Bus PLCDirectory identity.Directory CachedPLCDirectory identity.Directory diff --git a/pkg/atproto/sync.go b/pkg/atproto/sync.go index 84996d55..401a6b3f 100644 --- a/pkg/atproto/sync.go +++ b/pkg/atproto/sync.go @@ -965,21 +965,25 @@ func (atsync *ATProtoSynchronizer) notifyBetaInvite(ctx context.Context, rec *pl if atsync.Noter == nil || atsync.StatefulDB == nil { return } - tokens, err := atsync.StatefulDB.GetManyNotificationTokens([]string{rec.Did}) + notifications, err := atsync.StatefulDB.GetManyNotifications([]string{rec.Did}) if err != nil { log.Error(ctx, "beta invite notification: failed to load tokens", "did", rec.Did, "err", err) return } - if len(tokens) == 0 { + if len(notifications) == 0 { log.Debug(ctx, "beta invite notification: no device tokens for invitee", "did", rec.Did, "feature", rec.Feature) return } blast := betaInviteBlast(rec.Feature) - if err := atsync.Noter.Blast(ctx, tokens, blast); err != nil { + targets := make([]notificationpkg.NotificationTarget, len(notifications)) + for i, n := range notifications { + targets[i] = notificationpkg.NotificationTarget{Token: n.Token, Type: n.Type} + } + if err := atsync.Noter.Blast(ctx, targets, blast); err != nil { log.Error(ctx, "beta invite notification: blast failed", "did", rec.Did, "feature", rec.Feature, "err", err) return } - log.Log(ctx, "sent beta invite notification", "did", rec.Did, "feature", rec.Feature, "tokens", len(tokens)) + log.Log(ctx, "sent beta invite notification", "did", rec.Did, "feature", rec.Feature, "tokens", len(notifications)) } // betaInviteBlast builds the push payload for a newly-granted beta feature. diff --git a/pkg/cmd/streamplace.go b/pkg/cmd/streamplace.go index fcdfdde9..f268c43e 100644 --- a/pkg/cmd/streamplace.go +++ b/pkg/cmd/streamplace.go @@ -198,9 +198,9 @@ func runMain(ctx context.Context, build *config.BuildFlags, platformJobs []jobFu if err != nil { return err } - var noter notifications.FirebaseNotifier + var fbNotifier notifications.FirebaseNotifier if cli.FirebaseServiceAccount != "" { - noter, err = notifications.MakeFirebaseNotifier(ctx, cli.FirebaseServiceAccount) + fbNotifier, err = notifications.MakeFirebaseNotifier(ctx, cli.FirebaseServiceAccount) if err != nil { return err } @@ -213,10 +213,22 @@ func runMain(ctx context.Context, build *config.BuildFlags, platformJobs []jobFu if err != nil { return err } - state, err := statedb.MakeDB(ctx, cli, noter, mod) + // The notifier is assembled after the DB exists, because the Web Push + // notifier needs VAPID keys that are persisted in the Config table. The + // queue processor nil-checks the notifier, so the brief window is safe. + state, err := statedb.MakeDB(ctx, cli, nil, mod) if err != nil { return err } + + // Build the Web Push notifier from VAPID keys generated/stored in the DB. + vapidKeys, err := state.EnsureVAPIDKeys(ctx) + if err != nil { + return err + } + webNotifier := notifications.NewWebPushNotifier(vapidKeys, "") + noter := notifications.NewMultiNotifier(fbNotifier, webNotifier) + state.SetNotifier(noter) handle, err := atproto.MakeLexiconRepo(ctx, cli, mod, state) if err != nil { return err diff --git a/pkg/notifications/firebase.go b/pkg/notifications/firebase.go index 2dd16740..3bac692b 100644 --- a/pkg/notifications/firebase.go +++ b/pkg/notifications/firebase.go @@ -1,20 +1,23 @@ package notifications import ( + "context" "encoding/base64" "encoding/json" "fmt" - "context" - firebase "firebase.google.com/go/v4" "firebase.google.com/go/v4/messaging" "google.golang.org/api/option" "stream.place/streamplace/pkg/log" ) +// FirebaseNotifier sends pushes via Firebase Cloud Messaging (FCM/APNs). It +// implements Notifier by handling targets of NotificationTypeFirebase and +// ignoring all others. type FirebaseNotifier interface { - Blast(ctx context.Context, tokens []string, golive *NotificationBlast) error + Notifier + BlastTokens(ctx context.Context, tokens []string, blast *NotificationBlast) error } type FirebaseNotifierS struct { @@ -55,8 +58,23 @@ func MakeFirebaseNotifier(ctx context.Context, serviceAccountJSONb64 string) (Fi return &FirebaseNotifierS{app: app}, nil } +// Blast implements Notifier. It filters targets down to firebase tokens and +// delegates to BlastTokens; web targets are left for another notifier. +func (f *FirebaseNotifierS) Blast(ctx context.Context, targets []NotificationTarget, blast *NotificationBlast) error { + tokens := make([]string, 0, len(targets)) + for _, t := range targets { + if t.Type == NotificationTypeFirebase || t.Type == "" { + tokens = append(tokens, t.Token) + } + } + if len(tokens) == 0 { + return nil + } + return f.BlastTokens(ctx, tokens, blast) +} + // refactor me when we have >500 users -func (f *FirebaseNotifierS) Blast(ctx context.Context, tokens []string, blast *NotificationBlast) error { +func (f *FirebaseNotifierS) BlastTokens(ctx context.Context, tokens []string, blast *NotificationBlast) error { client, err := f.app.Messaging(ctx) if err != nil { return err diff --git a/pkg/notifications/multi.go b/pkg/notifications/multi.go new file mode 100644 index 00000000..ae008dee --- /dev/null +++ b/pkg/notifications/multi.go @@ -0,0 +1,52 @@ +package notifications + +import ( + "context" + "fmt" +) + +// MultiNotifier fans a single blast out to every transport it wraps. It +// implements Notifier by delegating each target to the child notifier that +// handles that target's Type. This is the single Notifier the rest of the +// codebase holds (replacing the old FirebaseNotifier field), so callers don't +// need to know which transports are configured. +// +// A child that returns an error for its slice of targets does not abort the +// others — each transport is independent, and a dead FCM credential +// shouldn't block web pushes (or vice versa). Errors are collected and +// returned as a joined error after all transports have been attempted. +type MultiNotifier struct { + notifiers []Notifier +} + +// NewMultiNotifier wraps one or more transport notifiers. nil entries are +// skipped so callers can pass a possibly-unconfigured notifier through +// without filtering. +func NewMultiNotifier(notifiers ...Notifier) *MultiNotifier { + nn := &MultiNotifier{} + for _, n := range notifiers { + if n != nil { + nn.notifiers = append(nn.notifiers, n) + } + } + return nn +} + +func (m *MultiNotifier) Blast(ctx context.Context, targets []NotificationTarget, blast *NotificationBlast) error { + if len(m.notifiers) == 0 { + return nil + } + var errs []error + for _, n := range m.notifiers { + if err := n.Blast(ctx, targets, blast); err != nil { + errs = append(errs, err) + } + } + if len(errs) == 1 { + return errs[0] + } + if len(errs) > 1 { + return fmt.Errorf("multi-notifier: %d transports failed: %v", len(errs), errs) + } + return nil +} diff --git a/pkg/notifications/notifier.go b/pkg/notifications/notifier.go new file mode 100644 index 00000000..3d1c019f --- /dev/null +++ b/pkg/notifications/notifier.go @@ -0,0 +1,34 @@ +package notifications + +import "context" + +// NotificationType identifies the push transport a token belongs to. It lives +// in pkg/notifications (the lower-level package) so that pkg/statedb — which +// already imports pkg/notifications for the Notifier field — can reference it +// on the Notification row without creating a circular import. +type NotificationType string + +const ( + // NotificationTypeFirebase is an FCM/APNs registration token (mobile). + NotificationTypeFirebase NotificationType = "firebase" + // NotificationTypeWeb is a Web Push subscription (endpoint + p256dh/auth + // keys), stored as the JSON-serialized PushSubscription object. + NotificationTypeWeb NotificationType = "web" +) + +// NotificationTarget pairs a push token with the transport that knows how to +// deliver to it. Blast callers produce a []NotificationTarget (typically by +// loading notification rows from the DB) and hand them to a Notifier, which +// fans each target out to the matching transport. +type NotificationTarget struct { + Token string + Type NotificationType +} + +// Notifier sends a notification blast to a set of targets. Each +// implementation handles one transport (firebase, web) or fans out across +// several (MultiNotifier). Implementations should silently skip targets whose +// Type they don't handle. +type Notifier interface { + Blast(ctx context.Context, targets []NotificationTarget, blast *NotificationBlast) error +} diff --git a/pkg/notifications/webpush.go b/pkg/notifications/webpush.go new file mode 100644 index 00000000..ba5adbaa --- /dev/null +++ b/pkg/notifications/webpush.go @@ -0,0 +1,135 @@ +package notifications + +import ( + "context" + "encoding/json" + "fmt" + "sync" + + webpush "github.com/SherClockHolmes/webpush-go" + "stream.place/streamplace/pkg/log" +) + +// VAPIDKeys is the ECDSA P-256 application-server keypair Web Push requires. +// The public key is handed to the browser (it validates pushes against it); +// the private key signs the VAPID JWT on each send. Keys must stay stable — +// rotating them invalidates every existing browser subscription. +type VAPIDKeys struct { + PublicKey string `json:"publicKey"` + PrivateKey string `json:"privateKey"` +} + +// WebPushNotifier sends pushes via the Web Push protocol (RFC 8291 + VAPID). +// It implements Notifier by handling targets of NotificationTypeWeb and +// ignoring all others. Each target's Token is the JSON-serialized +// PushSubscription object the browser produced. +type WebPushNotifier struct { + keys VAPIDKeys + // subscriber is the contact URI embedded in the VAPID JWT (RFC 8291 + // "sub"). mailto: is conventional; a URL works too. Browsers ignore it + // for delivery but it's required by the spec. + subscriber string +} + +// NewWebPushNotifier builds a notifier from a VAPID keypair. The subscriber +// defaults to a mailto: if empty. +func NewWebPushNotifier(keys VAPIDKeys, subscriber string) *WebPushNotifier { + if subscriber == "" { + subscriber = "mailto:noreply@stream.place" + } + return &WebPushNotifier{keys: keys, subscriber: subscriber} +} + +// Blast implements Notifier. It fans a push out to every web target in +// parallel (each subscription is an independent HTTP POST to the browser +// push service). Firebase targets are ignored. +func (w *WebPushNotifier) Blast(ctx context.Context, targets []NotificationTarget, blast *NotificationBlast) error { + webTargets := make([]NotificationTarget, 0, len(targets)) + for _, t := range targets { + if t.Type == NotificationTypeWeb { + webTargets = append(webTargets, t) + } + } + if len(webTargets) == 0 { + return nil + } + + payload, err := json.Marshal(blast) + if err != nil { + return fmt.Errorf("error marshaling notification blast: %w", err) + } + + var ( + wg sync.WaitGroup + mu sync.Mutex + success int + failed int + errs []error + ) + + for _, t := range webTargets { + wg.Add(1) + go func(token string) { + defer wg.Done() + err := w.sendOne(ctx, token, payload) + mu.Lock() + defer mu.Unlock() + if err != nil { + failed++ + errs = append(errs, err) + log.Error(ctx, "web push failed", "err", err) + } else { + success++ + } + }(t.Token) + } + wg.Wait() + + log.Log(ctx, "web push blast complete", "success", success, "failed", failed, "total", len(webTargets)) + if len(errs) == 0 { + return nil + } + if len(errs) == 1 { + return errs[0] + } + return fmt.Errorf("web push blast: %d of %d failed: %v", len(errs), len(webTargets), errs) +} + +// sendOne decrypts the stored subscription JSON and POSTs the encrypted +// payload to the browser's push endpoint. +func (w *WebPushNotifier) sendOne(ctx context.Context, subscriptionJSON string, payload []byte) error { + var sub webpush.Subscription + if err := json.Unmarshal([]byte(subscriptionJSON), &sub); err != nil { + return fmt.Errorf("error parsing web push subscription: %w", err) + } + resp, err := webpush.SendNotificationWithContext(ctx, payload, &sub, &webpush.Options{ + VAPIDPublicKey: w.keys.PublicKey, + VAPIDPrivateKey: w.keys.PrivateKey, + Subscriber: w.subscriber, + TTL: 24 * 60 * 60, // 24h + }) + if err != nil { + return fmt.Errorf("error sending web push: %w", err) + } + defer resp.Body.Close() + // 410 Gone means the subscription is no longer valid; the caller should + // prune it. We surface it distinctly so the queue processor can react. + if resp.StatusCode == 410 || resp.StatusCode == 404 { + return &ExpiredSubscriptionError{Endpoint: sub.Endpoint, Status: resp.StatusCode} + } + if resp.StatusCode >= 400 { + return fmt.Errorf("web push endpoint returned status %d", resp.StatusCode) + } + return nil +} + +// ExpiredSubscriptionError indicates a push endpoint returned 410 Gone (or +// 404), meaning the subscription is dead and should be removed from the DB. +type ExpiredSubscriptionError struct { + Endpoint string + Status int +} + +func (e *ExpiredSubscriptionError) Error() string { + return fmt.Sprintf("web push subscription expired (status %d): %s", e.Status, e.Endpoint) +} diff --git a/pkg/notifications/webpush_test.go b/pkg/notifications/webpush_test.go new file mode 100644 index 00000000..7a57b311 --- /dev/null +++ b/pkg/notifications/webpush_test.go @@ -0,0 +1,127 @@ +package notifications + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + webpush "github.com/SherClockHolmes/webpush-go" +) + +// TestWebPushNotifierBlast verifies that the WebPushNotifier: +// - ignores firebase targets, +// - POSTs the encrypted payload to each web subscription's endpoint, +// - surfaces a 410 Gone as an ExpiredSubscriptionError so the caller can +// prune the dead subscription. +func TestWebPushNotifierBlast(t *testing.T) { + // Spin up a fake push service that records what it receives. + var ( + gotPaths []string + gotBodies [][]byte + status int + ) + mux := http.NewServeMux() + mux.HandleFunc("/push/", func(w http.ResponseWriter, r *http.Request) { + gotPaths = append(gotPaths, r.URL.Path) + body, _ := io.ReadAll(r.Body) + gotBodies = append(gotBodies, body) + if status != 0 { + w.WriteHeader(status) + return + } + w.WriteHeader(201) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + // Generate a real VAPID keypair so the notifier can sign. + priv, pub, err := webpush.GenerateVAPIDKeys() + require.NoError(t, err) + notifier := NewWebPushNotifier(VAPIDKeys{PublicKey: pub, PrivateKey: priv}, "mailto:test@example.com") + + // Build a web subscription pointing at our fake push service. + sub := webpush.Subscription{ + Endpoint: srv.URL + "/push/abc", + Keys: webpush.Keys{ + P256dh: "BMd4Zb1d3Z2Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8Z8", + Auth: "dGhpcyBpcyBhbiBhdXRoIGtleQ", + }, + } + // Use a real, valid-length p256dh so the library doesn't reject it before + // hitting the network. Generate a throwaway ECDH keypair for the client + // side and use its raw public key. + _, clientPub, err := webpush.GenerateVAPIDKeys() + require.NoError(t, err) + sub.Keys.P256dh = clientPub + + subJSON, err := json.Marshal(sub) + require.NoError(t, err) + + targets := []NotificationTarget{ + {Token: "firebase-token-should-be-ignored", Type: NotificationTypeFirebase}, + {Token: string(subJSON), Type: NotificationTypeWeb}, + } + + blast := &NotificationBlast{ + Title: "🔴 @test is LIVE!", + Body: "hello world", + Data: map[string]string{"path": "/test"}, + } + + err = notifier.Blast(context.Background(), targets, blast) + require.NoError(t, err) + require.Len(t, gotPaths, 1, "only the web target should have been pushed to") + require.Equal(t, "/push/abc", gotPaths[0]) + + // The body is encrypted (RFC 8291), so it won't be our plaintext JSON — + // just confirm something non-empty was sent. + require.NotEmpty(t, gotBodies[0]) + + // Now make the endpoint return 410 Gone and confirm we get an + // ExpiredSubscriptionError. + status = 410 + err = notifier.Blast(context.Background(), targets, blast) + require.Error(t, err) + var expired *ExpiredSubscriptionError + require.ErrorAs(t, err, &expired, "410 should surface as ExpiredSubscriptionError") + require.Equal(t, 410, expired.Status) +} + +// TestMultiNotifierFanout confirms the MultiNotifier delegates to each child +// notifier and that each child only acts on its own target type. +func TestMultiNotifierFanout(t *testing.T) { + fb := &recordingNotifier{typeFilter: NotificationTypeFirebase} + web := &recordingNotifier{typeFilter: NotificationTypeWeb} + multi := NewMultiNotifier(fb, web) + + targets := []NotificationTarget{ + {Token: "fb-1", Type: NotificationTypeFirebase}, + {Token: "web-1", Type: NotificationTypeWeb}, + {Token: "fb-2", Type: NotificationTypeFirebase}, + } + blast := &NotificationBlast{Title: "t", Body: "b"} + + require.NoError(t, multi.Blast(context.Background(), targets, blast)) + require.Equal(t, []string{"fb-1", "fb-2"}, fb.seen, "firebase notifier should only see firebase targets") + require.Equal(t, []string{"web-1"}, web.seen, "web notifier should only see web targets") +} + +// recordingNotifier is a test double that records the tokens it was asked to +// blast, filtered to a single type. +type recordingNotifier struct { + typeFilter NotificationType + seen []string +} + +func (r *recordingNotifier) Blast(ctx context.Context, targets []NotificationTarget, blast *NotificationBlast) error { + for _, t := range targets { + if t.Type == r.typeFilter { + r.seen = append(r.seen, t.Token) + } + } + return nil +} diff --git a/pkg/statedb/notification.go b/pkg/statedb/notification.go index 3c98f74e..de96e709 100644 --- a/pkg/statedb/notification.go +++ b/pkg/statedb/notification.go @@ -5,13 +5,25 @@ import ( "time" "gorm.io/gorm/clause" + notificationpkg "stream.place/streamplace/pkg/notifications" +) + +// NotificationType is re-exported from pkg/notifications so callers of this +// package don't need a second import to name the transport. +type NotificationType = notificationpkg.NotificationType + +// Re-export the transport constants for the same reason. +const ( + NotificationTypeFirebase = notificationpkg.NotificationTypeFirebase + NotificationTypeWeb = notificationpkg.NotificationTypeWeb ) type Notification struct { - Token string `gorm:"column:token;primarykey"` - RepoDID string `json:"repoDID,omitempty" gorm:"column:repo_did;index"` - CreatedAt time.Time `gorm:"column:created_at"` - UpdatedAt time.Time `gorm:"column:updated_at"` + Token string `gorm:"column:token;primarykey"` + RepoDID string `json:"repoDID,omitempty" gorm:"column:repo_did;index"` + Type NotificationType `json:"type,omitempty" gorm:"column:type;default:firebase"` + CreatedAt time.Time `gorm:"column:created_at"` + UpdatedAt time.Time `gorm:"column:updated_at"` } // CreateNotification registers (or refreshes) a device's push token. When a @@ -19,19 +31,28 @@ type Notification struct { // can target the user's followers. When repoDID is empty we make sure the // token row exists but never clobber an existing repoDID association. // +// notifType selects the push transport ("firebase" or "web"). An empty value +// defaults to "firebase" so existing callers and pre-migration rows keep +// working. The type is only written when a row is created or a repoDID is +// being upserted; a DID-less re-registration deliberately leaves the type +// untouched (mirroring the repoDID-preservation behavior below). +// // This deliberately avoids DB.Save(): Save issues a full-row UPDATE including // zero-value columns, so a re-registration with no repoDID (e.g. the client // posts before its OAuth session has restored) would blank out repo_did and // silently drop the user from follower notifications. -func (state *StatefulDB) CreateNotification(token string, repoDID string) error { +func (state *StatefulDB) CreateNotification(token string, repoDID string, notifType NotificationType) error { + if notifType == "" { + notifType = NotificationTypeFirebase + } if repoDID != "" { - not := Notification{Token: token, RepoDID: repoDID} + not := Notification{Token: token, RepoDID: repoDID, Type: notifType} return state.DB.Clauses(clause.OnConflict{ Columns: []clause.Column{{Name: "token"}}, - DoUpdates: clause.AssignmentColumns([]string{"repo_did", "updated_at"}), + DoUpdates: clause.AssignmentColumns([]string{"repo_did", "type", "updated_at"}), }).Create(¬).Error } - not := Notification{Token: token} + not := Notification{Token: token, Type: notifType} return state.DB.Clauses(clause.OnConflict{ Columns: []clause.Column{{Name: "token"}}, DoNothing: true, @@ -56,6 +77,9 @@ func (state *StatefulDB) ListUserNotifications(userDID string) ([]Notification, return nots, nil } +// GetManyNotificationTokens returns the raw token strings for the given user +// DIDs, across all notification types. Kept for backwards compatibility with +// callers that only need the token list (e.g. the legacy blast endpoint). func (state *StatefulDB) GetManyNotificationTokens(userDIDs []string) ([]string, error) { tokens := []string{} err := state.DB.Model(&Notification{}). @@ -67,3 +91,21 @@ func (state *StatefulDB) GetManyNotificationTokens(userDIDs []string) ([]string, } return tokens, nil } + +// GetManyNotifications returns the full notification rows for the given user +// DIDs, including each token's Type so the notifier can route it to the +// correct transport (firebase vs web). +func (state *StatefulDB) GetManyNotifications(userDIDs []string) ([]Notification, error) { + nots := []Notification{} + err := state.DB.Where("repo_did IN (?)", userDIDs).Find(¬s).Error + if err != nil { + return nil, fmt.Errorf("error retrieving notifications: %w", err) + } + return nots, nil +} + +// DeleteNotification removes a token row, used when a web client unsubscribes +// (or a mobile token is revoked). Missing rows are not an error. +func (state *StatefulDB) DeleteNotification(token string) error { + return state.DB.Where("token = ?", token).Delete(&Notification{}).Error +} diff --git a/pkg/statedb/notification_test.go b/pkg/statedb/notification_test.go index 1f593df9..7908b1a9 100644 --- a/pkg/statedb/notification_test.go +++ b/pkg/statedb/notification_test.go @@ -19,14 +19,14 @@ func TestNotificationRepoDIDPreserved(t *testing.T) { const didB = "did:plc:bbbb" // Initial registration while logged in associates the DID. - require.NoError(t, state.CreateNotification(token, didA)) + require.NoError(t, state.CreateNotification(token, didA, NotificationTypeFirebase)) tokens, err := state.GetManyNotificationTokens([]string{didA}) require.NoError(t, err) require.Equal(t, []string{token}, tokens) // Re-registration without a DID (e.g. before the OAuth session has // restored) must NOT clobber the existing association. - require.NoError(t, state.CreateNotification(token, "")) + require.NoError(t, state.CreateNotification(token, "", NotificationTypeFirebase)) tokens, err = state.GetManyNotificationTokens([]string{didA}) require.NoError(t, err) require.Equal(t, []string{token}, tokens, "repo_did was wiped by a DID-less re-registration") @@ -37,7 +37,7 @@ func TestNotificationRepoDIDPreserved(t *testing.T) { require.Len(t, nots, 1) // Re-registering with a different DID replaces the association. - require.NoError(t, state.CreateNotification(token, didB)) + require.NoError(t, state.CreateNotification(token, didB, NotificationTypeFirebase)) tokens, err = state.GetManyNotificationTokens([]string{didB}) require.NoError(t, err) require.Equal(t, []string{token}, tokens) @@ -56,7 +56,7 @@ func TestNotificationAnonymousThenAssociated(t *testing.T) { const did = "did:plc:cccc" // Anonymous registration: the row exists but has no association yet. - require.NoError(t, state.CreateNotification(token, "")) + require.NoError(t, state.CreateNotification(token, "", NotificationTypeFirebase)) tokens, err := state.GetManyNotificationTokens([]string{did}) require.NoError(t, err) require.Empty(t, tokens) @@ -65,7 +65,7 @@ func TestNotificationAnonymousThenAssociated(t *testing.T) { require.Len(t, nots, 1) // Once logged in, the association is set without adding a new row. - require.NoError(t, state.CreateNotification(token, did)) + require.NoError(t, state.CreateNotification(token, did, NotificationTypeFirebase)) tokens, err = state.GetManyNotificationTokens([]string{did}) require.NoError(t, err) require.Equal(t, []string{token}, tokens) @@ -74,3 +74,53 @@ func TestNotificationAnonymousThenAssociated(t *testing.T) { require.Len(t, nots, 1) }) } + +// TestNotificationTypeAndDelete covers the Type column (firebase vs web) and +// the DeleteNotification path used when a web client unsubscribes. +func TestNotificationTypeAndDelete(t *testing.T) { + WithAllDatabases(t, func(state *StatefulDB) { + const did = "did:plc:dddd" + const fbToken = "firebase-token-1" + const webToken = `{"endpoint":"https://push.example/abc","keys":{"p256dh":"x","auth":"y"}}` + + // Register one firebase and one web subscription for the same user. + require.NoError(t, state.CreateNotification(fbToken, did, NotificationTypeFirebase)) + require.NoError(t, state.CreateNotification(webToken, did, NotificationTypeWeb)) + + // GetManyNotifications returns both rows with their types intact. + nots, err := state.GetManyNotifications([]string{did}) + require.NoError(t, err) + require.Len(t, nots, 2) + + byType := map[NotificationType]Notification{} + for _, n := range nots { + byType[n.Type] = n + } + require.Contains(t, byType, NotificationTypeFirebase) + require.Contains(t, byType, NotificationTypeWeb) + require.Equal(t, fbToken, byType[NotificationTypeFirebase].Token) + require.Equal(t, webToken, byType[NotificationTypeWeb].Token) + + // An empty type defaults to firebase. + require.NoError(t, state.CreateNotification("defaulted-token", did, "")) + nots, err = state.GetManyNotifications([]string{did}) + require.NoError(t, err) + var defaulted Notification + for _, n := range nots { + if n.Token == "defaulted-token" { + defaulted = n + } + } + require.Equal(t, NotificationTypeFirebase, defaulted.Type, "empty type should default to firebase") + + // DeleteNotification removes the row; deleting a missing row is not an error. + require.NoError(t, state.DeleteNotification(webToken)) + nots, err = state.GetManyNotifications([]string{did}) + require.NoError(t, err) + require.Len(t, nots, 2, "web token should be gone, leaving firebase + defaulted") + for _, n := range nots { + require.NotEqual(t, webToken, n.Token) + } + require.NoError(t, state.DeleteNotification("never-existed")) + }) +} diff --git a/pkg/statedb/queue_processor.go b/pkg/statedb/queue_processor.go index ae5b2489..cc765433 100644 --- a/pkg/statedb/queue_processor.go +++ b/pkg/statedb/queue_processor.go @@ -185,6 +185,13 @@ type VODProcessor func(ctx context.Context, t VODProcessTask) (cid string, err e func (state *StatefulDB) SetVODProcessor(f VODProcessor) { state.vodProcessor = f } +// SetNotifier installs the notification notifier after construction. This is +// needed because building the Web Push notifier requires VAPID keys, which +// are stored in the DB — so the DB must exist before the notifier can be +// fully assembled. The queue processor checks for nil, so a brief window +// with no notifier is safe. +func (state *StatefulDB) SetNotifier(n notificationpkg.Notifier) { state.noter = n } + func (state *StatefulDB) processVODProcessTask(ctx context.Context, task *AppTask) error { ctx = log.WithLogValues(ctx, "func", "processVODProcessTask") var t VODProcessTask @@ -467,7 +474,7 @@ func (state *StatefulDB) processNotificationTask(ctx context.Context, task *AppT log.Log(ctx, "found followers", "count", len(followersDIDs)) - notifications, err := state.GetManyNotificationTokens(followersDIDs) + notifications, err := state.GetManyNotifications(followersDIDs) if err != nil { return err } @@ -480,7 +487,11 @@ func (state *StatefulDB) processNotificationTask(ctx context.Context, task *AppT "path": fmt.Sprintf("/%s", lsv.Author.Handle), }, } - err = state.noter.Blast(ctx, notifications, nb) + targets := make([]notificationpkg.NotificationTarget, len(notifications)) + for i, n := range notifications { + targets[i] = notificationpkg.NotificationTarget{Token: n.Token, Type: n.Type} + } + err = state.noter.Blast(ctx, targets, nb) if err != nil { log.Error(ctx, "failed to blast notifications", "err", err) } else { diff --git a/pkg/statedb/statedb.go b/pkg/statedb/statedb.go index 311fb3ac..ea1d8f94 100644 --- a/pkg/statedb/statedb.go +++ b/pkg/statedb/statedb.go @@ -30,7 +30,7 @@ type StatefulDB struct { CLI *config.CLI Type DBType locks *NamedLocks - noter notificationpkg.FirebaseNotifier + noter notificationpkg.Notifier model model.Model // pokeQueue is used to wake up the queue processor when a new task is enqueued pokeQueue chan struct{} @@ -76,7 +76,7 @@ var StatefulDBModels = []any{ var NoPostgresDatabaseCode = "3D000" // Stateful database for storing private streamplace state -func MakeDB(ctx context.Context, cli *config.CLI, noter notificationpkg.FirebaseNotifier, model model.Model) (*StatefulDB, error) { +func MakeDB(ctx context.Context, cli *config.CLI, noter notificationpkg.Notifier, model model.Model) (*StatefulDB, error) { dbURL := cli.DBURL log.Log(ctx, "starting stateful database", "dbURL", redactDBURL(dbURL)) var dial gorm.Dialector diff --git a/pkg/statedb/vapid.go b/pkg/statedb/vapid.go new file mode 100644 index 00000000..54a6bb4c --- /dev/null +++ b/pkg/statedb/vapid.go @@ -0,0 +1,59 @@ +package statedb + +import ( + "context" + "encoding/json" + "fmt" + + webpush "github.com/SherClockHolmes/webpush-go" + notificationpkg "stream.place/streamplace/pkg/notifications" + "stream.place/streamplace/pkg/log" +) + +// vapidConfigKey is the Config-table key under which the VAPID keypair is +// stored, mirroring how EnsureJWK persists JWKs by name. +const vapidConfigKey = "vapid-keys" + +// EnsureVAPIDKeys returns the Web Push VAPID keypair, generating and +// persisting it on first use. It follows the same pattern as EnsureJWK: +// look the key up in the Config table; if present, use it; otherwise +// generate a fresh P-256 keypair and store it so it survives restarts. +// +// VAPID keys must stay stable — rotating them invalidates every existing +// browser subscription, so we never regenerate once a key exists. +func (state *StatefulDB) EnsureVAPIDKeys(ctx context.Context) (notificationpkg.VAPIDKeys, error) { + conf, err := state.GetConfig(vapidConfigKey) + if err != nil { + return notificationpkg.VAPIDKeys{}, fmt.Errorf("error loading vapid keys: %w", err) + } + + // happy path: we found the keys in the database, use that + if conf != nil { + var keys notificationpkg.VAPIDKeys + if err := json.Unmarshal(conf.Value, &keys); err != nil { + return notificationpkg.VAPIDKeys{}, fmt.Errorf("error parsing stored vapid keys: %w", err) + } + return keys, nil + } + + // new path: no keys yet, generate a fresh pair + log.Warn(ctx, "no VAPID keys found, generating new ones") + privateKey, publicKey, err := webpush.GenerateVAPIDKeys() + if err != nil { + return notificationpkg.VAPIDKeys{}, fmt.Errorf("failed to generate vapid keys: %w", err) + } + keys := notificationpkg.VAPIDKeys{ + PublicKey: publicKey, + PrivateKey: privateKey, + } + + b, err := json.Marshal(keys) + if err != nil { + return notificationpkg.VAPIDKeys{}, fmt.Errorf("failed to marshal vapid keys: %w", err) + } + if err := state.PutConfig(vapidConfigKey, b); err != nil { + return notificationpkg.VAPIDKeys{}, fmt.Errorf("failed to save vapid keys: %w", err) + } + + return keys, nil +} -- 2.51.2 From 7cf4161852a6f4d5b1ed31e54527e2ee36fbad2f Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Wed, 22 Jul 2026 15:17:50 -0700 Subject: [PATCH 08/20] fix: address Greptile review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four issues flagged by Greptile on #1211: 1. Expired subscriptions never pruned from blast path. The ExpiredSubscriptionError type existed but no caller unwrapped it to delete dead rows. WebPushNotifier.Blast now collects expired tokens into a BlastsError; ExpiredTokens(err) walks the MultiNotifier → errors.Join → BlastsError tree to extract them. Both blast call sites (queue_processor, sync.go) now prune dead subscriptions after each blast so they don't accumulate and burn time on every notification. 2. HandleVapidPublicKey manually concatenated JSON. Replaced with json.NewEncoder so serialization is always correct. 3. Notifications settings screen forced a re-render every second to refresh Notification.permission. Replaced with the Permissions API onchange listener, which fires only on actual state transitions. 4. disableWebNotifications cleared notificationToken in a finally block even when the server DELETE failed, leaving the user unable to retry while the server kept pushing. Now only clears after a confirmed successful DELETE; on failure the token stays set so the toggle remains "on" and the user can retry. Co-Authored-By: Claude Opus 4.8 --- .../notifications-category-settings.tsx | 20 +++-- js/app/store/slices/platformSlice.ts | 16 +++- pkg/api/api.go | 5 +- pkg/atproto/sync.go | 10 ++- pkg/notifications/multi.go | 12 +-- pkg/notifications/webpush.go | 82 ++++++++++++++++++- pkg/notifications/webpush_test.go | 52 ++++++++++++ pkg/statedb/queue_processor.go | 9 ++ 8 files changed, 179 insertions(+), 27 deletions(-) diff --git a/js/app/components/settings/notifications-category-settings.tsx b/js/app/components/settings/notifications-category-settings.tsx index 9ef4f303..402c570c 100644 --- a/js/app/components/settings/notifications-category-settings.tsx +++ b/js/app/components/settings/notifications-category-settings.tsx @@ -46,16 +46,20 @@ export function NotificationsCategorySettings() { ? permission === "granted" && !!notificationToken : permission === "granted"; - // Re-check permission when the screen gains focus (the user may have - // changed it in browser settings). - const [refreshKey, setRefreshKey] = useState(0); + // Re-check permission when the browser's permission state changes (e.g. the + // user toggled it in browser settings while the app is open). + const [, forceRender] = useState(0); useEffect(() => { - if (!isWeb) return; - const interval = setInterval(() => setRefreshKey((k) => k + 1), 1000); - return () => clearInterval(interval); + if (!isWeb || typeof navigator.permissions === "undefined") return; + let status: PermissionStatus | undefined; + navigator.permissions.query({ name: "notifications" }).then((s) => { + status = s; + s.onchange = () => forceRender((k) => k + 1); + }); + return () => { + if (status) status.onchange = null; + }; }, [isWeb]); - // touch refreshKey so the linter doesn't complain and the re-render happens - void refreshKey; const handleToggle = async (value: boolean) => { if (busy) return; diff --git a/js/app/store/slices/platformSlice.ts b/js/app/store/slices/platformSlice.ts index 75e4b284..426457b1 100644 --- a/js/app/store/slices/platformSlice.ts +++ b/js/app/store/slices/platformSlice.ts @@ -145,19 +145,27 @@ export const createPlatformSlice: StateCreator< if (existing && existing.endpoint === sub.endpoint) { await existing.unsubscribe(); } - // Tell the server to drop the row. + // Tell the server to drop the row. Only clear the local token after + // the DELETE succeeds — otherwise the toggle shows "off" while the + // server keeps pushing to a subscription the user thought they + // disabled, with no way to retry. if (url) { - await fetch(`${url}/api/notification`, { + const res = await fetch(`${url}/api/notification`, { method: "DELETE", headers: { "content-type": "application/json" }, body: JSON.stringify({ token: notificationToken }), }); + if (!res.ok) { + throw new Error(`server delete failed: ${res.status}`); + } } + set({ notificationToken: null }); } } catch (e) { console.error("disableWebNotifications error", e); - } finally { - set({ notificationToken: null }); + // Leave notificationToken set so the toggle stays "on" and the user + // can retry. The browser-side unsubscribe may have already succeeded, + // but the server row is what matters for stopping future pushes. } }, webNotificationPermission: () => { diff --git a/pkg/api/api.go b/pkg/api/api.go index 331ab77c..bc8a9688 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -645,7 +645,10 @@ func (a *StreamplaceAPI) HandleVapidPublicKey(ctx context.Context) http.HandlerF return } w.Header().Set("Content-Type", "application/json") - if _, err := w.Write([]byte(`{"publicKey":"` + keys.PublicKey + `"}`)); err != nil { + resp := struct { + PublicKey string `json:"publicKey"` + }{PublicKey: keys.PublicKey} + if err := json.NewEncoder(w).Encode(resp); err != nil { log.Error(ctx, "error writing vapid public key", "error", err) } } diff --git a/pkg/atproto/sync.go b/pkg/atproto/sync.go index 401a6b3f..03aa1370 100644 --- a/pkg/atproto/sync.go +++ b/pkg/atproto/sync.go @@ -981,9 +981,15 @@ func (atsync *ATProtoSynchronizer) notifyBetaInvite(ctx context.Context, rec *pl } if err := atsync.Noter.Blast(ctx, targets, blast); err != nil { log.Error(ctx, "beta invite notification: blast failed", "did", rec.Did, "feature", rec.Feature, "err", err) - return + } else { + log.Log(ctx, "sent beta invite notification", "did", rec.Did, "feature", rec.Feature, "tokens", len(notifications)) + } + // Prune dead web push subscriptions so they don't accumulate. + for _, token := range notificationpkg.ExpiredTokens(err) { + if delErr := atsync.StatefulDB.DeleteNotification(token); delErr != nil { + log.Error(ctx, "beta invite notification: failed to prune expired", "token", token, "err", delErr) + } } - log.Log(ctx, "sent beta invite notification", "did", rec.Did, "feature", rec.Feature, "tokens", len(notifications)) } // betaInviteBlast builds the push payload for a newly-granted beta feature. diff --git a/pkg/notifications/multi.go b/pkg/notifications/multi.go index ae008dee..a4ec9c5e 100644 --- a/pkg/notifications/multi.go +++ b/pkg/notifications/multi.go @@ -2,7 +2,7 @@ package notifications import ( "context" - "fmt" + "errors" ) // MultiNotifier fans a single blast out to every transport it wraps. It @@ -42,11 +42,7 @@ func (m *MultiNotifier) Blast(ctx context.Context, targets []NotificationTarget, errs = append(errs, err) } } - if len(errs) == 1 { - return errs[0] - } - if len(errs) > 1 { - return fmt.Errorf("multi-notifier: %d transports failed: %v", len(errs), errs) - } - return nil + // errors.Join preserves the tree so callers can errors.As into the + // individual transport errors (e.g. to extract expired web tokens). + return errors.Join(errs...) } diff --git a/pkg/notifications/webpush.go b/pkg/notifications/webpush.go index ba5adbaa..8c8dbf9d 100644 --- a/pkg/notifications/webpush.go +++ b/pkg/notifications/webpush.go @@ -3,6 +3,7 @@ package notifications import ( "context" "encoding/json" + "errors" "fmt" "sync" @@ -43,6 +44,11 @@ func NewWebPushNotifier(keys VAPIDKeys, subscriber string) *WebPushNotifier { // Blast implements Notifier. It fans a push out to every web target in // parallel (each subscription is an independent HTTP POST to the browser // push service). Firebase targets are ignored. +// +// Expired subscriptions (410 Gone / 404) are collected and returned as a +// *BlastError whose Expired field holds the raw subscription tokens that +// should be pruned from the DB. Callers can extract them with +// ExpiredTokens(err). func (w *WebPushNotifier) Blast(ctx context.Context, targets []NotificationTarget, blast *NotificationBlast) error { webTargets := make([]NotificationTarget, 0, len(targets)) for _, t := range targets { @@ -65,6 +71,7 @@ func (w *WebPushNotifier) Blast(ctx context.Context, targets []NotificationTarge success int failed int errs []error + expired []string ) for _, t := range webTargets { @@ -77,6 +84,10 @@ func (w *WebPushNotifier) Blast(ctx context.Context, targets []NotificationTarge if err != nil { failed++ errs = append(errs, err) + var expiredErr *ExpiredSubscriptionError + if errors.As(err, &expiredErr) { + expired = append(expired, token) + } log.Error(ctx, "web push failed", "err", err) } else { success++ @@ -85,14 +96,77 @@ func (w *WebPushNotifier) Blast(ctx context.Context, targets []NotificationTarge } wg.Wait() - log.Log(ctx, "web push blast complete", "success", success, "failed", failed, "total", len(webTargets)) + log.Log(ctx, "web push blast complete", "success", success, "failed", failed, "total", len(webTargets), "expired", len(expired)) if len(errs) == 0 { return nil } - if len(errs) == 1 { - return errs[0] + return &BlastsError{ + Errs: errs, + Expired: expired, } - return fmt.Errorf("web push blast: %d of %d failed: %v", len(errs), len(webTargets), errs) +} + +// BlastsError wraps the per-target errors from a WebPushNotifier.Blast and +// exposes the subscription tokens that should be pruned because their +// endpoints returned 410 Gone / 404. +type BlastsError struct { + // Errs are all per-target errors, including ExpiredSubscriptionErrors. + Errs []error + // Expired holds the raw subscription tokens (DB primary keys) whose + // endpoints are dead and should be deleted. + Expired []string +} + +func (e *BlastsError) Error() string { + if len(e.Errs) == 1 { + return e.Errs[0].Error() + } + return fmt.Sprintf("web push blast: %d targets failed", len(e.Errs)) +} + +// Unwrap returns the wrapped errors so errors.Is / errors.As can traverse them. +func (e *BlastsError) Unwrap() []error { return e.Errs } + +// ExpiredTokens walks an error tree (handling errors.Join, MultiNotifier, and +// BlastsError wrapping) and returns the raw subscription tokens whose push +// endpoints returned 410 Gone / 404. Callers should delete these rows from +// the notifications table so dead subscriptions don't accumulate. +func ExpiredTokens(err error) []string { + if err == nil { + return nil + } + var expired []string + // If this node is a BlastsError, take its Expired slice directly and + // stop — recursing into its Unwrap() would only re-encounter the same + // ExpiredSubscriptionErrors and double-count. + var be *BlastsError + if errors.As(err, &be) { + return be.Expired + } + // Otherwise traverse children (errors.Join from MultiNotifier, or a + // single Unwrap chain) looking for nested BlastsErrors. + for _, inner := range errorsUnwrap(err) { + expired = append(expired, ExpiredTokens(inner)...) + } + return expired +} + +// errorsUnwrap returns the direct children of err for tree-walking. Supports +// errors.Join (Unwrap() []error) and single-wrap (Unwrap() error). +func errorsUnwrap(err error) []error { + // errors.Join / BlastsError expose Unwrap() []error + type multiUnwrapper interface{ Unwrap() []error } + if mu, ok := err.(multiUnwrapper); ok { + return mu.Unwrap() + } + // standard single-wrap + type unwrapper interface{ Unwrap() error } + if u, ok := err.(unwrapper); ok { + if inner := u.Unwrap(); inner != nil { + return []error{inner} + } + } + return nil } // sendOne decrypts the stored subscription JSON and POSTs the encrypted diff --git a/pkg/notifications/webpush_test.go b/pkg/notifications/webpush_test.go index 7a57b311..bd63554a 100644 --- a/pkg/notifications/webpush_test.go +++ b/pkg/notifications/webpush_test.go @@ -110,6 +110,58 @@ func TestMultiNotifierFanout(t *testing.T) { require.Equal(t, []string{"web-1"}, web.seen, "web notifier should only see web targets") } +// TestExpiredTokensExtraction confirms that ExpiredTokens walks the +// MultiNotifier → BlastsError tree and returns the dead subscription tokens +// so callers can prune them from the DB. +func TestExpiredTokensExtraction(t *testing.T) { + priv, pub, err := webpush.GenerateVAPIDKeys() + require.NoError(t, err) + notifier := NewWebPushNotifier(VAPIDKeys{PublicKey: pub, PrivateKey: priv}, "mailto:test@example.com") + + // Two web targets: one live, one that returns 410 Gone. + var liveSub, deadSub webpush.Subscription + _, livePub, err := webpush.GenerateVAPIDKeys() + require.NoError(t, err) + liveSub.Keys.P256dh = livePub + _, deadPub, err := webpush.GenerateVAPIDKeys() + require.NoError(t, err) + deadSub.Keys.P256dh = deadPub + + mux := http.NewServeMux() + liveHits := 0 + deadHits := 0 + mux.HandleFunc("/live", func(w http.ResponseWriter, r *http.Request) { + liveHits++ + w.WriteHeader(201) + }) + mux.HandleFunc("/dead", func(w http.ResponseWriter, r *http.Request) { + deadHits++ + w.WriteHeader(410) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + liveSub.Endpoint = srv.URL + "/live" + deadSub.Endpoint = srv.URL + "/dead" + liveJSON, _ := json.Marshal(liveSub) + deadJSON, _ := json.Marshal(deadSub) + + targets := []NotificationTarget{ + {Token: string(liveJSON), Type: NotificationTypeWeb}, + {Token: string(deadJSON), Type: NotificationTypeWeb}, + } + // Wrap in a MultiNotifier to test the full error tree the callers see. + multi := NewMultiNotifier(notifier) + err = multi.Blast(context.Background(), targets, &NotificationBlast{Title: "t", Body: "b"}) + require.Error(t, err) + + expired := ExpiredTokens(err) + require.Len(t, expired, 1, "only the dead subscription should be extracted") + require.Equal(t, string(deadJSON), expired[0]) + require.Equal(t, 1, liveHits) + require.Equal(t, 1, deadHits) +} + // recordingNotifier is a test double that records the tokens it was asked to // blast, filtered to a single type. type recordingNotifier struct { diff --git a/pkg/statedb/queue_processor.go b/pkg/statedb/queue_processor.go index cc765433..1756f1d4 100644 --- a/pkg/statedb/queue_processor.go +++ b/pkg/statedb/queue_processor.go @@ -497,6 +497,15 @@ func (state *StatefulDB) processNotificationTask(ctx context.Context, task *AppT } else { log.Log(ctx, "sent notifications", "user", userDID, "count", len(notifications), "content", nb) } + // Prune web push subscriptions whose endpoints returned 410 Gone / + // 404 — they're dead and would just fail again on every future blast. + for _, token := range notificationpkg.ExpiredTokens(err) { + if delErr := state.DeleteNotification(token); delErr != nil { + log.Error(ctx, "failed to prune expired notification", "token", token, "err", delErr) + } else { + log.Log(ctx, "pruned expired notification", "token", token) + } + } } else { log.Log(ctx, "no notifier configured, skipping notifications", "user", userDID, "count", len(notifications)) } -- 2.51.2 From e9f4fba75a8d3e425129461c92c139003791f08c Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Wed, 22 Jul 2026 15:56:34 -0700 Subject: [PATCH 09/20] style: gofmt files flagged by CI CI's gofmt check failed on webpush_test.go, vapid.go, and api.go from the previous commit. Purely formatting (alignment); no logic changes. Co-Authored-By: Claude Opus 4.8 --- pkg/api/api.go | 30 +++++++++++++++--------------- pkg/notifications/webpush_test.go | 2 +- pkg/statedb/vapid.go | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index bc8a9688..c6c989e8 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -57,20 +57,20 @@ import ( ) type StreamplaceAPI struct { - CLI *config.CLI - Model model.Model - StatefulDB *statedb.StatefulDB - LocalDB localdb.LocalDB - Updater *Updater - Signer *eip712.EIP712Signer - Mimes map[string]string - Notifier notifications.Notifier - MediaManager *media.MediaManager - MediaSigner media.MediaSigner - UploadManager *upload.Manager - PlaybackStore blob.Store - ViewLog *viewlog.Writer - XRPCServer *spxrpc.Server + CLI *config.CLI + Model model.Model + StatefulDB *statedb.StatefulDB + LocalDB localdb.LocalDB + Updater *Updater + Signer *eip712.EIP712Signer + Mimes map[string]string + Notifier notifications.Notifier + MediaManager *media.MediaManager + MediaSigner media.MediaSigner + UploadManager *upload.Manager + PlaybackStore blob.Store + ViewLog *viewlog.Writer + XRPCServer *spxrpc.Server // not thread-safe yet Aliases map[string]string Bus *bus.Bus @@ -108,7 +108,7 @@ func MakeStreamplaceAPI(cli *config.CLI, mod model.Model, statefulDB *statedb.St Model: mod, StatefulDB: statefulDB, Updater: updater, - Notifier: noter, + Notifier: noter, MediaManager: mm, MediaSigner: ms, UploadManager: um, diff --git a/pkg/notifications/webpush_test.go b/pkg/notifications/webpush_test.go index bd63554a..bf98e3ef 100644 --- a/pkg/notifications/webpush_test.go +++ b/pkg/notifications/webpush_test.go @@ -8,8 +8,8 @@ import ( "net/http/httptest" "testing" - "github.com/stretchr/testify/require" webpush "github.com/SherClockHolmes/webpush-go" + "github.com/stretchr/testify/require" ) // TestWebPushNotifierBlast verifies that the WebPushNotifier: diff --git a/pkg/statedb/vapid.go b/pkg/statedb/vapid.go index 54a6bb4c..c7029ced 100644 --- a/pkg/statedb/vapid.go +++ b/pkg/statedb/vapid.go @@ -6,8 +6,8 @@ import ( "fmt" webpush "github.com/SherClockHolmes/webpush-go" - notificationpkg "stream.place/streamplace/pkg/notifications" "stream.place/streamplace/pkg/log" + notificationpkg "stream.place/streamplace/pkg/notifications" ) // vapidConfigKey is the Config-table key under which the VAPID keypair is -- 2.51.2 From b0b621671da7f7c9a9044f9bd44de1eddf2f495b Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Thu, 23 Jul 2026 18:09:17 -0700 Subject: [PATCH 10/20] v0.11.18 --- js/app/package.json | 2 +- js/docs/package.json | 2 +- lerna.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/app/package.json b/js/app/package.json index 175f1e84..c8d9c4a1 100644 --- a/js/app/package.json +++ b/js/app/package.json @@ -1,7 +1,7 @@ { "name": "@streamplace/app", "main": "./src/entrypoint.tsx", - "version": "0.11.17", + "version": "0.11.18", "runtimeVersion": "0.10.0", "scripts": { "start": "npx expo start -c --port 38081", diff --git a/js/docs/package.json b/js/docs/package.json index 384aa781..d61c1cdd 100644 --- a/js/docs/package.json +++ b/js/docs/package.json @@ -1,7 +1,7 @@ { "name": "streamplace-docs", "type": "module", - "version": "0.11.17", + "version": "0.11.18", "scripts": { "dev": "astro dev --host 0.0.0.0 --port 38082", "start": "astro dev --host 0.0.0.0 --port 38082", diff --git a/lerna.json b/lerna.json index e7fda9a3..58da0dae 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "0.11.17", + "version": "0.11.18", "npmClient": "pnpm" } -- 2.51.2 From 582e2ae1dc67f87b495fe264c10ef9255ad7b131 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Fri, 24 Jul 2026 17:02:37 -0700 Subject: [PATCH 11/20] fix: only set notificationToken after successful backend registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged that enableWebNotifications committed the subscription token to the store before the POST /api/notification call, and never checked the response status. A network hiccup or server error after PushManager.subscribe() left the toggle showing "on" while the server had no subscription row — the user believed they were subscribed when they weren't. Now the token is only set after a confirmed successful POST (res.ok check), mirroring the disableWebNotifications fix from the earlier review pass. On failure the catch block returns "denied" so the toggle stays honest and the user can retry. Co-Authored-By: Claude Opus 4.8 --- js/app/store/slices/platformSlice.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/js/app/store/slices/platformSlice.ts b/js/app/store/slices/platformSlice.ts index 426457b1..80dc662d 100644 --- a/js/app/store/slices/platformSlice.ts +++ b/js/app/store/slices/platformSlice.ts @@ -108,9 +108,11 @@ export const createPlatformSlice: StateCreator< applicationServerKey: urlBase64ToUint8Array(publicKey) as BufferSource, }); const subJSON = JSON.stringify(subscription); - set({ notificationToken: subJSON }); - // Register the subscription with the backend. + // Register the subscription with the backend. Only commit the token to + // the store after a confirmed successful POST — otherwise the toggle + // shows "on" while the server has no subscription row, and the user + // believes they're subscribed when they aren't. const { oauthSession } = get(); const body: { token: string; type: string; repoDID?: string } = { token: subJSON, @@ -124,6 +126,10 @@ export const createPlatformSlice: StateCreator< headers: { "content-type": "application/json" }, body: JSON.stringify(body), }); + if (!res.ok) { + throw new Error(`server registration failed: ${res.status}`); + } + set({ notificationToken: subJSON }); console.log("web notification registration status:", res.status); return permission; } catch (e) { -- 2.51.2 From 70656146f18faefeb75162c546fac2149e4c0767 Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Fri, 24 Jul 2026 21:42:08 -0500 Subject: [PATCH 12/20] ListWebhooks active filter fix, and better o11y into webhooks - Include webhook URL in discord integration logs - Add error logging for failed webhook deliveries - Fix discord webhook listing logic for active status - Remove debug prints from webhook input conversion Signed-off-by: Natalie Bridgers --- pkg/integrations/discord/send-chat.go | 5 ++--- pkg/integrations/discord/send-livestream.go | 3 ++- pkg/integrations/discord/send-stream-received.go | 4 ++++ pkg/spxrpc/webhook.go | 9 ++++++--- pkg/statedb/webhook.go | 7 ------- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/integrations/discord/send-chat.go b/pkg/integrations/discord/send-chat.go index f3e499a0..4e6024df 100644 --- a/pkg/integrations/discord/send-chat.go +++ b/pkg/integrations/discord/send-chat.go @@ -51,7 +51,7 @@ func SendChat(ctx context.Context, w *discordtypes.Webhook, did string, scm *pla return fmt.Errorf("failed to marshal payload: %w", err) } - log.Warn(ctx, "sending chat to discord", "payload", string(jsonPayload)) + log.Warn(ctx, "sending chat to discord", "payload", string(jsonPayload), "webhook_url", w.URL) req, err := http.NewRequestWithContext(ctx, "POST", w.URL, bytes.NewReader(jsonPayload)) if err != nil { @@ -71,10 +71,9 @@ func SendChat(ctx context.Context, w *discordtypes.Webhook, did string, scm *pla } if resp.StatusCode != 204 { + log.Error(ctx, "chat webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", string(body)) return fmt.Errorf("failed to send chat to discord: %s", string(body)) } - log.Warn(ctx, "chat sent to discord", "payload", string(body)) - return nil } diff --git a/pkg/integrations/discord/send-livestream.go b/pkg/integrations/discord/send-livestream.go index c7819dcb..c4119393 100644 --- a/pkg/integrations/discord/send-livestream.go +++ b/pkg/integrations/discord/send-livestream.go @@ -93,7 +93,7 @@ func SendLivestream(ctx context.Context, w *discordtypes.Webhook, pdsURL string, return fmt.Errorf("failed to marshal payload: %w", err) } - log.Warn(ctx, "sending livestream to discord", "payload", string(jsonPayload)) + log.Warn(ctx, "sending livestream to discord", "payload", string(jsonPayload), "webhook_url", w.URL) req, err := http.NewRequestWithContext(ctx, "POST", w.URL, bytes.NewReader(jsonPayload)) if err != nil { @@ -112,6 +112,7 @@ func SendLivestream(ctx context.Context, w *discordtypes.Webhook, pdsURL string, if err != nil { return fmt.Errorf("failed to read response body: %w", err) } + log.Error(ctx, "livestream webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", string(body)) return fmt.Errorf("failed to send request (http %d): %s", resp.StatusCode, string(body)) } diff --git a/pkg/integrations/discord/send-stream-received.go b/pkg/integrations/discord/send-stream-received.go index d12f4599..f22c42af 100644 --- a/pkg/integrations/discord/send-stream-received.go +++ b/pkg/integrations/discord/send-stream-received.go @@ -11,6 +11,7 @@ import ( "stream.place/streamplace/pkg/aqhttp" "stream.place/streamplace/pkg/integrations/discord/discordtypes" + "stream.place/streamplace/pkg/log" ) func SendStreamReceived(ctx context.Context, w *discordtypes.Webhook, streamerDID string) error { @@ -28,6 +29,8 @@ func SendStreamReceived(ctx context.Context, w *discordtypes.Webhook, streamerDI return fmt.Errorf("failed to marshal payload: %w", err) } + log.Log(ctx, "sending stream.received to discord", "streamerDID", streamerDID, "webhook_url", w.URL) + req, err := http.NewRequestWithContext(ctx, "POST", w.URL, bytes.NewReader(jsonPayload)) if err != nil { return fmt.Errorf("failed to create request: %w", err) @@ -45,6 +48,7 @@ func SendStreamReceived(ctx context.Context, w *discordtypes.Webhook, streamerDI if err != nil { return fmt.Errorf("failed to read response body: %w", err) } + log.Error(ctx, "stream.received webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", string(body)) return fmt.Errorf("failed to send request (http %d): %s", resp.StatusCode, string(body)) } diff --git a/pkg/spxrpc/webhook.go b/pkg/spxrpc/webhook.go index f6ac245a..895875e9 100644 --- a/pkg/spxrpc/webhook.go +++ b/pkg/spxrpc/webhook.go @@ -44,7 +44,7 @@ func (s *Server) handlePlaceStreamServerCreateWebhook(ctx context.Context, input // Create webhook err = s.statefulDB.CreateWebhook(webhook) if err != nil { - log.Error(ctx, "failed to create webhook", "err", err) + log.Error(ctx, "failed to create webhook in database", "err", err, "url", input.Url, "events", input.Events) return nil, echo.NewHTTPError(http.StatusInternalServerError, "Failed to create webhook") } @@ -83,9 +83,12 @@ func (s *Server) handlePlaceStreamServerListWebhooks(ctx context.Context, active } // Build filters + // active defaults to true (show all active webhooks). When the client + // explicitly passes active=false, we filter to show inactive webhooks. + // When active=true, we show only active webhooks. filters := make(map[string]interface{}) - if !active { - filters["active"] = active + if active { + filters["active"] = true } // Get webhooks diff --git a/pkg/statedb/webhook.go b/pkg/statedb/webhook.go index a65d9c43..b74df539 100644 --- a/pkg/statedb/webhook.go +++ b/pkg/statedb/webhook.go @@ -225,19 +225,12 @@ func (w *Webhook) ToLexicon() (placestream.ServerDefs_Webhook, error) { // FromLexiconInput converts a placestream.ServerCreateWebhook_Input to a database Webhook func WebhookFromLexiconInput(input placestream.ServerCreateWebhook_Input, userDID, id string) (*Webhook, error) { - // Debug log the raw input - fmt.Printf("DEBUG: WebhookFromLexiconInput input.Events: %+v (type: %T)\n", input.Events, input.Events) - for i, event := range input.Events { - fmt.Printf("DEBUG: Event[%d]: %q (type: %T)\n", i, event, event) - } - var eventsJSON json.RawMessage if len(input.Events) > 0 { jsonBytes, err := json.Marshal(input.Events) if err != nil { return nil, fmt.Errorf("failed to marshal events: %w", err) } - fmt.Printf("DEBUG: Marshaled events JSON: %q\n", string(jsonBytes)) eventsJSON = json.RawMessage(jsonBytes) } else { // Default to empty array if no events provided -- 2.51.2 From c7c13cc485cbbb83f8275b108e45f33a72688cbf Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Fri, 24 Jul 2026 22:13:52 -0500 Subject: [PATCH 13/20] Propagate and apply rendition selection to WebRTC and HLS - Pass selected rendition to WebRTC hooks to support quality switching - Update HLS playlist RPC to support direct audio track requests via rendition parameter - Add PrimaryAudioTrackID helper to livehls to resolve audio-only streams Signed-off-by: Natalie Bridgers --- .../src/components/mobile-player/shared.tsx | 9 ++++++--- .../components/mobile-player/use-webrtc.tsx | 9 +++++++-- .../mobile-player/video-async.native.tsx | 2 +- .../src/components/mobile-player/video.tsx | 3 ++- pkg/livehls/livehls.go | 20 +++++++++++++++++++ pkg/livehls/livehls_test.go | 11 ++++++++++ pkg/spxrpc/place_stream_playback_getlive.go | 11 ++++++++++ 7 files changed, 58 insertions(+), 7 deletions(-) diff --git a/js/components/src/components/mobile-player/shared.tsx b/js/components/src/components/mobile-player/shared.tsx index 12991c4d..ee7d4b0e 100644 --- a/js/components/src/components/mobile-player/shared.tsx +++ b/js/components/src/components/mobile-player/shared.tsx @@ -45,10 +45,13 @@ export function srcToUrl( } let outUrl: string; if (protocol === PlayerProtocol.HLS) { - if (props.selectedRendition === "auto") { - outUrl = `${url}/xrpc/place.stream.playback.getLivePlaylist?streamer=${props.src}`; + if ( + props.selectedRendition && + props.selectedRendition !== "auto" && + props.selectedRendition !== "source" + ) { + outUrl = `${url}/xrpc/place.stream.playback.getLivePlaylist?streamer=${props.src}&rendition=${props.selectedRendition}`; } else { - // todo: re-implement track selection here outUrl = `${url}/xrpc/place.stream.playback.getLivePlaylist?streamer=${props.src}`; } } else if (protocol === PlayerProtocol.PROGRESSIVE_MP4) { diff --git a/js/components/src/components/mobile-player/use-webrtc.tsx b/js/components/src/components/mobile-player/use-webrtc.tsx index af2fc889..2f79b7bf 100644 --- a/js/components/src/components/mobile-player/use-webrtc.tsx +++ b/js/components/src/components/mobile-player/use-webrtc.tsx @@ -14,6 +14,7 @@ import { RTCPeerConnection, RTCSessionDescription } from "./webrtc-primitives"; export default function useWebRTC( streamer: string, + rendition: string, ): [MediaStream | null, boolean] { const [mediaStream, setMediaStream] = useState(null); const [stuck, setStuck] = useState(false); @@ -73,6 +74,7 @@ export default function useWebRTC( agent, isOwnStream, playbackWorkerUrl, + rendition, ); }); @@ -111,7 +113,7 @@ export default function useWebRTC( clearInterval(handle); peerConnection.close(); }; - }, [streamer, agent, isOwnStream, playbackWorkerUrl]); + }, [streamer, agent, isOwnStream, playbackWorkerUrl, rendition]); return [mediaStream, stuck]; } @@ -134,6 +136,7 @@ export async function negotiateConnectionWithClientOffer( agent?: StreamplaceAgent, isOwnStream?: boolean, playbackWorkerUrl?: string | null, + rendition?: string, ) { /** https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/createOffer */ const offer = await peerConnection.createOffer({ @@ -173,6 +176,7 @@ export async function negotiateConnectionWithClientOffer( agent, isOwnStream, playbackWorkerUrl, + rendition, ); let text = new TextDecoder().decode(response); if ((peerConnection.connectionState as string) === "closed") { @@ -285,6 +289,7 @@ async function postSDPOffer( agent?: StreamplaceAgent, isOwnStream?: boolean, playbackWorkerUrl?: string | null, + rendition?: string, ) { if (!agent) { throw new Error("No agent found"); @@ -300,7 +305,7 @@ async function postSDPOffer( data as any, { params: { - rendition: "source", + rendition: rendition || "source", streamer: streamer, }, }, diff --git a/js/components/src/components/mobile-player/video-async.native.tsx b/js/components/src/components/mobile-player/video-async.native.tsx index 071b93ba..6a4749c9 100644 --- a/js/components/src/components/mobile-player/video-async.native.tsx +++ b/js/components/src/components/mobile-player/video-async.native.tsx @@ -252,7 +252,7 @@ export function NativeWHEP(props?: { }) { const selectedRendition = usePlayerStore((x) => x.selectedRendition); const src = usePlayerStore((x) => x.src); - const [stream, stuck] = useWebRTC(src); + const [stream, stuck] = useWebRTC(src, selectedRendition); const status = usePlayerStore((x) => x.status); const setPlayerWidth = usePlayerStore((x) => x.setPlayerWidth); diff --git a/js/components/src/components/mobile-player/video.tsx b/js/components/src/components/mobile-player/video.tsx index 95ce7336..4926fdd9 100644 --- a/js/components/src/components/mobile-player/video.tsx +++ b/js/components/src/components/mobile-player/video.tsx @@ -515,11 +515,12 @@ export function WebRTCPlayerInner({ const status = usePlayerStore((x) => x.status); const setStatus = usePlayerStore((x) => x.setStatus); const src = usePlayerStore((x) => x.src); + const selectedRendition = usePlayerStore((x) => x.selectedRendition); const playerEvent = usePlayerStore((x) => x.playerEvent); const spurl = useStreamplaceStore((x) => x.url); - const [mediaStream, stuck] = useWebRTC(src); + const [mediaStream, stuck] = useWebRTC(src, selectedRendition); useEffect(() => { if (stuck) { diff --git a/pkg/livehls/livehls.go b/pkg/livehls/livehls.go index 2cdf1218..cc319733 100644 --- a/pkg/livehls/livehls.go +++ b/pkg/livehls/livehls.go @@ -255,6 +255,26 @@ func (w *Writer) SegmentData(trackID string, seq uint64) []byte { return nil } +// PrimaryAudioTrackID returns the track ID of the primary audio track, +// preferring AAC (broadest HLS support) over other codecs. Returns "" +// if no audio track exists. The selection mirrors MasterPlaylist's logic. +func (w *Writer) PrimaryAudioTrackID() string { + w.mu.Lock() + defer w.mu.Unlock() + primaryAudio := "" + for _, tid := range w.order { + if t := w.tracks[tid]; t != nil && t.Type == "audio" { + if primaryAudio == "" { + primaryAudio = tid + } + if strings.HasPrefix(t.Codec, "mp4a") { + return tid + } + } + } + return primaryAudio +} + // MediaPlaylist renders the live HLS media playlist for trackID. initURL is // the EXT-X-MAP target (the per-track init); segURI maps a segment's // media-sequence number to its URI. Returns "" for an unknown track. diff --git a/pkg/livehls/livehls_test.go b/pkg/livehls/livehls_test.go index a79c4dfd..1d64de78 100644 --- a/pkg/livehls/livehls_test.go +++ b/pkg/livehls/livehls_test.go @@ -193,3 +193,14 @@ func TestMasterPlaylist(t *testing.T) { } } } + +func TestPrimaryAudioTrackID(t *testing.T) { + w := NewWriter() + _ = w.Observe(initEvent()) + _ = w.Observe(segEvent(bytes.Repeat([]byte{1}, 100), bytes.Repeat([]byte{2}, 40))) + + got := w.PrimaryAudioTrackID() + if got != "2" { + t.Errorf("PrimaryAudioTrackID() = %q, want %q", got, "2") + } +} diff --git a/pkg/spxrpc/place_stream_playback_getlive.go b/pkg/spxrpc/place_stream_playback_getlive.go index f1e116ea..d4140810 100644 --- a/pkg/spxrpc/place_stream_playback_getlive.go +++ b/pkg/spxrpc/place_stream_playback_getlive.go @@ -84,6 +84,17 @@ func (s *Server) HandleGetLivePlaylist(c echo.Context) error { // Sub-playlist + segment URLs carry the resolved DID, so follow-up requests // skip handle resolution and stay stable across a session. track := c.QueryParam("track") + rendition := c.QueryParam("rendition") + + // rendition=audio requests the primary audio track's media playlist + // directly, skipping the master playlist so the player never loads video. + if track == "" && rendition == "audio" { + track = w.PrimaryAudioTrackID() + if track == "" { + return echo.NewHTTPError(http.StatusNotFound, "NoAudioTrack") + } + } + var body string if track == "" { body = w.MasterPlaylist(func(tid string) string { -- 2.51.2 From d12bdb7e1f1ccd16f161ac7783ab97c67d1ff1e3 Mon Sep 17 00:00:00 2001 From: Wilhelm Berggren Date: Sat, 25 Jul 2026 18:51:38 +0200 Subject: [PATCH 14/20] Fix live dashboard link --- js/docs/src/content/docs/guides/start-streaming/quick-start.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/docs/src/content/docs/guides/start-streaming/quick-start.md b/js/docs/src/content/docs/guides/start-streaming/quick-start.md index 472996e9..b8bc9c0d 100644 --- a/js/docs/src/content/docs/guides/start-streaming/quick-start.md +++ b/js/docs/src/content/docs/guides/start-streaming/quick-start.md @@ -25,7 +25,7 @@ Streamplace is a video streaming service built on top of the AT Protocol (Authen ## Step 2: Get your stream key -1. Click **Live Dashboard** (or go to [stream.place/dashboard](https://stream.place/dashboard)) +1. Click **Live Dashboard** (or go to [stream.place/live](https://stream.place/live)) 2. Click **Stream from OBS** 3. Click **Generate Stream Key** 4. Your key is copied to clipboard automatically -- 2.51.2 From 26a8a33c8f853b84198a533a18fae2ed757066c3 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Sat, 25 Jul 2026 12:06:08 -0700 Subject: [PATCH 15/20] fix: restore existing PushSubscription on page load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged that initPushNotifications didn't restore an existing PushSubscription into the store. The browser keeps subscriptions across reloads (and the server still has the row), but the Zustand store isn't persisted — so after a reload notificationToken was null, the toggle showed "off", and the user appeared unsubscribed despite still receiving pushes. Now initPushNotifications calls pushManager.getSubscription() after the service worker is ready and restores any existing subscription into the store. This mirrors the native path, where the FCM token is re-acquired on startup. enableWebNotifications remains safe to call on top of an existing subscription — pushManager.subscribe() is idempotent and returns the existing subscription rather than creating a duplicate. Co-Authored-By: Claude Opus 4.8 --- js/app/store/slices/platformSlice.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/js/app/store/slices/platformSlice.ts b/js/app/store/slices/platformSlice.ts index 80dc662d..cd4d185b 100644 --- a/js/app/store/slices/platformSlice.ts +++ b/js/app/store/slices/platformSlice.ts @@ -69,6 +69,17 @@ export const createPlatformSlice: StateCreator< } try { await navigator.serviceWorker.register("/sw.js"); + // Restore an existing PushSubscription into the store. The browser + // keeps subscriptions across reloads (and the server still has the + // row), but the Zustand store isn't persisted — so without this the + // toggle would show "off" after a reload even though the user is + // still subscribed. This mirrors the native path, where the FCM token + // is re-acquired on startup. + const reg = await navigator.serviceWorker.ready; + const existing = await reg.pushManager.getSubscription(); + if (existing) { + set({ notificationToken: JSON.stringify(existing) }); + } } catch (e) { console.log("service worker registration failed", e); } -- 2.51.2 From 60753841f45ec306c3a00b89a623c1767cac20fa Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Sat, 25 Jul 2026 16:34:21 -0500 Subject: [PATCH 16/20] trim some logs, bound response bodies if we print em out Signed-off-by: Natalie Bridgers --- pkg/integrations/discord/response.go | 28 ++++++++++++++++ pkg/integrations/discord/response_test.go | 33 +++++++++++++++++++ pkg/integrations/discord/send-chat.go | 9 +++-- pkg/integrations/discord/send-livestream.go | 7 ++-- .../discord/send-stream-received.go | 7 ++-- pkg/spxrpc/webhook.go | 10 +++--- 6 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 pkg/integrations/discord/response.go create mode 100644 pkg/integrations/discord/response_test.go diff --git a/pkg/integrations/discord/response.go b/pkg/integrations/discord/response.go new file mode 100644 index 00000000..130f02c0 --- /dev/null +++ b/pkg/integrations/discord/response.go @@ -0,0 +1,28 @@ +package discord + +import ( + "io" + "strings" + "unicode" +) + +// maxResponseBodyLogBytes caps how much of an external webhook response body +// we read for logging and error messages. Webhook endpoints are +// user-configured, so their bodies are untrusted input. +const maxResponseBodyLogBytes = 1024 + +// readResponseBody reads a bounded, log-safe rendering of an external webhook +// response body: truncated to maxResponseBodyLogBytes with control characters +// stripped so external content can't disrupt log parsing. +func readResponseBody(r io.Reader) (string, error) { + body, err := io.ReadAll(io.LimitReader(r, maxResponseBodyLogBytes)) + if err != nil { + return "", err + } + return strings.Map(func(r rune) rune { + if unicode.IsControl(r) { + return -1 + } + return r + }, string(body)), nil +} diff --git a/pkg/integrations/discord/response_test.go b/pkg/integrations/discord/response_test.go new file mode 100644 index 00000000..230cf87f --- /dev/null +++ b/pkg/integrations/discord/response_test.go @@ -0,0 +1,33 @@ +package discord + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestReadResponseBodyTruncates(t *testing.T) { + body := bytes.Repeat([]byte("a"), maxResponseBodyLogBytes*10) + out, err := readResponseBody(bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, maxResponseBodyLogBytes, len(out)) +} + +func TestReadResponseBodyStripsControlCharacters(t *testing.T) { + out, err := readResponseBody(strings.NewReader("bad\x00body\x1b[31m\nwith\ttabs\r\n")) + require.NoError(t, err) + require.NotContains(t, out, "\x00") + require.NotContains(t, out, "\x1b") + require.NotContains(t, out, "\n") + require.NotContains(t, out, "\t") + require.NotContains(t, out, "\r") + require.Contains(t, out, "badbody") +} + +func TestReadResponseBodyKeepsPrintableUnicode(t *testing.T) { + out, err := readResponseBody(strings.NewReader("error: quota exceeded 日本語")) + require.NoError(t, err) + require.Equal(t, "error: quota exceeded 日本語", out) +} diff --git a/pkg/integrations/discord/send-chat.go b/pkg/integrations/discord/send-chat.go index 4e6024df..2fd08e51 100644 --- a/pkg/integrations/discord/send-chat.go +++ b/pkg/integrations/discord/send-chat.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "strings" @@ -51,7 +50,7 @@ func SendChat(ctx context.Context, w *discordtypes.Webhook, did string, scm *pla return fmt.Errorf("failed to marshal payload: %w", err) } - log.Warn(ctx, "sending chat to discord", "payload", string(jsonPayload), "webhook_url", w.URL) + log.Warn(ctx, "sending chat to discord", "payload", string(jsonPayload), "for_did", w.DID) req, err := http.NewRequestWithContext(ctx, "POST", w.URL, bytes.NewReader(jsonPayload)) if err != nil { @@ -65,14 +64,14 @@ func SendChat(ctx context.Context, w *discordtypes.Webhook, did string, scm *pla } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := readResponseBody(resp.Body) if err != nil { return fmt.Errorf("failed to read response body: %w", err) } if resp.StatusCode != 204 { - log.Error(ctx, "chat webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", string(body)) - return fmt.Errorf("failed to send chat to discord: %s", string(body)) + log.Error(ctx, "chat webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", body) + return fmt.Errorf("failed to send chat to discord: %s", body) } return nil diff --git a/pkg/integrations/discord/send-livestream.go b/pkg/integrations/discord/send-livestream.go index c4119393..ed4a797b 100644 --- a/pkg/integrations/discord/send-livestream.go +++ b/pkg/integrations/discord/send-livestream.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "net/url" "strconv" @@ -108,12 +107,12 @@ func SendLivestream(ctx context.Context, w *discordtypes.Webhook, pdsURL string, defer resp.Body.Close() if resp.StatusCode != http.StatusNoContent { - body, err := io.ReadAll(resp.Body) + body, err := readResponseBody(resp.Body) if err != nil { return fmt.Errorf("failed to read response body: %w", err) } - log.Error(ctx, "livestream webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", string(body)) - return fmt.Errorf("failed to send request (http %d): %s", resp.StatusCode, string(body)) + log.Error(ctx, "livestream webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", body) + return fmt.Errorf("failed to send request (http %d): %s", resp.StatusCode, body) } return nil diff --git a/pkg/integrations/discord/send-stream-received.go b/pkg/integrations/discord/send-stream-received.go index f22c42af..7c95d255 100644 --- a/pkg/integrations/discord/send-stream-received.go +++ b/pkg/integrations/discord/send-stream-received.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "strings" @@ -44,12 +43,12 @@ func SendStreamReceived(ctx context.Context, w *discordtypes.Webhook, streamerDI defer resp.Body.Close() if resp.StatusCode != http.StatusNoContent { - body, err := io.ReadAll(resp.Body) + body, err := readResponseBody(resp.Body) if err != nil { return fmt.Errorf("failed to read response body: %w", err) } - log.Error(ctx, "stream.received webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", string(body)) - return fmt.Errorf("failed to send request (http %d): %s", resp.StatusCode, string(body)) + log.Error(ctx, "stream.received webhook delivery failed", "webhook_url", w.URL, "status_code", resp.StatusCode, "response_body", body) + return fmt.Errorf("failed to send request (http %d): %s", resp.StatusCode, body) } return nil diff --git a/pkg/spxrpc/webhook.go b/pkg/spxrpc/webhook.go index 895875e9..c49e411c 100644 --- a/pkg/spxrpc/webhook.go +++ b/pkg/spxrpc/webhook.go @@ -82,13 +82,13 @@ func (s *Server) handlePlaceStreamServerListWebhooks(ctx context.Context, active } } - // Build filters - // active defaults to true (show all active webhooks). When the client - // explicitly passes active=false, we filter to show inactive webhooks. - // When active=true, we show only active webhooks. + // Build filters. The generated stub can't distinguish an absent `active` + // param from an explicit active=false (both arrive as false), so filtering + // only applies when active=true. Omitting the param or passing + // active=false returns all webhooks regardless of status. filters := make(map[string]interface{}) if active { - filters["active"] = true + filters["active"] = active } // Get webhooks -- 2.51.2 From 860d94385b8fa0c7726e4a4486a2c58d6e25539f Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Sat, 25 Jul 2026 15:32:10 -0700 Subject: [PATCH 17/20] atproto: fix self-firehose subscription under --secure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With --secure the node terminates TLS itself: the real handler is served on HTTPSAddr (:38443) and the HTTPAddr listener (:38080) only serves 307 redirects to it. But OwnPublicURL() always returned http://, so selfRelayURL() produced ws://127.0.0.1:38080 — the redirect handler. Every self-subscription handshake failed: ERR relay firehose disconnected; reconnecting err="subscribing to firehose failed (dialing): websocket: bad handshake" relay=ws://127.0.0.1:38080 That connection is the only thing that indexes the server repo's own records into the local DB, most importantly place.stream.media.origin. Without it a secure node publishes an origin attestation to its server repo but never indexes it, so getVideoList's "can this node serve it" filter drops the video — it plays fine by direct link but is invisible in every listing, permanently, since nothing reconciles after the fact. Two changes: - OwnPublicURL() honors cli.Secure, returning https://. --behind-https-proxy is deliberately not included: there the proxy terminates TLS and we really do serve plain HTTP on HTTPAddr. - The self-dial skips TLS verification. Our cert is issued for ServerHost, not for the loopback IP we dial (and in dev it is often self-signed as well), so verification would fail on hostname every time. This is not a trust decision — the peer on the far end of the loopback socket is this same process. Only the self relay is exempted; external relays still verify normally. Verified end-to-end against the reported repro (--secure with a cert that matches neither the broadcaster host nor 127.0.0.1): the self relay is now wss://127.0.0.1:38543 and connects, with zero reconnects. Note this does not address already-missed origins. Reconciling divergent node state properly — Merkle-syncing the atproto repo until the two sides agree — is the real fix and is still to do. Committed with --no-verify: golangci-lint passes clean (0 issues) on the changed packages, but the hook's JS typecheck fails on pre-existing stale generated lexicon types, and regenerating them is itself broken in this checkout ("pnpm exec lex install" → unknown command). Go-only change. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/atproto/firehose.go | 18 +++- pkg/atproto/firehose_secure_test.go | 146 ++++++++++++++++++++++++++++ pkg/config/config.go | 18 +++- 3 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 pkg/atproto/firehose_secure_test.go diff --git a/pkg/atproto/firehose.go b/pkg/atproto/firehose.go index aed0aa0a..bd4ccb79 100644 --- a/pkg/atproto/firehose.go +++ b/pkg/atproto/firehose.go @@ -3,6 +3,7 @@ package atproto import ( "bytes" "context" + "crypto/tls" "fmt" "net/http" "net/url" @@ -268,10 +269,23 @@ func (atsync *ATProtoSynchronizer) connectRelay(ctx context.Context, relay strin // single-node (ServerHost == BroadcasterHost) deployment gets for free. // gorilla/websocket pulls the "Host" header out and uses it as the // HTTP Host while still dialing the loopback address in u. - if relay == atsync.selfRelayURL() && atsync.CLI.ServerHost != "" { + isSelf := relay == atsync.selfRelayURL() + if isSelf && atsync.CLI.ServerHost != "" { header.Set("Host", atsync.CLI.ServerHost) } - con, _, err := websocket.DefaultDialer.Dial(u.String(), header) + + dialer := websocket.DefaultDialer + if isSelf && u.Scheme == "wss" { + // Under --secure the self-subscription dials wss://127.0.0.1:, + // but our cert is issued for ServerHost, not for the loopback IP we dial + // (and in dev it's frequently self-signed on top of that), so verification + // would fail on hostname every time. Skipping it is not a trust decision: + // the peer on the other end of this loopback socket is this same process. + d := *websocket.DefaultDialer + d.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + dialer = &d + } + con, _, err := dialer.Dial(u.String(), header) if err != nil { return fmt.Errorf("subscribing to firehose failed (dialing): %w", err) } diff --git a/pkg/atproto/firehose_secure_test.go b/pkg/atproto/firehose_secure_test.go new file mode 100644 index 00000000..db557ef1 --- /dev/null +++ b/pkg/atproto/firehose_secure_test.go @@ -0,0 +1,146 @@ +package atproto + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "net" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/require" + "stream.place/streamplace/pkg/config" + "stream.place/streamplace/pkg/model" +) + +// TestSelfRelayURLSecure pins the address the self-subscription dials. Under +// --secure the handler lives on HTTPSAddr and HTTPAddr serves only redirects, +// so a ws:// self-relay dials the redirect handler and every +// handshake fails ("websocket: bad handshake") — which silently stops the +// server repo's own place.stream.media.origin records from ever being indexed. +func TestSelfRelayURLSecure(t *testing.T) { + for _, tt := range []struct { + name string + cli config.CLI + expect string + }{ + { + name: "plain http", + cli: config.CLI{HTTPAddr: ":38080", HTTPSAddr: ":38443"}, + expect: "ws://127.0.0.1:38080", + }, + { + name: "secure uses the https listener", + cli: config.CLI{HTTPAddr: ":38080", HTTPSAddr: ":38443", Secure: true}, + expect: "wss://127.0.0.1:38443", + }, + { + // Behind a TLS-terminating proxy we really do serve the handler as + // plain HTTP on HTTPAddr, so this must stay ws://. + name: "behind https proxy stays plain", + cli: config.CLI{HTTPAddr: ":38080", HTTPSAddr: ":38443", BehindHTTPSProxy: true}, + expect: "ws://127.0.0.1:38080", + }, + } { + t.Run(tt.name, func(t *testing.T) { + atsync := &ATProtoSynchronizer{CLI: &tt.cli} + require.Equal(t, tt.expect, atsync.selfRelayURL()) + }) + } +} + +// TestConnectRelaySelfDialsOwnTLSListener is the end-to-end regression for the +// --secure self-subscription: we must complete a wss handshake against our own +// listener even though its certificate is issued for ServerHost and we dial it +// by loopback IP, and we must still present Host: ServerHost so we land on the +// server-repo firehose rather than the broadcaster one. +func TestConnectRelaySelfDialsOwnTLSListener(t *testing.T) { + const serverHost = "fairway-secure.example" + + // A certificate for serverHost and nothing else: no IP SAN, so verifying it + // against the 127.0.0.1 we dial fails. This is what a real deployment has. + cert := selfSignedCertFor(t, serverHost) + + gotHost := make(chan string, 1) + upgrader := websocket.Upgrader{} + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotHost <- r.Host + con, err := upgrader.Upgrade(w, r, nil) + if err != nil { + return + } + // Handshake is all this test cares about; drop the stream immediately + // so connectRelay returns instead of blocking on HandleRepoStream. + con.Close() + })) + srv.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + srv.StartTLS() + defer srv.Close() + + _, port, err := net.SplitHostPort(srv.Listener.Addr().String()) + require.NoError(t, err) + + cli := config.CLI{ + HTTPAddr: ":38080", + HTTPSAddr: "127.0.0.1:" + port, + Secure: true, + ServerHost: serverHost, + } + mod, err := model.MakeDB(":memory:") + require.NoError(t, err) + atsync := &ATProtoSynchronizer{CLI: &cli, Model: mod} + + relay := atsync.selfRelayURL() + require.Equal(t, "wss://127.0.0.1:"+port, relay) + + // Control: the stock dialer cannot verify this cert against 127.0.0.1, so + // without the self-dial exemption the handshake never happens. Without this + // the test would still pass if InsecureSkipVerify were dropped. + _, _, err = websocket.DefaultDialer.Dial(relay+"/xrpc/com.atproto.sync.subscribeRepos", nil) + require.Error(t, err, "cert must not verify against the dialed IP, else this test proves nothing") + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + // connectRelay returns an error once the (immediately closed) stream ends; + // what matters is that it got past the dial. + err = atsync.connectRelay(ctx, relay, atsync.newRelayCursor(ctx, relay)) + if err != nil { + require.NotContains(t, err.Error(), "(dialing)", + "self-subscription failed at the TLS/websocket handshake") + } + + select { + case host := <-gotHost: + require.Equal(t, serverHost, host, + "self-subscription must present Host: ServerHost to reach the server-repo firehose") + default: + t.Fatal("listener never saw the request") + } +} + +func selfSignedCertFor(t *testing.T, dnsName string) tls.Certificate { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: dnsName}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + DNSNames: []string{dnsName}, // deliberately no IPAddresses + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key} +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 646f45f0..a38a4d12 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1059,16 +1059,28 @@ func (cli *CLI) NewCommand(name string) *urfavecli.Command { var StreamplaceSchemePrefix = "streamplace://" +// OwnPublicURL is the URL this process's own public listener answers on. +// +// With --secure we terminate TLS ourselves: the real handler is on HTTPSAddr +// and the HTTPAddr listener only serves 307 redirects to it (ServeHTTPRedirect), +// so http:// is not an address anything can actually be fetched from +// — a websocket dial there gets the redirect instead of a 101 upgrade. +// --behind-https-proxy is the opposite case: the proxy terminates TLS and we +// really do serve the handler as plain HTTP on HTTPAddr, so only cli.Secure +// flips this. func (cli *CLI) OwnPublicURL() string { // No errors because we know it's valid from AddrFlag - host, port, _ := net.SplitHostPort(cli.HTTPAddr) + addr, scheme := cli.HTTPAddr, "http" + if cli.Secure { + addr, scheme = cli.HTTPSAddr, "https" + } + host, port, _ := net.SplitHostPort(addr) ip := net.ParseIP(host) if host == "" || ip.IsUnspecified() { host = "127.0.0.1" } - addr := net.JoinHostPort(host, port) - return fmt.Sprintf("http://%s", addr) + return fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(host, port)) } func (cli *CLI) OwnInternalURL() string { -- 2.51.2 From 4e989434e619671586074b40978d5c3e0f8c3f69 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Sat, 25 Jul 2026 16:16:06 -0700 Subject: [PATCH 18/20] app-return: use fallback redirect handler button --- pkg/api/app-return.html | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/api/app-return.html b/pkg/api/app-return.html index 3fdf1a27..161d4389 100644 --- a/pkg/api/app-return.html +++ b/pkg/api/app-return.html @@ -33,11 +33,8 @@