[wagenet]

Opened by Claude on behalf of @wagenet.

Problem

Deactivate uploads every file under Strom's recordings/<productionId>/ and writes one RecordingDoc per object with startedAt: doc.updatedAt. Segments that earlier sessions left on Strom (a failed upload, or a session recorded before object storage was configured) are swept by the next deactivate that has storage. They all get that session's start time, so a segment recorded in January can be listed as starting in March. After the upload they are deleted from Strom (#366), so the original date is lost.

Fix

Strom's recorder names segments {filename_prefix}_{YYYYmmdd_HHMMSS}_%05d.{ext} (backend/src/blocks/builtin/recorder.rs), stamped when the recorder block is built. recordingStartFromFileName reads that stamp, uploadRecordings returns it per segment, and deactivate uses it for startedAt. If a name has no stamp, deactivate falls back to doc.updatedAt as before.

Strom formats the stamp with chrono::Local and writes no offset. open-live reads it as UTC, which matches Strom's container image (no TZ is set). On a Strom host set to another time zone, startedAt is off by that offset. Before this change it could be off by weeks.

endedAt is still the upload time. That matches the RecordingDoc field comment ("finalized/uploaded") and is out of scope here.

Docs

• docs/specs/vod-recording-minio.md gains a "Segments left on Strom" section. It says the first deactivate with storage uploads the backlog, and where startedAt comes from.
• The README's activation-flow step for deactivate links to it.

Overlap with #359

#359 moves new recordings into per-activation directories named after the activation start. That covers segments recorded after it lands. Its shared-directory path, for files recorded before per-activation directories, still falls back to doc.updatedAt. This PR is based on main because main has the bug in full. If #359 merges first, the rebase should become seg.activationStartedAt ?? seg.startedAt ?? doc.updatedAt.

Tests

• src/__tests__/recording-leftover-sweep.test.ts drives the real deactivate route with segments from three sessions plus one name without a stamp. It checks each RecordingDoc.startedAt. With the route change reverted, it fails and every doc gets 2026-03-01T10:00:00.000Z.
• recording-wiring.test.ts gains unit tests for the parser, including rejection of out-of-range dates.
• pnpm typecheck and pnpm vitest run pass: 74 files, 768 tests.

🤖 Generated with Claude Code

[birme]

Reviewed by the daily-backlog-pr housekeeping pass (Phase 3.5). Verified against main: uploadRecordings already sweeps every file under recordings/<productionId>/ via strom.media.list, so backlog segments from earlier sessions are real (no dependency on #359). New recordingStartFromFileName parses Strom's {prefix}_{YYYYmmdd_HHMMSS}_%05d name and validates via a Date.UTC round-trip (rejects month 13 / Feb 30, per tests); startedAt is threaded through UploadedSegment and used as seg.startedAt ?? doc.updatedAt, dating backlog segments by their own session while legacy names fall back to this activation. Good unit + integration coverage. No fabricated open-live APIs.

Note: edits the same deactivate recording block as #405 — textual conflict; whichever merges second must resolve it. Branch is behind main after #406 merged; it will be refreshed server-side, then merged on a subsequent pass.

[birme]

This PR is part of a multi-PR recording-feature cluster that all touch the same deactivate/upload path and reference each other's merge/rebase order:

• fix(recording): wire PGM video and AAC audio into the recorder #358 (wire PGM video + AAC audio into the recorder — base)
• fix(recording): upload each recording once, including failed and orphaned sessions #359 (upload each recording once — "Builds on fix(recording): wire PGM video and AAC audio into the recorder #358, merge after fix(recording): wire PGM video and AAC audio into the recorder #358")
• feat(recording): record to Strom's disk without object storage #404 (record to Strom's disk without object storage — overlaps the deactivate block)
• fix(recording): date swept segments by their own session's start #407 (this PR — "Overlap with fix(recording): upload each recording once, including failed and orphaned sessions #359 … if fix(recording): upload each recording once, including failed and orphaned sessions #359 merges first, the rebase should become …")

Holding off on automated review/merge here — this cluster needs a human (or the relevant dev agent) to assess the whole set and fix a merge order, not a mechanical per-PR pass. Nothing further is needed from the author; this comment records the dependency on the PR itself.

[birme]

VERDICT: APPROVE

Reviewed as pr-reviewer (read-only; not the author), correctness-focused, with the extra guesswork/fabrication scrutiny the task calls for. The whole fix hinges on one external contract — the shape of the segment filename Strom's recorder writes — so I verified that against the authoritative source rather than trusting the PR description.

The contract — verified against authoritative source (this was the crux)

The PR claims Strom names segments {filename_prefix}_{YYYYmmdd_HHMMSS}_%05d.{ext}. This contradicts three pre-existing descriptions in open-live itself which all say {prefix}_%05d.{ext} with no datestamp:

• src/lib/recording-uploader.ts:5 ("confirmed by the PM on issue #41, 2026-09-15")
• src/config.ts:220
• src/lib/flow-generator.ts:884

That contradiction mattered: open-live sets the recorder prefix property to production._id (flow-generator.ts:893). If Strom produced prod-xyz_00000.mp4 (no stamp), recordingStartFromFileName would never match, always return undefined, and every segment would fall back to doc.updatedAt — i.e. the PR would be a silent no-op in production while its (mock-fed) tests stayed green.

I checked Eyevinn/strom (public) backend/src/blocks/builtin/recorder.rs:

• L637 let timestamp = chrono::Local::now().format("%Y%m%d_%H%M%S");
• L638-648 format!("{}/{}_{}_%05d.{}", output_dir, filename_prefix, timestamp, file_ext)

So the actual name is {filename_prefix}_{YYYYmmdd_HHMMSS}_%05d.{ext} — the PR's claim is correct and the three sibling comments (plus recorder.rs's own stale module doc at L33) are the outdated ones. chrono::Local::now() is evaluated once at recorder-block build time, so every segment of a session shares the stamp, and the stamp carries no offset — exactly matching the PR's UTC-read note. Not fabricated.

Verified correct

• RecordingDoc shape (src/db/types.ts:347-358): startedAt: string exists; only the comment changed. No fabricated field.
• UploadedSegment.startedAt? added (recording-uploader.ts:61); populated only when parseable (...(startedAt ? {startedAt} : {})), so the field stays optional and typecheck holds.
• Parser (recording-uploader.ts:297): end-anchored regex matches _YYYYMMDD_HHMMSS_<seq>.<ext>, immune to digits in the prefix; Date.UTC + round-trip field re-check rejects rolled-over values (month 13, Feb 30, etc.). Verified by the unit tests. No catastrophic-backtracking risk (fixed-width groups).
• Dating logic (routes/productions.ts): startedAt: seg.startedAt ?? doc.updatedAt. Segment with no derivable session → undefined → clean fallback, no crash, no mis-attribution. This is strictly better than the prior all-doc.updatedAt behavior and preserves it exactly for unparseable names.
• Sweep (uploadRecordings) lists recordings/<productionId>/, filters !is_dir, keys as ${prefix}${productionId}/${entry.name} — unchanged by this PR.

Fabrication / security findings

• None. No fabricated field or contract (the one high-risk claim was verified against Strom source). Test credentials are obviously fake (AKIAEXAMPLE / secretexamplekey) — no real secrets. Regex is linear/bounded.

Merge conflict with main

GitHub reports MERGEABLE / BEHIND. A local git merge --no-commit --no-ff origin/main into the PR head auto-merged cleanly (0 conflicted files) even though main has since changed src/routes/productions.ts (#418/#419/#421). So BEHIND is just out-of-date, no real conflict in the recording-sweep/deactivate path. Rebasing on current main is safe.

Checks

• gh pr checks 407: build-and-test pass.
• Local: pnpm install --frozen-lockfile ok; pnpm typecheck pass; full vitest run 782/782 pass (75 files); targeted recording tests 14/14.

Non-blocking suggestions (follow-up, not required to merge)

1. The three sibling comments that still say {prefix}_%05d.{ext} (recording-uploader.ts:5, config.ts:220, flow-generator.ts:884) are now contradicted by the verified format — worth updating so the next reader doesn't conclude this fix is dead code (as it first appeared).
2. The fallback comment "doc.updatedAt is this activation's start" is slightly imprecise if the production doc is written mid-session (updatedAt would then be later than activation start); behavior is unchanged from before, so this is cosmetic only.

🤖 Generated with Claude Code

[wagenet]

Claude (Claude Code), for @wagenet.

Landing order agreed with @wagenet: this PR and #404 merge before #359, unchanged. #359 then rebases onto main and takes on the conflicts. Nothing needs to change on this branch.

Checked by merging this branch (e4f0e3f) and #404 (7b97ec3) into #359 (0016343) locally:

• Under fix(recording): upload each recording once, including failed and orphaned sessions #359 each activation records into its own directory, and a segment is dated by that directory's start (open-live's UTC clock), with endedAt from Strom's last write. The resolution keeps both PRs' dates: startedAt: seg.activationStartedAt ?? seg.startedAt ?? doc.updatedAt and endedAt: seg.modifiedAt ?? finalizedAt. This PR's file-name start then covers files directly in recordings/<productionId>/, which fix(recording): upload each recording once, including failed and orphaned sessions #359 sweeps for a production activated before it.
• recording-leftover-sweep.test.ts needs is_directory entries and a recordings-db get mock under fix(recording): upload each recording once, including failed and orphaned sessions #359. fix(recording): upload each recording once, including failed and orphaned sessions #359 will also update this PR's "Segments left on Strom" spec section and the RecordingDoc comments to match.

With those, pnpm typecheck and pnpm test (1079 tests) pass on the three-way merge.

[birme]

This PR is part of wagenet's multi-PR recording feature line on open-live. Holding off on automated review/merge — this needs a human (or the relevant dev agent) to assess the whole set together, not a per-PR pass, because these touch the same recorder / deactivate-upload code and are merge-order-sensitive (e.g. #358 and #359 conflict in one test hunk).

Members, in dependency order:

• fix(recording): wire PGM video and AAC audio into the recorder #358 — fix recorder video pad + AAC audio wiring (base fix)
• fix(recording): upload each recording once, including failed and orphaned sessions #359 — upload each recording once (base fix; merges independently of fix(recording): wire PGM video and AAC audio into the recorder #358 but conflicts with it in a test hunk)
• feat(recording): record inputs chosen per source assignment #411 — record inputs chosen per source (Closes Record arriving inputs, chosen per source #410; stacked on fix(recording): wire PGM video and AAC audio into the recorder #358 + fix(recording): upload each recording once, including failed and orphaned sessions #359; draft)
• feat(recording): record through Strom's Live Recorder #489 — record through Strom's Live Recorder (stacked on feat(recording): record inputs chosen per source assignment #411; draft)
• feat(recording): record to Strom's disk without object storage #404 — record to Strom's disk without object storage (independent recording fix, same upload area)
• fix(recording): date swept segments by their own session's start #407 — date swept segments by their own session's start (independent recording fix, same upload area; already APPROVED)

Nothing further is needed from the author; this comment records the dependency on the PRs themselves rather than in a run log.