Skip to content

feat(recording): add DVR scheduler and recording library - #1190

Open
SalemOurabi wants to merge 13 commits into
4gray:masterfrom
SalemOurabi:dvr-recording-library
Open

feat(recording): add DVR scheduler and recording library#1190
SalemOurabi wants to merge 13 commits into
4gray:masterfrom
SalemOurabi:dvr-recording-library

Conversation

@SalemOurabi

Copy link
Copy Markdown
Contributor

What changed

  • add an Electron-only DVR data model with persisted recording lifecycle states
  • add scheduler recovery, cancellation, shutdown handling, and serialized playlist-deletion coordination
  • record M3U and Xtream live streams, plus the current Stalker program
  • use hidden embedded-MPV recording sessions with a hardened headless-VLC fallback
  • add the routed recording library with All, Upcoming, and Library filters
  • add EPG recording actions across M3U, Xtream, and Stalker live views
  • integrate the expandable workspace sidebar and localized Recordings navigation
  • add database migration, IPC, renderer, scheduler, recording-engine, UI, and Electron E2E regression coverage

Security and behavior

  • public renderer DTOs omit stream URLs, request headers, recording directories, and absolute file paths
  • VLC credentials are passed through transient mode-0600 M3U control files, not process arguments
  • VLC preflight rejects unsupported protected-header combinations before scheduling
  • recording-library actions use per-record locks and localized retry states
  • deleting a library entry removes metadata without deleting recorded media
  • IPTVnator must remain running for the recording window; schedules recover after restart, but there is no OS background service

Validation

  • full workspace lint passed for all 42 projects
  • frontend/Electron production build and TypeScript checks passed
  • targeted DVR, database, scheduler, VLC, EPG, routing, workspace-shell, and recording-library unit suites passed
  • EPG backend regression suite passed (14/14)
  • broad backend run reached 58/61 suites and 594/599 tests; the remaining external-player suites require loopback permissions unavailable in the sandbox
  • Electron recording-migration E2E coverage was added; local execution was blocked by the sandbox mock-server bind restriction
  • native embedded-MPV build was skipped because the vendored darwin-x64 runtime is not present
  • git diff --check passed

Documentation

Updated README.md, CLAUDE.md, and the DVR, embedded-MPV, Nx-boundary, SQLite-worker, and workspace-shell architecture documentation.

Dependency

This branch contains the expandable sidebar integration needed by the Recordings route and therefore overlaps with #1185. Rebase the DVR branch after #1185 merges to drop the duplicate sidebar commit before merging this PR.

Copy link
Copy Markdown
Contributor Author

@greptileai Please review this pull request thoroughly, with particular attention to DVR scheduling and recovery, SQLite migration and worker ordering, IPC data exposure, recording-engine lifecycle and cancellation, VLC credential/header handling, playlist-deletion races, and recording-library UI regressions.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a complete Electron-only DVR feature: a recording scheduler with lifecycle persistence, dual engine support (embedded MPV primary, headless VLC fallback), EPG-triggered recording actions across M3U/Xtream/Stalker views, a routed recording library UI, database migration, and IPC/preload wiring.

All four bugs flagged in the previous review pass have been addressed:

  • cancelForPlaylist and cancelAllActive both now guard against terminal-state races using recordingBecameTerminal before throwing.
  • settleWithin now resolves undefined on both timeout and rejection, giving consistent best-effort shutdown semantics.
  • toPublicRecordingItem migrated from synchronous existsSync to async fs/promises.access, eliminating the per-record main-process block.
  • The VLC session retention on failed SIGKILL is documented in code as intentional, enabling scheduler-level retry via hasActiveSession.

The overall architecture — exclusive-operation queues per recording ID, serialized playlist-delete coordination via the scheduling gate, hardware power-save blocker management, and startup recovery — holds up under detailed tracing of concurrent paths.

Confidence Score: 5/5

Safe to merge after the sidebar overlap with #1185 is resolved — the core DVR logic is solid and all previously-identified defects have been corrected.

Every concurrency edge case in the scheduler (terminal-state races, shutdown sequencing, lock-free timer re-arm, and exclusive-operation chaining) traces correctly. The one remaining comment is a cosmetic listener-cleanup suggestion in waitForProcessSpawn with no runtime impact. No regressions were introduced in the EPG or playlist-delete integration paths.

No files require special attention beyond the sidebar/recording-library overlap with PR #1185 noted in the PR description.

Important Files Changed

Filename Overview
apps/electron-backend/src/app/services/recording-scheduler.service.ts New DVR scheduler service — all previously-flagged terminal-state races in cancelForPlaylist and cancelAllActive fixed with recordingBecameTerminal guard
apps/electron-backend/src/app/services/recording-scheduler-shutdown.ts settleWithin previously re-threw rejections; now correctly resolves undefined on both timeout and operation failure, giving consistent best-effort semantics
apps/electron-backend/src/app/services/recording-scheduler.runtime.ts Core scheduling runtime — timer chaining, exclusive-operation queue, and engine-failure retry all look correct; long-delay timer re-arms are logically sound
apps/electron-backend/src/app/services/vlc-recording-engine.ts VLC recording engine — intentional session-retention-on-failed-SIGKILL is now documented with a code comment; stop/cleanup paths are correct
apps/electron-backend/src/app/services/vlc-process-control.ts stopVlcProcess promise handling is mostly correct; waitForProcessSpawn does not clean up the error listener when spawn fires, leaving a transient listener attached
apps/electron-backend/src/app/services/recording-scheduler.utils.ts Previously-flagged synchronous existsSync replaced with async access(); toPublicRecordingItem is now async and all callers use Promise.all
apps/electron-backend/src/app/services/recording-engine.ts DesktopRecordingEngine correctly delegates to MPV or VLC, manages power-save blocker, and cleans up activeEngines on stop failure when session no longer exists
apps/electron-backend/src/app/events/database/playlist.events.ts Delete-playlist path correctly calls cancelForPlaylist before the DB delete; restorePlaylistScheduling on failure and resumeSchedulingAfterDeleteAll in finally are both present
apps/electron-backend/src/app/services/recording-scheduling-gate.ts blockAllAndRun leaves blockAllScheduling=true after action; caller in playlist.events.ts always calls resumeSchedulingAfterDeleteAll in finally, so flag is always reset
libs/recording/feature/src/lib/recording-library/recording-library.component.ts Per-record pending locks, filter/sort computed signals, and action dispatch are all correct
libs/shared/database/src/lib/recording-table-migration.ts Table-rebuild migration for pre-release non-null stream_url is idempotent (PRAGMA check gates the rebuild) and wrapped in a transaction

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant R as Renderer
    participant P as Preload (IPC)
    participant S as RecordingSchedulerService
    participant G as RecordingSchedulingGate
    participant RT as RecordingSchedulerRuntime
    participant E as DesktopRecordingEngine
    participant DB as DatabaseWorker

    R->>P: recordingsSchedule(request)
    P->>S: schedule(request)
    S->>G: runForPlaylist(playlistId, ...)
    G->>RT: runExclusive(scheduleKey, ...)
    RT->>DB: create(id, request)
    RT->>RT: armRecording(recording)
    Note over RT: Sets start/end timers
    RT-->>S: recording created
    S-->>R: success + recording

    Note over RT: Start timer fires
    RT->>RT: runExclusive(recordingId, startRecording)
    RT->>DB: update status to recording
    RT->>E: start(recording)
    E->>E: selectAvailableEngine MPV or VLC
    E-->>RT: fileName, filePath, bytesRecorded
    RT->>DB: update engineResult
    Note over RT: Sets end timer

    Note over RT: End timer fires
    RT->>RT: runExclusive(recordingId, finishRecording)
    RT->>E: stop(recordingId)
    E-->>RT: fileName, filePath, bytesRecorded
    RT->>DB: update status to completed
    RT->>P: broadcastRecordingUpdate
    P->>R: onRecordingsUpdate event
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant R as Renderer
    participant P as Preload (IPC)
    participant S as RecordingSchedulerService
    participant G as RecordingSchedulingGate
    participant RT as RecordingSchedulerRuntime
    participant E as DesktopRecordingEngine
    participant DB as DatabaseWorker

    R->>P: recordingsSchedule(request)
    P->>S: schedule(request)
    S->>G: runForPlaylist(playlistId, ...)
    G->>RT: runExclusive(scheduleKey, ...)
    RT->>DB: create(id, request)
    RT->>RT: armRecording(recording)
    Note over RT: Sets start/end timers
    RT-->>S: recording created
    S-->>R: success + recording

    Note over RT: Start timer fires
    RT->>RT: runExclusive(recordingId, startRecording)
    RT->>DB: update status to recording
    RT->>E: start(recording)
    E->>E: selectAvailableEngine MPV or VLC
    E-->>RT: fileName, filePath, bytesRecorded
    RT->>DB: update engineResult
    Note over RT: Sets end timer

    Note over RT: End timer fires
    RT->>RT: runExclusive(recordingId, finishRecording)
    RT->>E: stop(recordingId)
    E-->>RT: fileName, filePath, bytesRecorded
    RT->>DB: update status to completed
    RT->>P: broadcastRecordingUpdate
    P->>R: onRecordingsUpdate event
Loading

Reviews (9): Last reviewed commit: "merge: sync latest upstream into DVR" | Re-trigger Greptile

Comment thread apps/electron-backend/src/app/services/recording-scheduler.utils.ts Outdated
Comment thread apps/electron-backend/src/app/services/vlc-recording-engine.ts
@SalemOurabi

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review the latest DVR follow-up, especially playlist-cancel races, best-effort shutdown, async file availability checks, VLC lifecycle tracking, and the CI/E2E fixes.

@codecov-commenter

codecov-commenter commented Jul 15, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 71.66860% with 489 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.65%. Comparing base (e91cbd5) to head (a503808).
⚠️ Report is 2020 commits behind head on master.

Files with missing lines Patch % Lines
...ctron-backend/src/app/services/recording-engine.ts 58.53% 20 Missing and 14 partials ⚠️
...n-backend/src/app/services/vlc-recording-engine.ts 70.79% 18 Missing and 15 partials ⚠️
...tron-backend/src/app/events/embedded-mpv.events.ts 23.07% 30 Missing ⚠️
.../src/app/services/embedded-mpv-recording-engine.ts 48.27% 18 Missing and 12 partials ⚠️
...nd/src/app/services/recording-scheduler.service.ts 77.41% 22 Missing and 6 partials ⚠️
...nd/src/app/services/recording-scheduler.runtime.ts 81.69% 14 Missing and 12 partials ⚠️
apps/electron-backend/src/main.ts 7.14% 26 Missing ⚠️
...recording/data-access/src/lib/recording.service.ts 74.22% 12 Missing and 13 partials ⚠️
...kend/src/app/services/recording-scheduler.utils.ts 63.15% 6 Missing and 15 partials ⚠️
...on-backend/src/app/services/vlc-process-control.ts 70.83% 12 Missing and 9 partials ⚠️
... and 41 more
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (e91cbd5) and HEAD (a503808). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (e91cbd5) HEAD (a503808)
4 0
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1190      +/-   ##
==========================================
- Coverage   71.05%   63.65%   -7.41%     
==========================================
  Files          40      704     +664     
  Lines         691    41123   +40432     
  Branches       87     8977    +8890     
==========================================
+ Hits          491    26177   +25686     
- Misses        176    11542   +11366     
- Partials       24     3404    +3380     
Flag Coverage Δ
unit 63.65% <71.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SalemOurabi

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review the latest Linux E2E follow-up. The SQLite migration fixture now receives its mode and paths through explicit test environment variables, so Chromium switches cannot shift positional arguments.

@SalemOurabi

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review the latest bulk-cancellation race fix. cancelAllActive() now shares the terminal-state recheck with cancelForPlaylist(), with regression coverage for a recording that completes between the active snapshot and sequential cancellation.

@SalemOurabi
SalemOurabi marked this pull request as ready for review July 15, 2026 06:08

Copy link
Copy Markdown
Contributor Author

@greptile-apps Please re-review the latest conflict-resolution merge. It preserves upstream's embedded-MPV frame-copy and shared player-control changes while keeping hidden main-process DVR sessions private and alive across renderer reloads/crashes. Regression coverage was added for that lifecycle boundary, and the affected tests, lint, typecheck, and Electron build pass.

# Conflicts:
#	CLAUDE.md
#	apps/electron-backend/src/app/database/schema.ts
#	apps/electron-backend/src/app/services/debug-trace.spec.ts
#	apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts
#	apps/electron-backend/src/app/services/embedded-mpv-native.service.ts
#	apps/electron-backend/src/main.ts

Copy link
Copy Markdown
Contributor Author

@greptile please re-review the latest DVR update. The failing CI lint was reproduced locally and fixed in d5f802e by extracting the EPG channel-mapping schema without changing its public exports or table contract. Validation: the exact 43-project CI lint command passes, database:build passes, and all 23 database tests pass.

Copy link
Copy Markdown
Contributor Author

@greptile one final re-review please: PR #1190 is now fully synchronized with the newest upstream commit (#1208) at ea8413d. The post-merge 43-project lint run passes, the new About/version suite passes 8/8, and git diff --check passes.

Adopts the upstream EPG mapping schema split (epg-mapping.schema.ts) and
drops the branch-local duplicate module so the table is defined once.
@SalemOurabi

Copy link
Copy Markdown
Contributor Author

@greptileai Please review this pull request

# Conflicts:
#	libs/portal/stalker/feature/src/lib/stalker-live-stream-layout/stalker-live-stream-layout.component.ts
@SalemOurabi

Copy link
Copy Markdown
Contributor Author

@greptile please re-review after the latest upstream sync and semantic conflict resolution. The DVR behavior and the new upstream MPV/EPG behavior were preserved together; targeted unit tests, formatting, diff checks, and affected-project lint all pass locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants