fix(recording): upload each recording once, including failed and orphaned sessions by wagenet · Pull Request #359 · Eyevinn/open-live

wagenet · 2026-09-28T05:03:29Z

Opened by Claude (Claude Code) on behalf of @wagenet.

Based on main; merges independently of #358. Merging #358 afterwards conflicts only in one hunk of src/__tests__/recording-wiring.test.ts (the pad name and filename_prefix assertions next to this PR's output_dir assertions); a scratch merge resolved it and passed typecheck and 1097 tests.

Deactivate uploads a production's recordings, then deletes them from Strom (#378). A file left behind, because its delete failed or was skipped when the production changed during the upload, was uploaded and registered again at the next deactivate, so the recordings list got duplicates. With this change each recording is uploaded and listed once. A session whose upload failed, or that ended without an explicit deactivate (idle timeout, lost flow), is uploaded at the production's next deactivate and dated by its own activation.

Live check against Strom upstream/main b28be77b with #358 underneath (it supplies the recorder's AAC audio), run before the rebase onto #378 and before this branch moved onto main, not repeated since. Two activations of one production, each deactivated through the real routes, gave 3 files, 3 uploads and 3 RecordingDocs with no duplicates, all H.264 High + AAC-LC 48 kHz stereo.

How to review

Five commits, in order:

2de79b6 fix(recording): upload only the ending activation's recordings
eae26c9 fix(recording): sweep every activation's recordings at deactivate
38cc1d9 fix(recording): stop the production sweep at the first auth rejection
d8ffaf2 fix(recording): upload a registered file again if its size changed
7d5f32a fix(recording): keep the file's end time when refreshing a registered recording

Cause

The recorder's output_dir was recordings/<production id> for every activation. Deactivate uploaded everything in it, nothing deleted the files, and each upload inserted a RecordingDoc with a fresh random id.

Fix

One directory per activation: recordings/<production id>/<activation start>-<uuid>, saved on the production as recorderOutputDir.
Deactivate sweeps every activation directory and skips files whose RecordingDoc exists. The id is now derived from bucket and key, so the check is one get, and concurrent deactivates conflict instead of duplicating. startedAt comes from the directory name, endedAt from the file's last write. No directory on Strom (404) means nothing to upload.
Legacy productions (recorder but no recorderOutputDir) also upload files directly in recordings/<production id>. Other productions skip them, because older sessions there are registered under random ids and would be registered again.
Stale recorder fields: the idle watchdog, startup reconcile and activation-failure reset now clear recorderBlockId and recorderOutputDir, and activation always writes both. Before, a later activation without a recorder kept the old block id, and its deactivate split it and registered the old session with the wrong start time.
MediaEntry matches Strom (is_directory, modified in epoch seconds). The uploader was checking is_dir, so it tried to download directories as files.
A registered file is skipped only if its size matches Strom's. A file registered while the recorder was still writing it left a truncated object; if its delete then failed, the next sweep would have skipped the complete file and deleted it. A size mismatch now uploads it again and updates the RecordingDoc's size and end time.
Deleting from Strom (feat(recording): delete recordings from Strom after successful upload #378) covers the whole sweep. When the isStillRecording guard passes, deactivate deletes every swept file that is in object storage, including files skipped as already registered, so a file whose delete failed is removed at the next deactivate. Each activation directory that ends up empty is removed.
An object-store auth rejection stops the whole sweep. fix(recording): fail fast on object-store auth errors in deactivate upload sweep #473 stops uploadFiles at the first S3AuthError. The production sweep now also skips the remaining activation directories and the legacy directory, so a rejected store costs one download, not one per directory. Files already in object storage are still deleted from Strom, as on main, and deactivate still logs the abort.

S3 keys are unchanged, so listing and playback need no changes.

Live check

Strom upstream/main b28be77b (0.6.10, has builtin.audioenc). A scratch vitest (not committed) drove the real POST /activate and POST /deactivate routes, the real flow generator and StromClient, with a __test1__ source and a recording output. CouchDB was in memory. MinIO was stubbed: signed PUTs (SigV4 Authorization present) were captured and their bodies written to disk.

Deactivate	Uploaded	`RecordingDoc`s inserted
1	activation 1 `…_100511_00000.mp4`	1
2	activation 1 `…_100511_00001.mp4`, activation 2 `…_100522_00000.mp4`	2, the first dated by activation 1

Activation 1's _00001 is the segment the split opens. The split is asynchronous and lands after deactivate 1 lists the directory, so deactivate 1 misses it. The sweep picks it up at deactivate 2. It holds the frames between the split and the flow stop. Nothing was uploaded twice.

Reviewer notes

With recording storage configured, every deactivate lists the production's recordings directory on Strom, even without a recorder this activation, plus one CouchDB get per file.
The watchdog and reconcile still don't upload; their sessions go up at the next explicit deactivate. The same applies to the tail segment after each deactivate's split (see Live check).
Deactivate uploads and deletes before it stops the flow, as on main. If the split's new tail segment were already on disk when the sweep lists the directory, the sweep would upload it part-written and then delete it while the recorder is still writing it. In the live run the split landed after the listing.
Files are deleted after upload and before their RecordingDoc is written, as in feat(recording): delete recordings from Strom after successful upload #378. If that write fails, the object is in the bucket but not listed, and the file is gone from Strom.
Known limits, none of them present on main:
- Object keys and RecordingDoc ids ignore the activation directory. Two activations holding a same-named file (Strom names files by wall-clock second, so this needs two recorder starts in one second, such as a DST fall-back) collide, and the later file's bytes can be lost.
- A RecordingDoc is trusted without checking the object is still in the bucket. If the object was wiped, the Strom copy is deleted without re-upload.
- On a Strom whose media folder has no recordings folder yet, it answers 400 rather than 404 to the production listing, so every deactivate logs an upload error. Deactivate still succeeds.
- If the RecordingDoc update after a re-upload fails, the listing keeps the old size although the object is complete; it is corrected at the next sweep only if the Strom file is still there.
Each uploaded MP4 carries about 95 MB of free boxes that the recorder's muxer reserves, whatever the recording's length. The uploader reads each file fully into memory.

Testing

recording-reupload.test.ts (new) drives the real deactivate and activate routes against a stateful fake CouchDB and Strom media store. production-lifecycle.test.ts checks that the watchdog and reconcile clear the fields. recording-wiring.test.ts asserts the per-activation output_dir and that a second activation gets a different one.
Each commit: pnpm typecheck passes; pnpm test passes 1077, 1085, 1086, 1088 and 1088 tests.
Reverts on this branch, each failing at least one test:

Reverted	Failing
flow generator's per-activation `output_dir`	`recording-wiring`
sweep skips already-registered files	3 in `recording-reupload` (skip registered, legacy directory, delete registered)
auth abort: break between activation directories	1 in `recording-reupload`
auth abort: skip of the legacy directory	the same test
watchdog and reconcile clearing the recorder fields	2 in `production-lifecycle`
size comparison, or the `RecordingDoc` update, for a file that grew	1 in `recording-reupload` each

The 404 handling is the exception: reverting it only adds an error log.

feat(recording): delete recordings from Strom after successful upload #378's uploadRecordings tests still pass. The meeting rig was not run on this branch.

🤖 Generated with Claude Code

birme · 2026-09-28T08:55:09Z

This PR looks like the top of a two-PR dependency stack (depends on #358):

fix(recording): wire PGM video and AAC audio into the recorder #358 — wire PGM video + AAC audio into the recorder (base; the recorder writes no files before it)
fix(recording): upload each recording once, including failed and orphaned sessions #359 (this PR) — upload each recording once, including failed/orphaned sessions

Holding off on automated review/merge here — this needs a human (or the relevant dev agent) to assess the whole stack together, not a per-PR pass, and both PRs are drafts. Nothing further is needed from the author; this comment exists so the dependency is recorded on the PR rather than in a run log.

birme · 2026-09-29T08:52:44Z

Heads up: main has moved (several recording/ws fixes merged), and this branch now reports a real
merge conflict with main (mergeable: CONFLICTING) — not mere staleness, so an automated
branch refresh won't resolve it. Please merge current main into this branch (or rebase) and
resolve the conflicting files, then push. This PR is otherwise still held for holistic review as
part of its dependency stack (see the earlier stack note); this comment only records the new
conflict so it isn't lost.

Resolutions: - productions.ts: keep Eyevinn#359's recording-doc helpers; main moved the guest invite sweep to services/guest-sweep.ts. Activation sets activationSucceeded before main's controller re-init. - production-lifecycle.test.ts: keep both sets of tests. - flow-generator-encoders.test.ts: Strom mock lists builtin.audioenc for Eyevinn#358's block check, and the recorder is fed on video_in_0 (Eyevinn#358). - record-inputs-lifecycle.test.ts: mock main's reinitConnectedControllers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Brings in main through Eyevinn#359. productions.ts: activation sets activationSucceeded before main's controller re-init. flow-generator-encoders.test.ts: the Strom mock lists builtin.audioenc for the block check. record-inputs-lifecycle.test.ts: mock main's reinitConnectedControllers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

wagenet · 2026-10-07T14:03:55Z