feat: add casting controls and expandable workspace navigation#1056
feat: add casting controls and expandable workspace navigation#1056SalemOurabi wants to merge 24 commits into
Conversation
Greptile SummaryThis PR adds remote playback controls (AirPlay, Google Cast, browser Remote Playback, Electron DLNA/UPnP) and an expandable workspace navigation rail, while fixing two previously noisy runtime issues: repeated Electron IPC rejection logs from background Xtream EPG requests, and stale E2E navigation selectors after the dashboard brand became a rail-toggle button.
Confidence Score: 5/5All changed paths are correct and well-tested; the error-flow, conflict resolution, and E2E selector updates are all verified. The structured Xtream error path is correctly threaded through all three layers (backend to ElectronService to XtreamApiService), with unit tests at each hop. The audio-player merge retains both the upstream remote-volume contract and the new casting signals without conflict. No stale conflict markers remain. DLNA SSRF protections are consistent with the backend URL-safety pattern. The CSP change is tightly scoped to the official Cast SDK origin. Previously flagged issues (module-level handler registration in casting.events.ts, 0.0.0.0/localhost gaps in isDirectCastUrl, hasPlaybackHeaders duplication) are all resolved in this head. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Renderer as Angular Renderer
participant ES as ElectronService
participant IPC as Electron IPC
participant XE as xtream.events.ts
participant XAS as XtreamApiService
Renderer->>XAS: getLiveStreams(credentials, opts)
XAS->>ES: sendIpcEvent(XTREAM_REQUEST, payload)
ES->>IPC: window.electron.xtreamRequest(payload)
IPC->>XE: ipcMain.handle('XTREAM_REQUEST')
alt Success
XE-->>IPC: "return { payload, action }"
IPC-->>ES: "resolves { payload, action }"
ES-->>XAS: "returns { type: XTREAM_RESPONSE, payload }"
XAS-->>Renderer: returns typed response
else Error
XE-->>IPC: return createXtreamErrorResponse(error)
IPC-->>ES: "resolves { type: ERROR, name?, status, message }"
ES->>ES: 'type' in response - throw response
ES->>ES: catch: normalize name/status
ES-->>XAS: "resolves { type: ERROR, name?, status, message }"
XAS->>XAS: "type===ERROR - throw Error with .name/.status"
XAS-->>Renderer: rejects Error
end
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1056 +/- ##
===========================================
- Coverage 71.05% 54.87% -16.19%
===========================================
Files 40 551 +511
Lines 691 30489 +29798
Branches 87 6519 +6432
===========================================
+ Hits 491 16730 +16239
- Misses 176 11235 +11059
- Partials 24 2524 +2500
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Please re-review the updated head |
|
Please perform the final re-review on head |
|
@greptileai please re-review the latest head |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
@greptileai please perform a final re-review of head |
Introduce a structured ElectronBridgeXtreamErrorResponse and union response type, and update the Xtream IPC flow to return error objects instead of throwing. Added createXtreamErrorResponse to normalize Axios/network errors (including canceled requests -> AbortError with status 499) and fixed syscall/hostname extraction in formatXtreamError. The backend now returns the structured error to avoid rejected Electron handlers; the web renderer checks for a response.type and rethrows to preserve existing error handling. Docs updated to explain why IPC handlers must not be rejected.
|
@greptileai please perform a fresh final review of head Since the last Greptile-reviewed head, this update:
Please specifically check correctness/security of the structured error path, the casting + remote-volume conflict resolution, stale conflict artifacts, and compatibility with the latest dashboard/settings refactor. Local typecheck, unit suites, and targeted Electron E2E suites are green; the updated validation matrix is in the PR description. |
Resolve the two xtream conflicts by unioning upstream's portal- compatibility fix (254879f) with this branch's SSRF + structured-error work, keeping both intents: - xtream.events.ts: keep BOTH createXtreamErrorResponse (structured IPC error responses + AbortError/499 cancellation mapping) AND upstream's buildXtreamApiUrl (normalizeXtreamServerUrl base + username/password trim). The merged XTREAM_REQUEST handler already routes through requestWithValidatedRedirects (per-hop SSRF validation) and uses both helpers. - xtream-api.service.ts: keep the richer thrown error that preserves name + status (needed for cancellation detection) over upstream's bare Error; upstream's normalize/trim sendRequest and multi-action getAccountInfo auto-merged around it. Fixes the "Web E2E on Ubuntu Chromium" failure of "@xtream playlist details edit is retained in the PWA browser context": the PWA save/validate path needs upstream's normalizeXtreamServerUrl, which this branch was missing. Verified locally: chromium passes; electron-backend builds; portal-xtream-data-access (101), shared- interfaces (29) and xtream.events.spec all green.
Move formatXtreamError, createXtreamErrorResponse and buildXtreamApiUrl out of xtream.events.ts into a focused xtream-request.util.ts. Behavior is unchanged; xtream.events.ts now holds only IPC registration and drops from 380 to 273 lines, back under the repo's 300-line guideline (CLAUDE.md TypeScript File Size Rule). Verified: electron-backend build + lint clean, xtream.events.spec green (372 passed; the 13 failures are the pre-existing Windows-only platform suites unrelated to this change).
The casting change to video-player.component.html binds [playback] on the inline player child; the spec's mock needs the matching input() declared or the template binding fails. (Carried over from the 4gray#1056 merge resolution that the topic cherry-pick missed.)
|
Closing in favor of three focused, independently reviewable PRs split out of this branch — each builds and tests green on top of the current
This branch had grown to ~20 commits spanning four unrelated concerns, which made review and merge hard and created a recurring conflict surface. Each topic is now isolated:
No changes were lost: the union of the three PRs equals this branch's diff against |
|
Too many files changed for review. ( |
- CSP: restrict the Google Cast https://www.gstatic.com script-src to the PWA build (new index.pwa.html wired via the web:build:pwa `index` input/output option); the Electron renderer keeps the tighter `script-src 'self'`. - dlna-protocol: set the absolute request deadline to 10s, longer than the 4s socket-inactivity timeout, so a trickle-data sender is still bounded in wall-clock time instead of both firing at the same instant. - cast-media: scope <video>/<audio> discovery via an explicit [data-cast-scope] attribute on the player hosts (web-player-view host + radio-hero) instead of hardcoded CSS class names, so a layout/class refactor cannot silently break Cast. - workspace-shell: convert toCastPlayback() to a castPlayback computed so the OnPush cast-control receives a stable object reference instead of a freshly allocated one on every change-detection pass. Verified: web pwa build emits index.html with gstatic; production build emits index.html with script-src 'self'; ui-playback 137/137, workspace-shell-feature 102/102, dlna-renderer.service.spec green; electron-backend build clean.
The VOD/series details view embeds the trailer via <iframe src="https://www.youtube.com/embed/{id}">. The hardened CSP's `frame-src 'none'` blocked it (ERR_BLOCKED_BY_CSP), so trailers failed to load in both the Electron renderer and the PWA. Allow the YouTube frame origins (youtube.com + youtube-nocookie.com) in frame-src for the base and PWA index; no other directive is loosened.
…lscreen The cast overlay re-show was wired to host pointer events on .web-player-view. That fails when a player captures pointer events (ArtPlayer — the control hides after a few seconds and never returns) or owns the fullscreen element (Video.js — it hides in fullscreen and doesn't reappear until exiting). Drive the re-show from document-level capture-phase pointer/key activity instead, gated to the player surface (host bounding box) or any active fullscreen, plus re-show on fullscreenchange. Listeners run outside Angular (rAF-throttled) so idle movement doesn't churn change detection; only a gated re-show re-enters the zone. Verified: ui-playback 137/137 (web-player-view spec updated for the new document-level activity path).
…ullscreen Each player fullscreened its own element (Video.js .video-js, ArtPlayer, or the native <video>), leaving the cast-control overlay — a sibling of the player in .web-player-view — outside the fullscreen context. The HTML5 player showed no cast icon in fullscreen at all (a native <video> cannot host overlay siblings), and the others only showed it intermittently. Make .web-player-view (which holds both the active player and the cast overlay) the single fullscreen target: - Add a fullscreen toggle to the cast overlay that calls requestFullscreen/ exitFullscreen on the host; isFullscreen tracks document.fullscreenElement via the existing fullscreenchange listener and drives the icon and label. - Disable each player's own fullscreen so this is the only entry point: Video.js controlBar.fullscreenToggle=false, HTML5 controlsList="nofullscreen", ArtPlayer fullscreen/fullscreenWeb=false. The cast control now shares one fullscreen context across Video.js, HTML5, and ArtPlayer. Esc still exits fullscreen natively. Verified: ui-playback 138/138 (added a web-player-view fullscreen test), web production build clean. Runtime fullscreen behavior needs a manual rebuild — it cannot be exercised from jsdom unit tests.
Summary
Security and behavior
AbortError, status499) through Electron IPC, renderer normalization, and the Xtream API serviceLatest review fixes
nameandstatuson structured Xtream errors so cancelled imports remain classified as cancelled rather than failedAudioPlayerComponent; both theplaybackcast metadata input and externalvolume/volumeChangecontract are retainedValidation
pnpm run typecheck:cipnpm nx test web --runInBand --runTestsByPath apps/web/src/app/services/electron.service.spec.ts apps/web/src/app/settings/settings.component.spec.ts(2 suites, 40 tests)pnpm nx test portal-xtream-data-access --runInBand(12 suites, 93 tests)pnpm nx test ui-playback --runInBand --runTestsByPath libs/ui/playback/src/lib/audio-player/audio-player.component.spec.ts(17 suites, 146 tests)pnpm nx test playlist-m3u-feature-player --runInBand --runTestsByPath libs/playlist/m3u/feature-player/src/lib/video-player/video-player.component.spec.ts(4 suites, 47 tests)pnpm nx run electron-backend-e2e:e2e-ci--src/settings.e2e.ts(6 passed)pnpm nx run electron-backend-e2e:e2e-ci--src/dashboard-activation.e2e.ts(1 passed)git diff --check, conflict-marker scan, andgit merge-tree --write-tree HEAD upstream/masterpassUpstream and conflicts
masteris synchronized with4gray/iptvnator:masterat1444047a0commits behindNotes
package.jsonorpnpm-lock.yamlREADME.md,docs/architecture/remote-playback.md,docs/architecture/electron-security.md, anddocs/architecture/workspace-shell.mdIPTVNATOR_WIKI_VAULTis not configured